mirror of
https://github.com/moltbot/moltbot.git
synced 2026-03-20 07:54:36 +00:00
fix(security): block HOME and ZDOTDIR env override injection
This commit is contained in:
@@ -25,6 +25,10 @@ enum HostEnvSanitizer {
|
||||
"LD_",
|
||||
"BASH_FUNC_",
|
||||
]
|
||||
private static let blockedOverrideKeys: Set<String> = [
|
||||
"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
|
||||
}
|
||||
|
||||
@@ -15,5 +15,6 @@
|
||||
"IFS",
|
||||
"SSLKEYLOGFILE"
|
||||
],
|
||||
"blockedOverrideKeys": ["HOME", "ZDOTDIR"],
|
||||
"blockedPrefixes": ["DYLD_", "LD_", "BASH_FUNC_"]
|
||||
}
|
||||
|
||||
@@ -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);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -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");
|
||||
|
||||
@@ -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<string>(HOST_DANGEROUS_ENV_KEY_VALUES);
|
||||
export const HOST_DANGEROUS_OVERRIDE_ENV_KEYS = new Set<string>(
|
||||
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<string, string | undefined>;
|
||||
overrides?: Record<string, string> | 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;
|
||||
|
||||
@@ -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);
|
||||
|
||||
Reference in New Issue
Block a user