From 3cdc69a0ead99e8d5929efc95477af1b9f8ed1d0 Mon Sep 17 00:00:00 2001 From: kapil971390 Date: Fri, 19 Jun 2026 02:00:46 +0530 Subject: [PATCH] fix(gateguard): check isDestructiveFindExec on each command segment to close compound-command bypass (#2292) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(gateguard): check isDestructiveFindExec on each command segment `isDestructiveBash` called `isDestructiveFindExec` only on the raw full command string. When the raw string starts with a non-find command (e.g. `echo x && find . -exec rm {} \;`), `isDestructiveFindExec` checks tokens[0] and returns false — then the per-segment loop never calls it again, letting the destructive `find -exec rm` segment through silently. Fix: call `isDestructiveFindExec(segment)` inside the per-segment loop so compound commands (`&&`, `;`, `|`) cannot be used to prepend a harmless command and bypass the find-exec destructive check. Adds three regression tests covering `&&`, `;`, and `|` bypass patterns. * fix(gateguard): use raw body segments for isDestructiveFindExec to close quoted-binary gap The previous per-segment call passed quote-stripped output from splitCommandSegments to isDestructiveFindExec, so a quoted exec binary like find . -exec 'rm' {} \; would arrive as find . -exec {} \; and the check would silently miss it. Switch to splitting collectExecutableBodies output on [;|&]+ without quote-stripping first, so the find-exec binary name is always intact when isDestructiveFindExec inspects it. This also covers || and background & separators that the original tests did not exercise. Adds a regression test for the || OR-chain bypass pattern. Addresses Greptile review comments on PR #2292. --------- Co-authored-by: kapilvus --- scripts/hooks/gateguard-fact-force.js | 16 +++++++++++++--- tests/hooks/gateguard-fact-force.test.js | 20 ++++++++++++++++++++ 2 files changed, 33 insertions(+), 3 deletions(-) diff --git a/scripts/hooks/gateguard-fact-force.js b/scripts/hooks/gateguard-fact-force.js index fa5faead..19d8b01a 100644 --- a/scripts/hooks/gateguard-fact-force.js +++ b/scripts/hooks/gateguard-fact-force.js @@ -545,10 +545,20 @@ function isDestructiveBash(command) { const extra = getExtraDestructiveRegex(); if (extra && extra.test(flattened)) return true; - // Check for destructive find -exec patterns - if (isDestructiveFindExec(raw)) return true; + // Check for destructive find -exec patterns on raw body segments (before quote-stripping) + // so that quoted exec binaries and compound-command prefixes are both handled correctly. + // splitCommandSegments strips quotes before splitting, so passing its output to + // 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); + for (const body of bodies) { + for (const rawSeg of body.split(/[;|&]+/).map(s => s.trim()).filter(Boolean)) { + if (isDestructiveFindExec(rawSeg)) return true; + } + } - const segments = collectExecutableBodies(raw).flatMap(splitCommandSegments); + const segments = bodies.flatMap(splitCommandSegments); for (const segment of segments) { const stripped = stripQuotedStrings(segment); if (DESTRUCTIVE_SQL_DD.test(stripped)) return true; diff --git a/tests/hooks/gateguard-fact-force.test.js b/tests/hooks/gateguard-fact-force.test.js index 610b782a..93dfd7ea 100644 --- a/tests/hooks/gateguard-fact-force.test.js +++ b/tests/hooks/gateguard-fact-force.test.js @@ -1613,6 +1613,26 @@ function runTests() { 'find -exec git reset --hard'); })) passed++; else failed++; + if (test('denies find -exec rm {} \\; preceded by && (bypass via compound command)', () => { + expectDestructiveDeny('echo x && find . -exec rm {} \\;', + 'compound command bypass: find -exec rm'); + })) passed++; else failed++; + + if (test('denies find -exec rm -rf {} \\; preceded by ; (bypass via semicolon)', () => { + expectDestructiveDeny('true; find . -name "*.log" -exec rm -rf {} \\;', + 'semicolon bypass: find -exec rm -rf'); + })) passed++; else failed++; + + if (test('denies find -exec rm {} \\; in pipeline (bypass via pipe)', () => { + expectDestructiveDeny('echo start | find . -exec rm {} \\;', + 'pipe bypass: find -exec rm'); + })) passed++; else failed++; + + if (test('denies find -exec rm {} \\; after || (OR-chain bypass)', () => { + expectDestructiveDeny('false || find . -exec rm {} \\;', + 'OR-chain bypass: find -exec rm'); + })) passed++; else failed++; + if (test('allows find -exec echo {} \\; (non-destructive, routine gate)', () => { clearState(); const input = { tool_name: 'Bash', tool_input: { command: 'find . -name "*.tmp" -exec echo {} \\;' } };