From 5155221bbe4d3e6d5b289866ed76b20697f9f71d Mon Sep 17 00:00:00 2001 From: Kartik <85060731+Kkartik14@users.noreply.github.com> Date: Wed, 6 May 2026 22:03:24 +0530 Subject: [PATCH] fix(cli): randomize sandbox container names (#26014) --- packages/cli/src/utils/sandbox.test.ts | 90 ++++++++++++++++++++++++-- packages/cli/src/utils/sandbox.ts | 27 +++----- 2 files changed, 92 insertions(+), 25 deletions(-) diff --git a/packages/cli/src/utils/sandbox.test.ts b/packages/cli/src/utils/sandbox.test.ts index 256b418e83..e0e6789b72 100644 --- a/packages/cli/src/utils/sandbox.test.ts +++ b/packages/cli/src/utils/sandbox.test.ts @@ -9,6 +9,7 @@ import { spawn, exec, execFile, execSync } from 'node:child_process'; import os from 'node:os'; import fs from 'node:fs'; import path from 'node:path'; +import { randomBytes } from 'node:crypto'; import { start_sandbox } from './sandbox.js'; import { FatalSandboxError, @@ -18,10 +19,12 @@ import { import { createMockSandboxConfig } from '@google/gemini-cli-test-utils'; import { EventEmitter } from 'node:events'; -const { mockedHomedir, mockedGetContainerPath } = vi.hoisted(() => ({ - mockedHomedir: vi.fn().mockReturnValue('/home/user'), - mockedGetContainerPath: vi.fn().mockImplementation((p: string) => p), -})); +const { mockedHomedir, mockedGetContainerPath, mockedExecCommands } = + vi.hoisted(() => ({ + mockedHomedir: vi.fn().mockReturnValue('/home/user'), + mockedGetContainerPath: vi.fn().mockImplementation((p: string) => p), + mockedExecCommands: [] as string[], + })); vi.mock('./sandboxUtils.js', async (importOriginal) => { const actual = await importOriginal(); @@ -34,6 +37,9 @@ vi.mock('./sandboxUtils.js', async (importOriginal) => { vi.mock('node:child_process'); vi.mock('node:os'); vi.mock('node:fs'); +vi.mock('node:crypto', () => ({ + randomBytes: vi.fn().mockReturnValue(Buffer.from('a1b2c3d4e5f6', 'hex')), +})); vi.mock('node:util', async (importOriginal) => { const actual = await importOriginal(); return { @@ -41,6 +47,7 @@ vi.mock('node:util', async (importOriginal) => { promisify: (fn: (...args: unknown[]) => unknown) => { if (fn === exec) { return async (cmd: string) => { + mockedExecCommands.push(cmd); if (cmd === 'id -u' || cmd === 'id -g') { return { stdout: '1000', stderr: '' }; } @@ -50,9 +57,6 @@ vi.mock('node:util', async (importOriginal) => { if (cmd.includes('getconf DARWIN_USER_CACHE_DIR')) { return { stdout: '/tmp/cache', stderr: '' }; } - if (cmd.includes('ps -a --format')) { - return { stdout: 'existing-container', stderr: '' }; - } return { stdout: '', stderr: '' }; }; } @@ -116,6 +120,7 @@ describe('sandbox', () => { beforeEach(() => { vi.clearAllMocks(); + mockedExecCommands.length = 0; process.env = { ...originalEnv }; process.argv = [...originalArgv]; mockProcessIn = { @@ -334,6 +339,77 @@ describe('sandbox', () => { expect.arrayContaining(['run', '-i', '--rm', '--init']), expect.objectContaining({ stdio: 'inherit' }), ); + + const containerName = 'gemini-cli-sandbox-a1b2c3d4e5f6'; + expect(randomBytes).toHaveBeenCalledWith(6); + expect(mockedExecCommands).not.toEqual( + expect.arrayContaining([expect.stringContaining('ps -a --format')]), + ); + expect(spawn).toHaveBeenNthCalledWith( + 2, + 'docker', + expect.arrayContaining([ + '--name', + containerName, + '--hostname', + containerName, + '--env', + `SANDBOX=${containerName}`, + ]), + expect.objectContaining({ stdio: 'inherit' }), + ); + }); + + it('should preserve the integration-test prefix for random container names', async () => { + const config: SandboxConfig = createMockSandboxConfig({ + command: 'docker', + image: 'gemini-cli-sandbox', + }); + process.env['GEMINI_CLI_INTEGRATION_TEST'] = 'true'; + + interface MockProcessWithStdout extends EventEmitter { + stdout: EventEmitter; + } + const mockImageCheckProcess = new EventEmitter() as MockProcessWithStdout; + mockImageCheckProcess.stdout = new EventEmitter(); + vi.mocked(spawn).mockImplementationOnce(() => { + setTimeout(() => { + mockImageCheckProcess.stdout.emit('data', Buffer.from('image-id')); + mockImageCheckProcess.emit('close', 0); + }, 1); + return mockImageCheckProcess as unknown as ReturnType; + }); + + const mockSpawnProcess = new EventEmitter() as unknown as ReturnType< + typeof spawn + >; + mockSpawnProcess.on = vi.fn().mockImplementation((event, cb) => { + if (event === 'close') { + setTimeout(() => cb(0), 10); + } + return mockSpawnProcess; + }); + vi.mocked(spawn).mockImplementationOnce(() => mockSpawnProcess); + + await expect( + start_sandbox(config, [], undefined, ['arg1']), + ).resolves.toBe(0); + + const containerName = 'gemini-cli-integration-test-a1b2c3d4e5f6'; + expect(randomBytes).toHaveBeenCalledWith(6); + expect(spawn).toHaveBeenNthCalledWith( + 2, + 'docker', + expect.arrayContaining([ + '--name', + containerName, + '--hostname', + containerName, + '--env', + `SANDBOX=${containerName}`, + ]), + expect.objectContaining({ stdio: 'inherit' }), + ); }); it('should pull image if missing', async () => { diff --git a/packages/cli/src/utils/sandbox.ts b/packages/cli/src/utils/sandbox.ts index ccd1f2e608..86bb1af96e 100644 --- a/packages/cli/src/utils/sandbox.ts +++ b/packages/cli/src/utils/sandbox.ts @@ -493,27 +493,18 @@ export async function start_sandbox( } } - // name container after image, plus random suffix to avoid conflicts + // Use a random suffix instead of probing existing containers so concurrent + // CLI starts cannot race on the same sequential name. const imageName = parseImageName(image); const isIntegrationTest = process.env['GEMINI_CLI_INTEGRATION_TEST'] === 'true'; - let containerName; - if (isIntegrationTest) { - containerName = `gemini-cli-integration-test-${randomBytes(4).toString( - 'hex', - )}`; - debugLogger.log(`ContainerName: ${containerName}`); - } else { - let index = 0; - const containerNameCheck = ( - await execAsync(`${command} ps -a --format "{{.Names}}"`) - ).stdout.trim(); - while (containerNameCheck.includes(`${imageName}-${index}`)) { - index++; - } - containerName = `${imageName}-${index}`; - debugLogger.log(`ContainerName (regular): ${containerName}`); - } + const containerNamePrefix = isIntegrationTest + ? 'gemini-cli-integration-test' + : imageName; + const containerName = `${containerNamePrefix}-${randomBytes(6).toString( + 'hex', + )}`; + debugLogger.log(`ContainerName: ${containerName}`); args.push('--name', containerName, '--hostname', containerName); // copy GEMINI_CLI_TEST_VAR for integration tests