mirror of https://github.com/garrytan/gstack.git
fix: fail closed across hook decision schemas
This commit is contained in:
parent
3623783b8a
commit
d1423cd1a9
|
|
@ -1,3 +0,0 @@
|
|||
# Dispatch inbox
|
||||
|
||||
- [ ] Investigate Windows-safe suite filtering and prerequisites for `test/readme-throughput.test.ts` and `browse/test/tab-isolation.test.ts`; `origin/main` reproduced the failures. (files: package.json, test/readme-throughput.test.ts, browse/test/tab-isolation.test.ts) — noticed 2026-07-10
|
||||
|
|
@ -58,7 +58,7 @@ From inside your repo, paste this. Switches you to team mode, bootstraps the rep
|
|||
(cd ~/.claude/skills/gstack && ./setup --team) && ~/.claude/skills/gstack/bin/gstack-team-init required && git add .claude/ CLAUDE.md && git commit -m "require gstack for AI-assisted work"
|
||||
```
|
||||
|
||||
No vendored gstack install in your repo, no version drift, no manual upgrades. Required mode commits one Node 18+ CommonJS enforcement hook, so the same project hook works from POSIX and Windows hook runners without duplicate shell-specific handlers. If the hook runner omits its project directory or provides a stale path, enforcement fails closed with an intentional denial instead of a hook error. Every Claude Code session starts with a fast auto-update check (throttled to once/hour, network-failure-safe, completely silent).
|
||||
Your project stays small, and everyone gets the same up-to-date version of gstack. If gstack is missing or its safety check cannot run, Claude Code and Copilot stop and explain what to do. If everything is ready, the check stays out of the way and your usual permission questions still appear. Updates happen quietly when a Claude Code session starts, at most once an hour.
|
||||
|
||||
Swap `required` for `optional` if you'd rather nudge teammates than block them.
|
||||
|
||||
|
|
|
|||
|
|
@ -155,11 +155,15 @@ Then restart your AI coding tool.
|
|||
`;
|
||||
|
||||
process.stderr.write(instructions);
|
||||
const decision = {
|
||||
permissionDecision: 'deny',
|
||||
permissionDecisionReason: instructions,
|
||||
};
|
||||
process.stdout.write(`${JSON.stringify({
|
||||
...decision,
|
||||
hookSpecificOutput: {
|
||||
hookEventName: 'PreToolUse',
|
||||
permissionDecision: 'deny',
|
||||
permissionDecisionReason: instructions,
|
||||
...decision,
|
||||
},
|
||||
})}\n`);
|
||||
}
|
||||
|
|
@ -179,10 +183,11 @@ HOOK_EOF
|
|||
if (!settings.hooks) settings.hooks = {};
|
||||
if (!settings.hooks.PreToolUse) settings.hooks.PreToolUse = [];
|
||||
|
||||
const hookCommand = \"node -e \\\"const deny=reason=>{console.error(reason);console.log(JSON.stringify({hookSpecificOutput:{hookEventName:'PreToolUse',permissionDecision:'deny',permissionDecisionReason:reason}}))};const projectDir=process.env.CLAUDE_PROJECT_DIR;if(!projectDir){deny('BLOCKED: CLAUDE_PROJECT_DIR is unavailable, so the required gstack hook cannot be loaded.')}else{try{require(require('node:path').join(projectDir, '.claude', 'hooks', 'check-gstack.cjs'))}catch{deny('BLOCKED: the required gstack hook could not be loaded. Verify project hook setup and retry.')}}\\\"\";
|
||||
const hookMatcher = 'Skill|skill';
|
||||
const hookCommand = \"node -e \\\"const deny=reason=>{const decision={permissionDecision:'deny',permissionDecisionReason:reason};console.error(reason);console.log(JSON.stringify({...decision,hookSpecificOutput:{hookEventName:'PreToolUse',...decision}}))};const projectDir=process.env.CLAUDE_PROJECT_DIR;if(!projectDir){deny('BLOCKED: CLAUDE_PROJECT_DIR is unavailable, so the required gstack hook cannot be loaded.')}else{try{require(require('node:path').join(projectDir, '.claude', 'hooks', 'check-gstack.cjs'))}catch{deny('BLOCKED: the required gstack hook could not be loaded. Verify project hook setup and retry.')}}\\\"\";
|
||||
let found = false;
|
||||
settings.hooks.PreToolUse = settings.hooks.PreToolUse.filter(entry => {
|
||||
if (entry.matcher !== 'Skill' || !entry.hooks) return true;
|
||||
if (!['Skill', hookMatcher].includes(entry.matcher) || !entry.hooks) return true;
|
||||
|
||||
const matchingHooks = entry.hooks.filter(
|
||||
h => h.command && h.command.includes('check-gstack')
|
||||
|
|
@ -193,6 +198,7 @@ HOOK_EOF
|
|||
h => !h.command || !h.command.includes('check-gstack')
|
||||
);
|
||||
if (!found) {
|
||||
entry.matcher = hookMatcher;
|
||||
entry.hooks.push({ type: 'command', command: hookCommand });
|
||||
found = true;
|
||||
}
|
||||
|
|
@ -201,7 +207,7 @@ HOOK_EOF
|
|||
|
||||
if (!found) {
|
||||
settings.hooks.PreToolUse.push({
|
||||
matcher: 'Skill',
|
||||
matcher: hookMatcher,
|
||||
hooks: [{
|
||||
type: 'command',
|
||||
command: hookCommand
|
||||
|
|
|
|||
|
|
@ -1,37 +0,0 @@
|
|||
<!doctype html>
|
||||
<html lang="en">
|
||||
<head>
|
||||
<meta charset="utf-8">
|
||||
<meta name="viewport" content="width=device-width, initial-scale=1">
|
||||
<title>Cross-platform gstack enforcement hook</title>
|
||||
</head>
|
||||
<body>
|
||||
<main>
|
||||
<h1>Cross-platform gstack enforcement hook</h1>
|
||||
|
||||
<h2>Design decisions</h2>
|
||||
<ul>
|
||||
<li>Required mode generates <code>.claude/hooks/check-gstack.cjs</code>. The explicit CommonJS extension is unaffected by a consumer repository's <code>package.json</code> module type.</li>
|
||||
<li>The one registered command launches Node 18+ and resolves <code>CLAUDE_PROJECT_DIR</code> inside JavaScript through <code>process.env</code>. It contains no Bash, PowerShell, or cmd environment expansion.</li>
|
||||
<li>The launcher validates <code>CLAUDE_PROJECT_DIR</code> before calling <code>path.join</code>. Missing or empty project context emits a structured <code>PreToolUse</code> denial, writes a clear reason to stderr, and exits zero instead of surfacing a hook execution error.</li>
|
||||
<li>Path resolution and module loading run inside the same <code>try/catch</code>. A nonexistent project, an existing project without the generated hook, or any other load failure returns the same concise denial without exposing a stack trace or the full project path.</li>
|
||||
<li>The generated hook resolves the user home in <code>HOME</code>, <code>USERPROFILE</code>, then <code>os.homedir()</code> order so POSIX, Git Bash, PowerShell, and cmd runners use the expected global install root.</li>
|
||||
<li>The hook contract follows <code>docs/spikes/claude-code-hook-mutation.md</code>: pass-through writes <code>{}</code>; denial writes a <code>hookSpecificOutput</code> object for <code>PreToolUse</code> with <code>permissionDecision: "deny"</code>.</li>
|
||||
<li>Missing or unreadable gstack is an intentional denial with exit code zero. Install instructions are present in stderr and <code>permissionDecisionReason</code>, so the runner does not report a hook execution error.</li>
|
||||
<li>Rerunning required mode replaces a matching legacy <code>check-gstack.sh</code> registration, removes duplicate matching registrations, and deletes the obsolete Bash hook only after the CommonJS hook is generated and registered successfully. If Git tracks the deleted hook, the printed <code>git add</code> command includes its path so the suggested migration commit stages the deletion; untracked removed hooks are omitted because they have no index change. Unrelated project hooks remain unchanged.</li>
|
||||
</ul>
|
||||
|
||||
<h2>Deviations</h2>
|
||||
<p>None. The implementation uses the requested Node launcher, one hook handler, and the repository's documented hook output schema.</p>
|
||||
|
||||
<h2>Tradeoffs</h2>
|
||||
<ul>
|
||||
<li>The generated project now requires Node 18+ at hook execution time. This removes shell selection ambiguity and matches the requested runtime floor.</li>
|
||||
<li>The generated hook is invoked through Node rather than as an executable file, so POSIX executable mode bits are no longer part of the contract.</li>
|
||||
</ul>
|
||||
|
||||
<h2>Open questions</h2>
|
||||
<p>None.</p>
|
||||
</main>
|
||||
</body>
|
||||
</html>
|
||||
|
|
@ -89,6 +89,8 @@ function expectStructuredDeny(
|
|||
expect(result.stderr.trim()).toBe(reason);
|
||||
expect(result.stderr).not.toContain('MODULE_NOT_FOUND');
|
||||
expect(JSON.parse(result.stdout)).toEqual({
|
||||
permissionDecision: 'deny',
|
||||
permissionDecisionReason: reason,
|
||||
hookSpecificOutput: {
|
||||
hookEventName: 'PreToolUse',
|
||||
permissionDecision: 'deny',
|
||||
|
|
@ -329,11 +331,11 @@ describe('gstack-team-init', () => {
|
|||
const settings = JSON.parse(fs.readFileSync(settingsPath, 'utf-8'));
|
||||
expect(settings.hooks.PreToolUse).toHaveLength(1);
|
||||
const entry = settings.hooks.PreToolUse[0];
|
||||
expect(entry.matcher).toBe('Skill');
|
||||
expect(entry.matcher).toBe('Skill|skill');
|
||||
expect(entry.hooks).toHaveLength(1);
|
||||
expect(entry.hooks[0]).not.toHaveProperty('shell');
|
||||
expect(entry.hooks[0].command).toBe(
|
||||
`node -e "const deny=reason=>{console.error(reason);console.log(JSON.stringify({hookSpecificOutput:{hookEventName:'PreToolUse',permissionDecision:'deny',permissionDecisionReason:reason}}))};const projectDir=process.env.CLAUDE_PROJECT_DIR;if(!projectDir){deny('BLOCKED: CLAUDE_PROJECT_DIR is unavailable, so the required gstack hook cannot be loaded.')}else{try{require(require('node:path').join(projectDir, '.claude', 'hooks', 'check-gstack.cjs'))}catch{deny('BLOCKED: the required gstack hook could not be loaded. Verify project hook setup and retry.')}}"`,
|
||||
`node -e "const deny=reason=>{const decision={permissionDecision:'deny',permissionDecisionReason:reason};console.error(reason);console.log(JSON.stringify({...decision,hookSpecificOutput:{hookEventName:'PreToolUse',...decision}}))};const projectDir=process.env.CLAUDE_PROJECT_DIR;if(!projectDir){deny('BLOCKED: CLAUDE_PROJECT_DIR is unavailable, so the required gstack hook cannot be loaded.')}else{try{require(require('node:path').join(projectDir, '.claude', 'hooks', 'check-gstack.cjs'))}catch{deny('BLOCKED: the required gstack hook could not be loaded. Verify project hook setup and retry.')}}"`,
|
||||
);
|
||||
expect(entry.hooks[0].command).not.toMatch(
|
||||
/\$CLAUDE_PROJECT_DIR|\$env:CLAUDE_PROJECT_DIR|%CLAUDE_PROJECT_DIR%/,
|
||||
|
|
@ -498,6 +500,10 @@ describe('gstack-team-init', () => {
|
|||
expect(result.stderr).toContain('git clone --depth 1');
|
||||
const decision = JSON.parse(result.stdout);
|
||||
expect(decision).toEqual({
|
||||
permissionDecision: 'deny',
|
||||
permissionDecisionReason: expect.stringContaining(
|
||||
'Then restart your AI coding tool.',
|
||||
),
|
||||
hookSpecificOutput: {
|
||||
hookEventName: 'PreToolUse',
|
||||
permissionDecision: 'deny',
|
||||
|
|
@ -508,6 +514,58 @@ describe('gstack-team-init', () => {
|
|||
});
|
||||
});
|
||||
|
||||
test('required: install verification errors return a structured deny', () => {
|
||||
run(`${TEAM_INIT} required`, { cwd: tmpDir });
|
||||
const settings = JSON.parse(
|
||||
fs.readFileSync(path.join(tmpDir, '.claude', 'settings.json'), 'utf-8'),
|
||||
);
|
||||
const command = settings.hooks.PreToolUse[0].hooks[0].command;
|
||||
const fakeHome = path.join(tmpDir, 'unreadable-home');
|
||||
const preload = path.join(tmpDir, 'fail-gstack-stat.cjs');
|
||||
fs.mkdirSync(fakeHome, { recursive: true });
|
||||
fs.writeFileSync(
|
||||
preload,
|
||||
`'use strict';
|
||||
const fs = require('node:fs');
|
||||
const original = fs.statSync;
|
||||
fs.statSync = function (target, ...args) {
|
||||
if (String(target).replace(/\\\\/g, '/').includes('skills/gstack/bin')) {
|
||||
const error = new Error('injected verification failure');
|
||||
error.code = 'EACCES';
|
||||
throw error;
|
||||
}
|
||||
return original.call(this, target, ...args);
|
||||
};
|
||||
`,
|
||||
);
|
||||
|
||||
const result = runHook(command, {
|
||||
cwd: tmpDir,
|
||||
env: {
|
||||
CLAUDE_PROJECT_DIR: tmpDir,
|
||||
HOME: fakeHome,
|
||||
USERPROFILE: fakeHome,
|
||||
NODE_OPTIONS: `--require=${preload}`,
|
||||
},
|
||||
});
|
||||
|
||||
expect(result.exitCode).toBe(0);
|
||||
expect(result.stderr).toContain(
|
||||
'BLOCKED: the global gstack install could not be verified.',
|
||||
);
|
||||
expect(result.stderr).not.toContain('injected verification failure');
|
||||
const decision = JSON.parse(result.stdout);
|
||||
expect(decision.permissionDecision).toBe('deny');
|
||||
expect(decision.permissionDecisionReason).toContain(
|
||||
'BLOCKED: the global gstack install could not be verified.',
|
||||
);
|
||||
expect(decision.hookSpecificOutput).toEqual({
|
||||
hookEventName: 'PreToolUse',
|
||||
permissionDecision: 'deny',
|
||||
permissionDecisionReason: decision.permissionDecisionReason,
|
||||
});
|
||||
});
|
||||
|
||||
test('required: rerun removes a legacy Bash hook without touching other hooks', () => {
|
||||
const hooksDir = path.join(tmpDir, '.claude', 'hooks');
|
||||
const legacyHook = path.join(hooksDir, 'check-gstack.sh');
|
||||
|
|
@ -576,6 +634,7 @@ describe('gstack-team-init', () => {
|
|||
).toBe('');
|
||||
expect(settings.hooks.PreToolUse).toHaveLength(1);
|
||||
expect(settings.hooks.PreToolUse[0].hooks).toHaveLength(1);
|
||||
expect(settings.hooks.PreToolUse[0].matcher).toBe('Skill|skill');
|
||||
expect(settings.hooks.PreToolUse[0].hooks[0].command).toContain(
|
||||
'check-gstack.cjs',
|
||||
);
|
||||
|
|
|
|||
Loading…
Reference in New Issue