mirror of
https://github.com/simonw/datasette.git
synced 2026-07-08 16:44:34 +02:00
Block API deletion of trusted stored queries
QueryUpdateView already rejected is_trusted queries but QueryDeleteView
did not, so an actor with delete-query could delete a config-defined
trusted query - which would then silently reappear on restart when the
config re-syncs. Both the POST endpoint and the HTML confirmation page
now return 403, matching update. datasette.remove_query() is unchanged
for internal use.
The docs already claimed this behavior ("Trusted stored queries cannot
be edited or deleted through the web interface or the JSON API") - the
code now matches them.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GrHZSypDfMnym1tM5XJAFZ
This commit is contained in:
parent
ea9c1b1524
commit
f3f5e891c9
4 changed files with 62 additions and 6 deletions
|
|
@ -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)
|
||||
|
|
|
|||
|
|
@ -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).
|
||||
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue