From 2fde692a3eab8a4d1569287020cba59ea5c1ca18 Mon Sep 17 00:00:00 2001 From: Simon Willison Date: Tue, 26 May 2026 16:34:48 -0700 Subject: [PATCH] Disallow edits of dangerous decsription_html/on_success_message_sql Refs https://github.com/simonw/datasette/pull/2741#issuecomment-4549891578 --- datasette/views/query_helpers.py | 15 +++--- tests/test_queries.py | 81 +++++++++++++++++++++++++++++++- 2 files changed, 85 insertions(+), 11 deletions(-) diff --git a/datasette/views/query_helpers.py b/datasette/views/query_helpers.py index 9906ac67..de732431 100644 --- a/datasette/views/query_helpers.py +++ b/datasette/views/query_helpers.py @@ -18,14 +18,12 @@ _query_fields = { "sql", "title", "description", - "description_html", "hide_sql", "fragment", "parameters", "params", "is_private", "on_success_message", - "on_success_message_sql", "on_success_redirect", "on_error_message", "on_error_redirect", @@ -35,7 +33,6 @@ _query_create_fields = _query_fields | {"name", "mode", "csrftoken"} _query_update_fields = _query_fields _query_write_fields = { "on_success_message", - "on_success_message_sql", "on_success_redirect", "on_error_message", "on_error_redirect", @@ -441,7 +438,9 @@ def _apply_query_data_types(data): async def _prepare_query_create(datasette, request, db, data): invalid_keys = set(data) - _query_create_fields if invalid_keys: - raise QueryValidationError("Invalid keys: {}".format(", ".join(invalid_keys))) + raise QueryValidationError( + "Invalid keys: {}".format(", ".join(sorted(invalid_keys))) + ) data = _apply_query_data_types(data) name = data.get("name") @@ -467,7 +466,6 @@ async def _prepare_query_create(datasette, request, db, data): "sql": data["sql"], "title": data.get("title"), "description": data.get("description"), - "description_html": data.get("description_html"), "hide_sql": _as_bool(data.get("hide_sql")), "fragment": data.get("fragment"), "parameters": parameters, @@ -477,7 +475,6 @@ async def _prepare_query_create(datasette, request, db, data): "source": "user", "owner_id": _actor_id(request.actor), "on_success_message": data.get("on_success_message"), - "on_success_message_sql": data.get("on_success_message_sql"), "on_success_redirect": data.get("on_success_redirect"), "on_error_message": data.get("on_error_message"), "on_error_redirect": data.get("on_error_redirect"), @@ -488,7 +485,9 @@ async def _prepare_query_create(datasette, request, db, data): async def _prepare_query_update(datasette, request, db, existing, update): invalid_keys = set(update) - _query_update_fields if invalid_keys: - raise QueryValidationError("Invalid keys: {}".format(", ".join(invalid_keys))) + raise QueryValidationError( + "Invalid keys: {}".format(", ".join(sorted(invalid_keys))) + ) update = _apply_query_data_types(update) sql = update.get("sql", existing["sql"]) @@ -519,14 +518,12 @@ async def _prepare_query_update(datasette, request, db, existing, update): "sql": sql, "title": update.get("title"), "description": update.get("description"), - "description_html": update.get("description_html"), "hide_sql": update.get("hide_sql"), "fragment": update.get("fragment"), "parameters": parameters, "is_write": query_is_write, "is_private": update.get("is_private"), "on_success_message": update.get("on_success_message"), - "on_success_message_sql": update.get("on_success_message_sql"), "on_success_redirect": update.get("on_success_redirect"), "on_error_message": update.get("on_error_message"), "on_error_redirect": update.get("on_error_redirect"), diff --git a/tests/test_queries.py b/tests/test_queries.py index ff9057ef..70fb7a03 100644 --- a/tests/test_queries.py +++ b/tests/test_queries.py @@ -186,7 +186,9 @@ async def test_config_queries_imported_to_internal_table(): "configured": { "sql": "select :name as name", "title": "Configured query", + "description_html": "

Configured HTML

", "params": ["name"], + "on_success_message_sql": "select 'Hello ' || :name", } } } @@ -202,7 +204,7 @@ async def test_config_queries_imported_to_internal_table(): "sql": "select :name as name", "title": "Configured query", "description": None, - "description_html": None, + "description_html": "

Configured HTML

", "hide_sql": False, "fragment": None, "params": ["name"], @@ -213,7 +215,7 @@ async def test_config_queries_imported_to_internal_table(): "source": "config", "owner_id": None, "on_success_message": None, - "on_success_message_sql": None, + "on_success_message_sql": "select 'Hello ' || :name", "on_success_redirect": None, "on_error_message": None, "on_error_redirect": None, @@ -887,6 +889,45 @@ async def test_query_store_api_rejects_is_trusted(): assert response.json()["errors"] == ["Invalid keys: is_trusted"] +@pytest.mark.asyncio +async def test_query_store_rejects_config_only_fields(): + ds = Datasette(memory=True, default_deny=True) + ds.root_enabled = True + ds.add_memory_database("query_config_only_fields_api", name="data") + await ds.invoke_startup() + + response = await ds.client.post( + "/data/-/queries/store", + actor={"id": "root"}, + json={ + "query": { + "name": "unsafe", + "sql": "select 1", + "description_html": "", + "on_success_message_sql": "select 'secret'", + } + }, + ) + form_response = await ds.client.post( + "/data/-/queries/store", + actor={"id": "root"}, + data={ + "name": "unsafe_form", + "sql": "select 1", + "description_html": "", + }, + ) + + assert response.status_code == 400 + assert response.json()["errors"] == [ + "Invalid keys: description_html, on_success_message_sql" + ] + assert form_response.status_code == 400 + assert "Invalid keys: description_html" in form_response.text + assert await ds.get_query("data", "unsafe") is None + assert await ds.get_query("data", "unsafe_form") is None + + @pytest.mark.asyncio async def test_query_store_api_creates_writable_query(): ds = Datasette(memory=True, default_deny=True) @@ -959,6 +1000,42 @@ async def test_query_update_and_delete_api(): assert await ds.get_query("data", "editable") is None +@pytest.mark.asyncio +async def test_query_update_api_rejects_config_only_fields(): + ds = Datasette(memory=True, default_deny=True) + ds.root_enabled = True + db = ds.add_memory_database("query_update_config_only_fields", name="data") + await db.execute_write("create table dogs (id integer primary key, name text)") + await ds.invoke_startup() + await ds.add_query( + "data", + "editable", + "insert into dogs (name) values (:name)", + is_write=True, + source="user", + owner_id="root", + ) + + response = await ds.client.post( + "/data/editable/-/update", + actor={"id": "root"}, + json={ + "update": { + "description_html": "", + "on_success_message_sql": "select 'secret'", + } + }, + ) + + assert response.status_code == 400 + assert response.json()["errors"] == [ + "Invalid keys: description_html, on_success_message_sql" + ] + query = await ds.get_query("data", "editable") + assert query["description_html"] is None + assert query["on_success_message_sql"] is None + + @pytest.mark.asyncio async def test_query_update_api_rejects_trusted_queries_but_internal_update_allowed(): ds = Datasette(