You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by ru...@apache.org on 2026/06/10 23:30:06 UTC
(superset) branch fix/chart-query-context-datasource-validation created (now d544bff0717)
This is an automated email from the ASF dual-hosted git repository.
rusackas pushed a change to branch fix/chart-query-context-datasource-validation
in repository https://gitbox.apache.org/repos/asf/superset.git
at d544bff0717 fix(chart): keep query-context updates bound to the chart's datasource
This branch includes the following new commits:
new d544bff0717 fix(chart): keep query-context updates bound to the chart's datasource
The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
(superset) 01/01: fix(chart): keep query-context updates bound to the chart's datasource
Posted by ru...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
rusackas pushed a commit to branch fix/chart-query-context-datasource-validation
in repository https://gitbox.apache.org/repos/asf/superset.git
commit d544bff0717888662ec1a0779b86123a5483bf67
Author: Claude Code <no...@anthropic.com>
AuthorDate: Wed Jun 10 16:29:39 2026 -0700
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 <no...@anthropic.com>
---
superset/commands/chart/exceptions.py | 13 +++++
superset/commands/chart/update.py | 53 +++++++++++++++++++
tests/unit_tests/commands/chart/update_test.py | 73 +++++++++++++++++++++++++-
3 files changed, 138 insertions(+), 1 deletion(-)
diff --git a/superset/commands/chart/exceptions.py b/superset/commands/chart/exceptions.py
index 72ef71f466e..03c7a0cbdaa 100644
--- a/superset/commands/chart/exceptions.py
+++ b/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."
diff --git a/superset/commands/chart/update.py b/superset/commands/chart/update.py
index 77679b6fe4b..d007cdcd4e3 100644
--- a/superset/commands/chart/update.py
+++ b/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:
diff --git a/tests/unit_tests/commands/chart/update_test.py b/tests/unit_tests/commands/chart/update_test.py
index d5758947951..235ab2cbe70 100644
--- a/tests/unit_tests/commands/chart/update_test.py
+++ b/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()