🪝 fix: MCP Refresh token on OAuth Discovery Failure

[danny-avila]

Summary

Fixed a security vulnerability in the MCP OAuth token refresh flow where a malicious MCP server could harvest refresh tokens by blocking .well-known metadata discovery, and resolved a companion vulnerability in the token revocation path.

• Replaced unsafe /token fallback in both refreshOAuthTokens branches (stored clientInfo and auto-discovered) with hard errors when OAuth metadata discovery fails and no stored endpoint is available, preventing refresh tokens from being POSTed to an unverified MCP resource server endpoint.
• Threaded storedTokenEndpoint and storedAuthMethods (persisted from the initial OAuth flow's SSRF-validated metadata) through MCPTokenStorage.getTokens() → MCPConnectionFactory.createRefreshTokensFunction() → MCPOAuthHandler.refreshOAuthTokens(), so legacy MCP servers without .well-known support remain functional after first authentication without reintroducing the token-leak vector.
• Fixed revokeOAuthToken to unconditionally run validateOAuthUrl on the constructed revocation URL, including the /revoke fallback path, which previously bypassed SSRF validation entirely when revocationEndpoint was absent.
• Redacted the raw refresh_token value and Authorization header from debug-level log output; replaced with grant_type and a has_auth_header boolean.
• Split the compound !oauthMetadata?.token_endpoint guard in branch 2 into separate !oauthMetadata and !oauthMetadata.token_endpoint checks, aligning error messages with branch 1 for consistent observability.
• Added a stored token endpoint fallback sub-describe in describe('refreshOAuthTokens') covering both branches: stored endpoint used when discovery fails, and hard-throw preserved when neither discovery nor stored endpoint is available.
• Added a missing test for branch 2 covering the case where discovery returns a valid metadata object with no token_endpoint field.
• Renamed a misleading test title from "metadata lacks token endpoint" to "metadata discovery returns undefined" to accurately reflect the mock behavior.
• Removed redundant afterEach mockFetch.mockClear() calls from two describe blocks already covered by jest.clearAllMocks() in beforeEach.
• Replaced a non-standard @see branch 1 comment JSDoc tag in branch 2 with a plain // comment referencing the security rationale above it.

Change Type

• Bug fix (non-breaking change which fixes an issue)

Testing

All changes are covered by the existing and new Jest unit tests in packages/api/src/mcp/__tests__/handler.test.ts. 76 tests pass across the three MCP OAuth test suites (handler.test.ts, MCPOAuthSecurity.test.ts, MCPOAuthTokenStorage.test.ts).

To reproduce the vulnerability this fixes: configure a legacy MCP server without .well-known endpoints, complete an initial OAuth flow (which succeeds via fallback metadata), let the access token expire, and observe that the pre-fix code would POST the refresh token to {serverUrl}/token (the resource server) rather than the validated authorization server. Post-fix, the stored token_endpoint from the initial flow is used; if none is stored, the request is rejected with an error rather than sent to an unvalidated endpoint.

Test Configuration:

Run from the packages/api directory:

npx jest mcp/__tests__/handler.test.ts
npx jest mcp/__tests__/MCPOAuthSecurity.test.ts
npx jest mcp/__tests__/MCPOAuthTokenStorage.test.ts

Checklist

• My code adheres to this project's style guidelines
• I have performed a self-review of my own code
• I have commented in any complex areas of my code
• My changes do not introduce new warnings
• I have written tests demonstrating that my changes are effective or that my feature works
• Local unit tests pass with my changes

[Copilot]

Pull request overview

This PR changes MCP OAuth token refresh behavior to require successful OAuth metadata discovery (and a token endpoint), removing the previous legacy /token fallback behavior, and updates unit tests accordingly.

Changes:

• Replace /token fallback behavior during token refresh with hard failures when OAuth discovery/token endpoint is missing.
• Update MCP OAuth handler tests to expect thrown errors (and no refresh HTTP call) when discovery fails.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File	Description
packages/api/src/mcp/oauth/handler.ts	Removes legacy /token fallback in refresh paths; throws when discovery/token endpoint missing.
packages/api/src/mcp/tests/handler.test.ts	Updates refresh tests to assert errors instead of fallback refresh calls.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

You can also share your feedback on Copilot code review. Take the survey.