diff --git a/docs/conventions/hook-budget/README.md b/docs/conventions/hook-budget/README.md index effc765cf0..ef7acb6d80 100644 --- a/docs/conventions/hook-budget/README.md +++ b/docs/conventions/hook-budget/README.md @@ -140,7 +140,15 @@ Every shipped hook row, except the shell-form rows named under "Scope", is exec - **What shipped.** Every row in `plugins/*/hooks/hooks.json` and in the skill-frontmatter hooks of `disk-hygiene:clean` and `repo-hygiene:clean`, except the shell-form rows named under "Scope", carries `args` and `"command": "node"`. Option gates - that used to be shell tests are launcher flags (`--require-true`, `--run-if-unset-or-true`). Two + that used to be shell tests are launcher flags (`--require-true`, `--run-if-unset-or-true`). + Two more flags skip the bash start on a row whose script would exit at once: + `--skip-if-all-false A,B,...` skips only when every named option is exactly `false` (the + guardrails verify rows, whose three verifiers each keep their own switch), and + `--skip-unless-stdin-contains TEXT` buffers stdin and skips a payload without TEXT (the testing + `Bash` row). `--skip-unless-stdin-contains` is advisory-only and must never gate a blocking + guard row: a stdin stall past the idle bound exits 0 without running the script, so a guard + behind it would fail open. The flag list is in the header of + [`lib/exec-bash.mjs`](../../../lib/exec-bash.mjs). Two scripts check the spelling. `scripts/check-exec-form-windows-probe.sh` rejects a `.sh` path, a `.cmd`/`.bat` shim, or bare `bash` as `command`; a non-Windows skip of its spawn half does not show that [anthropics/claude-code#90495](https://github.com/anthropics/claude-code/issues/90495) is @@ -160,7 +168,7 @@ Every shipped hook row, except the shell-form rows named under "Scope", is exec [`lib/exec-bash.mjs`](../../../lib/exec-bash.mjs)). - **Scope.** The `SessionStart` node-notice row of every hook plugin (the [prerequisites convention](../prerequisites/README.md#hook-notices)) and the `hook-failure-audit` Stop row in `harness-ops` stay shell form so they can report a missing `node`. Neither check script inspects a shell-form row; `scripts/node-notice-rows.test.sh` pins the node-notice - rows and the hook test of `harness-ops` pins its Stop row, so a sweep back to `node` fails them. A plugin hook config carries no + rows (and the matcher that skips them on compaction) and the hook test of `harness-ops` pins its Stop row, so a sweep back to `node` fails them. A plugin hook config carries no `${user_config.*}` token (the [philosophy Hooks row](../../plugin-philosophy.md#component-stances)), so no `userConfig` rule requires exec form and exec form fleet-wide is this sweep's choice. - **Measurement.** The reference figures above (Windows, 2026-07-31 and 2026-09-02) were taken before diff --git a/docs/conventions/prerequisites/README.md b/docs/conventions/prerequisites/README.md index 619ead0a38..5f2430f095 100644 --- a/docs/conventions/prerequisites/README.md +++ b/docs/conventions/prerequisites/README.md @@ -142,8 +142,16 @@ and `toolchain`) ships `check-prerequisites` and names that. No hook installs an | Notice | Fires | How | | --- | --- | --- | -| `node` is missing | `SessionStart`, once per session across every plugin | Each hook plugin carries one shell-form `SessionStart` row that runs `lib/prerequisites.sh node-notice`, then `lib/prerequisites.ps1 node-notice`. A POSIX shell (bash, or dash as `/bin/sh` on Debian and Ubuntu) takes the first and leaves at `${PPID:+exit}`, because every POSIX shell sets `PPID`; PowerShell, the default shell on Windows without Git Bash, has no `sh`, skips to the second. Both stubs share a latch keyed by session id in the temp directory, so a session with several hook plugins sees one notice. A plugin's `_enabled` kill switch, passed as the last argument, silences its row. | -| Another hook dependency is missing | `SessionStart` for an entry whose `for` names a hook, via `probe`; and at the point of use | `hook::require ` in `lib/hook-utils.sh`. It fails open: when `` is not on `PATH` it prints one skip notice per session and agent and exits 0. The text comes from the plugin's declared entry: `degrade`, the `docs` install link and `check`. An entry that names only a skill never notifies at session start. | +| `node` is missing | `SessionStart` except on compaction, once per session across every plugin | Each hook plugin carries one shell-form `SessionStart` row that runs `lib/prerequisites.sh node-notice`, then `lib/prerequisites.ps1 node-notice`. A POSIX shell (bash, or dash as `/bin/sh` on Debian and Ubuntu) takes the first and leaves at `${PPID:+exit}`, because every POSIX shell sets `PPID`; PowerShell, the default shell on Windows without Git Bash, has no `sh`, skips to the second. Both stubs share a latch keyed by session id in the temp directory, so a session with several hook plugins sees one notice. A plugin's `_enabled` kill switch, passed as the last argument, silences its row. | +| Another hook dependency is missing | `SessionStart` except on compaction, for an entry whose `for` names a hook, via `probe`; and at the point of use | `hook::require ` in `lib/hook-utils.sh`. It fails open: when `` is not on `PATH` it prints one skip notice per session and agent and exits 0. The text comes from the plugin's declared entry: `degrade`, the `docs` install link and `check`. An entry that names only a skill never notifies at session start. | + +The group holding the node-notice row and the group holding the `probe` row both carry +`"matcher": "startup|resume|clear|fork"`, every SessionStart source but `compact` (the source +list is the SessionStart matcher table in the +[hooks reference](https://code.claude.com/docs/en/hooks), as of 2026-10-04; recheck when that table +changes). A compaction keeps the session id (inferred from transcripts, which keep one sessionId across a compact boundary; not probed from a hook payload), and both latches with it, so a re-fire there prints +nothing and only starts processes. `clear`, `resume` and `fork` stay: each can begin a context +that has not seen the notice. `scripts/node-notice-rows.test.sh` pins the matcher on both rows. `hook::require` reads an entry with bash alone, from its `id` to the next one, so write `id` first in every entry. diff --git a/lib/exec-bash.gates.test.mjs b/lib/exec-bash.gates.test.mjs new file mode 100644 index 0000000000..efdd84812f --- /dev/null +++ b/lib/exec-bash.gates.test.mjs @@ -0,0 +1,221 @@ +// Launch flags of exec-bash.mjs that decide, before bash starts, whether a row +// has anything to do: --skip-if-all-false (option list) and +// --skip-unless-stdin-contains (payload text). The live cases run on every +// platform, Windows included. +import assert from "node:assert/strict"; +import { spawn, spawnSync } from "node:child_process"; +import { mkdtempSync, rmSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import path from "node:path"; +import process from "node:process"; +import { fileURLToPath, pathToFileURL } from "node:url"; +import { needsStdin, optionGateOpen, parseLaunchArgs, stdinGateOpen, stdinIdleMs } from "./exec-bash.mjs"; + +const VERIFY = "CLI_FLAG_VERIFY_ENABLED,SKILL_REFERENCE_VERIFY_ENABLED,STALE_PATH_VERIFY_ENABLED"; +const NAMES = VERIFY.split(","); +const env = (values) => + Object.fromEntries( + NAMES.flatMap((name, i) => (values[i] === undefined ? [] : [[`CLAUDE_PLUGIN_OPTION_${name}`, values[i]]])), + ); + +// --- parsing ----------------------------------------------------------------- +const verify = parseLaunchArgs(["--skip-if-all-false", VERIFY, "/hooks/run-guards.sh", "cli-flag-verify.sh"]); +assert.equal(verify.script, "/hooks/run-guards.sh"); +assert.deepEqual(verify.args, ["cli-flag-verify.sh"]); +assert.equal(needsStdin(verify.gates), false); + +for (const bad of ["", "A,,B", "A,", ",A", "cli_flag", "-A", "A B"]) { + const parsed = parseLaunchArgs(["--skip-if-all-false", bad, "x.sh"]); + assert.match(parsed.error ?? "", /^usage: --skip-if-all-false .*CLAUDE_PLUGIN_OPTION_/, JSON.stringify(bad)); +} +assert.match(parseLaunchArgs(["--skip-if-all-false"]).error, /^usage: --skip-if-all-false /); +assert.match(parseLaunchArgs(["--skip-unless-stdin-contains", ""]).error, /^usage: --skip-unless-stdin-contains /); +assert.match(parseLaunchArgs(["--skip-unless-stdin-contains", "x"]).error, /^usage: node exec-bash\.mjs /); + +// Flags combine in any order; the old gate keeps its meaning beside a new one. +const scan = parseLaunchArgs([ + "--require-true", + "TEST_GUARDS_ENABLED", + "--skip-unless-stdin-contains", + "bashEditDiff", + "/hooks/test-scan-bash.sh", +]); +assert.equal(scan.script, "/hooks/test-scan-bash.sh"); +assert.equal(needsStdin(scan.gates), true); +assert.equal(optionGateOpen(scan.gates, {}), false); +assert.equal(optionGateOpen(scan.gates, { CLAUDE_PLUGIN_OPTION_TEST_GUARDS_ENABLED: "true" }), true); + +// --- --skip-if-all-false: skips only when every switch is exactly "false" ----- +// Each verifier runs when its variable is unset, empty or "true", and exits at +// its switch otherwise; the gate is stricter still and runs on any value that is +// not literally "false", so a malformed value still starts bash. +assert.equal(optionGateOpen(verify.gates, env(["false", "false", "false"])), false); +for (const on of [undefined, "", "true", "TRUE", "0", " false", "False"]) { + for (let i = 0; i < 3; i += 1) { + const values = ["false", "false", "false"]; + values[i] = on; + assert.equal(optionGateOpen(verify.gates, env(values)), true, `${NAMES[i]}=${JSON.stringify(on)}`); + } +} +assert.equal(optionGateOpen(verify.gates, {}), true); + +// --- --skip-unless-stdin-contains ---------------------------------------------- +assert.equal(stdinGateOpen(scan.gates, Buffer.from('{"tool_response":{"bashEditDiff":{}}}')), true); +assert.equal(stdinGateOpen(scan.gates, Buffer.from('{"tool_response":{"stdout":"ok"}}')), false); +assert.equal(stdinGateOpen(scan.gates, Buffer.alloc(0)), false); + +// The idle bound reads stdin_read_timeout the way hook::resolve_read_timeout_to does. +assert.equal(stdinIdleMs({}), 2000); +assert.equal(stdinIdleMs({ CLAUDE_PLUGIN_OPTION_STDIN_READ_TIMEOUT: "0.5" }), 500); +assert.equal(stdinIdleMs({ CLAUDE_PLUGIN_OPTION_STDIN_READ_TIMEOUT: "5" }), 5000); +for (const bad of ["0", "0.0", "-1", "abc", "1e3", "", "0.000001"]) { + assert.equal(stdinIdleMs({ CLAUDE_PLUGIN_OPTION_STDIN_READ_TIMEOUT: bad }), 2000, bad); +} + +// --- live: a skipped row never resolves bash ---------------------------------- +// With an empty PATH on a stubbed win32 no bash resolves, so a launcher that got +// as far as resolving prints "no bash found" and exits 1; a skipped row exits 0 +// silently. +const launcher = fileURLToPath(new URL("./exec-bash.mjs", import.meta.url)); +const root = mkdtempSync(path.join(tmpdir(), "exec-bash-gates-")); +const stub = path.join(root, "win32-stub.mjs"); +writeFileSync(stub, 'Object.defineProperty(process, "platform", { value: "win32" });\n'); + +function runNoBash(args, extraEnv = {}, input = "") { + return spawnSync(process.execPath, ["--import", pathToFileURL(stub).href, launcher, ...args, "x.sh"], { + env: { SystemRoot: process.env.SystemRoot, PATH: "", ...extraEnv }, + input, + encoding: "utf8", + }); +} +function assertSkipped(run, label) { + assert.equal(run.status, 0, `${label}: ${run.stderr}`); + assert.equal(run.stderr, "", label); + assert.equal(run.stdout, "", label); +} +function assertReachedBash(run, label) { + assert.equal(run.status, 1, label); + assert.match(run.stderr, /no bash found/, label); +} + +const editMd = JSON.stringify({ hook_event_name: "PostToolUse", tool_name: "Edit", tool_input: { file_path: "a.md" } }); +assertSkipped(runNoBash(["--skip-if-all-false", VERIFY], env(["false", "false", "false"]), editMd), "all three off"); +assertReachedBash(runNoBash(["--skip-if-all-false", VERIFY], env(["false", "true", "false"]), editMd), "one on"); +assertReachedBash(runNoBash(["--skip-if-all-false", VERIFY], env(["false", "false"]), editMd), "one unset"); +assertReachedBash(runNoBash(["--skip-if-all-false", VERIFY], {}, editMd), "all unset"); + +// A malformed list is a launcher usage error: exit 1, nothing on stdout, one +// stderr line under 240 characters, and no hook runs. +for (const args of [["--skip-if-all-false", "a,b"], ["--skip-if-all-false", "A,,B"], ["--skip-unless-stdin-contains", ""]]) { + const run = runNoBash(args, {}, editMd); + assert.equal(run.status, 1, JSON.stringify(args)); + assert.equal(run.stdout, "", JSON.stringify(args)); + assert.equal(run.stderr.split("\n").length, 2, run.stderr); + assert.ok(run.stderr.length < 240, run.stderr); + assert.match(run.stderr, /^exec-bash: the launcher itself was called wrongly, so no hook ran: usage: --skip-/); +} + +const flag = ["--skip-unless-stdin-contains", "bashEditDiff"]; +assertSkipped(runNoBash(flag, {}, '{"tool_name":"Bash","tool_response":{"stdout":"x"}}'), "no diff"); +assertSkipped(runNoBash(flag, {}, ""), "empty stdin"); +assertReachedBash(runNoBash(flag, {}, '{"tool_response":{"bashEditDiff":{"changedFiles":[]}}}'), "diff"); + +// A closed option gate exits before stdin is read: stdin left open does not hold it. +function runOpen(args, extraEnv, { writeAfterMs = null, payload = "" } = {}) { + return new Promise((resolve) => { + const started = Date.now(); + const child = spawn(process.execPath, ["--import", pathToFileURL(stub).href, launcher, ...args], { + env: { SystemRoot: process.env.SystemRoot, PATH: "", ...extraEnv }, + }); + let stderr = ""; + child.stderr.on("data", (d) => { + stderr += d; + }); + child.stdin.on("error", () => {}); + if (writeAfterMs !== null) setTimeout(() => child.stdin.end(payload), writeAfterMs); + child.on("exit", (code) => { + child.stdin.destroy(); + resolve({ code, stderr, ms: Date.now() - started }); + }); + }); +} + +const gated = await runOpen(["--require-true", "TEST_GUARDS_ENABLED", ...flag, "x.sh"], { + CLAUDE_PLUGIN_OPTION_STDIN_READ_TIMEOUT: "30", +}); +assert.equal(gated.code, 0, gated.stderr); +assert.ok(gated.ms < 10000, `closed gate waited on stdin for ${gated.ms} ms`); + +// A stdin that stalls past the idle bound exits 0 without running the script. +const stalled = await runOpen([...flag, "x.sh"], { CLAUDE_PLUGIN_OPTION_STDIN_READ_TIMEOUT: "0.3" }); +assert.equal(stalled.code, 0, stalled.stderr); +assert.equal(stalled.stderr, ""); + +// A payload that arrives inside the idle bound is still read. +const late = await runOpen([...flag, "x.sh"], { CLAUDE_PLUGIN_OPTION_STDIN_READ_TIMEOUT: "5" }, { + writeAfterMs: 300, + payload: '{"bashEditDiff":{}}', +}); +assertReachedBash({ status: late.code, stderr: late.stderr }, "payload inside the bound"); + +// --- live with bash: the script gets the payload byte for byte ----------------- +const echoScript = path.join(root, "echo-stdin.sh"); +writeFileSync(echoScript, "cat\n"); +function runBash(args, input, extraEnv = {}) { + return spawnSync(process.execPath, [launcher, ...args, echoScript], { + env: { ...process.env, ...extraEnv }, + input, + }); +} +const big = Buffer.from(`{"tool_response":{"bashEditDiff":{"x":"${"y".repeat(200 * 1024)}"}}}`); +const unicode = Buffer.from('{"tool_response":{"bashEditDiff":{"path":"tëst-ü-日本.test.ts"}}}'); +const crlf = Buffer.from('{"tool_response":\r\n{"bashEditDiff":{"a":"b\r\n"}}}\r\n'); +for (const [label, payload] of [ + ["over 64 KB", big], + ["non-ASCII", unicode], + ["CRLF", crlf], +]) { + const run = runBash(flag, payload); + assert.equal(run.status, 0, `${label}: ${run.stderr}`); + assert.ok(Buffer.compare(run.stdout, payload) === 0, `${label}: payload changed on the way to the script`); +} + +// A payload dripped in chunks with gaps inside the idle bound arrives whole. +function dripRun(args, chunks, gapMs, extraEnv) { + return new Promise((resolve) => { + const child = spawn(process.execPath, [launcher, ...args, echoScript], { env: { ...process.env, ...extraEnv } }); + const out = []; + child.stdout.on("data", (d) => out.push(d)); + child.stdin.on("error", () => {}); + let i = 0; + const next = () => { + if (i === chunks.length) return child.stdin.end(); + child.stdin.write(chunks[i]); + i += 1; + setTimeout(next, gapMs); + }; + next(); + child.on("close", (code) => resolve({ code, stdout: Buffer.concat(out) })); + }); +} +const drip = ['{"tool_response":', '{"bashEd', 'itDiff":{', '"a":1}}}']; +const dripped = await dripRun(flag, drip, 150, { CLAUDE_PLUGIN_OPTION_STDIN_READ_TIMEOUT: "1" }); +assert.equal(dripped.code, 0); +assert.equal(dripped.stdout.toString(), drip.join("")); + +// A row without a stdin flag keeps stdin inherited: a payload that starts later +// than the stdin flag's idle bound still reaches the script. +const delayed = await new Promise((resolve) => { + const child = spawn(process.execPath, [launcher, echoScript], { + env: { ...process.env, CLAUDE_PLUGIN_OPTION_STDIN_READ_TIMEOUT: "0.2" }, + }); + const out = []; + child.stdout.on("data", (d) => out.push(d)); + setTimeout(() => child.stdin.end("late payload"), 800); + child.on("close", (code) => resolve({ code, stdout: Buffer.concat(out).toString() })); +}); +assert.equal(delayed.code, 0); +assert.equal(delayed.stdout, "late payload"); + +rmSync(root, { recursive: true, force: true }); +console.log("exec-bash gates: option list, stdin predicate, stall, and byte-for-byte delivery passed."); diff --git a/lib/exec-bash.gates.test.sh b/lib/exec-bash.gates.test.sh new file mode 100755 index 0000000000..d62910b212 --- /dev/null +++ b/lib/exec-bash.gates.test.sh @@ -0,0 +1,6 @@ +#!/usr/bin/env bash +# Owns lib/exec-bash.gates.test.mjs for the plugin test lane. +# scripts/run-outside-node-suites.sh treats a sibling .test.sh as the runner. +set -euo pipefail +cd "$(dirname "${BASH_SOURCE[0]}")" +exec node exec-bash.gates.test.mjs diff --git a/lib/exec-bash.mjs b/lib/exec-bash.mjs index f51cd990ab..522973eaec 100755 --- a/lib/exec-bash.mjs +++ b/lib/exec-bash.mjs @@ -1,16 +1,37 @@ #!/usr/bin/env node // Exec-form entry for a bash-scripted hook (#3686). // -// hooks.json spells `"command": "node"` and puts this file, then an -// optional option gate, then the shell script and that script's own -// arguments, in `args`. `--require-true NAME` exits 0 unless -// CLAUDE_PLUGIN_OPTION_NAME is exactly `true`. `--run-if-unset-or-true NAME` -// exits 0 only when that variable is set to something other than `true`. -// A closed gate does not resolve or spawn bash. Bare `bash` -// is not a legal exec-form command: on Windows it resolves to the WSL -// relay and a failed hook launch does not block. This process finds a -// real bash and spawns it with the script path as argv, stdin inherited, -// and the child's exit code forwarded. `shell` is not used. +// hooks.json spells `"command": "node"` and puts this file, then optional +// launch flags, then the shell script and that script's own arguments, in +// `args`. Each flag takes one value and sits before the script. Every flag +// must pass (AND); a failed one exits 0 without resolving or spawning bash. +// +// --require-true NAME run only when CLAUDE_PLUGIN_OPTION_NAME is +// exactly `true` (a default-off option). +// --run-if-unset-or-true NAME run unless that variable is set to something +// other than `true` (a default-on option). +// --skip-if-all-false A,B,... skip only when every named variable is +// exactly `false`. Unset, empty, `true` or any +// other value runs, so a row whose script holds +// several default-on switches skips only when +// all of them are provably off. +// --skip-unless-stdin-contains TEXT +// buffer stdin; skip when the payload lacks +// TEXT, else write the same bytes to the script. +// +// Option flags are decided first, before stdin is touched. A stdin flag reads +// stdin to EOF with an idle bound (2 s, or the stdin_read_timeout option read +// the way hook-utils.sh reads it), and a stall exits 0 without running the +// script. That fails OPEN, so a stdin flag is for advisory rows only: the +// shared library treats a stalled payload as fail-closed for a blocking guard, +// which must not use the flag as written. A row without a stdin flag keeps +// stdin inherited. +// +// Bare `bash` is not a legal exec-form command: on Windows it resolves to the +// WSL relay and a failed hook launch does not block. This process finds a +// real bash and spawns it with the script path as argv, stdin inherited (or +// written from the buffer), and the child's exit code forwarded. `shell` is +// not used. // // On Windows the candidates, in order, are CLAUDE_CODE_GIT_BASH_PATH // (accepted only when the file is named bash.exe, sh.exe, bash, or sh), @@ -67,41 +88,132 @@ function firstExisting(candidates, platform, exists) { return null; } +const OPTION_NAME = /^[A-Z0-9_]+$/; +const ONE_NAME = "needs the CLAUDE_PLUGIN_OPTION_ suffix (A-Z, digits, underscore)"; +const NAME_LIST = "needs a comma-separated list of CLAUDE_PLUGIN_OPTION_ suffixes (A-Z, digits, underscore)"; + +function oneName(value) { + return value && !value.startsWith("-") && OPTION_NAME.test(value) ? [value] : null; +} + +function nameList(value) { + if (!value || value.startsWith("-")) return null; + const names = value.split(","); + return names.every((name) => OPTION_NAME.test(name)) ? names : null; +} + +function literal(value) { + return value ? value : null; +} + +const optionValues = (names, env) => names.map((name) => env[`CLAUDE_PLUGIN_OPTION_${name}`]); +const unsetOrTrue = (v) => v === undefined || v === "" || v === "true"; + +// The launch flags. An `env` flag reads only CLAUDE_PLUGIN_OPTION_* values and +// is decided before stdin is touched; a `stdin` flag reads the buffered +// payload. `parse` returns the flag's value or null for a usage error; `open` +// says whether the script runs. +const FLAGS = { + "--require-true": { + phase: "env", + parse: oneName, + problem: ONE_NAME, + open: (names, env) => optionValues(names, env)[0] === "true", + }, + "--run-if-unset-or-true": { + phase: "env", + parse: oneName, + problem: ONE_NAME, + open: (names, env) => unsetOrTrue(optionValues(names, env)[0]), + }, + "--skip-if-all-false": { + phase: "env", + parse: nameList, + problem: NAME_LIST, + open: (names, env) => !optionValues(names, env).every((v) => v === "false"), + }, + // Advisory rows only: never put this on a blocking guard row. A stdin stall + // exits 0 without running the script, so a guard behind it would fail open. + "--skip-unless-stdin-contains": { + phase: "stdin", + parse: literal, + problem: "needs non-empty text to look for in stdin", + open: (text, input) => input.includes(text), + }, +}; + +const USAGE = `usage: node exec-bash.mjs [${Object.keys(FLAGS) + .map((flag) => `${flag} V`) + .join(" | ")}]...