From 0f7f7be4ef0a405b5dbea27009cd40c0c13be3d4 Mon Sep 17 00:00:00 2001 From: Spencer Date: Thu, 9 Apr 2026 17:14:07 -0400 Subject: [PATCH] fix(core): clear 5-minute timeouts in oauth flow to prevent memory leaks (#24968) --- packages/core/src/code_assist/oauth2.ts | 18 ++++++-- packages/core/src/mcp/oauth-provider.test.ts | 46 +++++++++++--------- packages/core/src/utils/oauth-flow.test.ts | 22 ++++++++++ packages/core/src/utils/oauth-flow.ts | 25 ++++++++--- 4 files changed, 81 insertions(+), 30 deletions(-) diff --git a/packages/core/src/code_assist/oauth2.ts b/packages/core/src/code_assist/oauth2.ts index cb4b645ab3..40be9c2236 100644 --- a/packages/core/src/code_assist/oauth2.ts +++ b/packages/core/src/code_assist/oauth2.ts @@ -424,6 +424,7 @@ async function authWithUserCode(client: OAuth2Client): Promise { '\n\n', ); + let authTimeoutId: NodeJS.Timeout | undefined; const code = await new Promise((resolve, reject) => { const rl = readline.createInterface({ input: process.stdin, @@ -431,20 +432,29 @@ async function authWithUserCode(client: OAuth2Client): Promise { terminal: true, }); - const timeout = setTimeout(() => { - rl.close(); - reject( + const abortController = new AbortController(); + authTimeoutId = setTimeout(() => { + abortController.abort( new FatalAuthenticationError( 'Authorization timed out after 5 minutes.', ), ); }, 300000); // 5 minute timeout + authTimeoutId.unref(); + + const onAbort = () => { + rl.close(); + reject(abortController.signal.reason); + }; + abortController.signal.addEventListener('abort', onAbort, { once: true }); rl.question('Enter the authorization code: ', (code) => { - clearTimeout(timeout); + abortController.signal.removeEventListener('abort', onAbort); rl.close(); resolve(code.trim()); }); + }).finally(() => { + if (authTimeoutId) clearTimeout(authTimeoutId); }); if (!code) { diff --git a/packages/core/src/mcp/oauth-provider.test.ts b/packages/core/src/mcp/oauth-provider.test.ts index 5cd4460e97..251ccb4a5e 100644 --- a/packages/core/src/mcp/oauth-provider.test.ts +++ b/packages/core/src/mcp/oauth-provider.test.ts @@ -1023,31 +1023,35 @@ describe('MCPOAuthProvider', () => { }); it('should handle callback timeout', async () => { - vi.mocked(http.createServer).mockImplementation( - () => mockHttpServer as unknown as http.Server, - ); + vi.useFakeTimers(); + try { + vi.mocked(http.createServer).mockImplementation( + () => mockHttpServer as unknown as http.Server, + ); - mockHttpServer.listen.mockImplementation((port, callback) => { - callback?.(); - // Don't trigger callback - simulate timeout - }); + mockHttpServer.listen.mockImplementation((port, callback) => { + callback?.(); + // Don't trigger callback - simulate timeout + }); - // Mock setTimeout to trigger timeout immediately - const originalSetTimeout = global.setTimeout; - global.setTimeout = vi.fn((callback, delay) => { - if (delay === 5 * 60 * 1000) { - // 5 minute timeout - callback(); - } - return originalSetTimeout(callback, 0); - }) as unknown as typeof setTimeout; + const authProvider = new MCPOAuthProvider(); - const authProvider = new MCPOAuthProvider(); - await expect( - authProvider.authenticate('test-server', mockConfig), - ).rejects.toThrow('OAuth callback timeout'); + const authPromise = authProvider + .authenticate('test-server', mockConfig) + .catch((e: Error) => { + if (e.message !== 'OAuth callback timeout') throw e; + return e; + }); - global.setTimeout = originalSetTimeout; + // Advance timers by 5 minutes + await vi.advanceTimersByTimeAsync(5 * 60 * 1000); + + const error = await authPromise; + expect(error).toBeInstanceOf(Error); + expect((error as Error).message).toBe('OAuth callback timeout'); + } finally { + vi.useRealTimers(); + } }); it('should use port from redirectUri if provided', async () => { diff --git a/packages/core/src/utils/oauth-flow.test.ts b/packages/core/src/utils/oauth-flow.test.ts index dee919c249..b4f28890e4 100644 --- a/packages/core/src/utils/oauth-flow.test.ts +++ b/packages/core/src/utils/oauth-flow.test.ts @@ -305,6 +305,28 @@ describe('oauth-flow', () => { 'Invalid value for OAUTH_CALLBACK_PORT', ); }); + + it('should settle on timeout without keeping the process alive', async () => { + vi.useFakeTimers(); + try { + const server = startCallbackServer('timeout-state'); + await server.port; + + const responsePromise = server.response.catch((e: Error) => { + if (e.message !== 'OAuth callback timeout') throw e; + return e; + }); + + // Advance timers by 5 minutes to trigger the timeout + await vi.advanceTimersByTimeAsync(5 * 60 * 1000); + + const error = await responsePromise; + expect(error).toBeInstanceOf(Error); + expect((error as Error).message).toBe('OAuth callback timeout'); + } finally { + vi.useRealTimers(); + } + }); }); describe('exchangeCodeForToken', () => { diff --git a/packages/core/src/utils/oauth-flow.ts b/packages/core/src/utils/oauth-flow.ts index e13fd37837..67062c9ec5 100644 --- a/packages/core/src/utils/oauth-flow.ts +++ b/packages/core/src/utils/oauth-flow.ts @@ -116,6 +116,8 @@ export function startCallbackServer( portReject = reject; }); + let timeoutId: NodeJS.Timeout | undefined; + const responsePromise = new Promise( (resolve, reject) => { let serverPort: number; @@ -221,18 +223,31 @@ export function startCallbackServer( portResolve(serverPort); // Resolve port promise immediately }); - // Timeout after 5 minutes - setTimeout( + const abortController = new AbortController(); + timeoutId = setTimeout( () => { - server.close(); - reject(new Error('OAuth callback timeout')); + abortController.abort(new Error('OAuth callback timeout')); }, 5 * 60 * 1000, ); + timeoutId.unref(); + + const onAbort = () => { + server.close(); + reject(abortController.signal.reason); + }; + abortController.signal.addEventListener('abort', onAbort, { once: true }); + + server.on('close', () => { + abortController.signal.removeEventListener('abort', onAbort); + }); }, ); - return { port: portPromise, response: responsePromise }; + return { + port: portPromise, + response: responsePromise, + }; } /**