You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by mi...@apache.org on 2026/01/13 13:50:24 UTC
(superset) branch 5.0-pulse created (now edbeadd249)
This is an automated email from the ASF dual-hosted git repository.
michaelsmolina pushed a change to branch 5.0-pulse
in repository https://gitbox.apache.org/repos/asf/superset.git
at edbeadd249 fix: enhance disallowed SQL functions list for improved security (#33084)
This branch includes the following new commits:
new 102df5f4d3 fix: remove unwanted info from tags REST API (#36266)
new 2eb9f4ca08 fix: dataset update with invalid SQL query (#35543)
new 8c9489d72f fix: update chart with dashboards validation (#35523)
new d8b01c668f fix: prevent anonymous code in Postgres (#34412)
new edbeadd249 fix: enhance disallowed SQL functions list for improved security (#33084)
The 5 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) 05/05: fix: enhance disallowed SQL functions list for improved security (#33084)
Posted by mi...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
michaelsmolina pushed a commit to branch 5.0-pulse
in repository https://gitbox.apache.org/repos/asf/superset.git
commit edbeadd2491c02325a9ea668a7e8ff421d1276b8
Author: sha174n <10...@users.noreply.github.com>
AuthorDate: Fri Jul 25 00:36:32 2025 +0100
fix: enhance disallowed SQL functions list for improved security (#33084)
(cherry picked from commit 7f44992c4b686f3439a51819c34b463f04911cd6)
---
superset/commands/dataset/update.py | 2 +-
superset/config.py | 87 +++++++++++++++++++++++++++++++++++--
2 files changed, 84 insertions(+), 5 deletions(-)
diff --git a/superset/commands/dataset/update.py b/superset/commands/dataset/update.py
index 4900b0caac..ad9f1f3c4d 100644
--- a/superset/commands/dataset/update.py
+++ b/superset/commands/dataset/update.py
@@ -79,7 +79,7 @@ class UpdateDatasetCommand(UpdateMixin, BaseCommand):
assert self._model
return DatasetDAO.update(self._model, attributes=self._properties)
- def validate(self) -> None:
+ def validate(self) -> None: # noqa: C901
exceptions: list[ValidationError] = []
owner_ids: Optional[list[int]] = self._properties.get("owners")
diff --git a/superset/config.py b/superset/config.py
index bc3841d23f..cd33946a09 100644
--- a/superset/config.py
+++ b/superset/config.py
@@ -1351,18 +1351,97 @@ DB_SQLA_URI_VALIDATOR: Callable[[URL], None] | None = None
# unsafe SQL functions in SQL Lab and Charts. The keys of the dictionary are the engine
# names, and the values are sets of disallowed functions.
DISALLOWED_SQL_FUNCTIONS: dict[str, set[str]] = {
+ # PostgreSQL functions that could reveal sensitive information
"postgresql": {
- "database_to_xml",
+ # System information functions
+ "current_database",
+ "current_schema",
+ "current_user",
+ "session_user",
+ "current_setting",
+ "version",
+ # Network/server information functions
"inet_client_addr",
+ "inet_client_port",
"inet_server_addr",
+ "inet_server_port",
+ # File system functions
+ "pg_read_file",
+ "pg_ls_dir",
+ "pg_read_binary_file",
+ # XML functions that can execute SQL
+ "database_to_xml",
+ "database_to_xmlschema",
"query_to_xml",
- "query_to_xml_and_xmlschema",
+ "query_to_xmlschema",
"table_to_xml",
"table_to_xml_and_xmlschema",
+ "query_to_xml_and_xmlschema",
+ "table_to_xmlschema",
+ # Other potentially dangerous functions
+ "pg_sleep",
+ "pg_terminate_backend",
+ },
+ # MySQL functions and variables that could reveal sensitive information
+ "mysql": {
+ # Functions
+ "database",
+ "schema",
+ "current_user",
+ "session_user",
+ "system_user",
+ "user",
+ "version",
+ "connection_id",
+ "load_file",
+ "sleep",
+ "benchmark",
+ "kill",
+ },
+ # SQLite functions that could reveal sensitive information
+ "sqlite": {
+ "sqlite_version",
+ "sqlite_source_id",
+ "sqlite_offset",
+ "sqlite_compileoption_used",
+ "sqlite_compileoption_get",
+ "load_extension",
+ },
+ # Microsoft SQL Server functions
+ "mssql": {
+ "db_name",
+ "suser_sname",
+ "user_name",
+ "host_name",
+ "host_id",
+ "suser_id",
+ "system_user",
+ "current_user",
+ "original_login",
+ "xp_cmdshell",
+ "xp_regread",
+ "xp_fileexist",
+ "xp_dirtree",
+ "serverproperty",
+ "is_srvrolemember",
+ "has_dbaccess",
+ "fn_virtualfilestats",
+ "fn_servershareddrives",
+ },
+ # Clickhouse functions
+ "clickhouse": {
+ "currentUser",
+ "currentDatabase",
+ "hostName",
+ "currentRoles",
"version",
+ "buildID",
+ "url",
+ "filesystemPath",
+ "getOSInformation",
+ "getMacro",
+ "getSetting",
},
- "clickhouse": {"url", "version", "currentDatabase", "hostName"},
- "mysql": {"version"},
}
(superset) 01/05: fix: remove unwanted info from tags REST API (#36266)
Posted by mi...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
michaelsmolina pushed a commit to branch 5.0-pulse
in repository https://gitbox.apache.org/repos/asf/superset.git
commit 102df5f4d3880fe6561b686e6d0aadf3ab992d06
Author: Daniel Vaz Gaspar <da...@gmail.com>
AuthorDate: Tue Nov 25 16:22:31 2025 +0000
fix: remove unwanted info from tags REST API (#36266)
(cherry picked from commit cd36845d568a3d91c592f00d7b26083c0450c51e)
---
superset/tags/api.py | 1 -
tests/integration_tests/tags/api_tests.py | 40 +++++++++++++++++++++++++++++++
2 files changed, 40 insertions(+), 1 deletion(-)
diff --git a/superset/tags/api.py b/superset/tags/api.py
index 7351a39a04..3c28f86c16 100644
--- a/superset/tags/api.py
+++ b/superset/tags/api.py
@@ -109,7 +109,6 @@ class TagRestApi(BaseSupersetModelRestApi):
"created_on_delta_humanized",
"created_by.first_name",
"created_by.last_name",
- "created_by",
]
base_related_field_filters = {
diff --git a/tests/integration_tests/tags/api_tests.py b/tests/integration_tests/tags/api_tests.py
index 79022d2805..a46a0eebed 100644
--- a/tests/integration_tests/tags/api_tests.py
+++ b/tests/integration_tests/tags/api_tests.py
@@ -151,6 +151,46 @@ class TestTagApi(SupersetTestCase):
db.session.delete(tag)
db.session.commit()
+ def test_get_tag_user_fields(self):
+ """
+ Query API: Test get tag only returns first_name and last_name for
+ created_by and changed_by fields
+ """
+ self.login(ADMIN_USERNAME)
+ # Create tag via API to ensure created_by is set
+ uri = "api/v1/tag/"
+ rv = self.client.post(
+ uri,
+ json={"name": "test_user_fields_tag", "objects_to_tag": []},
+ )
+ assert rv.status_code == 201
+
+ # Get the created tag
+ tag = db.session.query(Tag).filter(Tag.name == "test_user_fields_tag").first()
+ assert tag is not None
+
+ # Fetch the tag via GET API
+ uri = f"api/v1/tag/{tag.id}"
+ rv = self.client.get(uri)
+ assert rv.status_code == 200
+
+ data = json.loads(rv.data.decode("utf-8"))
+ result = data["result"]
+
+ # Verify created_by only contains first_name and last_name
+ assert result["created_by"] is not None
+ assert set(result["created_by"].keys()) == {"first_name", "last_name"}
+ assert result["created_by"]["first_name"] is not None
+ assert result["created_by"]["last_name"] is not None
+
+ # Verify changed_by only contains first_name and last_name (or is None)
+ if result["changed_by"] is not None:
+ assert set(result["changed_by"].keys()) == {"first_name", "last_name"}
+
+ # Cleanup
+ db.session.delete(tag)
+ db.session.commit()
+
def test_get_tag_not_found(self):
"""
Query API: Test get query not found
(superset) 04/05: fix: prevent anonymous code in Postgres (#34412)
Posted by mi...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
michaelsmolina pushed a commit to branch 5.0-pulse
in repository https://gitbox.apache.org/repos/asf/superset.git
commit d8b01c668fdd3e6d12de52e8df104f70b46b96b0
Author: Beto Dealmeida <ro...@dealmeida.net>
AuthorDate: Thu Jul 31 08:33:34 2025 -0400
fix: prevent anonymous code in Postgres (#34412)
(cherry picked from commit 6fc734da5154524b2e235c4df0d75977a7d5211f)
---
superset/sql/parse.py | 10 ++++++++++
tests/unit_tests/sql/parse_tests.py | 37 +++++++++++++++++++++++++++++++++++++
2 files changed, 47 insertions(+)
diff --git a/superset/sql/parse.py b/superset/sql/parse.py
index 792d0e9e6f..9286bca65a 100644
--- a/superset/sql/parse.py
+++ b/superset/sql/parse.py
@@ -382,6 +382,16 @@ class SQLStatement(BaseSQLStatement[exp.Expression]):
if isinstance(node, exp.Command) and node.name == "ALTER":
return True
+ if (
+ self._dialect == Dialects.POSTGRES
+ and isinstance(self._parsed, exp.Command)
+ and self._parsed.name == "DO"
+ ):
+ # anonymous blocks can be written in many different languages (the default
+ # is PL/pgSQL), so parsing them it out of scope of this class; we just
+ # assume the anonymous block is mutating
+ return True
+
# Postgres runs DMLs prefixed by `EXPLAIN ANALYZE`, see
# https://www.postgresql.org/docs/current/sql-explain.html
if (
diff --git a/tests/unit_tests/sql/parse_tests.py b/tests/unit_tests/sql/parse_tests.py
index 8dc06aeea3..dd93ad230c 100644
--- a/tests/unit_tests/sql/parse_tests.py
+++ b/tests/unit_tests/sql/parse_tests.py
@@ -1072,6 +1072,43 @@ def test_is_mutating(engine: str) -> None:
).is_mutating()
+@pytest.mark.parametrize(
+ "sql, expected",
+ [
+ (
+ """
+DO $$
+BEGIN
+ INSERT INTO public.users (name, real_name)
+ VALUES ('SQLLab bypass DML', 'SQLLab bypass DML');
+END;
+$$;
+ """,
+ True,
+ ),
+ (
+ """
+DO $$
+BEGIN
+ IF (SELECT COUNT(*) FROM orders WHERE status = 'pending') > 100 THEN
+ RAISE NOTICE 'High pending order volume detected';
+ END IF;
+END;
+$$;
+ """,
+ True,
+ ),
+ ],
+)
+def test_is_mutating_anonymous_block(sql: str, expected: bool) -> None:
+ """
+ Test for `is_mutating` with a Postgres anonymous block.
+
+ Since we can't parse the PL/pgSQL inside the block we always assume it is mutating.
+ """
+ assert SQLStatement(sql, "postgresql").is_mutating() == expected
+
+
def test_optimize() -> None:
"""
Test that the `optimize` method works as expected.
(superset) 03/05: fix: update chart with dashboards validation (#35523)
Posted by mi...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
michaelsmolina pushed a commit to branch 5.0-pulse
in repository https://gitbox.apache.org/repos/asf/superset.git
commit 8c9489d72f69a44b43b0d835990e51f78fb928e2
Author: Daniel Vaz Gaspar <da...@gmail.com>
AuthorDate: Tue Oct 7 07:17:49 2025 +0100
fix: update chart with dashboards validation (#35523)
(cherry picked from commit 9d50f1b8a244471659449672ac252d642bf27fe0)
---
superset/commands/chart/update.py | 27 ++++
tests/integration_tests/charts/commands_tests.py | 151 +++++++++++++++++++++++
2 files changed, 178 insertions(+)
diff --git a/superset/commands/chart/update.py b/superset/commands/chart/update.py
index b7d0a4dab9..25ca2539a1 100644
--- a/superset/commands/chart/update.py
+++ b/superset/commands/chart/update.py
@@ -37,6 +37,7 @@ from superset.commands.utils import get_datasource_by_id, update_tags, validate_
from superset.daos.chart import ChartDAO
from superset.daos.dashboard import DashboardDAO
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.decorators import on_error, transaction
@@ -71,6 +72,28 @@ class UpdateChartCommand(UpdateMixin, BaseCommand):
return ChartDAO.update(self._model, self._properties)
+ def _validate_new_dashboard_access(
+ self, requested_dashboards: list[Dashboard], exceptions: list[Exception]
+ ) -> None:
+ """
+ Validate user has access to any NEW dashboard relationships.
+ Existing relationships are preserved to maintain chart ownership rights.
+ """
+ if not self._model:
+ return
+
+ existing_dashboard_ids = {d.id for d in self._model.dashboards}
+ requested_dashboard_ids = {d.id for d in requested_dashboards}
+
+ if new_dashboard_ids := requested_dashboard_ids - existing_dashboard_ids:
+ # For NEW dashboard relationships, verify user has access
+ accessible_dashboards = DashboardDAO.find_by_ids(list(new_dashboard_ids))
+ accessible_dashboard_ids = {d.id for d in accessible_dashboards}
+ unauthorized_dashboard_ids = new_dashboard_ids - accessible_dashboard_ids
+
+ if unauthorized_dashboard_ids:
+ exceptions.append(DashboardsNotFoundValidationError())
+
def validate(self) -> None: # noqa: C901
exceptions: list[ValidationError] = []
dashboard_ids = self._properties.get("dashboards")
@@ -120,12 +143,16 @@ class UpdateChartCommand(UpdateMixin, BaseCommand):
# Validate/Populate dashboards only if it's a list
if dashboard_ids is not None:
+ # First, verify all requested dashboards exist
dashboards = DashboardDAO.find_by_ids(
dashboard_ids,
skip_base_filter=True,
)
if len(dashboards) != len(dashboard_ids):
exceptions.append(DashboardsNotFoundValidationError())
+ else:
+ # Then, validate user has access to any NEW dashboard relationships
+ self._validate_new_dashboard_access(dashboards, exceptions)
self._properties["dashboards"] = dashboards
if exceptions:
diff --git a/tests/integration_tests/charts/commands_tests.py b/tests/integration_tests/charts/commands_tests.py
index 3a193b40e2..c39caf489c 100644
--- a/tests/integration_tests/charts/commands_tests.py
+++ b/tests/integration_tests/charts/commands_tests.py
@@ -415,6 +415,157 @@ class TestChartsUpdateCommand(SupersetTestCase):
assert len(chart.owners) == 1
assert chart.owners[0] == admin
+ @patch("superset.commands.chart.update.g")
+ @patch("superset.utils.core.g")
+ @patch("superset.security.manager.g")
+ @pytest.mark.usefixtures("load_energy_table_with_slice")
+ def test_update_chart_dashboard_security_existing_relationship(
+ self, mock_sm_g, mock_u_g, mock_c_g
+ ):
+ """Test that chart owners can update charts linked to inaccessible
+ dashboards (existing relationships)"""
+ from superset.models.dashboard import Dashboard
+
+ # Create a chart owned by alpha
+ admin = security_manager.find_user(username="admin")
+ alpha = security_manager.find_user(username="alpha")
+
+ # Set user context for dashboard creation
+ mock_u_g.user = mock_c_g.user = mock_sm_g.user = admin
+
+ chart = db.session.query(Slice).first()
+ chart.owners = [alpha]
+
+ # Create a dashboard owned by admin (not accessible to alpha)
+ admin_dashboard = Dashboard(
+ dashboard_title="Admin Dashboard",
+ slug="admin-dashboard",
+ owners=[admin],
+ published=False,
+ )
+ db.session.add(admin_dashboard)
+
+ # Link chart to admin's dashboard (alpha owns chart, admin owns dashboard)
+ chart.dashboards.append(admin_dashboard)
+ db.session.commit()
+
+ # Alpha should still be able to update their chart
+ # even though it's linked to admin's dashboard
+ mock_u_g.user = mock_c_g.user = mock_sm_g.user = alpha
+
+ json_obj = {
+ "description": "Updated description",
+ "dashboards": [
+ d.id for d in chart.dashboards
+ ], # Keep existing relationships
+ }
+ command = UpdateChartCommand(chart.id, json_obj)
+ command.run()
+
+ # Should succeed - alpha can update their chart
+ updated_chart = db.session.query(Slice).get(chart.id)
+ assert updated_chart.description == "Updated description"
+
+ # Clean up
+ db.session.delete(admin_dashboard)
+ db.session.commit()
+
+ @patch("superset.commands.chart.update.g")
+ @patch("superset.utils.core.g")
+ @patch("superset.security.manager.g")
+ @pytest.mark.usefixtures("load_energy_table_with_slice")
+ def test_update_chart_dashboard_security_new_unauthorized_relationship(
+ self, mock_sm_g, mock_u_g, mock_c_g
+ ):
+ """Test that users cannot add charts to dashboards they don't have access to"""
+ from superset.commands.chart.exceptions import ChartInvalidError
+ from superset.models.dashboard import Dashboard
+
+ admin = security_manager.find_user(username="admin")
+ alpha = security_manager.find_user(username="alpha")
+
+ # Set user context for dashboard creation
+ mock_u_g.user = mock_c_g.user = mock_sm_g.user = admin
+
+ # Create chart owned by alpha
+ chart = db.session.query(Slice).first()
+ chart.owners = [alpha]
+
+ # Create private dashboard owned by admin (not accessible to alpha)
+ admin_dashboard = Dashboard(
+ dashboard_title="Admin Private Dashboard",
+ slug="admin-private-dashboard",
+ owners=[admin],
+ published=False, # Private dashboard
+ )
+ db.session.add(admin_dashboard)
+ db.session.commit()
+
+ # Alpha tries to add their chart to admin's private dashboard
+ mock_u_g.user = mock_c_g.user = mock_sm_g.user = alpha
+
+ json_obj = {
+ "description": "Trying to add to unauthorized dashboard",
+ "dashboards": [admin_dashboard.id], # NEW unauthorized relationship
+ }
+ command = UpdateChartCommand(chart.id, json_obj)
+
+ # Should fail - alpha cannot access admin's private dashboard
+ with self.assertRaises(ChartInvalidError): # noqa: PT027
+ command.run()
+
+ # Clean up
+ db.session.delete(admin_dashboard)
+ db.session.commit()
+
+ @patch("superset.commands.chart.update.g")
+ @patch("superset.utils.core.g")
+ @patch("superset.security.manager.g")
+ @pytest.mark.usefixtures("load_energy_table_with_slice")
+ def test_update_chart_dashboard_security_admin_bypass(
+ self, mock_sm_g, mock_u_g, mock_c_g
+ ):
+ """Test that admins can add charts to any dashboard"""
+ from superset.models.dashboard import Dashboard
+
+ admin = security_manager.find_user(username="admin")
+ alpha = security_manager.find_user(username="alpha")
+
+ # Set user context for dashboard creation
+ mock_u_g.user = mock_c_g.user = mock_sm_g.user = alpha
+
+ # Create chart owned by admin
+ chart = db.session.query(Slice).first()
+ chart.owners = [admin]
+
+ # Create private dashboard owned by alpha
+ alpha_dashboard = Dashboard(
+ dashboard_title="Alpha Private Dashboard",
+ slug="alpha-private-dashboard",
+ owners=[alpha],
+ published=False,
+ )
+ db.session.add(alpha_dashboard)
+ db.session.commit()
+
+ # Admin should be able to add chart to any dashboard
+ mock_u_g.user = mock_c_g.user = mock_sm_g.user = admin
+
+ json_obj = {
+ "description": "Admin adding to any dashboard",
+ "dashboards": [alpha_dashboard.id],
+ }
+ command = UpdateChartCommand(chart.id, json_obj)
+ command.run()
+
+ # Should succeed - admin has access to all dashboards
+ updated_chart = db.session.query(Slice).get(chart.id)
+ assert alpha_dashboard in updated_chart.dashboards
+
+ # Clean up
+ db.session.delete(alpha_dashboard)
+ db.session.commit()
+
class TestChartWarmUpCacheCommand(SupersetTestCase):
def test_warm_up_cache_command_chart_not_found(self):
(superset) 02/05: fix: dataset update with invalid SQL query (#35543)
Posted by mi...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
michaelsmolina pushed a commit to branch 5.0-pulse
in repository https://gitbox.apache.org/repos/asf/superset.git
commit 2eb9f4ca080cdb8ff6a817ffc3eba8d5e613a9df
Author: Daniel Vaz Gaspar <da...@gmail.com>
AuthorDate: Wed Oct 8 12:22:38 2025 +0100
fix: dataset update with invalid SQL query (#35543)
(cherry picked from commit 50a5854b250fc886647d1ac2f6f6d73326c9b40a)
---
.../SqlLab/components/SaveDatasetModal/index.tsx | 70 +++++++++++++---------
superset/commands/dataset/update.py | 57 +++++++++++++++++-
2 files changed, 95 insertions(+), 32 deletions(-)
diff --git a/superset-frontend/src/SqlLab/components/SaveDatasetModal/index.tsx b/superset-frontend/src/SqlLab/components/SaveDatasetModal/index.tsx
index ab17556a72..7d02144528 100644
--- a/superset-frontend/src/SqlLab/components/SaveDatasetModal/index.tsx
+++ b/superset-frontend/src/SqlLab/components/SaveDatasetModal/index.tsx
@@ -33,6 +33,7 @@ import {
QueryResponse,
QueryFormData,
VizType,
+ getClientErrorObject,
} from '@superset-ui/core';
import { useSelector, useDispatch } from 'react-redux';
import dayjs from 'dayjs';
@@ -204,39 +205,50 @@ export const SaveDatasetModal = ({
}
setLoading(true);
- const [, key] = await Promise.all([
- updateDataset(
- datasource?.dbId,
- datasetToOverwrite?.datasetid,
- datasource?.sql,
- datasource?.columns?.map(
- (d: { column_name: string; type: string; is_dttm: boolean }) => ({
- column_name: d.column_name,
- type: d.type,
- is_dttm: d.is_dttm,
- }),
+ try {
+ const [, key] = await Promise.all([
+ updateDataset(
+ datasource?.dbId,
+ datasetToOverwrite?.datasetid,
+ datasource?.sql,
+ datasource?.columns?.map(
+ (d: { column_name: string; type: string; is_dttm: boolean }) => ({
+ column_name: d.column_name,
+ type: d.type,
+ is_dttm: d.is_dttm,
+ }),
+ ),
+ datasetToOverwrite?.owners?.map((o: DatasetOwner) => o.id),
+ true,
),
- datasetToOverwrite?.owners?.map((o: DatasetOwner) => o.id),
- true,
- ),
- postFormData(datasetToOverwrite.datasetid, 'table', {
- ...formDataWithDefaults,
- datasource: `${datasetToOverwrite.datasetid}__table`,
- ...(defaultVizType === VizType.Table && {
- all_columns: datasource?.columns?.map(column => column.column_name),
+ postFormData(datasetToOverwrite.datasetid, 'table', {
+ ...formDataWithDefaults,
+ datasource: `${datasetToOverwrite.datasetid}__table`,
+ ...(defaultVizType === VizType.Table && {
+ all_columns: datasource?.columns?.map(column => column.column_name),
+ }),
}),
- }),
- ]);
- setLoading(false);
+ ]);
+ setLoading(false);
- const url = mountExploreUrl(null, {
- [URL_PARAMS.formDataKey.name]: key,
- });
- createWindow(url);
+ const url = mountExploreUrl(null, {
+ [URL_PARAMS.formDataKey.name]: key,
+ });
+ createWindow(url);
- setShouldOverwriteDataset(false);
- setDatasetName(getDefaultDatasetName());
- onHide();
+ setShouldOverwriteDataset(false);
+ setDatasetName(getDefaultDatasetName());
+ onHide();
+ } catch (error) {
+ setLoading(false);
+ getClientErrorObject(error).then((e: { error: string }) => {
+ dispatch(
+ addDangerToast(
+ e.error || t('An error occurred while overwriting the dataset'),
+ ),
+ );
+ });
+ }
};
const loadDatasetOverwriteOptions = useCallback(
diff --git a/superset/commands/dataset/update.py b/superset/commands/dataset/update.py
index 2772cc0ffa..4900b0caac 100644
--- a/superset/commands/dataset/update.py
+++ b/superset/commands/dataset/update.py
@@ -17,7 +17,7 @@
import logging
from collections import Counter
from functools import partial
-from typing import Any, Optional
+from typing import Any, cast, Optional
from flask_appbuilder.models.sqla import Model
from marshmallow import ValidationError
@@ -27,9 +27,11 @@ from superset import security_manager
from superset.commands.base import BaseCommand, UpdateMixin
from superset.commands.dataset.exceptions import (
DatabaseChangeValidationError,
+ DatabaseNotFoundValidationError,
DatasetColumnNotFoundValidationError,
DatasetColumnsDuplicateValidationError,
DatasetColumnsExistsValidationError,
+ DatasetDataAccessIsNotAllowed,
DatasetExistsValidationError,
DatasetForbiddenError,
DatasetInvalidError,
@@ -41,8 +43,9 @@ from superset.commands.dataset.exceptions import (
)
from superset.connectors.sqla.models import SqlaTable
from superset.daos.dataset import DatasetDAO
-from superset.exceptions import SupersetSecurityException
-from superset.sql_parse import Table
+from superset.exceptions import SupersetParseError, SupersetSecurityException
+from superset.models.core import Database
+from superset.sql.parse import Table
from superset.utils.decorators import on_error, transaction
logger = logging.getLogger(__name__)
@@ -99,12 +102,27 @@ class UpdateDatasetCommand(UpdateMixin, BaseCommand):
self._model.database.get_default_catalog()
)
+ new_db_connection: Database | None = None
+
+ if database_id and database_id != self._model.database.id:
+ if new_db_connection := DatasetDAO.get_database_by_id(database_id):
+ self._properties["database"] = new_db_connection
+ else:
+ exceptions.append(DatabaseNotFoundValidationError())
+ db = new_db_connection or self._model.database
+
table = Table(
self._properties.get("table_name"), # type: ignore
self._properties.get("schema"),
catalog,
)
+ schema = (
+ self._properties["schema"]
+ if "schema" in self._properties
+ else self._model.schema
+ )
+
# Validate uniqueness
if not DatasetDAO.validate_update_uniqueness(
self._model.database,
@@ -127,6 +145,9 @@ class UpdateDatasetCommand(UpdateMixin, BaseCommand):
except ValidationError as ex:
exceptions.append(ex)
+ # Validate SQL access
+ self._validate_sql_access(db, catalog, schema, exceptions)
+
# Validate columns
if columns := self._properties.get("columns"):
self._validate_columns(columns, exceptions)
@@ -138,6 +159,36 @@ class UpdateDatasetCommand(UpdateMixin, BaseCommand):
if exceptions:
raise DatasetInvalidError(exceptions=exceptions)
+ def _validate_sql_access(
+ self,
+ db: Database,
+ catalog: str | None,
+ schema: str | None,
+ exceptions: list[ValidationError],
+ ) -> None:
+ """Validate SQL query access if SQL is being updated."""
+ # we know we have a valid model
+ self._model = cast(SqlaTable, self._model)
+
+ sql = self._properties.get("sql")
+ if sql and sql != self._model.sql:
+ try:
+ security_manager.raise_for_access(
+ database=db,
+ sql=sql,
+ catalog=catalog,
+ schema=schema,
+ )
+ except SupersetSecurityException as ex:
+ exceptions.append(DatasetDataAccessIsNotAllowed(ex.error.message))
+ except SupersetParseError as ex:
+ exceptions.append(
+ ValidationError(
+ f"Invalid SQL: {ex.error.message}",
+ field_name="sql",
+ )
+ )
+
def _validate_columns(
self, columns: list[dict[str, Any]], exceptions: list[ValidationError]
) -> None: