fix(security): prevent scalar control string iteration in guest anti-tamper check (#43576)

superset/security/manager.py

@@ -760,6 +760,29 @@ def freeze_value(value: Any) -> str:
     return json.dumps(_strip_overridable_keys(value), sort_keys=True)
 
 
+def _ensure_list(value: Any) -> list[Any]:
+    """
+    Normalize a value to a list for iteration.
+
+    Some viz types (e.g. heatmap_v2's 'groupby' control) store a single
+    value as a bare string rather than a one-item list. Iterating a string
+    directly yields its individual characters, which silently breaks the
+    guest payload comparison for any such chart.
+
+    ``None`` and an empty string are treated as "no value set" (mirroring
+    ``_stored_param_values``'s treatment of an unset control) and return
+    ``[]`` — an unset scalar control must not be compared as if the guest
+    had explicitly requested an empty string. A ``list``/``tuple`` is
+    filtered the same way, element by element; any other scalar is wrapped
+    in a single-item list.
+    """
+    if value is None or value == "":
+        return []
+    if isinstance(value, (list, tuple, set)):
+        return [item for item in value if item is not None and item != ""]
+    return [value]
+
+
 # Frontend-only markers that ``normalizeTimeColumn`` adds when it synthesizes a
 # chart's x-axis into a ``BASE_AXIS`` column. Like ``timeGrain`` they decorate
 # the column without changing which data is queried, so they must not count as
@@ -905,10 +928,10 @@ def _native_filter_query_modified(
     # Columns, group-by, and series columns may only reference target column(s);
     # adhoc (free-form SQL) columns cannot be validated, so reject them.
     for key in ("columns", "groupby", "series_columns"):
-        for col in getattr(query, key, None) or []:
+        for col in _ensure_list(getattr(query, key, None)):
             if not isinstance(col, str) or col not in allowed_columns:
                 return True
-    for metric in getattr(query, "metrics", None) or []:
+    for metric in _ensure_list(getattr(query, "metrics", None)):
         if not _native_filter_term_allowed(metric, allowed_columns, allowed_metrics):
             return True
     # A series-limit metric ranks the top-N groups in the inner query, so it is
@@ -921,7 +944,7 @@ def _native_filter_query_modified(
     ):
         return True
     # order-by entries are ``(expression, asc)`` pairs.
-    for order in getattr(query, "orderby", None) or []:
+    for order in _ensure_list(getattr(query, "orderby", None)):
         expr = order[0] if isinstance(order, (list, tuple)) and order else order
         if not _native_filter_term_allowed(expr, allowed_columns, allowed_metrics):
             return True
@@ -1157,23 +1180,22 @@ def _collect_stored_orderby_entries(
     Frozen saved orderby entries a guest may replay exactly.
     """
     allowed: set[str] = {
-        freeze_value(entry) for entry in stored_chart.params_dict.get("orderby") or []
+        freeze_value(entry)
+        for entry in _ensure_list(stored_chart.params_dict.get("orderby"))
     }
     if stored_query_context:
         for query in stored_query_context.get("queries") or []:
-            allowed.update(freeze_value(entry) for entry in query.get("orderby") or [])
+            allowed.update(
+                freeze_value(entry) for entry in _ensure_list(query.get("orderby"))
+            )
     return allowed
 
 
 def _metric_control_values(value: Any) -> list[Any]:
     """
     Return non-empty values from a metric-valued control.
     """
-    if value is None or value == "":
-        return []
-    if isinstance(value, (list, tuple)):
-        return [item for item in value if item is not None and item != ""]
-    return [value]
+    return _ensure_list(value)
 
 
 def _add_frozen_metric_control_values(allowed: set[str], value: Any) -> None:
@@ -1372,7 +1394,7 @@ def _add_allowed_sql_from_query_context(
             if composed:
                 extras_allowed.add(composed)
         for key in ("columns", "groupby"):
-            for col in query.get(key) or []:
+            for col in _ensure_list(query.get(key)):
                 if isinstance(col, dict) and col.get("sqlExpression"):
                     col_allowed.add(col["sqlExpression"])
 
@@ -1569,11 +1591,11 @@ def _sql_filters_modified(
     # Vector 2: SQL adhoc filters in form_data
     stored_sql_filters: set[str] = {
         freeze_value(flt)
-        for flt in stored_chart.params_dict.get("adhoc_filters") or []
+        for flt in _ensure_list(stored_chart.params_dict.get("adhoc_filters"))
         if isinstance(flt, dict) and flt.get("expressionType") == "SQL"
     }
 
-    for flt in form_data.get("adhoc_filters") or []:
+    for flt in _ensure_list(form_data.get("adhoc_filters")):
         if not isinstance(flt, dict):
             continue
         if flt.get("expressionType") == "SQL":
@@ -1624,38 +1646,13 @@ def _stored_param_values(params: dict[str, Any], keys: tuple[str, ...]) -> set[s
     """
     values: set[str] = set()
     for key in keys:
-        value = params.get(key)
-        if value is None or value == "":
-            continue
-        items = value if isinstance(value, (list, tuple)) else [value]
+        items = _ensure_list(params.get(key))
         values.update(
             freeze_value(item) for item in items if item is not None and item != ""
         )
     return values
 
 
-def _ensure_list(value: Any) -> list[Any]:
-    """
-    Normalize a value to a list for iteration.
-
-    Some viz types (e.g. heatmap_v2's 'groupby' control) store a single
-    value as a bare string rather than a one-item list. Iterating a string
-    directly yields its individual characters, which silently breaks the
-    guest payload comparison for any such chart.
-
-    ``None`` and an empty string are treated as "no value set" (mirroring
-    ``_stored_param_values``'s treatment of an unset control) and return
-    ``[]`` — an unset scalar control must not be compared as if the guest
-    had explicitly requested an empty string. A ``list``/``tuple`` is
-    filtered the same way, element by element; any other scalar is wrapped
-    in a single-item list.
-    """
-    if value is None:
-        return []
-    items = value if isinstance(value, (list, tuple)) else [value]
-    return [item for item in items if item is not None and item != ""]
-
-
 def _columns_metrics_modified(
     query_context: "QueryContext",
     form_data: dict[str, Any],
@@ -1711,7 +1708,7 @@ def _columns_metrics_modified(
                 for equiv_key in equivalent:
                     stored_values.update(
                         _payload_value_identity(value, is_metric=is_metric)
-                        for value in query.get(equiv_key) or []
+                        for value in _ensure_list(query.get(equiv_key))
                     )
 
         if not queries_values.issubset(stored_values):

tests/unit_tests/db_engine_specs/test_crdb.py

@@ -44,6 +44,26 @@ def test_convert_dttm(
     assert_convert_dttm(spec, target_type, expected_result, dttm)
 
 
+def test_cockroachdb_properties() -> None:
+    from superset.db_engine_specs.cockroachdb import CockroachDbEngineSpec
+    from superset.db_engine_specs.postgres import PostgresEngineSpec
+
+    assert CockroachDbEngineSpec.engine == "cockroachdb"
+    assert CockroachDbEngineSpec.engine_name == "CockroachDB"
+    assert issubclass(CockroachDbEngineSpec, PostgresEngineSpec)
+    assert CockroachDbEngineSpec._extended_aggregations == {}
+
+
+def test_cockroachdb_metadata() -> None:
+    from superset.db_engine_specs.cockroachdb import CockroachDbEngineSpec
+
+    metadata = CockroachDbEngineSpec.metadata
+    assert "CockroachDB is a distributed SQL database" in metadata["description"]
+    assert metadata["logo"] == "cockroachdb.png"
+    assert "sqlalchemy-cockroachdb" in metadata["pypi_packages"]
+    assert metadata["default_port"] == 26257
+
+
 def test_dialect_loads_under_installed_sqlalchemy() -> None:
     """
     ``create_engine`` resolves and imports the ``cockroachdb`` SQLAlchemy

tests/unit_tests/db_engine_specs/test_duckdb.py

@@ -395,3 +395,22 @@ def test_extended_aggregation_func_median_stddev_var_executes() -> None:
             query = select(func(literal_column("sales"))).select_from(text("t"))
             result = conn.execute(query).scalar()
             assert result == pytest.approx(expected_value)
+
+
+def test_motherduck_properties() -> None:
+    from superset.db_engine_specs.duckdb import DuckDBEngineSpec, MotherDuckEngineSpec
+
+    assert MotherDuckEngineSpec.engine == "motherduck"
+    assert MotherDuckEngineSpec.engine_name == "MotherDuck"
+    assert issubclass(MotherDuckEngineSpec, DuckDBEngineSpec)
+
+
+def test_motherduck_metadata() -> None:
+    from superset.db_engine_specs.duckdb import MotherDuckEngineSpec
+
+    metadata = MotherDuckEngineSpec.metadata
+    assert (
+        "MotherDuck is a serverless cloud analytics platform" in metadata["description"]
+    )
+    assert metadata["logo"] == "motherduck.png"
+    assert "duckdb-engine" in metadata["pypi_packages"]

tests/unit_tests/db_engine_specs/test_dynamodb.py

@@ -46,6 +46,50 @@ def test_convert_dttm(
     assert_convert_dttm(spec, target_type, expected_result, dttm)
 
 
+def test_dynamodb_properties() -> None:
+    from superset.db_engine_specs.dynamodb import DynamoDBEngineSpec
+
+    assert DynamoDBEngineSpec.engine == "dynamodb"
+    assert DynamoDBEngineSpec.engine_name == "Amazon DynamoDB"
+
+
+def test_dynamodb_metadata() -> None:
+    from superset.db_engine_specs.dynamodb import DynamoDBEngineSpec
+
+    metadata = DynamoDBEngineSpec.metadata
+    assert "Amazon DynamoDB is a serverless NoSQL database" in metadata["description"]
+    assert metadata["logo"] == "aws.png"
+    assert "pydynamodb" in metadata["pypi_packages"]
+
+
+def test_epoch_to_dttm() -> None:
+    from superset.db_engine_specs.dynamodb import DynamoDBEngineSpec
+
+    assert (
+        DynamoDBEngineSpec.epoch_to_dttm().format(col="ts")
+        == "datetime(ts, 'unixepoch')"
+    )
+
+
+@pytest.mark.parametrize(
+    "time_grain,expected",
+    [
+        (None, "ts"),
+        ("PT1S", "DATETIME(STRFTIME('%Y-%m-%dT%H:%M:%S', ts))"),
+        ("PT1M", "DATETIME(STRFTIME('%Y-%m-%dT%H:%M:00', ts))"),
+        ("PT1H", "DATETIME(STRFTIME('%Y-%m-%dT%H:00:00', ts))"),
+        ("P1D", "DATETIME(ts, 'start of day')"),
+        ("P1M", "DATETIME(ts, 'start of month')"),
+        ("P1Y", "DATETIME(ts, 'start of year')"),
+    ],
+)
+def test_time_grain_expressions(time_grain: str | None, expected: str) -> None:
+    from superset.db_engine_specs.dynamodb import DynamoDBEngineSpec
+
+    actual = DynamoDBEngineSpec._time_grain_expressions[time_grain].format(col="ts")
+    assert actual == expected
+
+
 def test_convert_dttm_bounds_compare_with_iso_8601_strings() -> None:
     from superset.db_engine_specs.dynamodb import (
         DynamoDBEngineSpec as spec,  # noqa: N813

tests/unit_tests/db_engine_specs/test_firebolt.py

@@ -21,6 +21,7 @@
 
 import pytest
 
+from superset.constants import TimeGrain
 from tests.unit_tests.db_engine_specs.utils import assert_convert_dttm
 from tests.unit_tests.fixtures.common import dttm  # noqa: F401
 
@@ -59,3 +60,43 @@ def test_epoch_to_dttm() -> None:
         FireboltEngineSpec.epoch_to_dttm().format(col="timestamp_column")
         == "from_unixtime(timestamp_column)"
     )
+
+
+def test_firebolt_properties() -> None:
+    from superset.db_engine_specs.firebolt import FireboltEngineSpec
+
+    assert FireboltEngineSpec.engine == "firebolt"
+    assert FireboltEngineSpec.engine_name == "Firebolt"
+    assert FireboltEngineSpec.default_driver == "firebolt"
+
+
+def test_firebolt_metadata() -> None:
+    from superset.db_engine_specs.firebolt import FireboltEngineSpec
+
+    metadata = FireboltEngineSpec.metadata
+    assert "Firebolt is a cloud data warehouse" in metadata["description"]
+    assert metadata["logo"] == "firebolt.png"
+    assert "firebolt-sqlalchemy" in metadata["pypi_packages"]
+
+
+@pytest.mark.parametrize(
+    "time_grain,expected",
+    [
+        (None, "ts"),
+        (TimeGrain.SECOND, "date_trunc('second', CAST(ts AS TIMESTAMP))"),
+        (TimeGrain.MINUTE, "date_trunc('minute', CAST(ts AS TIMESTAMP))"),
+        (TimeGrain.HOUR, "date_trunc('hour', CAST(ts AS TIMESTAMP))"),
+        (TimeGrain.DAY, "date_trunc('day', CAST(ts AS TIMESTAMP))"),
+        (TimeGrain.WEEK, "date_trunc('week', CAST(ts AS TIMESTAMP))"),
+        (TimeGrain.MONTH, "date_trunc('month', CAST(ts AS TIMESTAMP))"),
+        (TimeGrain.QUARTER, "date_trunc('quarter', CAST(ts AS TIMESTAMP))"),
+        (TimeGrain.YEAR, "date_trunc('year', CAST(ts AS TIMESTAMP))"),
+    ],
+)
+def test_time_grain_expressions(time_grain: str | None, expected: str) -> None:
+    from superset.db_engine_specs.firebolt import FireboltEngineSpec
+
+    assert (
+        FireboltEngineSpec._time_grain_expressions[time_grain].format(col="ts")
+        == expected
+    )

tests/unit_tests/db_engine_specs/test_gsheets.py

@@ -1276,6 +1276,24 @@ def test_convert_dttm(
     assert_convert_dttm(GSheetsEngineSpec, target_type, expected_result, dttm)
 
 
+def test_gsheets_properties() -> None:
+    from superset.db_engine_specs.gsheets import GSheetsEngineSpec
+    from superset.db_engine_specs.shillelagh import ShillelaghEngineSpec
+
+    assert GSheetsEngineSpec.engine == "gsheets"
+    assert GSheetsEngineSpec.engine_name == "Google Sheets"
+    assert issubclass(GSheetsEngineSpec, ShillelaghEngineSpec)
+
+
+def test_gsheets_metadata() -> None:
+    from superset.db_engine_specs.gsheets import GSheetsEngineSpec
+
+    metadata = GSheetsEngineSpec.metadata
+    assert "Google Sheets" in metadata["description"]
+    assert metadata["logo"] == "google-sheets.svg"
+    assert "shillelagh[gsheetsapi]" in metadata["pypi_packages"]
+
+
 def test_upload_dates(mocker: MockerFixture) -> None:
     """
     Test that date and numpy values are uploaded as JSON values.

tests/unit_tests/db_engine_specs/test_mssql.py

@@ -879,3 +879,21 @@ def test_get_catalog_from_engine_params_empty_database_query_param_falls_back_to
     )
     assert "database" not in url.query
     assert MssqlEngineSpec.get_catalog_from_engine_params(url, {}) == "path_db"
+
+
+def test_azure_synapse_properties() -> None:
+    from superset.db_engine_specs.mssql import AzureSynapseSpec, MssqlEngineSpec
+
+    assert AzureSynapseSpec.engine == "mssql"
+    assert AzureSynapseSpec.engine_name == "Azure Synapse"
+    assert AzureSynapseSpec.default_driver == "pyodbc"
+    assert issubclass(AzureSynapseSpec, MssqlEngineSpec)
+
+
+def test_azure_synapse_metadata() -> None:
+    from superset.db_engine_specs.mssql import AzureSynapseSpec
+
+    metadata = AzureSynapseSpec.metadata
+    assert "Azure Synapse Analytics" in metadata["description"]
+    assert metadata["logo"] == "azure.svg"
+    assert "pymssql" in metadata["pypi_packages"]