sha174n opened a new pull request, #44496: URL: https://github.com/apache/superset/pull/44496
### SUMMARY `PUT`, `GET` and `REMOVE` (and the `RM` alias) are client-side file-transfer statements. They move files between the host running the query and a remote stage, so they perform file I/O on that host rather than reading or writing data in the database. They are not analytics queries. sqlglot has no structured node for any of them, so they fall back to an opaque `Command` and are not classified as mutating. As a result they were not covered by any of the existing query-execution checks, including the read-only (`allow_dml=false`) gate. This PR adds a small, single-source check on `SQLScript`/`SQLStatement` that identifies these command heads, and wires it into both SQL execution paths (`sql_lab.execute_sql_statements` and `SQLExecutor._check_security`). They are rejected in user-submitted SQL **regardless of the `allow_dml` setting**, since they are host file I/O rather than DML: allowing DML should not enable file transfer on the query host. - Detection lives in one place (`SQLStatement.get_client_file_transfer_command` / `SQLScript.get_client_file_transfer_commands`); both execution paths call it. - `LIST`/`LS` (staged-file listing, a read) are intentionally not included. - Ordinary statements are unaffected: an identifier named `get`/`put` parses as a normal expression, not a command head, so there are no false positives. ### TESTING INSTRUCTIONS New unit tests: - `tests/unit_tests/sql/parse_tests.py`: `get_client_file_transfer_command` / `get_client_file_transfer_commands` return the command heads for `PUT`/`GET`/`REMOVE`/`RM` (case-insensitive, multi-statement) and nothing for `SELECT`/`INSERT`/`LIST`. - `tests/unit_tests/sql/execution/test_executor.py`: `_check_security` rejects `PUT`/`GET`/`REMOVE` whether or not `allow_dml` is set, and leaves a normal `SELECT` untouched. ``` pytest tests/unit_tests/sql/parse_tests.py tests/unit_tests/sql/execution/test_executor.py ``` ### 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 -- 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]
