remote/datasette
1
0
Fork 0
mirror of https://github.com/simonw/datasette.git synced 2026-05-28 04:46:18 +02:00
Code Issues Projects Releases Packages Wiki Activity

Enforce query ownership and remove canned query hook

Browse source 
Refs #2735
This commit is contained in:
Simon Willison 2026-05-24 22:58:50 -07:00
commit 040e42ddca
11 changed files with 182 additions and 99 deletions
Download patch file Download diff file Expand all files Collapse all files

16
datasette/default_permissions/__init__.py
View file

@ -17,13 +17,6 @@ UNION/INTERSECT operations. The order of evaluation is:
from __future__ import annotations
from typing import TYPE_CHECKING
if TYPE_CHECKING:
from datasette.app import Datasette
from datasette import hookimpl
# Re-export all hooks and public utilities
from .restrictions import (
actor_restrictions_sql as actor_restrictions_sql,
@ -38,12 +31,3 @@ from .defaults import (
default_query_permissions_sql as default_query_permissions_sql,
DEFAULT_ALLOW_ACTIONS as DEFAULT_ALLOW_ACTIONS,
)
@hookimpl
def canned_queries(datasette: "Datasette", database: str, actor) -> dict:
"""Return canned queries defined in datasette.yaml configuration."""
queries = (
((datasette.config or {}).get("databases") or {}).get(database) or {}
).get("queries") or {}
return queries

30
datasette/default_permissions/defaults.py
View file

@ -74,6 +74,22 @@ async def default_query_permissions_sql(
actor: Optional[dict],
action: str,
) -> Optional[PermissionSQL]:
actor_id = actor.get("id") if isinstance(actor, dict) else None
if action in {"update-query", "delete-query"}:
if actor_id is None:
return None
return PermissionSQL(
sql="""
SELECT database_name AS parent, name AS child, 1 AS allow,
'query owner' AS reason
FROM queries
WHERE source = 'user'
AND owner_id = :query_owner_id
""",
params={"query_owner_id": actor_id},
)
if action != "view-query":
return None
@ -98,6 +114,19 @@ async def default_query_permissions_sql(
AND source IN ('config', 'plugin')
"""
user_writable_sql = ""
if actor_id is not None:
params["query_owner_id"] = actor_id
user_writable_sql = """
UNION ALL
SELECT database_name AS parent, name AS child, 1 AS allow,
'query owner' AS reason
FROM queries
WHERE is_write = 1
AND source = 'user'
AND owner_id = :query_owner_id
"""
return PermissionSQL(
sql=f"""
WITH execute_sql_allowed AS (
@ -118,6 +147,7 @@ async def default_query_permissions_sql(
WHERE q.is_write = 0
AND q.published = 0
{trusted_writable_sql}
{user_writable_sql}
""",
params=params,
)

5
datasette/hookspecs.py
View file

@ -137,11 +137,6 @@ def permission_resources_sql(datasette, actor, action):
"""
@hookspec
def canned_queries(datasette, database, actor):
"""Return a dictionary of canned query definitions or an awaitable function that returns them"""
@hookspec
def register_magic_parameters(datasette):
"""Return a list of (name, function) magic parameter functions"""

12
datasette/views/database.py
View file

@ -945,6 +945,18 @@ class QueryView(View):
# That should not have happened
raise DatasetteError("Unexpected table found on POST", status=404)
if not await datasette.allowed(
action="view-query",
resource=QueryResource(database=db.name, query=canned_query["name"]),
actor=request.actor,
):
raise Forbidden("You do not have permission to view this query")
if canned_query.get("write") and canned_query.get("source") == "user":
await datasette.ensure_query_write_permissions(
db.name, canned_query["sql"], actor=request.actor
)
# If database is immutable, return an error
if not db.is_mutable:
raise Forbidden("Database is immutable")

2
tests/fixtures.py
View file

@ -35,7 +35,6 @@ EXPECTED_PLUGINS = [
"hooks": [
"actor_from_request",
"asgi_wrapper",
"canned_queries",
"database_actions",
"extra_body_script",
"extra_css_urls",
@ -68,7 +67,6 @@ EXPECTED_PLUGINS = [
"hooks": [
"actor_from_request",
"asgi_wrapper",
"canned_queries",
"extra_js_urls",
"extra_template_vars",
"handle_exception",

5
tests/plugins/my_plugin.py
View file

@ -314,11 +314,6 @@ def startup(datasette):
_ = (Response, Forbidden, NotFound, hookimpl, actor_matches_allow)
@hookimpl
def canned_queries(datasette, database, actor):
return {"from_hook": f"select 1, '{actor['id'] if actor else 'null'}' as actor_id"}
@hookimpl
def register_magic_parameters():
from uuid import uuid4

26
tests/plugins/my_plugin_2.py
View file

@ -139,20 +139,20 @@ def startup(datasette):
datasette._startup_catalog_databases = [
row["database_name"] for row in catalog_rows
]
return inner
@hookimpl
def canned_queries(datasette, database):
async def inner():
return {
"from_async_hook": "select {}".format(
(
await datasette.get_database(database).execute("select 1 + 1")
).first()[0]
for database in datasette.databases:
await datasette.add_query(
database,
"from_hook",
"select 1, 'null' as actor_id",
source="plugin",
)
result = await datasette.get_database(database).execute("select 1 + 1")
await datasette.add_query(
database,
"from_async_hook",
"select {}".format(result.first()[0]),
source="plugin",
)
}
return inner

34
tests/test_canned_queries.py
View file

@ -1,10 +1,16 @@
from bs4 import BeautifulSoup as Soup
from asgiref.sync import async_to_sync
import json
import pytest
import re
from .fixtures import make_app_client
def update_query(client, name, **kwargs):
async_to_sync(client.ds.invoke_startup)()
async_to_sync(client.ds.update_query)("data", name, **kwargs)
@pytest.fixture
def canned_write_client(tmpdir):
template_dir = tmpdir / "canned_write_templates"
@ -153,9 +159,7 @@ def test_insert_error(canned_write_client):
)
assert [["UNIQUE constraint failed: names.rowid", 3]] == messages
# How about with a custom error message?
canned_write_client.ds.config["databases"]["data"]["queries"][
"add_name_specify_id"
]["on_error_message"] = "ERROR"
update_query(canned_write_client, "add_name_specify_id", on_error_message="ERROR")
response = canned_write_client.post(
"/data/add_name_specify_id",
{"rowid": 1, "name": "Should fail"},
@ -327,12 +331,16 @@ def magic_parameters_client():
],
)
def test_magic_parameters(magic_parameters_client, magic_parameter, expected_re):
magic_parameters_client.ds.config["databases"]["data"]["queries"]["runme_post"][
"sql"
] = f"insert into logs (line) values (:{magic_parameter})"
magic_parameters_client.ds.config["databases"]["data"]["queries"]["runme_get"][
"sql"
] = f"select :{magic_parameter} as result"
update_query(
magic_parameters_client,
"runme_post",
sql=f"insert into logs (line) values (:{magic_parameter})",
)
update_query(
magic_parameters_client,
"runme_get",
sql=f"select :{magic_parameter} as result",
)
cookies = {
"ds_actor": magic_parameters_client.actor_cookie({"id": "root"}),
"foo": "bar",
@ -366,9 +374,11 @@ def test_magic_parameters(magic_parameters_client, magic_parameter, expected_re)
@pytest.mark.parametrize("use_csrf", [True, False])
@pytest.mark.parametrize("return_json", [True, False])
def test_magic_parameters_csrf_json(magic_parameters_client, use_csrf, return_json):
magic_parameters_client.ds.config["databases"]["data"]["queries"]["runme_post"][
"sql"
] = "insert into logs (line) values (:_header_host)"
update_query(
magic_parameters_client,
"runme_post",
sql="insert into logs (line) values (:_header_host)",
)
qs = ""
if return_json:
qs = "?_json=1"

40
tests/test_permissions.py
View file

@ -1,4 +1,5 @@
import collections
from asgiref.sync import async_to_sync
from datasette.app import Datasette
from datasette.cli import cli
from datasette.default_permissions import restrictions_allow_action
@ -609,6 +610,10 @@ def test_padlocks_on_database_page(cascade_app_client):
previous_config = cascade_app_client.ds.config
try:
cascade_app_client.ds.config = config
async_to_sync(cascade_app_client.ds.invoke_startup)()
async_to_sync(cascade_app_client.ds.add_query)(
"fixtures", "query_two", "select 2", source="config"
)
response = cascade_app_client.get(
"/fixtures",
cookies={"ds_actor": cascade_app_client.actor_cookie({"id": "test"})},
@ -624,6 +629,7 @@ def test_padlocks_on_database_page(cascade_app_client):
assert ">simple_view</a></li>" in response.text
finally:
cascade_app_client.ds.config = previous_config
async_to_sync(cascade_app_client.ds.remove_query)("fixtures", "query_two")
@pytest.mark.asyncio
@ -954,39 +960,20 @@ async def test_permissions_in_config(
@pytest.mark.asyncio
async def test_allowed_resources_view_query_includes_actor_specific_canned_queries():
"""
Actor-specific canned queries should be listed by allowed_resources("view-query").
This test is intentionally explicit about the previous bug:
- the canned query only exists for actor "alice"
- the permission rule only allows actor "alice" to view it
- allowed() succeeds for that specific query resource
- allowed_resources("view-query", actor) must include the same query
Before the fix, QueryResource.resources_sql() called canned_queries(..., actor=None),
so the query was omitted from resource enumeration and allowed_resources() returned
an empty list even though allowed() returned True.
"""
async def test_allowed_resources_view_query_includes_actor_specific_query_permissions():
from datasette import hookimpl
from datasette.permissions import PermissionSQL
from datasette.resources import QueryResource
class ActorSpecificQueryPlugin:
__name__ = "ActorSpecificQueryPlugin"
@hookimpl
def canned_queries(self, datasette, database, actor):
if database == "testdb" and actor and actor.get("id") == "alice":
return {"user_only": {"sql": "select 1 as n"}}
return {}
class ActorSpecificQueryPermissionPlugin:
__name__ = "ActorSpecificQueryPermissionPlugin"
@hookimpl
def permission_resources_sql(self, datasette, actor, action):
if action == "view-query" and actor and actor.get("id") == "alice":
return PermissionSQL(sql="""
SELECT 'testdb' AS parent, 'user_only' AS child, 1 AS allow,
'alice can view her actor-specific canned query' AS reason
'alice can view this query' AS reason
""")
return None
@ -994,9 +981,10 @@ async def test_allowed_resources_view_query_includes_actor_specific_canned_queri
await ds.invoke_startup()
ds.add_memory_database("testdb")
await ds._refresh_schemas()
await ds.add_query("testdb", "user_only", "select 1 as n")
plugin = ActorSpecificQueryPlugin()
ds.pm.register(plugin, name="actor_specific_query_plugin")
plugin = ActorSpecificQueryPermissionPlugin()
ds.pm.register(plugin, name="actor_specific_query_permission_plugin")
try:
actor = {"id": "alice"}
@ -1012,7 +1000,7 @@ async def test_allowed_resources_view_query_includes_actor_specific_canned_queri
("testdb", "user_only")
]
finally:
ds.pm.unregister(name="actor_specific_query_plugin")
ds.pm.unregister(name="actor_specific_query_permission_plugin")
@pytest.mark.asyncio

27
tests/test_plugins.py
View file

@ -885,40 +885,27 @@ async def test_hook_startup_catalog_populated(ds_client):
@pytest.mark.asyncio
async def test_hook_canned_queries(ds_client):
async def test_plugin_startup_queries(ds_client):
queries = (await ds_client.get("/fixtures.json")).json()["queries"]
queries_by_name = {q["name"]: q for q in queries}
assert {
"sql": "select 2",
"name": "from_async_hook",
"private": False,
} == queries_by_name["from_async_hook"]
assert {
"sql": "select 1, 'null' as actor_id",
"name": "from_hook",
"private": False,
} == queries_by_name["from_hook"]
assert queries_by_name["from_async_hook"]["sql"] == "select 2"
assert queries_by_name["from_async_hook"]["private"] is False
assert queries_by_name["from_hook"]["sql"] == "select 1, 'null' as actor_id"
assert queries_by_name["from_hook"]["private"] is False
@pytest.mark.asyncio
async def test_hook_canned_queries_non_async(ds_client):
async def test_plugin_startup_query_from_hook(ds_client):
response = await ds_client.get("/fixtures/from_hook.json?_shape=array")
assert [{"1": 1, "actor_id": "null"}] == response.json()
@pytest.mark.asyncio
async def test_hook_canned_queries_async(ds_client):
async def test_plugin_startup_query_from_async_hook(ds_client):
response = await ds_client.get("/fixtures/from_async_hook.json?_shape=array")
assert [{"2": 2}] == response.json()
@pytest.mark.asyncio
async def test_hook_canned_queries_actor(ds_client):
assert (
await ds_client.get("/fixtures/from_hook.json?_bot=1&_shape=array")
).json() == [{"1": 1, "actor_id": "bot"}]
def test_hook_register_magic_parameters(restore_working_directory):
with make_app_client(
extra_databases={"data.db": "create table logs (line text)"},

84
tests/test_queries.py
View file

@ -490,3 +490,87 @@ async def test_create_query_ui_and_arbitrary_sql_save_link():
assert query_response.status_code == 200
assert "Save query" in query_response.text
assert "/data/-/queries/-/create?sql=select+%2A+from+dogs" in query_response.text
@pytest.mark.asyncio
async def test_query_owner_gets_update_delete_and_writable_view_defaults():
ds = Datasette(memory=True, default_deny=True)
ds.add_memory_database("query_owner_defaults", name="data")
await ds.invoke_startup()
await ds.add_query(
"data",
"insert_dog",
"insert into dogs (name) values (:name)",
is_write=True,
source="user",
owner_id="alice",
)
for action in ("view-query", "update-query", "delete-query"):
assert await ds.allowed(
action=action,
resource=QueryResource("data", "insert_dog"),
actor={"id": "alice"},
)
assert not await ds.allowed(
action=action,
resource=QueryResource("data", "insert_dog"),
actor={"id": "bob"},
)
@pytest.mark.asyncio
async def test_user_writable_query_execution_rechecks_table_permissions():
ds = Datasette(
memory=True,
default_deny=True,
config={
"databases": {
"data": {
"tables": {
"dogs": {
"permissions": {
"insert-row": {"id": "alice"},
}
}
}
}
}
},
)
db = ds.add_memory_database("query_write_execution", name="data")
await db.execute_write("create table dogs (id integer primary key, name text)")
await ds.invoke_startup()
await ds.add_query(
"data",
"insert_dog",
"insert into dogs (name) values (:name)",
is_write=True,
source="user",
owner_id="alice",
)
await ds.add_query(
"data",
"insert_cat",
"insert into dogs (name) values (:name)",
is_write=True,
source="user",
owner_id="bob",
)
allowed_response = await ds.client.post(
"/data/insert_dog?_json=1",
actor={"id": "alice"},
data={"name": "Cleo"},
)
denied_response = await ds.client.post(
"/data/insert_cat?_json=1",
actor={"id": "bob"},
data={"name": "Milo"},
)
assert allowed_response.status_code == 200
assert allowed_response.json()["ok"] is True
assert denied_response.status_code == 403
rows = (await db.execute("select name from dogs")).dicts()
assert rows == [{"name": "Cleo"}]