fix(chart): keep query-context updates bound to the chart's datasource

On the query-context-only update path UpdateChartCommand intentionally
skips the ownership check so report and alert workers can refresh a
chart's cached payload. Validate that the submitted query context still
targets the chart's own datasource (id and type) before saving, so a
cached payload cannot be repointed at an unrelated datasource. Payloads
without a parseable datasource fall back to the chart's datasource at
execution time and are left unchanged.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

superset/commands/chart/exceptions.py

@@ -103,6 +103,19 @@ class DatasourceTypeUpdateRequiredValidationError(ValidationError):
         )
 
 
+class ChartQueryContextDatasourceMismatchValidationError(ValidationError):
+    """
+    Raised when a query-context-only update carries a datasource that does not
+    match the chart's own datasource.
+    """
+
+    def __init__(self) -> None:
+        super().__init__(
+            _("The query context datasource does not match the chart datasource"),
+            field_name="query_context",
+        )
+
+
 class ChartNotFoundError(CommandException):
     message = "Chart not found."
 

superset/commands/chart/update.py

@@ -29,6 +29,7 @@ from superset.commands.chart.exceptions import (
     ChartForbiddenError,
     ChartInvalidError,
     ChartNotFoundError,
+    ChartQueryContextDatasourceMismatchValidationError,
     ChartUpdateFailedError,
     DashboardsForbiddenError,
     DashboardsNotFoundValidationError,
@@ -41,6 +42,7 @@ from superset.exceptions import SupersetSecurityException
 from superset.models.dashboard import Dashboard
 from superset.models.slice import Slice
 from superset.tags.models import ObjectType
+from superset.utils import json
 from superset.utils.decorators import on_error, transaction
 
 logger = logging.getLogger(__name__)
@@ -101,6 +103,51 @@ class UpdateChartCommand(UpdateMixin, BaseCommand):
                 if not security_manager.is_owner(dash):
                     raise DashboardsForbiddenError()
 
+    def _validate_query_context_datasource(
+        self, exceptions: list[ValidationError]
+    ) -> None:
+        """
+        Ensure a query-context-only update keeps the chart's own datasource.
+
+        The submitted query context is only verified when it carries a parseable
+        ``datasource`` object; a payload that references a different datasource than
+        the chart's persisted one is rejected. Payloads without a datasource fall
+        back to the chart's datasource at execution time and need no check.
+        """
+        if not self._model:
+            return
+
+        raw_query_context = self._properties.get("query_context")
+        if not raw_query_context:
+            return
+
+        try:
+            query_context = json.loads(raw_query_context)
+        except (TypeError, ValueError):
+            # An unparseable payload cannot be verified or replayed; leave it for
+            # downstream handling rather than guessing at its intent.
+            return
+
+        datasource = (
+            query_context.get("datasource") if isinstance(query_context, dict) else None
+        )
+        if not isinstance(datasource, dict):
+            return
+
+        try:
+            ids_match = int(datasource["id"]) == self._model.datasource_id
+        except (KeyError, TypeError, ValueError):
+            ids_match = False
+
+        datasource_type = datasource.get("type")
+        types_match = (
+            datasource_type is None
+            or str(datasource_type) == self._model.datasource_type
+        )
+
+        if not ids_match or not types_match:
+            exceptions.append(ChartQueryContextDatasourceMismatchValidationError())
+
     def validate(self) -> None:  # noqa: C901
         exceptions: list[ValidationError] = []
         dashboard_ids = self._properties.get("dashboards")
@@ -134,6 +181,12 @@ class UpdateChartCommand(UpdateMixin, BaseCommand):
                 raise ChartForbiddenError() from ex
             except ValidationError as ex:
                 exceptions.append(ex)
+        else:
+            # The query-context-only path skips the ownership check so report and
+            # alert workers can refresh a chart's cached payload. Keep that payload
+            # bound to the chart's own datasource so it cannot be repointed at an
+            # unrelated one.
+            self._validate_query_context_datasource(exceptions)
 
         # validate tags
         try:

superset/migrations/versions/2026-06-01_00-10_c8d2e3f4a5b6_add_guest_token_revoked_before.py

@@ -1,44 +0,0 @@
-# Licensed to the Apache Software Foundation (ASF) under one
-# or more contributor license agreements.  See the NOTICE file
-# distributed with this work for additional information
-# regarding copyright ownership.  The ASF licenses this file
-# to you under the Apache License, Version 2.0 (the
-# "License"); you may not use this file except in compliance
-# with the License.  You may obtain a copy of the License at
-#
-#   http://www.apache.org/licenses/LICENSE-2.0
-#
-# Unless required by applicable law or agreed to in writing,
-# software distributed under the License is distributed on an
-# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
-# KIND, either express or implied.  See the License for the
-# specific language governing permissions and limitations
-# under the License.
-"""add guest_token_revoked_before to embedded_dashboards
-
-Revision ID: c8d2e3f4a5b6
-Revises: 31dae2559c05
-Create Date: 2026-06-01 00:10:00.000000
-
-"""
-
-import sqlalchemy as sa
-
-from superset.migrations.shared.utils import add_columns, drop_columns
-
-# revision identifiers, used by Alembic.
-revision = "c8d2e3f4a5b6"
-down_revision = "31dae2559c05"
-
-
-def upgrade():
-    # Epoch seconds; guest tokens for this embedded dashboard issued (iat)
-    # before this value are rejected. NULL = no revocation.
-    add_columns(
-        "embedded_dashboards",
-        sa.Column("guest_token_revoked_before", sa.Integer(), nullable=True),
-    )
-
-
-def downgrade():
-    drop_columns("embedded_dashboards", "guest_token_revoked_before")

superset/models/embedded_dashboard.py

@@ -40,10 +40,6 @@ class EmbeddedDashboard(Model, AuditMixinNullable):
 
     uuid = Column(UUIDType(binary=True), default=uuid.uuid4, primary_key=True)
     allow_domain_list = Column(Text)  # reference the `allowed_domains` property instead
-    # Epoch seconds; guest tokens whose `iat` predates this are rejected. Set to
-    # "now" to revoke all currently-issued guest tokens for this embedded
-    # dashboard. NULL = no revocation.
-    guest_token_revoked_before = Column(Integer, nullable=True)
     dashboard_id = Column(
         Integer,
         ForeignKey("dashboards.id", ondelete="CASCADE"),

superset/security/manager.py

@@ -21,7 +21,6 @@ import logging
 import re
 import time
 from collections import defaultdict
-from math import ceil
 from types import SimpleNamespace
 from typing import Any, Callable, cast, NamedTuple, Optional, TYPE_CHECKING
 
@@ -87,7 +86,6 @@ from superset.utils.core import (
     get_username,
     RowLevelSecurityFilterType,
 )
-from superset.utils.decorators import transaction
 from superset.utils.filters import get_dataset_access_filters
 from superset.utils.urls import get_url_host
 
@@ -3704,31 +3702,18 @@ class SupersetSecurityManager(  # pylint: disable=too-many-public-methods
 
         return self.get_guest_user_from_token(cast(GuestToken, token))
 
-    @classmethod
-    def _is_guest_token_revoked(cls, token: dict[str, Any]) -> bool:
-        """
-        Determine whether a guest token has been revoked by any mechanism.
-
-        Two complementary revocation mechanisms apply:
-
-        - **Global version bump** (opt-in via ``GUEST_TOKEN_REVOCATION_ENABLED``):
-          a token is revoked if the version it was minted with is below the
-          expected version. Tokens minted before this feature existed carry no
-          version claim and are treated as
-          :data:`DEFAULT_GUEST_TOKEN_REVOCATION_VERSION` (0), so they only become
-          revoked once an admin has explicitly bumped the expected version above 0.
-        - **Per-embedded-dashboard cutoff** (``guest_token_revoked_before``): a
-          token is revoked if its ``iat`` predates the revocation cutoff of any of
-          its embedded-dashboard resources.
-        """
-        return cls._is_guest_token_revoked_by_version(
-            token
-        ) or cls._is_guest_token_revoked_by_embedded(token)
-
     @staticmethod
-    def _is_guest_token_revoked_by_version(token: dict[str, Any]) -> bool:
-        """Return True if the token's revocation version is below the expected
-        version. Gated on ``GUEST_TOKEN_REVOCATION_ENABLED``."""
+    def _is_guest_token_revoked(token: dict[str, Any]) -> bool:
+        """
+        Determine whether a guest token has been revoked via a version bump.
+
+        Revocation is opt-in (``GUEST_TOKEN_REVOCATION_ENABLED``). When disabled,
+        no token is ever considered revoked. When enabled, a token is revoked if
+        the version it was minted with is below the expected version. Tokens
+        minted before this feature existed carry no version claim and are treated
+        as :data:`DEFAULT_GUEST_TOKEN_REVOCATION_VERSION` (0), so they only become
+        revoked once an admin has explicitly bumped the expected version above 0.
+        """
         if not get_conf()["GUEST_TOKEN_REVOCATION_ENABLED"]:
             return False
         token_version = token.get(
@@ -3740,67 +3725,6 @@ class SupersetSecurityManager(  # pylint: disable=too-many-public-methods
             token_version = DEFAULT_GUEST_TOKEN_REVOCATION_VERSION
         return token_version < get_current_guest_token_revocation_version()
 
-    @staticmethod
-    def _is_guest_token_revoked_by_embedded(token: dict[str, Any]) -> bool:
-        """Return True if the token predates a revocation on any of its
-        embedded-dashboard resources (``guest_token_revoked_before``).
-
-        A token missing ``iat`` cannot prove it was issued after a revocation
-        cutoff, so it is treated as revoked whenever any of its dashboard
-        resources has an active cutoff; otherwise it is not revoked.
-        """
-        issued_at = token.get("iat")
-
-        # pylint: disable=import-outside-toplevel
-        from superset.daos.dashboard import EmbeddedDashboardDAO
-        from superset.models.dashboard import Dashboard
-
-        for resource in token.get("resources") or []:
-            if resource.get("type") != GuestTokenResourceType.DASHBOARD.value:
-                continue
-            resource_id = str(resource.get("id"))
-            # A dashboard resource id may be an embedded UUID or, during the
-            # UUID migration, a legacy dashboard id. Resolve the embedded
-            # config(s) for either form (mirrors validate_guest_token_resources).
-            embedded = EmbeddedDashboardDAO.find_by_id(resource_id)
-            if embedded:
-                embedded_configs = [embedded]
-            else:
-                dashboard = Dashboard.get(resource_id)
-                embedded_configs = dashboard.embedded if dashboard else []
-            for embedded_config in embedded_configs:
-                revoked_before = getattr(
-                    embedded_config, "guest_token_revoked_before", None
-                )
-                if revoked_before is None:
-                    continue
-                # Without an issued-at claim the token cannot be shown to
-                # postdate the cutoff, so fail closed and treat it as revoked.
-                if not issued_at or issued_at < revoked_before:
-                    return True
-        return False
-
-    @transaction()
-    def revoke_guest_token_access(
-        self, embedded_uuid: str, before: Optional[int] = None
-    ) -> None:
-        """Revoke all guest tokens issued for an embedded dashboard before
-        ``before`` (epoch seconds, default: now). Subsequent tokens are
-        unaffected."""
-        # pylint: disable=import-outside-toplevel
-        from superset.daos.dashboard import EmbeddedDashboardDAO
-
-        embedded = EmbeddedDashboardDAO.find_by_id(str(embedded_uuid))
-        if embedded is None:
-            return
-        # Round the cutoff up to the next whole second so that tokens whose
-        # fractional ``iat`` falls within the current second are reliably
-        # revoked (the column stores integer seconds). Rounding up fails
-        # closed: at worst it revokes a token issued slightly after the call.
-        embedded.guest_token_revoked_before = (
-            before if before is not None else ceil(self._get_current_epoch_time())
-        )
-
     def get_guest_user_from_token(self, token: GuestToken) -> GuestUser:
         return self.guest_user_cls(
             token=token,

tests/unit_tests/commands/chart/update_test.py

@@ -17,10 +17,11 @@
 import pytest
 from pytest_mock import MockerFixture
 
-from superset.commands.chart.exceptions import ChartForbiddenError
+from superset.commands.chart.exceptions import ChartForbiddenError, ChartInvalidError
 from superset.commands.chart.update import UpdateChartCommand
 from superset.errors import ErrorLevel, SupersetError, SupersetErrorType
 from superset.exceptions import SupersetSecurityException
+from superset.utils import json
 
 
 def _ownership_exc() -> SupersetSecurityException:
@@ -91,3 +92,73 @@ def test_update_chart_owner_can_perform_regular_update(
 
     find_by_id.assert_called_once_with(1)
     raise_for_ownership.assert_called_once()
+
+
+def _query_context_payload(datasource: object) -> dict[str, object]:
+    return {
+        "query_context": json.dumps({"datasource": datasource, "queries": []}),
+        "query_context_generation": True,
+    }
+
+
+def test_update_chart_query_context_matching_datasource_is_allowed(
+    mocker: MockerFixture,
+) -> None:
+    """A query context that targets the chart's own datasource is accepted."""
+    find_by_id = mocker.patch("superset.commands.chart.update.ChartDAO.find_by_id")
+    find_by_id.return_value = mocker.MagicMock(
+        id=1, tags=[], dashboards=[], datasource_id=42, datasource_type="table"
+    )
+    mocker.patch("superset.commands.chart.update.security_manager.raise_for_ownership")
+
+    UpdateChartCommand(
+        1, _query_context_payload({"id": 42, "type": "table"})
+    ).validate()
+
+
+@pytest.mark.parametrize(
+    "datasource",
+    [
+        {"id": 99, "type": "table"},  # different id
+        {"id": 42, "type": "query"},  # different type
+        {"id": "99", "type": "table"},  # different id as string
+    ],
+)
+def test_update_chart_query_context_mismatched_datasource_is_rejected(
+    mocker: MockerFixture,
+    datasource: dict[str, object],
+) -> None:
+    """A query context pointing at a different datasource is rejected with a 4xx."""
+    find_by_id = mocker.patch("superset.commands.chart.update.ChartDAO.find_by_id")
+    find_by_id.return_value = mocker.MagicMock(
+        id=1, tags=[], dashboards=[], datasource_id=42, datasource_type="table"
+    )
+    mocker.patch("superset.commands.chart.update.security_manager.raise_for_ownership")
+
+    with pytest.raises(ChartInvalidError):
+        UpdateChartCommand(1, _query_context_payload(datasource)).validate()
+
+
+@pytest.mark.parametrize(
+    "query_context",
+    [
+        "{}",  # no datasource key
+        '{"datasource": null}',  # null datasource
+        "not-json",  # unparseable payload
+    ],
+)
+def test_update_chart_query_context_without_datasource_is_allowed(
+    mocker: MockerFixture,
+    query_context: str,
+) -> None:
+    """Payloads with no verifiable datasource fall back to the chart's own."""
+    find_by_id = mocker.patch("superset.commands.chart.update.ChartDAO.find_by_id")
+    find_by_id.return_value = mocker.MagicMock(
+        id=1, tags=[], dashboards=[], datasource_id=42, datasource_type="table"
+    )
+    mocker.patch("superset.commands.chart.update.security_manager.raise_for_ownership")
+
+    UpdateChartCommand(
+        1,
+        {"query_context": query_context, "query_context_generation": True},
+    ).validate()

tests/unit_tests/security/test_guest_token_revocation.py

@@ -1,157 +0,0 @@
-# Licensed to the Apache Software Foundation (ASF) under one
-# or more contributor license agreements.  See the NOTICE file
-# distributed with this work for additional information
-# regarding copyright ownership.  The ASF licenses this file
-# to you under the Apache License, Version 2.0 (the
-# "License"); you may not use this file except in compliance
-# with the License.  You may obtain a copy of the License at
-#
-#   http://www.apache.org/licenses/LICENSE-2.0
-#
-# Unless required by applicable law or agreed to in writing,
-# software distributed under the License is distributed on an
-# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
-# KIND, either express or implied.  See the License for the
-# specific language governing permissions and limitations
-# under the License.
-
-from typing import Any
-from unittest.mock import MagicMock, patch
-
-from superset.security.manager import SupersetSecurityManager
-
-_DASHBOARD_RESOURCE = {"type": "dashboard", "id": "abc-uuid"}
-
-
-def _token(iat: int) -> dict[str, Any]:
-    return {"type": "guest", "iat": iat, "resources": [_DASHBOARD_RESOURCE]}
-
-
-def _embedded(revoked_before) -> MagicMock:
-    embedded = MagicMock()
-    embedded.guest_token_revoked_before = revoked_before
-    return embedded
-
-
-def test_guest_token_not_revoked_when_no_revocation_set() -> None:
-    with patch(
-        "superset.daos.dashboard.EmbeddedDashboardDAO.find_by_id",
-        return_value=_embedded(None),
-    ):
-        assert SupersetSecurityManager._is_guest_token_revoked(_token(1000)) is False
-
-
-def test_guest_token_revoked_when_issued_before_revocation() -> None:
-    with patch(
-        "superset.daos.dashboard.EmbeddedDashboardDAO.find_by_id",
-        return_value=_embedded(2000),
-    ):
-        # Token issued at 1000, revocation at 2000 -> revoked.
-        assert SupersetSecurityManager._is_guest_token_revoked(_token(1000)) is True
-
-
-def test_guest_token_valid_when_issued_after_revocation() -> None:
-    with patch(
-        "superset.daos.dashboard.EmbeddedDashboardDAO.find_by_id",
-        return_value=_embedded(2000),
-    ):
-        # Token issued at 3000, after the revocation cutoff -> still valid.
-        assert SupersetSecurityManager._is_guest_token_revoked(_token(3000)) is False
-
-
-def test_guest_token_without_iat_is_revoked_when_cutoff_set() -> None:
-    # A token lacking ``iat`` cannot prove it predates the cutoff, so it
-    # fails closed and is treated as revoked when a cutoff is configured.
-    token = {"type": "guest", "resources": [_DASHBOARD_RESOURCE]}
-    with patch(
-        "superset.daos.dashboard.EmbeddedDashboardDAO.find_by_id",
-        return_value=_embedded(2000),
-    ):
-        assert SupersetSecurityManager._is_guest_token_revoked(token) is True
-
-
-def test_guest_token_without_iat_is_not_revoked_when_no_cutoff() -> None:
-    token = {"type": "guest", "resources": [_DASHBOARD_RESOURCE]}
-    with patch(
-        "superset.daos.dashboard.EmbeddedDashboardDAO.find_by_id",
-        return_value=_embedded(None),
-    ):
-        assert SupersetSecurityManager._is_guest_token_revoked(token) is False
-
-
-def test_guest_token_revoked_via_legacy_dashboard_id_resource() -> None:
-    # During the UUID migration a dashboard resource id may be a legacy
-    # dashboard id rather than an embedded UUID. In that case the embedded
-    # config is resolved via Dashboard.get(...).embedded, and its revocation
-    # cutoff must still be honored.
-    dashboard = MagicMock()
-    dashboard.embedded = [_embedded(2000)]
-    with (
-        patch(
-            "superset.daos.dashboard.EmbeddedDashboardDAO.find_by_id",
-            return_value=None,
-        ),
-        patch(
-            "superset.models.dashboard.Dashboard.get",
-            return_value=dashboard,
-        ),
-    ):
-        assert SupersetSecurityManager._is_guest_token_revoked(_token(1000)) is True
-
-
-def test_guest_token_not_revoked_when_resource_unresolvable() -> None:
-    # If neither an embedded config nor a dashboard resolves, there is no
-    # cutoff to enforce and the token is treated as not revoked.
-    with (
-        patch(
-            "superset.daos.dashboard.EmbeddedDashboardDAO.find_by_id",
-            return_value=None,
-        ),
-        patch(
-            "superset.models.dashboard.Dashboard.get",
-            return_value=None,
-        ),
-    ):
-        assert SupersetSecurityManager._is_guest_token_revoked(_token(1000)) is False
-
-
-def _manager() -> SupersetSecurityManager:
-    # Build an instance without running the (heavy) FAB __init__: we only
-    # exercise revoke_guest_token_access, which depends on nothing but
-    # _get_current_epoch_time and the EmbeddedDashboardDAO lookup.
-    return SupersetSecurityManager.__new__(SupersetSecurityManager)
-
-
-def test_revoke_guest_token_access_uses_explicit_before() -> None:
-    embedded = _embedded(None)
-    with patch(
-        "superset.daos.dashboard.EmbeddedDashboardDAO.find_by_id",
-        return_value=embedded,
-    ):
-        _manager().revoke_guest_token_access("abc-uuid", before=1234)
-    assert embedded.guest_token_revoked_before == 1234
-
-
-def test_revoke_guest_token_access_defaults_to_ceil_of_now() -> None:
-    embedded = _embedded(None)
-    manager = _manager()
-    with (
-        patch(
-            "superset.daos.dashboard.EmbeddedDashboardDAO.find_by_id",
-            return_value=embedded,
-        ),
-        patch.object(manager, "_get_current_epoch_time", return_value=1000.25),
-    ):
-        manager.revoke_guest_token_access("abc-uuid")
-    # Cutoff is rounded up so fractional-``iat`` tokens issued in the same
-    # second are reliably revoked (fails closed).
-    assert embedded.guest_token_revoked_before == 1001
-
-
-def test_revoke_guest_token_access_noop_when_embedded_missing() -> None:
-    with patch(
-        "superset.daos.dashboard.EmbeddedDashboardDAO.find_by_id",
-        return_value=None,
-    ):
-        # Should simply return without raising when the UUID does not resolve.
-        _manager().revoke_guest_token_access("missing-uuid")

tests/unit_tests/utils/test_split.py

@@ -0,0 +1,81 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+from superset.utils.core import split
+
+
+def test_split_empty_string():
+    assert list(split("")) == [""]
+
+
+def test_split_leading_delimiter():
+    assert list(split(" a")) == [
+        "",
+        "a",
+    ]
+
+
+def test_split_trailing_delimiter():
+    assert list(split("a ")) == [
+        "a",
+        "",
+    ]
+
+
+def test_split_only_delimiter():
+    assert list(split(" ")) == [
+        "",
+        "",
+    ]
+
+
+def test_split_nested_parentheses():
+    assert list(
+        split(
+            "a,(b,(c,d))",
+            delimiter=",",
+        )
+    ) == [
+        "a",
+        "(b,(c,d))",
+    ]
+
+
+def test_branch_separator_found():
+    assert list(split("a b")) == [
+        "a",
+        "b",
+    ]
+
+
+def test_branch_separator_not_found():
+    assert list(split("ab")) == [
+        "ab",
+    ]
+
+
+def test_branch_parentheses():
+    assert list(split("(a b)")) == [
+        "(a b)",
+    ]
+
+
+def test_branch_escaped_quote():
+    assert list(split(r'"a\"b c" d')) == [
+        r'"a\"b c"',
+        "d",
+    ]