mirror of
https://github.com/garrytan/gstack.git
synced 2026-09-09 06:28:59 +02:00
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 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Fable 5.1
parent
f1a580e1f0
commit
55322de58a
@@ -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
|
||||
|
||||
@@ -110,6 +110,11 @@ function pathsStateRoot(env: Record<string, string>): 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 });
|
||||
|
||||
Reference in New Issue
Block a user