diff --git a/src/lib/docker.ts b/src/lib/docker.ts index 80251fa..9342c51 100644 --- a/src/lib/docker.ts +++ b/src/lib/docker.ts @@ -4,9 +4,8 @@ * health-checking, and cleanup for both registry and local modes. */ -import { execSync } from 'child_process'; -import { shellEscape } from './shell.js'; -import { DOCKER_WAIT_TIMEOUT, DOCKER_POLL_INTERVAL } from './constants.js'; +import { execFileSync } from 'child_process'; +import { DOCKER_WAIT_TIMEOUT } from './constants.js'; export interface ContainerSpec { challengeId: string; @@ -17,6 +16,11 @@ export interface ContainerSpec { targetContainerName: string; } +/** Synchronous sleep that works cross-platform (no shell, no `sleep` binary). */ +function sleepSync(ms: number): void { + Atomics.wait(new Int32Array(new SharedArrayBuffer(4)), 0, 0, ms); +} + /** * Pull a Docker image. Tries native platform first, falls back to linux/amd64 * if the image has no matching manifest (common for challenge images on Apple Silicon). @@ -28,7 +32,7 @@ export function pullImage(image: string, onProgress?: (line: string) => void): b } try { - execSync(`docker pull ${shellEscape(image)}`, { + execFileSync('docker', ['pull', image], { stdio: onProgress ? 'inherit' : 'pipe', encoding: 'utf-8', }); @@ -45,7 +49,7 @@ export function pullImage(image: string, onProgress?: (line: string) => void): b if (onProgress) { onProgress(`Pulling ${image} (linux/amd64 fallback)...`); } - execSync(`docker pull --platform linux/amd64 ${shellEscape(image)}`, { + execFileSync('docker', ['pull', '--platform', 'linux/amd64', image], { stdio: onProgress ? 'inherit' : 'pipe', encoding: 'utf-8', }); @@ -57,12 +61,12 @@ export function pullImage(image: string, onProgress?: (line: string) => void): b */ export function ensureNetwork(name: string): void { try { - execSync(`docker network inspect ${shellEscape(name)}`, { + execFileSync('docker', ['network', 'inspect', name], { stdio: 'pipe', encoding: 'utf-8', }); } catch { - execSync(`docker network create ${shellEscape(name)}`, { + execFileSync('docker', ['network', 'create', name], { stdio: 'pipe', encoding: 'utf-8', }); @@ -83,24 +87,23 @@ export function startContainers(spec: ContainerSpec, platforms?: PlatformOverrid cleanupStale(spec); ensureNetwork(spec.network); - const targetPlatformFlag = platforms?.target ? `--platform ${shellEscape(platforms.target)} ` : ''; - const kaliPlatformFlag = platforms?.kali ? `--platform ${shellEscape(platforms.kali)} ` : ''; - // Start target container - execSync( - `docker run -d ${targetPlatformFlag}--name ${shellEscape(spec.targetContainerName)} ` + - `--hostname target --network ${shellEscape(spec.network)} ` + - `${shellEscape(spec.targetImage)}`, - { stdio: 'pipe', encoding: 'utf-8' } - ); + const targetArgs = ['run', '-d']; + if (platforms?.target) targetArgs.push('--platform', platforms.target); + targetArgs.push('--name', spec.targetContainerName); + targetArgs.push('--hostname', 'target'); + targetArgs.push('--network', spec.network); + targetArgs.push(spec.targetImage); + execFileSync('docker', targetArgs, { stdio: 'pipe', encoding: 'utf-8' }); // Start kali container - execSync( - `docker run -d ${kaliPlatformFlag}--name ${shellEscape(spec.kaliContainerName)} ` + - `--hostname kali --network ${shellEscape(spec.network)} ` + - `${shellEscape(spec.kaliImage)} sleep infinity`, - { stdio: 'pipe', encoding: 'utf-8' } - ); + const kaliArgs = ['run', '-d']; + if (platforms?.kali) kaliArgs.push('--platform', platforms.kali); + kaliArgs.push('--name', spec.kaliContainerName); + kaliArgs.push('--hostname', 'kali'); + kaliArgs.push('--network', spec.network); + kaliArgs.push(spec.kaliImage, 'sleep', 'infinity'); + execFileSync('docker', kaliArgs, { stdio: 'pipe', encoding: 'utf-8' }); } /** @@ -134,18 +137,17 @@ export function waitForTarget( timeoutMs = DOCKER_WAIT_TIMEOUT ): void { const start = Date.now(); - const pollInterval = DOCKER_POLL_INTERVAL; while (Date.now() - start < timeoutMs) { try { - execSync( - `docker exec ${shellEscape(kaliContainer)} curl -sf ${shellEscape(targetUrl)}`, + execFileSync( + 'docker', ['exec', kaliContainer, 'curl', '-sf', targetUrl], { stdio: 'pipe', encoding: 'utf-8', timeout: 5000 } ); return; // Success } catch { // Not ready yet — wait and retry - execSync(`sleep 2`, { stdio: 'pipe' }); + sleepSync(2000); } } @@ -159,8 +161,8 @@ export function waitForTarget( */ export function cleanup(spec: ContainerSpec): void { try { - execSync( - `docker rm -f ${shellEscape(spec.targetContainerName)} ${shellEscape(spec.kaliContainerName)}`, + execFileSync( + 'docker', ['rm', '-f', spec.targetContainerName, spec.kaliContainerName], { stdio: 'pipe', encoding: 'utf-8' } ); } catch { @@ -168,7 +170,7 @@ export function cleanup(spec: ContainerSpec): void { } try { - execSync(`docker network rm ${shellEscape(spec.network)}`, { + execFileSync('docker', ['network', 'rm', spec.network], { stdio: 'pipe', encoding: 'utf-8', }); @@ -182,8 +184,8 @@ export function cleanup(spec: ContainerSpec): void { */ export function cleanupStale(spec: ContainerSpec): void { try { - execSync( - `docker rm -f ${shellEscape(spec.targetContainerName)} ${shellEscape(spec.kaliContainerName)} 2>/dev/null`, + execFileSync( + 'docker', ['rm', '-f', spec.targetContainerName, spec.kaliContainerName], { stdio: 'pipe', encoding: 'utf-8' } ); } catch { @@ -195,7 +197,7 @@ export function cleanupStale(spec: ContainerSpec): void { * Start containers from a docker-compose.yml in the given directory. */ export function startFromCompose(challengeDir: string): void { - execSync(`docker compose -f ${shellEscape(challengeDir)}/docker-compose.yml up -d --build`, { + execFileSync('docker', ['compose', '-f', `${challengeDir}/docker-compose.yml`, 'up', '-d', '--build'], { stdio: 'inherit', encoding: 'utf-8', }); @@ -205,7 +207,7 @@ export function startFromCompose(challengeDir: string): void { * Stop and remove containers from a docker-compose.yml in the given directory. */ export function stopFromCompose(challengeDir: string): void { - execSync(`docker compose -f ${shellEscape(challengeDir)}/docker-compose.yml down`, { + execFileSync('docker', ['compose', '-f', `${challengeDir}/docker-compose.yml`, 'down'], { stdio: 'pipe', encoding: 'utf-8', }); diff --git a/src/lib/env-check.ts b/src/lib/env-check.ts index 1678691..36ca03f 100644 --- a/src/lib/env-check.ts +++ b/src/lib/env-check.ts @@ -3,8 +3,7 @@ * Validates Docker, containers, and connectivity before running benchmarks. */ -import { execSync } from 'child_process'; -import { shellEscape } from './shell.js'; +import { execFileSync } from 'child_process'; import { DOCKER_STARTUP_POLL } from './constants.js'; import { ConfigError } from './errors.js'; import { getErrorStatus } from './retry.js'; @@ -32,7 +31,7 @@ export function checkDockerRunning(): EnvCheckResult { const hints: string[] = []; try { - execSync('docker ps', { encoding: 'utf-8', stdio: 'pipe' }); + execFileSync('docker', ['ps'], { encoding: 'utf-8', stdio: 'pipe' }); return { ok: true, errors: [], hints: [] }; } catch { errors.push('Docker is not running or not accessible'); @@ -51,7 +50,7 @@ export async function ensureDocker( ): Promise { // Check if docker is already running try { - execSync('docker ps', { encoding: 'utf-8', stdio: 'pipe' }); + execFileSync('docker', ['ps'], { encoding: 'utf-8', stdio: 'pipe' }); return { ok: true, autoStarted: false, errors: [], hints: [] }; } catch { // Docker not running — try to auto-start on macOS @@ -72,7 +71,7 @@ export async function ensureDocker( // macOS: try to launch Docker Desktop onStatus?.('Starting Docker Desktop...'); try { - execSync('open --background -a Docker', { stdio: 'pipe' }); + execFileSync('open', ['--background', '-a', 'Docker'], { stdio: 'pipe' }); } catch { return { ok: false, @@ -95,7 +94,7 @@ export async function ensureDocker( onStatus?.(`Waiting for Docker to be ready... (${elapsed}s)`); try { - execSync('docker ps', { encoding: 'utf-8', stdio: 'pipe' }); + execFileSync('docker', ['ps'], { encoding: 'utf-8', stdio: 'pipe' }); return { ok: true, autoStarted: true, errors: [], hints: [] }; } catch { // Not ready yet @@ -128,8 +127,8 @@ export function checkContainersRunning( const kaliContainer = containerName; // e.g. gatekeeper-kali-1 try { - const out = execSync( - `docker ps -a --format "{{.Names}}\t{{.Status}}"`, + const out = execFileSync( + 'docker', ['ps', '-a', '--format', '{{.Names}}\t{{.Status}}'], { encoding: 'utf-8', stdio: 'pipe' } ); @@ -184,14 +183,16 @@ export function checkTargetReachable( const hints: string[] = []; try { - // Extract host:port from URL (e.g. http://target:5000 -> target:5000) - const urlMatch = targetUrl.match(/^(?:https?:\/\/)?([^/]+)/); - const hostPort = urlMatch ? urlMatch[1] : targetUrl.replace(/^https?:\/\//, ''); - - const result = execSync( - `docker exec ${shellEscape(containerName)} curl -s -o /dev/null -w "%{http_code}" --connect-timeout 5 ${shellEscape(targetUrl)} 2>/dev/null || echo "FAIL"`, - { encoding: 'utf-8', stdio: 'pipe' } - ).trim(); + let result: string; + try { + result = execFileSync( + 'docker', + ['exec', containerName, 'curl', '-s', '-o', '/dev/null', '-w', '%{http_code}', '--connect-timeout', '5', targetUrl], + { encoding: 'utf-8', stdio: 'pipe' } + ).trim(); + } catch { + result = 'FAIL'; + } if (result === 'FAIL' || result === '000') { errors.push(`Target ${targetUrl} is not reachable from Kali container`); @@ -224,8 +225,8 @@ export function checkKaliTools(containerName: string): EnvCheckResult { for (const tool of REQUIRED_KALI_TOOLS) { try { - execSync( - `docker exec ${shellEscape(containerName)} which ${tool} 2>/dev/null`, + execFileSync( + 'docker', ['exec', containerName, 'which', tool], { encoding: 'utf-8', stdio: 'pipe' } ); } catch { @@ -351,7 +352,7 @@ export async function checkApiKey( */ export function runPreflightChecks( challengeId: string, - challengeDir: string, + _challengeDir: string, containerName: string, targetUrl: string ): EnvCheckResult { diff --git a/src/lib/shell.ts b/src/lib/shell.ts index 250cb91..24f51a8 100644 --- a/src/lib/shell.ts +++ b/src/lib/shell.ts @@ -1,6 +1,13 @@ // Shared shell utility -/** Escape a string for safe inclusion in a shell command (single-quote wrapping). */ +/** + * Escape a string for safe inclusion in a POSIX shell command (single-quote wrapping). + * + * NOTE: This function is POSIX-only (bash/zsh/sh). It does NOT handle Windows + * cmd.exe quoting. For cross-platform subprocess calls, use execFileSync/spawnSync + * with argument arrays instead of shell strings — this bypasses the shell entirely + * and avoids escaping issues on all platforms. + */ export function shellEscape(s: string): string { return "'" + s.replace(/'/g, "'\\''") + "'"; } diff --git a/tests/unit/docker.test.ts b/tests/unit/docker.test.ts index 6cff5a3..f6ad146 100644 --- a/tests/unit/docker.test.ts +++ b/tests/unit/docker.test.ts @@ -1,5 +1,5 @@ import { describe, it, expect, vi, beforeEach } from 'vitest'; -import { execSync } from 'child_process'; +import { execFileSync } from 'child_process'; import { pullImage, ensureNetwork, @@ -11,10 +11,10 @@ import { import type { ContainerSpec } from '../../src/lib/docker.js'; vi.mock('child_process', () => ({ - execSync: vi.fn(), + execFileSync: vi.fn(), })); -const mockExecSync = vi.mocked(execSync); +const mockExecFileSync = vi.mocked(execFileSync); beforeEach(() => { vi.clearAllMocks(); @@ -29,74 +29,87 @@ const SPEC: ContainerSpec = { targetContainerName: 'oasis-gatekeeper-target', }; +/** Helper: get all calls as [command, args] tuples */ +function getCalls(): Array<[string, string[]]> { + return mockExecFileSync.mock.calls.map(c => [c[0] as string, c[1] as string[]]); +} + +/** Helper: filter calls by command prefix in args */ +function getDockerCalls(subcommand: string): Array<[string, string[]]> { + return getCalls().filter(([cmd, args]) => cmd === 'docker' && args[0] === subcommand); +} + // ============================================================================= // pullImage // ============================================================================= describe('pullImage', () => { it('returns false when native pull succeeds', () => { - mockExecSync.mockReturnValue('' as any); + mockExecFileSync.mockReturnValue('' as any); const result = pullImage('myimage:latest'); expect(result).toBe(false); - expect(mockExecSync).toHaveBeenCalledTimes(1); - expect((mockExecSync.mock.calls[0][0] as string)).toContain('docker pull'); - expect((mockExecSync.mock.calls[0][0] as string)).not.toContain('--platform'); + const pulls = getDockerCalls('pull'); + expect(pulls).toHaveLength(1); + expect(pulls[0][1]).toEqual(['pull', 'myimage:latest']); }); it('returns true when native pull fails with manifest error and amd64 fallback succeeds', () => { - mockExecSync + mockExecFileSync .mockImplementationOnce(() => { const err: any = new Error('pull failed'); err.stderr = 'no matching manifest for linux/arm64/v8'; throw err; }) - .mockReturnValueOnce('' as any); // amd64 fallback succeeds + .mockReturnValue('' as any); const result = pullImage('myimage:latest'); expect(result).toBe(true); - expect(mockExecSync).toHaveBeenCalledTimes(2); - expect((mockExecSync.mock.calls[1][0] as string)).toContain('--platform linux/amd64'); + const pulls = getDockerCalls('pull'); + expect(pulls).toHaveLength(2); + expect(pulls[1][1]).toContain('--platform'); + expect(pulls[1][1]).toContain('linux/amd64'); }); it('also catches "no match for platform" variant', () => { - mockExecSync + mockExecFileSync .mockImplementationOnce(() => { const err: any = new Error('pull failed'); err.stderr = 'no match for platform'; throw err; }) - .mockReturnValueOnce('' as any); + .mockReturnValue('' as any); const result = pullImage('myimage:latest'); expect(result).toBe(true); }); it('rethrows non-manifest errors without attempting fallback', () => { - mockExecSync.mockImplementation(() => { + mockExecFileSync.mockImplementation(() => { const err: any = new Error('network timeout'); err.stderr = 'connection refused'; throw err; }); expect(() => pullImage('myimage:latest')).toThrow('network timeout'); - expect(mockExecSync).toHaveBeenCalledTimes(1); + const pulls = getDockerCalls('pull'); + expect(pulls).toHaveLength(1); }); it('calls onProgress callback during pull', () => { - mockExecSync.mockReturnValue('' as any); + mockExecFileSync.mockReturnValue('' as any); const messages: string[] = []; pullImage('myimage:latest', (msg) => messages.push(msg)); expect(messages).toContain('Pulling myimage:latest...'); }); it('calls onProgress for fallback path', () => { - mockExecSync + mockExecFileSync .mockImplementationOnce(() => { const err: any = new Error('fail'); err.stderr = 'no matching manifest'; throw err; }) - .mockReturnValueOnce('' as any); + .mockReturnValue('' as any); const messages: string[] = []; pullImage('myimage:latest', (msg) => messages.push(msg)); @@ -104,11 +117,12 @@ describe('pullImage', () => { expect(messages).toContain('Pulling myimage:latest (linux/amd64 fallback)...'); }); - it('shell-escapes the image name', () => { - mockExecSync.mockReturnValue('' as any); + it('passes image name as a separate argument (no shell escaping needed)', () => { + mockExecFileSync.mockReturnValue('' as any); pullImage("evil'image"); - const cmd = mockExecSync.mock.calls[0][0] as string; - expect(cmd).toContain("'evil'\\''image'"); + const pulls = getDockerCalls('pull'); + // Image name is passed as a discrete array element, not interpolated into a shell string + expect(pulls[0][1]).toContain("evil'image"); }); }); @@ -118,52 +132,41 @@ describe('pullImage', () => { describe('startContainers', () => { it('runs both containers without platform flag when no overrides', () => { - mockExecSync.mockReturnValue('' as any); + mockExecFileSync.mockReturnValue('' as any); startContainers(SPEC); - // cleanupStale + ensureNetwork inspect/create + 2 docker run = at least 4 calls - const runCalls = mockExecSync.mock.calls - .map(c => c[0] as string) - .filter(c => c.startsWith('docker run')); - expect(runCalls).toHaveLength(2); - for (const cmd of runCalls) { - expect(cmd).not.toContain('--platform'); + const runs = getDockerCalls('run'); + expect(runs).toHaveLength(2); + for (const [, args] of runs) { + expect(args).not.toContain('--platform'); } }); it('applies platform only to target when only target needs it', () => { - mockExecSync.mockReturnValue('' as any); + mockExecFileSync.mockReturnValue('' as any); startContainers(SPEC, { target: 'linux/amd64' }); - const runCalls = mockExecSync.mock.calls - .map(c => c[0] as string) - .filter(c => c.startsWith('docker run')); - expect(runCalls).toHaveLength(2); - // Target should have --platform - expect(runCalls[0]).toContain('--platform'); - expect(runCalls[0]).toContain('linux/amd64'); - // Kali should NOT have --platform - expect(runCalls[1]).not.toContain('--platform'); + const runs = getDockerCalls('run'); + expect(runs).toHaveLength(2); + expect(runs[0][1]).toContain('--platform'); + expect(runs[0][1]).toContain('linux/amd64'); + expect(runs[1][1]).not.toContain('--platform'); }); it('applies platform only to kali when only kali needs it', () => { - mockExecSync.mockReturnValue('' as any); + mockExecFileSync.mockReturnValue('' as any); startContainers(SPEC, { kali: 'linux/amd64' }); - const runCalls = mockExecSync.mock.calls - .map(c => c[0] as string) - .filter(c => c.startsWith('docker run')); - expect(runCalls).toHaveLength(2); - expect(runCalls[0]).not.toContain('--platform'); - expect(runCalls[1]).toContain('--platform'); + const runs = getDockerCalls('run'); + expect(runs).toHaveLength(2); + expect(runs[0][1]).not.toContain('--platform'); + expect(runs[1][1]).toContain('--platform'); }); it('applies platform to both when both need it', () => { - mockExecSync.mockReturnValue('' as any); + mockExecFileSync.mockReturnValue('' as any); startContainers(SPEC, { target: 'linux/amd64', kali: 'linux/amd64' }); - const runCalls = mockExecSync.mock.calls - .map(c => c[0] as string) - .filter(c => c.startsWith('docker run')); - expect(runCalls).toHaveLength(2); - expect(runCalls[0]).toContain('--platform'); - expect(runCalls[1]).toContain('--platform'); + const runs = getDockerCalls('run'); + expect(runs).toHaveLength(2); + expect(runs[0][1]).toContain('--platform'); + expect(runs[1][1]).toContain('--platform'); }); }); @@ -173,29 +176,23 @@ describe('startContainers', () => { describe('pullAndStartContainers', () => { it('pulls both images and starts without platform when both native', () => { - mockExecSync.mockReturnValue('' as any); + mockExecFileSync.mockReturnValue('' as any); pullAndStartContainers(SPEC); - const pullCalls = mockExecSync.mock.calls - .map(c => c[0] as string) - .filter(c => c.includes('docker pull')); - expect(pullCalls).toHaveLength(2); + const pulls = getDockerCalls('pull'); + expect(pulls).toHaveLength(2); - const runCalls = mockExecSync.mock.calls - .map(c => c[0] as string) - .filter(c => c.startsWith('docker run')); - for (const cmd of runCalls) { - expect(cmd).not.toContain('--platform'); + const runs = getDockerCalls('run'); + for (const [, args] of runs) { + expect(args).not.toContain('--platform'); } }); it('sets target platform only when target image needs amd64 fallback', () => { - let callIdx = 0; - mockExecSync.mockImplementation((cmd: unknown) => { - const cmdStr = cmd as string; - callIdx++; - // First docker pull (target) — manifest error - if (cmdStr.includes('docker pull') && !cmdStr.includes('--platform') && cmdStr.includes('gatekeeper')) { + mockExecFileSync.mockImplementation((cmd: unknown, args: unknown) => { + const argArr = args as string[]; + // First docker pull (target, no --platform) — manifest error + if (cmd === 'docker' && argArr[0] === 'pull' && !argArr.includes('--platform') && argArr.includes(SPEC.targetImage)) { const err: any = new Error('fail'); err.stderr = 'no matching manifest for linux/arm64/v8'; throw err; @@ -205,18 +202,16 @@ describe('pullAndStartContainers', () => { pullAndStartContainers(SPEC); - const runCalls = mockExecSync.mock.calls - .map(c => c[0] as string) - .filter(c => c.startsWith('docker run')); - expect(runCalls).toHaveLength(2); + const runs = getDockerCalls('run'); + expect(runs).toHaveLength(2); // Target run should have platform - expect(runCalls[0]).toContain('--platform'); + expect(runs[0][1]).toContain('--platform'); // Kali run should NOT - expect(runCalls[1]).not.toContain('--platform'); + expect(runs[1][1]).not.toContain('--platform'); }); it('reports progress via onProgress callback', () => { - mockExecSync.mockReturnValue('' as any); + mockExecFileSync.mockReturnValue('' as any); const messages: string[] = []; pullAndStartContainers(SPEC, (msg) => messages.push(msg)); expect(messages.some(m => m.includes('Pulling') && m.includes('gatekeeper'))).toBe(true); @@ -231,19 +226,21 @@ describe('pullAndStartContainers', () => { describe('ensureNetwork', () => { it('does not create network if inspect succeeds', () => { - mockExecSync.mockReturnValue('' as any); + mockExecFileSync.mockReturnValue('' as any); ensureNetwork('test-net'); - expect(mockExecSync).toHaveBeenCalledTimes(1); - expect((mockExecSync.mock.calls[0][0] as string)).toContain('network inspect'); + const calls = getDockerCalls('network'); + expect(calls).toHaveLength(1); + expect(calls[0][1]).toEqual(['network', 'inspect', 'test-net']); }); it('creates network when inspect fails', () => { - mockExecSync + mockExecFileSync .mockImplementationOnce(() => { throw new Error('not found'); }) - .mockReturnValueOnce('' as any); + .mockReturnValue('' as any); ensureNetwork('test-net'); - expect(mockExecSync).toHaveBeenCalledTimes(2); - expect((mockExecSync.mock.calls[1][0] as string)).toContain('network create'); + const calls = getDockerCalls('network'); + expect(calls).toHaveLength(2); + expect(calls[1][1]).toEqual(['network', 'create', 'test-net']); }); }); @@ -253,22 +250,22 @@ describe('ensureNetwork', () => { describe('cleanup', () => { it('removes containers and network', () => { - mockExecSync.mockReturnValue('' as any); + mockExecFileSync.mockReturnValue('' as any); cleanup(SPEC); - const cmds = mockExecSync.mock.calls.map(c => c[0] as string); - expect(cmds.some(c => c.includes('docker rm -f'))).toBe(true); - expect(cmds.some(c => c.includes('network rm'))).toBe(true); + const calls = getCalls(); + expect(calls.some(([cmd, args]) => cmd === 'docker' && args[0] === 'rm')).toBe(true); + expect(calls.some(([cmd, args]) => cmd === 'docker' && args[0] === 'network' && args[1] === 'rm')).toBe(true); }); it('ignores errors from rm and network rm', () => { - mockExecSync.mockImplementation(() => { throw new Error('no such container'); }); + mockExecFileSync.mockImplementation(() => { throw new Error('no such container'); }); expect(() => cleanup(SPEC)).not.toThrow(); }); }); describe('cleanupStale', () => { it('ignores errors when containers do not exist', () => { - mockExecSync.mockImplementation(() => { throw new Error('no such container'); }); + mockExecFileSync.mockImplementation(() => { throw new Error('no such container'); }); expect(() => cleanupStale(SPEC)).not.toThrow(); }); }); diff --git a/tests/unit/env-check.test.ts b/tests/unit/env-check.test.ts index 21b5c5d..5e82988 100644 --- a/tests/unit/env-check.test.ts +++ b/tests/unit/env-check.test.ts @@ -1,5 +1,5 @@ import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; -import { execSync } from 'child_process'; +import { execFileSync } from 'child_process'; import { checkDockerRunning, ensureDocker, @@ -15,10 +15,10 @@ import { // ============================================================================= vi.mock('child_process', () => ({ - execSync: vi.fn(), + execFileSync: vi.fn(), })); -const mockExecSync = vi.mocked(execSync); +const mockExecFileSync = vi.mocked(execFileSync); // Mock Anthropic SDK (dynamic import) const mockAnthropicCreate = vi.fn(); @@ -42,20 +42,30 @@ beforeEach(() => { vi.clearAllMocks(); }); +/** Helper to match calls by command and first arg */ +function matchCall(cmd: string, firstArg?: string): (call: unknown[]) => boolean { + return (call: unknown[]) => { + if (call[0] !== cmd) return false; + if (firstArg === undefined) return true; + const args = call[1] as string[]; + return args?.[0] === firstArg; + }; +} + // ============================================================================= // checkDockerRunning // ============================================================================= describe('checkDockerRunning', () => { it('returns ok when docker ps succeeds', () => { - mockExecSync.mockReturnValue('CONTAINER ID IMAGE ...\n' as any); + mockExecFileSync.mockReturnValue('CONTAINER ID IMAGE ...\n' as any); const result = checkDockerRunning(); expect(result.ok).toBe(true); expect(result.errors).toHaveLength(0); }); it('returns error when docker ps throws', () => { - mockExecSync.mockImplementation(() => { + mockExecFileSync.mockImplementation(() => { throw new Error('Cannot connect to Docker daemon'); }); const result = checkDockerRunning(); @@ -64,7 +74,7 @@ describe('checkDockerRunning', () => { }); it('includes helpful hints on failure', () => { - mockExecSync.mockImplementation(() => { + mockExecFileSync.mockImplementation(() => { throw new Error('not running'); }); const result = checkDockerRunning(); @@ -89,34 +99,34 @@ describe('ensureDocker', () => { }); it('returns immediately when Docker is already running', async () => { - mockExecSync.mockReturnValueOnce('' as any); // docker ps succeeds + mockExecFileSync.mockReturnValueOnce('' as any); // docker ps succeeds const result = await ensureDocker(); expect(result.ok).toBe(true); expect(result.autoStarted).toBe(false); - expect(mockExecSync).toHaveBeenCalledTimes(1); + expect(mockExecFileSync).toHaveBeenCalledTimes(1); }); it('returns error on non-macOS when Docker is not running', async () => { setPlatform('linux'); - mockExecSync.mockImplementation(() => { throw new Error('not running'); }); + mockExecFileSync.mockImplementation(() => { throw new Error('not running'); }); const result = await ensureDocker(); expect(result.ok).toBe(false); expect(result.autoStarted).toBe(false); expect(result.errors).toContain('Docker is not running or not accessible'); expect(result.hints.some((h) => h.includes('Start Docker Desktop'))).toBe(true); - // Should only have called docker ps once (no open command) - expect(mockExecSync).toHaveBeenCalledTimes(1); + expect(mockExecFileSync).toHaveBeenCalledTimes(1); }); it('auto-starts Docker Desktop on macOS and polls until ready', async () => { setPlatform('darwin'); let callCount = 0; - mockExecSync.mockImplementation((cmd: unknown) => { + mockExecFileSync.mockImplementation((cmd: unknown, args: unknown) => { callCount++; - const cmdStr = cmd as string; - if (cmdStr === 'open --background -a Docker') return '' as any; - // First docker ps fails, second succeeds (after polling) - if (cmdStr === 'docker ps') { + const argArr = args as string[]; + // open --background -a Docker + if (cmd === 'open') return '' as any; + // docker ps — fail first two, then succeed + if (cmd === 'docker' && argArr[0] === 'ps') { if (callCount <= 2) throw new Error('not running'); return '' as any; } @@ -130,10 +140,9 @@ describe('ensureDocker', () => { it('returns error when open command fails on macOS', async () => { setPlatform('darwin'); - mockExecSync.mockImplementation((cmd: unknown) => { - const cmdStr = cmd as string; - if (cmdStr === 'docker ps') throw new Error('not running'); - if (cmdStr === 'open --background -a Docker') throw new Error('app not found'); + mockExecFileSync.mockImplementation((cmd: unknown) => { + if (cmd === 'docker') throw new Error('not running'); + if (cmd === 'open') throw new Error('app not found'); throw new Error('unexpected'); }); @@ -145,9 +154,8 @@ describe('ensureDocker', () => { it('returns timeout error when Docker never becomes ready', async () => { setPlatform('darwin'); - mockExecSync.mockImplementation((cmd: unknown) => { - const cmdStr = cmd as string; - if (cmdStr === 'open --background -a Docker') return '' as any; + mockExecFileSync.mockImplementation((cmd: unknown) => { + if (cmd === 'open') return '' as any; throw new Error('not running'); }); @@ -159,12 +167,11 @@ describe('ensureDocker', () => { it('calls onStatus callback with progress messages', async () => { setPlatform('darwin'); let callCount = 0; - mockExecSync.mockImplementation((cmd: unknown) => { + mockExecFileSync.mockImplementation((cmd: unknown, args: unknown) => { callCount++; - const cmdStr = cmd as string; - if (cmdStr === 'open --background -a Docker') return '' as any; - if (cmdStr === 'docker ps') { - // Fail first check, succeed after first poll + const argArr = args as string[]; + if (cmd === 'open') return '' as any; + if (cmd === 'docker' && argArr[0] === 'ps') { if (callCount <= 2) throw new Error('not running'); return '' as any; } @@ -189,7 +196,7 @@ describe('checkContainersRunning', () => { const containerName = 'gatekeeper-kali-1'; it('returns ok when both containers are Up', () => { - mockExecSync.mockReturnValue( + mockExecFileSync.mockReturnValue( 'gatekeeper-target-1\tUp 5 minutes\ngatekeeper-kali-1\tUp 5 minutes\n' as any ); const result = checkContainersRunning(challengeId, containerName); @@ -198,14 +205,14 @@ describe('checkContainersRunning', () => { }); it('returns error when target container is missing', () => { - mockExecSync.mockReturnValue('gatekeeper-kali-1\tUp 5 minutes\n' as any); + mockExecFileSync.mockReturnValue('gatekeeper-kali-1\tUp 5 minutes\n' as any); const result = checkContainersRunning(challengeId, containerName); expect(result.ok).toBe(false); expect(result.errors.some((e) => e.includes('gatekeeper-target-1'))).toBe(true); }); it('returns error when target container is Exited', () => { - mockExecSync.mockReturnValue( + mockExecFileSync.mockReturnValue( 'gatekeeper-target-1\tExited (1) 2 minutes ago\ngatekeeper-kali-1\tUp 5 minutes\n' as any ); const result = checkContainersRunning(challengeId, containerName); @@ -216,14 +223,14 @@ describe('checkContainersRunning', () => { }); it('returns error when kali container is missing', () => { - mockExecSync.mockReturnValue('gatekeeper-target-1\tUp 5 minutes\n' as any); + mockExecFileSync.mockReturnValue('gatekeeper-target-1\tUp 5 minutes\n' as any); const result = checkContainersRunning(challengeId, containerName); expect(result.ok).toBe(false); expect(result.errors.some((e) => e.includes('gatekeeper-kali-1'))).toBe(true); }); it('returns error when kali container is Exited', () => { - mockExecSync.mockReturnValue( + mockExecFileSync.mockReturnValue( 'gatekeeper-target-1\tUp 5 minutes\ngatekeeper-kali-1\tExited (0) 1 minute ago\n' as any ); const result = checkContainersRunning(challengeId, containerName); @@ -233,8 +240,8 @@ describe('checkContainersRunning', () => { ).toBe(true); }); - it('returns error when execSync throws', () => { - mockExecSync.mockImplementation(() => { + it('returns error when execFileSync throws', () => { + mockExecFileSync.mockImplementation(() => { throw new Error('docker failed'); }); const result = checkContainersRunning(challengeId, containerName); @@ -252,49 +259,42 @@ describe('checkTargetReachable', () => { const url = 'http://target:5000'; it('returns ok when curl returns 200', () => { - mockExecSync.mockReturnValue('200' as any); + mockExecFileSync.mockReturnValue('200' as any); const result = checkTargetReachable(container, url); expect(result.ok).toBe(true); }); it('returns ok when curl returns 404 (connected)', () => { - mockExecSync.mockReturnValue('404' as any); + mockExecFileSync.mockReturnValue('404' as any); const result = checkTargetReachable(container, url); expect(result.ok).toBe(true); }); it('returns error when curl returns 000 (not reachable)', () => { - mockExecSync.mockReturnValue('000' as any); + mockExecFileSync.mockReturnValue('000' as any); const result = checkTargetReachable(container, url); expect(result.ok).toBe(false); expect(result.errors.some((e) => e.includes('not reachable'))).toBe(true); }); - it('returns error when curl returns FAIL', () => { - mockExecSync.mockReturnValue('FAIL' as any); - const result = checkTargetReachable(container, url); - expect(result.ok).toBe(false); - expect(result.errors.some((e) => e.includes('not reachable'))).toBe(true); - }); - - it('returns error when execSync throws (curl missing)', () => { - mockExecSync.mockImplementation(() => { + it('returns error when curl throws (treated as FAIL)', () => { + mockExecFileSync.mockImplementation(() => { throw new Error('exec failed'); }); const result = checkTargetReachable(container, url); expect(result.ok).toBe(false); - expect(result.errors.some((e) => e.includes('curl may be missing'))).toBe(true); + expect(result.errors.some((e) => e.includes('not reachable'))).toBe(true); }); - it('shell-escapes containerName and targetUrl in exec call', () => { - mockExecSync.mockReturnValue('200' as any); + it('passes containerName and targetUrl as separate arguments (no shell escaping needed)', () => { + mockExecFileSync.mockReturnValue('200' as any); checkTargetReachable("evil'name", "http://x';rm -rf /"); - const cmd = mockExecSync.mock.calls[0][0] as string; - // Values should be wrapped in single quotes with escaped internal quotes - expect(cmd).toContain("'evil'\\''name'"); - expect(cmd).toContain("'http://x'\\''"); - // The injected command should be inside single quotes, not a bare shell token - expect(cmd).not.toMatch(/[^'];\s*rm\s/); + const call = mockExecFileSync.mock.calls[0]; + const args = call[1] as string[]; + // With execFileSync, arguments are passed directly — no shell involved + expect(call[0]).toBe('docker'); + expect(args).toContain("evil'name"); + expect(args).toContain("http://x';rm -rf /"); }); }); @@ -306,14 +306,14 @@ describe('checkKaliTools', () => { const container = 'gatekeeper-kali-1'; it('returns ok when all tools are found', () => { - mockExecSync.mockReturnValue('/usr/bin/tool' as any); + mockExecFileSync.mockReturnValue('/usr/bin/tool' as any); const result = checkKaliTools(container); expect(result.ok).toBe(true); expect(result.errors).toHaveLength(0); }); it('returns error when one tool is missing', () => { - mockExecSync + mockExecFileSync .mockReturnValueOnce('/usr/bin/curl' as any) // curl found .mockImplementationOnce(() => { throw new Error('not found'); @@ -327,7 +327,7 @@ describe('checkKaliTools', () => { }); it('returns multiple errors and hints when multiple tools missing', () => { - mockExecSync.mockImplementation(() => { + mockExecFileSync.mockImplementation(() => { throw new Error('not found'); }); const result = checkKaliTools(container); @@ -337,12 +337,13 @@ describe('checkKaliTools', () => { expect(result.hints.some((h) => h.includes('Rebuild'))).toBe(true); }); - it('shell-escapes containerName in exec call', () => { - mockExecSync.mockReturnValue('/usr/bin/tool' as any); + it('passes containerName as a separate argument (no shell escaping needed)', () => { + mockExecFileSync.mockReturnValue('/usr/bin/tool' as any); checkKaliTools("evil'name"); - for (const call of mockExecSync.mock.calls) { - const cmd = call[0] as string; - expect(cmd).toContain("'evil'\\''name'"); + for (const call of mockExecFileSync.mock.calls) { + const args = call[1] as string[]; + // Container name is a discrete array element, not interpolated into a shell string + expect(args).toContain("evil'name"); } }); }); @@ -465,17 +466,17 @@ describe('runPreflightChecks', () => { function mockAllChecksPass() { // docker ps - mockExecSync.mockReturnValueOnce('' as any); + mockExecFileSync.mockReturnValueOnce('' as any); // docker ps -a (containers) - mockExecSync.mockReturnValueOnce( + mockExecFileSync.mockReturnValueOnce( 'gatekeeper-target-1\tUp 5 minutes\ngatekeeper-kali-1\tUp 5 minutes\n' as any ); // curl check - mockExecSync.mockReturnValueOnce('200' as any); + mockExecFileSync.mockReturnValueOnce('200' as any); // which curl, which wget, which python3 - mockExecSync.mockReturnValueOnce('/usr/bin/curl' as any); - mockExecSync.mockReturnValueOnce('/usr/bin/wget' as any); - mockExecSync.mockReturnValueOnce('/usr/bin/python3' as any); + mockExecFileSync.mockReturnValueOnce('/usr/bin/curl' as any); + mockExecFileSync.mockReturnValueOnce('/usr/bin/wget' as any); + mockExecFileSync.mockReturnValueOnce('/usr/bin/python3' as any); } it('returns ok when all checks pass', () => { @@ -485,7 +486,7 @@ describe('runPreflightChecks', () => { }); it('returns docker error on early failure (docker not running)', () => { - mockExecSync.mockImplementation(() => { + mockExecFileSync.mockImplementation(() => { throw new Error('docker not found'); }); const result = runPreflightChecks(challengeId, challengeDir, containerName, targetUrl); @@ -495,9 +496,9 @@ describe('runPreflightChecks', () => { it('returns container error when docker ok but containers fail', () => { // docker ps succeeds - mockExecSync.mockReturnValueOnce('' as any); + mockExecFileSync.mockReturnValueOnce('' as any); // docker ps -a returns no containers - mockExecSync.mockReturnValueOnce('' as any); + mockExecFileSync.mockReturnValueOnce('' as any); const result = runPreflightChecks(challengeId, challengeDir, containerName, targetUrl); expect(result.ok).toBe(false); expect(result.errors.some((e) => e.includes('not found'))).toBe(true); @@ -505,15 +506,15 @@ describe('runPreflightChecks', () => { it('returns tools error when other checks pass but tools missing', () => { // docker ps - mockExecSync.mockReturnValueOnce('' as any); + mockExecFileSync.mockReturnValueOnce('' as any); // docker ps -a (containers) - mockExecSync.mockReturnValueOnce( + mockExecFileSync.mockReturnValueOnce( 'gatekeeper-target-1\tUp 5 minutes\ngatekeeper-kali-1\tUp 5 minutes\n' as any ); // curl check succeeds - mockExecSync.mockReturnValueOnce('200' as any); + mockExecFileSync.mockReturnValueOnce('200' as any); // all tools missing - mockExecSync.mockImplementation(() => { + mockExecFileSync.mockImplementation(() => { throw new Error('not found'); }); const result = runPreflightChecks(challengeId, challengeDir, containerName, targetUrl);