fix(node-host): bind approved script operands

This commit is contained in:
Peter Steinberger
2026-03-07 23:03:26 +00:00
parent bfbe80ab7d
commit c76d29208b
12 changed files with 374 additions and 4 deletions

View File

@@ -128,4 +128,28 @@ describe("hardenApprovedExecutionPaths", () => {
}
});
}
it("captures mutable shell script operands in approval plans", () => {
const tmp = fs.mkdtempSync(path.join(os.tmpdir(), "openclaw-approval-script-plan-"));
const script = path.join(tmp, "run.sh");
fs.writeFileSync(script, "#!/bin/sh\necho SAFE\n");
fs.chmodSync(script, 0o755);
try {
const prepared = buildSystemRunApprovalPlan({
command: ["/bin/sh", "./run.sh"],
cwd: tmp,
});
expect(prepared.ok).toBe(true);
if (!prepared.ok) {
throw new Error("unreachable");
}
expect(prepared.plan.mutableFileOperand).toEqual({
argvIndex: 1,
path: fs.realpathSync(script),
sha256: expect.any(String),
});
} finally {
fs.rmSync(tmp, { recursive: true, force: true });
}
});
});

View File

@@ -1,8 +1,22 @@
import crypto from "node:crypto";
import fs from "node:fs";
import path from "node:path";
import type { SystemRunApprovalPlan } from "../infra/exec-approvals.js";
import type {
SystemRunApprovalFileOperand,
SystemRunApprovalPlan,
} from "../infra/exec-approvals.js";
import { resolveCommandResolutionFromArgv } from "../infra/exec-command-resolution.js";
import {
POSIX_SHELL_WRAPPERS,
normalizeExecutableToken,
unwrapKnownDispatchWrapperInvocation,
unwrapKnownShellMultiplexerInvocation,
} from "../infra/exec-wrapper-resolution.js";
import { sameFileIdentity } from "../infra/file-identity.js";
import {
POSIX_INLINE_COMMAND_FLAGS,
resolveInlineCommandMatch,
} from "../infra/shell-inline-command.js";
import { formatExecCommand, resolveSystemRunCommand } from "../infra/system-run-command.js";
export type ApprovedCwdSnapshot = {
@@ -10,6 +24,14 @@ export type ApprovedCwdSnapshot = {
stat: fs.Stats;
};
const MUTABLE_ARGV1_INTERPRETER_PATTERNS = [
/^(?:node|nodejs)$/,
/^perl$/,
/^php$/,
/^python(?:\d+(?:\.\d+)*)?$/,
/^ruby$/,
] as const;
function normalizeString(value: unknown): string | null {
if (typeof value !== "string") {
return null;
@@ -68,6 +90,125 @@ function shouldPinExecutableForApproval(params: {
return (params.wrapperChain?.length ?? 0) === 0;
}
function hashFileContentsSync(filePath: string): string {
return crypto.createHash("sha256").update(fs.readFileSync(filePath)).digest("hex");
}
function unwrapArgvForMutableOperand(argv: string[]): { argv: string[]; baseIndex: number } {
let current = argv;
let baseIndex = 0;
while (true) {
const dispatchUnwrap = unwrapKnownDispatchWrapperInvocation(current);
if (dispatchUnwrap.kind === "unwrapped") {
baseIndex += current.length - dispatchUnwrap.argv.length;
current = dispatchUnwrap.argv;
continue;
}
const shellMultiplexerUnwrap = unwrapKnownShellMultiplexerInvocation(current);
if (shellMultiplexerUnwrap.kind === "unwrapped") {
baseIndex += current.length - shellMultiplexerUnwrap.argv.length;
current = shellMultiplexerUnwrap.argv;
continue;
}
return { argv: current, baseIndex };
}
}
function resolvePosixShellScriptOperandIndex(argv: string[]): number | null {
if (
resolveInlineCommandMatch(argv, POSIX_INLINE_COMMAND_FLAGS, {
allowCombinedC: true,
}).valueTokenIndex !== null
) {
return null;
}
let afterDoubleDash = false;
for (let i = 1; i < argv.length; i += 1) {
const token = argv[i]?.trim() ?? "";
if (!token) {
continue;
}
if (token === "-") {
return null;
}
if (!afterDoubleDash && token === "--") {
afterDoubleDash = true;
continue;
}
if (!afterDoubleDash && token === "-s") {
return null;
}
if (!afterDoubleDash && token.startsWith("-")) {
continue;
}
return i;
}
return null;
}
function resolveMutableFileOperandIndex(argv: string[]): number | null {
const unwrapped = unwrapArgvForMutableOperand(argv);
const executable = normalizeExecutableToken(unwrapped.argv[0] ?? "");
if (!executable) {
return null;
}
if ((POSIX_SHELL_WRAPPERS as ReadonlySet<string>).has(executable)) {
const shellIndex = resolvePosixShellScriptOperandIndex(unwrapped.argv);
return shellIndex === null ? null : unwrapped.baseIndex + shellIndex;
}
if (!MUTABLE_ARGV1_INTERPRETER_PATTERNS.some((pattern) => pattern.test(executable))) {
return null;
}
const operand = unwrapped.argv[1]?.trim() ?? "";
if (!operand || operand === "-" || operand.startsWith("-")) {
return null;
}
return unwrapped.baseIndex + 1;
}
function resolveMutableFileOperandSnapshotSync(params: {
argv: string[];
cwd: string | undefined;
}): { ok: true; snapshot: SystemRunApprovalFileOperand | null } | { ok: false; message: string } {
const argvIndex = resolveMutableFileOperandIndex(params.argv);
if (argvIndex === null) {
return { ok: true, snapshot: null };
}
const rawOperand = params.argv[argvIndex]?.trim();
if (!rawOperand) {
return {
ok: false,
message: "SYSTEM_RUN_DENIED: approval requires a stable script operand",
};
}
const resolvedPath = path.resolve(params.cwd ?? process.cwd(), rawOperand);
let realPath: string;
let stat: fs.Stats;
try {
realPath = fs.realpathSync(resolvedPath);
stat = fs.statSync(realPath);
} catch {
return {
ok: false,
message: "SYSTEM_RUN_DENIED: approval requires an existing script operand",
};
}
if (!stat.isFile()) {
return {
ok: false,
message: "SYSTEM_RUN_DENIED: approval requires a file script operand",
};
}
return {
ok: true,
snapshot: {
argvIndex,
path: realPath,
sha256: hashFileContentsSync(realPath),
},
};
}
function resolveCanonicalApprovalCwdSync(cwd: string):
| {
ok: true;
@@ -135,6 +276,32 @@ export function revalidateApprovedCwdSnapshot(params: { snapshot: ApprovedCwdSna
return sameFileIdentity(params.snapshot.stat, current.snapshot.stat);
}
export function revalidateApprovedMutableFileOperand(params: {
snapshot: SystemRunApprovalFileOperand;
argv: string[];
cwd: string | undefined;
}): boolean {
const operand = params.argv[params.snapshot.argvIndex]?.trim();
if (!operand) {
return false;
}
const resolvedPath = path.resolve(params.cwd ?? process.cwd(), operand);
let realPath: string;
try {
realPath = fs.realpathSync(resolvedPath);
} catch {
return false;
}
if (realPath !== params.snapshot.path) {
return false;
}
try {
return hashFileContentsSync(realPath) === params.snapshot.sha256;
} catch {
return false;
}
}
export function hardenApprovedExecutionPaths(params: {
approvedByAsk: boolean;
argv: string[];
@@ -257,6 +424,13 @@ export function buildSystemRunApprovalPlan(params: {
const rawCommand = hardening.argvChanged
? formatExecCommand(hardening.argv) || null
: command.cmdText.trim() || null;
const mutableFileOperand = resolveMutableFileOperandSnapshotSync({
argv: hardening.argv,
cwd: hardening.cwd,
});
if (!mutableFileOperand.ok) {
return { ok: false, message: mutableFileOperand.message };
}
return {
ok: true,
plan: {
@@ -265,7 +439,8 @@ export function buildSystemRunApprovalPlan(params: {
rawCommand,
agentId: normalizeString(params.agentId),
sessionKey: normalizeString(params.sessionKey),
mutableFileOperand: mutableFileOperand.snapshot ?? undefined,
},
cmdText: command.cmdText,
cmdText: rawCommand ?? formatExecCommand(hardening.argv),
};
}

View File

@@ -2,6 +2,7 @@ import fs from "node:fs";
import os from "node:os";
import path from "node:path";
import { describe, expect, it, type Mock, vi } from "vitest";
import type { SystemRunApprovalPlan } from "../infra/exec-approvals.js";
import { saveExecApprovals } from "../infra/exec-approvals.js";
import type { ExecHostResponse } from "../infra/exec-host.js";
import { buildSystemRunApprovalPlan } from "./invoke-system-run-plan.js";
@@ -235,6 +236,7 @@ describe("handleSystemRunInvoke mac app exec host routing", () => {
runViaResponse?: ExecHostResponse | null;
command?: string[];
rawCommand?: string | null;
systemRunPlan?: SystemRunApprovalPlan | null;
cwd?: string;
security?: "full" | "allowlist";
ask?: "off" | "on-miss" | "always";
@@ -289,6 +291,7 @@ describe("handleSystemRunInvoke mac app exec host routing", () => {
params: {
command: params.command ?? ["echo", "ok"],
rawCommand: params.rawCommand,
systemRunPlan: params.systemRunPlan,
cwd: params.cwd,
approved: params.approved ?? false,
sessionKey: "agent:main:main",
@@ -687,6 +690,76 @@ describe("handleSystemRunInvoke mac app exec host routing", () => {
}
});
it("denies approval-based execution when a script operand changes after approval", async () => {
const tmp = fs.mkdtempSync(path.join(os.tmpdir(), "openclaw-approval-script-drift-"));
const script = path.join(tmp, "run.sh");
fs.writeFileSync(script, "#!/bin/sh\necho SAFE\n");
fs.chmodSync(script, 0o755);
try {
const prepared = buildSystemRunApprovalPlan({
command: ["/bin/sh", "./run.sh"],
cwd: tmp,
});
expect(prepared.ok).toBe(true);
if (!prepared.ok) {
throw new Error("unreachable");
}
fs.writeFileSync(script, "#!/bin/sh\necho PWNED\n");
const { runCommand, sendInvokeResult } = await runSystemInvoke({
preferMacAppExecHost: false,
command: prepared.plan.argv,
rawCommand: prepared.plan.rawCommand,
systemRunPlan: prepared.plan,
cwd: prepared.plan.cwd ?? tmp,
approved: true,
security: "full",
ask: "off",
});
expect(runCommand).not.toHaveBeenCalled();
expectInvokeErrorMessage(sendInvokeResult, {
message: "SYSTEM_RUN_DENIED: approval script operand changed before execution",
exact: true,
});
} finally {
fs.rmSync(tmp, { recursive: true, force: true });
}
});
it("keeps approved shell script execution working when the script is unchanged", async () => {
const tmp = fs.mkdtempSync(path.join(os.tmpdir(), "openclaw-approval-script-stable-"));
const script = path.join(tmp, "run.sh");
fs.writeFileSync(script, "#!/bin/sh\necho SAFE\n");
fs.chmodSync(script, 0o755);
try {
const prepared = buildSystemRunApprovalPlan({
command: ["/bin/sh", "./run.sh"],
cwd: tmp,
});
expect(prepared.ok).toBe(true);
if (!prepared.ok) {
throw new Error("unreachable");
}
const { runCommand, sendInvokeResult } = await runSystemInvoke({
preferMacAppExecHost: false,
command: prepared.plan.argv,
rawCommand: prepared.plan.rawCommand,
systemRunPlan: prepared.plan,
cwd: prepared.plan.cwd ?? tmp,
approved: true,
security: "full",
ask: "off",
});
expect(runCommand).toHaveBeenCalledTimes(1);
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

@@ -15,6 +15,7 @@ import {
import type { ExecHostRequest, ExecHostResponse, ExecHostRunResult } from "../infra/exec-host.js";
import { resolveExecSafeBinRuntimePolicy } from "../infra/exec-safe-bin-runtime-policy.js";
import { sanitizeSystemRunEnvOverrides } from "../infra/host-env-security.js";
import { normalizeSystemRunApprovalPlan } from "../infra/system-run-approval-binding.js";
import { resolveSystemRunCommand } from "../infra/system-run-command.js";
import { logWarn } from "../logger.js";
import { evaluateSystemRunPolicy, resolveExecApprovalDecision } from "./exec-policy.js";
@@ -27,6 +28,7 @@ import {
import {
hardenApprovedExecutionPaths,
revalidateApprovedCwdSnapshot,
revalidateApprovedMutableFileOperand,
type ApprovedCwdSnapshot,
} from "./invoke-system-run-plan.js";
import type {
@@ -63,6 +65,7 @@ type SystemRunParsePhase = {
argv: string[];
shellCommand: string | null;
cmdText: string;
approvalPlan: import("../infra/exec-approvals.js").SystemRunApprovalPlan | null;
agentId: string | undefined;
sessionKey: string;
runId: string;
@@ -92,6 +95,8 @@ type SystemRunPolicyPhase = SystemRunParsePhase & {
const safeBinTrustedDirWarningCache = new Set<string>();
const APPROVAL_CWD_DRIFT_DENIED_MESSAGE =
"SYSTEM_RUN_DENIED: approval cwd changed before execution";
const APPROVAL_SCRIPT_OPERAND_DRIFT_DENIED_MESSAGE =
"SYSTEM_RUN_DENIED: approval script operand changed before execution";
function warnWritableTrustedDirOnce(message: string): void {
if (safeBinTrustedDirWarningCache.has(message)) {
@@ -197,6 +202,17 @@ async function parseSystemRunPhase(
const shellCommand = command.shellCommand;
const cmdText = command.cmdText;
const approvalPlan =
opts.params.systemRunPlan === undefined
? null
: normalizeSystemRunApprovalPlan(opts.params.systemRunPlan);
if (opts.params.systemRunPlan !== undefined && !approvalPlan) {
await opts.sendInvokeResult({
ok: false,
error: { code: "INVALID_REQUEST", message: "systemRunPlan invalid" },
});
return null;
}
const agentId = opts.params.agentId?.trim() || undefined;
const sessionKey = opts.params.sessionKey?.trim() || "node";
const runId = opts.params.runId?.trim() || crypto.randomUUID();
@@ -208,6 +224,7 @@ async function parseSystemRunPhase(
argv: command.argv,
shellCommand,
cmdText,
approvalPlan,
agentId,
sessionKey,
runId,
@@ -361,6 +378,21 @@ async function executeSystemRunPhase(
});
return;
}
if (
phase.approvalPlan?.mutableFileOperand &&
!revalidateApprovedMutableFileOperand({
snapshot: phase.approvalPlan.mutableFileOperand,
argv: phase.argv,
cwd: phase.cwd,
})
) {
logWarn(`security: system.run approval script drift blocked (runId=${phase.runId})`);
await sendSystemRunDenied(opts, phase.execution, {
reason: "approval-required",
message: APPROVAL_SCRIPT_OPERAND_DRIFT_DENIED_MESSAGE,
});
return;
}
const useMacAppExec = opts.preferMacAppExecHost;
if (useMacAppExec) {

View File

@@ -1,8 +1,9 @@
import type { SkillBinTrustEntry } from "../infra/exec-approvals.js";
import type { SkillBinTrustEntry, SystemRunApprovalPlan } from "../infra/exec-approvals.js";
export type SystemRunParams = {
command: string[];
rawCommand?: string | null;
systemRunPlan?: SystemRunApprovalPlan | null;
cwd?: string | null;
env?: Record<string, string>;
timeoutMs?: number | null;