From 06af34240f25f1bcdbc9a3c180f0955b60dbdf57 Mon Sep 17 00:00:00 2001 From: Simon Willison Date: Thu, 23 Oct 2025 14:53:07 -0700 Subject: [PATCH] Fix permission endpoint tests by resolving method signature conflicts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Renamed internal allowed_resources_sql() to _build_permission_rules_sql() to avoid conflict with public method - Made public allowed_resources_sql() keyword-only to prevent argument order bugs - Fixed PermissionRulesView to use _build_permission_rules_sql() which returns full permission rules (with allow/deny) instead of filtered resources - Fixed _build_permission_rules_sql() to pass actor dict to build_rules_union() - Added actor_id extraction in AllowedResourcesView - Added root_enabled=True to test fixture to grant permissions-debug to root user All 51 tests in test_permission_endpoints.py now pass. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude --- datasette/app.py | 15 ++++++++------- datasette/views/special.py | 3 ++- tests/test_permission_endpoints.py | 1 + 3 files changed, 11 insertions(+), 8 deletions(-) diff --git a/datasette/app.py b/datasette/app.py index ce3ac337..892d667a 100644 --- a/datasette/app.py +++ b/datasette/app.py @@ -1066,12 +1066,13 @@ class Datasette: ) return result - async def allowed_resources_sql( + async def _build_permission_rules_sql( self, actor: dict | None, action: str ) -> tuple[str, dict]: """Combine permission_resources_sql PermissionSQL blocks into a UNION query. Returns a (sql, params) tuple suitable for execution against SQLite. + Internal helper for permission_allowed_2. """ plugin_blocks: List[PermissionSQL] = [] for block in pm.hook.permission_resources_sql( @@ -1093,9 +1094,8 @@ class Datasette: continue plugin_blocks.append(candidate) - actor_id = actor.get("id") if actor else None sql, params = build_rules_union( - actor=str(actor_id) if actor_id is not None else "", + actor=actor, plugins=plugin_blocks, ) return sql, params @@ -1123,7 +1123,7 @@ class Datasette: elif resource is not None: raise TypeError("resource must be None, str, or (parent, child) tuple") - union_sql, union_params = await self.allowed_resources_sql(actor_dict, action) + union_sql, union_params = await self._build_permission_rules_sql(actor_dict, action) query = f""" WITH rules AS ( @@ -1275,6 +1275,7 @@ class Datasette: async def allowed_resources_sql( self, + *, action: str, actor: dict | None = None, ) -> tuple[str, dict]: @@ -1285,7 +1286,7 @@ class Datasette: The query returns rows with (parent, child, reason) columns. Example: - query, params = await datasette.allowed_resources_sql("view-table", actor) + query, params = await datasette.allowed_resources_sql(action="view-table", actor=actor) result = await datasette.get_internal_database().execute(query, params) """ from datasette.utils.actions_sql import build_allowed_resources_sql @@ -1318,7 +1319,7 @@ class Datasette: if not action_obj: raise ValueError(f"Unknown action: {action}") - query, params = await self.allowed_resources_sql(action, actor) + query, params = await self.allowed_resources_sql(action=action, actor=actor) result = await self.get_internal_database().execute(query, params) # Instantiate the appropriate Resource subclass for each row @@ -1355,7 +1356,7 @@ class Datasette: if not action_obj: raise ValueError(f"Unknown action: {action}") - query, params = await self.allowed_resources_sql(action, actor) + query, params = await self.allowed_resources_sql(action=action, actor=actor) result = await self.get_internal_database().execute(query, params) resource_class = action_obj.resource_class diff --git a/datasette/views/special.py b/datasette/views/special.py index fd80bb5b..5f4b541c 100644 --- a/datasette/views/special.py +++ b/datasette/views/special.py @@ -238,6 +238,7 @@ class AllowedResourcesView(BaseView): ) actor = request.actor if isinstance(request.actor, dict) else None + actor_id = actor.get("id") if actor else None parent_filter = request.args.get("parent") child_filter = request.args.get("child") if child_filter and not parent_filter: @@ -424,7 +425,7 @@ class PermissionRulesView(BaseView): page_size = max_page_size offset = (page - 1) * page_size - union_sql, union_params = await self.ds.allowed_resources_sql(actor, action) + union_sql, union_params = await self.ds._build_permission_rules_sql(actor, action) await self.ds.refresh_schemas() db = self.ds.get_internal_database() diff --git a/tests/test_permission_endpoints.py b/tests/test_permission_endpoints.py index 3952259e..0210fb95 100644 --- a/tests/test_permission_endpoints.py +++ b/tests/test_permission_endpoints.py @@ -30,6 +30,7 @@ async def ds_with_permissions(): } } ) + ds.root_enabled = True await ds.invoke_startup() # Add some test databases ds.add_memory_database("content")