From 249b0ecf7fddbf4ca16147fd4eb02e5075914cc1 Mon Sep 17 00:00:00 2001 From: Nguyen Thanh Dat Date: Fri, 21 Aug 2026 08:39:59 +0700 Subject: [PATCH] test(gateguard): fail the allow-cases when the hook returns nothing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The "does not gate as destructive" cases guarded the decision behind `if (output && output.hookSpecificOutput)`, so a crashed or silent hook made parseOutput return null and the test passed having asserted nothing. Assert the exit code and that output parsed first, then branch: a decision object must not be a Destructive deny, and pass-through must echo the input back — the same shape the existing retry case already checks. Raised in review on #2829. --- tests/hooks/gateguard-fact-force.test.js | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/tests/hooks/gateguard-fact-force.test.js b/tests/hooks/gateguard-fact-force.test.js index fcf6f400b..f83677b0d 100644 --- a/tests/hooks/gateguard-fact-force.test.js +++ b/tests/hooks/gateguard-fact-force.test.js @@ -315,13 +315,22 @@ function runTests() { // be mistaken for a destructive denial. runBashHook({ tool_name: 'Bash', tool_input: { command: 'printf ready' } }); const result = runBashHook({ tool_name: 'Bash', tool_input: { command } }); + // Assert the hook actually answered before reading the decision: a + // crashed or silent hook makes parseOutput return null, and a bare + // `if (output)` would let this case pass without testing anything. + assert.strictEqual(result.code, 0, `hook should exit 0 for ${command}`); const output = parseOutput(result.stdout); - if (output && output.hookSpecificOutput) { - const reason = output.hookSpecificOutput.permissionDecisionReason || ''; + assert.ok(output, `hook should produce JSON output for ${command}`); + const decision = output.hookSpecificOutput; + if (decision) { + const reason = decision.permissionDecisionReason || ''; assert.ok( - output.hookSpecificOutput.permissionDecision !== 'deny' || !reason.includes('Destructive'), + decision.permissionDecision !== 'deny' || !reason.includes('Destructive'), `${command} must not be gated as destructive` ); + } else { + // Pass-through echoes the input back unchanged. + assert.strictEqual(output.tool_name, 'Bash', 'pass-through should preserve input'); } }) )