diff --git a/CHANGELOG.md b/CHANGELOG.md index 8f8987b18f3..a0b57cc1b45 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -615,6 +615,7 @@ Docs: https://docs.openclaw.ai - Models/provider config precedence: prefer exact `models.providers.` matches before normalized provider aliases in embedded model resolution, preventing alias/canonical key collisions from applying the wrong provider `api`, `baseUrl`, or headers. (#35934) thanks @RealKai42. - Hooks/auth throttling: reject non-`POST` `/hooks/*` requests before auth-failure accounting so unsupported methods can no longer burn the hook auth lockout budget and block legitimate webhook delivery. Thanks @JNX03 for reporting. - Network/fetch guard redirect auth stripping: switch cross-origin redirect handling in `fetchWithSsrFGuard` from a narrow sensitive-header denylist to a safe-header allowlist so custom auth headers like `X-Api-Key` and `Private-Token` no longer leak on origin changes. Thanks @Rickidevs for reporting. +- Exec/system.run env sanitization: block dangerous override-only env pivots such as `GIT_SSH_COMMAND`, editor/pager hooks, and `GIT_CONFIG_` / `NPM_CONFIG_` override prefixes so allowlisted tools cannot smuggle helper command execution through subprocess environment overrides. Thanks @tdjackey and @SnailSploit for reporting. - Logging/Subsystem console timestamps: route subsystem console timestamp rendering through `formatConsoleTimestamp(...)` so `pretty` and timestamp-prefix output use local timezone formatting consistently instead of inline UTC `toISOString()` paths. (#25970) Thanks @openperf. - Feishu/Multi-account + reply reliability: add `channels.feishu.defaultAccount` outbound routing support with schema validation, prevent inbound preview text from leaking into prompt system events, keep quoted-message extraction text-first (post/interactive/file placeholders instead of raw JSON), route Feishu video sends as `msg_type: "file"`, and avoid websocket event blocking by using non-blocking event handling in monitor dispatch. Landed from contributor PRs #31209, #29610, #30432, #30331, and #29501. Thanks @stakeswky, @hclsys, @bmendonca3, @patrick-yingxi-pan, and @zwffff. - Feishu/Target routing + replies + dedupe: normalize provider-prefixed targets (`feishu:`/`lark:`), prefer configured `channels.feishu.defaultAccount` for tool execution, honor Feishu outbound `renderMode` in adapter text/caption sends, fall back to normal send when reply targets are withdrawn/deleted, and add synchronous in-memory dedupe guard for concurrent duplicate inbound events. Landed from contributor PRs #30428, #30438, #29958, #30444, and #29463. Thanks @bmendonca3 and @Yaxuan42. diff --git a/apps/macos/Sources/OpenClaw/HostEnvSanitizer.swift b/apps/macos/Sources/OpenClaw/HostEnvSanitizer.swift index e1c4f5b8531..d5d27a212f5 100644 --- a/apps/macos/Sources/OpenClaw/HostEnvSanitizer.swift +++ b/apps/macos/Sources/OpenClaw/HostEnvSanitizer.swift @@ -6,6 +6,7 @@ enum HostEnvSanitizer { private static let blockedKeys = HostEnvSecurityPolicy.blockedKeys private static let blockedPrefixes = HostEnvSecurityPolicy.blockedPrefixes private static let blockedOverrideKeys = HostEnvSecurityPolicy.blockedOverrideKeys + private static let blockedOverridePrefixes = HostEnvSecurityPolicy.blockedOverridePrefixes private static let shellWrapperAllowedOverrideKeys: Set = [ "TERM", "LANG", @@ -22,6 +23,11 @@ enum HostEnvSanitizer { return self.blockedPrefixes.contains(where: { upperKey.hasPrefix($0) }) } + private static func isBlockedOverride(_ upperKey: String) -> Bool { + if self.blockedOverrideKeys.contains(upperKey) { return true } + return self.blockedOverridePrefixes.contains(where: { upperKey.hasPrefix($0) }) + } + private static func filterOverridesForShellWrapper(_ overrides: [String: String]?) -> [String: String]? { guard let overrides else { return nil } var filtered: [String: String] = [:] @@ -57,7 +63,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.isBlockedOverride(upper) { continue } if self.isBlocked(upper) { continue } merged[key] = value } diff --git a/apps/macos/Sources/OpenClaw/HostEnvSecurityPolicy.generated.swift b/apps/macos/Sources/OpenClaw/HostEnvSecurityPolicy.generated.swift index b126d03de21..2981a60bbf7 100644 --- a/apps/macos/Sources/OpenClaw/HostEnvSecurityPolicy.generated.swift +++ b/apps/macos/Sources/OpenClaw/HostEnvSecurityPolicy.generated.swift @@ -27,7 +27,35 @@ enum HostEnvSecurityPolicy { static let blockedOverrideKeys: Set = [ "HOME", - "ZDOTDIR" + "ZDOTDIR", + "GIT_SSH_COMMAND", + "GIT_SSH", + "GIT_PROXY_COMMAND", + "GIT_ASKPASS", + "SSH_ASKPASS", + "LESSOPEN", + "LESSCLOSE", + "PAGER", + "MANPAGER", + "GIT_PAGER", + "EDITOR", + "VISUAL", + "FCEDIT", + "SUDO_EDITOR", + "PROMPT_COMMAND", + "HISTFILE", + "PERL5DB", + "PERL5DBCMD", + "OPENSSL_CONF", + "OPENSSL_ENGINES", + "PYTHONSTARTUP", + "WGETRC", + "CURL_HOME" + ] + + static let blockedOverridePrefixes: [String] = [ + "GIT_CONFIG_", + "NPM_CONFIG_" ] static let blockedPrefixes: [String] = [ diff --git a/scripts/generate-host-env-security-policy-swift.mjs b/scripts/generate-host-env-security-policy-swift.mjs index 4de64ad8d98..b87966c491e 100644 --- a/scripts/generate-host-env-security-policy-swift.mjs +++ b/scripts/generate-host-env-security-policy-swift.mjs @@ -24,7 +24,7 @@ const outputPath = path.join( "HostEnvSecurityPolicy.generated.swift", ); -/** @type {{blockedKeys: string[]; blockedOverrideKeys?: string[]; blockedPrefixes: string[]}} */ +/** @type {{blockedKeys: string[]; blockedOverrideKeys?: string[]; blockedOverridePrefixes?: string[]; blockedPrefixes: string[]}} */ const policy = JSON.parse(fs.readFileSync(policyPath, "utf8")); const renderSwiftStringArray = (items) => items.map((item) => ` "${item}"`).join(",\n"); @@ -44,6 +44,10 @@ ${renderSwiftStringArray(policy.blockedKeys)} ${renderSwiftStringArray(policy.blockedOverrideKeys ?? [])} ] + static let blockedOverridePrefixes: [String] = [ +${renderSwiftStringArray(policy.blockedOverridePrefixes ?? [])} + ] + static let blockedPrefixes: [String] = [ ${renderSwiftStringArray(policy.blockedPrefixes)} ] diff --git a/src/infra/host-env-security-policy.json b/src/infra/host-env-security-policy.json index 4335bc43183..8b8f3cf3333 100644 --- a/src/infra/host-env-security-policy.json +++ b/src/infra/host-env-security-policy.json @@ -18,6 +18,33 @@ "IFS", "SSLKEYLOGFILE" ], - "blockedOverrideKeys": ["HOME", "ZDOTDIR"], + "blockedOverrideKeys": [ + "HOME", + "ZDOTDIR", + "GIT_SSH_COMMAND", + "GIT_SSH", + "GIT_PROXY_COMMAND", + "GIT_ASKPASS", + "SSH_ASKPASS", + "LESSOPEN", + "LESSCLOSE", + "PAGER", + "MANPAGER", + "GIT_PAGER", + "EDITOR", + "VISUAL", + "FCEDIT", + "SUDO_EDITOR", + "PROMPT_COMMAND", + "HISTFILE", + "PERL5DB", + "PERL5DBCMD", + "OPENSSL_CONF", + "OPENSSL_ENGINES", + "PYTHONSTARTUP", + "WGETRC", + "CURL_HOME" + ], + "blockedOverridePrefixes": ["GIT_CONFIG_", "NPM_CONFIG_"], "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 49b631d25a4..8ed1990e803 100644 --- a/src/infra/host-env-security.policy-parity.test.ts +++ b/src/infra/host-env-security.policy-parity.test.ts @@ -5,6 +5,7 @@ import { describe, expect, it } from "vitest"; type HostEnvSecurityPolicy = { blockedKeys: string[]; blockedOverrideKeys?: string[]; + blockedOverridePrefixes?: string[]; blockedPrefixes: string[]; }; @@ -40,6 +41,10 @@ describe("host env security policy parity", () => { generatedSource, "static let blockedOverrideKeys", ); + const swiftBlockedOverridePrefixes = parseSwiftStringArray( + generatedSource, + "static let blockedOverridePrefixes", + ); const swiftBlockedPrefixes = parseSwiftStringArray( generatedSource, "static let blockedPrefixes", @@ -47,6 +52,7 @@ describe("host env security policy parity", () => { expect(swiftBlockedKeys).toEqual(policy.blockedKeys); expect(swiftBlockedOverrideKeys).toEqual(policy.blockedOverrideKeys ?? []); + expect(swiftBlockedOverridePrefixes).toEqual(policy.blockedOverridePrefixes ?? []); expect(swiftBlockedPrefixes).toEqual(policy.blockedPrefixes); expect(sanitizerSource).toContain( @@ -55,6 +61,9 @@ describe("host env security policy parity", () => { expect(sanitizerSource).toContain( "private static let blockedOverrideKeys = HostEnvSecurityPolicy.blockedOverrideKeys", ); + expect(sanitizerSource).toContain( + "private static let blockedOverridePrefixes = HostEnvSecurityPolicy.blockedOverridePrefixes", + ); expect(sanitizerSource).toContain( "private static let blockedPrefixes = HostEnvSecurityPolicy.blockedPrefixes", ); diff --git a/src/infra/host-env-security.test.ts b/src/infra/host-env-security.test.ts index e0156077ae2..116006dbbcf 100644 --- a/src/infra/host-env-security.test.ts +++ b/src/infra/host-env-security.test.ts @@ -57,6 +57,10 @@ describe("sanitizeHostExecEnv", () => { HOME: "/tmp/evil-home", ZDOTDIR: "/tmp/evil-zdotdir", BASH_ENV: "/tmp/pwn.sh", + GIT_SSH_COMMAND: "touch /tmp/pwned", + EDITOR: "/tmp/editor", + NPM_CONFIG_USERCONFIG: "/tmp/npmrc", + GIT_CONFIG_GLOBAL: "/tmp/gitconfig", SHELLOPTS: "xtrace", PS4: "$(touch /tmp/pwned)", SAFE: "ok", @@ -65,6 +69,10 @@ describe("sanitizeHostExecEnv", () => { expect(env.PATH).toBe("/usr/bin:/bin"); expect(env.BASH_ENV).toBeUndefined(); + expect(env.GIT_SSH_COMMAND).toBeUndefined(); + expect(env.EDITOR).toBeUndefined(); + expect(env.NPM_CONFIG_USERCONFIG).toBeUndefined(); + expect(env.GIT_CONFIG_GLOBAL).toBeUndefined(); expect(env.SHELLOPTS).toBeUndefined(); expect(env.PS4).toBeUndefined(); expect(env.SAFE).toBe("ok"); @@ -110,6 +118,10 @@ describe("isDangerousHostEnvOverrideVarName", () => { it("matches override-only blocked keys case-insensitively", () => { expect(isDangerousHostEnvOverrideVarName("HOME")).toBe(true); expect(isDangerousHostEnvOverrideVarName("zdotdir")).toBe(true); + expect(isDangerousHostEnvOverrideVarName("GIT_SSH_COMMAND")).toBe(true); + expect(isDangerousHostEnvOverrideVarName("editor")).toBe(true); + expect(isDangerousHostEnvOverrideVarName("NPM_CONFIG_USERCONFIG")).toBe(true); + expect(isDangerousHostEnvOverrideVarName("git_config_global")).toBe(true); expect(isDangerousHostEnvOverrideVarName("BASH_ENV")).toBe(false); expect(isDangerousHostEnvOverrideVarName("FOO")).toBe(false); }); @@ -192,3 +204,58 @@ describe("shell wrapper exploit regression", () => { expect(fs.existsSync(marker)).toBe(false); }); }); + +describe("git env exploit regression", () => { + it("blocks GIT_SSH_COMMAND override so git cannot execute helper payloads", async () => { + if (process.platform === "win32") { + return; + } + const gitPath = "/usr/bin/git"; + if (!fs.existsSync(gitPath)) { + return; + } + + const marker = path.join(os.tmpdir(), `openclaw-git-ssh-command-${process.pid}-${Date.now()}`); + try { + fs.unlinkSync(marker); + } catch { + // no-op + } + + const target = "ssh://127.0.0.1:1/does-not-matter"; + const exploitValue = `touch ${JSON.stringify(marker)}; false`; + const baseEnv = { + PATH: process.env.PATH ?? "/usr/bin:/bin", + GIT_TERMINAL_PROMPT: "0", + }; + + const unsafeEnv = { + ...baseEnv, + GIT_SSH_COMMAND: exploitValue, + }; + + await new Promise((resolve) => { + const child = spawn(gitPath, ["ls-remote", target], { env: unsafeEnv, stdio: "ignore" }); + child.once("error", () => resolve()); + child.once("close", () => resolve()); + }); + + expect(fs.existsSync(marker)).toBe(true); + fs.unlinkSync(marker); + + const safeEnv = sanitizeHostExecEnv({ + baseEnv, + overrides: { + GIT_SSH_COMMAND: exploitValue, + }, + }); + + await new Promise((resolve) => { + const child = spawn(gitPath, ["ls-remote", target], { env: safeEnv, stdio: "ignore" }); + child.once("error", () => resolve()); + child.once("close", () => resolve()); + }); + + expect(fs.existsSync(marker)).toBe(false); + }); +}); diff --git a/src/infra/host-env-security.ts b/src/infra/host-env-security.ts index 79ccd1f0a7a..56b30bd0818 100644 --- a/src/infra/host-env-security.ts +++ b/src/infra/host-env-security.ts @@ -5,6 +5,7 @@ const PORTABLE_ENV_VAR_KEY = /^[A-Za-z_][A-Za-z0-9_]*$/; type HostEnvSecurityPolicy = { blockedKeys: string[]; blockedOverrideKeys?: string[]; + blockedOverridePrefixes?: string[]; blockedPrefixes: string[]; }; @@ -19,6 +20,9 @@ export const HOST_DANGEROUS_ENV_PREFIXES: readonly string[] = Object.freeze( 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_OVERRIDE_ENV_PREFIXES: readonly string[] = Object.freeze( + (HOST_ENV_SECURITY_POLICY.blockedOverridePrefixes ?? []).map((prefix) => prefix.toUpperCase()), +); export const HOST_SHELL_WRAPPER_ALLOWED_OVERRIDE_ENV_KEY_VALUES: readonly string[] = Object.freeze([ "TERM", "LANG", @@ -68,7 +72,11 @@ export function isDangerousHostEnvOverrideVarName(rawKey: string): boolean { if (!key) { return false; } - return HOST_DANGEROUS_OVERRIDE_ENV_KEYS.has(key.toUpperCase()); + const upper = key.toUpperCase(); + if (HOST_DANGEROUS_OVERRIDE_ENV_KEYS.has(upper)) { + return true; + } + return HOST_DANGEROUS_OVERRIDE_ENV_PREFIXES.some((prefix) => upper.startsWith(prefix)); } export function sanitizeHostExecEnv(params?: {