From d9efa859248894ca0b6484b1364aaa736a71dc19 Mon Sep 17 00:00:00 2001 From: Jason Woltje Date: Fri, 6 Feb 2026 13:46:47 -0600 Subject: [PATCH] fix(SEC-ORCH-22): Validate Docker image tag format before pull Add validateImageTag() method to DockerSandboxService that validates Docker image references against a safe character pattern before any container creation. Rejects empty tags, tags exceeding 256 characters, and tags containing shell metacharacters (;, &, |, $, backtick, etc.) to prevent injection attacks. Also validates the default image tag at service construction time to fail fast on misconfiguration. Co-Authored-By: Claude Opus 4.6 --- .../spawner/docker-sandbox.service.spec.ts | 203 ++++++++++++++++++ .../src/spawner/docker-sandbox.service.ts | 50 +++++ 2 files changed, 253 insertions(+) diff --git a/apps/orchestrator/src/spawner/docker-sandbox.service.spec.ts b/apps/orchestrator/src/spawner/docker-sandbox.service.spec.ts index 02e8573..c7e2de1 100644 --- a/apps/orchestrator/src/spawner/docker-sandbox.service.spec.ts +++ b/apps/orchestrator/src/spawner/docker-sandbox.service.spec.ts @@ -5,6 +5,8 @@ import { DockerSandboxService, DEFAULT_ENV_WHITELIST, DEFAULT_SECURITY_OPTIONS, + DOCKER_IMAGE_TAG_PATTERN, + MAX_IMAGE_TAG_LENGTH, } from "./docker-sandbox.service"; import { DockerSecurityOptions, LinuxCapability } from "./types/docker-sandbox.types"; import Docker from "dockerode"; @@ -605,6 +607,207 @@ describe("DockerSandboxService", () => { }); }); + describe("Docker image tag validation", () => { + describe("DOCKER_IMAGE_TAG_PATTERN", () => { + it("should match simple image names", () => { + expect(DOCKER_IMAGE_TAG_PATTERN.test("node")).toBe(true); + expect(DOCKER_IMAGE_TAG_PATTERN.test("ubuntu")).toBe(true); + expect(DOCKER_IMAGE_TAG_PATTERN.test("alpine")).toBe(true); + }); + + it("should match image names with tags", () => { + expect(DOCKER_IMAGE_TAG_PATTERN.test("node:20-alpine")).toBe(true); + expect(DOCKER_IMAGE_TAG_PATTERN.test("ubuntu:22.04")).toBe(true); + expect(DOCKER_IMAGE_TAG_PATTERN.test("python:3.11-slim")).toBe(true); + }); + + it("should match image names with registry", () => { + expect(DOCKER_IMAGE_TAG_PATTERN.test("docker.io/library/node")).toBe(true); + expect(DOCKER_IMAGE_TAG_PATTERN.test("ghcr.io/owner/image:latest")).toBe(true); + expect(DOCKER_IMAGE_TAG_PATTERN.test("registry.example.com/myapp:v1.0")).toBe(true); + }); + + it("should match image names with sha256 digest", () => { + expect(DOCKER_IMAGE_TAG_PATTERN.test("node@sha256:abc123def456")).toBe(true); + expect( + DOCKER_IMAGE_TAG_PATTERN.test( + "ubuntu@sha256:a1b2c3d4e5f6a1b2c3d4e5f6a1b2c3d4e5f6a1b2c3d4e5f6a1b2c3d4e5f6a1b2" + ) + ).toBe(true); + }); + + it("should reject images with shell metacharacters", () => { + expect(DOCKER_IMAGE_TAG_PATTERN.test("node;rm -rf /")).toBe(false); + expect(DOCKER_IMAGE_TAG_PATTERN.test("node|cat /etc/passwd")).toBe(false); + expect(DOCKER_IMAGE_TAG_PATTERN.test("node&echo pwned")).toBe(false); + expect(DOCKER_IMAGE_TAG_PATTERN.test("node$(whoami)")).toBe(false); + expect(DOCKER_IMAGE_TAG_PATTERN.test("node`whoami`")).toBe(false); + expect(DOCKER_IMAGE_TAG_PATTERN.test("node > /tmp/out")).toBe(false); + expect(DOCKER_IMAGE_TAG_PATTERN.test("node < /etc/passwd")).toBe(false); + }); + + it("should reject images with spaces", () => { + expect(DOCKER_IMAGE_TAG_PATTERN.test("node 20-alpine")).toBe(false); + expect(DOCKER_IMAGE_TAG_PATTERN.test(" node")).toBe(false); + expect(DOCKER_IMAGE_TAG_PATTERN.test("node ")).toBe(false); + }); + + it("should reject images with newlines", () => { + expect(DOCKER_IMAGE_TAG_PATTERN.test("node\n")).toBe(false); + expect(DOCKER_IMAGE_TAG_PATTERN.test("node\rmalicious")).toBe(false); + }); + + it("should reject images starting with non-alphanumeric characters", () => { + expect(DOCKER_IMAGE_TAG_PATTERN.test(".node")).toBe(false); + expect(DOCKER_IMAGE_TAG_PATTERN.test("-node")).toBe(false); + expect(DOCKER_IMAGE_TAG_PATTERN.test("/node")).toBe(false); + expect(DOCKER_IMAGE_TAG_PATTERN.test("_node")).toBe(false); + }); + }); + + describe("MAX_IMAGE_TAG_LENGTH", () => { + it("should be 256", () => { + expect(MAX_IMAGE_TAG_LENGTH).toBe(256); + }); + }); + + describe("validateImageTag", () => { + it("should accept valid simple image names", () => { + expect(() => service.validateImageTag("node")).not.toThrow(); + expect(() => service.validateImageTag("ubuntu")).not.toThrow(); + expect(() => service.validateImageTag("node:20-alpine")).not.toThrow(); + }); + + it("should accept valid registry-qualified image names", () => { + expect(() => service.validateImageTag("docker.io/library/node:20")).not.toThrow(); + expect(() => service.validateImageTag("ghcr.io/owner/image:latest")).not.toThrow(); + expect(() => + service.validateImageTag("registry.example.com/namespace/image:v1.2.3") + ).not.toThrow(); + }); + + it("should accept valid image names with sha256 digest", () => { + expect(() => service.validateImageTag("node@sha256:abc123def456")).not.toThrow(); + }); + + it("should reject empty image tags", () => { + expect(() => service.validateImageTag("")).toThrow("Docker image tag must not be empty"); + }); + + it("should reject whitespace-only image tags", () => { + expect(() => service.validateImageTag(" ")).toThrow("Docker image tag must not be empty"); + }); + + it("should reject image tags exceeding maximum length", () => { + const longImage = "a" + "b".repeat(MAX_IMAGE_TAG_LENGTH); + expect(() => service.validateImageTag(longImage)).toThrow( + "Docker image tag exceeds maximum length" + ); + }); + + it("should reject image tags with shell metacharacters", () => { + expect(() => service.validateImageTag("node;rm -rf /")).toThrow( + "Docker image tag contains invalid characters" + ); + expect(() => service.validateImageTag("node|cat /etc/passwd")).toThrow( + "Docker image tag contains invalid characters" + ); + expect(() => service.validateImageTag("node&echo pwned")).toThrow( + "Docker image tag contains invalid characters" + ); + expect(() => service.validateImageTag("node$(whoami)")).toThrow( + "Docker image tag contains invalid characters" + ); + expect(() => service.validateImageTag("node`whoami`")).toThrow( + "Docker image tag contains invalid characters" + ); + }); + + it("should reject image tags with spaces", () => { + expect(() => service.validateImageTag("node 20-alpine")).toThrow( + "Docker image tag contains invalid characters" + ); + }); + + it("should reject image tags starting with non-alphanumeric", () => { + expect(() => service.validateImageTag(".hidden")).toThrow( + "Docker image tag contains invalid characters" + ); + expect(() => service.validateImageTag("-hyphen")).toThrow( + "Docker image tag contains invalid characters" + ); + }); + }); + + describe("createContainer with image tag validation", () => { + it("should reject container creation with invalid image tag", async () => { + const agentId = "agent-123"; + const taskId = "task-456"; + const workspacePath = "/workspace/agent-123"; + const options = { image: "malicious;rm -rf /" }; + + await expect( + service.createContainer(agentId, taskId, workspacePath, options) + ).rejects.toThrow("Docker image tag contains invalid characters"); + + expect(mockDocker.createContainer).not.toHaveBeenCalled(); + }); + + it("should reject container creation with empty image tag", async () => { + const agentId = "agent-123"; + const taskId = "task-456"; + const workspacePath = "/workspace/agent-123"; + const options = { image: "" }; + + await expect( + service.createContainer(agentId, taskId, workspacePath, options) + ).rejects.toThrow("Docker image tag must not be empty"); + + expect(mockDocker.createContainer).not.toHaveBeenCalled(); + }); + + it("should allow container creation with valid image tag", async () => { + const agentId = "agent-123"; + const taskId = "task-456"; + const workspacePath = "/workspace/agent-123"; + const options = { image: "node:20-alpine" }; + + await service.createContainer(agentId, taskId, workspacePath, options); + + expect(mockDocker.createContainer).toHaveBeenCalledWith( + expect.objectContaining({ + Image: "node:20-alpine", + }) + ); + }); + + it("should validate default image tag on construction", () => { + // Constructor with valid default image should succeed + expect(() => new DockerSandboxService(mockConfigService, mockDocker)).not.toThrow(); + }); + + it("should reject construction with invalid default image tag", () => { + const badConfigService = { + get: vi.fn((key: string, defaultValue?: unknown) => { + const config: Record = { + "orchestrator.docker.socketPath": "/var/run/docker.sock", + "orchestrator.sandbox.enabled": true, + "orchestrator.sandbox.defaultImage": "bad image;inject", + "orchestrator.sandbox.defaultMemoryMB": 512, + "orchestrator.sandbox.defaultCpuLimit": 1.0, + "orchestrator.sandbox.networkMode": "bridge", + }; + return config[key] !== undefined ? config[key] : defaultValue; + }), + } as unknown as ConfigService; + + expect(() => new DockerSandboxService(badConfigService, mockDocker)).toThrow( + "Docker image tag contains invalid characters" + ); + }); + }); + }); + describe("security hardening options", () => { describe("DEFAULT_SECURITY_OPTIONS", () => { it("should drop all Linux capabilities by default", () => { diff --git a/apps/orchestrator/src/spawner/docker-sandbox.service.ts b/apps/orchestrator/src/spawner/docker-sandbox.service.ts index 705f2c6..d0eabbd 100644 --- a/apps/orchestrator/src/spawner/docker-sandbox.service.ts +++ b/apps/orchestrator/src/spawner/docker-sandbox.service.ts @@ -8,6 +8,23 @@ import { LinuxCapability, } from "./types/docker-sandbox.types"; +/** + * Maximum allowed length for a Docker image reference. + * Docker image names rarely exceed 128 characters; 256 provides generous headroom. + */ +export const MAX_IMAGE_TAG_LENGTH = 256; + +/** + * Regex pattern for validating Docker image tag references. + * Allows: registry/namespace/image:tag or image@sha256:digest + * Valid characters: alphanumeric, dots, hyphens, underscores, forward slashes, colons, and @. + * Blocks shell metacharacters (;, &, |, $, backtick, spaces, newlines, etc.) to prevent injection. + * + * Uses a simple character-class approach (no alternation or nested quantifiers) + * to avoid catastrophic backtracking. + */ +export const DOCKER_IMAGE_TAG_PATTERN = /^[a-zA-Z0-9][a-zA-Z0-9./_:@-]*$/; + /** * Default whitelist of allowed environment variable names/patterns for Docker containers. * Only these variables will be passed to spawned agent containers. @@ -127,6 +144,9 @@ export class DockerSandboxService { noNewPrivileges: configNoNewPrivileges ?? DEFAULT_SECURITY_OPTIONS.noNewPrivileges, }; + // Validate default image tag at startup to fail fast on misconfiguration + this.validateImageTag(this.defaultImage); + this.logger.log( `DockerSandboxService initialized (enabled: ${this.sandboxEnabled.toString()}, socket: ${socketPath})` ); @@ -144,6 +164,32 @@ export class DockerSandboxService { } } + /** + * Validate a Docker image tag reference. + * Ensures the image tag only contains safe characters and is within length limits. + * Blocks shell metacharacters and suspicious patterns to prevent injection attacks. + * @param imageTag The Docker image tag to validate + * @throws Error if the image tag is invalid + */ + validateImageTag(imageTag: string): void { + if (!imageTag || imageTag.trim().length === 0) { + throw new Error("Docker image tag must not be empty"); + } + + if (imageTag.length > MAX_IMAGE_TAG_LENGTH) { + throw new Error( + `Docker image tag exceeds maximum length of ${MAX_IMAGE_TAG_LENGTH.toString()} characters` + ); + } + + if (!DOCKER_IMAGE_TAG_PATTERN.test(imageTag)) { + throw new Error( + `Docker image tag contains invalid characters: "${imageTag}". ` + + "Only alphanumeric characters, dots, hyphens, underscores, forward slashes, colons, and sha256 digests are allowed." + ); + } + } + /** * Create a Docker container for agent isolation * @param agentId Unique agent identifier @@ -160,6 +206,10 @@ export class DockerSandboxService { ): Promise { try { const image = options?.image ?? this.defaultImage; + + // Validate image tag format before any Docker operations + this.validateImageTag(image); + const memoryMB = options?.memoryMB ?? this.defaultMemoryMB; const cpuLimit = options?.cpuLimit ?? this.defaultCpuLimit; const networkMode = options?.networkMode ?? this.defaultNetworkMode;