remote/datasette
1
0
Fork 0
mirror of https://github.com/simonw/datasette.git synced 2026-05-31 14:16:59 +02:00
Code Issues Projects Releases Packages Wiki Activity

Require full row mutation permissions for raw SQL

Browse source 
Raw SQL insert and update statements can have broader effects than their SQLite authorizer callbacks reveal. INSERT OR REPLACE and UPDATE OR REPLACE can delete conflicting rows while only surfacing insert or update operations.

Expand table insert and update operations to require insert-row, update-row, and delete-row together. Keep delete operations mapped to delete-row, and update the analysis UI/API to report and evaluate multiple required permissions for a single operation.

Refs https://github.com/simonw/datasette/pull/2749#issuecomment-4559083539
This commit is contained in:
Simon Willison 2026-05-27 15:17:10 -07:00
commit 03b2c66f63
3 changed files with 290 additions and 45 deletions
Download patch file Download diff file Expand all files Collapse all files

108
datasette/stored_queries.py
View file

@ -588,10 +588,25 @@ async def list_queries(
)
PermissionRequirement = tuple[str, Resource]
@dataclass(frozen=True)
class PermissionRequirement:
action: str
resource: Resource
def permission_for_operation(operation: Operation) -> PermissionRequirement | None:
def row_mutation_requirements(
database: str, table: str
) -> tuple[PermissionRequirement, ...]:
resource = TableResource(database=database, table=table)
return tuple(
PermissionRequirement(action=action, resource=resource)
for action in ("insert-row", "update-row", "delete-row")
)
def permission_requirements_for_operation(
operation: Operation,
) -> tuple[PermissionRequirement, ...]:
if (
operation.operation == "read"
and operation.target_type == "table"
@ -599,31 +614,45 @@ def permission_for_operation(operation: Operation) -> PermissionRequirement | No
and operation.table is not None
):
return (
"view-table",
TableResource(database=operation.database, table=operation.table),
PermissionRequirement(
action="view-table",
resource=TableResource(
database=operation.database, table=operation.table
),
),
)
write_actions = {
"insert": "insert-row",
"update": "update-row",
"delete": "delete-row",
}
action = write_actions.get(operation.operation)
if (
action
operation.operation in {"insert", "update"}
and operation.target_type == "table"
and operation.database is not None
and operation.table is not None
):
return row_mutation_requirements(
database=operation.database,
table=operation.table,
)
if (
operation.operation == "delete"
and operation.target_type == "table"
and operation.database is not None
and operation.table is not None
):
return (
action,
TableResource(database=operation.database, table=operation.table),
PermissionRequirement(
action="delete-row",
resource=TableResource(
database=operation.database, table=operation.table
),
),
)
if operation.operation == "create" and operation.target_type == "table":
if operation.database is None:
return None
return ()
return (
"create-table",
DatabaseResource(database=operation.database),
PermissionRequirement(
action="create-table",
resource=DatabaseResource(database=operation.database),
),
)
if (
operation.operation == "alter"
@ -632,8 +661,12 @@ def permission_for_operation(operation: Operation) -> PermissionRequirement | No
and operation.table is not None
):
return (
"alter-table",
TableResource(database=operation.database, table=operation.table),
PermissionRequirement(
action="alter-table",
resource=TableResource(
database=operation.database, table=operation.table
),
),
)
if (
operation.operation == "drop"
@ -642,8 +675,12 @@ def permission_for_operation(operation: Operation) -> PermissionRequirement | No
and operation.table is not None
):
return (
"drop-table",
TableResource(database=operation.database, table=operation.table),
PermissionRequirement(
action="drop-table",
resource=TableResource(
database=operation.database, table=operation.table
),
),
)
if (
operation.operation in {"create", "drop"}
@ -652,10 +689,14 @@ def permission_for_operation(operation: Operation) -> PermissionRequirement | No
and operation.table is not None
):
return (
"alter-table",
TableResource(database=operation.database, table=operation.table),
PermissionRequirement(
action="alter-table",
resource=TableResource(
database=operation.database, table=operation.table
),
),
)
return None
return ()
def operation_should_be_ignored(operation: Operation) -> bool:
@ -704,20 +745,23 @@ async def ensure_query_write_permissions(
for operation in analysis.operations:
if operation_should_be_ignored(operation):
continue
permission = permission_for_operation(operation)
if permission is None:
permissions = permission_requirements_for_operation(operation)
if not permissions:
raise Forbidden(
"Unsupported SQL operation: {} {}".format(
operation.operation, operation.target_type
)
)
action, resource = permission
if operation.database != database:
raise Forbidden("Writable queries may not access attached databases")
if not await datasette.allowed(
action=action,
resource=resource,
actor=actor,
):
raise Forbidden(f"Permission denied: need {action} on {resource}")
for permission in permissions:
if not await datasette.allowed(
action=permission.action,
resource=permission.resource,
actor=actor,
):
raise Forbidden(
f"Permission denied: need {permission.action} "
f"on {permission.resource}"
)
return analysis

27
datasette/views/query_helpers.py
View file

@ -6,7 +6,7 @@ from datasette.stored_queries import (
StoredQuery,
operation_is_write,
operation_should_be_ignored,
permission_for_operation,
permission_requirements_for_operation,
)
from datasette.utils import (
named_parameters as derive_named_parameters,
@ -216,8 +216,10 @@ def _display_operations(analysis: SQLAnalysis) -> list[Operation]:
def _analysis_rows(analysis: SQLAnalysis) -> list[dict[str, object]]:
rows = []
for operation in _display_operations(analysis):
permission = permission_for_operation(operation)
required_permission = permission[0] if permission else ""
permissions = permission_requirements_for_operation(operation)
required_permission = ", ".join(
permission.action for permission in permissions
)
rows.append(
{
"operation": operation.operation,
@ -236,14 +238,17 @@ async def _analysis_rows_with_permissions(
rows = _analysis_rows(analysis)
is_write = _analysis_is_write(analysis)
for row, operation in zip(rows, _display_operations(analysis)):
permission = permission_for_operation(operation)
if permission:
action, resource = permission
row["allowed"] = await datasette.allowed(
action=action,
resource=resource,
actor=actor,
)
permissions = permission_requirements_for_operation(operation)
if permissions:
row["allowed"] = True
for permission in permissions:
if not await datasette.allowed(
action=permission.action,
resource=permission.resource,
actor=actor,
):
row["allowed"] = False
break
elif is_write:
row["allowed"] = False
else:

200
tests/test_queries.py
View file

@ -508,6 +508,8 @@ async def test_analyze_write_query_requires_table_permissions():
"dogs": {
"permissions": {
"insert-row": {"id": "writer"},
"update-row": {"id": "writer"},
"delete-row": {"id": "writer"},
}
}
}
@ -1429,7 +1431,7 @@ async def test_execute_write_analyze_endpoint_uses_sql_only():
"operation": "insert",
"database": "data",
"table": "dogs",
"required_permission": "insert-row",
"required_permission": "insert-row, update-row, delete-row",
"source": None,
"allowed": True,
}
@ -1627,6 +1629,40 @@ async def test_execute_write_post_requires_database_and_table_permissions():
}
}
}
missing_update_permission = await ds.client.post(
"/data/-/execute-write",
actor={"id": "writer"},
json={
"sql": "insert into dogs (name) values (:name)",
"params": {"name": "Cleo"},
},
)
assert missing_update_permission.status_code == 403
assert missing_update_permission.json()["errors"] == [
"Permission denied: need update-row on data/dogs"
]
ds.config["databases"]["data"]["tables"]["dogs"]["permissions"][
"update-row"
] = {"id": "writer"}
missing_delete_permission = await ds.client.post(
"/data/-/execute-write",
actor={"id": "writer"},
json={
"sql": "insert into dogs (name) values (:name)",
"params": {"name": "Cleo"},
},
)
assert missing_delete_permission.status_code == 403
assert missing_delete_permission.json()["errors"] == [
"Permission denied: need delete-row on data/dogs"
]
ds.config["databases"]["data"]["tables"]["dogs"]["permissions"][
"delete-row"
] = {"id": "writer"}
allowed = await ds.client.post(
"/data/-/execute-write",
actor={"id": "writer"},
@ -1643,6 +1679,156 @@ async def test_execute_write_post_requires_database_and_table_permissions():
assert (await db.execute("select name from dogs")).first()[0] == "Cleo"
@pytest.mark.asyncio
async def test_execute_write_insert_or_replace_requires_delete_row_permission():
ds = Datasette(
memory=True,
default_deny=True,
config={
"databases": {
"data": {
"permissions": {
"view-database": {"id": "writer"},
"execute-write-sql": {"id": "writer"},
},
"tables": {
"users": {
"permissions": {
"insert-row": {"id": "writer"},
"update-row": {"id": "writer"},
"view-table": {"id": "writer"},
}
}
},
}
}
},
)
db = ds.add_memory_database("execute_write_insert_or_replace", name="data")
await db.execute_write(
"create table users (id integer primary key, email text unique)"
)
await db.execute_write(
"insert into users (id, email) values "
"(1, 'a@example.com'), (2, 'b@example.com')"
)
await ds.invoke_startup()
denied_response = await ds.client.post(
"/data/-/execute-write",
actor={"id": "writer"},
json={
"sql": (
"insert or replace into users(id, email) "
"values (3, 'b@example.com')"
)
},
)
assert denied_response.status_code == 403
assert denied_response.json()["errors"] == [
"Permission denied: need delete-row on data/users"
]
assert (await db.execute("select id, email from users order by id")).dicts() == [
{"id": 1, "email": "a@example.com"},
{"id": 2, "email": "b@example.com"},
]
@pytest.mark.asyncio
async def test_execute_write_update_or_replace_requires_delete_row_permission():
ds = Datasette(
memory=True,
default_deny=True,
config={
"databases": {
"data": {
"permissions": {
"view-database": {"id": "writer"},
"execute-write-sql": {"id": "writer"},
},
"tables": {
"users": {
"permissions": {
"insert-row": {"id": "writer"},
"update-row": {"id": "writer"},
"view-table": {"id": "writer"},
}
}
},
}
}
},
)
db = ds.add_memory_database("execute_write_update_or_replace", name="data")
await db.execute_write(
"create table users (id integer primary key, email text unique)"
)
await db.execute_write(
"insert into users (id, email) values "
"(1, 'a@example.com'), (2, 'b@example.com')"
)
await ds.invoke_startup()
denied_response = await ds.client.post(
"/data/-/execute-write",
actor={"id": "writer"},
json={"sql": "update or replace users set email = 'b@example.com' where id = 1"},
)
assert denied_response.status_code == 403
assert denied_response.json()["errors"] == [
"Permission denied: need delete-row on data/users"
]
assert (await db.execute("select id, email from users order by id")).dicts() == [
{"id": 1, "email": "a@example.com"},
{"id": 2, "email": "b@example.com"},
]
@pytest.mark.asyncio
async def test_execute_write_update_requires_insert_row_permission():
ds = Datasette(
memory=True,
default_deny=True,
config={
"databases": {
"data": {
"permissions": {
"view-database": {"id": "writer"},
"execute-write-sql": {"id": "writer"},
},
"tables": {
"users": {
"permissions": {
"update-row": {"id": "writer"},
"delete-row": {"id": "writer"},
"view-table": {"id": "writer"},
}
}
},
}
}
},
)
db = ds.add_memory_database("execute_write_update_requires_insert", name="data")
await db.execute_write("create table users (id integer primary key, name text)")
await db.execute_write("insert into users (id, name) values (1, 'Alice')")
await ds.invoke_startup()
denied_response = await ds.client.post(
"/data/-/execute-write",
actor={"id": "writer"},
json={"sql": "update users set name = 'Alicia' where id = 1"},
)
assert denied_response.status_code == 403
assert denied_response.json()["errors"] == [
"Permission denied: need insert-row on data/users"
]
assert (await db.execute("select name from users where id = 1")).first()[0] == "Alice"
@pytest.mark.asyncio
async def test_execute_write_insert_select_requires_view_table_on_source():
ds = Datasette(
@ -1659,7 +1845,13 @@ async def test_execute_write_insert_select_requires_view_table_on_source():
"secret": {
"permissions": {"view-table": {"id": "someone-else"}}
},
"public_log": {"permissions": {"insert-row": {"id": "writer"}}},
"public_log": {
"permissions": {
"insert-row": {"id": "writer"},
"update-row": {"id": "writer"},
"delete-row": {"id": "writer"},
}
},
},
}
}
@ -1740,6 +1932,8 @@ async def test_execute_write_rejects_function_operations():
"dogs": {
"permissions": {
"insert-row": {"id": "writer"},
"update-row": {"id": "writer"},
"delete-row": {"id": "writer"},
}
}
},
@ -2117,6 +2311,8 @@ async def test_user_writable_query_execution_rechecks_table_permissions():
"dogs": {
"permissions": {
"insert-row": {"id": "alice"},
"update-row": {"id": "alice"},
"delete-row": {"id": "alice"},
}
}
},