This is an automated email from the ASF dual-hosted git repository.
rusackas 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 fc110d8428f fix(versioning): enforce is_managed_externally on version
restore (#44013)
fc110d8428f is described below
commit fc110d8428f35249a2092778ca0a3e26a2de0b14
Author: Mike Bridge <[email protected]>
AuthorDate: Tue Sep 8 22:55:08 2026 -0600
fix(versioning): enforce is_managed_externally on version restore (#44013)
Co-authored-by: Mike Bridge <[email protected]>
Co-authored-by: Claude Fable 5 <[email protected]>
---
UPDATING.md | 1 +
superset/commands/version_restore.py | 18 +++-
.../charts/version_restore_tests.py | 42 +++++++++
.../commands/test_base_restore_version_command.py | 99 ++++++++++++++++++++++
4 files changed, 157 insertions(+), 3 deletions(-)
diff --git a/UPDATING.md b/UPDATING.md
index 389d6b90370..4c998c267db 100644
--- a/UPDATING.md
+++ b/UPDATING.md
@@ -230,6 +230,7 @@ before retrying. Preview or recheck failures fail closed
rather than treating
unknown impact as zero. Chart and dashboard purge endpoints are unchanged.
- The dashboard datasource-based visibility fallback now fails closed: a
dashboard whose member charts’ datasources cannot be resolved (deleted
datasource rows, missing `datasource_id`, or unsupported datasource types) is
no longer accessible to users without explicit editor/viewer rights, and a
dashboard composed of semantic-view charts now requires `datasource_access` on
(at least one of) its semantic views or their parent semantic layer —
previously any authenticated user could open s [...]
+- Version restore (`POST
/api/v1/{chart,dashboard,dataset}/<uuid>/versions/<version_uuid>/restore`) now
refuses an **externally managed** entity (`is_managed_externally = True`) with
HTTP 403, enforcing server-side what the docs already promised. Previously the
refusal existed only in the browser, so an otherwise-authorized editor could
restore such an entity by calling the endpoint directly and have the restore
overwritten on the next external sync. Soft-delete recovery is deliberately
[...]
- `SAMPLES_ROW_LIMIT` is now the default for `/datasource/samples` requests
without a valid explicit `per_page`, rather than a hard per-request ceiling;
explicit limits are honored up to the existing global row-limit ceiling,
matching `/chart/data` SAMPLES requests.
- The `cockroachdb` extra (`pip install apache-superset[cockroachdb]`) now
installs `sqlalchemy-cockroachdb` instead of the abandoned `cockroachdb`
package, whose SQLAlchemy dialect could not be imported under SQLAlchemy 2.0.
Existing environments with the old package installed should `pip uninstall
cockroachdb && pip install sqlalchemy-cockroachdb` (or simply reinstall the
extra) to restore CockroachDB connectivity.
diff --git a/superset/commands/version_restore.py
b/superset/commands/version_restore.py
index 7a79a40dde8..4fe3f4f3645 100644
--- a/superset/commands/version_restore.py
+++ b/superset/commands/version_restore.py
@@ -64,9 +64,10 @@ class BaseRestoreVersionCommand(BaseCommand):
#: failure modes. ``not_found_exc`` covers "no such entity",
#: "version_uuid not on this entity", and "capture disabled" (the
#: route is inert under the kill-switch); the API handler maps each
- #: to HTTP 404. ``forbidden_exc`` covers the row-level editorship
- #: denial (HTTP 403). ``failed_exc`` wraps unexpected failures inside
- #: the transaction (HTTP 422).
+ #: to HTTP 404. ``forbidden_exc`` covers the row-level editorship denial
+ #: and the refusal to restore an externally managed entity (both HTTP
+ #: 403). ``failed_exc`` wraps unexpected failures inside the transaction
+ #: (HTTP 422).
not_found_exc: ClassVar[type[Exception]]
forbidden_exc: ClassVar[type[Exception]]
failed_exc: ClassVar[type[Exception]]
@@ -149,4 +150,15 @@ class BaseRestoreVersionCommand(BaseCommand):
security_manager.raise_for_editorship(entity)
except SupersetSecurityException as ex:
raise self.forbidden_exc() from ex
+ # Restore is withheld from externally managed entities: their source of
+ # truth lives outside Superset and would overwrite the restore on the
+ # next sync (documented in version-history.mdx). This must be enforced
+ # server-side, not only in the browser — an authorized editor could
+ # otherwise call the endpoint directly. Raised as forbidden_exc (HTTP
+ # 403); FAB's response_403 returns a fixed ``{"message": "Forbidden"}``
+ # body with no reason detail, identical to the editorship denial above,
+ # so the refusal discloses nothing but also can't be distinguished from
+ # a permission denial.
+ if entity.is_managed_externally:
+ raise self.forbidden_exc()
return entity
diff --git a/tests/integration_tests/charts/version_restore_tests.py
b/tests/integration_tests/charts/version_restore_tests.py
index dd31ea0d501..b3e3e150f96 100644
--- a/tests/integration_tests/charts/version_restore_tests.py
+++ b/tests/integration_tests/charts/version_restore_tests.py
@@ -120,6 +120,48 @@ class TestChartRestoreApi(SupersetTestCase):
chart.slice_name = original_name
db.session.commit()
+ def test_restore_refuses_externally_managed_chart(self) -> None:
+ """sc-115616: restore is withheld server-side from an externally
+ managed chart even for an admin who could otherwise edit it — the
+ endpoint returns 403, not 200, so a direct API call cannot bypass the
+ browser gate.
+
+ Chart is the representative real-model/real-endpoint case; the guard
+ lives in the shared BaseRestoreVersionCommand.validate(), so the
+ dashboard and dataset commands inherit it (pinned across all three by
+ the parametrized unit test in
+ tests/unit_tests/commands/test_base_restore_version_command.py)."""
+ _persist_fixture_state()
+ chart: Slice = (
+ db.session.query(Slice).filter(Slice.slice_name == "Boys").first()
+ )
+ assert chart is not None
+ chart_uuid = str(chart.uuid)
+
+ # A save so there is a prior version to target.
+ chart.slice_name = "Boys v1"
+ db.session.commit()
+
+ self.login(ADMIN_USERNAME)
+ listing = _json.loads(self._list(chart_uuid).data.decode("utf-8"))
+ target_uuid = listing["result"][-1]["version_uuid"]
+
+ # Mark the chart as externally managed, then attempt the restore.
+ chart.is_managed_externally = True
+ db.session.commit()
+ try:
+ rv = self._restore(chart_uuid, target_uuid)
+ assert rv.status_code == 403, rv.data
+ # The refusal did not mutate the chart.
+ db.session.expire_all()
+ chart = db.session.query(Slice).filter(Slice.uuid ==
chart.uuid).one()
+ assert chart.slice_name == "Boys v1"
+ finally:
+ # Cleanup
+ chart.is_managed_externally = False
+ chart.slice_name = "Boys"
+ db.session.commit()
+
def test_restore_returns_404_for_unknown_uuid(self) -> None:
self.login(ADMIN_USERNAME)
rv = self._restore(
diff --git a/tests/unit_tests/commands/test_base_restore_version_command.py
b/tests/unit_tests/commands/test_base_restore_version_command.py
new file mode 100644
index 00000000000..988b6128829
--- /dev/null
+++ b/tests/unit_tests/commands/test_base_restore_version_command.py
@@ -0,0 +1,99 @@
+# 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.
+"""Unit tests for ``BaseRestoreVersionCommand.validate`` (version restore).
+
+Exercises the shared ``validate()`` through each of the three concrete
+commands so the contract is pinned against the real subclasses (and their real
+``forbidden_exc`` types), guarding against a subclass overriding it. This is
+the *version* restore command (revert to a past version), distinct from the
+soft-delete recovery command covered in ``test_base_restore_command.py``. The
+real-model / real-endpoint 403 is exercised end-to-end in the per-entity
+``version_restore_tests.py`` integration suites (e.g.
+``tests/integration_tests/charts/version_restore_tests.py``).
+"""
+
+from __future__ import annotations
+
+from collections.abc import Iterator
+from contextlib import contextmanager
+from unittest.mock import MagicMock, patch
+from uuid import uuid4
+
+import pytest
+
+from superset.commands.chart.restore_version import RestoreChartVersionCommand
+from superset.commands.dashboard.restore_version import
RestoreDashboardVersionCommand
+from superset.commands.dataset.restore_version import
RestoreDatasetVersionCommand
+from superset.commands.version_restore import BaseRestoreVersionCommand
+
+_COMMAND_CLASSES = [
+ RestoreChartVersionCommand,
+ RestoreDashboardVersionCommand,
+ RestoreDatasetVersionCommand,
+]
+
+
+@contextmanager
+def _validate_context(entity: MagicMock) -> Iterator[None]:
+ """Patch the base command's collaborators so ``validate()`` reaches the
+ is_managed_externally guard: capture is on, the entity is found, and the
+ editorship check passes. What varies between tests is only the entity's
+ ``is_managed_externally`` value.
+ """
+ with (
+ patch("superset.commands.version_restore.capture_enabled",
return_value=True),
+ patch(
+ "superset.commands.version_restore.find_active_by_uuid",
+ return_value=entity,
+ ),
+ patch("superset.commands.version_restore.security_manager") as
mock_sec,
+ ):
+ mock_sec.raise_for_editorship = MagicMock(return_value=None)
+ yield
+
+
[email protected]("command_cls", _COMMAND_CLASSES)
+def test_validate_refuses_externally_managed_entity(
+ command_cls: type[BaseRestoreVersionCommand], app_context: None
+) -> None:
+ """sc-115616: version restore is withheld server-side from externally
+ managed entities — an otherwise-authorized editor must not be able to
+ bypass the browser gate by calling the endpoint directly. Each concrete
+ command raises its own ``forbidden_exc`` (mapped to HTTP 403)."""
+ entity = MagicMock()
+ entity.is_managed_externally = True
+ cmd = command_cls(uuid4(), uuid4())
+
+ with _validate_context(entity):
+ with pytest.raises(command_cls.forbidden_exc):
+ cmd.validate()
+
+
[email protected]("command_cls", _COMMAND_CLASSES)
+def test_validate_returns_entity_when_not_managed_externally(
+ command_cls: type[BaseRestoreVersionCommand], app_context: None
+) -> None:
+ """The reverted-fix control: an editable, non-externally-managed entity
+ passes validation and is returned to ``run()``. Removing the guard would
+ make the refusal test above pass here too, so this pins that the guard is
+ what rejects the managed case."""
+ entity = MagicMock()
+ entity.is_managed_externally = False
+ cmd = command_cls(uuid4(), uuid4())
+
+ with _validate_context(entity):
+ assert cmd.validate() is entity
