This is an automated email from the ASF dual-hosted git repository.
kaxil pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/airflow.git
The following commit(s) were added to refs/heads/main by this push:
new ae4343cad45 Reject empty allowed_tables in SQLToolset instead of
allowing all tables (#73381)
ae4343cad45 is described below
commit ae4343cad4551da310575121749ce53723b46d27
Author: Jyun-An Chen <[email protected]>
AuthorDate: Mon Sep 21 19:38:52 2026 +0800
Reject empty allowed_tables in SQLToolset instead of allowing all tables
(#73381)
* Reject empty allowed_tables in SQLToolset instead of allowing all tables
* Note the empty allowed_tables rejection in the common-ai docs and
changelog
Rejecting an empty allowed_tables breaks construction that 0.8.0 and 0.9.0
accepted, so an upgrading Dag that builds the list dynamically now fails at
import and needs to be told that None is the way to ask for allow-all. The
toolsets page carries the same parameter list as the docstring and is where
users of this toolset land, so it has to describe the guard as well.
---
providers/common/ai/docs/changelog.rst | 8 ++++++++
providers/common/ai/docs/toolsets.rst | 3 ++-
.../ai/src/airflow/providers/common/ai/toolsets/sql.py | 12 +++++++++---
.../common/ai/tests/unit/common/ai/toolsets/test_sql.py | 4 ++++
4 files changed, 23 insertions(+), 4 deletions(-)
diff --git a/providers/common/ai/docs/changelog.rst
b/providers/common/ai/docs/changelog.rst
index 1eb0b82aca7..dd5daeb3047 100644
--- a/providers/common/ai/docs/changelog.rst
+++ b/providers/common/ai/docs/changelog.rst
@@ -36,6 +36,14 @@ Changelog
or inspect its ``.exceptions`` attribute for the original per-model errors.
See
:doc:`retry_policies`, "When the connection also carries a fallback chain".
+.. note::
+ ``SQLToolset(allowed_tables=[])`` now raises ``ValueError``. Up to 0.9.0 an
empty list
+ was accepted and exposed every table in the schema -- the same as
``allowed_tables=None``
+ -- so a Dag that builds the list dynamically (a ``Variable.get``, a config
file, a
+ filtered comprehension) silently handed the agent the whole schema whenever
the list
+ came back empty. Such a Dag now fails at import instead. Pass ``None``
explicitly if
+ exposing every table is what you meant.
+
0.9.0
.....
diff --git a/providers/common/ai/docs/toolsets.rst
b/providers/common/ai/docs/toolsets.rst
index 2db6edc5b5b..3461935fe62 100644
--- a/providers/common/ai/docs/toolsets.rst
+++ b/providers/common/ai/docs/toolsets.rst
@@ -207,7 +207,8 @@ Parameters
- ``db_conn_id``: Airflow connection ID for the database.
- ``allowed_tables``: Restrict the agent to a fixed set of tables. ``None``
- (default) exposes all tables in ``schema``. Entries may be schema-qualified
+ (default) exposes all tables in ``schema``; an empty list raises
``ValueError``
+ rather than silently exposing them all. Entries may be schema-qualified
(``"SCHEMA.TABLE"``) to span multiple schemas; see above. Matching is
case-insensitive. When set, the list is enforced on ``query`` and
``check_query`` as well as discovery -- every table a query references must
be
diff --git
a/providers/common/ai/src/airflow/providers/common/ai/toolsets/sql.py
b/providers/common/ai/src/airflow/providers/common/ai/toolsets/sql.py
index 584fd475c93..5011ba99c20 100644
--- a/providers/common/ai/src/airflow/providers/common/ai/toolsets/sql.py
+++ b/providers/common/ai/src/airflow/providers/common/ai/toolsets/sql.py
@@ -171,9 +171,10 @@ class SQLToolset(AbstractToolset[Any]):
:param db_conn_id: Airflow connection ID for the database.
:param allowed_tables: Restrict the agent to a fixed set of tables.
``None``
- (default) exposes every table in ``schema``. Entries may be
schema-qualified
- (``"SCHEMA.TABLE"``) to span multiple schemas in one database --
common on
- warehouses such as Snowflake. ``list_tables`` introspects each
referenced
+ (default) exposes every table in ``schema``; an empty list raises
+ ``ValueError`` rather than silently exposing every table. Entries may
be
+ schema-qualified (``"SCHEMA.TABLE"``) to span multiple schemas in one
database
+ -- common on warehouses such as Snowflake. ``list_tables`` introspects
each referenced
schema and returns the matching tables fully qualified, and
``get_schema``
routes to the table's own schema. Unqualified entries use ``schema``.
Matching is case-insensitive, since databases reflect identifiers in
their
@@ -246,6 +247,11 @@ class SQLToolset(AbstractToolset[Any]):
max_rows: int = 50,
max_result_bytes: int = DEFAULT_MAX_RESULT_BYTES,
) -> None:
+ if allowed_tables is not None and not allowed_tables:
+ raise ValueError(
+ "allowed_tables must not be empty. Pass None to allow every
table in the schema, "
+ "or list the tables the agent may access."
+ )
self._db_conn_id = db_conn_id
self._allowed_tables: frozenset[str] | None =
frozenset(allowed_tables) if allowed_tables else None
# Case-folded so matching a query's function names (also case-folded)
is
diff --git a/providers/common/ai/tests/unit/common/ai/toolsets/test_sql.py
b/providers/common/ai/tests/unit/common/ai/toolsets/test_sql.py
index 0a00ef9c04a..455805d1043 100644
--- a/providers/common/ai/tests/unit/common/ai/toolsets/test_sql.py
+++ b/providers/common/ai/tests/unit/common/ai/toolsets/test_sql.py
@@ -96,6 +96,10 @@ class TestSQLToolsetInit:
ts = SQLToolset("my_pg")
assert ts.id == "sql-my_pg"
+ def test_empty_allowed_tables_raises(self):
+ with pytest.raises(ValueError, match="allowed_tables must not be
empty"):
+ SQLToolset("my_pg", allowed_tables=[])
+
class TestSQLToolsetGetTools:
def test_returns_four_tools(self):
