fix(security): preserve system.run wrapper approval semantics

This commit is contained in:
Peter Steinberger
2026-03-02 17:20:52 +00:00
parent 104d32bb64
commit dded569626
5 changed files with 140 additions and 0 deletions

View File

@@ -46,6 +46,7 @@ Docs: https://docs.openclaw.ai
### Fixes
- Security/Node exec approvals: preserve shell/dispatch-wrapper argv semantics during approval hardening so approved wrapper commands (for example `env sh -c ...`) cannot drift into a different runtime command shape, and add regression coverage for both approval-plan generation and approved runtime execution paths. Thanks @tdjackey for reporting.
- Sandbox/Bootstrap context boundary hardening: reject symlink/hardlink alias bootstrap seed files that resolve outside the source workspace and switch post-compaction `AGENTS.md` context reads to boundary-verified file opens, preventing host file content from being injected via workspace aliasing. Thanks @tdjackey for reporting.
- Browser/Security output boundary hardening: replace check-then-rename output commits with root-bound fd-verified writes, unify install/skills canonical path-boundary checks, and add regression coverage for symlink-rebind race paths across browser output and shared fs-safe write flows. Thanks @tdjackey for reporting.
- Gateway/Security hardening: tie loopback-origin dev allowance to actual local socket clients (not Host header claims), add explicit warnings/metrics when `gateway.controlUi.dangerouslyAllowHostHeaderOriginFallback` accepts websocket origins, harden safe-regex detection for quantified ambiguous alternation patterns (for example `(a|aa)+`), and bound large regex-evaluation inputs for session-filter and log-redaction paths.

View File

@@ -0,0 +1,87 @@
import fs from "node:fs";
import os from "node:os";
import path from "node:path";
import { describe, expect, it } from "vitest";
import {
buildSystemRunApprovalPlan,
hardenApprovedExecutionPaths,
} from "./invoke-system-run-plan.js";
describe("hardenApprovedExecutionPaths", () => {
it.runIf(process.platform !== "win32")(
"preserves shell-wrapper argv during approval hardening",
() => {
const tmp = fs.mkdtempSync(path.join(os.tmpdir(), "openclaw-shell-wrapper-"));
try {
const prepared = buildSystemRunApprovalPlan({
command: ["env", "sh", "-c", "echo SAFE"],
cwd: tmp,
});
expect(prepared.ok).toBe(true);
if (!prepared.ok) {
throw new Error("unreachable");
}
expect(prepared.plan.argv).toEqual(["env", "sh", "-c", "echo SAFE"]);
expect(prepared.cmdText).toBe("echo SAFE");
} finally {
fs.rmSync(tmp, { recursive: true, force: true });
}
},
);
it.runIf(process.platform !== "win32")(
"preserves dispatch-wrapper argv during approval hardening",
() => {
const tmp = fs.mkdtempSync(path.join(os.tmpdir(), "openclaw-dispatch-wrapper-"));
try {
const hardened = hardenApprovedExecutionPaths({
approvedByAsk: true,
argv: ["env", "tr", "a", "b"],
shellCommand: null,
cwd: tmp,
});
expect(hardened.ok).toBe(true);
if (!hardened.ok) {
throw new Error("unreachable");
}
expect(hardened.argv).toEqual(["env", "tr", "a", "b"]);
} finally {
fs.rmSync(tmp, { recursive: true, force: true });
}
},
);
it.runIf(process.platform !== "win32")(
"pins direct PATH-token executable during approval hardening",
() => {
const tmp = fs.mkdtempSync(path.join(os.tmpdir(), "openclaw-direct-pin-"));
const binDir = path.join(tmp, "bin");
fs.mkdirSync(binDir, { recursive: true });
const link = path.join(binDir, "poccmd");
fs.symlinkSync("/bin/echo", link);
const expected = fs.realpathSync(link);
const oldPath = process.env.PATH;
process.env.PATH = `${binDir}${path.delimiter}${oldPath ?? ""}`;
try {
const hardened = hardenApprovedExecutionPaths({
approvedByAsk: true,
argv: ["poccmd", "SAFE"],
shellCommand: null,
cwd: tmp,
});
expect(hardened.ok).toBe(true);
if (!hardened.ok) {
throw new Error("unreachable");
}
expect(hardened.argv).toEqual([expected, "SAFE"]);
} finally {
if (oldPath === undefined) {
delete process.env.PATH;
} else {
process.env.PATH = oldPath;
}
fs.rmSync(tmp, { recursive: true, force: true });
}
},
);
});

