diff --git a/apps/orchestrator/src/spawner/docker-sandbox.service.spec.ts b/apps/orchestrator/src/spawner/docker-sandbox.service.spec.ts index 7f50fac..3f4c2ae 100644 --- a/apps/orchestrator/src/spawner/docker-sandbox.service.spec.ts +++ b/apps/orchestrator/src/spawner/docker-sandbox.service.spec.ts @@ -1,7 +1,7 @@ import { ConfigService } from "@nestjs/config"; import { Logger } from "@nestjs/common"; import { describe, it, expect, beforeEach, vi, afterEach } from "vitest"; -import { DockerSandboxService } from "./docker-sandbox.service"; +import { DockerSandboxService, DEFAULT_ENV_WHITELIST } from "./docker-sandbox.service"; import Docker from "dockerode"; describe("DockerSandboxService", () => { @@ -127,14 +127,14 @@ describe("DockerSandboxService", () => { ); }); - it("should create a container with custom environment variables", async () => { + it("should create a container with whitelisted environment variables", async () => { const agentId = "agent-123"; const taskId = "task-456"; const workspacePath = "/workspace/agent-123"; const options = { env: { - CUSTOM_VAR: "value123", - ANOTHER_VAR: "value456", + NODE_ENV: "production", + LOG_LEVEL: "debug", }, }; @@ -145,8 +145,8 @@ describe("DockerSandboxService", () => { Env: expect.arrayContaining([ `AGENT_ID=${agentId}`, `TASK_ID=${taskId}`, - "CUSTOM_VAR=value123", - "ANOTHER_VAR=value456", + "NODE_ENV=production", + "LOG_LEVEL=debug", ]), }) ); @@ -373,4 +373,227 @@ describe("DockerSandboxService", () => { expect(warnSpy).not.toHaveBeenCalledWith(expect.stringContaining("SECURITY WARNING")); }); }); + + describe("environment variable whitelist", () => { + describe("getEnvWhitelist", () => { + it("should return default whitelist when no custom whitelist is configured", () => { + const whitelist = service.getEnvWhitelist(); + + expect(whitelist).toEqual(DEFAULT_ENV_WHITELIST); + expect(whitelist).toContain("AGENT_ID"); + expect(whitelist).toContain("TASK_ID"); + expect(whitelist).toContain("NODE_ENV"); + expect(whitelist).toContain("LOG_LEVEL"); + }); + + it("should return custom whitelist when configured", () => { + const customWhitelist = ["CUSTOM_VAR_1", "CUSTOM_VAR_2"]; + 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.envWhitelist": customWhitelist, + }; + return config[key] !== undefined ? config[key] : defaultValue; + }), + } as unknown as ConfigService; + + const customService = new DockerSandboxService(customConfigService, mockDocker); + const whitelist = customService.getEnvWhitelist(); + + expect(whitelist).toEqual(customWhitelist); + }); + }); + + describe("filterEnvVars", () => { + it("should allow whitelisted environment variables", () => { + const envVars = { + NODE_ENV: "production", + LOG_LEVEL: "debug", + TZ: "UTC", + }; + + const result = service.filterEnvVars(envVars); + + expect(result.allowed).toEqual({ + NODE_ENV: "production", + LOG_LEVEL: "debug", + TZ: "UTC", + }); + expect(result.filtered).toEqual([]); + }); + + it("should filter non-whitelisted environment variables", () => { + const envVars = { + NODE_ENV: "production", + DATABASE_URL: "postgres://secret@host/db", + API_KEY: "sk-secret-key", + AWS_SECRET_ACCESS_KEY: "super-secret", + }; + + const result = service.filterEnvVars(envVars); + + expect(result.allowed).toEqual({ + NODE_ENV: "production", + }); + expect(result.filtered).toContain("DATABASE_URL"); + expect(result.filtered).toContain("API_KEY"); + expect(result.filtered).toContain("AWS_SECRET_ACCESS_KEY"); + expect(result.filtered).toHaveLength(3); + }); + + it("should handle empty env vars object", () => { + const result = service.filterEnvVars({}); + + expect(result.allowed).toEqual({}); + expect(result.filtered).toEqual([]); + }); + + it("should handle all vars being filtered", () => { + const envVars = { + SECRET_KEY: "secret", + PASSWORD: "password123", + PRIVATE_TOKEN: "token", + }; + + const result = service.filterEnvVars(envVars); + + expect(result.allowed).toEqual({}); + expect(result.filtered).toEqual(["SECRET_KEY", "PASSWORD", "PRIVATE_TOKEN"]); + }); + }); + + describe("createContainer with filtering", () => { + let warnSpy: ReturnType; + + beforeEach(() => { + warnSpy = vi.spyOn(Logger.prototype, "warn").mockImplementation(() => undefined); + }); + + afterEach(() => { + warnSpy.mockRestore(); + }); + + it("should filter non-whitelisted vars and only pass allowed vars to container", async () => { + const agentId = "agent-123"; + const taskId = "task-456"; + const workspacePath = "/workspace/agent-123"; + const options = { + env: { + NODE_ENV: "production", + DATABASE_URL: "postgres://secret@host/db", + LOG_LEVEL: "info", + }, + }; + + await service.createContainer(agentId, taskId, workspacePath, options); + + // Should include whitelisted vars + expect(mockDocker.createContainer).toHaveBeenCalledWith( + expect.objectContaining({ + Env: expect.arrayContaining([ + `AGENT_ID=${agentId}`, + `TASK_ID=${taskId}`, + "NODE_ENV=production", + "LOG_LEVEL=info", + ]), + }) + ); + + // Should NOT include filtered vars + const callArgs = (mockDocker.createContainer as ReturnType).mock.calls[0][0]; + expect(callArgs.Env).not.toContain("DATABASE_URL=postgres://secret@host/db"); + }); + + it("should log warning when env vars are filtered", async () => { + const agentId = "agent-123"; + const taskId = "task-456"; + const workspacePath = "/workspace/agent-123"; + const options = { + env: { + DATABASE_URL: "postgres://secret@host/db", + API_KEY: "sk-secret", + }, + }; + + await service.createContainer(agentId, taskId, workspacePath, options); + + expect(warnSpy).toHaveBeenCalledWith( + expect.stringContaining("SECURITY: Filtered 2 non-whitelisted env var(s)") + ); + expect(warnSpy).toHaveBeenCalledWith(expect.stringContaining("DATABASE_URL")); + expect(warnSpy).toHaveBeenCalledWith(expect.stringContaining("API_KEY")); + }); + + it("should not log warning when all vars are whitelisted", async () => { + const agentId = "agent-123"; + const taskId = "task-456"; + const workspacePath = "/workspace/agent-123"; + const options = { + env: { + NODE_ENV: "production", + LOG_LEVEL: "debug", + }, + }; + + await service.createContainer(agentId, taskId, workspacePath, options); + + expect(warnSpy).not.toHaveBeenCalledWith(expect.stringContaining("SECURITY: Filtered")); + }); + + it("should not log warning when no env vars are provided", async () => { + const agentId = "agent-123"; + const taskId = "task-456"; + const workspacePath = "/workspace/agent-123"; + + await service.createContainer(agentId, taskId, workspacePath); + + expect(warnSpy).not.toHaveBeenCalledWith(expect.stringContaining("SECURITY: Filtered")); + }); + }); + }); + + describe("DEFAULT_ENV_WHITELIST", () => { + it("should contain essential agent identification vars", () => { + expect(DEFAULT_ENV_WHITELIST).toContain("AGENT_ID"); + expect(DEFAULT_ENV_WHITELIST).toContain("TASK_ID"); + }); + + it("should contain Node.js runtime vars", () => { + expect(DEFAULT_ENV_WHITELIST).toContain("NODE_ENV"); + expect(DEFAULT_ENV_WHITELIST).toContain("NODE_OPTIONS"); + }); + + it("should contain logging vars", () => { + expect(DEFAULT_ENV_WHITELIST).toContain("LOG_LEVEL"); + expect(DEFAULT_ENV_WHITELIST).toContain("DEBUG"); + }); + + it("should contain locale vars", () => { + expect(DEFAULT_ENV_WHITELIST).toContain("LANG"); + expect(DEFAULT_ENV_WHITELIST).toContain("LC_ALL"); + expect(DEFAULT_ENV_WHITELIST).toContain("TZ"); + }); + + it("should contain Mosaic-specific safe vars", () => { + expect(DEFAULT_ENV_WHITELIST).toContain("MOSAIC_WORKSPACE_ID"); + expect(DEFAULT_ENV_WHITELIST).toContain("MOSAIC_PROJECT_ID"); + expect(DEFAULT_ENV_WHITELIST).toContain("MOSAIC_AGENT_TYPE"); + }); + + it("should NOT contain sensitive var patterns", () => { + // Verify common sensitive vars are not in the whitelist + expect(DEFAULT_ENV_WHITELIST).not.toContain("DATABASE_URL"); + expect(DEFAULT_ENV_WHITELIST).not.toContain("API_KEY"); + expect(DEFAULT_ENV_WHITELIST).not.toContain("SECRET"); + expect(DEFAULT_ENV_WHITELIST).not.toContain("PASSWORD"); + expect(DEFAULT_ENV_WHITELIST).not.toContain("AWS_SECRET_ACCESS_KEY"); + expect(DEFAULT_ENV_WHITELIST).not.toContain("ANTHROPIC_API_KEY"); + }); + }); }); diff --git a/apps/orchestrator/src/spawner/docker-sandbox.service.ts b/apps/orchestrator/src/spawner/docker-sandbox.service.ts index 331a92e..305054d 100644 --- a/apps/orchestrator/src/spawner/docker-sandbox.service.ts +++ b/apps/orchestrator/src/spawner/docker-sandbox.service.ts @@ -3,6 +3,31 @@ import { ConfigService } from "@nestjs/config"; import Docker from "dockerode"; import { DockerSandboxOptions, ContainerCreateResult } from "./types/docker-sandbox.types"; +/** + * Default whitelist of allowed environment variable names/patterns for Docker containers. + * Only these variables will be passed to spawned agent containers. + * This prevents accidental leakage of secrets like API keys, database credentials, etc. + */ +export const DEFAULT_ENV_WHITELIST: readonly string[] = [ + // Agent identification + "AGENT_ID", + "TASK_ID", + // Node.js runtime + "NODE_ENV", + "NODE_OPTIONS", + // Logging + "LOG_LEVEL", + "DEBUG", + // Locale + "LANG", + "LC_ALL", + "TZ", + // Application-specific safe vars + "MOSAIC_WORKSPACE_ID", + "MOSAIC_PROJECT_ID", + "MOSAIC_AGENT_TYPE", +] as const; + /** * Service for managing Docker container isolation for agents * Provides secure sandboxing with resource limits and cleanup @@ -16,6 +41,7 @@ export class DockerSandboxService { private readonly defaultMemoryMB: number; private readonly defaultCpuLimit: number; private readonly defaultNetworkMode: string; + private readonly envWhitelist: readonly string[]; constructor( private readonly configService: ConfigService, @@ -50,6 +76,10 @@ export class DockerSandboxService { "bridge" ); + // Load custom whitelist from config, or use defaults + const customWhitelist = this.configService.get("orchestrator.sandbox.envWhitelist"); + this.envWhitelist = customWhitelist ?? DEFAULT_ENV_WHITELIST; + this.logger.log( `DockerSandboxService initialized (enabled: ${this.sandboxEnabled.toString()}, socket: ${socketPath})` ); @@ -87,13 +117,23 @@ export class DockerSandboxService { // Convert CPU limit to NanoCPUs (1.0 = 1,000,000,000 nanocpus) const nanoCpus = Math.floor(cpuLimit * 1000000000); - // Build environment variables + // Build environment variables with whitelist filtering const env = [`AGENT_ID=${agentId}`, `TASK_ID=${taskId}`]; if (options?.env) { - Object.entries(options.env).forEach(([key, value]) => { + const { allowed, filtered } = this.filterEnvVars(options.env); + + // Add allowed vars + Object.entries(allowed).forEach(([key, value]) => { env.push(`${key}=${value}`); }); + + // Log warning for filtered vars + if (filtered.length > 0) { + this.logger.warn( + `SECURITY: Filtered ${filtered.length.toString()} non-whitelisted env var(s) for agent ${agentId}: ${filtered.join(", ")}` + ); + } } // Container name with timestamp to ensure uniqueness @@ -246,4 +286,44 @@ export class DockerSandboxService { isEnabled(): boolean { return this.sandboxEnabled; } + + /** + * Get the current environment variable whitelist + * @returns The configured whitelist of allowed env var names + */ + getEnvWhitelist(): readonly string[] { + return this.envWhitelist; + } + + /** + * Filter environment variables against the whitelist + * @param envVars Object of environment variables to filter + * @returns Object with allowed vars and array of filtered var names + */ + filterEnvVars(envVars: Record): { + allowed: Record; + filtered: string[]; + } { + const allowed: Record = {}; + const filtered: string[] = []; + + for (const [key, value] of Object.entries(envVars)) { + if (this.isEnvVarAllowed(key)) { + allowed[key] = value; + } else { + filtered.push(key); + } + } + + return { allowed, filtered }; + } + + /** + * Check if an environment variable name is allowed by the whitelist + * @param varName Environment variable name to check + * @returns True if allowed + */ + private isEnvVarAllowed(varName: string): boolean { + return this.envWhitelist.includes(varName); + } }