diff --git a/datasette/permissions.py b/datasette/permissions.py
index a9a3cc7c..917c58ab 100644
--- a/datasette/permissions.py
+++ b/datasette/permissions.py
@@ -58,16 +58,6 @@ class Resource(ABC):
self.child = child
self._private = None # Sentinel to track if private was set
- def __str__(self) -> str:
- return "/".join(
- str(part) for part in (self.parent, self.child) if part is not None
- )
-
- def __repr__(self) -> str:
- return "{}(parent={!r}, child={!r})".format(
- self.__class__.__name__, self.parent, self.child
- )
-
@property
def private(self) -> bool:
"""
diff --git a/datasette/stored_queries.py b/datasette/stored_queries.py
index fd1cabf3..bcfdfdb4 100644
--- a/datasette/stored_queries.py
+++ b/datasette/stored_queries.py
@@ -2,26 +2,14 @@ from __future__ import annotations
from dataclasses import dataclass
import json
-from typing import Any, Iterable, TYPE_CHECKING
+from typing import Any, Iterable
-from .resources import DatabaseResource, TableResource
-from .permissions import Resource
+from .resources import TableResource
from .utils import named_parameters, sqlite3, tilde_encode, urlsafe_components
from .utils.asgi import Forbidden
-from .utils.sql_analysis import Operation, SQLAnalysis
-
-if TYPE_CHECKING:
- from .app import Datasette
UNCHANGED = object()
-
-class QueryWriteRejected(Exception):
- def __init__(self, message: str):
- self.message = message
- super().__init__(message)
-
-
QUERY_OPTION_FIELDS = (
"hide_sql",
"fragment",
@@ -595,158 +583,20 @@ async def list_queries(
)
-@dataclass(frozen=True)
-class PermissionRequirement:
- action: str
- resource: Resource
-
-
-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"
- and operation.database is not None
- and operation.table is not None
- ):
- return (
- PermissionRequirement(
- action="view-table",
- resource=TableResource(
- database=operation.database, table=operation.table
- ),
- ),
- )
- if (
- 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 (
- 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 ()
- return (
- PermissionRequirement(
- action="create-table",
- resource=DatabaseResource(database=operation.database),
- ),
- )
- if (
- operation.operation == "alter"
- and operation.target_type == "table"
- and operation.database is not None
- and operation.table is not None
- ):
- return (
- PermissionRequirement(
- action="alter-table",
- resource=TableResource(
- database=operation.database, table=operation.table
- ),
- ),
- )
- if (
- operation.operation == "drop"
- and operation.target_type == "table"
- and operation.database is not None
- and operation.table is not None
- ):
- return (
- PermissionRequirement(
- action="drop-table",
- resource=TableResource(
- database=operation.database, table=operation.table
- ),
- ),
- )
- if (
- operation.operation in {"create", "drop"}
- and operation.target_type == "index"
- and operation.database is not None
- and operation.table is not None
- ):
- return (
- PermissionRequirement(
- action="alter-table",
- resource=TableResource(
- database=operation.database, table=operation.table
- ),
- ),
- )
- return ()
-
-
-def operation_should_be_ignored(operation: Operation) -> bool:
- return operation.internal or operation.operation == "select"
-
-
-def operation_forbidden_message(operation: Operation) -> str | None:
- if operation.operation == "vacuum":
- return "VACUUM is not allowed in user-supplied SQL"
- return None
-
-
-def operation_is_write(operation: Operation) -> bool:
- return operation.operation in {
- "insert",
- "update",
- "delete",
- "create",
- "alter",
- "drop",
- "begin",
- "commit",
- "rollback",
- "savepoint",
- "attach",
- "detach",
- "pragma",
- "analyze",
- "reindex",
- "vacuum",
- "unknown",
- }
-
-
async def ensure_query_write_permissions(
- datasette: Datasette,
+ datasette: Any,
database: str,
sql: str,
*,
- actor: dict[str, object] | None = None,
- params: dict[str, object] | None = None,
- analysis: SQLAnalysis | None = None,
-) -> SQLAnalysis:
+ actor: dict[str, Any] | None = None,
+ params: dict[str, Any] | None = None,
+ analysis: Any = None,
+) -> Any:
+ write_actions = {
+ "insert": "insert-row",
+ "update": "update-row",
+ "delete": "delete-row",
+ }
db = datasette.get_database(database)
if analysis is None:
if params is None:
@@ -756,29 +606,18 @@ async def ensure_query_write_permissions(
except sqlite3.DatabaseError as ex:
raise Forbidden(f"Could not analyze query: {ex}") from ex
- for operation in analysis.operations:
- if operation_should_be_ignored(operation):
+ for access in analysis.table_accesses:
+ action = write_actions.get(access.operation)
+ if action is None:
continue
- forbidden_message = operation_forbidden_message(operation)
- if forbidden_message is not None:
- raise QueryWriteRejected(forbidden_message)
- permissions = permission_requirements_for_operation(operation)
- if not permissions:
+ if access.database != database:
+ raise Forbidden("Writable queries may not write to attached databases")
+ if not await datasette.allowed(
+ action=action,
+ resource=TableResource(database=access.database, table=access.table),
+ actor=actor,
+ ):
raise Forbidden(
- "Unsupported SQL operation: {} {}".format(
- operation.operation, operation.target_type
- )
+ f"Permission denied: need {action} on {access.database}/{access.table}"
)
- if operation.database != database:
- raise Forbidden("Writable queries may not access attached databases")
- 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
diff --git a/datasette/templates/query_create.html b/datasette/templates/query_create.html
index ec910456..f5dadbff 100644
--- a/datasette/templates/query_create.html
+++ b/datasette/templates/query_create.html
@@ -106,6 +106,9 @@ form.sql .query-create-sql textarea#sql-editor {
.query-create-analysis-note {
margin: 0;
}
+.query-create-action {
+ margin: 0.35rem 0 1rem;
+}
.query-create-analysis {
margin-top: 0.8rem;
}
@@ -168,6 +171,10 @@ form.sql .query-create-sql textarea#sql-editor {
Queries marked private can only be seen by you, their creator.
+ {% if sql and analysis_is_write %}
+ Execute write SQL
+ {% endif %}
+
diff --git a/datasette/utils/sql_analysis.py b/datasette/utils/sql_analysis.py
index f2eb903f..b5317b62 100644
--- a/datasette/utils/sql_analysis.py
+++ b/datasette/utils/sql_analysis.py
@@ -3,76 +3,22 @@ from typing import Literal
from datasette.utils.sqlite import sqlite3
-SQLOperation = Literal[
- "read",
- "insert",
- "update",
- "delete",
- "select",
- "function",
- "create",
- "alter",
- "drop",
- "begin",
- "commit",
- "rollback",
- "savepoint",
- "attach",
- "detach",
- "pragma",
- "analyze",
- "reindex",
- "vacuum",
- "unknown",
-]
-SQLTargetType = Literal[
- "table",
- "index",
- "view",
- "trigger",
- "virtual-table",
- "schema",
- "statement",
- "transaction",
- "database",
- "pragma",
- "function",
- "unknown",
-]
SQLTableOperation = Literal["read", "insert", "update", "delete"]
-SQLSchemaOperation = Literal["create", "drop"]
-SQLSchemaTargetType = Literal["index", "table", "trigger", "view", "virtual-table"]
@dataclass(frozen=True)
-class Operation:
- operation: SQLOperation
- target_type: SQLTargetType
+class SQLTableAccess:
+ operation: SQLTableOperation
database: str | None
- table: str | None
+ table: str
sqlite_schema: str | None
- target: str | None = None
columns: tuple[str, ...] = ()
source: str | None = None
- internal: bool = False
@dataclass(frozen=True)
class SQLAnalysis:
- operations: tuple[Operation, ...]
-
-
-# Hashable dict key for grouping repeated authorizer callbacks while collecting columns.
-@dataclass(frozen=True)
-class OperationKey:
- operation: SQLOperation
- target_type: SQLTargetType
- database: str | None
- table: str | None
- sqlite_schema: str | None
- target: str | None
- source: str | None
- internal: bool
+ table_accesses: tuple[SQLTableAccess, ...]
_ACTION_TO_OPERATION: dict[int, SQLTableOperation] = {
@@ -82,114 +28,6 @@ _ACTION_TO_OPERATION: dict[int, SQLTableOperation] = {
sqlite3.SQLITE_DELETE: "delete",
}
-# Values are (operation, target_type) pairs used to construct Operation objects.
-_CREATE_ACTIONS: dict[int, tuple[SQLSchemaOperation, SQLSchemaTargetType]] = {
- sqlite3.SQLITE_CREATE_INDEX: ("create", "index"),
- sqlite3.SQLITE_CREATE_TABLE: ("create", "table"),
- sqlite3.SQLITE_CREATE_TRIGGER: ("create", "trigger"),
- sqlite3.SQLITE_CREATE_VIEW: ("create", "view"),
-}
-_DROP_ACTIONS: dict[int, tuple[SQLSchemaOperation, SQLSchemaTargetType]] = {
- sqlite3.SQLITE_DROP_INDEX: ("drop", "index"),
- sqlite3.SQLITE_DROP_TABLE: ("drop", "table"),
- sqlite3.SQLITE_DROP_TRIGGER: ("drop", "trigger"),
- sqlite3.SQLITE_DROP_VIEW: ("drop", "view"),
-}
-
-
-def _add_schema_action(
- action_name: str,
- operation: SQLSchemaOperation,
- target_type: SQLSchemaTargetType,
-) -> None:
- action_value = getattr(sqlite3, action_name, None)
- if action_value is not None:
- actions = _CREATE_ACTIONS if operation == "create" else _DROP_ACTIONS
- actions[action_value] = (operation, target_type)
-
-
-_TEMP_SCHEMA_ACTIONS: tuple[
- tuple[str, SQLSchemaOperation, SQLSchemaTargetType], ...
-] = (
- ("SQLITE_CREATE_TEMP_INDEX", "create", "index"),
- ("SQLITE_CREATE_TEMP_TABLE", "create", "table"),
- ("SQLITE_CREATE_TEMP_TRIGGER", "create", "trigger"),
- ("SQLITE_CREATE_TEMP_VIEW", "create", "view"),
- ("SQLITE_DROP_TEMP_INDEX", "drop", "index"),
- ("SQLITE_DROP_TEMP_TABLE", "drop", "table"),
- ("SQLITE_DROP_TEMP_TRIGGER", "drop", "trigger"),
- ("SQLITE_DROP_TEMP_VIEW", "drop", "view"),
-)
-for schema_action in _TEMP_SCHEMA_ACTIONS:
- _add_schema_action(*schema_action)
-
-_VTABLE_SCHEMA_ACTIONS: tuple[
- tuple[str, SQLSchemaOperation, SQLSchemaTargetType], ...
-] = (
- ("SQLITE_CREATE_VTABLE", "create", "virtual-table"),
- ("SQLITE_DROP_VTABLE", "drop", "virtual-table"),
-)
-for schema_action in _VTABLE_SCHEMA_ACTIONS:
- _add_schema_action(*schema_action)
-
-_SQLITE_SCHEMA_TABLES = {
- "sqlite_master",
- "sqlite_schema",
- "sqlite_temp_master",
- "sqlite_temp_schema",
-}
-_SQLITE_INTERNAL_SCHEMA_FUNCTIONS = {
- "length",
- "like",
- "printf",
- "sqlite_drop_column",
- "sqlite_rename_column",
- "sqlite_rename_quotefix",
- "sqlite_rename_table",
- "sqlite_rename_test",
- "substr",
-}
-
-_AUTHORIZER_ACTION_NAMES = {
- getattr(sqlite3, name): name
- for name in (
- "SQLITE_CREATE_INDEX",
- "SQLITE_CREATE_TABLE",
- "SQLITE_CREATE_TEMP_INDEX",
- "SQLITE_CREATE_TEMP_TABLE",
- "SQLITE_CREATE_TEMP_TRIGGER",
- "SQLITE_CREATE_TEMP_VIEW",
- "SQLITE_CREATE_TRIGGER",
- "SQLITE_CREATE_VIEW",
- "SQLITE_DELETE",
- "SQLITE_DROP_INDEX",
- "SQLITE_DROP_TABLE",
- "SQLITE_DROP_TEMP_INDEX",
- "SQLITE_DROP_TEMP_TABLE",
- "SQLITE_DROP_TEMP_TRIGGER",
- "SQLITE_DROP_TEMP_VIEW",
- "SQLITE_DROP_TRIGGER",
- "SQLITE_DROP_VIEW",
- "SQLITE_INSERT",
- "SQLITE_PRAGMA",
- "SQLITE_READ",
- "SQLITE_SELECT",
- "SQLITE_TRANSACTION",
- "SQLITE_UPDATE",
- "SQLITE_ATTACH",
- "SQLITE_DETACH",
- "SQLITE_ALTER_TABLE",
- "SQLITE_REINDEX",
- "SQLITE_ANALYZE",
- "SQLITE_CREATE_VTABLE",
- "SQLITE_DROP_VTABLE",
- "SQLITE_FUNCTION",
- "SQLITE_SAVEPOINT",
- "SQLITE_RECURSIVE",
- )
- if hasattr(sqlite3, name)
-}
-
def analyze_sql_tables(
conn,
@@ -200,13 +38,15 @@ def analyze_sql_tables(
schema_to_database: dict[str, str] | None = None,
) -> SQLAnalysis:
"""
- Return operations performed by a SQL statement according to SQLite's authorizer.
+ Return tables accessed by a SQL statement according to SQLite's authorizer.
This function is synchronous and connection-based. It temporarily installs a
- SQLite authorizer, prepares ``EXPLAIN
``, and returns the operation
+ SQLite authorizer, prepares ``EXPLAIN ``, and returns the table access
callbacks observed while SQLite compiles the statement.
"""
- operations: dict[OperationKey, set[str]] = {}
+ accesses: dict[
+ tuple[SQLTableOperation, str | None, str, str | None, str | None], set[str]
+ ] = {}
def database_for_schema(sqlite_schema):
if schema_to_database and sqlite_schema in schema_to_database:
@@ -215,304 +55,45 @@ def analyze_sql_tables(
return database_name
return sqlite_schema
- def record(
- operation: SQLOperation,
- target_type: SQLTargetType,
- *,
- database: str | None,
- table: str | None,
- sqlite_schema: str | None,
- target: str | None,
- source: str | None,
- column: str | None = None,
- internal: bool = False,
- ):
- key = OperationKey(
- operation=operation,
- target_type=target_type,
- database=database,
- table=table,
- sqlite_schema=sqlite_schema,
- target=target,
- source=source,
- internal=internal,
- )
- columns = operations.setdefault(key, set())
- if column is not None:
- columns.add(column)
-
def authorizer(action, arg1, arg2, sqlite_schema, source):
operation = _ACTION_TO_OPERATION.get(action)
- if operation is not None and arg1 is not None:
- target_type = "schema" if arg1 in _SQLITE_SCHEMA_TABLES else "table"
- column = (
- arg2 if operation in ("read", "update") and arg2 is not None else None
- )
- record(
- operation,
- target_type,
- database=database_for_schema(sqlite_schema),
- table=arg1 if target_type == "table" else None,
- sqlite_schema=sqlite_schema,
- target=arg1,
- source=source,
- column=column,
- )
+ if operation is None or arg1 is None:
return sqlite3.SQLITE_OK
- create_operation = _CREATE_ACTIONS.get(action)
- if create_operation is not None and arg1 is not None:
- operation, target_type = create_operation
- related_table = arg2 if target_type in {"index", "trigger"} else arg1
- record(
- operation,
- target_type,
- database=database_for_schema(sqlite_schema),
- table=related_table,
- sqlite_schema=sqlite_schema,
- target=arg1,
- source=source,
- )
- return sqlite3.SQLITE_OK
-
- drop_operation = _DROP_ACTIONS.get(action)
- if drop_operation is not None and arg1 is not None:
- operation, target_type = drop_operation
- related_table = arg2 if target_type in {"index", "trigger"} else arg1
- record(
- operation,
- target_type,
- database=database_for_schema(sqlite_schema),
- table=related_table,
- sqlite_schema=sqlite_schema,
- target=arg1,
- source=source,
- )
- return sqlite3.SQLITE_OK
-
- if action == sqlite3.SQLITE_ALTER_TABLE and arg2 is not None:
- record(
- "alter",
- "table",
- database=database_for_schema(arg1),
- table=arg2,
- sqlite_schema=arg1,
- target=arg2,
- source=source,
- )
- return sqlite3.SQLITE_OK
-
- if action == sqlite3.SQLITE_TRANSACTION and arg1 is not None:
- record(
- arg1.lower(),
- "transaction",
- database=None,
- table=None,
- sqlite_schema=None,
- target=arg1,
- source=source,
- )
- return sqlite3.SQLITE_OK
-
- if action == sqlite3.SQLITE_ATTACH and arg1 is not None:
- record(
- "attach",
- "database",
- database=None,
- table=None,
- sqlite_schema=None,
- target=arg1,
- source=source,
- )
- return sqlite3.SQLITE_OK
-
- if action == sqlite3.SQLITE_DETACH and arg1 is not None:
- record(
- "detach",
- "database",
- database=None,
- table=None,
- sqlite_schema=None,
- target=arg1,
- source=source,
- )
- return sqlite3.SQLITE_OK
-
- if action == sqlite3.SQLITE_PRAGMA and arg1 is not None:
- record(
- "pragma",
- "pragma",
- database=None,
- table=None,
- sqlite_schema=sqlite_schema,
- target=arg1,
- source=source,
- )
- return sqlite3.SQLITE_OK
-
- if action == sqlite3.SQLITE_ANALYZE:
- record(
- "analyze",
- "database" if arg1 is None else "table",
- database=database_for_schema(sqlite_schema),
- table=arg1,
- sqlite_schema=sqlite_schema,
- target=arg1,
- source=source,
- )
- return sqlite3.SQLITE_OK
-
- if action == sqlite3.SQLITE_REINDEX and arg1 is not None:
- record(
- "reindex",
- "index",
- database=database_for_schema(sqlite_schema),
- table=None,
- sqlite_schema=sqlite_schema,
- target=arg1,
- source=source,
- )
- return sqlite3.SQLITE_OK
-
- if action == sqlite3.SQLITE_SELECT:
- record(
- "select",
- "statement",
- database=None,
- table=None,
- sqlite_schema=sqlite_schema,
- target=None,
- source=source,
- )
- return sqlite3.SQLITE_OK
-
- if action == sqlite3.SQLITE_FUNCTION and arg2 is not None:
- record(
- "function",
- "function",
- database=None,
- table=None,
- sqlite_schema=sqlite_schema,
- target=arg2,
- source=source,
- )
- return sqlite3.SQLITE_OK
-
- if action == sqlite3.SQLITE_SAVEPOINT and arg1 is not None:
- record(
- "savepoint",
- "transaction",
- database=None,
- table=None,
- sqlite_schema=sqlite_schema,
- target="{} {}".format(arg1, arg2) if arg2 is not None else arg1,
- source=source,
- )
- return sqlite3.SQLITE_OK
-
- action_name = _AUTHORIZER_ACTION_NAMES.get(action, "SQLITE_{}".format(action))
- record(
- "unknown",
- "unknown",
- database=database_for_schema(sqlite_schema),
- table=None,
- sqlite_schema=sqlite_schema,
- target=action_name,
- source=source,
+ key = (
+ operation,
+ database_for_schema(sqlite_schema),
+ arg1,
+ sqlite_schema,
+ source,
)
+ columns = accesses.setdefault(key, set())
+ if operation in ("read", "update") and arg2 is not None:
+ columns.add(arg2)
return sqlite3.SQLITE_OK
conn.set_authorizer(authorizer)
try:
- explain_rows = conn.execute(
- "EXPLAIN " + sql, params if params is not None else {}
- ).fetchall()
+ conn.execute("EXPLAIN " + sql, params if params is not None else {}).fetchall()
finally:
conn.set_authorizer(None)
- if not operations:
- vacuum_row = next((row for row in explain_rows if row[1] == "Vacuum"), None)
- if vacuum_row is not None:
- schema_by_index = {
- row[0]: row[1] for row in conn.execute("PRAGMA database_list")
- }
- sqlite_schema = schema_by_index.get(vacuum_row[2])
- database = database_for_schema(sqlite_schema)
- record(
- "vacuum",
- "database",
- database=database,
- table=None,
- sqlite_schema=sqlite_schema,
- target=database,
- source=None,
- )
- else:
- record(
- "unknown",
- "statement",
- database=database_name,
- table=None,
- sqlite_schema=None,
- target=None,
- source=None,
- )
-
- has_schema_operation = any(
- key.target_type in {"table", "index", "view", "trigger", "virtual-table"}
- and key.operation in {"create", "alter", "drop"}
- for key in operations
- )
- dropped_tables = {
- (key.database, key.table)
- for key in operations
- if key.operation == "drop" and key.target_type == "table"
- }
-
- def key_is_drop_table_delete(key: OperationKey) -> bool:
- return (
- key.operation == "delete"
- and key.target_type == "table"
- and (key.database, key.table) in dropped_tables
- )
-
- has_user_table_access_in_schema_operation = any(
- key.operation in {"read", "insert", "update", "delete"}
- and key.target_type == "table"
- and not key.internal
- and not key_is_drop_table_delete(key)
- for key in operations
- )
-
- def operation_is_internal(key: OperationKey) -> bool:
- if key.internal or (has_schema_operation and key.target_type == "schema"):
- return True
- if has_schema_operation and key.operation == "reindex":
- return True
- if (
- has_schema_operation
- and not has_user_table_access_in_schema_operation
- and key.operation == "function"
- and key.target in _SQLITE_INTERNAL_SCHEMA_FUNCTIONS
- ):
- return True
- if key_is_drop_table_delete(key):
- return True
- return False
-
return SQLAnalysis(
- operations=tuple(
- Operation(
- operation=key.operation,
- target_type=key.target_type,
- database=key.database,
- table=key.table,
- sqlite_schema=key.sqlite_schema,
- target=key.target,
+ table_accesses=tuple(
+ SQLTableAccess(
+ operation=operation,
+ database=database,
+ table=table,
+ sqlite_schema=sqlite_schema,
columns=tuple(sorted(columns)),
- source=key.source,
- internal=operation_is_internal(key),
+ source=source,
)
- for key, columns in operations.items()
+ for (
+ operation,
+ database,
+ table,
+ sqlite_schema,
+ source,
+ ), columns in accesses.items()
)
)
diff --git a/datasette/views/database.py b/datasette/views/database.py
index ae1cf375..b558b002 100644
--- a/datasette/views/database.py
+++ b/datasette/views/database.py
@@ -13,7 +13,7 @@ import textwrap
from datasette.events import AlterTableEvent, CreateTableEvent, InsertRowsEvent
from datasette.database import QueryInterrupted
from datasette.resources import DatabaseResource, QueryResource
-from datasette.stored_queries import QueryWriteRejected, stored_query_to_dict
+from datasette.stored_queries import stored_query_to_dict
from datasette.utils import (
add_cors_headers,
await_me_maybe,
@@ -453,24 +453,9 @@ class QueryView(View):
):
raise Forbidden("You do not have permission to view this query")
- try:
- await _ensure_stored_query_execution_permissions(
- datasette, db, stored_query, request.actor
- )
- except QueryWriteRejected as ex:
- if request.headers.get("accept") == "application/json" or request.args.get(
- "_json"
- ):
- return Response.json(
- {
- "ok": False,
- "message": ex.message,
- "redirect": None,
- },
- status=403,
- )
- datasette.add_message(request, ex.message, datasette.ERROR)
- return Response.redirect(stored_query.on_error_redirect or request.path)
+ await _ensure_stored_query_execution_permissions(
+ datasette, db, stored_query, request.actor
+ )
# If database is immutable, return an error
if not db.is_mutable:
diff --git a/datasette/views/execute_write.py b/datasette/views/execute_write.py
index 57c4d78e..0054300c 100644
--- a/datasette/views/execute_write.py
+++ b/datasette/views/execute_write.py
@@ -99,7 +99,9 @@ class ExecuteWriteView(BaseView):
"parameter_names": parameter_names,
"parameter_values": parameter_values,
"analysis_error": analysis_error,
- "analysis_rows": analysis_rows,
+ "analysis_rows": [
+ row for row in analysis_rows if row["operation"] != "read"
+ ],
"execution_message": execution_message,
"execution_links": execution_links,
"execution_ok": execution_ok,
@@ -163,15 +165,13 @@ class ExecuteWriteView(BaseView):
except QueryValidationError as ex:
if _wants_json(request, is_json, data):
return _block_framing(_error([ex.message], ex.status))
- if ex.flash:
- self.ds.add_message(request, ex.message, self.ds.ERROR)
return await self._render_form(
request,
db,
sql=sql or "",
parameter_values=provided_params,
- analysis_error=None if ex.flash else ex.message,
- execution_message=None if ex.flash else ex.message,
+ analysis_error=ex.message,
+ execution_message=ex.message,
execution_ok=False,
status=ex.status,
)
@@ -193,12 +193,9 @@ class ExecuteWriteView(BaseView):
status=400,
)
- if cursor.rowcount == -1:
- message = "Query executed"
- else:
- message = "Query executed, {} row{} affected".format(
- cursor.rowcount, "" if cursor.rowcount == 1 else "s"
- )
+ message = "Query executed, {} row{} affected".format(
+ cursor.rowcount, "" if cursor.rowcount == 1 else "s"
+ )
if _wants_json(request, is_json, data):
return _block_framing(
Response.json(
diff --git a/datasette/views/query_helpers.py b/datasette/views/query_helpers.py
index 92328ff3..46d71b8e 100644
--- a/datasette/views/query_helpers.py
+++ b/datasette/views/query_helpers.py
@@ -1,14 +1,8 @@
import json
import re
-from datasette.resources import DatabaseResource
-from datasette.stored_queries import (
- QueryWriteRejected,
- StoredQuery,
- operation_is_write,
- operation_should_be_ignored,
- permission_requirements_for_operation,
-)
+from datasette.resources import DatabaseResource, TableResource
+from datasette.stored_queries import StoredQuery
from datasette.utils import (
named_parameters as derive_named_parameters,
escape_sqlite,
@@ -18,7 +12,6 @@ from datasette.utils import (
InvalidSql,
)
from datasette.utils.asgi import Forbidden
-from datasette.utils.sql_analysis import Operation, SQLAnalysis
_query_name_re = re.compile(r"^[^/\.\n]+$")
@@ -48,11 +41,9 @@ _query_write_fields = {
class QueryValidationError(Exception):
- def __init__(self, message, status=400, *, flash=False):
+ def __init__(self, message, status=400):
self.message = message
self.status = status
- self.flash = flash
- super().__init__(message)
def _actor_id(actor):
@@ -132,8 +123,11 @@ def _coerce_query_parameters(value, derived):
return parameters
-def _analysis_is_write(analysis: SQLAnalysis) -> bool:
- return any(operation_is_write(operation) for operation in analysis.operations)
+def _analysis_is_write(analysis):
+ return any(
+ access.operation in {"insert", "update", "delete"}
+ for access in analysis.table_accesses
+ )
def _block_framing(response):
@@ -197,8 +191,6 @@ async def _analyze_user_query(datasette, db, sql, *, actor):
await datasette.ensure_query_write_permissions(
db.name, sql, actor=actor, analysis=analysis
)
- except QueryWriteRejected as ex:
- raise QueryValidationError(ex.message, status=403, flash=True) from ex
except Forbidden as ex:
raise QueryValidationError(str(ex), status=403) from ex
else:
@@ -209,51 +201,34 @@ async def _analyze_user_query(datasette, db, sql, *, actor):
return is_write, derived, analysis
-def _display_operations(analysis: SQLAnalysis) -> list[Operation]:
- operations = []
- for operation in analysis.operations:
- if operation_should_be_ignored(operation):
- continue
- operations.append(operation)
- return operations
+def _analysis_rows(analysis):
+ write_actions = {
+ "insert": "insert-row",
+ "update": "update-row",
+ "delete": "delete-row",
+ }
+ return [
+ {
+ "operation": access.operation,
+ "database": access.database,
+ "table": access.table,
+ "required_permission": write_actions.get(access.operation, ""),
+ "source": access.source,
+ }
+ for access in analysis.table_accesses
+ ]
-def _analysis_rows(analysis: SQLAnalysis) -> list[dict[str, object]]:
- rows = []
- for operation in _display_operations(analysis):
- permissions = permission_requirements_for_operation(operation)
- required_permission = ", ".join(permission.action for permission in permissions)
- rows.append(
- {
- "operation": operation.operation,
- "database": operation.database,
- "table": operation.table or operation.target,
- "required_permission": required_permission,
- "source": operation.source,
- }
- )
- return rows
-
-
-async def _analysis_rows_with_permissions(
- datasette, analysis: SQLAnalysis, actor
-) -> list[dict[str, object]]:
+async def _analysis_rows_with_permissions(datasette, analysis, actor):
rows = _analysis_rows(analysis)
- is_write = _analysis_is_write(analysis)
- for row, operation in zip(rows, _display_operations(analysis)):
- 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
+ for row in rows:
+ permission = row["required_permission"]
+ if permission:
+ row["allowed"] = await datasette.allowed(
+ action=permission,
+ resource=TableResource(row["database"], row["table"]),
+ actor=actor,
+ )
else:
row["allowed"] = None
return rows
@@ -302,8 +277,6 @@ async def _prepare_execute_write(datasette, db, sql, params, actor):
await datasette.ensure_query_write_permissions(
db.name, sql, actor=actor, analysis=analysis
)
- except QueryWriteRejected as ex:
- raise QueryValidationError(ex.message, status=403, flash=True) from ex
except Forbidden as ex:
raise QueryValidationError(str(ex), status=403) from ex
return parameter_names, params, analysis
@@ -353,7 +326,7 @@ async def _execute_write_analysis_data(datasette, db, sql, actor):
"ok": analysis_error is None,
"parameters": parameter_names,
"analysis_error": analysis_error,
- "analysis_rows": analysis_rows,
+ "analysis_rows": [row for row in analysis_rows if row["operation"] != "read"],
"execute_disabled": bool(
(not sql)
or analysis_error
@@ -367,7 +340,6 @@ async def _query_create_analysis_data(datasette, db, sql, actor):
parameter_names = []
analysis_rows = []
analysis_error = None
- analysis: SQLAnalysis | None = None
if has_sql:
try:
parameter_names = _derived_query_parameters(sql)
@@ -384,7 +356,9 @@ async def _query_create_analysis_data(datasette, db, sql, actor):
"analysis_error": analysis_error,
"analysis_rows": analysis_rows,
"has_sql": has_sql,
- "analysis_is_write": _analysis_is_write(analysis) if analysis else False,
+ "analysis_is_write": bool(
+ analysis_rows and any(row["required_permission"] for row in analysis_rows)
+ ),
"save_disabled": bool(
(not has_sql)
or analysis_error
@@ -424,19 +398,15 @@ async def _inserted_row_url(datasette, db, analysis, cursor):
if lastrowid is None:
return None
direct_inserts = [
- operation
- for operation in analysis.operations
- if operation.operation == "insert"
- and operation.target_type == "table"
- and not operation.internal
- and operation.source is None
- and operation.database == db.name
+ access
+ for access in analysis.table_accesses
+ if access.operation == "insert"
+ and access.source is None
+ and access.database == db.name
]
if len(direct_inserts) != 1:
return None
table = direct_inserts[0].table
- if table is None:
- return None
pks = await db.primary_keys(table)
use_rowid = not pks
select = (
diff --git a/docs/json_api.rst b/docs/json_api.rst
index fffc16d7..48c70af6 100644
--- a/docs/json_api.rst
+++ b/docs/json_api.rst
@@ -505,68 +505,6 @@ The JSON write API
Datasette provides a write API for JSON data. This is a POST-only API that requires an authenticated API token, see :ref:`CreateTokenView`. The token will need to have the specified :ref:`authentication_permissions`.
-.. _ExecuteWriteView:
-
-Executing write SQL
-~~~~~~~~~~~~~~~~~~~
-
-Actors with the :ref:`actions_execute_write_sql` permission can execute arbitrary writable SQL against a mutable database using ``/-/execute-write``.
-
-::
-
- POST //-/execute-write
- Content-Type: application/json
- Authorization: Bearer dstok_
-
-The request body must include a ``"sql"`` string. Named SQL parameters can be provided using the optional ``"params"`` object:
-
-.. code-block:: json
-
- {
- "sql": "insert into dogs (name) values (:name)",
- "params": {
- "name": "Cleo"
- }
- }
-
-The SQL must be writable. Read-only ``select`` queries should use the regular :ref:`custom SQL query API ` instead.
-
-Datasette analyzes the SQL before executing it. The actor must have ``execute-write-sql`` permission for the database, and must also have any permissions required by the operations in the SQL. For example, inserts and updates against a table require ``insert-row``, ``update-row`` and ``delete-row`` permissions for that table. Reads performed as part of the write, such as ``insert into dogs select ... from other_table``, require ``view-table`` permission on the source table.
-
-A successful response includes a message, the SQLite ``rowcount`` and a summary of the operations that were executed:
-
-The shape of the ``"analysis"`` block is not yet considered a stable API and may change in future Datasette releases.
-
-.. code-block:: json
-
- {
- "ok": true,
- "message": "Query executed, 1 row affected",
- "rowcount": 1,
- "analysis": [
- {
- "operation": "insert",
- "database": "data",
- "table": "dogs",
- "required_permission": "insert-row, update-row, delete-row",
- "source": null
- }
- ]
- }
-
-If SQLite reports ``-1`` for the row count, the message will be ``"Query executed"``.
-
-Errors use the standard Datasette error format:
-
-.. code-block:: json
-
- {
- "ok": false,
- "errors": [
- "Permission denied: need execute-write-sql"
- ]
- }
-
.. _TableInsertView:
Inserting rows
diff --git a/tests/test_actions_sql.py b/tests/test_actions_sql.py
index a1fca971..863d2529 100644
--- a/tests/test_actions_sql.py
+++ b/tests/test_actions_sql.py
@@ -12,22 +12,10 @@ import pytest
import pytest_asyncio
from datasette.app import Datasette
from datasette.permissions import PermissionSQL
-from datasette.resources import DatabaseResource, QueryResource, TableResource
+from datasette.resources import TableResource
from datasette import hookimpl
-def test_resource_string_representations():
- assert str(DatabaseResource("content")) == "content"
- assert repr(DatabaseResource("content")) == (
- "DatabaseResource(parent='content', child=None)"
- )
- assert str(TableResource("content", "dogs")) == "content/dogs"
- assert repr(TableResource("content", "dogs")) == (
- "TableResource(parent='content', child='dogs')"
- )
- assert str(QueryResource("content", "insert-a-dog")) == "content/insert-a-dog"
-
-
# Test plugin that provides permission rules
class PermissionRulesPlugin:
def __init__(self, rules_callback):
diff --git a/tests/test_internals_database.py b/tests/test_internals_database.py
index d6e130b4..5481a398 100644
--- a/tests/test_internals_database.py
+++ b/tests/test_internals_database.py
@@ -698,17 +698,14 @@ async def test_analyze_sql():
assert [
(
- operation.operation,
- operation.database,
- operation.sqlite_schema,
- operation.table,
- operation.columns,
- operation.source,
+ access.operation,
+ access.database,
+ access.sqlite_schema,
+ access.table,
+ access.columns,
+ access.source,
)
- for operation in analysis.operations
- if operation.target_type == "table"
- and operation.operation in {"read", "insert", "update", "delete"}
- and not operation.internal
+ for access in analysis.table_accesses
] == [
("read", "data", "main", "dogs", ("id", "name"), None),
]
@@ -725,17 +722,14 @@ async def test_analyze_sql_insert_select():
assert {
(
- operation.operation,
- operation.database,
- operation.sqlite_schema,
- operation.table,
- operation.columns,
- operation.source,
+ access.operation,
+ access.database,
+ access.sqlite_schema,
+ access.table,
+ access.columns,
+ access.source,
)
- for operation in analysis.operations
- if operation.target_type == "table"
- and operation.operation in {"read", "insert", "update", "delete"}
- and not operation.internal
+ for access in analysis.table_accesses
} == {
("insert", "data", "main", "dogs", (), None),
("read", "data", "main", "cats", ("name",), None),
diff --git a/tests/test_queries.py b/tests/test_queries.py
index b6e1637d..59fab8c0 100644
--- a/tests/test_queries.py
+++ b/tests/test_queries.py
@@ -508,8 +508,6 @@ async def test_analyze_write_query_requires_table_permissions():
"dogs": {
"permissions": {
"insert-row": {"id": "writer"},
- "update-row": {"id": "writer"},
- "delete-row": {"id": "writer"},
}
}
}
@@ -1183,10 +1181,11 @@ async def test_create_query_ui_and_arbitrary_sql_save_link():
assert 'Required permission | ' in create_response.text
assert 'Source | ' not in create_response.text
assert "read | " in create_response.text
- assert "view-table | " in create_response.text
assert (
- 'n/a | '
- not in create_response.text
+ create_response.text.count(
+ 'n/a | '
+ )
+ == 2
)
assert create_response.text.index(
'value="Save query"'
@@ -1256,9 +1255,9 @@ async def test_create_query_analyze_endpoint_uses_sql_only():
"operation": "read",
"database": "data",
"table": "dogs",
- "required_permission": "view-table",
+ "required_permission": "",
"source": None,
- "allowed": True,
+ "allowed": None,
}
]
@@ -1376,8 +1375,7 @@ async def test_execute_write_get_prepopulates_without_executing():
assert 'Required permission | ' in response.text
assert "insert | " in response.text
assert "update | " in response.text
- assert "read | " in response.text
- assert "view-table | " in response.text
+ assert "read | " not in response.text
assert 'action="/data/-/execute-write"' in response.text
assert "insert into dogs (name) values ('Cleo')" in response.text
assert (await db.execute("select count(*) from dogs")).first()[0] == 0
@@ -1431,7 +1429,7 @@ async def test_execute_write_analyze_endpoint_uses_sql_only():
"operation": "insert",
"database": "data",
"table": "dogs",
- "required_permission": "insert-row, update-row, delete-row",
+ "required_permission": "insert-row",
"source": None,
"allowed": True,
}
@@ -1629,40 +1627,6 @@ 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"},
@@ -1679,687 +1643,6 @@ 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(
- memory=True,
- default_deny=True,
- config={
- "databases": {
- "data": {
- "permissions": {
- "view-database": {"id": "writer"},
- "execute-write-sql": {"id": "writer"},
- },
- "tables": {
- "secret": {
- "permissions": {"view-table": {"id": "someone-else"}}
- },
- "public_log": {
- "permissions": {
- "insert-row": {"id": "writer"},
- "update-row": {"id": "writer"},
- "delete-row": {"id": "writer"},
- }
- },
- },
- }
- }
- },
- )
- db = ds.add_memory_database("execute_write_insert_select_source", name="data")
- await db.execute_write("create table secret (value text)")
- await db.execute_write("create table public_log (value text)")
- await db.execute_write("insert into secret values ('sensitive')")
- await ds.invoke_startup()
-
- denied_response = await ds.client.post(
- "/data/-/execute-write",
- actor={"id": "writer"},
- json={"sql": "insert into public_log(value) select value from secret"},
- )
-
- assert denied_response.status_code == 403
- assert denied_response.json()["errors"] == [
- "Permission denied: need view-table on data/secret"
- ]
- assert (await db.execute("select value from public_log")).dicts() == []
-
-
-@pytest.mark.asyncio
-async def test_execute_write_rejects_sqlite_master_reads():
- ds = Datasette(
- memory=True,
- default_deny=True,
- config={
- "databases": {
- "data": {
- "permissions": {
- "view-database": {"id": "writer"},
- "execute-write-sql": {"id": "writer"},
- },
- "tables": {
- "secret": {
- "permissions": {"view-table": {"id": "someone-else"}}
- },
- "log": {
- "permissions": {
- "insert-row": {"id": "writer"},
- "update-row": {"id": "writer"},
- "delete-row": {"id": "writer"},
- }
- },
- },
- }
- }
- },
- )
- db = ds.add_memory_database("execute_write_sqlite_master_read", name="data")
- await db.execute_write("create table secret (value text)")
- await db.execute_write("create table log (value text)")
- await ds.invoke_startup()
-
- denied_response = await ds.client.post(
- "/data/-/execute-write",
- actor={"id": "writer"},
- json={
- "sql": (
- "insert into log " "select sql from sqlite_master where name = 'secret'"
- )
- },
- )
-
- assert denied_response.status_code == 403
- assert denied_response.json()["errors"] == [
- "Unsupported SQL operation: read schema"
- ]
- assert (await db.execute("select value from log")).dicts() == []
-
-
-@pytest.mark.asyncio
-async def test_execute_write_create_table_as_select_requires_view_table_on_source():
- ds = Datasette(
- memory=True,
- default_deny=True,
- config={
- "databases": {
- "data": {
- "permissions": {
- "view-database": {"id": "creator"},
- "execute-write-sql": {"id": "creator"},
- "create-table": {"id": "creator"},
- },
- "tables": {
- "secret": {
- "permissions": {"view-table": {"id": "someone-else"}}
- }
- },
- }
- }
- },
- )
- db = ds.add_memory_database("execute_write_create_as_select_source", name="data")
- await db.execute_write("create table secret (value text)")
- await db.execute_write("insert into secret values ('sensitive')")
- await ds.invoke_startup()
-
- denied_response = await ds.client.post(
- "/data/-/execute-write",
- actor={"id": "creator"},
- json={"sql": "create table copied_secret as select value from secret"},
- )
-
- assert denied_response.status_code == 403
- assert denied_response.json()["errors"] == [
- "Permission denied: need view-table on data/secret"
- ]
- assert not await db.table_exists("copied_secret")
-
-
-@pytest.mark.asyncio
-async def test_execute_write_rejects_function_operations():
- ds = Datasette(
- memory=True,
- default_deny=True,
- config={
- "databases": {
- "data": {
- "permissions": {
- "view-database": {"id": "writer"},
- "execute-write-sql": {"id": "writer"},
- },
- "tables": {
- "dogs": {
- "permissions": {
- "insert-row": {"id": "writer"},
- "update-row": {"id": "writer"},
- "delete-row": {"id": "writer"},
- }
- }
- },
- }
- }
- },
- )
- db = ds.add_memory_database("execute_write_function_operation", name="data")
- await db.execute_write("create table dogs (id integer primary key, name text)")
- await ds.invoke_startup()
-
- denied_response = await ds.client.post(
- "/data/-/execute-write",
- actor={"id": "writer"},
- json={"sql": "insert into dogs (name) values (upper('cleo'))"},
- )
-
- assert denied_response.status_code == 403
- assert denied_response.json()["errors"] == [
- "Unsupported SQL operation: function function"
- ]
- assert (await db.execute("select name from dogs")).dicts() == []
-
-
-@pytest.mark.asyncio
-async def test_execute_write_rejects_vacuum_operation():
- ds = Datasette(
- memory=True,
- default_deny=True,
- config={
- "databases": {
- "data": {
- "permissions": {
- "view-database": {"id": "writer"},
- "execute-write-sql": {"id": "writer"},
- }
- }
- }
- },
- )
- ds.add_memory_database("execute_write_vacuum_operation", name="data")
- await ds.invoke_startup()
-
- denied_response = await ds.client.post(
- "/data/-/execute-write",
- actor={"id": "writer"},
- json={"sql": "vacuum"},
- )
-
- assert denied_response.status_code == 403
- assert denied_response.json()["errors"] == [
- "VACUUM is not allowed in user-supplied SQL"
- ]
-
-
-@pytest.mark.asyncio
-async def test_execute_write_form_rejects_vacuum_operation_with_flash_error():
- ds = Datasette(
- memory=True,
- default_deny=True,
- config={
- "databases": {
- "data": {
- "permissions": {
- "view-database": {"id": "writer"},
- "execute-write-sql": {"id": "writer"},
- }
- }
- }
- },
- )
- ds.add_memory_database("execute_write_vacuum_operation_form", name="data")
- await ds.invoke_startup()
-
- denied_response = await ds.client.post(
- "/data/-/execute-write",
- actor={"id": "writer"},
- data={"sql": "vacuum"},
- )
-
- assert denied_response.status_code == 403
- assert (
- 'VACUUM is not allowed in user-supplied SQL
'
- in denied_response.text
- )
- assert denied_response.text.count("VACUUM is not allowed in user-supplied SQL") == 1
-
-
-@pytest.mark.asyncio
-async def test_untrusted_stored_write_query_rejects_vacuum_operation():
- ds = Datasette(
- memory=True,
- default_deny=True,
- config={
- "databases": {
- "data": {
- "permissions": {
- "view-database": {"id": "writer"},
- "view-query": {"id": "writer"},
- "execute-write-sql": {"id": "writer"},
- }
- }
- }
- },
- )
- ds.add_memory_database("stored_query_vacuum_operation", name="data")
- await ds.invoke_startup()
- await ds.add_query(
- "data",
- "vacuum_db",
- "vacuum",
- is_write=True,
- is_trusted=False,
- source="user",
- owner_id="writer",
- )
-
- denied_response = await ds.client.post(
- "/data/vacuum_db?_json=1",
- actor={"id": "writer"},
- data={},
- )
-
- assert denied_response.status_code == 403
- assert "VACUUM is not allowed in user-supplied SQL" in denied_response.text
-
-
-@pytest.mark.asyncio
-async def test_untrusted_stored_write_query_rejects_vacuum_operation_with_flash_error():
- ds = Datasette(
- memory=True,
- default_deny=True,
- config={
- "databases": {
- "data": {
- "permissions": {
- "view-database": {"id": "writer"},
- "view-query": {"id": "writer"},
- "execute-write-sql": {"id": "writer"},
- }
- }
- }
- },
- )
- ds.add_memory_database("stored_query_vacuum_operation_form", name="data")
- await ds.invoke_startup()
- await ds.add_query(
- "data",
- "vacuum_db",
- "vacuum",
- is_write=True,
- is_trusted=False,
- source="user",
- owner_id="writer",
- )
-
- denied_response = await ds.client.post(
- "/data/vacuum_db",
- actor={"id": "writer"},
- data={},
- )
-
- assert denied_response.status_code == 302
- assert denied_response.headers["location"] == "/data/vacuum_db"
- assert ds.unsign(denied_response.cookies["ds_messages"], "messages") == [
- ["VACUUM is not allowed in user-supplied SQL", ds.ERROR]
- ]
-
-
-@pytest.mark.asyncio
-async def test_trusted_stored_write_query_skips_vacuum_filtering():
- ds = Datasette(
- memory=True,
- default_deny=True,
- config={
- "databases": {
- "data": {
- "permissions": {
- "view-database": {"id": "writer"},
- "view-query": {"id": "writer"},
- }
- }
- }
- },
- )
- ds.add_memory_database("trusted_stored_query_vacuum", name="data")
- await ds.invoke_startup()
- await ds.add_query(
- "data",
- "trusted_vacuum",
- "vacuum",
- is_write=True,
- is_trusted=True,
- source="config",
- )
-
- response = await ds.client.post(
- "/data/trusted_vacuum?_json=1",
- actor={"id": "writer"},
- data={},
- )
-
- assert response.status_code == 200
- assert response.json()["ok"] is True
-
-
-@pytest.mark.asyncio
-async def test_execute_write_create_table_uses_create_table_permission():
- ds = Datasette(
- memory=True,
- default_deny=True,
- config={
- "permissions": {
- "insert-row": {"id": "row-writer"},
- "update-row": {"id": "row-writer"},
- },
- "databases": {
- "data": {
- "permissions": {
- "view-database": {"id": ["creator", "row-writer"]},
- "execute-write-sql": {"id": ["creator", "row-writer"]},
- "create-table": {"id": "creator"},
- }
- }
- },
- },
- )
- db = ds.add_memory_database("execute_write_create_table", name="data")
- await ds.invoke_startup()
-
- analysis_response = await ds.client.get(
- "/data/-/execute-write/analyze",
- actor={"id": "creator"},
- params={"sql": "create table foobar (id integer primary key, name text)"},
- )
- allowed_response = await ds.client.post(
- "/data/-/execute-write",
- actor={"id": "creator"},
- json={"sql": "create table foobar (id integer primary key, name text)"},
- )
- row_permission_response = await ds.client.post(
- "/data/-/execute-write",
- actor={"id": "row-writer"},
- json={"sql": "create table should_not_exist (id integer primary key)"},
- )
-
- assert analysis_response.status_code == 200
- analysis_data = analysis_response.json()
- assert analysis_data["ok"] is True
- assert analysis_data["execute_disabled"] is False
- assert analysis_data["analysis_rows"] == [
- {
- "operation": "create",
- "database": "data",
- "table": "foobar",
- "required_permission": "create-table",
- "source": None,
- "allowed": True,
- }
- ]
-
- assert allowed_response.status_code == 200
- assert allowed_response.json()["ok"] is True
- assert allowed_response.json()["message"] == "Query executed"
- assert await db.table_exists("foobar")
-
- assert row_permission_response.status_code == 403
- assert row_permission_response.json()["errors"] == [
- "Permission denied: need create-table on data"
- ]
- assert not await db.table_exists("should_not_exist")
-
-
-@pytest.mark.asyncio
-async def test_execute_write_alter_and_drop_table_use_schema_permissions():
- ds = Datasette(
- memory=True,
- default_deny=True,
- config={
- "permissions": {
- "delete-row": {"id": "row-writer"},
- "update-row": {"id": "row-writer"},
- },
- "databases": {
- "data": {
- "permissions": {
- "view-database": {"id": ["alterer", "dropper", "row-writer"]},
- "execute-write-sql": {
- "id": ["alterer", "dropper", "row-writer"]
- },
- },
- "tables": {
- "dogs": {
- "permissions": {
- "alter-table": {"id": "alterer"},
- "drop-table": {"id": "dropper"},
- "view-table": {"id": "alterer"},
- }
- }
- },
- }
- },
- },
- )
- db = ds.add_memory_database("execute_write_alter_drop_table", name="data")
- await db.execute_write("create table dogs (id integer primary key, name text)")
- await db.execute_write("create table cats (id integer primary key, name text)")
- await ds.invoke_startup()
-
- alter_allowed_response = await ds.client.post(
- "/data/-/execute-write",
- actor={"id": "alterer"},
- json={"sql": "alter table dogs add column age integer"},
- )
- alter_row_permission_response = await ds.client.post(
- "/data/-/execute-write",
- actor={"id": "row-writer"},
- json={"sql": "alter table cats add column age integer"},
- )
-
- assert alter_allowed_response.status_code == 200
- assert "age" in [column.name for column in await db.table_column_details("dogs")]
- assert alter_row_permission_response.status_code == 403
- assert alter_row_permission_response.json()["errors"] == [
- "Permission denied: need alter-table on data/cats"
- ]
- assert "age" not in [
- column.name for column in await db.table_column_details("cats")
- ]
-
- create_index_allowed_response = await ds.client.post(
- "/data/-/execute-write",
- actor={"id": "alterer"},
- json={"sql": "create index idx_dogs_name on dogs(name)"},
- )
- create_index_row_permission_response = await ds.client.post(
- "/data/-/execute-write",
- actor={"id": "row-writer"},
- json={"sql": "create index idx_cats_name on cats(name)"},
- )
- drop_index_allowed_response = await ds.client.post(
- "/data/-/execute-write",
- actor={"id": "alterer"},
- json={"sql": "drop index idx_dogs_name"},
- )
-
- assert create_index_allowed_response.status_code == 200
- assert create_index_row_permission_response.status_code == 403
- assert create_index_row_permission_response.json()["errors"] == [
- "Permission denied: need alter-table on data/cats"
- ]
- assert drop_index_allowed_response.status_code == 200
-
- drop_allowed_response = await ds.client.post(
- "/data/-/execute-write",
- actor={"id": "dropper"},
- json={"sql": "drop table dogs"},
- )
- drop_row_permission_response = await ds.client.post(
- "/data/-/execute-write",
- actor={"id": "row-writer"},
- json={"sql": "drop table cats"},
- )
-
- assert drop_allowed_response.status_code == 200
- assert not await db.table_exists("dogs")
- assert drop_row_permission_response.status_code == 403
- assert drop_row_permission_response.json()["errors"] == [
- "Permission denied: need drop-table on data/cats"
- ]
- assert await db.table_exists("cats")
-
-
@pytest.mark.asyncio
async def test_execute_write_insert_links_to_inserted_row():
ds = Datasette(memory=True, default_deny=True)
@@ -2546,8 +1829,6 @@ async def test_user_writable_query_execution_rechecks_table_permissions():
"dogs": {
"permissions": {
"insert-row": {"id": "alice"},
- "update-row": {"id": "alice"},
- "delete-row": {"id": "alice"},
}
}
},
diff --git a/tests/test_utils_sql_analysis.py b/tests/test_utils_sql_analysis.py
index df4b3625..5730cd0d 100644
--- a/tests/test_utils_sql_analysis.py
+++ b/tests/test_utils_sql_analysis.py
@@ -26,20 +26,17 @@ def conn():
conn.close()
-def table_operation_tuples(analysis):
+def as_tuples(analysis):
return [
(
- operation.operation,
- operation.database,
- operation.sqlite_schema,
- operation.table,
- operation.columns,
- operation.source,
+ access.operation,
+ access.database,
+ access.sqlite_schema,
+ access.table,
+ access.columns,
+ access.source,
)
- for operation in analysis.operations
- if operation.target_type == "table"
- and operation.operation in {"read", "insert", "update", "delete"}
- and not operation.internal
+ for access in analysis.table_accesses
]
@@ -51,7 +48,7 @@ def test_analyze_select_tables(conn):
database_name="data",
)
- assert set(table_operation_tuples(analysis)) == {
+ assert set(as_tuples(analysis)) == {
("read", "data", "main", "cats", ("id", "name"), None),
("read", "data", "main", "dogs", ("age", "id", "name"), None),
}
@@ -60,231 +57,11 @@ def test_analyze_select_tables(conn):
def test_analyze_uses_sqlite_schema_as_default_database(conn):
analysis = analyze_sql_tables(conn, "select name from dogs")
- assert set(table_operation_tuples(analysis)) == {
+ assert set(as_tuples(analysis)) == {
("read", "main", "main", "dogs", ("name",), None),
}
-def test_analyze_user_schema_table_read_is_not_internal(conn):
- analysis = analyze_sql_tables(
- conn,
- "insert into log select sql from sqlite_master where name = 'dogs'",
- database_name="data",
- )
-
- assert {
- "operation": "read",
- "target_type": "schema",
- "database": "data",
- "sqlite_schema": "main",
- "table": None,
- "target": "sqlite_master",
- "columns": ("name", "sql"),
- "source": None,
- "internal": False,
- } in [operation_dict(operation) for operation in analysis.operations]
-
-
-def operation_dict(operation):
- return {
- "operation": operation.operation,
- "target_type": operation.target_type,
- "database": operation.database,
- "sqlite_schema": operation.sqlite_schema,
- "table": operation.table,
- "target": operation.target,
- "columns": operation.columns,
- "source": operation.source,
- "internal": operation.internal,
- }
-
-
-def test_analyze_create_table_operation():
- conn = sqlite3.connect(":memory:")
- try:
- analysis = analyze_sql_tables(
- conn,
- "create table foobar (id integer primary key, name text)",
- database_name="data",
- )
- finally:
- conn.close()
-
- assert {
- "operation": "create",
- "target_type": "table",
- "database": "data",
- "sqlite_schema": "main",
- "table": "foobar",
- "target": "foobar",
- "columns": (),
- "source": None,
- "internal": False,
- } in [operation_dict(operation) for operation in analysis.operations]
- assert not [
- operation
- for operation in analysis.operations
- if operation.table in {"sqlite_master", "sqlite_schema"}
- and not operation.internal
- ]
-
-
-def test_analyze_vacuum_operation():
- conn = sqlite3.connect(":memory:")
- try:
- analysis = analyze_sql_tables(conn, "vacuum", database_name="data")
- finally:
- conn.close()
-
- assert [operation_dict(operation) for operation in analysis.operations] == [
- {
- "operation": "vacuum",
- "target_type": "database",
- "database": "data",
- "sqlite_schema": "main",
- "table": None,
- "target": "data",
- "columns": (),
- "source": None,
- "internal": False,
- }
- ]
-
-
-def test_analyze_statement_with_no_authorizer_callbacks_is_unknown():
- conn = sqlite3.connect(":memory:")
- try:
- analysis = analyze_sql_tables(conn, "reindex", database_name="data")
- finally:
- conn.close()
-
- assert [operation_dict(operation) for operation in analysis.operations] == [
- {
- "operation": "unknown",
- "target_type": "statement",
- "database": "data",
- "sqlite_schema": None,
- "table": None,
- "target": None,
- "columns": (),
- "source": None,
- "internal": False,
- }
- ]
-
-
-def test_analyze_transaction_operation(conn):
- analysis = analyze_sql_tables(conn, "commit", database_name="data")
-
- assert [operation_dict(operation) for operation in analysis.operations] == [
- {
- "operation": "commit",
- "target_type": "transaction",
- "database": None,
- "sqlite_schema": None,
- "table": None,
- "target": "COMMIT",
- "columns": (),
- "source": None,
- "internal": False,
- }
- ]
-
-
-def test_analyze_savepoint_operation(conn):
- analysis = analyze_sql_tables(conn, "savepoint s", database_name="data")
-
- assert [operation_dict(operation) for operation in analysis.operations] == [
- {
- "operation": "savepoint",
- "target_type": "transaction",
- "database": None,
- "sqlite_schema": None,
- "table": None,
- "target": "BEGIN s",
- "columns": (),
- "source": None,
- "internal": False,
- }
- ]
-
-
-def test_analyze_function_operation(conn):
- analysis = analyze_sql_tables(
- conn,
- "insert into dogs (name) values (upper(:name))",
- {"name": "Cleo"},
- database_name="data",
- )
-
- assert {
- (
- operation.operation,
- operation.target_type,
- operation.target,
- operation.database,
- operation.table,
- )
- for operation in analysis.operations
- } == {
- ("insert", "table", "dogs", "data", "dogs"),
- ("function", "function", "upper", None, None),
- ("read", "table", "dogs", "data", "dogs"),
- ("update", "table", "cats", "data", "cats"),
- ("read", "table", "cats", "data", "cats"),
- ("insert", "table", "log", "data", "log"),
- }
-
-
-def test_analyze_create_virtual_table_operation():
- conn = sqlite3.connect(":memory:")
- try:
- analysis = analyze_sql_tables(
- conn,
- "create virtual table docs using fts5(body)",
- database_name="data",
- )
- finally:
- conn.close()
-
- assert {
- "operation": "create",
- "target_type": "virtual-table",
- "database": "data",
- "sqlite_schema": "main",
- "table": "docs",
- "target": "docs",
- "columns": (),
- "source": None,
- "internal": False,
- } in [operation_dict(operation) for operation in analysis.operations]
-
-
-def test_analyze_create_table_as_select_function_is_not_internal():
- conn = sqlite3.connect(":memory:")
- try:
- conn.execute("create table secret(value text)")
- analysis = analyze_sql_tables(
- conn,
- "create table copied as select substr(value, 1, 1) from secret",
- database_name="data",
- )
- finally:
- conn.close()
-
- assert {
- "operation": "function",
- "target_type": "function",
- "database": None,
- "sqlite_schema": None,
- "table": None,
- "target": "substr",
- "columns": (),
- "source": None,
- "internal": False,
- } in [operation_dict(operation) for operation in analysis.operations]
-
-
def test_analyze_insert_tables(conn):
analysis = analyze_sql_tables(
conn,
@@ -293,7 +70,7 @@ def test_analyze_insert_tables(conn):
database_name="data",
)
- assert set(table_operation_tuples(analysis)) == {
+ assert set(as_tuples(analysis)) == {
("insert", "data", "main", "dogs", (), None),
("read", "data", "main", "dogs", ("id", "name"), "dogs_after_insert"),
("update", "data", "main", "cats", ("name",), "dogs_after_insert"),
@@ -310,7 +87,7 @@ def test_analyze_update_tables(conn):
database_name="data",
)
- assert set(table_operation_tuples(analysis)) == {
+ assert set(as_tuples(analysis)) == {
("update", "data", "main", "dogs", ("age",), None),
("read", "data", "main", "dogs", ("age", "name"), None),
}
@@ -324,7 +101,7 @@ def test_analyze_delete_tables(conn):
database_name="data",
)
- assert set(table_operation_tuples(analysis)) == {
+ assert set(as_tuples(analysis)) == {
("delete", "data", "main", "dogs", (), None),
("read", "data", "main", "dogs", ("name",), None),
}
@@ -344,7 +121,7 @@ def test_analyze_insert_select_with_cte(conn):
database_name="data",
)
- assert set(table_operation_tuples(analysis)) == {
+ assert set(as_tuples(analysis)) == {
("insert", "data", "main", "cats", (), None),
("read", "data", "main", "dogs", ("age", "name"), "old_dogs"),
}
@@ -358,7 +135,7 @@ def test_analyze_view_with_instead_of_trigger(conn):
database_name="data",
)
- assert set(table_operation_tuples(analysis)) == {
+ assert set(as_tuples(analysis)) == {
("update", "data", "main", "dog_names", ("name",), None),
("read", "data", "main", "dogs", ("id", "name"), "dog_names"),
("read", "data", "main", "dog_names", ("id", "name"), "dog_names"),
@@ -386,7 +163,7 @@ def test_analyze_attached_database_tables(conn):
schema_to_database={"extra": "extra_db"},
)
- assert set(table_operation_tuples(analysis)) == {
+ assert set(as_tuples(analysis)) == {
("insert", "extra_db", "extra", "people", (), None),
("read", "data", "main", "dogs", ("name",), None),
}