From e72191ba74085a440fd2bd5023e210a94d5da70f Mon Sep 17 00:00:00 2001
From: dajiaohuang <mikewushuwen@outlook.com>
Date: Fri, 28 Aug 2026 06:56:00 +0800
Subject: [PATCH] fix: harden heredoc command filtering

---
 scripts/hooks/gateguard-fact-force.js    | 163 +--------------------
 scripts/hooks/gateguard-heredoc.js       | 172 +++++++++++++++++++++++
 tests/hooks/gateguard-fact-force.test.js |  65 +++++++++
 3 files changed, 238 insertions(+), 162 deletions(-)
 create mode 100644 scripts/hooks/gateguard-heredoc.js

diff --git a/scripts/hooks/gateguard-fact-force.js b/scripts/hooks/gateguard-fact-force.js
index 2cd852a93..203092d64 100644
--- a/scripts/hooks/gateguard-fact-force.js
+++ b/scripts/hooks/gateguard-fact-force.js
@@ -26,6 +26,7 @@ const crypto = require('crypto');
 const fs = require('fs');
 const path = require('path');
 const { extractCommandSubstitutions, extractSubshellGroups, extractBraceGroups } = require('../lib/shell-substitution');
+const { stripHeredocBodies } = require('./gateguard-heredoc');
 
 // Session state — scoped per session to avoid cross-session races.
 const STATE_DIR = process.env.GATEGUARD_STATE_DIR || path.join(process.env.HOME || process.env.USERPROFILE || '/tmp', '.gateguard');
@@ -151,168 +152,6 @@ function stripQuotedStrings(input) {
   return input.replace(/'(?:[^'\\]|\\.)*'/g, "''").replace(/"(?:[^"\\]|\\.)*"/g, '""');
 }
 
