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), }