This is an automated email from the ASF dual-hosted git repository.
EnxDev pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/superset.git
The following commit(s) were added to refs/heads/master by this push:
new 3bd6e103c14 fix(security): apply global guest RLS rules inside adhoc
sub-queries (#44645)
3bd6e103c14 is described below
commit 3bd6e103c14d7c2aea1e18ffb6d516d4f85321e0
Author: Enzo Martellucci <[email protected]>
AuthorDate: Fri Sep 25 12:14:55 2026 +0200
fix(security): apply global guest RLS rules inside adhoc sub-queries
(#44645)
Co-authored-by: Claude Opus 5.5 (1M context) <[email protected]>
---
UPDATING.md | 13 ++
superset/models/helpers.py | 2 +
superset/utils/rls.py | 28 ++--
.../models/test_virtual_dataset_format.py | 28 ++++
tests/unit_tests/security/guest_rls_test.py | 153 ++++++++++++++++++++-
tests/unit_tests/sql_lab_test.py | 4 +-
6 files changed, 209 insertions(+), 19 deletions(-)
diff --git a/UPDATING.md b/UPDATING.md
index ba734724776..8a396f8ad35 100644
--- a/UPDATING.md
+++ b/UPDATING.md
@@ -24,6 +24,19 @@ assists people when migrating to a new version.
## Next
+### Guest token RLS rules without a dataset apply inside sub-queries
+
+A guest token RLS rule with no `dataset` key applies to every dataset. Such
+rules are now also injected into sub-queries of custom SQL expressions (with
+`ALLOW_ADHOC_SUBQUERY` enabled) and into SQL Lab queries, for every table that
+resolves to a dataset, instead of only into the chart's outer query. A virtual
+dataset's inner SQL is unchanged, since its outer query already applies them.
+
+If a sub-query reads a dataset that lacks a column the rule references, the
+query now fails with a column-not-found error instead of reading rows the rule
+was meant to exclude. To keep such charts working, set `dataset` on the rule so
+it only targets the datasets that have the column.
+
### MCP response size guard: byte limit instead of estimated token count
The MCP response-size guard no longer estimates LLM token counts (it
diff --git a/superset/models/helpers.py b/superset/models/helpers.py
index 9de91751d0a..c344f4ac392 100644
--- a/superset/models/helpers.py
+++ b/superset/models/helpers.py
@@ -3771,6 +3771,7 @@ class ExploreMixin: # pylint:
disable=too-many-public-methods
self.schema or default_schema or "",
statement,
exclude_dataset_id=self_id,
+ include_global_guest_rls=False,
):
rls_applied = True
@@ -3799,6 +3800,7 @@ class ExploreMixin: # pylint:
disable=too-many-public-methods
self.database,
self.database.get_default_catalog(),
exclude_dataset_id=self_id,
+ include_global_guest_rls=False,
)
for statement in parsed_script.statements
for table in statement.tables
diff --git a/superset/utils/rls.py b/superset/utils/rls.py
index 3de35db1636..b64b1c8a772 100644
--- a/superset/utils/rls.py
+++ b/superset/utils/rls.py
@@ -60,6 +60,7 @@ def apply_rls(
schema: str,
parsed_statement: BaseSQLStatement[Any],
exclude_dataset_id: int | None = None,
+ include_global_guest_rls: bool = True,
) -> bool:
"""
Modify statement inplace to ensure RLS rules are applied.
@@ -69,6 +70,10 @@ def apply_rls(
on top of the outer-WHERE application (avoids double-apply when the
virtual dataset's table_name collides with a table in its own SQL — for
example, after converting a physical dataset with RLS to virtual).
+ :param include_global_guest_rls: Also inject global (unscoped) guest RLS
+ rules. Pass False only for a virtual dataset's inner SQL, whose outer
+ query already applies them. Any other statement, such as a SQL Lab
query
+ or an adhoc sub-query, is not constrained by such an outer query.
:returns: True if any RLS predicates were actually applied, False
otherwise.
"""
# There are two ways to insert RLS: either replacing the table with a
subquery
@@ -88,6 +93,7 @@ def apply_rls(
database,
default_catalog,
exclude_dataset_id=exclude_dataset_id,
+ include_global_guest_rls=include_global_guest_rls,
)
if predicate
]
@@ -186,6 +192,7 @@ def get_predicates_for_table(
database: Database,
default_catalog: str | None,
exclude_dataset_id: int | None = None,
+ include_global_guest_rls: bool = True,
) -> list[str]:
"""
Get the RLS predicates for a table.
@@ -193,6 +200,9 @@ def get_predicates_for_table(
This is used to inject RLS rules into SQL statements run in SQL Lab. Note
that the
table must be fully qualified, with catalog (null if the DB doesn't
support) and
schema.
+
+ :param include_global_guest_rls: Also return global (unscoped) guest RLS
rules.
+ See ``apply_rls``.
"""
datasets = _find_datasets(
table,
@@ -215,16 +225,11 @@ def get_predicates_for_table(
if not datasets:
return []
- # Exclude global (unscoped) guest RLS to prevent double application in
- # virtual datasets. Global guest rules will be applied to the outer query
- # via get_sqla_row_level_filters() on the virtual dataset itself.
- # Dataset-scoped guest rules are still included here because they target
- # this specific physical dataset and won't match on the outer query.
- # Note: this path is also used by SQL Lab (sql_lab.py, executor.py) via
- # apply_rls(). Guest users with the default Public role cannot access SQL
Lab
- # (PUBLIC_EXCLUDED_VIEW_MENUS in security/manager.py). If the guest role is
- # extended to include SQL Lab access, global guest RLS predicates for
- # underlying tables would be skipped here.
+ # For a virtual dataset's inner SQL, callers exclude global (unscoped)
guest
+ # RLS to prevent double application. Global guest rules will be applied to
+ # the outer query via get_sqla_row_level_filters() on the virtual dataset
+ # itself. Dataset-scoped guest rules are still included here because they
+ # target this specific physical dataset and won't match on the outer query.
# A folded match can resolve to several datasets naming the same physical
# table, in which case every one's predicates apply; deduplicated because a
# single RLS rule can be attached to more than one of them.
@@ -233,7 +238,7 @@ def get_predicates_for_table(
str(predicate.compile(dialect=dialect,
compile_kwargs={"literal_binds": True}))
for dataset in datasets
for predicate in dataset.get_sqla_row_level_filters(
- include_global_guest_rls=False
+ include_global_guest_rls=include_global_guest_rls
)
)
@@ -279,6 +284,7 @@ def collect_rls_predicates_for_sql(
database,
default_catalog,
exclude_dataset_id=exclude_dataset_id,
+ include_global_guest_rls=False,
)
}
)
diff --git a/tests/unit_tests/models/test_virtual_dataset_format.py
b/tests/unit_tests/models/test_virtual_dataset_format.py
index ee0898c09dd..f881ff69071 100644
--- a/tests/unit_tests/models/test_virtual_dataset_format.py
+++ b/tests/unit_tests/models/test_virtual_dataset_format.py
@@ -497,3 +497,31 @@ class TestVirtualDatasetRLSFailClosed:
virtual_datasource.get_from_clause(template_processor=None)
mock_db.session.rollback.assert_called_once()
+
+
+@patch(
+ "superset.models.helpers.get_predicates_for_table",
+ return_value=["user_id = 42"],
+)
+@patch(
+ "superset.models.helpers.apply_rls",
+ side_effect=NotImplementedError("engine cannot apply RLS"),
+)
+def test_get_from_clause_excludes_global_guest_rls(
+ mock_apply_rls: MagicMock,
+ mock_get_predicates: MagicMock,
+ virtual_datasource: MagicMock,
+ app: Flask,
+) -> None:
+ """
+ The virtual dataset's outer query already applies global guest RLS rules,
+ so the inner SQL must opt out of them to avoid applying them twice, both
+ when injecting RLS and when checking whether a failed injection matters.
+ """
+ _set_virtual_sql(virtual_datasource, "SELECT pen_id FROM public.pens")
+
+ with pytest.raises(QueryObjectValidationError):
+ virtual_datasource.get_from_clause(template_processor=None)
+
+ assert mock_apply_rls.call_args.kwargs["include_global_guest_rls"] is False
+ assert mock_get_predicates.call_args.kwargs["include_global_guest_rls"] is
False
diff --git a/tests/unit_tests/security/guest_rls_test.py
b/tests/unit_tests/security/guest_rls_test.py
index 6210570d66f..56679da5e67 100644
--- a/tests/unit_tests/security/guest_rls_test.py
+++ b/tests/unit_tests/security/guest_rls_test.py
@@ -15,11 +15,13 @@
# specific language governing permissions and limitations
# under the License.
"""
-Tests for guest RLS scoping in virtual dataset scenarios.
+Tests for guest RLS scoping in virtual dataset and adhoc sub-query scenarios.
Verifies that dataset-scoped guest RLS rules are correctly applied
-when querying through virtual datasets, and that global (unscoped)
-guest rules are not duplicated across inner and outer queries.
+when querying through virtual datasets, that global (unscoped)
+guest rules are not duplicated across inner and outer queries, and
+that global guest rules still reach adhoc sub-queries, which no outer
+query constrains.
"""
from __future__ import annotations
@@ -253,9 +255,9 @@ def
test_global_guest_rule_excluded_through_get_predicates_for_table(
mocker: MockerFixture,
) -> None:
"""
- Global (unscoped) guest RLS rules are excluded when
- get_predicates_for_table() calls get_sqla_row_level_filters()
- with include_global_guest_rls=False.
+ Global (unscoped) guest RLS rules are excluded when a virtual dataset's
+ inner SQL calls get_predicates_for_table() with
+ include_global_guest_rls=False.
This prevents double application: global guest rules match any dataset,
so they would appear both in inner SQL (underlying table) and outer query
@@ -289,10 +291,147 @@ def
test_global_guest_rule_excluded_through_get_predicates_for_table(
),
):
table = Table("physical_table", "public", "examples")
- predicates = get_predicates_for_table(table, database, "examples")
+ predicates = get_predicates_for_table(
+ table, database, "examples", include_global_guest_rls=False
+ )
assert not any("org_id" in p for p in predicates), (
f"Global guest rule 'org_id = 1' should be excluded from "
f"get_predicates_for_table() to prevent double application "
f"in virtual datasets. Got: {predicates}"
)
+
+
+def
test_global_guest_rule_included_by_default_through_get_predicates_for_table(
+ app: Flask,
+ mocker: MockerFixture,
+) -> None:
+ """
+ Global (unscoped) guest RLS rules are included by default, so a statement
+ that no outer query constrains, such as a SQL Lab query, is still scoped
+ to the guest token.
+ """
+ from sqlalchemy.dialects import sqlite
+
+ global_rule = GuestTokenRlsRule(dataset=None, clause="org_id = 1")
+ guest_user = _make_guest_user(rules=[global_rule])
+
+ mock_pd = _make_datasource_with_real_rls(42)
+
+ database = mocker.MagicMock()
+ database.get_dialect.return_value = sqlite.dialect()
+ db = mocker.patch("superset.utils.rls.db")
+ db.session.query().filter().one_or_none.return_value = mock_pd
+
+ with (
+ patch(
+ "superset.connectors.sqla.models.security_manager.get_rls_filters",
+ return_value=[],
+ ),
+ patch(
+
"superset.connectors.sqla.models.security_manager.get_guest_rls_filters",
+ wraps=_guest_rls_filter(guest_user),
+ ),
+ patch(
+ "superset.connectors.sqla.models.is_feature_enabled",
+ return_value=True,
+ ),
+ ):
+ table = Table("physical_table", "public", "examples")
+ predicates = get_predicates_for_table(table, database, "examples")
+
+ assert any("org_id" in p for p in predicates), (
+ f"Global guest rule 'org_id = 1' should be included by default. "
+ f"Got: {predicates}"
+ )
+
+
+def _validate_adhoc_subquery_as_guest(
+ mocker: MockerFixture,
+ rules: list[GuestTokenRlsRule],
+ sql: str,
+) -> str:
+ """
+ Run ``validate_adhoc_subquery`` for a guest holding ``rules``, with the
+ sub-query's table resolving to a physical dataset.
+ """
+ from sqlalchemy.dialects import sqlite
+
+ from superset.models.helpers import validate_adhoc_subquery
+ from superset.sql.parse import RLSMethod
+
+ guest_user = _make_guest_user(rules=rules)
+ mock_pd = _make_datasource_with_real_rls(42)
+
+ database = mocker.MagicMock()
+ database.get_dialect.return_value = sqlite.dialect()
+ database.get_default_catalog.return_value = None
+ database.db_engine_spec.engine = "sqlite"
+ database.db_engine_spec.get_rls_method.return_value =
RLSMethod.AS_PREDICATE
+ db = mocker.patch("superset.utils.rls.db")
+ # SQLite folds unquoted identifiers, so datasets are looked up with
``all()``
+ db.session.query().filter().all.return_value = [mock_pd]
+
+ with (
+ patch(
+ "superset.connectors.sqla.models.security_manager.get_rls_filters",
+ return_value=[],
+ ),
+ patch(
+
"superset.connectors.sqla.models.security_manager.get_guest_rls_filters",
+ wraps=_guest_rls_filter(guest_user),
+ ),
+ patch(
+ "superset.connectors.sqla.models.is_feature_enabled",
+ return_value=True,
+ ),
+ patch(
+ "superset.models.helpers.is_feature_enabled",
+ return_value=True,
+ ),
+ ):
+ return validate_adhoc_subquery(sql, database, None, "public", "sqlite")
+
+
+def test_global_guest_rule_applied_to_adhoc_subquery(
+ app: Flask,
+ mocker: MockerFixture,
+) -> None:
+ """
+ Global (unscoped) guest RLS rules are injected into adhoc sub-queries.
+
+ Unlike a virtual dataset's inner SQL, an adhoc sub-query is not constrained
+ by the outer query's WHERE clause, so skipping the global guest rules there
+ would let a guest read rows from every tenant.
+ """
+ sql = _validate_adhoc_subquery_as_guest(
+ mocker,
+ [GuestTokenRlsRule(dataset=None, clause="org_id = 1")],
+ "SELECT MAX((SELECT COUNT(DISTINCT org_id) FROM physical_table))",
+ )
+
+ assert "org_id = 1" in sql, (
+ f"Global guest rule 'org_id = 1' must be applied inside the adhoc "
+ f"sub-query. Got: {sql}"
+ )
+
+
+def test_scoped_guest_rule_applied_to_adhoc_subquery(
+ app: Flask,
+ mocker: MockerFixture,
+) -> None:
+ """
+ Dataset-scoped guest RLS rules keep being injected into adhoc sub-queries
+ alongside global ones.
+ """
+ sql = _validate_adhoc_subquery_as_guest(
+ mocker,
+ [
+ GuestTokenRlsRule(dataset=None, clause="org_id = 1"),
+ GuestTokenRlsRule(dataset="42", clause="tenant_id = 5"),
+ ],
+ "SELECT (SELECT COUNT(*) FROM physical_table)",
+ )
+
+ assert "org_id = 1" in sql, f"Global guest rule missing. Got: {sql}"
+ assert "tenant_id = 5" in sql, f"Scoped guest rule missing. Got: {sql}"
diff --git a/tests/unit_tests/sql_lab_test.py b/tests/unit_tests/sql_lab_test.py
index e3e2a65957c..def0830eb27 100644
--- a/tests/unit_tests/sql_lab_test.py
+++ b/tests/unit_tests/sql_lab_test.py
@@ -656,12 +656,14 @@ def test_apply_rls(mocker: MockerFixture) -> None:
database,
"examples",
exclude_dataset_id=None,
+ include_global_guest_rls=True,
),
mocker.call(
Table("t2", "public", "examples"),
database,
"examples",
exclude_dataset_id=None,
+ include_global_guest_rls=True,
),
]
)
@@ -703,7 +705,7 @@ def test_get_predicates_for_table(mocker: MockerFixture) ->
None:
table = Table("t1", "public", "examples")
assert get_predicates_for_table(table, database, "examples") == ["c1 = 1"]
dataset.get_sqla_row_level_filters.assert_called_once_with(
- include_global_guest_rls=False
+ include_global_guest_rls=True
)
