From 8be1384279894900281089e96a3b560b862d3687 Mon Sep 17 00:00:00 2001
From: makiaveli1 <makiaveli1@users.noreply.github.com>
Date: Wed, 30 Sep 2026 16:59:40 +0100
Subject: [PATCH] Fix lost actor/action params for plugin permission rules

gather_permission_sql_from_hooks() adds :actor, :actor_id and :action
defaults to each PermissionSQL returned by the permission_resources_sql
hook, but "params = permission_sql.params or {}" built a throwaway dict
whenever a rule passed no params of its own (None or {}), so those
defaults were silently discarded.

The anonymous pass in build_allowed_resources_sql() rewrites plugin SQL by
iterating over permission_sql.params and renaming :key to :anon_key. With
no params attached, a rule that references :actor is never rewritten, so
the anonymous branch binds :actor to the current actor instead of NULL.
A table that anonymous users cannot view is then reported as
is_private=0, and its deny/allow reason is computed for the wrong actor.
---
 datasette/utils/permissions.py        |  4 +-
 tests/test_permission_actor_params.py | 77 +++++++++++++++++++++++++++
 2 files changed, 80 insertions(+), 1 deletion(-)
 create mode 100644 tests/test_permission_actor_params.py

diff --git a/datasette/utils/permissions.py b/datasette/utils/permissions.py
index 5a8ee8e2..3b7036b8 100644
--- a/datasette/utils/permissions.py
+++ b/datasette/utils/permissions.py
@@ -46,7 +46,9 @@ async def gather_permission_sql_from_hooks(
         for permission_sql in _iter_permission_sql_from_result(resolved, action=action):
             if not permission_sql.source:
                 permission_sql.source = default_source
-            params = permission_sql.params or {}
+            if permission_sql.params is None:
+                permission_sql.params = {}
+            params = permission_sql.params
             params.setdefault("action", action)
             params.setdefault("actor", actor_json)
             params.setdefault("actor_id", actor_id)
diff --git a/tests/test_permission_actor_params.py b/tests/test_permission_actor_params.py
new file mode 100644
index 00000000..0a4ef589
--- /dev/null
+++ b/tests/test_permission_actor_params.py
@@ -0,0 +1,77 @@
+import pytest
+import pytest_asyncio
+
+from datasette import hookimpl
+from datasette.app import Datasette
+from datasette.permissions import PermissionSQL
+from datasette.utils.permissions import gather_permission_sql_from_hooks
+
+# A rule that depends on who is asking, and which passes no params of its own
+ALICE_ONLY_RULE = """
+select 'data' as parent, 'secret' as child,
+       case when json_extract(:actor, '$.id') = 'alice' then 1 else 0 end as allow,
+       'alice only' as reason
+"""
+
+
+class ActorAwarePlugin:
+    @hookimpl
+    def permission_resources_sql(self, datasette, actor, action):
+        if action == "view-table":
+            return PermissionSQL(sql=ALICE_ONLY_RULE)
+
+
+@pytest_asyncio.fixture
+async def ds_with_actor_aware_rule():
+    ds = Datasette(memory=True)
+    ds.pm.register(ActorAwarePlugin(), name="actor-aware")
+    await ds.invoke_startup()
+    db = ds.add_memory_database("data")
+    await db.execute_write("create table secret (id integer primary key)")
+    await db.execute_write("create table open (id integer primary key)")
+    await ds._refresh_schemas()
+    try:
+        yield ds
+    finally:
+        ds.pm.unregister(name="actor-aware")
+
+
+@pytest.mark.asyncio
+async def test_gather_permission_sql_from_hooks_attaches_defaults(
+    ds_with_actor_aware_rule,
+):
+    "actor, actor_id and action should be available to rules that pass no params"
+    permission_sqls = await gather_permission_sql_from_hooks(
+        datasette=ds_with_actor_aware_rule,
+        actor={"id": "alice"},
+        action="view-table",
+    )
+    ours = [p for p in permission_sqls if p.sql == ALICE_ONLY_RULE]
+    assert len(ours) == 1
+    assert ours[0].params == {
+        "action": "view-table",
+        "actor": '{"id": "alice"}',
+        "actor_id": "alice",
+    }
+
+
+@pytest.mark.asyncio
+async def test_anonymous_rules_are_evaluated_as_the_anonymous_actor(
+    ds_with_actor_aware_rule,
+):
+    "Rules that reference :actor must see NULL in the anonymous pass, not the current actor"
+    ds = ds_with_actor_aware_rule
+    resources = await ds.allowed_resources_sql(
+        action="view-table", actor={"id": "alice"}, include_is_private=True
+    )
+    # The anonymous pass renames :actor to :anon_actor so it cannot collide with
+    # the parameter used for the current actor
+    assert resources.params["anon_actor"] is None
+    assert resources.params["anon_action"] == "view-table"
+
+    rows = await ds.get_internal_database().execute(resources.sql, resources.params)
+    by_child = {row["child"]: dict(row) for row in rows}
+    # alice can view secret, anonymous cannot - so it is a private resource
+    assert by_child["secret"]["is_private"] == 1
+    # open is visible to everyone
+    assert by_child["open"]["is_private"] == 0