From 993169ae496aa0fa30271b6cb4dfc50202f6e7c1 Mon Sep 17 00:00:00 2001 From: Simon Willison Date: Thu, 11 Jun 2026 08:24:37 -0700 Subject: [PATCH 01/90] Release 1.0a33 Refs #2735, #2677, #2680, #2711, #2756, #2761, #2768, #2754 --- datasette/version.py | 2 +- docs/changelog.rst | 40 ++++++++++++++++++++++++++++++++++------ 2 files changed, 35 insertions(+), 7 deletions(-) diff --git a/datasette/version.py b/datasette/version.py index 1e8c61d5..9536d459 100644 --- a/datasette/version.py +++ b/datasette/version.py @@ -1,2 +1,2 @@ -__version__ = "1.0a32" +__version__ = "1.0a33" __version_info__ = tuple(__version__.split(".")) diff --git a/docs/changelog.rst b/docs/changelog.rst index 19089dd1..48bef0bf 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -9,15 +9,43 @@ Changelog 1.0a33 (unreleased) ------------------- -- Stored queries can now be edited and deleted from the web interface. The stored query page gained a "Query actions" menu with **Edit this query** and **Delete this query** links for actors with the necessary permissions. The owner of a query can always edit or delete it; for queries that are not private, any actor with the :ref:`update-query ` or :ref:`delete-query ` permission can do so too. Private queries remain editable and deletable only by their owner. See :ref:`stored_queries` for details. (:issue:`2735`) -- Row and query JSON pages now support the same ``?_extra=`` mechanism as table pages. Row pages can request extras such as ``foreign_key_tables``, ``query``, ``metadata`` and ``database_color``; arbitrary SQL and stored query pages can request extras such as ``columns``, ``query``, ``metadata`` and ``private``. The implementation was refactored into a registry of extra classes shared by all three page types. See :ref:`json_api_extra` for the full list. -- New generated reference documentation for every ``?_extra=`` parameter available on table, row and query JSON pages, with example output captured from a live Datasette instance at documentation build time. See :ref:`json_api_extra`. -- ``?_extra=`` values can be separated by commas as well as repeated, e.g. ``?_extra=count,next_url``. Previously a comma-separated value that included ``columns`` failed to include the ``columns`` key in the response. -- The ``?_extra=private`` extra on arbitrary SQL query pages now correctly reflects whether the SQL execution permission is private to the current actor - it previously always returned ``false``. -- The ``?_extra=query`` extra on query pages now reports the named parameters that were actually bound when the query executed, including parameters declared in a stored query's ``params`` list. Magic ``_``-prefixed parameters are no longer echoed back with unbound values taken from the querystring. +Stored queries can now be edited and deleted through the web interface, and the JSON API ``?_extra=`` mechanism has been extended to cover row and query pages in addition to tables. This release also fixes two security issues: an identifier-quoting bug involving table and column names that contain ``]``, and an open redirect. + +Editing and deleting stored queries +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +The stored query page gained a "Query actions" menu with **Edit this query** and **Delete this query** links for actors with the necessary permissions. The owner of a query can always edit or delete it; for queries that are not private, any actor with the :ref:`update-query ` or :ref:`delete-query ` permission can do so too. Private queries remain editable and deletable only by their owner. See :ref:`stored_queries` for details. (:issue:`2735`) + +``?_extra=`` support for row and query pages +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +Row and query JSON pages now support the same ``?_extra=`` mechanism as table pages. Row pages can request extras such as ``foreign_key_tables``, ``query``, ``metadata`` and ``database_color``; arbitrary SQL and stored query pages can request extras such as ``columns``, ``query``, ``metadata`` and ``private``. The implementation was refactored into a registry of extra classes shared by all three page types. + +New generated reference documentation describes every ``?_extra=`` parameter available on table, row and query JSON pages, with example output captured from a live Datasette instance at documentation build time. See :ref:`json_api_extra` for the full list. + +You can explore the new extras using this `Datasette extras API explorer tool `__. + +Other improvements and fixes to the extras mechanism: + - Extras that exist to serve the HTML interface (``filters``, ``actions``, ``display_rows``) are no longer advertised or reachable through the JSON API, where requesting them previously returned a 500 serialization error. - The pre-1.0 ``?_extras=`` (plural) parameter on row pages has been removed - use ``?_extra=foreign_key_tables`` instead. +Security fixes +~~~~~~~~~~~~~~ + +- Fixed an identifier-quoting bug in ``datasette.utils.escape_sqlite()``. Datasette uses this helper when constructing SQL around table and column names; identifiers containing ``]`` could break out of SQLite bracket quoting and alter the generated SQL, for example by adding a ``UNION SELECT``. Identifiers containing ``]`` are now quoted using double quotes instead. (:issue:`2677`) +- Fixed an open redirect vulnerability. Requesting a path such as ``/\example.com/`` produced a redirect with a ``Location: /\example.com`` header - browsers normalize backslashes to forward slashes, turning that into the protocol-relative URL ``//example.com`` and redirecting the user off-site. Any run of leading slashes and backslashes in a redirect path is now collapsed to a single slash. (:issue:`2680`) + +Bug fixes +~~~~~~~~~ + +- ``can_render()`` callbacks registered by the :ref:`register_output_renderer() ` plugin hook now receive the result ``rows`` and ``columns`` for stored queries. Previously renderers that inspect the available columns - such as `datasette-atom `__ and `datasette-ics `__ - never appeared as export options on stored query pages. (:issue:`2711`) +- Fixed a 500 error from the :ref:`/-/check ` permission debugging endpoint when checking query actions such as ``view-query``, ``update-query`` and ``delete-query``. (:issue:`2756`) +- Write queries that use a named parameter called ``:sql`` no longer fail with an error. (:issue:`2761`) +- :ref:`db.execute_isolated_fn() ` now works against immutable databases, using a read-only connection that bypasses the write thread. It previously always attempted to open a writable connection, which would fail - breaking features built on top of it, such as the SQL analysis step used when storing a query. An exception raised while opening the connection for an isolated function no longer crashes the write thread. (:issue:`2768`) +- Facet counts are now displayed on the same line as the facet value instead of wrapping onto a second line. (:issue:`2754`) +- Datasette's pytest plugin no longer imports the rest of Datasette at pytest startup time. This means plugin test suites using ``pytest-cov`` now correctly record coverage of code that runs when ``datasette`` modules are first imported. + .. _v1_0_a32: 1.0a32 (2026-05-31) From 1d4212122e5597f2e13625193fb7d45b25928447 Mon Sep 17 00:00:00 2001 From: Simon Willison Date: Thu, 11 Jun 2026 10:36:16 -0700 Subject: [PATCH 02/90] Add release date for 1.0a33 --- docs/changelog.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/changelog.rst b/docs/changelog.rst index 48bef0bf..c0bd7e6b 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -6,7 +6,7 @@ Changelog .. _v1_0_a33: -1.0a33 (unreleased) +1.0a33 (2026-06-11) ------------------- Stored queries can now be edited and deleted through the web interface, and the JSON API ``?_extra=`` mechanism has been extended to cover row and query pages in addition to tables. This release also fixes two security issues: an identifier-quoting bug involving table and column names that contain ``]``, and an open redirect. From fa86ac7b11c44ef80146db6eed25d88c954ee37a Mon Sep 17 00:00:00 2001 From: Simon Willison Date: Thu, 11 Jun 2026 19:41:24 -0700 Subject: [PATCH 03/90] Clearer examples and descriptions for JSON API extras (#2773) Review of the generated ?_extra= documentation found several extras with no example output or with examples that needed explanation: - extras: now shows an abbreviated example of the toggle list and has a clearer description (which also improves the live API output) - set_column_type_ui: example of the shape seen with set-column-type permission, plus a note that it is null otherwise - column_types: live example generated from a table with an assigned column type instead of an empty {} - metadata: live table example now demonstrates a table description and column descriptions; row and query examples gained explanatory notes - expandable_columns, foreign_key_tables, facets_timed_out, next_url, renderers: notes explaining the shape of their output Also added docs_note cross-references to the relevant documentation: facets, pagination, render_cell and register_output_renderer plugin hooks, column type configuration and API, metadata, custom templates, permissions and foreign key label expansion. foreign_key_tables is now flagged as potentially executing additional queries. https://claude.ai/code/session_01EfjBe6E817m9XNFW7EX3Vm Co-authored-by: Claude --- datasette/views/table_extras.py | 182 +++++++++++++++++++++++++-- docs/json_api.rst | 215 ++++++++++++++++++++++++++------ docs/json_api_doc.py | 19 ++- 3 files changed, 367 insertions(+), 49 deletions(-) diff --git a/datasette/views/table_extras.py b/datasette/views/table_extras.py index ce1d7bdf..948f3daa 100644 --- a/datasette/views/table_extras.py +++ b/datasette/views/table_extras.py @@ -184,6 +184,7 @@ class FacetResultsExtra(Extra): ) scopes = {ExtraScope.TABLE} expensive = True + docs_note = "See :ref:`facets` for details of how facets work." async def resolve(self, context, facet_instances): facet_results = {} @@ -215,7 +216,12 @@ class FacetResultsExtra(Extra): class FacetsTimedOutExtra(Extra): description = "Facet calculations that timed out" example = ExtraExample( - "/fixtures/facetable.json?_facet=state&_extra=facets_timed_out" + "/fixtures/facetable.json?_facet=state&_extra=facets_timed_out", + note=( + "A list of the names of any facets that exceeded the " + ":ref:`setting_facet_time_limit_ms` time limit - an empty list " + "if every facet calculation completed." + ), ) scopes = {ExtraScope.TABLE} @@ -236,6 +242,9 @@ class SuggestedFacetsExtra(Extra): ) scopes = {ExtraScope.TABLE} expensive = True + docs_note = ( + "Suggestions are controlled by the :ref:`setting_suggest_facets` setting." + ) async def resolve(self, context, facet_instances): suggested_facets = [] @@ -278,7 +287,13 @@ class HumanDescriptionEnExtra(Extra): class NextUrlExtra(Extra): description = "Full URL for the next page of results" - example = ExtraExample("/fixtures/facetable.json?_size=1&_extra=next_url") + example = ExtraExample( + "/fixtures/facetable.json?_size=1&_extra=next_url", + note=( + "``null`` if there are no more pages of results. " + "See :ref:`json_api_pagination`." + ), + ) scopes = {ExtraScope.TABLE} async def resolve(self, context): @@ -366,6 +381,10 @@ class IsViewExtra(Extra): class DebugExtra(Extra): description = "Extra debug information" + docs_note = ( + "The contents of this block are not a stable part of the Datasette " + "API and may change without warning." + ) example = ExtraExample("/fixtures/facetable.json?_extra=debug") examples = { ExtraScope.ROW: ExtraExample( @@ -482,6 +501,10 @@ class DisplayRowsExtra(Extra): class RenderCellExtra(Extra): description = "Rendered HTML for each cell using the render_cell plugin hook" + docs_note = ( + "See the :ref:`render_cell() plugin hook ` " + "documentation." + ) example = ExtraExample( value={ "rows": [ @@ -598,7 +621,28 @@ class QueryExtra(Extra): class ColumnTypesExtra(Extra): description = "Column type assignments for this table" - example = ExtraExample(value={}) + docs_note = ( + "An empty object if no column types have been assigned. Column types " + "can be assigned in :ref:`configuration " + "` or using the :ref:`set column " + "type API `." + ) + example = ExtraExample( + "/fixtures/facetable.json?_size=0&_extra=column_types", + note=( + "This example is from an instance where the ``tags`` column has " + "been assigned the ``json`` column type." + ), + ) + examples = { + ExtraScope.ROW: ExtraExample( + "/fixtures/facetable/1.json?_extra=column_types", + note=( + "This example is from an instance where the ``tags`` column " + "has been assigned the ``json`` column type." + ), + ) + } scopes = {ExtraScope.TABLE, ExtraScope.ROW} async def resolve(self, context): @@ -615,7 +659,40 @@ class ColumnTypesExtra(Extra): class SetColumnTypeUiExtra(Extra): - description = "Column type UI metadata for this table" + description = "Information needed to build an interface for assigning column types" + docs_note = ( + "``null`` unless the current actor is allowed to use the :ref:`set " + "column type API ` for this table." + ) + example = ExtraExample( + value={ + "path": "/fixtures/facetable/-/set-column-type", + "columns": { + "created": { + "current": None, + "options": [ + {"name": "email", "description": "Email address"}, + {"name": "json", "description": "JSON data"}, + {"name": "url", "description": "URL"}, + ], + }, + "tags": { + "current": {"type": "json", "config": None}, + "options": [ + {"name": "email", "description": "Email address"}, + {"name": "json", "description": "JSON data"}, + {"name": "url", "description": "URL"}, + ], + }, + }, + }, + note=( + "Shape abbreviated to two columns, as seen by an actor with " + "``set-column-type`` permission. ``current`` is the column type " + "currently assigned to each column and ``options`` lists the " + "types that could be assigned to it." + ), + ) scopes = {ExtraScope.TABLE} async def resolve(self, context): @@ -667,13 +744,33 @@ class SetColumnTypeUiExtra(Extra): class MetadataExtra(Extra): description = "Metadata about the table, database or stored query" - example = ExtraExample("/fixtures/facetable.json?_extra=metadata") + docs_note = "See :ref:`metadata` for how to attach metadata to tables." + example = ExtraExample( + "/fixtures/facetable.json?_extra=metadata", + note=( + "This example is from an instance where the ``facetable`` table " + "has a metadata ``description`` and a :ref:`column description " + "` for its ``state`` column. The " + "``columns`` object is empty for tables with no column " + "descriptions." + ), + ) examples = { ExtraScope.ROW: ExtraExample( - "/fixtures/simple_primary_key/1.json?_extra=metadata" + "/fixtures/simple_primary_key/1.json?_extra=metadata", + note=( + "This table has no metadata, so only an empty ``columns`` " + "object is returned." + ), ), ExtraScope.QUERY: ExtraExample( - "/fixtures/neighborhood_search.json?text=town&_extra=metadata" + "/fixtures/neighborhood_search.json?text=town&_extra=metadata", + note=( + "For stored queries this returns the full configuration of " + "the query, including the :ref:`stored query options " + "`. For ``?sql=`` queries it returns an " + "empty object." + ), ), } scopes = {ExtraScope.TABLE, ExtraScope.ROW, ExtraScope.QUERY} @@ -733,6 +830,10 @@ class TableExtra(Extra): class DatabaseColorExtra(Extra): description = "Color assigned to the database" + docs_note = ( + "A six character hex color, without the leading ``#``, derived from " + "a hash of the database name and used in the Datasette interface." + ) example = ExtraExample("/fixtures/facetable.json?_extra=database_color") examples = { ExtraScope.ROW: ExtraExample( @@ -780,6 +881,11 @@ class FiltersExtra(Extra): class CustomTableTemplatesExtra(Extra): description = "Custom template names considered for this table" + docs_note = ( + "The first template in this list that exists will be used to render " + "the table on the HTML version of this page. See " + ":ref:`customization_custom_templates`." + ) example = ExtraExample("/fixtures/facetable.json?_extra=custom_table_templates") scopes = {ExtraScope.TABLE} @@ -793,6 +899,12 @@ class CustomTableTemplatesExtra(Extra): class SortedFacetResultsExtra(Extra): description = "Facet results sorted for display" + docs_note = ( + "The same data as ``facet_results``, as a list in the order used by " + "the HTML interface: facets from :ref:`facet configuration " + "` first, then other facets ordered by their number " + "of results." + ) example = ExtraExample( "/fixtures/facetable.json?_facet=state&_extra=sorted_facet_results" ) @@ -849,7 +961,15 @@ class ViewDefinitionExtra(Extra): class RenderersExtra(Extra): description = "Alternative output renderers available for this table" - example = ExtraExample("/fixtures/facetable.json?_extra=renderers") + example = ExtraExample( + "/fixtures/facetable.json?_extra=renderers", + note=( + "Each key is the name of an output format, each value the URL " + "for this data in that format. Plugins can add additional " + "formats using the :ref:`register_output_renderer() plugin hook " + "`." + ), + ) scopes = {ExtraScope.TABLE} async def resolve(self, context, expandable_columns, query): @@ -887,6 +1007,10 @@ class RenderersExtra(Extra): class PrivateExtra(Extra): description = "Whether this resource is private to the current actor" + docs_note = ( + "``true`` if the current actor can see this resource but an " + "anonymous user could not. See :ref:`authentication_permissions`." + ) example = ExtraExample("/fixtures/facetable.json?_extra=private") examples = { ExtraScope.ROW: ExtraExample( @@ -904,7 +1028,15 @@ class PrivateExtra(Extra): class ExpandableColumnsExtra(Extra): description = "Foreign key columns that can be expanded with labels" - example = ExtraExample("/fixtures/facetable.json?_extra=expandable_columns") + docs_note = "See :ref:`expand_foreign_keys` for how to expand these labels." + example = ExtraExample( + "/fixtures/facetable.json?_extra=expandable_columns", + note=( + "Each item is a ``[foreign_key, label_column]`` pair: the " + "foreign key relationship, then the column in the other table " + "that would be used as the label for each expanded value." + ), + ) scopes = {ExtraScope.TABLE} async def resolve(self, context): @@ -919,9 +1051,14 @@ class ExpandableColumnsExtra(Extra): class ForeignKeyTablesExtra(Extra): description = "Tables that link to this row using foreign keys" example = ExtraExample( - "/fixtures/simple_primary_key/1.json?_extra=foreign_key_tables" + "/fixtures/simple_primary_key/1.json?_extra=foreign_key_tables", + note=( + "``count`` is the number of rows in the other table that " + "reference this row, and ``link`` is a URL to browse those rows." + ), ) scopes = {ExtraScope.ROW} + expensive = True async def resolve(self, context): return await context.foreign_key_tables( @@ -930,7 +1067,30 @@ class ForeignKeyTablesExtra(Extra): class ExtrasExtra(Extra): - description = "Available ?_extra= blocks" + description = "List of ?_extra= blocks that can be used on this page" + example = ExtraExample( + value=[ + { + "name": "count", + "description": "Total count of rows matching these filters", + "toggle_url": "http://localhost/fixtures/facetable.json?_extra=extras&_extra=count", + "selected": False, + }, + { + "name": "extras", + "description": "List of ?_extra= blocks that can be used on this page", + "toggle_url": "http://localhost/fixtures/facetable.json", + "selected": True, + }, + ], + note=( + "Shape abbreviated from /fixtures/facetable.json?_extra=extras - " + "the full response lists every extra described on this page. " + "``toggle_url`` is the current URL with that extra added or " + "removed, and ``selected`` is ``true`` for extras included in " + "the current request." + ), + ) scopes = {ExtraScope.TABLE, ExtraScope.ROW, ExtraScope.QUERY} async def resolve(self, context): diff --git a/docs/json_api.rst b/docs/json_api.rst index 6b595577..fbc3cf60 100644 --- a/docs/json_api.rst +++ b/docs/json_api.rst @@ -276,7 +276,7 @@ The available table extras are listed below. "select count(*) from facetable " ``facet_results`` - Results of facets calculated against this data (May execute additional queries.) + Results of facets calculated against this data (May execute additional queries. See :ref:`facets` for details of how facets work.) Shape abbreviated from /fixtures/facetable.json?_facet=state&_extra=facet_results. @@ -309,12 +309,14 @@ The available table extras are listed below. ``GET /fixtures/facetable.json?_facet=state&_extra=facets_timed_out`` + A list of the names of any facets that exceeded the :ref:`setting_facet_time_limit_ms` time limit - an empty list if every facet calculation completed. + .. code-block:: json [] ``suggested_facets`` - Suggestions for facets that might return interesting results (May execute additional queries.) + Suggestions for facets that might return interesting results (May execute additional queries. Suggestions are controlled by the :ref:`setting_suggest_facets` setting.) Shape abbreviated from /fixtures/facetable.json?_extra=suggested_facets. @@ -341,6 +343,8 @@ The available table extras are listed below. ``GET /fixtures/facetable.json?_size=1&_extra=next_url`` + ``null`` if there are no more pages of results. See :ref:`json_api_pagination`. + .. code-block:: json "http://localhost/fixtures/facetable.json?_size=1&_extra=next_url&_next=1" @@ -426,7 +430,7 @@ The available table extras are listed below. ] ``render_cell`` - Rendered HTML for each cell using the render_cell plugin hook + Rendered HTML for each cell using the render_cell plugin hook (See the :ref:`render_cell() plugin hook ` documentation.) The ``render_cell`` array has one item per row, in the same order as the ``rows`` array. Each object is keyed by column name. Only columns whose rendered value differs from the default are included. @@ -452,7 +456,7 @@ The available table extras are listed below. } ``debug`` - Extra debug information + Extra debug information (The contents of this block are not a stable part of the Datasette API and may change without warning.) ``GET /fixtures/facetable.json?_extra=debug`` @@ -501,28 +505,108 @@ The available table extras are listed below. } ``column_types`` - Column type assignments for this table + Column type assignments for this table (An empty object if no column types have been assigned. Column types can be assigned in :ref:`configuration ` or using the :ref:`set column type API `.) - .. code-block:: json + ``GET /fixtures/facetable.json?_size=0&_extra=column_types`` - {} - -``set_column_type_ui`` - Column type UI metadata for this table - -``metadata`` - Metadata about the table, database or stored query - - ``GET /fixtures/facetable.json?_extra=metadata`` + This example is from an instance where the ``tags`` column has been assigned the ``json`` column type. .. code-block:: json { - "columns": {} + "tags": { + "type": "json", + "config": null + } + } + +``set_column_type_ui`` + Information needed to build an interface for assigning column types (``null`` unless the current actor is allowed to use the :ref:`set column type API ` for this table.) + + Shape abbreviated to two columns, as seen by an actor with ``set-column-type`` permission. ``current`` is the column type currently assigned to each column and ``options`` lists the types that could be assigned to it. + + .. code-block:: json + + { + "path": "/fixtures/facetable/-/set-column-type", + "columns": { + "created": { + "current": null, + "options": [ + { + "name": "email", + "description": "Email address" + }, + { + "name": "json", + "description": "JSON data" + }, + { + "name": "url", + "description": "URL" + } + ] + }, + "tags": { + "current": { + "type": "json", + "config": null + }, + "options": [ + { + "name": "email", + "description": "Email address" + }, + { + "name": "json", + "description": "JSON data" + }, + { + "name": "url", + "description": "URL" + } + ] + } + } + } + +``metadata`` + Metadata about the table, database or stored query (See :ref:`metadata` for how to attach metadata to tables.) + + ``GET /fixtures/facetable.json?_extra=metadata`` + + This example is from an instance where the ``facetable`` table has a metadata ``description`` and a :ref:`column description ` for its ``state`` column. The ``columns`` object is empty for tables with no column descriptions. + + .. code-block:: json + + { + "description": "A demo table of places, used to demonstrate facets", + "columns": { + "state": "Two letter US state code" + } } ``extras`` - Available ?_extra= blocks + List of ?_extra= blocks that can be used on this page + + Shape abbreviated from /fixtures/facetable.json?_extra=extras - the full response lists every extra described on this page. ``toggle_url`` is the current URL with that extra added or removed, and ``selected`` is ``true`` for extras included in the current request. + + .. code-block:: json + + [ + { + "name": "count", + "description": "Total count of rows matching these filters", + "toggle_url": "http://localhost/fixtures/facetable.json?_extra=extras&_extra=count", + "selected": false + }, + { + "name": "extras", + "description": "List of ?_extra= blocks that can be used on this page", + "toggle_url": "http://localhost/fixtures/facetable.json", + "selected": true + } + ] ``database`` Database name @@ -543,7 +627,7 @@ The available table extras are listed below. "facetable" ``database_color`` - Color assigned to the database + Color assigned to the database (A six character hex color, without the leading ``#``, derived from a hash of the database name and used in the Datasette interface.) ``GET /fixtures/facetable.json?_extra=database_color`` @@ -556,6 +640,8 @@ The available table extras are listed below. ``GET /fixtures/facetable.json?_extra=renderers`` + Each key is the name of an output format, each value the URL for this data in that format. Plugins can add additional formats using the :ref:`register_output_renderer() plugin hook `. + .. code-block:: json { @@ -563,7 +649,7 @@ The available table extras are listed below. } ``custom_table_templates`` - Custom template names considered for this table + Custom template names considered for this table (The first template in this list that exists will be used to render the table on the HTML version of this page. See :ref:`customization_custom_templates`.) ``GET /fixtures/facetable.json?_extra=custom_table_templates`` @@ -576,7 +662,7 @@ The available table extras are listed below. ] ``sorted_facet_results`` - Facet results sorted for display + Facet results sorted for display (The same data as ``facet_results``, as a list in the order used by the HTML interface: facets from :ref:`facet configuration ` first, then other facets ordered by their number of results.) ``GET /fixtures/facetable.json?_facet=state&_extra=sorted_facet_results`` @@ -643,7 +729,7 @@ The available table extras are listed below. true ``private`` - Whether this resource is private to the current actor + Whether this resource is private to the current actor (``true`` if the current actor can see this resource but an anonymous user could not. See :ref:`authentication_permissions`.) ``GET /fixtures/facetable.json?_extra=private`` @@ -652,10 +738,12 @@ The available table extras are listed below. false ``expandable_columns`` - Foreign key columns that can be expanded with labels + Foreign key columns that can be expanded with labels (See :ref:`expand_foreign_keys` for how to expand these labels.) ``GET /fixtures/facetable.json?_extra=expandable_columns`` + Each item is a ``[foreign_key, label_column]`` pair: the foreign key relationship, then the column in the other table that would be used as the label for each expanded value. + .. code-block:: json [ @@ -720,7 +808,7 @@ The following extras are available for row JSON responses. ] ``render_cell`` - Rendered HTML for each cell using the render_cell plugin hook + Rendered HTML for each cell using the render_cell plugin hook (See the :ref:`render_cell() plugin hook ` documentation.) The ``render_cell`` array has one item for the requested row. The object is keyed by column name. Only columns whose rendered value differs from the default are included. @@ -741,7 +829,7 @@ The following extras are available for row JSON responses. } ``debug`` - Extra debug information + Extra debug information (The contents of this block are not a stable part of the Datasette API and may change without warning.) ``GET /fixtures/simple_primary_key/1.json?_extra=debug`` @@ -803,17 +891,28 @@ The following extras are available for row JSON responses. } ``column_types`` - Column type assignments for this table + Column type assignments for this table (An empty object if no column types have been assigned. Column types can be assigned in :ref:`configuration ` or using the :ref:`set column type API `.) + + ``GET /fixtures/facetable/1.json?_extra=column_types`` + + This example is from an instance where the ``tags`` column has been assigned the ``json`` column type. .. code-block:: json - {} + { + "tags": { + "type": "json", + "config": null + } + } ``metadata`` - Metadata about the table, database or stored query + Metadata about the table, database or stored query (See :ref:`metadata` for how to attach metadata to tables.) ``GET /fixtures/simple_primary_key/1.json?_extra=metadata`` + This table has no metadata, so only an empty ``columns`` object is returned. + .. code-block:: json { @@ -821,7 +920,26 @@ The following extras are available for row JSON responses. } ``extras`` - Available ?_extra= blocks + List of ?_extra= blocks that can be used on this page + + Shape abbreviated from /fixtures/facetable.json?_extra=extras - the full response lists every extra described on this page. ``toggle_url`` is the current URL with that extra added or removed, and ``selected`` is ``true`` for extras included in the current request. + + .. code-block:: json + + [ + { + "name": "count", + "description": "Total count of rows matching these filters", + "toggle_url": "http://localhost/fixtures/facetable.json?_extra=extras&_extra=count", + "selected": false + }, + { + "name": "extras", + "description": "List of ?_extra= blocks that can be used on this page", + "toggle_url": "http://localhost/fixtures/facetable.json", + "selected": true + } + ] ``database`` Database name @@ -842,7 +960,7 @@ The following extras are available for row JSON responses. "simple_primary_key" ``database_color`` - Color assigned to the database + Color assigned to the database (A six character hex color, without the leading ``#``, derived from a hash of the database name and used in the Datasette interface.) ``GET /fixtures/simple_primary_key/1.json?_extra=database_color`` @@ -851,7 +969,7 @@ The following extras are available for row JSON responses. "9403e5" ``private`` - Whether this resource is private to the current actor + Whether this resource is private to the current actor (``true`` if the current actor can see this resource but an anonymous user could not. See :ref:`authentication_permissions`.) ``GET /fixtures/simple_primary_key/1.json?_extra=private`` @@ -860,10 +978,12 @@ The following extras are available for row JSON responses. false ``foreign_key_tables`` - Tables that link to this row using foreign keys + Tables that link to this row using foreign keys (May execute additional queries.) ``GET /fixtures/simple_primary_key/1.json?_extra=foreign_key_tables`` + ``count`` is the number of rows in the other table that reference this row, and ``link`` is a URL to browse those rows. + .. code-block:: json [ @@ -921,7 +1041,7 @@ The following extras are available for arbitrary SQL query responses and stored, ] ``render_cell`` - Rendered HTML for each cell using the render_cell plugin hook + Rendered HTML for each cell using the render_cell plugin hook (See the :ref:`render_cell() plugin hook ` documentation.) The ``render_cell`` array has one item per query result row, in the same order as the ``rows`` array. Each object is keyed by column name. Only columns whose rendered value differs from the default are included. @@ -941,7 +1061,7 @@ The following extras are available for arbitrary SQL query responses and stored, } ``debug`` - Extra debug information + Extra debug information (The contents of this block are not a stable part of the Datasette API and may change without warning.) ``GET /fixtures/-/query.json?sql=select+1+as+one&_extra=debug`` @@ -1000,10 +1120,12 @@ The following extras are available for arbitrary SQL query responses and stored, } ``metadata`` - Metadata about the table, database or stored query + Metadata about the table, database or stored query (See :ref:`metadata` for how to attach metadata to tables.) ``GET /fixtures/neighborhood_search.json?text=town&_extra=metadata`` + For stored queries this returns the full configuration of the query, including the :ref:`stored query options `. For ``?sql=`` queries it returns an empty object. + .. code-block:: json { @@ -1029,7 +1151,26 @@ The following extras are available for arbitrary SQL query responses and stored, } ``extras`` - Available ?_extra= blocks + List of ?_extra= blocks that can be used on this page + + Shape abbreviated from /fixtures/facetable.json?_extra=extras - the full response lists every extra described on this page. ``toggle_url`` is the current URL with that extra added or removed, and ``selected`` is ``true`` for extras included in the current request. + + .. code-block:: json + + [ + { + "name": "count", + "description": "Total count of rows matching these filters", + "toggle_url": "http://localhost/fixtures/facetable.json?_extra=extras&_extra=count", + "selected": false + }, + { + "name": "extras", + "description": "List of ?_extra= blocks that can be used on this page", + "toggle_url": "http://localhost/fixtures/facetable.json", + "selected": true + } + ] ``database`` Database name @@ -1041,7 +1182,7 @@ The following extras are available for arbitrary SQL query responses and stored, "fixtures" ``database_color`` - Color assigned to the database + Color assigned to the database (A six character hex color, without the leading ``#``, derived from a hash of the database name and used in the Datasette interface.) ``GET /fixtures/-/query.json?sql=select+1+as+one&_extra=database_color`` @@ -1050,7 +1191,7 @@ The following extras are available for arbitrary SQL query responses and stored, "9403e5" ``private`` - Whether this resource is private to the current actor + Whether this resource is private to the current actor (``true`` if the current actor can see this resource but an anonymous user could not. See :ref:`authentication_permissions`.) ``GET /fixtures/-/query.json?sql=select+1+as+one&_extra=private`` diff --git a/docs/json_api_doc.py b/docs/json_api_doc.py index 44ef4a42..422e67f4 100644 --- a/docs/json_api_doc.py +++ b/docs/json_api_doc.py @@ -93,9 +93,26 @@ async def _fetch_live_examples(scoped_classes): datasette = Datasette( [str(db_path)], settings={"num_sql_threads": 1}, + metadata={ + "databases": { + "fixtures": { + "tables": { + "facetable": { + "description": "A demo table of places, used to demonstrate facets", + "columns": {"state": "Two letter US state code"}, + } + } + } + } + }, config={ "databases": { "fixtures": { + "tables": { + "facetable": { + "column_types": {"tags": "json"}, + } + }, "queries": { "neighborhood_search": { "sql": textwrap.dedent(""" @@ -108,7 +125,7 @@ async def _fetch_live_examples(scoped_classes): """), "title": "Search neighborhoods", } - } + }, } } }, From 88878b418473dcb399de376658d2bd7423b66c97 Mon Sep 17 00:00:00 2001 From: Simon Willison Date: Fri, 12 Jun 2026 12:51:40 -0700 Subject: [PATCH 04/90] datasette.allowed_many() method --- datasette/app.py | 101 +++++++--- datasette/utils/actions_sql.py | 221 ++++++++++++++------- docs/internals.rst | 33 ++++ docs/plugin_hooks.rst | 6 + tests/conftest.py | 11 +- tests/test_allowed_many.py | 341 +++++++++++++++++++++++++++++++++ 6 files changed, 613 insertions(+), 100 deletions(-) create mode 100644 tests/test_allowed_many.py diff --git a/datasette/app.py b/datasette/app.py index 81d23acb..a6696ad9 100644 --- a/datasette/app.py +++ b/datasette/app.py @@ -2,7 +2,7 @@ from __future__ import annotations import asyncio import contextvars -from typing import TYPE_CHECKING, Any, Dict, Iterable, List +from typing import TYPE_CHECKING, Any, Dict, Iterable, List, Sequence if TYPE_CHECKING: from datasette.permissions import Resource @@ -1817,46 +1817,97 @@ class Datasette: # For global actions, resource can be omitted: can_debug = await datasette.allowed(action="permissions-debug", actor=actor) """ - from datasette.utils.actions_sql import check_permission_for_resource + results = await self.allowed_many( + actions=[action], resource=resource, actor=actor + ) + return results[action] - # For global actions, resource remains None + async def allowed_many( + self, + *, + actions: Sequence[str], + resource: "Resource" = None, + actor: dict | None = None, + ) -> dict[str, bool]: + """ + Check several actions against one resource for one actor. - # Check if this action has also_requires - if so, check that action first - action_obj = self.actions.get(action) - if action_obj and action_obj.also_requires: - # Must have the required action first - if not await self.allowed( - action=action_obj.also_requires, - resource=resource, + Resolves every action (plus any also_requires dependencies) with a + single internal database query, instead of one or two queries per + action. + + Example: + from datasette.resources import TableResource + results = await datasette.allowed_many( + actions=["edit-schema", "drop-table", "insert-row"], + resource=TableResource(database="data", table="exercise"), actor=actor, - ): - return False + ) + # {"edit-schema": True, "drop-table": True, "insert-row": False} + """ + from datasette.utils.actions_sql import check_permissions_for_actions # For global actions, resource is None parent = resource.parent if resource else None child = resource.child if resource else None - result = await check_permission_for_resource( + # Expand also_requires dependencies (transitively) so that each + # dependency is resolved within the same batch + expanded = [] + + def add_action(name): + if name in expanded: + return + action_obj = self.actions.get(name) + if action_obj is None: + raise ValueError(f"Unknown action: {name}") + expanded.append(name) + if action_obj.also_requires: + add_action(action_obj.also_requires) + + requested = list(dict.fromkeys(actions)) + for name in requested: + add_action(name) + + raw = await check_permissions_for_actions( datasette=self, actor=actor, - action=action, + actions=expanded, parent=parent, child=child, ) + final = {} - # Log the permission check for debugging - self._permission_checks.append( - PermissionCheck( - when=datetime.datetime.now(datetime.timezone.utc).isoformat(), - actor=actor, - action=action, - parent=parent, - child=child, - result=result, + def resolve(name): + # final verdict = own rules AND verdict of also_requires chain + if name in final: + return final[name] + result = raw[name] + action_obj = self.actions.get(name) + if result and action_obj.also_requires: + result = resolve(action_obj.also_requires) + final[name] = result + return result + + for name in expanded: + resolve(name) + + # Log every check for the debug page, dependencies before the + # actions that required them + when = datetime.datetime.now(datetime.timezone.utc).isoformat() + for name in reversed(expanded): + self._permission_checks.append( + PermissionCheck( + when=when, + actor=actor, + action=name, + parent=parent, + child=child, + result=final[name], + ) ) - ) - return result + return {name: final[name] for name in requested} async def ensure_permission( self, diff --git a/datasette/utils/actions_sql.py b/datasette/utils/actions_sql.py index 891ee913..a422c1ed 100644 --- a/datasette/utils/actions_sql.py +++ b/datasette/utils/actions_sql.py @@ -21,6 +21,8 @@ The core pattern is: - Across levels, child beats parent beats global """ +import asyncio +import re from typing import TYPE_CHECKING from datasette.utils.permissions import gather_permission_sql_from_hooks @@ -495,6 +497,146 @@ async def build_permission_rules_sql( return rules_union, all_params, restriction_sqls +async def check_permissions_for_actions( + *, + datasette: "Datasette", + actor: dict | None, + actions: list[str], + parent: str | None, + child: str | None, +) -> dict[str, bool]: + """ + Check several actions for one actor and resource in a single query. + + Args: + datasette: The Datasette instance + actor: The actor dict (or None) + actions: List of action names to check + parent: The parent resource identifier (e.g., database name, or None) + child: The child resource identifier (e.g., table name, or None) + + Returns: + Dict mapping each action name to True (allowed) or False (denied) + + Each action contributes its own tagged block of permission rules + (gathered from the permission_resources_sql hook, with parameters + namespaced per action to avoid collisions) plus an optional + restriction allowlist CTE. One internal database query resolves + the winning rule per action using the same specificity-then-deny + ordering as the rest of the permission system. + + Note: this resolves each action independently - also_requires + dependencies are handled by the caller (Datasette.allowed_many). + """ + from datasette.utils.permissions import SKIP_PERMISSION_CHECKS + + for action in actions: + if not datasette.actions.get(action): + raise ValueError(f"Unknown action: {action}") + + # Dedupe while preserving order + unique_actions = list(dict.fromkeys(actions)) + if not unique_actions: + return {} + + # Gather hook results for each action concurrently - hooks within a + # single action still run sequentially, preserving existing semantics + gathered = await asyncio.gather( + *( + gather_permission_sql_from_hooks( + datasette=datasette, actor=actor, action=action + ) + for action in unique_actions + ) + ) + + if any(result is SKIP_PERMISSION_CHECKS for result in gathered): + return {action: True for action in unique_actions} + + params = {"_check_parent": parent, "_check_child": child} + ctes = [] + selects = [] + verdicts = {} + + for i, (action, permission_sqls) in enumerate(zip(unique_actions, gathered)): + prefix = f"a{i}_" + rule_parts = [] + restriction_parts = [] + + for permission_sql in permission_sqls: + sql = permission_sql.sql + restriction_sql = permission_sql.restriction_sql + # Namespace this block's params so identical names used for + # different actions cannot collide + for key in permission_sql.params or {}: + new_key = prefix + key + params[new_key] = permission_sql.params[key] + pattern = re.compile(":" + re.escape(key) + r"(?![A-Za-z0-9_])") + if sql: + sql = pattern.sub(":" + new_key, sql) + if restriction_sql: + restriction_sql = pattern.sub(":" + new_key, restriction_sql) + + if restriction_sql: + restriction_parts.append(restriction_sql) + + # Skip plugins that only provide restriction_sql (no permission rules) + if sql is None: + continue + rule_parts.append( + f"SELECT parent, child, allow, reason, '{permission_sql.source}' AS source_plugin FROM (\n{sql}\n)" + ) + + if not rule_parts: + # No rules from any plugin - default deny. Restrictions can + # only restrict, never grant, so no SQL is needed at all + verdicts[action] = False + continue + ctes.append(f"a{i}_rules AS (\n" + "\nUNION ALL\n".join(rule_parts) + "\n)") + + # Winning rule for this action: most specific depth first, then + # deny-beats-allow, then source_plugin as a stable tie-break + verdict_sql = f"""COALESCE(( + SELECT allow FROM ( + SELECT allow, source_plugin, + CASE + WHEN child IS NOT NULL THEN 2 + WHEN parent IS NOT NULL THEN 1 + ELSE 0 + END AS depth + FROM a{i}_rules + WHERE (parent IS NULL OR parent = :_check_parent) + AND (child IS NULL OR child = :_check_child) + ORDER BY + depth DESC, + CASE WHEN allow = 0 THEN 0 ELSE 1 END, + source_plugin + LIMIT 1 + ) +), 0)""" + + if restriction_parts: + # Database-level restrictions (parent, NULL) match all children + restriction_intersect = "\nINTERSECT\n".join( + f"SELECT * FROM ({sql})" for sql in restriction_parts + ) + ctes.append(f"a{i}_restriction AS (\n{restriction_intersect}\n)") + verdict_sql = f"""({verdict_sql}) AND EXISTS ( + SELECT 1 FROM a{i}_restriction r + WHERE (r.parent = :_check_parent OR r.parent IS NULL) + AND (r.child = :_check_child OR r.child IS NULL) +)""" + + selects.append(f"SELECT {i} AS action_idx, ({verdict_sql}) AS is_allowed") + + if selects: + query = "WITH\n" + ",\n".join(ctes) + "\n" + "\nUNION ALL\n".join(selects) + result = await datasette.get_internal_database().execute(query, params) + for row in result.rows: + verdicts[unique_actions[row[0]]] = bool(row[1]) + return verdicts + + async def check_permission_for_resource( *, datasette: "Datasette", @@ -515,77 +657,12 @@ async def check_permission_for_resource( Returns: True if the actor is allowed, False otherwise - - This builds the cascading permission query and checks if the specific - resource is in the allowed set. """ - rules_union, all_params, restriction_sqls = await build_permission_rules_sql( - datasette, actor, action + results = await check_permissions_for_actions( + datasette=datasette, + actor=actor, + actions=[action], + parent=parent, + child=child, ) - - # If no rules (empty SQL), default deny - if not rules_union: - return False - - # Add parameters for the resource we're checking - all_params["_check_parent"] = parent - all_params["_check_child"] = child - - # If there are restriction filters, check if the resource passes them first - if restriction_sqls: - # Check if resource is in restriction allowlist - # Database-level restrictions (parent, NULL) should match all children (parent, *) - # Wrap each restriction_sql in a subquery to avoid operator precedence issues - restriction_check = "\nINTERSECT\n".join( - f"SELECT * FROM ({sql})" for sql in restriction_sqls - ) - restriction_query = f""" -WITH restriction_list AS ( - {restriction_check} -) -SELECT EXISTS ( - SELECT 1 FROM restriction_list - WHERE (parent = :_check_parent OR parent IS NULL) - AND (child = :_check_child OR child IS NULL) -) AS in_allowlist -""" - result = await datasette.get_internal_database().execute( - restriction_query, all_params - ) - if result.rows and not result.rows[0][0]: - # Resource not in restriction allowlist - deny - return False - - query = f""" -WITH -all_rules AS ( - {rules_union} -), -matched_rules AS ( - SELECT ar.*, - CASE - WHEN ar.child IS NOT NULL THEN 2 -- child-level (most specific) - WHEN ar.parent IS NOT NULL THEN 1 -- parent-level - ELSE 0 -- root/global - END AS depth - FROM all_rules ar - WHERE (ar.parent IS NULL OR ar.parent = :_check_parent) - AND (ar.child IS NULL OR ar.child = :_check_child) -), -winner AS ( - SELECT * - FROM matched_rules - ORDER BY - depth DESC, -- specificity first (higher depth wins) - CASE WHEN allow=0 THEN 0 ELSE 1 END, -- then deny over allow - source_plugin -- stable tie-break - LIMIT 1 -) -SELECT COALESCE((SELECT allow FROM winner), 0) AS is_allowed -""" - - # Execute the query against the internal database - result = await datasette.get_internal_database().execute(query, all_params) - if result.rows: - return bool(result.rows[0][0]) - return False + return results[action] diff --git a/docs/internals.rst b/docs/internals.rst index f269155a..f3c1152a 100644 --- a/docs/internals.rst +++ b/docs/internals.rst @@ -512,6 +512,39 @@ Example usage: The method returns ``True`` if the permission is granted, ``False`` if denied. +.. _datasette_allowed_many: + +await .allowed_many(\*, actions, resource, actor=None) +------------------------------------------------------ + +``actions`` - list of strings + The names of the actions to permission check. + +``resource`` - Resource object + A Resource object representing the database, table, or other resource that each action is checked against. Omit for global actions. + +``actor`` - dictionary, optional + The authenticated actor. This is usually ``request.actor``. Defaults to ``None`` for unauthenticated requests. + +Checks several actions against the same resource for the same actor, returning a dictionary mapping each action name to ``True`` or ``False``. The whole batch - including any actions pulled in through ``also_requires`` dependencies - is resolved with a single SQL query against the internal database, so this is much faster than calling :ref:`datasette.allowed() ` once per action. + +Example usage: + +.. code-block:: python + + from datasette.resources import TableResource + + results = await datasette.allowed_many( + actions=["insert-row", "delete-row", "drop-table"], + resource=TableResource( + database="fixtures", table="facetable" + ), + actor=request.actor, + ) + # {"insert-row": True, "delete-row": True, "drop-table": False} + +Actions for which no plugin provides any permission rules are resolved to ``False`` directly, without being included in the SQL query at all. + .. _datasette_allowed_resources: await .allowed_resources(action, actor=None, \*, parent=None, include_is_private=False, include_reasons=False, limit=100, next=None) diff --git a/docs/plugin_hooks.rst b/docs/plugin_hooks.rst index 2a0ddc93..0a55f8ec 100644 --- a/docs/plugin_hooks.rst +++ b/docs/plugin_hooks.rst @@ -1458,6 +1458,12 @@ to avoid conflicts with other plugins. The recommended convention is to prefix p plugin's source name (e.g., ``myplugin_user_id``). The system reserves these parameter names: ``:actor``, ``:actor_id``, ``:action``, and ``:filter_parent``. +This hook may be called for many actions in rapid succession - for example +:ref:`datasette.allowed_many() ` gathers rules for every action in its batch +concurrently. Hook implementations must not assume that checks for different actions arrive one +page-render apart, and expensive work (such as network calls) should be cached independently of the +``action`` argument where possible. + You can also use return ``PermissionSQL.allow(reason="reason goes here")`` or ``PermissionSQL.deny(reason="reason goes here")`` as shortcuts for simple root-level allow or deny rules. These will create SQL snippets that look like this: .. code-block:: sql diff --git a/tests/conftest.py b/tests/conftest.py index b9b3c35e..27d6fa77 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -146,6 +146,7 @@ def restore_working_directory(tmpdir, request): @pytest.fixture(scope="session", autouse=True) def check_actions_are_documented(): from datasette.plugins import pm + from datasette.default_actions import register_actions as default_register_actions content = ( pathlib.Path(__file__).parent.parent / "docs" / "authentication.rst" @@ -154,6 +155,9 @@ def check_actions_are_documented(): documented_actions = set(permissions_re.findall(content)).union( UNDOCUMENTED_PERMISSIONS ) + # Only Datasette core actions need to be documented - actions registered + # by (test) plugins are checked for registration but not documentation + core_actions = {action.name for action in default_register_actions()} def before(hook_name, hook_impls, kwargs): if hook_name == "permission_resources_sql": @@ -165,9 +169,10 @@ def check_actions_are_documented(): + " (or maybe a test forgot to do await ds.invoke_startup())" ) action = kwargs.get("action").replace("-", "_") - assert ( - action in documented_actions - ), "Undocumented permission action: {}".format(action) + if kwargs["action"] in core_actions: + assert ( + action in documented_actions + ), "Undocumented permission action: {}".format(action) pm.add_hookcall_monitoring( before=before, after=lambda outcome, hook_name, hook_impls, kwargs: None diff --git a/tests/test_allowed_many.py b/tests/test_allowed_many.py new file mode 100644 index 00000000..53d0ffd9 --- /dev/null +++ b/tests/test_allowed_many.py @@ -0,0 +1,341 @@ +""" +Tests for the datasette.allowed_many() batch permission API, which +resolves multiple actions against one resource in a single internal +database query. datasette.allowed() is implemented on top of it, so +both entry points share one resolution code path. +""" + +import pytest +import pytest_asyncio +from datasette.app import Datasette +from datasette.permissions import PermissionSQL, SkipPermissions +from datasette.resources import DatabaseResource, TableResource +from datasette import hookimpl + + +@pytest_asyncio.fixture +async def ds(): + ds = Datasette() + await ds.invoke_startup() + db = ds.add_memory_database("analytics") + await db.execute_write("CREATE TABLE IF NOT EXISTS users (id INTEGER PRIMARY KEY)") + await db.execute_write("CREATE TABLE IF NOT EXISTS events (id INTEGER PRIMARY KEY)") + await ds._refresh_schemas() + return ds + + +class MatrixRulesPlugin: + """Different rules per action for actor carol, to exercise resolution.""" + + @hookimpl + def permission_resources_sql(self, datasette, actor, action): + if not actor or actor.get("id") != "carol": + return None + if action == "view-table": + return PermissionSQL(sql=""" + SELECT NULL AS parent, NULL AS child, 1 AS allow, 'global allow' AS reason + UNION ALL + SELECT 'analytics' AS parent, 'sensitive' AS child, 0 AS allow, 'deny sensitive' AS reason + """) + if action == "insert-row": + return PermissionSQL( + sql="SELECT 'analytics' AS parent, NULL AS child, 1 AS allow, 'analytics writes' AS reason" + ) + # Everything else: no opinion (implicit deny unless defaults allow) + return None + + +@pytest.mark.asyncio +async def test_allowed_many_basic(ds): + plugin = MatrixRulesPlugin() + ds.pm.register(plugin, name="matrix") + try: + results = await ds.allowed_many( + actions=["view-table", "insert-row", "drop-table"], + resource=TableResource("analytics", "users"), + actor={"id": "carol"}, + ) + assert results == { + "view-table": True, + "insert-row": True, + "drop-table": False, + } + # Child-level deny beats global allow + sensitive = await ds.allowed_many( + actions=["view-table"], + resource=TableResource("analytics", "sensitive"), + actor={"id": "carol"}, + ) + assert sensitive == {"view-table": False} + finally: + ds.pm.unregister(name="matrix") + + +@pytest.mark.asyncio +async def test_allowed_many_matches_allowed(ds): + """Every action resolved by allowed_many() must match allowed().""" + plugin = MatrixRulesPlugin() + ds.pm.register(plugin, name="matrix") + try: + all_actions = list(ds.actions) + for resource in ( + TableResource("analytics", "users"), + TableResource("analytics", "sensitive"), + DatabaseResource("analytics"), + ): + batched = await ds.allowed_many( + actions=all_actions, resource=resource, actor={"id": "carol"} + ) + assert set(batched) == set(all_actions) + for action in all_actions: + individual = await ds.allowed( + action=action, resource=resource, actor={"id": "carol"} + ) + assert ( + batched[action] == individual + ), f"Mismatch for {action} on {resource}" + finally: + ds.pm.unregister(name="matrix") + + +@pytest.mark.asyncio +async def test_allowed_many_unknown_action_raises(ds): + with pytest.raises(ValueError, match="Unknown action"): + await ds.allowed_many( + actions=["view-table", "no-such-action"], + resource=TableResource("analytics", "users"), + actor=None, + ) + + +@pytest.mark.asyncio +async def test_allowed_many_empty_actions(ds): + assert ( + await ds.allowed_many( + actions=[], resource=TableResource("analytics", "users"), actor=None + ) + == {} + ) + + +class AlsoRequiresRulesPlugin: + """dave: store-query allowed but execute-sql explicitly denied. + erin: store-query allowed (execute-sql stays default-allowed).""" + + @hookimpl + def permission_resources_sql(self, datasette, actor, action): + actor_id = actor.get("id") if actor else None + if actor_id == "dave": + if action == "store-query": + return PermissionSQL( + sql="SELECT NULL AS parent, NULL AS child, 1 AS allow, 'dave can store' AS reason" + ) + if action == "execute-sql": + return PermissionSQL( + sql="SELECT NULL AS parent, NULL AS child, 0 AS allow, 'dave no sql' AS reason" + ) + if actor_id == "erin" and action == "store-query": + return PermissionSQL( + sql="SELECT NULL AS parent, NULL AS child, 1 AS allow, 'erin can store' AS reason" + ) + return None + + +@pytest.mark.asyncio +async def test_allowed_many_also_requires(ds): + # store-query also_requires execute-sql, which also_requires view-database + plugin = AlsoRequiresRulesPlugin() + ds.pm.register(plugin, name="also_requires") + try: + resource = DatabaseResource("analytics") + dave = await ds.allowed_many( + actions=["store-query", "execute-sql", "view-database"], + resource=resource, + actor={"id": "dave"}, + ) + # execute-sql denied, so store-query must be denied too + assert dave == { + "store-query": False, + "execute-sql": False, + "view-database": True, + } + erin = await ds.allowed_many( + actions=["store-query"], resource=resource, actor={"id": "erin"} + ) + assert erin == {"store-query": True} + # Must match the single-check path + assert ( + await ds.allowed( + action="store-query", resource=resource, actor={"id": "dave"} + ) + is False + ) + assert ( + await ds.allowed( + action="store-query", resource=resource, actor={"id": "erin"} + ) + is True + ) + finally: + ds.pm.unregister(name="also_requires") + + +@pytest.mark.asyncio +async def test_allowed_many_respects_restrictions(ds): + """Token-style _r restrictions are enforced within the batch.""" + actor = {"id": "root", "_r": {"d": {"analytics": ["vt"]}}} + results = await ds.allowed_many( + actions=["view-table", "drop-table"], + resource=TableResource("analytics", "users"), + actor=actor, + ) + # root could normally do both, but the token only allows view-table + # on the analytics database + assert results == {"view-table": True, "drop-table": False} + other_db = await ds.allowed_many( + actions=["view-table"], + resource=TableResource("production", "stuff"), + actor=actor, + ) + assert other_db == {"view-table": False} + # Equivalence with allowed() + assert ( + await ds.allowed( + action="view-table", + resource=TableResource("analytics", "users"), + actor=actor, + ) + is True + ) + assert ( + await ds.allowed( + action="drop-table", + resource=TableResource("analytics", "users"), + actor=actor, + ) + is False + ) + + +class ParamCollisionPlugin: + """Same parameter name with a different value for every action.""" + + @hookimpl + def permission_resources_sql(self, datasette, actor, action): + if not actor or actor.get("id") != "paula": + return None + flag = 1 if action in ("drop-table", "insert-row") else 0 + return PermissionSQL( + sql="SELECT NULL AS parent, NULL AS child, :flag AS allow, 'flagged' AS reason", + params={"flag": flag}, + ) + + +@pytest.mark.asyncio +async def test_allowed_many_namespaces_params_across_actions(ds): + """Many actions whose rules use identical param names must not collide.""" + plugin = ParamCollisionPlugin() + ds.pm.register(plugin, name="collision") + try: + all_actions = list(ds.actions) + assert len(all_actions) >= 15 + resource = TableResource("analytics", "users") + results = await ds.allowed_many( + actions=all_actions, resource=resource, actor={"id": "paula"} + ) + # Spot-check: only the flagged actions resolve True + assert results["drop-table"] is True + assert results["create-table"] is False + # Full equivalence against single checks + for action in all_actions: + assert results[action] == await ds.allowed( + action=action, resource=resource, actor={"id": "paula"} + ), f"Mismatch for {action}" + finally: + ds.pm.unregister(name="collision") + + +@pytest.mark.asyncio +async def test_allowed_many_single_internal_db_query(ds): + internal_db = ds.get_internal_database() + calls = [] + original_execute = internal_db.execute + + async def counting_execute(sql, params=None, **kwargs): + calls.append(sql) + return await original_execute(sql, params, **kwargs) + + internal_db.execute = counting_execute + try: + results = await ds.allowed_many( + actions=["view-table", "insert-row", "delete-row", "drop-table"], + resource=TableResource("analytics", "users"), + actor={"id": "root", "_r": {"d": {"analytics": ["vt"]}}}, + ) + assert len(results) == 4 + assert len(calls) == 1 + finally: + internal_db.execute = original_execute + + +@pytest.mark.asyncio +async def test_allowed_many_no_query_when_no_rules(ds): + """Actions with no rules from any plugin are denied without SQL. + + Restrictions can only restrict, never grant, so an action with no + rule rows is always False - it should not contribute to the query, + and if no action has rules there should be no query at all.""" + internal_db = ds.get_internal_database() + calls = [] + original_execute = internal_db.execute + + async def counting_execute(sql, params=None, **kwargs): + calls.append(sql) + return await original_execute(sql, params, **kwargs) + + internal_db.execute = counting_execute + try: + # bob gets no rules at all for these write actions + results = await ds.allowed_many( + actions=["drop-table", "delete-row"], + resource=TableResource("analytics", "users"), + actor={"id": "bob"}, + ) + assert results == {"drop-table": False, "delete-row": False} + assert len(calls) == 0 + # A mixed batch still needs exactly one query + calls.clear() + results = await ds.allowed_many( + actions=["view-table", "drop-table"], + resource=TableResource("analytics", "users"), + actor={"id": "bob"}, + ) + assert results == {"view-table": True, "drop-table": False} + assert len(calls) == 1 + finally: + internal_db.execute = original_execute + + +@pytest.mark.asyncio +async def test_allowed_many_global_actions_without_resource(ds): + results = await ds.allowed_many( + actions=["view-instance", "permissions-debug"], + actor={"id": "root"}, + ) + assert results["view-instance"] is True + # Equivalence with single checks for global actions + for action in ("view-instance", "permissions-debug"): + assert results[action] == await ds.allowed(action=action, actor={"id": "root"}) + anon = await ds.allowed_many(actions=["permissions-debug"], actor=None) + assert anon == {"permissions-debug": False} + + +@pytest.mark.asyncio +async def test_allowed_many_skip_permission_checks(ds): + with SkipPermissions(): + results = await ds.allowed_many( + actions=["view-table", "drop-table"], + resource=TableResource("analytics", "users"), + actor=None, + ) + assert results == {"view-table": True, "drop-table": True} From bb59c61c9f9b6766199ce1434c7008739653f141 Mon Sep 17 00:00:00 2001 From: Simon Willison Date: Fri, 12 Jun 2026 13:11:09 -0700 Subject: [PATCH 05/90] Request-scoped permission check cache Adds a per-request cache for permission check results, plus wiring that resolves action permissions in bulk before plugin hooks need them: - New _permission_check_cache contextvar, set to a fresh dict for each request by DatasetteRouter and reset when the request ends. Keys include the full serialized actor, so actors differing in any field (e.g. token restrictions) never share entries. SkipPermissions mode bypasses the cache entirely. - datasette.allowed_many() now consults the cache and stores its results there, so repeated datasette.allowed() checks within one request resolve without further SQL. - Table pages resolve all registered table-level actions against the current table and all database-level actions against its database (database pages likewise) in batched queries before invoking the table_actions/database_actions plugin hooks - allowed() calls made inside those hooks are then served from the cache with no plugin changes required. Actions with no permission rules from any plugin are resolved to False without touching the database. Benchmarks (benchmarks/) with a simulated 12-plugin ecosystem making 18 checks per table page show 34 -> 13 internal-DB queries per page; with 2ms-per-query internal DB latency (modelling Datasette Cloud) table page time drops from 77.9ms to 27.6ms - the caching layer accounts for ~91% of that improvement over allowed_many() alone. Co-Authored-By: Claude Fable 5 --- datasette/app.py | 67 +++++-- datasette/permissions.py | 8 + datasette/views/database.py | 13 ++ datasette/views/table_extras.py | 26 ++- docs/internals.rst | 4 + docs/plugin_hooks.rst | 6 +- tests/test_allowed_many.py | 340 +++++++++++++++++++++++++++++++- 7 files changed, 443 insertions(+), 21 deletions(-) diff --git a/datasette/app.py b/datasette/app.py index a6696ad9..9979b6c5 100644 --- a/datasette/app.py +++ b/datasette/app.py @@ -291,6 +291,15 @@ DEFAULT_NOT_SET = object() ResourcesSQL = collections.namedtuple("ResourcesSQL", ("sql", "params")) +def _permission_cache_key(actor, action, parent, child): + # Key on the full serialized actor so actors differing in any field + # (e.g. token restrictions) never share cache entries + actor_key = ( + json.dumps(actor, sort_keys=True, default=repr) if actor is not None else None + ) + return (actor_key, action, parent, child) + + async def favicon(request, send): await asgi_send_file( send, @@ -1834,7 +1843,9 @@ class Datasette: Resolves every action (plus any also_requires dependencies) with a single internal database query, instead of one or two queries per - action. + action. Results are stored in the request-scoped permission cache, + so subsequent datasette.allowed() calls for the same checks within + the same request are served from the cache. Example: from datasette.resources import TableResource @@ -1846,6 +1857,10 @@ class Datasette: # {"edit-schema": True, "drop-table": True, "insert-row": False} """ from datasette.utils.actions_sql import check_permissions_for_actions + from datasette.permissions import ( + _permission_check_cache, + _skip_permission_checks, + ) # For global actions, resource is None parent = resource.parent if resource else None @@ -1869,14 +1884,30 @@ class Datasette: for name in requested: add_action(name) - raw = await check_permissions_for_actions( - datasette=self, - actor=actor, - actions=expanded, - parent=parent, - child=child, - ) + # Consult the request-scoped cache, unless permission checks are + # being skipped (skip-mode verdicts must never be cached) + skip = _skip_permission_checks.get() + cache = None if skip else _permission_check_cache.get() + final = {} + to_check = [] + for name in expanded: + if cache is not None: + key = _permission_cache_key(actor, name, parent, child) + if key in cache: + final[name] = cache[key] + continue + to_check.append(name) + + raw = {} + if to_check: + raw = await check_permissions_for_actions( + datasette=self, + actor=actor, + actions=to_check, + parent=parent, + child=child, + ) def resolve(name): # final verdict = own rules AND verdict of also_requires chain @@ -1892,8 +1923,13 @@ class Datasette: for name in expanded: resolve(name) - # Log every check for the debug page, dependencies before the - # actions that required them + # Cache the freshly computed checks + if cache is not None: + for name in to_check: + cache[_permission_cache_key(actor, name, parent, child)] = final[name] + + # Log every check (including cache hits) for the debug page, + # dependencies before the actions that required them when = datetime.datetime.now(datetime.timezone.utc).isoformat() for name in reversed(expanded): self._permission_checks.append( @@ -2663,7 +2699,16 @@ class DatasetteRouter: if raw_path: path = raw_path.decode("ascii") path = path.partition("?")[0] - return await self.route_path(scope, receive, send, path) + # Give each request a fresh permission check cache, so repeated + # datasette.allowed() checks within the request are memoized but + # results never persist beyond it + from datasette.permissions import _permission_check_cache + + cache_token = _permission_check_cache.set({}) + try: + return await self.route_path(scope, receive, send, path) + finally: + _permission_check_cache.reset(cache_token) async def route_path(self, scope, receive, send, path): # Strip off base_url if present before routing diff --git a/datasette/permissions.py b/datasette/permissions.py index a9a3cc7c..786dc026 100644 --- a/datasette/permissions.py +++ b/datasette/permissions.py @@ -8,6 +8,14 @@ _skip_permission_checks = contextvars.ContextVar( "skip_permission_checks", default=False ) +# Request-scoped cache of permission check results. The ASGI router sets +# this to a fresh dict at the start of each request, so cached verdicts +# never outlive a request or leak between actors. Keys are +# (actor_json, action, parent, child) tuples, values are booleans. +_permission_check_cache: contextvars.ContextVar[dict | None] = contextvars.ContextVar( + "permission_check_cache", default=None +) + class SkipPermissions: """Context manager to temporarily skip permission checks. diff --git a/datasette/views/database.py b/datasette/views/database.py index f1756863..6afd9734 100644 --- a/datasette/views/database.py +++ b/datasette/views/database.py @@ -118,6 +118,19 @@ class DatabaseView(View): ) async def database_actions(): + # Resolve the registered database-level actions for this + # database in one batched query, seeding the request permission + # cache so that allowed() calls made inside the plugin hooks + # below are served from the cache + await datasette.allowed_many( + actions=[ + name + for name, action in datasette.actions.items() + if action.resource_class is DatabaseResource + ], + resource=DatabaseResource(database), + actor=request.actor, + ) links = [] for hook in pm.hook.database_actions( datasette=datasette, diff --git a/datasette/views/table_extras.py b/datasette/views/table_extras.py index 948f3daa..a0308e49 100644 --- a/datasette/views/table_extras.py +++ b/datasette/views/table_extras.py @@ -4,7 +4,7 @@ from dataclasses import dataclass from datasette.database import QueryInterrupted from datasette.extras import Extra, ExtraExample, ExtraRegistry, ExtraScope, Provider from datasette.plugins import pm -from datasette.resources import TableResource +from datasette.resources import DatabaseResource, TableResource from datasette.utils import ( await_me_maybe, call_with_supported_arguments, @@ -361,6 +361,30 @@ class ActionsExtra(Extra): else: kwargs["table"] = context.table_name method = pm.hook.table_actions + # Resolve the registered table-level actions for this table + # and the database-level actions for its database in two + # batched queries, seeding the request permission cache so + # that allowed() calls made inside the plugin hooks below + # are served from the cache + datasette = context.datasette + await datasette.allowed_many( + actions=[ + name + for name, action in datasette.actions.items() + if action.resource_class is TableResource + ], + resource=TableResource(context.database_name, context.table_name), + actor=context.request.actor, + ) + await datasette.allowed_many( + actions=[ + name + for name, action in datasette.actions.items() + if action.resource_class is DatabaseResource + ], + resource=DatabaseResource(context.database_name), + actor=context.request.actor, + ) for hook in method(**kwargs): extra_links = await await_me_maybe(hook) if extra_links: diff --git a/docs/internals.rst b/docs/internals.rst index f3c1152a..641286f8 100644 --- a/docs/internals.rst +++ b/docs/internals.rst @@ -512,6 +512,8 @@ Example usage: The method returns ``True`` if the permission is granted, ``False`` if denied. +Results are cached for the duration of the current request, so checking the same ``(actor, action, resource)`` combination twice within one request only does the underlying permission resolution work once. + .. _datasette_allowed_many: await .allowed_many(\*, actions, resource, actor=None) @@ -543,6 +545,8 @@ Example usage: ) # {"insert-row": True, "delete-row": True, "drop-table": False} +Each result is stored in the per-request permission check cache, so subsequent ``datasette.allowed()`` calls for the same checks within the same request are served from that cache. Datasette uses this before running the ``table_actions`` and ``database_actions`` plugin hooks: it resolves every registered table-level action against the current table and every database-level action against its database first, which means ``allowed()`` calls made by those plugin hooks are usually served from the cache instead of triggering additional queries. + Actions for which no plugin provides any permission rules are resolved to ``False`` directly, without being included in the SQL query at all. .. _datasette_allowed_resources: diff --git a/docs/plugin_hooks.rst b/docs/plugin_hooks.rst index 0a55f8ec..580f7402 100644 --- a/docs/plugin_hooks.rst +++ b/docs/plugin_hooks.rst @@ -1460,9 +1460,9 @@ plugin's source name (e.g., ``myplugin_user_id``). The system reserves these par This hook may be called for many actions in rapid succession - for example :ref:`datasette.allowed_many() ` gathers rules for every action in its batch -concurrently. Hook implementations must not assume that checks for different actions arrive one -page-render apart, and expensive work (such as network calls) should be cached independently of the -``action`` argument where possible. +concurrently before table and database pages render their action menus. Hook implementations must not +assume that checks for different actions arrive one page-render apart, and expensive work (such as +network calls) should be cached independently of the ``action`` argument where possible. You can also use return ``PermissionSQL.allow(reason="reason goes here")`` or ``PermissionSQL.deny(reason="reason goes here")`` as shortcuts for simple root-level allow or deny rules. These will create SQL snippets that look like this: diff --git a/tests/test_allowed_many.py b/tests/test_allowed_many.py index 53d0ffd9..3d0d0c9a 100644 --- a/tests/test_allowed_many.py +++ b/tests/test_allowed_many.py @@ -1,18 +1,52 @@ """ -Tests for the datasette.allowed_many() batch permission API, which -resolves multiple actions against one resource in a single internal -database query. datasette.allowed() is implemented on top of it, so -both entry points share one resolution code path. +Tests for request-scoped permission check memoization and the +datasette.allowed_many() batch permission API. + +Layer 1: per-request cache consulted by datasette.allowed() +Layer 2: allowed_many() resolves multiple actions in one internal-DB query +Layer 3: table/database views precompute all registered actions before + invoking table_actions/database_actions plugin hooks """ import pytest import pytest_asyncio from datasette.app import Datasette -from datasette.permissions import PermissionSQL, SkipPermissions +from datasette.permissions import ( + PermissionSQL, + SkipPermissions, + _permission_check_cache, +) from datasette.resources import DatabaseResource, TableResource from datasette import hookimpl +class CountingRulesPlugin: + """Counts permission_resources_sql gathers and grants rules for alice.""" + + def __init__(self): + self.calls = [] + + @hookimpl + def permission_resources_sql(self, datasette, actor, action): + actor_id = actor.get("id") if actor else None + self.calls.append((actor_id, action)) + if actor_id == "alice": + return PermissionSQL( + sql="SELECT NULL AS parent, NULL AS child, 1 AS allow, 'alice allowed' AS reason" + ) + return None + + def count(self, actor_id=None, action=None): + return len( + [ + (a, c) + for a, c in self.calls + if (actor_id is None or a == actor_id) + and (action is None or c == action) + ] + ) + + @pytest_asyncio.fixture async def ds(): ds = Datasette() @@ -24,6 +58,154 @@ async def ds(): return ds +@pytest_asyncio.fixture +async def counting_ds(ds): + plugin = CountingRulesPlugin() + ds.pm.register(plugin, name="counting") + try: + yield ds, plugin + finally: + ds.pm.unregister(name="counting") + + +# ---------------------------------------------------------------------- +# Layer 1: request-scoped memoization +# ---------------------------------------------------------------------- + + +@pytest.mark.asyncio +async def test_allowed_memoized_when_cache_active(counting_ds): + ds, plugin = counting_ds + resource = TableResource("analytics", "users") + token = _permission_check_cache.set({}) + try: + first = await ds.allowed( + action="view-table", resource=resource, actor={"id": "alice"} + ) + gathers_after_first = plugin.count(actor_id="alice", action="view-table") + assert gathers_after_first > 0 + second = await ds.allowed( + action="view-table", resource=resource, actor={"id": "alice"} + ) + assert first is True + assert second is True + # The second identical check must not gather hooks again + assert plugin.count(actor_id="alice", action="view-table") == ( + gathers_after_first + ) + finally: + _permission_check_cache.reset(token) + + +@pytest.mark.asyncio +async def test_allowed_not_memoized_without_cache(counting_ds): + ds, plugin = counting_ds + resource = TableResource("analytics", "users") + assert _permission_check_cache.get() is None + await ds.allowed(action="view-table", resource=resource, actor={"id": "alice"}) + first_count = plugin.count(actor_id="alice", action="view-table") + await ds.allowed(action="view-table", resource=resource, actor={"id": "alice"}) + # No request cache active - hooks gathered again + assert plugin.count(actor_id="alice", action="view-table") == first_count * 2 + + +@pytest.mark.asyncio +async def test_cache_keyed_on_full_actor_identity(counting_ds): + """Interleaved checks for different actors never share cache entries.""" + # Uses drop-table because default permissions deny it to non-root actors + ds, plugin = counting_ds + resource = TableResource("analytics", "users") + token = _permission_check_cache.set({}) + try: + assert ( + await ds.allowed( + action="drop-table", resource=resource, actor={"id": "alice"} + ) + is True + ) + assert ( + await ds.allowed( + action="drop-table", resource=resource, actor={"id": "bob"} + ) + is False + ) + # Repeat interleaved - cached results must stay correct per actor + assert ( + await ds.allowed( + action="drop-table", resource=resource, actor={"id": "alice"} + ) + is True + ) + assert ( + await ds.allowed( + action="drop-table", resource=resource, actor={"id": "bob"} + ) + is False + ) + # Actors differing in fields beyond id must not collide either + assert ( + await ds.allowed( + action="drop-table", + resource=resource, + actor={"id": "alice", "_r": {"a": []}}, + ) + is False + ) + finally: + _permission_check_cache.reset(token) + + +@pytest.mark.asyncio +async def test_cache_keyed_on_resource(counting_ds): + ds, plugin = counting_ds + token = _permission_check_cache.set({}) + try: + await ds.allowed( + action="view-table", + resource=TableResource("analytics", "users"), + actor={"id": "alice"}, + ) + count = plugin.count(actor_id="alice", action="view-table") + # Different resource - must not be served from cache + await ds.allowed( + action="view-table", + resource=TableResource("analytics", "events"), + actor={"id": "alice"}, + ) + assert plugin.count(actor_id="alice", action="view-table") == count * 2 + finally: + _permission_check_cache.reset(token) + + +@pytest.mark.asyncio +async def test_skip_permission_checks_bypasses_cache(counting_ds): + ds, plugin = counting_ds + resource = TableResource("analytics", "users") + token = _permission_check_cache.set({}) + try: + with SkipPermissions(): + assert ( + await ds.allowed( + action="drop-table", resource=resource, actor={"id": "bob"} + ) + is True + ) + # The skip-mode True must not have been cached + assert ( + await ds.allowed( + action="drop-table", resource=resource, actor={"id": "bob"} + ) + is False + ) + finally: + _permission_check_cache.reset(token) + + +# ---------------------------------------------------------------------- +# Layer 2: allowed_many() +# ---------------------------------------------------------------------- + + class MatrixRulesPlugin: """Different rules per action for actor carol, to exercise resolution.""" @@ -233,7 +415,7 @@ class ParamCollisionPlugin: @pytest.mark.asyncio async def test_allowed_many_namespaces_params_across_actions(ds): - """Many actions whose rules use identical param names must not collide.""" + """40+ actions whose rules use identical param names must not collide.""" plugin = ParamCollisionPlugin() ds.pm.register(plugin, name="collision") try: @@ -330,6 +512,24 @@ async def test_allowed_many_global_actions_without_resource(ds): assert anon == {"permissions-debug": False} +@pytest.mark.asyncio +async def test_allowed_many_seeds_request_cache(counting_ds): + ds, plugin = counting_ds + resource = TableResource("analytics", "users") + actions = ["view-table", "insert-row", "drop-table"] + token = _permission_check_cache.set({}) + try: + await ds.allowed_many(actions=actions, resource=resource, actor={"id": "alice"}) + gathers = plugin.count(actor_id="alice") + assert gathers > 0 + for action in actions: + await ds.allowed(action=action, resource=resource, actor={"id": "alice"}) + # Every allowed() call must have been served from the seeded cache + assert plugin.count(actor_id="alice") == gathers + finally: + _permission_check_cache.reset(token) + + @pytest.mark.asyncio async def test_allowed_many_skip_permission_checks(ds): with SkipPermissions(): @@ -339,3 +539,131 @@ async def test_allowed_many_skip_permission_checks(ds): actor=None, ) assert results == {"view-table": True, "drop-table": True} + + +# ---------------------------------------------------------------------- +# Layer 3: precompute before table_actions / database_actions hooks +# ---------------------------------------------------------------------- + + +class ActionHooksPlugin: + """Plugin hooks that make allowed() checks, like real action plugins do.""" + + @hookimpl + def table_actions(self, datasette, actor, database, table): + async def inner(): + links = [] + if await datasette.allowed( + action="drop-table", + resource=TableResource(database, table), + actor=actor, + ): + links.append( + {"href": "/drop", "label": "Drop this table (test-plugin)"} + ) + if await datasette.allowed( + action="create-table", + resource=DatabaseResource(database), + actor=actor, + ): + links.append( + {"href": "/create", "label": "Create a table (test-plugin)"} + ) + return links + + return inner + + @hookimpl + def database_actions(self, datasette, actor, database): + async def inner(): + if await datasette.allowed( + action="create-table", + resource=DatabaseResource(database), + actor=actor, + ): + return [{"href": "/create", "label": "Create a table (test-plugin)"}] + return [] + + return inner + + +@pytest_asyncio.fixture +async def spying_ds(ds, monkeypatch): + """ds with the ActionHooksPlugin plus a spy recording every batch of + actions sent to check_permissions_for_actions.""" + from datasette.utils import actions_sql + + plugin = ActionHooksPlugin() + ds.pm.register(plugin, name="action_hooks") + ds.root_enabled = True + recorded = [] + original = actions_sql.check_permissions_for_actions + + async def spy(**kwargs): + recorded.append(kwargs["actions"]) + return await original(**kwargs) + + monkeypatch.setattr(actions_sql, "check_permissions_for_actions", spy) + try: + yield ds, recorded + finally: + ds.pm.unregister(name="action_hooks") + + +@pytest.mark.asyncio +async def test_table_page_precomputes_action_permissions(spying_ds): + ds, recorded = spying_ds + cookies = {"ds_actor": ds.client.actor_cookie({"id": "root"})} + response = await ds.client.get("/analytics/users", cookies=cookies) + assert response.status_code == 200 + # The plugin's permission checks were served from the precomputed batch + assert "Drop this table (test-plugin)" in response.text + assert "Create a table (test-plugin)" in response.text + # One batch covered the table-level actions for the table resource, + # and one covered the database-level actions for the database resource + batches = [batch for batch in recorded if len(batch) > 1] + assert any("drop-table" in batch for batch in batches) + assert any("create-table" in batch for batch in batches) + # The precompute is scoped to actions relevant to each resource: + # no global or query-level actions in any batch, and no mixing of + # table-level and database-level actions + for batch in batches: + assert "view-instance" not in batch + assert "view-query" not in batch + assert not ("drop-table" in batch and "create-table" in batch) + # The hook's own allowed() calls hit the cache - no single-action + # fallback queries for the actions it checked + assert ["drop-table"] not in recorded + assert ["create-table"] not in recorded + + +@pytest.mark.asyncio +async def test_database_page_precomputes_action_permissions(spying_ds): + ds, recorded = spying_ds + cookies = {"ds_actor": ds.client.actor_cookie({"id": "root"})} + response = await ds.client.get("/analytics", cookies=cookies) + assert response.status_code == 200 + assert "Create a table (test-plugin)" in response.text + batches = [batch for batch in recorded if len(batch) > 1] + assert any("create-table" in batch for batch in batches) + # Scoped to database-level actions only + for batch in batches: + assert "view-instance" not in batch + assert "drop-table" not in batch + assert ["create-table"] not in recorded + + +@pytest.mark.asyncio +async def test_cache_does_not_leak_across_requests(counting_ds): + ds, plugin = counting_ds + cookies = {"ds_actor": ds.client.actor_cookie({"id": "alice"})} + response = await ds.client.get("/analytics/users.json", cookies=cookies) + assert response.status_code == 200 + first_request_gathers = plugin.count(actor_id="alice", action="view-table") + assert first_request_gathers > 0 + response = await ds.client.get("/analytics/users.json", cookies=cookies) + assert response.status_code == 200 + # Second request must re-gather (fresh cache), not reuse the first one + assert ( + plugin.count(actor_id="alice", action="view-table") == first_request_gathers * 2 + ) From d4cb8b464bf1cbe69a8921fc8c9315e04a5f49cb Mon Sep 17 00:00:00 2001 From: Simon Willison Date: Fri, 12 Jun 2026 13:21:58 -0700 Subject: [PATCH 06/90] Fix for trace_child_tasks exception handling I had Claude Fable 5 review our use of contextvar and it spotted this place where exceptions were not correctly handled. --- datasette/tracer.py | 6 ++++-- tests/test_tracer.py | 13 +++++++++++++ 2 files changed, 17 insertions(+), 2 deletions(-) diff --git a/datasette/tracer.py b/datasette/tracer.py index 9e66613b..28f3cc09 100644 --- a/datasette/tracer.py +++ b/datasette/tracer.py @@ -27,8 +27,10 @@ def get_task_id(): @contextmanager def trace_child_tasks(): token = trace_task_id.set(get_task_id()) - yield - trace_task_id.reset(token) + try: + yield + finally: + trace_task_id.reset(token) @contextmanager diff --git a/tests/test_tracer.py b/tests/test_tracer.py index 6cc80fc4..9db211d3 100644 --- a/tests/test_tracer.py +++ b/tests/test_tracer.py @@ -70,6 +70,19 @@ def test_trace_query_errors(): assert trace_info["traces"][-1]["error"] == "no such table: non_existent_table" +@pytest.mark.asyncio +async def test_trace_child_tasks_resets_contextvar_on_exception(): + from datasette import tracer + + before = tracer.trace_task_id.get() + with pytest.raises(ValueError): + with tracer.trace_child_tasks(): + assert tracer.trace_task_id.get() is not None + raise ValueError("simulated error") + # The contextvar must be reset even though the block raised + assert tracer.trace_task_id.get() == before + + def test_trace_parallel_queries(): with make_app_client(settings={"trace_debug": True}) as client: response = client.get("/parallel-queries?_trace=1") From 86334d233dd2e668169e54fb2312cae2705e7ffc Mon Sep 17 00:00:00 2001 From: Simon Willison Date: Sat, 13 Jun 2026 11:09:28 -0700 Subject: [PATCH 07/90] Switch to CTE to handle 600+ actions at once GPT-5.5 xhigh in Codex spotted this problem and fixed it with a CTE: https://gisthost.github.io/?46076499ee685acddc988ff6b47a74b0 --- datasette/utils/actions_sql.py | 15 ++++++++++---- tests/test_allowed_many.py | 38 ++++++++++++++++++++++++++++++++++ 2 files changed, 49 insertions(+), 4 deletions(-) diff --git a/datasette/utils/actions_sql.py b/datasette/utils/actions_sql.py index a422c1ed..c7137e6b 100644 --- a/datasette/utils/actions_sql.py +++ b/datasette/utils/actions_sql.py @@ -555,7 +555,7 @@ async def check_permissions_for_actions( params = {"_check_parent": parent, "_check_child": child} ctes = [] - selects = [] + result_rows = [] verdicts = {} for i, (action, permission_sqls) in enumerate(zip(unique_actions, gathered)): @@ -627,10 +627,17 @@ async def check_permissions_for_actions( AND (r.child = :_check_child OR r.child IS NULL) )""" - selects.append(f"SELECT {i} AS action_idx, ({verdict_sql}) AS is_allowed") + result_rows.append(f"({i}, ({verdict_sql}))") - if selects: - query = "WITH\n" + ",\n".join(ctes) + "\n" + "\nUNION ALL\n".join(selects) + if result_rows: + ctes.append( + "results(action_idx, is_allowed) AS (VALUES\n" + + ",\n".join(result_rows) + + "\n)" + ) + query = ( + "WITH\n" + ",\n".join(ctes) + "\nSELECT action_idx, is_allowed FROM results" + ) result = await datasette.get_internal_database().execute(query, params) for row in result.rows: verdicts[unique_actions[row[0]]] = bool(row[1]) diff --git a/tests/test_allowed_many.py b/tests/test_allowed_many.py index 3d0d0c9a..08b952fb 100644 --- a/tests/test_allowed_many.py +++ b/tests/test_allowed_many.py @@ -12,6 +12,7 @@ import pytest import pytest_asyncio from datasette.app import Datasette from datasette.permissions import ( + Action, PermissionSQL, SkipPermissions, _permission_check_cache, @@ -541,6 +542,43 @@ async def test_allowed_many_skip_permission_checks(ds): assert results == {"view-table": True, "drop-table": True} +class ManyActionsPlugin: + """Registers enough actions to exceed SQLite's compound SELECT limit.""" + + def __init__(self, count): + self.action_names = [f"bulk-action-{i}" for i in range(count)] + self.action_names_set = set(self.action_names) + + @hookimpl + def register_actions(self, datasette): + return [ + Action(name=name, abbr=None, description="Bulk test action") + for name in self.action_names + ] + + @hookimpl + def permission_resources_sql(self, datasette, actor, action): + if action in self.action_names_set: + return PermissionSQL( + sql="SELECT NULL AS parent, NULL AS child, 1 AS allow, 'bulk allow' AS reason", + params={}, + ) + + +@pytest.mark.asyncio +async def test_allowed_many_more_than_sqlite_compound_select_limit(): + plugin = ManyActionsPlugin(600) + ds = Datasette() + ds.pm.register(plugin, name="many_actions") + try: + await ds.invoke_startup() + results = await ds.allowed_many(actions=plugin.action_names, actor=None) + assert len(results) == 600 + assert all(results.values()) + finally: + ds.pm.unregister(name="many_actions") + + # ---------------------------------------------------------------------- # Layer 3: precompute before table_actions / database_actions hooks # ---------------------------------------------------------------------- From ab19b0382bc560ef7ae511f899aa7051935577af Mon Sep 17 00:00:00 2001 From: Simon Willison Date: Sat, 13 Jun 2026 11:12:31 -0700 Subject: [PATCH 08/90] Removed note from permission_resources_sql Refs https://github.com/simonw/datasette/pull/2775/changes#r3408385197 --- docs/plugin_hooks.rst | 6 ------ 1 file changed, 6 deletions(-) diff --git a/docs/plugin_hooks.rst b/docs/plugin_hooks.rst index 580f7402..2a0ddc93 100644 --- a/docs/plugin_hooks.rst +++ b/docs/plugin_hooks.rst @@ -1458,12 +1458,6 @@ to avoid conflicts with other plugins. The recommended convention is to prefix p plugin's source name (e.g., ``myplugin_user_id``). The system reserves these parameter names: ``:actor``, ``:actor_id``, ``:action``, and ``:filter_parent``. -This hook may be called for many actions in rapid succession - for example -:ref:`datasette.allowed_many() ` gathers rules for every action in its batch -concurrently before table and database pages render their action menus. Hook implementations must not -assume that checks for different actions arrive one page-render apart, and expensive work (such as -network calls) should be cached independently of the ``action`` argument where possible. - You can also use return ``PermissionSQL.allow(reason="reason goes here")`` or ``PermissionSQL.deny(reason="reason goes here")`` as shortcuts for simple root-level allow or deny rules. These will create SQL snippets that look like this: .. code-block:: sql From f2927a164746a1a2da3e14680948bfbdddfd626b Mon Sep 17 00:00:00 2001 From: Simon Willison Date: Sat, 13 Jun 2026 11:15:47 -0700 Subject: [PATCH 09/90] Fix for gen.throw(*sys.exc_info()) warning Closes #2776 --- datasette/database.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/datasette/database.py b/datasette/database.py index 6cd5d11e..e7fe1ed9 100644 --- a/datasette/database.py +++ b/datasette/database.py @@ -829,10 +829,10 @@ def _apply_write_wrapper(fn, wrapper_factory, track_event): # Execute the actual write try: result = fn(conn) - except Exception: + except Exception as e: # Throw exception into generator so it can handle it try: - gen.throw(*sys.exc_info()) + gen.throw(e) except StopIteration: pass # Re-raise the original exception From e3a1f190572708b422457e2d50eb031f85ba4f52 Mon Sep 17 00:00:00 2001 From: Simon Willison Date: Sat, 13 Jun 2026 14:19:39 -0700 Subject: [PATCH 10/90] Edit/delete icon prototype --- datasette/static/app.css | 55 +++++++++++++++++++++++++++++++++ datasette/views/table.py | 67 +++++++++++++++++++++++++++++++++++----- 2 files changed, 115 insertions(+), 7 deletions(-) diff --git a/datasette/static/app.css b/datasette/static/app.css index 6d675d9f..ec3a85fb 100644 --- a/datasette/static/app.css +++ b/datasette/static/app.css @@ -1192,6 +1192,50 @@ dialog.set-column-type-dialog::backdrop { cursor: wait; } +.row-link-with-actions { + display: inline-flex; + align-items: center; + gap: 0.4rem; + flex-wrap: wrap; +} + +.row-inline-actions { + display: inline-flex; + gap: 0.2rem; + align-items: center; +} + +.row-inline-action { + appearance: none; + border: 1px solid rgba(74, 85, 104, 0.24); + background: transparent; + color: #4a5568; + border-radius: 4px; + cursor: pointer; + display: inline-grid; + place-items: center; + min-height: 24px; + min-width: 24px; + padding: 2px; + position: relative; +} + +.row-inline-action:hover, +.row-inline-action:focus { + background: rgba(74, 85, 104, 0.07); +} + +.row-inline-action:focus { + outline: 3px solid #b3d4ff; + outline-offset: 1px; +} + +.row-inline-action-icon { + display: block; + height: 13px; + width: 13px; +} + @media (max-width: 640px) { dialog.mobile-column-actions-dialog { width: 95vw; @@ -1239,6 +1283,17 @@ dialog.set-column-type-dialog::backdrop { padding-left: 18px; padding-right: 18px; } + + .row-inline-action { + min-height: 30px; + min-width: 30px; + padding: 4px; + } + + .row-inline-action-icon { + height: 14px; + width: 14px; + } } @media only screen and (max-width: 576px) { diff --git a/datasette/views/table.py b/datasette/views/table.py index 65388c9c..49238ff4 100644 --- a/datasette/views/table.py +++ b/datasette/views/table.py @@ -194,6 +194,13 @@ async def display_columns_and_rows( pks_for_display = pks if not pks_for_display: pks_for_display = ["rowid"] + row_action_permissions = {} + if link_column and request is not None and db.is_mutable: + row_action_permissions = await datasette.allowed_many( + actions=["update-row", "delete-row"], + resource=TableResource(database=database_name, table=table_name), + actor=request.actor, + ) columns = [] for r in description: @@ -233,19 +240,65 @@ async def display_columns_and_rows( if link_column: is_special_link_column = len(pks) != 1 pk_path = path_from_row_pks(row, pks, not pks, False) + row_path = path_from_row_pks(row, pks, not pks) + row_link = '{flat_pks}'.format( + table_path=datasette.urls.table(database_name, table_name), + flat_pks=str(markupsafe.escape(pk_path)), + flat_pks_quoted=row_path, + ) + edit_icon = ( + '" + ) + delete_icon = ( + '" + ) + row_actions = [] + if row_action_permissions.get("update-row"): + row_actions.append( + '".format( + edit_icon=edit_icon, + row_label=markupsafe.escape(pk_path), + ) + ) + if row_action_permissions.get("delete-row"): + row_actions.append( + '".format( + delete_icon=delete_icon, + row_label=markupsafe.escape(pk_path), + ) + ) + if row_actions: + row_link = ( + '{row_link}' + '' + "{row_actions}" + ).format(row_link=row_link, row_actions="".join(row_actions)) cells.append( { "column": pks[0] if len(pks) == 1 else "Link", "value_type": "pk", "is_special_link_column": is_special_link_column, "raw": pk_path, - "value": markupsafe.Markup( - '{flat_pks}'.format( - table_path=datasette.urls.table(database_name, table_name), - flat_pks=str(markupsafe.escape(pk_path)), - flat_pks_quoted=path_from_row_pks(row, pks, not pks), - ) - ), + "value": markupsafe.Markup(row_link), } ) From de5f72dd8847be184f5283491a4df38c63f823bd Mon Sep 17 00:00:00 2001 From: Simon Willison Date: Sat, 13 Jun 2026 14:38:49 -0700 Subject: [PATCH 11/90] hash busting thing on table.js Addd for Codex Desktop, which has real trouble with edits to files like this as the in-app browser does not seem to have a cache-busting reload option. --- datasette/app.py | 1 + datasette/templates/table.html | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/datasette/app.py b/datasette/app.py index 9979b6c5..4931f486 100644 --- a/datasette/app.py +++ b/datasette/app.py @@ -2312,6 +2312,7 @@ class Datasette: and "ds_actor" in request.cookies and request.actor, "app_css_hash": self.app_css_hash(), + "table_js_hash": self.static_hash("table.js"), "zip": zip, "body_scripts": body_scripts, "format_bytes": format_bytes, diff --git a/datasette/templates/table.html b/datasette/templates/table.html index c841e1be..4dc908e0 100644 --- a/datasette/templates/table.html +++ b/datasette/templates/table.html @@ -5,7 +5,7 @@ {% block extra_head %} {{- super() -}} - +