github/openclaw
1
1
Fork 0
mirror of https://github.com/openclaw/openclaw.git synced 2026-02-19 18:39:20 -05:00
Code Issues Packages Projects Releases Wiki Activity

fix: harden exec PATH handling

Browse Source
This commit is contained in:
Peter Steinberger
2026-02-14 19:50:33 +01:00
parent 53af46ffb8
commit 013e8f6b3b
6 changed files with 169 additions and 51 deletions
Download Patch File Download Diff File Expand all files Collapse all files

1
CHANGELOG.md
View File

@@ -19,6 +19,7 @@ Docs: https://docs.openclaw.ai
- Security/Node Host: enforce `system.run` rawCommand/argv consistency to prevent allowlist/approval bypass. Thanks @christos-eth.
- Security/Exec approvals: prevent safeBins allowlist bypass via shell expansion (host exec allowlist mode only; not enabled by default). Thanks @christos-eth.
- Security/Gateway: block `system.execApprovals.*` via `node.invoke` (use `exec.approvals.node.*` instead). Thanks @christos-eth.
- Security/Exec: harden PATH handling by disabling project-local `node_modules/.bin` bootstrapping by default, disallowing node-host `PATH` overrides, and spawning ACP servers via the current executable by default. Thanks @akhmittra.
- CLI: fix lazy core command registration so top-level maintenance commands (`doctor`, `dashboard`, `reset`, `uninstall`) resolve correctly instead of exposing a non-functional `maintenance` placeholder command.
- Security/Agents: scope CLI process cleanup to owned child PIDs to avoid killing unrelated processes on shared hosts. Thanks @aether-ai-agent.
- Security/Agents (macOS): prevent shell injection when writing Claude CLI keychain credentials. (#15924) Thanks @aether-ai-agent.

33
src/acp/client.ts
View File

@@ -7,8 +7,11 @@ import {
type SessionNotification,
} from "@agentclientprotocol/sdk";
import { spawn, type ChildProcess } from "node:child_process";
import fs from "node:fs";
import path from "node:path";
import * as readline from "node:readline";
import { Readable, Writable } from "node:stream";
import { fileURLToPath } from "node:url";
import { ensureOpenClawCliOnPath } from "../infra/path-env.js";
import { DANGEROUS_ACP_TOOLS } from "../security/dangerous-tools.js";
@@ -260,6 +263,25 @@ function buildServerArgs(opts: AcpClientOptions): string[] {
return args;
}
function resolveSelfEntryPath(): string | null {
// Prefer a path relative to the built module location (dist/acp/client.js -> dist/entry.js).
try {
const here = fileURLToPath(import.meta.url);
const candidate = path.resolve(path.dirname(here), "..", "entry.js");
if (fs.existsSync(candidate)) {
return candidate;
}
} catch {
// ignore
}
const argv1 = process.argv[1]?.trim();
if (argv1) {
return path.isAbsolute(argv1) ? argv1 : path.resolve(process.cwd(), argv1);
}
return null;
}
function printSessionUpdate(notification: SessionNotification): void {
const update = notification.update;
if (!("sessionUpdate" in update)) {
@@ -300,13 +322,16 @@ export async function createAcpClient(opts: AcpClientOptions = {}): Promise<AcpC
const verbose = Boolean(opts.verbose);
const log = verbose ? (msg: string) => console.error(`[acp-client] ${msg}`) : () => {};
ensureOpenClawCliOnPath({ cwd });
const serverCommand = opts.serverCommand ?? "openclaw";
ensureOpenClawCliOnPath();
const serverArgs = buildServerArgs(opts);
log(`spawning: ${serverCommand} ${serverArgs.join(" ")}`);
const entryPath = resolveSelfEntryPath();
const serverCommand = opts.serverCommand ?? (entryPath ? process.execPath : "openclaw");
const effectiveArgs = opts.serverCommand || !entryPath ? serverArgs : [entryPath, ...serverArgs];
const agent = spawn(serverCommand, serverArgs, {
log(`spawning: ${serverCommand} ${effectiveArgs.join(" ")}`);
const agent = spawn(serverCommand, effectiveArgs, {
stdio: ["pipe", "pipe", "inherit"],
cwd,
});

65
src/infra/path-env.test.ts
View File

@@ -75,12 +75,6 @@ describe("ensureOpenClawCliOnPath", () => {
await fs.writeFile(appCli, "#!/bin/sh\necho ok\n", "utf-8");
await fs.chmod(appCli, 0o755);
const localBinDir = path.join(tmp, "node_modules", ".bin");
await fs.mkdir(localBinDir, { recursive: true });
const localCli = path.join(localBinDir, "openclaw");
await fs.writeFile(localCli, "#!/bin/sh\necho ok\n", "utf-8");
await fs.chmod(localCli, 0o755);
const miseDataDir = path.join(tmp, "mise");
const shimsDir = path.join(miseDataDir, "shims");
await fs.mkdir(shimsDir, { recursive: true });
@@ -98,11 +92,9 @@ describe("ensureOpenClawCliOnPath", () => {
const updated = process.env.PATH ?? "";
const parts = updated.split(path.delimiter);
const appBinIndex = parts.indexOf(appBinDir);
const localIndex = parts.indexOf(localBinDir);
const shimsIndex = parts.indexOf(shimsDir);
expect(appBinIndex).toBeGreaterThanOrEqual(0);
expect(localIndex).toBeGreaterThan(appBinIndex);
expect(shimsIndex).toBeGreaterThan(localIndex);
expect(shimsIndex).toBeGreaterThan(appBinIndex);
} finally {
process.env.PATH = originalPath;
if (originalFlag === undefined) {
@@ -119,6 +111,61 @@ describe("ensureOpenClawCliOnPath", () => {
}
});
it("only appends project-local node_modules/.bin when explicitly enabled", async () => {
const tmp = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-path-"));
const originalPath = process.env.PATH;
const originalFlag = process.env.OPENCLAW_PATH_BOOTSTRAPPED;
try {
const appBinDir = path.join(tmp, "AppBin");
await fs.mkdir(appBinDir, { recursive: true });
const appCli = path.join(appBinDir, "openclaw");
await fs.writeFile(appCli, "#!/bin/sh\necho ok\n", "utf-8");
await fs.chmod(appCli, 0o755);
const localBinDir = path.join(tmp, "node_modules", ".bin");
await fs.mkdir(localBinDir, { recursive: true });
const localCli = path.join(localBinDir, "openclaw");
await fs.writeFile(localCli, "#!/bin/sh\necho ok\n", "utf-8");
await fs.chmod(localCli, 0o755);
process.env.PATH = "/usr/bin";
delete process.env.OPENCLAW_PATH_BOOTSTRAPPED;
ensureOpenClawCliOnPath({
execPath: appCli,
cwd: tmp,
homeDir: tmp,
platform: "darwin",
});
const withoutOptIn = (process.env.PATH ?? "").split(path.delimiter);
expect(withoutOptIn.includes(localBinDir)).toBe(false);
process.env.PATH = "/usr/bin";
delete process.env.OPENCLAW_PATH_BOOTSTRAPPED;
ensureOpenClawCliOnPath({
execPath: appCli,
cwd: tmp,
homeDir: tmp,
platform: "darwin",
allowProjectLocalBin: true,
});
const withOptIn = (process.env.PATH ?? "").split(path.delimiter);
const usrBinIndex = withOptIn.indexOf("/usr/bin");
const localIndex = withOptIn.indexOf(localBinDir);
expect(usrBinIndex).toBeGreaterThanOrEqual(0);
expect(localIndex).toBeGreaterThan(usrBinIndex);
} finally {
process.env.PATH = originalPath;
if (originalFlag === undefined) {
delete process.env.OPENCLAW_PATH_BOOTSTRAPPED;
} else {
process.env.OPENCLAW_PATH_BOOTSTRAPPED = originalFlag;
}
await fs.rm(tmp, { recursive: true, force: true });
}
});
it("prepends Linuxbrew dirs when present", async () => {
const tmp = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-path-"));
const originalPath = process.env.PATH;

56
src/infra/path-env.ts
View File

@@ -10,6 +10,7 @@ type EnsureOpenClawPathOpts = {
homeDir?: string;
platform?: NodeJS.Platform;
pathEnv?: string;
allowProjectLocalBin?: boolean;
};
function isExecutable(filePath: string): boolean {
@@ -29,16 +30,17 @@ function isDirectory(dirPath: string): boolean {
}
}
function mergePath(params: { existing: string; prepend: string[] }): string {
function mergePath(params: { existing: string; prepend?: string[]; append?: string[] }): string {
const partsExisting = params.existing
.split(path.delimiter)
.map((part) => part.trim())
.filter(Boolean);
const partsPrepend = params.prepend.map((part) => part.trim()).filter(Boolean);
const partsPrepend = (params.prepend ?? []).map((part) => part.trim()).filter(Boolean);
const partsAppend = (params.append ?? []).map((part) => part.trim()).filter(Boolean);
const seen = new Set<string>();
const merged: string[] = [];
for (const part of [...partsPrepend, ...partsExisting]) {
for (const part of [...partsPrepend, ...partsExisting, ...partsAppend]) {
if (!seen.has(part)) {
seen.add(part);
merged.push(part);
@@ -47,54 +49,60 @@ function mergePath(params: { existing: string; prepend: string[] }): string {
return merged.join(path.delimiter);
}
function candidateBinDirs(opts: EnsureOpenClawPathOpts): string[] {
function candidateBinDirs(opts: EnsureOpenClawPathOpts): { prepend: string[]; append: string[] } {
const execPath = opts.execPath ?? process.execPath;
const cwd = opts.cwd ?? process.cwd();
const homeDir = opts.homeDir ?? os.homedir();
const platform = opts.platform ?? process.platform;
const candidates: string[] = [];
const prepend: string[] = [];
const append: string[] = [];
// Bundled macOS app: `openclaw` lives next to the executable (process.execPath).
try {
const execDir = path.dirname(execPath);
const siblingCli = path.join(execDir, "openclaw");
if (isExecutable(siblingCli)) {
candidates.push(execDir);
prepend.push(execDir);
}
} catch {
// ignore
}
// Project-local installs (best effort): if a `node_modules/.bin/openclaw` exists near cwd,
// include it. This helps when running under launchd or other minimal PATH environments.
const localBinDir = path.join(cwd, "node_modules", ".bin");
if (isExecutable(path.join(localBinDir, "openclaw"))) {
candidates.push(localBinDir);
// Project-local installs are a common repo-based attack vector (bin hijacking). Keep this
// disabled by default; if an operator explicitly enables it, only append (never prepend).
const allowProjectLocalBin =
opts.allowProjectLocalBin === true ||
isTruthyEnvValue(process.env.OPENCLAW_ALLOW_PROJECT_LOCAL_BIN);
if (allowProjectLocalBin) {
const localBinDir = path.join(cwd, "node_modules", ".bin");
if (isExecutable(path.join(localBinDir, "openclaw"))) {
append.push(localBinDir);
}
}
const miseDataDir = process.env.MISE_DATA_DIR ?? path.join(homeDir, ".local", "share", "mise");
const miseShims = path.join(miseDataDir, "shims");
if (isDirectory(miseShims)) {
candidates.push(miseShims);
prepend.push(miseShims);
}
candidates.push(...resolveBrewPathDirs({ homeDir }));
prepend.push(...resolveBrewPathDirs({ homeDir }));
// Common global install locations (macOS first).
if (platform === "darwin") {
candidates.push(path.join(homeDir, "Library", "pnpm"));
prepend.push(path.join(homeDir, "Library", "pnpm"));
}
if (process.env.XDG_BIN_HOME) {
candidates.push(process.env.XDG_BIN_HOME);
prepend.push(process.env.XDG_BIN_HOME);
}
candidates.push(path.join(homeDir, ".local", "bin"));
candidates.push(path.join(homeDir, ".local", "share", "pnpm"));
candidates.push(path.join(homeDir, ".bun", "bin"));
candidates.push(path.join(homeDir, ".yarn", "bin"));
candidates.push("/opt/homebrew/bin", "/usr/local/bin", "/usr/bin", "/bin");
prepend.push(path.join(homeDir, ".local", "bin"));
prepend.push(path.join(homeDir, ".local", "share", "pnpm"));
prepend.push(path.join(homeDir, ".bun", "bin"));
prepend.push(path.join(homeDir, ".yarn", "bin"));
prepend.push("/opt/homebrew/bin", "/usr/local/bin", "/usr/bin", "/bin");
return candidates.filter(isDirectory);
return { prepend: prepend.filter(isDirectory), append: append.filter(isDirectory) };
}
/**
@@ -108,12 +116,12 @@ export function ensureOpenClawCliOnPath(opts: EnsureOpenClawPathOpts = {}) {
process.env.OPENCLAW_PATH_BOOTSTRAPPED = "1";
const existing = opts.pathEnv ?? process.env.PATH ?? "";
const prepend = candidateBinDirs(opts);
if (prepend.length === 0) {
const { prepend, append } = candidateBinDirs(opts);
if (prepend.length === 0 && append.length === 0) {
return;
}
const merged = mergePath({ existing, prepend });
const merged = mergePath({ existing, prepend, append });
if (merged) {
process.env.PATH = merged;
}

48
src/node-host/invoke.sanitize-env.test.ts Normal file
View File

@@ -0,0 +1,48 @@
import { describe, expect, it } from "vitest";
import { sanitizeEnv } from "./invoke.js";
describe("node-host sanitizeEnv", () => {
it("ignores PATH overrides", () => {
const prev = process.env.PATH;
process.env.PATH = "/usr/bin";
try {
const env = sanitizeEnv({ PATH: "/tmp/evil:/usr/bin" }) ?? {};
expect(env.PATH).toBe("/usr/bin");
} finally {
if (prev === undefined) {
delete process.env.PATH;
} else {
process.env.PATH = prev;
}
}
});
it("blocks dangerous env keys/prefixes", () => {
const prevPythonPath = process.env.PYTHONPATH;
const prevLdPreload = process.env.LD_PRELOAD;
try {
delete process.env.PYTHONPATH;
delete process.env.LD_PRELOAD;
const env =
sanitizeEnv({
PYTHONPATH: "/tmp/pwn",
LD_PRELOAD: "/tmp/pwn.so",
FOO: "bar",
}) ?? {};
expect(env.FOO).toBe("bar");
expect(env.PYTHONPATH).toBeUndefined();
expect(env.LD_PRELOAD).toBeUndefined();
} finally {
if (prevPythonPath === undefined) {
delete process.env.PYTHONPATH;
} else {
process.env.PYTHONPATH = prevPythonPath;
}
if (prevLdPreload === undefined) {
delete process.env.LD_PRELOAD;
} else {
process.env.LD_PRELOAD = prevLdPreload;
}
}
});
});

17
src/node-host/invoke.ts
View File

@@ -135,33 +135,22 @@ function resolveExecAsk(value?: string): ExecAsk {
return value === "off" || value === "on-miss" || value === "always" ? value : "on-miss";
}
function sanitizeEnv(
export function sanitizeEnv(
overrides?: Record<string, string> | null,
): Record<string, string> | undefined {
if (!overrides) {
return undefined;
}
const merged = { ...process.env } as Record<string, string>;
const basePath = process.env.PATH ?? DEFAULT_NODE_PATH;
for (const [rawKey, value] of Object.entries(overrides)) {
const key = rawKey.trim();
if (!key) {
continue;
}
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 (upper === "PATH") {
const trimmed = value.trim();
if (!trimmed) {
continue;
}
if (!basePath || trimmed === basePath) {
merged[key] = trimmed;
continue;
}
const suffix = `${path.delimiter}${basePath}`;
if (trimmed.endsWith(suffix)) {
merged[key] = trimmed;
}
continue;
}
if (blockedEnvKeys.has(upper)) {