From e1582c14246b289074fe7682f18a7750f1d243a7 Mon Sep 17 00:00:00 2001 From: Simon Willison Date: Fri, 24 Oct 2025 14:35:23 -0700 Subject: [PATCH] Fix actor restrictions to work with new actions system MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Updated restrictions_allow_action() to use datasette.actions instead of datasette.permissions - Changed references from Permission to Action objects - Updated takes_database checks to takes_parent - Added get_action() method to Datasette class for looking up actions by name or abbreviation - Integrated actor restriction checking into allowed() method - Actor restrictions (_r in actor dict) are now properly enforced after SQL permission checks This fixes tests in test_api_write.py where actors with restricted permissions were incorrectly being granted access to actions outside their restrictions. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude --- datasette/app.py | 29 +++++++++++++++++++++++++++++ datasette/default_permissions.py | 18 +++++++++--------- 2 files changed, 38 insertions(+), 9 deletions(-) diff --git a/datasette/app.py b/datasette/app.py index fb41c284..473ea488 100644 --- a/datasette/app.py +++ b/datasette/app.py @@ -547,6 +547,18 @@ class Datasette: "No permission found with name or abbreviation {}".format(name_or_abbr) ) + def get_action(self, name_or_abbr: str): + """ + Returns an Action object for the given name or abbreviation. Returns None if not found. + """ + if name_or_abbr in self.actions: + return self.actions[name_or_abbr] + # Try abbreviation + for action in self.actions.values(): + if action.abbr == name_or_abbr: + return action + return None + async def refresh_schemas(self): if self._refresh_schemas_lock.locked(): return @@ -1282,6 +1294,23 @@ class Datasette: child=resource.child, ) + # Check actor restrictions after SQL permissions + # If the SQL check says "yes" but actor has restrictions, verify action is allowed + if result and actor and "_r" in actor: + from datasette.default_permissions import restrictions_allow_action + + # Convert Resource to old-style format for restrictions check + if resource.parent and resource.child: + old_style_resource = (resource.parent, resource.child) + elif resource.parent: + old_style_resource = resource.parent + else: + old_style_resource = None + + # If restrictions don't allow this action, deny it + if not restrictions_allow_action(self, actor["_r"], action, old_style_resource): + result = False + # Log the permission check for debugging # Convert Resource to old-style format for backward compatibility with debug tools if resource.parent and resource.child: diff --git a/datasette/default_permissions.py b/datasette/default_permissions.py index 19d0e4f8..04993a68 100644 --- a/datasette/default_permissions.py +++ b/datasette/default_permissions.py @@ -306,20 +306,20 @@ def restrictions_allow_action( "Do these restrictions allow the requested action against the requested resource?" if action == "view-instance": # Special case for view-instance: it's allowed if the restrictions include any - # permissions that have the implies_can_view=True flag set + # actions that have the implies_can_view=True flag set all_rules = restrictions.get("a") or [] for database_rules in (restrictions.get("d") or {}).values(): all_rules += database_rules for database_resource_rules in (restrictions.get("r") or {}).values(): for resource_rules in database_resource_rules.values(): all_rules += resource_rules - permissions = [datasette.get_permission(action) for action in all_rules] - if any(p for p in permissions if p.implies_can_view): + actions = [datasette.get_action(action) for action in all_rules] + if any(a for a in actions if a and a.implies_can_view): return True if action == "view-database": # Special case for view-database: it's allowed if the restrictions include any - # permissions that have the implies_can_view=True flag set AND takes_database + # actions that have the implies_can_view=True flag set AND takes_parent all_rules = restrictions.get("a") or [] database_rules = list((restrictions.get("d") or {}).get(resource) or []) all_rules += database_rules @@ -327,15 +327,15 @@ def restrictions_allow_action( for resource_rules in (restrictions.get("r") or {}).values(): for table_rules in resource_rules.values(): all_rules += table_rules - permissions = [datasette.get_permission(action) for action in all_rules] - if any(p for p in permissions if p.implies_can_view and p.takes_database): + actions = [datasette.get_action(action) for action in all_rules] + if any(a for a in actions if a and a.implies_can_view and a.takes_parent): return True # Does this action have an abbreviation? to_check = {action} - permission = datasette.permissions.get(action) - if permission and permission.abbr: - to_check.add(permission.abbr) + action_obj = datasette.actions.get(action) + if action_obj and action_obj.abbr: + to_check.add(action_obj.abbr) # If restrictions is defined then we use those to further restrict the actor # Crucially, we only use this to say NO (return False) - we never