From 2f03de029c966cfcb8a79c5b5a30016c42649bbc Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Fri, 13 Mar 2026 12:57:21 +0000 Subject: [PATCH] fix(node-host): harden pnpm approval binding --- CHANGELOG.md | 2 +- docs/tools/exec-approvals.md | 2 + src/node-host/invoke-system-run-plan.test.ts | 37 +++++++++++++-- src/node-host/invoke-system-run-plan.ts | 49 +++++++++++++++++--- 4 files changed, 79 insertions(+), 11 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3bc13f724e6..c5f4354995f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -25,7 +25,7 @@ Docs: https://docs.openclaw.ai - Config/web fetch: restore runtime validation for documented `tools.web.fetch.readability` and `tools.web.fetch.firecrawl` settings so valid web fetch configs no longer fail with unrecognized-key errors. (#42583) Thanks @stim64045-spec. - Signal/config validation: add `channels.signal.groups` schema support so per-group `requireMention`, `tools`, and `toolsBySender` overrides no longer get rejected during config validation. (#27199) Thanks @unisone. - Config/discovery: accept `discovery.wideArea.domain` in strict config validation so unicast DNS-SD gateway configs no longer fail with an unrecognized-key error. (#35615) Thanks @ingyukoh. - +- Security/exec approvals: unwrap more `pnpm` runtime forms during approval binding, including `pnpm --reporter ... exec` and direct `pnpm node` file runs, with matching regression coverage and docs updates. ## 2026.3.12 ### Changes diff --git a/docs/tools/exec-approvals.md b/docs/tools/exec-approvals.md index 0bca1dee488..830dfa6f159 100644 --- a/docs/tools/exec-approvals.md +++ b/docs/tools/exec-approvals.md @@ -271,6 +271,8 @@ Approval-backed interpreter/runtime runs are intentionally conservative: - Exact argv/cwd/env context is always bound. - Direct shell script and direct runtime file forms are best-effort bound to one concrete local file snapshot. +- Common package-manager wrapper forms that still resolve to one direct local file (for example + `pnpm exec`, `pnpm node`, `npm exec`, `npx`) are unwrapped before binding. - If OpenClaw cannot identify exactly one concrete local file for an interpreter/runtime command (for example package scripts, eval forms, runtime-specific loader chains, or ambiguous multi-file forms), approval-backed execution is denied instead of claiming semantic coverage it does not diff --git a/src/node-host/invoke-system-run-plan.test.ts b/src/node-host/invoke-system-run-plan.test.ts index 438163d1d66..0fa76f391dc 100644 --- a/src/node-host/invoke-system-run-plan.test.ts +++ b/src/node-host/invoke-system-run-plan.test.ts @@ -40,6 +40,7 @@ type RuntimeFixture = { initialBody: string; expectedArgvIndex: number; binName?: string; + binNames?: string[]; }; function createScriptOperandFixture(tmp: string, fixture?: RuntimeFixture): ScriptOperandFixture { @@ -356,6 +357,20 @@ describe("hardenApprovedExecutionPaths", () => { initialBody: 'console.log("SAFE");\n', expectedArgvIndex: 3, }, + { + name: "pnpm reporter exec tsx file", + argv: ["pnpm", "--reporter", "silent", "exec", "tsx", "./run.ts"], + scriptName: "run.ts", + initialBody: 'console.log("SAFE");\n', + expectedArgvIndex: 5, + }, + { + name: "pnpm reporter-equals exec tsx file", + argv: ["pnpm", "--reporter=silent", "exec", "tsx", "./run.ts"], + scriptName: "run.ts", + initialBody: 'console.log("SAFE");\n', + expectedArgvIndex: 4, + }, { name: "pnpm js shim exec tsx file", argv: ["./pnpm.js", "exec", "tsx", "./run.ts"], @@ -370,6 +385,22 @@ describe("hardenApprovedExecutionPaths", () => { initialBody: 'console.log("SAFE");\n', expectedArgvIndex: 4, }, + { + name: "pnpm node file", + argv: ["pnpm", "node", "./run.js"], + scriptName: "run.js", + initialBody: 'console.log("SAFE");\n', + expectedArgvIndex: 2, + binNames: ["pnpm", "node"], + }, + { + name: "pnpm node double-dash file", + argv: ["pnpm", "node", "--", "./run.js"], + scriptName: "run.js", + initialBody: 'console.log("SAFE");\n', + expectedArgvIndex: 3, + binNames: ["pnpm", "node"], + }, { name: "npx tsx file", argv: ["npx", "tsx", "./run.ts"], @@ -395,9 +426,9 @@ describe("hardenApprovedExecutionPaths", () => { for (const runtimeCase of mutableOperandCases) { it(`captures mutable ${runtimeCase.name} operands in approval plans`, () => { - const binNames = runtimeCase.binName - ? [runtimeCase.binName] - : ["bunx", "pnpm", "npm", "npx", "tsx"]; + const binNames = + runtimeCase.binNames ?? + (runtimeCase.binName ? [runtimeCase.binName] : ["bunx", "pnpm", "npm", "npx", "tsx"]); withFakeRuntimeBins({ binNames, run: () => { diff --git a/src/node-host/invoke-system-run-plan.ts b/src/node-host/invoke-system-run-plan.ts index 867ea9f696f..3fe37676776 100644 --- a/src/node-host/invoke-system-run-plan.ts +++ b/src/node-host/invoke-system-run-plan.ts @@ -164,6 +164,26 @@ const NPM_EXEC_FLAG_OPTIONS = new Set([ "-y", ]); +const PNPM_OPTIONS_WITH_VALUE = new Set([ + "--config", + "--dir", + "--filter", + "--reporter", + "--stream", + "--test-pattern", + "--workspace-concurrency", + "-C", +]); + +const PNPM_FLAG_OPTIONS = new Set([ + "--aggregate-output", + "--color", + "--recursive", + "--silent", + "--workspace-root", + "-r", +]); + type FileOperandCollection = { hits: number[]; sawOptionValueFile: boolean; @@ -299,6 +319,8 @@ function normalizePackageManagerExecToken(token: string): string { if (!normalized) { return normalized; } + // Approval binding only promises best-effort recovery of the effective runtime + // command for common package-manager shims; it is not full package-manager semantics. return normalized.replace(/\.(?:c|m)?js$/i, ""); } @@ -315,17 +337,30 @@ function unwrapPnpmExecInvocation(argv: string[]): string[] | null { continue; } if (!token.startsWith("-")) { - if (token !== "exec" || idx + 1 >= argv.length) { - return null; + if (token === "exec") { + if (idx + 1 >= argv.length) { + return null; + } + const tail = argv.slice(idx + 1); + return tail[0] === "--" ? (tail.length > 1 ? tail.slice(1) : null) : tail; } - const tail = argv.slice(idx + 1); - return tail[0] === "--" ? (tail.length > 1 ? tail.slice(1) : null) : tail; + if (token === "node") { + const tail = argv.slice(idx + 1); + const normalizedTail = tail[0] === "--" ? tail.slice(1) : tail; + return ["node", ...normalizedTail]; + } + return null; } - if ((token === "-C" || token === "--dir" || token === "--filter") && !token.includes("=")) { - idx += 2; + const [flag] = token.toLowerCase().split("=", 2); + if (PNPM_OPTIONS_WITH_VALUE.has(flag)) { + idx += token.includes("=") ? 1 : 2; continue; } - idx += 1; + if (PNPM_FLAG_OPTIONS.has(flag)) { + idx += 1; + continue; + } + return null; } return null; }