From c6ee14d60e4cbd6a82f9b2d74ebeb1e8ee814964 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Sat, 21 Feb 2026 11:18:19 +0100 Subject: [PATCH] fix(security): block grep safe-bin file-read bypass --- CHANGELOG.md | 1 + docs/tools/exec-approvals.md | 2 ++ src/infra/exec-approvals.test.ts | 16 ++++++++++++++++ src/infra/exec-safe-bin-policy.test.ts | 22 ++++++++++++++++++++++ src/infra/exec-safe-bin-policy.ts | 5 ++++- 5 files changed, 45 insertions(+), 1 deletion(-) create mode 100644 src/infra/exec-safe-bin-policy.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index a7e4cfd55af..97775abc61b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -202,6 +202,7 @@ Docs: https://docs.openclaw.ai - Security/Exec: remove file-existence oracle behavior from `tools.exec.safeBins` by using deterministic argv-only stdin-safe validation and blocking file-oriented flags (for example `sort -o`, `jq -f`, `grep -f`) so allow/deny results no longer disclose host file presence. This ships in the next npm release. Thanks @nedlir for reporting. - Security/Browser: route browser URL navigation through one SSRF-guarded validation path for tab-open/CDP-target/Playwright navigation flows and block private/metadata destinations by default (configurable via `browser.ssrfPolicy`). This ships in the next npm release. Thanks @dorjoos for reporting. - Security/Exec: for the next npm release, harden safe-bin stdin-only enforcement by blocking output/recursive flags (`sort -o/--output`, grep recursion) and tightening default safe bins to remove `sort`/`grep`, preventing safe-bin allowlist bypass for file writes/recursive reads. Thanks @nedlir for reporting. +- Security/Exec: block grep safe-bin positional operand bypass by setting grep positional budget to zero, so `-e/--regexp` cannot smuggle bare filename reads (for example `.env`) via ambiguous positionals; safe-bin grep patterns must come from `-e/--regexp`. This ships in the next npm release. Thanks @athuljayaram for reporting. - Security/Gateway/Agents: remove implicit admin scopes from agent tool gateway calls by classifying methods to least-privilege operator scopes, and enforce owner-only tooling (`cron`, `gateway`, `whatsapp_login`) through centralized tool-policy wrappers plus tool metadata to prevent non-owner DM privilege escalation. Ships in the next npm release. Thanks @Adam55A-code for reporting. - Security/Gateway: centralize gateway method-scope authorization and default non-CLI gateway callers to least-privilege method scopes, with explicit CLI scope handling, full core-handler scope classification coverage, and regression guards to prevent scope drift. - Security/Net: block SSRF bypass via NAT64 (`64:ff9b::/96`, `64:ff9b:1::/48`), 6to4 (`2002::/16`), and Teredo (`2001:0000::/32`) IPv6 transition addresses, and fail closed on IPv6 parse errors. Thanks @jackhax. diff --git a/docs/tools/exec-approvals.md b/docs/tools/exec-approvals.md index f813b89c87b..567706d2d61 100644 --- a/docs/tools/exec-approvals.md +++ b/docs/tools/exec-approvals.md @@ -146,6 +146,8 @@ Default safe bins: `jq`, `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. +For `grep` in safe-bin mode, provide the pattern with `-e`/`--regexp`; positional pattern form is +rejected so file operands cannot be smuggled as ambiguous positionals. ## Control UI editing diff --git a/src/infra/exec-approvals.test.ts b/src/infra/exec-approvals.test.ts index 887f1980c57..b1a3d20228c 100644 --- a/src/infra/exec-approvals.test.ts +++ b/src/infra/exec-approvals.test.ts @@ -497,6 +497,22 @@ describe("exec approvals safe bins", () => { safeBins: ["grep"], executableName: "grep", }, + { + name: "blocks grep file positional when pattern uses -e", + argv: ["grep", "-e", "needle", ".env"], + resolvedPath: "/usr/bin/grep", + expected: false, + safeBins: ["grep"], + executableName: "grep", + }, + { + name: "blocks grep file positional after -- terminator", + argv: ["grep", "-e", "needle", "--", ".env"], + resolvedPath: "/usr/bin/grep", + expected: false, + safeBins: ["grep"], + executableName: "grep", + }, ]; for (const testCase of cases) { diff --git a/src/infra/exec-safe-bin-policy.test.ts b/src/infra/exec-safe-bin-policy.test.ts new file mode 100644 index 00000000000..5e808a320b5 --- /dev/null +++ b/src/infra/exec-safe-bin-policy.test.ts @@ -0,0 +1,22 @@ +import { describe, expect, it } from "vitest"; +import { SAFE_BIN_PROFILES, validateSafeBinArgv } from "./exec-safe-bin-policy.js"; + +describe("exec safe bin policy grep", () => { + const grepProfile = SAFE_BIN_PROFILES.grep; + + it("allows stdin-only grep when pattern comes from flags", () => { + expect(validateSafeBinArgv(["-e", "needle"], grepProfile)).toBe(true); + expect(validateSafeBinArgv(["--regexp=needle"], grepProfile)).toBe(true); + }); + + it("blocks grep positional pattern form to avoid filename ambiguity", () => { + expect(validateSafeBinArgv(["needle"], grepProfile)).toBe(false); + }); + + it("blocks file positionals when pattern comes from -e/--regexp", () => { + expect(validateSafeBinArgv(["-e", "SECRET", ".env"], grepProfile)).toBe(false); + expect(validateSafeBinArgv(["--regexp", "KEY", "config.py"], grepProfile)).toBe(false); + expect(validateSafeBinArgv(["--regexp=KEY", ".env"], grepProfile)).toBe(false); + expect(validateSafeBinArgv(["-e", "KEY", "--", ".env"], grepProfile)).toBe(false); + }); +}); diff --git a/src/infra/exec-safe-bin-policy.ts b/src/infra/exec-safe-bin-policy.ts index dad6af0992e..a2986190ae4 100644 --- a/src/infra/exec-safe-bin-policy.ts +++ b/src/infra/exec-safe-bin-policy.ts @@ -91,7 +91,10 @@ export const SAFE_BIN_PROFILE_FIXTURES: Record = ], }, grep: { - maxPositional: 1, + // Keep grep stdin-only: pattern must come from -e/--regexp. + // Allowing one positional is ambiguous because -e consumes the pattern and + // frees the positional slot for a filename. + maxPositional: 0, valueFlags: [ "--regexp", "--file",