diff --git a/src/infra/exec-approvals.test.ts b/src/infra/exec-approvals.test.ts index 9ce901a8482..39ee8b3f3ed 100644 --- a/src/infra/exec-approvals.test.ts +++ b/src/infra/exec-approvals.test.ts @@ -24,6 +24,14 @@ import { type ExecAllowlistEntry, } from "./exec-approvals.js"; +function buildNestedEnvShellCommand(params: { + envExecutable: string; + depth: number; + payload: string; +}): string[] { + return [...Array(params.depth).fill(params.envExecutable), "/bin/sh", "-c", params.payload]; +} + describe("exec approvals allowlist matching", () => { const baseResolution = { rawExecutable: "rg", @@ -311,7 +319,11 @@ describe("exec approvals command resolution", () => { fs.chmodSync(envPath, 0o755); const analysis = analyzeArgvCommand({ - argv: [envPath, envPath, envPath, envPath, envPath, "/bin/sh", "-c", "echo pwned"], + argv: buildNestedEnvShellCommand({ + envExecutable: envPath, + depth: 5, + payload: "echo pwned", + }), cwd: dir, env: makePathEnv(binDir), }); diff --git a/src/infra/exec-wrapper-resolution.ts b/src/infra/exec-wrapper-resolution.ts index 6d09029da05..1f91c3b4a1f 100644 --- a/src/infra/exec-wrapper-resolution.ts +++ b/src/infra/exec-wrapper-resolution.ts @@ -448,6 +448,19 @@ function isSemanticDispatchWrapperUsage(wrapper: string, argv: string[]): boolea return !TRANSPARENT_DISPATCH_WRAPPERS.has(wrapper); } +function blockedDispatchWrapperPlan(params: { + argv: string[]; + wrappers: string[]; + blockedWrapper: string; +}): DispatchWrapperExecutionPlan { + return { + argv: params.argv, + wrappers: params.wrappers, + policyBlocked: true, + blockedWrapper: params.blockedWrapper, + }; +} + export function resolveDispatchWrapperExecutionPlan( argv: string[], maxDepth = MAX_DISPATCH_WRAPPER_DEPTH, @@ -457,36 +470,33 @@ export function resolveDispatchWrapperExecutionPlan( for (let depth = 0; depth < maxDepth; depth += 1) { const unwrap = unwrapKnownDispatchWrapperInvocation(current); if (unwrap.kind === "blocked") { - return { + return blockedDispatchWrapperPlan({ argv: current, wrappers, - policyBlocked: true, blockedWrapper: unwrap.wrapper, - }; + }); } if (unwrap.kind !== "unwrapped" || unwrap.argv.length === 0) { break; } wrappers.push(unwrap.wrapper); if (isSemanticDispatchWrapperUsage(unwrap.wrapper, current)) { - return { + return blockedDispatchWrapperPlan({ argv: current, wrappers, - policyBlocked: true, blockedWrapper: unwrap.wrapper, - }; + }); } current = unwrap.argv; } if (wrappers.length >= maxDepth) { const overflow = unwrapKnownDispatchWrapperInvocation(current); if (overflow.kind === "blocked" || overflow.kind === "unwrapped") { - return { + return blockedDispatchWrapperPlan({ argv: current, wrappers, - policyBlocked: true, blockedWrapper: overflow.wrapper, - }; + }); } } return { argv: current, wrappers, policyBlocked: false }; diff --git a/src/node-host/invoke-system-run.test.ts b/src/node-host/invoke-system-run.test.ts index 675f108b2ee..2d939c7726e 100644 --- a/src/node-host/invoke-system-run.test.ts +++ b/src/node-host/invoke-system-run.test.ts @@ -21,6 +21,30 @@ describe("formatSystemRunAllowlistMissMessage", () => { }); describe("handleSystemRunInvoke mac app exec host routing", () => { + function buildNestedEnvShellCommand(params: { depth: number; payload: string }): string[] { + return [...Array(params.depth).fill("/usr/bin/env"), "/bin/sh", "-c", params.payload]; + } + + async function withTempApprovalsHome(params: { + approvals: Parameters[0]; + run: (ctx: { tempHome: string }) => Promise; + }): Promise { + const tempHome = fs.mkdtempSync(path.join(os.tmpdir(), "openclaw-exec-approvals-")); + const previousOpenClawHome = process.env.OPENCLAW_HOME; + process.env.OPENCLAW_HOME = tempHome; + saveExecApprovals(params.approvals); + try { + return await params.run({ tempHome }); + } finally { + if (previousOpenClawHome === undefined) { + delete process.env.OPENCLAW_HOME; + } else { + process.env.OPENCLAW_HOME = previousOpenClawHome; + } + fs.rmSync(tempHome, { recursive: true, force: true }); + } + } + async function runSystemInvoke(params: { preferMacAppExecHost: boolean; runViaResponse?: ExecHostResponse | null; @@ -254,22 +278,6 @@ describe("handleSystemRunInvoke mac app exec host routing", () => { }); it("denies ./skill-bin even when autoAllowSkills trust entry exists", async () => { - const tempHome = fs.mkdtempSync(path.join(os.tmpdir(), "openclaw-skill-path-spoof-")); - const previousOpenClawHome = process.env.OPENCLAW_HOME; - const skillBinPath = path.join(tempHome, "skill-bin"); - fs.writeFileSync(skillBinPath, "#!/bin/sh\necho should-not-run\n", { mode: 0o755 }); - fs.chmodSync(skillBinPath, 0o755); - process.env.OPENCLAW_HOME = tempHome; - saveExecApprovals({ - version: 1, - defaults: { - security: "allowlist", - ask: "on-miss", - askFallback: "deny", - autoAllowSkills: true, - }, - agents: {}, - }); const runCommand = vi.fn(async () => ({ success: true, stdout: "local-ok", @@ -282,39 +290,47 @@ describe("handleSystemRunInvoke mac app exec host routing", () => { const sendInvokeResult = vi.fn(async () => {}); const sendNodeEvent = vi.fn(async () => {}); - try { - await handleSystemRunInvoke({ - client: {} as never, - params: { - command: ["./skill-bin", "--help"], - cwd: tempHome, - sessionKey: "agent:main:main", + await withTempApprovalsHome({ + approvals: { + version: 1, + defaults: { + security: "allowlist", + ask: "on-miss", + askFallback: "deny", + autoAllowSkills: true, }, - skillBins: { - current: async () => [{ name: "skill-bin", resolvedPath: skillBinPath }], - }, - execHostEnforced: false, - execHostFallbackAllowed: true, - resolveExecSecurity: () => "allowlist", - resolveExecAsk: () => "on-miss", - isCmdExeInvocation: () => false, - sanitizeEnv: () => undefined, - runCommand, - runViaMacAppExecHost: vi.fn(async () => null), - sendNodeEvent, - buildExecEventPayload: (payload) => payload, - sendInvokeResult, - sendExecFinishedEvent: vi.fn(async () => {}), - preferMacAppExecHost: false, - }); - } finally { - if (previousOpenClawHome === undefined) { - delete process.env.OPENCLAW_HOME; - } else { - process.env.OPENCLAW_HOME = previousOpenClawHome; - } - fs.rmSync(tempHome, { recursive: true, force: true }); - } + agents: {}, + }, + run: async ({ tempHome }) => { + const skillBinPath = path.join(tempHome, "skill-bin"); + fs.writeFileSync(skillBinPath, "#!/bin/sh\necho should-not-run\n", { mode: 0o755 }); + fs.chmodSync(skillBinPath, 0o755); + await handleSystemRunInvoke({ + client: {} as never, + params: { + command: ["./skill-bin", "--help"], + cwd: tempHome, + sessionKey: "agent:main:main", + }, + skillBins: { + current: async () => [{ name: "skill-bin", resolvedPath: skillBinPath }], + }, + execHostEnforced: false, + execHostFallbackAllowed: true, + resolveExecSecurity: () => "allowlist", + resolveExecAsk: () => "on-miss", + isCmdExeInvocation: () => false, + sanitizeEnv: () => undefined, + runCommand, + runViaMacAppExecHost: vi.fn(async () => null), + sendNodeEvent, + buildExecEventPayload: (payload) => payload, + sendInvokeResult, + sendExecFinishedEvent: vi.fn(async () => {}), + preferMacAppExecHost: false, + }); + }, + }); expect(runCommand).not.toHaveBeenCalled(); expect(sendNodeEvent).toHaveBeenCalledWith( @@ -353,82 +369,59 @@ describe("handleSystemRunInvoke mac app exec host routing", () => { if (process.platform === "win32") { return; } - const tempHome = fs.mkdtempSync(path.join(os.tmpdir(), "openclaw-env-depth-overflow-")); - const previousOpenClawHome = process.env.OPENCLAW_HOME; - const marker = path.join(tempHome, "pwned.txt"); - process.env.OPENCLAW_HOME = tempHome; - saveExecApprovals({ - version: 1, - defaults: { - security: "allowlist", - ask: "on-miss", - askFallback: "deny", - }, - agents: { - main: { - allowlist: [{ pattern: "/usr/bin/env" }], - }, - }, - }); const runCommand = vi.fn(async () => { - fs.writeFileSync(marker, "executed"); - return { - success: true, - stdout: "local-ok", - stderr: "", - timedOut: false, - truncated: false, - exitCode: 0, - error: null, - }; + throw new Error("runCommand should not be called for nested env depth overflow"); }); const sendInvokeResult = vi.fn(async () => {}); const sendNodeEvent = vi.fn(async () => {}); - try { - await handleSystemRunInvoke({ - client: {} as never, - params: { - command: [ - "/usr/bin/env", - "/usr/bin/env", - "/usr/bin/env", - "/usr/bin/env", - "/usr/bin/env", - "/bin/sh", - "-c", - `echo PWNED > ${marker}`, - ], - sessionKey: "agent:main:main", + await withTempApprovalsHome({ + approvals: { + version: 1, + defaults: { + security: "allowlist", + ask: "on-miss", + askFallback: "deny", }, - skillBins: { - current: async () => [], + agents: { + main: { + allowlist: [{ pattern: "/usr/bin/env" }], + }, }, - execHostEnforced: false, - execHostFallbackAllowed: true, - resolveExecSecurity: () => "allowlist", - resolveExecAsk: () => "on-miss", - isCmdExeInvocation: () => false, - sanitizeEnv: () => undefined, - runCommand, - runViaMacAppExecHost: vi.fn(async () => null), - sendNodeEvent, - buildExecEventPayload: (payload) => payload, - sendInvokeResult, - sendExecFinishedEvent: vi.fn(async () => {}), - preferMacAppExecHost: false, - }); - } finally { - if (previousOpenClawHome === undefined) { - delete process.env.OPENCLAW_HOME; - } else { - process.env.OPENCLAW_HOME = previousOpenClawHome; - } - fs.rmSync(tempHome, { recursive: true, force: true }); - } + }, + run: async ({ tempHome }) => { + const marker = path.join(tempHome, "pwned.txt"); + await handleSystemRunInvoke({ + client: {} as never, + params: { + command: buildNestedEnvShellCommand({ + depth: 5, + payload: `echo PWNED > ${marker}`, + }), + sessionKey: "agent:main:main", + }, + skillBins: { + current: async () => [], + }, + execHostEnforced: false, + execHostFallbackAllowed: true, + resolveExecSecurity: () => "allowlist", + resolveExecAsk: () => "on-miss", + isCmdExeInvocation: () => false, + sanitizeEnv: () => undefined, + runCommand, + runViaMacAppExecHost: vi.fn(async () => null), + sendNodeEvent, + buildExecEventPayload: (payload) => payload, + sendInvokeResult, + sendExecFinishedEvent: vi.fn(async () => {}), + preferMacAppExecHost: false, + }); + expect(fs.existsSync(marker)).toBe(false); + }, + }); expect(runCommand).not.toHaveBeenCalled(); - expect(fs.existsSync(marker)).toBe(false); expect(sendNodeEvent).toHaveBeenCalledWith( expect.anything(), "exec.denied",