claudia.keller/openclaw-vainplex
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

fix(security): enforce strict environment variable validation in exec tool (#4896)

Browse source
This commit is contained in:
Hasan FLeyah 2026-02-02 02:36:24 +03:00 committed by GitHub
parent b796f6ec01
commit 0a5821a811
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
2 changed files with 62 additions and 7 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

15
src/agents/bash-tools.exec.path.test.ts
View file

@ -86,7 +86,7 @@ describe("exec PATH login shell merge", () => {
expect(shellPathMock).toHaveBeenCalledTimes(1);
});
it("skips login-shell PATH when env.PATH is provided", async () => {
it("throws security violation when env.PATH is provided", async () => {
if (isWin) {
return;
}
@ -98,13 +98,14 @@ describe("exec PATH login shell merge", () => {
shellPathMock.mockClear();
const tool = createExecTool({ host: "gateway", security: "full", ask: "off" });
const result = await tool.execute("call1", {
command: "echo $PATH",
env: { PATH: "/explicit/bin" },
});
const entries = normalizePathEntries(result.content.find((c) => c.type === "text")?.text);
expect(entries).toEqual(["/explicit/bin"]);
await expect(
tool.execute("call1", {
command: "echo $PATH",
env: { PATH: "/explicit/bin" },
}),
).rejects.toThrow(/Security Violation: Custom 'PATH' variable is forbidden/);
expect(shellPathMock).not.toHaveBeenCalled();
});
});

54
src/agents/bash-tools.exec.ts
View file

@ -56,6 +56,49 @@ import { getShellConfig, sanitizeBinaryOutput } from "./shell-utils.js";
import { callGatewayTool } from "./tools/gateway.js";
import { listNodes, resolveNodeIdFromList } from "./tools/nodes-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",
]);
// Centralized sanitization helper.
// Throws an error if dangerous variables or PATH modifications are detected on the host.
function validateHostEnv(env: Record<string, string>): void {
for (const key of Object.keys(env)) {
const upperKey = key.toUpperCase();
// 1. Block known dangerous variables (Fail Closed)
if (DANGEROUS_HOST_ENV_VARS.has(upperKey)) {
throw new Error(
`Security Violation: Environment variable '${key}' is forbidden during host execution.`,
);
}
// 2. Strictly block PATH modification on host
// Allowing custom PATH on the gateway/node can lead to binary hijacking.
if (upperKey === "PATH") {
throw new Error(
"Security Violation: Custom 'PATH' variable is forbidden during host execution.",
);
}
}
}
const DEFAULT_MAX_OUTPUT = clampNumber(
readEnvInt("PI_BASH_MAX_OUTPUT_CHARS"),
200_000,
@ -916,7 +959,15 @@ export function createExecTool(
}
const baseEnv = coerceEnv(process.env);
// 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 mergedEnv = params.env ? { ...baseEnv, ...params.env } : baseEnv;
const env = sandbox
? buildSandboxEnv({
defaultPath: DEFAULT_PATH,
@ -925,6 +976,7 @@ export function createExecTool(
containerWorkdir: containerWorkdir ?? sandbox.containerWorkdir,
})
: mergedEnv;
if (!sandbox && host === "gateway" && !params.env?.PATH) {
const shellPath = getShellPathFromLoginShell({
env: process.env,
@ -976,7 +1028,9 @@ export function createExecTool(
);
}
const argv = buildNodeShellCommand(params.command, nodeInfo?.platform);
const nodeEnv = params.env ? { ...params.env } : undefined;
if (nodeEnv) {
applyPathPrepend(nodeEnv, defaultPathPrepend, { requireExisting: true });
}