mikebridge opened a new pull request, #44013:
URL: https://github.com/apache/superset/pull/44013
### SUMMARY
`version-history.mdx` documents that version **restore** is withheld from
externally managed entities — their source of truth lives outside Superset and
would overwrite the restore on the next sync. But that refusal lived only in
the browser: `BaseRestoreVersionCommand.validate()` checked the
`ENABLE_VERSIONING_CAPTURE` gate, existence, and editorship, and never
`is_managed_externally`. So an otherwise-authorized editor could restore an
externally managed chart, dashboard, or dataset by calling `POST
/api/v1/{type}/<uuid>/versions/<version_uuid>/restore` directly.
`validate()` now refuses an externally managed entity after the editorship
check, raising the command's `forbidden_exc` (HTTP 403) — matching the intent
of the browser gate and enforcing the documented promise server-side. The 403
carries no body, so it reads the same as a permission denial (a known FAB
`response_403` limitation); routing to `failed_exc`/422 to carry an explanatory
message would log the refusal as an error via `logger.exception`, so
`forbidden_exc` is used.
Soft-delete recovery is deliberately **left available** — it changes
visibility, not content, so it doesn't conflict with the external source of
truth.
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
Not applicable — backend authorization fix, no UI.
### TESTING INSTRUCTIONS
- Unit: `tests/unit_tests/commands/test_base_restore_version_command.py` —
parametrized over the three concrete restore commands
(chart/dashboard/dataset), asserting each raises its own `forbidden_exc` for an
externally managed entity, with a reverted-fix control (an editable,
non-managed entity still validates, so the guard is what rejects the managed
case). `pytest tests/unit_tests/commands/test_base_restore_version_command.py`
(6 passed).
- Integration:
`tests/integration_tests/charts/version_restore_tests.py::...test_restore_refuses_externally_managed_chart`
— a real chart marked `is_managed_externally=True` → the restore endpoint
returns 403 and the chart is not mutated.
- The unit guard was verified to fail across all three commands when the
`is_managed_externally` check is reverted.
- Changed-file pre-commit (mypy/ruff/ruff-format/pylint) clean.
### ADDITIONAL INFORMATION
- [x] Has associated issue: sc-115616 (Preset-internal)
- [ ] Required feature flags:
- [ ] Changes UI
- [ ] Includes DB Migration
- [ ] Introduces new feature or API
- [ ] Removes existing feature or API
🤖 Generated with [Claude Code](https://claude.com/claude-code)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
