diff --git a/CHANGELOG.md b/CHANGELOG.md index 8d7fc9e0607..ffc9794fc6c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -45,6 +45,7 @@ Docs: https://docs.openclaw.ai - Security/Native images: enforce `tools.fs.workspaceOnly` for native prompt image auto-load (including history refs), preventing out-of-workspace sandbox mounts from being implicitly ingested as vision input. This ships in the next npm release. Thanks @tdjackey for reporting. - Security/Exec approvals: bind `system.run` command display/approval text to full argv when shell-wrapper inline payloads carry positional argv values, and reject payload-only `rawCommand` mismatches for those wrapper-carrier forms, preventing hidden command execution under misleading approval text. This ships in the next npm release. Thanks @tdjackey for reporting. - Security/Exec companion host: forward canonical `system.run` display text (not payload-only shell snippets) to the macOS exec host, and enforce rawCommand/argv consistency there for shell-wrapper positional-argv carriers and env-modifier preludes, preventing companion-side approval/display drift. This ships in the next npm release. Thanks @tdjackey for reporting. +- Security/Exec approvals: fail closed when transparent dispatch-wrapper unwrapping exceeds the depth cap, so nested `/usr/bin/env` chains cannot bypass shell-wrapper approval gating in `allowlist` + `ask=on-miss` mode. This ships in the next npm release. Thanks @tdjackey for reporting. - Security/Exec: limit default safe-bin trusted directories to immutable system paths (`/bin`, `/usr/bin`) and require explicit opt-in (`tools.exec.safeBinTrustedDirs`) for package-manager/user bin paths (for example Homebrew), add security-audit findings for risky trusted-dir choices, warn at runtime when explicitly trusted dirs are group/world writable, and add doctor hints when configured `safeBins` resolve outside trusted dirs. This ships in the next npm release. Thanks @tdjackey for reporting. - Telegram/Media fetch: prioritize IPv4 before IPv6 in SSRF pinned DNS address ordering so media downloads still work on hosts with broken IPv6 routing. (#24295, #23975) Thanks @Glucksberg. - Telegram/Replies: when markdown formatting renders to empty HTML (for example syntax-only chunks in threaded replies), retry delivery with plain text, and fail loud when both formatted and plain payloads are empty to avoid false delivered states. (#25096, #25091) Thanks @Glucksberg. diff --git a/src/infra/exec-approvals.test.ts b/src/infra/exec-approvals.test.ts index 407261f43a3..9ce901a8482 100644 --- a/src/infra/exec-approvals.test.ts +++ b/src/infra/exec-approvals.test.ts @@ -299,6 +299,36 @@ describe("exec approvals command resolution", () => { expect(allowlistEval.segmentSatisfiedBy).toEqual([null]); }); + it("fails closed when transparent env wrappers exceed unwrap depth", () => { + if (process.platform === "win32") { + return; + } + const dir = makeTempDir(); + const binDir = path.join(dir, "bin"); + fs.mkdirSync(binDir, { recursive: true }); + const envPath = path.join(binDir, "env"); + fs.writeFileSync(envPath, "#!/bin/sh\n"); + fs.chmodSync(envPath, 0o755); + + const analysis = analyzeArgvCommand({ + argv: [envPath, envPath, envPath, envPath, envPath, "/bin/sh", "-c", "echo pwned"], + cwd: dir, + env: makePathEnv(binDir), + }); + const allowlistEval = evaluateExecAllowlist({ + analysis, + allowlist: [{ pattern: envPath }], + safeBins: normalizeSafeBins([]), + cwd: dir, + }); + + expect(analysis.ok).toBe(true); + expect(analysis.segments[0]?.resolution?.policyBlocked).toBe(true); + expect(analysis.segments[0]?.resolution?.blockedWrapper).toBe("env"); + expect(allowlistEval.allowlistSatisfied).toBe(false); + expect(allowlistEval.segmentSatisfiedBy).toEqual([null]); + }); + it("unwraps env wrapper with shell inner executable", () => { const resolution = resolveCommandResolutionFromArgv(["/usr/bin/env", "bash", "-lc", "echo hi"]); expect(resolution?.rawExecutable).toBe("bash"); diff --git a/src/infra/exec-wrapper-resolution.ts b/src/infra/exec-wrapper-resolution.ts index 55e05842e36..6d09029da05 100644 --- a/src/infra/exec-wrapper-resolution.ts +++ b/src/infra/exec-wrapper-resolution.ts @@ -478,6 +478,17 @@ export function resolveDispatchWrapperExecutionPlan( } current = unwrap.argv; } + if (wrappers.length >= maxDepth) { + const overflow = unwrapKnownDispatchWrapperInvocation(current); + if (overflow.kind === "blocked" || overflow.kind === "unwrapped") { + return { + 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 a3be712655d..675f108b2ee 100644 --- a/src/node-host/invoke-system-run.test.ts +++ b/src/node-host/invoke-system-run.test.ts @@ -348,4 +348,99 @@ describe("handleSystemRunInvoke mac app exec host routing", () => { }), ); }); + + it("denies nested env shell payloads when wrapper depth is exceeded", async () => { + 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, + }; + }); + 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", + }, + 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, + }); + } finally { + if (previousOpenClawHome === undefined) { + delete process.env.OPENCLAW_HOME; + } else { + process.env.OPENCLAW_HOME = previousOpenClawHome; + } + fs.rmSync(tempHome, { recursive: true, force: true }); + } + + expect(runCommand).not.toHaveBeenCalled(); + expect(fs.existsSync(marker)).toBe(false); + expect(sendNodeEvent).toHaveBeenCalledWith( + expect.anything(), + "exec.denied", + expect.objectContaining({ reason: "approval-required" }), + ); + expect(sendInvokeResult).toHaveBeenCalledWith( + expect.objectContaining({ + ok: false, + error: expect.objectContaining({ + message: "SYSTEM_RUN_DENIED: approval required", + }), + }), + ); + }); });