restrict url schemes allowed in oauth metadata#877
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
|
@claude can you fix this in the tests: |
Updated test expectations to match the new error message that includes javascript:, data:, and vbscript: schemes in the validation error. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
src/shared/auth.ts
Outdated
| {message: "URL must be parseable"} | ||
| ).refine( | ||
| (url) => { | ||
| const u = url.trim().toLowerCase(); |
There was a problem hiding this comment.
Could just use URL.protocol, which will do the lowercase for us (as per https://url.spec.whatwg.org/#scheme-start-state )
const u = new URL(url);
return (u !== 'javascript:') && (u !== 'data:') && (u !== 'vbscript:');
Or... wondering if we should just allow https: and http:, otherwise futurescript: will be at risk when it comes out.
There was a problem hiding this comment.
nice, yea checking protocol sounds better.
for allow vs. deny, some discussion in #841 (comment) -- there are legitimate app url schemes that I think we need to allow, e.g. for mobile.
There was a problem hiding this comment.
had to get a little funky with Zod to make the parseable URL check to fail. (using superRefine). Looks like zod 4.0 makes this a little simpler if we upgrade.
2 participants
Motivation and Context
See #841 (cc @arjunkmrm )
Since authorization_url is often opened in a browser, we don't want it to be javascript.
How Has This Been Tested?
Added tests
Breaking Changes
No
Types of changes
Checklist
Additional context