fix(core): clear 5-minute timeouts in oauth flow to prevent memory leaks (#24968)

This commit is contained in:
Spencer
2026-04-09 17:14:07 -04:00
committed by GitHub
parent f744913584
commit 0f7f7be4ef
4 changed files with 81 additions and 30 deletions
+14 -4
View File
@@ -424,6 +424,7 @@ async function authWithUserCode(client: OAuth2Client): Promise<boolean> {
'\n\n', '\n\n',
); );
let authTimeoutId: NodeJS.Timeout | undefined;
const code = await new Promise<string>((resolve, reject) => { const code = await new Promise<string>((resolve, reject) => {
const rl = readline.createInterface({ const rl = readline.createInterface({
input: process.stdin, input: process.stdin,
@@ -431,20 +432,29 @@ async function authWithUserCode(client: OAuth2Client): Promise<boolean> {
terminal: true, terminal: true,
}); });
const timeout = setTimeout(() => { const abortController = new AbortController();
rl.close(); authTimeoutId = setTimeout(() => {
reject( abortController.abort(
new FatalAuthenticationError( new FatalAuthenticationError(
'Authorization timed out after 5 minutes.', 'Authorization timed out after 5 minutes.',
), ),
); );
}, 300000); // 5 minute timeout }, 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) => { rl.question('Enter the authorization code: ', (code) => {
clearTimeout(timeout); abortController.signal.removeEventListener('abort', onAbort);
rl.close(); rl.close();
resolve(code.trim()); resolve(code.trim());
}); });
}).finally(() => {
if (authTimeoutId) clearTimeout(authTimeoutId);
}); });
if (!code) { if (!code) {
+25 -21
View File
@@ -1023,31 +1023,35 @@ describe('MCPOAuthProvider', () => {
}); });
it('should handle callback timeout', async () => { it('should handle callback timeout', async () => {
vi.mocked(http.createServer).mockImplementation( vi.useFakeTimers();
() => mockHttpServer as unknown as http.Server, try {
); vi.mocked(http.createServer).mockImplementation(
() => mockHttpServer as unknown as http.Server,
);
mockHttpServer.listen.mockImplementation((port, callback) => { mockHttpServer.listen.mockImplementation((port, callback) => {
callback?.(); callback?.();
// Don't trigger callback - simulate timeout // Don't trigger callback - simulate timeout
}); });
// Mock setTimeout to trigger timeout immediately const authProvider = new MCPOAuthProvider();
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 authPromise = authProvider
await expect( .authenticate('test-server', mockConfig)
authProvider.authenticate('test-server', mockConfig), .catch((e: Error) => {
).rejects.toThrow('OAuth callback timeout'); 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 () => { it('should use port from redirectUri if provided', async () => {
@@ -305,6 +305,28 @@ describe('oauth-flow', () => {
'Invalid value for OAUTH_CALLBACK_PORT', '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', () => { describe('exchangeCodeForToken', () => {
+20 -5
View File
@@ -116,6 +116,8 @@ export function startCallbackServer(
portReject = reject; portReject = reject;
}); });
let timeoutId: NodeJS.Timeout | undefined;
const responsePromise = new Promise<OAuthAuthorizationResponse>( const responsePromise = new Promise<OAuthAuthorizationResponse>(
(resolve, reject) => { (resolve, reject) => {
let serverPort: number; let serverPort: number;
@@ -221,18 +223,31 @@ export function startCallbackServer(
portResolve(serverPort); // Resolve port promise immediately portResolve(serverPort); // Resolve port promise immediately
}); });
// Timeout after 5 minutes const abortController = new AbortController();
setTimeout( timeoutId = setTimeout(
() => { () => {
server.close(); abortController.abort(new Error('OAuth callback timeout'));
reject(new Error('OAuth callback timeout'));
}, },
5 * 60 * 1000, 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,
};
} }
/** /**