-/**
- * Find simple heredoc redirections on one complete shell command line.
- * Anything ambiguous is rejected so the caller can fail closed and run the
- * destructive checks against the original input. Supported delimiters are
- * shell identifiers, either unquoted or wholly single/double quoted.
- *
- * @param {string} line
- * @returns {{ delimiter: string, quoted: boolean, stripTabs: boolean }[] | null}
- */
-function findHeredocs(line) {
-  const heredocs = [];
-  let quote = null;
-  let escaped = false;
-
-  for (let i = 0; i < line.length; i += 1) {
-    const ch = line[i];
-    if (escaped) {
-      escaped = false;
-      continue;
-    }
-    if (ch === '\\') {
-      escaped = true;
-      continue;
-    }
-    if (quote) {
-      if (ch === quote) quote = null;
-      continue;
-    }
-    if (ch === '"' || ch === "'") {
-      quote = ch;
-      continue;
-    }
-    if ((ch === '$' && line[i + 1] === '(' && line[i + 2] === '(') || (ch === '(' && line[i + 1] === '(')) {
-      // Arithmetic syntax also uses `<<`. Treat the complete input
-      // conservatively instead of trying to parse nested arithmetic here.
-      return null;
-    }
-    if (ch === '$' && line[i + 1] === '[') return null;
-    if (ch === '#' && (i === 0 || /[\s;&|()]/.test(line[i - 1]))) {
-      break;
-    }
-    if (ch !== '<' || line[i + 1] !== '<' || line[i + 2] === '<') {
-      continue;
-    }
-
-    // `<<` is also an operator inside arithmetic and [[ ... ]] expressions.
-    // A partial shell parser cannot distinguish every nested form safely.
-    const prefix = line.slice(0, i);
-    if (prefix.includes('((') || prefix.includes('[[')) return null;
-
-    i += 2;
-    const stripTabs = line[i] === '-';
-    if (stripTabs) i += 1;
-    while (i < line.length && /[ \t]/.test(line[i])) i += 1;
-
-    let delimiter = '';
-    let quoted = false;
-    const delimiterQuote = line[i] === '"' || line[i] === "'" ? line[i] : null;
-    if (delimiterQuote) {
-      quoted = true;
-      const endQuote = line.indexOf(delimiterQuote, i + 1);
-      if (endQuote < 0) return null;
-      delimiter = line.slice(i + 1, endQuote);
-      i = endQuote;
-    } else {
-      const match = line.slice(i).match(/^[A-Za-z_][A-Za-z0-9_]*/);
-      if (!match) return null;
-      delimiter = match[0];
-      i += delimiter.length - 1;
-    }
-
-    if (!/^[A-Za-z_][A-Za-z0-9_]*$/.test(delimiter)) return null;
-    const next = line[i + 1];
-    if (next && !/[\s;&|<>()]/.test(next)) return null;
-    heredocs.push({ delimiter, quoted, stripTabs });
-  }
-
-  return quote || escaped ? null : heredocs;
-}
-
-/**
- * Extract executable substitutions from an unquoted heredoc. Quote characters
- * in its payload are literal and do not suppress expansion, so each unescaped
- * `$(` or backtick is parsed from its own position rather than by feeding the
- * complete payload through normal shell quote handling.
- *
- * @param {string[]} body
- * @returns {string[]}
- */
-function extractHeredocCommandSubstitutions(body) {
-  const text = body.join('\n');
-  const substitutions = new Set();
-  let escaped = false;
-  for (let i = 0; i < text.length; i += 1) {
-    const ch = text[i];
-    if (escaped) {
-      escaped = false;
-      continue;
-    }
-    if (ch === '\\') {
-      escaped = true;
-      continue;
-    }
-    if (ch === '`' || (ch === '$' && text[i + 1] === '(')) {
-      for (const substitution of extractCommandSubstitutions(text.slice(i))) {
-        substitutions.add(substitution);
-      }
-    }
-  }
-  return [...substitutions];
-}
-
-/**
- * Remove heredoc payload text before classifying the surrounding shell
- * command. Prose in a heredoc is data, so matching it as a command produces
- * false positives. Unquoted heredocs can still execute `$()` and backtick
- * substitutions; retain the complete payload whenever either syntax appears.
- * Quoted heredoc delimiters disable expansion, so their payload is fully inert.
- * Ambiguous shell syntax returns the original input unchanged (fail closed).
- *
- * @param {string} input
- * @returns {string}
- */
-function stripHeredocBodies(input) {
-  const raw = String(input || '');
-  const kept = [];
-  const pending = [];
-
-  for (const line of raw.split(/\r?\n/)) {
-    if (pending.length > 0) {
-      const current = pending[0];
-      // Bash removes backslash-newline pairs in an unquoted heredoc before
-      // comparing delimiters. Preserve the original input when physical lines
-      // can be joined into a terminator or executable expansion.
-      if (!current.quoted && /\\$/.test(line)) return raw;
-      const delimiterLine = current.stripTabs ? line.replace(/^\t+/, '') : line;
-      if (delimiterLine === current.delimiter) {
-        if (!current.quoted) {
-          kept.push(...extractHeredocCommandSubstitutions(current.body));
-        }
-        pending.shift();
-      } else {
-        current.body.push(line);
-      }
-      continue;
-    }
-
-    kept.push(line);
-    const heredocs = findHeredocs(line);
-    if (heredocs === null) return raw;
-    pending.push(...heredocs.map(heredoc => ({ ...heredoc, body: [] })));
-  }
-
-  for (const current of pending) {
-    if (!current.quoted) {
-      kept.push(...extractHeredocCommandSubstitutions(current.body));
-    }
-  }
-
-  return kept.join('\n');
-}
-
 /**
  * Promote subshell delimiters to top-level segment separators so the
  * destructive check applies inside `$(...)` and backtick subshells.
diff --git a/scripts/hooks/gateguard-heredoc.js b/scripts/hooks/gateguard-heredoc.js
new file mode 100644
index 000000000..57cd6f459
--- /dev/null
+++ b/scripts/hooks/gateguard-heredoc.js
@@ -0,0 +1,172 @@
+'use strict';
+
+const { extractCommandSubstitutions } = require('../lib/shell-substitution');
+
+/**
+ * Recognize the deliberately narrow passive sink supported by this parser.
+ * Shell operators and substitutions make the payload's destination ambiguous,
+ * so every other form retains the original input for fail-closed checks.
+ *
+ * @param {string} line
+ * @returns {boolean}
+ */
+function isProvenPassiveHeredocLine(line) {
+  const trimmed = line.trim();
+  return /^cat(?=\s|[<>])/.test(trimmed) && !/[;&|()`]/.test(trimmed);
+}
+
+/**
+ * Parse a heredoc delimiter after a verified `<<` operator.
+ *
+ * @param {string} line
+ * @param {number} operatorIndex
+ * @returns {{ heredoc: { delimiter: string, quoted: boolean, stripTabs: boolean }, endIndex: number } | null}
+ */
+function parseHeredocDelimiter(line, operatorIndex) {
+  let endIndex = operatorIndex + 2;
+  const stripTabs = line[endIndex] === '-';
+  if (stripTabs) endIndex += 1;
+  while (endIndex < line.length && /[ \t]/.test(line[endIndex])) endIndex += 1;
+
+  let delimiter = '';
+  let quoted = false;
+  const delimiterQuote = line[endIndex] === '"' || line[endIndex] === "'" ? line[endIndex] : null;
+  if (delimiterQuote) {
+    quoted = true;
+    const closingQuote = line.indexOf(delimiterQuote, endIndex + 1);
+    if (closingQuote < 0) return null;
+    delimiter = line.slice(endIndex + 1, closingQuote);
+    endIndex = closingQuote;
+  } else {
+    const match = line.slice(endIndex).match(/^[A-Za-z_][A-Za-z0-9_]*/);
+    if (!match) return null;
+    delimiter = match[0];
+    endIndex += delimiter.length - 1;
+  }
+
+  if (!/^[A-Za-z_][A-Za-z0-9_]*$/.test(delimiter)) return null;
+  const next = line[endIndex + 1];
+  if (next && !/[\s;&|<>()]/.test(next)) return null;
+  return { heredoc: { delimiter, quoted, stripTabs }, endIndex };
+}
+
+/**
+ * Find simple heredoc redirections on one complete shell command line.
+ * Anything ambiguous returns null so the caller can fail closed.
+ *
+ * @param {string} line
+ * @returns {{ delimiter: string, quoted: boolean, stripTabs: boolean }[] | null}
+ */
+function findHeredocs(line) {
+  const heredocs = [];
+  let quote = null;
+  let escaped = false;
+  for (let i = 0; i < line.length; i += 1) {
+    const ch = line[i];
+    if (quote === "'") {
+      if (ch === "'") quote = null;
+      continue;
+    }
+    if (escaped) {
+      escaped = false;
+      continue;
+    }
+    if (ch === '\\') {
+      escaped = true;
+      continue;
+    }
+    if (quote === '"') {
+      if (ch === quote) quote = null;
+      continue;
+    }
+    if (ch === '"' || ch === "'") {
+      quote = ch;
+      continue;
+    }
+    if ((ch === '$' && line[i + 1] === '(' && line[i + 2] === '(') || (ch === '(' && line[i + 1] === '(')) return null;
+    if (ch === '$' && line[i + 1] === '[') return null;
+    if (ch === '#' && (i === 0 || /[\s;&|()]/.test(line[i - 1]))) break;
+    if (ch !== '<' || line[i + 1] !== '<') continue;
+    if (line[i + 2] === '<') return null;
+    const prefix = line.slice(0, i);
+    if (prefix.includes('((') || prefix.includes('[[')) return null;
+    const parsed = parseHeredocDelimiter(line, i);
+    if (!parsed) return null;
+    heredocs.push(parsed.heredoc);
+    i = parsed.endIndex;
+  }
+  return quote || escaped ? null : heredocs;
+}
+
+/**
+ * Extract executable substitutions from an unquoted heredoc. Quote characters
+ * in its payload are literal and do not suppress expansion.
+ *
+ * @param {string[]} body
+ * @returns {string[]}
+ */
+function extractHeredocCommandSubstitutions(body) {
+  const text = body.join('\n');
+  const substitutions = new Set();
+  let escaped = false;
+  for (let i = 0; i < text.length; i += 1) {
+    const ch = text[i];
+    if (escaped) {
+      escaped = false;
+      continue;
+    }
+    if (ch === '\\') {
+      escaped = true;
+      continue;
+    }
+    if (ch === '`' || (ch === '$' && text[i + 1] === '(')) {
+      for (const substitution of extractCommandSubstitutions(text.slice(i))) {
+        substitutions.add(substitution);
+      }
+    }
+  }
+  return [...substitutions];
+}
+
+/**
+ * Remove heredoc payload text before classifying the surrounding shell
+ * command. Prose in a heredoc is data, so matching it as a command produces
+ * false positives. Unquoted heredocs can still execute `$()` and backtick
+ * substitutions; retain only those substitution bodies for classification and
+ * drop the remaining payload text. Quoted heredoc payloads are fully inert.
+ * Ambiguous shell syntax returns the original input unchanged (fail closed).
+ *
+ * @param {string} input
+ * @returns {string}
+ */
+function stripHeredocBodies(input) {
+  const raw = String(input || '');
+  const kept = [];
+  const pending = [];
+  let completedHeredoc = false;
+  for (const line of raw.split(/\r?\n/)) {
+    if (pending.length > 0) {
+      const current = pending[0];
+      if (!current.quoted && /\\$/.test(line)) return raw;
+      const delimiterLine = current.stripTabs ? line.replace(/^\t+/, '') : line;
+      if (delimiterLine === current.delimiter) {
+        if (!current.quoted) kept.push(...extractHeredocCommandSubstitutions(current.body));
+        pending.shift();
+        if (pending.length === 0) completedHeredoc = true;
+      } else {
+        current.body.push(line);
+      }
+      continue;
+    }
+    if (completedHeredoc && line.trim()) return raw;
+    kept.push(line);
+    const heredocs = findHeredocs(line);
+    if (heredocs === null) return raw;
+    if (heredocs.length > 0 && !isProvenPassiveHeredocLine(line)) return raw;
+    pending.push(...heredocs.map(heredoc => ({ ...heredoc, body: [] })));
+  }
+  if (pending.length > 0) return raw;
+  return kept.join('\n');
+}
+
+module.exports = { stripHeredocBodies };
diff --git a/tests/hooks/gateguard-fact-force.test.js b/tests/hooks/gateguard-fact-force.test.js
index 477fa28f8..b5b18cf8c 100644
--- a/tests/hooks/gateguard-fact-force.test.js
+++ b/tests/hooks/gateguard-fact-force.test.js
@@ -1522,6 +1522,71 @@ function runTests() {
     passed++;
   else failed++;
 
+  if (
+    test('handles multiple heredoc redirections in declaration order', () => {
+      expectAllow(
+        [
+          "cat <<ONE <<'TWO'",
+          'DELETE FROM sessions is documentation here.',
+          'ONE',
+          '$(rm -rf /tmp/example-only)',
+          'TWO'
+        ].join('\n'),
+        'multiple heredoc redirections'
+      );
+    })
+  )
+    passed++;
+  else failed++;
+
+  if (
+    test('fails closed when a shell consumes the heredoc payload', () => {
+      for (const command of [
+        ['bash <<EOF', 'rm -rf /tmp/shell-input-target', 'EOF'].join('\n'),
+        ["sh <<'EOF'", 'git reset --hard', 'EOF'].join('\n'),
+        ['cat <<EOF | sh', 'rm -rf /tmp/piped-shell-target', 'EOF'].join('\n'),
+        ["cat > /tmp/review-script <<'EOF'", 'rm -rf /tmp/persisted-target', 'EOF', 'bash /tmp/review-script'].join('\n')
+      ]) {
+        expectDestructiveDeny(command, 'shell-executed heredoc payload');
+      }
+    })
+  )
+    passed++;
+  else failed++;
+
+  if (
+    test('does not rescan a here-string as a heredoc', () => {
+      expectDestructiveDeny(
+        ['cat <<<EOF', 'rm -rf /tmp/here-string-followup'].join('\n'),
+        'command after here-string'
+      );
+    })
+  )
+    passed++;
+  else failed++;
+
+  if (
+    test('uses shell-correct single-quote escaping while finding heredocs', () => {
+      expectDestructiveDeny(
+        ["echo 'a\\'X'<<EOF 'Y'b\\'", 'rm -rf /tmp/quoted-followup'].join('\n'),
+        'command after quoted non-heredoc text'
+      );
+    })
+  )
+    passed++;
+  else failed++;
+
+  if (
+    test('fails closed on an unclosed heredoc body', () => {
+      expectDestructiveDeny(
+        ['cat <<EOF', 'rm -rf /tmp/unclosed-heredoc'].join('\n'),
+        'unclosed heredoc body'
+      );
+    })
+  )
+    passed++;
+  else failed++;
+
   if (
     test('still denies destructive commands after a heredoc terminator', () => {
       expectDestructiveDeny(