From ef43c103880fe819206f4e0dd12fa62add1c927c Mon Sep 17 00:00:00 2001 From: Simon Willison Date: Mon, 25 May 2026 08:30:49 -0700 Subject: [PATCH] Add arbitrary write SQL execution page Refs #2735 --- datasette/app.py | 21 +- datasette/default_actions.py | 7 + datasette/templates/execute_write.html | 71 +++++++ datasette/templates/query_create.html | 3 + datasette/views/database.py | 266 +++++++++++++++++++++++-- docs/authentication.rst | 12 +- docs/json_api.rst | 9 + tests/test_queries.py | 122 ++++++++++++ 8 files changed, 487 insertions(+), 24 deletions(-) create mode 100644 datasette/templates/execute_write.html diff --git a/datasette/app.py b/datasette/app.py index ce85f447..409aed23 100644 --- a/datasette/app.py +++ b/datasette/app.py @@ -46,6 +46,7 @@ from .views import Context from .views.database import ( database_download, DatabaseView, + ExecuteWriteView, TableCreateView, QueryView, QueryCreateView, @@ -1249,18 +1250,22 @@ class Datasette: ) return {row["name"]: self._query_row_to_dict(row) for row in rows} - async def ensure_query_write_permissions(self, database, sql, *, actor=None): + async def ensure_query_write_permissions( + self, database, sql, *, actor=None, params=None, analysis=None + ): write_actions = { "insert": "insert-row", "update": "update-row", "delete": "delete-row", } db = self.get_database(database) - params = {name: "" for name in named_parameters(sql)} - try: - analysis = await db.analyze_sql(sql, params) - except sqlite3.DatabaseError as ex: - raise Forbidden(f"Could not analyze query: {ex}") from ex + if analysis is None: + if params is None: + params = {name: "" for name in named_parameters(sql)} + try: + analysis = await db.analyze_sql(sql, params) + except sqlite3.DatabaseError as ex: + raise Forbidden(f"Could not analyze query: {ex}") from ex for access in analysis.table_accesses: action = write_actions.get(access.operation) @@ -2547,6 +2552,10 @@ class Datasette: QueryInsertView.as_view(self), r"/(?P[^\/\.]+)/-/queries/-/insert$", ) + add_route( + ExecuteWriteView.as_view(self), + r"/(?P[^\/\.]+)/-/execute-write$", + ) add_route( DatabaseSchemaView.as_view(self), r"/(?P[^\/\.]+)/-/schema(\.(?Pjson|md))?$", diff --git a/datasette/default_actions.py b/datasette/default_actions.py index e0e0aee5..6787b80e 100644 --- a/datasette/default_actions.py +++ b/datasette/default_actions.py @@ -48,6 +48,13 @@ def register_actions(): resource_class=DatabaseResource, also_requires="view-database", ), + Action( + name="execute-write-sql", + abbr="ews", + description="Execute writable SQL queries", + resource_class=DatabaseResource, + also_requires="view-database", + ), Action( name="create-table", abbr="ct", diff --git a/datasette/templates/execute_write.html b/datasette/templates/execute_write.html new file mode 100644 index 00000000..5b4f30d9 --- /dev/null +++ b/datasette/templates/execute_write.html @@ -0,0 +1,71 @@ +{% extends "base.html" %} + +{% block title %}Execute write SQL{% endblock %} + +{% block extra_head %} +{{- super() -}} +{% include "_codemirror.html" %} +{% endblock %} + +{% block body_class %}execute-write db-{{ database|to_css_class }}{% endblock %} + +{% block crumbs %} +{{ crumbs.nav(request=request, database=database) }} +{% endblock %} + +{% block content %} + +

Execute write SQL

+ +{% if execution_message %} +

{{ execution_message }}

+{% endif %} + +
+

+ + {% if parameter_names %} +

Parameters

+ {% for parameter in parameter_names %} +

+ {% endfor %} + {% endif %} + +

