From f23da067f67e81af0e6817984ec650f1712a61f8 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Sat, 21 Feb 2026 14:27:13 +0100 Subject: [PATCH] fix(security): harden heredoc allowlist parsing --- CHANGELOG.md | 1 + src/agents/bash-tools.exec-host-gateway.ts | 23 ++++++++--- src/infra/exec-approvals-analysis.ts | 46 ++++++++++++++++------ src/infra/exec-approvals.test.ts | 29 +++++++++++--- 4 files changed, 75 insertions(+), 24 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c24316adefe..54b138350ef 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -29,6 +29,7 @@ Docs: https://docs.openclaw.ai ### Fixes +- Security/Exec: block unquoted heredoc body expansion tokens in shell allowlist analysis, reject unterminated heredocs, and require explicit approval for allowlisted heredoc execution on gateway hosts to prevent heredoc substitution allowlist bypass. This ships in the next npm release. Thanks @torturado for reporting. - WhatsApp/Security: enforce allowlist JID authorization for reaction actions so authenticated callers cannot target non-allowlisted chats by forging `chatJid` + valid `messageId` pairs. This ships in the next npm release. Thanks @aether-ai-agent for reporting. - Agents/Tool images: include source filenames in `agents/tool-images` resize logs so compression events can be traced back to specific files. - ACP/Security: escape control and delimiter characters in ACP `resource_link` title/URI metadata before prompt interpolation to prevent metadata-driven prompt injection through resource links. This ships in the next npm release. Thanks @aether-ai-agent for reporting. diff --git a/src/agents/bash-tools.exec-host-gateway.ts b/src/agents/bash-tools.exec-host-gateway.ts index 3a804abc9eb..d3cc26c467c 100644 --- a/src/agents/bash-tools.exec-host-gateway.ts +++ b/src/agents/bash-tools.exec-host-gateway.ts @@ -77,12 +77,23 @@ export async function processGatewayAllowlist( const analysisOk = allowlistEval.analysisOk; const allowlistSatisfied = hostSecurity === "allowlist" && analysisOk ? allowlistEval.allowlistSatisfied : false; - const requiresAsk = requiresExecApproval({ - ask: hostAsk, - security: hostSecurity, - analysisOk, - allowlistSatisfied, - }); + const hasHeredocSegment = allowlistEval.segments.some((segment) => + segment.argv.some((token) => token.startsWith("<<")), + ); + const requiresHeredocApproval = + hostSecurity === "allowlist" && analysisOk && allowlistSatisfied && hasHeredocSegment; + const requiresAsk = + requiresExecApproval({ + ask: hostAsk, + security: hostSecurity, + analysisOk, + allowlistSatisfied, + }) || requiresHeredocApproval; + if (requiresHeredocApproval) { + params.warnings.push( + "Warning: heredoc execution requires explicit approval in allowlist mode.", + ); + } if (requiresAsk) { const approvalId = crypto.randomUUID(); diff --git a/src/infra/exec-approvals-analysis.ts b/src/infra/exec-approvals-analysis.ts index f7c49b5321a..8eecd13a0a6 100644 --- a/src/infra/exec-approvals-analysis.ts +++ b/src/infra/exec-approvals-analysis.ts @@ -410,6 +410,30 @@ function splitShellPipeline(command: string): { ok: boolean; reason?: string; se buf = ""; }; + const isEscapedInHeredocLine = (line: string, index: number): boolean => { + let slashes = 0; + for (let i = index - 1; i >= 0 && line[i] === "\\"; i -= 1) { + slashes += 1; + } + return slashes % 2 === 1; + }; + + const hasUnquotedHeredocExpansionToken = (line: string): boolean => { + for (let i = 0; i < line.length; i += 1) { + const ch = line[i]; + if (ch === "`" && !isEscapedInHeredocLine(line, i)) { + return true; + } + if (ch === "$" && !isEscapedInHeredocLine(line, i)) { + const next = line[i + 1]; + if (next === "(" || next === "{") { + return true; + } + } + } + return false; + }; + for (let i = 0; i < command.length; i += 1) { const ch = command[i]; const next = command[i + 1]; @@ -421,6 +445,8 @@ function splitShellPipeline(command: string): { ok: boolean; reason?: string; se const line = current.stripTabs ? heredocLine.replace(/^\t+/, "") : heredocLine; if (line === current.delimiter) { pendingHeredocs.shift(); + } else if (!current.quoted && hasUnquotedHeredocExpansionToken(heredocLine)) { + return { ok: false, reason: "command substitution in unquoted heredoc", segments: [] }; } } heredocLine = ""; @@ -431,19 +457,6 @@ function splitShellPipeline(command: string): { ok: boolean; reason?: string; se i += 1; } } else { - // When the heredoc delimiter is unquoted, the shell performs expansion - // on the body (variable substitution, command substitution, etc.). - // Reject commands that use unquoted heredocs containing expansion tokens - // to prevent allowlist bypass via embedded command substitution. - const currentHeredoc = pendingHeredocs[0]; - if (currentHeredoc && !currentHeredoc.quoted) { - if (ch === "`") { - return { ok: false, reason: "command substitution in unquoted heredoc", segments: [] }; - } - if (ch === "$" && (next === "(" || next === "{")) { - return { ok: false, reason: "command substitution in unquoted heredoc", segments: [] }; - } - } heredocLine += ch; } continue; @@ -565,9 +578,16 @@ function splitShellPipeline(command: string): { ok: boolean; reason?: string; se const line = current.stripTabs ? heredocLine.replace(/^\t+/, "") : heredocLine; if (line === current.delimiter) { pendingHeredocs.shift(); + if (pendingHeredocs.length === 0) { + inHeredocBody = false; + } } } + if (pendingHeredocs.length > 0 || inHeredocBody) { + return { ok: false, reason: "unterminated heredoc", segments: [] }; + } + if (escaped || inSingle || inDouble) { return { ok: false, reason: "unterminated shell quote/escape", segments: [] }; } diff --git a/src/infra/exec-approvals.test.ts b/src/infra/exec-approvals.test.ts index c2158335a1d..eb5072d7fb3 100644 --- a/src/infra/exec-approvals.test.ts +++ b/src/infra/exec-approvals.test.ts @@ -264,25 +264,27 @@ describe("exec approvals shell parsing", () => { }); it("allows heredoc operator (<<)", () => { - const res = analyzeShellCommand({ command: "/usr/bin/tee /tmp/file << 'EOF'" }); + const res = analyzeShellCommand({ command: "/usr/bin/tee /tmp/file << 'EOF'\nEOF" }); expect(res.ok).toBe(true); expect(res.segments[0]?.argv[0]).toBe("/usr/bin/tee"); }); it("allows heredoc without space before delimiter", () => { - const res = analyzeShellCommand({ command: "/usr/bin/tee /tmp/file < { - const res = analyzeShellCommand({ command: "/usr/bin/cat <<-DELIM" }); + const res = analyzeShellCommand({ command: "/usr/bin/cat <<-DELIM\n\tDELIM" }); expect(res.ok).toBe(true); expect(res.segments[0]?.argv[0]).toBe("/usr/bin/cat"); }); it("allows heredoc in pipeline", () => { - const res = analyzeShellCommand({ command: "/usr/bin/cat << 'EOF' | /usr/bin/grep pattern" }); + const res = analyzeShellCommand({ + command: "/usr/bin/cat << 'EOF' | /usr/bin/grep pattern\npattern\nEOF", + }); expect(res.ok).toBe(true); expect(res.segments).toHaveLength(2); expect(res.segments[0]?.argv[0]).toBe("/usr/bin/cat"); @@ -329,6 +331,14 @@ describe("exec approvals shell parsing", () => { expect(res.reason).toBe("command substitution in unquoted heredoc"); }); + it("allows escaped command substitution in unquoted heredoc body", () => { + const res = analyzeShellCommand({ + command: "/usr/bin/cat < { const res = analyzeShellCommand({ command: "/usr/bin/cat <<'EOF'\n$(id)\nEOF", @@ -347,7 +357,8 @@ describe("exec approvals shell parsing", () => { it("rejects nested command substitution in unquoted heredoc", () => { const res = analyzeShellCommand({ - command: "/usr/bin/cat < { expect(res.segments[0]?.argv[0]).toBe("/usr/bin/cat"); }); + it("rejects unterminated heredoc", () => { + const res = analyzeShellCommand({ + command: "/usr/bin/cat < { const res = analyzeShellCommand({ command: "/usr/bin/echo first line\n/usr/bin/echo second line",