diff --git a/CHANGELOG.md b/CHANGELOG.md index 125a43bb8..9f7fa5b6e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -86,7 +86,7 @@ Docs: https://docs.openclaw.ai - Security/Audit: add `openclaw security audit` finding `gateway.nodes.allow_commands_dangerous` for risky `gateway.nodes.allowCommands` overrides, with severity upgraded to critical on remote gateway exposure. - Gateway/Control plane: reduce cross-client write limiter contention by adding `connId` fallback keying when device ID and client IP are both unavailable. - Security/Config: block prototype-key traversal during config merge patch and legacy migration merge helpers (`__proto__`, `constructor`, `prototype`) to prevent prototype pollution during config mutation flows. (#22968) Thanks @Clawborn. -- Security/Shell env: validate login-shell executable paths for shell-env fallback (`/etc/shells` + trusted prefixes) and block `SHELL` in dangerous env override policy paths so untrusted shell-path injection falls back safely to `/bin/sh`. Thanks @athuljayaram for reporting. +- Security/Shell env: validate login-shell executable paths for shell-env fallback (`/etc/shells` + trusted prefixes), block `SHELL`/`HOME`/`ZDOTDIR` in config env ingestion before fallback execution, and sanitize fallback shell exec env to pin `HOME` to the real user home while dropping `ZDOTDIR` and other dangerous startup vars. This ships in the next npm release. Thanks @tdjackey for reporting. - Security/Config: make parsed chat allowlist checks fail closed when `allowFrom` is empty, restoring expected DM/pairing gating. - Security/Exec: in non-default setups that manually add `sort` to `tools.exec.safeBins`, block `sort --compress-program` so allowlist-mode safe-bin checks cannot bypass approval. Thanks @tdjackey for reporting. - Security/Exec approvals: when users choose `allow-always` for shell-wrapper commands (for example `/bin/zsh -lc ...`), persist allowlist patterns for the inner executable(s) instead of the wrapper shell binary, preventing accidental broad shell allowlisting in moderate mode. (#23276) Thanks @xrom2863. diff --git a/src/config/config.env-vars.test.ts b/src/config/config.env-vars.test.ts index acfbf62ad..d29273879 100644 --- a/src/config/config.env-vars.test.ts +++ b/src/config/config.env-vars.test.ts @@ -31,13 +31,21 @@ describe("config env vars", () => { it("blocks dangerous startup env vars from config env", async () => { await withEnvOverride( - { BASH_ENV: undefined, SHELL: undefined, OPENROUTER_API_KEY: undefined }, + { + BASH_ENV: undefined, + SHELL: undefined, + HOME: undefined, + ZDOTDIR: undefined, + OPENROUTER_API_KEY: undefined, + }, async () => { const config = { env: { vars: { BASH_ENV: "/tmp/pwn.sh", SHELL: "/tmp/evil-shell", + HOME: "/tmp/evil-home", + ZDOTDIR: "/tmp/evil-zdotdir", OPENROUTER_API_KEY: "config-key", }, }, @@ -45,11 +53,15 @@ describe("config env vars", () => { const entries = collectConfigRuntimeEnvVars(config as OpenClawConfig); expect(entries.BASH_ENV).toBeUndefined(); expect(entries.SHELL).toBeUndefined(); + expect(entries.HOME).toBeUndefined(); + expect(entries.ZDOTDIR).toBeUndefined(); expect(entries.OPENROUTER_API_KEY).toBe("config-key"); applyConfigEnvVars(config as OpenClawConfig); expect(process.env.BASH_ENV).toBeUndefined(); expect(process.env.SHELL).toBeUndefined(); + expect(process.env.HOME).toBeUndefined(); + expect(process.env.ZDOTDIR).toBeUndefined(); expect(process.env.OPENROUTER_API_KEY).toBe("config-key"); }, ); diff --git a/src/config/env-vars.ts b/src/config/env-vars.ts index a26d69a62..f9480b9f5 100644 --- a/src/config/env-vars.ts +++ b/src/config/env-vars.ts @@ -1,6 +1,14 @@ -import { isDangerousHostEnvVarName, normalizeEnvVarKey } from "../infra/host-env-security.js"; +import { + isDangerousHostEnvOverrideVarName, + isDangerousHostEnvVarName, + normalizeEnvVarKey, +} from "../infra/host-env-security.js"; import type { OpenClawConfig } from "./types.js"; +function isBlockedConfigEnvVar(key: string): boolean { + return isDangerousHostEnvVarName(key) || isDangerousHostEnvOverrideVarName(key); +} + function collectConfigEnvVarsByTarget(cfg?: OpenClawConfig): Record { const envConfig = cfg?.env; if (!envConfig) { @@ -18,7 +26,7 @@ function collectConfigEnvVarsByTarget(cfg?: OpenClawConfig): Record { } }); + it("sanitizes startup-related env vars before shell fallback exec", () => { + const env: NodeJS.ProcessEnv = { + SHELL: "/bin/bash", + HOME: "/tmp/evil-home", + ZDOTDIR: "/tmp/evil-zdotdir", + BASH_ENV: "/tmp/evil-bash-env", + PS4: "$(touch /tmp/pwned)", + }; + let receivedEnv: NodeJS.ProcessEnv | undefined; + const exec = vi.fn((_shell: string, _args: string[], options: { env: NodeJS.ProcessEnv }) => { + receivedEnv = options.env; + return Buffer.from("OPENAI_API_KEY=from-shell\0"); + }); + + const res = loadShellEnvFallback({ + enabled: true, + env, + expectedKeys: ["OPENAI_API_KEY"], + exec: exec as unknown as Parameters[0]["exec"], + }); + + expect(res.ok).toBe(true); + expect(exec).toHaveBeenCalledTimes(1); + expect(receivedEnv).toBeDefined(); + expect(receivedEnv?.BASH_ENV).toBeUndefined(); + expect(receivedEnv?.PS4).toBeUndefined(); + expect(receivedEnv?.ZDOTDIR).toBeUndefined(); + expect(receivedEnv?.SHELL).toBeUndefined(); + expect(receivedEnv?.HOME).toBe(os.homedir()); + }); + + it("sanitizes startup-related env vars before login-shell PATH probe", () => { + resetShellPathCacheForTests(); + const env: NodeJS.ProcessEnv = { + SHELL: "/bin/bash", + HOME: "/tmp/evil-home", + ZDOTDIR: "/tmp/evil-zdotdir", + BASH_ENV: "/tmp/evil-bash-env", + PS4: "$(touch /tmp/pwned)", + }; + let receivedEnv: NodeJS.ProcessEnv | undefined; + const exec = vi.fn((_shell: string, _args: string[], options: { env: NodeJS.ProcessEnv }) => { + receivedEnv = options.env; + return Buffer.from("PATH=/usr/local/bin:/usr/bin\0HOME=/tmp\0"); + }); + + const result = getShellPathFromLoginShell({ + env, + exec: exec as unknown as Parameters[0]["exec"], + platform: "linux", + }); + + expect(result).toBe("/usr/local/bin:/usr/bin"); + expect(exec).toHaveBeenCalledTimes(1); + expect(receivedEnv).toBeDefined(); + expect(receivedEnv?.BASH_ENV).toBeUndefined(); + expect(receivedEnv?.PS4).toBeUndefined(); + expect(receivedEnv?.ZDOTDIR).toBeUndefined(); + expect(receivedEnv?.SHELL).toBeUndefined(); + expect(receivedEnv?.HOME).toBe(os.homedir()); + }); + it("returns null without invoking shell on win32", () => { resetShellPathCacheForTests(); const exec = vi.fn(() => Buffer.from("PATH=/usr/local/bin:/usr/bin\0HOME=/tmp\0")); diff --git a/src/infra/shell-env.ts b/src/infra/shell-env.ts index 0c752ce66..30f255cbc 100644 --- a/src/infra/shell-env.ts +++ b/src/infra/shell-env.ts @@ -1,7 +1,9 @@ import { execFileSync } from "node:child_process"; import fs from "node:fs"; +import os from "node:os"; import path from "node:path"; import { isTruthyEnvValue } from "./env.js"; +import { sanitizeHostExecEnv } from "./host-env-security.js"; const DEFAULT_TIMEOUT_MS = 15_000; const DEFAULT_MAX_BUFFER_BYTES = 2 * 1024 * 1024; @@ -17,6 +19,22 @@ let lastAppliedKeys: string[] = []; let cachedShellPath: string | null | undefined; let cachedEtcShells: Set | null | undefined; +function resolveShellExecEnv(env: NodeJS.ProcessEnv): NodeJS.ProcessEnv { + const execEnv = sanitizeHostExecEnv({ baseEnv: env }); + + // Startup-file resolution must stay pinned to the real user home. + const home = os.homedir().trim(); + if (home) { + execEnv.HOME = home; + } else { + delete execEnv.HOME; + } + + // Avoid zsh startup-file redirection via env poisoning. + delete execEnv.ZDOTDIR; + return execEnv; +} + function resolveTimeoutMs(timeoutMs: number | undefined): number { if (typeof timeoutMs !== "number" || !Number.isFinite(timeoutMs)) { return DEFAULT_TIMEOUT_MS; @@ -145,10 +163,11 @@ export function loadShellEnvFallback(opts: ShellEnvFallbackOptions): ShellEnvFal const timeoutMs = resolveTimeoutMs(opts.timeoutMs); const shell = resolveShell(opts.env); + const execEnv = resolveShellExecEnv(opts.env); let stdout: Buffer; try { - stdout = execLoginShellEnvZero({ shell, env: opts.env, exec, timeoutMs }); + stdout = execLoginShellEnvZero({ shell, env: execEnv, exec, timeoutMs }); } catch (err) { const msg = err instanceof Error ? err.message : String(err); logger.warn(`[openclaw] shell env fallback failed: ${msg}`); @@ -213,10 +232,11 @@ export function getShellPathFromLoginShell(opts: { const exec = opts.exec ?? execFileSync; const timeoutMs = resolveTimeoutMs(opts.timeoutMs); const shell = resolveShell(opts.env); + const execEnv = resolveShellExecEnv(opts.env); let stdout: Buffer; try { - stdout = execLoginShellEnvZero({ shell, env: opts.env, exec, timeoutMs }); + stdout = execLoginShellEnvZero({ shell, env: execEnv, exec, timeoutMs }); } catch { cachedShellPath = null; return cachedShellPath;