mirror of
https://github.com/openclaw/openclaw.git
synced 2026-05-08 10:01:24 +00:00
fix(security): harden shell env fallback startup env handling
This commit is contained in:
@@ -1,4 +1,5 @@
|
||||
import fs from "node:fs";
|
||||
import os from "node:os";
|
||||
import { describe, expect, it, vi } from "vitest";
|
||||
import {
|
||||
getShellPathFromLoginShell,
|
||||
@@ -165,6 +166,68 @@ describe("shell env fallback", () => {
|
||||
}
|
||||
});
|
||||
|
||||
it("sanitizes startup-related env vars before shell fallback exec", () => {
|
||||
const env: NodeJS.ProcessEnv = {
|
||||
SHELL: "/bin/bash",
|
||||
HOME: "/tmp/evil-home",
|
||||
ZDOTDIR: "/tmp/evil-zdotdir",
|
||||
BASH_ENV: "/tmp/evil-bash-env",
|
||||
PS4: "$(touch /tmp/pwned)",
|
||||
};
|
||||
let receivedEnv: NodeJS.ProcessEnv | undefined;
|
||||
const exec = vi.fn((_shell: string, _args: string[], options: { env: NodeJS.ProcessEnv }) => {
|
||||
receivedEnv = options.env;
|
||||
return Buffer.from("OPENAI_API_KEY=from-shell\0");
|
||||
});
|
||||
|
||||
const res = loadShellEnvFallback({
|
||||
enabled: true,
|
||||
env,
|
||||
expectedKeys: ["OPENAI_API_KEY"],
|
||||
exec: exec as unknown as Parameters<typeof loadShellEnvFallback>[0]["exec"],
|
||||
});
|
||||
|
||||
expect(res.ok).toBe(true);
|
||||
expect(exec).toHaveBeenCalledTimes(1);
|
||||
expect(receivedEnv).toBeDefined();
|
||||
expect(receivedEnv?.BASH_ENV).toBeUndefined();
|
||||
expect(receivedEnv?.PS4).toBeUndefined();
|
||||
expect(receivedEnv?.ZDOTDIR).toBeUndefined();
|
||||
expect(receivedEnv?.SHELL).toBeUndefined();
|
||||
expect(receivedEnv?.HOME).toBe(os.homedir());
|
||||
});
|
||||
|
||||
it("sanitizes startup-related env vars before login-shell PATH probe", () => {
|
||||
resetShellPathCacheForTests();
|
||||
const env: NodeJS.ProcessEnv = {
|
||||
SHELL: "/bin/bash",
|
||||
HOME: "/tmp/evil-home",
|
||||
ZDOTDIR: "/tmp/evil-zdotdir",
|
||||
BASH_ENV: "/tmp/evil-bash-env",
|
||||
PS4: "$(touch /tmp/pwned)",
|
||||
};
|
||||
let receivedEnv: NodeJS.ProcessEnv | undefined;
|
||||
const exec = vi.fn((_shell: string, _args: string[], options: { env: NodeJS.ProcessEnv }) => {
|
||||
receivedEnv = options.env;
|
||||
return Buffer.from("PATH=/usr/local/bin:/usr/bin\0HOME=/tmp\0");
|
||||
});
|
||||
|
||||
const result = getShellPathFromLoginShell({
|
||||
env,
|
||||
exec: exec as unknown as Parameters<typeof getShellPathFromLoginShell>[0]["exec"],
|
||||
platform: "linux",
|
||||
});
|
||||
|
||||
expect(result).toBe("/usr/local/bin:/usr/bin");
|
||||
expect(exec).toHaveBeenCalledTimes(1);
|
||||
expect(receivedEnv).toBeDefined();
|
||||
expect(receivedEnv?.BASH_ENV).toBeUndefined();
|
||||
expect(receivedEnv?.PS4).toBeUndefined();
|
||||
expect(receivedEnv?.ZDOTDIR).toBeUndefined();
|
||||
expect(receivedEnv?.SHELL).toBeUndefined();
|
||||
expect(receivedEnv?.HOME).toBe(os.homedir());
|
||||
});
|
||||
|
||||
it("returns null without invoking shell on win32", () => {
|
||||
resetShellPathCacheForTests();
|
||||
const exec = vi.fn(() => Buffer.from("PATH=/usr/local/bin:/usr/bin\0HOME=/tmp\0"));
|
||||
|
||||
@@ -1,7 +1,9 @@
|
||||
import { execFileSync } from "node:child_process";
|
||||
import fs from "node:fs";
|
||||
import os from "node:os";
|
||||
import path from "node:path";
|
||||
import { isTruthyEnvValue } from "./env.js";
|
||||
import { sanitizeHostExecEnv } from "./host-env-security.js";
|
||||
|
||||
const DEFAULT_TIMEOUT_MS = 15_000;
|
||||
const DEFAULT_MAX_BUFFER_BYTES = 2 * 1024 * 1024;
|
||||
@@ -17,6 +19,22 @@ let lastAppliedKeys: string[] = [];
|
||||
let cachedShellPath: string | null | undefined;
|
||||
let cachedEtcShells: Set<string> | null | undefined;
|
||||
|
||||
function resolveShellExecEnv(env: NodeJS.ProcessEnv): NodeJS.ProcessEnv {
|
||||
const execEnv = sanitizeHostExecEnv({ baseEnv: env });
|
||||
|
||||
// Startup-file resolution must stay pinned to the real user home.
|
||||
const home = os.homedir().trim();
|
||||
if (home) {
|
||||
execEnv.HOME = home;
|
||||
} else {
|
||||
delete execEnv.HOME;
|
||||
}
|
||||
|
||||
// Avoid zsh startup-file redirection via env poisoning.
|
||||
delete execEnv.ZDOTDIR;
|
||||
return execEnv;
|
||||
}
|
||||
|
||||
function resolveTimeoutMs(timeoutMs: number | undefined): number {
|
||||
if (typeof timeoutMs !== "number" || !Number.isFinite(timeoutMs)) {
|
||||
return DEFAULT_TIMEOUT_MS;
|
||||
@@ -145,10 +163,11 @@ export function loadShellEnvFallback(opts: ShellEnvFallbackOptions): ShellEnvFal
|
||||
const timeoutMs = resolveTimeoutMs(opts.timeoutMs);
|
||||
|
||||
const shell = resolveShell(opts.env);
|
||||
const execEnv = resolveShellExecEnv(opts.env);
|
||||
|
||||
let stdout: Buffer;
|
||||
try {
|
||||
stdout = execLoginShellEnvZero({ shell, env: opts.env, exec, timeoutMs });
|
||||
stdout = execLoginShellEnvZero({ shell, env: execEnv, exec, timeoutMs });
|
||||
} catch (err) {
|
||||
const msg = err instanceof Error ? err.message : String(err);
|
||||
logger.warn(`[openclaw] shell env fallback failed: ${msg}`);
|
||||
@@ -213,10 +232,11 @@ export function getShellPathFromLoginShell(opts: {
|
||||
const exec = opts.exec ?? execFileSync;
|
||||
const timeoutMs = resolveTimeoutMs(opts.timeoutMs);
|
||||
const shell = resolveShell(opts.env);
|
||||
const execEnv = resolveShellExecEnv(opts.env);
|
||||
|
||||
let stdout: Buffer;
|
||||
try {
|
||||
stdout = execLoginShellEnvZero({ shell, env: opts.env, exec, timeoutMs });
|
||||
stdout = execLoginShellEnvZero({ shell, env: execEnv, exec, timeoutMs });
|
||||
} catch {
|
||||
cachedShellPath = null;
|
||||
return cachedShellPath;
|
||||
|
||||
Reference in New Issue
Block a user