diff --git a/CHANGELOG.md b/CHANGELOG.md index eb3c40f7f69..2c7293f1955 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -88,6 +88,7 @@ Docs: https://docs.openclaw.ai - Telegram/exec approvals: rewrite shared `/approve … allow-always` callback payloads to `/approve … always` before Telegram button rendering so plugin approval IDs still fit Telegram's `callback_data` limit and keep the Allow Always action visible. (#59217) Thanks @jameslcowan. - Cron/exec timeouts: surface timed-out `exec` and `bash` failures in isolated cron runs even when `verbose: off`, including custom session-target cron jobs, so scheduled runs stop failing silently. (#58247) Thanks @skainguyen1412. - Telegram/exec approvals: fall back to the origin session key for async approval followups and keep resume-failure status delivery sanitized so Telegram followups still land without leaking raw exec metadata. (#59351) Thanks @seonang. +- Node-host/exec approvals: bind `pnpm dlx` invocations through the approval planner's mutable-script path so the effective runtime command is resolved for approval instead of being left unbound. (#58374) ## 2026.4.2 diff --git a/src/node-host/invoke-system-run-plan.test.ts b/src/node-host/invoke-system-run-plan.test.ts index 866fb5267fc..d81c0e60aba 100644 --- a/src/node-host/invoke-system-run-plan.test.ts +++ b/src/node-host/invoke-system-run-plan.test.ts @@ -6,6 +6,7 @@ import { formatExecCommand } from "../infra/system-run-command.js"; import { buildSystemRunApprovalPlan, hardenApprovedExecutionPaths, + revalidateApprovedMutableFileOperand, resolveMutableFileOperandSnapshotSync, } from "./invoke-system-run-plan.js"; @@ -173,6 +174,15 @@ function expectRuntimeApprovalDenied(command: string[], cwd: string) { expect(prepared).toEqual(DENIED_RUNTIME_APPROVAL); } +function expectApprovalPlanWithoutMutableOperand(command: string[], cwd: string) { + const prepared = buildSystemRunApprovalPlan({ command, cwd }); + expect(prepared.ok).toBe(true); + if (!prepared.ok) { + throw new Error("unreachable"); + } + expect(prepared.plan.mutableFileOperand).toBeUndefined(); +} + const unsafeRuntimeInvocationCases: UnsafeRuntimeInvocationCase[] = [ { name: "rejects bun package script names that do not bind a concrete file", @@ -256,6 +266,42 @@ const unsafeRuntimeInvocationCases: UnsafeRuntimeInvocationCase[] = [ fs.writeFileSync(path.join(tmp, "run.js"), 'console.log("SAFE")\n'); }, }, + { + name: "rejects pnpm dlx invocations with unrecognized flags that cannot be safely bound", + binName: "pnpm", + tmpPrefix: "openclaw-pnpm-dlx-unknown-flag-", + command: ["pnpm", "dlx", "--future-flag", "tsx", "./run.ts"], + setup: (tmp) => { + fs.writeFileSync(path.join(tmp, "run.ts"), 'console.log("SAFE")\n'); + }, + }, + { + name: "rejects pnpm dlx invocations with unrecognized global flags before dlx when they hide a mutable script", + binName: "pnpm", + tmpPrefix: "openclaw-pnpm-dlx-unknown-prefix-", + command: ["pnpm", "--future-flag", "dlx", "tsx", "./run.ts"], + setup: (tmp) => { + fs.writeFileSync(path.join(tmp, "run.ts"), 'console.log("SAFE")\n'); + }, + }, + { + name: "rejects pnpm dlx invocations with unrecognized global flags that take a value before dlx", + binName: "pnpm", + tmpPrefix: "openclaw-pnpm-dlx-unknown-prefix-value-", + command: ["pnpm", "--future-flag", "value", "dlx", "tsx", "./run.ts"], + setup: (tmp) => { + fs.writeFileSync(path.join(tmp, "run.ts"), 'console.log("SAFE")\n'); + }, + }, + { + name: "rejects pnpm dlx invocations with unrecognized flags after a global option terminator", + binName: "pnpm", + tmpPrefix: "openclaw-pnpm-dlx-global-double-dash-", + command: ["pnpm", "--", "dlx", "--future-flag", "tsx", "./run.ts"], + setup: (tmp) => { + fs.writeFileSync(path.join(tmp, "run.ts"), 'console.log("SAFE")\n'); + }, + }, ]; describe("hardenApprovedExecutionPaths", () => { @@ -487,6 +533,69 @@ describe("hardenApprovedExecutionPaths", () => { initialBody: 'console.log("SAFE");\n', expectedArgvIndex: 3, }, + { + name: "pnpm parallel exec tsx file", + argv: ["pnpm", "--parallel", "exec", "tsx", "./run.ts"], + scriptName: "run.ts", + initialBody: 'console.log("SAFE");\n', + expectedArgvIndex: 4, + }, + { + name: "pnpm workspace-root exec tsx file", + argv: ["pnpm", "-w", "exec", "tsx", "./run.ts"], + scriptName: "run.ts", + initialBody: 'console.log("SAFE");\n', + expectedArgvIndex: 4, + }, + { + name: "pnpm workspace-root dlx tsx file", + argv: ["pnpm", "-w", "dlx", "tsx", "./run.ts"], + scriptName: "run.ts", + initialBody: 'console.log("SAFE");\n', + expectedArgvIndex: 4, + }, + { + name: "pnpm dlx tsx file", + argv: ["pnpm", "dlx", "tsx", "./run.ts"], + scriptName: "run.ts", + initialBody: 'console.log("SAFE");\n', + expectedArgvIndex: 3, + }, + { + name: "pnpm global double-dash dlx tsx file", + argv: ["pnpm", "--", "dlx", "tsx", "./run.ts"], + scriptName: "run.ts", + initialBody: 'console.log("SAFE");\n', + expectedArgvIndex: 4, + }, + { + name: "pnpm pre-dlx package-equals tsx file", + argv: ["pnpm", "--package=tsx", "dlx", "tsx", "./run.ts"], + scriptName: "run.ts", + initialBody: 'console.log("SAFE");\n', + expectedArgvIndex: 4, + }, + { + name: "pnpm reporter dlx package tsx file", + argv: ["pnpm", "--reporter", "silent", "dlx", "--package", "tsx", "tsx", "./run.ts"], + scriptName: "run.ts", + initialBody: 'console.log("SAFE");\n', + expectedArgvIndex: 7, + }, + { + name: "pnpm reporter dlx short-package tsx file", + argv: ["pnpm", "--reporter", "silent", "dlx", "-p", "tsx", "tsx", "./run.ts"], + scriptName: "run.ts", + initialBody: 'console.log("SAFE");\n', + expectedArgvIndex: 7, + }, + { + name: "pnpm silent dlx tsx file", + argv: ["pnpm", "dlx", "-s", "tsx", "./run.ts"], + scriptName: "run.ts", + initialBody: 'console.log("SAFE");\n', + expectedArgvIndex: 4, + }, { name: "pnpm reporter exec tsx file", argv: ["pnpm", "--reporter", "silent", "exec", "tsx", "./run.ts"], @@ -615,6 +724,169 @@ describe("hardenApprovedExecutionPaths", () => { }); }); + it("detects rewritten script operands for pnpm dlx approval plans", () => { + withFakeRuntimeBins({ + binNames: ["pnpm", "tsx"], + run: () => { + withScriptOperandPlanFixture( + { + tmpPrefix: "openclaw-pnpm-dlx-approval-", + fixture: { + name: "pnpm dlx rewritten script", + argv: ["pnpm", "dlx", "tsx", "./run.ts"], + scriptName: "run.ts", + initialBody: 'console.log("SAFE");\n', + expectedArgvIndex: 3, + }, + }, + (fixture, tmp) => { + const prepared = buildSystemRunApprovalPlan({ + command: fixture.command, + cwd: tmp, + }); + expect(prepared.ok).toBe(true); + if (!prepared.ok) { + throw new Error("unreachable"); + } + expect(prepared.plan.mutableFileOperand).toBeDefined(); + fs.writeFileSync(fixture.scriptPath, 'console.log("PWNED");\n'); + expect( + revalidateApprovedMutableFileOperand({ + snapshot: prepared.plan.mutableFileOperand!, + argv: prepared.plan.argv, + cwd: prepared.plan.cwd ?? tmp, + }), + ).toBe(false); + }, + ); + }, + }); + }); + + it("does not bind pnpm dlx shell-mode commands to a mutable file operand", () => { + withFakeRuntimeBins({ + binNames: ["pnpm", "tsx"], + run: () => { + const tmp = fs.mkdtempSync(path.join(os.tmpdir(), "openclaw-pnpm-dlx-shell-mode-")); + try { + fs.writeFileSync(path.join(tmp, "run.ts"), 'console.log("SAFE");\n'); + expect( + resolveMutableFileOperandSnapshotSync({ + argv: ["pnpm", "dlx", "--shell-mode", "tsx ./run.ts"], + cwd: tmp, + shellCommand: null, + }), + ).toEqual({ ok: true, snapshot: null }); + } finally { + fs.rmSync(tmp, { recursive: true, force: true }); + } + }, + }); + }); + + it("allows pnpm dlx package binaries that do not bind a mutable local file", () => { + withFakeRuntimeBin({ + binName: "pnpm", + run: () => { + const tmp = fs.mkdtempSync(path.join(os.tmpdir(), "openclaw-pnpm-dlx-package-bin-")); + try { + expectApprovalPlanWithoutMutableOperand(["pnpm", "dlx", "cowsay", "hello"], tmp); + } finally { + fs.rmSync(tmp, { recursive: true, force: true }); + } + }, + }); + }); + + it("allows pnpm dlx package binaries with data-like runtime names", () => { + withFakeRuntimeBin({ + binName: "pnpm", + run: () => { + const tmp = fs.mkdtempSync( + path.join(os.tmpdir(), "openclaw-pnpm-dlx-package-runtime-token-"), + ); + try { + expectApprovalPlanWithoutMutableOperand(["pnpm", "dlx", "cowsay", "node"], tmp); + } finally { + fs.rmSync(tmp, { recursive: true, force: true }); + } + }, + }); + }); + + it("allows pnpm dlx package binaries with multi-token data-like runtime names", () => { + withFakeRuntimeBin({ + binName: "pnpm", + run: () => { + const tmp = fs.mkdtempSync( + path.join(os.tmpdir(), "openclaw-pnpm-dlx-package-runtime-token-multi-"), + ); + try { + expectApprovalPlanWithoutMutableOperand(["pnpm", "dlx", "cowsay", "node", "hello"], tmp); + } finally { + fs.rmSync(tmp, { recursive: true, force: true }); + } + }, + }); + }); + + it("allows pnpm dlx package binaries with local file arguments", () => { + withFakeRuntimeBins({ + binNames: ["pnpm", "eslint"], + run: () => { + const tmp = fs.mkdtempSync(path.join(os.tmpdir(), "openclaw-pnpm-dlx-package-file-")); + try { + fs.mkdirSync(path.join(tmp, "src"), { recursive: true }); + fs.writeFileSync(path.join(tmp, "src", "index.ts"), 'console.log("SAFE");\n'); + expectApprovalPlanWithoutMutableOperand(["pnpm", "dlx", "eslint", "src/index.ts"], tmp); + } finally { + fs.rmSync(tmp, { recursive: true, force: true }); + } + }, + }); + }); + + it("allows pnpm dlx package binaries with interpreter-like data tails", () => { + withFakeRuntimeBin({ + binName: "pnpm", + run: () => { + const tmp = fs.mkdtempSync(path.join(os.tmpdir(), "openclaw-pnpm-dlx-package-data-tail-")); + try { + fs.writeFileSync(path.join(tmp, "run.ts"), 'console.log("SAFE");\n'); + expectApprovalPlanWithoutMutableOperand( + ["pnpm", "dlx", "cowsay", "tsx", "./run.ts"], + tmp, + ); + } finally { + fs.rmSync(tmp, { recursive: true, force: true }); + } + }, + }); + }); + + it("treats -- as the end of pnpm dlx option parsing", () => { + withFakeRuntimeBins({ + binNames: ["pnpm", "tsx"], + run: () => { + withScriptOperandPlanFixture( + { + tmpPrefix: "openclaw-pnpm-dlx-double-dash-", + fixture: { + name: "pnpm dlx double dash", + argv: ["pnpm", "dlx", "--", "tsx", "./run.ts"], + scriptName: "run.ts", + initialBody: 'console.log("SAFE");\n', + expectedArgvIndex: 4, + }, + }, + (fixture, tmp) => { + expectMutableFileOperandApprovalPlan(fixture, tmp); + }, + ); + }, + }); + }); + it("captures the real shell script operand after value-taking shell flags", () => { const tmp = fs.mkdtempSync(path.join(os.tmpdir(), "openclaw-shell-option-value-")); try { diff --git a/src/node-host/invoke-system-run-plan.ts b/src/node-host/invoke-system-run-plan.ts index bf22b9e0c10..c6f3a72479d 100644 --- a/src/node-host/invoke-system-run-plan.ts +++ b/src/node-host/invoke-system-run-plan.ts @@ -179,12 +179,17 @@ const PNPM_OPTIONS_WITH_VALUE = new Set([ const PNPM_FLAG_OPTIONS = new Set([ "--aggregate-output", "--color", + "--parallel", "--recursive", "--silent", "--workspace-root", "-r", + "-s", + "-w", ]); +const PNPM_DLX_OPTIONS_WITH_VALUE = new Set(["--allow-build", "--package", "-p"]); + type FileOperandCollection = { hits: number[]; sawOptionValueFile: boolean; @@ -345,6 +350,9 @@ function unwrapPnpmExecInvocation(argv: string[]): string[] | null { const tail = argv.slice(idx + 1); return tail[0] === "--" ? (tail.length > 1 ? tail.slice(1) : null) : tail; } + if (token === "dlx") { + return unwrapPnpmDlxInvocation(argv.slice(idx + 1)); + } if (token === "node") { const tail = argv.slice(idx + 1); const normalizedTail = tail[0] === "--" ? tail.slice(1) : tail; @@ -353,7 +361,41 @@ function unwrapPnpmExecInvocation(argv: string[]): string[] | null { return null; } const [flag] = token.toLowerCase().split("=", 2); - if (PNPM_OPTIONS_WITH_VALUE.has(flag)) { + if (PNPM_OPTIONS_WITH_VALUE.has(flag) || PNPM_DLX_OPTIONS_WITH_VALUE.has(flag)) { + idx += token.includes("=") ? 1 : 2; + continue; + } + if (PNPM_FLAG_OPTIONS.has(flag)) { + idx += 1; + continue; + } + return null; + } + return null; +} + +function unwrapPnpmDlxInvocation(argv: string[]): string[] | null { + let idx = 0; + while (idx < argv.length) { + const token = argv[idx]?.trim() ?? ""; + if (!token) { + idx += 1; + continue; + } + if (token === "--") { + const tail = argv.slice(idx + 1); + return tail.length > 0 ? tail : null; + } + if (!token.startsWith("-")) { + // Once dlx-specific flags are stripped, the first positional token is the + // package binary pnpm will execute inside the temporary environment. + return argv.slice(idx); + } + const [flag] = token.toLowerCase().split("=", 2); + if (flag === "-c" || flag === "--shell-mode") { + return null; + } + if (PNPM_OPTIONS_WITH_VALUE.has(flag) || PNPM_DLX_OPTIONS_WITH_VALUE.has(flag)) { idx += token.includes("=") ? 1 : 2; continue; } @@ -780,6 +822,9 @@ function requiresStableInterpreterApprovalBindingWithShellCommand(params: { if (params.shellCommand !== null) { return shellPayloadNeedsStableBinding(params.shellCommand, params.cwd); } + if (pnpmDlxInvocationNeedsFailClosedBinding(params.argv, params.cwd)) { + return true; + } const unwrapped = unwrapArgvForMutableOperand(params.argv); const executable = normalizeExecutableToken(unwrapped.argv[0] ?? ""); if (!executable) { @@ -791,6 +836,84 @@ function requiresStableInterpreterApprovalBindingWithShellCommand(params: { return isMutableScriptRunner(executable); } +function pnpmDlxInvocationNeedsFailClosedBinding(argv: string[], cwd: string | undefined): boolean { + if (normalizePackageManagerExecToken(argv[0] ?? "") !== "pnpm") { + return false; + } + + let idx = 1; + while (idx < argv.length) { + const token = argv[idx]?.trim() ?? ""; + if (!token) { + idx += 1; + continue; + } + if (token === "--") { + idx += 1; + continue; + } + if (!token.startsWith("-")) { + if (token !== "dlx") { + return false; + } + return pnpmDlxTailNeedsFailClosedBinding(argv.slice(idx + 1), cwd); + } + const [flag] = token.toLowerCase().split("=", 2); + if (PNPM_OPTIONS_WITH_VALUE.has(flag) || PNPM_DLX_OPTIONS_WITH_VALUE.has(flag)) { + idx += token.includes("=") ? 1 : 2; + continue; + } + if (PNPM_FLAG_OPTIONS.has(flag)) { + idx += 1; + continue; + } + return true; + } + + return false; +} + +function pnpmDlxTailNeedsFailClosedBinding(argv: string[], cwd: string | undefined): boolean { + let idx = 0; + while (idx < argv.length) { + const token = argv[idx]?.trim() ?? ""; + if (!token) { + idx += 1; + continue; + } + if (token === "--") { + return pnpmDlxTailMayNeedStableBinding(argv.slice(idx + 1), cwd); + } + if (!token.startsWith("-")) { + return pnpmDlxTailMayNeedStableBinding(argv.slice(idx), cwd); + } + const [flag] = token.toLowerCase().split("=", 2); + if (flag === "-c" || flag === "--shell-mode") { + return false; + } + if (PNPM_OPTIONS_WITH_VALUE.has(flag) || PNPM_DLX_OPTIONS_WITH_VALUE.has(flag)) { + idx += token.includes("=") ? 1 : 2; + continue; + } + if (PNPM_FLAG_OPTIONS.has(flag)) { + idx += 1; + continue; + } + return true; + } + + return true; +} + +function pnpmDlxTailMayNeedStableBinding(argv: string[], cwd: string | undefined): boolean { + const snapshot = resolveMutableFileOperandSnapshotSync({ + argv, + cwd, + shellCommand: null, + }); + return snapshot.ok && snapshot.snapshot !== null; +} + export function resolveMutableFileOperandSnapshotSync(params: { argv: string[]; cwd: string | undefined;