fix(embedded): compare each query's result type against its own stored query, not the flattened set

_result_type_modified previously unioned every stored query's result_type
into one set and checked requested result types against that set, so a
result type stored on one query (e.g. samples) could be requested for a
different query in the same query context. Compare each requested query's
effective result type (its own result_type, falling back to the query
context's top-level result_type - mirroring how the value is resolved at
render time) against the corresponding stored query at the same position
instead.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

superset/async_events/async_query_manager.py

@@ -238,24 +238,17 @@ class AsyncQueryManager:
         secret so the value is unguessable to outside callers.
         """
         token = guest_user.guest_token
-        # ``iat`` uniquely identifies a guest token issuance, so it provides
-        # per-token isolation while remaining stable across the lifetime of a
-        # single embedded session.
-        message = json.dumps(
-            {
-                "user": token.get("user"),
-                "resources": token.get("resources"),
-                "iat": token.get("iat"),
-                "exp": token.get("exp"),
-                "aud": token.get("aud"),
-                # ``datasets`` and ``rev`` are optional scope claims, so tokens
-                # that differ only in their dataset allowlist or revocation
-                # version still derive distinct channels.
-                "datasets": token.get("datasets"),
-                "rev": token.get("rev"),
-            },
-            sort_keys=True,
-        ).encode("utf-8")
+        # HMAC over the complete claim set so that tokens differing in *any*
+        # claim derive distinct channels. Enumerating claims here is unsafe:
+        # omitting one that scopes the session -- most importantly
+        # ``rls_rules``, the primary tenant-isolation mechanism for embedded
+        # dashboards -- would let two tenants' tokens minted in the same
+        # second with identical user/resources collide on one channel,
+        # exposing job events (including error strings) and cross-tenant
+        # cancellation. ``iat`` uniquely identifies a token issuance, so it
+        # provides per-token isolation while remaining stable across the
+        # lifetime of a single embedded session.
+        message = json.dumps(token, sort_keys=True).encode("utf-8")
         digest = hmac.new(
             self._jwt_secret.encode("utf-8"), message, hashlib.sha256
         ).hexdigest()

superset/embedded/view.py

@@ -59,7 +59,6 @@ class EmbeddedView(BaseSupersetView):
             abort(404)
 
         assert embedded is not None
-        dashboard = embedded.dashboard
 
         # validate request referrer in allowed domains
         is_referrer_allowed = not embedded.allowed_domains
@@ -109,11 +108,14 @@ class EmbeddedView(BaseSupersetView):
             },
         }
 
+        # This page renders before any guest token has been presented, and the
+        # Referer / Sec-Fetch-Dest checks above are browser cooperation only --
+        # a non-browser client can forge or omit both. Serve a neutral shell:
+        # no dashboard title or description here; the embedded SPA fetches
+        # dashboard metadata through the guest-token-authenticated API.
         return self.render_template(
             "superset/spa.html",
             entry="embedded",
-            title=dashboard.dashboard_title,
-            dashboard_description=dashboard.description,
             bootstrap_data=json.dumps(
                 bootstrap_data, default=json.pessimistic_json_iso_dttm_ser
             ),

superset/security/manager.py

@@ -73,6 +73,7 @@ from sqlalchemy.orm.mapper import Mapper
 from sqlalchemy.orm.query import Query as SqlaQuery
 from sqlalchemy.sql import exists
 
+from superset.common.chart_data import ChartDataResultType
 from superset.constants import RouteMethod
 from superset.errors import ErrorLevel, SupersetError, SupersetErrorType
 from superset.exceptions import (
@@ -1562,6 +1563,146 @@ def _columns_metrics_modified(
     return False
 
 
+def _annotation_layer_identity(layer: Any) -> Optional[tuple[str, str]]:
+    """
+    Identity of an annotation layer for tamper comparison: the source type and
+    the underlying source it reads (a native annotation-layer id or a chart
+    id). Cosmetic keys (``name``, styling, overrides) are not part of the
+    identity. Returns ``None`` for a malformed (non-dict) layer.
+    """
+    if not isinstance(layer, dict):
+        return None
+    return (
+        freeze_value(layer.get("sourceType")),
+        freeze_value(layer.get("value")),
+    )
+
+
+def _annotation_layers_modified(
+    query_context: "QueryContext",
+    form_data: dict[str, Any],
+    stored_chart: "Slice",
+    stored_query_context: Optional[dict[str, Any]],
+) -> bool:
+    """
+    Whether the request references annotation layers the stored chart does
+    not already carry.
+
+    ``annotation_layers`` is accepted on any query object, and native layers
+    resolve every annotation of each referenced layer id with no further
+    access check, so a guest injecting a layer the chart was not saved with
+    would read data that was never shared with them. Replaying the chart's
+    own stored layers is not tampering.
+    """
+    requested: set[Optional[tuple[str, str]]] = {
+        _annotation_layer_identity(layer)
+        for layer in form_data.get("annotation_layers") or []
+    }
+    requested.update(
+        _annotation_layer_identity(layer)
+        for query in query_context.queries
+        for layer in getattr(query, "annotation_layers", None) or []
+    )
+    if not requested:
+        return False
+    # A malformed (non-dict) layer is nothing the frontend produces from a
+    # stored chart; treat it as tampering rather than crashing on it later.
+    if None in requested:
+        return True
+
+    stored: set[Optional[tuple[str, str]]] = {
+        _annotation_layer_identity(layer)
+        for layer in stored_chart.params_dict.get("annotation_layers") or []
+    }
+    if stored_query_context:
+        for query in stored_query_context.get("queries") or []:
+            stored.update(
+                _annotation_layer_identity(layer)
+                for layer in query.get("annotation_layers") or []
+            )
+    return not requested.issubset(stored)
+
+
+#: Result types that make the server rewrite the query to return raw rows of
+#: every datasource column (``_prepare_samples_query`` and
+#: ``_prepare_drill_detail_query`` in ``superset.common.query_actions``).
+_ROW_EXPANDING_RESULT_TYPES = {
+    ChartDataResultType.SAMPLES.value,
+    ChartDataResultType.DRILL_DETAIL.value,
+}
+
+
+def _result_type_value(result_type: Any) -> str:
+    """Normalize a result type (enum member or raw string) to its value."""
+    return str(getattr(result_type, "value", result_type)).lower()
+
+
+def _effective_result_type(
+    query_result_type: Any, default_result_type: Any
+) -> Optional[str]:
+    """
+    The result type a query actually runs with: its own ``result_type`` if
+    set, else the query context's top-level default.
+
+    Mirrors ``query_obj.result_type or query_context.result_type``
+    (``QueryContextProcessor.get_payload``), so this reads the same value the
+    server uses to pick the samples/drill_detail preparer for that query.
+    """
+    if query_result_type:
+        return _result_type_value(query_result_type)
+    if default_result_type:
+        return _result_type_value(default_result_type)
+    return None
+
+
+def _result_type_modified(
+    query_context: "QueryContext",
+    stored_query_context: Optional[dict[str, Any]],
+) -> bool:
+    """
+    Whether the request asks for a result type that expands one of its
+    queries to raw datasource rows beyond what the stored chart runs at that
+    same query position.
+
+    The ``samples`` and ``drill_detail`` preparers replace a query's columns
+    with every column on the datasource - and drop its metrics - *after*
+    ``raise_for_access`` has run, so the subset comparisons on columns and
+    metrics in ``query_context_modified`` still pass while the response
+    contains the full underlying table. A guest's entitlement is only what
+    each query on the stored chart itself renders, so each requested query's
+    effective result type is compared against its own corresponding stored
+    query's effective result type by position - never against result types
+    used by other queries in the same query context - matching how
+    ``query_obj.result_type or query_context.result_type`` is resolved
+    per-query at runtime.
+    """
+    stored_queries: list[dict[str, Any]] = []
+    stored_default_result_type: Any = None
+    if stored_query_context:
+        stored_default_result_type = stored_query_context.get("result_type")
+        stored_queries = [
+            stored_query
+            for stored_query in stored_query_context.get("queries") or []
+            if isinstance(stored_query, dict)
+        ]
+
+    for index, query in enumerate(query_context.queries):
+        requested = _effective_result_type(query.result_type, query_context.result_type)
+        if requested not in _ROW_EXPANDING_RESULT_TYPES:
+            continue
+        stored = (
+            _effective_result_type(
+                stored_queries[index].get("result_type"), stored_default_result_type
+            )
+            if index < len(stored_queries)
+            else None
+        )
+        if requested != stored:
+            return True
+
+    return False
+
+
 def query_context_modified(query_context: "QueryContext") -> bool:
     """
     Check if a query context has been modified.
