[stack 7/8] fix(providers): harden MCP OAuth and Codex transports - #1164
[stack 7/8] fix(providers): harden MCP OAuth and Codex transports#1164sethkarten wants to merge 13 commits into
Conversation
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
| } | ||
|
|
||
| /** Follow RFC 9728 resource metadata, then fall back to co-located AS discovery. */ | ||
| async function discover( |
There was a problem hiding this comment.
🟠 High mcp/oauth.ts:282
Scope selection uses discovery.challengedScope ?? config.scopes ?? discovery.resourceScopes?.join(" ") and never falls back to metadata.scopes_supported. When a server has no protected-resource scopes_supported (including the co-located fallback path) but does advertise authorization-server scopes, the authorization request omits scope entirely. This regresses the previous behavior of using advertised scopes and can yield an unusable or rejected token. Consider adding metadata.scopes_supported?.join(" ") as a final fallback in the scope selection chain.
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @packages/ai/src/mcp/oauth.ts around line 282:
Scope selection uses `discovery.challengedScope ?? config.scopes ?? discovery.resourceScopes?.join(" ")` and never falls back to `metadata.scopes_supported`. When a server has no protected-resource `scopes_supported` (including the co-located fallback path) but does advertise authorization-server scopes, the authorization request omits `scope` entirely. This regresses the previous behavior of using advertised scopes and can yield an unusable or rejected token. Consider adding `metadata.scopes_supported?.join(" ")` as a final fallback in the scope selection chain.
| if (typeof metadata.registration_endpoint !== "string") throw new Error("Invalid OAuth registration_endpoint"); | ||
| endpoints.push(metadata.registration_endpoint); | ||
| } | ||
| for (const endpoint of endpoints) { |
There was a problem hiding this comment.
🟠 High mcp/oauth.ts:249
validateAuthorizationServerMetadata rejects any authorization, token, or registration endpoint whose origin differs from the issuer, so a standards-compliant server that hosts endpoints on separate domains is discovered but then rejected, making MCP login impossible. RFC 8414 does not require these endpoints to share the issuer origin. Consider validating each endpoint independently with validateOAuthNetworkUrl instead of enforcing origin equality with the issuer.
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @packages/ai/src/mcp/oauth.ts around line 249:
`validateAuthorizationServerMetadata` rejects any authorization, token, or registration endpoint whose origin differs from the issuer, so a standards-compliant server that hosts endpoints on separate domains is discovered but then rejected, making MCP login impossible. RFC 8414 does not require these endpoints to share the issuer origin. Consider validating each endpoint independently with `validateOAuthNetworkUrl` instead of enforcing origin equality with the issuer.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 74a91a6. Configure here.
| const nat64Prefixes = config.nat64Prefixes ? [...config.nat64Prefixes] : undefined; | ||
| const nat64CacheKey = {}; | ||
| const resourcePolicy: OAuthFetchPolicy = { ...createOAuthFetchPolicy(config.url), nat64Prefixes, nat64CacheKey }; | ||
| const oauthPolicy: OAuthFetchPolicy = { nat64Prefixes, nat64CacheKey }; |
There was a problem hiding this comment.
Loopback OAuth AS policy mismatch
Medium Severity
createOAuthFetchPolicy permits HTTP on the configured loopback resource origin, but oauthPolicy omits allowedHttpOrigin. Colocated discovery, authorization-server metadata, DCR, and token exchange against that same loopback origin then fail the HTTPS-only check, so local HTTP MCP OAuth cannot complete.
Additional Locations (2)
Reviewed by Cursor Bugbot for commit 74a91a6. Configure here.
74a91a6 to
f4f6308
Compare
| const nat64Prefixes = config.nat64Prefixes ? [...config.nat64Prefixes] : undefined; | ||
| const nat64CacheKey = {}; | ||
| const resourcePolicy: OAuthFetchPolicy = { ...createOAuthFetchPolicy(config.url), nat64Prefixes, nat64CacheKey }; | ||
| const oauthPolicy: OAuthFetchPolicy = { nat64Prefixes, nat64CacheKey }; |
There was a problem hiding this comment.
🟠 High mcp/oauth.ts:584
oauthPolicy omits the loopback HTTP allowance from createOAuthFetchPolicy(config.url), so OAuth discovery and token requests reject configured local HTTP MCP origins like http://127.0.0.1:3000. resourcePolicy preserves the allowance via allowedHttpOrigin, but oauthPolicy does not, so validateOAuthNetworkUrl throws on the loopback issuer during login and refresh. Spread createOAuthFetchPolicy(config.url) into oauthPolicy so the configured loopback origin is permitted for OAuth endpoints.
| const oauthPolicy: OAuthFetchPolicy = { nat64Prefixes, nat64CacheKey }; | |
| const oauthPolicy: OAuthFetchPolicy = { ...createOAuthFetchPolicy(config.url), nat64Prefixes, nat64CacheKey }; |
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @packages/ai/src/mcp/oauth.ts around line 584:
`oauthPolicy` omits the loopback HTTP allowance from `createOAuthFetchPolicy(config.url)`, so OAuth discovery and token requests reject configured local HTTP MCP origins like `http://127.0.0.1:3000`. `resourcePolicy` preserves the allowance via `allowedHttpOrigin`, but `oauthPolicy` does not, so `validateOAuthNetworkUrl` throws on the loopback issuer during login and refresh. Spread `createOAuthFetchPolicy(config.url)` into `oauthPolicy` so the configured loopback origin is permitted for OAuth endpoints.
| } else if (kinds.some((kind) => kind !== "public")) { | ||
| throw new Error(`OAuth hostname resolves to a non-public address: ${hostname}`); | ||
| } | ||
| const selected = addresses[0]; |
There was a problem hiding this comment.
🟠 High mcp/safe-fetch.ts:335
resolveTarget validates every resolved address but then pins the request to only addresses[0]. If that first address is unreachable while another validated address is healthy, the request fails instead of falling back. The pinnedLookup closure only ever returns target.address, so Undici has no other candidates to try. Consider preserving the full validated address set in the pinned lookup so the HTTP client can fall back to a healthy address.
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @packages/ai/src/mcp/safe-fetch.ts around line 335:
`resolveTarget` validates every resolved address but then pins the request to only `addresses[0]`. If that first address is unreachable while another validated address is healthy, the request fails instead of falling back. The `pinnedLookup` closure only ever returns `target.address`, so Undici has no other candidates to try. Consider preserving the full validated address set in the pinned lookup so the HTTP client can fall back to a healthy address.
1 participant
Stack 7/8 — fix(providers): harden MCP OAuth and Codex transports
Base:
stack/external-06-windowsReview order: merge only after the preceding stack layer is accepted. This PR is not intended to merge independently out of order.
Stack navigation
Summary
Validation
npm run check; targeted AI provider suites 99/99; MCP manager 14/14.Provenance
upstream/mainusing issue reports and PR descriptions/comments only.Linked-item disposition
Fixed on merge
Independently superseded pull requests
Reviewer notes
main, to avoid cumulative duplicate diffs.