codeant-ai-for-open-source[bot] commented on PR #35021: URL: https://github.com/apache/superset/pull/35021#issuecomment-3614280729
## Nitpicks 🔍 <table> <tr><td>🔒 <strong>No security issues identified</strong></td></tr> <tr><td>⚡ <strong>Recommended areas for review</strong><br><br> - [ ] <a href='https://github.com/apache/superset/pull/35021/files#diff-0828f57d532e9d3a6986d5b38bb6bf08a59f885171116e81db8ff8d19c448165R166-R177'><strong>Unsafe redirect</strong></a><br>The page assigns unvalidated query param values directly to window.location.href in multiple places, allowing navigation to arbitrary schemes (e.g., javascript:, data:) or open-redirect targets. Validate the URL (absolute, allowed protocols like http/https) before performing automatic or user-triggered redirects.<br> - [ ] <a href='https://github.com/apache/superset/pull/35021/files#diff-57c919f0e5873259ac908182bf8df4d96d0f3dcb3d8f8cea20baacf12b31296eR70-R102'><strong>Unsafe URL schemes</strong></a><br>The link processor only treats http/https links as candidates for redirection and leaves other schemes (e.g., `javascript:`, `data:`, `mailto:`) untouched. These non-http schemes can be abused in HTML emails (XSS, script execution, data URIs) and should be explicitly handled (blocked, neutralized, or normalized) rather than left as-is.<br> - [ ] <a href='https://github.com/apache/superset/pull/35021/files#diff-6db9870e227e46937dd15e74335374447818170cb4ff43aea41f40893c34fe83R70-R82'><strong>Scheme handling / open redirect</strong></a><br>The code calls is_safe_redirect_url(target_url) and redirects directly if it returns True. However, URLs with non-http(s) schemes (e.g. mailto:, ftp:) or scheme-less but non-relative values can be treated as "safe" by the current logic and be redirected without warning. This can cause unexpected behavior or bypass the warning for potentially dangerous or unintended target schemes. Ensure only http/https (or true relative paths) are allowed for automatic redirects.<br> - [ ] <a href='https://github.com/apache/superset/pull/35021/files#diff-64900fd3f4e1f2c84ff1d1a922d9934cdc8ddefab4ef9626e45e3e426b0b45b1R433-R435'><strong>Open redirect risk</strong></a><br>Adding a RedirectView endpoint can introduce open-redirect / SSRF / unsafe external navigation paths if the view accepts arbitrary `url` query parameters. Verify the RedirectView implementation validates/normalizes URLs (no javascript:, data:, or other dangerous schemes), enforces a whitelist or confirmation flow for external hosts, defends against double-encoding tricks, and uses safe redirect helpers when redirecting to internal targets.<br> - [ ] <a href='https://github.com/apache/superset/pull/35021/files#diff-6db9870e227e46937dd15e74335374447818170cb4ff43aea41f40893c34fe83R90-R99'><strong>normalize_url fragility</strong></a><br>The inline normalize_url() builds strings using parsed.scheme and parsed.netloc directly. If a URL lacks a scheme (or is protocol-relative: '//host'), the normalized string can be malformed (e.g., '://host/...') which may break comparisons with trusted entries and allow bypasses. Use urlunparse/urljoin or explicitly handle missing schemes and protocol-relative URLs to produce consistent canonical forms.<br> - [ ] <a href='https://github.com/apache/superset/pull/35021/files#diff-0828f57d532e9d3a6986d5b38bb6bf08a59f885171116e81db8ff8d19c448165R156-R164'><strong>Double decoding / parsing edge-case</strong></a><br>The code calls decodeURIComponent on the value returned by URLSearchParams.get (which is already decoded). This may double-decode or throw on certain inputs. Use URLSearchParams directly and validate/normalize safely.<br> - [ ] <a href='https://github.com/apache/superset/pull/35021/files#diff-57c919f0e5873259ac908182bf8df4d96d0f3dcb3d8f8cea20baacf12b31296eR54-R67'><strong>Redirect target construction</strong></a><br>`_get_redirect_url` uses `REDIRECT_URL_PAGE` verbatim if present and otherwise appends `/redirect/?url=...` to `base_url`. If `REDIRECT_URL_PAGE` is misconfigured to an external domain or `base_url` contains unexpected trailing components, this may create open-redirect-like behaviour or malformed redirect links. Validate or constrain configured redirect targets and ensure the redirect handling page validates the target URL.<br> - [ ] <a href='https://github.com/apache/superset/pull/35021/files#diff-c99ae4b2b09b756ab2189a99a9685229f9d12633fc2616c368ea869770f603bfR1748-R1751'><strong>Open redirect</strong></a><br>New redirect behavior can introduce open redirect vulnerabilities if the target URL is not validated or properly encoded. The redirect endpoint that will consume a query parameter (`url=...`) must validate scheme/host, percent-encode the parameter when generating links, and avoid reflecting untrusted user input directly into HTML or headers.<br> </td></tr> </table> -- 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]