@@ -1613,6 +1754,17 @@ def query_context_modified(query_context: "QueryContext") -> bool:
     # Use ``is not None`` so an empty-but-present stored context reads as present.
     stored_context_state = "present" if stored_query_context is not None else "missing"
 
+    # Reject result types that would have the server expand the query to raw
+    # datasource rows regardless of the stored chart's columns and metrics.
+    if _result_type_modified(query_context, stored_query_context):
+        logger.warning(
+            "Guest chart payload rejected for slice %s: result type expands "
+            "the chart to raw datasource rows (stored query_context %s)",
+            stored_chart.id,
+            stored_context_state,
+        )
+        return True
+
     # compare columns and metrics in form_data with stored values. Order-by is
     # handled separately: a strict subset check there would reject a guest
     # legitimately sorting an embedded chart by one of its existing columns.
@@ -1652,6 +1804,20 @@ def query_context_modified(query_context: "QueryContext") -> bool:
         )
         return True
 
+    # Native annotation layers resolve every annotation of each referenced
+    # layer with no further access check on this path, so a layer the chart
+    # was not saved with reads data that was never shared with the guest.
+    if _annotation_layers_modified(
+        query_context, form_data, stored_chart, stored_query_context
+    ):
+        logger.warning(
+            "Guest chart payload rejected for slice %s: annotation layer not "
+            "on the stored chart (stored query_context %s)",
+            stored_chart.id,
+            stored_context_state,
+        )
+        return True
+
     # SQL predicates (extras.where/having, SQL adhoc filters) must match
     # what was saved on the chart; injected custom SQL is rejected.
     if _sql_filters_modified(

tests/integration_tests/embedded/test_view.py

@@ -138,3 +138,26 @@ def test_get_embedded_dashboard_allows_iframe_sec_fetch_dest(
     uri = f"embedded/{embedded.uuid}"
     response = client.get(uri, headers={"Sec-Fetch-Dest": "iframe"})
     assert response.status_code == 200
+
+
+@pytest.mark.usefixtures("load_birth_names_dashboard_with_slices")
+@mock.patch.dict(
+    "superset.extensions.feature_flag_manager._feature_flags",
+    EMBEDDED_SUPERSET=True,
+)
+def test_get_embedded_dashboard_is_neutral_shell(client: FlaskClient[Any]):  # noqa: F811
+    """The pre-token page must not disclose dashboard metadata.
+
+    The Referer / Sec-Fetch-Dest checks are browser cooperation only -- a
+    non-browser client can forge them -- so anything rendered here is
+    effectively public. Title and description belong behind the
+    guest-token-authenticated API.
+    """
+    dash = db.session.query(Dashboard).filter_by(slug="births").first()
+    dash.description = "internal-only dashboard description"
+    embedded = EmbeddedDashboardDAO.upsert(dash, [])
+    db.session.flush()
+    response = client.get(f"embedded/{embedded.uuid}")
+    assert response.status_code == 200
+    assert dash.dashboard_title.encode() not in response.data
+    assert b"internal-only dashboard description" not in response.data

tests/unit_tests/async_events/async_query_manager_tests.py

@@ -263,6 +263,44 @@ def test_parse_channel_id_from_request_as_guest_user_differs_per_scope(
     assert with_datasets != with_rev
 
 
+@mock.patch("superset.is_feature_enabled")
+def test_parse_channel_id_from_request_as_guest_user_differs_per_rls_rules(
+    is_feature_enabled_mock, async_query_manager
+):
+    """
+    Tokens that differ only in their ``rls_rules`` must derive distinct
+    channel ids. Two tenants embedded via the same dashboard are typically
+    minted tokens with identical user/resources blocks and, with an
+    externally minted integer-second ``iat``, the same issuance time; if the
+    channel ignored ``rls_rules``, tenant B could observe tenant A's job
+    events and cancel A's jobs.
+    """
+    is_feature_enabled_mock.return_value = True
+
+    base_token = {
+        "user": {},
+        "resources": [{"type": "dashboard", "id": "some-uuid"}],
+        "rls_rules": [{"clause": '"TENANT_ID" = 1'}],
+        "iat": 1700000000.0,
+        "exp": 1700000300.0,
+        "aud": "http://0.0.0.0:8080/",
+        "type": "guest",
+    }
+
+    request = Mock()
+    request.cookies = {}
+
+    g.user = security_manager.get_guest_user_from_token(dict(base_token))
+    tenant_a = async_query_manager.parse_channel_id_from_request(request)
+
+    g.user = security_manager.get_guest_user_from_token(
+        {**base_token, "rls_rules": [{"clause": '"TENANT_ID" = 2'}]}
+    )
+    tenant_b = async_query_manager.parse_channel_id_from_request(request)
+
+    assert tenant_a != tenant_b
+
+
 @mark.parametrize(
     "cache_type, cache_backend",
     [

tests/unit_tests/security/manager_test.py

@@ -29,6 +29,7 @@ from flask_appbuilder.const import AUTH_DB, AUTH_REMOTE_USER
 from flask_appbuilder.security.sqla.models import Role, User
 from pytest_mock import MockerFixture
 
+from superset.common.chart_data import ChartDataResultType
 from superset.common.query_object import QueryObject
 from superset.connectors.sqla.models import Database, SqlaTable
 from superset.exceptions import SupersetSecurityException
@@ -1381,6 +1382,156 @@ def test_query_context_modified_tampered(
     assert query_context_modified(query_context)
 
 
+def test_query_context_modified_injected_annotation_layer(
+    mocker: MockerFixture,
+    stored_metrics: list[AdhocMetric],
+) -> None:
+    """
+    A guest must not be able to inject annotation layers the stored chart
+    does not carry: native annotation layers resolve all their annotations
+    with no further access check. Replaying the chart's own stored layers is
+    not tampering.
+    """
+    layer = {
+        "annotationType": "INTERVAL",
+        "sourceType": "NATIVE",
+        "value": 1,
+        "name": "Incidents",
+    }
+
+    # replaying the stored layer is allowed
+    query_context = mocker.MagicMock()
+    query_context.slice_.id = 42
+    query_context.slice_.query_context = None
+    query_context.slice_.params_dict = {
+        "metrics": stored_metrics,
+        "annotation_layers": [layer],
+    }
+    query_context.form_data = {
+        "slice_id": 42,
+        "metrics": stored_metrics,
+        "annotation_layers": [layer],
+    }
+    query_context.queries = [
+        QueryObject(metrics=stored_metrics, annotation_layers=[layer])  # type: ignore
+    ]
+    assert not query_context_modified(query_context)
+
+    # injecting a layer the chart was not saved with is tampering
+    injected = {**layer, "value": 2}
+    query_context.slice_.params_dict = {
+        "metrics": stored_metrics,
+    }
+    query_context.form_data = {
+        "slice_id": 42,
+        "metrics": stored_metrics,
+        "annotation_layers": [injected],
+    }
+    query_context.queries = [
+        QueryObject(metrics=stored_metrics, annotation_layers=[injected])  # type: ignore
+    ]
+    assert query_context_modified(query_context)
+
+
+def test_query_context_modified_result_type_expansion(
+    mocker: MockerFixture,
+    stored_metrics: list[AdhocMetric],
+) -> None:
+    """
+    Requesting the ``samples``/``drill_detail`` result types is tampering:
+    the server-side preparers expand those queries to every datasource
+    column after the subset checks on columns/metrics have run.
+    """
+    query_context = mocker.MagicMock()
+    query_context.slice_.id = 42
+    query_context.slice_.query_context = None
+    query_context.slice_.params_dict = {
+        "metrics": stored_metrics,
+    }
+    query_context.form_data = {
+        "slice_id": 42,
+        "metrics": stored_metrics,
+    }
+    query_context.queries = [QueryObject(metrics=stored_metrics)]  # type: ignore
+
+    # Top-level result type rewritten to samples.
+    query_context.result_type = ChartDataResultType.SAMPLES
+    assert query_context_modified(query_context)
+
+    # Per-query result type rewritten to drill_detail.
+    query_context.result_type = ChartDataResultType.FULL
+    query_context.queries[0].result_type = ChartDataResultType.DRILL_DETAIL
+    assert query_context_modified(query_context)
+
+    # The chart's own result type is not tampering.
+    query_context.queries[0].result_type = None
+    assert not query_context_modified(query_context)
+
+
+def test_query_context_modified_result_type_per_query_position(
+    mocker: MockerFixture,
+    stored_metrics: list[AdhocMetric],
+) -> None:
+    """
+    A chart's queries are validated by position: only the query stored with
+    ``samples``/``drill_detail`` may request it, and swapping which query
+    index carries that result type is tampering even though the type itself
+    is used somewhere on the stored chart.
+    """
+    query_context = mocker.MagicMock()
+    query_context.slice_.id = 42
+    query_context.slice_.params_dict = {"metrics": stored_metrics}
+    query_context.slice_.query_context = json.dumps(
+        {
+            "result_type": "full",
+            "queries": [
+                {"metrics": stored_metrics, "result_type": "full"},
+                {"metrics": stored_metrics, "result_type": "samples"},
+            ],
+        }
+    )
+    query_context.form_data = {"slice_id": 42, "metrics": stored_metrics}
+    query_context.result_type = ChartDataResultType.FULL
+
+    # Replaying each query's own stored result type, in the stored order, is
+    # not tampering.
+    query_context.queries = [
+        QueryObject(
+            metrics=stored_metrics,  # type: ignore
+            result_type=ChartDataResultType.FULL,
+        ),
+        QueryObject(
+            metrics=stored_metrics,  # type: ignore
+            result_type=ChartDataResultType.SAMPLES,
+        ),
+    ]
+    assert not query_context_modified(query_context)
+
+    # Requesting `samples` for the query stored at index 0 - a position the
+    # chart never renders with raw datasource rows - is tampering, even
+    # though `samples` is the stored result type of a different query.
+    query_context.queries = [
+        QueryObject(
+            metrics=stored_metrics,  # type: ignore
+            result_type=ChartDataResultType.SAMPLES,
+        ),
+        QueryObject(
+            metrics=stored_metrics,  # type: ignore
+            result_type=ChartDataResultType.FULL,
+        ),
+    ]
+    assert query_context_modified(query_context)
+
+    # Neither the query nor the query context names a result type: nothing
+    # resolves to a row-expanding type, so this is not tampering.
+    query_context.result_type = None
+    query_context.queries = [
+        QueryObject(metrics=stored_metrics),  # type: ignore
+        QueryObject(metrics=stored_metrics),  # type: ignore
+    ]
+    assert not query_context_modified(query_context)
+
+
 def test_query_context_modified_singular_metric_param(
     mocker: MockerFixture,
 ) -> None: