From 772a94ae1bb2deebdb7fd17a19bf295467a5d274 Mon Sep 17 00:00:00 2001 From: sha174n Date: Thu, 25 Jun 2026 16:05:54 +0100 Subject: [PATCH 1/2] fix(datasource): validate expressions through the shared adhoc-expression checks Route validate_expression through validate_stored_expression so custom expression validation applies the same single-statement / no-set-operation / no-subquery (unless ALLOW_ADHOC_SUBQUERY) parsing policy already used for stored adhoc column and metric expressions, keeping one parsing policy across the query pipeline. Adds regression coverage that the validator rejects sub-queries and set operations before the validation query is built or run. --- superset/models/helpers.py | 11 ++++++ .../models/test_validate_expression.py | 38 ++++++++++++++++--- 2 files changed, 44 insertions(+), 5 deletions(-) diff --git a/superset/models/helpers.py b/superset/models/helpers.py index 72ce09a390d9..12362b0139a2 100644 --- a/superset/models/helpers.py +++ b/superset/models/helpers.py @@ -2899,6 +2899,17 @@ def validate_expression( tp = self.get_template_processor() processed_expression = self._process_expression_template(expression, tp) + # Apply the same parsing policy used for stored adhoc column and + # metric expressions (single statement, no set operations, and no + # sub-queries unless ALLOW_ADHOC_SUBQUERY is enabled), so expression + # validation follows one policy across the query pipeline. Imported + # locally to avoid a circular import with the connectors package. + from superset.connectors.sqla.models import validate_stored_expression + + validate_stored_expression( + self.database, self.catalog, self.schema or "", processed_expression + ) + # Build validation query tbl, cte = self.get_from_clause(tp) validation_query = self._build_validation_query( diff --git a/tests/unit_tests/models/test_validate_expression.py b/tests/unit_tests/models/test_validate_expression.py index c4e18e4526c7..ef9a3ea7f906 100644 --- a/tests/unit_tests/models/test_validate_expression.py +++ b/tests/unit_tests/models/test_validate_expression.py @@ -33,6 +33,7 @@ def setup_method(self): self.table.schema = "test_schema" self.table.catalog = None self.table.database = MagicMock() + self.table.database.backend = "sqlite" self.table.database.db_engine_spec = MagicMock() self.table.database.db_engine_spec.make_sqla_column_compatible = lambda x, _: x self.table.columns = [] @@ -105,10 +106,8 @@ def test_validate_having_expression(self, mock_execute): @patch("superset.connectors.sqla.models.SqlaTable._execute_validation_query") def test_validate_invalid_expression(self, mock_execute): - """Test validation of invalid SQL expressions""" - # Mock _execute_validation_query to raise an exception - mock_execute.side_effect = Exception("Invalid SQL syntax") - + """Unparseable SQL is rejected by the shared expression parser before the + validation query is built or executed.""" result = self.table.validate_expression( expression="INVALID SQL HERE", expression_type=SqlExpressionType.COLUMN, @@ -116,7 +115,8 @@ def test_validate_invalid_expression(self, mock_execute): assert result["valid"] is False assert len(result["errors"]) == 1 - assert "Invalid SQL syntax" in result["errors"][0]["message"] + assert result["errors"][0]["message"] + mock_execute.assert_not_called() @patch("superset.connectors.sqla.models.SqlaTable._execute_validation_query") def test_validate_having_with_non_aggregated_column(self, mock_execute): @@ -152,6 +152,34 @@ def test_validate_empty_expression(self, mock_execute): # The actual error message will come from the exception assert "empty" in result["errors"][0]["message"].lower() + @patch("superset.models.helpers.is_feature_enabled", return_value=False) + @patch("superset.connectors.sqla.models.SqlaTable._execute_validation_query") + def test_validate_expression_rejects_subquery(self, mock_execute, mock_ff): + """A sub-query expression is rejected by the same validate_adhoc_subquery + gate used for stored adhoc expressions, before any validation query is + built or run (with ALLOW_ADHOC_SUBQUERY off, the default). Locks in that + expression validation never executes the sub-query.""" + result = self.table.validate_expression( + expression="(SELECT 1) IS NOT NULL OR 1 = 1", + expression_type=SqlExpressionType.WHERE, + ) + + assert result["valid"] is False + mock_execute.assert_not_called() + + @patch("superset.models.helpers.is_feature_enabled", return_value=False) + @patch("superset.connectors.sqla.models.SqlaTable._execute_validation_query") + def test_validate_expression_rejects_set_operation(self, mock_execute, mock_ff): + """A set-operation expression is rejected before the validation query is + built or run, matching the stored-adhoc-expression policy.""" + result = self.table.validate_expression( + expression="1 UNION SELECT 1", + expression_type=SqlExpressionType.WHERE, + ) + + assert result["valid"] is False + mock_execute.assert_not_called() + @patch("superset.connectors.sqla.models.SqlaTable._execute_validation_query") def test_validate_expression_with_rls(self, mock_execute): """Test that RLS filters are applied during validation""" From 8e13c805961705411b09d2e2fcf8ea2e9aa3f68a Mon Sep 17 00:00:00 2001 From: sha174n Date: Thu, 25 Jun 2026 18:09:41 +0100 Subject: [PATCH 2/2] test: add type hints to new expression-validation test methods Co-Authored-By: Claude Opus 4.8 --- tests/unit_tests/models/test_validate_expression.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/tests/unit_tests/models/test_validate_expression.py b/tests/unit_tests/models/test_validate_expression.py index ef9a3ea7f906..5623fc6a4ff1 100644 --- a/tests/unit_tests/models/test_validate_expression.py +++ b/tests/unit_tests/models/test_validate_expression.py @@ -154,7 +154,9 @@ def test_validate_empty_expression(self, mock_execute): @patch("superset.models.helpers.is_feature_enabled", return_value=False) @patch("superset.connectors.sqla.models.SqlaTable._execute_validation_query") - def test_validate_expression_rejects_subquery(self, mock_execute, mock_ff): + def test_validate_expression_rejects_subquery( + self, mock_execute: MagicMock, mock_ff: MagicMock + ) -> None: """A sub-query expression is rejected by the same validate_adhoc_subquery gate used for stored adhoc expressions, before any validation query is built or run (with ALLOW_ADHOC_SUBQUERY off, the default). Locks in that @@ -169,7 +171,9 @@ def test_validate_expression_rejects_subquery(self, mock_execute, mock_ff): @patch("superset.models.helpers.is_feature_enabled", return_value=False) @patch("superset.connectors.sqla.models.SqlaTable._execute_validation_query") - def test_validate_expression_rejects_set_operation(self, mock_execute, mock_ff): + def test_validate_expression_rejects_set_operation( + self, mock_execute: MagicMock, mock_ff: MagicMock + ) -> None: """A set-operation expression is rejected before the validation query is built or run, matching the stored-adhoc-expression policy.""" result = self.table.validate_expression(