`run_sql` is not read-only: stacked statements bypass `check_query()`

[sounkou-bioinfo]

run_sql is described as “read-only” in the tool schema, its MCP-style annotation, and the package documentation, but the current implementation does not enforce that property.

source_query() calls check_query() and then passes the original string to DBI::dbGetQuery(). check_query() only rejects a denylisted keyword at the start of the string. It neither requires a SELECT nor requires exactly one statement.

Reproducer on current main

pkgload::load_all(".")

src <- data_source(sales = data.frame(x = 1L))
sql <- "SELECT 1 AS ok; DROP TABLE sales;"

check_query(sql)       # accepted
source_query(src, sql) # executes the DROP (with a DBI warning)
DBI::dbExistsTable(src$con, "sales")
#> [1] FALSE

I reproduced this with R 4.6.0 and DuckDB 1.5.5. The owned DuckDB configuration lock does not prevent local DDL/DML. Other non-SELECT statement classes such as SET and PRAGMA also pass check_query(); their effects depend on the supplied connection and its privileges.

Consequently, these currently overstate the guarantee:

• read_only_hint = TRUE
• “Run a read-only SELECT query”
• “The run_sql tool runs only read-only SELECT queries”

Proposal: classify DuckDB SQL with DuckDB's parser

For DuckDB-backed sources, use core json_serialize_sql() before executing agent-authored SQL:

1. Quote the candidate with DBI::dbQuoteString() and pass it as the required constant string. json_serialize_sql() parses; it does not execute the candidate.
2. Reject missing output, parser/serializer errors, and error: true.
3. Require exactly one serialized statement.
4. Require the root node to be SELECT_NODE.
5. Recursively inspect function nodes and reject dynamic-SQL executors such as query() / query_table() rather than trusting their string arguments.
6. Add known-good/known-bad canaries and fail closed if DuckDB changes the JSON shape.

This handles the immediate cases structurally:

• SELECT 1; SELECT 2 serializes as two statements.
• SELECT 1; DROP TABLE sales returns error: true because the serializer only admits SELECT statements.
• quoted or qualified function spellings normalize to the same AST function_name.

We use this DuckDB AST-serialization pattern in pi-bio-agent to detect normalized dynamic-SQL calls (implementation) and to inspect function calls without SQL-text matching (implementation and fail-closed behavior). It also has a parser/plan format canary so version drift disables the claimed property rather than silently weakening it (canary).

data_source() also accepts arbitrary DBI connections and SQL dialects, so DuckDB's parser cannot prove those statements read-only. That path needs a backend-specific validator and/or a genuinely read-only connection. Until it has one, I suggest describing the weaker guarantee accurately and setting read_only_hint = FALSE for sources where commons cannot enforce it.

Suggested regression coverage

• Reject the stacked-statement reproducer and verify sales still exists.
• Reject a second SELECT, DDL/DML, SET, and PRAGMA.
• Accept one SELECT / WITH ... SELECT.
• Do not reject keywords or semicolons inside literals, quoted identifiers, or comments.
• Reject quoted and qualified dynamic-SQL function calls.
• Fail closed when AST serialization is unavailable or its expected JSON shape changes.

---

Disclosure: This issue was generated in Pi using GPT Sol 5.6 High.