From 3f16bbeca19c09f19629194fa30cdf3645aa7e18 Mon Sep 17 00:00:00 2001 From: Jason Woltje Date: Thu, 5 Feb 2026 18:21:43 -0600 Subject: [PATCH] fix(#338): Add Docker security hardening (CapDrop, ReadonlyRootfs, PidsLimit) - Drop all Linux capabilities by default (CapDrop: ALL) - Enable read-only root filesystem (agents write to mounted /workspace volume) - Limit process count to 100 to prevent fork bombs (PidsLimit) - Add no-new-privileges security option to prevent privilege escalation - Add DockerSecurityOptions type with configurable security settings - All options are configurable via config but secure by default - Add comprehensive tests for security hardening options (20+ new tests) Refs #338 Co-Authored-By: Claude Opus 4.5 --- .../spawner/docker-sandbox.service.spec.ts | 305 +++++++++++++++++- .../src/spawner/docker-sandbox.service.ts | 116 ++++++- .../src/spawner/types/docker-sandbox.types.ts | 87 +++++ 3 files changed, 496 insertions(+), 12 deletions(-) diff --git a/apps/orchestrator/src/spawner/docker-sandbox.service.spec.ts b/apps/orchestrator/src/spawner/docker-sandbox.service.spec.ts index 3f4c2ae..02e8573 100644 --- a/apps/orchestrator/src/spawner/docker-sandbox.service.spec.ts +++ b/apps/orchestrator/src/spawner/docker-sandbox.service.spec.ts @@ -1,7 +1,12 @@ import { ConfigService } from "@nestjs/config"; import { Logger } from "@nestjs/common"; import { describe, it, expect, beforeEach, vi, afterEach } from "vitest"; -import { DockerSandboxService, DEFAULT_ENV_WHITELIST } from "./docker-sandbox.service"; +import { + DockerSandboxService, + DEFAULT_ENV_WHITELIST, + DEFAULT_SECURITY_OPTIONS, +} from "./docker-sandbox.service"; +import { DockerSecurityOptions, LinuxCapability } from "./types/docker-sandbox.types"; import Docker from "dockerode"; describe("DockerSandboxService", () => { @@ -59,7 +64,7 @@ describe("DockerSandboxService", () => { }); describe("createContainer", () => { - it("should create a container with default configuration", async () => { + it("should create a container with default configuration and security hardening", async () => { const agentId = "agent-123"; const taskId = "task-456"; const workspacePath = "/workspace/agent-123"; @@ -80,7 +85,10 @@ describe("DockerSandboxService", () => { NetworkMode: "bridge", Binds: [`${workspacePath}:/workspace`], AutoRemove: false, - ReadonlyRootfs: false, + ReadonlyRootfs: true, // Security hardening: read-only root filesystem + PidsLimit: 100, // Security hardening: prevent fork bombs + SecurityOpt: ["no-new-privileges:true"], // Security hardening: prevent privilege escalation + CapDrop: ["ALL"], // Security hardening: drop all capabilities }, WorkingDir: "/workspace", Env: [`AGENT_ID=${agentId}`, `TASK_ID=${taskId}`], @@ -596,4 +604,295 @@ describe("DockerSandboxService", () => { expect(DEFAULT_ENV_WHITELIST).not.toContain("ANTHROPIC_API_KEY"); }); }); + + describe("security hardening options", () => { + describe("DEFAULT_SECURITY_OPTIONS", () => { + it("should drop all Linux capabilities by default", () => { + expect(DEFAULT_SECURITY_OPTIONS.capDrop).toEqual(["ALL"]); + }); + + it("should not add any capabilities back by default", () => { + expect(DEFAULT_SECURITY_OPTIONS.capAdd).toEqual([]); + }); + + it("should enable read-only root filesystem by default", () => { + expect(DEFAULT_SECURITY_OPTIONS.readonlyRootfs).toBe(true); + }); + + it("should limit PIDs to 100 by default", () => { + expect(DEFAULT_SECURITY_OPTIONS.pidsLimit).toBe(100); + }); + + it("should disable new privileges by default", () => { + expect(DEFAULT_SECURITY_OPTIONS.noNewPrivileges).toBe(true); + }); + }); + + describe("getSecurityOptions", () => { + it("should return default security options when none configured", () => { + const options = service.getSecurityOptions(); + + expect(options.capDrop).toEqual(["ALL"]); + expect(options.capAdd).toEqual([]); + expect(options.readonlyRootfs).toBe(true); + expect(options.pidsLimit).toBe(100); + expect(options.noNewPrivileges).toBe(true); + }); + + it("should return custom security options when configured", () => { + const customConfigService = { + get: vi.fn((key: string, defaultValue?: unknown) => { + const config: Record = { + "orchestrator.docker.socketPath": "/var/run/docker.sock", + "orchestrator.sandbox.enabled": true, + "orchestrator.sandbox.defaultImage": "node:20-alpine", + "orchestrator.sandbox.defaultMemoryMB": 512, + "orchestrator.sandbox.defaultCpuLimit": 1.0, + "orchestrator.sandbox.networkMode": "bridge", + "orchestrator.sandbox.security.capDrop": ["NET_RAW", "SYS_ADMIN"], + "orchestrator.sandbox.security.capAdd": ["CHOWN"], + "orchestrator.sandbox.security.readonlyRootfs": false, + "orchestrator.sandbox.security.pidsLimit": 200, + "orchestrator.sandbox.security.noNewPrivileges": false, + }; + return config[key] !== undefined ? config[key] : defaultValue; + }), + } as unknown as ConfigService; + + const customService = new DockerSandboxService(customConfigService, mockDocker); + const options = customService.getSecurityOptions(); + + expect(options.capDrop).toEqual(["NET_RAW", "SYS_ADMIN"]); + expect(options.capAdd).toEqual(["CHOWN"]); + expect(options.readonlyRootfs).toBe(false); + expect(options.pidsLimit).toBe(200); + expect(options.noNewPrivileges).toBe(false); + }); + }); + + describe("createContainer with security options", () => { + it("should apply CapDrop to container HostConfig", async () => { + const agentId = "agent-123"; + const taskId = "task-456"; + const workspacePath = "/workspace/agent-123"; + + await service.createContainer(agentId, taskId, workspacePath); + + const callArgs = (mockDocker.createContainer as ReturnType).mock + .calls[0][0] as Docker.ContainerCreateOptions; + expect(callArgs.HostConfig?.CapDrop).toEqual(["ALL"]); + }); + + it("should apply custom CapDrop when specified in options", async () => { + const agentId = "agent-123"; + const taskId = "task-456"; + const workspacePath = "/workspace/agent-123"; + const options = { + security: { + capDrop: ["NET_RAW", "SYS_ADMIN"] as LinuxCapability[], + }, + }; + + await service.createContainer(agentId, taskId, workspacePath, options); + + const callArgs = (mockDocker.createContainer as ReturnType).mock + .calls[0][0] as Docker.ContainerCreateOptions; + expect(callArgs.HostConfig?.CapDrop).toEqual(["NET_RAW", "SYS_ADMIN"]); + }); + + it("should apply CapAdd when specified in options", async () => { + const agentId = "agent-123"; + const taskId = "task-456"; + const workspacePath = "/workspace/agent-123"; + const options = { + security: { + capAdd: ["CHOWN", "SETUID"] as LinuxCapability[], + }, + }; + + await service.createContainer(agentId, taskId, workspacePath, options); + + const callArgs = (mockDocker.createContainer as ReturnType).mock + .calls[0][0] as Docker.ContainerCreateOptions; + expect(callArgs.HostConfig?.CapAdd).toEqual(["CHOWN", "SETUID"]); + }); + + it("should not include CapAdd when empty", async () => { + const agentId = "agent-123"; + const taskId = "task-456"; + const workspacePath = "/workspace/agent-123"; + + await service.createContainer(agentId, taskId, workspacePath); + + const callArgs = (mockDocker.createContainer as ReturnType).mock + .calls[0][0] as Docker.ContainerCreateOptions; + expect(callArgs.HostConfig?.CapAdd).toBeUndefined(); + }); + + it("should apply ReadonlyRootfs to container HostConfig", async () => { + const agentId = "agent-123"; + const taskId = "task-456"; + const workspacePath = "/workspace/agent-123"; + + await service.createContainer(agentId, taskId, workspacePath); + + const callArgs = (mockDocker.createContainer as ReturnType).mock + .calls[0][0] as Docker.ContainerCreateOptions; + expect(callArgs.HostConfig?.ReadonlyRootfs).toBe(true); + }); + + it("should disable ReadonlyRootfs when specified in options", async () => { + const agentId = "agent-123"; + const taskId = "task-456"; + const workspacePath = "/workspace/agent-123"; + const options = { + security: { + readonlyRootfs: false, + }, + }; + + await service.createContainer(agentId, taskId, workspacePath, options); + + const callArgs = (mockDocker.createContainer as ReturnType).mock + .calls[0][0] as Docker.ContainerCreateOptions; + expect(callArgs.HostConfig?.ReadonlyRootfs).toBe(false); + }); + + it("should apply PidsLimit to container HostConfig", async () => { + const agentId = "agent-123"; + const taskId = "task-456"; + const workspacePath = "/workspace/agent-123"; + + await service.createContainer(agentId, taskId, workspacePath); + + const callArgs = (mockDocker.createContainer as ReturnType).mock + .calls[0][0] as Docker.ContainerCreateOptions; + expect(callArgs.HostConfig?.PidsLimit).toBe(100); + }); + + it("should apply custom PidsLimit when specified in options", async () => { + const agentId = "agent-123"; + const taskId = "task-456"; + const workspacePath = "/workspace/agent-123"; + const options = { + security: { + pidsLimit: 50, + }, + }; + + await service.createContainer(agentId, taskId, workspacePath, options); + + const callArgs = (mockDocker.createContainer as ReturnType).mock + .calls[0][0] as Docker.ContainerCreateOptions; + expect(callArgs.HostConfig?.PidsLimit).toBe(50); + }); + + it("should not set PidsLimit when set to 0 (unlimited)", async () => { + const agentId = "agent-123"; + const taskId = "task-456"; + const workspacePath = "/workspace/agent-123"; + const options = { + security: { + pidsLimit: 0, + }, + }; + + await service.createContainer(agentId, taskId, workspacePath, options); + + const callArgs = (mockDocker.createContainer as ReturnType).mock + .calls[0][0] as Docker.ContainerCreateOptions; + expect(callArgs.HostConfig?.PidsLimit).toBeUndefined(); + }); + + it("should apply no-new-privileges security option", async () => { + const agentId = "agent-123"; + const taskId = "task-456"; + const workspacePath = "/workspace/agent-123"; + + await service.createContainer(agentId, taskId, workspacePath); + + const callArgs = (mockDocker.createContainer as ReturnType).mock + .calls[0][0] as Docker.ContainerCreateOptions; + expect(callArgs.HostConfig?.SecurityOpt).toContain("no-new-privileges:true"); + }); + + it("should not apply no-new-privileges when disabled in options", async () => { + const agentId = "agent-123"; + const taskId = "task-456"; + const workspacePath = "/workspace/agent-123"; + const options = { + security: { + noNewPrivileges: false, + }, + }; + + await service.createContainer(agentId, taskId, workspacePath, options); + + const callArgs = (mockDocker.createContainer as ReturnType).mock + .calls[0][0] as Docker.ContainerCreateOptions; + expect(callArgs.HostConfig?.SecurityOpt).toBeUndefined(); + }); + + it("should merge partial security options with defaults", async () => { + const agentId = "agent-123"; + const taskId = "task-456"; + const workspacePath = "/workspace/agent-123"; + const options = { + security: { + pidsLimit: 200, // Override just this one + } as DockerSecurityOptions, + }; + + await service.createContainer(agentId, taskId, workspacePath, options); + + const callArgs = (mockDocker.createContainer as ReturnType).mock + .calls[0][0] as Docker.ContainerCreateOptions; + // Overridden + expect(callArgs.HostConfig?.PidsLimit).toBe(200); + // Defaults still applied + expect(callArgs.HostConfig?.CapDrop).toEqual(["ALL"]); + expect(callArgs.HostConfig?.ReadonlyRootfs).toBe(true); + expect(callArgs.HostConfig?.SecurityOpt).toContain("no-new-privileges:true"); + }); + + it("should not include CapDrop when empty array specified", async () => { + const agentId = "agent-123"; + const taskId = "task-456"; + const workspacePath = "/workspace/agent-123"; + const options = { + security: { + capDrop: [] as LinuxCapability[], + }, + }; + + await service.createContainer(agentId, taskId, workspacePath, options); + + const callArgs = (mockDocker.createContainer as ReturnType).mock + .calls[0][0] as Docker.ContainerCreateOptions; + expect(callArgs.HostConfig?.CapDrop).toBeUndefined(); + }); + }); + + describe("security hardening logging", () => { + let logSpy: ReturnType; + + beforeEach(() => { + logSpy = vi.spyOn(Logger.prototype, "log").mockImplementation(() => undefined); + }); + + afterEach(() => { + logSpy.mockRestore(); + }); + + it("should log security hardening configuration on initialization", () => { + new DockerSandboxService(mockConfigService, mockDocker); + + expect(logSpy).toHaveBeenCalledWith(expect.stringContaining("Security hardening:")); + expect(logSpy).toHaveBeenCalledWith(expect.stringContaining("capDrop=ALL")); + expect(logSpy).toHaveBeenCalledWith(expect.stringContaining("readonlyRootfs=true")); + expect(logSpy).toHaveBeenCalledWith(expect.stringContaining("pidsLimit=100")); + expect(logSpy).toHaveBeenCalledWith(expect.stringContaining("noNewPrivileges=true")); + }); + }); + }); }); diff --git a/apps/orchestrator/src/spawner/docker-sandbox.service.ts b/apps/orchestrator/src/spawner/docker-sandbox.service.ts index 305054d..705f2c6 100644 --- a/apps/orchestrator/src/spawner/docker-sandbox.service.ts +++ b/apps/orchestrator/src/spawner/docker-sandbox.service.ts @@ -1,7 +1,12 @@ import { Injectable, Logger } from "@nestjs/common"; import { ConfigService } from "@nestjs/config"; import Docker from "dockerode"; -import { DockerSandboxOptions, ContainerCreateResult } from "./types/docker-sandbox.types"; +import { + DockerSandboxOptions, + ContainerCreateResult, + DockerSecurityOptions, + LinuxCapability, +} from "./types/docker-sandbox.types"; /** * Default whitelist of allowed environment variable names/patterns for Docker containers. @@ -28,6 +33,22 @@ export const DEFAULT_ENV_WHITELIST: readonly string[] = [ "MOSAIC_AGENT_TYPE", ] as const; +/** + * Default security hardening options for Docker containers. + * These settings follow security best practices: + * - Drop all Linux capabilities (principle of least privilege) + * - Read-only root filesystem (agents write to mounted /workspace volume) + * - PID limit to prevent fork bombs + * - No new privileges to prevent privilege escalation + */ +export const DEFAULT_SECURITY_OPTIONS: Required = { + capDrop: ["ALL"], + capAdd: [], + readonlyRootfs: true, + pidsLimit: 100, + noNewPrivileges: true, +} as const; + /** * Service for managing Docker container isolation for agents * Provides secure sandboxing with resource limits and cleanup @@ -42,6 +63,7 @@ export class DockerSandboxService { private readonly defaultCpuLimit: number; private readonly defaultNetworkMode: string; private readonly envWhitelist: readonly string[]; + private readonly defaultSecurityOptions: Required; constructor( private readonly configService: ConfigService, @@ -80,9 +102,40 @@ export class DockerSandboxService { const customWhitelist = this.configService.get("orchestrator.sandbox.envWhitelist"); this.envWhitelist = customWhitelist ?? DEFAULT_ENV_WHITELIST; + // Load security options from config, merging with secure defaults + const configCapDrop = this.configService.get( + "orchestrator.sandbox.security.capDrop" + ); + const configCapAdd = this.configService.get( + "orchestrator.sandbox.security.capAdd" + ); + const configReadonlyRootfs = this.configService.get( + "orchestrator.sandbox.security.readonlyRootfs" + ); + const configPidsLimit = this.configService.get( + "orchestrator.sandbox.security.pidsLimit" + ); + const configNoNewPrivileges = this.configService.get( + "orchestrator.sandbox.security.noNewPrivileges" + ); + + this.defaultSecurityOptions = { + capDrop: configCapDrop ?? DEFAULT_SECURITY_OPTIONS.capDrop, + capAdd: configCapAdd ?? DEFAULT_SECURITY_OPTIONS.capAdd, + readonlyRootfs: configReadonlyRootfs ?? DEFAULT_SECURITY_OPTIONS.readonlyRootfs, + pidsLimit: configPidsLimit ?? DEFAULT_SECURITY_OPTIONS.pidsLimit, + noNewPrivileges: configNoNewPrivileges ?? DEFAULT_SECURITY_OPTIONS.noNewPrivileges, + }; + this.logger.log( `DockerSandboxService initialized (enabled: ${this.sandboxEnabled.toString()}, socket: ${socketPath})` ); + this.logger.log( + `Security hardening: capDrop=${this.defaultSecurityOptions.capDrop.join(",") || "none"}, ` + + `readonlyRootfs=${this.defaultSecurityOptions.readonlyRootfs.toString()}, ` + + `pidsLimit=${this.defaultSecurityOptions.pidsLimit.toString()}, ` + + `noNewPrivileges=${this.defaultSecurityOptions.noNewPrivileges.toString()}` + ); if (!this.sandboxEnabled) { this.logger.warn( @@ -111,6 +164,9 @@ export class DockerSandboxService { const cpuLimit = options?.cpuLimit ?? this.defaultCpuLimit; const networkMode = options?.networkMode ?? this.defaultNetworkMode; + // Merge security options with defaults + const security = this.mergeSecurityOptions(options?.security); + // Convert memory from MB to bytes const memoryBytes = memoryMB * 1024 * 1024; @@ -143,18 +199,33 @@ export class DockerSandboxService { `Creating container for agent ${agentId} (image: ${image}, memory: ${memoryMB.toString()}MB, cpu: ${cpuLimit.toString()})` ); + // Build HostConfig with security hardening + const hostConfig: Docker.HostConfig = { + Memory: memoryBytes, + NanoCpus: nanoCpus, + NetworkMode: networkMode, + Binds: [`${workspacePath}:/workspace`], + AutoRemove: false, // Manual cleanup for audit trail + ReadonlyRootfs: security.readonlyRootfs, + PidsLimit: security.pidsLimit > 0 ? security.pidsLimit : undefined, + SecurityOpt: security.noNewPrivileges ? ["no-new-privileges:true"] : undefined, + }; + + // Add capability dropping if configured + if (security.capDrop.length > 0) { + hostConfig.CapDrop = security.capDrop; + } + + // Add capabilities back if configured (useful when dropping ALL first) + if (security.capAdd.length > 0) { + hostConfig.CapAdd = security.capAdd; + } + const container = await this.docker.createContainer({ Image: image, name: containerName, User: "node:node", // Non-root user for security - HostConfig: { - Memory: memoryBytes, - NanoCpus: nanoCpus, - NetworkMode: networkMode, - Binds: [`${workspacePath}:/workspace`], - AutoRemove: false, // Manual cleanup for audit trail - ReadonlyRootfs: false, // Allow writes within container - }, + HostConfig: hostConfig, WorkingDir: "/workspace", Env: env, }); @@ -326,4 +397,31 @@ export class DockerSandboxService { private isEnvVarAllowed(varName: string): boolean { return this.envWhitelist.includes(varName); } + + /** + * Get the current default security options + * @returns The configured security options + */ + getSecurityOptions(): Required { + return { ...this.defaultSecurityOptions }; + } + + /** + * Merge provided security options with defaults + * @param options Optional security options to merge + * @returns Complete security options with all fields + */ + private mergeSecurityOptions(options?: DockerSecurityOptions): Required { + if (!options) { + return { ...this.defaultSecurityOptions }; + } + + return { + capDrop: options.capDrop ?? this.defaultSecurityOptions.capDrop, + capAdd: options.capAdd ?? this.defaultSecurityOptions.capAdd, + readonlyRootfs: options.readonlyRootfs ?? this.defaultSecurityOptions.readonlyRootfs, + pidsLimit: options.pidsLimit ?? this.defaultSecurityOptions.pidsLimit, + noNewPrivileges: options.noNewPrivileges ?? this.defaultSecurityOptions.noNewPrivileges, + }; + } } diff --git a/apps/orchestrator/src/spawner/types/docker-sandbox.types.ts b/apps/orchestrator/src/spawner/types/docker-sandbox.types.ts index 04fcfff..40b162f 100644 --- a/apps/orchestrator/src/spawner/types/docker-sandbox.types.ts +++ b/apps/orchestrator/src/spawner/types/docker-sandbox.types.ts @@ -3,6 +3,91 @@ */ export type NetworkMode = "bridge" | "host" | "none"; +/** + * Linux capabilities that can be dropped from containers. + * See https://man7.org/linux/man-pages/man7/capabilities.7.html + */ +export type LinuxCapability = + | "ALL" + | "AUDIT_CONTROL" + | "AUDIT_READ" + | "AUDIT_WRITE" + | "BLOCK_SUSPEND" + | "CHOWN" + | "DAC_OVERRIDE" + | "DAC_READ_SEARCH" + | "FOWNER" + | "FSETID" + | "IPC_LOCK" + | "IPC_OWNER" + | "KILL" + | "LEASE" + | "LINUX_IMMUTABLE" + | "MAC_ADMIN" + | "MAC_OVERRIDE" + | "MKNOD" + | "NET_ADMIN" + | "NET_BIND_SERVICE" + | "NET_BROADCAST" + | "NET_RAW" + | "SETFCAP" + | "SETGID" + | "SETPCAP" + | "SETUID" + | "SYS_ADMIN" + | "SYS_BOOT" + | "SYS_CHROOT" + | "SYS_MODULE" + | "SYS_NICE" + | "SYS_PACCT" + | "SYS_PTRACE" + | "SYS_RAWIO" + | "SYS_RESOURCE" + | "SYS_TIME" + | "SYS_TTY_CONFIG" + | "SYSLOG" + | "WAKE_ALARM"; + +/** + * Security hardening options for Docker containers + */ +export interface DockerSecurityOptions { + /** + * Linux capabilities to drop from the container. + * Default: ["ALL"] - drops all capabilities for maximum security. + * Set to empty array to keep default Docker capabilities. + */ + capDrop?: LinuxCapability[]; + + /** + * Linux capabilities to add back after dropping. + * Only effective when capDrop includes "ALL". + * Default: [] - no capabilities added back. + */ + capAdd?: LinuxCapability[]; + + /** + * Make the root filesystem read-only. + * Containers can still write to mounted volumes. + * Default: true for security (agents write to /workspace mount). + */ + readonlyRootfs?: boolean; + + /** + * Maximum number of processes (PIDs) allowed in the container. + * Prevents fork bomb attacks. + * Default: 100 - sufficient for most agent workloads. + * Set to 0 or -1 for unlimited (not recommended). + */ + pidsLimit?: number; + + /** + * Disable privilege escalation via setuid/setgid. + * Default: true - prevents privilege escalation. + */ + noNewPrivileges?: boolean; +} + /** * Options for creating a Docker sandbox container */ @@ -17,6 +102,8 @@ export interface DockerSandboxOptions { image?: string; /** Additional environment variables */ env?: Record; + /** Security hardening options */ + security?: DockerSecurityOptions; } /**