mirror of
https://github.com/moltbot/moltbot.git
synced 2026-03-07 22:44:16 +00:00
fix(exec): block dangerous override-only env pivots
This commit is contained in:
@@ -615,6 +615,7 @@ Docs: https://docs.openclaw.ai
|
||||
- Models/provider config precedence: prefer exact `models.providers.<name>` 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.
|
||||
|
||||
@@ -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<String> = [
|
||||
"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
|
||||
}
|
||||
|
||||
@@ -27,7 +27,35 @@ enum HostEnvSecurityPolicy {
|
||||
|
||||
static let blockedOverrideKeys: Set<String> = [
|
||||
"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] = [
|
||||
|
||||
@@ -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)}
|
||||
]
|
||||
|
||||
@@ -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_"]
|
||||
}
|
||||
|
||||
@@ -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",
|
||||
);
|
||||
|
||||
@@ -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<void>((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<void>((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);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -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?: {
|
||||
|
||||
Reference in New Issue
Block a user