View File

@@ -56,6 +56,7 @@ function hasMutableSymlinkPathComponentSync(targetPath: string): boolean {
export function hardenApprovedExecutionPaths(params: {
approvedByAsk: boolean;
argv: string[];
shellCommand: string | null;
cwd: string | undefined;
}): { ok: true; argv: string[]; cwd: string | undefined } | { ok: false; message: string } {
if (!params.approvedByAsk) {
@@ -115,7 +116,19 @@ export function hardenApprovedExecutionPaths(params: {
return { ok: true, argv: params.argv, cwd: hardenedCwd };
}
// Preserve shell-wrapper semantics. Rewriting argv[0] for wrappers can change
// runtime behavior (for example: env sh -c ... -> /bin/sh sh -c ...).
if (params.shellCommand !== null) {
return { ok: true, argv: params.argv, cwd: hardenedCwd };
}
const resolution = resolveCommandResolutionFromArgv(params.argv, hardenedCwd);
// Preserve transparent wrapper semantics for approval-based execution.
// Pinning the effective executable while keeping wrapper argv shape can shift
// positional arguments and execute a different command than approved.
if ((resolution?.wrapperChain?.length ?? 0) > 0) {
return { ok: true, argv: params.argv, cwd: hardenedCwd };
}
const pinnedExecutable = resolution?.resolvedRealPath ?? resolution?.resolvedPath;
if (!pinnedExecutable) {
return {
@@ -149,6 +162,7 @@ export function buildSystemRunApprovalPlan(params: {
const hardening = hardenApprovedExecutionPaths({
approvedByAsk: true,
argv: command.argv,
shellCommand: command.shellCommand,
cwd: normalizeString(params.cwd) ?? undefined,
});
if (!hardening.ok) {

View File

@@ -478,6 +478,43 @@ describe("handleSystemRunInvoke mac app exec host routing", () => {
fs.rmSync(tmp, { recursive: true, force: true });
}
});
it.runIf(process.platform !== "win32")(
"preserves wrapper argv for approved env shell commands",
async () => {
const tmp = fs.mkdtempSync(path.join(os.tmpdir(), "openclaw-approved-wrapper-"));
const marker = path.join(tmp, "marker");
const attackerScript = path.join(tmp, "sh");
fs.writeFileSync(attackerScript, "#!/bin/sh\necho exploited > marker\n");
fs.chmodSync(attackerScript, 0o755);
const runCommand = vi.fn(async (argv: string[]) => {
if (argv[0] === "/bin/sh" && argv[1] === "sh" && argv[2] === "-c") {
fs.writeFileSync(marker, "rewritten");
}
return createLocalRunResult();
});
const sendInvokeResult = vi.fn(async () => {});
try {
await runSystemInvoke({
preferMacAppExecHost: false,
command: ["env", "sh", "-c", "echo SAFE"],
cwd: tmp,
approved: true,
security: "allowlist",
ask: "on-miss",
runCommand,
sendInvokeResult,
});
const runArgs = vi.mocked(runCommand).mock.calls[0]?.[0] as string[] | undefined;
expect(runArgs).toEqual(["env", "sh", "-c", "echo SAFE"]);
expect(fs.existsSync(marker)).toBe(false);
expectInvokeOk(sendInvokeResult);
} finally {
fs.rmSync(tmp, { recursive: true, force: true });
}
},
);
it("denies ./sh wrapper spoof in allowlist on-miss mode before execution", async () => {
const marker = path.join(os.tmpdir(), `openclaw-wrapper-spoof-${process.pid}-${Date.now()}`);
const runCommand = vi.fn(async () => {

View File

@@ -300,6 +300,7 @@ async function evaluateSystemRunPolicyPhase(
const hardenedPaths = hardenApprovedExecutionPaths({
approvedByAsk: policy.approvedByAsk,
argv: parsed.argv,
shellCommand: parsed.shellCommand,
cwd: parsed.cwd,
});
if (!hardenedPaths.ok) {