-
-
Notifications
You must be signed in to change notification settings - Fork 737
feat: add Streamable HTTP transport with OAuth 2.1 for remote MCP access #363
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
0f5990a
5a55318
4a8e1b6
908138b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,318 @@ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * Streamable HTTP transport for DesktopCommanderMCP with OAuth 2.1 support. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * Allows Claude Web (claude.ai) to connect to this MCP server over HTTP. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * Intended to sit behind an nginx reverse proxy that handles TLS termination. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * OAuth flow (handled automatically by Claude Web): | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * 1. Client POSTs to /mcp, gets 401 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * 2. Client discovers metadata at /.well-known/oauth-protected-resource/mcp | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * 3. Client gets auth server metadata at /.well-known/oauth-authorization-server | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * 4. Client registers via DCR at /register (or uses pre-configured client_id/secret) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * 5. Client redirects user to /authorize | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * 6. Server auto-approves and redirects back with auth code | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * 7. Client exchanges code for token at /token | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * 8. Client uses Bearer token for /mcp requests | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * Usage: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * node dist/index.js http | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * PUBLIC_URL=https://serea.xyz OAUTH_CLIENT_ID=... OAUTH_CLIENT_SECRET=... node dist/index.js http | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { randomUUID } from 'node:crypto'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { createServer as createHttpServer } from 'node:http'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { StreamableHTTPServerTransport } from '@modelcontextprotocol/sdk/server/streamableHttp.js'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { createMcpExpressApp } from '@modelcontextprotocol/sdk/server/express.js'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { isInitializeRequest } from '@modelcontextprotocol/sdk/types.js'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { mcpAuthRouter, getOAuthProtectedResourceMetadataUrl } from '@modelcontextprotocol/sdk/server/auth/router.js'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { requireBearerAuth } from '@modelcontextprotocol/sdk/server/auth/middleware/bearerAuth.js'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import type { OAuthServerProvider, AuthorizationParams } from '@modelcontextprotocol/sdk/server/auth/provider.js'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import type { OAuthRegisteredClientsStore } from '@modelcontextprotocol/sdk/server/auth/clients.js'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import type { AuthInfo } from '@modelcontextprotocol/sdk/server/auth/types.js'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import type { OAuthClientInformationFull, OAuthTokens } from '@modelcontextprotocol/sdk/shared/auth.js'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // @ts-ignore - express types not installed | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import type { Response } from 'express'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { createServer } from './server.js'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { configManager } from './config-manager.js'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { featureFlagManager } from './utils/feature-flags.js'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const PORT = parseInt(process.env.PORT || '3100', 10); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const HOST = '127.0.0.1'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const PUBLIC_URL = process.env.PUBLIC_URL || `http://localhost:${PORT}`; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const OAUTH_CLIENT_ID = process.env.OAUTH_CLIENT_ID!; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const OAUTH_CLIENT_SECRET = process.env.OAUTH_CLIENT_SECRET!; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (!OAUTH_CLIENT_ID || !OAUTH_CLIENT_SECRET) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| console.error('[http] OAUTH_CLIENT_ID and OAUTH_CLIENT_SECRET env vars are required.'); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| process.exit(1); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // --------------------------------------------------------------------------- | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Pre-registered client store (no DCR — only known client_id can connect) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // --------------------------------------------------------------------------- | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| class PreRegisteredClientsStore implements OAuthRegisteredClientsStore { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| private client: OAuthClientInformationFull; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| constructor(clientId: string, clientSecret: string) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| this.client = { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| client_id: clientId, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| client_secret: clientSecret, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| client_id_issued_at: Math.floor(Date.now() / 1000), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| redirect_uris: [ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| 'https://claude.ai/api/mcp/auth_callback', | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| 'https://claude.com/api/mcp/auth_callback', | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ], | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| grant_types: ['authorization_code'], | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| response_types: ['code'], | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| token_endpoint_auth_method: 'client_secret_post', | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| client_name: 'Claude Web', | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } as OAuthClientInformationFull; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| getClient(clientId: string): OAuthClientInformationFull | undefined { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (clientId === this.client.client_id) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return this.client; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return undefined; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // No registerClient method = DCR disabled | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| class AutoApproveOAuthProvider implements OAuthServerProvider { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| readonly clientsStore: PreRegisteredClientsStore; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| private codes = new Map<string, { client: OAuthClientInformationFull; params: AuthorizationParams }>(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| private tokens = new Map<string, { token: string; clientId: string; scopes: string[]; expiresAt: number; resource?: URL }>(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| constructor(clientId: string, clientSecret: string) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| this.clientsStore = new PreRegisteredClientsStore(clientId, clientSecret); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| async authorize(client: OAuthClientInformationFull, params: AuthorizationParams, res: Response): Promise<void> { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const code = randomUUID(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| this.codes.set(code, { client, params }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const searchParams = new URLSearchParams({ code }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (params.state !== undefined) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| searchParams.set('state', params.state); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Auto-approve: redirect immediately with auth code | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const targetUrl = new URL(params.redirectUri); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| targetUrl.search = searchParams.toString(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| console.log(`[oauth] Auto-approved authorization for client ${client.client_id}`); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| res.redirect(targetUrl.toString()); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| async challengeForAuthorizationCode(_client: OAuthClientInformationFull, authorizationCode: string): Promise<string> { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const codeData = this.codes.get(authorizationCode); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (!codeData) throw new Error('Invalid authorization code'); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return codeData.params.codeChallenge; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| async exchangeAuthorizationCode( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| client: OAuthClientInformationFull, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| authorizationCode: string, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| _codeVerifier?: string, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| _redirectUri?: string, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| _resource?: URL, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ): Promise<OAuthTokens> { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const codeData = this.codes.get(authorizationCode); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (!codeData) throw new Error('Invalid authorization code'); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (codeData.client.client_id !== client.client_id) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| throw new Error('Authorization code was not issued to this client'); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| this.codes.delete(authorizationCode); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const token = randomUUID(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| this.tokens.set(token, { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| token, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| clientId: client.client_id, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| scopes: codeData.params.scopes || [], | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| expiresAt: Date.now() + 3600000, // 1 hour | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| resource: codeData.params.resource, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| console.log(`[oauth] Issued access token for client ${client.client_id}`); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| access_token: token, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| token_type: 'bearer', | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| expires_in: 3600, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| scope: (codeData.params.scopes || []).join(' '), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+114
to
+145
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. PKCE code verifier is not validated. The 🔒 Proposed fix to validate PKCE code verifier+import { createHash } from 'node:crypto';
+
+function verifyPkce(codeVerifier: string, codeChallenge: string): boolean {
+ const hash = createHash('sha256').update(codeVerifier).digest('base64url');
+ return hash === codeChallenge;
+}
+
async exchangeAuthorizationCode(
client: OAuthClientInformationFull,
authorizationCode: string,
- _codeVerifier?: string,
+ codeVerifier?: string,
_redirectUri?: string,
_resource?: URL,
): Promise<OAuthTokens> {
const codeData = this.codes.get(authorizationCode);
if (!codeData) throw new Error('Invalid authorization code');
if (codeData.client.client_id !== client.client_id) {
throw new Error('Authorization code was not issued to this client');
}
+ if (codeData.params.codeChallenge && codeVerifier) {
+ if (!verifyPkce(codeVerifier, codeData.params.codeChallenge)) {
+ throw new Error('Invalid code verifier');
+ }
+ }🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| async exchangeRefreshToken( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| _client: OAuthClientInformationFull, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+136
to
+148
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Suggestion: Access tokens are stored indefinitely in the in-memory Severity Level: Major
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }); | |
| console.log(`[oauth] Issued access token for client ${client.client_id}`); | |
| return { | |
| access_token: token, | |
| token_type: 'bearer', | |
| expires_in: 3600, | |
| scope: (codeData.params.scopes || []).join(' '), | |
| }; | |
| } | |
| async exchangeRefreshToken( | |
| _client: OAuthClientInformationFull, | |
| async verifyAccessToken(token: string): Promise<AuthInfo> { | |
| const tokenData = this.tokens.get(token); | |
| if (!tokenData) { | |
| throw new Error('Invalid or expired token'); | |
| } | |
| if (tokenData.expiresAt < Date.now()) { | |
| this.tokens.delete(token); | |
| throw new Error('Invalid or expired token'); | |
| } | |
| return { | |
| token, | |
| clientId: tokenData.clientId, | |
| scopes: tokenData.scopes, | |
| expiresAt: Math.floor(tokenData.expiresAt / 1000), | |
| resource: tokenData.resource, | |
| }; | |
| } |
Steps of Reproduction ✅
1. Start the HTTP MCP server via `main()` in `src/http-server.ts:157-167`; this creates an
`AutoApproveOAuthProvider` instance at line 167 with an internal `tokens` map defined at
line 77.
2. Complete the OAuth authorization code flow at least once via the `/authorize` and
`/token` routes wired by `mcpAuthRouter` at `src/http-server.ts:173-179`; the `/token`
handler calls `AutoApproveOAuthProvider.exchangeAuthorizationCode()` at lines 99-127.
3. Inside `exchangeAuthorizationCode()`, a new access token is generated and stored in
`this.tokens` at `src/http-server.ts:112-119`, increasing `this.tokens.size` by one for
each completed flow.
4. Repeat the auth flow many times over the lifetime of the process; expired tokens (older
than one hour per `expiresAt` set at line 117) are never removed anywhere in the file,
since `verifyAccessToken()` at `src/http-server.ts:136-148` only checks `expiresAt <
Date.now()` and throws but does not delete entries. Observing `this.tokens.size` via
instrumentation or a debugger shows it monotonically increasing, indicating unbounded
in-memory growth tied to the number of issued tokens.Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** src/http-server.ts
**Line:** 136:148
**Comment:**
*Resource Leak: Access tokens are stored indefinitely in the in-memory `tokens` map and are never removed when expired, causing unbounded growth over time and a memory leak under repeated authentications.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Suggestion: Access tokens are stored in an in-memory Map and never removed when they expire or are rejected, causing unbounded growth of the token store over time in a long-running process and risking memory exhaustion. [resource leak]
Severity Level: Major ⚠️
- ⚠️ In-memory token Map grows with every authorization flow.
- ⚠️ Long-running HTTP MCP server risks gradual memory exhaustion.
- ⚠️ Increased GC pressure may degrade MCP request latency.| _scopes?: string[], | |
| _resource?: URL, | |
| ): Promise<OAuthTokens> { | |
| throw new Error('Refresh tokens not supported'); | |
| } | |
| async verifyAccessToken(token: string): Promise<AuthInfo> { | |
| const tokenData = this.tokens.get(token); | |
| if (!tokenData || tokenData.expiresAt < Date.now()) { | |
| throw new Error('Invalid or expired token'); | |
| } | |
| return { | |
| token, | |
| async verifyAccessToken(token: string): Promise<AuthInfo> { | |
| const tokenData = this.tokens.get(token); | |
| if (!tokenData) { | |
| throw new Error('Invalid or expired token'); | |
| } | |
| if (tokenData.expiresAt < Date.now()) { | |
| this.tokens.delete(token); | |
| throw new Error('Invalid or expired token'); | |
| } | |
| return { | |
| token, | |
| clientId: tokenData.clientId, | |
| scopes: tokenData.scopes, | |
| expiresAt: Math.floor(tokenData.expiresAt / 1000), | |
| resource: tokenData.resource, | |
| }; | |
| } |
Steps of Reproduction ✅
1. Start the HTTP MCP server via `node dist/index.js http` so that `main()` in
`src/http-server.ts:179-305` initializes `AutoApproveOAuthProvider` and its in-memory
`tokens` Map (defined at `src/http-server.ts:83-87`).
2. From a client, perform the OAuth authorization code flow multiple times, causing
`AutoApproveOAuthProvider.exchangeAuthorizationCode()` at `src/http-server.ts:114-139` to
execute and call `this.tokens.set(token, {...})` for each new access token.
3. Keep the server running for hours or days; no code anywhere in the repository calls
`this.tokens.delete(...)` on these entries (the only references are `set` at line 124 and
`get` at line 151), so expired tokens remain stored indefinitely.
4. When an expired token is presented on any `/mcp` request (routes at
`src/http-server.ts:216-271` protected by `authMiddleware` created at lines 209-213),
`requireBearerAuth` invokes `provider.verifyAccessToken()` at
`src/http-server.ts:150-162`, which throws for expired tokens but never removes them from
`this.tokens`, allowing unbounded growth visible in heap snapshots or instrumentation.Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** src/http-server.ts
**Line:** 150:162
**Comment:**
*Resource Leak: Access tokens are stored in an in-memory Map and never removed when they expire or are rejected, causing unbounded growth of the token store over time in a long-running process and risking memory exhaustion.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Enforce the advertised scope on /mcp bearer auth.
Line 211 advertises mcp:tools, but Line 218 accepts any token (requiredScopes: []). This weakens authorization checks.
🔒 Suggested fix
const authMiddleware = requireBearerAuth({
verifier: provider,
- requiredScopes: [],
+ requiredScopes: ['mcp:tools'],
resourceMetadataUrl: getOAuthProtectedResourceMetadataUrl(mcpServerUrl),
});📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| scopesSupported: ['mcp:tools'], | |
| resourceName: 'DesktopCommanderMCP', | |
| })); | |
| // ---------- Bearer auth middleware for /mcp ---------- | |
| const authMiddleware = requireBearerAuth({ | |
| verifier: provider, | |
| requiredScopes: [], | |
| resourceMetadataUrl: getOAuthProtectedResourceMetadataUrl(mcpServerUrl), | |
| scopesSupported: ['mcp:tools'], | |
| resourceName: 'DesktopCommanderMCP', | |
| })); | |
| // ---------- Bearer auth middleware for /mcp ---------- | |
| const authMiddleware = requireBearerAuth({ | |
| verifier: provider, | |
| requiredScopes: ['mcp:tools'], | |
| resourceMetadataUrl: getOAuthProtectedResourceMetadataUrl(mcpServerUrl), |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/http-server.ts` around lines 211 - 219, The bearer auth is advertising
mcp:tools via resourceName/metadata but auth middleware uses requiredScopes: []
so tokens with no scope pass; update the requireBearerAuth call (authMiddleware)
to enforce the advertised scope by setting requiredScopes: ['mcp:tools'] (or
derive it from the resource metadata/resourceName) while keeping verifier:
provider and resourceMetadataUrl:
getOAuthProtectedResourceMetadataUrl(mcpServerUrl) unchanged so /mcp actually
requires the mcp:tools scope.
Uh oh!
There was an error while loading. Please reload this page.