fix(import): enforce write permission for objects created by legacy import paths#42205
fix(import): enforce write permission for objects created by legacy import paths#42205sha174n wants to merge 4 commits into
Conversation
…mport paths The unversioned (v0) dashboard and dataset import paths create or update nested objects without checking that the caller may write those object types. The legacy dashboard import upserts the embedded datasets, and the legacy dataset import upserts the embedded database connections, in both cases relying only on the top-level import permission. Align these paths with the versioned (v1) import commands by requiring the nested object type's own write permission before creating or updating it: dataset write permission for the datasets embedded in a dashboard export, and database write permission for the database connections embedded in a dataset export. The check is only enforced when a request user is present, so the command-line import paths (which run without a request user) keep working unchanged. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
dosubot
Bot
added
authentication:access-control
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #42205 +/- ##
==========================================
- Coverage 65.19% 65.18% -0.01%
==========================================
Files 2767 2767
Lines 156062 156064 +2
Branches 35717 35716 -1
==========================================
- Hits 101741 101735 -6
- Misses 52361 52366 +5
- Partials 1960 1963 +3
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
![bito-code-review[bot]](https://avatars.githubusercontent.com/in/1061978?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review Agent Run #708587
Actionable Suggestions - 1
-
superset/commands/dataset/importers/v0.py - 1
- Wrong exception for permission error · Line 227-228
Review Details
-
Files reviewed - 6 · Commit Range:
872c07c..96547ef- superset/commands/dashboard/importers/v0.py
- superset/commands/dataset/importers/v0.py
- tests/unit_tests/dashboards/commands/importers/v0/__init__.py
- tests/unit_tests/dashboards/commands/importers/v0/import_test.py
- tests/unit_tests/datasets/commands/importers/v0/__init__.py
- tests/unit_tests/datasets/commands/importers/v0/import_test.py
-
Files skipped - 0
-
Tools
- MyPy (Static Code Analysis) - ✔︎ Successful
- Astral Ruff (Static Code Analysis) - ✔︎ Successful
- Whispers (Secret Scanner) - ✔︎ Successful
- Detect-secrets (Secret Scanner) - ✔︎ Successful
Bito Usage Guide
Commands
Type the following command in the pull request comment and save the comment.
-
/review- Manually triggers a full AI review. -
/pause- Pauses automatic reviews on this pull request. -
/resume- Resumes automatic reviews. -
/resolve- Marks all Bito-posted review comments as resolved. -
/abort- Cancels all in-progress reviews.
Refer to the documentation for additional commands.
Configuration
This repository uses Superset You can customize the agent settings here or contact your Bito workspace admin at evan@preset.io.
Documentation & Help
AI Code Review powered by
| raise ImportFailedError( | ||
| "User doesn't have permission to create or update databases" |
There was a problem hiding this comment.
Permission denial returns HTTP 500 via ImportFailedError instead of 403. The related dashboard import at v0.py:320 uses the same wrong exception type, but correct patterns exist in database/importers/v1/utils.py:56 which use contextually appropriate exceptions. Use ForbiddenError (status=403) since this is an authorization failure, not an import failure.
Code Review Run #708587
Should Bito avoid suggestions like this for future reviews? (Manage Rules)
- Yes, avoid them
1 participant
SUMMARY
The unversioned ("v0") dashboard and dataset import paths create or update nested objects without checking that the caller may write those object types:
relying only on the top-level import permission.
This aligns those paths with the versioned (v1) import commands by requiring the nested object type's own write permission before creating or updating it: dataset write permission for the datasets embedded in a dashboard export, and database write permission for the database connections embedded in a dataset export. The check is only enforced when a request user is present, so the command-line import paths (which run without a request user) keep working unchanged.
TESTING INSTRUCTIONS
They cover: a user without the relevant write permission is refused and the nested upsert is not called; a permitted user proceeds; and the no-request-user (CLI) path skips the check and proceeds.
ADDITIONAL INFORMATION