From 454e1555a7b1d50f77d77c2fd7b6690930e52260 Mon Sep 17 00:00:00 2001 From: Tam Nhu Tran Date: Sat, 27 Jun 2026 10:54:50 -0400 Subject: [PATCH 1/3] fix(cliproxy): clean up launch-settings overlay on synchronous spawn failure The runtime settings overlay cleanup was registered only on the child 'exit'/'error' events, which run after spawn() returns. A synchronous spawn() throw (e.g. invalid arg/env) propagated out of launchClaude before those handlers were wired, orphaning the secret-bearing 0600 overlay file in os.tmpdir(). Wrap the spawn in try/catch and run the idempotent cleanup before rethrowing. --- .../__tests__/claude-launcher.test.ts | 26 ++++++++++++++ src/cliproxy/executor/claude-launcher.ts | 36 +++++++++++-------- 2 files changed, 48 insertions(+), 14 deletions(-) diff --git a/src/cliproxy/executor/__tests__/claude-launcher.test.ts b/src/cliproxy/executor/__tests__/claude-launcher.test.ts index e6111e50..3e587ca9 100644 --- a/src/cliproxy/executor/__tests__/claude-launcher.test.ts +++ b/src/cliproxy/executor/__tests__/claude-launcher.test.ts @@ -87,6 +87,16 @@ mock.module('../../quota/quota-manager', () => ({ stopQuotaMonitor: jest.fn(), })); +const mockCleanupLaunchSettings = jest.fn(); +const mockPrepareLaunchSettings = jest.fn().mockReturnValue({ + settingsPath: '/tmp/fake-settings-overlay.json', + cleanup: mockCleanupLaunchSettings, +}); + +mock.module('../launch-settings', () => ({ + prepareLaunchSettings: mockPrepareLaunchSettings, +})); + // ── Subject under test ──────────────────────────────────────────────────────── import { launchClaude } from '../claude-launcher'; @@ -131,6 +141,12 @@ describe('launchClaude', () => { mockSpawn.mockClear(); mockSetupCleanupHandlers.mockClear(); mockEscapeShellArg.mockClear(); + mockCleanupLaunchSettings.mockClear(); + mockPrepareLaunchSettings.mockClear(); + mockPrepareLaunchSettings.mockReturnValue({ + settingsPath: '/tmp/fake-settings-overlay.json', + cleanup: mockCleanupLaunchSettings, + }); }); it('calls spawn with claudeCli and includes --settings arg', async () => { @@ -189,6 +205,16 @@ describe('launchClaude', () => { expect(result).toBe(mockSpawnResult); }); + it('calls cleanup and rethrows when spawn throws synchronously', async () => { + const spawnErr = new Error('ERR_INVALID_ARG_VALUE'); + mockSpawn.mockImplementationOnce(() => { + throw spawnErr; + }); + + await expect(launchClaude(baseContext())).rejects.toThrow('ERR_INVALID_ARG_VALUE'); + expect(mockCleanupLaunchSettings).toHaveBeenCalledTimes(1); + }); + describe('Windows shell escaping', () => { const originalPlatform = process.platform; diff --git a/src/cliproxy/executor/claude-launcher.ts b/src/cliproxy/executor/claude-launcher.ts index ca539971..ff189bbf 100644 --- a/src/cliproxy/executor/claude-launcher.ts +++ b/src/cliproxy/executor/claude-launcher.ts @@ -144,20 +144,28 @@ export async function launchClaude(context: ClaudeLaunchContext): Promise Date: Sat, 27 Jun 2026 10:55:00 -0400 Subject: [PATCH 2/3] test(cliproxy): cover launch-settings corrupt-JSON fallback and overlay permissions Add a test for the env-only overlay fallback when the persisted settings file is corrupt (the existing 'missing file' test skips JSON.parse), and a POSIX-gated assertion that the overlay is written 0600 inside a 0700 dir, since it carries an auth token. --- .../__tests__/launch-settings.test.ts | 27 +++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/src/cliproxy/executor/__tests__/launch-settings.test.ts b/src/cliproxy/executor/__tests__/launch-settings.test.ts index 6865cedf..b08d27bc 100644 --- a/src/cliproxy/executor/__tests__/launch-settings.test.ts +++ b/src/cliproxy/executor/__tests__/launch-settings.test.ts @@ -91,6 +91,17 @@ describe('buildLaunchSettingsOverlay', () => { 'http://127.0.0.1:50118/api/provider/codex' ); }); + + it('falls back to an env-only overlay when the settings file is corrupt', () => { + fs.writeFileSync(settingsPath, '{ not valid json'); + const { settings, changed } = buildLaunchSettingsOverlay(settingsPath, { + ANTHROPIC_BASE_URL: 'http://127.0.0.1:50118/api/provider/codex', + } as NodeJS.ProcessEnv); + expect(changed).toBe(true); + expect((settings.env as Record).ANTHROPIC_BASE_URL).toBe( + 'http://127.0.0.1:50118/api/provider/codex' + ); + }); }); describe('prepareLaunchSettings', () => { @@ -127,4 +138,20 @@ describe('prepareLaunchSettings', () => { result.cleanup(); expect(fs.existsSync(settingsPath)).toBe(true); }); + + it('writes the overlay with 0600 file mode inside a 0700 dir (POSIX)', () => { + if (process.platform === 'win32') return; // chmod modes are not enforced on Windows + writePersisted({ + env: { ANTHROPIC_BASE_URL: 'http://127.0.0.1:8317/api/provider/codex' }, + }); + const result = prepareLaunchSettings(settingsPath, { + ANTHROPIC_BASE_URL: 'http://127.0.0.1:50118/api/provider/codex', + } as NodeJS.ProcessEnv); + try { + expect(fs.statSync(result.settingsPath).mode & 0o777).toBe(0o600); + expect(fs.statSync(path.dirname(result.settingsPath)).mode & 0o777).toBe(0o700); + } finally { + result.cleanup(); + } + }); }); From 13e23e0fc5b55c10af16e00a82535ca8c71a161f Mon Sep 17 00:00:00 2001 From: Tam Nhu Tran Date: Sat, 27 Jun 2026 10:55:11 -0400 Subject: [PATCH 3/3] docs(cliproxy): document strip-vs-overlay launch-settings siblings and model-key superset Cross-reference the two helpers that solve the persisted-settings-env override (overlay in launch-settings.ts, strip in openai-compat-launch-settings.ts) and mark shell-executor's ANTHROPIC_MODEL_ENV_KEYS as the intentional superset distinct from the 4-key list in extended-context-utils. See issue #1609. --- src/cliproxy/executor/launch-settings.ts | 7 +++++++ src/utils/openai-compat-launch-settings.ts | 6 ++++++ src/utils/shell-executor.ts | 4 ++++ 3 files changed, 17 insertions(+) diff --git a/src/cliproxy/executor/launch-settings.ts b/src/cliproxy/executor/launch-settings.ts index c834e1ae..5e33bf7e 100644 --- a/src/cliproxy/executor/launch-settings.ts +++ b/src/cliproxy/executor/launch-settings.ts @@ -27,6 +27,13 @@ import * as path from 'path'; import { ANTHROPIC_MODEL_ENV_KEYS, ANTHROPIC_ROUTING_ENV_KEYS } from '../../utils/shell-executor'; +// SIBLING HELPER: src/utils/openai-compat-launch-settings.ts solves the same +// "persisted --settings env clobbers runtime routing env" problem by STRIPPING +// routing keys (process env wins by absence). This module instead OVERLAYS the +// resolved values (settings wins by overwrite). The two differ intentionally: +// strip vs overlay diverge when a key is present on disk but absent from the +// process env. Unifying them needs an explicit force-absent API — see issue #1609. + /** * Environment keys that control provider routing/model selection and are read * by Claude from the settings `env` block. These must reflect the resolved diff --git a/src/utils/openai-compat-launch-settings.ts b/src/utils/openai-compat-launch-settings.ts index bfb0887d..1f305419 100644 --- a/src/utils/openai-compat-launch-settings.ts +++ b/src/utils/openai-compat-launch-settings.ts @@ -10,6 +10,12 @@ export interface OpenAICompatLaunchSettings { cleanup: () => void; } +// SIBLING HELPER: src/cliproxy/executor/launch-settings.ts (prepareLaunchSettings) +// solves the same problem by OVERLAYING resolved routing values instead of +// stripping them. This strip-based variant is required where callers deliberately +// delete a routing key (e.g. ANTHROPIC_API_KEY in settings-flow) and need it +// ABSENT from the launch settings. Do not unify without an explicit force-absent +// key list — see issue #1609. export function createOpenAICompatLaunchSettings( settingsPath: string, settings: Settings diff --git a/src/utils/shell-executor.ts b/src/utils/shell-executor.ts index 58d459f6..68a9f1f6 100644 --- a/src/utils/shell-executor.ts +++ b/src/utils/shell-executor.ts @@ -40,6 +40,10 @@ export const ANTHROPIC_ROUTING_ENV_KEYS = [ 'ANTHROPIC_API_KEY', ]; const ANTHROPIC_ROUTING_ENV_KEY_SET = new Set(ANTHROPIC_ROUTING_ENV_KEYS); +// NOTE: This is the intentional routing-overlay SUPERSET of model env keys +// (includes ANTHROPIC_SMALL_FAST_MODEL). A separate 4-key `ANTHROPIC_MODEL_ENV_KEYS` +// exists in src/shared/extended-context-utils.ts (re-exported as MODEL_ENV_VAR_KEYS). +// The two are NOT interchangeable — import deliberately by purpose. See issue #1609. export const ANTHROPIC_MODEL_ENV_KEYS = [ 'ANTHROPIC_MODEL', 'ANTHROPIC_DEFAULT_OPUS_MODEL',