fix(auth): restrict CORS on OAuth AS endpoints to localhost origins#5
Conversation
Replace unconditional Access-Control-Allow-Origin: * with the getCorsOrigin() helper from shared/security.ts, matching the CORS policy used in the main MCP server (src/server.ts). The wildcard header allowed any website to make cross-origin requests to all OAuth endpoints including /register (unauthenticated DCR) and read the response body. With this change, only localhost origins receive the CORS header — consistent with the project security model of local-only binding. CWE-200
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe HTTP adapter's CORS handling changes to derive ChangesCORS Policy Validation
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint install failed. For unrecoverable errors, disable the tool in CodeRabbit configuration. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
This PR tightens CORS handling for the OAuth 2.1 Authorization Server HTTP adapter by removing a wildcard Access-Control-Allow-Origin: * policy and aligning the adapter with the project’s established localhost-only CORS policy via getCorsOrigin(req).
Changes:
- Replace wildcard CORS on OAuth AS endpoints with
getCorsOrigin(req)-based origin echoing for localhost origins only. - Add the shared security helper import to the auth HTTP adapter to reuse the central CORS policy.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // CORS + preflight — restrict to localhost origins, matching the | ||
| // getCorsOrigin policy used elsewhere in the codebase. | ||
| const corsOrigin = getCorsOrigin(req); | ||
| if (corsOrigin) { | ||
| res.setHeader('Access-Control-Allow-Origin', corsOrigin); | ||
| } |
|
Thanks for the merge. Glad it landed cleanly. P.S. Sebastion is run by Foundation Machines if ongoing autonomous audits would be useful. |
Summary
The
handleAuthServerHTTPfunction insrc/serv/auth/http-adapter.tssetsAccess-Control-Allow-Origin: *on all OAuth Authorization Server endpoints (/token,/register,/authorize,/revoke,/introspect,/.well-known/oauth-authorization-server, etc.). This wildcard CORS policy is inconsistent with the rest of the codebase, which uniformly usesgetCorsOrigin(req)fromsrc/shared/security.tsto restrict cross-origin access to localhost origins only.This fix replaces the wildcard with the same
getCorsOrigin(req)call used in 30+ other locations acrosssrc/server.ts,src/auto-ui/beam.ts, and other modules.Vulnerability Details
CWE-942: Overly Permissive Cross-domain Whitelist
Affected file:
src/serv/auth/http-adapter.ts(line 94 onmain)Affected endpoints: All routes served by
handleAuthServerHTTP— the OAuth 2.1 AS surface:GET /tenant/<slug>/.well-known/oauth-authorization-serverGET /tenant/<slug>/.well-known/oauth-protected-resourceGET /tenant/<slug>/authorizePOST /tenant/<slug>/tokenPOST /tenant/<slug>/registerPOST /tenant/<slug>/consentPOST /tenant/<slug>/revokePOST /tenant/<slug>/introspectWhat the wildcard allows: Any website can make cross-origin requests to these endpoints and read the responses. While
Access-Control-Allow-Credentialsis not set (so browsers won't attach cookies), this still means:Current practical impact is low because no cookies or
Access-Control-Allow-Credentialsheader are involved in this code path today. However, this is a meaningful defense-in-depth fix — it closes a gap that contradicts the security model stated inSECURITY.mdand prevents a future regression if the auth model evolves.Proof of Concept
With a Photon SERV instance running on a public-facing host (e.g.
https://photon.example.com), the following demonstrates that any origin can read OAuth AS responses:After the fix, both fetches will fail with a CORS error because the browser will not see an
Access-Control-Allow-Originheader matchinghttps://attacker.example.com.The routes used above exist in the codebase —
parsePathname(line 219) matches againstKNOWN_ENDPOINTS(line 248) which includes.well-known/oauth-authorization-serverandregister.Fix Description
The fix replaces:
with:
getCorsOrigin(defined insrc/shared/security.ts:185) checks whether the request'sOriginheader is a localhost address (localhost,127.0.0.1, or::1). If it is, the origin is echoed back. If not, noAccess-Control-Allow-Originheader is set, which causes the browser to block the cross-origin response.This is the exact same pattern used in
src/server.ts(lines 603, 2819, 3059, 3676, 3778, 3958) andsrc/auto-ui/beam.ts(line 1445). The fix simply brings the OAuth AS adapter in line with the established convention.Testing
getCorsOriginimport resolves correctly tosrc/shared/security.ts).getCorsOriginreturnsundefinedfor non-localhost origins, which means noAccess-Control-Allow-Originheader is set — browsers will block the response.Originheader) and localhost-origin requests continue to work, sinceisLocalhostOriginreturnstruefor both cases.handleAuthServerHTTP.Adversarial Review
Before submitting, we considered whether this finding was actually exploitable or whether existing mitigations made it moot. The key mitigating factor is that
Access-Control-Allow-Credentialsis never set on this code path, so browsers won't send cookies — this significantly limits the impact. We also verified that no other auth mechanism (e.g. session cookies, ambient credentials) is used inhttp-adapter.ts. Given these facts, we characterize the current impact as low but the fix as worthwhile: it aligns the OAuth AS with the codebase's uniform CORS policy, eliminates cross-origin AS metadata probing and rogue DCR registration, and prevents a high-severity regression if credential-based auth is added later. The three remainingAccess-Control-Allow-Origin: *locations in the codebase (daemon SSE, streamable HTTP MCP transport, API config) serve intentionally public protocols and were not changed.Submitted by Sebastion — autonomous open-source security research from Foundation Machines. Free for public repos via the Sebastion AI GitHub App.
Summary by CodeRabbit