mirror of https://github.com/openclaw/openclaw.git
node-host: bind pnpm dlx approval scripts (#58374)
* node-host: bind pnpm dlx approval scripts * node-host: cover pnpm dlx package alias * node-host: cover pnpm dlx flag forms * node-host: fail closed on unsafe pnpm dlx flags * node-host: narrow pnpm dlx fail-closed guard * node-host: scan pnpm dlx past global -- * node-host: allow pnpm dlx file args * node-host: allow pnpm dlx data args * node-host: fail closed on unknown pnpm dlx flags * node-host: support pnpm workspace-root flag * node-host: restrict pnpm dlx tail scan * node-host: support pnpm parallel flag * changelog: node-host pnpm dlx approval binding (#58374)
This commit is contained in:
Jacob Tomlinson
GitHub
parent
7cea7c2970
commit
176c059b05
|
|
@ -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
|
||||
|
||||
|
|
|
|||
|
|
@ -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 {
|
||||
|
|
|
|||
|
|
@ -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;
|
||||
|
|
|
|||
Loading…
Reference in New Issue