From c2c7114ed39a547ab6276e1e933029b9530ee906 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Sun, 22 Feb 2026 09:41:55 +0100 Subject: [PATCH] fix(security): block HOME and ZDOTDIR env override injection --- .../Sources/OpenClaw/HostEnvSanitizer.swift | 5 +++++ src/infra/host-env-security-policy.json | 1 + .../host-env-security.policy-parity.test.ts | 6 ++++++ src/infra/host-env-security.test.ts | 18 ++++++++++++++++-- src/infra/host-env-security.ts | 17 ++++++++++++++++- src/node-host/invoke.sanitize-env.test.ts | 11 +++++++++++ 6 files changed, 55 insertions(+), 3 deletions(-) diff --git a/apps/macos/Sources/OpenClaw/HostEnvSanitizer.swift b/apps/macos/Sources/OpenClaw/HostEnvSanitizer.swift index b387c36d3a4..846c8978191 100644 --- a/apps/macos/Sources/OpenClaw/HostEnvSanitizer.swift +++ b/apps/macos/Sources/OpenClaw/HostEnvSanitizer.swift @@ -25,6 +25,10 @@ enum HostEnvSanitizer { "LD_", "BASH_FUNC_", ] + private static let blockedOverrideKeys: Set = [ + "HOME", + "ZDOTDIR", + ] private static func isBlocked(_ upperKey: String) -> Bool { if self.blockedKeys.contains(upperKey) { return true } @@ -49,6 +53,7 @@ enum HostEnvSanitizer { // PATH is part of the security boundary (command resolution + safe-bin checks). Never // allow request-scoped PATH overrides from agents/gateways. if upper == "PATH" { continue } + if self.blockedOverrideKeys.contains(upper) { continue } if self.isBlocked(upper) { continue } merged[key] = value } diff --git a/src/infra/host-env-security-policy.json b/src/infra/host-env-security-policy.json index aeb8200ec0a..341af1c5db3 100644 --- a/src/infra/host-env-security-policy.json +++ b/src/infra/host-env-security-policy.json @@ -15,5 +15,6 @@ "IFS", "SSLKEYLOGFILE" ], + "blockedOverrideKeys": ["HOME", "ZDOTDIR"], "blockedPrefixes": ["DYLD_", "LD_", "BASH_FUNC_"] } diff --git a/src/infra/host-env-security.policy-parity.test.ts b/src/infra/host-env-security.policy-parity.test.ts index 1b989d52244..4ee46265447 100644 --- a/src/infra/host-env-security.policy-parity.test.ts +++ b/src/infra/host-env-security.policy-parity.test.ts @@ -4,6 +4,7 @@ import { describe, expect, it } from "vitest"; type HostEnvSecurityPolicy = { blockedKeys: string[]; + blockedOverrideKeys?: string[]; blockedPrefixes: string[]; }; @@ -27,12 +28,17 @@ describe("host env security policy parity", () => { const swiftSource = fs.readFileSync(swiftPath, "utf8"); const swiftBlockedKeys = parseSwiftStringArray(swiftSource, "private static let blockedKeys"); + const swiftBlockedOverrideKeys = parseSwiftStringArray( + swiftSource, + "private static let blockedOverrideKeys", + ); const swiftBlockedPrefixes = parseSwiftStringArray( swiftSource, "private static let blockedPrefixes", ); expect(swiftBlockedKeys).toEqual(policy.blockedKeys); + expect(swiftBlockedOverrideKeys).toEqual(policy.blockedOverrideKeys ?? []); expect(swiftBlockedPrefixes).toEqual(policy.blockedPrefixes); }); }); diff --git a/src/infra/host-env-security.test.ts b/src/infra/host-env-security.test.ts index aefd6cd4005..df1ccd874b8 100644 --- a/src/infra/host-env-security.test.ts +++ b/src/infra/host-env-security.test.ts @@ -1,5 +1,6 @@ import { describe, expect, it } from "vitest"; import { + isDangerousHostEnvOverrideVarName, isDangerousHostEnvVarName, normalizeEnvVarKey, sanitizeHostExecEnv, @@ -39,10 +40,13 @@ describe("sanitizeHostExecEnv", () => { const env = sanitizeHostExecEnv({ baseEnv: { PATH: "/usr/bin:/bin", - HOME: "/tmp/home", + HOME: "/tmp/trusted-home", + ZDOTDIR: "/tmp/trusted-zdotdir", }, overrides: { PATH: "/tmp/evil", + HOME: "/tmp/evil-home", + ZDOTDIR: "/tmp/evil-zdotdir", BASH_ENV: "/tmp/pwn.sh", SAFE: "ok", }, @@ -51,7 +55,8 @@ describe("sanitizeHostExecEnv", () => { expect(env.PATH).toBe("/usr/bin:/bin"); expect(env.BASH_ENV).toBeUndefined(); expect(env.SAFE).toBe("ok"); - expect(env.HOME).toBe("/tmp/home"); + expect(env.HOME).toBe("/tmp/trusted-home"); + expect(env.ZDOTDIR).toBe("/tmp/trusted-zdotdir"); }); it("drops non-portable env key names", () => { @@ -72,6 +77,15 @@ describe("sanitizeHostExecEnv", () => { }); }); +describe("isDangerousHostEnvOverrideVarName", () => { + it("matches override-only blocked keys case-insensitively", () => { + expect(isDangerousHostEnvOverrideVarName("HOME")).toBe(true); + expect(isDangerousHostEnvOverrideVarName("zdotdir")).toBe(true); + expect(isDangerousHostEnvOverrideVarName("BASH_ENV")).toBe(false); + expect(isDangerousHostEnvOverrideVarName("FOO")).toBe(false); + }); +}); + describe("normalizeEnvVarKey", () => { it("normalizes and validates keys", () => { expect(normalizeEnvVarKey(" OPENROUTER_API_KEY ")).toBe("OPENROUTER_API_KEY"); diff --git a/src/infra/host-env-security.ts b/src/infra/host-env-security.ts index f5cd775e70a..b1d869cf9a2 100644 --- a/src/infra/host-env-security.ts +++ b/src/infra/host-env-security.ts @@ -4,6 +4,7 @@ const PORTABLE_ENV_VAR_KEY = /^[A-Za-z_][A-Za-z0-9_]*$/; type HostEnvSecurityPolicy = { blockedKeys: string[]; + blockedOverrideKeys?: string[]; blockedPrefixes: string[]; }; @@ -15,7 +16,13 @@ export const HOST_DANGEROUS_ENV_KEY_VALUES: readonly string[] = Object.freeze( export const HOST_DANGEROUS_ENV_PREFIXES: readonly string[] = Object.freeze( HOST_ENV_SECURITY_POLICY.blockedPrefixes.map((prefix) => prefix.toUpperCase()), ); +export const HOST_DANGEROUS_OVERRIDE_ENV_KEY_VALUES: readonly string[] = Object.freeze( + (HOST_ENV_SECURITY_POLICY.blockedOverrideKeys ?? []).map((key) => key.toUpperCase()), +); export const HOST_DANGEROUS_ENV_KEYS = new Set(HOST_DANGEROUS_ENV_KEY_VALUES); +export const HOST_DANGEROUS_OVERRIDE_ENV_KEYS = new Set( + HOST_DANGEROUS_OVERRIDE_ENV_KEY_VALUES, +); export function normalizeEnvVarKey( rawKey: string, @@ -43,6 +50,14 @@ export function isDangerousHostEnvVarName(rawKey: string): boolean { return HOST_DANGEROUS_ENV_PREFIXES.some((prefix) => upper.startsWith(prefix)); } +export function isDangerousHostEnvOverrideVarName(rawKey: string): boolean { + const key = normalizeEnvVarKey(rawKey); + if (!key) { + return false; + } + return HOST_DANGEROUS_OVERRIDE_ENV_KEYS.has(key.toUpperCase()); +} + export function sanitizeHostExecEnv(params?: { baseEnv?: Record; overrides?: Record | null; @@ -82,7 +97,7 @@ export function sanitizeHostExecEnv(params?: { if (blockPathOverrides && upper === "PATH") { continue; } - if (isDangerousHostEnvVarName(upper)) { + if (isDangerousHostEnvVarName(upper) || isDangerousHostEnvOverrideVarName(upper)) { continue; } merged[key] = value; diff --git a/src/node-host/invoke.sanitize-env.test.ts b/src/node-host/invoke.sanitize-env.test.ts index 7fef6e3a198..fe91432198b 100644 --- a/src/node-host/invoke.sanitize-env.test.ts +++ b/src/node-host/invoke.sanitize-env.test.ts @@ -26,6 +26,17 @@ describe("node-host sanitizeEnv", () => { }); }); + it("blocks dangerous override-only env keys", () => { + withEnv({ HOME: "/Users/trusted", ZDOTDIR: "/Users/trusted/.zdot" }, () => { + const env = sanitizeEnv({ + HOME: "/tmp/evil-home", + ZDOTDIR: "/tmp/evil-zdotdir", + }); + expect(env.HOME).toBe("/Users/trusted"); + expect(env.ZDOTDIR).toBe("/Users/trusted/.zdot"); + }); + }); + it("drops dangerous inherited env keys even without overrides", () => { withEnv({ PATH: "/usr/bin:/bin", BASH_ENV: "/tmp/pwn.sh" }, () => { const env = sanitizeEnv(undefined);