You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by "kaxil (via GitHub)" <gi...@apache.org> on 2026/07/20 14:37:48 UTC
[PR] Harden common.ai SQLToolset allowed_tables against function/COPY bypass [airflow]
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Harden common.ai SQLToolset allowed_tables against function/COPY bypass [airflow]
Posted by "kaxil (via GitHub)" <gi...@apache.org>.
kaxil merged PR #70134:
URL: https://github.com/apache/airflow/pull/70134
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Harden common.ai SQLToolset allowed_tables against function/COPY bypass [airflow]
Posted by "kaxil (via GitHub)" <gi...@apache.org>.
kaxil commented on code in PR #70134:
URL: https://github.com/apache/airflow/pull/70134#discussion_r3617796688
##########
providers/common/ai/src/airflow/providers/common/ai/example_dags/example_agent.py:
##########
@@ -65,6 +65,13 @@ def example_agent_operator_sql():
"the schema and answer the question with data."
),
toolsets=[
+ # ``allowed_tables`` scopes the agent's intent, but it is an
+ # application-level guardrail, not a security boundary. Point
+ # ``postgres_default`` at a least-privilege role whose SELECT grants
+ # are limited to these tables -- that is the boundary that holds even
+ # if the agent (which may be under prompt injection) reaches for data
+ # through a function the parser cannot see. See the "Security" section
+ # of the toolsets docs.
SQLToolset(
db_conn_id="postgres_default",
Review Comment:
Added `allowed_functions=["json_build_object"]` to the example with a comment on when it's needed, in 0bc16c8.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Harden common.ai SQLToolset allowed_tables against function/COPY bypass [airflow]
Posted by "Lee-W (via GitHub)" <gi...@apache.org>.
Lee-W commented on code in PR #70134:
URL: https://github.com/apache/airflow/pull/70134#discussion_r3615315006
##########
providers/common/ai/src/airflow/providers/common/ai/example_dags/example_agent.py:
##########
@@ -65,6 +65,13 @@ def example_agent_operator_sql():
"the schema and answer the question with data."
),
toolsets=[
+ # ``allowed_tables`` scopes the agent's intent, but it is an
+ # application-level guardrail, not a security boundary. Point
+ # ``postgres_default`` at a least-privilege role whose SELECT grants
+ # are limited to these tables -- that is the boundary that holds even
+ # if the agent (which may be under prompt injection) reaches for data
+ # through a function the parser cannot see. See the "Security" section
+ # of the toolsets docs.
SQLToolset(
db_conn_id="postgres_default",
Review Comment:
do we want to also add `allow_functions` as part of the example
##########
providers/common/ai/src/airflow/providers/common/ai/utils/sql_validation.py:
##########
@@ -289,6 +311,38 @@ def collect_table_references(statements: list[exp.Expr]) -> TableScan:
if isinstance(stmt, (exp.Command, exp.Execute)):
unverifiable.append(f"a {type(stmt).__name__.lower()} statement")
continue
+ # COPY moves data between a table and the server filesystem or a spawned program
+ # (``COPY t FROM/TO PROGRAM '...'``, ``COPY t FROM/TO '<file>'``). The table it
+ # names is real and allow-listed, but the data channel -- a file or a program --
+ # is not a table the allow-list can describe, and ``FROM PROGRAM`` is arbitrary
+ # command execution. Top-level COPY is already blocked in read-only mode by the
+ # statement-type allow-list; this also refuses it under ``allow_writes`` while an
+ # allow-list is active, where only this scan runs.
+ if isinstance(stmt, exp.Copy):
+ unverifiable.append("a COPY statement")
+ continue
+ # A function whose string argument reaches a file, another table, or a program
+ # (``pg_read_file``, ``query_to_xml``, scalar ``dblink``, ...) carries no
+ # ``exp.Table`` node, so the table scan below cannot see it. sqlglot parses any
+ # function it cannot type as ``exp.Anonymous`` (typed builtins like ``count`` are
+ # ``exp.Func`` subclasses), so reject every ``exp.Anonymous`` not explicitly
+ # allow-listed rather than chase an unbounded denylist of dangerous names. A
+ # schema-qualified call (``pg_catalog.pg_read_file(...)``) parses as
+ # ``Dot(this=..., expression=Anonymous)`` with the bare name on the nested
+ # ``Anonymous``, so ``find_all`` still reaches it.
+ unknown = sorted(
+ {
+ name
+ for fn in stmt.find_all(exp.Anonymous)
+ if (name := fn.name.casefold()) not in allowed_functions
+ }
+ )
+ if unknown:
+ unverifiable.append(
+ f"function(s) the parser cannot verify against allowed_tables "
+ f"({', '.join(unknown)}); if safe, permit them via allowed_functions"
Review Comment:
```suggestion
f"({', '.join(unknown)}); if these functions are trusted, permit them via allowed_functions"
```
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Harden common.ai SQLToolset allowed_tables against function/COPY bypass [airflow]
Posted by "gopidesupavan (via GitHub)" <gi...@apache.org>.
gopidesupavan commented on code in PR #70134:
URL: https://github.com/apache/airflow/pull/70134#discussion_r3615542359
##########
providers/common/ai/src/airflow/providers/common/ai/utils/sql_validation.py:
##########
@@ -244,7 +248,9 @@ def _is_in_scope_cte(table: exp.Table) -> bool:
return False
-def collect_table_references(statements: list[exp.Expr]) -> TableScan:
+def collect_table_references(
Review Comment:
LIST @mystage is valid Snowflake SQL for listing stage files, but sqlglot currently parses it as `exp.Alias` can we consider this cases aswell?
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Harden common.ai SQLToolset allowed_tables against function/COPY bypass [airflow]
Posted by "kaxil (via GitHub)" <gi...@apache.org>.
kaxil commented on code in PR #70134:
URL: https://github.com/apache/airflow/pull/70134#discussion_r3617795525
##########
providers/common/ai/src/airflow/providers/common/ai/utils/sql_validation.py:
##########
@@ -244,7 +248,9 @@ def _is_in_scope_cte(table: exp.Table) -> bool:
return False
-def collect_table_references(statements: list[exp.Expr]) -> TableScan:
+def collect_table_references(
Review Comment:
Good catch. `LIST @mystage` / `LS @mystage` mis-parse to a top-level `exp.Alias` (`Column AS Parameter`) with no table or function node, so on the `allow_writes` path they slipped the scan (read-only mode already rejects them via the statement-type allow-list). A top-level `Alias` is only ever a mis-parse, so I now refuse it as unverifiable, with tests for both. Fixed in 0bc16c8.
##########
providers/common/ai/src/airflow/providers/common/ai/utils/sql_validation.py:
##########
@@ -289,6 +311,38 @@ def collect_table_references(statements: list[exp.Expr]) -> TableScan:
if isinstance(stmt, (exp.Command, exp.Execute)):
unverifiable.append(f"a {type(stmt).__name__.lower()} statement")
continue
+ # COPY moves data between a table and the server filesystem or a spawned program
+ # (``COPY t FROM/TO PROGRAM '...'``, ``COPY t FROM/TO '<file>'``). The table it
+ # names is real and allow-listed, but the data channel -- a file or a program --
+ # is not a table the allow-list can describe, and ``FROM PROGRAM`` is arbitrary
+ # command execution. Top-level COPY is already blocked in read-only mode by the
+ # statement-type allow-list; this also refuses it under ``allow_writes`` while an
+ # allow-list is active, where only this scan runs.
+ if isinstance(stmt, exp.Copy):
+ unverifiable.append("a COPY statement")
+ continue
+ # A function whose string argument reaches a file, another table, or a program
+ # (``pg_read_file``, ``query_to_xml``, scalar ``dblink``, ...) carries no
+ # ``exp.Table`` node, so the table scan below cannot see it. sqlglot parses any
+ # function it cannot type as ``exp.Anonymous`` (typed builtins like ``count`` are
+ # ``exp.Func`` subclasses), so reject every ``exp.Anonymous`` not explicitly
+ # allow-listed rather than chase an unbounded denylist of dangerous names. A
+ # schema-qualified call (``pg_catalog.pg_read_file(...)``) parses as
+ # ``Dot(this=..., expression=Anonymous)`` with the bare name on the nested
+ # ``Anonymous``, so ``find_all`` still reaches it.
+ unknown = sorted(
+ {
+ name
+ for fn in stmt.find_all(exp.Anonymous)
+ if (name := fn.name.casefold()) not in allowed_functions
+ }
+ )
+ if unknown:
+ unverifiable.append(
+ f"function(s) the parser cannot verify against allowed_tables "
+ f"({', '.join(unknown)}); if safe, permit them via allowed_functions"
Review Comment:
Done, reworded to "if these functions are trusted" in 0bc16c8.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org