mirror of
https://github.com/tiennm99/ccs.git
synced 2026-10-06 08:13:49 +00:00
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.
This commit is contained in:
1 parent
97e16d1d1c
commit
454e1555a7
2 files changed
+48
-14
No files matched your search
@@ -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;
|
||||
|
||||
|
||||
@@ -144,20 +144,28 @@ export async function launchClaude(context: ClaudeLaunchContext): Promise<ChildP
|
||||
|
||||
// Spawn: Windows .cmd/.bat/.ps1 need shell escaping; all others spawn directly
|
||||
let claude: ChildProcess;
|
||||
if (needsShell) {
|
||||
const cmdString = [claudeCli, ...launchArgs].map(escapeShellArg).join(' ');
|
||||
claude = spawn(cmdString, {
|
||||
stdio: 'inherit',
|
||||
windowsHide: true,
|
||||
shell: getWindowsEscapedCommandShell(),
|
||||
env: tracedEnv,
|
||||
});
|
||||
} else {
|
||||
claude = spawn(claudeCli, launchArgs, {
|
||||
stdio: 'inherit',
|
||||
windowsHide: true,
|
||||
env: tracedEnv,
|
||||
});
|
||||
try {
|
||||
if (needsShell) {
|
||||
const cmdString = [claudeCli, ...launchArgs].map(escapeShellArg).join(' ');
|
||||
claude = spawn(cmdString, {
|
||||
stdio: 'inherit',
|
||||
windowsHide: true,
|
||||
shell: getWindowsEscapedCommandShell(),
|
||||
env: tracedEnv,
|
||||
});
|
||||
} else {
|
||||
claude = spawn(claudeCli, launchArgs, {
|
||||
stdio: 'inherit',
|
||||
windowsHide: true,
|
||||
env: tracedEnv,
|
||||
});
|
||||
}
|
||||
} catch (spawnError) {
|
||||
// spawn() can throw synchronously (e.g. invalid arg/env). Remove the
|
||||
// runtime settings overlay before propagating so the secret-bearing temp
|
||||
// file is not orphaned. cleanup is idempotent.
|
||||
cleanupLaunchSettings();
|
||||
throw spawnError;
|
||||
}
|
||||
|
||||
// Remove the runtime settings overlay once Claude has read it and exited.
|
||||
|
||||
Reference in new issue
Block a user