EnxDev opened a new pull request, #45342: URL: https://github.com/apache/superset/pull/45342
### SUMMARY Follow-up to #45266, covering the review comments from @sadpandajoe and me there ([approval note](https://github.com/apache/superset/pull/45266#pullrequestreview-5533494799)). **This branch is stacked on #45266.** Until that merges the diff includes its commits. The change for this PR is only the top commit (59e7b40). #45266 stops editorship from authorizing the strict raw-SQL paths (SQL Lab, MetaDB, MCP, alerts/reports). Three non-strict callers still accepted editorship of a dataset named after a referenced table. That kept the decoy trick open: create a `SELECT 1` virtual dataset named `public.customer_pii`, become its editor, then read the real table without `datasource_access`: 1. `CreateDatasetCommand.validate`: a second virtual dataset with `SELECT * FROM public.customer_pii` passes. 2. `UpdateDatasetCommand._validate_sql_access`: the decoy's own SQL can be updated to read the real table. 3. `QueryContextProcessor.raise_for_access` for a `query` datasource: SQL denied in SQL Lab still leaves a persisted `Query` row, and chart-data on it ran the stored SQL. **Fix:** add `allow_dataset_editorship` (default `True`) to `raise_for_access` and pass `False` from those three call sites. I didn't reuse `force_dataset_match=True` as suggested in review because it does more than drop editorship. It also skips the `catalog_access`/`schema_access` fallthroughs, requires a schema on every table, and refuses unparseable or schema-changing statements. On dataset create/update, that would block users who create virtual datasets today through a schema grant. The new flag only removes the editorship accept and leaves the rest alone. Unchanged: physical dataset checks (`database=..., table=...`), `/table_metadata/`, `/select_star/`, and the `is_editor` accept on the datasource branch (chart-data and samples on a dataset you edit, #20382). Also fixes the nit that the comment next to the editorship check didn't list alert/report queries the way the docstring does. I left out an UPDATING.md entry, per the discussion on #45266 about holding those notes until release. ### TESTING INSTRUCTIONS Unit tests: ``` pytest tests/unit_tests/security/manager_test.py \ tests/unit_tests/common/test_query_context_processor.py \ tests/unit_tests/commands/dataset/ ``` New tests: - `test_raise_for_access_editorship_opt_out_keeps_schema_access`: editorship is denied with the flag, and `schema_access` still authorizes. - One test per call site, asserting each one passes `allow_dataset_editorship=False`. All four fail on the base branch and pass here. Manual check: as a role with dataset write and no access to `public.customer_pii`: 1. Create a virtual dataset named `customer_pii` in schema `public` with `SELECT 1`. 2. Create another virtual dataset with `SELECT * FROM public.customer_pii`. It should be rejected with a data-access error. 3. Update the decoy's SQL to `SELECT * FROM public.customer_pii`. It should be rejected too. ### ADDITIONAL INFORMATION - [ ] Has associated issue: - [ ] Required feature flags: - [ ] Changes UI - [ ] Includes DB Migration (follow approval process in [SIP-59](https://github.com/apache/superset/issues/13351)) - [ ] 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 🤖 Generated with [Claude Code](https://claude.com/claude-code) -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
