From 087f1584df161c0545a34a21ac741d04204b8107 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Fri, 17 Apr 2026 20:17:52 +0100 Subject: [PATCH] test: streamline system run hotspot coverage --- src/node-host/invoke-system-run-plan.test.ts | 81 +++++++++----------- src/node-host/invoke-system-run.test.ts | 51 ++++++------ src/node-host/invoke-system-run.ts | 24 +++++- 3 files changed, 81 insertions(+), 75 deletions(-) diff --git a/src/node-host/invoke-system-run-plan.test.ts b/src/node-host/invoke-system-run-plan.test.ts index 9b061779157..04da21e1d5a 100644 --- a/src/node-host/invoke-system-run-plan.test.ts +++ b/src/node-host/invoke-system-run-plan.test.ts @@ -1047,55 +1047,44 @@ describe("hardenApprovedExecutionPaths", () => { }); }); - it("allows pnpm dlx package binaries that do not bind a mutable local file", () => { - withFakeRuntimeBin({ - binName: "pnpm", - run: () => { - const tmp = createFixtureDir("openclaw-pnpm-dlx-package-bin-"); - expectApprovalPlanWithoutMutableOperand(["pnpm", "dlx", "cowsay", "hello"], tmp); - }, - }); - }); - - it("allows pnpm dlx package binaries with data-like runtime names", () => { - withFakeRuntimeBin({ - binName: "pnpm", - run: () => { - const tmp = createFixtureDir("openclaw-pnpm-dlx-package-runtime-token-"); - expectApprovalPlanWithoutMutableOperand(["pnpm", "dlx", "cowsay", "node"], tmp); - }, - }); - }); - - it("allows pnpm dlx package binaries with multi-token data-like runtime names", () => { - withFakeRuntimeBin({ - binName: "pnpm", - run: () => { - const tmp = createFixtureDir("openclaw-pnpm-dlx-package-runtime-token-multi-"); - expectApprovalPlanWithoutMutableOperand(["pnpm", "dlx", "cowsay", "node", "hello"], tmp); - }, - }); - }); - - it("allows pnpm dlx package binaries with local file arguments", () => { + it("allows pnpm dlx package binaries that do not bind mutable local files", () => { withFakeRuntimeBins({ binNames: ["pnpm", "eslint"], run: () => { - const tmp = createFixtureDir("openclaw-pnpm-dlx-package-file-"); - 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); - }, - }); - }); - - it("allows pnpm dlx package binaries with interpreter-like data tails", () => { - withFakeRuntimeBin({ - binName: "pnpm", - run: () => { - const tmp = createFixtureDir("openclaw-pnpm-dlx-package-data-tail-"); - fs.writeFileSync(path.join(tmp, "run.ts"), 'console.log("SAFE");\n'); - expectApprovalPlanWithoutMutableOperand(["pnpm", "dlx", "cowsay", "tsx", "./run.ts"], tmp); + const cases = [ + { + prefix: "openclaw-pnpm-dlx-package-bin-", + command: ["pnpm", "dlx", "cowsay", "hello"], + }, + { + prefix: "openclaw-pnpm-dlx-package-runtime-token-", + command: ["pnpm", "dlx", "cowsay", "node"], + }, + { + prefix: "openclaw-pnpm-dlx-package-runtime-token-multi-", + command: ["pnpm", "dlx", "cowsay", "node", "hello"], + }, + { + prefix: "openclaw-pnpm-dlx-package-file-", + command: ["pnpm", "dlx", "eslint", "src/index.ts"], + setup: (tmp: string) => { + fs.mkdirSync(path.join(tmp, "src"), { recursive: true }); + fs.writeFileSync(path.join(tmp, "src", "index.ts"), 'console.log("SAFE");\n'); + }, + }, + { + prefix: "openclaw-pnpm-dlx-package-data-tail-", + command: ["pnpm", "dlx", "cowsay", "tsx", "./run.ts"], + setup: (tmp: string) => { + fs.writeFileSync(path.join(tmp, "run.ts"), 'console.log("SAFE");\n'); + }, + }, + ]; + for (const testCase of cases) { + const tmp = createFixtureDir(testCase.prefix); + testCase.setup?.(tmp); + expectApprovalPlanWithoutMutableOperand(testCase.command, tmp); + } }, }); }); diff --git a/src/node-host/invoke-system-run.test.ts b/src/node-host/invoke-system-run.test.ts index 6b8ee9dee8d..8aba51f6f73 100644 --- a/src/node-host/invoke-system-run.test.ts +++ b/src/node-host/invoke-system-run.test.ts @@ -36,11 +36,8 @@ type MockedSendExecFinishedEvent = Mock; describe("formatSystemRunAllowlistMissMessage", () => { - it("returns legacy allowlist miss message by default", () => { + it("returns the default message and cmd.exe guidance variant", () => { expect(formatSystemRunAllowlistMissMessage()).toBe("SYSTEM_RUN_DENIED: allowlist miss"); - }); - - it("adds Windows shell-wrapper guidance when blocked by cmd.exe policy", () => { expect( formatSystemRunAllowlistMissMessage({ windowsShellWrapperBlocked: true, @@ -955,8 +952,8 @@ describe("handleSystemRunInvoke mac app exec host routing", () => { } }); - for (const runtime of ["bun", "deno", "tsx", "jiti"] as const) { - it(`validates approved ${runtime} script operand stability`, async () => { + it("validates approved runtime script operand stability", async () => { + for (const runtime of ["bun", "deno", "tsx", "jiti"] as const) { await withFakeRuntimeOnPath({ runtime, run: async () => { @@ -1024,8 +1021,8 @@ describe("handleSystemRunInvoke mac app exec host routing", () => { } }, }); - }); - } + } + }); it("denies approval-based execution when tsx is missing a required mutable script binding", async () => { await withFakeRuntimeOnPath({ @@ -1172,7 +1169,13 @@ describe("handleSystemRunInvoke mac app exec host routing", () => { process.platform === "win32" ? ["cmd.exe", "/d", "/s", "/c", "echo ok"] : ["/bin/sh", "-lc", "echo ok"]; - const cases = [ + const cases: Array<{ + label: string; + command?: string[]; + env?: Record; + message: string; + details: string[]; + }> = [ { label: "blocked override", env: { CLASSPATH: "/tmp/evil-classpath" }, @@ -1286,26 +1289,24 @@ describe("handleSystemRunInvoke mac app exec host routing", () => { expectApprovalRequiredDenied({ sendNodeEvent, sendInvokeResult }); } - it("denies env-wrapped shell payloads at the dispatch depth boundary", async () => { + it("denies env-wrapped shell payloads at and past the dispatch depth boundary", async () => { if (process.platform === "win32") { return; } - await expectNestedEnvShellDenied({ - depth: 4, - markerName: "depth4-pwned.txt", - errorLabel: "runCommand should not be called for depth-boundary shell wrappers", - }); - }); - - it("denies nested env shell payloads when wrapper depth is exceeded", async () => { - if (process.platform === "win32") { - return; + for (const testCase of [ + { + depth: 4, + markerName: "depth4-pwned.txt", + errorLabel: "runCommand should not be called for depth-boundary shell wrappers", + }, + { + depth: 5, + markerName: "pwned.txt", + errorLabel: "runCommand should not be called for nested env depth overflow", + }, + ]) { + await expectNestedEnvShellDenied(testCase); } - await expectNestedEnvShellDenied({ - depth: 5, - markerName: "pwned.txt", - errorLabel: "runCommand should not be called for nested env depth overflow", - }); }); it("requires explicit approval for strict inline-eval carriers", async () => { diff --git a/src/node-host/invoke-system-run.ts b/src/node-host/invoke-system-run.ts index e4a00e78070..ef1e969093c 100644 --- a/src/node-host/invoke-system-run.ts +++ b/src/node-host/invoke-system-run.ts @@ -1,5 +1,4 @@ import crypto from "node:crypto"; -import { resolveAgentConfig } from "../agents/agent-scope.js"; import type { OpenClawConfig } from "../config/types.openclaw.js"; import type { GatewayClient } from "../gateway/client.js"; import { @@ -32,6 +31,7 @@ import { import { normalizeSystemRunApprovalPlan } from "../infra/system-run-approval-binding.js"; import { resolveSystemRunCommandRequest } from "../infra/system-run-command.js"; import { logWarn } from "../logger.js"; +import { normalizeAgentId } from "../routing/session-key.js"; import { normalizeOptionalString } from "../shared/string-coerce.js"; import { evaluateSystemRunPolicy, resolveExecApprovalDecision } from "./exec-policy.js"; import { @@ -123,6 +123,7 @@ const APPROVAL_SCRIPT_OPERAND_BINDING_DENIED_MESSAGE = "SYSTEM_RUN_DENIED: approval missing script operand binding"; const APPROVAL_SCRIPT_OPERAND_DRIFT_DENIED_MESSAGE = "SYSTEM_RUN_DENIED: approval script operand changed before execution"; +type ExecToolConfig = NonNullable["exec"]>; function warnWritableTrustedDirOnce(message: string): void { if (safeBinTrustedDirWarningCache.has(message)) { @@ -146,6 +147,23 @@ function normalizeDeniedReason(reason: string | null | undefined): SystemRunDeni } } +function resolveAgentExecConfig( + cfg: OpenClawConfig, + agentId: string | undefined, +): ExecToolConfig | undefined { + if (!agentId) { + return undefined; + } + const normalizedAgentId = normalizeAgentId(agentId); + const entry = cfg.agents?.list?.find( + (candidate) => + candidate !== null && + typeof candidate === "object" && + normalizeAgentId(candidate.id) === normalizedAgentId, + ); + return entry?.tools?.exec; +} + export type HandleSystemRunInvokeOptions = { client: GatewayClient; params: SystemRunParams; @@ -353,9 +371,7 @@ async function evaluateSystemRunPolicyPhase( parsed: SystemRunParsePhase, ): Promise { const cfg = await loadSystemRunConfig(opts); - const agentExec = parsed.agentId - ? resolveAgentConfig(cfg, parsed.agentId)?.tools?.exec - : undefined; + const agentExec = resolveAgentExecConfig(cfg, parsed.agentId); const configuredSecurity = opts.resolveExecSecurity( agentExec?.security ?? cfg.tools?.exec?.security, );