diff --git a/datasette/views/stored_queries.py b/datasette/views/stored_queries.py index 2753f876..d1f151dd 100644 --- a/datasette/views/stored_queries.py +++ b/datasette/views/stored_queries.py @@ -610,6 +610,8 @@ class QueryDeleteView(BaseView): resource=QueryResource(db.name, query_name), actor=request.actor, ) + if existing.is_trusted: + return _error(["Trusted queries cannot be deleted using the API"], 403) return await self.render( ["query_delete.html"], request, @@ -631,6 +633,8 @@ class QueryDeleteView(BaseView): actor=request.actor, ): return _error(["Permission denied: need delete-query"], 403) + if existing.is_trusted: + return _error(["Trusted queries cannot be deleted using the API"], 403) data, is_json = await _json_or_form_payload(request) await self.ds.remove_query(db.name, query_name) diff --git a/existing-api.md b/existing-api.md index 9947edeb..e11491fe 100644 --- a/existing-api.md +++ b/existing-api.md @@ -1093,8 +1093,9 @@ updates use `/-/update`. `QueryDeleteView` (app.py:2707-2710; views/stored_queries.py:594-644). GET renders an HTML confirmation page. -- **Permission:** `delete-query` (403 `need delete-query`). Unlike update, - **trusted queries are not blocked** from API deletion. +- **Permission:** `delete-query` (403 `need delete-query`). Trusted + queries → 403 `"Trusted queries cannot be deleted using the API"`, + matching update. - **Response:** JSON request → 200 `{"ok": true}`; form → 302; 404 `"Query not found: x"`. No `confirm` field required (unlike table drop). diff --git a/stable-api-recommendations.md b/stable-api-recommendations.md index 11f0a75d..8c3131e9 100644 --- a/stable-api-recommendations.md +++ b/stable-api-recommendations.md @@ -332,12 +332,15 @@ Concerns: ## 8. Behavior that looks like a bug and should be resolved before freezing -1. **Trusted queries: update is blocked, delete is not.** +1. ~~**Trusted queries: update is blocked, delete is not.** `QueryUpdateView` rejects `is_trusted` queries with 403 (stored_queries.py:426-427) but `QueryDeleteView.post` never checks `is_trusted` — an actor with `delete-query` can delete a config-defined trusted query via the API (it will resync on restart, making the - behavior confusing rather than catastrophic). Align delete with update. + behavior confusing rather than catastrophic). Align delete with update.~~ + ✅ **Done** — both the POST endpoint and the HTML confirmation page now + return 403 `"Trusted queries cannot be deleted using the API"`; + `datasette.remove_query()` remains available for internal use. 2. ~~**GET `/db/-/query` with no `?sql=` returns 200 `{"ok": true, "rows": []}`** while `.csv` on the same request returns 400 `"?sql= is required"`. The JSON behavior masks caller bugs; return 400 on both.~~ @@ -401,8 +404,8 @@ Two details make tiering urgent rather than optional: ✅ Done. 7. Publish explicit stability tiers, including extras and pagination-token opacity (§9). -8. Resolve the looks-like-a-bug list (§8), especially trusted-query delete - and row-delete 500. +8. Resolve the looks-like-a-bug list (§8), especially ~~trusted-query delete + and row-delete 500~~ (both done). Everything in P2 is worth doing now because each item is breaking-to-fix later; each P3 can be resolved by a sentence of documentation declaring the diff --git a/tests/test_queries.py b/tests/test_queries.py index b79a9af4..c5c1c3cd 100644 --- a/tests/test_queries.py +++ b/tests/test_queries.py @@ -3654,3 +3654,51 @@ async def test_stored_write_query_with_truncated_returning_message(): assert response.status_code == 200 assert response.json()["ok"] is True assert response.json()["message"] == "Query executed" + + +@pytest.mark.asyncio +async def test_query_delete_api_rejects_trusted_queries(): + ds = Datasette( + memory=True, + default_deny=True, + config={ + "databases": { + "data": { + "permissions": { + "view-query": {"id": "editor"}, + "delete-query": {"id": "editor"}, + }, + "queries": { + "trusted_report": { + "sql": "select 1 as one", + }, + }, + } + } + }, + ) + ds.add_memory_database("query_delete_trusted_api", name="data") + await ds.invoke_startup() + + response = await ds.client.post( + "/data/trusted_report/-/delete", + actor={"id": "editor"}, + json={}, + ) + assert response.status_code == 403 + assert response.json()["errors"] == [ + "Trusted queries cannot be deleted using the API" + ] + # The query must still exist + assert await ds.get_query("data", "trusted_report") is not None + + # The HTML confirmation page refuses too + get_response = await ds.client.get( + "/data/trusted_report/-/delete", + actor={"id": "editor"}, + ) + assert get_response.status_code == 403 + + # datasette.remove_query() remains available for internal use + await ds.remove_query("data", "trusted_report") + assert await ds.get_query("data", "trusted_report") is None