fix(security): block startup-file env injection across host execution paths
This commit is contained in:
@@ -3,6 +3,7 @@ import type { AgentToolResult } from "@mariozechner/pi-agent-core";
|
||||
import { Type } from "@sinclair/typebox";
|
||||
import type { ExecAsk, ExecHost, ExecSecurity } from "../infra/exec-approvals.js";
|
||||
import { requestHeartbeatNow } from "../infra/heartbeat-wake.js";
|
||||
import { isDangerousHostEnvVarName } from "../infra/host-env-security.js";
|
||||
import { mergePathPrepend } from "../infra/path-prepend.js";
|
||||
import { enqueueSystemEvent } from "../infra/system-events.js";
|
||||
import type { ProcessSession } from "./bash-process-registry.js";
|
||||
@@ -28,28 +29,6 @@ import {
|
||||
import { buildCursorPositionResponse, stripDsrRequests } from "./pty-dsr.js";
|
||||
import { getShellConfig, sanitizeBinaryOutput } from "./shell-utils.js";
|
||||
|
||||
// Security: Blocklist of environment variables that could alter execution flow
|
||||
// or inject code when running on non-sandboxed hosts (Gateway/Node).
|
||||
const DANGEROUS_HOST_ENV_VARS = new Set([
|
||||
"LD_PRELOAD",
|
||||
"LD_LIBRARY_PATH",
|
||||
"LD_AUDIT",
|
||||
"DYLD_INSERT_LIBRARIES",
|
||||
"DYLD_LIBRARY_PATH",
|
||||
"NODE_OPTIONS",
|
||||
"NODE_PATH",
|
||||
"PYTHONPATH",
|
||||
"PYTHONHOME",
|
||||
"RUBYLIB",
|
||||
"PERL5LIB",
|
||||
"BASH_ENV",
|
||||
"ENV",
|
||||
"GCONV_PATH",
|
||||
"IFS",
|
||||
"SSLKEYLOGFILE",
|
||||
]);
|
||||
const DANGEROUS_HOST_ENV_PREFIXES = ["DYLD_", "LD_"];
|
||||
|
||||
// Centralized sanitization helper.
|
||||
// Throws an error if dangerous variables or PATH modifications are detected on the host.
|
||||
export function validateHostEnv(env: Record<string, string>): void {
|
||||
@@ -57,12 +36,7 @@ export function validateHostEnv(env: Record<string, string>): void {
|
||||
const upperKey = key.toUpperCase();
|
||||
|
||||
// 1. Block known dangerous variables (Fail Closed)
|
||||
if (DANGEROUS_HOST_ENV_PREFIXES.some((prefix) => upperKey.startsWith(prefix))) {
|
||||
throw new Error(
|
||||
`Security Violation: Environment variable '${key}' is forbidden during host execution.`,
|
||||
);
|
||||
}
|
||||
if (DANGEROUS_HOST_ENV_VARS.has(upperKey)) {
|
||||
if (isDangerousHostEnvVarName(upperKey)) {
|
||||
throw new Error(
|
||||
`Security Violation: Environment variable '${key}' is forbidden during host execution.`,
|
||||
);
|
||||
|
||||
@@ -351,6 +351,50 @@ describe("applySkillEnvOverrides", () => {
|
||||
}
|
||||
});
|
||||
|
||||
it("blocks dangerous host env overrides even when declared", async () => {
|
||||
const workspaceDir = await makeWorkspace();
|
||||
const skillDir = path.join(workspaceDir, "skills", "dangerous-env-skill");
|
||||
await writeSkill({
|
||||
dir: skillDir,
|
||||
name: "dangerous-env-skill",
|
||||
description: "Needs env",
|
||||
metadata: '{"openclaw":{"requires":{"env":["BASH_ENV"]}}}',
|
||||
});
|
||||
|
||||
const entries = loadWorkspaceSkillEntries(workspaceDir, {
|
||||
managedSkillsDir: path.join(workspaceDir, ".managed"),
|
||||
});
|
||||
|
||||
const originalBashEnv = process.env.BASH_ENV;
|
||||
delete process.env.BASH_ENV;
|
||||
|
||||
const restore = applySkillEnvOverrides({
|
||||
skills: entries,
|
||||
config: {
|
||||
skills: {
|
||||
entries: {
|
||||
"dangerous-env-skill": {
|
||||
env: {
|
||||
BASH_ENV: "/tmp/pwn.sh",
|
||||
},
|
||||
},
|
||||
},
|
||||
},
|
||||
},
|
||||
});
|
||||
|
||||
try {
|
||||
expect(process.env.BASH_ENV).toBeUndefined();
|
||||
} finally {
|
||||
restore();
|
||||
if (originalBashEnv === undefined) {
|
||||
expect(process.env.BASH_ENV).toBeUndefined();
|
||||
} else {
|
||||
expect(process.env.BASH_ENV).toBe(originalBashEnv);
|
||||
}
|
||||
}
|
||||
});
|
||||
|
||||
it("allows required env overrides from snapshots", async () => {
|
||||
const workspaceDir = await makeWorkspace();
|
||||
const skillDir = path.join(workspaceDir, "skills", "snapshot-env-skill");
|
||||
|
||||
@@ -1,4 +1,5 @@
|
||||
import type { OpenClawConfig } from "../../config/config.js";
|
||||
import { isDangerousHostEnvVarName } from "../../infra/host-env-security.js";
|
||||
import { sanitizeEnvVars, validateEnvVarValue } from "../sandbox/sanitize-env-vars.js";
|
||||
import { resolveSkillConfig } from "./config.js";
|
||||
import { resolveSkillKey } from "./frontmatter.js";
|
||||
@@ -13,18 +14,19 @@ type SanitizedSkillEnvOverrides = {
|
||||
warnings: string[];
|
||||
};
|
||||
|
||||
// Never allow skill env overrides that can alter runtime loader flags.
|
||||
const HARD_BLOCKED_SKILL_ENV_PATTERNS: ReadonlyArray<RegExp> = [
|
||||
/^NODE_OPTIONS$/i,
|
||||
/^OPENSSL_CONF$/i,
|
||||
/^LD_PRELOAD$/i,
|
||||
/^DYLD_INSERT_LIBRARIES$/i,
|
||||
];
|
||||
// Always block skill env overrides that can alter runtime loading or host execution behavior.
|
||||
const SKILL_ALWAYS_BLOCKED_ENV_PATTERNS: ReadonlyArray<RegExp> = [/^OPENSSL_CONF$/i];
|
||||
|
||||
function matchesAnyPattern(value: string, patterns: readonly RegExp[]): boolean {
|
||||
return patterns.some((pattern) => pattern.test(value));
|
||||
}
|
||||
|
||||
function isAlwaysBlockedSkillEnvKey(key: string): boolean {
|
||||
return (
|
||||
isDangerousHostEnvVarName(key) || matchesAnyPattern(key, SKILL_ALWAYS_BLOCKED_ENV_PATTERNS)
|
||||
);
|
||||
}
|
||||
|
||||
function sanitizeSkillEnvOverrides(params: {
|
||||
overrides: Record<string, string>;
|
||||
allowedSensitiveKeys: Set<string>;
|
||||
@@ -33,19 +35,22 @@ function sanitizeSkillEnvOverrides(params: {
|
||||
return { allowed: {}, blocked: [], warnings: [] };
|
||||
}
|
||||
|
||||
const result = sanitizeEnvVars(params.overrides, {
|
||||
customBlockedPatterns: HARD_BLOCKED_SKILL_ENV_PATTERNS,
|
||||
});
|
||||
const allowed = { ...result.allowed };
|
||||
const blocked: string[] = [];
|
||||
const result = sanitizeEnvVars(params.overrides);
|
||||
const allowed: Record<string, string> = {};
|
||||
const blocked = new Set<string>();
|
||||
const warnings = [...result.warnings];
|
||||
|
||||
for (const [key, value] of Object.entries(result.allowed)) {
|
||||
if (isAlwaysBlockedSkillEnvKey(key)) {
|
||||
blocked.add(key);
|
||||
continue;
|
||||
}
|
||||
allowed[key] = value;
|
||||
}
|
||||
|
||||
for (const key of result.blocked) {
|
||||
if (
|
||||
matchesAnyPattern(key, HARD_BLOCKED_SKILL_ENV_PATTERNS) ||
|
||||
!params.allowedSensitiveKeys.has(key)
|
||||
) {
|
||||
blocked.push(key);
|
||||
if (isAlwaysBlockedSkillEnvKey(key) || !params.allowedSensitiveKeys.has(key)) {
|
||||
blocked.add(key);
|
||||
continue;
|
||||
}
|
||||
const value = params.overrides[key];
|
||||
@@ -55,7 +60,7 @@ function sanitizeSkillEnvOverrides(params: {
|
||||
const warning = validateEnvVarValue(value);
|
||||
if (warning) {
|
||||
if (warning === "Contains null bytes") {
|
||||
blocked.push(key);
|
||||
blocked.add(key);
|
||||
continue;
|
||||
}
|
||||
warnings.push(`${key}: ${warning}`);
|
||||
@@ -63,7 +68,7 @@ function sanitizeSkillEnvOverrides(params: {
|
||||
allowed[key] = value;
|
||||
}
|
||||
|
||||
return { allowed, blocked, warnings };
|
||||
return { allowed, blocked: [...blocked], warnings };
|
||||
}
|
||||
|
||||
function applySkillConfigEnvOverrides(params: {
|
||||
|
||||
Reference in New Issue
Block a user