mirror of
https://github.com/affaan-m/everything-claude-code.git
synced 2026-06-11 02:33:10 +08:00
feat(gateguard): add env knobs for routine bash gate + extra destructive patterns (#2161)
* feat(gateguard): add env knobs for routine bash gate + extra destructive patterns The JS port of gateguard-fact-force has two bash gates: a destructive gate (rm -rf, drop table, git push --force, etc.) that operators want to keep, and a once-per-session routine gate that fires on the very first bash invocation regardless of intent. Operators on hosts where the routine gate is friction without signal (Cursor, OpenCode, etc.) have been maintaining local patches that get clobbered on every plugin update; the Python upstream gateguard-ai already exposes equivalent config via .gateguard.yml. Adds two env vars, both off-by-default so existing behavior is preserved: - GATEGUARD_BASH_ROUTINE_DISABLED — truthy values (1, true, on, yes, enabled) skip the routine bash gate. Destructive gate is unaffected. - GATEGUARD_BASH_EXTRA_DESTRUCTIVE — regex source string for additional destructive patterns. Matches against the same quote-stripped, subshell-flattened command the built-in DESTRUCTIVE_SQL_DD regex sees, so a custom phrase inside $(...) or backticks is also caught. A malformed regex is logged once to stderr and treated as not configured rather than crashing the hook (hooks must never block tool execution unexpectedly). Twelve new tests pin both env vars (truthy aliases, falsy values, unset baseline, destructive-gate-still-fires, alternation members, malformed regex degrades safely, custom phrase inside command substitution). Existing 2619/2619 tests still pass; eslint clean. Fixes #2078 * fix(gateguard): reset extra-destructive warn-once gate when env value changes Both reviewers (CodeRabbit + cubic) flagged that extraDestructiveWarnLogged was never reset when GATEGUARD_BASH_EXTRA_DESTRUCTIVE flipped from one invalid regex to a different invalid regex. The sticky boolean meant a long-running process saw bad-pattern-a's warning then silently swallowed bad-pattern-b's parse failure. Fix: clear extraDestructiveWarnLogged whenever the cache key changes (i.e. before the regex compile attempt). The warn-once-per-distinct- pattern invariant now matches the per-key cache invariant. Adds a same-process regression test via loadDirectHook() that spies on process.stderr.write and asserts: same bad pattern warns once across multiple invocations; switching to a different bad pattern emits a second warning; switching to a valid regex emits zero warnings.
This commit is contained in:
Gaurav Dubey
@@ -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)) {
|
||||
|
||||
Reference in New Issue
Block a user