mirror of
https://github.com/garrytan/gstack.git
synced 2026-10-31 14:06:50 +01:00
E1: freeze normalizes boundary and target the same way on Windows; hooks spawn through cmd.exe (#2876, #2354)
gstack_hook_normalize_path (careful/bin/hook-extract.sh) maps C:\..., c:/..., UNC \\server\share (leading // kept) and /cygdrive/c to one form; check-freeze runs both the saved boundary and file_path through it before the absolute test, after symlink resolution, and compares case-insensitively on Windows bash. freeze, guard and careful register hooks as bash -c "exec bash \"$HOME/...\"" so cmd.exe, which never expands $HOME, no longer turns the hook into a silent exit 127. Co-authored-by: Kevin Grasso <kevin@biblingo.com> Co-authored-by: Xuan-Ce Wang <261591073+xwang4-svg@users.noreply.github.com>
This commit is contained in:
1 parent
b34cc829ed
commit
fb4d2983e9
6 files changed
+250
-11
No files matched your search
@@ -19,7 +19,7 @@ hooks:
|
||||
- matcher: "Bash"
|
||||
hooks:
|
||||
- type: command
|
||||
command: "bash $HOME/.claude/skills/gstack/careful/bin/check-careful.sh"
|
||||
command: 'bash -c "exec bash \"$HOME/.claude/skills/gstack/careful/bin/check-careful.sh\""'
|
||||
statusMessage: "Checking for destructive commands..."
|
||||
sensitive: true
|
||||
---
|
||||
|
||||
@@ -1,5 +1,5 @@
|
||||
#!/usr/bin/env bash
|
||||
# hook-extract.sh — SHARED JSON helpers for gstack PreToolUse hooks.
|
||||
# hook-extract.sh — SHARED JSON and path helpers for gstack PreToolUse hooks.
|
||||
# Sourced (never executed) by careful/bin/check-careful.sh and
|
||||
# freeze/bin/check-freeze.sh via a path relative to each hook script.
|
||||
#
|
||||
@@ -32,6 +32,48 @@ sys.stdout.write(c if isinstance(c, str) else "")' "$_ghef_field" 2>/dev/null &&
|
||||
return 1
|
||||
}
|
||||
|
||||
# Windows bash (Git Bash / MSYS / Cygwin), read from bash's own OSTYPE so the
|
||||
# check never costs a fork: careful sources this file on every Bash call.
|
||||
case "${OSTYPE:-}" in
|
||||
msys*|cygwin*|win32*) GSTACK_HOOK_IS_WINDOWS=1 ;;
|
||||
*) GSTACK_HOOK_IS_WINDOWS=0 ;;
|
||||
esac
|
||||
_GSTACK_HOOK_AZ_UPPER=ABCDEFGHIJKLMNOPQRSTUVWXYZ
|
||||
_GSTACK_HOOK_AZ_LOWER=abcdefghijklmnopqrstuvwxyz
|
||||
GSTACK_HOOK_PATH=""
|
||||
|
||||
# gstack_hook_normalize_path PATH
|
||||
# The one path canonicalization for freeze's boundary AND the tool's
|
||||
# file_path (#2876). Leaves the result in GSTACK_HOOK_PATH (stdout is the
|
||||
# hook's decision channel, and $(...) would cost a fork):
|
||||
# C:\dev\x c:/dev/x -> /c/dev/x (separators, drive-letter case)
|
||||
# \\srv\share\x -> //srv/share/x (UNC: the leading // is kept)
|
||||
# /cygdrive/c/x -> /c/x (Windows bash only)
|
||||
# Drive-letter and UNC shapes are recognised lexically on every platform.
|
||||
# A path without those shapes is rewritten only on Windows bash: on POSIX
|
||||
# '\' is a legal filename character, and rewriting it would let a name like
|
||||
# 'a\..\..\etc\x' change which directory the check sees. Remaining case
|
||||
# differences on Windows are handled by comparing under nocasematch.
|
||||
# Builtins only, bash 3.2 compatible.
|
||||
gstack_hook_normalize_path() {
|
||||
GSTACK_HOOK_PATH="$1"
|
||||
case "$GSTACK_HOOK_PATH" in
|
||||
[A-Za-z]:[\\/]*|[A-Za-z]:|\\\\[!\\]*) ;;
|
||||
*) [ "$GSTACK_HOOK_IS_WINDOWS" = 1 ] || return 0 ;;
|
||||
esac
|
||||
GSTACK_HOOK_PATH="${GSTACK_HOOK_PATH//\\//}"
|
||||
case "$GSTACK_HOOK_PATH" in
|
||||
[A-Za-z]:/*|[A-Za-z]:)
|
||||
_ghnp_d="${GSTACK_HOOK_PATH%%:*}"
|
||||
_ghnp_pre="${_GSTACK_HOOK_AZ_UPPER%%"$_ghnp_d"*}"
|
||||
[ "$_ghnp_pre" = "$_GSTACK_HOOK_AZ_UPPER" ] || _ghnp_d="${_GSTACK_HOOK_AZ_LOWER:${#_ghnp_pre}:1}"
|
||||
GSTACK_HOOK_PATH="/$_ghnp_d${GSTACK_HOOK_PATH#?:}"
|
||||
;;
|
||||
/cygdrive/[A-Za-z]/*|/cygdrive/[A-Za-z]) GSTACK_HOOK_PATH="${GSTACK_HOOK_PATH#/cygdrive}" ;;
|
||||
esac
|
||||
return 0
|
||||
}
|
||||
|
||||
# gstack_hook_json_string TEXT
|
||||
# Prints TEXT as a JSON string literal (surrounding quotes included),
|
||||
# encoding quotes, backslashes, control characters and newlines. Never build
|
||||
|
||||
@@ -20,12 +20,12 @@ hooks:
|
||||
- matcher: "Edit"
|
||||
hooks:
|
||||
- type: command
|
||||
command: "bash $HOME/.claude/skills/gstack/freeze/bin/check-freeze.sh"
|
||||
command: 'bash -c "exec bash \"$HOME/.claude/skills/gstack/freeze/bin/check-freeze.sh\""'
|
||||
statusMessage: "Checking freeze boundary..."
|
||||
- matcher: "Write"
|
||||
hooks:
|
||||
- type: command
|
||||
command: "bash $HOME/.claude/skills/gstack/freeze/bin/check-freeze.sh"
|
||||
command: 'bash -c "exec bash \"$HOME/.claude/skills/gstack/freeze/bin/check-freeze.sh\""'
|
||||
statusMessage: "Checking freeze boundary..."
|
||||
sensitive: true
|
||||
---
|
||||
|
||||
@@ -56,7 +56,7 @@ fi
|
||||
# A helper from an older install that lacks the function must fail CLOSED
|
||||
# (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
|
||||
if ! command -v gstack_hook_state_root >/dev/null 2>&1 || ! command -v gstack_hook_normalize_path >/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
|
||||
@@ -93,6 +93,12 @@ if [ -z "$FREEZE_DIR" ]; then
|
||||
exit 0
|
||||
fi
|
||||
|
||||
# Both sides of the comparison go through one canonicalization: the state
|
||||
# file may hold C:\dev\proj\ or the /c/dev/proj/ that `pwd` prints in Git
|
||||
# Bash, and Claude Code on Windows reports file_path as C:\... (#2876).
|
||||
gstack_hook_normalize_path "$FREEZE_DIR"
|
||||
FREEZE_DIR="$GSTACK_HOOK_PATH"
|
||||
|
||||
case "$FREEZE_DIR" in
|
||||
/*) ;;
|
||||
*)
|
||||
@@ -123,6 +129,11 @@ if [ -z "$FILE_PATH" ]; then
|
||||
exit 0
|
||||
fi
|
||||
|
||||
# Canonicalize before the absolute test: a drive-letter path does not start
|
||||
# with '/', so it used to be joined onto cwd and every edit was denied.
|
||||
gstack_hook_normalize_path "$FILE_PATH"
|
||||
FILE_PATH="$GSTACK_HOOK_PATH"
|
||||
|
||||
# Resolve file_path to absolute if it isn't already
|
||||
case "$FILE_PATH" in
|
||||
/*) ;; # already absolute
|
||||
@@ -131,8 +142,14 @@ case "$FILE_PATH" in
|
||||
;;
|
||||
esac
|
||||
|
||||
# Normalize: remove double slashes and trailing slash
|
||||
# Normalize: remove double slashes and trailing slash. A leading '//' is a UNC
|
||||
# host (//server/share) and is kept, or the check would compare a local path.
|
||||
_FP_LEAD=""
|
||||
case "$FILE_PATH" in
|
||||
//[!/]*) _FP_LEAD="/" ;;
|
||||
esac
|
||||
FILE_PATH=$(printf '%s' "$FILE_PATH" | sed 's|/\+|/|g;s|/$||')
|
||||
FILE_PATH="$_FP_LEAD$FILE_PATH"
|
||||
[ -n "$FILE_PATH" ] || FILE_PATH="/"
|
||||
|
||||
# Resolve symlinks and .. sequences (POSIX-portable, works on macOS).
|
||||
@@ -157,12 +174,16 @@ _resolve_path() {
|
||||
_base="$(basename "$_p")"
|
||||
if [ "$_base" = / ]; then printf '/'; return; fi
|
||||
_dir="$(cd "$_dir" 2>/dev/null && pwd -P || printf '%s' "$_dir")"
|
||||
printf '%s/%s' "${_dir%/}" "$_base"
|
||||
# pwd -P answers in the platform's own dialect (/cygdrive/c/... on Cygwin).
|
||||
gstack_hook_normalize_path "${_dir%/}/$_base"
|
||||
printf '%s' "$GSTACK_HOOK_PATH"
|
||||
}
|
||||
FILE_PATH=$(_resolve_path "$FILE_PATH")
|
||||
FREEZE_DIR=$(_resolve_path "$FREEZE_DIR")
|
||||
|
||||
# Check: does the file path start with the freeze directory?
|
||||
# Check: does the file path start with the freeze directory? Windows paths
|
||||
# are case-insensitive, so compare that way there.
|
||||
[ "$GSTACK_HOOK_IS_WINDOWS" = 1 ] && shopt -s nocasematch
|
||||
case "$FILE_PATH" in
|
||||
"${FREEZE_DIR%/}/"*|"${FREEZE_DIR}")
|
||||
# Inside freeze boundary — allow
|
||||
|
||||
+3
-3
@@ -20,17 +20,17 @@ hooks:
|
||||
- matcher: "Bash"
|
||||
hooks:
|
||||
- type: command
|
||||
command: "bash $HOME/.claude/skills/gstack/careful/bin/check-careful.sh"
|
||||
command: 'bash -c "exec bash \"$HOME/.claude/skills/gstack/careful/bin/check-careful.sh\""'
|
||||
statusMessage: "Checking for destructive commands..."
|
||||
- matcher: "Edit"
|
||||
hooks:
|
||||
- type: command
|
||||
command: "bash $HOME/.claude/skills/gstack/freeze/bin/check-freeze.sh"
|
||||
command: 'bash -c "exec bash \"$HOME/.claude/skills/gstack/freeze/bin/check-freeze.sh\""'
|
||||
statusMessage: "Checking freeze boundary..."
|
||||
- matcher: "Write"
|
||||
hooks:
|
||||
- type: command
|
||||
command: "bash $HOME/.claude/skills/gstack/freeze/bin/check-freeze.sh"
|
||||
command: 'bash -c "exec bash \"$HOME/.claude/skills/gstack/freeze/bin/check-freeze.sh\""'
|
||||
statusMessage: "Checking freeze boundary..."
|
||||
sensitive: true
|
||||
---
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
import { describe, expect, test } from 'bun:test';
|
||||
import * as fs from 'fs';
|
||||
import * as path from 'path';
|
||||
import * as os from 'os';
|
||||
|
||||
const ROOT = path.resolve(import.meta.dir, '..');
|
||||
const FILES = ['investigate/SKILL.md.tmpl', 'investigate/SKILL.md'];
|
||||
@@ -32,3 +33,178 @@ describe('investigate freeze path resolution', () => {
|
||||
});
|
||||
}
|
||||
});
|
||||
|
||||
// E1 (#2876, #2354 residual). One canonicalization for the saved boundary and
|
||||
// the tool's file_path, exercised through the real hook script. Hooks run via
|
||||
// explicit `bash` argv with template-literal paths so the file stays in the
|
||||
// windows-free lane (scripts/test-free-shards.ts --windows-only --list).
|
||||
const IS_WIN = process.platform === 'win32';
|
||||
const FREEZE_HOOK = `${ROOT}/freeze/bin/check-freeze.sh`;
|
||||
|
||||
function decision(stdout: string): string {
|
||||
const raw = stdout.trim();
|
||||
if (raw === '{}') return 'allow';
|
||||
return JSON.parse(raw)?.hookSpecificOutput?.permissionDecision ?? `unparsed: ${raw}`;
|
||||
}
|
||||
|
||||
function withState<T>(boundary: string, fn: (env: Record<string, string>) => T): T {
|
||||
const stateDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gstack-e1-freeze-'));
|
||||
try {
|
||||
fs.writeFileSync(path.join(stateDir, 'freeze-dir.txt'), `${boundary}\n`);
|
||||
return fn({ ...process.env, GSTACK_STATE_ROOT: stateDir } as Record<string, string>);
|
||||
} finally {
|
||||
fs.rmSync(stateDir, { recursive: true, force: true });
|
||||
}
|
||||
}
|
||||
|
||||
function freezeDecision(boundary: string, filePath: string): string {
|
||||
return withState(boundary, (env) => {
|
||||
const r = Bun.spawnSync(['bash', FREEZE_HOOK], {
|
||||
stdin: Buffer.from(JSON.stringify({ tool_input: { file_path: filePath } })),
|
||||
env,
|
||||
timeout: 15_000,
|
||||
});
|
||||
expect(r.exitCode).toBe(0);
|
||||
return decision(r.stdout.toString());
|
||||
});
|
||||
}
|
||||
|
||||
describe('E1: freeze boundary and target share one platform-aware normalization (#2876)', () => {
|
||||
test.each([
|
||||
['C:\\dev\\proj\\src\\', 'C:\\dev\\proj\\src\\a.py', 'allow'],
|
||||
['/c/dev/proj/src/', 'C:\\dev\\proj\\src\\a.py', 'allow'],
|
||||
['C:\\dev\\proj\\src\\', '/c/dev/proj/src/a.py', 'allow'],
|
||||
['c:/dev/proj/src/', 'C:\\dev\\proj\\src\\a.py', 'allow'],
|
||||
['C:/dev/proj/src', 'c:\\dev\\proj\\src\\deep\\new\\file.py', 'allow'],
|
||||
['C:\\dev\\proj\\src\\', 'C:\\dev\\proj\\src-old\\a.py', 'deny'],
|
||||
['C:\\dev\\proj\\src\\', 'D:\\dev\\proj\\src\\a.py', 'deny'],
|
||||
['C:\\dev\\proj\\src\\', 'C:\\dev\\proj\\other\\missing\\a.py', 'deny'],
|
||||
['\\\\server\\share\\proj\\', '\\\\server\\share\\proj\\a.py', 'allow'],
|
||||
['\\\\server\\share\\proj\\', '\\\\server\\other\\proj\\a.py', 'deny'],
|
||||
['\\\\server\\share\\proj\\', '/server/share/proj/a.py', 'deny'],
|
||||
])('boundary %s, target %s → %s', (boundary, target, expected) => {
|
||||
expect(freezeDecision(boundary, target)).toBe(expected);
|
||||
});
|
||||
|
||||
test.skipIf(!IS_WIN)('Windows: drive and path case differences do not decide the outcome', () => {
|
||||
expect(freezeDecision('C:\\Dev\\Proj\\Src\\', 'c:\\dev\\proj\\src\\a.py')).toBe('allow');
|
||||
expect(freezeDecision('c:\\dev\\proj\\src\\', 'C:\\DEV\\PROJ\\SRC-OLD\\a.py')).toBe('deny');
|
||||
});
|
||||
|
||||
test.skipIf(!IS_WIN)('Windows: a junction inside the boundary that points outside is denied', () => {
|
||||
const base = fs.realpathSync(fs.mkdtempSync(path.join(os.tmpdir(), 'gstack-e1-junction-')));
|
||||
try {
|
||||
fs.mkdirSync(path.join(base, 'boundary'));
|
||||
fs.mkdirSync(path.join(base, 'outside'));
|
||||
fs.symlinkSync(path.join(base, 'outside'), path.join(base, 'boundary', 'escape'), 'junction');
|
||||
expect(freezeDecision(`${path.join(base, 'boundary')}\\`, path.join(base, 'boundary', 'escape', 'a.py'))).toBe('deny');
|
||||
expect(freezeDecision(`${path.join(base, 'boundary')}\\`, path.join(base, 'boundary', 'a.py'))).toBe('allow');
|
||||
} finally {
|
||||
fs.rmSync(base, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
|
||||
test.skipIf(IS_WIN)('POSIX: a backslash in a filename stays a filename character', () => {
|
||||
const base = fs.realpathSync(fs.mkdtempSync(path.join(os.tmpdir(), 'gstack-e1-bslash-')));
|
||||
try {
|
||||
const boundary = path.join(base, 'boundary');
|
||||
fs.mkdirSync(boundary);
|
||||
expect(freezeDecision(`${boundary}/`, `${boundary}/a\\..\\..\\etc\\x`)).toBe('allow');
|
||||
} finally {
|
||||
fs.rmSync(base, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
// #2354 residual: on Windows Claude Code spawns hook commands through
|
||||
// `cmd.exe /d /s /c`, which never expands $HOME, so `bash $HOME/...` handed
|
||||
// bash a literal "$HOME/..." path, exited 127, and the hook silently failed
|
||||
// open. Run every registered freeze/guard/careful command the way cmd.exe
|
||||
// does (no $ expansion, MSVC argv quoting) and the way POSIX sh does.
|
||||
function registeredHookCommands(rel: string): string[] {
|
||||
const fm = fs.readFileSync(path.join(ROOT, rel), 'utf-8').split('\n---')[0];
|
||||
return [...fm.matchAll(/^\s*command:\s*(.+)$/gm)].map((m) => {
|
||||
const v = m[1].trim();
|
||||
if (v.startsWith("'")) return v.slice(1, -1).replaceAll("''", "'");
|
||||
if (v.startsWith('"')) return JSON.parse(v);
|
||||
return v;
|
||||
});
|
||||
}
|
||||
|
||||
/** argv as a Windows program (bash.exe) parses its command line from cmd.exe. */
|
||||
function msvcArgv(command: string): string[] {
|
||||
const args: string[] = [];
|
||||
let cur = '';
|
||||
let inQuotes = false;
|
||||
let started = false;
|
||||
for (let i = 0; i < command.length; i++) {
|
||||
const c = command[i];
|
||||
if (c === '\\') {
|
||||
let n = 0;
|
||||
while (command[i] === '\\') { n++; i++; }
|
||||
if (command[i] === '"') {
|
||||
cur += '\\'.repeat(Math.floor(n / 2));
|
||||
if (n % 2) cur += '"'; else { inQuotes = !inQuotes; }
|
||||
} else {
|
||||
cur += '\\'.repeat(n);
|
||||
i--;
|
||||
}
|
||||
started = true;
|
||||
} else if (c === '"') {
|
||||
inQuotes = !inQuotes;
|
||||
started = true;
|
||||
} else if ((c === ' ' || c === '\t') && !inQuotes) {
|
||||
if (started) args.push(cur);
|
||||
cur = '';
|
||||
started = false;
|
||||
} else {
|
||||
cur += c;
|
||||
started = true;
|
||||
}
|
||||
}
|
||||
if (started) args.push(cur);
|
||||
return args;
|
||||
}
|
||||
|
||||
function withInstalledHome<T>(fn: (home: string) => T): T {
|
||||
const home = fs.mkdtempSync(path.join(os.tmpdir(), 'gstack-e1-home-'));
|
||||
try {
|
||||
fs.mkdirSync(path.join(home, '.claude', 'skills'), { recursive: true });
|
||||
fs.symlinkSync(ROOT, path.join(home, '.claude', 'skills', 'gstack'), 'junction');
|
||||
return fn(home);
|
||||
} finally {
|
||||
fs.rmSync(home, { recursive: true, force: true });
|
||||
}
|
||||
}
|
||||
|
||||
const HOOKED = ['freeze/SKILL.md.tmpl', 'guard/SKILL.md.tmpl', 'careful/SKILL.md.tmpl'];
|
||||
|
||||
describe('E1: registered hook commands run under cmd.exe and POSIX sh (#2354)', () => {
|
||||
for (const rel of HOOKED) {
|
||||
const commands = registeredHookCommands(rel);
|
||||
test(`${rel} registers $HOME-anchored hooks`, () => {
|
||||
expect(commands.length).toBeGreaterThan(0);
|
||||
});
|
||||
for (const [i, command] of commands.entries()) {
|
||||
const isFreeze = command.includes('check-freeze.sh');
|
||||
const input = isFreeze ? { tool_input: { file_path: `${os.tmpdir()}${path.sep}outside-boundary.txt` } } : { tool_input: { command: 'rm -rf /' } };
|
||||
const expected = 'deny';
|
||||
const shells: Array<[string, (home: string) => string[]]> = IS_WIN
|
||||
? [['cmd.exe', () => ['cmd.exe', '/d', '/s', '/c', `"${command}"`]]]
|
||||
: [['cmd.exe argv', () => msvcArgv(command)], ['POSIX shell', () => ['sh', '-c', command]]];
|
||||
for (const [label, argv] of shells) {
|
||||
test(`${rel} hook #${i + 1} reaches a decision via ${label}`, () => {
|
||||
withInstalledHome((home) => withState(path.join(home, 'frozen'), (env) => {
|
||||
const r = Bun.spawnSync(argv(home), {
|
||||
stdin: Buffer.from(JSON.stringify(input)),
|
||||
env: { ...env, HOME: home },
|
||||
timeout: 15_000,
|
||||
});
|
||||
expect(r.exitCode, r.stderr.toString()).toBe(0);
|
||||
expect(decision(r.stdout.toString())).toBe(expected);
|
||||
}));
|
||||
});
|
||||
}
|
||||
}
|
||||
}
|
||||
});
|
||||
Reference in new issue
Block a user