From e4f5b5c30fa58de5897929d07133f55e96d529de Mon Sep 17 00:00:00 2001 From: Simon Willison Date: Thu, 23 Oct 2025 21:41:13 -0700 Subject: [PATCH] Fix schema mismatch in empty result query When no permission rules exist, the query was returning 2 columns (parent, child) but the function contract specifies 3 columns (parent, child, reason). This could cause schema mismatches in consuming code. Added 'NULL AS reason' to match the documented 3-column schema. Added regression test that verifies the schema has 3 columns even when no permission rules are returned. The test fails without the fix (showing only 2 columns) and passes with it. Thanks to @asg017 for catching this --- datasette/utils/actions_sql.py | 2 +- tests/test_actions_sql.py | 56 ++++++++++++++++++++++++++++++++++ 2 files changed, 57 insertions(+), 1 deletion(-) diff --git a/datasette/utils/actions_sql.py b/datasette/utils/actions_sql.py index 77aab236..89c6cba9 100644 --- a/datasette/utils/actions_sql.py +++ b/datasette/utils/actions_sql.py @@ -116,7 +116,7 @@ async def build_allowed_resources_sql( # If no rules, return empty result (deny all) if not rule_sqls: - return "SELECT NULL AS parent, NULL AS child WHERE 0", {} + return "SELECT NULL AS parent, NULL AS child, NULL AS reason WHERE 0", {} # Build the cascading permission query rules_union = " UNION ALL ".join(rule_sqls) diff --git a/tests/test_actions_sql.py b/tests/test_actions_sql.py index 143faec9..08f36f23 100644 --- a/tests/test_actions_sql.py +++ b/tests/test_actions_sql.py @@ -311,3 +311,59 @@ async def test_sql_does_filtering_not_python(test_ds): finally: pm.unregister(plugin, name="test_plugin") + + +@pytest.mark.asyncio +async def test_no_permission_rules_returns_correct_schema(): + """ + Test that when no permission rules exist, the empty result has correct schema. + + This is a regression test for a bug where the empty result returned only + 2 columns (parent, child) instead of the documented 3 columns + (parent, child, reason), causing schema mismatches. + + See: https://github.com/simonw/datasette/pull/2515#discussion_r2457803901 + """ + from datasette.utils.actions_sql import build_allowed_resources_sql + + # Create a fresh datasette instance + ds = Datasette() + await ds.invoke_startup() + + # Add a test database + db = ds.add_memory_database("testdb") + await db.execute_write( + "CREATE TABLE IF NOT EXISTS test_table (id INTEGER PRIMARY KEY)" + ) + await ds._refresh_schemas() + + # Temporarily block all permission_resources_sql hooks to simulate no rules + original_hook = pm.hook.permission_resources_sql + + def empty_hook(*args, **kwargs): + return [] + + pm.hook.permission_resources_sql = empty_hook + + try: + # Call build_allowed_resources_sql directly which will hit the no-rules code path + sql, params = await build_allowed_resources_sql( + ds, actor={"id": "nobody"}, action="view-table" + ) + + # Execute the query to verify it has correct column structure + result = await ds.get_internal_database().execute(sql, params) + + # Should have 3 columns: parent, child, reason + # This assertion would fail if the empty result only had 2 columns + assert ( + len(result.columns) == 3 + ), f"Expected 3 columns, got {len(result.columns)}: {result.columns}" + assert result.columns == ["parent", "child", "reason"] + + # Should have no rows (no rules = no access) + assert len(result.rows) == 0 + + finally: + # Restore original hook + pm.hook.permission_resources_sql = original_hook