[aminghadersohi]

SUMMARY

The permission event hooks in superset/security/manager.py rewrite perm, schema_perm and catalog_perm of datasets, semantic views and charts with Core UPDATEs on the flush connection. A datasource instance that is already loaded in the session keeps the previous strings. set_related_perm (the chart before_update hook) copies a chart's perm strings from that instance. So a chart written later in the same transaction writes the previous strings back over the Core values.

A log probe in set_related_perm during PUT /api/v1/database/<id> showed this. The dataset was already loaded in the session, and the chart was flushed when the database permission sync set its schema_perm. At that point the dataset instance held perm='[old_db].[t1](id:1)', the dataset row held [new_db].[t1](id:1), and the chart already held the new perm before the hook overwrote it. Charts of datasets without a schema are not touched by that sync, so they kept the right perm. A viewer whose access comes from the dataset grant then gets 404 on GET /api/v1/chart/<id>/data/ until the chart is saved again.

Where it happens:

• Database rename (database_after_update): only when the datasets are already loaded in the session before the rename, e.g. a long-lived app context or a script. A rename from a fresh request session is not affected; test_rename_database_keeps_chart_perms_in_sync passes before and after this change.
• Dataset table rename, schema change or database change (dataset_before_update): the dataset being flushed is always loaded. This is reachable through asset import: re-importing a bundle whose dataset has a new table name or schema leaves its chart with the old perm and no schema perm (test_import_assets_dataset_change_keeps_chart_perms).
• Semantic layer rename (semantic_layer_before_update): the same pattern for loaded semantic views.

The fix (commit 8990688, pinned by test_rename_database_with_loaded_datasets_keeps_chart_perms_in_sync) records the values the Core UPDATE wrote on the loaded instances with set_committed_value; after a database rename it reads back only the datasets loaded in the session. Attributes that are not loaded, or that carry a pending change, are left alone. _update_vm_datasources_access is not changed: the database rename path reads the rewritten perms back from the table after it runs.

Not changed here: a database renamed through asset import keeps the old database name in the schema_perm of its datasets and charts, because import does not run the schema permission sync. That is a separate gap, not this stale-instance problem.

Related: #45864 changes _update_vm_datasources_access; this PR does not touch those lines.

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

Not applicable (backend only).

TESTING INSTRUCTIONS

New tests, each failing with superset/security/manager.py as on master and passing with commit 8990688:

• tests/integration_tests/security/perm_sync_after_rename_tests.py::TestDatabaseRenamePermissions::test_rename_database_with_loaded_datasets_keeps_chart_perms_in_sync: real PUT /api/v1/database/<id> with a dataset with a schema and one without; asserts every chart's perm/schema_perm uses the new name and that a viewer with only the dataset grant gets 200 on GET /api/v1/chart/<id>/data/.
• TestDatasetChangePermissions::test_table_rename_then_chart_write, test_schema_change_then_chart_write, test_database_change_then_chart_write (same file).
• tests/integration_tests/commands_test.py::TestImportAssetsCommand::test_import_assets_dataset_change_keeps_chart_perms.
• tests/unit_tests/semantic_layers/models_test.py::test_semantic_layer_rename_then_chart_write_keeps_slice_perm.

TestDatabaseRenamePermissions::test_rename_database_keeps_chart_perms_in_sync covers the same PUT from a fresh session and passes before and after.

pytest tests/integration_tests/security/perm_sync_after_rename_tests.py \
  tests/integration_tests/commands_test.py::TestImportAssetsCommand
pytest tests/unit_tests/semantic_layers/models_test.py -k rename

ADDITIONAL INFORMATION

• Has associated issue:
• Required feature flags:
• Changes UI
• Includes DB Migration (follow approval process in SIP-59)
  • Migration is atomic, supports rollback & is backwards-compatible
  • Confirm DB migration upgrade and downgrade tested
  • Runtime estimates and downtime expectations provided
• Introduces new feature or API
• Removes existing feature or API

[codecov]

Codecov Report

❌ Patch coverage is 85.18519% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 84.71%. Comparing base (4e6d03b) to head (8990688).

Files with missing lines	Patch %	Lines
superset/security/manager.py	85.18%	1 Missing and 3 partials ⚠️

Additional details and impacted files

@@             Coverage Diff             @@
##           master   #45874       +/-   ##
===========================================
+ Coverage   65.62%   84.71%   +19.09%     
===========================================
  Files        3069     3069               
  Lines      205661   205687       +26     
  Branches    48336    48343        +7     
===========================================
+ Hits       134955   174240    +39285     
+ Misses      67625    28151    -39474     
- Partials     3081     3296      +215     

Flag	Coverage Δ
hive	34.50% <18.51%> (-0.01%)	⬇️
mysql	52.60% <77.77%> (+0.01%)	⬆️
postgres	52.62% <77.77%> (+0.02%)	⬆️
presto	36.23% <18.51%> (-0.01%)	⬇️
python	87.75% <85.18%> (+34.89%)	⬆️
sqlite	52.37% <77.77%> (+0.01%)	⬆️
unit	82.48% <85.18%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[aminghadersohi]

Acceptance run at 8990688: real Superset app in-process, a real SQLite metadata DB, real requests through the Flask test client. The same script ran at the merge-base (4e6d03b) and at head.

• PUT /api/v1/database/<id> rename after the datasets were loaded in the same session: at the merge-base, 2 of 4 charts (the dataset with a schema) keep the old perm, and a viewer with only the dataset grant gets 404 on 2 of 4 GET /api/v1/chart/<id>/data/ calls. At head all 4 charts have the new perm/schema_perm and all 4 calls return 200. A user without the grant still gets 404 on all 4.
• /api/v1/assets/import/ of a bundle whose dataset has a new table name and schema: at the merge-base the charts keep the old perm/schema_perm. At head they match the dataset.
• Rename from a fresh session, and PUT /api/v1/dataset/<id> table rename: same result at both commits (same permission names, 241 -> 241 permission-view rows, no duplicates, no new warnings).
• Semantic layer: tests/unit_tests/semantic_layers/models_test.py and tests/unit_tests/security/ pass at head (871 tests). The new rename test fails at the merge-base.

Pre-existing and not changed by this PR (the body already says so): an asset import that renames a database leaves the old database name in schema_perm of its datasets and charts. A role granted schema_access on the new name gets 404 on those charts, at both commits.

[codeant-ai-for-open-source]

  CodeAnt PR Risk

🟢  Low Risk

• The PR appears safe to merge; no actionable issues were identified in the review.

Assessed commit: 8990688ef63f

[rebenitez1802]

Approve: real stale-perm bug, minimal and well-targeted fix, every new call is pinned by a test that fails on master and passes here; no blocking issues.