From 5eadccbec9c11cda64051e14803f1af88d6c70b9 Mon Sep 17 00:00:00 2001 From: Sol Date: Fri, 20 Mar 2026 16:34:37 +0000 Subject: [PATCH 1/5] fix(security): harden docker provisioning env validation --- .../lib/services/docker-sandbox-provider.ts | 21 +++--- packages/lib/services/docker-sandbox-utils.ts | 48 ++++++++++++- .../tests/unit/docker-infrastructure.test.ts | 70 +++++++++++++++++++ 3 files changed, 126 insertions(+), 13 deletions(-) diff --git a/packages/lib/services/docker-sandbox-provider.ts b/packages/lib/services/docker-sandbox-provider.ts index 631234ee0..9e91f7597 100644 --- a/packages/lib/services/docker-sandbox-provider.ts +++ b/packages/lib/services/docker-sandbox-provider.ts @@ -24,6 +24,10 @@ import { shellQuote, validateAgentId, validateAgentName, + validateContainerName, + validateEnvKey, + validateEnvValue, + validateVolumePath, WEBUI_PORT_MAX, WEBUI_PORT_MIN, } from "./docker-sandbox-utils"; @@ -257,6 +261,8 @@ export class DockerSandboxProvider implements SandboxProvider { const webUiPort = allocatePort(WEBUI_PORT_MIN, WEBUI_PORT_MAX, usedPorts); const containerName = getContainerName(agentId); const volumePath = getVolumePath(agentId); + validateContainerName(containerName); + validateVolumePath(volumePath); // 4. Optionally prepare Headscale VPN const headscaleEnabled = !!process.env.HEADSCALE_API_KEY; @@ -303,19 +309,12 @@ export class DockerSandboxProvider implements SandboxProvider { MILADY_ALLOWED_ORIGINS: `https://${agentId}.${getAgentBaseDomain()}`, }; - // Validate env var keys to prevent shell command injection via malformed keys - for (const key of Object.keys(allEnv)) { - if (!/^[a-zA-Z_][a-zA-Z0-9_]*$/.test(key)) { - throw new Error(`[docker-sandbox] Invalid environment variable key: "${key}"`); - } + // Validate env keys/values before they are interpolated into remote shell commands. + for (const [key, value] of Object.entries(allEnv)) { + validateEnvKey(key); + validateEnvValue(value); } - // Note: Values do not need control-character validation. The shellQuote() function - // wraps each "key=value" pair in single quotes and escapes embedded single quotes as '"'"', - // which makes all values (including those with newlines, tabs, or other control chars) - // safe inside the shell command. Single-quoted strings in bash preserve all characters - // literally except single quotes (which shellQuote already handles). - const envFlags = Object.entries(allEnv) .map(([key, value]) => `-e ${shellQuote(`${key}=${value}`)}`) .join(" "); diff --git a/packages/lib/services/docker-sandbox-utils.ts b/packages/lib/services/docker-sandbox-utils.ts index 85cb12b98..4fa4ce8f2 100644 --- a/packages/lib/services/docker-sandbox-utils.ts +++ b/packages/lib/services/docker-sandbox-utils.ts @@ -66,6 +66,45 @@ export function validateAgentName(name: string): void { } } +/** Env keys must be uppercase shell-safe identifiers. */ +export function validateEnvKey(key: string): void { + if (!/^[A-Z_][A-Z0-9_]*$/.test(key)) { + throw new Error( + `Invalid environment variable key "${key}": must match ^[A-Z_][A-Z0-9_]*$.`, + ); + } +} + +/** Env values may not contain null bytes or other control characters. */ +export function validateEnvValue(value: string): void { + if (/[\x00-\x1f\x7f]/.test(value)) { + throw new Error(`Invalid environment variable value: contains control characters.`); + } +} + +/** Docker container names must be simple shell-safe identifiers. */ +export function validateContainerName(containerName: string): void { + if (!/^[a-zA-Z0-9][a-zA-Z0-9_.-]{0,127}$/.test(containerName)) { + throw new Error(`Invalid container name "${containerName}".`); + } +} + +/** Docker host volume paths must be absolute, normalized, and shell-safe. */ +export function validateVolumePath(volumePath: string): void { + if (!/^\/[A-Za-z0-9._/-]+$/.test(volumePath)) { + throw new Error(`Invalid volume path "${volumePath}".`); + } + if ( + volumePath.includes("//") || + volumePath.includes("/./") || + volumePath.includes("/../") || + volumePath.endsWith("/.") || + volumePath.endsWith("/..") + ) { + throw new Error(`Invalid volume path "${volumePath}": path must be normalized.`); + } +} + // --------------------------------------------------------------------------- // Port Allocation // --------------------------------------------------------------------------- @@ -107,13 +146,18 @@ export function allocatePort(min: number, max: number, excluded: Set): n * patterns and can collide on the same node). */ export function getContainerName(agentId: string): string { - return `milady-${agentId}`; + validateAgentId(agentId); + const containerName = `milady-${agentId}`; + validateContainerName(containerName); + return containerName; } /** Volume path on the Docker host for persistent agent data. */ export function getVolumePath(agentId: string): string { validateAgentId(agentId); - return `/data/agents/${agentId}`; + const volumePath = `/data/agents/${agentId}`; + validateVolumePath(volumePath); + return volumePath; } // --------------------------------------------------------------------------- diff --git a/packages/tests/unit/docker-infrastructure.test.ts b/packages/tests/unit/docker-infrastructure.test.ts index 655a6e591..e819d52e7 100644 --- a/packages/tests/unit/docker-infrastructure.test.ts +++ b/packages/tests/unit/docker-infrastructure.test.ts @@ -13,6 +13,10 @@ import { shellQuote, validateAgentId, validateAgentName, + validateContainerName, + validateEnvKey, + validateEnvValue, + validateVolumePath, } from "@/lib/services/docker-sandbox-utils"; // --------------------------------------------------------------------------- @@ -205,6 +209,72 @@ describe("Docker Infrastructure - Pure Functions", () => { }); }); + // ------------------------------------------------------------------------- + describe("validateEnvKey", () => { + test("accepts uppercase underscore keys", () => { + expect(() => validateEnvKey("JWT_SECRET")).not.toThrow(); + expect(() => validateEnvKey("A1_B2")).not.toThrow(); + }); + + test("rejects lowercase keys", () => { + expect(() => validateEnvKey("jwt_secret")).toThrow(/Invalid environment variable key/); + }); + + test("rejects keys starting with a digit", () => { + expect(() => validateEnvKey("1SECRET")).toThrow(/Invalid environment variable key/); + }); + + test("rejects punctuation", () => { + expect(() => validateEnvKey("JWT-SECRET")).toThrow(/Invalid environment variable key/); + }); + }); + + // ------------------------------------------------------------------------- + describe("validateEnvValue", () => { + test("accepts printable values", () => { + expect(() => validateEnvValue("hello-world_123")).not.toThrow(); + }); + + test("rejects null bytes", () => { + expect(() => validateEnvValue("abc\x00def")).toThrow(/control characters/); + }); + + test("rejects newlines", () => { + expect(() => validateEnvValue("abc\ndef")).toThrow(/control characters/); + }); + + test("rejects tabs", () => { + expect(() => validateEnvValue("abc\tdef")).toThrow(/control characters/); + }); + }); + + // ------------------------------------------------------------------------- + describe("container name and volume path validation", () => { + test("getContainerName returns a valid deterministic name", () => { + expect(getContainerName("agent_1")).toBe("milady-agent_1"); + }); + + test("validateContainerName accepts docker-safe names", () => { + expect(() => validateContainerName("milady-agent_1.test-2")).not.toThrow(); + }); + + test("validateContainerName rejects shell metacharacters", () => { + expect(() => validateContainerName("bad;name")).toThrow(/Invalid container name/); + }); + + test("getVolumePath returns a normalized absolute path", () => { + expect(getVolumePath("agent_1")).toBe("/data/agents/agent_1"); + }); + + test("validateVolumePath rejects traversal", () => { + expect(() => validateVolumePath("/data/agents/../escape")).toThrow(/Invalid volume path/); + }); + + test("validateVolumePath rejects non-absolute paths", () => { + expect(() => validateVolumePath("data/agents/agent_1")).toThrow(/Invalid volume path/); + }); + }); + // ------------------------------------------------------------------------- describe("allocatePort", () => { test("returns a port within [min, max)", () => { From d269a1ae591b8a23baa36c0e7e29a8f1a8d870de Mon Sep 17 00:00:00 2001 From: Sol Date: Fri, 20 Mar 2026 16:42:23 +0000 Subject: [PATCH 2/5] fix: address PR review on docker env validation --- .../lib/services/docker-sandbox-provider.ts | 6 +-- packages/lib/services/docker-sandbox-utils.ts | 17 +++++--- .../tests/unit/docker-infrastructure.test.ts | 43 ++++++++++++++++--- 3 files changed, 52 insertions(+), 14 deletions(-) diff --git a/packages/lib/services/docker-sandbox-provider.ts b/packages/lib/services/docker-sandbox-provider.ts index 9e91f7597..5edeecd38 100644 --- a/packages/lib/services/docker-sandbox-provider.ts +++ b/packages/lib/services/docker-sandbox-provider.ts @@ -261,8 +261,6 @@ export class DockerSandboxProvider implements SandboxProvider { const webUiPort = allocatePort(WEBUI_PORT_MIN, WEBUI_PORT_MAX, usedPorts); const containerName = getContainerName(agentId); const volumePath = getVolumePath(agentId); - validateContainerName(containerName); - validateVolumePath(volumePath); // 4. Optionally prepare Headscale VPN const headscaleEnabled = !!process.env.HEADSCALE_API_KEY; @@ -310,9 +308,11 @@ export class DockerSandboxProvider implements SandboxProvider { }; // Validate env keys/values before they are interpolated into remote shell commands. + // Internal env vars must also remain UPPER_SNAKE_CASE so validation stays + // consistent across caller-supplied and provider-generated values. for (const [key, value] of Object.entries(allEnv)) { validateEnvKey(key); - validateEnvValue(value); + validateEnvValue(key, value); } const envFlags = Object.entries(allEnv) diff --git a/packages/lib/services/docker-sandbox-utils.ts b/packages/lib/services/docker-sandbox-utils.ts index 4fa4ce8f2..3acaf4a43 100644 --- a/packages/lib/services/docker-sandbox-utils.ts +++ b/packages/lib/services/docker-sandbox-utils.ts @@ -75,10 +75,16 @@ export function validateEnvKey(key: string): void { } } -/** Env values may not contain null bytes or other control characters. */ -export function validateEnvValue(value: string): void { +/** + * Env values are shell-safe once single-quoted, but we still reject control + * characters so multi-line payloads and invisible bytes cannot reach the remote + * shell command. Callers should pass a key so production errors are debuggable. + */ +export function validateEnvValue(key: string, value: string): void { if (/[\x00-\x1f\x7f]/.test(value)) { - throw new Error(`Invalid environment variable value: contains control characters.`); + throw new Error( + `Invalid environment variable value for key "${key}": contains control characters (newlines and PEM-encoded values are not supported).`, + ); } } @@ -91,7 +97,7 @@ export function validateContainerName(containerName: string): void { /** Docker host volume paths must be absolute, normalized, and shell-safe. */ export function validateVolumePath(volumePath: string): void { - if (!/^\/[A-Za-z0-9._/-]+$/.test(volumePath)) { + if (!/^\/[A-Za-z0-9._/\-]+$/.test(volumePath)) { throw new Error(`Invalid volume path "${volumePath}".`); } if ( @@ -99,7 +105,8 @@ export function validateVolumePath(volumePath: string): void { volumePath.includes("/./") || volumePath.includes("/../") || volumePath.endsWith("/.") || - volumePath.endsWith("/..") + volumePath.endsWith("/..") || + (volumePath.length > 1 && volumePath.endsWith("/")) ) { throw new Error(`Invalid volume path "${volumePath}": path must be normalized.`); } diff --git a/packages/tests/unit/docker-infrastructure.test.ts b/packages/tests/unit/docker-infrastructure.test.ts index e819d52e7..f47a43edf 100644 --- a/packages/tests/unit/docker-infrastructure.test.ts +++ b/packages/tests/unit/docker-infrastructure.test.ts @@ -232,19 +232,34 @@ describe("Docker Infrastructure - Pure Functions", () => { // ------------------------------------------------------------------------- describe("validateEnvValue", () => { test("accepts printable values", () => { - expect(() => validateEnvValue("hello-world_123")).not.toThrow(); + expect(() => validateEnvValue("JWT_SECRET", "hello-world_123")).not.toThrow(); }); - test("rejects null bytes", () => { - expect(() => validateEnvValue("abc\x00def")).toThrow(/control characters/); + test("accepts UUIDs, URLs, and base64-like tokens", () => { + expect(() => + validateEnvValue( + "MIXED_VALUE", + "550e8400-e29b-41d4-a716-446655440000 https://example.com/a?b=c token+/=", + ), + ).not.toThrow(); }); - test("rejects newlines", () => { - expect(() => validateEnvValue("abc\ndef")).toThrow(/control characters/); + test("rejects null bytes and includes the key name", () => { + expect(() => validateEnvValue("JWT_SECRET", "abc\x00def")).toThrow( + /JWT_SECRET.*control characters/, + ); + }); + + test("rejects newlines and explains PEM-style values are unsupported", () => { + expect(() => validateEnvValue("TLS_CERT", "abc\ndef")).toThrow( + /TLS_CERT.*newlines and PEM-encoded values are not supported/, + ); }); test("rejects tabs", () => { - expect(() => validateEnvValue("abc\tdef")).toThrow(/control characters/); + expect(() => validateEnvValue("JWT_SECRET", "abc\tdef")).toThrow( + /control characters/, + ); }); }); @@ -258,10 +273,22 @@ describe("Docker Infrastructure - Pure Functions", () => { expect(() => validateContainerName("milady-agent_1.test-2")).not.toThrow(); }); + test("validateContainerName accepts the 128-character boundary", () => { + const containerName = `m${"a".repeat(127)}`; + expect(containerName).toHaveLength(128); + expect(() => validateContainerName(containerName)).not.toThrow(); + }); + test("validateContainerName rejects shell metacharacters", () => { expect(() => validateContainerName("bad;name")).toThrow(/Invalid container name/); }); + test("validateContainerName rejects names longer than 128 characters", () => { + const containerName = `m${"a".repeat(128)}`; + expect(containerName).toHaveLength(129); + expect(() => validateContainerName(containerName)).toThrow(/Invalid container name/); + }); + test("getVolumePath returns a normalized absolute path", () => { expect(getVolumePath("agent_1")).toBe("/data/agents/agent_1"); }); @@ -270,6 +297,10 @@ describe("Docker Infrastructure - Pure Functions", () => { expect(() => validateVolumePath("/data/agents/../escape")).toThrow(/Invalid volume path/); }); + test("validateVolumePath rejects trailing slashes", () => { + expect(() => validateVolumePath("/data/agents/")).toThrow(/path must be normalized/); + }); + test("validateVolumePath rejects non-absolute paths", () => { expect(() => validateVolumePath("data/agents/agent_1")).toThrow(/Invalid volume path/); }); From 0d7c79257243cd6499072710fe11ec4d3e0f6f9e Mon Sep 17 00:00:00 2001 From: Sol Date: Fri, 20 Mar 2026 16:49:56 +0000 Subject: [PATCH 3/5] fix: align agent id validation with docker name limits --- packages/lib/services/docker-sandbox-utils.ts | 16 +++++++++++----- .../tests/unit/docker-infrastructure.test.ts | 14 ++++++++++---- 2 files changed, 21 insertions(+), 9 deletions(-) diff --git a/packages/lib/services/docker-sandbox-utils.ts b/packages/lib/services/docker-sandbox-utils.ts index 3acaf4a43..11674fc01 100644 --- a/packages/lib/services/docker-sandbox-utils.ts +++ b/packages/lib/services/docker-sandbox-utils.ts @@ -26,6 +26,10 @@ export const BRIDGE_PORT_MIN = 18790; export const BRIDGE_PORT_MAX = 19790; export const WEBUI_PORT_MIN = 20000; export const WEBUI_PORT_MAX = 25000; +export const DOCKER_CONTAINER_NAME_MAX_LENGTH = 128; +export const MILADY_CONTAINER_NAME_PREFIX = "milady-"; +export const MAX_AGENT_ID_LENGTH = + DOCKER_CONTAINER_NAME_MAX_LENGTH - MILADY_CONTAINER_NAME_PREFIX.length; // --------------------------------------------------------------------------- // Shell Quoting @@ -44,13 +48,14 @@ export function shellQuote(value: string): string { // --------------------------------------------------------------------------- /** - * Validate an agent ID before using it in shell commands. - * Must be a UUID (hex + hyphens) or alphanumeric with hyphens/underscores. + * Validate an agent ID before using it in Docker-derived names and shell commands. + * Must fit within Docker's 128-char container name limit after the `milady-` + * prefix is applied. */ export function validateAgentId(agentId: string): void { - if (!/^[a-zA-Z0-9_-]{1,128}$/.test(agentId)) { + if (!new RegExp(`^[a-zA-Z0-9_-]{1,${MAX_AGENT_ID_LENGTH}}$`).test(agentId)) { throw new Error( - `Invalid agent ID "${agentId}": must be 1-128 chars, alphanumeric / hyphens / underscores only.`, + `Invalid agent ID "${agentId}": must be 1-${MAX_AGENT_ID_LENGTH} chars, alphanumeric / hyphens / underscores only.`, ); } } @@ -154,7 +159,8 @@ export function allocatePort(min: number, max: number, excluded: Set): n */ export function getContainerName(agentId: string): string { validateAgentId(agentId); - const containerName = `milady-${agentId}`; + const containerName = `${MILADY_CONTAINER_NAME_PREFIX}${agentId}`; + // Keep this derived-output validation as a guardrail if the naming template changes. validateContainerName(containerName); return containerName; } diff --git a/packages/tests/unit/docker-infrastructure.test.ts b/packages/tests/unit/docker-infrastructure.test.ts index f47a43edf..7aa195e52 100644 --- a/packages/tests/unit/docker-infrastructure.test.ts +++ b/packages/tests/unit/docker-infrastructure.test.ts @@ -9,6 +9,7 @@ import { allocatePort, getContainerName, getVolumePath, + MAX_AGENT_ID_LENGTH, parseDockerNodes, shellQuote, validateAgentId, @@ -129,8 +130,8 @@ describe("Docker Infrastructure - Pure Functions", () => { expect(() => validateAgentId("a")).not.toThrow(); }); - test("accepts exactly 128 characters", () => { - const id = "a".repeat(128); + test("accepts exactly the Docker-safe maximum length", () => { + const id = "a".repeat(MAX_AGENT_ID_LENGTH); expect(() => validateAgentId(id)).not.toThrow(); }); @@ -138,8 +139,8 @@ describe("Docker Infrastructure - Pure Functions", () => { expect(() => validateAgentId("")).toThrow(/Invalid agent ID/); }); - test("throws for 129 characters (too long)", () => { - const id = "a".repeat(129); + test("throws when the agentId would exceed the Docker container name limit", () => { + const id = "a".repeat(MAX_AGENT_ID_LENGTH + 1); expect(() => validateAgentId(id)).toThrow(/Invalid agent ID/); }); @@ -372,6 +373,11 @@ describe("Docker Infrastructure - Pure Functions", () => { test("works with a short agentId", () => { expect(getContainerName("x")).toBe("milady-x"); }); + + test("rejects an agentId that would exceed the Docker container name limit", () => { + const longId = "a".repeat(MAX_AGENT_ID_LENGTH + 1); + expect(() => getContainerName(longId)).toThrow(/Invalid agent ID/); + }); }); // ------------------------------------------------------------------------- From 9e7f60ce29c9b25ce6724ebdc45ec7edb5882ade Mon Sep 17 00:00:00 2001 From: Sol Date: Fri, 20 Mar 2026 16:59:00 +0000 Subject: [PATCH 4/5] fix: reject control characters in docker validators --- packages/lib/services/docker-sandbox-utils.ts | 27 ++++++++++++++----- .../tests/unit/docker-infrastructure.test.ts | 16 +++++++++++ 2 files changed, 36 insertions(+), 7 deletions(-) diff --git a/packages/lib/services/docker-sandbox-utils.ts b/packages/lib/services/docker-sandbox-utils.ts index 11674fc01..1334bbaf6 100644 --- a/packages/lib/services/docker-sandbox-utils.ts +++ b/packages/lib/services/docker-sandbox-utils.ts @@ -47,13 +47,22 @@ export function shellQuote(value: string): string { // Validation // --------------------------------------------------------------------------- +function hasControlChars(value: string): boolean { + return /[\x00-\x1f\x7f]/.test(value); +} + /** * Validate an agent ID before using it in Docker-derived names and shell commands. * Must fit within Docker's 128-char container name limit after the `milady-` * prefix is applied. */ export function validateAgentId(agentId: string): void { - if (!new RegExp(`^[a-zA-Z0-9_-]{1,${MAX_AGENT_ID_LENGTH}}$`).test(agentId)) { + if ( + agentId.length === 0 || + agentId.length > MAX_AGENT_ID_LENGTH || + hasControlChars(agentId) || + !/^[a-zA-Z0-9_-]+$/.test(agentId) + ) { throw new Error( `Invalid agent ID "${agentId}": must be 1-${MAX_AGENT_ID_LENGTH} chars, alphanumeric / hyphens / underscores only.`, ); @@ -71,9 +80,9 @@ export function validateAgentName(name: string): void { } } -/** Env keys must be uppercase shell-safe identifiers. */ +/** Env keys must be uppercase shell-safe identifiers; lowercase keys are intentionally rejected. */ export function validateEnvKey(key: string): void { - if (!/^[A-Z_][A-Z0-9_]*$/.test(key)) { + if (hasControlChars(key) || !/^[A-Z_][A-Z0-9_]*$/.test(key)) { throw new Error( `Invalid environment variable key "${key}": must match ^[A-Z_][A-Z0-9_]*$.`, ); @@ -86,7 +95,7 @@ export function validateEnvKey(key: string): void { * shell command. Callers should pass a key so production errors are debuggable. */ export function validateEnvValue(key: string, value: string): void { - if (/[\x00-\x1f\x7f]/.test(value)) { + if (hasControlChars(value)) { throw new Error( `Invalid environment variable value for key "${key}": contains control characters (newlines and PEM-encoded values are not supported).`, ); @@ -95,14 +104,18 @@ export function validateEnvValue(key: string, value: string): void { /** Docker container names must be simple shell-safe identifiers. */ export function validateContainerName(containerName: string): void { - if (!/^[a-zA-Z0-9][a-zA-Z0-9_.-]{0,127}$/.test(containerName)) { - throw new Error(`Invalid container name "${containerName}".`); + if (hasControlChars(containerName) || !/^[a-zA-Z0-9][a-zA-Z0-9_.-]{0,127}$/.test(containerName)) { + throw new Error( + `Invalid container name "${containerName}": must match ^[a-zA-Z0-9][a-zA-Z0-9_.-]{0,127}$.`, + ); } } /** Docker host volume paths must be absolute, normalized, and shell-safe. */ export function validateVolumePath(volumePath: string): void { - if (!/^\/[A-Za-z0-9._/\-]+$/.test(volumePath)) { + // First allow only absolute shell-safe path characters, then separately enforce + // normalized-form rules like no traversal, repeated separators, or trailing slash. + if (hasControlChars(volumePath) || !/^\/[A-Za-z0-9._/\-]+$/.test(volumePath)) { throw new Error(`Invalid volume path "${volumePath}".`); } if ( diff --git a/packages/tests/unit/docker-infrastructure.test.ts b/packages/tests/unit/docker-infrastructure.test.ts index 7aa195e52..591cb8bcf 100644 --- a/packages/tests/unit/docker-infrastructure.test.ts +++ b/packages/tests/unit/docker-infrastructure.test.ts @@ -148,6 +148,10 @@ describe("Docker Infrastructure - Pure Functions", () => { expect(() => validateAgentId("agent;rm -rf /")).toThrow(/Invalid agent ID/); }); + test("throws for trailing newline", () => { + expect(() => validateAgentId("agent_1\n")).toThrow(/Invalid agent ID/); + }); + test("throws for dots", () => { expect(() => validateAgentId("agent.1")).toThrow(/Invalid agent ID/); }); @@ -228,6 +232,10 @@ describe("Docker Infrastructure - Pure Functions", () => { test("rejects punctuation", () => { expect(() => validateEnvKey("JWT-SECRET")).toThrow(/Invalid environment variable key/); }); + + test("rejects trailing newlines", () => { + expect(() => validateEnvKey("JWT_SECRET\n")).toThrow(/Invalid environment variable key/); + }); }); // ------------------------------------------------------------------------- @@ -284,6 +292,10 @@ describe("Docker Infrastructure - Pure Functions", () => { expect(() => validateContainerName("bad;name")).toThrow(/Invalid container name/); }); + test("validateContainerName rejects trailing newlines", () => { + expect(() => validateContainerName("milady-agent\n")).toThrow(/Invalid container name/); + }); + test("validateContainerName rejects names longer than 128 characters", () => { const containerName = `m${"a".repeat(128)}`; expect(containerName).toHaveLength(129); @@ -302,6 +314,10 @@ describe("Docker Infrastructure - Pure Functions", () => { expect(() => validateVolumePath("/data/agents/")).toThrow(/path must be normalized/); }); + test("validateVolumePath rejects trailing newlines", () => { + expect(() => validateVolumePath("/data/agents/agent_1\n")).toThrow(/Invalid volume path/); + }); + test("validateVolumePath rejects non-absolute paths", () => { expect(() => validateVolumePath("data/agents/agent_1")).toThrow(/Invalid volume path/); }); From ff267f6967e1234a4ca2d833b90f3dd168f04ae9 Mon Sep 17 00:00:00 2001 From: Sol Date: Sun, 22 Mar 2026 08:56:00 +0000 Subject: [PATCH 5/5] test: cover empty env keys and root volume path --- packages/lib/services/docker-sandbox-utils.ts | 11 ++++++++--- packages/tests/unit/docker-infrastructure.test.ts | 8 ++++++++ 2 files changed, 16 insertions(+), 3 deletions(-) diff --git a/packages/lib/services/docker-sandbox-utils.ts b/packages/lib/services/docker-sandbox-utils.ts index 1334bbaf6..a74d61016 100644 --- a/packages/lib/services/docker-sandbox-utils.ts +++ b/packages/lib/services/docker-sandbox-utils.ts @@ -113,9 +113,14 @@ export function validateContainerName(containerName: string): void { /** Docker host volume paths must be absolute, normalized, and shell-safe. */ export function validateVolumePath(volumePath: string): void { - // First allow only absolute shell-safe path characters, then separately enforce - // normalized-form rules like no traversal, repeated separators, or trailing slash. - if (hasControlChars(volumePath) || !/^\/[A-Za-z0-9._/\-]+$/.test(volumePath)) { + // First allow only absolute shell-safe path characters, reject the root path, + // then separately enforce normalized-form rules like no traversal, repeated + // separators, or trailing slash. + if ( + hasControlChars(volumePath) || + volumePath === "/" || + !/^\/[A-Za-z0-9._/\-]+$/.test(volumePath) + ) { throw new Error(`Invalid volume path "${volumePath}".`); } if ( diff --git a/packages/tests/unit/docker-infrastructure.test.ts b/packages/tests/unit/docker-infrastructure.test.ts index 591cb8bcf..180be7a97 100644 --- a/packages/tests/unit/docker-infrastructure.test.ts +++ b/packages/tests/unit/docker-infrastructure.test.ts @@ -221,6 +221,10 @@ describe("Docker Infrastructure - Pure Functions", () => { expect(() => validateEnvKey("A1_B2")).not.toThrow(); }); + test("rejects empty keys", () => { + expect(() => validateEnvKey("")).toThrow(/Invalid environment variable key/); + }); + test("rejects lowercase keys", () => { expect(() => validateEnvKey("jwt_secret")).toThrow(/Invalid environment variable key/); }); @@ -321,6 +325,10 @@ describe("Docker Infrastructure - Pure Functions", () => { test("validateVolumePath rejects non-absolute paths", () => { expect(() => validateVolumePath("data/agents/agent_1")).toThrow(/Invalid volume path/); }); + + test("validateVolumePath rejects the root path", () => { + expect(() => validateVolumePath("/")).toThrow(/Invalid volume path/); + }); }); // -------------------------------------------------------------------------