From 045a581069dcb524df3c25170815428d682d0eb6 Mon Sep 17 00:00:00 2001 From: Josh Avant <830519+joshavant@users.noreply.github.com> Date: Sat, 16 May 2026 17:36:05 -0500 Subject: [PATCH] fix(sandbox): honor explicit docker env (#82763) * fix(sandbox): honor explicit docker env * docs(changelog): note sandbox env fix --- CHANGELOG.md | 1 + docs/gateway/sandboxing.md | 1 + docs/tools/skills-config.md | 2 + src/agents/sandbox-create-args.test.ts | 28 +++++++--- src/agents/sandbox/browser.create.test.ts | 54 ++++++++++++++++++- src/agents/sandbox/browser.ts | 2 + src/agents/sandbox/config-hash.ts | 4 ++ .../docker.config-hash-recreate.test.ts | 52 +++++++++++++++++- src/agents/sandbox/docker.ts | 53 +++++++++++++++--- src/agents/sandbox/sanitize-env-vars.test.ts | 29 +++++++++- src/agents/sandbox/sanitize-env-vars.ts | 28 ++++++++++ src/config/types.sandbox.ts | 2 +- 12 files changed, 239 insertions(+), 17 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1effc9c7b4b..3a021e90b9f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -107,6 +107,7 @@ Docs: https://docs.openclaw.ai - Agents/OpenAI: honor `openai-codex:*` entries placed ahead of API-key backups in `auth.order.openai` for explicit OpenAI PI runs, and accept `models auth login --provider openai-codex --device-code` for headless sign-in. Fixes #82521. (#82605) - CLI/channels: install missing externalized same-id channel plugins during `channels add --channel `, so recovery for WhatsApp and other externalized stock channels does not require a separate `plugins enable` step. Fixes #82533. - MCP plugin tools: forward host MCP `tools/call` `AbortSignal` through `createPluginToolsMcpHandlers().callTool` into plugin `tool.execute`, so host cancellation actually cancels in-flight plugin tool calls instead of letting them run to completion. Fixes #82424. (#82443) Thanks @joshavant. +- Agents/sandbox: honor explicit Docker sandbox env variables with credential-looking names during container creation, and recreate affected sandbox containers when the effective env policy changes. Fixes #82695. (#82763) Thanks @joshavant. - Plugins: accept deprecated `api.on("deactivate")` registrations as a dated compatibility alias for `gateway_stop`, so external plugin cleanup handlers run on Gateway shutdown while authors get migration guidance. - Media: ignore image MIME and filename hints when bytes sniff as generic containers, so zip/octet-stream payloads mislabeled as images do not become local image media or keep image file extensions when staged. - Update/doctor: avoid materializing `groupAllowFrom` for channel schemas that reject it, so package-swap doctor repairs do not fail on externalized Slack configs. diff --git a/docs/gateway/sandboxing.md b/docs/gateway/sandboxing.md index 0353b6d8f3f..2163afb203c 100644 --- a/docs/gateway/sandboxing.md +++ b/docs/gateway/sandboxing.md @@ -486,6 +486,7 @@ Paths: - `readOnlyRoot: true` prevents writes; set `readOnlyRoot: false` or bake a custom image. - `user` must be root for package installs (omit `user` or set `user: "0:0"`). - Sandbox exec does **not** inherit host `process.env`. Use `agents.defaults.sandbox.docker.env` (or a custom image) for skill API keys. + - Values in `agents.defaults.sandbox.docker.env` are passed as explicit Docker container environment variables. Anyone with Docker daemon access can inspect them with Docker metadata commands such as `docker inspect`. Use a custom image, mounted secret file, or another secret delivery path if that metadata exposure is not acceptable. diff --git a/docs/tools/skills-config.md b/docs/tools/skills-config.md index 65a465f83cb..8efe06dea9f 100644 --- a/docs/tools/skills-config.md +++ b/docs/tools/skills-config.md @@ -175,6 +175,8 @@ Use one of: - `agents.defaults.sandbox.docker.env` for the Docker backend (or per-agent `agents.list[].sandbox.docker.env`). - Bake the env into your custom sandbox image or remote sandbox environment. +For Docker sandboxes, configured `sandbox.docker.env` values become explicit container environment variables. Users with Docker daemon access can inspect them through Docker metadata, so use a mounted secret file, custom image, or another delivery path when that exposure is not acceptable. + ## Related diff --git a/src/agents/sandbox-create-args.test.ts b/src/agents/sandbox-create-args.test.ts index ec25ac95457..06ad063455c 100644 --- a/src/agents/sandbox-create-args.test.ts +++ b/src/agents/sandbox-create-args.test.ts @@ -120,10 +120,18 @@ describe("buildSandboxCreateArgs", () => { expectFlagValues(args, "--ulimit", ["nofile=1024:2048", "nproc=128", "core=0"]); }); - it("preserves the OpenClaw exec marker when strict env sanitization is enabled", () => { + it("passes explicit configured sandbox env through even when names look sensitive", () => { const cfg = createSandboxConfig({ env: { - NODE_ENV: "test", + ANTHROPIC_ADMIN_KEY: "dummy-anthropic-admin-key", + GEMINI_API_KEY: "dummy-gemini-api-key", + GOOGLE_CLIENT_ID: "dummy-google-client-id", + GOOGLE_CLIENT_SECRET: "dummy-google-client-secret", + HIMALAYA_CONFIG: "dummy-himalaya-config", + HIMALAYA_PASSWORD: "dummy-himalaya-password", + OURA_CLIENT_ID: "dummy-oura-client-id", + OURA_CLIENT_SECRET: "dummy-oura-client-secret", + RESEND_API_KEY: "dummy-resend-api-key", }, }); @@ -132,12 +140,20 @@ describe("buildSandboxCreateArgs", () => { cfg, scopeKey: "main", createdAtMs: 1700000000000, - envSanitizationOptions: { - strictMode: true, - }, }); - expectFlagValues(args, "--env", ["NODE_ENV=test", `OPENCLAW_CLI=${OPENCLAW_CLI_ENV_VALUE}`]); + expectFlagValues(args, "--env", [ + "ANTHROPIC_ADMIN_KEY=dummy-anthropic-admin-key", + "GEMINI_API_KEY=dummy-gemini-api-key", + "GOOGLE_CLIENT_ID=dummy-google-client-id", + "GOOGLE_CLIENT_SECRET=dummy-google-client-secret", + "HIMALAYA_CONFIG=dummy-himalaya-config", + "HIMALAYA_PASSWORD=dummy-himalaya-password", + "OURA_CLIENT_ID=dummy-oura-client-id", + "OURA_CLIENT_SECRET=dummy-oura-client-secret", + "RESEND_API_KEY=dummy-resend-api-key", + `OPENCLAW_CLI=${OPENCLAW_CLI_ENV_VALUE}`, + ]); }); it("emits Docker GPU passthrough as a separate argument", () => { diff --git a/src/agents/sandbox/browser.create.test.ts b/src/agents/sandbox/browser.create.test.ts index 882feddda6c..dd1d3f943ba 100644 --- a/src/agents/sandbox/browser.create.test.ts +++ b/src/agents/sandbox/browser.create.test.ts @@ -1,6 +1,14 @@ import { readFileSync } from "node:fs"; import { beforeAll, beforeEach, describe, expect, it, vi } from "vitest"; -import { SANDBOX_BROWSER_IMAGE_CONTRACT_EPOCH } from "./constants.js"; +import { + computeSandboxBrowserConfigHash, + SANDBOX_DOCKER_EXPLICIT_ENV_POLICY_EPOCH, +} from "./config-hash.js"; +import { resolveSandboxBrowserDockerCreateConfig } from "./config.js"; +import { + SANDBOX_BROWSER_IMAGE_CONTRACT_EPOCH, + SANDBOX_BROWSER_SECURITY_HASH_EPOCH, +} from "./constants.js"; import { collectDockerFlagValues, findDockerArgsCall } from "./test-args.js"; import type { SandboxConfig } from "./types.js"; import { SANDBOX_MOUNT_FORMAT_VERSION } from "./workspace-mounts.js"; @@ -302,6 +310,50 @@ describe("ensureSandboxBrowser create args", () => { expect(result?.noVncUrl).toBeUndefined(); }); + it("includes the explicit env policy epoch in the browser config hash when needed", async () => { + const cfg = buildConfig(false); + cfg.docker.env = { + LANG: "C.UTF-8", + GEMINI_API_KEY: "dummy-gemini", + }; + const scopeKey = "session-1"; + const workspaceDir = "/tmp/workspace"; + const agentWorkspaceDir = "/tmp/workspace"; + const browserDockerCfg = resolveSandboxBrowserDockerCreateConfig({ + docker: cfg.docker, + browser: cfg.browser, + }); + const expectedHash = computeSandboxBrowserConfigHash({ + docker: browserDockerCfg, + dockerEnvPolicyEpoch: SANDBOX_DOCKER_EXPLICIT_ENV_POLICY_EPOCH, + browser: { + cdpPort: cfg.browser.cdpPort, + vncPort: cfg.browser.vncPort, + noVncPort: cfg.browser.noVncPort, + headless: cfg.browser.headless, + enableNoVnc: cfg.browser.enableNoVnc, + autoStartTimeoutMs: cfg.browser.autoStartTimeoutMs, + cdpSourceRange: undefined, + }, + securityEpoch: SANDBOX_BROWSER_SECURITY_HASH_EPOCH, + workspaceAccess: cfg.workspaceAccess, + workspaceDir, + agentWorkspaceDir, + mountFormatVersion: SANDBOX_MOUNT_FORMAT_VERSION, + }); + + await ensureTestSandboxBrowser({ + scopeKey, + workspaceDir, + agentWorkspaceDir, + cfg, + }); + + const createArgs = requireDockerCreateArgs(); + expect(createArgs).toContain(`openclaw.configHash=${expectedHash}`); + expect(collectDockerFlagValues(createArgs, "--env")).toContain("GEMINI_API_KEY=dummy-gemini"); + }); + it("fails before creating a browser container when Docker daemon is unavailable", async () => { dockerMocks.execDocker.mockImplementation(async (args: string[]) => { if (args[0] === "network" && args[1] === "inspect") { diff --git a/src/agents/sandbox/browser.ts b/src/agents/sandbox/browser.ts index 4d8158a5ed8..5b35058d89e 100644 --- a/src/agents/sandbox/browser.ts +++ b/src/agents/sandbox/browser.ts @@ -35,6 +35,7 @@ import { readDockerContainerEnvVar, readDockerContainerLabel, readDockerPort, + resolveDockerEnvPolicyEpoch, } from "./docker.js"; import { buildNoVncObserverTokenUrl, @@ -228,6 +229,7 @@ export async function ensureSandboxBrowser(params: { }); const expectedHash = computeSandboxBrowserConfigHash({ docker: browserDockerCfg, + dockerEnvPolicyEpoch: resolveDockerEnvPolicyEpoch(browserDockerCfg.env), browser: { cdpPort: params.cfg.browser.cdpPort, vncPort: params.cfg.browser.vncPort, diff --git a/src/agents/sandbox/config-hash.ts b/src/agents/sandbox/config-hash.ts index 3d0d914cf67..37a99759431 100644 --- a/src/agents/sandbox/config-hash.ts +++ b/src/agents/sandbox/config-hash.ts @@ -1,8 +1,11 @@ import { hashTextSha256 } from "./hash.js"; import type { SandboxBrowserConfig, SandboxDockerConfig, SandboxWorkspaceAccess } from "./types.js"; +export const SANDBOX_DOCKER_EXPLICIT_ENV_POLICY_EPOCH = "explicit-config-env-v1"; + type SandboxHashInput = { docker: SandboxDockerConfig; + dockerEnvPolicyEpoch?: string; workspaceAccess: SandboxWorkspaceAccess; workspaceDir: string; agentWorkspaceDir: string; @@ -11,6 +14,7 @@ type SandboxHashInput = { type SandboxBrowserHashInput = { docker: SandboxDockerConfig; + dockerEnvPolicyEpoch?: string; browser: Pick< SandboxBrowserConfig, | "cdpPort" diff --git a/src/agents/sandbox/docker.config-hash-recreate.test.ts b/src/agents/sandbox/docker.config-hash-recreate.test.ts index 76f5c3b9365..b80d9abd1db 100644 --- a/src/agents/sandbox/docker.config-hash-recreate.test.ts +++ b/src/agents/sandbox/docker.config-hash-recreate.test.ts @@ -1,7 +1,10 @@ import { EventEmitter } from "node:events"; import { Readable } from "node:stream"; import { beforeEach, describe, expect, it, vi } from "vitest"; -import { computeSandboxConfigHash } from "./config-hash.js"; +import { + computeSandboxConfigHash, + SANDBOX_DOCKER_EXPLICIT_ENV_POLICY_EPOCH, +} from "./config-hash.js"; import { collectDockerFlagValues } from "./test-args.js"; import type { SandboxConfig } from "./types.js"; import { SANDBOX_MOUNT_FORMAT_VERSION } from "./workspace-mounts.js"; @@ -111,6 +114,7 @@ function createSandboxConfig( dns: string[], binds?: string[], workspaceAccess: "rw" | "ro" | "none" = "rw", + env: Record = { LANG: "C.UTF-8" }, ): SandboxConfig { return { mode: "all", @@ -126,7 +130,7 @@ function createSandboxConfig( tmpfs: ["/tmp", "/var/tmp", "/run"], network: "none", capDrop: ["ALL"], - env: { LANG: "C.UTF-8" }, + env, dns, extraHosts: ["host.docker.internal:host-gateway"], binds: binds ?? ["/tmp/workspace:/workspace:rw"], @@ -246,6 +250,50 @@ describe("ensureSandboxContainer config-hash recreation", () => { expect(registryUpdate?.configHash).toBe(newHash); }); + it("recreates shared container when previously filtered explicit env becomes allowed", async () => { + const workspaceDir = "/tmp/workspace"; + const cfg = createSandboxConfig(["1.1.1.1"], undefined, "rw", { + LANG: "C.UTF-8", + GEMINI_API_KEY: "dummy-gemini", + }); + + const oldHash = computeSandboxConfigHash({ + docker: cfg.docker, + workspaceAccess: cfg.workspaceAccess, + workspaceDir, + agentWorkspaceDir: workspaceDir, + mountFormatVersion: SANDBOX_MOUNT_FORMAT_VERSION, + }); + const newHash = computeSandboxConfigHash({ + docker: cfg.docker, + dockerEnvPolicyEpoch: SANDBOX_DOCKER_EXPLICIT_ENV_POLICY_EPOCH, + workspaceAccess: cfg.workspaceAccess, + workspaceDir, + agentWorkspaceDir: workspaceDir, + mountFormatVersion: SANDBOX_MOUNT_FORMAT_VERSION, + }); + expect(newHash).not.toBe(oldHash); + + spawnState.labelHash = oldHash; + registryMocks.readRegistryEntry.mockResolvedValue({ + containerName: "oc-test-shared", + sessionKey: "shared", + createdAtMs: 1, + lastUsedAtMs: 0, + image: cfg.docker.image, + configHash: oldHash, + }); + + const createCall = await ensureSandboxCreateCallForTest({ cfg, workspaceDir }); + expect(createCall.args).toContain(`openclaw.configHash=${newHash}`); + expect(collectDockerFlagValues(createCall.args, "--env")).toEqual( + expect.arrayContaining(["LANG=C.UTF-8", "GEMINI_API_KEY=dummy-gemini"]), + ); + + const registryUpdate = registryMocks.updateRegistry.mock.calls.at(-1)?.[0]; + expect(registryUpdate?.configHash).toBe(newHash); + }); + it("applies custom binds after workspace mounts so overlapping binds can override", async () => { const workspaceDir = "/tmp/workspace"; const cfg = createSandboxConfig( diff --git a/src/agents/sandbox/docker.ts b/src/agents/sandbox/docker.ts index 597e749965e..b1de2188a07 100644 --- a/src/agents/sandbox/docker.ts +++ b/src/agents/sandbox/docker.ts @@ -4,8 +4,11 @@ import { materializeWindowsSpawnProgram, resolveWindowsSpawnProgram, } from "../../plugin-sdk/windows-spawn.js"; -import { sanitizeEnvVars } from "./sanitize-env-vars.js"; -import type { EnvSanitizationOptions } from "./sanitize-env-vars.js"; +import { + sanitizeEnvVars, + sanitizeExplicitSandboxEnvVars, + type EnvSanitizationOptions, +} from "./sanitize-env-vars.js"; type ExecDockerRawOptions = { allowFailure?: boolean; @@ -165,7 +168,10 @@ export function execDockerRaw( import { formatCliCommand } from "../../cli/command-format.js"; import { markOpenClawExecEnv } from "../../infra/openclaw-exec-env.js"; import { defaultRuntime } from "../../runtime.js"; -import { computeSandboxConfigHash } from "./config-hash.js"; +import { + computeSandboxConfigHash, + SANDBOX_DOCKER_EXPLICIT_ENV_POLICY_EPOCH, +} from "./config-hash.js"; import { DEFAULT_SANDBOX_IMAGE } from "./constants.js"; import { readRegistryEntry, updateRegistry } from "./registry.js"; import { resolveSandboxAgentId, resolveSandboxScopeKey, slugifySessionKey } from "./shared.js"; @@ -179,6 +185,31 @@ const HOT_CONTAINER_WINDOW_MS = 5 * 60 * 1000; export type ExecDockerOptions = ExecDockerRawOptions; +function envRecordsEqual(left: Record, right: Record): boolean { + const leftEntries = Object.entries(left).toSorted(([leftKey], [rightKey]) => + leftKey.localeCompare(rightKey), + ); + const rightEntries = Object.entries(right).toSorted(([leftKey], [rightKey]) => + leftKey.localeCompare(rightKey), + ); + if (leftEntries.length !== rightEntries.length) { + return false; + } + return leftEntries.every(([key, value], index) => { + const rightEntry = rightEntries[index]; + return rightEntry?.[0] === key && rightEntry[1] === value; + }); +} + +export function resolveDockerEnvPolicyEpoch(env: Record | undefined) { + const explicitEnv = env ?? {}; + const previousAllowed = sanitizeEnvVars(explicitEnv).allowed; + const currentAllowed = sanitizeExplicitSandboxEnvVars(explicitEnv).allowed; + return envRecordsEqual(previousAllowed, currentAllowed) + ? undefined + : SANDBOX_DOCKER_EXPLICIT_ENV_POLICY_EPOCH; +} + export async function execDocker(args: string[], opts?: ExecDockerOptions) { const result = await execDockerRaw(args, opts); return { @@ -382,6 +413,11 @@ export function buildSandboxCreateArgs(params: { allowSourcesOutsideAllowedRoots?: boolean; allowReservedContainerTargets?: boolean; allowContainerNamespaceJoin?: boolean; + /** + * @deprecated Docker container creation now treats cfg.env as explicit sandbox + * configuration and ignores host-env name filters. This field is kept so SDK + * callers with existing object literals do not hit excess-property failures. + */ envSanitizationOptions?: EnvSanitizationOptions; }) { // Runtime security validation: blocks dangerous bind mounts, network modes, and profiles. @@ -425,12 +461,16 @@ export function buildSandboxCreateArgs(params: { if (params.cfg.user) { args.push("--user", params.cfg.user); } - const envSanitization = sanitizeEnvVars(params.cfg.env ?? {}, params.envSanitizationOptions); + const envSanitization = sanitizeExplicitSandboxEnvVars(params.cfg.env ?? {}); if (envSanitization.blocked.length > 0) { - log.warn(`Blocked sensitive environment variables: ${envSanitization.blocked.join(", ")}`); + log.warn( + `Blocked invalid configured sandbox environment variables: ${envSanitization.blocked.join(", ")}`, + ); } if (envSanitization.warnings.length > 0) { - log.warn(`Suspicious environment variables: ${envSanitization.warnings.join(", ")}`); + log.warn( + `Suspicious configured sandbox environment variables: ${envSanitization.warnings.join(", ")}`, + ); } for (const [key, value] of Object.entries(markOpenClawExecEnv(envSanitization.allowed))) { args.push("--env", `${key}=${value}`); @@ -562,6 +602,7 @@ export async function ensureSandboxContainer(params: { const containerName = name.slice(0, 63); const expectedHash = computeSandboxConfigHash({ docker: params.cfg.docker, + dockerEnvPolicyEpoch: resolveDockerEnvPolicyEpoch(params.cfg.docker.env), workspaceAccess: params.cfg.workspaceAccess, workspaceDir: params.workspaceDir, agentWorkspaceDir: params.agentWorkspaceDir, diff --git a/src/agents/sandbox/sanitize-env-vars.test.ts b/src/agents/sandbox/sanitize-env-vars.test.ts index c52bec4a4f7..cf8237ae9f4 100644 --- a/src/agents/sandbox/sanitize-env-vars.test.ts +++ b/src/agents/sandbox/sanitize-env-vars.test.ts @@ -1,5 +1,5 @@ import { describe, expect, it } from "vitest"; -import { sanitizeEnvVars } from "./sanitize-env-vars.js"; +import { sanitizeEnvVars, sanitizeExplicitSandboxEnvVars } from "./sanitize-env-vars.js"; describe("sanitizeEnvVars", () => { it("keeps normal env vars and blocks obvious credentials", () => { @@ -65,4 +65,31 @@ describe("sanitizeEnvVars", () => { expect(result.allowed).toEqual({ NODE_ENV: "test" }); expect(result.blocked).toStrictEqual([]); }); + + it("allows explicit configured sandbox env names that look like credentials", () => { + const result = sanitizeExplicitSandboxEnvVars({ + GEMINI_API_KEY: "dummy-gemini-api-key", + GOOGLE_CLIENT_SECRET: "dummy-google-client-secret", + HIMALAYA_PASSWORD: "dummy-himalaya-password", + RESEND_API_KEY: "dummy-resend-api-key", + }); + + expect(result.allowed).toEqual({ + GEMINI_API_KEY: "dummy-gemini-api-key", + GOOGLE_CLIENT_SECRET: "dummy-google-client-secret", + HIMALAYA_PASSWORD: "dummy-himalaya-password", + RESEND_API_KEY: "dummy-resend-api-key", + }); + expect(result.blocked).toStrictEqual([]); + }); + + it("still blocks invalid explicit configured sandbox env values", () => { + const result = sanitizeExplicitSandboxEnvVars({ + SAFE_SECRET: "ok", + NULL_SECRET: "a\0b", + }); + + expect(result.allowed).toEqual({ SAFE_SECRET: "ok" }); + expect(result.blocked).toStrictEqual(["NULL_SECRET"]); + }); }); diff --git a/src/agents/sandbox/sanitize-env-vars.ts b/src/agents/sandbox/sanitize-env-vars.ts index 3caea56ac3f..3489cbd44b0 100644 --- a/src/agents/sandbox/sanitize-env-vars.ts +++ b/src/agents/sandbox/sanitize-env-vars.ts @@ -100,3 +100,31 @@ export function sanitizeEnvVars( return { allowed, blocked, warnings }; } + +export function sanitizeExplicitSandboxEnvVars( + envVars: Record, +): EnvVarSanitizationResult { + const allowed: Record = {}; + const blocked: string[] = []; + const warnings: string[] = []; + + for (const [rawKey, value] of Object.entries(envVars)) { + const key = rawKey.trim(); + if (!key || value === undefined) { + continue; + } + + const warning = validateEnvVarValue(value); + if (warning) { + if (warning === "Contains null bytes") { + blocked.push(key); + continue; + } + warnings.push(`${key}: ${warning}`); + } + + allowed[key] = value; + } + + return { allowed, blocked, warnings }; +} diff --git a/src/config/types.sandbox.ts b/src/config/types.sandbox.ts index 1a4670d3c2a..7901af6c7a2 100644 --- a/src/config/types.sandbox.ts +++ b/src/config/types.sandbox.ts @@ -17,7 +17,7 @@ export type SandboxDockerSettings = { user?: string; /** Drop Linux capabilities. */ capDrop?: string[]; - /** Extra environment variables for sandbox exec. */ + /** Explicit environment variables for sandbox container creation and exec. */ env?: Record; /** Optional setup command run once after container creation (array entries are joined by newline). */ setupCommand?: string;