rusackas opened a new pull request, #40662: URL: https://github.com/apache/superset/pull/40662
> **Draft / `hold:testing`** — changes the SQL sent to each engine's `estimate_query_cost`. Needs validation against the supported analytics databases before merge (see "Why draft"). ### SUMMARY The SQL Lab cost-estimation path (`QueryEstimationCommand`) enforced only **database-level access** and skipped the per-statement controls that the execution path (`sql_lab.execute_sql_statements`) applies. So `EXPLAIN`/cost estimation could be used to: - probe **disallowed functions** (`DISALLOWED_SQL_FUNCTIONS`), - probe **disallowed tables** (`DISALLOWED_SQL_TABLES`), - bypass the **DML guard** (`allow_dml`), - and — because **RLS predicates were never injected** — confirm the existence / cardinality of rows hidden by row-level security (a data-existence oracle behind RLS). ASVS 1.2.4. This adds `_apply_sql_security()` mirroring the executor: it rejects disallowed functions/tables and DML, and injects RLS predicates into each parsed statement (when `RLS_IN_SQLLAB` is enabled), then estimates the **RLS-constrained** SQL — the same query the user would actually be permitted to run. ### WHY DRAFT (`hold:testing`) The change rewrites the SQL handed to `db_engine_spec.estimate_query_cost` (RLS-injected). That goes through each engine's EXPLAIN implementation, which I can't exercise locally across the supported databases. Needs validation that: - EXPLAIN still succeeds on RLS-injected SQL for the common engines (Postgres, MySQL, Trino, BigQuery, …); - the disallowed/DML rejections surface as clean errors in the estimate UI. ### TESTING INSTRUCTIONS ``` pytest tests/unit_tests/commands/sql_lab/test_estimate.py ``` New tests: DML blocked when `allow_dml=False` (and allowed when True); a disallowed table is rejected. **Before merge:** estimate an RLS-protected dataset query end-to-end and confirm the cost reflects the RLS filter. ### ADDITIONAL INFORMATION - [ ] Has associated issue: - [ ] Required feature flags: RLS_IN_SQLLAB (for the RLS portion) - [ ] Changes UI - [ ] Includes DB Migration - [ ] 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]
