You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by di...@apache.org on 2026/10/19 07:52:01 UTC
(superset) branch master updated: fix(embedded): apply the chart-data guest redaction to samples and the legacy query endpoint (#45273)
This is an automated email from the ASF dual-hosted git repository.
geido 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 663d691c311 fix(embedded): apply the chart-data guest redaction to samples and the legacy query endpoint (#45273)
663d691c311 is described below
commit 663d691c311f3408720a354b31439ad0094614ba
Author: GeidÅ <60...@users.noreply.github.com>
AuthorDate: Mon Oct 19 10:51:54 2026 +0300
fix(embedded): apply the chart-data guest redaction to samples and the legacy query endpoint (#45273)
Embedded guests whose role lacks "can view query on Dashboard" no longer
receive the generated SQL from `/datasource/samples` (Drill to detail) or
the legacy `POST /api/v1/query/`, matching `/api/v1/chart/data`.
Stacktraces and raw driver errors are redacted for guests on all three.
The redaction is extracted into a shared `redact_sensitive_query_fields`
helper used by all three endpoints.
Co-Authored-By: Claude Opus 4.8 <no...@anthropic.com>
Co-Authored-By: Claude Opus 5.5 (1M context) <no...@anthropic.com>
---
superset/charts/data/api.py | 29 +----
superset/utils/error_sanitization.py | 38 ++++++
superset/views/api.py | 5 +
superset/views/datasource/views.py | 8 ++
tests/unit_tests/utils/test_error_sanitization.py | 94 ++++++++++++++
tests/unit_tests/views/datasource/views_test.py | 145 +++++++++++++++++++++-
tests/unit_tests/views/test_base.py | 72 +++++++++++
7 files changed, 367 insertions(+), 24 deletions(-)
diff --git a/superset/charts/data/api.py b/superset/charts/data/api.py
index 5dd2b41168c..59aef99222f 100644
--- a/superset/charts/data/api.py
+++ b/superset/charts/data/api.py
@@ -68,7 +68,7 @@ from superset.utils.core import (
)
from superset.utils.decorators import logs_context
from superset.utils.error_sanitization import (
- is_sanitization_required,
+ redact_sensitive_query_fields,
sanitize_error_message,
)
from superset.views.base import CsvResponse, generate_download_headers, XlsxResponse
@@ -553,28 +553,11 @@ class ChartDataRestApi(ChartRestApi):
):
query["timing"] = query_result.timing.as_public_dict()
- # Resolve the redaction decision once so the stacktrace pop and the
- # error-message sanitization below stay consistent: the guarded
- # ``is_sanitization_required`` fails closed for a guest whose
- # principal can't be resolved, so the block never half-redacts
- # (popping ``stacktrace`` while leaking the raw ``error``).
- if sanitize_required := is_sanitization_required():
- # Guests may see the generated SQL only when the role attached to
- # their guest token has been granted "can view query on Dashboard",
- # mirroring the permission the frontend uses to expose the
- # "View query" action. Stacktraces and driver errors stay redacted
- # regardless, as those leak details of the deployment itself.
- can_view_query = security_manager.can_access(
- "can_view_query", "Dashboard"
- )
- for query in queries:
- if not can_view_query:
- query.pop("query", None)
- query.pop("stacktrace", None)
- if query.get("error"):
- query["error"] = sanitize_error_message(
- query["error"], required=sanitize_required
- )
+ # Guest (embedded) responses drop the generated SQL unless the guest
+ # role has "can view query on Dashboard", and always drop stacktraces
+ # and raw driver errors; a no-op for other principals. See
+ # ``redact_sensitive_query_fields``.
+ redact_sensitive_query_fields(queries)
payload: dict[str, Any] = {"result": queries}
if dashboard_filter_context is not None:
diff --git a/superset/utils/error_sanitization.py b/superset/utils/error_sanitization.py
index ae0ba565cff..f4cf81ae283 100644
--- a/superset/utils/error_sanitization.py
+++ b/superset/utils/error_sanitization.py
@@ -168,6 +168,44 @@ def sanitize_error_message(
return str(GENERIC_ERROR_MESSAGE)
+def redact_sensitive_query_fields(results: list[dict[str, Any]]) -> None:
+ """
+ Redact generated SQL, stacktraces and raw errors from query results in place.
+
+ Embedded (guest token) viewers must not receive the generated ``query`` --
+ it exposes the underlying virtual-dataset SQL, table names, joins and
+ applied filter/RLS clauses -- unless the role attached to their guest token
+ carries "can view query on Dashboard", the same permission the frontend uses
+ to expose the "View query" action. ``stacktrace`` and raw driver ``error``
+ text describe the deployment itself and are redacted for guests regardless.
+
+ Used by ``/api/v1/chart/data``, ``/datasource/samples`` (Drill to detail)
+ and the legacy ``/api/v1/query/``, which all return query-context payloads
+ to embedded guests, so the three apply one rule: the chart-data response has
+ stripped the SQL for guests since CVE-2025-55673.
+
+ The sanitization decision is resolved once here -- ``is_sanitization_required``
+ fails closed for a guest whose principal can't be resolved -- and threaded
+ into the error sanitization, so a result is never half-redacted (its
+ ``stacktrace`` popped while the raw ``error`` leaks). The call is a no-op for
+ principals that don't require sanitization, so callers invoke it
+ unconditionally.
+ """
+ if not is_sanitization_required():
+ return
+
+ # pylint: disable=import-outside-toplevel
+ from superset import security_manager
+
+ can_view_query = security_manager.can_access("can_view_query", "Dashboard")
+ for result in results:
+ if not can_view_query:
+ result.pop("query", None)
+ result.pop("stacktrace", None)
+ if result.get("error"):
+ result["error"] = sanitize_error_message(result["error"], required=True)
+
+
def sanitize_superset_error(
error: SupersetError, required: bool | None = None
) -> SupersetError:
diff --git a/superset/views/api.py b/superset/views/api.py
index b5d8532c9b0..8c6737160ae 100644
--- a/superset/views/api.py
+++ b/superset/views/api.py
@@ -35,6 +35,7 @@ from superset.legacy import update_time_range
from superset.superset_typing import FlaskResponse
from superset.utils import json
from superset.utils.date_parser import get_since_until
+from superset.utils.error_sanitization import redact_sensitive_query_fields
from superset.views.base import api, BaseSupersetView
from superset.views.error_handling import handle_api_exception
@@ -74,6 +75,10 @@ class Api(BaseSupersetView):
query_context.raise_for_access()
result = query_context.get_payload()
payload_json = result["queries"]
+ # These are the same query-context payloads ``/api/v1/chart/data``
+ # returns, including the generated SQL under ``query``, so apply the same
+ # guest redaction before responding.
+ redact_sensitive_query_fields(payload_json)
return self.json_response(payload_json)
@event_logger.log_this
diff --git a/superset/views/datasource/views.py b/superset/views/datasource/views.py
index 650890a884e..d23d5daea87 100644
--- a/superset/views/datasource/views.py
+++ b/superset/views/datasource/views.py
@@ -42,6 +42,7 @@ from superset.sql.parse import Table
from superset.superset_typing import FlaskResponse
from superset.utils import json
from superset.utils.core import DatasourceType
+from superset.utils.error_sanitization import redact_sensitive_query_fields
from superset.views.base import api, BaseSupersetView, deprecated, json_error_response
from superset.views.datasource.schemas import (
ExternalMetadataParams,
@@ -444,6 +445,13 @@ class Datasource(BaseSupersetView):
datasource=dataset,
dashboard_id=dashboard_id,
)
+ # Drill to detail is available to embedded guests (see the guest branch
+ # above), and the samples payload carries the generated SQL under
+ # ``query``. Apply the same guest redaction as ``/api/v1/chart/data``
+ # (CVE-2025-55673, gated on "can view query on Dashboard" since #43151)
+ # so the SQL is not disclosed to a guest whose role lacks that
+ # permission.
+ redact_sensitive_query_fields([rv])
return self.json_response({"result": rv})
diff --git a/tests/unit_tests/utils/test_error_sanitization.py b/tests/unit_tests/utils/test_error_sanitization.py
index 8ccf79f7605..673bb45ccd2 100644
--- a/tests/unit_tests/utils/test_error_sanitization.py
+++ b/tests/unit_tests/utils/test_error_sanitization.py
@@ -28,6 +28,7 @@ from superset.utils.error_sanitization import (
GENERIC_ACCESS_MESSAGE,
GENERIC_ERROR_MESSAGE,
is_sanitization_required,
+ redact_sensitive_query_fields,
sanitize_error_dicts,
sanitize_error_message,
sanitize_superset_error,
@@ -81,6 +82,99 @@ def test_db_error_is_replaced_for_guest_users(app: SupersetApp, guest: None) ->
assert "engine_name" not in (sanitized.extra or {})
+def test_redact_query_fields_strips_sql_for_guest_without_permission(
+ app: SupersetApp, guest: None
+) -> None:
+ results = [{"query": "SELECT 1", "stacktrace": "Traceback", "data": [{"a": 1}]}]
+ with patch(
+ "superset.security.SupersetSecurityManager.can_access",
+ return_value=False,
+ ):
+ redact_sensitive_query_fields(results)
+ assert "query" not in results[0]
+ assert "stacktrace" not in results[0]
+ # Non-sensitive fields the client needs for rendering are left alone.
+ assert results[0]["data"] == [{"a": 1}]
+
+
+def test_redact_query_fields_keeps_sql_when_guest_may_view_query(
+ app: SupersetApp, guest: None
+) -> None:
+ results = [{"query": "SELECT 1", "stacktrace": "Traceback"}]
+ with patch(
+ "superset.security.SupersetSecurityManager.can_access",
+ return_value=True,
+ ) as can_access:
+ redact_sensitive_query_fields(results)
+ assert results[0]["query"] == "SELECT 1"
+ # "can view query" unlocks only the SQL; the stacktrace still leaves.
+ assert "stacktrace" not in results[0]
+ can_access.assert_called_once_with("can_view_query", "Dashboard")
+
+
+def test_redact_query_fields_sanitizes_error_for_guest(
+ app: SupersetApp, guest: None
+) -> None:
+ results = [{"query": "SELECT 1", "error": DB_ERROR}]
+ with patch(
+ "superset.security.SupersetSecurityManager.can_access",
+ return_value=False,
+ ):
+ redact_sensitive_query_fields(results)
+ assert results[0]["error"] == str(GENERIC_ERROR_MESSAGE)
+
+
+def test_redact_query_fields_is_noop_for_regular_users(
+ app: SupersetApp, not_guest: None
+) -> None:
+ original = {"query": "SELECT 1", "stacktrace": "Traceback", "error": DB_ERROR}
+ results = [dict(original)]
+ redact_sensitive_query_fields(results)
+ assert results[0] == original
+
+
+def test_redact_query_fields_on_samples_payload_keeps_rendering_fields(
+ app: SupersetApp, guest: None
+) -> None:
+ """Shaped like a real ``/datasource/samples`` (Drill to detail) result: the
+ generated SQL, stacktrace and raw error are removed for a guest without
+ "can view query on Dashboard", while every field the client needs to render
+ the samples grid (data, column names/types, label map, counts, cache key)
+ is preserved untouched."""
+ sample_result = {
+ "query": "SELECT * FROM (SELECT * FROM secret_table) AS virtual_table",
+ "stacktrace": "Traceback (most recent call last): ...",
+ "error": DB_ERROR,
+ "data": [{"id": 1, "amount": 100.0}],
+ "colnames": ["id", "amount"],
+ "coltypes": [0, 0],
+ "label_map": {"id": ["id"], "amount": ["amount"]},
+ "cache_key": "abc123",
+ "rowcount": 1,
+ "page": 1,
+ "per_page": 50,
+ "total_count": 1,
+ }
+ results = [sample_result]
+ with patch(
+ "superset.security.SupersetSecurityManager.can_access",
+ return_value=False,
+ ):
+ redact_sensitive_query_fields(results)
+
+ redacted = results[0]
+ assert "query" not in redacted
+ assert "stacktrace" not in redacted
+ assert redacted["error"] == str(GENERIC_ERROR_MESSAGE)
+ # Everything the samples grid renders from is left intact.
+ assert redacted["data"] == [{"id": 1, "amount": 100.0}]
+ assert redacted["colnames"] == ["id", "amount"]
+ assert redacted["coltypes"] == [0, 0]
+ assert redacted["label_map"] == {"id": ["id"], "amount": ["amount"]}
+ assert redacted["cache_key"] == "abc123"
+ assert redacted["total_count"] == 1
+
+
def test_safe_error_types_survive_for_guest_users(
app: SupersetApp, guest: None
) -> None:
diff --git a/tests/unit_tests/views/datasource/views_test.py b/tests/unit_tests/views/datasource/views_test.py
index c1953675b3d..a5e7f22c413 100644
--- a/tests/unit_tests/views/datasource/views_test.py
+++ b/tests/unit_tests/views/datasource/views_test.py
@@ -27,12 +27,17 @@ from unittest.mock import ANY, call, MagicMock, patch
import pytest
from flask import Flask
-from superset.commands.dataset.exceptions import DatasetForbiddenError
+from superset.commands.dataset.exceptions import (
+ DatasetForbiddenError,
+ DatasetSamplesFailedError,
+)
from superset.connectors.sqla.models import SqlaTable
from superset.errors import ErrorLevel, SupersetError, SupersetErrorType
from superset.exceptions import SupersetSecurityException
from superset.sql.parse import Table
from superset.utils import json as superset_json
+from superset.utils.error_sanitization import GENERIC_ERROR_MESSAGE
+from superset.views.error_handling import handle_api_exception
def _identity_gettext(message: str, **variables: Any) -> str:
@@ -888,6 +893,144 @@ def test_samples_proceeds_for_supported_datasource_type(
view.json_response.assert_called_once_with({"result": {"rows": []}})
+@patch("superset.views.datasource.views._", _identity_gettext)
+@patch("superset.security_manager", new_callable=MagicMock)
+@patch("superset.views.datasource.views.get_samples")
+@patch("superset.views.datasource.views.DashboardDAO")
+@patch("superset.views.datasource.views.DatasetDAO")
+@patch("superset.views.datasource.views.security_manager", new_callable=MagicMock)
+def test_samples_guest_response_is_redacted_by_the_real_helper(
+ mock_view_security_manager: MagicMock,
+ mock_dataset_dao: MagicMock,
+ mock_dashboard_dao: MagicMock,
+ mock_get_samples: MagicMock,
+ mock_global_security_manager: MagicMock,
+) -> None:
+ """An embedded guest without "can view query on Dashboard" gets the generated
+ SQL stripped from the actual samples response.
+
+ Exercises the real ``redact_sensitive_query_fields`` (not a mock) through the
+ guest branch, so a regression that made the redaction a no-op for guests
+ would fail here. ``mock_view_security_manager`` drives the view's guest-access
+ checks; ``mock_global_security_manager`` is the ``from superset import
+ security_manager`` the helper resolves (guest, lacking the permission)."""
+ mock_view_security_manager.is_guest_user.return_value = True
+ mock_view_security_manager.can_drill_dataset_via_dashboard_access.return_value = (
+ True
+ )
+ # The helper's principal: a guest without "can view query on Dashboard".
+ mock_global_security_manager.is_guest_user.return_value = True
+ mock_global_security_manager.can_access.return_value = False
+
+ rv = {
+ "query": "SELECT secret_col FROM private_table",
+ "stacktrace": "Traceback (most recent call last): ...",
+ "data": [{"id": 1}],
+ "colnames": ["id"],
+ "total_count": 1,
+ }
+ mock_get_samples.return_value = rv
+
+ # Serialize what the view hands to ``json_response`` at call time, as the
+ # real ``json_response`` does, so the assertions see what a client receives
+ # and would catch the redaction running only after the response is built.
+ sent: list[dict[str, Any]] = []
+ view = _view_self()
+ view.json_response.side_effect = lambda obj, *args, **kwargs: sent.append(
+ superset_json.loads(superset_json.dumps(obj))
+ )
+ app = Flask(__name__)
+ with app.test_request_context(
+ "/datasource/samples?datasource_type=table&datasource_id=1&dashboard_id=5",
+ method="POST",
+ json={},
+ ):
+ _get_view_func("samples")(view)
+
+ # The serialized response carries no generated SQL or stacktrace, while the
+ # rendering fields were left intact.
+ assert len(sent) == 1
+ result = sent[0]["result"]
+ assert "query" not in result
+ assert "stacktrace" not in result
+ assert result["data"] == [{"id": 1}]
+ mock_global_security_manager.can_access.assert_called_once_with(
+ "can_view_query", "Dashboard"
+ )
+
+
+# A raw driver error can quote the full statement (SQLAlchemy appends
+# ``[SQL: ...]``), so it must not reach a guest on the failure path either.
+_RAW_SAMPLES_ERROR = (
+ '(psycopg2.errors.UndefinedColumn) column "secret_col" does not exist\n'
+ "LINE 1: SELECT secret_col FROM private_table\n"
+ "[SQL: SELECT secret_col FROM private_table]"
+)
+
+
+def _run_failing_samples(app: Flask, *, is_guest: bool, query_string: str) -> Any:
+ """Run the samples view body under its real ``handle_api_exception`` for a
+ query that fails with a raw driver error, as a guest or an authenticated
+ user. On failure ``get_samples`` raises rather than returning the payload,
+ so the error handler -- not ``redact_sensitive_query_fields`` -- decides
+ what the client sees."""
+ with (
+ patch(
+ "superset.views.datasource.views.security_manager",
+ new_callable=MagicMock,
+ ) as view_security_manager,
+ patch("superset.views.datasource.views.DatasetDAO"),
+ patch("superset.views.datasource.views.DashboardDAO"),
+ patch(
+ "superset.views.datasource.views.get_samples",
+ side_effect=DatasetSamplesFailedError(_RAW_SAMPLES_ERROR),
+ ),
+ patch(
+ "superset.security.SupersetSecurityManager.is_guest_user",
+ return_value=is_guest,
+ ),
+ app.test_request_context(
+ f"/datasource/samples?{query_string}", method="POST", json={}
+ ),
+ ):
+ view_security_manager.is_guest_user.return_value = is_guest
+ view_security_manager.can_drill_dataset_via_dashboard_access.return_value = True
+ return handle_api_exception(_get_view_func("samples"))(_view_self())
+
+
+def test_samples_failure_returns_generic_error_to_guests(app: Flask) -> None:
+ """When the Drill to detail query fails for an embedded guest, the response
+ carries the generic message: no SQL, table or column names leak through the
+ driver error, and no ``query``/``stacktrace`` payload is serialized."""
+ response = _run_failing_samples(
+ app,
+ is_guest=True,
+ query_string="datasource_type=table&datasource_id=1&dashboard_id=5",
+ )
+
+ assert response.status_code == 422
+ body = response.get_data(as_text=True)
+ assert superset_json.loads(body) == {"error": str(GENERIC_ERROR_MESSAGE)}
+ for leaked in ("private_table", "secret_col", "SELECT"):
+ assert leaked not in body
+
+
+def test_samples_failure_keeps_driver_error_for_authenticated_users(
+ app: Flask,
+) -> None:
+ """The guest redaction is scoped: an authenticated user still gets the
+ actionable driver error from a failed samples query."""
+ response = _run_failing_samples(
+ app,
+ is_guest=False,
+ query_string="datasource_type=query&datasource_id=1",
+ )
+
+ assert response.status_code == 422
+ body = response.get_data(as_text=True)
+ assert superset_json.loads(body) == {"error": _RAW_SAMPLES_ERROR}
+
+
@patch("superset.views.datasource.views._", _identity_gettext)
@patch("superset.views.datasource.views.get_samples")
@patch("superset.views.datasource.views.DatasourceDAO.get_datasource")
diff --git a/tests/unit_tests/views/test_base.py b/tests/unit_tests/views/test_base.py
index 89e0cd322d8..f56417964c5 100644
--- a/tests/unit_tests/views/test_base.py
+++ b/tests/unit_tests/views/test_base.py
@@ -299,6 +299,78 @@ def test_api_query_returns_json_content_type() -> None:
assert response.content_type.startswith("application/json")
+def _run_api_query(*, is_guest: bool) -> list[dict[str, Any]]:
+ """Run the ``Api.query`` handler body for a payload that carries the
+ generated SQL, a stacktrace and a raw driver error, and return the parsed
+ response as a guest without "can view query on Dashboard" or as a regular
+ user."""
+ from flask import current_app
+
+ from superset.utils import json
+ from superset.views.api import Api
+
+ handler = Api.query
+ while hasattr(handler, "__wrapped__"):
+ handler = handler.__wrapped__
+
+ query_context = MagicMock()
+ query_context.get_payload.return_value = {
+ "queries": [
+ {
+ "data": [{"a": 1}],
+ "query": "SELECT secret_col FROM private_table",
+ "stacktrace": "Traceback (most recent call last): ...",
+ "error": "relation private_table does not exist",
+ }
+ ]
+ }
+ factory = MagicMock()
+ factory.create.return_value = query_context
+ api_view = Api()
+
+ with (
+ patch.object(api_view, "get_query_context_factory", return_value=factory),
+ patch(
+ "superset.security.SupersetSecurityManager.is_guest_user",
+ return_value=is_guest,
+ ),
+ patch(
+ "superset.security.SupersetSecurityManager.can_access",
+ return_value=False,
+ ),
+ current_app.test_request_context(
+ data={"query_context": '{"datasource": {"id": 1}}'}
+ ),
+ ):
+ response = handler(api_view)
+
+ return json.loads(response.get_data(as_text=True))
+
+
+def test_api_query_redacts_generated_sql_for_guests() -> None:
+ """``Api.query`` returns the same query-context payloads as chart data, so
+ an embedded guest without "can view query on Dashboard" gets the generated
+ SQL and stacktrace removed and the driver error replaced, while the data is
+ preserved."""
+ from superset.utils.error_sanitization import GENERIC_ERROR_MESSAGE
+
+ (result,) = _run_api_query(is_guest=True)
+
+ assert "query" not in result
+ assert "stacktrace" not in result
+ assert result["error"] == str(GENERIC_ERROR_MESSAGE)
+ assert result["data"] == [{"a": 1}]
+
+
+def test_api_query_keeps_payload_for_regular_users() -> None:
+ """The redaction is scoped to guests: a regular user keeps the generated
+ SQL and the driver error."""
+ (result,) = _run_api_query(is_guest=False)
+
+ assert result["query"] == "SELECT secret_col FROM private_table"
+ assert result["error"] == "relation private_table does not exist"
+
+
@pytest.mark.parametrize(
"encoding, payload, expected",
[