CalvinBackup/gstack
1
0
Fork 0
mirror of https://github.com/garrytan/gstack.git synced 2026-09-09 14:38:59 +02:00
Code Issues Packages Projects Releases Wiki Activity

fix(freeze): hook reads the same state root /freeze writes — fails closed under GSTACK_HOME (#1459, #1509)

Browse Source 
check-freeze.sh resolved its state dir as ${CLAUDE_PLUGIN_DATA:-$HOME/.gstack}
while every writer (/freeze, /guard, /unfreeze, /investigate) resolves through
bin/gstack-paths, GSTACK_HOME first. With GSTACK_HOME set, /freeze wrote
freeze-dir.txt under GSTACK_HOME, the hook read $HOME/.gstack, found no file,
and allowed everything — a deny-tier boundary failing open.

One resolver now: gstack_hook_state_root() in careful/bin/hook-extract.sh
(already sourced by both check-freeze.sh and check-careful.sh) implements the
exact gstack-paths chain, including the CLAUDE_PLUGIN_ROOT guard that keeps a
CLAUDE_PLUGIN_DATA leaked from another plugin from redirecting our state.
check-freeze.sh and gstack_hook_log_fire both call it; nothing spawns
gstack-paths from a hook.

Tests: the GSTACK_HOME deny regression, GSTACK_HOME-over-CLAUDE_PLUGIN_DATA
precedence, plugin-root guard both ways, and a byte-parity check against
bin/gstack-paths across six env combinations. Existing freeze tests now pass
CLAUDE_PLUGIN_ROOT like a real plugin install would.

Idea from PR #1509 (@NikhileshNanduri); implemented natively against the shared
resolver rather than a second fallback chain.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
This commit is contained in:
Garry Tan
2026-09-03 01:18:18 +00:00
co-authored by Claude Fable 5.1
parent 0d1bd5616c
commit 17733302cc
3 changed files with 151 additions and 20 deletions
Download Patch File Download Diff File Expand all files Collapse all files
careful/bin/hook-extract.sh
+26 -3
View File
@@ -63,12 +63,35 @@ gstack_hook_decision() {
printf '{"hookSpecificOutput":{"hookEventName":"PreToolUse","permissionDecision":"%s","permissionDecisionReason":%s}}\n' "$_ghd_decision" "$_ghd_encoded"
}
# gstack_hook_state_root
# Print the gstack state root, resolved with EXACTLY the chain bin/gstack-paths
# uses (GSTACK_STATE_ROOT): GSTACK_HOME, then CLAUDE_PLUGIN_DATA only when
# CLAUDE_PLUGIN_ROOT names gstack (a CLAUDE_PLUGIN_DATA leaked from another
# plugin via CLAUDE_ENV_FILE must not redirect our state), then $HOME/.gstack,
# then a project-local .gstack. Hooks run on every Edit/Bash call, so this is
# pure bash — never spawn gstack-paths from a hook. The writers (/freeze,
# /guard, /unfreeze, /investigate) resolve through gstack-paths; a reader that
# used a different chain failed OPEN whenever GSTACK_HOME was set (#1459).
# test/hook-scripts.test.ts pins parity against gstack-paths.
gstack_hook_state_root() {
if [ -n "${GSTACK_HOME:-}" ]; then
printf '%s\n' "$GSTACK_HOME"
elif [ -n "${CLAUDE_PLUGIN_DATA:-}" ] && printf '%s' "${CLAUDE_PLUGIN_ROOT:-}" | grep -qi "gstack"; then
printf '%s\n' "$CLAUDE_PLUGIN_DATA"
elif [ -n "${HOME:-}" ]; then
printf '%s\n' "$HOME/.gstack"
else
printf '%s\n' ".gstack"
fi
}
# gstack_hook_log_fire SKILL PATTERN
# Append a hook_fire analytics record (pattern name only, never command
# content). Respects GSTACK_HOME so tests never pollute the operator's real
# analytics file. Best-effort: failures never affect the hook decision.
# content). Resolves the state root through gstack_hook_state_root so tests
# never pollute the operator's real analytics file. Best-effort: failures
# never affect the hook decision.
gstack_hook_log_fire() {
_ghlf_dir="${GSTACK_HOME:-$HOME/.gstack}/analytics"
_ghlf_dir="$(gstack_hook_state_root)/analytics"
mkdir -p "$_ghlf_dir" 2>/dev/null || true
# Fields are JSON-encoded (a repo basename can carry quotes/backslashes) —
# same rule this file states for decisions: never raw-interpolate into JSON.
freeze/bin/check-freeze.sh
+6 -2
View File
@@ -31,8 +31,12 @@ if [ ! -f "$_HOOK_HELPER" ] || ! . "$_HOOK_HELPER" 2>/dev/null; then
exit 0
fi
# Locate the freeze directory state file
STATE_DIR="${CLAUDE_PLUGIN_DATA:-$HOME/.gstack}"
# Locate the freeze directory state file. The writer (/freeze via
# gstack-paths) and this reader MUST resolve the same root or the boundary
# fails open: with GSTACK_HOME set, /freeze wrote freeze-dir.txt under
# GSTACK_HOME while this hook read $HOME/.gstack, found nothing, and allowed
# everything (#1459, #1509). gstack_hook_state_root mirrors gstack-paths.
STATE_DIR="$(gstack_hook_state_root)"
FREEZE_FILE="$STATE_DIR/freeze-dir.txt"
# If no freeze file exists, allow everything (not yet configured)
test/hook-scripts.test.ts
+119 -15
View File
@@ -76,6 +76,34 @@ function withFreezeDir(freezePath: string, fn: (stateDir: string) => void) {
}
}
// The freeze WRITER resolves its state root through bin/gstack-paths, which
// trusts CLAUDE_PLUGIN_DATA only when CLAUDE_PLUGIN_ROOT names gstack; the
// reader mirrors that exact chain (#1459 / #1509). A test standing in for a
// plugin install must supply both, and must neutralize a GSTACK_HOME inherited
// from the shard's process.env (an empty value reads as unset in ${VAR:-}).
function freezeEnv(stateDir: string, extra: Record<string, string> = {}): Record<string, string> {
return { GSTACK_HOME: '', CLAUDE_PLUGIN_DATA: stateDir, CLAUDE_PLUGIN_ROOT: '/plugins/gstack', ...extra };
}
const HOOK_EXTRACT = path.join(ROOT, 'careful', 'bin', 'hook-extract.sh');
const GSTACK_PATHS = path.join(ROOT, 'bin', 'gstack-paths');
/** What the hook helper resolves as the state root under a given env. */
function hookStateRoot(env: Record<string, string>): string {
const r = spawnSync('bash', ['-c', `. "${HOOK_EXTRACT}" && gstack_hook_state_root`], {
env: { PATH: process.env.PATH ?? '', ...env }, encoding: 'utf-8', timeout: 5000,
});
return r.stdout.trim();
}
/** What bin/gstack-paths resolves as GSTACK_STATE_ROOT under the same env. */
function pathsStateRoot(env: Record<string, string>): string {
const r = spawnSync('bash', ['-c', `eval "$("${GSTACK_PATHS}")" && printf '%s' "$GSTACK_STATE_ROOT"`], {
env: { PATH: process.env.PATH ?? '', ...env }, encoding: 'utf-8', timeout: 5000,
});
return r.stdout.trim();
}
// ============================================================
// Frontmatter hook wiring (#2469 / #1871)
// ============================================================
@@ -687,7 +715,7 @@ describe('check-freeze.sh', () => {
const { exitCode, output } = runHook(
FREEZE_SCRIPT,
freezeInput('/Users/dev/project/src/index.ts'),
{ CLAUDE_PLUGIN_DATA: stateDir },
freezeEnv(stateDir),
);
expect(exitCode).toBe(0);
expect(output.hookSpecificOutput?.permissionDecision).toBeUndefined();
@@ -699,7 +727,7 @@ describe('check-freeze.sh', () => {
const { exitCode, output } = runHook(
FREEZE_SCRIPT,
freezeInput('/Users/dev/project/src/components/Button.tsx'),
{ CLAUDE_PLUGIN_DATA: stateDir },
freezeEnv(stateDir),
);
expect(exitCode).toBe(0);
expect(output.hookSpecificOutput?.permissionDecision).toBeUndefined();
@@ -713,7 +741,7 @@ describe('check-freeze.sh', () => {
const { exitCode, output } = runHook(
FREEZE_SCRIPT,
freezeInput('/Users/dev/other-project/index.ts'),
{ CLAUDE_PLUGIN_DATA: stateDir },
freezeEnv(stateDir),
);
expect(exitCode).toBe(0);
expect(output.hookSpecificOutput?.permissionDecision).toBe('deny');
@@ -727,7 +755,7 @@ describe('check-freeze.sh', () => {
const { exitCode, output } = runHook(
FREEZE_SCRIPT,
freezeInput('/etc/hosts'),
{ CLAUDE_PLUGIN_DATA: stateDir },
freezeEnv(stateDir),
);
expect(exitCode).toBe(0);
expect(output.hookSpecificOutput?.permissionDecision).toBe('deny');
@@ -743,7 +771,7 @@ describe('check-freeze.sh', () => {
const { exitCode, output } = runHook(
FREEZE_SCRIPT,
freezeInput('/Users/dev/project/src-old/index.ts'),
{ CLAUDE_PLUGIN_DATA: stateDir },
freezeEnv(stateDir),
);
expect(exitCode).toBe(0);
expect(output.hookSpecificOutput?.permissionDecision).toBe('deny');
@@ -759,7 +787,7 @@ describe('check-freeze.sh', () => {
const { exitCode, output } = runHook(
FREEZE_SCRIPT,
freezeInput('/anywhere/at/all.ts'),
{ CLAUDE_PLUGIN_DATA: stateDir },
freezeEnv(stateDir),
);
expect(exitCode).toBe(0);
expect(output.hookSpecificOutput?.permissionDecision).toBeUndefined();
@@ -775,7 +803,7 @@ describe('check-freeze.sh', () => {
const { exitCode, output } = runHook(
FREEZE_SCRIPT,
{ tool_input: {} },
{ CLAUDE_PLUGIN_DATA: stateDir },
freezeEnv(stateDir),
);
expect(exitCode).toBe(0);
expect(output.hookSpecificOutput?.permissionDecision).toBeUndefined();
@@ -787,7 +815,7 @@ describe('check-freeze.sh', () => {
const { exitCode, output } = runHookRaw(
FREEZE_SCRIPT,
'not json at all {{{{',
{ CLAUDE_PLUGIN_DATA: stateDir },
freezeEnv(stateDir),
);
expect(exitCode).toBe(0);
expect(output.hookSpecificOutput?.permissionDecision).toBe('deny');
@@ -803,7 +831,7 @@ describe('check-freeze.sh', () => {
const { exitCode, output, raw } = runHook(
FREEZE_SCRIPT,
freezeInput('/tmp/evil"quoted/x.ts'),
{ CLAUDE_PLUGIN_DATA: stateDir },
freezeEnv(stateDir),
);
expect(exitCode).toBe(0);
expect(() => JSON.parse(raw)).not.toThrow();
@@ -816,7 +844,7 @@ describe('check-freeze.sh', () => {
const { exitCode, output, raw } = runHook(
FREEZE_SCRIPT,
freezeInput('/tmp/evil\npath.ts'),
{ CLAUDE_PLUGIN_DATA: stateDir },
freezeEnv(stateDir),
);
expect(exitCode).toBe(0);
expect(() => JSON.parse(raw)).not.toThrow();
@@ -834,11 +862,11 @@ describe('check-freeze.sh', () => {
fs.mkdirSync(boundary, { recursive: true });
try {
withFreezeDir(boundary + '/', (stateDir) => {
const inside = runHook(FREEZE_SCRIPT, freezeInput(path.join(boundary, 'index.ts')), { CLAUDE_PLUGIN_DATA: stateDir });
const inside = runHook(FREEZE_SCRIPT, freezeInput(path.join(boundary, 'index.ts')), freezeEnv(stateDir));
expect(inside.exitCode).toBe(0);
expect(inside.output.hookSpecificOutput?.permissionDecision).toBeUndefined();
const outside = runHook(FREEZE_SCRIPT, freezeInput(path.join(base, 'elsewhere.ts')), { CLAUDE_PLUGIN_DATA: stateDir });
const outside = runHook(FREEZE_SCRIPT, freezeInput(path.join(base, 'elsewhere.ts')), freezeEnv(stateDir));
expect(outside.exitCode).toBe(0);
expect(outside.output.hookSpecificOutput?.permissionDecision).toBe('deny');
});
@@ -859,7 +887,7 @@ describe('check-freeze.sh', () => {
fs.copyFileSync(FREEZE_SCRIPT, script);
try {
withFreezeDir('/Users/dev/project/src/', (stateDir) => {
const { exitCode, output } = runHook(script, freezeInput('/Users/dev/project/src/x.ts'), { CLAUDE_PLUGIN_DATA: stateDir });
const { exitCode, output } = runHook(script, 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');
@@ -884,13 +912,13 @@ describe('check-freeze.sh', () => {
fs.symlinkSync(path.join(outside, 'secret.txt'), path.join(boundary, 'link.txt'));
try {
withFreezeDir(boundary + '/', (stateDir) => {
const viaLink = runHook(FREEZE_SCRIPT, freezeInput(path.join(boundary, 'link.txt')), { CLAUDE_PLUGIN_DATA: stateDir });
const viaLink = runHook(FREEZE_SCRIPT, freezeInput(path.join(boundary, 'link.txt')), freezeEnv(stateDir));
expect(viaLink.exitCode).toBe(0);
expect(viaLink.output.hookSpecificOutput?.permissionDecision).toBe('deny');
// A real in-boundary file is unaffected.
fs.writeFileSync(path.join(boundary, 'real.txt'), 'y');
const real = runHook(FREEZE_SCRIPT, freezeInput(path.join(boundary, 'real.txt')), { CLAUDE_PLUGIN_DATA: stateDir });
const real = runHook(FREEZE_SCRIPT, freezeInput(path.join(boundary, 'real.txt')), freezeEnv(stateDir));
expect(real.exitCode).toBe(0);
expect(real.output.hookSpecificOutput?.permissionDecision).toBeUndefined();
});
@@ -900,3 +928,79 @@ describe('check-freeze.sh', () => {
});
});
});
// ============================================================
// check-freeze.sh state-root resolution (#1459 / #1509)
// ============================================================
// /freeze writes freeze-dir.txt under the root gstack-paths resolves
// (GSTACK_HOME first). The reader used to read ${CLAUDE_PLUGIN_DATA:-$HOME/.gstack}
// — so with GSTACK_HOME set it found no file and ALLOWED everything. A deny-tier
// boundary that fails open is not a boundary; writer and reader now share one
// chain (gstack_hook_state_root in careful/bin/hook-extract.sh).
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) => {
withEmptyDir((fakeHome) => {
const { exitCode, output } = runHook(FREEZE_SCRIPT, freezeInput(OUTSIDE), {
GSTACK_HOME: gstackHome, HOME: fakeHome, CLAUDE_PLUGIN_DATA: '', CLAUDE_PLUGIN_ROOT: '',
});
expect(exitCode).toBe(0);
expect(output.hookSpecificOutput?.permissionDecision).toBe('deny');
});
});
});
test('GSTACK_HOME wins over CLAUDE_PLUGIN_DATA (matches gstack-paths precedence)', () => {
withFreezeDir(BOUNDARY, (pluginData) => {
withEmptyDir((gstackHome) => {
// The freeze file lives under CLAUDE_PLUGIN_DATA, but GSTACK_HOME is set and
// has none — the writer would have written there, so the reader must look there.
const { output } = runHook(FREEZE_SCRIPT, freezeInput(OUTSIDE),
freezeEnv(pluginData, { GSTACK_HOME: gstackHome }));
expect(output.hookSpecificOutput?.permissionDecision).toBeUndefined();
});
});
});
test('CLAUDE_PLUGIN_DATA is ignored when CLAUDE_PLUGIN_ROOT is another plugin', () => {
withFreezeDir(BOUNDARY, (pluginData) => {
withEmptyDir((fakeHome) => {
const { output } = runHook(FREEZE_SCRIPT, freezeInput(OUTSIDE),
freezeEnv(pluginData, { CLAUDE_PLUGIN_ROOT: '/plugins/codex', HOME: fakeHome }));
// Falls through to $HOME/.gstack, which has no freeze file → allow.
expect(output.hookSpecificOutput?.permissionDecision).toBeUndefined();
});
});
});
test('CLAUDE_PLUGIN_DATA is honoured when CLAUDE_PLUGIN_ROOT names gstack', () => {
withFreezeDir(BOUNDARY, (pluginData) => {
withEmptyDir((fakeHome) => {
const { output } = runHook(FREEZE_SCRIPT, freezeInput(OUTSIDE), freezeEnv(pluginData, { HOME: fakeHome }));
expect(output.hookSpecificOutput?.permissionDecision).toBe('deny');
});
});
});
test('gstack_hook_state_root is byte-identical to gstack-paths GSTACK_STATE_ROOT', () => {
const combos: Record<string, string>[] = [
{ HOME: '/home/u', GSTACK_HOME: '/state/x', CLAUDE_PLUGIN_DATA: '/plug/data', CLAUDE_PLUGIN_ROOT: '/plugins/gstack' },
{ HOME: '/home/u', GSTACK_HOME: '', CLAUDE_PLUGIN_DATA: '/plug/data', CLAUDE_PLUGIN_ROOT: '/plugins/gstack' },
{ HOME: '/home/u', GSTACK_HOME: '', CLAUDE_PLUGIN_DATA: '/plug/data', CLAUDE_PLUGIN_ROOT: '/plugins/codex' },
{ HOME: '/home/u', GSTACK_HOME: '', CLAUDE_PLUGIN_DATA: '/plug/data', CLAUDE_PLUGIN_ROOT: '' },
{ HOME: '/home/u', GSTACK_HOME: '', CLAUDE_PLUGIN_DATA: '', CLAUDE_PLUGIN_ROOT: '' },
{ HOME: '', GSTACK_HOME: '', CLAUDE_PLUGIN_DATA: '', CLAUDE_PLUGIN_ROOT: '' },
];
for (const env of combos) {
expect(hookStateRoot(env)).toBe(pathsStateRoot(env));
}
});
});