From 55322de58a173060b99f1bed330c23311388a6e2 Mon Sep 17 00:00:00 2001 From: Garry Tan Date: Fri, 4 Sep 2026 17:55:07 +0000 Subject: [PATCH] fix(freeze): an unexpected non-zero death denies via an EXIT backstop instead of exiting with no decision set -e plus a failing pipeline (a tool on PATH exiting non-zero, a deleted cwd) ended the deny-tier hook with no JSON, which Claude Code treats as non-blocking: the edit outside the boundary proceeded. The EXIT trap now prints a deny for any non-zero exit that happens before a decision was written; every deliberate output sets _FREEZE_DECIDED first so a late failure never prints a second object. Tests also pin careful's state-root precedence (GSTACK_HOME over CLAUDE_PLUGIN_DATA, plugin data when CLAUDE_PLUGIN_ROOT names gstack) and the specific "out of date" deny for a helper without gstack_hook_state_root. Co-Authored-By: Claude Fable 5.1 --- freeze/bin/check-freeze.sh | 24 +++++++++++++++ test/hook-scripts.test.ts | 60 ++++++++++++++++++++++++++++++++------ 2 files changed, 75 insertions(+), 9 deletions(-) diff --git a/freeze/bin/check-freeze.sh b/freeze/bin/check-freeze.sh index 803dc9494..43c5f3aea 100755 --- a/freeze/bin/check-freeze.sh +++ b/freeze/bin/check-freeze.sh @@ -12,6 +12,22 @@ # not a boundary. set -euo pipefail +# Deny-tier backstop: any unexpected non-zero death (a failing pipeline under +# set -e, a deleted cwd, EACCES) would otherwise exit with no decision JSON, +# which Claude Code treats as non-blocking — the edit proceeds. Every +# deliberate output below sets _FREEZE_DECIDED first so a late failure after +# a decision never prints a second JSON object. +_FREEZE_DECIDED="" +_freeze_backstop() { + local rc=$? + if [ "$rc" -ne 0 ] && [ -z "$_FREEZE_DECIDED" ]; then + _FREEZE_DECIDED=1 + printf '{"hookSpecificOutput":{"hookEventName":"PreToolUse","permissionDecision":"deny","permissionDecisionReason":"[freeze] Hook failed unexpectedly (exit %s) - blocked, fail closed. Re-run ./setup or /unfreeze."}}\n' "$rc" + exit 0 + fi +} +trap _freeze_backstop EXIT + # Read stdin INPUT=$(cat) @@ -27,6 +43,7 @@ _HOOK_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" # (an if-guard cannot catch it) — the existence check must come first. _HOOK_HELPER="$_HOOK_DIR/../../careful/bin/hook-extract.sh" if [ ! -f "$_HOOK_HELPER" ] || ! . "$_HOOK_HELPER" 2>/dev/null; then + _FREEZE_DECIDED=1 printf '{"hookSpecificOutput":{"hookEventName":"PreToolUse","permissionDecision":"deny","permissionDecisionReason":"[freeze] Hook helpers unavailable (broken install?) - blocked, fail closed. Reinstall gstack or run /unfreeze."}}\n' exit 0 fi @@ -40,6 +57,7 @@ fi # (the existence check above only proves the file sourced), never exit 127 # with no JSON — Claude Code treats that as non-blocking. if ! command -v gstack_hook_state_root >/dev/null 2>&1; then + _FREEZE_DECIDED=1 printf '{"hookSpecificOutput":{"hookEventName":"PreToolUse","permissionDecision":"deny","permissionDecisionReason":"[freeze] Hook helpers out of date (partial upgrade?) - blocked, fail closed. Re-run ./setup or /unfreeze."}}\n' exit 0 fi @@ -48,6 +66,7 @@ FREEZE_FILE="$STATE_DIR/freeze-dir.txt" # If no freeze file exists, allow everything (not yet configured) if [ ! -f "$FREEZE_FILE" ]; then + _FREEZE_DECIDED=1 echo '{}' exit 0 fi @@ -66,6 +85,7 @@ esac # If freeze dir is empty, allow if [ -z "$FREEZE_DIR" ]; then + _FREEZE_DECIDED=1 echo '{}' exit 0 fi @@ -79,12 +99,14 @@ set -e # Unparseable payload (or no parser available): DENY. A boundary hook that # allows what it cannot read is not a boundary. if [ "$EXTRACT_RC" -ne 0 ] && [ -n "$INPUT" ]; then + _FREEZE_DECIDED=1 gstack_hook_decision deny "[freeze] Could not parse the tool payload to check the freeze boundary. Blocked (fail closed). Freeze boundary: $FREEZE_DIR" exit 0 fi # Parsed fine but no file_path field: a non-file tool payload — allow. if [ -z "$FILE_PATH" ]; then + _FREEZE_DECIDED=1 echo '{}' exit 0 fi @@ -130,6 +152,7 @@ FREEZE_DIR=$(_resolve_path "$FREEZE_DIR") case "$FILE_PATH" in "${FREEZE_DIR}/"*|"${FREEZE_DIR}") # Inside freeze boundary — allow + _FREEZE_DECIDED=1 echo '{}' ;; *) @@ -140,6 +163,7 @@ case "$FILE_PATH" in # The reason is JSON-encoded by the shared helper. Never interpolate paths # into hand-built JSON: a path containing a quote or newline produced # malformed JSON here, and the deny silently no-oped. + _FREEZE_DECIDED=1 gstack_hook_decision deny "[freeze] Blocked: $FILE_PATH is outside the freeze boundary ($FREEZE_DIR). Only edits within the frozen directory are allowed." ;; esac diff --git a/test/hook-scripts.test.ts b/test/hook-scripts.test.ts index 2a3514e7c..1eb1bf9f6 100644 --- a/test/hook-scripts.test.ts +++ b/test/hook-scripts.test.ts @@ -110,6 +110,11 @@ function pathsStateRoot(env: Record): string { // Frontmatter hooks run before any runtime variable exists, so a // ${CLAUDE_SKILL_DIR}-relative command silently never resolves and the guard // never fires. Every command: line must anchor on $HOME like careful/freeze. +function withEmptyDir(fn: (dir: string) => void) { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'gstack-hook-empty-')); + try { fn(dir); } finally { fs.rmSync(dir, { recursive: true, force: true }); } +} + describe('frontmatter hook command paths', () => { test.each(['investigate/SKILL.md', 'careful/SKILL.md', 'freeze/SKILL.md', 'guard/SKILL.md'])( '%s hook commands are $HOME-anchored, never CLAUDE_SKILL_DIR', @@ -716,6 +721,30 @@ describe('check-careful.sh', () => { } }); + test('plugin install: patterns under CLAUDE_PLUGIN_DATA load when CLAUDE_PLUGIN_ROOT names gstack (same root the writer uses)', () => { + withPatternFile('terraform\\s+destroy\n', (pluginData) => { + withEmptyDir((fakeHome) => { + const { exitCode, output } = runHook(CAREFUL_SCRIPT, carefulInput('terraform destroy'), + { HOME: fakeHome, GSTACK_HOME: '', CLAUDE_PLUGIN_DATA: pluginData, CLAUDE_PLUGIN_ROOT: '/plugins/gstack' }); + expect(exitCode).toBe(0); + expect(output.hookSpecificOutput?.permissionDecision).toBe('ask'); + expect(output.hookSpecificOutput?.permissionDecisionReason).toContain('Project rule'); + }); + }); + }); + + test('GSTACK_HOME outranks CLAUDE_PLUGIN_DATA for careful patterns, exactly as for the freeze file', () => { + withPatternFile('terraform\\s+destroy\n', (gstackHome) => { + withEmptyDir((pluginData) => { + const { exitCode, output } = runHook(CAREFUL_SCRIPT, carefulInput('terraform destroy'), + { GSTACK_HOME: gstackHome, CLAUDE_PLUGIN_DATA: pluginData, CLAUDE_PLUGIN_ROOT: '/plugins/gstack' }); + expect(exitCode).toBe(0); + expect(output.hookSpecificOutput?.permissionDecision).toBe('ask'); + expect(output.hookSpecificOutput?.permissionDecisionReason).toContain('Project rule'); + }); + }); + }); + test('safe commands still allow with a pattern file present', () => { withPatternFile('terraform\\s+destroy\n', (gstackHome) => { const { exitCode, output } = runHook(CAREFUL_SCRIPT, carefulInput('ls -la'), { GSTACK_HOME: gstackHome }); @@ -963,10 +992,6 @@ describe('check-freeze.sh state-root resolution (#1459 / #1509)', () => { const BOUNDARY = '/Users/dev/project/src/'; const OUTSIDE = '/Users/dev/other-project/index.ts'; - function withEmptyDir(fn: (dir: string) => void) { - const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'gstack-freeze-empty-')); - try { fn(dir); } finally { fs.rmSync(dir, { recursive: true, force: true }); } - } test('REGRESSION: freeze file under GSTACK_HOME (HOME has none) denies an outside edit', () => { withFreezeDir(BOUNDARY, (gstackHome) => { @@ -1039,10 +1064,6 @@ describe('gstack_hook_log_fire writes under the resolved state root', () => { const BOUNDARY = '/Users/dev/project/src/'; const OUTSIDE = '/Users/dev/other-project/index.ts'; - function withEmptyDir(fn: (dir: string) => void) { - const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'gstack-logfire-')); - try { fn(dir); } finally { fs.rmSync(dir, { recursive: true, force: true }); } - } function lastRecord(file: string): any { const lines = fs.readFileSync(file, 'utf-8').trim().split('\n'); return JSON.parse(lines[lines.length - 1]); @@ -1095,6 +1116,25 @@ describe('gstack_hook_log_fire writes under the resolved state root', () => { } }); + test('an unexpected set -e death inside the hook (a tool on PATH failing) DENIES via the EXIT backstop instead of exiting with no JSON', () => { + const base = fs.mkdtempSync(path.join(os.tmpdir(), 'gstack-freeze-backstop-')); + const fakeBin = path.join(base, 'bin'); + fs.mkdirSync(fakeBin); + fs.writeFileSync(path.join(fakeBin, 'head'), '#!/bin/sh\nexit 1\n'); + fs.chmodSync(path.join(fakeBin, 'head'), 0o755); + try { + withFreezeDir(BOUNDARY, (stateDir) => { + const { exitCode, output } = runHook(FREEZE_SCRIPT, freezeInput('/Users/dev/project/src/x.ts'), + freezeEnv(stateDir, { PATH: `${fakeBin}:${process.env.PATH ?? ''}` })); + expect(exitCode).toBe(0); + expect(output.hookSpecificOutput?.permissionDecision).toBe('deny'); + expect(output.hookSpecificOutput?.permissionDecisionReason).toContain('failed unexpectedly'); + }); + } finally { + fs.rmSync(base, { recursive: true, force: true }); + } + }); + test('a hook helper from an older install that lacks gstack_hook_state_root DENIES (fail closed), never exit 127', () => { const base = fs.mkdtempSync(path.join(os.tmpdir(), 'gstack-freeze-oldhelper-')); const freezeBin = path.join(base, 'freeze', 'bin'); @@ -1111,7 +1151,9 @@ describe('gstack_hook_log_fire writes under the resolved state root', () => { const { exitCode, output } = runHook(path.join(freezeBin, 'check-freeze.sh'), freezeInput('/Users/dev/project/src/x.ts'), freezeEnv(stateDir)); expect(exitCode).toBe(0); expect(output.hookSpecificOutput?.permissionDecision).toBe('deny'); - expect(output.hookSpecificOutput?.permissionDecisionReason).toContain('fail closed'); + // 'out of date' is the helper-without-function branch; the plain + // helpers-unavailable deny also says 'fail closed', so pin the specific one. + expect(output.hookSpecificOutput?.permissionDecisionReason).toContain('out of date'); }); } finally { fs.rmSync(base, { recursive: true, force: true });