From 7cfc9b36081f36b8bef7a0cf92ea440acef75431 Mon Sep 17 00:00:00 2001 From: Frank_zhu <58329837+Frank-zhu0404@users.noreply.github.com> Date: Thu, 17 Sep 2026 17:29:26 +0000 Subject: [PATCH] fix(gateguard): ignore heredoc prose for tee and path-qualified sinks (#2886) Expand proven-passive heredoc recognition beyond bare `cat` so documentation writes via `tee`, `/bin/cat`, and `command cat` no longer trip the destructive command detector on body text, while still failing closed for shells and pipes. --- scripts/hooks/gateguard-heredoc.js | 15 +++-- tests/hooks/gateguard-fact-force.test.js | 81 ++++++++++++++++++++++++ 2 files changed, 92 insertions(+), 4 deletions(-) diff --git a/scripts/hooks/gateguard-heredoc.js b/scripts/hooks/gateguard-heredoc.js index 31e41b29b..79e2df50f 100644 --- a/scripts/hooks/gateguard-heredoc.js +++ b/scripts/hooks/gateguard-heredoc.js @@ -3,16 +3,23 @@ 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. + * Recognize proven-passive sinks whose heredoc payload is data, not a command + * stream. `cat` and `tee` (optionally path-qualified, or wrapped in + * `command`/`builtin`/`env`) only write stdin; they do not execute the body. + * Shell operators or substitution markers make the 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); + // Fail closed on control operators / grouping / command substitutions. + if (/[;&|()`]/.test(trimmed)) return false; + // Optional wrapper + optional path prefix + cat|tee, then args or redirect. + return /^(?:(?:command|builtin|env)\s+)?(?:(?:\.\/|\/(?:[\w.+-]+\/)*)?(?:cat|tee))(?=\s|[<>])/.test( + trimmed + ); } /** diff --git a/tests/hooks/gateguard-fact-force.test.js b/tests/hooks/gateguard-fact-force.test.js index 495928691..8acde76e1 100644 --- a/tests/hooks/gateguard-fact-force.test.js +++ b/tests/hooks/gateguard-fact-force.test.js @@ -1858,6 +1858,87 @@ function runTests() { passed++; else failed++; + if ( + test('allows #2886 migration-doc heredoc repro with DROP TABLE prose', () => { + expectAllow( + [ + "cat > migration-notes.md <<'EOF'", + "This migration will DROP TABLE old_sessions once we've verified nothing reads from it anymore.", + 'EOF' + ].join('\n'), + 'issue #2886 cat heredoc repro' + ); + }) + ) + passed++; + else failed++; + + if ( + test('allows destructive SQL prose inside a tee heredoc', () => { + expectAllow( + [ + "tee migration-notes.md <<'EOF'", + 'This migration will DROP TABLE old_sessions after verification.', + 'EOF' + ].join('\n'), + 'tee heredoc SQL prose' + ); + }) + ) + passed++; + else failed++; + + if ( + test('allows destructive rm prose inside a path-qualified cat heredoc', () => { + expectAllow( + [ + "/bin/cat > notes.md <<'EOF'", + 'Cleanup steps mention rm -rf old-cache; do not run yet.', + 'EOF' + ].join('\n'), + 'path-qualified cat heredoc prose' + ); + }) + ) + passed++; + else failed++; + + if ( + test('allows destructive prose inside a command-wrapped cat heredoc', () => { + expectAllow( + [ + "command cat > notes.md <<'EOF'", + 'Notes: DELETE FROM sessions; truncate staging.', + 'EOF' + ].join('\n'), + 'command-wrapped cat heredoc prose' + ); + }) + ) + passed++; + else failed++; + + if ( + test('still denies real destructive commands (not heredoc prose)', () => { + expectDestructiveDeny('rm -rf /tmp/real-destructive-target', 'real rm -rf'); + expectDestructiveDeny('git reset --hard', 'real git reset --hard'); + expectDestructiveDeny('drop table old_sessions', 'real drop table command text'); + }) + ) + passed++; + else failed++; + + if ( + test('fails closed when tee pipes heredoc payload into a shell', () => { + expectDestructiveDeny( + ['tee notes.md < { for (const payload of [