diff --git a/scripts/hooks/gateguard-fact-force.js b/scripts/hooks/gateguard-fact-force.js index bf91eb78a..1228816b6 100644 --- a/scripts/hooks/gateguard-fact-force.js +++ b/scripts/hooks/gateguard-fact-force.js @@ -53,7 +53,11 @@ const ECC_ENABLE_VALUES = new Set(['1', 'true', 'on', 'enabled', 'enable', 'yes' // phrases without shell-flag ordering concerns. Quoted strings are // stripped before this regex runs so a commit message mentioning // "drop table" no longer triggers a false positive. -const DESTRUCTIVE_SQL_DD = /\b(drop\s+table|delete\s+from|truncate|dd\s+if=)\b/i; +// The trailing \b applies only to the arms that end in a word character. +// `dd\s+if=` ends in `=`, so a shared \b demanded that the NEXT character be a +// word character and the disk-wipe spellings slipped through: `dd if=/dev/zero` +// and `dd if=./img` were allowed while `dd if=x` was denied (#2642). +const DESTRUCTIVE_SQL_DD = /\b(?:drop\s+table|delete\s+from|truncate)\b|\bdd\s+if=/i; // Operator-supplied additional destructive patterns. Lazily compiled from // `GATEGUARD_BASH_EXTRA_DESTRUCTIVE` (regex source) on first use, then diff --git a/tests/hooks/gateguard-fact-force.test.js b/tests/hooks/gateguard-fact-force.test.js index 54a19c0e0..f12fedda5 100644 --- a/tests/hooks/gateguard-fact-force.test.js +++ b/tests/hooks/gateguard-fact-force.test.js @@ -255,6 +255,67 @@ function runTests() { passed++; else failed++; + // --- Test 4b: dd targets that do not start with a word character --- + /** + * #2642: DESTRUCTIVE_SQL_DD carried one trailing \b across every alternation + * arm. `dd\s+if=` ends in `=`, so that \b demanded the NEXT character be a + * word character: `dd if=x` was denied while the disk-wipe spelling + * `dd if=/dev/zero of=/dev/sda` and the relative `dd if=./img` were allowed. + * These run through the real hook, since the report is specifically that the + * published hook lets the slash-prefixed form through. + */ + for (const command of [ + 'dd if=/dev/zero of=/dev/sda', + 'dd if=./disk.img of=/dev/sdb', + 'dd if="/dev/zero" of=/dev/sda' + ]) { + clearState(); + if ( + test(`denies dd whose input path is not word-initial: ${command}`, () => { + const result = runBashHook({ tool_name: 'Bash', tool_input: { command } }); + const output = parseOutput(result.stdout); + assert.ok(output, 'hook should produce JSON output'); + assert.ok(output.hookSpecificOutput, 'hook should return a permission decision'); + assert.strictEqual( + output.hookSpecificOutput.permissionDecision, + 'deny', + `${command} must be gated as destructive` + ); + assert.ok(output.hookSpecificOutput.permissionDecisionReason.includes('Destructive')); + }) + ) + passed++; + else failed++; + } + + // --- Test 4c: widening the dd arm must not gate ordinary commands --- + /** + * The fix drops the trailing \b only from the dd arm, so the arms that end in + * a word character keep theirs. Without that split, `truncated` would match + * `truncate`, and `add if=` would match `dd if=`. + */ + for (const command of ['echo add if=1', 'echo truncated output', 'git status']) { + clearState(); + if ( + test(`does not gate as destructive: ${command}`, () => { + // Prime the session so the separate first-command routine gate cannot + // be mistaken for a destructive denial. + runBashHook({ tool_name: 'Bash', tool_input: { command: 'printf ready' } }); + const result = runBashHook({ tool_name: 'Bash', tool_input: { command } }); + const output = parseOutput(result.stdout); + if (output && output.hookSpecificOutput) { + const reason = output.hookSpecificOutput.permissionDecisionReason || ''; + assert.ok( + output.hookSpecificOutput.permissionDecision !== 'deny' || !reason.includes('Destructive'), + `${command} must not be gated as destructive` + ); + } + }) + ) + passed++; + else failed++; + } + // --- Test 5: denies first routine Bash, allows second --- clearState(); if (