From 7bd2761b92eef20011a719aaff99caba319e90ef Mon Sep 17 00:00:00 2001 From: Jacob Tomlinson Date: Tue, 31 Mar 2026 02:58:17 -0700 Subject: [PATCH] Exec approvals: detect command carriers in strict inline eval (#57842) * Exec approvals: detect command carriers in strict inline eval * Exec approvals: cover carrier option edge cases * Exec approvals: cover make and find carriers * Exec approvals: catch attached eval flags * Exec approvals: keep sed -E out of inline eval * Exec approvals: treat sed in-place flags as optional --- src/infra/exec-inline-eval.test.ts | 45 +++++++ src/infra/exec-inline-eval.ts | 172 +++++++++++++++++++----- src/node-host/invoke-system-run.test.ts | 139 ++++++++++++++++--- src/security/audit.test.ts | 6 +- 4 files changed, 311 insertions(+), 51 deletions(-) diff --git a/src/infra/exec-inline-eval.test.ts b/src/infra/exec-inline-eval.test.ts index ed8b67958ef..35cf9678d49 100644 --- a/src/infra/exec-inline-eval.test.ts +++ b/src/infra/exec-inline-eval.test.ts @@ -19,10 +19,51 @@ describe("exec inline eval detection", () => { expect(describeInterpreterInlineEval(hit!)).toBe(expected); }); + it.each([ + { argv: ["awk", 'BEGIN{system("id")}', "/dev/null"], expected: "awk inline program" }, + { + argv: ["awk", "-F", ",", 'BEGIN{system("id")}', "/dev/null"], + expected: "awk inline program", + }, + { argv: ["gawk", "-e", 'BEGIN{system("id")}', "/dev/null"], expected: "gawk -e" }, + { + argv: ["gawk", "-f", "library.awk", '--source=BEGIN{system("id")}', "/dev/null"], + expected: "gawk --source", + }, + { argv: ["find", ".", "-exec", "id", "{}", ";"], expected: "find -exec" }, + { argv: ["find", "--", ".", "-exec", "id", "{}", ";"], expected: "find -exec" }, + { argv: ["find", ".", "-ok", "id", "{}", ";"], expected: "find -ok" }, + { argv: ["find", ".", "-okdir", "id", "{}", ";"], expected: "find -okdir" }, + { argv: ["xargs", "id"], expected: "xargs inline command" }, + { argv: ["xargs", "-I", "{}", "sh", "-c", "id"], expected: "xargs inline command" }, + { argv: ["xargs", "--replace", "id"], expected: "xargs inline command" }, + { argv: ["make", "-f", "evil.mk"], expected: "make -f" }, + { argv: ["make", "-E", "$(shell id)"], expected: "make -E" }, + { argv: ["make", "-E$(shell id)"], expected: "make -E" }, + { argv: ["make", "--eval=$(shell id)"], expected: "make --eval" }, + { argv: ["sed", "s/.*/id/e", "/dev/null"], expected: "sed inline program" }, + { argv: ["gsed", "-e", "s/.*/id/e", "/dev/null"], expected: "gsed -e" }, + { argv: ["sed", "-es/.*/id/e", "/dev/null"], expected: "sed -e" }, + ] as const)("detects command carriers for %j", ({ argv, expected }) => { + const hit = detectInterpreterInlineEvalArgv([...argv]); + expect(hit).not.toBeNull(); + expect(describeInterpreterInlineEval(hit!)).toBe(expected); + }); + it("ignores normal script execution", () => { expect(detectInterpreterInlineEvalArgv(["python3", "script.py"])).toBeNull(); expect(detectInterpreterInlineEvalArgv(["node", "script.js"])).toBeNull(); expect(detectInterpreterInlineEvalArgv(["awk", "-f", "script.awk", "data.csv"])).toBeNull(); + expect(detectInterpreterInlineEvalArgv(["find", ".", "-name", "*.ts"])).toBeNull(); + expect(detectInterpreterInlineEvalArgv(["xargs", "-0"])).toBeNull(); + expect(detectInterpreterInlineEvalArgv(["make", "test"])).toBeNull(); + expect(detectInterpreterInlineEvalArgv(["sed", "-f", "script.sed", "input.txt"])).toBeNull(); + expect( + detectInterpreterInlineEvalArgv(["sed", "-i", "-f", "script.sed", "input.txt"]), + ).toBeNull(); + expect( + detectInterpreterInlineEvalArgv(["sed", "-E", "-f", "script.sed", "input.txt"]), + ).toBeNull(); }); it("matches interpreter-like allowlist patterns", () => { @@ -32,6 +73,10 @@ describe("exec inline eval detection", () => { expect(isInterpreterLikeAllowlistPattern("**/gawk")).toBe(true); expect(isInterpreterLikeAllowlistPattern("/usr/bin/mawk")).toBe(true); expect(isInterpreterLikeAllowlistPattern("nawk")).toBe(true); + expect(isInterpreterLikeAllowlistPattern("**/find")).toBe(true); + expect(isInterpreterLikeAllowlistPattern("xargs.exe")).toBe(true); + expect(isInterpreterLikeAllowlistPattern("/usr/bin/gmake")).toBe(true); + expect(isInterpreterLikeAllowlistPattern("**/gsed")).toBe(true); expect(isInterpreterLikeAllowlistPattern("/usr/bin/rg")).toBe(false); }); }); diff --git a/src/infra/exec-inline-eval.ts b/src/infra/exec-inline-eval.ts index ca26afdc3ba..de893190eb1 100644 --- a/src/infra/exec-inline-eval.ts +++ b/src/infra/exec-inline-eval.ts @@ -7,31 +7,69 @@ export type InterpreterInlineEvalHit = { argv: string[]; }; +type PrefixFlagSpec = { + label: string; + prefix: string; +}; + type InterpreterFlagSpec = { names: readonly string[]; exactFlags: ReadonlySet; - prefixFlags?: readonly string[]; + rawExactFlags?: ReadonlyMap; + rawPrefixFlags?: readonly PrefixFlagSpec[]; + prefixFlags?: readonly PrefixFlagSpec[]; + scanPastDoubleDash?: boolean; }; type PositionalInterpreterSpec = { names: readonly string[]; - fileFlags: ReadonlySet; + fileFlags?: ReadonlySet; fileFlagPrefixes?: readonly string[]; - exactValueFlags: ReadonlySet; + exactValueFlags?: ReadonlySet; + exactOptionalValueFlags?: ReadonlySet; prefixValueFlags?: readonly string[]; + flag: "" | ""; }; -const INTERPRETER_INLINE_EVAL_SPECS: readonly InterpreterFlagSpec[] = [ +const FLAG_INTERPRETER_INLINE_EVAL_SPECS: readonly InterpreterFlagSpec[] = [ { names: ["python", "python2", "python3", "pypy", "pypy3"], exactFlags: new Set(["-c"]) }, { names: ["node", "nodejs", "bun", "deno"], exactFlags: new Set(["-e", "--eval", "-p", "--print"]), }, + { + names: ["awk", "gawk", "mawk", "nawk"], + exactFlags: new Set(["-e", "--source"]), + prefixFlags: [{ label: "--source", prefix: "--source=" }], + }, { names: ["ruby"], exactFlags: new Set(["-e"]) }, { names: ["perl"], exactFlags: new Set(["-e", "-E"]) }, { names: ["php"], exactFlags: new Set(["-r"]) }, { names: ["lua"], exactFlags: new Set(["-e"]) }, { names: ["osascript"], exactFlags: new Set(["-e"]) }, + { + names: ["find"], + exactFlags: new Set(["-exec", "-execdir", "-ok", "-okdir"]), + scanPastDoubleDash: true, + }, + { + names: ["make", "gmake"], + exactFlags: new Set(["-f", "--file", "--makefile", "--eval"]), + rawExactFlags: new Map([["-E", "-E"]]), + rawPrefixFlags: [{ label: "-E", prefix: "-E" }], + prefixFlags: [ + { label: "-f", prefix: "-f" }, + { label: "--file", prefix: "--file=" }, + { label: "--makefile", prefix: "--makefile=" }, + { label: "--eval", prefix: "--eval=" }, + ], + }, + { + names: ["sed", "gsed"], + exactFlags: new Set(), + rawExactFlags: new Map([["-e", "-e"]]), + rawPrefixFlags: [{ label: "-e", prefix: "-e" }], + }, ]; const POSITIONAL_INTERPRETER_INLINE_EVAL_SPECS: readonly PositionalInterpreterSpec[] = [ @@ -53,18 +91,69 @@ const POSITIONAL_INTERPRETER_INLINE_EVAL_SPECS: readonly PositionalInterpreterSp "-W", ]), prefixValueFlags: ["-F", "--field-separator=", "-v", "--assign=", "--include=", "--load="], + flag: "", + }, + { + names: ["xargs"], + exactValueFlags: new Set([ + "-a", + "--arg-file", + "-d", + "--delimiter", + "-E", + "-I", + "-L", + "--max-lines", + "-n", + "--max-args", + "-P", + "--max-procs", + "-s", + "--max-chars", + ]), + exactOptionalValueFlags: new Set(["--eof", "--replace"]), + prefixValueFlags: [ + "-a", + "--arg-file=", + "-d", + "--delimiter=", + "-E", + "--eof=", + "-I", + "--replace=", + "-i", + "-L", + "--max-lines=", + "-l", + "-n", + "--max-args=", + "-P", + "--max-procs=", + "-s", + "--max-chars=", + ], + flag: "", + }, + { + names: ["sed", "gsed"], + fileFlags: new Set(["-f", "--file"]), + fileFlagPrefixes: ["-f", "--file="], + exactValueFlags: new Set(["-f", "--file", "-l", "--line-length"]), + exactOptionalValueFlags: new Set(["-i", "--in-place"]), + prefixValueFlags: ["-f", "--file=", "--in-place=", "--line-length="], + flag: "", }, ]; const INTERPRETER_ALLOWLIST_NAMES = new Set( - INTERPRETER_INLINE_EVAL_SPECS.flatMap((entry) => entry.names).concat( + FLAG_INTERPRETER_INLINE_EVAL_SPECS.flatMap((entry) => entry.names).concat( POSITIONAL_INTERPRETER_INLINE_EVAL_SPECS.flatMap((entry) => entry.names), ), ); function findInterpreterSpec(executable: string): InterpreterFlagSpec | null { const normalized = normalizeExecutableToken(executable); - for (const spec of INTERPRETER_INLINE_EVAL_SPECS) { + for (const spec of FLAG_INTERPRETER_INLINE_EVAL_SPECS) { if (spec.names.includes(normalized)) { return spec; } @@ -82,6 +171,19 @@ function findPositionalInterpreterSpec(executable: string): PositionalInterprete return null; } +function createInlineEvalHit( + executable: string, + argv: string[], + flag: string, +): InterpreterInlineEvalHit { + return { + executable, + normalizedExecutable: normalizeExecutableToken(executable), + flag, + argv, + }; +} + export function detectInterpreterInlineEvalArgv( argv: string[] | undefined | null, ): InterpreterInlineEvalHit | null { @@ -100,24 +202,30 @@ export function detectInterpreterInlineEvalArgv( continue; } if (token === "--") { + if (spec.scanPastDoubleDash) { + continue; + } break; } + const rawExactFlag = spec.rawExactFlags?.get(token); + if (rawExactFlag) { + return createInlineEvalHit(executable, argv, rawExactFlag); + } + const rawPrefixFlag = spec.rawPrefixFlags?.find( + ({ prefix }) => token.startsWith(prefix) && token.length > prefix.length, + ); + if (rawPrefixFlag) { + return createInlineEvalHit(executable, argv, rawPrefixFlag.label); + } const lower = token.toLowerCase(); if (spec.exactFlags.has(lower)) { - return { - executable, - normalizedExecutable: normalizeExecutableToken(executable), - flag: lower, - argv, - }; + return createInlineEvalHit(executable, argv, lower); } - if (spec.prefixFlags?.some((prefix) => lower.startsWith(prefix))) { - return { - executable, - normalizedExecutable: normalizeExecutableToken(executable), - flag: lower, - argv, - }; + const prefixFlag = spec.prefixFlags?.find( + ({ prefix }) => lower.startsWith(prefix) && lower.length > prefix.length, + ); + if (prefixFlag) { + return createInlineEvalHit(executable, argv, prefixFlag.label); } } } @@ -126,6 +234,8 @@ export function detectInterpreterInlineEvalArgv( if (!positionalSpec) { return null; } + + // These tools can execute user-provided programs once the first non-option token is reached. for (let idx = 1; idx < argv.length; idx += 1) { const token = argv[idx]?.trim(); if (!token) { @@ -136,14 +246,9 @@ export function detectInterpreterInlineEvalArgv( if (!nextToken) { return null; } - return { - executable, - normalizedExecutable: normalizeExecutableToken(executable), - flag: "", - argv, - }; + return createInlineEvalHit(executable, argv, positionalSpec.flag); } - if (positionalSpec.fileFlags.has(token)) { + if (positionalSpec.fileFlags?.has(token)) { return null; } if ( @@ -153,10 +258,13 @@ export function detectInterpreterInlineEvalArgv( ) { return null; } - if (positionalSpec.exactValueFlags.has(token)) { + if (positionalSpec.exactValueFlags?.has(token)) { idx += 1; continue; } + if (positionalSpec.exactOptionalValueFlags?.has(token)) { + continue; + } if ( positionalSpec.prefixValueFlags?.some( (prefix) => token.startsWith(prefix) && token.length > prefix.length, @@ -167,17 +275,15 @@ export function detectInterpreterInlineEvalArgv( if (token.startsWith("-")) { continue; } - return { - executable, - normalizedExecutable: normalizeExecutableToken(executable), - flag: "", - argv, - }; + return createInlineEvalHit(executable, argv, positionalSpec.flag); } return null; } export function describeInterpreterInlineEval(hit: InterpreterInlineEvalHit): string { + if (hit.flag === "") { + return `${hit.normalizedExecutable} inline command`; + } if (hit.flag === "") { return `${hit.normalizedExecutable} inline program`; } diff --git a/src/node-host/invoke-system-run.test.ts b/src/node-host/invoke-system-run.test.ts index 4c1be533edb..f96890bd939 100644 --- a/src/node-host/invoke-system-run.test.ts +++ b/src/node-host/invoke-system-run.test.ts @@ -66,6 +66,14 @@ describe("handleSystemRunInvoke mac app exec host routing", () => { }; } + function createTempExecutable(params: { dir: string; name: string }): string { + const fileName = process.platform === "win32" ? `${params.name}.exe` : params.name; + const executablePath = path.join(params.dir, fileName); + fs.writeFileSync(executablePath, ""); + fs.chmodSync(executablePath, 0o755); + return executablePath; + } + function expectInvokeOk( sendInvokeResult: MockedSendInvokeResult, params?: { payloadContains?: string }, @@ -366,6 +374,7 @@ describe("handleSystemRunInvoke mac app exec host routing", () => { cwd?: string; security?: "full" | "allowlist"; ask?: "off" | "on-miss" | "always"; + approvalDecision?: "allow" | "allow-always" | "deny" | null; approved?: boolean; runCommand?: HandleSystemRunInvokeOptions["runCommand"]; runViaMacAppExecHost?: HandleSystemRunInvokeOptions["runViaMacAppExecHost"]; @@ -420,6 +429,7 @@ describe("handleSystemRunInvoke mac app exec host routing", () => { rawCommand: params.rawCommand, systemRunPlan: params.systemRunPlan, cwd: params.cwd, + approvalDecision: params.approvalDecision, approved: params.approved ?? false, sessionKey: "agent:main:main", }, @@ -1254,7 +1264,32 @@ describe("handleSystemRunInvoke mac app exec host routing", () => { }); }); - it("requires explicit approval for inline eval when strictInlineEval is enabled", async () => { + it.each([ + { + command: ["python3", "-c", "print('hi')"], + expected: "python3 -c requires explicit approval in strictInlineEval mode", + }, + { + command: ["awk", 'BEGIN{system("id")}', "/dev/null"], + expected: "awk inline program requires explicit approval in strictInlineEval mode", + }, + { + command: ["find", ".", "-exec", "id", "{}", ";"], + expected: "find -exec requires explicit approval in strictInlineEval mode", + }, + { + command: ["xargs", "id"], + expected: "xargs inline command requires explicit approval in strictInlineEval mode", + }, + { + command: ["make", "-f", "evil.mk"], + expected: "make -f requires explicit approval in strictInlineEval mode", + }, + { + command: ["sed", "s/.*/id/e", "/dev/null"], + expected: "sed inline program requires explicit approval in strictInlineEval mode", + }, + ] as const)("requires explicit approval for strict inline-eval carrier %j", async (testCase) => { setRuntimeConfigSnapshot({ tools: { exec: { @@ -1265,7 +1300,7 @@ describe("handleSystemRunInvoke mac app exec host routing", () => { try { const { runCommand, sendInvokeResult, sendNodeEvent } = await runSystemInvoke({ preferMacAppExecHost: false, - command: ["python3", "-c", "print('hi')"], + command: [...testCase.command], security: "full", ask: "off", }); @@ -1277,14 +1312,64 @@ describe("handleSystemRunInvoke mac app exec host routing", () => { expect.objectContaining({ reason: "approval-required" }), ); expectInvokeErrorMessage(sendInvokeResult, { - message: "python3 -c requires explicit approval in strictInlineEval mode", + message: testCase.expected, }); } finally { clearRuntimeConfigSnapshot(); } }); - it("does not persist allow-always interpreter approvals when strictInlineEval is enabled", async () => { + it.each([ + { executable: "python3", args: ["-c", "print('hi')"] }, + { executable: "awk", args: ['BEGIN{system("id")}', "/dev/null"] }, + { executable: "find", args: [".", "-exec", "id", "{}", ";"] }, + { executable: "xargs", args: ["id"] }, + { executable: "sed", args: ["s/.*/id/e", "/dev/null"] }, + ] as const)( + "does not persist allow-always approvals for strict inline-eval carrier %j", + async (testCase) => { + setRuntimeConfigSnapshot({ + tools: { + exec: { + strictInlineEval: true, + }, + }, + }); + try { + await withTempApprovalsHome({ + approvals: createAllowlistOnMissApprovals(), + run: async () => { + const tempDir = fs.mkdtempSync(path.join(os.tmpdir(), "openclaw-inline-eval-bin-")); + try { + const executablePath = createTempExecutable({ + dir: tempDir, + name: testCase.executable, + }); + const { runCommand, sendInvokeResult } = await runSystemInvoke({ + preferMacAppExecHost: false, + command: [executablePath, ...testCase.args], + security: "allowlist", + ask: "on-miss", + approvalDecision: "allow-always", + approved: true, + runCommand: vi.fn(async () => createLocalRunResult("inline-eval-ok")), + }); + + expect(runCommand).toHaveBeenCalledTimes(1); + expectInvokeOk(sendInvokeResult, { payloadContains: "inline-eval-ok" }); + expect(loadExecApprovals().agents?.main?.allowlist ?? []).toEqual([]); + } finally { + fs.rmSync(tempDir, { recursive: true, force: true }); + } + }, + }); + } finally { + clearRuntimeConfigSnapshot(); + } + }, + ); + + it("does not persist allow-always approvals for strict inline-eval make carriers", async () => { setRuntimeConfigSnapshot({ tools: { exec: { @@ -1296,18 +1381,42 @@ describe("handleSystemRunInvoke mac app exec host routing", () => { await withTempApprovalsHome({ approvals: createAllowlistOnMissApprovals(), run: async () => { - const { runCommand, sendInvokeResult } = await runSystemInvoke({ - preferMacAppExecHost: false, - command: ["python3", "-c", "print('hi')"], - security: "allowlist", - ask: "on-miss", - approved: true, - runCommand: vi.fn(async () => createLocalRunResult("inline-eval-ok")), - }); + const tempDir = fs.mkdtempSync(path.join(os.tmpdir(), "openclaw-inline-eval-make-")); + try { + const executablePath = createTempExecutable({ + dir: tempDir, + name: "make", + }); + const makefilePath = path.join(tempDir, "Makefile"); + fs.writeFileSync(makefilePath, "all:\n\t@echo inline-eval-ok\n"); + const prepared = buildSystemRunApprovalPlan({ + command: [executablePath, "-f", makefilePath], + cwd: tempDir, + }); + expect(prepared.ok).toBe(true); + if (!prepared.ok) { + throw new Error("unreachable"); + } - expect(runCommand).toHaveBeenCalledTimes(1); - expectInvokeOk(sendInvokeResult, { payloadContains: "inline-eval-ok" }); - expect(loadExecApprovals().agents?.main?.allowlist ?? []).toEqual([]); + const { runCommand, sendInvokeResult } = await runSystemInvoke({ + preferMacAppExecHost: false, + command: prepared.plan.argv, + rawCommand: prepared.plan.commandText, + systemRunPlan: prepared.plan, + cwd: prepared.plan.cwd ?? tempDir, + security: "allowlist", + ask: "on-miss", + approvalDecision: "allow-always", + approved: true, + runCommand: vi.fn(async () => createLocalRunResult("inline-eval-ok")), + }); + + expect(runCommand).toHaveBeenCalledTimes(1); + expectInvokeOk(sendInvokeResult, { payloadContains: "inline-eval-ok" }); + expect(loadExecApprovals().agents?.main?.allowlist ?? []).toEqual([]); + } finally { + fs.rmSync(tempDir, { recursive: true, force: true }); + } }, }); } finally { diff --git a/src/security/audit.test.ts b/src/security/audit.test.ts index b66531b71fc..bb8c8e652df 100644 --- a/src/security/audit.test.ts +++ b/src/security/audit.test.ts @@ -917,10 +917,10 @@ description: test skill version: 1, agents: { main: { - allowlist: [{ pattern: "/usr/bin/python3" }], + allowlist: [{ pattern: "/usr/bin/python3" }, { pattern: "/usr/bin/awk" }], }, ops: { - allowlist: [{ pattern: "/usr/local/bin/awk" }], + allowlist: [{ pattern: "/usr/local/bin/node" }, { pattern: "/usr/local/bin/find" }], }, }, }); @@ -942,7 +942,7 @@ description: test skill version: 1, agents: { main: { - allowlist: [{ pattern: "/usr/bin/python3" }], + allowlist: [{ pattern: "/usr/bin/python3" }, { pattern: "/usr/bin/xargs" }], }, }, });