Analysis

+ {% if analysis_error %} +

{{ analysis_error }}

+ {% elif analysis_rows %} +
+ + + + + + + + + + + + {% for row in analysis_rows %} + + + + + + + + + {% endfor %} + +
OperationDatabaseTablerequired permissionAllowedSource
{{ row.operation }}{{ row.database }}{{ row.table }}{{ row.required_permission }}{% if row.allowed is none %}{% elif row.allowed %}yes{% else %}no{% endif %}{{ row.source or "" }}
+ {% else %} +

Analysis will show each affected table and required permission.

+ {% endif %} + +

+
+ +{% include "_codemirror_foot.html" %} + +{% endblock %} diff --git a/datasette/templates/query_create.html b/datasette/templates/query_create.html index 0e6a7b37..1b3d30a8 100644 --- a/datasette/templates/query_create.html +++ b/datasette/templates/query_create.html @@ -30,6 +30,9 @@ {% if can_publish %}

{% endif %} + {% if sql and analysis_is_write %} +

Execute write SQL

+ {% endif %}

Analysis

{% if analysis_error %} diff --git a/datasette/views/database.py b/datasette/views/database.py index d521f7ad..a90d889e 100644 --- a/datasette/views/database.py +++ b/datasette/views/database.py @@ -508,6 +508,27 @@ def _coerce_query_parameters(value, derived): return parameters +def _analysis_is_write(analysis): + return any( + access.operation in {"insert", "update", "delete"} + for access in analysis.table_accesses + ) + + +def _block_framing(response): + response.headers["Content-Security-Policy"] = "frame-ancestors 'none'" + response.headers["X-Frame-Options"] = "DENY" + return response + + +def _wants_json(request, is_json, data): + return ( + is_json + or request.headers.get("accept") == "application/json" + or (isinstance(data, dict) and data.get("_json")) + ) + + async def _json_or_form_payload(request): content_type = request.headers.get("content-type", "") if content_type.startswith("application/json"): @@ -538,15 +559,14 @@ async def _analyze_user_query(datasette, db, sql, *, actor, published): except sqlite3.DatabaseError as ex: raise QueryValidationError("Could not analyze query: {}".format(ex)) from ex - is_write = any( - access.operation in {"insert", "update", "delete"} - for access in analysis.table_accesses - ) + is_write = _analysis_is_write(analysis) if is_write: if published: raise QueryValidationError("Writable queries cannot be published") try: - await datasette.ensure_query_write_permissions(db.name, sql, actor=actor) + await datasette.ensure_query_write_permissions( + db.name, sql, actor=actor, analysis=analysis + ) except Forbidden as ex: raise QueryValidationError(str(ex), status=403) from ex else: @@ -575,6 +595,69 @@ def _analysis_rows(analysis): ] +async def _analysis_rows_with_permissions(datasette, analysis, actor): + rows = _analysis_rows(analysis) + 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 + + +def _coerce_execute_write_payload(data, is_json): + if not isinstance(data, dict): + raise QueryValidationError("JSON must be a dictionary") + if is_json: + invalid_keys = set(data) - {"sql", "params"} + if invalid_keys: + raise QueryValidationError( + "Invalid keys: {}".format(", ".join(sorted(invalid_keys))) + ) + params = data.get("params") or {} + else: + params = { + key: value + for key, value in data.items() + if key not in {"sql", "csrftoken", "_json"} + } + if not isinstance(params, dict): + raise QueryValidationError("params must be a dictionary") + return data.get("sql"), params + + +async def _prepare_execute_write(datasette, db, sql, params, actor): + if not sql or not isinstance(sql, str): + raise QueryValidationError("SQL is required") + parameter_names = _derived_query_parameters(sql) + extra_params = set(params) - set(parameter_names) + if extra_params: + raise QueryValidationError( + "Unknown parameters: {}".format(", ".join(sorted(extra_params))) + ) + params = {name: params.get(name, "") for name in parameter_names} + try: + analysis = await db.analyze_sql(sql, params) + except sqlite3.DatabaseError as ex: + raise QueryValidationError("Could not analyze query: {}".format(ex)) from ex + if not _analysis_is_write(analysis): + raise QueryValidationError( + "Use /-/query for read-only SQL; this endpoint only executes writes" + ) + try: + await datasette.ensure_query_write_permissions( + db.name, sql, actor=actor, analysis=analysis + ) + except Forbidden as ex: + raise QueryValidationError(str(ex), status=403) from ex + return parameter_names, params, analysis + + def _apply_query_data_types(data): typed = dict(data) for key in ("hide_sql", "published"): @@ -707,6 +790,160 @@ async def _prepare_query_update(datasette, request, db, existing, update): return update_kwargs +class ExecuteWriteView(BaseView): + name = "execute-write" + has_json_alternate = False + + async def _render_form( + self, + request, + db, + *, + sql="", + parameter_values=None, + analysis=None, + analysis_error=None, + execution_message=None, + execution_ok=None, + status=200, + ): + parameter_values = parameter_values or {} + parameter_names = [] + analysis_rows = [] + if sql and analysis_error is None: + try: + parameter_names = _derived_query_parameters(sql) + if analysis is None: + params = {parameter: "" for parameter in parameter_names} + analysis = await db.analyze_sql(sql, params) + if _analysis_is_write(analysis): + analysis_rows = await _analysis_rows_with_permissions( + self.ds, analysis, request.actor + ) + else: + analysis_error = ( + "Use /-/query for read-only SQL; " + "this endpoint only executes writes" + ) + except (QueryValidationError, sqlite3.DatabaseError) as ex: + analysis_error = getattr(ex, "message", str(ex)) + + response = await self.render( + ["execute_write.html"], + request, + { + "database": db.name, + "database_color": db.color, + "sql": sql, + "parameter_names": parameter_names, + "parameter_values": parameter_values, + "analysis_error": analysis_error, + "analysis_rows": analysis_rows, + "execution_message": execution_message, + "execution_ok": execution_ok, + "execute_disabled": bool( + (not sql) + or analysis_error + or any(row["allowed"] is False for row in analysis_rows) + ), + }, + ) + response.status = status + return _block_framing(response) + + async def get(self, request): + db = await self.ds.resolve_database(request) + await self.ds.ensure_permission( + action="execute-write-sql", + resource=DatabaseResource(db.name), + actor=request.actor, + ) + return await self._render_form( + request, + db, + sql=request.args.get("sql") or "", + ) + + async def post(self, request): + db = await self.ds.resolve_database(request) + if not await self.ds.allowed( + action="execute-write-sql", + resource=DatabaseResource(db.name), + actor=request.actor, + ): + return _block_framing( + _error(["Permission denied: need execute-write-sql"], 403) + ) + if not db.is_mutable: + return _block_framing(_error(["Database is immutable"], 403)) + + data = {} + is_json = request.headers.get("content-type", "").startswith("application/json") + sql = "" + provided_params = {} + try: + data, is_json = await _json_or_form_payload(request) + sql, provided_params = _coerce_execute_write_payload(data, is_json) + parameter_names, params, analysis = await _prepare_execute_write( + self.ds, db, sql, provided_params, request.actor + ) + except QueryValidationError as ex: + if _wants_json(request, is_json, data): + return _block_framing(_error([ex.message], ex.status)) + return await self._render_form( + request, + db, + sql=sql or "", + parameter_values=provided_params, + analysis_error=ex.message, + execution_message=ex.message, + execution_ok=False, + status=ex.status, + ) + + try: + cursor = await db.execute_write(sql, params, request=request) + except sqlite3.DatabaseError as ex: + message = str(ex) + if _wants_json(request, is_json, data): + return _block_framing(_error([message], 400)) + return await self._render_form( + request, + db, + sql=sql, + parameter_values=params, + analysis=analysis, + execution_message=message, + execution_ok=False, + status=400, + ) + + 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( + { + "ok": True, + "message": message, + "rowcount": cursor.rowcount, + "analysis": _analysis_rows(analysis), + } + ) + ) + + return await self._render_form( + request, + db, + sql=sql, + parameter_values={name: params.get(name, "") for name in parameter_names}, + analysis=analysis, + execution_message=message, + execution_ok=True, + ) + + class QueryListView(BaseView): name = "query-list" @@ -753,18 +990,9 @@ class QueryCreateView(BaseView): parameter_names = _derived_query_parameters(sql) params = {parameter: "" for parameter in parameter_names} analysis = await db.analyze_sql(sql, params) - rows = _analysis_rows(analysis) - for row in rows: - permission = row["required_permission"] - if permission: - row["allowed"] = await self.ds.allowed( - action=permission, - resource=TableResource(row["database"], row["table"]), - actor=request.actor, - ) - else: - row["allowed"] = None - analysis_rows = rows + analysis_rows = await _analysis_rows_with_permissions( + self.ds, analysis, request.actor + ) except (QueryValidationError, sqlite3.DatabaseError) as ex: analysis_error = getattr(ex, "message", str(ex)) @@ -783,6 +1011,10 @@ class QueryCreateView(BaseView): ), "analysis_error": analysis_error, "analysis_rows": analysis_rows, + "analysis_is_write": bool( + analysis_rows + and any(row["required_permission"] for row in analysis_rows) + ), "save_disabled": bool( analysis_error or any(row["allowed"] is False for row in analysis_rows) diff --git a/docs/authentication.rst b/docs/authentication.rst index 543f069b..b6a4cb7e 100644 --- a/docs/authentication.rst +++ b/docs/authentication.rst @@ -1423,13 +1423,23 @@ Actor is allowed to drop a database table. execute-sql ----------- -Actor is allowed to run arbitrary SQL queries against a specific database, e.g. https://latest.datasette.io/fixtures/-/query?sql=select+100 +Actor is allowed to run arbitrary read-only SQL queries against a specific database, e.g. https://latest.datasette.io/fixtures/-/query?sql=select+100 ``resource`` - ``datasette.resources.DatabaseResource(database)`` ``database`` is the name of the database (string) See also :ref:`the default_allow_sql setting `. +.. _actions_execute_write_sql: + +execute-write-sql +----------------- + +Actor is allowed to run arbitrary writable SQL queries against a specific database, subject to table-level write permissions such as ``insert-row``, ``update-row`` and ``delete-row``. + +``resource`` - ``datasette.resources.DatabaseResource(database)`` + ``database`` is the name of the database (string) + .. _actions_permissions_debug: permissions-debug diff --git a/docs/json_api.rst b/docs/json_api.rst index d5cd231c..e4c9e86e 100644 --- a/docs/json_api.rst +++ b/docs/json_api.rst @@ -526,6 +526,15 @@ Creating saved queries ``POST //-/queries/-/insert`` creates a saved query. This requires ``execute-sql`` and ``insert-query`` for the database. +.. _ExecuteWriteView: + +Executing write SQL +~~~~~~~~~~~~~~~~~~~ + +``GET //-/execute-write`` displays a form for executing writable SQL. A ``?sql=`` query string pre-populates the form without executing it. + +``POST //-/execute-write`` executes writable SQL. This requires ``execute-write-sql`` for the database plus the relevant table-level write permissions. + .. _QueryDefinitionView: Getting a saved query definition diff --git a/tests/test_queries.py b/tests/test_queries.py index c6685d6c..05bc5ee1 100644 --- a/tests/test_queries.py +++ b/tests/test_queries.py @@ -212,6 +212,7 @@ async def test_query_actions_are_registered(): ds = Datasette() await ds.invoke_startup() + assert ds.get_action("execute-write-sql").resource_class is DatabaseResource assert ds.get_action("insert-query").resource_class is DatabaseResource assert ds.get_action("publish-query").resource_class is DatabaseResource assert ds.get_action("update-query").resource_class is QueryResource @@ -492,6 +493,127 @@ async def test_create_query_ui_and_arbitrary_sql_save_link(): assert "/data/-/queries/-/create?sql=select+%2A+from+dogs" in query_response.text +@pytest.mark.asyncio +async def test_execute_write_get_prepopulates_without_executing(): + ds = Datasette(memory=True, default_deny=True) + ds.root_enabled = True + db = ds.add_memory_database("execute_write_get", name="data") + await db.execute_write("create table dogs (id integer primary key, name text)") + await ds.invoke_startup() + + response = await ds.client.get( + "/data/-/execute-write?sql=insert+into+dogs+(name)+values+('Cleo')", + actor={"id": "root"}, + ) + + assert response.status_code == 200 + assert response.headers["content-security-policy"] == "frame-ancestors 'none'" + assert response.headers["x-frame-options"] == "DENY" + assert "Execute write SQL" 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 + + +@pytest.mark.asyncio +async def test_execute_write_post_requires_database_and_table_permissions(): + ds = Datasette( + memory=True, + default_deny=True, + config={ + "databases": { + "data": { + "permissions": { + "view-database": {"id": "writer"}, + "execute-write-sql": {"id": "writer"}, + } + } + } + }, + ) + db = ds.add_memory_database("execute_write_permissions", name="data") + await db.execute_write("create table dogs (id integer primary key, name text)") + await ds.invoke_startup() + + no_database_permission = await ds.client.post( + "/data/-/execute-write", + actor={"id": "outsider"}, + json={ + "sql": "insert into dogs (name) values (:name)", + "params": {"name": "Cleo"}, + }, + ) + no_table_permission = await ds.client.post( + "/data/-/execute-write", + actor={"id": "writer"}, + json={ + "sql": "insert into dogs (name) values (:name)", + "params": {"name": "Cleo"}, + }, + ) + + assert no_database_permission.status_code == 403 + assert no_database_permission.json()["errors"] == [ + "Permission denied: need execute-write-sql" + ] + assert no_table_permission.status_code == 403 + assert no_table_permission.json()["errors"] == [ + "Permission denied: need insert-row on data/dogs" + ] + + ds.config = { + "databases": { + "data": { + "permissions": { + "view-database": {"id": "writer"}, + "execute-write-sql": {"id": "writer"}, + }, + "tables": { + "dogs": { + "permissions": { + "insert-row": {"id": "writer"}, + } + } + }, + } + } + } + allowed = await ds.client.post( + "/data/-/execute-write", + actor={"id": "writer"}, + json={ + "sql": "insert into dogs (name) values (:name)", + "params": {"name": "Cleo"}, + }, + ) + + assert allowed.status_code == 200 + assert allowed.json()["ok"] is True + assert allowed.json()["rowcount"] == 1 + assert allowed.json()["analysis"][0]["operation"] == "insert" + assert (await db.execute("select name from dogs")).first()[0] == "Cleo" + + +@pytest.mark.asyncio +async def test_execute_write_post_rejects_read_only_sql(): + ds = Datasette(memory=True, default_deny=True) + ds.root_enabled = True + db = ds.add_memory_database("execute_write_read_only", name="data") + await db.execute_write("create table dogs (id integer primary key, name text)") + await ds.invoke_startup() + + response = await ds.client.post( + "/data/-/execute-write", + actor={"id": "root"}, + json={"sql": "select * from dogs"}, + ) + + assert response.status_code == 400 + assert response.json()["errors"] == [ + "Use /-/query for read-only SQL; this endpoint only executes writes" + ] + + @pytest.mark.asyncio async def test_query_owner_gets_update_delete_and_writable_view_defaults(): ds = Datasette(memory=True, default_deny=True)