From 6c2a3b74e37e49ec9efddb870b83231d9d405fd4 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Sun, 22 Mar 2026 10:04:23 -0700 Subject: [PATCH] fix(exec): harden jq safe-bin policy --- CHANGELOG.md | 1 + docs/tools/exec-approvals.md | 10 +++++++--- src/infra/exec-approvals-safe-bins.test.ts | 8 +++++++- src/infra/exec-command-resolution.ts | 2 +- src/infra/exec-safe-bin-policy-profiles.ts | 17 +++++++++++++++-- src/infra/exec-safe-bin-policy-validator.ts | 5 ++++- src/infra/exec-safe-bin-policy.test.ts | 15 +++++++++++++++ 7 files changed, 50 insertions(+), 8 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 763e13ff723..3c343113e8f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -79,6 +79,7 @@ Docs: https://docs.openclaw.ai - Plugins/bundler TDZ: fix `RESERVED_COMMANDS` temporal dead zone error that prevented device-pair, phone-control, and talk-voice plugins from registering when the bundler placed the commands module after call sites in the same output chunk. Thanks @BunsDev. - Plugins/imports: fix stale googlechat runtime-api import paths and signal SDK circular re-exports broken by recent plugin-sdk refactors. Thanks @BunsDev. - Telegram/setup: seed fresh setups with `channels.telegram.groups["*"].requireMention=true` so new bots stay mention-gated in groups unless you explicitly open them up. Thanks @vincentkoc. +- Security/exec safe bins: remove `jq` from the default safe-bin allowlist and fail closed on the `jq` `env` builtin when operators explicitly opt `jq` back in, so `jq -n env` cannot dump host secrets without an explicit trust path. Thanks @gladiator9797 for reporting. - Google auth/Node 25: patch `gaxios` to use native fetch without injecting `globalThis.window`, while translating proxy and mTLS transport settings so Google Vertex and Google Chat auth keep working on Node 25. (#47914) Thanks @pdd-cli. - Gateway/startup: load bundled channel plugins from compiled `dist/extensions` entries in built installs, so gateway boot no longer recompiles bundled extension TypeScript on every startup and WhatsApp-class cold starts drop back to seconds instead of tens of seconds or worse. (#47560) Thanks @ngutman. - Agents/openai-responses: strip `prompt_cache_key` and `prompt_cache_retention` for non-OpenAI-compatible Responses endpoints while keeping them on direct OpenAI and Azure OpenAI paths, so third-party OpenAI-compatible providers no longer reject those requests with HTTP 400. (#49877) Thanks @ShaunTsai. diff --git a/docs/tools/exec-approvals.md b/docs/tools/exec-approvals.md index 454e129d390..f2d4359cced 100644 --- a/docs/tools/exec-approvals.md +++ b/docs/tools/exec-approvals.md @@ -160,7 +160,7 @@ Important trust notes: ## Safe bins (stdin-only) -`tools.exec.safeBins` defines a small list of **stdin-only** binaries (for example `jq`) +`tools.exec.safeBins` defines a small list of **stdin-only** binaries (for example `cut`) that can run in allowlist mode **without** explicit allowlist entries. Safe bins reject positional file args and path-like tokens, so they can only operate on the incoming stream. Treat this as a narrow fast-path for stream filters, not a general trust list. @@ -215,7 +215,7 @@ etc.) so inner executables are persisted instead of multiplexer binaries. If a w multiplexer cannot be safely unwrapped, no allowlist entry is persisted automatically. If you allowlist interpreters like `python3` or `node`, prefer `tools.exec.strictInlineEval=true` so inline eval still requires an explicit approval. -Default safe bins: `jq`, `cut`, `uniq`, `head`, `tail`, `tr`, `wc`. +Default safe bins: `cut`, `uniq`, `head`, `tail`, `tr`, `wc`. `grep` and `sort` are not in the default list. If you opt in, keep explicit allowlist entries for their non-stdin workflows. @@ -229,7 +229,7 @@ rejected so file operands cannot be smuggled as ambiguous positionals. | Goal | Auto-allow narrow stdin filters | Explicitly trust specific executables | | Match type | Executable name + safe-bin argv policy | Resolved executable path glob pattern | | Argument scope | Restricted by safe-bin profile and literal-token rules | Path match only; arguments are otherwise your responsibility | -| Typical examples | `jq`, `head`, `tail`, `wc` | `python3`, `node`, `ffmpeg`, custom CLIs | +| Typical examples | `head`, `tail`, `tr`, `wc` | `jq`, `python3`, `node`, `ffmpeg`, custom CLIs | | Best use | Low-risk text transforms in pipelines | Any tool with broader behavior or side effects | Configuration location: @@ -261,6 +261,10 @@ Custom profile example: } ``` +If you explicitly opt `jq` into `safeBins`, OpenClaw still rejects the `env` builtin in safe-bin +mode so `jq -n env` cannot dump the host process environment without an explicit allowlist path +or approval prompt. + ## Control UI editing Use the **Control UI → Nodes → Exec approvals** card to edit defaults, per‑agent diff --git a/src/infra/exec-approvals-safe-bins.test.ts b/src/infra/exec-approvals-safe-bins.test.ts index b24b24a81f8..e8fb40877a6 100644 --- a/src/infra/exec-approvals-safe-bins.test.ts +++ b/src/infra/exec-approvals-safe-bins.test.ts @@ -174,6 +174,12 @@ describe("exec approvals safe bins", () => { resolvedPath: "/usr/bin/jq", expected: true, }, + { + name: "blocks jq env builtin even when jq is explicitly opted in", + argv: ["jq", "env"], + resolvedPath: "/usr/bin/jq", + expected: false, + }, { name: "blocks safe bins with file args", argv: ["jq", ".foo", "secret.json"], @@ -328,7 +334,7 @@ describe("exec approvals safe bins", () => { it("does not include sort/grep in default safeBins", () => { const defaults = resolveSafeBins(undefined); - expect(defaults.has("jq")).toBe(true); + expect(defaults.has("jq")).toBe(false); expect(defaults.has("sort")).toBe(false); expect(defaults.has("grep")).toBe(false); }); diff --git a/src/infra/exec-command-resolution.ts b/src/infra/exec-command-resolution.ts index d87b9a264dc..ae6bbc55e30 100644 --- a/src/infra/exec-command-resolution.ts +++ b/src/infra/exec-command-resolution.ts @@ -6,7 +6,7 @@ import { resolveDispatchWrapperExecutionPlan } from "./exec-wrapper-resolution.j import { resolveExecutablePath as resolveExecutableCandidatePath } from "./executable-path.js"; import { expandHomePrefix } from "./home-dir.js"; -export const DEFAULT_SAFE_BINS = ["jq", "cut", "uniq", "head", "tail", "tr", "wc"]; +export const DEFAULT_SAFE_BINS = ["cut", "uniq", "head", "tail", "tr", "wc"]; export type CommandResolution = { rawExecutable: string; diff --git a/src/infra/exec-safe-bin-policy-profiles.ts b/src/infra/exec-safe-bin-policy-profiles.ts index b450325d2fe..1602a0f2d96 100644 --- a/src/infra/exec-safe-bin-policy-profiles.ts +++ b/src/infra/exec-safe-bin-policy-profiles.ts @@ -3,6 +3,7 @@ export type SafeBinProfile = { maxPositional?: number; allowedValueFlags?: ReadonlySet; deniedFlags?: ReadonlySet; + positionalValidator?: (positional: readonly string[]) => boolean; // Precomputed long-option metadata for GNU abbreviation resolution. knownLongFlags?: readonly string[]; knownLongFlagsSet?: ReadonlySet; @@ -68,7 +69,18 @@ export function buildLongFlagPrefixMap( return prefixMap; } -function compileSafeBinProfile(fixture: SafeBinProfileFixture): SafeBinProfile { +const JQ_ENV_FILTER_PATTERN = /(^|[^.$A-Za-z0-9_])env([^A-Za-z0-9_]|$)/; + +function validateJqSafeBinPositional(positional: readonly string[]): boolean { + for (const token of positional) { + if (JQ_ENV_FILTER_PATTERN.test(token)) { + return false; + } + } + return true; +} + +function compileSafeBinProfile(name: string, fixture: SafeBinProfileFixture): SafeBinProfile { const allowedValueFlags = toFlagSet(fixture.allowedValueFlags); const deniedFlags = toFlagSet(fixture.deniedFlags); const knownLongFlags = collectKnownLongFlags(allowedValueFlags, deniedFlags); @@ -77,6 +89,7 @@ function compileSafeBinProfile(fixture: SafeBinProfileFixture): SafeBinProfile { maxPositional: fixture.maxPositional, allowedValueFlags, deniedFlags, + positionalValidator: name === "jq" ? validateJqSafeBinPositional : undefined, knownLongFlags, knownLongFlagsSet: new Set(knownLongFlags), longFlagPrefixMap: buildLongFlagPrefixMap(knownLongFlags), @@ -87,7 +100,7 @@ function compileSafeBinProfiles( fixtures: Record, ): Record { return Object.fromEntries( - Object.entries(fixtures).map(([name, fixture]) => [name, compileSafeBinProfile(fixture)]), + Object.entries(fixtures).map(([name, fixture]) => [name, compileSafeBinProfile(name, fixture)]), ) as Record; } diff --git a/src/infra/exec-safe-bin-policy-validator.ts b/src/infra/exec-safe-bin-policy-validator.ts index 83160285242..a495815c8da 100644 --- a/src/infra/exec-safe-bin-policy-validator.ts +++ b/src/infra/exec-safe-bin-policy-validator.ts @@ -127,7 +127,10 @@ function validatePositionalCount(positional: string[], profile: SafeBinProfile): if (positional.length < minPositional) { return false; } - return typeof profile.maxPositional !== "number" || positional.length <= profile.maxPositional; + if (typeof profile.maxPositional === "number" && positional.length > profile.maxPositional) { + return false; + } + return profile.positionalValidator?.(positional) ?? true; } export function validateSafeBinArgv(args: string[], profile: SafeBinProfile): boolean { diff --git a/src/infra/exec-safe-bin-policy.test.ts b/src/infra/exec-safe-bin-policy.test.ts index 4af387b73dc..bc9735050a0 100644 --- a/src/infra/exec-safe-bin-policy.test.ts +++ b/src/infra/exec-safe-bin-policy.test.ts @@ -44,6 +44,21 @@ describe("exec safe bin policy grep", () => { }); }); +describe("exec safe bin policy jq", () => { + const jqProfile = SAFE_BIN_PROFILES.jq; + + it("allows normal jq field filters", () => { + expect(validateSafeBinArgv([".foo"], jqProfile)).toBe(true); + expect(validateSafeBinArgv([".env"], jqProfile)).toBe(true); + }); + + it("blocks jq env builtin filters in safe-bin mode", () => { + expect(validateSafeBinArgv(["env"], jqProfile)).toBe(false); + expect(validateSafeBinArgv(["env.FOO"], jqProfile)).toBe(false); + expect(validateSafeBinArgv([".foo | env"], jqProfile)).toBe(false); + }); +}); + describe("exec safe bin policy sort", () => { const sortProfile = SAFE_BIN_PROFILES.sort;