YuriyKrasilnikov commented on PR #37371: URL: https://github.com/apache/superset/pull/37371#issuecomment-4764512234
@rusackas thanks again for rebasing this and for the security-review guidance. I pushed one more pass in b68d882830 and the PR is green now, including `pre-commit (current)`, `unit-tests (current)`, and `unit-tests-required`. I re-traced the guest sorting path from the Table frontend down to backend validation and tightened the security boundary a bit more: - the guest orderby whitelist now respects Table `column_config[<columnKey>].visible === false`, so a manually modified guest payload cannot sort by a selected-but-hidden Table column; - label extraction now uses Superset's backend `get_column_name()` / `get_metric_name()` helpers, which keeps adhoc SIMPLE metrics without custom labels aligned with actual result keys like `SUM(sales)`; - new guest `orderby` entries now have to be strict `[term, bool]` pairs, and newly supplied SQL expression objects / malformed SIMPLE metric dicts fail closed; - saved/frozen owner-defined `orderby` entries are still allowed as before. Local targeted validation: `.venv/bin/pytest tests/unit_tests/security/manager_test.py -q` passes with `89 passed`. Since this does loosen guest-user query validation and CI is green now, CC @dpgaspar @mistercrunch for the careful security pass you asked for. -- 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]
