From a67689a7e3ad494b6637c76235a664322d526f9e Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Tue, 24 Feb 2026 03:06:34 +0000 Subject: [PATCH] fix: harden allow-always shell multiplexer wrapper handling --- CHANGELOG.md | 1 + docs/tools/exec-approvals.md | 4 +- src/infra/exec-approvals-allow-always.test.ts | 100 ++++++++++++++++++ src/infra/exec-approvals-allowlist.ts | 25 +++++ .../exec-safe-bin-runtime-policy.test.ts | 2 + src/infra/exec-safe-bin-runtime-policy.ts | 2 + src/infra/exec-wrapper-resolution.ts | 55 ++++++++++ src/infra/system-run-command.test.ts | 5 + 8 files changed, 193 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 03b2e4ecaae..ec759b137e0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -23,6 +23,7 @@ Docs: https://docs.openclaw.ai - Security/Exec: harden `safeBins` long-option validation by rejecting unknown/ambiguous GNU long-option abbreviations and denying sort filesystem-dependent flags (`--random-source`, `--temporary-directory`, `-T`), closing safe-bin denylist bypasses. This ships in the next npm release. Thanks @tdjackey and @jiseoung for reporting. - Security/Channels: unify dangerous name-matching policy checks (`dangerouslyAllowNameMatching`) across core and extension channels, share mutable-allowlist detectors between `openclaw doctor` and `openclaw security audit`, and scan all configured accounts (not only the default account) in channel security audit findings. - Security/Exec approvals: enforce canonical wrapper execution plans across allowlist analysis and runtime execution (node host + gateway host), fail closed on semantic `env` wrapper usage, and reject unknown short safe-bin flags to prevent `env -S/--split-string` interpretation-mismatch bypasses. This ships in the next npm release. Thanks @tdjackey for reporting. +- Security/Exec approvals: recognize `busybox`/`toybox` shell applets in wrapper analysis and allow-always persistence, persist inner executables instead of multiplexer wrapper binaries, and fail closed when multiplexer unwrapping is unsafe to prevent allow-always bypasses. This ships in the next npm release. Thanks @jiseoung for reporting. - Security/Image tool: enforce `tools.fs.workspaceOnly` for sandboxed `image` path resolution so mounted out-of-workspace paths are blocked before media bytes are loaded/sent to vision providers. This ships in the next npm release. Thanks @tdjackey for reporting. - Security/Session export: harden exported HTML image rendering against data-URL attribute injection by validating image MIME/base64 fields, rejecting malformed base64 input in media ingestion paths, and dropping invalid tool-image payloads. diff --git a/docs/tools/exec-approvals.md b/docs/tools/exec-approvals.md index 0e6d0f52899..f155fbbd790 100644 --- a/docs/tools/exec-approvals.md +++ b/docs/tools/exec-approvals.md @@ -178,7 +178,9 @@ For shell wrappers (`bash|sh|zsh ... -c/-lc`), request-scoped env overrides are small explicit allowlist (`TERM`, `LANG`, `LC_*`, `COLORTERM`, `NO_COLOR`, `FORCE_COLOR`). For allow-always decisions in allowlist mode, known dispatch wrappers (`env`, `nice`, `nohup`, `stdbuf`, `timeout`) persist inner executable paths instead of wrapper -paths. If a wrapper cannot be safely unwrapped, no allowlist entry is persisted automatically. +paths. Shell multiplexers (`busybox`, `toybox`) are also unwrapped for shell applets (`sh`, `ash`, +etc.) so inner executables are persisted instead of multiplexer binaries. If a wrapper or +multiplexer cannot be safely unwrapped, no allowlist entry is persisted automatically. Default safe bins: `jq`, `cut`, `uniq`, `head`, `tail`, `tr`, `wc`. diff --git a/src/infra/exec-approvals-allow-always.test.ts b/src/infra/exec-approvals-allow-always.test.ts index ab43ff17ec5..640ea8706d6 100644 --- a/src/infra/exec-approvals-allow-always.test.ts +++ b/src/infra/exec-approvals-allow-always.test.ts @@ -153,6 +153,60 @@ describe("resolveAllowAlwaysPatterns", () => { expect(patterns).not.toContain("/usr/bin/nice"); }); + it("unwraps busybox/toybox shell applets and persists inner executables", () => { + if (process.platform === "win32") { + return; + } + const dir = makeTempDir(); + const busybox = makeExecutable(dir, "busybox"); + makeExecutable(dir, "toybox"); + const whoami = makeExecutable(dir, "whoami"); + const env = { PATH: `${dir}${path.delimiter}${process.env.PATH ?? ""}` }; + const patterns = resolveAllowAlwaysPatterns({ + segments: [ + { + raw: `${busybox} sh -lc whoami`, + argv: [busybox, "sh", "-lc", "whoami"], + resolution: { + rawExecutable: busybox, + resolvedPath: busybox, + executableName: "busybox", + }, + }, + ], + cwd: dir, + env, + platform: process.platform, + }); + expect(patterns).toEqual([whoami]); + expect(patterns).not.toContain(busybox); + }); + + it("fails closed for unsupported busybox/toybox applets", () => { + if (process.platform === "win32") { + return; + } + const dir = makeTempDir(); + const busybox = makeExecutable(dir, "busybox"); + const patterns = resolveAllowAlwaysPatterns({ + segments: [ + { + raw: `${busybox} sed -n 1p`, + argv: [busybox, "sed", "-n", "1p"], + resolution: { + rawExecutable: busybox, + resolvedPath: busybox, + executableName: "busybox", + }, + }, + ], + cwd: dir, + env: makePathEnv(dir), + platform: process.platform, + }); + expect(patterns).toEqual([]); + }); + it("fails closed for unresolved dispatch wrappers", () => { const patterns = resolveAllowAlwaysPatterns({ segments: [ @@ -171,6 +225,52 @@ describe("resolveAllowAlwaysPatterns", () => { expect(patterns).toEqual([]); }); + it("prevents allow-always bypass for busybox shell applets", () => { + if (process.platform === "win32") { + return; + } + const dir = makeTempDir(); + const busybox = makeExecutable(dir, "busybox"); + const echo = makeExecutable(dir, "echo"); + makeExecutable(dir, "id"); + const safeBins = resolveSafeBins(undefined); + const env = { PATH: `${dir}${path.delimiter}${process.env.PATH ?? ""}` }; + + const first = evaluateShellAllowlist({ + command: `${busybox} sh -c 'echo warmup-ok'`, + allowlist: [], + safeBins, + cwd: dir, + env, + platform: process.platform, + }); + const persisted = resolveAllowAlwaysPatterns({ + segments: first.segments, + cwd: dir, + env, + platform: process.platform, + }); + expect(persisted).toEqual([echo]); + + const second = evaluateShellAllowlist({ + command: `${busybox} sh -c 'id > marker'`, + allowlist: [{ pattern: echo }], + safeBins, + cwd: dir, + env, + platform: process.platform, + }); + expect(second.allowlistSatisfied).toBe(false); + expect( + requiresExecApproval({ + ask: "on-miss", + security: "allowlist", + analysisOk: second.analysisOk, + allowlistSatisfied: second.allowlistSatisfied, + }), + ).toBe(true); + }); + it("prevents allow-always bypass for dispatch-wrapper + shell-wrapper chains", () => { if (process.platform === "win32") { return; diff --git a/src/infra/exec-approvals-allowlist.ts b/src/infra/exec-approvals-allowlist.ts index bff632d46be..25d06994977 100644 --- a/src/infra/exec-approvals-allowlist.ts +++ b/src/infra/exec-approvals-allowlist.ts @@ -21,6 +21,7 @@ import { extractShellWrapperInlineCommand, isDispatchWrapperExecutable, isShellWrapperExecutable, + unwrapKnownShellMultiplexerInvocation, unwrapKnownDispatchWrapperInvocation, } from "./exec-wrapper-resolution.js"; @@ -299,6 +300,30 @@ function collectAllowAlwaysPatterns(params: { return; } + const shellMultiplexerUnwrap = unwrapKnownShellMultiplexerInvocation(params.segment.argv); + if (shellMultiplexerUnwrap.kind === "blocked") { + return; + } + if (shellMultiplexerUnwrap.kind === "unwrapped") { + collectAllowAlwaysPatterns({ + segment: { + raw: shellMultiplexerUnwrap.argv.join(" "), + argv: shellMultiplexerUnwrap.argv, + resolution: resolveCommandResolutionFromArgv( + shellMultiplexerUnwrap.argv, + params.cwd, + params.env, + ), + }, + cwd: params.cwd, + env: params.env, + platform: params.platform, + depth: params.depth + 1, + out: params.out, + }); + return; + } + const candidatePath = resolveAllowlistCandidatePath(params.segment.resolution, params.cwd); if (!candidatePath) { return; diff --git a/src/infra/exec-safe-bin-runtime-policy.test.ts b/src/infra/exec-safe-bin-runtime-policy.test.ts index 29f29864be2..e9ee3230405 100644 --- a/src/infra/exec-safe-bin-runtime-policy.test.ts +++ b/src/infra/exec-safe-bin-runtime-policy.test.ts @@ -15,6 +15,8 @@ describe("exec safe-bin runtime policy", () => { { bin: "node20", expected: true }, { bin: "ruby3.2", expected: true }, { bin: "bash", expected: true }, + { bin: "busybox", expected: true }, + { bin: "toybox", expected: true }, { bin: "myfilter", expected: false }, { bin: "jq", expected: false }, ]; diff --git a/src/infra/exec-safe-bin-runtime-policy.ts b/src/infra/exec-safe-bin-runtime-policy.ts index a6f71d16f91..9ed56bfe680 100644 --- a/src/infra/exec-safe-bin-runtime-policy.ts +++ b/src/infra/exec-safe-bin-runtime-policy.ts @@ -17,6 +17,7 @@ export type ExecSafeBinConfigScope = { const INTERPRETER_LIKE_SAFE_BINS = new Set([ "ash", "bash", + "busybox", "bun", "cmd", "cmd.exe", @@ -40,6 +41,7 @@ const INTERPRETER_LIKE_SAFE_BINS = new Set([ "python3", "ruby", "sh", + "toybox", "wscript", "zsh", ]); diff --git a/src/infra/exec-wrapper-resolution.ts b/src/infra/exec-wrapper-resolution.ts index 58fc18b0015..55e05842e36 100644 --- a/src/infra/exec-wrapper-resolution.ts +++ b/src/infra/exec-wrapper-resolution.ts @@ -7,6 +7,7 @@ const WINDOWS_EXE_SUFFIX = ".exe"; const POSIX_SHELL_WRAPPER_NAMES = ["ash", "bash", "dash", "fish", "ksh", "sh", "zsh"] as const; const WINDOWS_CMD_WRAPPER_NAMES = ["cmd"] as const; const POWERSHELL_WRAPPER_NAMES = ["powershell", "pwsh"] as const; +const SHELL_MULTIPLEXER_WRAPPER_NAMES = ["busybox", "toybox"] as const; const DISPATCH_WRAPPER_NAMES = [ "chrt", "doas", @@ -42,6 +43,7 @@ export const DISPATCH_WRAPPER_EXECUTABLES = new Set(withWindowsExeAliases(DISPAT const POSIX_SHELL_WRAPPER_CANONICAL = new Set(POSIX_SHELL_WRAPPER_NAMES); const WINDOWS_CMD_WRAPPER_CANONICAL = new Set(WINDOWS_CMD_WRAPPER_NAMES); const POWERSHELL_WRAPPER_CANONICAL = new Set(POWERSHELL_WRAPPER_NAMES); +const SHELL_MULTIPLEXER_WRAPPER_CANONICAL = new Set(SHELL_MULTIPLEXER_WRAPPER_NAMES); const DISPATCH_WRAPPER_CANONICAL = new Set(DISPATCH_WRAPPER_NAMES); const SHELL_WRAPPER_CANONICAL = new Set([ ...POSIX_SHELL_WRAPPER_NAMES, @@ -133,6 +135,39 @@ function findShellWrapperSpec(baseExecutable: string): ShellWrapperSpec | null { return null; } +export type ShellMultiplexerUnwrapResult = + | { kind: "not-wrapper" } + | { kind: "blocked"; wrapper: string } + | { kind: "unwrapped"; wrapper: string; argv: string[] }; + +export function unwrapKnownShellMultiplexerInvocation( + argv: string[], +): ShellMultiplexerUnwrapResult { + const token0 = argv[0]?.trim(); + if (!token0) { + return { kind: "not-wrapper" }; + } + const wrapper = normalizeExecutableToken(token0); + if (!SHELL_MULTIPLEXER_WRAPPER_CANONICAL.has(wrapper)) { + return { kind: "not-wrapper" }; + } + + let appletIndex = 1; + if (argv[appletIndex]?.trim() === "--") { + appletIndex += 1; + } + const applet = argv[appletIndex]?.trim(); + if (!applet || !isShellWrapperExecutable(applet)) { + return { kind: "blocked", wrapper }; + } + + const unwrapped = argv.slice(appletIndex); + if (unwrapped.length === 0) { + return { kind: "blocked", wrapper }; + } + return { kind: "unwrapped", wrapper, argv: unwrapped }; +} + export function isEnvAssignment(token: string): boolean { return /^[A-Za-z_][A-Za-z0-9_]*=.*/.test(token); } @@ -474,6 +509,18 @@ function hasEnvManipulationBeforeShellWrapperInternal( ); } + const shellMultiplexerUnwrap = unwrapKnownShellMultiplexerInvocation(argv); + if (shellMultiplexerUnwrap.kind === "blocked") { + return false; + } + if (shellMultiplexerUnwrap.kind === "unwrapped") { + return hasEnvManipulationBeforeShellWrapperInternal( + shellMultiplexerUnwrap.argv, + depth + 1, + envManipulationSeen, + ); + } + const wrapper = findShellWrapperSpec(normalizeExecutableToken(token0)); if (!wrapper) { return false; @@ -577,6 +624,14 @@ function extractShellWrapperCommandInternal( return extractShellWrapperCommandInternal(dispatchUnwrap.argv, rawCommand, depth + 1); } + const shellMultiplexerUnwrap = unwrapKnownShellMultiplexerInvocation(argv); + if (shellMultiplexerUnwrap.kind === "blocked") { + return { isWrapper: false, command: null }; + } + if (shellMultiplexerUnwrap.kind === "unwrapped") { + return extractShellWrapperCommandInternal(shellMultiplexerUnwrap.argv, rawCommand, depth + 1); + } + const base0 = normalizeExecutableToken(token0); const wrapper = findShellWrapperSpec(base0); if (!wrapper) { diff --git a/src/infra/system-run-command.test.ts b/src/infra/system-run-command.test.ts index 43a1b6fae79..4b99c5e1365 100644 --- a/src/infra/system-run-command.test.ts +++ b/src/infra/system-run-command.test.ts @@ -57,6 +57,11 @@ describe("system run command helpers", () => { expect(extractShellCommandFromArgv(["pwsh", "-Command", "Get-Date"])).toBe("Get-Date"); }); + test("extractShellCommandFromArgv unwraps busybox/toybox shell applets", () => { + expect(extractShellCommandFromArgv(["busybox", "sh", "-c", "echo hi"])).toBe("echo hi"); + expect(extractShellCommandFromArgv(["toybox", "ash", "-lc", "echo hi"])).toBe("echo hi"); + }); + test("extractShellCommandFromArgv ignores env wrappers when no shell wrapper follows", () => { expect(extractShellCommandFromArgv(["/usr/bin/env", "FOO=bar", "/usr/bin/printf", "ok"])).toBe( null,