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
2026-01-06 11:31:40 -05:00
parent 0557f93f2f
commit 472497fb03
3 changed files with 55 additions and 10 deletions
+30
View File
@@ -8,6 +8,9 @@
import { EventEmitter } from 'events';
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 {
sessionId: string;
provider: string;
@@ -19,6 +22,31 @@ export const authSessionEvents = new EventEmitter();
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
*/
@@ -33,6 +61,8 @@ export function registerAuthSession(
startedAt: Date.now(),
process,
});
// H8: Start TTL cleanup when first session registered
startCleanupInterval();
authSessionEvents.emit('session:started', sessionId, provider);
}
+15 -3
View File
@@ -17,13 +17,18 @@ import {
isProjectList,
generateSessionId,
requestProjectSelection,
cancelProjectSelection,
type GCloudProject,
type ProjectSelectionPrompt,
} from '../project-selection-handler';
import { ProviderOAuthConfig } from './auth-types';
import { getTimeoutTroubleshooting, showStep } from './environment-detector';
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 {
registerAuthSession,
@@ -150,7 +155,7 @@ async function handleStdout(
provider: options.provider,
userCode: state.userCode,
verificationUrl,
expiresAt: Date.now() + 900000, // 15 minutes
expiresAt: Date.now() + DEVICE_CODE_TIMEOUT_MS,
};
deviceCodeEvents.emit('deviceCode:received', deviceCodePrompt);
@@ -311,8 +316,13 @@ export function executeOAuthProcess(options: OAuthProcessOptions): Promise<Accou
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
// H8: Also clear stdinKeepalive interval to prevent memory leak
const cleanup = () => {
if (stdinKeepalive) clearInterval(stdinKeepalive);
if (authProcess && !authProcess.killed) {
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
// non-blocking check, the prompt blocks forever on stdin.
// Workaround: Send newline every 16s to skip the manual prompt and continue polling.
let stdinKeepalive: ReturnType<typeof setInterval> | null = null;
if (!isDeviceCodeFlow && stdinMode === 'pipe') {
stdinKeepalive = setInterval(() => {
if (authProcess.stdin && !authProcess.stdin.destroyed) {
@@ -415,6 +424,7 @@ export function executeOAuthProcess(options: OAuthProcessOptions): Promise<Accou
process.removeListener('SIGTERM', cleanup);
authSessionEvents.removeListener('session:cancelled', handleCancel);
unregisterAuthSession(state.sessionId);
cancelProjectSelection(state.sessionId);
authProcess.kill();
console.log('');
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);
authSessionEvents.removeListener('session:cancelled', handleCancel);
unregisterAuthSession(state.sessionId);
cancelProjectSelection(state.sessionId);
const elapsed = ((Date.now() - startTime) / 1000).toFixed(1);
if (code === 0) {
@@ -488,6 +499,7 @@ export function executeOAuthProcess(options: OAuthProcessOptions): Promise<Accou
process.removeListener('SIGTERM', cleanup);
authSessionEvents.removeListener('session:cancelled', handleCancel);
unregisterAuthSession(state.sessionId);
cancelProjectSelection(state.sessionId);
console.log('');
console.log(fail(`Failed to start auth process: ${error.message}`));
resolve(null);
+10 -7
View File
@@ -218,13 +218,16 @@ export async function testLocalhostBinding(port: number): Promise<BindingTestRes
const server = net.createServer();
server.once('error', (err: NodeJS.ErrnoException) => {
if (err.code === 'EADDRINUSE') {
resolve({ success: false, message: `Port ${port} is already in use` });
} else if (err.code === 'EACCES') {
resolve({ success: false, message: `Permission denied for port ${port}` });
} else {
resolve({ success: false, message: `Cannot bind to port ${port}: ${err.message}` });
}
// H8: Close server to prevent fd leak on error path
server.close(() => {
if (err.code === 'EADDRINUSE') {
resolve({ success: false, message: `Port ${port} is already in use` });
} else if (err.code === 'EACCES') {
resolve({ success: false, message: `Permission denied for port ${port}` });
} else {
resolve({ success: false, message: `Cannot bind to port ${port}: ${err.message}` });
}
});
});
server.once('listening', () => {