This is an automated email from the ASF dual-hosted git repository.
rusackas 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 206fe7ab120 fix(embedded): stop rejecting guest chart data built from
control-specific params keys (#42295)
206fe7ab120 is described below
commit 206fe7ab1200a4d5e1869c6987ee756c2f351d95
Author: Evan Rusackas <[email protected]>
AuthorDate: Thu Jul 23 13:31:04 2026 -0700
fix(embedded): stop rejecting guest chart data built from control-specific
params keys (#42295)
Co-authored-by: Claude Code <[email protected]>
---
superset/security/manager.py | 74 ++++++++++++++++++++----
tests/unit_tests/security/manager_test.py | 93 +++++++++++++++++++++++++++++++
2 files changed, 156 insertions(+), 11 deletions(-)
diff --git a/superset/security/manager.py b/superset/security/manager.py
index 7a6eb2c717a..1040e00beff 100644
--- a/superset/security/manager.py
+++ b/superset/security/manager.py
@@ -987,6 +987,57 @@ def _orderby_modified(
return False
+#: Chart params keys that hold the metrics a chart renders. Different chart
+#: types store their metrics under control-specific keys (``metric`` for
+#: big number, ``x``/``y``/``size`` for bubble, and so on); a guest requesting
+#: the exact stored value reads nothing beyond what the chart already shows.
+_STORED_METRIC_PARAMS = (
+ "metrics",
+ "metric",
+ "percent_metrics",
+ "secondary_metric",
+ "series_limit_metric",
+ "timeseries_limit_metric",
+ "x",
+ "y",
+ "size",
+)
+
+#: Chart params keys that hold the columns/group-bys a chart renders, across
+#: the control names chart types use for them (``entity``/``series`` for
+#: bubble and world map, ``granularity_sqla`` for the temporal axis, etc.).
+_STORED_COLUMN_PARAMS = (
+ "columns",
+ "groupby",
+ "all_columns",
+ "entity",
+ "series",
+ "series_columns",
+ "x_axis",
+ "granularity_sqla",
+)
+
+
+def _stored_param_values(params: dict[str, Any], keys: tuple[str, ...]) ->
set[str]:
+ """
+ Frozen values stored under any of the given chart params keys.
+
+ Scalar-valued controls (``metric``, ``entity``, ...) contribute their
single
+ value; list-valued controls contribute each element. Matching stays exact
+ (via ``freeze_value``) — no label or expression equivalence is applied.
+ """
+ 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]
+ values.update(
+ freeze_value(item) for item in items if item is not None and item
!= ""
+ )
+ return values
+
+
def _columns_metrics_modified(
query_context: "QueryContext",
form_data: dict[str, Any],
@@ -998,19 +1049,20 @@ def _columns_metrics_modified(
chart exposes. Each requested set must be a subset of the values stored on
the chart (params and, when present, the stored query context).
"""
- for key, equivalent in [
- ("metrics", ["metrics"]),
- ("columns", ["columns", "groupby"]),
- ("groupby", ["columns", "groupby"]),
+ for key, stored_params_keys, equivalent in [
+ ("metrics", _STORED_METRIC_PARAMS, ["metrics"]),
+ ("columns", _STORED_COLUMN_PARAMS, ["columns", "groupby"]),
+ ("groupby", _STORED_COLUMN_PARAMS, ["columns", "groupby"]),
]:
requested_values = {freeze_value(value) for value in
form_data.get(key) or []}
- stored_values = {
- freeze_value(value) for value in stored_chart.params_dict.get(key)
or []
- }
- # ``form_data`` values are checked against ``params_dict`` alone;
- # ``query_context`` values are checked below against the fuller set
that
- # also includes the stored query context. This asymmetry is
intentional:
- # each requested source is compared to its corresponding stored source.
+ # Stored params are read across every control name that can hold a
+ # metric or column for some chart type: charts whose query is built
+ # from e.g. ``metric``/``entity``/``groupby`` never store a literal
+ # ``metrics``/``columns`` key, yet their generated payload uses those,
+ # and a guest replaying the chart's own values is not tampering.
+ stored_values = _stored_param_values(
+ stored_chart.params_dict, stored_params_keys
+ )
if not requested_values.issubset(stored_values):
return True
diff --git a/tests/unit_tests/security/manager_test.py
b/tests/unit_tests/security/manager_test.py
index 564e2597c7a..09d85b4cd4b 100644
--- a/tests/unit_tests/security/manager_test.py
+++ b/tests/unit_tests/security/manager_test.py
@@ -669,6 +669,99 @@ def test_query_context_modified_tampered(
assert query_context_modified(query_context)
+def test_query_context_modified_singular_metric_param(
+ mocker: MockerFixture,
+) -> None:
+ """
+ A chart storing its metric under the singular ``metric`` params key (big
+ number, world map, ...) generates a payload with a plural ``metrics`` list;
+ replaying the chart's own metric is not tampering.
+ """
+ query_context = mocker.MagicMock()
+ query_context.slice_.id = 42
+ query_context.slice_.query_context = None
+ query_context.slice_.params_dict = {
+ "metric": "sum__SP_POP_TOTL",
+ "groupby": [],
+ }
+
+ query_context.form_data = {
+ "slice_id": 42,
+ "metric": "sum__SP_POP_TOTL",
+ }
+ query_context.queries = [QueryObject(metrics=["sum__SP_POP_TOTL"])]
+ assert not query_context_modified(query_context)
+
+
+def test_query_context_modified_control_specific_column_params(
+ mocker: MockerFixture,
+) -> None:
+ """
+ Charts store their queried columns under control-specific params keys
+ (``entity``/``series`` for bubble, ``groupby`` for most, the temporal
+ column under ``granularity_sqla``); the generated payload carries them in
+ ``columns``, which must not read as tampering.
+ """
+ query_context = mocker.MagicMock()
+ query_context.slice_.id = 42
+ query_context.slice_.query_context = None
+ query_context.slice_.params_dict = {
+ "entity": "country_name",
+ "series": "region",
+ "granularity_sqla": "year",
+ "x": "sum__SP_RUR_TOTL_ZS",
+ "y": "sum__SP_DYN_LE00_IN",
+ "size": "sum__SP_POP_TOTL",
+ }
+
+ query_context.form_data = {"slice_id": 42}
+ query_context.queries = [
+ QueryObject(
+ columns=["country_name", "region", "year"],
+ metrics=[
+ "sum__SP_RUR_TOTL_ZS",
+ "sum__SP_DYN_LE00_IN",
+ "sum__SP_POP_TOTL",
+ ],
+ )
+ ]
+ assert not query_context_modified(query_context)
+
+
+def test_query_context_modified_novel_values_still_tampered(
+ mocker: MockerFixture,
+) -> None:
+ """
+ The control-specific params equivalence only authorizes exact stored
+ values: a metric or column the chart does not reference anywhere is still
+ rejected, as is an adhoc expression label-spoofing a stored column.
+ """
+ query_context = mocker.MagicMock()
+ query_context.slice_.id = 42
+ query_context.slice_.query_context = None
+ query_context.slice_.params_dict = {
+ "metric": "sum__SP_POP_TOTL",
+ "entity": "country_code",
+ "granularity_sqla": "year",
+ }
+ query_context.form_data = {"slice_id": 42}
+
+ query_context.queries = [QueryObject(metrics=["sum__SH_DYN_AIDS"])]
+ assert query_context_modified(query_context)
+
+ query_context.queries = [QueryObject(columns=["some_other_column"])]
+ assert query_context_modified(query_context)
+
+ query_context.queries = [
+ QueryObject(
+ columns=[
+ {"label": "country_code", "sqlExpression": "(select 1)"},
+ ],
+ )
+ ]
+ assert query_context_modified(query_context)
+
+
def _native_filter_ctx(
mocker: MockerFixture,
queries: list[Any],
