From 5d27f10367f2e869079c6850e8b24aa2b98242f1 Mon Sep 17 00:00:00 2001 From: "Kai (Tam Nhu) Tran" <61256810+kaitranntt@users.noreply.github.com> Date: Sat, 23 May 2026 17:29:18 -0400 Subject: [PATCH] refactor(cliproxy): remove orphaned bg keepalive wiring (#1353) Red-team review of #1340 found that backgroundProbe / shouldKeepSessionProxiesAlive / startKeepAliveWatcher and related types were never reachable: the sole caller (claude-launcher.ts) passed no hasBackgroundWorkerUsingBaseUrl detector, so backgroundProbe was always undefined and the keepalive branch never executed. Remove the unreachable scaffold: - Delete CLAUDE_BG_WORKER_ARGS, hasClaudeBackgroundWorkerUsingBaseUrl{,InProcessList}, shouldKeepSessionProxiesAlive, ShouldKeepSessionProxiesAliveOptions, and the SessionProxyKeepAliveOptions interface from session-bridge.ts - Remove the backgroundProbe/startKeepAliveWatcher block and keepalive exit branch from setupCleanupHandlers; the function now always calls stopSessionResources on Claude exit - Remove backgroundKeepAliveBaseUrl wiring from claude-launcher.ts - Remove the execFileSync import (was only used by the deleted ps-based detector) - Update test file: remove tests for deleted exports, keep placeholder A trusted bg-worker detector can be re-added in a future PR if the feature is actually needed; the keepalive logic in shouldKeepSessionProxiesAlive can be restored then. --- src/cliproxy/executor/claude-launcher.ts | 6 +- src/cliproxy/executor/session-bridge.ts | 101 +----------------- .../session-bridge-bg-keepalive.test.ts | 66 +++--------- 3 files changed, 16 insertions(+), 157 deletions(-) diff --git a/src/cliproxy/executor/claude-launcher.ts b/src/cliproxy/executor/claude-launcher.ts index 66663f05..ab74f0a4 100644 --- a/src/cliproxy/executor/claude-launcher.ts +++ b/src/cliproxy/executor/claude-launcher.ts @@ -163,9 +163,6 @@ export async function launchClaude(context: ClaudeLaunchContext): Promise boolean; -} - -export interface ShouldKeepSessionProxiesAliveOptions { - code: number | null; - signal: NodeJS.Signals | null; - backgroundKeepAliveBaseUrl?: string; - hasBackgroundWorkerUsingBaseUrl: (baseUrl: string) => boolean; -} - -const CLAUDE_BG_WORKER_ARGS = ['--bg-spare', '--bg-pty-host'] as const; - -export function hasClaudeBackgroundWorkerUsingBaseUrlInProcessList( - processList: string, - baseUrl: string -): boolean { - const envNeedle = `ANTHROPIC_BASE_URL=${baseUrl}`; - return processList - .split(/\r?\n/) - .some( - (line) => line.includes(envNeedle) && CLAUDE_BG_WORKER_ARGS.some((arg) => line.includes(arg)) - ); -} - -export function hasClaudeBackgroundWorkerUsingBaseUrl(baseUrl: string): boolean { - if (process.platform === 'win32') { - return false; - } - - for (const args of [['auxEww'], ['auxeww'], ['eww', '-axo', 'pid=,command=']]) { - try { - const processList = execFileSync('ps', args, { - encoding: 'utf8', - stdio: ['ignore', 'pipe', 'ignore'], - }); - if (hasClaudeBackgroundWorkerUsingBaseUrlInProcessList(processList, baseUrl)) { - return true; - } - } catch { - continue; - } - } - - return false; -} - -export function shouldKeepSessionProxiesAlive( - options: ShouldKeepSessionProxiesAliveOptions -): boolean { - if (options.code !== 0 || options.signal) { - return false; - } - - const baseUrl = options.backgroundKeepAliveBaseUrl; - if (!baseUrl) { - return false; - } - - return options.hasBackgroundWorkerUsingBaseUrl(baseUrl); -} - /** * Check for existing proxy and handle version mismatch, or determine if new spawn needed */ @@ -244,8 +180,7 @@ export function setupCleanupHandlers( codexReasoningProxy: unknown, toolSanitizationProxy: unknown, httpsTunnel: unknown, - verbose: boolean, - keepAliveOptions: SessionProxyKeepAliveOptions = {} + verbose: boolean ): void { const log = (msg: string) => { if (verbose) { @@ -275,20 +210,6 @@ export function setupCleanupHandlers( } }; - // Security hardening: only allow keepalive when caller provides an explicit, - // trusted detector. Falling back to global process-list probing is spoofable. - const backgroundProbe = keepAliveOptions.hasBackgroundWorkerUsingBaseUrl; - - const startKeepAliveWatcher = (baseUrl: string) => { - const timer = setInterval(() => { - if (backgroundProbe?.(baseUrl)) return; - log('No Claude bg worker is using the session proxy; cleaning up'); - stopSessionResources(); - process.exit(0); - }, keepAliveOptions.keepAlivePollIntervalMs ?? 30000); - timer.unref(); - }; - const cleanup = () => { log('Parent signal received, cleaning up'); stopSessionResources(); @@ -297,24 +218,6 @@ export function setupCleanupHandlers( claude.on('exit', (code, signal) => { log(`Claude exited: code=${code}, signal=${signal}`); - const backgroundKeepAliveBaseUrl = keepAliveOptions.backgroundKeepAliveBaseUrl; - - if ( - backgroundProbe && - shouldKeepSessionProxiesAlive({ - code, - signal, - backgroundKeepAliveBaseUrl, - hasBackgroundWorkerUsingBaseUrl: backgroundProbe, - }) && - backgroundKeepAliveBaseUrl - ) { - stopQuotaMonitor(); - log(`Keeping session proxy alive for Claude bg worker: ${backgroundKeepAliveBaseUrl}`); - startKeepAliveWatcher(backgroundKeepAliveBaseUrl); - return; - } - stopSessionResources(); if (signal) { diff --git a/tests/unit/cliproxy/session-bridge-bg-keepalive.test.ts b/tests/unit/cliproxy/session-bridge-bg-keepalive.test.ts index f911be55..9d588987 100644 --- a/tests/unit/cliproxy/session-bridge-bg-keepalive.test.ts +++ b/tests/unit/cliproxy/session-bridge-bg-keepalive.test.ts @@ -1,57 +1,17 @@ -import { describe, it, expect } from 'bun:test'; -import { - hasClaudeBackgroundWorkerUsingBaseUrlInProcessList, - shouldKeepSessionProxiesAlive, -} from '../../../src/cliproxy/executor/session-bridge'; +/** + * The bg-keepalive feature (shouldKeepSessionProxiesAlive, hasClaudeBackgroundWorker*, + * backgroundProbe) was removed as dead code: the sole caller (claude-launcher.ts) never + * provided a trusted hasBackgroundWorkerUsingBaseUrl detector, so the keepalive path was + * unreachable. The ps-based detector was also spoofable (fixed in #1340). This test file + * is kept as a placeholder to prevent the test suite from failing on missing imports. + * + * If a trusted bg-worker detector is implemented in the future, new tests belong here. + */ + +import { describe, it } from 'bun:test'; describe('session bridge background proxy keepalive', () => { - it('keeps per-session proxies alive after a clean Claude exit when a bg worker inherited the base URL', () => { - expect( - shouldKeepSessionProxiesAlive({ - code: 0, - signal: null, - backgroundKeepAliveBaseUrl: 'http://127.0.0.1:64314/api/provider/codex', - hasBackgroundWorkerUsingBaseUrl: () => true, - }) - ).toBe(true); - }); - - it('cleans up per-session proxies when no bg worker inherited the base URL', () => { - expect( - shouldKeepSessionProxiesAlive({ - code: 0, - signal: null, - backgroundKeepAliveBaseUrl: 'http://127.0.0.1:64314/api/provider/codex', - hasBackgroundWorkerUsingBaseUrl: () => false, - }) - ).toBe(false); - }); - - it('cleans up per-session proxies for signaled Claude exits', () => { - expect( - shouldKeepSessionProxiesAlive({ - code: null, - signal: 'SIGTERM', - backgroundKeepAliveBaseUrl: 'http://127.0.0.1:64314/api/provider/codex', - hasBackgroundWorkerUsingBaseUrl: () => true, - }) - ).toBe(false); - }); - - it('detects Claude bg workers that inherited the exact base URL', () => { - const baseUrl = 'http://127.0.0.1:64314/api/provider/codex'; - const processList = ` -123 /opt/claude/claude.exe --bg-spare /tmp/claim.sock ANTHROPIC_BASE_URL=${baseUrl} ANTHROPIC_AUTH_TOKEN=ccs-internal-managed -456 /opt/claude/claude.exe --bg-spare /tmp/claim.sock ANTHROPIC_BASE_URL=http://127.0.0.1:62075/api/provider/codex -789 /opt/claude/claude.exe --print ANTHROPIC_BASE_URL=${baseUrl} -`; - - expect(hasClaudeBackgroundWorkerUsingBaseUrlInProcessList(processList, baseUrl)).toBe(true); - expect( - hasClaudeBackgroundWorkerUsingBaseUrlInProcessList( - processList, - 'http://127.0.0.1:65535/api/provider/codex' - ) - ).toBe(false); + it('bg keepalive wiring removed — no dead code to test', () => { + // No-op: feature scaffold removed per red-team finding on #1340. }); });