From ea9c1b1524279c03f0368c714438c5aacebcbec3 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 4 Jul 2026 15:39:11 +0000 Subject: [PATCH] Return 400 for query data formats when ?sql= is missing GET /db/-/query.json with no (or blank) ?sql= previously returned 200 with empty rows, masking caller bugs, while the .csv format returned 400 "?sql= is required" for the same request. All data formats now return the 400; the HTML SQL editor page is unchanged. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01GrHZSypDfMnym1tM5XJAFZ --- datasette/views/database.py | 2 ++ existing-api.md | 5 +++-- stable-api-recommendations.md | 6 ++++-- tests/test_error_shape.py | 24 ++++++++++++++++++++++++ tests/test_table_api.py | 6 ++++-- 5 files changed, 37 insertions(+), 6 deletions(-) diff --git a/datasette/views/database.py b/datasette/views/database.py index cf7a6db3..99e85d2b 100644 --- a/datasette/views/database.py +++ b/datasette/views/database.py @@ -851,6 +851,8 @@ class QueryView(View): return await stream_csv(datasette, fetch_data_for_csv, request, db.name) elif format_ in datasette.renderers.keys(): + if not sql: + raise DatasetteError("?sql= is required", status=400) data = {"ok": True, "rows": rows, "columns": columns} extras = extra_names_from_request(request) if extras: diff --git a/existing-api.md b/existing-api.md index 8c6e1c4e..9947edeb 100644 --- a/existing-api.md +++ b/existing-api.md @@ -486,8 +486,9 @@ queries section). HTTP 400 `{"ok": false, "error": "", "rows": [], "truncated": false}`. - Time limit → 400 titled `"SQL Interrupted"` (the `error` value contains an HTML fragment). - - `?sql=` omitted → 200 `{"ok": true, "rows": [], "truncated": false}` - (the CSV format instead errors 400 `"?sql= is required"`). + - `?sql=` omitted or blank → 400 `"?sql= is required"` for all data + formats (`.json`, `.csv`, plugin formats). The HTML page remains the + SQL editor. - `.csv` streams CSV; unknown extensions → 404. ### GET /\/-/query/parameters diff --git a/stable-api-recommendations.md b/stable-api-recommendations.md index 273a272c..11f0a75d 100644 --- a/stable-api-recommendations.md +++ b/stable-api-recommendations.md @@ -338,9 +338,11 @@ Concerns: `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. -2. **GET `/db/-/query` with no `?sql=` returns 200 `{"ok": true, "rows": +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. + required"`. The JSON behavior masks caller bugs; return 400 on both.~~ + ✅ **Done** — all data formats now return 400; the HTML SQL editor page + is unchanged. 3. **`_shape=object` HTTP 200 error** (§1b) — almost certainly unintended. 4. ~~**Row delete 500** (§1c) — inconsistent with every sibling endpoint.~~ ✅ Done — now 400. diff --git a/tests/test_error_shape.py b/tests/test_error_shape.py index 49b2c314..9d4a566c 100644 --- a/tests/test_error_shape.py +++ b/tests/test_error_shape.py @@ -483,3 +483,27 @@ async def test_token_when_signed_tokens_disabled_returns_401(tmp_path_factory): assert "not enabled" in data["error"] finally: ds.close() + + +# GET /db/-/query without SQL: 400 for data formats, HTML editor stays 200 + + +@pytest.mark.asyncio +@pytest.mark.parametrize( + "path", + ( + "/fixtures/-/query.json", + "/fixtures/-/query.json?sql=", + ), +) +async def test_query_json_without_sql_is_400(ds_client, path): + response = await ds_client.get(path) + data = assert_canonical_error(response, 400) + assert data["errors"] == ["?sql= is required"] + + +@pytest.mark.asyncio +async def test_query_html_without_sql_is_still_the_editor(ds_client): + response = await ds_client.get("/fixtures/-/query") + assert response.status_code == 200 + assert response.headers["content-type"].startswith("text/html") diff --git a/tests/test_table_api.py b/tests/test_table_api.py index c8ba31b7..41c89f39 100644 --- a/tests/test_table_api.py +++ b/tests/test_table_api.py @@ -180,9 +180,11 @@ def test_query_extra_query_reports_bound_params(): assert response.json["query"]["params"] == {} -def test_query_extra_query_does_not_echo_querystring_without_sql(): +def test_query_extra_query_does_not_echo_querystring(): with make_app_client() as client: - response = client.get("/fixtures/-/query.json?_extra=query&foo=bar") + response = client.get( + "/fixtures/-/query.json?sql=select+1&_extra=query&foo=bar" + ) assert response.status == 200 assert response.json["query"]["params"] == {}