mirror of
https://github.com/openclaw/openclaw.git
synced 2026-03-21 06:51:01 +00:00
Exec: harden host env override handling across gateway and node (#51207)
* Exec: harden host env override enforcement and fail closed * Node host: enforce env override diagnostics before shell filtering * Env overrides: align Windows key handling and mac node rejection
This commit is contained in:
@@ -153,6 +153,7 @@ Docs: https://docs.openclaw.ai
|
||||
- Hardening: refresh stale device pairing requests and pending metadata (#50695) Thanks @smaeljaish771 and @joshavant.
|
||||
- Gateway: harden OpenResponses file-context escaping (#50782) Thanks @YLChen-007 and @joshavant.
|
||||
- LINE: harden Express webhook parsing to verified raw body (#51202) Thanks @gladiator9797 and @joshavant.
|
||||
- Exec: harden host env override handling across gateway and node (#51207) Thanks @gladiator9797 and @joshavant.
|
||||
- xAI/models: rename the bundled Grok 4.20 catalog entries to the GA IDs and normalize saved deprecated beta IDs at runtime so existing configs and sessions keep resolving. (#50772) thanks @Jaaneek
|
||||
|
||||
### Fixes
|
||||
|
||||
@@ -1,5 +1,10 @@
|
||||
import Foundation
|
||||
|
||||
struct HostEnvOverrideDiagnostics: Equatable {
|
||||
var blockedKeys: [String]
|
||||
var invalidKeys: [String]
|
||||
}
|
||||
|
||||
enum HostEnvSanitizer {
|
||||
/// Generated from src/infra/host-env-security-policy.json via scripts/generate-host-env-security-policy-swift.mjs.
|
||||
/// Parity is validated by src/infra/host-env-security.policy-parity.test.ts.
|
||||
@@ -41,6 +46,67 @@ enum HostEnvSanitizer {
|
||||
return filtered.isEmpty ? nil : filtered
|
||||
}
|
||||
|
||||
private static func isPortableHead(_ scalar: UnicodeScalar) -> Bool {
|
||||
let value = scalar.value
|
||||
return value == 95 || (65...90).contains(value) || (97...122).contains(value)
|
||||
}
|
||||
|
||||
private static func isPortableTail(_ scalar: UnicodeScalar) -> Bool {
|
||||
let value = scalar.value
|
||||
return self.isPortableHead(scalar) || (48...57).contains(value)
|
||||
}
|
||||
|
||||
private static func normalizeOverrideKey(_ rawKey: String) -> String? {
|
||||
let key = rawKey.trimmingCharacters(in: .whitespacesAndNewlines)
|
||||
guard !key.isEmpty else { return nil }
|
||||
guard let first = key.unicodeScalars.first, self.isPortableHead(first) else {
|
||||
return nil
|
||||
}
|
||||
for scalar in key.unicodeScalars.dropFirst() {
|
||||
if self.isPortableTail(scalar) || scalar == "(" || scalar == ")" {
|
||||
continue
|
||||
}
|
||||
return nil
|
||||
}
|
||||
return key
|
||||
}
|
||||
|
||||
private static func sortedUnique(_ values: [String]) -> [String] {
|
||||
Array(Set(values)).sorted()
|
||||
}
|
||||
|
||||
static func inspectOverrides(
|
||||
overrides: [String: String]?,
|
||||
blockPathOverrides: Bool = true) -> HostEnvOverrideDiagnostics
|
||||
{
|
||||
guard let overrides else {
|
||||
return HostEnvOverrideDiagnostics(blockedKeys: [], invalidKeys: [])
|
||||
}
|
||||
|
||||
var blocked: [String] = []
|
||||
var invalid: [String] = []
|
||||
for (rawKey, _) in overrides {
|
||||
let candidate = rawKey.trimmingCharacters(in: .whitespacesAndNewlines)
|
||||
guard let normalized = self.normalizeOverrideKey(rawKey) else {
|
||||
invalid.append(candidate.isEmpty ? rawKey : candidate)
|
||||
continue
|
||||
}
|
||||
let upper = normalized.uppercased()
|
||||
if blockPathOverrides, upper == "PATH" {
|
||||
blocked.append(upper)
|
||||
continue
|
||||
}
|
||||
if self.isBlockedOverride(upper) || self.isBlocked(upper) {
|
||||
blocked.append(upper)
|
||||
continue
|
||||
}
|
||||
}
|
||||
|
||||
return HostEnvOverrideDiagnostics(
|
||||
blockedKeys: self.sortedUnique(blocked),
|
||||
invalidKeys: self.sortedUnique(invalid))
|
||||
}
|
||||
|
||||
static func sanitize(overrides: [String: String]?, shellWrapper: Bool = false) -> [String: String] {
|
||||
var merged: [String: String] = [:]
|
||||
for (rawKey, value) in ProcessInfo.processInfo.environment {
|
||||
@@ -57,8 +123,7 @@ enum HostEnvSanitizer {
|
||||
|
||||
guard let effectiveOverrides else { return merged }
|
||||
for (rawKey, value) in effectiveOverrides {
|
||||
let key = rawKey.trimmingCharacters(in: .whitespacesAndNewlines)
|
||||
guard !key.isEmpty else { continue }
|
||||
guard let key = self.normalizeOverrideKey(rawKey) else { continue }
|
||||
let upper = key.uppercased()
|
||||
// PATH is part of the security boundary (command resolution + safe-bin checks). Never
|
||||
// allow request-scoped PATH overrides from agents/gateways.
|
||||
|
||||
@@ -63,7 +63,23 @@ enum HostEnvSecurityPolicy {
|
||||
"OPENSSL_ENGINES",
|
||||
"PYTHONSTARTUP",
|
||||
"WGETRC",
|
||||
"CURL_HOME"
|
||||
"CURL_HOME",
|
||||
"CLASSPATH",
|
||||
"CGO_CFLAGS",
|
||||
"CGO_LDFLAGS",
|
||||
"GOFLAGS",
|
||||
"CORECLR_PROFILER_PATH",
|
||||
"PHPRC",
|
||||
"PHP_INI_SCAN_DIR",
|
||||
"DENO_DIR",
|
||||
"BUN_CONFIG_REGISTRY",
|
||||
"LUA_PATH",
|
||||
"LUA_CPATH",
|
||||
"GEM_HOME",
|
||||
"GEM_PATH",
|
||||
"BUNDLE_GEMFILE",
|
||||
"COMPOSER_HOME",
|
||||
"XDG_CONFIG_HOME"
|
||||
]
|
||||
|
||||
static let blockedOverridePrefixes: [String] = [
|
||||
|
||||
@@ -465,6 +465,23 @@ actor MacNodeRuntime {
|
||||
? params.sessionKey!.trimmingCharacters(in: .whitespacesAndNewlines)
|
||||
: self.mainSessionKey
|
||||
let runId = UUID().uuidString
|
||||
let envOverrideDiagnostics = HostEnvSanitizer.inspectOverrides(
|
||||
overrides: params.env,
|
||||
blockPathOverrides: true)
|
||||
if !envOverrideDiagnostics.blockedKeys.isEmpty || !envOverrideDiagnostics.invalidKeys.isEmpty {
|
||||
var details: [String] = []
|
||||
if !envOverrideDiagnostics.blockedKeys.isEmpty {
|
||||
details.append("blocked override keys: \(envOverrideDiagnostics.blockedKeys.joined(separator: ", "))")
|
||||
}
|
||||
if !envOverrideDiagnostics.invalidKeys.isEmpty {
|
||||
details.append(
|
||||
"invalid non-portable override keys: \(envOverrideDiagnostics.invalidKeys.joined(separator: ", "))")
|
||||
}
|
||||
return Self.errorResponse(
|
||||
req,
|
||||
code: .invalidRequest,
|
||||
message: "SYSTEM_RUN_DENIED: environment override rejected (\(details.joined(separator: "; ")))")
|
||||
}
|
||||
let evaluation = await ExecApprovalEvaluator.evaluate(
|
||||
command: command,
|
||||
rawCommand: params.rawCommand,
|
||||
|
||||
@@ -33,4 +33,24 @@ struct HostEnvSanitizerTests {
|
||||
let env = HostEnvSanitizer.sanitize(overrides: ["OPENCLAW_TOKEN": "secret"])
|
||||
#expect(env["OPENCLAW_TOKEN"] == "secret")
|
||||
}
|
||||
|
||||
@Test func `inspect overrides rejects blocked and invalid keys`() {
|
||||
let diagnostics = HostEnvSanitizer.inspectOverrides(overrides: [
|
||||
"CLASSPATH": "/tmp/evil-classpath",
|
||||
"BAD-KEY": "x",
|
||||
"ProgramFiles(x86)": "C:\\Program Files (x86)",
|
||||
])
|
||||
|
||||
#expect(diagnostics.blockedKeys == ["CLASSPATH"])
|
||||
#expect(diagnostics.invalidKeys == ["BAD-KEY"])
|
||||
}
|
||||
|
||||
@Test func `sanitize accepts Windows-style override key names`() {
|
||||
let env = HostEnvSanitizer.sanitize(overrides: [
|
||||
"ProgramFiles(x86)": "D:\\SDKs",
|
||||
"CommonProgramFiles(x86)": "D:\\Common",
|
||||
])
|
||||
#expect(env["ProgramFiles(x86)"] == "D:\\SDKs")
|
||||
#expect(env["CommonProgramFiles(x86)"] == "D:\\Common")
|
||||
}
|
||||
}
|
||||
|
||||
@@ -21,6 +21,32 @@ struct MacNodeRuntimeTests {
|
||||
#expect(response.ok == false)
|
||||
}
|
||||
|
||||
@Test func `handle invoke rejects blocked system run env override before execution`() async throws {
|
||||
let runtime = MacNodeRuntime()
|
||||
let params = OpenClawSystemRunParams(
|
||||
command: ["/bin/sh", "-lc", "echo ok"],
|
||||
env: ["CLASSPATH": "/tmp/evil-classpath"])
|
||||
let json = try String(data: JSONEncoder().encode(params), encoding: .utf8)
|
||||
let response = await runtime.handleInvoke(
|
||||
BridgeInvokeRequest(id: "req-2c", command: OpenClawSystemCommand.run.rawValue, paramsJSON: json))
|
||||
#expect(response.ok == false)
|
||||
#expect(response.error?.message.contains("SYSTEM_RUN_DENIED: environment override rejected") == true)
|
||||
#expect(response.error?.message.contains("CLASSPATH") == true)
|
||||
}
|
||||
|
||||
@Test func `handle invoke rejects invalid system run env override key before execution`() async throws {
|
||||
let runtime = MacNodeRuntime()
|
||||
let params = OpenClawSystemRunParams(
|
||||
command: ["/bin/sh", "-lc", "echo ok"],
|
||||
env: ["BAD-KEY": "x"])
|
||||
let json = try String(data: JSONEncoder().encode(params), encoding: .utf8)
|
||||
let response = await runtime.handleInvoke(
|
||||
BridgeInvokeRequest(id: "req-2d", command: OpenClawSystemCommand.run.rawValue, paramsJSON: json))
|
||||
#expect(response.ok == false)
|
||||
#expect(response.error?.message.contains("SYSTEM_RUN_DENIED: environment override rejected") == true)
|
||||
#expect(response.error?.message.contains("BAD-KEY") == true)
|
||||
}
|
||||
|
||||
@Test func `handle invoke rejects empty system which`() async throws {
|
||||
let runtime = MacNodeRuntime()
|
||||
let params = OpenClawSystemWhichParams(bins: [])
|
||||
|
||||
@@ -130,6 +130,22 @@ describe("exec PATH login shell merge", () => {
|
||||
expect(shellPathMock).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("fails closed when a blocked runtime override key is requested", async () => {
|
||||
if (isWin) {
|
||||
return;
|
||||
}
|
||||
const tool = createExecTool({ host: "gateway", security: "full", ask: "off" });
|
||||
|
||||
await expect(
|
||||
tool.execute("call-blocked-runtime-env", {
|
||||
command: "echo ok",
|
||||
env: { CLASSPATH: "/tmp/evil-classpath" },
|
||||
}),
|
||||
).rejects.toThrow(
|
||||
/Security Violation: Environment variable 'CLASSPATH' is forbidden during host execution\./,
|
||||
);
|
||||
});
|
||||
|
||||
it("does not apply login-shell PATH when probe rejects unregistered absolute SHELL", async () => {
|
||||
if (isWin) {
|
||||
return;
|
||||
|
||||
@@ -3,6 +3,7 @@ import path from "node:path";
|
||||
import type { AgentTool, AgentToolResult } from "@mariozechner/pi-agent-core";
|
||||
import { type ExecHost, loadExecApprovals, maxAsk, minSecurity } from "../infra/exec-approvals.js";
|
||||
import { resolveExecSafeBinRuntimePolicy } from "../infra/exec-safe-bin-runtime-policy.js";
|
||||
import { sanitizeHostExecEnvWithDiagnostics } from "../infra/host-env-security.js";
|
||||
import {
|
||||
getShellPathFromLoginShell,
|
||||
resolveShellEnvFallbackTimeoutMs,
|
||||
@@ -25,9 +26,7 @@ import {
|
||||
renderExecHostLabel,
|
||||
resolveApprovalRunningNoticeMs,
|
||||
runExecProcess,
|
||||
sanitizeHostBaseEnv,
|
||||
execSchema,
|
||||
validateHostEnv,
|
||||
} from "./bash-tools.exec-runtime.js";
|
||||
import type {
|
||||
ExecElevatedDefaults,
|
||||
@@ -362,24 +361,58 @@ export function createExecTool(
|
||||
}
|
||||
|
||||
const inheritedBaseEnv = coerceEnv(process.env);
|
||||
const baseEnv = host === "sandbox" ? inheritedBaseEnv : sanitizeHostBaseEnv(inheritedBaseEnv);
|
||||
|
||||
// Logic: Sandbox gets raw env. Host (gateway/node) must pass validation.
|
||||
// We validate BEFORE merging to prevent any dangerous vars from entering the stream.
|
||||
if (host !== "sandbox" && params.env) {
|
||||
validateHostEnv(params.env);
|
||||
const hostEnvResult =
|
||||
host === "sandbox"
|
||||
? null
|
||||
: sanitizeHostExecEnvWithDiagnostics({
|
||||
baseEnv: inheritedBaseEnv,
|
||||
overrides: params.env,
|
||||
blockPathOverrides: true,
|
||||
});
|
||||
if (
|
||||
hostEnvResult &&
|
||||
params.env &&
|
||||
(hostEnvResult.rejectedOverrideBlockedKeys.length > 0 ||
|
||||
hostEnvResult.rejectedOverrideInvalidKeys.length > 0)
|
||||
) {
|
||||
const blockedKeys = hostEnvResult.rejectedOverrideBlockedKeys;
|
||||
const invalidKeys = hostEnvResult.rejectedOverrideInvalidKeys;
|
||||
const pathBlocked = blockedKeys.includes("PATH");
|
||||
if (pathBlocked && blockedKeys.length === 1 && invalidKeys.length === 0) {
|
||||
throw new Error(
|
||||
"Security Violation: Custom 'PATH' variable is forbidden during host execution.",
|
||||
);
|
||||
}
|
||||
if (blockedKeys.length === 1 && invalidKeys.length === 0) {
|
||||
throw new Error(
|
||||
`Security Violation: Environment variable '${blockedKeys[0]}' is forbidden during host execution.`,
|
||||
);
|
||||
}
|
||||
const details: string[] = [];
|
||||
if (blockedKeys.length > 0) {
|
||||
details.push(`blocked override keys: ${blockedKeys.join(", ")}`);
|
||||
}
|
||||
if (invalidKeys.length > 0) {
|
||||
details.push(`invalid non-portable override keys: ${invalidKeys.join(", ")}`);
|
||||
}
|
||||
const suffix = details.join("; ");
|
||||
if (pathBlocked) {
|
||||
throw new Error(
|
||||
`Security Violation: Custom 'PATH' variable is forbidden during host execution (${suffix}).`,
|
||||
);
|
||||
}
|
||||
throw new Error(`Security Violation: ${suffix}.`);
|
||||
}
|
||||
|
||||
const mergedEnv = params.env ? { ...baseEnv, ...params.env } : baseEnv;
|
||||
|
||||
const env = sandbox
|
||||
? buildSandboxEnv({
|
||||
defaultPath: DEFAULT_PATH,
|
||||
paramsEnv: params.env,
|
||||
sandboxEnv: sandbox.env,
|
||||
containerWorkdir: containerWorkdir ?? sandbox.containerWorkdir,
|
||||
})
|
||||
: mergedEnv;
|
||||
const env =
|
||||
sandbox && host === "sandbox"
|
||||
? buildSandboxEnv({
|
||||
defaultPath: DEFAULT_PATH,
|
||||
paramsEnv: params.env,
|
||||
sandboxEnv: sandbox.env,
|
||||
containerWorkdir: containerWorkdir ?? sandbox.containerWorkdir,
|
||||
})
|
||||
: (hostEnvResult?.env ?? inheritedBaseEnv);
|
||||
|
||||
if (!sandbox && host === "gateway" && !params.env?.PATH) {
|
||||
const shellPath = getShellPathFromLoginShell({
|
||||
|
||||
@@ -56,7 +56,23 @@
|
||||
"OPENSSL_ENGINES",
|
||||
"PYTHONSTARTUP",
|
||||
"WGETRC",
|
||||
"CURL_HOME"
|
||||
"CURL_HOME",
|
||||
"CLASSPATH",
|
||||
"CGO_CFLAGS",
|
||||
"CGO_LDFLAGS",
|
||||
"GOFLAGS",
|
||||
"CORECLR_PROFILER_PATH",
|
||||
"PHPRC",
|
||||
"PHP_INI_SCAN_DIR",
|
||||
"DENO_DIR",
|
||||
"BUN_CONFIG_REGISTRY",
|
||||
"LUA_PATH",
|
||||
"LUA_CPATH",
|
||||
"GEM_HOME",
|
||||
"GEM_PATH",
|
||||
"BUNDLE_GEMFILE",
|
||||
"COMPOSER_HOME",
|
||||
"XDG_CONFIG_HOME"
|
||||
],
|
||||
"blockedOverridePrefixes": ["GIT_CONFIG_", "NPM_CONFIG_"],
|
||||
"blockedPrefixes": ["DYLD_", "LD_", "BASH_FUNC_"]
|
||||
|
||||
@@ -8,6 +8,7 @@ import {
|
||||
isDangerousHostEnvVarName,
|
||||
normalizeEnvVarKey,
|
||||
sanitizeHostExecEnv,
|
||||
sanitizeHostExecEnvWithDiagnostics,
|
||||
sanitizeSystemRunEnvOverrides,
|
||||
} from "./host-env-security.js";
|
||||
import { OPENCLAW_CLI_ENV_VALUE } from "./openclaw-exec-env.js";
|
||||
@@ -114,6 +115,10 @@ describe("sanitizeHostExecEnv", () => {
|
||||
GIT_CONFIG_GLOBAL: "/tmp/gitconfig",
|
||||
SHELLOPTS: "xtrace",
|
||||
PS4: "$(touch /tmp/pwned)",
|
||||
CLASSPATH: "/tmp/evil-classpath",
|
||||
GOFLAGS: "-mod=mod",
|
||||
PHPRC: "/tmp/evil-php.ini",
|
||||
XDG_CONFIG_HOME: "/tmp/evil-config",
|
||||
SAFE: "ok",
|
||||
},
|
||||
});
|
||||
@@ -128,6 +133,10 @@ describe("sanitizeHostExecEnv", () => {
|
||||
expect(env.GIT_CONFIG_GLOBAL).toBeUndefined();
|
||||
expect(env.SHELLOPTS).toBeUndefined();
|
||||
expect(env.PS4).toBeUndefined();
|
||||
expect(env.CLASSPATH).toBeUndefined();
|
||||
expect(env.GOFLAGS).toBeUndefined();
|
||||
expect(env.PHPRC).toBeUndefined();
|
||||
expect(env.XDG_CONFIG_HOME).toBeUndefined();
|
||||
expect(env.SAFE).toBe("ok");
|
||||
expect(env.HOME).toBe("/tmp/trusted-home");
|
||||
expect(env.ZDOTDIR).toBe("/tmp/trusted-zdotdir");
|
||||
@@ -183,7 +192,7 @@ describe("sanitizeHostExecEnv", () => {
|
||||
expect(env.OPENCLAW_CLI).toBe(OPENCLAW_CLI_ENV_VALUE);
|
||||
});
|
||||
|
||||
it("drops non-string inherited values and non-portable inherited keys", () => {
|
||||
it("drops non-string inherited values while preserving non-portable inherited keys", () => {
|
||||
const env = sanitizeHostExecEnv({
|
||||
baseEnv: {
|
||||
PATH: "/usr/bin:/bin",
|
||||
@@ -191,6 +200,7 @@ describe("sanitizeHostExecEnv", () => {
|
||||
// oxlint-disable-next-line typescript/no-explicit-any
|
||||
BAD_NUMBER: 1 as any,
|
||||
"NOT-PORTABLE": "x",
|
||||
"ProgramFiles(x86)": "C:\\Program Files (x86)",
|
||||
},
|
||||
});
|
||||
|
||||
@@ -198,6 +208,8 @@ describe("sanitizeHostExecEnv", () => {
|
||||
OPENCLAW_CLI: OPENCLAW_CLI_ENV_VALUE,
|
||||
PATH: "/usr/bin:/bin",
|
||||
GOOD: "1",
|
||||
"NOT-PORTABLE": "x",
|
||||
"ProgramFiles(x86)": "C:\\Program Files (x86)",
|
||||
});
|
||||
});
|
||||
});
|
||||
@@ -212,11 +224,58 @@ describe("isDangerousHostEnvOverrideVarName", () => {
|
||||
expect(isDangerousHostEnvOverrideVarName("git_config_global")).toBe(true);
|
||||
expect(isDangerousHostEnvOverrideVarName("GRADLE_USER_HOME")).toBe(true);
|
||||
expect(isDangerousHostEnvOverrideVarName("gradle_user_home")).toBe(true);
|
||||
expect(isDangerousHostEnvOverrideVarName("CLASSPATH")).toBe(true);
|
||||
expect(isDangerousHostEnvOverrideVarName("classpath")).toBe(true);
|
||||
expect(isDangerousHostEnvOverrideVarName("GOFLAGS")).toBe(true);
|
||||
expect(isDangerousHostEnvOverrideVarName("goflags")).toBe(true);
|
||||
expect(isDangerousHostEnvOverrideVarName("CORECLR_PROFILER_PATH")).toBe(true);
|
||||
expect(isDangerousHostEnvOverrideVarName("coreclr_profiler_path")).toBe(true);
|
||||
expect(isDangerousHostEnvOverrideVarName("XDG_CONFIG_HOME")).toBe(true);
|
||||
expect(isDangerousHostEnvOverrideVarName("xdg_config_home")).toBe(true);
|
||||
expect(isDangerousHostEnvOverrideVarName("BASH_ENV")).toBe(false);
|
||||
expect(isDangerousHostEnvOverrideVarName("FOO")).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
describe("sanitizeHostExecEnvWithDiagnostics", () => {
|
||||
it("reports blocked and invalid requested overrides", () => {
|
||||
const result = sanitizeHostExecEnvWithDiagnostics({
|
||||
baseEnv: {
|
||||
PATH: "/usr/bin:/bin",
|
||||
},
|
||||
overrides: {
|
||||
PATH: "/tmp/evil",
|
||||
CLASSPATH: "/tmp/evil-classpath",
|
||||
SAFE_KEY: "ok",
|
||||
"BAD-KEY": "bad",
|
||||
},
|
||||
});
|
||||
|
||||
expect(result.rejectedOverrideBlockedKeys).toEqual(["CLASSPATH", "PATH"]);
|
||||
expect(result.rejectedOverrideInvalidKeys).toEqual(["BAD-KEY"]);
|
||||
expect(result.env.SAFE_KEY).toBe("ok");
|
||||
expect(result.env.PATH).toBe("/usr/bin:/bin");
|
||||
expect(result.env.CLASSPATH).toBeUndefined();
|
||||
});
|
||||
|
||||
it("allows Windows-style override names while still rejecting invalid keys", () => {
|
||||
const result = sanitizeHostExecEnvWithDiagnostics({
|
||||
baseEnv: {
|
||||
PATH: "/usr/bin:/bin",
|
||||
"ProgramFiles(x86)": "C:\\Program Files (x86)",
|
||||
},
|
||||
overrides: {
|
||||
"ProgramFiles(x86)": "D:\\SDKs",
|
||||
"BAD-KEY": "bad",
|
||||
},
|
||||
});
|
||||
|
||||
expect(result.rejectedOverrideBlockedKeys).toEqual([]);
|
||||
expect(result.rejectedOverrideInvalidKeys).toEqual(["BAD-KEY"]);
|
||||
expect(result.env["ProgramFiles(x86)"]).toBe("D:\\SDKs");
|
||||
});
|
||||
});
|
||||
|
||||
describe("normalizeEnvVarKey", () => {
|
||||
it("normalizes and validates keys", () => {
|
||||
expect(normalizeEnvVarKey(" OPENROUTER_API_KEY ")).toBe("OPENROUTER_API_KEY");
|
||||
|
||||
@@ -2,6 +2,7 @@ import HOST_ENV_SECURITY_POLICY_JSON from "./host-env-security-policy.json" with
|
||||
import { markOpenClawExecEnv } from "./openclaw-exec-env.js";
|
||||
|
||||
const PORTABLE_ENV_VAR_KEY = /^[A-Za-z_][A-Za-z0-9_]*$/;
|
||||
const WINDOWS_COMPAT_OVERRIDE_ENV_VAR_KEY = /^[A-Za-z_][A-Za-z0-9_()]*$/;
|
||||
|
||||
type HostEnvSecurityPolicy = {
|
||||
blockedKeys: string[];
|
||||
@@ -42,6 +43,17 @@ export const HOST_SHELL_WRAPPER_ALLOWED_OVERRIDE_ENV_KEYS = new Set<string>(
|
||||
HOST_SHELL_WRAPPER_ALLOWED_OVERRIDE_ENV_KEY_VALUES,
|
||||
);
|
||||
|
||||
export type HostExecEnvSanitizationResult = {
|
||||
env: Record<string, string>;
|
||||
rejectedOverrideBlockedKeys: string[];
|
||||
rejectedOverrideInvalidKeys: string[];
|
||||
};
|
||||
|
||||
export type HostExecEnvOverrideDiagnostics = {
|
||||
rejectedOverrideBlockedKeys: string[];
|
||||
rejectedOverrideInvalidKeys: string[];
|
||||
};
|
||||
|
||||
export function normalizeEnvVarKey(
|
||||
rawKey: string,
|
||||
options?: { portable?: boolean },
|
||||
@@ -56,6 +68,17 @@ export function normalizeEnvVarKey(
|
||||
return key;
|
||||
}
|
||||
|
||||
function normalizeHostOverrideEnvVarKey(rawKey: string): string | null {
|
||||
const key = normalizeEnvVarKey(rawKey);
|
||||
if (!key) {
|
||||
return null;
|
||||
}
|
||||
if (PORTABLE_ENV_VAR_KEY.test(key) || WINDOWS_COMPAT_OVERRIDE_ENV_VAR_KEY.test(key)) {
|
||||
return key;
|
||||
}
|
||||
return null;
|
||||
}
|
||||
|
||||
export function isDangerousHostEnvVarName(rawKey: string): boolean {
|
||||
const key = normalizeEnvVarKey(rawKey);
|
||||
if (!key) {
|
||||
@@ -80,15 +103,16 @@ export function isDangerousHostEnvOverrideVarName(rawKey: string): boolean {
|
||||
return HOST_DANGEROUS_OVERRIDE_ENV_PREFIXES.some((prefix) => upper.startsWith(prefix));
|
||||
}
|
||||
|
||||
function listNormalizedPortableEnvEntries(
|
||||
function listNormalizedEnvEntries(
|
||||
source: Record<string, string | undefined>,
|
||||
options?: { portable?: boolean },
|
||||
): Array<[string, string]> {
|
||||
const entries: Array<[string, string]> = [];
|
||||
for (const [rawKey, value] of Object.entries(source)) {
|
||||
if (typeof value !== "string") {
|
||||
continue;
|
||||
}
|
||||
const key = normalizeEnvVarKey(rawKey, { portable: true });
|
||||
const key = normalizeEnvVarKey(rawKey, options);
|
||||
if (!key) {
|
||||
continue;
|
||||
}
|
||||
@@ -97,41 +121,112 @@ function listNormalizedPortableEnvEntries(
|
||||
return entries;
|
||||
}
|
||||
|
||||
export function sanitizeHostExecEnv(params?: {
|
||||
function sortUnique(values: Iterable<string>): string[] {
|
||||
return Array.from(new Set(values)).toSorted((a, b) => a.localeCompare(b));
|
||||
}
|
||||
|
||||
function sanitizeHostEnvOverridesWithDiagnostics(params?: {
|
||||
overrides?: Record<string, string> | null;
|
||||
blockPathOverrides?: boolean;
|
||||
}): {
|
||||
acceptedOverrides?: Record<string, string>;
|
||||
rejectedOverrideBlockedKeys: string[];
|
||||
rejectedOverrideInvalidKeys: string[];
|
||||
} {
|
||||
const overrides = params?.overrides ?? undefined;
|
||||
if (!overrides) {
|
||||
return {
|
||||
acceptedOverrides: undefined,
|
||||
rejectedOverrideBlockedKeys: [],
|
||||
rejectedOverrideInvalidKeys: [],
|
||||
};
|
||||
}
|
||||
|
||||
const blockPathOverrides = params?.blockPathOverrides ?? true;
|
||||
const acceptedOverrides: Record<string, string> = {};
|
||||
const rejectedBlocked: string[] = [];
|
||||
const rejectedInvalid: string[] = [];
|
||||
|
||||
for (const [rawKey, value] of Object.entries(overrides)) {
|
||||
if (typeof value !== "string") {
|
||||
continue;
|
||||
}
|
||||
const normalized = normalizeHostOverrideEnvVarKey(rawKey);
|
||||
if (!normalized) {
|
||||
const candidate = rawKey.trim();
|
||||
rejectedInvalid.push(candidate || rawKey);
|
||||
continue;
|
||||
}
|
||||
const upper = normalized.toUpperCase();
|
||||
// PATH is part of the security boundary (command resolution + safe-bin checks). Never allow
|
||||
// request-scoped PATH overrides from agents/gateways.
|
||||
if (blockPathOverrides && upper === "PATH") {
|
||||
rejectedBlocked.push(upper);
|
||||
continue;
|
||||
}
|
||||
if (isDangerousHostEnvVarName(upper) || isDangerousHostEnvOverrideVarName(upper)) {
|
||||
rejectedBlocked.push(upper);
|
||||
continue;
|
||||
}
|
||||
acceptedOverrides[normalized] = value;
|
||||
}
|
||||
|
||||
return {
|
||||
acceptedOverrides,
|
||||
rejectedOverrideBlockedKeys: sortUnique(rejectedBlocked),
|
||||
rejectedOverrideInvalidKeys: sortUnique(rejectedInvalid),
|
||||
};
|
||||
}
|
||||
|
||||
export function sanitizeHostExecEnvWithDiagnostics(params?: {
|
||||
baseEnv?: Record<string, string | undefined>;
|
||||
overrides?: Record<string, string> | null;
|
||||
blockPathOverrides?: boolean;
|
||||
}): Record<string, string> {
|
||||
}): HostExecEnvSanitizationResult {
|
||||
const baseEnv = params?.baseEnv ?? process.env;
|
||||
const overrides = params?.overrides ?? undefined;
|
||||
const blockPathOverrides = params?.blockPathOverrides ?? true;
|
||||
|
||||
const merged: Record<string, string> = {};
|
||||
for (const [key, value] of listNormalizedPortableEnvEntries(baseEnv)) {
|
||||
for (const [key, value] of listNormalizedEnvEntries(baseEnv)) {
|
||||
if (isDangerousHostEnvVarName(key)) {
|
||||
continue;
|
||||
}
|
||||
merged[key] = value;
|
||||
}
|
||||
|
||||
if (!overrides) {
|
||||
return markOpenClawExecEnv(merged);
|
||||
const overrideResult = sanitizeHostEnvOverridesWithDiagnostics({
|
||||
overrides: params?.overrides ?? undefined,
|
||||
blockPathOverrides: params?.blockPathOverrides ?? true,
|
||||
});
|
||||
if (overrideResult.acceptedOverrides) {
|
||||
for (const [key, value] of Object.entries(overrideResult.acceptedOverrides)) {
|
||||
merged[key] = value;
|
||||
}
|
||||
}
|
||||
|
||||
for (const [key, value] of listNormalizedPortableEnvEntries(overrides)) {
|
||||
const upper = key.toUpperCase();
|
||||
// PATH is part of the security boundary (command resolution + safe-bin checks). Never allow
|
||||
// request-scoped PATH overrides from agents/gateways.
|
||||
if (blockPathOverrides && upper === "PATH") {
|
||||
continue;
|
||||
}
|
||||
if (isDangerousHostEnvVarName(upper) || isDangerousHostEnvOverrideVarName(upper)) {
|
||||
continue;
|
||||
}
|
||||
merged[key] = value;
|
||||
}
|
||||
return {
|
||||
env: markOpenClawExecEnv(merged),
|
||||
rejectedOverrideBlockedKeys: overrideResult.rejectedOverrideBlockedKeys,
|
||||
rejectedOverrideInvalidKeys: overrideResult.rejectedOverrideInvalidKeys,
|
||||
};
|
||||
}
|
||||
|
||||
return markOpenClawExecEnv(merged);
|
||||
export function inspectHostExecEnvOverrides(params?: {
|
||||
overrides?: Record<string, string> | null;
|
||||
blockPathOverrides?: boolean;
|
||||
}): HostExecEnvOverrideDiagnostics {
|
||||
const result = sanitizeHostEnvOverridesWithDiagnostics(params);
|
||||
return {
|
||||
rejectedOverrideBlockedKeys: result.rejectedOverrideBlockedKeys,
|
||||
rejectedOverrideInvalidKeys: result.rejectedOverrideInvalidKeys,
|
||||
};
|
||||
}
|
||||
|
||||
export function sanitizeHostExecEnv(params?: {
|
||||
baseEnv?: Record<string, string | undefined>;
|
||||
overrides?: Record<string, string> | null;
|
||||
blockPathOverrides?: boolean;
|
||||
}): Record<string, string> {
|
||||
return sanitizeHostExecEnvWithDiagnostics(params).env;
|
||||
}
|
||||
|
||||
export function sanitizeSystemRunEnvOverrides(params?: {
|
||||
@@ -146,7 +241,7 @@ export function sanitizeSystemRunEnvOverrides(params?: {
|
||||
return overrides;
|
||||
}
|
||||
const filtered: Record<string, string> = {};
|
||||
for (const [key, value] of listNormalizedPortableEnvEntries(overrides)) {
|
||||
for (const [key, value] of listNormalizedEnvEntries(overrides, { portable: true })) {
|
||||
if (!HOST_SHELL_WRAPPER_ALLOWED_OVERRIDE_ENV_KEYS.has(key.toUpperCase())) {
|
||||
continue;
|
||||
}
|
||||
|
||||
@@ -336,6 +336,7 @@ describe("handleSystemRunInvoke mac app exec host routing", () => {
|
||||
preferMacAppExecHost: boolean;
|
||||
runViaResponse?: ExecHostResponse | null;
|
||||
command?: string[];
|
||||
env?: Record<string, string>;
|
||||
rawCommand?: string | null;
|
||||
systemRunPlan?: SystemRunApprovalPlan | null;
|
||||
cwd?: string;
|
||||
@@ -391,6 +392,7 @@ describe("handleSystemRunInvoke mac app exec host routing", () => {
|
||||
client: {} as never,
|
||||
params: {
|
||||
command: params.command ?? ["echo", "ok"],
|
||||
env: params.env,
|
||||
rawCommand: params.rawCommand,
|
||||
systemRunPlan: params.systemRunPlan,
|
||||
cwd: params.cwd,
|
||||
@@ -1106,6 +1108,65 @@ describe("handleSystemRunInvoke mac app exec host routing", () => {
|
||||
expectApprovalRequiredDenied({ sendNodeEvent, sendInvokeResult });
|
||||
});
|
||||
|
||||
it("rejects blocked environment overrides before execution", async () => {
|
||||
const { runCommand, sendInvokeResult } = await runSystemInvoke({
|
||||
preferMacAppExecHost: false,
|
||||
security: "full",
|
||||
ask: "off",
|
||||
env: { CLASSPATH: "/tmp/evil-classpath" },
|
||||
});
|
||||
|
||||
expect(runCommand).not.toHaveBeenCalled();
|
||||
expectInvokeErrorMessage(sendInvokeResult, {
|
||||
message: "SYSTEM_RUN_DENIED: environment override rejected",
|
||||
});
|
||||
expectInvokeErrorMessage(sendInvokeResult, {
|
||||
message: "CLASSPATH",
|
||||
});
|
||||
});
|
||||
|
||||
it("rejects blocked environment overrides for shell-wrapper commands", async () => {
|
||||
const shellCommand =
|
||||
process.platform === "win32"
|
||||
? ["cmd.exe", "/d", "/s", "/c", "echo ok"]
|
||||
: ["/bin/sh", "-lc", "echo ok"];
|
||||
const { runCommand, sendInvokeResult } = await runSystemInvoke({
|
||||
preferMacAppExecHost: false,
|
||||
security: "full",
|
||||
ask: "off",
|
||||
command: shellCommand,
|
||||
env: {
|
||||
CLASSPATH: "/tmp/evil-classpath",
|
||||
LANG: "C",
|
||||
},
|
||||
});
|
||||
|
||||
expect(runCommand).not.toHaveBeenCalled();
|
||||
expectInvokeErrorMessage(sendInvokeResult, {
|
||||
message: "SYSTEM_RUN_DENIED: environment override rejected",
|
||||
});
|
||||
expectInvokeErrorMessage(sendInvokeResult, {
|
||||
message: "CLASSPATH",
|
||||
});
|
||||
});
|
||||
|
||||
it("rejects invalid non-portable environment override keys before execution", async () => {
|
||||
const { runCommand, sendInvokeResult } = await runSystemInvoke({
|
||||
preferMacAppExecHost: false,
|
||||
security: "full",
|
||||
ask: "off",
|
||||
env: { "BAD-KEY": "x" },
|
||||
});
|
||||
|
||||
expect(runCommand).not.toHaveBeenCalled();
|
||||
expectInvokeErrorMessage(sendInvokeResult, {
|
||||
message: "SYSTEM_RUN_DENIED: environment override rejected",
|
||||
});
|
||||
expectInvokeErrorMessage(sendInvokeResult, {
|
||||
message: "BAD-KEY",
|
||||
});
|
||||
});
|
||||
|
||||
async function expectNestedEnvShellDenied(params: {
|
||||
depth: number;
|
||||
markerName: string;
|
||||
|
||||
@@ -14,7 +14,10 @@ import {
|
||||
} from "../infra/exec-approvals.js";
|
||||
import type { ExecHostRequest, ExecHostResponse, ExecHostRunResult } from "../infra/exec-host.js";
|
||||
import { resolveExecSafeBinRuntimePolicy } from "../infra/exec-safe-bin-runtime-policy.js";
|
||||
import { sanitizeSystemRunEnvOverrides } from "../infra/host-env-security.js";
|
||||
import {
|
||||
inspectHostExecEnvOverrides,
|
||||
sanitizeSystemRunEnvOverrides,
|
||||
} from "../infra/host-env-security.js";
|
||||
import { normalizeSystemRunApprovalPlan } from "../infra/system-run-approval-binding.js";
|
||||
import { resolveSystemRunCommandRequest } from "../infra/system-run-command.js";
|
||||
import { logWarn } from "../logger.js";
|
||||
@@ -244,6 +247,34 @@ async function parseSystemRunPhase(
|
||||
const sessionKey = opts.params.sessionKey?.trim() || "node";
|
||||
const runId = opts.params.runId?.trim() || crypto.randomUUID();
|
||||
const suppressNotifyOnExit = opts.params.suppressNotifyOnExit === true;
|
||||
const envOverrideDiagnostics = inspectHostExecEnvOverrides({
|
||||
overrides: opts.params.env ?? undefined,
|
||||
blockPathOverrides: true,
|
||||
});
|
||||
if (
|
||||
envOverrideDiagnostics.rejectedOverrideBlockedKeys.length > 0 ||
|
||||
envOverrideDiagnostics.rejectedOverrideInvalidKeys.length > 0
|
||||
) {
|
||||
const details: string[] = [];
|
||||
if (envOverrideDiagnostics.rejectedOverrideBlockedKeys.length > 0) {
|
||||
details.push(
|
||||
`blocked override keys: ${envOverrideDiagnostics.rejectedOverrideBlockedKeys.join(", ")}`,
|
||||
);
|
||||
}
|
||||
if (envOverrideDiagnostics.rejectedOverrideInvalidKeys.length > 0) {
|
||||
details.push(
|
||||
`invalid non-portable override keys: ${envOverrideDiagnostics.rejectedOverrideInvalidKeys.join(", ")}`,
|
||||
);
|
||||
}
|
||||
await opts.sendInvokeResult({
|
||||
ok: false,
|
||||
error: {
|
||||
code: "INVALID_REQUEST",
|
||||
message: `SYSTEM_RUN_DENIED: environment override rejected (${details.join("; ")})`,
|
||||
},
|
||||
});
|
||||
return null;
|
||||
}
|
||||
const envOverrides = sanitizeSystemRunEnvOverrides({
|
||||
overrides: opts.params.env ?? undefined,
|
||||
shellWrapper: shellPayload !== null,
|
||||
|
||||
@@ -51,6 +51,13 @@ describe("node-host sanitizeEnv", () => {
|
||||
expect(env.BASH_ENV).toBeUndefined();
|
||||
});
|
||||
});
|
||||
|
||||
it("preserves inherited non-portable Windows-style env keys", () => {
|
||||
withEnv({ "ProgramFiles(x86)": "C:\\Program Files (x86)" }, () => {
|
||||
const env = sanitizeEnv(undefined);
|
||||
expect(env["ProgramFiles(x86)"]).toBe("C:\\Program Files (x86)");
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
describe("node-host output decoding", () => {
|
||||
|
||||
Reference in New Issue
Block a user