From 7c5452f4fa213139f2fb49780129f0afead9c9bd Mon Sep 17 00:00:00 2001 From: Affaan Mustafa Date: Thu, 30 Apr 2026 10:57:24 -0400 Subject: [PATCH] fix: keep gateguard destructive gate strict --- scripts/hooks/gateguard-fact-force.js | 10 +++---- tests/hooks/gateguard-fact-force.test.js | 34 ++++++++++++++++++++++-- 2 files changed, 37 insertions(+), 7 deletions(-) diff --git a/scripts/hooks/gateguard-fact-force.js b/scripts/hooks/gateguard-fact-force.js index a57bee9e..5c18d901 100644 --- a/scripts/hooks/gateguard-fact-force.js +++ b/scripts/hooks/gateguard-fact-force.js @@ -38,7 +38,6 @@ const READ_HEARTBEAT_MS = 60 * 1000; const MAX_CHECKED_ENTRIES = 500; const MAX_SESSION_KEYS = 50; const ROUTINE_BASH_SESSION_KEY = '__bash_session__'; -const LEGACY_DISABLE_VALUES = new Set(['1', 'true', 'yes', 'on']); const ECC_DISABLE_VALUES = new Set(['0', 'false', 'off', 'disabled', 'disable']); const DESTRUCTIVE_BASH = /\b(rm\s+-rf|git\s+reset\s+--hard|git\s+checkout\s+--|git\s+clean\s+-f|drop\s+table|delete\s+from|truncate|git\s+push\s+--force(?!-with-lease)|git\s+commit\s+--amend|dd\s+if=)\b/i; @@ -50,7 +49,7 @@ function normalizeEnvValue(value) { } function isGateGuardDisabled() { - if (LEGACY_DISABLE_VALUES.has(normalizeEnvValue(process.env.GATEGUARD_DISABLED))) { + if (normalizeEnvValue(process.env.GATEGUARD_DISABLED) === '1') { return true; } @@ -376,13 +375,14 @@ function withRecoveryHint(message) { // --- Deny helper --- -function denyResult(reason) { +function denyResult(reason, options = {}) { + const includeRecoveryHint = options.includeRecoveryHint !== false; return { stdout: JSON.stringify({ hookSpecificOutput: { hookEventName: 'PreToolUse', permissionDecision: 'deny', - permissionDecisionReason: withRecoveryHint(reason) + permissionDecisionReason: includeRecoveryHint ? withRecoveryHint(reason) : reason } }), exitCode: 0 @@ -462,7 +462,7 @@ function run(rawInput) { if (!markChecked(key)) { return allowWithStateWarning(); } - return denyResult(destructiveBashMsg()); + return denyResult(destructiveBashMsg(), { includeRecoveryHint: false }); } return rawInput; // allow retry after facts presented } diff --git a/tests/hooks/gateguard-fact-force.test.js b/tests/hooks/gateguard-fact-force.test.js index 5b43890e..7d3fb9b4 100644 --- a/tests/hooks/gateguard-fact-force.test.js +++ b/tests/hooks/gateguard-fact-force.test.js @@ -440,7 +440,21 @@ function runTests() { assert.ok(!fs.existsSync(stateFile), 'disabled gate should not create or mutate gate state'); })) passed++; else failed++; - // --- Test 12: denial messages show an escape hatch --- + // --- Test 12: legacy GATEGUARD_DISABLED compatibility is scoped to =1 --- + clearState(); + if (test('does not treat GATEGUARD_DISABLED=true as a disable flag', () => { + const input = { + tool_name: 'Bash', + tool_input: { command: 'npm test' } + }; + const result = runBashHook(input, { GATEGUARD_DISABLED: 'true' }); + const output = parseOutput(result.stdout); + + assert.strictEqual(output.hookSpecificOutput.permissionDecision, 'deny'); + assert.ok(output.hookSpecificOutput.permissionDecisionReason.includes('current user request')); + })) passed++; else failed++; + + // --- Test 13: denial messages show an escape hatch --- clearState(); if (test('denial messages include direct recovery escape hatch', () => { const input = { @@ -457,7 +471,23 @@ function runTests() { 'denial reason should mention the existing hook-id disable control'); })) passed++; else failed++; - // --- Test 13: MultiEdit gates first unchecked file --- + // --- Test 14: destructive Bash denials do not advertise the recovery escape hatch --- + clearState(); + if (test('destructive Bash denials omit recovery escape hatch', () => { + const input = { + tool_name: 'Bash', + tool_input: { command: 'rm -rf /tmp/demo' } + }; + const result = runBashHook(input); + const output = parseOutput(result.stdout); + + assert.strictEqual(output.hookSpecificOutput.permissionDecision, 'deny'); + assert.ok(output.hookSpecificOutput.permissionDecisionReason.includes('Destructive command detected')); + assert.ok(!output.hookSpecificOutput.permissionDecisionReason.includes('ECC_GATEGUARD=off'), + 'destructive gate should not advertise disabling GateGuard'); + })) passed++; else failed++; + + // --- Test 15: MultiEdit gates first unchecked file --- clearState(); if (test('denies first MultiEdit with unchecked file', () => { const input = {