fix(oauth): harden cleanup for edge cases in auth process

- Clear stdinKeepalive interval on SIGINT/SIGTERM signal handlers
- Add cancelProjectSelection() to timeout/exit/error handlers
- Close server on error path in testLocalhostBinding to prevent fd leak
- Add TTL-based cleanup for stale auth sessions (10 min expiry)
- Use DEVICE_CODE_TIMEOUT_MS constant instead of hardcoded value (DRY)
This commit is contained in:
kaitranntt committed 2026-01-06 11:31:40 -05:00
1 parent 0557f93f2f
commit 472497fb03
3 files changed
+55 -10

No files matched your search

+30
View File
@@ -8,6 +8,9 @@
import { EventEmitter } from 'events'; import { EventEmitter } from 'events';
import { ChildProcess } from 'child_process'; import { ChildProcess } from 'child_process';
// H8: TTL for stale session cleanup (10 minutes - generous for OAuth flows)
const SESSION_TTL_MS = 10 * 60 * 1000;
export interface ActiveAuthSession { export interface ActiveAuthSession {
sessionId: string; sessionId: string;
provider: string; provider: string;
@@ -19,6 +22,31 @@ export const authSessionEvents = new EventEmitter();
const activeSessions = new Map<string, ActiveAuthSession>(); const activeSessions = new Map<string, ActiveAuthSession>();
// H8: Periodic cleanup of stale sessions (prevents memory leak from orphaned sessions)
let cleanupInterval: ReturnType<typeof setInterval> | null = null;
function startCleanupInterval(): void {
if (cleanupInterval) return;
cleanupInterval = setInterval(() => {
const now = Date.now();
for (const [sessionId, session] of activeSessions.entries()) {
if (now - session.startedAt > SESSION_TTL_MS) {
// Stale session - kill process if still running, then remove
if (session.process && !session.process.killed) {
session.process.kill('SIGTERM');
}
activeSessions.delete(sessionId);
authSessionEvents.emit('session:expired', sessionId);
}
}
// Stop interval if no active sessions
if (activeSessions.size === 0 && cleanupInterval) {
clearInterval(cleanupInterval);
cleanupInterval = null;
}
}, 60000); // Check every minute
}
/** /**
* Register an active OAuth session * Register an active OAuth session
*/ */
@@ -33,6 +61,8 @@ export function registerAuthSession(
startedAt: Date.now(), startedAt: Date.now(),
process, process,
}); });
// H8: Start TTL cleanup when first session registered
startCleanupInterval();
authSessionEvents.emit('session:started', sessionId, provider); authSessionEvents.emit('session:started', sessionId, provider);
} }
+15 -3
View File
@@ -17,13 +17,18 @@ import {
isProjectList, isProjectList,
generateSessionId, generateSessionId,
requestProjectSelection, requestProjectSelection,
cancelProjectSelection,
type GCloudProject, type GCloudProject,
type ProjectSelectionPrompt, type ProjectSelectionPrompt,
} from '../project-selection-handler'; } from '../project-selection-handler';
import { ProviderOAuthConfig } from './auth-types'; import { ProviderOAuthConfig } from './auth-types';
import { getTimeoutTroubleshooting, showStep } from './environment-detector'; import { getTimeoutTroubleshooting, showStep } from './environment-detector';
import { isAuthenticated, registerAccountFromToken } from './token-manager'; import { isAuthenticated, registerAccountFromToken } from './token-manager';
import { deviceCodeEvents, type DeviceCodePrompt } from '../device-code-handler'; import {
deviceCodeEvents,
DEVICE_CODE_TIMEOUT_MS,
type DeviceCodePrompt,
} from '../device-code-handler';
import { OAUTH_FLOW_TYPES } from '../../management'; import { OAUTH_FLOW_TYPES } from '../../management';
import { import {
registerAuthSession, registerAuthSession,
@@ -150,7 +155,7 @@ async function handleStdout(
provider: options.provider, provider: options.provider,
userCode: state.userCode, userCode: state.userCode,
verificationUrl, verificationUrl,
expiresAt: Date.now() + 900000, // 15 minutes expiresAt: Date.now() + DEVICE_CODE_TIMEOUT_MS,
}; };
deviceCodeEvents.emit('deviceCode:received', deviceCodePrompt); deviceCodeEvents.emit('deviceCode:received', deviceCodePrompt);
@@ -311,8 +316,13 @@ export function executeOAuthProcess(options: OAuthProcessOptions): Promise<Accou
env: { ...process.env, CLI_PROXY_AUTH_DIR: tokenDir }, env: { ...process.env, CLI_PROXY_AUTH_DIR: tokenDir },
}); });
// H7: Mutable ref for stdin keepalive interval (set later, needed in cleanup)
let stdinKeepalive: ReturnType<typeof setInterval> | null = null;
// H5: Signal handling - properly kill child process on SIGINT/SIGTERM // H5: Signal handling - properly kill child process on SIGINT/SIGTERM
// H8: Also clear stdinKeepalive interval to prevent memory leak
const cleanup = () => { const cleanup = () => {
if (stdinKeepalive) clearInterval(stdinKeepalive);
if (authProcess && !authProcess.killed) { if (authProcess && !authProcess.killed) {
authProcess.kill('SIGTERM'); authProcess.kill('SIGTERM');
} }
@@ -352,7 +362,6 @@ export function executeOAuthProcess(options: OAuthProcessOptions): Promise<Accou
// If the user completes browser auth after this timer fires but before the // If the user completes browser auth after this timer fires but before the
// non-blocking check, the prompt blocks forever on stdin. // non-blocking check, the prompt blocks forever on stdin.
// Workaround: Send newline every 16s to skip the manual prompt and continue polling. // Workaround: Send newline every 16s to skip the manual prompt and continue polling.
let stdinKeepalive: ReturnType<typeof setInterval> | null = null;
if (!isDeviceCodeFlow && stdinMode === 'pipe') { if (!isDeviceCodeFlow && stdinMode === 'pipe') {
stdinKeepalive = setInterval(() => { stdinKeepalive = setInterval(() => {
if (authProcess.stdin && !authProcess.stdin.destroyed) { if (authProcess.stdin && !authProcess.stdin.destroyed) {
@@ -415,6 +424,7 @@ export function executeOAuthProcess(options: OAuthProcessOptions): Promise<Accou
process.removeListener('SIGTERM', cleanup); process.removeListener('SIGTERM', cleanup);
authSessionEvents.removeListener('session:cancelled', handleCancel); authSessionEvents.removeListener('session:cancelled', handleCancel);
unregisterAuthSession(state.sessionId); unregisterAuthSession(state.sessionId);
cancelProjectSelection(state.sessionId);
authProcess.kill(); authProcess.kill();
console.log(''); console.log('');
console.log(fail(`OAuth timed out after ${headless ? 5 : 2} minutes`)); console.log(fail(`OAuth timed out after ${headless ? 5 : 2} minutes`));
@@ -433,6 +443,7 @@ export function executeOAuthProcess(options: OAuthProcessOptions): Promise<Accou
process.removeListener('SIGTERM', cleanup); process.removeListener('SIGTERM', cleanup);
authSessionEvents.removeListener('session:cancelled', handleCancel); authSessionEvents.removeListener('session:cancelled', handleCancel);
unregisterAuthSession(state.sessionId); unregisterAuthSession(state.sessionId);
cancelProjectSelection(state.sessionId);
const elapsed = ((Date.now() - startTime) / 1000).toFixed(1); const elapsed = ((Date.now() - startTime) / 1000).toFixed(1);
if (code === 0) { if (code === 0) {
@@ -488,6 +499,7 @@ export function executeOAuthProcess(options: OAuthProcessOptions): Promise<Accou
process.removeListener('SIGTERM', cleanup); process.removeListener('SIGTERM', cleanup);
authSessionEvents.removeListener('session:cancelled', handleCancel); authSessionEvents.removeListener('session:cancelled', handleCancel);
unregisterAuthSession(state.sessionId); unregisterAuthSession(state.sessionId);
cancelProjectSelection(state.sessionId);
console.log(''); console.log('');
console.log(fail(`Failed to start auth process: ${error.message}`)); console.log(fail(`Failed to start auth process: ${error.message}`));
resolve(null); resolve(null);
+10 -7
View File
@@ -218,13 +218,16 @@ export async function testLocalhostBinding(port: number): Promise<BindingTestRes
const server = net.createServer(); const server = net.createServer();
server.once('error', (err: NodeJS.ErrnoException) => { server.once('error', (err: NodeJS.ErrnoException) => {
if (err.code === 'EADDRINUSE') { // H8: Close server to prevent fd leak on error path
resolve({ success: false, message: `Port ${port} is already in use` }); server.close(() => {
} else if (err.code === 'EACCES') { if (err.code === 'EADDRINUSE') {
resolve({ success: false, message: `Permission denied for port ${port}` }); resolve({ success: false, message: `Port ${port} is already in use` });
} else { } else if (err.code === 'EACCES') {
resolve({ success: false, message: `Cannot bind to port ${port}: ${err.message}` }); resolve({ success: false, message: `Permission denied for port ${port}` });
} } else {
resolve({ success: false, message: `Cannot bind to port ${port}: ${err.message}` });
}
});
}); });
server.once('listening', () => { server.once('listening', () => {