Remove automatic parameter namespacing from permission plugins

Simplifies the permission system by removing automatic parameter namespacing.
Plugins are now responsible for using unique parameter names. The recommended
convention is to prefix parameters with the plugin source name (e.g.,
:myplugin_user_id). System reserves :actor, :actor_id, :action, :filter_parent.

- Remove _namespace_params() function from datasette/utils/permissions.py
- Update build_rules_union() to use plugin params directly
- Document parameter naming convention in plugin_hooks.rst
- Update example plugins to use prefixed parameters
- Add test_multiple_plugins_with_own_parameters() to verify convention works

datasette/utils/permissions.py

@@ -13,49 +13,33 @@ from datasette.permissions import PermissionSQL
 # -----------------------------
 
 
-def _namespace_params(i: int, params: Dict[str, Any]) -> Tuple[str, Dict[str, Any]]:
-    """
-    Rewrite parameter placeholders to distinct names per plugin block.
-    Returns (rewritten_sql, namespaced_params).
-    """
-
-    replacements = {key: f"{key}_{i}" for key in params.keys()}
-
-    def rewrite(s: str) -> str:
-        for key in sorted(replacements.keys(), key=len, reverse=True):
-            s = s.replace(f":{key}", f":{replacements[key]}")
-        return s
-
-    namespaced: Dict[str, Any] = {}
-    for key, value in params.items():
-        namespaced[replacements[key]] = value
-    return rewrite, namespaced
-
-
 def build_rules_union(
     actor: dict | None, plugins: Sequence[PermissionSQL]
 ) -> Tuple[str, Dict[str, Any]]:
     """
-    Compose plugin SQL into a UNION ALL with namespaced parameters.
+    Compose plugin SQL into a UNION ALL.
 
     Returns:
       union_sql: a SELECT with columns (parent, child, allow, reason, source_plugin)
-      params:    dict of bound parameters including :actor (JSON), :actor_id, and namespaced plugin params
+      params:    dict of bound parameters including :actor (JSON), :actor_id, and plugin params
+
+    Note: Plugins are responsible for ensuring their parameter names don't conflict.
+    The system reserves these parameter names: :actor, :actor_id, :action, :filter_parent
+    Plugin parameters should be prefixed with a unique identifier (e.g., source name).
     """
     parts: List[str] = []
     actor_json = json.dumps(actor) if actor else None
     actor_id = actor.get("id") if actor else None
     params: Dict[str, Any] = {"actor": actor_json, "actor_id": actor_id}
 
-    for i, p in enumerate(plugins):
-        rewrite, ns_params = _namespace_params(i, p.params)
-        sql_block = rewrite(p.sql)
-        params.update(ns_params)
+    for p in plugins:
+        # No namespacing - just use plugin params as-is
+        params.update(p.params)
 
         parts.append(
             f"""
             SELECT parent, child, allow, reason, '{p.source}' AS source_plugin FROM (
-                {sql_block}
+                {p.sql}
             )
             """.strip()
         )

docs/plugin_hooks.rst

@@ -1461,8 +1461,12 @@ Return value
 
 Datasette's action-based permission resolver calls this hook to gather SQL rows describing which
 resources an actor may access (``allow = 1``) or should be denied (``allow = 0``) for a specific action.
-Each SQL snippet should return ``parent``, ``child``, ``allow`` and ``reason`` columns. Any bound parameters
-supplied via ``PluginSQL.params`` are automatically namespaced per plugin.
+Each SQL snippet should return ``parent``, ``child``, ``allow`` and ``reason`` columns.
+
+**Parameter naming convention:** Plugin parameters in ``PluginSQL.params`` should use unique names
+to avoid conflicts with other plugins. The recommended convention is to prefix parameters with your
+plugin's source name (e.g., ``myplugin_user_id``). The system reserves these parameter names:
+``:actor``, ``:actor_id``, ``:action``, and ``:filter_parent``.
 
 
 Permission plugin examples
