Repository navigation
Description
Summary
mcpAuthRouter's registration handler (server/auth/handlers/register.js) validates redirect_uris only with SafeUrlSchema, which rejects javascript:, data: and vbscript: but accepts:
- plain
http://redirect URIs on non-loopback hosts, although the MCP authorization spec (2025-11-25, Communication Security) says "All redirect URIs MUST be eitherlocalhostor use HTTPS"; - redirect URIs with a fragment, which RFC 6749 §3.1.2 forbids ("The endpoint URI MUST NOT include a fragment component").
Any server that relies on the SDK's handler without adding its own validation inherits this.
Versions
@modelcontextprotocol/sdk1.32.2 (server/auth/handlers/register.js,shared/auth.js)@modelcontextprotocol/server-legacy2.4.0 uses the same schema (I understand it is frozen; reporting for v1 users)
Reproduction
With the OAuth example server (node dist/esm/examples/server/simpleStreamableHttp.js --oauth):
curl -s -o /dev/null -w "%{http_code}\n" -X POST http://localhost:3001/register \
-H 'content-type: application/json' \
-d '{"redirect_uris":["http://attacker.example/callback"],"token_endpoint_auth_method":"none"}'
# 201
curl -s -o /dev/null -w "%{http_code}\n" -X POST http://localhost:3001/register \
-H 'content-type: application/json' \
-d '{"redirect_uris":["https://client.example/callback#fragment"],"token_endpoint_auth_method":"none"}'
# 201For comparison, javascript:alert(1) is correctly refused with 400.
Expected
400 invalid_redirect_uri (RFC 7591 §3.2.2) for both.
Suggested fix
Validate in the registration handler rather than in the shared OAuthClientMetadataSchema, since clients also use that schema to parse metadata:
- reject any redirect URI with a fragment;
- reject
http:unless the host is a loopback address (localhost,127.0.0.0/8,[::1]), in line with RFC 8252 §7.3; - optionally let the server configure whether private-use schemes (RFC 8252 §7.1) are allowed.
Impact
Low: this is hardening, not a direct exploit. Authorization codes for such clients could travel in cleartext, and inconsistent validation makes redirect-based phishing easier to set up.
Found while testing authorization servers with mcp-authprobe (check MAP-RDR04).
Activity
techaurah commentedon Nov 3, 2026
Hi, we are TechAura. We would like to work on this if a maintainer is willing to assign it to us. Our read: mcpAuthRouter's registration handler validates redirect_uris only with SafeUrlSchema, so plain-HTTP non-loopback URIs and fragment URIs are accepted. The fix we have in mind follows the suggested approach: validate in the registration handler (not the shared client metadata schema), rejecting fragments and http unless loopback, per RFC 6749 / RFC 7591 / RFC 8252. Happy to adjust if maintainers prefer a different shape.
techaurah commentedon Nov 3, 2026
Correction to our comment above: we have now seen open PR #3143, which already covers this fix, so we are stepping back and will not open a competing PR. Happy for #3143 to proceed.
media-ctrl commentedon Nov 4, 2026
Patch for this, following the reproduction in the report. It lives in the registration handler rather than
OAuthClientMetadataSchemafor the reason you give: clients parse server metadata with that schema, and a stricter shared schema would start rejecting servers whose storedredirect_urispredate the rule.In
packages/server-legacy/src/auth/handlers/register.ts:called from the handler right after
OAuthClientMetadataSchema.safeParse, throwing a newInvalidRedirectUriErrorwith codeinvalid_redirect_uri(RFC 7591 3.2.2). The code already exists inOAuthErrorCode, nothing used it for this path.Two decisions worth your review:
127.0.0.0/8and[::1], because RFC 8252 7.3 depends on it for native apps. Only non-loopbackhttp:is refused, so the change cannot break a local callback.cursor://...,vscode://...) are untouched.SafeUrlSchemaalready refuses the dangerous schemes, and restricting to http/https would break every native MCP client.Tests: nine cases in
test/auth/handlers/register.test.ts(two rejections, one mixed list where a single bad URI must block registration entirely, six accept cases includinghttp://127.0.0.2:1234/cbandhttp://[::1]:8080/callback). On top of main the auth suite is 167 passing (158 before), with eslint and prettier clean.The branch is
fix/dcr-redirect-uri-validationon my fork, diff is 3 files. Happy to turn it into a PR if you want it that way, or to keep it here as a patch and answer questions against the branch.Enigmacker commentedon Nov 4, 2026
Thanks, this looks right to me, and both decisions match what I had in mind: loopback http must stay accepted for native apps (RFC 8252 §7.3), and private-use schemes should not be touched. Using invalid_redirect_uri (RFC 7591 §3.2.2) also seems the better error code.
I'm only the reporter, not a maintainer, so the call is theirs. Note that #3143 already targets v1.x, while your branch targets server-legacy on main; they look complementary rather than competing. Since server-legacy is described as frozen, I'd suggest opening a PR against main that references #3142 and #3143, and letting the maintainers decide whether they want the fix in both lines.
media-ctrl commentedon Nov 4, 2026
Thanks for the read. Two notes so the thread stays actionable:
packages/server-legacy/src/auth/handlers/register.ts). It can be lifted verbatim into whichever line maintainers prefer, v1.x or main.One detail worth deciding explicitly rather than leaving implicit: the loopback check accepts the whole
127.0.0.0/8range, not only127.0.0.1, and strips IPv6 brackets so[::1]and::1both pass. That matches RFC 8252 section 7.3. If maintainers prefer a stricter single address, it is a one line change.techaurah commentedon Nov 4, 2026
Coming back to this one with fresh eyes. We stepped back yesterday because PR #3143 already covers the v1.x line. The thread since then suggests the main line (server-legacy) is a separate, complementary fix, and @media-ctrl has kindly left a self-contained patch above and said it can be lifted into whichever line maintainers prefer, while noting they would rather not maintain a fork PR themselves.
We are happy to open that main-line PR using media-ctrl's patch, with full credit to them in the PR description and commit, referencing this issue and #3143, if the maintainers or @samuelmbabhazi would like that. If you would rather fold it into #3143 or handle it differently, we will stay out of the way, with no competing work from us either way.
fix(auth): answer invalid_redirect_uri for a rejected redirect URI