Skip to content

Conversation

@tarrencev
Copy link
Contributor

Potential fix for https://github.com/cartridge-gg/controller/security/code-scanning/155

To eliminate any risk of client-side cross-site scripting or open redirect, ensure that the validated URL is always absolute, uses only "http:" or "https:" protocols, and cannot be "javascript:" or another dangerous protocol—even in edge cases. In the redirection step, use the already-parsed result from validateRedirectUrl (not the raw user input) by returning and reusing the safely-parsed absolute URL (e.g., via url.href). This prevents manipulation via tricky input strings that might be mis-parsed. Adjust the validator to always return the canonical, validated, url.href and update the redirector to strictly use this, never the raw redirectUrl string. This also future-proofs against possible developer changes that introduce new vector(s).

Steps needed:

  1. In validateRedirectUrl, if valid, return the parsed/canonicalized URL as a property (e.g., validatedUrl or similar) along with the isValid flag.
  2. In safeRedirect, use this validatedUrl instead of the original redirectUrl for assignment to window.location.href.
  3. No new dependencies are required; URL is built in.
  4. No change to existing validator logic, just enhancement of its output to provide canonical data.

Suggested fixes powered by Copilot Autofix. Review carefully before merging.

… scripting

Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
@vercel
Copy link

vercel bot commented Nov 3, 2025

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Preview Updated (UTC)
controller-example-next Ready Ready Preview Nov 3, 2025 4:52pm
controller-example-next-compat Ready Ready Preview Nov 3, 2025 4:52pm
keychain Ready Ready Preview Nov 3, 2025 4:52pm
keychain-storybook Ready Ready Preview Nov 3, 2025 4:52pm

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants