fix(hooks): nest freeze/careful permissionDecision under hookSpecificOutput

Claude Code ignores a top-level permissionDecision, so the /freeze deny and
/careful ask guards silently allowed everything. Nest both under
hookSpecificOutput with permissionDecisionReason, update the shape-blind
tests to pin the nested form, and document the constraint in both skill
templates (regen included).

Closes half of #1459 (freeze enforcement chain).

Contributed by @jawadakram20 (PR #2331; team-init hunk deferred to the
dedicated team-init fix).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
Garry Tan 2026-08-14 15:34:35 -07:00
parent 1d41ee3ab3
commit 2918542c78
No known key found for this signature in database
GPG Key ID: C1F69E85C74EFE1D
7 changed files with 92 additions and 80 deletions

View File

@ -61,7 +61,9 @@ These patterns are allowed without warning:
## How it works
The hook reads the command from the tool input JSON, checks it against the
patterns above, and returns `permissionDecision: "ask"` with a warning message
if a match is found. You can always override the warning and proceed.
patterns above, and returns a `hookSpecificOutput` payload with
`permissionDecision: "ask"` and a warning reason if a match is found (the
decision must be nested under `hookSpecificOutput` — Claude Code ignores a
top-level `permissionDecision`). You can always override the warning and proceed.
To deactivate, end the conversation or start a new one. Hooks are session-scoped.

View File

@ -56,7 +56,9 @@ These patterns are allowed without warning:
## How it works
The hook reads the command from the tool input JSON, checks it against the
patterns above, and returns `permissionDecision: "ask"` with a warning message
if a match is found. You can always override the warning and proceed.
patterns above, and returns a `hookSpecificOutput` payload with
`permissionDecision: "ask"` and a warning reason if a match is found (the
decision must be nested under `hookSpecificOutput` — Claude Code ignores a
top-level `permissionDecision`). You can always override the warning and proceed.
To deactivate, end the conversation or start a new one. Hooks are session-scoped.

View File

@ -1,7 +1,9 @@
#!/usr/bin/env bash
# check-careful.sh — PreToolUse hook for /careful skill
# Reads JSON from stdin, checks Bash command for destructive patterns.
# Returns {"permissionDecision":"ask","message":"..."} to warn, or {} to allow.
# Returns a PreToolUse hookSpecificOutput with permissionDecision "ask" to warn,
# or {} to allow. The decision MUST be nested under hookSpecificOutput — Claude
# Code ignores a top-level permissionDecision, which silently no-ops the warning.
set -euo pipefail
# Read stdin (JSON with tool_input)
@ -101,7 +103,7 @@ if [ -n "$WARN" ]; then
echo '{"event":"hook_fire","skill":"careful","pattern":"'"$PATTERN"'","ts":"'$(date -u +%Y-%m-%dT%H:%M:%SZ)'","repo":"'$(basename "$(git rev-parse --show-toplevel 2>/dev/null)" 2>/dev/null || echo "unknown")'"}' >> ~/.gstack/analytics/skill-usage.jsonl 2>/dev/null || true
WARN_ESCAPED=$(printf '%s' "$WARN" | sed 's/"/\\"/g')
printf '{"permissionDecision":"ask","message":"[careful] %s"}\n' "$WARN_ESCAPED"
printf '{"hookSpecificOutput":{"hookEventName":"PreToolUse","permissionDecision":"ask","permissionDecisionReason":"[careful] %s"}}\n' "$WARN_ESCAPED"
else
echo '{}'
fi

View File

@ -77,8 +77,10 @@ again. To remove it, run `/unfreeze` or end the session."
## How it works
The hook reads `file_path` from the Edit/Write tool input JSON, then checks
whether the path starts with the freeze directory. If not, it returns
`permissionDecision: "deny"` to block the operation.
whether the path starts with the freeze directory. If not, it returns a
`hookSpecificOutput` payload with `permissionDecision: "deny"` to block the
operation (nested under `hookSpecificOutput` — Claude Code ignores a top-level
`permissionDecision`).
The freeze boundary persists for the session via the state file. The hook
script reads it on every Edit/Write invocation.

View File

@ -72,8 +72,10 @@ again. To remove it, run `/unfreeze` or end the session."
## How it works
The hook reads `file_path` from the Edit/Write tool input JSON, then checks
whether the path starts with the freeze directory. If not, it returns
`permissionDecision: "deny"` to block the operation.
whether the path starts with the freeze directory. If not, it returns a
`hookSpecificOutput` payload with `permissionDecision: "deny"` to block the
operation (nested under `hookSpecificOutput` — Claude Code ignores a top-level
`permissionDecision`).
The freeze boundary persists for the session via the state file. The hook
script reads it on every Edit/Write invocation.

View File

@ -1,7 +1,9 @@
#!/usr/bin/env bash
# check-freeze.sh — PreToolUse hook for /freeze skill
# Reads JSON from stdin, checks if file_path is within the freeze boundary.
# Returns {"permissionDecision":"deny","message":"..."} to block, or {} to allow.
# Returns a PreToolUse hookSpecificOutput with permissionDecision "deny" to block,
# or {} to allow. The decision MUST be nested under hookSpecificOutput — Claude
# Code ignores a top-level permissionDecision, which silently no-ops the block.
set -euo pipefail
# Read stdin
@ -74,6 +76,6 @@ case "$FILE_PATH" in
mkdir -p ~/.gstack/analytics 2>/dev/null || true
echo '{"event":"hook_fire","skill":"freeze","pattern":"boundary_deny","ts":"'$(date -u +%Y-%m-%dT%H:%M:%SZ)'","repo":"'$(basename "$(git rev-parse --show-toplevel 2>/dev/null)" 2>/dev/null || echo "unknown")'"}' >> ~/.gstack/analytics/skill-usage.jsonl 2>/dev/null || true
printf '{"permissionDecision":"deny","message":"[freeze] Blocked: %s is outside the freeze boundary (%s). Only edits within the frozen directory are allowed."}\n' "$FILE_PATH" "$FREEZE_DIR"
printf '{"hookSpecificOutput":{"hookEventName":"PreToolUse","permissionDecision":"deny","permissionDecisionReason":"[freeze] Blocked: %s is outside the freeze boundary (%s). Only edits within the frozen directory are allowed."}}\n' "$FILE_PATH" "$FREEZE_DIR"
;;
esac

View File

@ -67,34 +67,34 @@ describe('check-careful.sh', () => {
test('rm -rf /var/data warns with recursive delete message', () => {
const { exitCode, output } = runHook(CAREFUL_SCRIPT, carefulInput('rm -rf /var/data'));
expect(exitCode).toBe(0);
expect(output.permissionDecision).toBe('ask');
expect(output.message).toContain('recursive delete');
expect(output.hookSpecificOutput?.permissionDecision).toBe('ask');
expect(output.hookSpecificOutput?.permissionDecisionReason).toContain('recursive delete');
});
test('rm -r ./some-dir warns', () => {
const { exitCode, output } = runHook(CAREFUL_SCRIPT, carefulInput('rm -r ./some-dir'));
expect(exitCode).toBe(0);
expect(output.permissionDecision).toBe('ask');
expect(output.message).toContain('recursive delete');
expect(output.hookSpecificOutput?.permissionDecision).toBe('ask');
expect(output.hookSpecificOutput?.permissionDecisionReason).toContain('recursive delete');
});
test('rm -rf node_modules allows (safe exception)', () => {
const { exitCode, output } = runHook(CAREFUL_SCRIPT, carefulInput('rm -rf node_modules'));
expect(exitCode).toBe(0);
expect(output.permissionDecision).toBeUndefined();
expect(output.hookSpecificOutput?.permissionDecision).toBeUndefined();
});
test('rm -rf .next dist allows (multiple safe targets)', () => {
const { exitCode, output } = runHook(CAREFUL_SCRIPT, carefulInput('rm -rf .next dist'));
expect(exitCode).toBe(0);
expect(output.permissionDecision).toBeUndefined();
expect(output.hookSpecificOutput?.permissionDecision).toBeUndefined();
});
test('rm -rf node_modules /var/data warns (mixed safe+unsafe)', () => {
const { exitCode, output } = runHook(CAREFUL_SCRIPT, carefulInput('rm -rf node_modules /var/data'));
expect(exitCode).toBe(0);
expect(output.permissionDecision).toBe('ask');
expect(output.message).toContain('recursive delete');
expect(output.hookSpecificOutput?.permissionDecision).toBe('ask');
expect(output.hookSpecificOutput?.permissionDecisionReason).toContain('recursive delete');
});
// The safe exception matches the COMPLETE command against an anchored
@ -103,22 +103,22 @@ describe('check-careful.sh', () => {
test('rm -rf /; rm -rf node_modules warns (semicolon chain, dangerous first)', () => {
const { exitCode, output } = runHook(CAREFUL_SCRIPT, carefulInput('rm -rf /; rm -rf node_modules'));
expect(exitCode).toBe(0);
expect(output.permissionDecision).toBe('ask');
expect(output.message).toContain('recursive delete');
expect(output.hookSpecificOutput?.permissionDecision).toBe('ask');
expect(output.hookSpecificOutput?.permissionDecisionReason).toContain('recursive delete');
});
test('rm -rf /etc/data && rm -rf dist warns (&& chain, dangerous first)', () => {
const { exitCode, output } = runHook(CAREFUL_SCRIPT, carefulInput('rm -rf /etc/data && rm -rf dist'));
expect(exitCode).toBe(0);
expect(output.permissionDecision).toBe('ask');
expect(output.message).toContain('recursive delete');
expect(output.hookSpecificOutput?.permissionDecision).toBe('ask');
expect(output.hookSpecificOutput?.permissionDecisionReason).toContain('recursive delete');
});
test('rm -rf node_modules; rm -rf /home/user/data warns (safe first, dangerous last)', () => {
const { exitCode, output } = runHook(CAREFUL_SCRIPT, carefulInput('rm -rf node_modules; rm -rf /home/user/data'));
expect(exitCode).toBe(0);
expect(output.permissionDecision).toBe('ask');
expect(output.message).toContain('recursive delete');
expect(output.hookSpecificOutput?.permissionDecision).toBe('ask');
expect(output.hookSpecificOutput?.permissionDecisionReason).toContain('recursive delete');
});
// Command substitution can end in a whitelisted suffix while running
@ -127,15 +127,15 @@ describe('check-careful.sh', () => {
test('rm -rf $(./wipe-all)/node_modules warns (command substitution)', () => {
const { exitCode, output } = runHook(CAREFUL_SCRIPT, carefulInput('rm -rf $(./wipe-all)/node_modules'));
expect(exitCode).toBe(0);
expect(output.permissionDecision).toBe('ask');
expect(output.message).toContain('recursive delete');
expect(output.hookSpecificOutput?.permissionDecision).toBe('ask');
expect(output.hookSpecificOutput?.permissionDecisionReason).toContain('recursive delete');
});
test('rm -rf `./wipe-all`/node_modules warns (backtick substitution)', () => {
const { exitCode, output } = runHook(CAREFUL_SCRIPT, carefulInput('rm -rf `./wipe-all`/node_modules'));
expect(exitCode).toBe(0);
expect(output.permissionDecision).toBe('ask');
expect(output.message).toContain('recursive delete');
expect(output.hookSpecificOutput?.permissionDecision).toBe('ask');
expect(output.hookSpecificOutput?.permissionDecisionReason).toContain('recursive delete');
});
// Capital -R is the documented recursive flag on BSD rm (macOS) and accepted
@ -144,21 +144,21 @@ describe('check-careful.sh', () => {
test('rm -R / warns (capital -R recursive)', () => {
const { exitCode, output } = runHook(CAREFUL_SCRIPT, carefulInput('rm -R /'));
expect(exitCode).toBe(0);
expect(output.permissionDecision).toBe('ask');
expect(output.message).toContain('recursive delete');
expect(output.hookSpecificOutput?.permissionDecision).toBe('ask');
expect(output.hookSpecificOutput?.permissionDecisionReason).toContain('recursive delete');
});
test('rm -fR /home/user warns (capital R in flag cluster)', () => {
const { exitCode, output } = runHook(CAREFUL_SCRIPT, carefulInput('rm -fR /home/user'));
expect(exitCode).toBe(0);
expect(output.permissionDecision).toBe('ask');
expect(output.message).toContain('recursive delete');
expect(output.hookSpecificOutput?.permissionDecision).toBe('ask');
expect(output.hookSpecificOutput?.permissionDecisionReason).toContain('recursive delete');
});
test('rm -Rf node_modules allows (capital R, single safe target)', () => {
const { exitCode, output } = runHook(CAREFUL_SCRIPT, carefulInput('rm -Rf node_modules'));
expect(exitCode).toBe(0);
expect(output.permissionDecision).toBeUndefined();
expect(output.hookSpecificOutput?.permissionDecision).toBeUndefined();
});
// JSON-escaped newline (literal two-char \n surviving the grep extraction
@ -166,8 +166,8 @@ describe('check-careful.sh', () => {
test('newline-chained rm warns (escaped-newline separator branch)', () => {
const { exitCode, output } = runHook(CAREFUL_SCRIPT, carefulInput('rm -rf /etc/x\nrm -rf node_modules'));
expect(exitCode).toBe(0);
expect(output.permissionDecision).toBe('ask');
expect(output.message).toContain('recursive delete');
expect(output.hookSpecificOutput?.permissionDecision).toBe('ask');
expect(output.hookSpecificOutput?.permissionDecisionReason).toContain('recursive delete');
});
// Deliberate false positive, pinned: a safe-prefix chain ending in a safe rm
@ -177,8 +177,8 @@ describe('check-careful.sh', () => {
test('cd app && rm -rf node_modules asks (fail-closed on chains, by design)', () => {
const { exitCode, output } = runHook(CAREFUL_SCRIPT, carefulInput('cd app && rm -rf node_modules'));
expect(exitCode).toBe(0);
expect(output.permissionDecision).toBe('ask');
expect(output.message).toContain('recursive delete');
expect(output.hookSpecificOutput?.permissionDecision).toBe('ask');
expect(output.hookSpecificOutput?.permissionDecisionReason).toContain('recursive delete');
});
test.each([
@ -192,8 +192,8 @@ describe('check-careful.sh', () => {
])('never lets a safe-looking target hide a destructive command: %s', (command) => {
const { exitCode, output } = runHook(CAREFUL_SCRIPT, carefulInput(command));
expect(exitCode).toBe(0);
expect(output.permissionDecision).toBe('ask');
expect(output.message).toContain('recursive delete');
expect(output.hookSpecificOutput?.permissionDecision).toBe('ask');
expect(output.hookSpecificOutput?.permissionDecisionReason).toContain('recursive delete');
});
});
@ -207,22 +207,22 @@ describe('check-careful.sh', () => {
test('psql DROP TABLE warns with DROP in message', () => {
const { exitCode, output } = runHook(CAREFUL_SCRIPT, carefulInput('psql -c DROP TABLE users;'));
expect(exitCode).toBe(0);
expect(output.permissionDecision).toBe('ask');
expect(output.message).toContain('DROP');
expect(output.hookSpecificOutput?.permissionDecision).toBe('ask');
expect(output.hookSpecificOutput?.permissionDecisionReason).toContain('DROP');
});
test('mysql drop database warns (case insensitive)', () => {
const { exitCode, output } = runHook(CAREFUL_SCRIPT, carefulInput('mysql -e drop database mydb'));
expect(exitCode).toBe(0);
expect(output.permissionDecision).toBe('ask');
expect(output.message.toLowerCase()).toContain('drop');
expect(output.hookSpecificOutput?.permissionDecision).toBe('ask');
expect(output.hookSpecificOutput?.permissionDecisionReason.toLowerCase()).toContain('drop');
});
test('psql TRUNCATE warns', () => {
const { exitCode, output } = runHook(CAREFUL_SCRIPT, carefulInput('psql -c TRUNCATE orders;'));
expect(exitCode).toBe(0);
expect(output.permissionDecision).toBe('ask');
expect(output.message).toContain('TRUNCATE');
expect(output.hookSpecificOutput?.permissionDecision).toBe('ask');
expect(output.hookSpecificOutput?.permissionDecisionReason).toContain('TRUNCATE');
});
});
@ -232,36 +232,36 @@ describe('check-careful.sh', () => {
test('git push --force warns with force-push', () => {
const { exitCode, output } = runHook(CAREFUL_SCRIPT, carefulInput('git push --force origin main'));
expect(exitCode).toBe(0);
expect(output.permissionDecision).toBe('ask');
expect(output.message).toContain('force-push');
expect(output.hookSpecificOutput?.permissionDecision).toBe('ask');
expect(output.hookSpecificOutput?.permissionDecisionReason).toContain('force-push');
});
test('git push -f warns', () => {
const { exitCode, output } = runHook(CAREFUL_SCRIPT, carefulInput('git push -f origin main'));
expect(exitCode).toBe(0);
expect(output.permissionDecision).toBe('ask');
expect(output.message).toContain('force-push');
expect(output.hookSpecificOutput?.permissionDecision).toBe('ask');
expect(output.hookSpecificOutput?.permissionDecisionReason).toContain('force-push');
});
test('git reset --hard warns with uncommitted', () => {
const { exitCode, output } = runHook(CAREFUL_SCRIPT, carefulInput('git reset --hard HEAD~3'));
expect(exitCode).toBe(0);
expect(output.permissionDecision).toBe('ask');
expect(output.message).toContain('uncommitted');
expect(output.hookSpecificOutput?.permissionDecision).toBe('ask');
expect(output.hookSpecificOutput?.permissionDecisionReason).toContain('uncommitted');
});
test('git checkout . warns', () => {
const { exitCode, output } = runHook(CAREFUL_SCRIPT, carefulInput('git checkout .'));
expect(exitCode).toBe(0);
expect(output.permissionDecision).toBe('ask');
expect(output.message).toContain('uncommitted');
expect(output.hookSpecificOutput?.permissionDecision).toBe('ask');
expect(output.hookSpecificOutput?.permissionDecisionReason).toContain('uncommitted');
});
test('git restore . warns', () => {
const { exitCode, output } = runHook(CAREFUL_SCRIPT, carefulInput('git restore .'));
expect(exitCode).toBe(0);
expect(output.permissionDecision).toBe('ask');
expect(output.message).toContain('uncommitted');
expect(output.hookSpecificOutput?.permissionDecision).toBe('ask');
expect(output.hookSpecificOutput?.permissionDecisionReason).toContain('uncommitted');
});
});
@ -271,22 +271,22 @@ describe('check-careful.sh', () => {
test('kubectl delete warns with kubectl in message', () => {
const { exitCode, output } = runHook(CAREFUL_SCRIPT, carefulInput('kubectl delete pod my-pod'));
expect(exitCode).toBe(0);
expect(output.permissionDecision).toBe('ask');
expect(output.message).toContain('kubectl');
expect(output.hookSpecificOutput?.permissionDecision).toBe('ask');
expect(output.hookSpecificOutput?.permissionDecisionReason).toContain('kubectl');
});
test('docker rm -f warns', () => {
const { exitCode, output } = runHook(CAREFUL_SCRIPT, carefulInput('docker rm -f container123'));
expect(exitCode).toBe(0);
expect(output.permissionDecision).toBe('ask');
expect(output.message).toContain('Docker');
expect(output.hookSpecificOutput?.permissionDecision).toBe('ask');
expect(output.hookSpecificOutput?.permissionDecisionReason).toContain('Docker');
});
test('docker system prune -a warns', () => {
const { exitCode, output } = runHook(CAREFUL_SCRIPT, carefulInput('docker system prune -a'));
expect(exitCode).toBe(0);
expect(output.permissionDecision).toBe('ask');
expect(output.message).toContain('Docker');
expect(output.hookSpecificOutput?.permissionDecision).toBe('ask');
expect(output.hookSpecificOutput?.permissionDecisionReason).toContain('Docker');
});
});
@ -305,7 +305,7 @@ describe('check-careful.sh', () => {
test(`"${cmd}" allows`, () => {
const { exitCode, output } = runHook(CAREFUL_SCRIPT, carefulInput(cmd));
expect(exitCode).toBe(0);
expect(output.permissionDecision).toBeUndefined();
expect(output.hookSpecificOutput?.permissionDecision).toBeUndefined();
});
}
});
@ -316,13 +316,13 @@ describe('check-careful.sh', () => {
test('empty command allows gracefully', () => {
const { exitCode, output } = runHook(CAREFUL_SCRIPT, carefulInput(''));
expect(exitCode).toBe(0);
expect(output.permissionDecision).toBeUndefined();
expect(output.hookSpecificOutput?.permissionDecision).toBeUndefined();
});
test('missing command field allows gracefully', () => {
const { exitCode, output } = runHook(CAREFUL_SCRIPT, { tool_input: {} });
expect(exitCode).toBe(0);
expect(output.permissionDecision).toBeUndefined();
expect(output.hookSpecificOutput?.permissionDecision).toBeUndefined();
});
test('malformed JSON input allows gracefully (exit 0, output {})', () => {
@ -339,8 +339,8 @@ describe('check-careful.sh', () => {
const rawJson = '{"tool_input":{"command":\n"rm -rf /tmp/important"}}';
const { exitCode, output } = runHookRaw(CAREFUL_SCRIPT, rawJson);
expect(exitCode).toBe(0);
expect(output.permissionDecision).toBe('ask');
expect(output.message).toContain('recursive delete');
expect(output.hookSpecificOutput?.permissionDecision).toBe('ask');
expect(output.hookSpecificOutput?.permissionDecisionReason).toContain('recursive delete');
});
});
});
@ -359,7 +359,7 @@ describe('check-freeze.sh', () => {
{ CLAUDE_PLUGIN_DATA: stateDir },
);
expect(exitCode).toBe(0);
expect(output.permissionDecision).toBeUndefined();
expect(output.hookSpecificOutput?.permissionDecision).toBeUndefined();
});
});
@ -371,7 +371,7 @@ describe('check-freeze.sh', () => {
{ CLAUDE_PLUGIN_DATA: stateDir },
);
expect(exitCode).toBe(0);
expect(output.permissionDecision).toBeUndefined();
expect(output.hookSpecificOutput?.permissionDecision).toBeUndefined();
});
});
});
@ -385,9 +385,9 @@ describe('check-freeze.sh', () => {
{ CLAUDE_PLUGIN_DATA: stateDir },
);
expect(exitCode).toBe(0);
expect(output.permissionDecision).toBe('deny');
expect(output.message).toContain('freeze');
expect(output.message).toContain('outside');
expect(output.hookSpecificOutput?.permissionDecision).toBe('deny');
expect(output.hookSpecificOutput?.permissionDecisionReason).toContain('freeze');
expect(output.hookSpecificOutput?.permissionDecisionReason).toContain('outside');
});
});
@ -399,9 +399,9 @@ describe('check-freeze.sh', () => {
{ CLAUDE_PLUGIN_DATA: stateDir },
);
expect(exitCode).toBe(0);
expect(output.permissionDecision).toBe('deny');
expect(output.message).toContain('freeze');
expect(output.message).toContain('outside');
expect(output.hookSpecificOutput?.permissionDecision).toBe('deny');
expect(output.hookSpecificOutput?.permissionDecisionReason).toContain('freeze');
expect(output.hookSpecificOutput?.permissionDecisionReason).toContain('outside');
});
});
});
@ -415,8 +415,8 @@ describe('check-freeze.sh', () => {
{ CLAUDE_PLUGIN_DATA: stateDir },
);
expect(exitCode).toBe(0);
expect(output.permissionDecision).toBe('deny');
expect(output.message).toContain('outside');
expect(output.hookSpecificOutput?.permissionDecision).toBe('deny');
expect(output.hookSpecificOutput?.permissionDecisionReason).toContain('outside');
});
});
});
@ -431,7 +431,7 @@ describe('check-freeze.sh', () => {
{ CLAUDE_PLUGIN_DATA: stateDir },
);
expect(exitCode).toBe(0);
expect(output.permissionDecision).toBeUndefined();
expect(output.hookSpecificOutput?.permissionDecision).toBeUndefined();
} finally {
fs.rmSync(stateDir, { recursive: true, force: true });
}
@ -447,7 +447,7 @@ describe('check-freeze.sh', () => {
{ CLAUDE_PLUGIN_DATA: stateDir },
);
expect(exitCode).toBe(0);
expect(output.permissionDecision).toBeUndefined();
expect(output.hookSpecificOutput?.permissionDecision).toBeUndefined();
});
});
});