mirror of
https://github.com/simonw/datasette.git
synced 2026-07-10 01:24:45 +02:00
Schema endpoints check permission before database existence
/db/-/schema previously returned 404 for missing databases before checking view-database, letting unauthorized actors probe for database existence. The permission check now runs first, so actors without view-database get a uniform 403. The table schema endpoint also now returns 404 for an unknown database instead of an unhandled KeyError. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GrHZSypDfMnym1tM5XJAFZ
This commit is contained in:
parent
b958d03c0f
commit
9ee95cab3d
4 changed files with 53 additions and 9 deletions
|
|
@ -1321,17 +1321,17 @@ class DatabaseSchemaView(SchemaBaseView):
|
|||
database_name = request.url_vars["database"]
|
||||
format_ = request.url_vars.get("format") or "html"
|
||||
|
||||
# Check if database exists
|
||||
if database_name not in self.ds.databases:
|
||||
return self.format_error_response("Database not found", format_)
|
||||
|
||||
# Check view-database permission
|
||||
# Permission check comes first, so actors without view-database
|
||||
# cannot distinguish existing databases from missing ones
|
||||
await self.ds.ensure_permission(
|
||||
action="view-database",
|
||||
resource=DatabaseResource(database=database_name),
|
||||
actor=request.actor,
|
||||
)
|
||||
|
||||
if database_name not in self.ds.databases:
|
||||
return self.format_error_response("Database not found", format_)
|
||||
|
||||
schema = await self.get_database_schema(database_name)
|
||||
|
||||
if format_ == "json":
|
||||
|
|
@ -1365,6 +1365,9 @@ class TableSchemaView(SchemaBaseView):
|
|||
actor=request.actor,
|
||||
)
|
||||
|
||||
if database_name not in self.ds.databases:
|
||||
return self.format_error_response("Database not found", format_)
|
||||
|
||||
# Get schema for the table
|
||||
db = self.ds.databases[database_name]
|
||||
result = await db.execute(
|
||||
|
|
|
|||
|
|
@ -618,8 +618,8 @@ views/table_create_alter.py:965-1005).
|
|||
|
||||
- **Permission:** `view-database` (denied → `Forbidden` → 403 HTML).
|
||||
- **Unknown database** → 404; for `.json`:
|
||||
`{"ok": false, "error": "Database not found"}`. (The existence check runs
|
||||
before the permission check.)
|
||||
`{"ok": false, "error": "Database not found"}`. The permission check runs
|
||||
first, so unauthorized actors cannot probe for database existence.
|
||||
- **Responses:** `.json` → 200 `{"ok": true, "database": "<name>", "schema": "<SQL>"}`
|
||||
(concatenated `sqlite_master.sql` joined with `;\n`); `.md` →
|
||||
`text/markdown`; no extension → HTML.
|
||||
|
|
|
|||
|
|
@ -288,11 +288,13 @@ Concerns:
|
|||
databases this leaks filesystem paths and database names. Filter it, or
|
||||
gate it behind `permissions-debug`.~~ ✅ **Done** — the endpoint now
|
||||
filters through `allowed_resources("view-database", actor)`.
|
||||
- **(P2) `/db/-/schema` checks existence before permission**
|
||||
- ~~**(P2) `/db/-/schema` checks existence before permission**
|
||||
(views/special.py:1308-1317): an actor without `view-database` can
|
||||
distinguish "database exists" (403) from "does not exist" (404).
|
||||
Standardize on permission-check-first (as the table view does) so
|
||||
unauthorized actors get a uniform response.
|
||||
unauthorized actors get a uniform response.~~ ✅ **Done** — permission is
|
||||
checked first; the table schema view also now 404s (instead of a 500
|
||||
KeyError) for an unknown database.
|
||||
- **(P2) `/-/threads` exposes runtime internals** (thread idents, asyncio
|
||||
task reprs including file paths) behind only `view-instance`. Consider
|
||||
`permissions-debug`, alongside `/-/actions` which already requires it.
|
||||
|
|
|
|||
|
|
@ -531,3 +531,42 @@ async def test_row_update_return_uses_rows_list(ds_error_shape):
|
|||
assert data["ok"] is True
|
||||
assert "row" not in data
|
||||
assert data["rows"] == [{"id": 1, "title": "Updated"}]
|
||||
|
||||
|
||||
# Schema endpoints: no existence oracle, no 500 on unknown database
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_schema_endpoints_no_existence_oracle(tmp_path_factory):
|
||||
db_directory = tmp_path_factory.mktemp("dbs")
|
||||
db_path = str(db_directory / "data.db")
|
||||
conn = sqlite3.connect(db_path)
|
||||
conn.execute("vacuum")
|
||||
conn.execute("create table docs (id integer primary key)")
|
||||
conn.close()
|
||||
ds = Datasette([db_path], default_deny=True)
|
||||
ds.root_enabled = True
|
||||
try:
|
||||
# An actor without view-database cannot distinguish an existing
|
||||
# database from a missing one
|
||||
denied_existing = await ds.client.get("/data/-/schema.json")
|
||||
denied_missing = await ds.client.get("/nope/-/schema.json")
|
||||
assert denied_existing.status_code == denied_missing.status_code == 403
|
||||
|
||||
# An authorized actor sees the real thing
|
||||
root_existing = await ds.client.get(
|
||||
"/data/-/schema.json", actor={"id": "root"}
|
||||
)
|
||||
assert root_existing.status_code == 200
|
||||
root_missing = await ds.client.get(
|
||||
"/nope/-/schema.json", actor={"id": "root"}
|
||||
)
|
||||
assert root_missing.status_code == 404
|
||||
finally:
|
||||
ds.close()
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_table_schema_unknown_database_is_404_not_500(ds_client):
|
||||
response = await ds_client.get("/no_such_db/some_table/-/schema.json")
|
||||
assert_canonical_error(response, 404)
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue