diff --git a/scripts/hooks/gateguard-fact-force.js b/scripts/hooks/gateguard-fact-force.js index c7c84c5a..741d3032 100644 --- a/scripts/hooks/gateguard-fact-force.js +++ b/scripts/hooks/gateguard-fact-force.js @@ -46,6 +46,7 @@ const ROUTINE_BASH_SESSION_KEY = '__bash_session__'; const EDIT_WRITE_HOOK_ID = 'pre:edit-write:gateguard-fact-force'; const BASH_HOOK_ID = 'pre:bash:gateguard-fact-force'; const ECC_DISABLE_VALUES = new Set(['0', 'false', 'off', 'disabled', 'disable']); +const ECC_ENABLE_VALUES = new Set(['1', 'true', 'on', 'enabled', 'enable', 'yes']); // SQL-keyword + dd patterns stay as a single regex — they are stable // phrases without shell-flag ordering concerns. Quoted strings are @@ -53,6 +54,54 @@ const ECC_DISABLE_VALUES = new Set(['0', 'false', 'off', 'disabled', 'disable']) // "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; +// Operator-supplied additional destructive patterns. Lazily compiled from +// `GATEGUARD_BASH_EXTRA_DESTRUCTIVE` (regex source) on first use, then +// memoized keyed by the env-var value so a test or long-running process +// that flips the env between calls re-reads it without paying for a +// recompile on every invocation. A malformed regex is treated as +// "not configured" (the gate falls back to the built-in patterns) and +// the parse failure is logged once via `[gateguard-fact-force]` to +// stderr — hooks must never crash tool execution because of operator +// config errors. +let extraDestructiveCacheKey = null; +let extraDestructiveCacheRegex = null; +let extraDestructiveWarnLogged = false; +function getExtraDestructiveRegex() { + const raw = process.env.GATEGUARD_BASH_EXTRA_DESTRUCTIVE || ''; + if (!raw) { + extraDestructiveCacheKey = ''; + extraDestructiveCacheRegex = null; + return null; + } + if (raw === extraDestructiveCacheKey) { + return extraDestructiveCacheRegex; + } + // The env value just changed; reset the once-per-pattern warning gate + // so a subsequent *different* invalid regex is also reported once. The + // previous shape kept the flag sticky and silently swallowed the + // second bad pattern in a long-running process. + extraDestructiveCacheKey = raw; + extraDestructiveWarnLogged = false; + try { + extraDestructiveCacheRegex = new RegExp(raw, 'i'); + } catch (err) { + extraDestructiveCacheRegex = null; + if (!extraDestructiveWarnLogged) { + try { + process.stderr.write( + `[gateguard-fact-force] ignoring invalid GATEGUARD_BASH_EXTRA_DESTRUCTIVE regex: ${err.message}\n` + ); + } catch (_) { /* stderr write failure is non-fatal */ } + extraDestructiveWarnLogged = true; + } + } + return extraDestructiveCacheRegex; +} + +function isRoutineBashGateDisabled() { + return ECC_ENABLE_VALUES.has(normalizeEnvValue(process.env.GATEGUARD_BASH_ROUTINE_DISABLED)); +} + /** * Strip the contents of single- and double-quoted strings so phrases * mentioned inside a commit message or echoed argument do not trigger @@ -414,9 +463,17 @@ function isDestructiveBash(command) { const flattened = explodeSubshells(stripQuotedStrings(raw)); if (DESTRUCTIVE_SQL_DD.test(flattened)) return true; + // Operator-supplied additional destructive patterns. Same scope as the + // built-in SQL/dd regex: matched against the quote-stripped, subshell- + // exploded command so a phrase inside `$(...)` or backticks is caught. + const extra = getExtraDestructiveRegex(); + if (extra && extra.test(flattened)) return true; + const segments = collectExecutableBodies(raw).flatMap(splitCommandSegments); for (const segment of segments) { - if (DESTRUCTIVE_SQL_DD.test(stripQuotedStrings(segment))) return true; + const stripped = stripQuotedStrings(segment); + if (DESTRUCTIVE_SQL_DD.test(stripped)) return true; + if (extra && extra.test(stripped)) return true; const tokens = tokenize(segment); if (isDestructiveRm(tokens)) return true; if (isDestructiveGit(tokens)) return true; @@ -883,6 +940,14 @@ function run(rawInput) { return rawInput; // allow retry after facts presented } + // Operator opt-out: skip the routine-bash gate entirely. The destructive + // gate above still fires. This is the documented escape hatch for hosts + // (Cursor, OpenCode, etc.) where the once-per-session routine gate is + // friction without signal. + if (isRoutineBashGateDisabled()) { + return rawInput; // routine gate opted out via env + } + if (!isChecked(ROUTINE_BASH_SESSION_KEY)) { if (!markChecked(ROUTINE_BASH_SESSION_KEY)) { return allowWithStateWarning(); diff --git a/tests/hooks/gateguard-fact-force.test.js b/tests/hooks/gateguard-fact-force.test.js index 127b95ea..10c790c5 100644 --- a/tests/hooks/gateguard-fact-force.test.js +++ b/tests/hooks/gateguard-fact-force.test.js @@ -1406,6 +1406,218 @@ function runTests() { 'foo{ token inside brace body'); })) passed++; else failed++; + // --- Issue #2078: GATEGUARD_BASH_ROUTINE_DISABLED env var --- + // Operators on hosts that don't benefit from the once-per-session + // routine bash gate (Cursor, OpenCode, etc.) get an env-var opt-out. + // The destructive gate is unaffected. + + clearState(); + if (test('GATEGUARD_BASH_ROUTINE_DISABLED=1 skips routine bash gate', () => { + const input = { tool_name: 'Bash', tool_input: { command: 'ls -la' } }; + const result = runBashHook(input, { GATEGUARD_BASH_ROUTINE_DISABLED: '1' }); + assert.strictEqual(result.code, 0, 'exit code should be 0'); + const output = parseOutput(result.stdout); + assert.ok(output, 'should produce valid JSON output'); + if (output.hookSpecificOutput) { + assert.notStrictEqual(output.hookSpecificOutput.permissionDecision, 'deny', + 'routine bash should not be denied when env opts out'); + } else { + assert.strictEqual(output.tool_name, 'Bash', 'pass-through should preserve input'); + } + })) passed++; else failed++; + + clearState(); + if (test('GATEGUARD_BASH_ROUTINE_DISABLED accepts truthy aliases (true, on, yes, enabled)', () => { + for (const value of ['true', 'on', 'yes', 'enabled', 'TRUE', 'On']) { + clearState(); + const result = runBashHook( + { tool_name: 'Bash', tool_input: { command: 'grep foo bar.txt' } }, + { GATEGUARD_BASH_ROUTINE_DISABLED: value } + ); + const output = parseOutput(result.stdout); + assert.ok(output, `value=${value}: should produce JSON`); + if (output.hookSpecificOutput) { + assert.notStrictEqual(output.hookSpecificOutput.permissionDecision, 'deny', + `value=${value}: should not deny routine bash`); + } + } + })) passed++; else failed++; + + clearState(); + if (test('GATEGUARD_BASH_ROUTINE_DISABLED unset preserves baseline (denies first routine bash)', () => { + const input = { tool_name: 'Bash', tool_input: { command: 'ls -la' } }; + const result = runBashHook(input); + assert.strictEqual(result.code, 0, 'exit code should be 0'); + const output = parseOutput(result.stdout); + assert.ok(output, 'should produce JSON'); + assert.strictEqual(output.hookSpecificOutput.permissionDecision, 'deny', + 'baseline routine gate must still fire when env is unset'); + })) passed++; else failed++; + + clearState(); + if (test('GATEGUARD_BASH_ROUTINE_DISABLED=0 / off / false keeps current behavior', () => { + for (const value of ['0', 'false', 'off', '', 'random-value']) { + clearState(); + const result = runBashHook( + { tool_name: 'Bash', tool_input: { command: 'ls -la' } }, + { GATEGUARD_BASH_ROUTINE_DISABLED: value } + ); + const output = parseOutput(result.stdout); + assert.ok(output, `value="${value}": should produce JSON`); + assert.strictEqual(output.hookSpecificOutput.permissionDecision, 'deny', + `value="${value}": routine gate should still fire`); + } + })) passed++; else failed++; + + clearState(); + if (test('GATEGUARD_BASH_ROUTINE_DISABLED=1 does NOT disable destructive bash gate', () => { + const input = { tool_name: 'Bash', tool_input: { command: 'rm -rf /important/data' } }; + const result = runBashHook(input, { GATEGUARD_BASH_ROUTINE_DISABLED: '1' }); + const output = parseOutput(result.stdout); + assert.ok(output, 'should produce JSON'); + assert.strictEqual(output.hookSpecificOutput.permissionDecision, 'deny', + 'destructive gate must still fire even when routine gate is opted out'); + assert.ok(output.hookSpecificOutput.permissionDecisionReason.includes('Destructive'), + 'reason should mention Destructive'); + })) passed++; else failed++; + + // --- Issue #2078: GATEGUARD_BASH_EXTRA_DESTRUCTIVE env var --- + // Operators can register additional destructive patterns without + // patching the bundled JS. Same matching scope as the built-in + // SQL/dd regex (matches against quote-stripped, subshell-flattened + // command) so a custom phrase inside `$(...)` is also caught. + + clearState(); + if (test('GATEGUARD_BASH_EXTRA_DESTRUCTIVE custom phrase fires destructive gate', () => { + const input = { tool_name: 'Bash', tool_input: { command: 'supabase db reset --linked' } }; + const result = runBashHook(input, { + GATEGUARD_BASH_EXTRA_DESTRUCTIVE: 'supabase\\s+db\\s+reset|prisma\\s+migrate\\s+reset' + }); + const output = parseOutput(result.stdout); + assert.ok(output, 'should produce JSON'); + assert.strictEqual(output.hookSpecificOutput.permissionDecision, 'deny', + 'custom destructive phrase should be gated'); + assert.ok(output.hookSpecificOutput.permissionDecisionReason.includes('Destructive'), + 'reason should mention Destructive'); + })) passed++; else failed++; + + clearState(); + if (test('GATEGUARD_BASH_EXTRA_DESTRUCTIVE second member of alternation also fires', () => { + const input = { tool_name: 'Bash', tool_input: { command: 'prisma migrate reset --force' } }; + const result = runBashHook(input, { + GATEGUARD_BASH_EXTRA_DESTRUCTIVE: 'supabase\\s+db\\s+reset|prisma\\s+migrate\\s+reset' + }); + const output = parseOutput(result.stdout); + assert.ok(output, 'should produce JSON'); + assert.strictEqual(output.hookSpecificOutput.permissionDecision, 'deny', + 'second alternation member should be gated'); + })) passed++; else failed++; + + clearState(); + if (test('GATEGUARD_BASH_EXTRA_DESTRUCTIVE invalid regex degrades to baseline (no crash)', () => { + // Unbalanced paren is a regex parse error. Hook must NOT crash; it + // should fall back to the built-in patterns. A plain `ls` should + // therefore hit the routine gate (denied first time) and a + // built-in destructive (`rm -rf`) should still fire the destructive gate. + const lsResult = runBashHook( + { tool_name: 'Bash', tool_input: { command: 'ls -la' } }, + { GATEGUARD_BASH_EXTRA_DESTRUCTIVE: '(unclosed' } + ); + assert.strictEqual(lsResult.code, 0, 'malformed regex must not crash hook'); + const lsOutput = parseOutput(lsResult.stdout); + assert.ok(lsOutput, 'should produce JSON despite bad env regex'); + // Note: with invalid extra regex, the bash branch behaves as if the + // env var was unset — routine gate fires on first `ls`, destructive + // gate fires on `rm -rf`. + assert.strictEqual(lsOutput.hookSpecificOutput.permissionDecision, 'deny', + 'baseline routine gate should still fire when extra-regex is malformed'); + })) passed++; else failed++; + + clearState(); + if (test('GATEGUARD_BASH_EXTRA_DESTRUCTIVE unset does not affect baseline', () => { + const input = { tool_name: 'Bash', tool_input: { command: 'supabase db reset --linked' } }; + const result = runBashHook(input); + assert.strictEqual(result.code, 0, 'exit code should be 0'); + const output = parseOutput(result.stdout); + assert.ok(output, 'should produce JSON'); + // Without the extra regex, `supabase db reset` is a routine bash + // command and should hit the routine gate (deny first time) — the + // destructive gate's "rollback" guidance must NOT appear, since this + // is the routine, not destructive, deny path. + assert.strictEqual(output.hookSpecificOutput.permissionDecision, 'deny', + 'routine gate fires when extra-regex is unset'); + assert.ok(!output.hookSpecificOutput.permissionDecisionReason.includes('rollback'), + 'should be routine deny (no "rollback" guidance), not destructive'); + assert.ok(!output.hookSpecificOutput.permissionDecisionReason.includes('Destructive'), + 'should not be the destructive deny message'); + })) passed++; else failed++; + + clearState(); + if (test('GATEGUARD_BASH_EXTRA_DESTRUCTIVE custom phrase inside $(...) also caught', () => { + const input = { + tool_name: 'Bash', + tool_input: { command: 'echo "running" && $(supabase db reset)' } + }; + const result = runBashHook(input, { + GATEGUARD_BASH_EXTRA_DESTRUCTIVE: 'supabase\\s+db\\s+reset' + }); + const output = parseOutput(result.stdout); + assert.ok(output, 'should produce JSON'); + assert.strictEqual(output.hookSpecificOutput.permissionDecision, 'deny', + 'custom phrase inside command substitution should be gated'); + })) passed++; else failed++; + + // --- Issue #2078 review fix: warning emitted once per *distinct* + // invalid regex, not once per process. Verifies the same-process + // path that the reviewers (CodeRabbit + cubic) flagged. + clearState(); + if (test('GATEGUARD_BASH_EXTRA_DESTRUCTIVE warns once per distinct invalid regex (not once per process)', () => { + // We can't easily intercept stderr from a spawnSync child without + // re-running the hook in the same process, so we exercise + // checkCommand-equivalent behavior via a same-process require. + const originalEnv = process.env.GATEGUARD_BASH_EXTRA_DESTRUCTIVE; + const originalStderrWrite = process.stderr.write.bind(process.stderr); + const captured = []; + process.stderr.write = (chunk) => { + const s = typeof chunk === 'string' ? chunk : chunk.toString(); + if (s.includes('GATEGUARD_BASH_EXTRA_DESTRUCTIVE')) { + captured.push(s.trim()); + } + // Don't forward to real stderr — keeps test output clean. + return true; + }; + try { + // First bad pattern — should warn once. + process.env.GATEGUARD_BASH_EXTRA_DESTRUCTIVE = '(unclosed-a'; + const hook1 = loadDirectHook({ GATEGUARD_BASH_EXTRA_DESTRUCTIVE: '(unclosed-a' }); + hook1.run(JSON.stringify({ tool_name: 'Bash', tool_input: { command: 'ls' } })); + hook1.run(JSON.stringify({ tool_name: 'Bash', tool_input: { command: 'ls' } })); + assert.strictEqual(captured.length, 1, + `same invalid pattern should warn exactly once, got ${captured.length}: ${JSON.stringify(captured)}`); + + // Switch to a *different* bad pattern — should warn again (this is + // the bug both reviewers flagged: the sticky flag was never reset + // when the cache key changed). + process.env.GATEGUARD_BASH_EXTRA_DESTRUCTIVE = '(unclosed-b'; + hook1.run(JSON.stringify({ tool_name: 'Bash', tool_input: { command: 'ls' } })); + assert.strictEqual(captured.length, 2, + `distinct invalid pattern should produce a second warning, got ${captured.length}: ${JSON.stringify(captured)}`); + + // Switch back to a valid regex — no extra warning. + process.env.GATEGUARD_BASH_EXTRA_DESTRUCTIVE = 'valid\\s+pattern'; + hook1.run(JSON.stringify({ tool_name: 'Bash', tool_input: { command: 'ls' } })); + assert.strictEqual(captured.length, 2, + `valid regex should not emit a warning, got ${captured.length}: ${JSON.stringify(captured)}`); + } finally { + process.stderr.write = originalStderrWrite; + if (originalEnv === undefined) { + delete process.env.GATEGUARD_BASH_EXTRA_DESTRUCTIVE; + } else { + process.env.GATEGUARD_BASH_EXTRA_DESTRUCTIVE = originalEnv; + } + } + })) passed++; else failed++; + // Cleanup only the temp directory created by this test file. try { if (fs.existsSync(stateDir)) {