EnxDev opened a new pull request, #44650: URL: https://github.com/apache/superset/pull/44650
### SUMMARY Follow-up to #44645, from @msyavuz's review there. **Stacked on #44645**: until that merges, this diff also shows its three commits, so only d164260af6 is new here. When a guest queries a virtual dataset, global guest RLS rules (no `dataset` key) are left out of the inner SQL and applied once, on the outer query (#37395, for #37359). The outer filter only narrows the rows the inner SQL returns, though. A table read inside a sub-query of the virtual SQL isn't constrained by it, so in ```sql SELECT a.*, (SELECT count(*) FROM b) AS n FROM a ``` with a global `org_id = 1`, `n` counted every org's rows in `b`. The fix separates the two kinds of table read: - `SQLStatement.apply_rls` takes an optional `subquery_predicates` set. It's used for reads inside a sqlglot `SUBQUERY` scope (scalar, `IN`, `EXISTS`), for any scope nested inside one, and for CTEs such a scope reads from (plus their nested scopes). Every other read keeps using `predicates`. A `LATERAL` / `CROSS APPLY` body is also typed as a `SUBQUERY` scope, but its rows reach the output like a join's, so it's left out. - On the virtual dataset path, `utils.apply_rls` leaves global guest rules out of `predicates` as before, but includes them in `subquery_predicates` when the statement has a sub-query. `FROM`, join, derived table, CTE and `LATERAL` reads still don't get the rule twice. - `SQLStatement.apply_rls` returns whether it actually injected a rule, and `utils.apply_rls` returns that. This way an untouched statement isn't re-rendered through sqlglot, which is the round trip the `rls_applied` guard exists to avoid. - The fail-closed check in `get_from_clause`, which runs when injecting RLS raised, counts global guest rules for statements with a sub-query, matching what `apply_rls` would have injected. Scope decision: joins are left to the outer filter as before. Applying global rules to joined tables would also close the join case, but it would break every virtual dataset that joins a registered lookup table without the rule's column. This PR only covers sub-queries, whose rows never reach the outer filter at all. ### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF N/A, backend only. ### TESTING INSTRUCTIONS ``` pytest tests/unit_tests/sql/parse_tests.py tests/unit_tests/security/guest_rls_test.py tests/unit_tests/models/test_virtual_dataset_format.py tests/unit_tests/models/test_double_rls_virtual_dataset.py tests/unit_tests/utils/rls_test.py tests/unit_tests/sql_lab_test.py ``` - `test_rls_subquery_predicates` (parse tests) covers which reads get which predicates. Cases: plain read, join, derived table, CTE in `FROM`, scalar / `IN` / `EXISTS` sub-queries, the same table both in `FROM` and in a sub-query, a derived table inside a sub-query, and a CTE read only from a sub-query. There's also an `AS_SUBQUERY` variant, a `LATERAL` / `CROSS APPLY` case, and a test for the new return value. - `test_global_guest_rule_applied_to_virtual_dataset_subquery` runs the example above through `apply_rls` the way `get_from_clause` does. It fails on #44645's head and passes here. `test_global_guest_rule_left_to_outer_query_without_subquery` checks that the join case keeps its single application. - `test_get_from_clause_fail_closed_counts_global_guest_rls_in_subqueries` covers the fail-closed check. Manual check: with `EMBEDDED_SUPERSET` on, create a virtual dataset like the one above, embed a chart on it, and request its data with a guest token carrying a clause-only rule. The generated SQL (`result_type: "query"`) has the clause inside the scalar sub-query, and still only once for `a`, on the outer query. ### ADDITIONAL INFORMATION - [x] Has associated issue: follow-up to #44645 (review comment), related to #37359 - [x] Required feature flags: `EMBEDDED_SUPERSET` - [ ] 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 The UPDATING.md entry added in #44645 is updated to describe the virtual dataset behaviour. 🤖 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]
