From 472497fb0324a92993b2a7e7fd27c8f071a9e7c6 Mon Sep 17 00:00:00 2001
From: kaitranntt <kaitran.ntt@gmail.com>
Date: Tue, 6 Jan 2026 11:31:40 -0500
Subject: [PATCH] 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)
---
 src/cliproxy/auth-session-manager.ts | 30 ++++++++++++++++++++++++++++
 src/cliproxy/auth/oauth-process.ts   | 18 ++++++++++++++---
 src/utils/port-utils.ts              | 17 +++++++++-------
 3 files changed, 55 insertions(+), 10 deletions(-)

diff --git a/src/cliproxy/auth-session-manager.ts b/src/cliproxy/auth-session-manager.ts
index ce718be9..0ad0dd7c 100644
--- a/src/cliproxy/auth-session-manager.ts
+++ b/src/cliproxy/auth-session-manager.ts
@@ -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);
 }
 
diff --git a/src/cliproxy/auth/oauth-process.ts b/src/cliproxy/auth/oauth-process.ts
index 2eb1cbdb..6b5cb997 100644
--- a/src/cliproxy/auth/oauth-process.ts
+++ b/src/cliproxy/auth/oauth-process.ts
@@ -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);
diff --git a/src/utils/port-utils.ts b/src/utils/port-utils.ts
index 31f87208..4ddb9313 100644
--- a/src/utils/port-utils.ts
+++ b/src/utils/port-utils.ts
@@ -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', () => {