From 95bef977c1cf15e68e5540f9b10f350ae5e48c61 Mon Sep 17 00:00:00 2001 From: Affaan Mustafa Date: Thu, 30 Apr 2026 08:07:02 -0400 Subject: [PATCH] fix: fail open on gateguard state write errors --- scripts/hooks/gateguard-fact-force.js | 28 +++++++++++++++++++----- tests/hooks/gateguard-fact-force.test.js | 24 ++++++++++++++++++++ 2 files changed, 47 insertions(+), 5 deletions(-) diff --git a/scripts/hooks/gateguard-fact-force.js b/scripts/hooks/gateguard-fact-force.js index 67f4d700..111dd2f7 100644 --- a/scripts/hooks/gateguard-fact-force.js +++ b/scripts/hooks/gateguard-fact-force.js @@ -171,6 +171,7 @@ function saveState(state) { } } tmpFile = null; + return true; } catch (_) { if (tmpFile) { try { @@ -179,6 +180,7 @@ function saveState(state) { /* ignore */ } } + return false; } } @@ -186,8 +188,9 @@ function markChecked(key) { const state = loadState(); if (!state.checked.includes(key)) { state.checked.push(key); - saveState(state); + return saveState(state); } + return true; } function isChecked(key) { @@ -364,6 +367,13 @@ function denyResult(reason) { }; } +function allowWithStateWarning() { + return { + stderr: '[Fact-Forcing Gate] GateGuard state could not be persisted; allowing this operation to avoid a permanent retry loop. Check GATEGUARD_STATE_DIR or filesystem permissions.', + exitCode: 0 + }; +} + // --- Core logic (exported for run-with-flags.js) --- function run(rawInput) { @@ -389,7 +399,9 @@ function run(rawInput) { } if (!isChecked(filePath)) { - markChecked(filePath); + if (!markChecked(filePath)) { + return allowWithStateWarning(); + } return denyResult(toolName === 'Edit' ? editGateMsg(filePath) : writeGateMsg(filePath)); } @@ -401,7 +413,9 @@ function run(rawInput) { for (const edit of edits) { const filePath = edit.file_path || ''; if (filePath && !isClaudeSettingsPath(filePath) && !isChecked(filePath)) { - markChecked(filePath); + if (!markChecked(filePath)) { + return allowWithStateWarning(); + } return denyResult(editGateMsg(filePath)); } } @@ -418,14 +432,18 @@ function run(rawInput) { // Gate destructive commands on first attempt; allow retry after facts presented const key = '__destructive__' + crypto.createHash('sha256').update(command).digest('hex').slice(0, 16); if (!isChecked(key)) { - markChecked(key); + if (!markChecked(key)) { + return allowWithStateWarning(); + } return denyResult(destructiveBashMsg()); } return rawInput; // allow retry after facts presented } if (!isChecked(ROUTINE_BASH_SESSION_KEY)) { - markChecked(ROUTINE_BASH_SESSION_KEY); + if (!markChecked(ROUTINE_BASH_SESSION_KEY)) { + return allowWithStateWarning(); + } return denyResult(routineBashMsg()); } diff --git a/tests/hooks/gateguard-fact-force.test.js b/tests/hooks/gateguard-fact-force.test.js index 20dd5cc6..80f03454 100644 --- a/tests/hooks/gateguard-fact-force.test.js +++ b/tests/hooks/gateguard-fact-force.test.js @@ -191,6 +191,30 @@ function runTests() { assert.ok(output.hookSpecificOutput.permissionDecisionReason.includes('call this new file')); })) passed++; else failed++; + // --- Test 3b: fails open when retry state cannot be persisted --- + clearState(); + if (test('fails open with warning when state path cannot be persisted', () => { + const invalidStateDir = path.join(stateDir, 'not-a-directory'); + fs.writeFileSync(invalidStateDir, 'not a directory', 'utf8'); + + const input = { + tool_name: 'Write', + tool_input: { file_path: '/src/state-failure.js', content: 'module.exports = {};' } + }; + const result = runHook(input, { GATEGUARD_STATE_DIR: invalidStateDir }); + 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', + 'unpersistable state must not deny a retry that can never be recorded'); + } else { + assert.strictEqual(output.tool_name, 'Write', 'pass-through should preserve input'); + } + assert.ok(result.stderr.includes('GateGuard state could not be persisted'), + 'should warn that state persistence failed'); + })) passed++; else failed++; + // --- Test 4: denies destructive Bash, allows retry --- clearState(); if (test('denies destructive Bash commands, allows retry after facts presented', () => {