diff --git a/.github/workflows/test-cross-plugin.yml b/.github/workflows/test-cross-plugin.yml index 3c6eff24..6d74a3c4 100644 --- a/.github/workflows/test-cross-plugin.yml +++ b/.github/workflows/test-cross-plugin.yml @@ -13,6 +13,9 @@ name: Cross-Plugin Guards # plugin.json (bumped on every release) and CLAUDE-APPEND are negated: the # contract never reads core's manifest or template, so those edits can't # change its outcome. +# The hooks/** glob plus the three dev guard scripts cover the hook stdin +# contract: every fleet hook keeps its own sealed stdin copy, so a drifted drain +# or fail direction is only caught by re-running that corpus here. on: push: branches: [main] @@ -25,6 +28,10 @@ on: - 'plugins/*/state-templates/CLAUDE-APPEND.md' - '!plugins/claude-code-hermit/state-templates/CLAUDE-APPEND.md' - 'plugins/claude-code-hermit/scripts/**' + - 'plugins/*/hooks/**' + - 'plugins/claude-code-dev-hermit/scripts/git-push-guard.ts' + - 'plugins/claude-code-dev-hermit/scripts/worktree-boundary-guard.ts' + - 'plugins/claude-code-dev-hermit/scripts/record-test-result.ts' - 'tests/cross-plugin/**' - 'tests/lib/**' - '.github/workflows/test-cross-plugin.yml' @@ -41,6 +48,10 @@ on: - 'plugins/*/state-templates/CLAUDE-APPEND.md' - '!plugins/claude-code-hermit/state-templates/CLAUDE-APPEND.md' - 'plugins/claude-code-hermit/scripts/**' + - 'plugins/*/hooks/**' + - 'plugins/claude-code-dev-hermit/scripts/git-push-guard.ts' + - 'plugins/claude-code-dev-hermit/scripts/worktree-boundary-guard.ts' + - 'plugins/claude-code-dev-hermit/scripts/record-test-result.ts' - 'tests/cross-plugin/**' - 'tests/lib/**' - '.github/workflows/test-cross-plugin.yml' diff --git a/plugins/claude-code-dev-hermit/CHANGELOG.md b/plugins/claude-code-dev-hermit/CHANGELOG.md index 0d200a25..1383074c 100644 --- a/plugins/claude-code-dev-hermit/CHANGELOG.md +++ b/plugins/claude-code-dev-hermit/CHANGELOG.md @@ -1,5 +1,13 @@ # Changelog +## [Unreleased] + +### Fixed +- `git-push-guard`, `worktree-boundary-guard`, and `record-test-result` exited mid-stream on stdin past their 1MB cap, leaving the pipe half-read; they now stop buffering but keep consuming to EOF before failing open. +- `record-test-result` crashed with exit 1 on a valid-JSON payload that is not an object (`null`), instead of failing open. +- `worktree-boundary-guard` exited on `WORKTREE_GUARD=off` before reading stdin, so disabling the guard broke the pipe on any payload larger than the pipe buffer; the drain now runs before the switch is honored. +- All three hooks exited 1 on an unhandled stdin stream error; `main()` now catches and exits 0. + ## [0.4.8] - 2026-07-26 ### Fixed diff --git a/plugins/claude-code-dev-hermit/scripts/git-push-guard.ts b/plugins/claude-code-dev-hermit/scripts/git-push-guard.ts index 309519c9..37105905 100644 --- a/plugins/claude-code-dev-hermit/scripts/git-push-guard.ts +++ b/plugins/claude-code-dev-hermit/scripts/git-push-guard.ts @@ -84,13 +84,17 @@ function block(msg: string): void { async function main() { // Read stdin first (every hook must consume stdin), then gate on profile — // so a non-strict session can still surface a one-line "guard inactive" notice. + // Past the cap we stop buffering but keep consuming: exiting mid-stream would + // leave the pipe half-read (broken-pipe errors on the writer's side). const chunks: Buffer[] = []; let total = 0; + let oversize = false; for await (const chunk of process.stdin) { total += chunk.length; - if (total > MAX_STDIN) process.exit(0); + if (total > MAX_STDIN) { oversize = true; continue; } chunks.push(chunk); } + if (oversize) process.exit(0); const raw = Buffer.concat(chunks).toString('utf-8').trim(); if (!raw) process.exit(0); @@ -159,4 +163,4 @@ async function main() { process.exit(0); } -main(); +main().catch(() => process.exit(0)); diff --git a/plugins/claude-code-dev-hermit/scripts/record-test-result.ts b/plugins/claude-code-dev-hermit/scripts/record-test-result.ts index 8a25775f..030b2cc4 100644 --- a/plugins/claude-code-dev-hermit/scripts/record-test-result.ts +++ b/plugins/claude-code-dev-hermit/scripts/record-test-result.ts @@ -111,19 +111,28 @@ if (argv[2] === 'write') { } async function main() { + // Past the cap we stop buffering but keep consuming to EOF — exiting + // mid-stream would leave the pipe half-read. const chunks: Buffer[] = []; let total = 0; + let oversize = false; for await (const chunk of process.stdin) { total += chunk.length; - if (total > MAX_STDIN) process.exit(0); + if (total > MAX_STDIN) { oversize = true; continue; } chunks.push(chunk); } + if (oversize) process.exit(0); const raw = Buffer.concat(chunks).toString('utf-8').trim(); if (!raw) process.exit(0); let data: Json; try { data = JSON.parse(raw); } catch { process.exit(0); } + // Valid JSON that is not an object (`null`, `[]`, `"x"`) is still not a hook + // payload — without this the `null` case throws past the catch above and the + // hook exits 1 instead of failing open. + if (typeof data !== 'object' || data === null || Array.isArray(data)) process.exit(0); + if (data.tool_response?.interrupted === true) process.exit(0); const command = data.tool_input?.command || ''; @@ -146,4 +155,4 @@ async function main() { process.exit(0); } -main(); +main().catch(() => process.exit(0)); diff --git a/plugins/claude-code-dev-hermit/scripts/worktree-boundary-guard.ts b/plugins/claude-code-dev-hermit/scripts/worktree-boundary-guard.ts index 7bd22532..1bab1bd4 100644 --- a/plugins/claude-code-dev-hermit/scripts/worktree-boundary-guard.ts +++ b/plugins/claude-code-dev-hermit/scripts/worktree-boundary-guard.ts @@ -20,16 +20,24 @@ function isUnder(child: string, parent: string): boolean { } async function main() { - if ((process.env.WORKTREE_GUARD || '').trim().toLowerCase() === 'off') process.exit(0); + const guardOff = (process.env.WORKTREE_GUARD || '').trim().toLowerCase() === 'off'; - // Drain stdin to completion (avoids broken-pipe errors) with a size cap. + // Drain stdin to completion (avoids broken-pipe errors) with a size cap: past + // the cap we stop buffering but keep consuming to EOF. This runs BEFORE the + // WORKTREE_GUARD gate — exiting on the env switch without reading would leave + // the pipe half-read for any payload larger than the pipe buffer. When the + // guard is off the payload is never parsed, so consume without buffering. const chunks: Buffer[] = []; let total = 0; + let oversize = false; for await (const chunk of process.stdin) { + if (guardOff) continue; total += chunk.length; - if (total > MAX_STDIN) process.exit(0); + if (total > MAX_STDIN) { oversize = true; continue; } chunks.push(chunk); } + if (guardOff) process.exit(0); + if (oversize) process.exit(0); const raw = Buffer.concat(chunks).toString('utf-8').trim(); if (!raw) process.exit(0); @@ -70,4 +78,4 @@ async function main() { process.exit(0); } -main(); +main().catch(() => process.exit(0)); diff --git a/scripts/test-all.sh b/scripts/test-all.sh index cc572f57..459db2cd 100755 --- a/scripts/test-all.sh +++ b/scripts/test-all.sh @@ -43,4 +43,24 @@ for slug in "${BUN_TEST_SLUGS[@]}" "${RUN_ALL_SLUGS[@]}"; do fi done +# Repo-root guards spanning more than one plugin — nothing above runs them, since +# they live outside every plugin's own discovery. Deliberately serial, after the +# plugin suites: they spawn a hook subprocess per corpus case, and the parallel +# phase is already saturated enough that HA's CPU-bound gate-corpus tests sit +# near their 5s per-test timeout. Keeping this out of that phase leaves the +# tuned parallelism untouched, at ~10s of extra wall time. +cp_start=$(now) +( cd "$ROOT" && bun test tests/cross-plugin ) >"$LOGDIR/cross-plugin.log" 2>&1 +cp_rc=$? +cp_elapsed=$(( $(now) - cp_start )) +if [ "$cp_rc" -eq 0 ]; then + printf "%-32s %-6s %5ss\n" "cross-plugin" "PASS" "$cp_elapsed" +else + printf "%-32s %-6s %5ss\n" "cross-plugin" "FAIL" "$cp_elapsed" + overall_rc=1 + echo "--- cross-plugin (last 20 lines) ---" + tail -20 "$LOGDIR/cross-plugin.log" + echo "---" +fi + exit "$overall_rc" diff --git a/tests/cross-plugin/hook-stdin-contract.test.ts b/tests/cross-plugin/hook-stdin-contract.test.ts new file mode 100644 index 00000000..ccb39673 --- /dev/null +++ b/tests/cross-plugin/hook-stdin-contract.test.ts @@ -0,0 +1,265 @@ +// Cross-plugin guard for the hook stdin contract stated in +// plugins/claude-code-hermit/scripts/lib/hook-input.ts: every hook consumes +// stdin to completion even past its size cap, and each hook has ONE declared +// fail direction for input it cannot parse. +// +// Five stdin idioms ship across the fleet (shared helper, hand-rolled loops, +// readFileSync(0), Bun.stdin.text()) — deliberately, since fleet plugins cannot +// import core at runtime. This test is what keeps the sealed copies honest: it +// feeds every gate the same adversarial corpus on REAL stdin and pins both the +// exit code and the drain. +// +// Not covered here (owned elsewhere, on purpose): the JSON permissionDecision +// envelope bytes, pinned byte-for-byte by HA's tests/gate-corpus.test.ts; and +// each hook's positive gating behavior, owned by its own plugin suite. +// +// Lives at the repo root (outside every plugin's `bun test` / run-all.sh +// discovery) so it never blocks a plugin release; the path-scoped +// test-cross-plugin.yml workflow runs it when any fleet hook changes. + +import { test, expect, describe, beforeAll, afterAll } from 'bun:test'; +import fs from 'node:fs'; +import os from 'node:os'; +import path from 'node:path'; + +const ROOT = path.resolve(import.meta.dir, '../..'); + +const MAX_HOOK_STDIN = 1024 * 1024; // mirrors lib/hook-input.ts and the dev copies + +interface Spec { + name: string; + script: string; + /** Well-formed payload this hook must allow (exit 0). */ + benign: unknown; + /** Exit code for EVERY adversarial shape: empty, garbage, non-object, oversize. */ + failExit: number; +} + +const BASH_LS = { tool_name: 'Bash', tool_input: { command: 'ls -la' } }; +const BENIGN_EDIT = { tool_name: 'Edit', tool_input: { file_path: 'scratch.txt' } }; + +// failExit is the hook's DECLARED fail direction — 0 = fail open (the default +// hook contract), 2 = fail closed. HA's mcp-safety-gate is the fleet's only +// default-deny gate: a payload it cannot parse is exactly the shape an evasion +// takes, so it blocks (hooks/mcp-safety-gate.ts fail() at :79-88, non-object +// check at :114). Changing a value here is a deliberate contract change. +const SPECS: Spec[] = [ + { + name: 'core/pause-gate', + script: 'plugins/claude-code-hermit/scripts/pause-gate.ts', + benign: BASH_LS, + failExit: 0, // unpaused: denyIfPaused() returns, runHook exits 0 + }, + { + name: 'core/ask-gate', + script: 'plugins/claude-code-hermit/scripts/ask-gate.ts', + benign: BASH_LS, + failExit: 0, + }, + { + name: 'core/enforce-deny-patterns', + script: 'plugins/claude-code-hermit/scripts/enforce-deny-patterns.ts', + benign: BASH_LS, + failExit: 0, + }, + { + name: 'core/cache-edit-guard', + script: 'plugins/claude-code-hermit/scripts/cache-edit-guard.ts', + benign: BENIGN_EDIT, + failExit: 0, + }, + { + name: 'dev/git-push-guard', + script: 'plugins/claude-code-dev-hermit/scripts/git-push-guard.ts', + benign: BASH_LS, + failExit: 0, + }, + { + name: 'dev/worktree-boundary-guard', + script: 'plugins/claude-code-dev-hermit/scripts/worktree-boundary-guard.ts', + benign: BENIGN_EDIT, + failExit: 0, + }, + { + name: 'dev/record-test-result', + script: 'plugins/claude-code-dev-hermit/scripts/record-test-result.ts', + benign: BASH_LS, + failExit: 0, + }, + { + name: 'ha/mcp-safety-gate', + script: 'plugins/claude-code-homeassistant-hermit/hooks/mcp-safety-gate.ts', + // A read-only tool is allowed before any entity/config resolution, so the + // benign case stays independent of the machine's HA configuration. + benign: { tool_name: 'mcp__homeassistant__GetDateTime', tool_input: {} }, + failExit: 2, // the fleet's only fail-closed gate + }, + { + name: 'ha/curl-host-gate', + script: 'plugins/claude-code-homeassistant-hermit/hooks/curl-host-gate.ts', + benign: BASH_LS, + failExit: 0, + }, + { + name: 'forge/write-confirm-gate', + script: 'plugins/laravel-forge-hermit/hooks/write-confirm-gate.ts', + benign: BASH_LS, + failExit: 0, + }, + { + name: 'feed/fetch-guard', + script: 'plugins/feed-hermit/hooks/fetch-guard.ts', + // No feed-sources.md in the sandbox cwd → the allowlist read fails open. + benign: { tool_name: 'WebFetch', tool_input: { url: 'https://example.com/x' } }, + failExit: 0, + }, +]; + +let sandbox: string; +const payloadFiles = new Map(); + +// The oversize case is generated, never committed — a 2MB fixture in git to +// assert "the hook kept reading" is not worth the repo weight (HA's gate-corpus +// generates its oversize cases the same way). +function corpus(): Array<{ label: string; body: string }> { + return [ + { label: 'empty stdin', body: '' }, + { label: 'non-JSON garbage', body: 'not json at all {{{' }, + { label: 'non-object JSON (array)', body: '[1,2]' }, + { label: 'non-object JSON (null)', body: 'null' }, + { label: 'non-object JSON (string)', body: '"str"' }, + { label: `oversize ${MAX_HOOK_STDIN * 2} bytes, unparseable`, body: 'x'.repeat(MAX_HOOK_STDIN * 2) }, + ]; +} + +// Built once: the oversize body is a 2MB string, and the registration loop +// below only needs the labels. +const CORPUS = corpus(); + +beforeAll(() => { + sandbox = fs.mkdtempSync(path.join(os.tmpdir(), 'hook-stdin-corpus-')); + for (const { label, body } of CORPUS) { + const file = path.join(sandbox, `payload-${payloadFiles.size}.bin`); + fs.writeFileSync(file, body); + payloadFiles.set(label, file); + } +}); + +afterAll(() => { + fs.rmSync(sandbox, { recursive: true, force: true }); +}); + +/** + * Pipe a payload file into the hook and return BOTH pipeline exit codes. + * + * The writer's code is the drain signal: a hook that stops reading before EOF + * leaves the writer pushing into a closed pipe, which is SIGPIPE (141). Piping + * through bash and reading PIPESTATUS is what makes that observable — writing + * the payload from the test process only surfaces the hook's own exit code, and + * a mid-stream exit and a clean drain both report 0 there. + * + * Clean env (PATH/HOME only) so an ambient AGENT_HOOK_PROFILE or HOMEASSISTANT_* + * on the dev box cannot change a verdict. + */ +async function feed( + scriptRel: string, + payloadFile: string, + extraEnv: Record = {}, +): Promise<{ writer: number; hook: number }> { + const proc = Bun.spawn({ + cmd: [ + 'bash', + '-c', + 'cat "$1" | bun "$2" >/dev/null 2>&1; echo "${PIPESTATUS[0]} ${PIPESTATUS[1]}"', + '_', + payloadFile, + path.join(ROOT, scriptRel), + ], + cwd: sandbox, + env: { PATH: process.env.PATH ?? '', HOME: process.env.HOME ?? '', ...extraEnv }, + stdout: 'pipe', + stderr: 'pipe', + }); + const [out] = await Promise.all([new Response(proc.stdout).text(), proc.exited]); + const [writer, hook] = out.trim().split(/\s+/).map(Number); + return { writer: writer!, hook: hook! }; +} + +for (const spec of SPECS) { + describe(`hook stdin contract — ${spec.name}`, () => { + test('the script exists at its registered path', () => { + expect(fs.existsSync(path.join(ROOT, spec.script))).toBe(true); + }); + + test('allows a well-formed payload', async () => { + const file = path.join(sandbox, `benign-${spec.name.replace(/\W/g, '-')}.json`); + fs.writeFileSync(file, JSON.stringify(spec.benign)); + const r = await feed(spec.script, file); + expect(r.hook).toBe(0); + expect(r.writer).toBe(0); + }); + + for (const { label } of CORPUS) { + test(`${label} → exit ${spec.failExit}, stdin drained`, async () => { + const r = await feed(spec.script, payloadFiles.get(label)!); + // Declared fail direction. + expect(r.hook).toBe(spec.failExit); + // Drain: the writer completed. Only the oversize case can actually fail + // this — smaller payloads fit in the pipe buffer, so the writer finishes + // whether or not the hook ever reads. That one case is the whole point. + expect(r.writer).toBe(0); + }); + } + }); +} + +// A hook's own disable switch must not become a mid-stream exit: the guard is +// off, but the pipe still has to be drained or the writer takes SIGPIPE. +test('dev/worktree-boundary-guard drains stdin even with WORKTREE_GUARD=off', async () => { + const r = await feed( + 'plugins/claude-code-dev-hermit/scripts/worktree-boundary-guard.ts', + payloadFiles.get(`oversize ${MAX_HOOK_STDIN * 2} bytes, unparseable`)!, + { WORKTREE_GUARD: 'off' }, + ); + expect(r.hook).toBe(0); + expect(r.writer).toBe(0); +}); + +test('the corpus covers every fleet hook registered in a hooks.json', () => { + // Auto-discovery keeps SPECS honest in BOTH directions. Hardcoded plugin + // lists went stale twice before (see domain-hatch.contract.test.ts) — derive + // from the filesystem instead. + const registered = new Set(); + const registeredPreToolUse = new Set(); + for (const slug of fs.readdirSync(path.join(ROOT, 'plugins'))) { + const hooksFile = path.join(ROOT, 'plugins', slug, 'hooks', 'hooks.json'); + if (!fs.existsSync(hooksFile)) continue; + const raw = fs.readFileSync(hooksFile, 'utf8'); + for (const m of raw.matchAll(/([\w./-]+\.ts)/g)) { + const script = m[1]!.split('/').pop()!; + registered.add(`${slug}:${script}`); + } + for (const entry of JSON.parse(raw)?.hooks?.PreToolUse ?? []) { + for (const hook of entry.hooks ?? []) { + // Both registration forms: `args: [".../gate.ts"]` and an inline + // `command: "bun .../gate.ts"` (core's hooks.json already uses the + // inline form elsewhere). Reading only `args` would let a gate + // registered the other way silently skip the corpus. + const candidates: string[] = [...(hook.args ?? []), ...String(hook.command ?? '').split(/\s+/)]; + const script = candidates.find(a => a.endsWith('.ts')); + if (script) registeredPreToolUse.add(`${slug}:${script.split('/').pop()}`); + } + } + } + const covered = new Set(SPECS.map(s => `${s.script.split('/')[1]}:${s.script.split('/').pop()}`)); + // A SPEC that no longer matches a real registration is stale or renamed. + for (const entry of covered) { + expect(registered.has(entry)).toBe(true); + } + // A registered PreToolUse gate missing from SPECS would silently skip the + // corpus. Other hook types aren't required here — dev/record-test-result is + // PostToolUse and is in SPECS by hand. + for (const entry of registeredPreToolUse) { + expect(covered.has(entry)).toBe(true); + } +});