@@ -1531,10 +1535,10 @@ will pass through to the SQL snippet.
                     1 AS allow,
                     'execute-sql allowed for analytics_*' AS reason
                 FROM catalog_databases
-                WHERE database_name LIKE :prefix
+                WHERE database_name LIKE :analytics_prefix
             """,
             params={
-                "prefix": "analytics_%",
+                "analytics_prefix": "analytics_%",
             },
         )
 
@@ -1564,12 +1568,12 @@ with columns ``(actor_id, action, parent, child, allow, reason)``.
                     allow,
                     COALESCE(reason, 'permission_grants table') AS reason
                 FROM permission_grants
-                WHERE actor_id = :actor_id
-                  AND action = :action
+                WHERE actor_id = :grants_actor_id
+                  AND action = :grants_action
             """,
             params={
-                "actor_id": actor.get("id"),
-                "action": action,
+                "grants_actor_id": actor.get("id"),
+                "grants_action": action,
             },
         )
 

tests/test_utils_permissions.py

@@ -519,3 +519,77 @@ async def test_actor_actor_id_action_parameters_available(db):
     assert "view-table" in reason
     # The :actor parameter should be the JSON string
     assert "Actor JSON:" in reason
+
+
+@pytest.mark.asyncio
+async def test_multiple_plugins_with_own_parameters(db):
+    """
+    Test that multiple plugins can use their own parameter names without conflict.
+
+    This verifies that the parameter naming convention works: plugins prefix their
+    parameters (e.g., :plugin1_pattern, :plugin2_message) and both sets of parameters
+    are successfully bound in the SQL queries.
+    """
+    await seed_catalog(db)
+
+    def plugin_one() -> Callable[[str], PermissionSQL]:
+        def provider(action: str) -> PermissionSQL:
+            if action != "view-table":
+                return PermissionSQL("plugin_one", "SELECT NULL WHERE 0", {})
+            return PermissionSQL(
+                "plugin_one",
+                """
+                SELECT database_name AS parent, table_name AS child,
+                       1 AS allow, 'Plugin one used param: ' || :plugin1_param AS reason
+                FROM catalog_tables
+                WHERE database_name = 'accounting'
+                """,
+                {
+                    "plugin1_param": "value1",
+                },
+            )
+
+        return provider
+
+    def plugin_two() -> Callable[[str], PermissionSQL]:
+        def provider(action: str) -> PermissionSQL:
+            if action != "view-table":
+                return PermissionSQL("plugin_two", "SELECT NULL WHERE 0", {})
+            return PermissionSQL(
+                "plugin_two",
+                """
+                SELECT database_name AS parent, table_name AS child,
+                       1 AS allow, 'Plugin two used param: ' || :plugin2_param AS reason
+                FROM catalog_tables
+                WHERE database_name = 'hr'
+                """,
+                {
+                    "plugin2_param": "value2",
+                },
+            )
+
+        return provider
+
+    plugins = [plugin_one(), plugin_two()]
+
+    rows = await resolve_permissions_from_catalog(
+        db,
+        {"id": "test_user"},
+        plugins,
+        "view-table",
+        TABLE_CANDIDATES_SQL,
+        implicit_deny=False,
+    )
+
+    # Both plugins should contribute results with their parameters successfully bound
+    plugin_one_rows = [r for r in rows if r.get("reason") and "Plugin one" in r["reason"]]
+    plugin_two_rows = [r for r in rows if r.get("reason") and "Plugin two" in r["reason"]]
+
+    assert len(plugin_one_rows) > 0, "Plugin one should contribute rules"
+    assert len(plugin_two_rows) > 0, "Plugin two should contribute rules"
+
+    # Verify each plugin's parameters were successfully bound in the SQL
+    assert any("value1" in r.get("reason", "") for r in plugin_one_rows), \
+        "Plugin one's :plugin1_param should be bound"
+    assert any("value2" in r.get("reason", "") for r in plugin_two_rows), \
+        "Plugin two's :plugin2_param should be bound"