rusackas commented on PR #42930: URL: https://github.com/apache/superset/pull/42930#issuecomment-5259423796
Went through Bito's three "Additional Suggestions" on this one. The annotation cache-key one is a misread. `_annotation_cache_context` only feeds `security_manager.get_rls_cache_key()` into `QueryObject.cache_key()`, which hashes the whole dict before it ever leaves the server. No RLS clause or chart data makes it back to the requester, so there's nothing to derive. Same for the backslash-redirect test. `is_safe_redirect_url` normalizes backslashes to slashes before *any* structural check, so an absolute URL like `https:/\evil.com` goes through the exact same netloc allowlist as every other absolute URL. It's not passing "by accident", that's the actual design. The missing cache-header test was fair though, added one in 2e2cdb0272. -- 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]
