From f35654179f0f6765db1ce7367b31dbd7eb7ce6fa Mon Sep 17 00:00:00 2001 From: Shaitan <105581038+sha174n@users.noreply.github.com> Date: Thu, 9 Apr 2026 21:47:15 +0100 Subject: [PATCH] fix(sql-lab): apply access check in SqlExecutionResultsCommand (#38952) Co-authored-by: Claude Sonnet 4.6 (cherry picked from commit f49310b8ff852d67b5795f2f918b1afc2ac4b510) --- superset/commands/sql_lab/results.py | 18 +++++++++- .../sql_lab/commands_tests.py | 33 +++++++++++++++++-- 2 files changed, 48 insertions(+), 3 deletions(-) diff --git a/superset/commands/sql_lab/results.py b/superset/commands/sql_lab/results.py index a3cff3bf1ea..94083c82616 100644 --- a/superset/commands/sql_lab/results.py +++ b/superset/commands/sql_lab/results.py @@ -25,7 +25,11 @@ from flask_babel import gettext as __ from superset import db, results_backend, results_backend_use_msgpack from superset.commands.base import BaseCommand from superset.errors import ErrorLevel, SupersetError, SupersetErrorType -from superset.exceptions import SerializationError, SupersetErrorException +from superset.exceptions import ( + SerializationError, + SupersetErrorException, + SupersetSecurityException, +) from superset.models.sql_lab import Query from superset.sqllab.utils import apply_display_max_row_configuration_if_require from superset.utils import core as utils @@ -83,6 +87,18 @@ class SqlExecutionResultsCommand(BaseCommand): status=404, ) + try: + self._query.raise_for_access() + except SupersetSecurityException as ex: + raise SupersetErrorException( + SupersetError( + message=__("Cannot access the query"), + error_type=SupersetErrorType.QUERY_SECURITY_ACCESS_ERROR, + level=ErrorLevel.ERROR, + ), + status=403, + ) from ex + # Now fetch results from backend (query exists, so this is a valid request) read_from_results_backend_start = now_as_float() self._blob = results_backend.get(self._key) diff --git a/tests/integration_tests/sql_lab/commands_tests.py b/tests/integration_tests/sql_lab/commands_tests.py index e3c09fc5091..d272a895383 100644 --- a/tests/integration_tests/sql_lab/commands_tests.py +++ b/tests/integration_tests/sql_lab/commands_tests.py @@ -37,6 +37,7 @@ from superset.models.sql_lab import Query from superset.sqllab.limiting_factor import LimitingFactor from superset.sqllab.schemas import EstimateQueryCostSchema from superset.utils import core as utils +from superset.utils.core import override_user from superset.utils.database import get_example_database from tests.integration_tests.base_tests import SupersetTestCase @@ -251,6 +252,7 @@ class TestSqlExecutionResultsCommand(SupersetTestCase): def create_database_and_query(self): with self.create_app().app_context(): database = get_example_database() + admin = self.get_user("admin") query_obj = Query( client_id="test", database=database, @@ -264,6 +266,7 @@ class TestSqlExecutionResultsCommand(SupersetTestCase): rows=104, error_message="none", results_key="abc_query", + user_id=admin.id, ) db.session.add(query_obj) @@ -344,6 +347,29 @@ class TestSqlExecutionResultsCommand(SupersetTestCase): == SupersetErrorType.RESULTS_BACKEND_ERROR ) + @pytest.mark.usefixtures("create_database_and_query") + @patch("superset.commands.sql_lab.results.results_backend_use_msgpack", False) + def test_validation_unauthorized_access(self) -> None: + command = results.SqlExecutionResultsCommand("abc_query", 1000) + + with mock.patch( + "superset.models.sql_lab.Query.raise_for_access", + side_effect=SupersetSecurityException( + SupersetError( + "dummy", + SupersetErrorType.DATASOURCE_SECURITY_ACCESS_ERROR, + ErrorLevel.ERROR, + ) + ), + ): + with pytest.raises(SupersetErrorException) as ex_info: + command.run() + assert ( + ex_info.value.error.error_type + == SupersetErrorType.QUERY_SECURITY_ACCESS_ERROR + ) + assert ex_info.value.status == 403 + @pytest.mark.usefixtures("create_database_and_query") @patch("superset.commands.sql_lab.results.results_backend_use_msgpack", False) def test_run_succeeds(self) -> None: @@ -359,8 +385,11 @@ class TestSqlExecutionResultsCommand(SupersetTestCase): results.results_backend = mock.Mock() results.results_backend.get.return_value = compressed - command = results.SqlExecutionResultsCommand("abc_query", 1000) - result = command.run() + admin = self.get_user("admin") + with current_app.test_request_context(): + with override_user(admin): + command = results.SqlExecutionResultsCommand("abc_query", 1000) + result = command.run() assert result.get("status") == "success" assert result["query"].get("rows") == 104