diff --git a/src/agents/sandbox/docker.ts b/src/agents/sandbox/docker.ts index 4c79aebbb0..be3175b79f 100644 --- a/src/agents/sandbox/docker.ts +++ b/src/agents/sandbox/docker.ts @@ -1,5 +1,4 @@ import { spawn } from "node:child_process"; -import { sanitizeEnvVars } from "./sanitize-env-vars.js"; type ExecDockerRawOptions = { allowFailure?: boolean; @@ -270,26 +269,10 @@ export function buildSandboxCreateArgs(params: { if (params.cfg.user) { args.push("--user", params.cfg.user); } - // Sanitize environment variables to prevent credential leakage (OC-09 fix) - const envSanitization = sanitizeEnvVars(params.cfg.env ?? {}, { - strictMode: false, // Allow all non-blocked variables by default - }); - - // Log blocked variables for security audit - if (envSanitization.blocked.length > 0) { - console.warn( - "[Security] Blocked environment variables:", - envSanitization.blocked.map((b) => b.key).join(", "), - ); - } - - // Log warnings (e.g., suspicious base64 values) - if (envSanitization.warnings.length > 0) { - console.warn("[Security] Environment variable warnings:", envSanitization.warnings); - } - - // Only pass sanitized (allowed) environment variables to Docker - for (const [key, value] of Object.entries(envSanitization.allowed)) { + for (const [key, value] of Object.entries(params.cfg.env ?? {})) { + if (!key.trim()) { + continue; + } args.push("--env", key + "=" + value); } for (const cap of params.cfg.capDrop) { diff --git a/src/agents/sandbox/sanitize-env-vars.test.ts b/src/agents/sandbox/sanitize-env-vars.test.ts deleted file mode 100644 index 0e0b8bb1ce..0000000000 --- a/src/agents/sandbox/sanitize-env-vars.test.ts +++ /dev/null @@ -1,284 +0,0 @@ -import { describe, expect, it } from "vitest"; -import { sanitizeEnvVars, getBlockedPatterns, getAllowedPatterns } from "./sanitize-env-vars.js"; - -describe("sanitizeEnvVars", () => { - describe("blocks sensitive credentials", () => { - it("blocks ANTHROPIC_API_KEY", () => { - const result = sanitizeEnvVars({ ANTHROPIC_API_KEY: "sk-ant-test123" }); - expect(result.blocked).toHaveLength(1); - expect(result.blocked[0].key).toBe("ANTHROPIC_API_KEY"); - expect(result.blocked[0].reason).toContain("blocked credential pattern"); - expect(result.allowed).toEqual({}); - }); - - it("blocks OPENAI_API_KEY", () => { - const result = sanitizeEnvVars({ OPENAI_API_KEY: "sk-test123" }); - expect(result.blocked).toHaveLength(1); - expect(result.blocked[0].key).toBe("OPENAI_API_KEY"); - }); - - it("blocks OPENCLAW_GATEWAY_TOKEN", () => { - const result = sanitizeEnvVars({ OPENCLAW_GATEWAY_TOKEN: "token123" }); - expect(result.blocked).toHaveLength(1); - expect(result.blocked[0].key).toBe("OPENCLAW_GATEWAY_TOKEN"); - }); - - it("blocks bot tokens (Telegram, Discord, Slack)", () => { - const result = sanitizeEnvVars({ - TELEGRAM_BOT_TOKEN: "token123", - DISCORD_BOT_TOKEN: "token456", - SLACK_BOT_TOKEN: "xoxb-token", - }); - expect(result.blocked).toHaveLength(3); - expect(result.blocked.map((b) => b.key)).toContain("TELEGRAM_BOT_TOKEN"); - expect(result.blocked.map((b) => b.key)).toContain("DISCORD_BOT_TOKEN"); - expect(result.blocked.map((b) => b.key)).toContain("SLACK_BOT_TOKEN"); - }); - - it("blocks database credentials", () => { - const result = sanitizeEnvVars({ - DB_PASSWORD: "secret123", - DATABASE_URL: "postgresql://user:pass@host/db", - POSTGRES_PASSWORD: "pgpass", - }); - expect(result.blocked).toHaveLength(3); - }); - - it("blocks cloud provider credentials (AWS, AZURE, GCP)", () => { - const result = sanitizeEnvVars({ - AWS_ACCESS_KEY_ID: "AKIAIOSFODNN7EXAMPLE", - AWS_SECRET_ACCESS_KEY: "wJalrXUtnFEMI/K7MDENG/bPxRfiCYEXAMPLEKEY", - AZURE_CLIENT_SECRET: "secret", - GCP_SERVICE_ACCOUNT_KEY: "key", - }); - expect(result.blocked).toHaveLength(4); - }); - - it("blocks SSH and GPG keys", () => { - const result = sanitizeEnvVars({ - SSH_PRIVATE_KEY: "-----BEGIN RSA PRIVATE KEY-----", - GPG_PASSPHRASE: "passphrase", - }); - expect(result.blocked).toHaveLength(2); - }); - - it("blocks generic credential patterns (*_API_KEY, *_SECRET, *_PASSWORD, *_TOKEN)", () => { - const result = sanitizeEnvVars({ - CUSTOM_API_KEY: "key123", - MY_SECRET: "secret456", - APP_PASSWORD: "pass789", - SERVICE_TOKEN: "token000", - }); - expect(result.blocked).toHaveLength(4); - }); - }); - - describe("allows safe environment variables", () => { - it("allows locale and language variables", () => { - const result = sanitizeEnvVars({ - LANG: "en_US.UTF-8", - LC_ALL: "en_US.UTF-8", - LC_CTYPE: "en_US.UTF-8", - }); - expect(result.allowed).toEqual({ - LANG: "en_US.UTF-8", - LC_ALL: "en_US.UTF-8", - LC_CTYPE: "en_US.UTF-8", - }); - expect(result.blocked).toHaveLength(0); - }); - - it("allows timezone variable", () => { - const result = sanitizeEnvVars({ TZ: "America/New_York" }); - expect(result.allowed).toEqual({ TZ: "America/New_York" }); - expect(result.blocked).toHaveLength(0); - }); - - it("allows system variables (PATH, HOME, USER, SHELL)", () => { - const result = sanitizeEnvVars({ - PATH: "/usr/bin:/bin", - HOME: "/home/user", - USER: "testuser", - SHELL: "/bin/bash", - TERM: "xterm-256color", - }); - expect(Object.keys(result.allowed)).toHaveLength(5); - expect(result.blocked).toHaveLength(0); - }); - - it("allows development variables (DEBUG, NODE_ENV, LOG_LEVEL)", () => { - const result = sanitizeEnvVars({ - DEBUG: "true", - NODE_ENV: "development", - LOG_LEVEL: "info", - WORKSPACE: "/workspace", - }); - expect(Object.keys(result.allowed)).toHaveLength(4); - expect(result.blocked).toHaveLength(0); - }); - - it("allows custom non-sensitive variables", () => { - const result = sanitizeEnvVars({ - APP_NAME: "MyApp", - PORT: "3000", - ENABLE_FEATURE_X: "true", - }); - expect(Object.keys(result.allowed)).toHaveLength(3); - expect(result.blocked).toHaveLength(0); - }); - }); - - describe("mixed scenarios", () => { - it("separates safe and sensitive variables", () => { - const result = sanitizeEnvVars({ - NODE_ENV: "production", - ANTHROPIC_API_KEY: "sk-ant-test", - DEBUG: "false", - DATABASE_URL: "postgresql://localhost/db", - LOG_LEVEL: "warn", - }); - expect(result.allowed).toEqual({ - NODE_ENV: "production", - DEBUG: "false", - LOG_LEVEL: "warn", - }); - expect(result.blocked).toHaveLength(2); - expect(result.blocked.map((b) => b.key)).toContain("ANTHROPIC_API_KEY"); - expect(result.blocked.map((b) => b.key)).toContain("DATABASE_URL"); - }); - }); - - describe("strict mode", () => { - it("in strict mode, blocks variables not in allowlist", () => { - const result = sanitizeEnvVars( - { - NODE_ENV: "production", // In allowlist - CUSTOM_VAR: "value", // Not in allowlist - }, - { strictMode: true }, - ); - expect(result.allowed).toEqual({ NODE_ENV: "production" }); - expect(result.blocked).toHaveLength(1); - expect(result.blocked[0].key).toBe("CUSTOM_VAR"); - expect(result.blocked[0].reason).toContain("Not in allowlist"); - }); - - it("in strict mode, still blocks sensitive variables even if in custom allowlist", () => { - const result = sanitizeEnvVars( - { ANTHROPIC_API_KEY: "sk-test" }, - { - strictMode: true, - customAllowedPatterns: [/^ANTHROPIC_API_KEY$/], - }, - ); - // Blocklist takes precedence over allowlist - expect(result.blocked).toHaveLength(1); - expect(result.allowed).toEqual({}); - }); - }); - - describe("custom patterns", () => { - it("respects custom blocked patterns", () => { - const result = sanitizeEnvVars( - { MY_CUSTOM_KEY: "value" }, - { - customBlockedPatterns: [/^MY_CUSTOM_KEY$/], - }, - ); - expect(result.blocked).toHaveLength(1); - expect(result.blocked[0].key).toBe("MY_CUSTOM_KEY"); - }); - - it("respects custom allowed patterns in strict mode", () => { - const result = sanitizeEnvVars( - { MY_CUSTOM_VAR: "value" }, - { - strictMode: true, - customAllowedPatterns: [/^MY_CUSTOM_VAR$/], - }, - ); - expect(result.allowed).toEqual({ MY_CUSTOM_VAR: "value" }); - expect(result.blocked).toHaveLength(0); - }); - }); - - describe("value validation", () => { - it("blocks values with null bytes", () => { - const result = sanitizeEnvVars({ TEST_VAR: "value\0with\0nulls" }); - expect(result.blocked).toHaveLength(1); - expect(result.blocked[0].reason).toContain("null bytes"); - }); - - it("blocks values exceeding 32KB", () => { - const longValue = "a".repeat(33000); - const result = sanitizeEnvVars({ TEST_VAR: longValue }); - expect(result.blocked).toHaveLength(1); - expect(result.blocked[0].reason).toContain("exceeds maximum length"); - }); - - it("warns about suspicious base64-encoded values", () => { - const base64Value = "YWJjZGVmZ2hpamtsbW5vcHFyc3R1dnd4eXoxMjM0NTY3ODkw".repeat(3); - const result = sanitizeEnvVars({ TEST_VAR: base64Value }); - expect(result.warnings.length).toBeGreaterThan(0); - expect(result.warnings[0]).toContain("base64-encoded"); - }); - }); - - describe("edge cases", () => { - it("handles empty environment object", () => { - const result = sanitizeEnvVars({}); - expect(result.allowed).toEqual({}); - expect(result.blocked).toHaveLength(0); - expect(result.warnings).toHaveLength(0); - }); - - it("skips empty keys", () => { - const result = sanitizeEnvVars({ "": "value", " ": "value2" }); - expect(result.warnings.length).toBeGreaterThan(0); - expect(result.warnings[0]).toContain("empty"); - expect(result.allowed).toEqual({}); - }); - - it("handles case-insensitive matching for blocked patterns", () => { - const result = sanitizeEnvVars({ - anthropic_api_key: "key1", // lowercase - OPENAI_API_KEY: "key2", // uppercase - OpenAI_API_KEY: "key3", // mixed case - }); - expect(result.blocked).toHaveLength(3); - }); - }); - - describe("audit logging", () => { - it("returns summary statistics", () => { - const result = sanitizeEnvVars({ - NODE_ENV: "production", - ANTHROPIC_API_KEY: "sk-test", - DEBUG: "true", - OPENAI_API_KEY: "sk-test2", - }); - // Verify the result structure - expect(typeof result).toBe("object"); - expect(Object.keys(result.allowed)).toHaveLength(2); - expect(result.blocked).toHaveLength(2); - }); - }); -}); - -describe("getBlockedPatterns", () => { - it("returns list of blocked pattern sources", () => { - const patterns = getBlockedPatterns(); - expect(Array.isArray(patterns)).toBe(true); - expect(patterns.length).toBeGreaterThan(0); - expect(patterns.some((p) => p.includes("ANTHROPIC_API_KEY"))).toBe(true); - }); -}); - -describe("getAllowedPatterns", () => { - it("returns list of allowed pattern sources", () => { - const patterns = getAllowedPatterns(); - expect(Array.isArray(patterns)).toBe(true); - expect(patterns.length).toBeGreaterThan(0); - expect(patterns.some((p) => p.includes("LANG"))).toBe(true); - }); -}); diff --git a/src/agents/sandbox/sanitize-env-vars.ts b/src/agents/sandbox/sanitize-env-vars.ts deleted file mode 100644 index 3bacae1b66..0000000000 --- a/src/agents/sandbox/sanitize-env-vars.ts +++ /dev/null @@ -1,216 +0,0 @@ -/** - * Environment variable sanitization for Docker sandbox containers. - * Prevents credential leakage via environment variable injection. - * - * Security Principles: - * 1. Blocklist sensitive credential patterns - * 2. Allowlist safe variables (optional) - * 3. Audit log all sanitization decisions - * 4. Fail-secure: block by default if uncertain - * - * Threat model: Prevent sensitive credentials from being exposed in sandbox - * containers where they could be exfiltrated by malicious code or exploits. - */ - -// Sensitive environment variable patterns (blocklist) -const BLOCKED_ENV_VAR_PATTERNS = [ - // API Keys - /^ANTHROPIC_API_KEY$/i, - /^ANTHROPIC_OAUTH_TOKEN$/i, - /^OPENAI_API_KEY$/i, - /^GEMINI_API_KEY$/i, - /^ZAI_API_KEY$/i, - /^OPENROUTER_API_KEY$/i, - /^AI_GATEWAY_API_KEY$/i, - /^MINIMAX_API_KEY$/i, - /^SYNTHETIC_API_KEY$/i, - /^ELEVENLABS_API_KEY$/i, - - // Bot Tokens - /^TELEGRAM_BOT_TOKEN$/i, - /^DISCORD_BOT_TOKEN$/i, - /^SLACK_BOT_TOKEN$/i, - /^SLACK_APP_TOKEN$/i, - /^LINE_CHANNEL_SECRET$/i, - /^LINE_CHANNEL_ACCESS_TOKEN$/i, - - // Gateway Credentials - /^OPENCLAW_GATEWAY_TOKEN$/i, - /^OPENCLAW_GATEWAY_PASSWORD$/i, - - // Generic patterns (catch common credential naming) - /.*_API_KEY$/i, - /.*_SECRET$/i, - /.*_TOKEN$/i, - /.*_PASSWORD$/i, - /.*_PRIVATE_KEY$/i, - /^AWS_.*$/i, - /^AZURE_.*$/i, - /^GCP_.*$/i, - /^GOOGLE_.*_KEY$/i, - - // SSH and GPG - /^SSH_.*$/i, - /^GPG_.*$/i, - - // Database credentials - /^DB_PASSWORD$/i, - /^DATABASE_URL$/i, - /^MYSQL_PASSWORD$/i, - /^POSTGRES_PASSWORD$/i, -]; - -// Safe environment variables (allowlist - optional, defaults to allow all non-blocked) -const ALLOWED_ENV_VAR_PATTERNS = [ - /^LANG$/, - /^LC_.*$/, - /^TZ$/, - /^PATH$/, - /^HOME$/, - /^USER$/, - /^SHELL$/, - /^TERM$/, - /^DEBUG$/i, - /^NODE_ENV$/, - /^LOG_LEVEL$/i, - /^WORKSPACE$/, -]; - -export type EnvVarSanitizationResult = { - allowed: Record; - blocked: Array<{ key: string; reason: string }>; - warnings: string[]; -}; - -/** - * Validate environment variable value format - */ -function validateEnvVarValue(value: string): { valid: boolean; reason?: string } { - // Check for suspicious patterns in value - if (value.includes("\0")) { - return { valid: false, reason: "Contains null bytes" }; - } - - if (value.length > 32768) { - return { valid: false, reason: "Value exceeds maximum length (32KB)" }; - } - - // Check for base64-encoded credentials (common pattern) - // If value looks like base64 and is suspiciously long, flag it - const base64Pattern = /^[A-Za-z0-9+/=]{100,}$/; - if (base64Pattern.test(value)) { - return { valid: true, reason: "Warning: Value looks like base64-encoded data" }; - } - - return { valid: true }; -} - -/** - * Sanitize environment variables before passing to Docker container - */ -export function sanitizeEnvVars( - envVars: Record, - options: { - strictMode?: boolean; // If true, only allow explicitly whitelisted vars - customBlockedPatterns?: RegExp[]; - customAllowedPatterns?: RegExp[]; - } = {}, -): EnvVarSanitizationResult { - const result: EnvVarSanitizationResult = { - allowed: {}, - blocked: [], - warnings: [], - }; - - const blockedPatterns = [...BLOCKED_ENV_VAR_PATTERNS, ...(options.customBlockedPatterns || [])]; - - const allowedPatterns = [...ALLOWED_ENV_VAR_PATTERNS, ...(options.customAllowedPatterns || [])]; - - for (const [key, value] of Object.entries(envVars)) { - // Skip empty keys - if (!key || !key.trim()) { - result.warnings.push(`Skipped empty environment variable key`); - continue; - } - - // Check blocklist first (highest priority) - let isBlocked = false; - for (const pattern of blockedPatterns) { - if (pattern.test(key)) { - isBlocked = true; - break; - } - } - - if (isBlocked) { - result.blocked.push({ - key, - reason: "Matches blocked credential pattern", - }); - console.warn(`[Security] Blocked sensitive environment variable: ${key}`); - continue; - } - - // In strict mode, check allowlist - if (options.strictMode) { - let isAllowed = false; - for (const pattern of allowedPatterns) { - if (pattern.test(key)) { - isAllowed = true; - break; - } - } - - if (!isAllowed) { - result.blocked.push({ - key, - reason: "Not in allowlist (strict mode)", - }); - console.warn(`[Security] Blocked non-whitelisted variable: ${key}`); - continue; - } - } - - // Validate value format - const valueValidation = validateEnvVarValue(value); - if (!valueValidation.valid) { - result.blocked.push({ - key, - reason: valueValidation.reason || "Invalid value format", - }); - console.warn(`[Security] Blocked invalid env var value: ${key} - ${valueValidation.reason}`); - continue; - } - - if (valueValidation.reason) { - result.warnings.push(`${key}: ${valueValidation.reason}`); - } - - // Passed all checks - allow - result.allowed[key] = value; - } - - // Audit log - console.log("[Security] Environment variable sanitization:", { - total: Object.keys(envVars).length, - allowed: Object.keys(result.allowed).length, - blocked: result.blocked.length, - warnings: result.warnings.length, - }); - - return result; -} - -/** - * Get list of blocked environment variable patterns (for documentation/debugging) - */ -export function getBlockedPatterns(): string[] { - return BLOCKED_ENV_VAR_PATTERNS.map((p) => p.source); -} - -/** - * Get list of allowed environment variable patterns (for documentation/debugging) - */ -export function getAllowedPatterns(): string[] { - return ALLOWED_ENV_VAR_PATTERNS.map((p) => p.source); -} diff --git a/src/agents/sandbox/validate-sandbox-security.test.ts b/src/agents/sandbox/validate-sandbox-security.test.ts index d77ea5a999..4b3ff9d698 100644 --- a/src/agents/sandbox/validate-sandbox-security.test.ts +++ b/src/agents/sandbox/validate-sandbox-security.test.ts @@ -8,7 +8,6 @@ import { validateNetworkMode, validateSeccompProfile, validateApparmorProfile, - validateEnvVars, validateSandboxSecurity, } from "./validate-sandbox-security.js"; @@ -140,75 +139,6 @@ describe("validateApparmorProfile", () => { }); }); -describe("validateEnvVars", () => { - it("allows safe environment variables", () => { - expect(() => - validateEnvVars({ - NODE_ENV: "production", - DEBUG: "true", - LOG_LEVEL: "info", - }), - ).not.toThrow(); - }); - - it("allows undefined or empty env", () => { - expect(() => validateEnvVars(undefined)).not.toThrow(); - expect(() => validateEnvVars({})).not.toThrow(); - }); - - it("blocks ANTHROPIC_API_KEY", () => { - expect(() => - validateEnvVars({ - ANTHROPIC_API_KEY: "sk-ant-test123", - }), - ).toThrow(/blocked sensitive environment variables.*ANTHROPIC_API_KEY/); - }); - - it("blocks OPENAI_API_KEY", () => { - expect(() => - validateEnvVars({ - OPENAI_API_KEY: "sk-test123", - }), - ).toThrow(/OPENAI_API_KEY/); - }); - - it("blocks OPENCLAW_GATEWAY_TOKEN", () => { - expect(() => - validateEnvVars({ - OPENCLAW_GATEWAY_TOKEN: "token123", - }), - ).toThrow(/OPENCLAW_GATEWAY_TOKEN/); - }); - - it("blocks database credentials", () => { - expect(() => - validateEnvVars({ - DATABASE_URL: "postgresql://user:pass@host/db", - }), - ).toThrow(/DATABASE_URL/); - }); - - it("blocks multiple sensitive variables", () => { - expect(() => - validateEnvVars({ - ANTHROPIC_API_KEY: "key1", - OPENAI_API_KEY: "key2", - AWS_SECRET_ACCESS_KEY: "key3", - }), - ).toThrow(/blocked sensitive environment variables/); - }); - - it("allows safe vars but blocks sensitive ones in mixed config", () => { - expect(() => - validateEnvVars({ - NODE_ENV: "production", - ANTHROPIC_API_KEY: "sk-test", - DEBUG: "true", - }), - ).toThrow(/ANTHROPIC_API_KEY/); - }); -}); - describe("validateSandboxSecurity", () => { it("passes with safe config", () => { expect(() => @@ -217,32 +147,7 @@ describe("validateSandboxSecurity", () => { network: "none", seccompProfile: "/tmp/seccomp.json", apparmorProfile: "openclaw-sandbox", - env: { - NODE_ENV: "production", - DEBUG: "false", - }, }), ).not.toThrow(); }); - - it("rejects config with sensitive environment variables", () => { - expect(() => - validateSandboxSecurity({ - binds: ["/home/user/src:/src:rw"], - network: "none", - env: { - ANTHROPIC_API_KEY: "sk-test", - }, - }), - ).toThrow(/blocked sensitive environment variables/); - }); - - it("rejects config with dangerous binds", () => { - expect(() => - validateSandboxSecurity({ - binds: ["/etc/passwd:/etc/passwd:ro"], - env: { NODE_ENV: "production" }, - }), - ).toThrow(/blocked path/); - }); }); diff --git a/src/agents/sandbox/validate-sandbox-security.ts b/src/agents/sandbox/validate-sandbox-security.ts index 9fdbb08257..2ed84e9c93 100644 --- a/src/agents/sandbox/validate-sandbox-security.ts +++ b/src/agents/sandbox/validate-sandbox-security.ts @@ -7,7 +7,6 @@ import { existsSync, realpathSync } from "node:fs"; import { posix } from "node:path"; -import { sanitizeEnvVars } from "./sanitize-env-vars.js"; // Targeted denylist: host paths that should never be exposed inside sandbox containers. // Exported for reuse in security audit collectors. @@ -183,35 +182,14 @@ export function validateApparmorProfile(profile: string | undefined): void { } } -/** - * Validate environment variables - throws if any sensitive credentials are detected - */ -export function validateEnvVars(env: Record | undefined): void { - if (!env || Object.keys(env).length === 0) { - return; - } - - const result = sanitizeEnvVars(env, { strictMode: false }); - - if (result.blocked.length > 0) { - throw new Error( - `Sandbox security: blocked sensitive environment variables: ${result.blocked.map((b) => b.key).join(", ")}. ` + - "Passing credentials as environment variables to sandbox containers is not allowed. " + - "Use secure credential storage or remove these variables from sandbox configuration.", - ); - } -} - export function validateSandboxSecurity(cfg: { binds?: string[]; network?: string; seccompProfile?: string; apparmorProfile?: string; - env?: Record; }): void { validateBindMounts(cfg.binds); validateNetworkMode(cfg.network); validateSeccompProfile(cfg.seccompProfile); validateApparmorProfile(cfg.apparmorProfile); - validateEnvVars(cfg.env); }