Merge pull request #64 from KryptSec/fix/windows-shell-escape

fix: bypass shell entirely for cross-platform Docker commands

Author: Treelovah

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,16 +161,16 @@ 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 {
     // Containers may not exist
   }
 
   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',
   });

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<DockerStartResult> {
   // 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 {

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, "'\\''") + "'";
 }