You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by "Ujjwaljain16 (via GitHub)" <gi...@apache.org> on 2026/09/10 13:12:02 UTC
[PR] fix(security): delete deprecated permissions left over after upgrades [superset]
Ujjwaljain16 opened a new pull request, #44142:
URL: https://github.com/apache/superset/pull/44142
### SUMMARY
Deprecated permissions are never cleaned up after an upgrade (#33272). `superset init` → `sync_role_definitions` creates missing permissions and runs `clean_perms`, but `clean_perms` only ever deletes `PermissionView` rows whose `Permission`/`ViewMenu` foreign keys are `NULL` — it never removes well-formed PVMs whose underlying endpoint/method has since been deleted from the codebase. `can_select_star` (the reporter's example) is one of many such permissions that silently accumulate on every upgraded install while never appearing on a fresh one.
The maintainer thread on the issue correctly flagged the risk here: a naive "delete everything not in a hardcoded current-permissions allow-list" approach could delete permissions a custom role or an installed extension still legitimately needs, since Superset has no built-in way to distinguish "genuinely dead" from "just not used by a built-in role." This PR deliberately avoids that failure mode:
- **No generic sweep, no allow-list.** Both migrations target an explicit, individually-verified list of `(view_menu, permission)` pairs. There is no "find by permission name" logic anywhere, so the per-object `database_access`/`datasource_access`/`schema_access`/`catalog_access` permissions — which are legitimately reused across many view menus (one per Database/SqlaTable/schema/catalog) — are structurally unreachable by this code, not just excluded by convention.
- **Every candidate was individually verified**, not inferred from "absent from a live DB." Each of the 25 pure-deletion candidates was cross-checked against current `superset/views`/API code, the entire `superset/migrations/` history (to rule out double-handling), and `UPDATING.md`'s actual PR history for the underlying feature removal. Anything without affirmative "removed, not renamed" evidence was left out, even when it looked plausible.
- **Renames preserve role assignments.** 12 permissions were deprecated by being consolidated onto a permission that already exists on current master (mostly the `ModelRestApi` migration of the old monolithic `Superset` view — e.g. `can_explore_json` → `can_read` on `Chart`). These use the existing `migrate_roles()` machinery unmodified, so a role holding the old permission is moved onto the verified live successor rather than just losing it. Two of these mappings also correct an actual inaccuracy in `UPDATING.md` itself (`can_sql_json`/`can_results`'s stated successors don't exist in a live DB; the real ones, confirmed by querying `ab_permission`/`ab_permission_view`, are `can_execute_sql_query`/`can_get_results` on `SQLLab`).
Implementation-wise, `migrate_roles()` can't express a pure deletion with no successor (an empty replacement tuple is silently never processed — confirmed by reading the function), so this adds one small new helper, `delete_pvms()`, to the existing `superset/migrations/shared/security_converge.py` module. It reuses `_delete_old_permissions()`'s existing orphan-safe deletion logic (delete the `PermissionView`, then the `Permission`/`ViewMenu` only if no other PVM still references them) rather than duplicating it, and does its own role-unassignment pass first. `migrate_roles()`, `clean_perms()`, and FAB's `security_cleanup()` are all untouched.
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
Not applicable — this is a backend/migration-only change with no UI impact.
### TESTING INSTRUCTIONS
- `pytest tests/unit_tests/migrations/shared/security_converge_test.py` — 7 tests covering `delete_pvms()` directly: removal + role-association cleanup, orphan deletion, shared-permission preservation, idempotency (running twice is a safe no-op), and that a dynamic/object-style permission is left untouched because it's simply never in the explicit input list.
- `pytest tests/unit_tests/migrations/test_deprecated_permissions_33272.py` — 4 tests that import the two actual migration modules and exercise their real `PVM_LIST`/`PVM_MAP` (not hand-copied subsets), so editing either list in the future automatically re-exercises the updated data.
- `pytest tests/unit_tests/migrations/` — full suite, 154 passed, no regressions.
- Manually verified end-to-end on a disposable database: `superset db upgrade` from a fresh install runs both migrations cleanly and lands on a single new head with no branching; `superset db downgrade` back past both migrations also completes without error (both `downgrade()`s are intentionally documented no-ops, since there's nothing meaningful to restore for a pure deletion, and reversing the renames would strip active permissions from roles).
- `ruff format --check` / `ruff check` clean on all changed files.
### ADDITIONAL INFORMATION
- [x] Has associated issue: Fixes #33272
- [ ] Required feature flags:
- [ ] Changes UI
- [x] Includes DB Migration (follow approval process in [SIP-59](https://github.com/apache/superset/issues/13351))
- [x] Migration is atomic, supports rollback & is backwards-compatible
- [x] Confirm DB migration upgrade and downgrade tested
- [x] Runtime estimates and downtime expectations provided
- [ ] Introduces new feature or API
- [ ] Removes existing feature or API
Runtime/downtime note: both migrations only touch `ab_permission`/`ab_view_menu`/`ab_permission_view`/`ab_permission_view_role` rows for a fixed, small (37 total) set of named permissions — not a table scan — so runtime is negligible regardless of instance size, and no downtime is expected.
🤖 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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(security): delete deprecated permissions left over after upgrades [superset]
Posted by "rebenitez1802 (via GitHub)" <gi...@apache.org>.
rebenitez1802 commented on PR #44142:
URL: https://github.com/apache/superset/pull/44142#issuecomment-5677373630
Request changes: genuinely high-quality migration — helper logic is sound, the security model is intact (no privilege escalation/loss in any of the 12 renames), the alembic chain is clean (single head, chains correctly), and every one of the 25 deletions I could confirm dead *is* dead. But two entries carry the wrong permission name and silently no-op, and one of those loses a custom-role grant it was meant to preserve. Note: despite the `fix(security)` title, none of these are security-boundary issues — the worst real impact is incomplete cleanup + one dropped grant.
🟡 Medium — `can_sqllab_table_viz` is deleted, but the rename that should preserve it targets a phantom name
In the delete migration, `Pvm("Superset", "can_sqllab_table_viz")` (line ~107) is treated as a no-successor deletion. But that permission was generated by `def sqllab_table_viz` (route `/get_or_create_table/`), which was `@deprecated(new_target="api/v1/dataset/get_or_create/")` — its capability *moved* to the live `("Dataset","can_get_or_create_dataset")`, it didn't disappear. Meanwhile the rename migration maps `("Superset","can_get_or_create_table") → ("Dataset","can_get_or_create_dataset")`, but no method named `get_or_create_table` ever existed (the method was always `sqllab_table_viz`), so `can_get_or_create_table` was never a real PVM and that rename is a no-op on every real DB. Net effect: custom roles granted `can_sqllab_table_viz` silently lose it instead of being migrated to `can_get_or_create_dataset`. Fix: remove `can_sqllab_table_viz` from `PVM_LIST` and replace the phantom `can_get_or_create_table` rename entry with `Pvm("Superset","can_sqllab_table_viz"): (Pvm("
Dataset","can_get_or_create_dataset"),)`.
🟡 Medium — `can_test_conn` deletion is a phantom; the real leftover permission is `can_testconn`
`PVM_LIST` deletes `Pvm("Superset", "can_test_conn")` (line ~87), but the old method was `def testconn` (route `/testconn`, `@deprecated`, no `@permission_name` override — verified across history; `def test_conn` never existed), so FAB generated `("Superset","can_testconn")`, without the underscore. The `can_test_conn` spelling only ever lived as a hardcoded string in `SQLLAB_ONLY_PERMISSIONS` (which this PR correctly removes) and appears never to have matched a real PVM. So on upgraded DBs this entry deletes nothing while the genuinely-dead `can_testconn` — the exact kind of leftover #33272 is about — survives. Fix: change the entry to `Pvm("Superset", "can_testconn")`. More broadly, these two phantoms suggest some names were derived from route paths / the manager.py string rather than the actual FAB method names — worth re-deriving `PVM_LIST`/`PVM_MAP` against a real pre-3.0 `ab_permission` dump.
🟢 Low — Cleanup is incomplete; docstring over-claims an exhaustive sweep
Four other permissions from `@expose`'d methods on the same monolithic `Superset` view, removed in the same 3.0-era API-ification, are in neither list and will persist exactly like the targeted ones: `can_override_role_permissions` and `can_theme` (pure deletes — `theme` is superseded by the live `Theme` view menu), and `can_annotation_json` and `can_fave_dashboards_by_username` (renames paralleling the already-handled `can_slice_json` and `can_fave_dashboards`/favorite_status → `can_read`). The delete migration's docstring claims each entry was confirmed "by cross-checking current views/API code, the full migrations history, and UPDATING.md," implying completeness. Fix: add these four (or, at minimum, soften the docstring so it doesn't assert an exhaustive sweep).
🟢 Low — Stale MCP doc reference to a removed permission
`superset/mcp_service/SECURITY.md:405` documents `execute_sql` as requiring `can_sql_json`, which this PR renames to `can_execute_sql_query`. The file isn't touched by the PR (and the row was already partly wrong — it says view menu "Database"), but after this migration the reference is fully dangling; the code already gates on `execute_sql_query`/`SQLLab`. Fix: update the row to `can_execute_sql_query` on `SQLLab`.
🟢 Low — Missing regression guard for the PVM_LIST / PVM_MAP disjointness invariant
The whole two-migration design relies on the 25 deleted pairs and 12 rename keys being disjoint (an overlap would be order-dependent and could permanently drop grants). It currently holds, and both constants are already imported side-by-side in `test_deprecated_permissions_33272.py`, so the guard is one line. Fix: add `assert set(PVM_LIST).isdisjoint(PVM_MAP.keys())` (and optionally that no `PVM_LIST` entry is a `NEW_PVMS` successor).
🟢 Low — No test that a role keeps its *other* permissions after a delete/rename
`delete_pvms`/`migrate_roles` do a targeted `role.permissions.remove(old_pvm)`, but every role-bearing test seeds the role with only the single targeted PVM, so `assert role.permissions == []` can't distinguish "removed the target" from "cleared everything." Fix: add a case seeding a role with `[targeted_pvm, untargeted_pvm]` and assert the untargeted one survives (and, for rename, that the successor was added alongside it).
🟢 Low — Test polish (nits)
Two symmetry gaps, both cheap: no end-to-end test runs *both* chained migrations on one DB (each test exercises one migration in isolation — the migrations are provably non-interacting, so this is completeness only), and rename-path idempotency is untested while the delete path has an explicit twice-run test. Fix: add a chained-run test and a rename idempotency test mirroring `test_delete_migration_is_idempotent_with_real_pvm_list`.
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(security): delete deprecated permissions left over after upgrades [superset]
Posted by "sadpandajoe (via GitHub)" <gi...@apache.org>.
sadpandajoe commented on PR #44142:
URL: https://github.com/apache/superset/pull/44142#issuecomment-5722332546
@Ujjwaljain16 this looks good to go, can you just check if your migrations have conflicts, if it doesn't I can merge your pr.
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(security): delete deprecated permissions left over after upgrades [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #44142:
URL: https://github.com/apache/superset/pull/44142#issuecomment-5695892262
AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
<details>
<summary>
Bito Usage Guide
</summary>
**Commands**
Type the following command in the pull request comment and save the comment.
- `/review` - Manually triggers an incremental AI Review.
- `/review full` - 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 <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/available-commands" target="_blank" rel="noopener noreferrer">documentation</a> for additional commands.
**Configuration**
This repository uses `Superset` You can customize the agent settings <a href="https://alpha.bito.ai/home/ai-agents/code-review-agent" target="_blank" rel="noopener noreferrer">here</a> or contact your Bito workspace admin at evan@preset.io.
**Documentation & Help**
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/getting-started/install-run-using-bito-cloud/create-or-customize-an-agent-instance" target="_blank" rel="noopener noreferrer">Customize agent settings</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/implementing-custom-code-review-rules" target="_blank" rel="noopener noreferrer">Review rules</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent" target="_blank" rel="noopener noreferrer">General documentation</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/faqs" target="_blank" rel="noopener noreferrer">FAQ</a>
</details>
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(security): delete deprecated permissions left over after upgrades [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #44142:
URL: https://github.com/apache/superset/pull/44142#issuecomment-5786429933
AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
<details>
<summary>
Bito Usage Guide
</summary>
**Commands**
Type the following command in the pull request comment and save the comment.
- `/review` - Manually triggers an incremental AI Review.
- `/review full` - 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 <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/available-commands" target="_blank" rel="noopener noreferrer">documentation</a> for additional commands.
**Configuration**
This repository uses `Superset` You can customize the agent settings <a href="https://alpha.bito.ai/home/ai-agents/code-review-agent" target="_blank" rel="noopener noreferrer">here</a> or contact your Bito workspace admin at evan@preset.io.
**Documentation & Help**
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/getting-started/install-run-using-bito-cloud/create-or-customize-an-agent-instance" target="_blank" rel="noopener noreferrer">Customize agent settings</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/implementing-custom-code-review-rules" target="_blank" rel="noopener noreferrer">Review rules</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent" target="_blank" rel="noopener noreferrer">General documentation</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/faqs" target="_blank" rel="noopener noreferrer">FAQ</a>
</details>
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(security): delete deprecated permissions left over after upgrades [superset]
Posted by "Ujjwaljain16 (via GitHub)" <gi...@apache.org>.
Ujjwaljain16 commented on PR #44142:
URL: https://github.com/apache/superset/pull/44142#issuecomment-5784221580
CI was red because of another Alembic head split not a problem with the permission-cleanup logic itself
Root cause:
master landed `2026-09-16_00-00_93d1b4a76c02_add_report_execution_ownership.py` as its own head, but it never got joined into this branch's merge chain, so after the last few branch syncs there were two heads: `93d1b4a76c02` (master's real head) and `53a7abc31422` (this branch's prior merge tip)
that's exactly what `enforce-single-migration-head` reported and it's also what took down `docker-build (dev)` its sanity check hits the same `flask_migrate` "Multiple head revisions" error during `db upgrade`.
the rest of the failing jobs (cypress, playwright, mysql/postgres/sqlite) are downstream of the same startup failure
fixed by adding one more no-op merge migration joining the two heads
no changes to the actual permission delete/rename logic in this commit
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(security): delete deprecated permissions left over after upgrades [superset]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #44142:
URL: https://github.com/apache/superset/pull/44142#issuecomment-5763336263
# 🙅♂️ 🙅♂️ 🙅♂️ 🙅♂️ 🙅♂️ 🙅♂️ 🙅♂️ 🙅♂️ 🙅♂️ 🙅♂️ 🙅♂️ 🙅♂️ 🙅♂️ 🙅♂️ 🙅♂️ 🙅♂️❗ @Ujjwaljain16 Your base branch `master` has also updated `superset/migrations`.
**Please consider rebasing your branch and [resolving potential db migration conflicts](https://superset.apache.org/docs/contributing/development#merging-db-migrations).**
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(security): delete deprecated permissions left over after upgrades [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #44142:
URL: https://github.com/apache/superset/pull/44142#issuecomment-5709959076
AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
<details>
<summary>
Bito Usage Guide
</summary>
**Commands**
Type the following command in the pull request comment and save the comment.
- `/review` - Manually triggers an incremental AI Review.
- `/review full` - 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 <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/available-commands" target="_blank" rel="noopener noreferrer">documentation</a> for additional commands.
**Configuration**
This repository uses `Superset` You can customize the agent settings <a href="https://alpha.bito.ai/home/ai-agents/code-review-agent" target="_blank" rel="noopener noreferrer">here</a> or contact your Bito workspace admin at evan@preset.io.
**Documentation & Help**
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/getting-started/install-run-using-bito-cloud/create-or-customize-an-agent-instance" target="_blank" rel="noopener noreferrer">Customize agent settings</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/implementing-custom-code-review-rules" target="_blank" rel="noopener noreferrer">Review rules</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent" target="_blank" rel="noopener noreferrer">General documentation</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/faqs" target="_blank" rel="noopener noreferrer">FAQ</a>
</details>
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(security): delete deprecated permissions left over after upgrades [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #44142:
URL: https://github.com/apache/superset/pull/44142#issuecomment-5665003774
AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
<details>
<summary>
Bito Usage Guide
</summary>
**Commands**
Type the following command in the pull request comment and save the comment.
- `/review` - Manually triggers an incremental AI Review.
- `/review full` - 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 <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/available-commands" target="_blank" rel="noopener noreferrer">documentation</a> for additional commands.
**Configuration**
This repository uses `Superset` You can customize the agent settings <a href="https://alpha.bito.ai/home/ai-agents/code-review-agent" target="_blank" rel="noopener noreferrer">here</a> or contact your Bito workspace admin at evan@preset.io.
**Documentation & Help**
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/getting-started/install-run-using-bito-cloud/create-or-customize-an-agent-instance" target="_blank" rel="noopener noreferrer">Customize agent settings</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/implementing-custom-code-review-rules" target="_blank" rel="noopener noreferrer">Review rules</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent" target="_blank" rel="noopener noreferrer">General documentation</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/faqs" target="_blank" rel="noopener noreferrer">FAQ</a>
</details>
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(security): delete deprecated permissions left over after upgrades [superset]
Posted by "gabotorresruiz (via GitHub)" <gi...@apache.org>.
gabotorresruiz commented on code in PR #44142:
URL: https://github.com/apache/superset/pull/44142#discussion_r4030055127
##########
superset/migrations/versions/2026-09-10_00-01_3ce9a4572f8a_rename_deprecated_permissions_33272.py:
##########
@@ -0,0 +1,206 @@
+# 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.
+"""migrate roles off deprecated, renamed permissions (#33272)
+
+Companion to the previous migration (which deletes deprecated permissions
+confirmed to have no successor). This one handles the other half of
+apache/superset#33272: permissions that were deprecated but effectively
+renamed/consolidated onto a permission that already exists on current master,
+almost always as part of the API-ification of the old ``Superset`` monolithic
+view (``superset/views/core.py``) into per-resource ``ModelRestApi``s.
+
+``PVM_MAP`` started as an 11-entry list. Two follow-up investigation passes
+found: (1) 19 permissions originally believed to be pure deletions (in the
+delete migration's earlier draft) actually have a verified live successor and
+belong here instead; (2) the original ``can_get_or_create_table`` key was
+itself a phantom -- no method named ``get_or_create_table`` was ever
+generated by FAB, so it never matched any real historical permission and
+would have silently no-op'd, leaving its intended target
+(``can_sqllab_table_viz``, the real permission for ``def sqllab_table_viz`` /
+route ``/get_or_create_table/``) to be deleted with no migration; and (3) two
+more genuinely-real permissions (``can_annotation_json``,
+``can_fave_dashboards_by_username``) were missing from both lists entirely.
+All three are fixed below.
+
+Two entries rest on strong-but-not-decorator-level evidence rather than a
+formal ``@deprecated(new_target=...)`` marker: ``can_my_queries`` -> both
+``SavedQuery.can_list`` (the old endpoint's redirect target, gating the page
+shell) and ``SavedQuery.can_read`` (gating the ``/api/v1/saved_query/`` list
+call the page actually renders -- without it, a migrated role gets a page
+with no visible data) -- and
+``can_import_dashboards`` -> ``Dashboard.can_write`` (a same-commit
+functional replacement) -- flagged here for reviewer awareness, not hidden.
+
+Each mapping was verified against a genuine live ``superset init`` DB (not
+just ``UPDATING.md`` wording, which is wrong for the ``can_sql_json`` and
+``can_results`` rows -- it names a non-existent "can_execute"/"can_results"
+permission; the correct live successors, confirmed by querying
+``ab_permission``/``ab_permission_view``/``ab_view_menu``, are
+``can_execute_sql_query`` and ``can_get_results`` on the ``SQLLab`` view
+menu).
+
+This uses the existing, unmodified ``migrate_roles``/``add_pvms`` helpers:
+``add_pvms`` first guarantees every successor PVM exists so ``migrate_roles``
+can never resolve a target to ``None``, then ``migrate_roles`` moves any role
+holding an old PVM onto its successor(s) and deletes the old PVM (and its
+Permission/ViewMenu, if now orphaned). Chained after the deletion migration,
+though the two touch disjoint PVMs and their relative order does not matter.
+
+Revision ID: 3ce9a4572f8a
+Revises: 1f5f4fb8bfc1
+Create Date: 2026-09-10 00:01:00.000000
+
+"""
+
+# revision identifiers, used by Alembic.
+revision = "3ce9a4572f8a"
+down_revision = "1f5f4fb8bfc1"
+
+from alembic import op # noqa: E402
+from sqlalchemy.exc import SQLAlchemyError # noqa: E402
+from sqlalchemy.orm import Session # noqa: E402
+
+from superset.migrations.shared.security_converge import ( # noqa: E402
+ add_pvms,
+ migrate_roles,
+ Pvm,
+)
+
+OLD_VIEW_MENU = "Superset"
+
+# The live permissions these old ones are being folded into. Ensured present
+# before `migrate_roles` runs so lookups can't resolve to None (they normally
+# already exist -- FAB creates them on startup -- this just makes the
+# migration self-contained).
+NEW_PVMS = {
+ "Chart": ("can_read", "can_write"),
+ "Query": ("can_read",),
+ "Database": ("can_read", "can_write"),
+ "Datasource": ("can_get_column_values",),
+ "Log": ("can_recent_activity",),
+ "Dataset": ("can_get_or_create_dataset", "can_read", "can_write"),
+ "SQLLab": ("can_estimate_query_cost", "can_execute_sql_query", "can_get_results"),
+ "Dashboard": ("can_read", "can_write"),
+ "AvailableDomains": ("can_read",),
+ "SecurityRestApi": ("can_read",),
+ "SavedQuery": ("can_list", "can_read"),
+}
+
+# Map each deprecated (view, permission) to its live successor(s).
+# `migrate_roles` will, for every role holding an old PVM: add the successor
+# (if missing), remove the old PVM, then delete the old PVM row -- and the
+# underlying Permission/ViewMenu too, if they become orphans.
+PVM_MAP = {
+ # Unchanged from the original migration -- already correct.
+ Pvm(OLD_VIEW_MENU, "can_explore_json"): (Pvm("Chart", "can_read"),),
+ Pvm(OLD_VIEW_MENU, "can_slice_json"): (Pvm("Chart", "can_read"),),
+ Pvm(OLD_VIEW_MENU, "can_search_queries"): (Pvm("Query", "can_read"),),
+ Pvm(OLD_VIEW_MENU, "can_tables"): (Pvm("Database", "can_read"),),
+ Pvm(OLD_VIEW_MENU, "can_queries"): (Pvm("Query", "can_read"),),
+ Pvm(OLD_VIEW_MENU, "can_filter"): (Pvm("Datasource", "can_get_column_values"),),
+ Pvm(OLD_VIEW_MENU, "can_recent_activity"): (Pvm("Log", "can_recent_activity"),),
+ Pvm(OLD_VIEW_MENU, "can_stop_query"): (Pvm("Query", "can_read"),),
+ Pvm(OLD_VIEW_MENU, "can_estimate_query_cost"): (
+ Pvm("SQLLab", "can_estimate_query_cost"),
+ ),
+ Pvm(OLD_VIEW_MENU, "can_sql_json"): (Pvm("SQLLab", "can_execute_sql_query"),),
+ Pvm(OLD_VIEW_MENU, "can_results"): (Pvm("SQLLab", "can_get_results"),),
+ # Corrected: the old key here was "can_get_or_create_table", a permission
+ # name that never actually existed (no method named get_or_create_table
+ # was ever generated by FAB) -- the real historical permission for this
+ # capability was "can_sqllab_table_viz" (def sqllab_table_viz, route
+ # /get_or_create_table/, PR #22931/#24375).
+ Pvm(OLD_VIEW_MENU, "can_sqllab_table_viz"): (
+ Pvm("Dataset", "can_get_or_create_dataset"),
+ ),
+ # Reclassified from the delete migration: each of these was originally
+ # believed to have no successor, but actually has a verified live one
+ # (PR body / @deprecated(new_target=...) evidence -- see file docstring).
+ Pvm(OLD_VIEW_MENU, "can_select_star"): (Pvm("Database", "can_read"),),
+ Pvm(OLD_VIEW_MENU, "can_testconn"): (Pvm("Database", "can_write"),),
Review Comment:
Confirmed resolved, thanks. On `dc234b16` a role holding only `can_testconn` comes out of `superset db upgrade` holding nothing at all, and the same for `can_sqllab_viz` and `can_import_dashboards`, on both SQLite and PostgreSQL. Good catch pulling `can_add_slices` in as well.
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(security): delete deprecated permissions left over after upgrades [superset]
Posted by "rusackas (via GitHub)" <gi...@apache.org>.
rusackas commented on PR #44142:
URL: https://github.com/apache/superset/pull/44142#issuecomment-5647611016
@Ujjwaljain16 nice work tracking down the actual removed/renamed permissions here, the migration docstrings are thorough.
Pre-commit's failing for real though: `security_converge_test.py:202` passes a `dict[Pvm, tuple[Pvm]]` where `migrate_roles` expects `dict[Pvm, tuple[Pvm, ...]]`, dicts are invariant so mypy rejects it. Once that's fixed I think this is good to go. I'll see about making that touchup...
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(security): delete deprecated permissions left over after upgrades [superset]
Posted by "Ujjwaljain16 (via GitHub)" <gi...@apache.org>.
Ujjwaljain16 commented on PR #44142:
URL: https://github.com/apache/superset/pull/44142#issuecomment-5663551032
@rusackas
ci is green
thanks for the review
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(security): delete deprecated permissions left over after upgrades [superset]
Posted by "netlify[bot] (via GitHub)" <gi...@apache.org>.
netlify[bot] commented on PR #44142:
URL: https://github.com/apache/superset/pull/44142#issuecomment-5619260840
### <span aria-hidden="true">✅</span> Deploy Preview for *superset-docs-preview* ready!
| Name | Link |
|:-:|------------------------|
|<span aria-hidden="true">🔨</span> Latest commit | 119195e40df40274709f8642b9b8440147fe050d |
|<span aria-hidden="true">🔍</span> Latest deploy log | https://app.netlify.com/projects/superset-docs-preview/deploys/6aa2aca26194da000892d889 |
|<span aria-hidden="true">😎</span> Deploy Preview | [https://deploy-preview-44142--superset-docs-preview.netlify.app](https://deploy-preview-44142--superset-docs-preview.netlify.app) |
|<span aria-hidden="true">📱</span> Preview on mobile | <details><summary> Toggle QR Code... </summary><br /><br /><br /><br />_Use your smartphone camera to open QR code link._</details> |
|<span aria-hidden="true">🤖</span> Make changes | [Run an agent on this branch](https://app.netlify.com/projects/superset-docs-preview/agent-runs#DctBDoMgEEDRu8y6RDoYC97AY0yZMZIoTMAmNU3vLru_-O8HWsuhJ8wAD3hXynHrvabvwKJVIp3CRqUeqbVUcjNxF8ofNc7hC7vp216uhbuaiJAi4fQMI5O11gdk7wP8bw) |
---
<!-- [superset-docs-preview Preview](https://deploy-preview-44142--superset-docs-preview.netlify.app) -->
_To edit notification comments on pull requests, go to your [Netlify project configuration](https://app.netlify.com/projects/superset-docs-preview/configuration/notifications#deploy-notifications)._
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(security): delete deprecated permissions left over after upgrades [superset]
Posted by "gabotorresruiz (via GitHub)" <gi...@apache.org>.
gabotorresruiz commented on code in PR #44142:
URL: https://github.com/apache/superset/pull/44142#discussion_r4020495273
##########
superset/migrations/versions/2026-09-10_00-01_3ce9a4572f8a_rename_deprecated_permissions_33272.py:
##########
@@ -0,0 +1,206 @@
+# 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.
+"""migrate roles off deprecated, renamed permissions (#33272)
+
+Companion to the previous migration (which deletes deprecated permissions
+confirmed to have no successor). This one handles the other half of
+apache/superset#33272: permissions that were deprecated but effectively
+renamed/consolidated onto a permission that already exists on current master,
+almost always as part of the API-ification of the old ``Superset`` monolithic
+view (``superset/views/core.py``) into per-resource ``ModelRestApi``s.
+
+``PVM_MAP`` started as an 11-entry list. Two follow-up investigation passes
+found: (1) 19 permissions originally believed to be pure deletions (in the
+delete migration's earlier draft) actually have a verified live successor and
+belong here instead; (2) the original ``can_get_or_create_table`` key was
+itself a phantom -- no method named ``get_or_create_table`` was ever
+generated by FAB, so it never matched any real historical permission and
+would have silently no-op'd, leaving its intended target
+(``can_sqllab_table_viz``, the real permission for ``def sqllab_table_viz`` /
+route ``/get_or_create_table/``) to be deleted with no migration; and (3) two
+more genuinely-real permissions (``can_annotation_json``,
+``can_fave_dashboards_by_username``) were missing from both lists entirely.
+All three are fixed below.
+
+Two entries rest on strong-but-not-decorator-level evidence rather than a
+formal ``@deprecated(new_target=...)`` marker: ``can_my_queries`` -> both
Review Comment:
Just a small NIT: the docstrings here and in the delete migration narrate this PR's own draft evolution (the 25-entry earlier draft, the 11-entry original map). Since no migration with those lists ever shipped, a future reader may go hunting for one. Consider trimming to just the verified-evidence summary; the per-entry comments in `PVM_LIST`/`PVM_MAP` already carry the important context.
##########
superset/migrations/versions/2026-09-10_00-01_3ce9a4572f8a_rename_deprecated_permissions_33272.py:
##########
@@ -0,0 +1,206 @@
+# 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.
+"""migrate roles off deprecated, renamed permissions (#33272)
+
+Companion to the previous migration (which deletes deprecated permissions
+confirmed to have no successor). This one handles the other half of
+apache/superset#33272: permissions that were deprecated but effectively
+renamed/consolidated onto a permission that already exists on current master,
+almost always as part of the API-ification of the old ``Superset`` monolithic
+view (``superset/views/core.py``) into per-resource ``ModelRestApi``s.
+
+``PVM_MAP`` started as an 11-entry list. Two follow-up investigation passes
+found: (1) 19 permissions originally believed to be pure deletions (in the
+delete migration's earlier draft) actually have a verified live successor and
+belong here instead; (2) the original ``can_get_or_create_table`` key was
+itself a phantom -- no method named ``get_or_create_table`` was ever
+generated by FAB, so it never matched any real historical permission and
+would have silently no-op'd, leaving its intended target
+(``can_sqllab_table_viz``, the real permission for ``def sqllab_table_viz`` /
+route ``/get_or_create_table/``) to be deleted with no migration; and (3) two
+more genuinely-real permissions (``can_annotation_json``,
+``can_fave_dashboards_by_username``) were missing from both lists entirely.
+All three are fixed below.
+
+Two entries rest on strong-but-not-decorator-level evidence rather than a
+formal ``@deprecated(new_target=...)`` marker: ``can_my_queries`` -> both
+``SavedQuery.can_list`` (the old endpoint's redirect target, gating the page
+shell) and ``SavedQuery.can_read`` (gating the ``/api/v1/saved_query/`` list
+call the page actually renders -- without it, a migrated role gets a page
+with no visible data) -- and
+``can_import_dashboards`` -> ``Dashboard.can_write`` (a same-commit
+functional replacement) -- flagged here for reviewer awareness, not hidden.
+
+Each mapping was verified against a genuine live ``superset init`` DB (not
+just ``UPDATING.md`` wording, which is wrong for the ``can_sql_json`` and
+``can_results`` rows -- it names a non-existent "can_execute"/"can_results"
+permission; the correct live successors, confirmed by querying
+``ab_permission``/``ab_permission_view``/``ab_view_menu``, are
+``can_execute_sql_query`` and ``can_get_results`` on the ``SQLLab`` view
+menu).
+
+This uses the existing, unmodified ``migrate_roles``/``add_pvms`` helpers:
+``add_pvms`` first guarantees every successor PVM exists so ``migrate_roles``
+can never resolve a target to ``None``, then ``migrate_roles`` moves any role
+holding an old PVM onto its successor(s) and deletes the old PVM (and its
+Permission/ViewMenu, if now orphaned). Chained after the deletion migration,
+though the two touch disjoint PVMs and their relative order does not matter.
+
+Revision ID: 3ce9a4572f8a
+Revises: 1f5f4fb8bfc1
+Create Date: 2026-09-10 00:01:00.000000
+
+"""
+
+# revision identifiers, used by Alembic.
+revision = "3ce9a4572f8a"
+down_revision = "1f5f4fb8bfc1"
+
+from alembic import op # noqa: E402
+from sqlalchemy.exc import SQLAlchemyError # noqa: E402
+from sqlalchemy.orm import Session # noqa: E402
+
+from superset.migrations.shared.security_converge import ( # noqa: E402
+ add_pvms,
+ migrate_roles,
+ Pvm,
+)
+
+OLD_VIEW_MENU = "Superset"
+
+# The live permissions these old ones are being folded into. Ensured present
+# before `migrate_roles` runs so lookups can't resolve to None (they normally
+# already exist -- FAB creates them on startup -- this just makes the
+# migration self-contained).
+NEW_PVMS = {
+ "Chart": ("can_read", "can_write"),
+ "Query": ("can_read",),
+ "Database": ("can_read", "can_write"),
+ "Datasource": ("can_get_column_values",),
+ "Log": ("can_recent_activity",),
+ "Dataset": ("can_get_or_create_dataset", "can_read", "can_write"),
+ "SQLLab": ("can_estimate_query_cost", "can_execute_sql_query", "can_get_results"),
+ "Dashboard": ("can_read", "can_write"),
+ "AvailableDomains": ("can_read",),
+ "SecurityRestApi": ("can_read",),
+ "SavedQuery": ("can_list", "can_read"),
+}
+
+# Map each deprecated (view, permission) to its live successor(s).
+# `migrate_roles` will, for every role holding an old PVM: add the successor
+# (if missing), remove the old PVM, then delete the old PVM row -- and the
+# underlying Permission/ViewMenu too, if they become orphans.
+PVM_MAP = {
+ # Unchanged from the original migration -- already correct.
+ Pvm(OLD_VIEW_MENU, "can_explore_json"): (Pvm("Chart", "can_read"),),
+ Pvm(OLD_VIEW_MENU, "can_slice_json"): (Pvm("Chart", "can_read"),),
+ Pvm(OLD_VIEW_MENU, "can_search_queries"): (Pvm("Query", "can_read"),),
+ Pvm(OLD_VIEW_MENU, "can_tables"): (Pvm("Database", "can_read"),),
+ Pvm(OLD_VIEW_MENU, "can_queries"): (Pvm("Query", "can_read"),),
+ Pvm(OLD_VIEW_MENU, "can_filter"): (Pvm("Datasource", "can_get_column_values"),),
+ Pvm(OLD_VIEW_MENU, "can_recent_activity"): (Pvm("Log", "can_recent_activity"),),
+ Pvm(OLD_VIEW_MENU, "can_stop_query"): (Pvm("Query", "can_read"),),
+ Pvm(OLD_VIEW_MENU, "can_estimate_query_cost"): (
+ Pvm("SQLLab", "can_estimate_query_cost"),
+ ),
+ Pvm(OLD_VIEW_MENU, "can_sql_json"): (Pvm("SQLLab", "can_execute_sql_query"),),
+ Pvm(OLD_VIEW_MENU, "can_results"): (Pvm("SQLLab", "can_get_results"),),
+ # Corrected: the old key here was "can_get_or_create_table", a permission
+ # name that never actually existed (no method named get_or_create_table
+ # was ever generated by FAB) -- the real historical permission for this
+ # capability was "can_sqllab_table_viz" (def sqllab_table_viz, route
+ # /get_or_create_table/, PR #22931/#24375).
+ Pvm(OLD_VIEW_MENU, "can_sqllab_table_viz"): (
+ Pvm("Dataset", "can_get_or_create_dataset"),
+ ),
+ # Reclassified from the delete migration: each of these was originally
+ # believed to have no successor, but actually has a verified live one
+ # (PR body / @deprecated(new_target=...) evidence -- see file docstring).
+ Pvm(OLD_VIEW_MENU, "can_select_star"): (Pvm("Database", "can_read"),),
+ Pvm(OLD_VIEW_MENU, "can_testconn"): (Pvm("Database", "can_write"),),
Review Comment:
A security-awareness question, not a blocker: for the three write-level successors (`can_testconn` to `Database.can_write` here, plus `can_sqllab_viz` to `Dataset.can_write` and `can_import_dashboards` to `Dashboard.can_write` below), the migration grants a strictly broader capability than the role has actually had since 3.0. I verified it on this branch: a custom role holding only the dead `can_testconn` (no live capability at all today, the endpoint is gone) comes out of the migration holding `Database.can_write`, which covers creating, editing and deleting database connections, not just testing one. The `@deprecated(new_target="/api/v1/database/test_connection/")` decorator does sanction the mapping, and the read-level rows all look clearly right to me. Are we comfortable resurrecting years-dead grants as live write permissions, or would moving these three rows to the delete migration be the safer default? UPDATING.md already told operators to re-add the successors to custom ro
les for these endpoints, so deletion would not strand anyone who followed it.
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(security): delete deprecated permissions left over after upgrades [superset]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #44142:
URL: https://github.com/apache/superset/pull/44142#issuecomment-5719102148
# 🙅♂️ 🙅♂️ 🙅♂️ 🙅♂️ 🙅♂️ 🙅♂️ 🙅♂️ 🙅♂️ 🙅♂️ 🙅♂️ 🙅♂️ 🙅♂️ 🙅♂️ 🙅♂️ 🙅♂️ 🙅♂️❗ @Ujjwaljain16 Your base branch `master` has also updated `superset/migrations`.
**Please consider rebasing your branch and [resolving potential db migration conflicts](https://superset.apache.org/docs/contributing/development#merging-db-migrations).**
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(security): delete deprecated permissions left over after upgrades [superset]
Posted by "Ujjwaljain16 (via GitHub)" <gi...@apache.org>.
Ujjwaljain16 commented on PR #44142:
URL: https://github.com/apache/superset/pull/44142#issuecomment-5679974441
@rebenitez1802 really appreciate the deep dive here both of your Medium findings checked out, and pulling that thread turned out to matter a lot more than i expected
`can_sqllab_table_viz`/`can_get_or_create_table` was exactly as you described `can_get_or_create_table` was never a real permission turns out `UPDATING.md` itself has the wrong name, which is probably where it got picked up from. Moved it into the rename migration under the right key
`can_test_conn` needed more than the spelling fix, though
once i went and looked at the actual PR that removed `testconn()` (#24354), it had a real successor all along `Database.can_write` so instead of just renaming the string i moved it out of the delete list entirely
the bigger surprise was yr "incomplete cleanup" note i went and actually checked PR #24400's diff for the six permissions you called out as a minor doc nit, and every single one of them has a real `@deprecated(new_target=...)` pointing at a live endpoint that's not a doc problem, that's the same silent-drop bug you found just hiding in six more places so i stopped trusting the original "no successor" labels entirely and re-checked all 25 the same way actual PR diffs, actual deprecation decorators, tracing each one to a real live endpoint
19 of the 25 had a real successor the delete list is down to 6 now the rename list is up to 34
also added `can_annotation_json` and `can_fave_dashboards_by_username`, which were genuinely missing from both lists didn't add `can_theme` though checked its whole history and it never had any access decorator on it, so it was never a real permission to begin with, would've just been a third phantom entry.
fixed the `SECURITY.md` line, and added tests for the disjointness check, for roles keeping their other permissions, and for running both migrations back to back
one more thing came up on a final pass i did over the shakier mappings before merging: `can_my_queries` needed to go to *both* `SavedQuery.can_list` and `SavedQuery.can_read`, not just `can_list` that permission alone only unlocks the page itself, the actual query list comes from a separately-gated API call would've left people staring at a blank page otherwise
two mappings (`can_my_queries` and `can_import_dashboards`) don't have a formal deprecation decorator backing them like the rest do have called that out directly in the migration's docstring rather than papering over it
tests went from 154 to 157
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(security): delete deprecated permissions left over after upgrades [superset]
Posted by "Ujjwaljain16 (via GitHub)" <gi...@apache.org>.
Ujjwaljain16 commented on PR #44142:
URL: https://github.com/apache/superset/pull/44142#issuecomment-5693830204
@gabotorresruiz thanks for both passes on this — the head-conflict catch and the write-permission question in particular were exactly the kind of thing worth slowing down for. Here's everything that's landed since:
**The Alembic head conflict** — fixed with a merge migration reconciling all three heads, like you suggested. Then master moved again and brought in its own fix for the same underlying split (a different merge migration resolving the same two heads), which made mine redundant and would've quietly created a new two-way split underneath it. Caught that by actually computing the head graph from all the migration files rather than trusting `git status`, dropped my old merge migration, and added a smaller one reconciling just master's current head with ours. Single head confirmed end to end with a real `superset db upgrade` run each time, not just eyeballing the chain.
On the write-permission question — you were right, and chasing it further turned up more of the same. `can_testconn`, `can_sqllab_viz`, and `can_import_dashboards` all move to the delete migration now instead of resurrecting a broader live permission, exactly as you flagged. While I was in there I found a fourth one you hadn't mentioned, `can_add_slices` → `Chart.can_write`, with the same problem plus what looks like a wrong-resource mapping on top (the old action mutated a dashboard's chart list, not chart records themselves) — moved that too. `can_copy_dash` is the one I kept as a rename, since Superset's actual `copy_dash` endpoint today is still gated by that exact permission — a real like-for-like match rather than accidental broadening, not just the same pattern as the others.
Also trimmed both migration docstrings per your nit — they don't narrate the earlier 25-entry/11-entry draft history anymore, just the final evidence. And added the `UPDATING.md` entry you suggested, explaining the delete-vs-rename split for anyone with custom roles.
One more thing came out of an automated review pass afterward, which I checked by hand before touching anything: two real bugs in the tests from the previous round. The `PVM_LIST`/`PVM_MAP` disjointness check was comparing against a dict key (`"Superset"`) that never actually exists in `NEW_PVMS`, so it passed no matter what was in the list — fixed to flatten every successor permission across all views and check against that instead. And the idempotency test's "is it really gone" check only matched on permission name, which would've spuriously failed if the dict's first entry ever happened to be one of the few same-name-different-view pairs already in the map (`can_recent_activity` on Superset vs. `Log`, for instance) — now it filters on both. Neither touched any production migration logic.
Tests are at 161 passing, ruff's clean, and I re-ran the full chain fresh after every change rather than assuming it still worked.
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(security): delete deprecated permissions left over after upgrades [superset]
Posted by "Ujjwaljain16 (via GitHub)" <gi...@apache.org>.
Ujjwaljain16 commented on PR #44142:
URL: https://github.com/apache/superset/pull/44142#issuecomment-5758904312
@sadpandajoe no conflicts
CI is also green
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(security): delete deprecated permissions left over after upgrades [superset]
Posted by "rusackas (via GitHub)" <gi...@apache.org>.
rusackas commented on code in PR #44142:
URL: https://github.com/apache/superset/pull/44142#discussion_r4077763665
##########
superset/migrations/versions/2026-09-10_00-01_3ce9a4572f8a_rename_deprecated_permissions_33272.py:
##########
@@ -0,0 +1,206 @@
+# 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.
+"""migrate roles off deprecated, renamed permissions (#33272)
+
+Companion to the previous migration (which deletes deprecated permissions
+confirmed to have no successor). This one handles the other half of
+apache/superset#33272: permissions that were deprecated but effectively
+renamed/consolidated onto a permission that already exists on current master,
+almost always as part of the API-ification of the old ``Superset`` monolithic
+view (``superset/views/core.py``) into per-resource ``ModelRestApi``s.
+
+``PVM_MAP`` started as an 11-entry list. Two follow-up investigation passes
+found: (1) 19 permissions originally believed to be pure deletions (in the
+delete migration's earlier draft) actually have a verified live successor and
+belong here instead; (2) the original ``can_get_or_create_table`` key was
+itself a phantom -- no method named ``get_or_create_table`` was ever
+generated by FAB, so it never matched any real historical permission and
+would have silently no-op'd, leaving its intended target
+(``can_sqllab_table_viz``, the real permission for ``def sqllab_table_viz`` /
+route ``/get_or_create_table/``) to be deleted with no migration; and (3) two
+more genuinely-real permissions (``can_annotation_json``,
+``can_fave_dashboards_by_username``) were missing from both lists entirely.
+All three are fixed below.
+
+Two entries rest on strong-but-not-decorator-level evidence rather than a
+formal ``@deprecated(new_target=...)`` marker: ``can_my_queries`` -> both
+``SavedQuery.can_list`` (the old endpoint's redirect target, gating the page
+shell) and ``SavedQuery.can_read`` (gating the ``/api/v1/saved_query/`` list
+call the page actually renders -- without it, a migrated role gets a page
+with no visible data) -- and
+``can_import_dashboards`` -> ``Dashboard.can_write`` (a same-commit
+functional replacement) -- flagged here for reviewer awareness, not hidden.
+
+Each mapping was verified against a genuine live ``superset init`` DB (not
+just ``UPDATING.md`` wording, which is wrong for the ``can_sql_json`` and
+``can_results`` rows -- it names a non-existent "can_execute"/"can_results"
+permission; the correct live successors, confirmed by querying
+``ab_permission``/``ab_permission_view``/``ab_view_menu``, are
+``can_execute_sql_query`` and ``can_get_results`` on the ``SQLLab`` view
+menu).
+
+This uses the existing, unmodified ``migrate_roles``/``add_pvms`` helpers:
+``add_pvms`` first guarantees every successor PVM exists so ``migrate_roles``
+can never resolve a target to ``None``, then ``migrate_roles`` moves any role
+holding an old PVM onto its successor(s) and deletes the old PVM (and its
+Permission/ViewMenu, if now orphaned). Chained after the deletion migration,
+though the two touch disjoint PVMs and their relative order does not matter.
+
+Revision ID: 3ce9a4572f8a
+Revises: 1f5f4fb8bfc1
+Create Date: 2026-09-10 00:01:00.000000
+
+"""
+
+# revision identifiers, used by Alembic.
+revision = "3ce9a4572f8a"
+down_revision = "1f5f4fb8bfc1"
+
+from alembic import op # noqa: E402
+from sqlalchemy.exc import SQLAlchemyError # noqa: E402
+from sqlalchemy.orm import Session # noqa: E402
+
+from superset.migrations.shared.security_converge import ( # noqa: E402
+ add_pvms,
+ migrate_roles,
+ Pvm,
+)
+
+OLD_VIEW_MENU = "Superset"
+
+# The live permissions these old ones are being folded into. Ensured present
+# before `migrate_roles` runs so lookups can't resolve to None (they normally
+# already exist -- FAB creates them on startup -- this just makes the
+# migration self-contained).
+NEW_PVMS = {
+ "Chart": ("can_read", "can_write"),
+ "Query": ("can_read",),
+ "Database": ("can_read", "can_write"),
+ "Datasource": ("can_get_column_values",),
+ "Log": ("can_recent_activity",),
+ "Dataset": ("can_get_or_create_dataset", "can_read", "can_write"),
+ "SQLLab": ("can_estimate_query_cost", "can_execute_sql_query", "can_get_results"),
+ "Dashboard": ("can_read", "can_write"),
+ "AvailableDomains": ("can_read",),
+ "SecurityRestApi": ("can_read",),
+ "SavedQuery": ("can_list", "can_read"),
+}
+
+# Map each deprecated (view, permission) to its live successor(s).
+# `migrate_roles` will, for every role holding an old PVM: add the successor
+# (if missing), remove the old PVM, then delete the old PVM row -- and the
+# underlying Permission/ViewMenu too, if they become orphans.
+PVM_MAP = {
+ # Unchanged from the original migration -- already correct.
+ Pvm(OLD_VIEW_MENU, "can_explore_json"): (Pvm("Chart", "can_read"),),
+ Pvm(OLD_VIEW_MENU, "can_slice_json"): (Pvm("Chart", "can_read"),),
+ Pvm(OLD_VIEW_MENU, "can_search_queries"): (Pvm("Query", "can_read"),),
+ Pvm(OLD_VIEW_MENU, "can_tables"): (Pvm("Database", "can_read"),),
+ Pvm(OLD_VIEW_MENU, "can_queries"): (Pvm("Query", "can_read"),),
+ Pvm(OLD_VIEW_MENU, "can_filter"): (Pvm("Datasource", "can_get_column_values"),),
+ Pvm(OLD_VIEW_MENU, "can_recent_activity"): (Pvm("Log", "can_recent_activity"),),
+ Pvm(OLD_VIEW_MENU, "can_stop_query"): (Pvm("Query", "can_read"),),
+ Pvm(OLD_VIEW_MENU, "can_estimate_query_cost"): (
+ Pvm("SQLLab", "can_estimate_query_cost"),
+ ),
+ Pvm(OLD_VIEW_MENU, "can_sql_json"): (Pvm("SQLLab", "can_execute_sql_query"),),
+ Pvm(OLD_VIEW_MENU, "can_results"): (Pvm("SQLLab", "can_get_results"),),
+ # Corrected: the old key here was "can_get_or_create_table", a permission
+ # name that never actually existed (no method named get_or_create_table
+ # was ever generated by FAB) -- the real historical permission for this
+ # capability was "can_sqllab_table_viz" (def sqllab_table_viz, route
+ # /get_or_create_table/, PR #22931/#24375).
+ Pvm(OLD_VIEW_MENU, "can_sqllab_table_viz"): (
+ Pvm("Dataset", "can_get_or_create_dataset"),
+ ),
+ # Reclassified from the delete migration: each of these was originally
+ # believed to have no successor, but actually has a verified live one
+ # (PR body / @deprecated(new_target=...) evidence -- see file docstring).
+ Pvm(OLD_VIEW_MENU, "can_select_star"): (Pvm("Database", "can_read"),),
+ Pvm(OLD_VIEW_MENU, "can_testconn"): (Pvm("Database", "can_write"),),
Review Comment:
Confirmed on `77e39422`, `can_testconn`/`can_sqllab_viz`/`can_import_dashboards`/`can_add_slices` are all gone from `PVM_MAP` now, `can_copy_dash` is the only write-level rename left and it's got a comment explaining why. Resolving.
##########
superset/migrations/versions/2026-09-10_00-01_3ce9a4572f8a_rename_deprecated_permissions_33272.py:
##########
@@ -0,0 +1,206 @@
+# 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.
+"""migrate roles off deprecated, renamed permissions (#33272)
+
+Companion to the previous migration (which deletes deprecated permissions
+confirmed to have no successor). This one handles the other half of
+apache/superset#33272: permissions that were deprecated but effectively
+renamed/consolidated onto a permission that already exists on current master,
+almost always as part of the API-ification of the old ``Superset`` monolithic
+view (``superset/views/core.py``) into per-resource ``ModelRestApi``s.
+
+``PVM_MAP`` started as an 11-entry list. Two follow-up investigation passes
+found: (1) 19 permissions originally believed to be pure deletions (in the
+delete migration's earlier draft) actually have a verified live successor and
+belong here instead; (2) the original ``can_get_or_create_table`` key was
+itself a phantom -- no method named ``get_or_create_table`` was ever
+generated by FAB, so it never matched any real historical permission and
+would have silently no-op'd, leaving its intended target
+(``can_sqllab_table_viz``, the real permission for ``def sqllab_table_viz`` /
+route ``/get_or_create_table/``) to be deleted with no migration; and (3) two
+more genuinely-real permissions (``can_annotation_json``,
+``can_fave_dashboards_by_username``) were missing from both lists entirely.
+All three are fixed below.
+
+Two entries rest on strong-but-not-decorator-level evidence rather than a
+formal ``@deprecated(new_target=...)`` marker: ``can_my_queries`` -> both
Review Comment:
Confirmed fixed, the draft-evolution narration is gone from both docstrings now. Resolving.
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(security): delete deprecated permissions left over after upgrades [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #44142:
URL: https://github.com/apache/superset/pull/44142#issuecomment-5619235174
AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(security): delete deprecated permissions left over after upgrades [superset]
Posted by "codecov[bot] (via GitHub)" <gi...@apache.org>.
codecov[bot] commented on PR #44142:
URL: https://github.com/apache/superset/pull/44142#issuecomment-5619407132
## [Codecov](https://app.codecov.io/gh/apache/superset/pull/44142?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
:x: Patch coverage is `12.50000%` with `14 lines` in your changes missing coverage. Please review.
:white_check_mark: Project coverage is 57.44%. Comparing base ([`e5c2a3e`](https://app.codecov.io/gh/apache/superset/commit/e5c2a3e4f68291a9bac490cee0829bf5cea76ed3?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`7e7e756`](https://app.codecov.io/gh/apache/superset/commit/7e7e7564b1450079adde4b03719f13926b1694ec?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
| [Files with missing lines](https://app.codecov.io/gh/apache/superset/pull/44142?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Patch % | Lines |
|---|---|---|
| [superset/migrations/shared/security\_converge.py](https://app.codecov.io/gh/apache/superset/pull/44142?src=pr&el=tree&filepath=superset%2Fmigrations%2Fshared%2Fsecurity_converge.py&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3VwZXJzZXQvbWlncmF0aW9ucy9zaGFyZWQvc2VjdXJpdHlfY29udmVyZ2UucHk=) | 12.50% | [14 Missing :warning: ](https://app.codecov.io/gh/apache/superset/pull/44142?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
> :exclamation: There is a different number of reports uploaded between BASE (e5c2a3e) and HEAD (7e7e756). Click for more details.
>
> <details><summary>HEAD has 21 uploads less than BASE</summary>
>
>| Flag | BASE (e5c2a3e) | HEAD (7e7e756) |
>|------|------|------|
>|python|19|1|
>|presto|4|1|
></details>
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## master #44142 +/- ##
===========================================
- Coverage 80.14% 57.44% -22.71%
===========================================
Files 2924 2924
Lines 172370 172386 +16
Branches 40018 40024 +6
===========================================
- Hits 138147 99021 -39126
- Misses 31637 72434 +40797
+ Partials 2586 931 -1655
```
| [Flag](https://app.codecov.io/gh/apache/superset/pull/44142/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [hive](https://app.codecov.io/gh/apache/superset/pull/44142/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [mysql](https://app.codecov.io/gh/apache/superset/pull/44142/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [postgres](https://app.codecov.io/gh/apache/superset/pull/44142/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [presto](https://app.codecov.io/gh/apache/superset/pull/44142/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `39.23% <12.50%> (-0.01%)` | :arrow_down: |
| [python](https://app.codecov.io/gh/apache/superset/pull/44142/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `39.23% <12.50%> (-45.28%)` | :arrow_down: |
| [sqlite](https://app.codecov.io/gh/apache/superset/pull/44142/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [unit](https://app.codecov.io/gh/apache/superset/pull/44142/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#carryforward-flags-in-the-pull-request-comment) to find out more.
</details>
[:umbrella: View full report in Codecov by Harness](https://app.codecov.io/gh/apache/superset/pull/44142?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
<details><summary> :rocket: New features to boost your workflow: </summary>
- :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
- :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
</details>
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(security): delete deprecated permissions left over after upgrades [superset]
Posted by "rusackas (via GitHub)" <gi...@apache.org>.
rusackas merged PR #44142:
URL: https://github.com/apache/superset/pull/44142
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(security): delete deprecated permissions left over after upgrades [superset]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #44142:
URL: https://github.com/apache/superset/pull/44142#issuecomment-5704375990
# 🙅♂️ 🙅♂️ 🙅♂️ 🙅♂️ 🙅♂️ 🙅♂️ 🙅♂️ 🙅♂️ 🙅♂️ 🙅♂️ 🙅♂️ 🙅♂️ 🙅♂️ 🙅♂️ 🙅♂️ 🙅♂️❗ @Ujjwaljain16 Your base branch `master` has also updated `superset/migrations`.
**Please consider rebasing your branch and [resolving potential db migration conflicts](https://superset.apache.org/docs/contributing/development#merging-db-migrations).**
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(security): delete deprecated permissions left over after upgrades [superset]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #44142:
URL: https://github.com/apache/superset/pull/44142#issuecomment-5706356840
# 🙅♂️ 🙅♂️ 🙅♂️ 🙅♂️ 🙅♂️ 🙅♂️ 🙅♂️ 🙅♂️ 🙅♂️ 🙅♂️ 🙅♂️ 🙅♂️ 🙅♂️ 🙅♂️ 🙅♂️ 🙅♂️❗ @Ujjwaljain16 Your base branch `master` has also updated `superset/migrations`.
**Please consider rebasing your branch and [resolving potential db migration conflicts](https://superset.apache.org/docs/contributing/development#merging-db-migrations).**
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(security): delete deprecated permissions left over after upgrades [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #44142:
URL: https://github.com/apache/superset/pull/44142#issuecomment-5682609152
AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
<details>
<summary>
Bito Usage Guide
</summary>
**Commands**
Type the following command in the pull request comment and save the comment.
- `/review` - Manually triggers an incremental AI Review.
- `/review full` - 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 <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/available-commands" target="_blank" rel="noopener noreferrer">documentation</a> for additional commands.
**Configuration**
This repository uses `Superset` You can customize the agent settings <a href="https://alpha.bito.ai/home/ai-agents/code-review-agent" target="_blank" rel="noopener noreferrer">here</a> or contact your Bito workspace admin at evan@preset.io.
**Documentation & Help**
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/getting-started/install-run-using-bito-cloud/create-or-customize-an-agent-instance" target="_blank" rel="noopener noreferrer">Customize agent settings</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/implementing-custom-code-review-rules" target="_blank" rel="noopener noreferrer">Review rules</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent" target="_blank" rel="noopener noreferrer">General documentation</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/faqs" target="_blank" rel="noopener noreferrer">FAQ</a>
</details>
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org