kaxil opened a new pull request, #70134:
URL: https://github.com/apache/airflow/pull/70134
`SQLToolset(allowed_tables=[...])` restricts every table an LLM agent's SQL
can reach. The enforcement walks the parsed statement for table references
(`exp.Table` nodes) and rejects any that are not on the list. A SQL function
whose argument is a file path or a SQL string carries **no table node**, so it
was invisible to that scan: with `allowed_tables=["orders"]`, an agent could
still run
- `SELECT pg_read_file('/etc/passwd')` — read a server file,
- `SELECT query_to_xml('SELECT * FROM secrets', ...)` — read a table off the
list (needs only an ordinary read role),
- `COPY orders FROM PROGRAM 'id'` (under `allow_writes=True`) — run a shell
command,
and the guardrail reported the query as allowed.
`collect_table_references` now rejects, before the query runs (handed back
to the agent as a correctable error):
- **`COPY`** in any form (`FROM`/`TO`, `PROGRAM` or file). Top-level `COPY`
was already blocked in read-only mode by the statement-type allow-list; this
also refuses it on the `allow_writes=True` path, where only the table scan runs.
- **Any function sqlglot cannot type** (`exp.Anonymous`) — the channel
`pg_read_file` / `query_to_xml` / scalar `dblink` use. Ordinary builtins
(`count`, `lower`, `sum`) are recognised and pass.
## Design rationale
- **Fail-closed, not a denylist.** The obvious approach — a list of
dangerous function names — is unbounded, engine-specific, and fails *open* on
anything missed (it's easy to forget `table_to_xml`, MySQL `load_file`, the
next release's new function). Instead, reject *every* function sqlglot doesn't
recognise as a typed builtin. An incomplete allow-list refuses a query
(recoverable); an incomplete denylist leaks. This also matches the module's own
stated philosophy ("an allowlist is safer than a denylist because
new/unexpected things are blocked by default").
- **`allowed_functions` escape hatch.** ~10% of common analytics builtins
(`json_build_object`, `jsonb_agg`, `age`) and any project UDF also parse to
`exp.Anonymous`, so an operator whose agent needs one lists it explicitly:
`SQLToolset(allowed_tables=["orders"],
allowed_functions=["json_build_object"])`. This inverts the maintenance burden
from an unbounded, security-critical denylist we own to a small, local,
fail-safe allow-list the operator owns.
- **Best-effort by construction — the DB role is the real boundary.** A
query the engine parses differently from sqlglot, or an `allowed_functions`
entry that turns out unsafe, is a residual gap. `allowed_tables` encodes the
agent's *intent*; the only hard guarantee is pointing `db_conn_id` at a
least-privilege database role. The docstrings, the toolset guide (now a `..
warning::`), and the example DAG all say so, and the file-read / RCE payloads
additionally require a privileged DB role.
## Notes
- Detection keys on sqlglot modeling unrecognised functions as
`exp.Anonymous` (verified at the `>=30.0.0` floor and current 30.12.0); the
parametrized tests are the tripwire if that ever changes.
- No behaviour change for typical analytics SQL: recognised builtins over
allowed tables still run; only `COPY` and unrecognised functions (absent
`allowed_functions`) are refused.
- Docstrings on `SQLToolset` and `collect_table_references`, the toolset
guide, and the SQL example DAG are updated to match and to lead with the
least-privilege-role guidance.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
