From 962380c452b9e507c5e894b20bb6eda555346b91 Mon Sep 17 00:00:00 2001 From: dajiaohuang Date: Fri, 28 Aug 2026 05:03:12 +0800 Subject: [PATCH] fix: ignore heredoc prose in GateGuard --- scripts/hooks/gateguard-fact-force.js | 169 ++++++++++++- tests/hooks/gateguard-fact-force.test.js | 291 +++++++++++++++++++++++ 2 files changed, 457 insertions(+), 3 deletions(-) diff --git a/scripts/hooks/gateguard-fact-force.js b/scripts/hooks/gateguard-fact-force.js index 3f7f9ed80..2cd852a93 100644 --- a/scripts/hooks/gateguard-fact-force.js +++ b/scripts/hooks/gateguard-fact-force.js @@ -151,6 +151,168 @@ 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. @@ -672,7 +834,8 @@ function isDestructiveBash(command) { // after quoting AND subshell delimiters are normalized so phrases // inside `$(...)` or backticks are also caught. const raw = String(command || ''); - const flattened = explodeSubshells(stripQuotedStrings(raw)); + const executable = stripHeredocBodies(raw); + const flattened = explodeSubshells(stripQuotedStrings(executable)); if (DESTRUCTIVE_SQL_DD.test(flattened)) return true; // Operator-supplied additional destructive patterns. Same scope as the @@ -687,7 +850,7 @@ function isDestructiveBash(command) { // isDestructiveFindExec would turn `find . -exec 'rm' {} \;` into `find . -exec {} \;` // — the binary name disappears and the check returns false. Using raw body text avoids // that false-negative while also catching `&&`, `;`, `|`, and `||` compound forms. - const bodies = collectExecutableBodies(raw); + const bodies = collectExecutableBodies(executable); for (const body of bodies) { for (const rawSeg of body .split(/[;|&]+/) @@ -709,7 +872,7 @@ function isDestructiveBash(command) { // Quote-aware pass: closes the quoted-command-word, newline-separator, // quoted-find-exec, and sh/bash -c bypasses (GHSA-4v57-ph3x-gf55). - if (isDestructiveQuoteAware(raw)) return true; + if (isDestructiveQuoteAware(executable)) return true; return false; } diff --git a/tests/hooks/gateguard-fact-force.test.js b/tests/hooks/gateguard-fact-force.test.js index 8912eb994..477fa28f8 100644 --- a/tests/hooks/gateguard-fact-force.test.js +++ b/tests/hooks/gateguard-fact-force.test.js @@ -1477,6 +1477,297 @@ function runTests() { passed++; else failed++; + if ( + test('allows destructive SQL prose inside a quoted heredoc', () => { + expectAllow( + [ + "cat > migration-notes.md <<'EOF'", + 'This migration will DROP TABLE old_sessions after verification.', + 'EOF' + ].join('\n'), + 'quoted heredoc SQL prose' + ); + }) + ) + passed++; + else failed++; + + if ( + test('allows destructive prose and separators inside an unquoted heredoc', () => { + expectAllow( + [ + 'cat > migration-notes.md < { + expectAllow( + [ + 'cat > migration-notes.md <<-EOF', + '\tTRUNCATE old_sessions; rm -rf old-cache', + '\tEOF' + ].join('\n'), + 'tab-stripping heredoc prose' + ); + }) + ) + passed++; + else failed++; + + if ( + test('still denies destructive commands after a heredoc terminator', () => { + expectDestructiveDeny( + [ + "cat > migration-notes.md <<'EOF'", + 'DROP TABLE is documentation here.', + 'EOF', + 'rm -rf /tmp/real-target' + ].join('\n'), + 'command after heredoc terminator' + ); + }) + ) + passed++; + else failed++; + + if ( + test('still denies command substitutions inside an unquoted heredoc', () => { + expectDestructiveDeny( + [ + 'cat > output.txt < { + expectAllow( + [ + "cat > example.md <<'EOF'", + '$(rm -rf /tmp/example-only)', + 'EOF' + ].join('\n'), + 'quoted heredoc command-substitution prose' + ); + }) + ) + passed++; + else failed++; + + if ( + test('does not mistake an arithmetic shift for a heredoc', () => { + expectDestructiveDeny( + ['echo $((1 << 2))', 'rm -rf /tmp/real-target'].join('\n'), + 'command after arithmetic shift' + ); + }) + ) + passed++; + else failed++; + + if ( + test('does not mistake a named arithmetic shift operand for a heredoc', () => { + expectDestructiveDeny( + ['echo $((flags << WIDTH))', 'rm -rf /tmp/real-target'].join('\n'), + 'command after named arithmetic shift' + ); + }) + ) + passed++; + else failed++; + + if ( + test('fails closed on multiline arithmetic shift contexts', () => { + for (const arithmetic of [ + ['((', 'flags << WIDTH', '))'], + ['$((', 'flags << WIDTH', '))'], + ['$[', 'flags << WIDTH', ']'] + ]) { + expectDestructiveDeny( + [...arithmetic, 'rm -rf /tmp/real-target'].join('\n'), + 'command after multiline arithmetic shift' + ); + } + }) + ) + passed++; + else failed++; + + if ( + test('does not mistake a conditional string operator for a heredoc', () => { + expectDestructiveDeny( + ['[[ alpha << omega ]]', 'rm -rf /tmp/real-target'].join('\n'), + 'command after conditional shift-like operator' + ); + }) + ) + passed++; + else failed++; + + if ( + test('does not parse heredocs inside operator-adjacent comments', () => { + expectDestructiveDeny( + ['true;# < { + expectDestructiveDeny( + ['printf \'%s\' "literal', '< { + expectDestructiveDeny( + ["cat <<$'EOF'", 'documentation', 'EOF', 'rm -rf /tmp/real-target'].join('\n'), + 'command after ANSI-C heredoc' + ); + }) + ) + passed++; + else failed++; + + if ( + test('fails closed on escaped heredoc delimiter words', () => { + expectDestructiveDeny( + ['cat < { + expectDestructiveDeny( + ['cat < { + expectDestructiveDeny( + ['cat < { + expectDestructiveDeny( + ['cat < { + expectDestructiveDeny( + ['cat < { + expectAllow( + ['cat < { + for (const payload of [ + "'$(rm -rf /tmp/expanded-target)'", + '"$(rm -rf /tmp/expanded-target)"', + "'`rm -rf /tmp/expanded-target`'" + ]) { + expectDestructiveDeny( + ['cat < { + expectAllow( + ['cat < { + expectDestructiveDeny( + ['echo $((1 << 2))', 'rm -rf /tmp/shift-target'].join('\n'), + 'command after $((...)) arithmetic shift' + ); + expectDestructiveDeny( + ['echo $((x << 2))', 'rm -rf /tmp/shift-target'].join('\n'), + 'command after $((...)) identifier shift' + ); + expectDestructiveDeny( + ['(( 1 << 2 ))', 'rm -rf /tmp/shift-target'].join('\n'), + 'command after ((...)) arithmetic shift' + ); + expectDestructiveDeny( + ['echo $[x << 1]', 'rm -rf /tmp/shift-target'].join('\n'), + 'command after legacy $[...] arithmetic shift' + ); + }) + ) + passed++; + else failed++; + if ( test('allows git push --force-if-includes as a safety-checked variant', () => { expectAllow('git push --force-with-lease --force-if-includes origin main', 'git push --force-if-includes');