fix(cli): ensure sandbox proxy cleanup and remove handler leaks (#26065)

This commit is contained in:
Emily Hedlund
2026-04-27 12:35:41 -07:00
committed by GitHub
parent 71f313b51a
commit b1a50a58af
2 changed files with 81 additions and 10 deletions
+59
View File
@@ -719,6 +719,65 @@ describe('sandbox', () => {
expect(entrypointCmd).toContain('su -p gemini');
});
it('should register and unregister proxy exit handlers', async () => {
vi.stubEnv('GEMINI_SANDBOX_PROXY_COMMAND', 'some-proxy-cmd');
const config: SandboxConfig = createMockSandboxConfig({
command: 'docker',
image: 'gemini-cli-sandbox',
});
const onSpy = vi.spyOn(process, 'on');
const offSpy = vi.spyOn(process, 'off');
interface MockProcessWithStdout extends EventEmitter {
stdout: EventEmitter;
}
vi.mocked(spawn).mockImplementation((cmd, args) => {
const a = args as string[];
if (cmd === 'docker' && a && a[0] === 'images') {
const mockImageCheckProcess =
new EventEmitter() as MockProcessWithStdout;
mockImageCheckProcess.stdout = new EventEmitter();
setTimeout(() => {
mockImageCheckProcess.stdout.emit('data', Buffer.from('image-id'));
mockImageCheckProcess.emit('close', 0);
}, 1);
return mockImageCheckProcess as unknown as ReturnType<typeof spawn>;
}
if (cmd === 'docker' && a && a[0] === 'run') {
const mockSpawnProcess = new EventEmitter() as unknown as ReturnType<
typeof spawn
>;
mockSpawnProcess.on = vi.fn().mockImplementation((event, cb) => {
if (event === 'close') {
if (a.includes('gemini-cli-sandbox-proxy')) {
// Proxy container shouldn't exit during the test
} else {
setTimeout(() => cb(0), 10);
}
}
return mockSpawnProcess;
});
return mockSpawnProcess;
}
return new EventEmitter() as unknown as ReturnType<typeof spawn>;
});
await start_sandbox(config);
expect(onSpy).toHaveBeenCalledWith('exit', expect.any(Function));
expect(onSpy).toHaveBeenCalledWith('SIGINT', expect.any(Function));
expect(onSpy).toHaveBeenCalledWith('SIGTERM', expect.any(Function));
expect(offSpy).toHaveBeenCalledWith('exit', expect.any(Function));
expect(offSpy).toHaveBeenCalledWith('SIGINT', expect.any(Function));
expect(offSpy).toHaveBeenCalledWith('SIGTERM', expect.any(Function));
onSpy.mockRestore();
offSpy.mockRestore();
});
describe('LXC sandbox', () => {
const LXC_RUNNING = JSON.stringify([
{ name: 'gemini-sandbox', status: 'Running' },
+22 -10
View File
@@ -55,6 +55,8 @@ export async function start_sandbox(
});
patcher.patch();
let stopProxy: (() => void) | undefined = undefined;
try {
if (config.command === 'sandbox-exec') {
// disallow BUILD_SANDBOX
@@ -188,17 +190,18 @@ export async function start_sandbox(
detached: true,
});
// install handlers to stop proxy on exit/signal
const stopProxy = () => {
stopProxy = () => {
debugLogger.log('stopping proxy ...');
if (proxyProcess?.pid) {
process.kill(-proxyProcess.pid, 'SIGTERM');
try {
process.kill(-proxyProcess.pid, 'SIGTERM');
} catch {
// ignore
}
}
};
process.off('exit', stopProxy);
process.on('exit', stopProxy);
process.off('SIGINT', stopProxy);
process.on('SIGINT', stopProxy);
process.off('SIGTERM', stopProxy);
process.on('SIGTERM', stopProxy);
// commented out as it disrupts ink rendering
@@ -746,15 +749,18 @@ export async function start_sandbox(
detached: true,
});
// install handlers to stop proxy on exit/signal
const stopProxy = () => {
stopProxy = () => {
debugLogger.log('stopping proxy container ...');
execSync(`${command} rm -f ${SANDBOX_PROXY_NAME}`);
try {
spawnSync(command, ['rm', '-f', SANDBOX_PROXY_NAME], {
stdio: 'ignore',
});
} catch {
// ignore
}
};
process.off('exit', stopProxy);
process.on('exit', stopProxy);
process.off('SIGINT', stopProxy);
process.on('SIGINT', stopProxy);
process.off('SIGTERM', stopProxy);
process.on('SIGTERM', stopProxy);
// commented out as it disrupts ink rendering
@@ -806,6 +812,12 @@ export async function start_sandbox(
});
});
} finally {
if (stopProxy) {
stopProxy();
process.off('exit', stopProxy);
process.off('SIGINT', stopProxy);
process.off('SIGTERM', stopProxy);
}
patcher.cleanup();
}
}