From 0337fba234bf574629d56be631468ea060495fa0 Mon Sep 17 00:00:00 2001 From: Simon Willison Date: Mon, 10 Aug 2026 15:03:29 -0700 Subject: [PATCH 01/63] disable_fts() before dropping table Closes #2874 --- datasette/views/table.py | 4 +++- tests/test_api_write.py | 37 +++++++++++++++++++++++++++++++++++++ 2 files changed, 40 insertions(+), 1 deletion(-) diff --git a/datasette/views/table.py b/datasette/views/table.py index 3eb80854..7c814b27 100644 --- a/datasette/views/table.py +++ b/datasette/views/table.py @@ -1391,7 +1391,9 @@ class TableDropView(BaseView): # Drop table def drop_table(conn): - sqlite_utils.Database(conn)[table_name].drop() + table = sqlite_utils.Database(conn)[table_name] + table.disable_fts() + table.drop() await db.execute_write_fn(drop_table, request=request) await self.ds.track_event( diff --git a/tests/test_api_write.py b/tests/test_api_write.py index 7801d0a3..11ef30de 100644 --- a/tests/test_api_write.py +++ b/tests/test_api_write.py @@ -1,6 +1,7 @@ import time import pytest +import sqlite_utils from datasette.app import Datasette from datasette.events import RenameTableEvent @@ -1725,6 +1726,42 @@ async def test_drop_table(ds_write, scenario): assert (await ds_write.client.get("/data/docs")).status_code == 404 +@pytest.mark.asyncio +async def test_drop_table_cleans_up_fts(ds_write): + db = ds_write.get_database("data") + + def enable_fts(conn): + sqlite_utils.Database(conn)["docs"].enable_fts(["title"], create_triggers=True) + + await db.execute_write_fn(enable_fts) + assert { + row[0] + for row in await db.execute( + "select name from sqlite_master where type = 'table' and name like 'docs_fts%'" + ) + } == { + "docs_fts", + "docs_fts_config", + "docs_fts_data", + "docs_fts_docsize", + "docs_fts_idx", + } + + response = await ds_write.client.post( + "/data/docs/-/drop", + json={"confirm": True}, + headers=_headers(write_token(ds_write)), + ) + + assert response.json() == {"ok": True} + assert [ + row[0] + for row in await db.execute( + "select name from sqlite_master where type = 'table' and name like 'docs_fts%'" + ) + ] == [] + + @pytest.mark.asyncio @pytest.mark.parametrize( "input,expected_status,expected_response,expected_events", From e78b8a2e6ac69310c06fdacc6ca0a6ab309ffe0b Mon Sep 17 00:00:00 2001 From: Alex Garcia Date: Tue, 1 Sep 2026 09:32:37 -0700 Subject: [PATCH 02/63] Run datasette serve startup and uvicorn on a single event loop (#2886) * Run datasette serve startup and uvicorn on a single event loop * Move the serve-subprocess test plumbing into a conftest fixture * Fix datasette-litestream URL and trim marker-task test comments * Explain why serve_with_plugins needs a subprocess and plugin files * Apply ruff 0.16 and black fixes * Tweaked some comments --- datasette/cli.py | 91 ++++++++++++++----------- pyproject.toml | 2 +- tests/conftest.py | 84 ++++++++++++++++++++++- tests/test_cli_serve_server.py | 117 +++++++++++++++++++++++++++++++++ 4 files changed, 253 insertions(+), 41 deletions(-) diff --git a/datasette/cli.py b/datasette/cli.py index 57db83b6..12024a14 100644 --- a/datasette/cli.py +++ b/datasette/cli.py @@ -663,16 +663,6 @@ def serve( # Private utility mechanism for writing unit tests return ds - # Run async soundness checks before startup hooks, since invoke_startup - # now populates internal tables which requires querying each database - run_sync(lambda: check_databases(ds)) - - # Run the "startup" plugin hooks - try: - run_sync(ds.invoke_startup) - except StartupError as e: - raise click.ClickException(e.args[0]) - if headers and not get: raise click.ClickException("--headers can only be used with --get") @@ -680,6 +670,14 @@ def serve( raise click.ClickException("--token can only be used with --get") if get: + # --get means we don't run Uvicorn at all + run_sync(lambda: check_databases(ds)) + + try: + run_sync(ds.invoke_startup) + except StartupError as e: + raise click.ClickException(e.args[0]) + client = TestClient(ds) request_headers = {} if token: @@ -704,34 +702,51 @@ def serve( sys.exit(exit_code) return - # Start the server - url = None - if root: - ds.root_enabled = True - url = "http://{}:{}{}?token={}".format( - host, port, ds.urls.path("-/auth-token"), ds._root_token - ) - click.echo(url) - if open_browser: - if url is None: - # Figure out most convenient URL - to table, database or homepage - path = run_sync(lambda: initial_path_for_datasette(ds)) - url = f"http://{host}:{port}{path}" - webbrowser.open(url) - uvicorn_kwargs = { - "host": host, - "port": port, - "log_level": "info", - "lifespan": "on", - "workers": 1, - } - if uds: - uvicorn_kwargs["uds"] = uds - if ssl_keyfile: - uvicorn_kwargs["ssl_keyfile"] = ssl_keyfile - if ssl_certfile: - uvicorn_kwargs["ssl_certfile"] = ssl_certfile - uvicorn.run(ds.app(), **uvicorn_kwargs) + # check_databases, invoke_startup() and the uvicorn server all run on a + # single event loop, so that anything a plugin's "startup" hook schedules + # on the loop (asyncio.create_task, Lock/Queue/Event objects, ...) is + # still alive when the server starts handling requests. + async def _serve_async(): + # Populate internal catalog tables before invoke_startup + await check_databases(ds) + + # Run the "startup" plugin hooks + try: + await ds.invoke_startup() + except StartupError as e: + raise click.ClickException(e.args[0]) + + # Start the server + url = None + if root: + ds.root_enabled = True + url = "http://{}:{}{}?token={}".format( + host, port, ds.urls.path("-/auth-token"), ds._root_token + ) + click.echo(url) + if open_browser: + if url is None: + # Figure out most convenient URL - to table, database or homepage + path = await initial_path_for_datasette(ds) + url = f"http://{host}:{port}{path}" + webbrowser.open(url) + uvicorn_kwargs = { + "host": host, + "port": port, + "log_level": "info", + "lifespan": "on", + "workers": 1, + } + if uds: + uvicorn_kwargs["uds"] = uds + if ssl_keyfile: + uvicorn_kwargs["ssl_keyfile"] = ssl_keyfile + if ssl_certfile: + uvicorn_kwargs["ssl_certfile"] = ssl_certfile + server = uvicorn.Server(uvicorn.Config(ds.app(), **uvicorn_kwargs)) + await server.serve() + + asyncio.run(_serve_async()) @cli.command() diff --git a/pyproject.toml b/pyproject.toml index cf5db905..e658955f 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -30,7 +30,7 @@ dependencies = [ "hupper>=1.9", "httpx>=0.20,<1.0", "pluggy>=1.0", - "uvicorn>=0.11", + "uvicorn>=0.29", "aiofiles>=0.4", "PyYAML>=5.3", "mergedeep>=1.1.1", diff --git a/tests/conftest.py b/tests/conftest.py index a2e6aba2..12dce417 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -2,6 +2,7 @@ import importlib.metadata import os import pathlib import re +import socket import subprocess import sys import tempfile @@ -32,17 +33,31 @@ UNDOCUMENTED_PERMISSIONS = { } -def wait_until_responds(url, timeout=5.0, client=httpx, **kwargs): +def wait_until_responds(url, timeout=5.0, client=httpx, process=None, **kwargs): start = time.time() while time.time() - start < timeout: + # If the server died there is no point waiting out the timeout - fail + # now, with its output, instead of after `timeout` seconds of silence + if process is not None and process.poll() is not None: + raise AssertionError( + "Server exited early with returncode {}\n{}".format( + process.returncode, process.stdout.read().decode("utf-8") + ) + ) try: client.get(url, **kwargs) return - except httpx.ConnectError: + except httpx.TransportError: time.sleep(0.1) raise AssertionError(f"Timed out waiting for {url} to respond") +def find_free_port(): + with socket.socket() as sock: + sock.bind(("127.0.0.1", 0)) + return sock.getsockname()[1] + + @pytest.fixture def bare_ds(): """ @@ -301,6 +316,71 @@ def ds_unix_domain_socket_server(tmp_path_factory): pass +@pytest.fixture +def serve_with_plugins(tmp_path): + """Factory fixture for starting ``datasette serve`` in a subprocess with + plugins written to a temporary ``--plugins-dir``. + + For tests that need the real serve path: event-loop wiring, exit codes, + signals. The usual in-process ``pm.register`` plugin pattern can't reach + a subprocess, so plugin source is written out as importable files instead. + + Unlike ``ds_localhost_http_server`` this is function-scoped and takes a + fresh port each time, because each test needs its own plugins. Call it as:: + + proc, port = serve_with_plugins({"my_plugin": PLUGIN_SOURCE}) + + ``plugins`` maps module name to Python source. Pass + ``wait_for_startup=False`` when the server is expected to fail during + startup rather than begin serving. Extra CLI arguments are passed through. + Every process started is terminated when the test ends. + """ + processes = [] + + def start(plugins, *extra_args, wait_for_startup=True): + plugins_dir = tmp_path / "plugins" + plugins_dir.mkdir(exist_ok=True) + for module_name, source in plugins.items(): + (plugins_dir / f"{module_name}.py").write_text(source, "utf-8") + port = find_free_port() + proc = subprocess.Popen( + [ + sys.executable, + "-m", + "datasette", + "--memory", + "--plugins-dir", + str(plugins_dir), + "-h", + "127.0.0.1", + "-p", + str(port), + *extra_args, + ], + stdout=subprocess.PIPE, + stderr=subprocess.STDOUT, + # Avoid FileNotFoundError: [Errno 2] No such file or directory: + cwd=tempfile.gettempdir(), + ) + processes.append(proc) + if wait_for_startup: + wait_until_responds( + f"http://127.0.0.1:{port}/-/versions.json", process=proc + ) + return proc, port + + yield start + + for proc in processes: + if proc.poll() is None: + proc.terminate() + try: + proc.wait(timeout=5) + except subprocess.TimeoutExpired: + proc.kill() + proc.wait() + + # Import fixtures from fixtures.py to make them available from .fixtures import ( # noqa: F401 TEMP_PLUGIN_SECRET_FILE, diff --git a/tests/test_cli_serve_server.py b/tests/test_cli_serve_server.py index b7604bb8..b76180fd 100644 --- a/tests/test_cli_serve_server.py +++ b/tests/test_cli_serve_server.py @@ -1,4 +1,5 @@ import socket +import time import httpx import pytest @@ -28,3 +29,119 @@ def test_serve_unix_domain_socket(ds_unix_domain_socket_server): "path": "/_memory", "tables": [], }.items() <= response.json().items() + + +# Shaped after datasette-litestream's startup hook, which schedules a +# background task with asyncio.get_running_loop().create_task(...): +# https://github.com/datasette/datasette-litestream +MARKER_TASK_PLUGIN = """ +import asyncio +from datasette import hookimpl +from datasette.utils.asgi import Response + + +@hookimpl +def startup(datasette): + datasette._startup_calls = getattr(datasette, "_startup_calls", 0) + 1 + + async def _mark(): + # Must await before setting the flag: a task with no internal + # await point could finish on the throwaway loop before it + # closed, masking the regression this test guards against. + await asyncio.sleep(0.2) + datasette._marker_task_ran = True + + asyncio.get_running_loop().create_task(_mark()) + + +@hookimpl +def register_routes(): + async def marker_status(datasette): + return Response.json( + { + "marker_task_ran": getattr(datasette, "_marker_task_ran", False), + "startup_calls": getattr(datasette, "_startup_calls", 0), + } + ) + + return [(r"^/-/marker-task-ran$", marker_status)] +""" + + +STARTUP_ERROR_PLUGIN = """ +from datasette import hookimpl +from datasette.utils import StartupError + + +@hookimpl +def startup(datasette): + raise StartupError("boom from plugin") +""" + + +@pytest.mark.serial +def test_startup_hook_background_task_runs_on_serving_loop(serve_with_plugins): + """ + Litestream-shaped regression test: a startup hook that does + asyncio.get_running_loop().create_task(...) must have that task + actually execute before/while the server is handling requests. This + only holds if invoke_startup() and uvicorn.Server.serve() share one + event loop. This test fails against unmodified main, where + invoke_startup() runs on a throwaway loop that is closed before + uvicorn opens its own loop to serve. + """ + _, port = serve_with_plugins({"marker_task_plugin": MARKER_TASK_PLUGIN}) + # The fixture has already waited for the server to answer requests. The + # marker task deliberately awaits before setting its flag, so poll for a + # moment rather than assuming it landed before the first request arrived. + deadline = time.time() + 3.0 + payload = {} + while time.time() < deadline: + payload = httpx.get( + f"http://127.0.0.1:{port}/-/marker-task-ran", timeout=1.0 + ).json() + if payload["marker_task_ran"]: + break + time.sleep(0.05) + assert payload.get("marker_task_ran"), ( + "The startup hook's asyncio.create_task(...) never ran - " + "invoke_startup() and the server are not sharing an event loop" + ) + # Polling above means this test would also pass if the startup hook were + # re-run on the serving loop by the first-request fallback - which would + # hide exactly the bug being tested. invoke_startup() is idempotent today + # so that cannot happen; assert it explicitly so that if the idempotency + # guard is ever removed this test fails loudly instead of silently + # becoming a no-op. + assert payload["startup_calls"] == 1, ( + "startup hook ran {} times - the marker may have been set by a " + "re-run on the serving loop rather than by the original task".format( + payload["startup_calls"] + ) + ) + + +@pytest.mark.serial +def test_startup_error_fails_fast_before_port_binds(serve_with_plugins): + """ + A "startup" plugin hook that raises StartupError must fail fast: print + the message, exit non-zero, and never accept a connection on the port - + the failure must happen before uvicorn.Server binds the socket. + """ + proc, port = serve_with_plugins( + {"startup_error_plugin": STARTUP_ERROR_PLUGIN}, wait_for_startup=False + ) + stdout, _ = proc.communicate(timeout=15) + output = stdout.decode("utf-8") + assert proc.returncode not in (0, None), output + assert "boom from plugin" in output, output + + # Nothing is listening on the port now the process has exited. This + # confirms the socket was not left bound; on its own it cannot prove the + # failure preceded the bind, since a port nothing ever touched also + # refuses connections. + with ( + pytest.raises(OSError), + socket.create_connection(("127.0.0.1", port), timeout=0.2), + ): + pass From 3e018bb1b571cef87c67ae718a5472f23d6b3c6f Mon Sep 17 00:00:00 2001 From: Alex Garcia Date: Tue, 1 Sep 2026 09:39:25 -0700 Subject: [PATCH 03/63] Run startup via ASGI lifespan instead of waiting for the first request (#2887) * Run startup via ASGI lifespan instead of waiting for the first request * Ensure immutable table counts still precompute when startup ran first --- datasette/app.py | 47 ++++++-- datasette/cli.py | 7 +- datasette/utils/asgi.py | 42 +++++-- tests/test_lifespan.py | 259 ++++++++++++++++++++++++++++++++++++++++ 4 files changed, 337 insertions(+), 18 deletions(-) create mode 100644 tests/test_lifespan.py diff --git a/datasette/app.py b/datasette/app.py index c82ea075..42be7425 100644 --- a/datasette/app.py +++ b/datasette/app.py @@ -453,8 +453,10 @@ class Datasette: self.databases = collections.OrderedDict() self.actions = {} # .invoke_startup() will populate this self._column_types = {} # .invoke_startup() will populate this + self._setup_db_done = False try: self._refresh_schemas_lock = asyncio.Lock() + self._startup_lock = asyncio.Lock() except RuntimeError as rex: # Workaround for intermittent test failure, see: # https://github.com/simonw/datasette/issues/1802 @@ -462,6 +464,7 @@ class Datasette: loop = asyncio.new_event_loop() asyncio.set_event_loop(loop) self._refresh_schemas_lock = asyncio.Lock() + self._startup_lock = asyncio.Lock() else: raise self.crossdb = crossdb @@ -2803,24 +2806,52 @@ class Datasette: raise RowNotFound(db.name, table_name, pk_values) return ResolvedRow(db, table_name, sql, params, pks, pk_values, results.first()) + async def _startup_sequence(self): + """Idempotently run the full startup sequence: table counts for + immutable databases, then invoke_startup(). Safe to call more than + once and safe to call concurrently - callers block until whichever + call got there first has finished. + + This is the single entry point used by both AsgiLifespan (so + real deployments finish startup before accepting requests) and + AsgiRunOnFirstRequest (the fallback for hosts that never send + lifespan events, e.g. DatasetteClient's httpx.ASGITransport), and + `datasette serve` (cli.py) calls it too. The fast path below checks + both `_startup_invoked` and `_setup_db_done` - not just the former - + so that a bare `await ds.invoke_startup()` made by a caller ahead of + `_startup_sequence()` (which only sets `_startup_invoked`) can't + make this method skip the immutable-database table-count precompute. + """ + if self._startup_invoked and self._setup_db_done: + return + async with self._startup_lock: + if self._startup_invoked and self._setup_db_done: + return + if not self._setup_db_done: + # First time server starts up, calculate table counts for + # immutable databases + for database in self.databases.values(): + if not database.is_mutable: + await database.table_counts(limit=60 * 60 * 1000) + self._setup_db_done = True + await self.invoke_startup() + def app(self): """Returns an ASGI app function that serves the whole of Datasette""" routes = self._routes() - async def setup_db(): - # First time server starts up, calculate table counts for immutable databases - for database in self.databases.values(): - if not database.is_mutable: - await database.table_counts(limit=60 * 60 * 1000) - async def _close_on_shutdown(): self.close() asgi = CrossOriginProtectionMiddleware(DatasetteRouter(self, routes), self) if self.setting("trace_debug"): asgi = AsgiTracer(asgi) - asgi = AsgiLifespan(asgi, on_shutdown=[_close_on_shutdown]) - asgi = AsgiRunOnFirstRequest(asgi, on_startup=[setup_db, self.invoke_startup]) + asgi = AsgiLifespan( + asgi, + on_startup=[self._startup_sequence], + on_shutdown=[_close_on_shutdown], + ) + asgi = AsgiRunOnFirstRequest(asgi, on_startup=[self._startup_sequence]) for wrapper in pm.hook.asgi_wrapper(datasette=self): asgi = wrapper(asgi) return asgi diff --git a/datasette/cli.py b/datasette/cli.py index 12024a14..2694c1f6 100644 --- a/datasette/cli.py +++ b/datasette/cli.py @@ -710,9 +710,12 @@ def serve( # Populate internal catalog tables before invoke_startup await check_databases(ds) - # Run the "startup" plugin hooks + # Run the full startup sequence (immutable-database table-count + # precompute + the "startup" plugin hooks) via the same entry point + # AsgiLifespan/AsgiRunOnFirstRequest use, so it's not skipped when + # uvicorn's lifespan.startup fires moments later. try: - await ds.invoke_startup() + await ds._startup_sequence() except StartupError as e: raise click.ClickException(e.args[0]) diff --git a/datasette/utils/asgi.py b/datasette/utils/asgi.py index 812194fd..2614ad02 100644 --- a/datasette/utils/asgi.py +++ b/datasette/utils/asgi.py @@ -1,3 +1,4 @@ +import asyncio import json import re from http.cookies import Morsel, SimpleCookie @@ -300,12 +301,24 @@ class AsgiLifespan: while True: message = await receive() if message["type"] == "lifespan.startup": - for fn in self.on_startup: - await fn() + try: + for fn in self.on_startup: + await fn() + except Exception as e: # noqa: BLE001 + await send( + {"type": "lifespan.startup.failed", "message": str(e)} + ) + return await send({"type": "lifespan.startup.complete"}) elif message["type"] == "lifespan.shutdown": - for fn in self.on_shutdown: - await fn() + try: + for fn in self.on_shutdown: + await fn() + except Exception as e: # noqa: BLE001 + await send( + {"type": "lifespan.shutdown.failed", "message": str(e)} + ) + return await send({"type": "lifespan.shutdown.complete"}) return else: @@ -624,10 +637,23 @@ class AsgiRunOnFirstRequest: self.asgi = asgi self.on_startup = on_startup self._started = False + # Guards against concurrent early requests interleaving with startup: + # without this, several requests could all observe `_started is + # False` and proceed before any of them finish running the hooks. + self._lock = asyncio.Lock() async def __call__(self, scope, receive, send): - if not self._started: - self._started = True - for hook in self.on_startup: - await hook() + # Leave "lifespan" scope events alone - this shim only exists as a + # fallback for hosts that never send them. It wraps AsgiLifespan, so + # if it ran on_startup here too, a startup exception would escape + # before AsgiLifespan's own try/except got a chance to turn it into + # a lifespan.startup.failed message. + if scope["type"] != "lifespan" and not self._started: + async with self._lock: + # Re-check: another request may have finished startup while + # we were waiting for the lock. + if not self._started: + for hook in self.on_startup: + await hook() + self._started = True return await self.asgi(scope, receive, send) diff --git a/tests/test_lifespan.py b/tests/test_lifespan.py new file mode 100644 index 00000000..3655285e --- /dev/null +++ b/tests/test_lifespan.py @@ -0,0 +1,259 @@ +""" +Tests for wiring Datasette startup (setup_db table counts + invoke_startup) +into the ASGI lifespan protocol. + +These exercise Datasette._startup_sequence() via three different callers: +- AsgiLifespan, by hand-driving lifespan.startup messages (no HTTP request) +- AsgiRunOnFirstRequest, the fallback for hosts that never send lifespan + events (this is what DatasetteClient / plain httpx.ASGITransport uses) +- Both at once, to prove startup hooks run at most once +""" + +import asyncio +import contextlib +import sqlite3 + +import httpx +import pytest + +from datasette import hookimpl +from datasette.app import Datasette +from datasette.database import Database +from datasette.plugins import pm + + +async def _drive_lifespan_startup(app): + """Send a single lifespan.startup message into app's ASGI lifespan loop + and return the list of messages sent back - without ever sending + lifespan.shutdown. Mirrors what a real server does: after startup + completes it parks waiting for the next event. We cancel that wait + once we've observed the startup response, rather than closing the + Datasette instance down with a shutdown message. + """ + messages_sent = [] + startup_responded = asyncio.Event() + delivered = False + + async def receive(): + nonlocal delivered + if not delivered: + delivered = True + return {"type": "lifespan.startup"} + # No further messages: block until the task is cancelled below, + # same as a real server parked waiting for lifespan.shutdown. + await asyncio.Event().wait() + + async def send(message): + messages_sent.append(message) + startup_responded.set() + + task = asyncio.create_task(app({"type": "lifespan"}, receive, send)) + try: + await asyncio.wait_for(startup_responded.wait(), timeout=5) + finally: + task.cancel() + with contextlib.suppress(asyncio.CancelledError): + await task + return messages_sent + + +@pytest.mark.asyncio +async def test_lifespan_startup_runs_before_any_request(): + ds = Datasette(memory=True) + assert ds._startup_invoked is False + app = ds.app() + + messages = await _drive_lifespan_startup(app) + + assert {"type": "lifespan.startup.complete"} in messages + assert ds._startup_invoked is True + # Internal catalog tables should be populated too, entirely without an + # HTTP request having been made. + internal_db = ds.get_internal_database() + databases = await internal_db.execute("select * from catalog_databases") + assert len(databases.rows) >= 1 + + +@pytest.mark.asyncio +async def test_lifespan_startup_failure_reports_lifespan_startup_failed(): + class RaisingStartupPlugin: + __name__ = "RaisingStartupPlugin" + + @hookimpl + def startup(self, datasette): + async def inner(): + raise RuntimeError("boom from startup hook") + + return inner + + ds = Datasette(memory=True) + pm.register(RaisingStartupPlugin(), name="raising_startup_plugin") + try: + app = ds.app() + messages = await _drive_lifespan_startup(app) + finally: + pm.unregister(name="raising_startup_plugin") + + assert messages == [ + {"type": "lifespan.startup.failed", "message": "boom from startup hook"} + ] + # The exception happened before invoke_startup() got to the end of its + # body, so startup is not considered to have completed. + assert ds._startup_invoked is False + + +@pytest.mark.asyncio +async def test_startup_runs_exactly_once_across_lifespan_and_first_request(): + call_count = {"n": 0} + + class CountingStartupPlugin: + __name__ = "CountingStartupPlugin" + + @hookimpl + def startup(self, datasette): + async def inner(): + call_count["n"] += 1 + + return inner + + ds = Datasette(memory=True) + pm.register(CountingStartupPlugin(), name="counting_startup_plugin") + try: + # Build the ASGI app once, the way a real deployment does - and + # reuse the SAME app instance for both the lifespan drive and the + # HTTP requests below, since a fresh ds.app() call would reset the + # AsgiRunOnFirstRequest fallback's state. + app = ds.app() + + messages = await _drive_lifespan_startup(app) + assert {"type": "lifespan.startup.complete"} in messages + assert call_count["n"] == 1 + + # A first HTTP request (as if the host never sent lifespan events, + # or lifespan already ran) should not run the hook again. + transport = httpx.ASGITransport(app=app) + async with httpx.AsyncClient( + transport=transport, base_url="http://localhost" + ) as client: + response1 = await client.get("/-/versions.json") + assert response1.status_code == 200 + # ... nor should a second, repeat request. + response2 = await client.get("/-/versions.json") + assert response2.status_code == 200 + finally: + pm.unregister(name="counting_startup_plugin") + + assert call_count["n"] == 1 + + +@pytest.mark.asyncio +async def test_no_lifespan_first_request_still_triggers_startup(): + # Pin today's behavior: a client that never drives ASGI lifespan events + # at all (like httpx.ASGITransport, which DatasetteClient uses) still + # gets startup armed by the AsgiRunOnFirstRequest fallback. + ds = Datasette(memory=True) + assert ds._startup_invoked is False + app = ds.app() + transport = httpx.ASGITransport(app=app) + async with httpx.AsyncClient( + transport=transport, base_url="http://localhost" + ) as client: + response = await client.get("/-/versions.json") + assert response.status_code == 200 + + assert ds._startup_invoked is True + internal_db = ds.get_internal_database() + databases = await internal_db.execute("select * from catalog_databases") + assert len(databases.rows) >= 1 + + +@pytest.mark.asyncio +async def test_datasette_client_first_request_triggers_startup(): + # Same as above, but through the real DatasetteClient (ds.client) that + # plugins and tests actually use, to confirm nothing regressed there. + ds = Datasette(memory=True) + assert ds._startup_invoked is False + response = await ds.client.get("/-/versions.json") + assert response.status_code == 200 + assert ds._startup_invoked is True + + +@pytest.mark.asyncio +async def test_concurrent_first_requests_all_wait_for_slow_startup(): + call_count = {"n": 0} + + class SlowStartupPlugin: + __name__ = "SlowStartupPlugin" + + @hookimpl + def startup(self, datasette): + async def inner(): + call_count["n"] += 1 + await asyncio.sleep(0.2) + + return inner + + ds = Datasette(memory=True) + pm.register(SlowStartupPlugin(), name="slow_startup_plugin") + try: + app = ds.app() + transport = httpx.ASGITransport(app=app) + async with httpx.AsyncClient( + transport=transport, base_url="http://localhost" + ) as client: + responses = await asyncio.gather( + *[client.get("/-/versions.json") for _ in range(10)] + ) + finally: + pm.unregister(name="slow_startup_plugin") + + # Every one of the 10 simultaneous first requests must have blocked + # until startup actually finished, not raced ahead of it. + assert all(response.status_code == 200 for response in responses) + assert call_count["n"] == 1 + assert ds._startup_invoked is True + + +@pytest.mark.asyncio +async def test_setup_db_still_runs_when_invoke_startup_ran_first(tmp_path, monkeypatch): + # Regression test: `datasette serve` (cli.py _serve_async) calls + # ds.invoke_startup() directly, before uvicorn ever sends a + # lifespan.startup event that drives _startup_sequence(). If + # _startup_sequence()'s fast path only checked `_startup_invoked`, it + # would see startup already done and skip the immutable-database + # table-count precompute (setup_db) entirely - a silent regression + # versus main, where AsgiRunOnFirstRequest ran setup_db unconditionally + # on request #1. + db_path = tmp_path / "immutable.db" + conn = sqlite3.connect(str(db_path)) + conn.execute("create table t (id integer primary key)") + conn.commit() + conn.close() + + ds = Datasette([], immutables=[str(db_path)]) + + call_count = {"n": 0} + original_table_counts = Database.table_counts + + async def counting_table_counts(self, *args, **kwargs): + call_count["n"] += 1 + return await original_table_counts(self, *args, **kwargs) + + monkeypatch.setattr(Database, "table_counts", counting_table_counts) + + # Simulate the CLI path: invoke_startup() runs directly and completes + # BEFORE _startup_sequence() ever gets a chance to run setup_db. + await ds.invoke_startup() + assert ds._startup_invoked is True + assert call_count["n"] == 0 + + # The lifespan/first-request path (or the CLI itself, per the fix) + # calling the shared entry point afterwards must still precompute + # table counts for immutable databases. + await ds._startup_sequence() + assert call_count["n"] == 1 + assert ds._setup_db_done is True + + # Idempotency: a second call must not recompute. + await ds._startup_sequence() + assert call_count["n"] == 1 From bdc973174096cae350ddaa733a10ed8b3ffd970b Mon Sep 17 00:00:00 2001 From: Simon Willison Date: Tue, 1 Sep 2026 13:37:15 -0700 Subject: [PATCH 04/63] check-latest: true, add 3.15 to test matrix, to test RCs (#2895) See https://simonwillison.net/2026/Sep/1/python-315-rc-2/ --- .github/workflows/test.yml | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 751eedfd..2a8c0ae4 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -11,16 +11,17 @@ jobs: strategy: fail-fast: false matrix: - python-version: ["3.10", "3.11", "3.12", "3.13", "3.14"] + python-version: ["3.10", "3.11", "3.12", "3.13", "3.14", "3.15"] steps: - uses: actions/checkout@v7 - name: Set up Python ${{ matrix.python-version }} - uses: actions/setup-python@v6 + uses: actions/setup-python@v7 with: python-version: ${{ matrix.python-version }} allow-prereleases: true cache: pip cache-dependency-path: pyproject.toml + check-latest: true - name: Build extension for --load-extension test run: |- (cd tests && gcc ext.c -fPIC -shared -o ext.so) From 7403ae68bb0e1c39f2ff1927953d2775b932b9d3 Mon Sep 17 00:00:00 2001 From: Zain Dana Harper <17142659+HarperZ9@users.noreply.github.com> Date: Sun, 26 Jul 2026 14:22:17 -0700 Subject: [PATCH 05/63] Give each non-blocking write a distinct task id, refs #2860, #2859 execute_write_fn(fn, block=False) is documented to return "a UUID representing the queued task". Two things stopped that being true. _send_to_write_thread() derived the id from uuid.uuid5(NAMESPACE_DNS, "datasette.io"), which is deterministic, so every non-blocking write in every database in every process returned 3f143baa-4e3d-5842-a36f-4fa2f683b72f. A constant cannot identify a particular task. Now uuid4(). Refs #2860. With num_sql_threads=0 there is no write thread, so execute_write_fn took the synchronous branch and `result` was the write function's return value, normally None. The block=False path then unpacked it unconditionally and raised TypeError: cannot unpack non-iterable NoneType object. The non-threaded branch now returns the same (task_id, reply_future) shape, with the future already resolved because the write has finished, so both modes share one code path. Refs #2859. test_execute_write_fn_block_false only asserted isinstance(task_id, uuid.UUID), which a constant satisfies. The new test is parametrized over threaded and non-threaded and asserts two calls return different ids, so either regression fails it. --- datasette/database.py | 11 ++++++++++- tests/test_internals_database.py | 27 +++++++++++++++++++++++++++ 2 files changed, 37 insertions(+), 1 deletion(-) diff --git a/datasette/database.py b/datasette/database.py index e162d34e..90c4e429 100644 --- a/datasette/database.py +++ b/datasette/database.py @@ -354,6 +354,15 @@ class Database: result = fn(self._write_connection) else: result = fn(self._write_connection) + if not block: + # There is no write thread here, so the write has already + # finished. Hand back the same (task_id, reply_future) shape + # _send_to_write_thread() returns, with the future already + # resolved, so the block=False path below is identical in + # both modes. + reply_future = asyncio.get_running_loop().create_future() + reply_future.set_result(result) + result = (uuid.uuid4(), reply_future) else: result = await self._send_to_write_thread( fn, block=block, transaction=transaction @@ -425,7 +434,7 @@ class Database: ) self._write_thread.name = f"_execute_writes for database {self.name}" self._write_thread.start() - task_id = uuid.uuid5(uuid.NAMESPACE_DNS, "datasette.io") + task_id = uuid.uuid4() loop = asyncio.get_running_loop() reply_future = loop.create_future() self._write_queue.put( diff --git a/tests/test_internals_database.py b/tests/test_internals_database.py index b1093b1c..97513123 100644 --- a/tests/test_internals_database.py +++ b/tests/test_internals_database.py @@ -705,6 +705,33 @@ async def test_execute_write_fn_block_false(db): assert isinstance(task_id, uuid.UUID) +@pytest.mark.asyncio +@pytest.mark.parametrize("disable_threads", (False, True)) +async def test_execute_write_fn_block_false_returns_uuid(tmp_path, disable_threads): + # block=False is documented to return "a UUID representing the queued task". + # With num_sql_threads=0 there is no write thread, so the non-threaded branch + # has to satisfy the same contract as the threaded one. + settings = {"num_sql_threads": 0} if disable_threads else {} + ds = Datasette([], memory=True, settings=settings) + await ds.invoke_startup() + db = ds.add_memory_database("test_block_false") + await db.execute_write( + "create table if not exists t (id integer primary key, v text)" + ) + + def write_fn(conn): + conn.execute("insert into t (v) values ('a')") + # Returns None, like most write functions. + + task_id = await db.execute_write_fn(write_fn, block=False) + + assert isinstance(task_id, uuid.UUID) + # Distinct per call, so a caller can tell two queued tasks apart. + second = await db.execute_write_fn(write_fn, block=False) + assert isinstance(second, uuid.UUID) + assert second != task_id + + @pytest.mark.asyncio async def test_execute_write_fn_block_true(db): def write_fn(conn): From bdaa8cc76cc69b4016747cc04f0ec50b418fbb7b Mon Sep 17 00:00:00 2001 From: Simon Willison Date: Thu, 3 Sep 2026 14:45:03 -0700 Subject: [PATCH 06/63] Disable extension loading once --load-extension extensions are loaded Refs GHSA-2mvv-ffvc-q5p6 Co-authored-by: Alex Garcia <15178711+asg017@users.noreply.github.com> --- datasette/app.py | 29 ++++++++++++++++++------- tests/test_load_extensions.py | 41 +++++++++++++++++++++++++++++++++++ 2 files changed, 62 insertions(+), 8 deletions(-) diff --git a/datasette/app.py b/datasette/app.py index 42be7425..b89ab30c 100644 --- a/datasette/app.py +++ b/datasette/app.py @@ -1532,15 +1532,28 @@ class Datasette: conn.row_factory = sqlite3.Row conn.text_factory = lambda x: str(x, "utf-8", "replace") if self.sqlite_extensions and database != INTERNAL_DB_NAME: + # Extension loading is only enabled for as long as it takes to + # load the configured extensions. Leaving it enabled would let + # anyone who can execute SQL call load_extension() themselves. conn.enable_load_extension(True) - for extension in self.sqlite_extensions: - # "extension" is either a string path to the extension - # or a 2-item tuple that specifies which entrypoint to load. - if isinstance(extension, tuple): - path, entrypoint = extension - conn.execute("SELECT load_extension(?, ?)", [path, entrypoint]) - else: - conn.execute("SELECT load_extension(?)", [extension]) + try: + for extension in self.sqlite_extensions: + # "extension" is either a string path to the extension + # or a 2-item tuple that specifies which entrypoint to load. + if isinstance(extension, tuple): + path, entrypoint = extension + if sys.version_info >= (3, 12): + conn.load_extension(path, entrypoint=entrypoint) + else: + # Connection.load_extension() only gained the + # entrypoint argument in Python 3.12 + conn.execute( + "SELECT load_extension(?, ?)", [path, entrypoint] + ) + else: + conn.load_extension(extension) + finally: + conn.enable_load_extension(False) if self.setting("cache_size_kb"): conn.execute(f"PRAGMA cache_size=-{self.setting('cache_size_kb')}") # pylint: disable=no-member diff --git a/tests/test_load_extensions.py b/tests/test_load_extensions.py index 61cdb3e0..a7c2bc24 100644 --- a/tests/test_load_extensions.py +++ b/tests/test_load_extensions.py @@ -1,4 +1,5 @@ from pathlib import Path +from unittest import mock import pytest @@ -20,6 +21,29 @@ def has_compiled_ext(): return False +@pytest.mark.parametrize("load_fails", (False, True)) +def test_load_extension_is_disabled(load_fails): + ds = Datasette(sqlite_extensions=[COMPILED_EXTENSION_PATH]) + connection = mock.Mock() + if load_fails: + connection.load_extension.side_effect = RuntimeError + + if load_fails: + with pytest.raises(RuntimeError): + ds._prepare_connection(connection, "data") + else: + ds._prepare_connection(connection, "data") + + # Extensions are loaded using the Python API, never via SQL + assert connection.load_extension.mock_calls == [ + mock.call(COMPILED_EXTENSION_PATH), + ] + assert connection.enable_load_extension.mock_calls == [ + mock.call(True), + mock.call(False), + ] + + @pytest.mark.asyncio @pytest.mark.skipif(not has_compiled_ext(), reason="Requires compiled ext.c") async def test_load_extension_default_entrypoint(): @@ -64,3 +88,20 @@ async def test_load_extension_multiple_entrypoints(): response = await ds.client.get("/_memory/-/query.json?_shape=arrays&sql=select+c()") assert response.status_code == 200 assert response.json()["rows"][0][0] == "c" + + +@pytest.mark.asyncio +@pytest.mark.skipif(not has_compiled_ext(), reason="Requires compiled ext.c") +async def test_sql_cannot_load_additional_extension(): + ds = Datasette(sqlite_extensions=[COMPILED_EXTENSION_PATH]) + + response = await ds.client.get( + "/_memory/-/query.json", + params={ + "sql": "select load_extension(:path, :entrypoint)", + "path": COMPILED_EXTENSION_PATH, + "entrypoint": "sqlite3_ext_b_init", + }, + ) + assert response.status_code == 400 + assert response.json()["error"] == "not authorized" From c7944fc454c9c7014719cfd6dc3dbb76f4841a9b Mon Sep 17 00:00:00 2001 From: Simon Willison Date: Thu, 3 Sep 2026 14:27:18 -0700 Subject: [PATCH 07/63] Skip deploy if environment variables are missing --- .github/workflows/deploy-latest.yml | 33 ++++++++++++++++++++++++++--- 1 file changed, 30 insertions(+), 3 deletions(-) diff --git a/.github/workflows/deploy-latest.yml b/.github/workflows/deploy-latest.yml index 3fc83438..46f03b01 100644 --- a/.github/workflows/deploy-latest.yml +++ b/.github/workflows/deploy-latest.yml @@ -14,24 +14,46 @@ jobs: deploy: runs-on: ubuntu-latest steps: + - name: Check deployment prerequisites + id: deployment-prerequisites + env: + GCP_SA_KEY: ${{ secrets.GCP_SA_KEY }} + LATEST_DATASETTE_SECRET: ${{ secrets.LATEST_DATASETTE_SECRET }} + run: | + missing=() + for variable in GCP_SA_KEY LATEST_DATASETTE_SECRET; do + if [[ -z "${!variable:-}" ]]; then + missing+=("$variable") + fi + done + if (( ${#missing[@]} )); then + echo "::notice::Skipping deployment because required environment variables are missing: ${missing[*]}" + echo "available=false" >> "$GITHUB_OUTPUT" + else + echo "available=true" >> "$GITHUB_OUTPUT" + fi - name: Check out datasette + if: ${{ steps.deployment-prerequisites.outputs.available == 'true' }} uses: actions/checkout@v7 - name: Set up Python + if: ${{ steps.deployment-prerequisites.outputs.available == 'true' }} uses: actions/setup-python@v6 with: python-version: "3.13" cache: pip - name: Install Python dependencies + if: ${{ steps.deployment-prerequisites.outputs.available == 'true' }} run: | python -m pip install --upgrade pip python -m pip install . --group dev python -m pip install sphinx-to-sqlite==0.1a1 - name: Run tests - if: ${{ github.ref == 'refs/heads/main' }} + if: ${{ steps.deployment-prerequisites.outputs.available == 'true' && github.ref == 'refs/heads/main' }} run: | pytest -n auto -m "not serial" pytest -m "serial" - name: Build fixtures.db and other files needed to deploy the demo + if: ${{ steps.deployment-prerequisites.outputs.available == 'true' }} run: |- python tests/fixtures.py \ fixtures.db \ @@ -40,13 +62,14 @@ jobs: plugins \ --extra-db-filename extra_database.db - name: Build docs.db - if: ${{ github.ref == 'refs/heads/main' }} + if: ${{ steps.deployment-prerequisites.outputs.available == 'true' && github.ref == 'refs/heads/main' }} run: |- cd docs DISABLE_SPHINX_INLINE_TABS=1 sphinx-build -b xml . _build sphinx-to-sqlite ../docs.db _build cd .. - name: Set up the alternate-route demo + if: ${{ steps.deployment-prerequisites.outputs.available == 'true' }} run: | echo ' from datasette import hookimpl @@ -58,6 +81,7 @@ jobs: ' > plugins/alternative_route.py cp fixtures.db fixtures2.db - name: And the counters writable stored query demo + if: ${{ steps.deployment-prerequisites.outputs.available == 'true' }} run: | cat > plugins/counters.py < Date: Thu, 3 Sep 2026 14:35:35 -0700 Subject: [PATCH 08/63] execute-write: Check view-table for every table in a CREATE VIEW Refs GHSA-53fc-rhfg-h7qp Co-authored-by: Alex Garcia <15178711+asg017@users.noreply.github.com> --- datasette/utils/sql_analysis.py | 62 +++++++++++++++++++++++++++++- tests/test_queries.py | 68 --------------------------------- 2 files changed, 60 insertions(+), 70 deletions(-) diff --git a/datasette/utils/sql_analysis.py b/datasette/utils/sql_analysis.py index 334545bd..22bb55c4 100644 --- a/datasette/utils/sql_analysis.py +++ b/datasette/utils/sql_analysis.py @@ -1,6 +1,7 @@ from dataclasses import dataclass from typing import Literal +from datasette.utils import escape_sqlite from datasette.utils.sqlite import SQLiteTableType, sqlite3, sqlite_table_type SQLOperation = Literal[ @@ -208,7 +209,9 @@ def analyze_sql_tables( This function is synchronous and connection-based. It temporarily installs a SQLite authorizer, prepares ``EXPLAIN ``, and returns the operation - callbacks observed while SQLite compiles the statement. + callbacks observed while SQLite compiles the statement. ``CREATE VIEW`` is + additionally executed inside a rolled-back savepoint so its source-table reads + can be discovered by analyzing a query against the temporary view. """ operations: dict[OperationKey, set[str]] = {} @@ -532,7 +535,7 @@ def analyze_sql_tables( return None return table_kind_cache[(key.sqlite_schema, key.table)] - return SQLAnalysis( + analysis = SQLAnalysis( operations=tuple( Operation( operation=key.operation, @@ -549,3 +552,58 @@ def analyze_sql_tables( for key, columns in operations.items() ) ) + + # SQLite does not resolve the SELECT body of a view when preparing CREATE + # VIEW, so its authorizer does not report reads from the view's source + # tables. Temporarily create the view, analyze a query against it (which + # does resolve the body), then roll the schema change back. Database-level + # callers use an isolated writable connection for this analysis. + create_view_operations = tuple( + operation + for operation in analysis.operations + if operation.operation == "create" and operation.target_type == "view" + ) + if not create_view_operations: + return analysis + + savepoint = "datasette_analyze_create_view" + conn.execute(f"SAVEPOINT {savepoint}") + try: + conn.execute(sql, params if params is not None else {}) + dependency_reads = [] + for view_operation in create_view_operations: + if view_operation.sqlite_schema is None or view_operation.table is None: + raise sqlite3.OperationalError( + "Could not determine the created view name" + ) + quoted_schema = escape_sqlite(view_operation.sqlite_schema) + quoted_view = escape_sqlite(view_operation.table) + qualified_view = f"{quoted_schema}.{quoted_view}" + view_analysis = analyze_sql_tables( + conn, + f"SELECT * FROM {qualified_view}", + database_name=database_name, + schema_to_database=schema_to_database, + ) + dependency_reads.extend( + operation + for operation in view_analysis.operations + if operation.operation == "read" + and not ( + operation.sqlite_schema == view_operation.sqlite_schema + and operation.table == view_operation.table + ) + ) + finally: + conn.execute(f"ROLLBACK TO {savepoint}") + conn.execute(f"RELEASE {savepoint}") + + existing_operations = set(analysis.operations) + return SQLAnalysis( + operations=analysis.operations + + tuple( + operation + for operation in dependency_reads + if operation not in existing_operations + ) + ) diff --git a/tests/test_queries.py b/tests/test_queries.py index 15b7ad0f..ebe8b832 100644 --- a/tests/test_queries.py +++ b/tests/test_queries.py @@ -3248,74 +3248,6 @@ async def test_execute_write_create_table_uses_create_table_permission(): assert not await db.table_exists("should_not_exist") -@pytest.mark.asyncio -async def test_execute_write_create_view_uses_create_view_permission(): - ds = Datasette( - memory=True, - default_deny=True, - config={ - "permissions": { - "insert-row": {"id": "row-writer"}, - "update-row": {"id": "row-writer"}, - }, - "databases": { - "data": { - "permissions": { - "view-database": {"id": ["creator", "row-writer"]}, - "execute-write-sql": {"id": ["creator", "row-writer"]}, - "create-view": {"id": "creator"}, - } - } - }, - }, - ) - db = ds.add_memory_database("execute_write_create_view", name="data") - await db.execute_write("create table dogs (id integer primary key, name text)") - await ds.invoke_startup() - - analysis_response = await ds.client.get( - "/data/-/execute-write/analyze", - actor={"id": "creator"}, - params={"sql": "create view dog_names as select id, name from dogs"}, - ) - allowed_response = await ds.client.post( - "/data/-/execute-write", - actor={"id": "creator"}, - json={"sql": "create view dog_names as select id, name from dogs"}, - ) - row_permission_response = await ds.client.post( - "/data/-/execute-write", - actor={"id": "row-writer"}, - json={"sql": "create view should_not_exist as select id from dogs"}, - ) - - assert analysis_response.status_code == 200 - analysis_data = analysis_response.json() - assert analysis_data["ok"] is True - assert analysis_data["execute_disabled"] is False - assert analysis_data["analysis_rows"] == [ - { - "operation": "create", - "database": "data", - "table": "dog_names", - "required_permission": "create-view", - "source": None, - "allowed": True, - } - ] - - assert allowed_response.status_code == 200 - assert allowed_response.json()["ok"] is True - assert allowed_response.json()["message"] == "Query executed" - assert await db.view_exists("dog_names") - - assert row_permission_response.status_code == 403 - assert row_permission_response.json()["errors"] == [ - "Permission denied: need create-view on data" - ] - assert not await db.view_exists("should_not_exist") - - @pytest.mark.parametrize( ( "database_name", From c280c47424e87019376f534fbd349fd1a55d53a3 Mon Sep 17 00:00:00 2001 From: Simon Willison Date: Thu, 3 Sep 2026 14:35:40 -0700 Subject: [PATCH 09/63] POST /db/-/create checks table-level insert/update/alter permissions Refs GHSA-53fc-rhfg-h7qp Co-authored-by: Alex Garcia <15178711+asg017@users.noreply.github.com> --- datasette/views/table_create_alter.py | 12 +-- tests/test_api_write.py | 116 ++++++++++++++++++++++++++ 2 files changed, 123 insertions(+), 5 deletions(-) diff --git a/datasette/views/table_create_alter.py b/datasette/views/table_create_alter.py index 56b28877..f8f8c31e 100644 --- a/datasette/views/table_create_alter.py +++ b/datasette/views/table_create_alter.py @@ -821,16 +821,18 @@ class TableCreateView(BaseView): ignore = create_request.ignore replace = create_request.replace + table_name = create_request.table + table_exists = await db.table_exists(table_name) + table_resource = TableResource(database=database_name, table=table_name) + # Replacing rows requires update-row permission if replace and not await self.ds.allowed( action="update-row", - resource=DatabaseResource(database=database_name), + resource=table_resource, actor=request.actor, ): return Response.error(["Permission denied: need update-row"], 403) - table_name = create_request.table - table_exists = await db.table_exists(table_name) columns = create_request.columns rows = create_request.rows_list @@ -838,7 +840,7 @@ class TableCreateView(BaseView): # Must have insert-row permission if not await self.ds.allowed( action="insert-row", - resource=DatabaseResource(database=database_name), + resource=table_resource, actor=request.actor, ): return Response.error(["Permission denied: need insert-row"], 403) @@ -857,7 +859,7 @@ class TableCreateView(BaseView): if create_request.alter: if not await self.ds.allowed( action="alter-table", - resource=DatabaseResource(database=database_name), + resource=table_resource, actor=request.actor, ): return Response.error( diff --git a/tests/test_api_write.py b/tests/test_api_write.py index 11ef30de..1c560cf5 100644 --- a/tests/test_api_write.py +++ b/tests/test_api_write.py @@ -2745,3 +2745,119 @@ async def test_create_using_alter_against_existing_table( insert_rows_event = ds_write._tracked_events[1] assert insert_rows_event.name == "insert-rows" assert insert_rows_event.num_rows == 1 + + +@pytest.mark.asyncio +@pytest.mark.parametrize( + ("denied_action", "request_body"), + ( + ( + "insert-row", + { + "table": "salaries", + "rows": [{"id": 9, "note": "INJ-VIA-CREATE"}], + }, + ), + ( + "update-row", + { + "table": "salaries", + "rows": [{"id": 1, "note": "REPLACED"}], + "pk": "id", + "replace": True, + }, + ), + ( + "alter-table", + { + "table": "salaries", + "rows": [{"id": 9, "note": "INSERTED", "extra": "NEW"}], + "alter": True, + }, + ), + ), +) +async def test_create_table_existing_table_respects_table_level_denial( + denied_action, request_body +): + # GHSA-53fc-rhfg-h7qp issue 2: POST /db/-/create against an existing table + # inserts rows into it, so insert-row (and update-row / alter-table) must be + # checked against the TableResource, not just the DatabaseResource. + ds = Datasette( + memory=True, + config={ + "databases": { + # id=editor user has each permission at the database level, but + # the selected action is explicitly denied on the salaries table + "data": { + "permissions": { + "create-table": {"id": "editor"}, + "insert-row": {"id": "editor"}, + "update-row": {"id": "editor"}, + "alter-table": {"id": "editor"}, + }, + "tables": { + "salaries": {"permissions": {denied_action: False}}, + }, + } + } + }, + ) + db = ds.add_memory_database( + f"create_table_existing_table_denied_{denied_action}", name="data" + ) + await db.execute_write("create table salaries (id integer primary key, note text)") + await db.execute_write("insert into salaries values (1, 'TOPSECRET-A')") + await ds.invoke_startup() + + if denied_action == "insert-row": + # Sanity: direct insert into salaries is denied for this actor + direct = await ds.client.post( + "/data/salaries/-/insert", + actor={"id": "editor"}, + json={"row": {"id": 9, "note": "INJ-DIRECT"}}, + ) + assert direct.status_code == 403 + + response = await ds.client.post( + "/data/-/create", + actor={"id": "editor"}, + json=request_body, + ) + assert response.status_code == 403, response.json() + assert response.json()["errors"] == [f"Permission denied: need {denied_action}"] + rows = (await db.execute("select id, note from salaries order by id")).rows + assert [tuple(r) for r in rows] == [(1, "TOPSECRET-A")] + assert await db.table_columns("salaries") == ["id", "note"] + + +@pytest.mark.asyncio +async def test_create_table_respects_predeclared_table_level_denial(): + ds = Datasette( + memory=True, + config={ + "databases": { + "data": { + "permissions": { + "create-table": {"id": "editor"}, + "insert-row": {"id": "editor"}, + }, + "tables": { + "planned_table": {"permissions": {"insert-row": False}}, + }, + } + } + }, + ) + db = ds.add_memory_database("create_table_predeclared_denial", name="data") + await ds.invoke_startup() + + response = await ds.client.post( + "/data/-/create", + actor={"id": "editor"}, + json={"table": "planned_table", "rows": [{"id": 1}]}, + ) + + assert response.status_code == 403, response.json() + assert response.json()["errors"] == ["Permission denied: need insert-row"] + assert not await db.table_exists("planned_table") From 577aeb73f06ec48df630e75af47713bf029fc0c8 Mon Sep 17 00:00:00 2001 From: Simon Willison Date: Thu, 3 Sep 2026 14:35:45 -0700 Subject: [PATCH 10/63] Disallow ?_through= if user lacks view-table permission Refs GHSA-53fc-rhfg-h7qp Co-authored-by: Alex Garcia <15178711+asg017@users.noreply.github.com> --- datasette/filters.py | 7 ++++++- tests/test_table_api.py | 31 +++++++++++++++++++++++++++++++ 2 files changed, 37 insertions(+), 1 deletion(-) diff --git a/datasette/filters.py b/datasette/filters.py index 3cfb36e5..af922eda 100644 --- a/datasette/filters.py +++ b/datasette/filters.py @@ -2,7 +2,7 @@ import json from typing import ClassVar from datasette import hookimpl -from datasette.resources import DatabaseResource +from datasette.resources import DatabaseResource, TableResource from datasette.utils.asgi import BadRequest from datasette.views.base import DatasetteError @@ -135,6 +135,11 @@ def through_filters(request, database, table, datasette): through_table = through_data["table"] other_column = through_data["column"] value = through_data["value"] + await datasette.ensure_permission( + action="view-table", + resource=TableResource(database=database, table=through_table), + actor=request.actor, + ) db = datasette.get_database(database) outgoing_foreign_keys = await db.foreign_keys_for_table(through_table) fk_to_us = next( diff --git a/tests/test_table_api.py b/tests/test_table_api.py index 6c0c021b..ec4a1368 100644 --- a/tests/test_table_api.py +++ b/tests/test_table_api.py @@ -1778,3 +1778,34 @@ async def test_next_url_included_by_default(ds_client): data = response.json() assert data["next"] is None assert data["next_url"] is None + + +@pytest.mark.asyncio +async def test_table_through_requires_view_table_on_through_table(): + # GHSA-53fc-rhfg-h7qp issue 3: ?_through= runs a sub-select against the + # caller-supplied through table, so the actor must be allowed to view it. + # Otherwise it is an equality oracle over any column of a denied table. + from datasette.app import Datasette + + ds = Datasette( + memory=True, + config={"databases": {"data": {"tables": {"salaries": {"allow": False}}}}}, + ) + db = ds.add_memory_database("table_through_denied", name="data") + await db.execute_write("create table people (id integer primary key, name text)") + await db.execute_write( + "create table salaries (id integer primary key, " + "person_id integer references people(id), note text)" + ) + await db.execute_write("insert into people values (1, 'alice'), (2, 'bob')") + await db.execute_write("insert into salaries values (1, 1, 'TOPSECRET-A')") + await ds.invoke_startup() + + # Sanity: anonymous cannot read salaries directly + assert (await ds.client.get("/data/salaries.json")).status_code == 403 + + response = await ds.client.get( + "/data/people.json?_shape=array" + '&_through={"table":"salaries","column":"note","value":"TOPSECRET-A"}' + ) + assert response.status_code == 403, response.text From f8e8e65af7403666f227bb6f0d523bcf2d1e11aa Mon Sep 17 00:00:00 2001 From: Simon Willison Date: Thu, 3 Sep 2026 14:35:48 -0700 Subject: [PATCH 11/63] actor cookie respects expire_after Refs GHSA-53fc-rhfg-h7qp Co-authored-by: Alex Garcia <15178711+asg017@users.noreply.github.com> --- datasette/app.py | 2 +- tests/test_auth.py | 22 ++++++++++++++++++++++ 2 files changed, 23 insertions(+), 1 deletion(-) diff --git a/datasette/app.py b/datasette/app.py index b89ab30c..6683d4dc 100644 --- a/datasette/app.py +++ b/datasette/app.py @@ -2462,7 +2462,7 @@ class Datasette: ): data = {"a": actor} if expire_after: - expires_at = int(time.time()) + (24 * 60 * 60) + expires_at = int(time.time()) + expire_after data["e"] = baseconv.base62.encode(expires_at) response.set_cookie("ds_actor", self.sign(data, "actor")) diff --git a/tests/test_auth.py b/tests/test_auth.py index e7a5402e..6024e3bb 100644 --- a/tests/test_auth.py +++ b/tests/test_auth.py @@ -524,3 +524,25 @@ async def test_root_without_root_enabled_no_special_permissions(ds_client): ) is not True ), "Root without root_enabled should not automatically get set-column-type" + + +@pytest.mark.parametrize("expire_after", (1, 300, 3600, 30 * 24 * 60 * 60)) +def test_set_actor_cookie_honours_expire_after(expire_after): + # GHSA-53fc-rhfg-h7qp issue 4: expire_after is documented as a number of + # seconds, but every value was being replaced with 24 hours. + from datasette.app import Datasette + from datasette.utils.asgi import Response + + ds = Datasette(memory=True) + response = Response.text("") + before = int(time.time()) + ds.set_actor_cookie(response, {"id": "test"}, expire_after=expire_after) + after = int(time.time()) + + (header,) = response._set_cookie_headers + assert header.startswith("ds_actor=") + value = header[len("ds_actor=") :].split(";", 1)[0] + data = ds.unsign(value, "actor") + assert data["a"] == {"id": "test"} + expires_at = baseconv.base62.decode(data["e"]) + assert before + expire_after <= expires_at <= after + expire_after From 435e55ff0a254a77f700a06f5c31bb9f3bf31764 Mon Sep 17 00:00:00 2001 From: Simon Willison Date: Thu, 3 Sep 2026 14:35:53 -0700 Subject: [PATCH 12/63] Remove JSON syntax highlighting Refs GHSA-hp2x-vx2r-6vxg Co-authored-by: Alex Garcia <15178711+asg017@users.noreply.github.com> --- .../static/json-format-highlight-1.0.1.js | 56 ------------------- datasette/templates/api_explorer.html | 5 +- datasette/templates/debug_allowed.html | 5 +- datasette/templates/debug_check.html | 5 +- datasette/templates/debug_rules.html | 5 +- 5 files changed, 8 insertions(+), 68 deletions(-) delete mode 100644 datasette/static/json-format-highlight-1.0.1.js diff --git a/datasette/static/json-format-highlight-1.0.1.js b/datasette/static/json-format-highlight-1.0.1.js deleted file mode 100644 index 0e6e2c29..00000000 --- a/datasette/static/json-format-highlight-1.0.1.js +++ /dev/null @@ -1,56 +0,0 @@ -/* -https://github.com/luyilin/json-format-highlight -From https://unpkg.com/json-format-highlight@1.0.1/dist/json-format-highlight.js -MIT Licensed -*/ -(function (global, factory) { - typeof exports === "object" && typeof module !== "undefined" - ? (module.exports = factory()) - : typeof define === "function" && define.amd - ? define(factory) - : (global.jsonFormatHighlight = factory()); -})(this, function () { - "use strict"; - - var defaultColors = { - keyColor: "dimgray", - numberColor: "lightskyblue", - stringColor: "lightcoral", - trueColor: "lightseagreen", - falseColor: "#f66578", - nullColor: "cornflowerblue", - }; - - function index(json, colorOptions) { - if (colorOptions === void 0) colorOptions = {}; - - if (!json) { - return; - } - if (typeof json !== "string") { - json = JSON.stringify(json, null, 2); - } - var colors = Object.assign({}, defaultColors, colorOptions); - json = json.replace(/&/g, "&").replace(//g, ">"); - return json.replace( - /("(\\u[a-zA-Z0-9]{4}|\\[^u]|[^\\"])*"(\s*:)?|\b(true|false|null)\b|-?\d+(?:\.\d*)?(?:[eE][+]?\d+)?)/g, - function (match) { - var color = colors.numberColor; - if (/^"/.test(match)) { - color = /:$/.test(match) ? colors.keyColor : colors.stringColor; - } else { - color = /true/.test(match) - ? colors.trueColor - : /false/.test(match) - ? colors.falseColor - : /null/.test(match) - ? colors.nullColor - : color; - } - return '' + match + ""; - }, - ); - } - - return index; -}); diff --git a/datasette/templates/api_explorer.html b/datasette/templates/api_explorer.html index 4927cb8d..32686af1 100644 --- a/datasette/templates/api_explorer.html +++ b/datasette/templates/api_explorer.html @@ -3,7 +3,6 @@ {% block title %}API Explorer{% endblock %} {% block extra_head %} - {% endblock %} {% block content %} @@ -126,7 +125,7 @@ getForm.addEventListener("submit", (ev) => { document.getElementById('response-status').textContent = response.status; return response.json(); }).then((data) => { - output.querySelector('pre').innerHTML = jsonFormatHighlight(data); + output.querySelector('pre').textContent = JSON.stringify(data, null, 2); errorList.style.display = 'none'; }).catch((error) => { alert(error); @@ -174,7 +173,7 @@ postForm.addEventListener("submit", (ev) => { } else { errorList.style.display = 'none'; } - output.querySelector('pre').innerHTML = jsonFormatHighlight(data); + output.querySelector('pre').textContent = JSON.stringify(data, null, 2); output.style.display = 'block'; }).catch(err => { alert("Error: " + err); diff --git a/datasette/templates/debug_allowed.html b/datasette/templates/debug_allowed.html index 80249d9c..c73cdfb7 100644 --- a/datasette/templates/debug_allowed.html +++ b/datasette/templates/debug_allowed.html @@ -3,7 +3,6 @@ {% block title %}Allowed Resources{% endblock %} {% block extra_head %} - {% include "_permission_ui_styles.html" %} {% include "_debug_common_functions.html" %} {% endblock %} @@ -198,7 +197,7 @@ function displayResults(data) { } // Update raw JSON - document.getElementById('raw-json').innerHTML = jsonFormatHighlight(data); + document.getElementById('raw-json').textContent = JSON.stringify(data, null, 2); } function displayError(data) { @@ -208,7 +207,7 @@ function displayError(data) { resultsContent.innerHTML = `
Error: ${escapeHtml(data.error || 'Unknown error')}
`; - document.getElementById('raw-json').innerHTML = jsonFormatHighlight(data); + document.getElementById('raw-json').textContent = JSON.stringify(data, null, 2); } // Disable child input if parent is empty diff --git a/datasette/templates/debug_check.html b/datasette/templates/debug_check.html index b9fc636a..c0081c66 100644 --- a/datasette/templates/debug_check.html +++ b/datasette/templates/debug_check.html @@ -3,7 +3,6 @@ {% block title %}Explain a permission decision{% endblock %} {% block extra_head %} - {% include "_permission_ui_styles.html" %} {% include "_debug_common_functions.html" %}