refactor(guardrails): buffer_stdin stdin reads; blocking guards fail closed on timeout

[kyle-sexton]

Summary

Second half of the #313 item-3 stdin migration (sibling: #443, the 8 advisory plugins). All seven guardrails entry hooks now read stdin via the shared hook::buffer_stdin helper (bounded read -t, default 2s) instead of a bare cat, so the Windows Win32-pipe late-EOF stall can no longer hang a hook — and with it every tool call — indefinitely.

The load-bearing decision — blocking guards fail closed on timeout. hook::buffer_stdin distinguishes rc 1 (empty/incomplete stdin) from rc 2 (read timed out before a complete JSON payload). The audit-hook idiom collapses both to a skip; for a security guard that would fail OPEN — a timed-out read means the guard could not evaluate the command, and skipping would pass exactly the traffic it exists to stop (dangerous git, hook bypass, --no-verify, secrets, hardcoded paths). Instead:

• 5 blocking guards (block-dangerous-git, block-hook-bypass, block-no-verify, secret-pattern-detection, hardcoded-path-check): rc 2 → exit 2 (block; buffer_stdin already printed the BLOCKED: reason to stderr); rc 1 → exit 0 (skip, matching the previous empty-payload behavior — these guards already skipped on empty COMMAND/fields after the bare cat).
• 2 advisory hooks (flag-commit-pr-skill-bypass, workflow-resilience-check): any read failure → skip, as before.

guardrails 0.8.0 (behavior change on the timeout path; the previous behavior was an indefinite hang, not a skip).

Verification

• shellcheck (--rcfile=.shellcheckrc) + shfmt -d clean on all 7 hooks
• All 7 guardrails test suites green (194/37/75/44/28/19/10 passes)
• markdownlint clean on the CHANGELOG

Related

Epic #313 (deferred-backlog item 3, guardrails half). Sibling PR #443. Also relates to #317, #323.

No linked issue: incremental epic work; closes nothing.

🤖 Generated with Claude Code

[claude]

Claude finished @kyle-sexton's task in 4m 23s —— View job

---

PR Review: refactor(guardrails): buffer_stdin stdin reads; blocking guards fail closed on timeout

• Gather context and read PR diff
• Review plugin.json version bump
• Review blocking guard hooks (5 hooks)
• Review advisory hooks (2 hooks)
• Review CHANGELOG
• Post findings

---

Overall Assessment

The core change is correct and well-reasoned: swapping bare cat for hook::buffer_stdin in all 7 hook entry points fixes the Win32-pipe stall, and the fail-closed decision for blocking guards is the only defensible choice. The shfmt reformatting in block-dangerous-git.sh is behavior-neutral. Two findings worth addressing before merge, one informational.

---

Finding 1 — Medium: stdin_read_timeout not declared in plugin.json userConfig

hook-utils.sh:241

local read_timeout="${CLAUDE_PLUGIN_OPTION_STDIN_READ_TIMEOUT:-2}"

hook::buffer_stdin exposes a configurable timeout via CLAUDE_PLUGIN_OPTION_STDIN_READ_TIMEOUT. This is now the key safety knob: on a slow host, a 2s timeout that expires before the full JSON payload arrives will cause all five blocking guards to block the tool call — a spurious hard block that the agent can't recover from.

But stdin_read_timeout is not declared in plugin.json userConfig, so consumers cannot set it via claude plugin configure guardrails or the --config flag at install time. The README also has no mention of it. This leaves them with no documented escape valve when the default 2s is too short for their environment.

Suggested fix: Add a stdin_read_timeout entry to userConfig:

"stdin_read_timeout": {
  "type": "number",
  "title": "stdin read timeout (seconds)",
  "description": "Maximum seconds to wait for a complete hook JSON payload on stdin. Blocking guards fail closed if this elapses before a complete payload arrives. Increase on slow systems. Default: 2.",
  "default": 2
}

And document it in the README alongside the per-hook kill switches. Fix this →

---

Finding 2 — Low: No tests for the fail-closed (rc=2) or empty-stdin (rc=1) paths

The test suite's run helper always pipes a fully-formed JSON payload:

env "$@" bash "$HOOK" <<<"$(command_json "$command")" >/dev/null 2>&1

There are no test cases that:

• Pipe empty stdin (</dev/null or : | bash "$HOOK") to verify the five blocking guards exit 0 (skip)
• Simulate a timed-out read to verify they exit 2 (block)

The core behavioral change in this PR — fail closed on rc=2, skip on rc=1 — is never exercised by the test suite. Given that this is the "load-bearing decision" the PR body calls out, it should have at least a contract test. A simple approach for each blocking guard:

# rc=1: empty stdin → skip
bash "$HOOK" </dev/null >/dev/null 2>&1
assert_exit "empty stdin: skip" 0 $?

# rc=2: synthesize a timed-out read by setting a 0s timeout and slow pipe
# (or by mocking buffer_stdin to return 2)

Fix this →

---

Informational: cli-flag-verify.sh still reads stdin via jq directly

cli-flag-verify.sh calls hook::read_file_path(), which invokes jq directly from fd0 — it never passed through cat and is not in this PR's scope. However, jq can also stall on a Windows Win32-pipe late-EOF. Since this hook is PostToolUse and advisory (never exits 2), a stall there only delays, not blocks. Tracking it as a remaining gap but no action needed in this PR.

---

Everything else looks good

• Fail-closed logic is correct. The || { rc=$?; ((rc == 2)) && exit 2; exit 0; } idiom correctly captures buffer_stdin's exit code. $? inside the || block is the exit code of the failing command. set -u is not a concern here since rc is always set before it's read.
• Advisory hooks correctly collapse rc=1 and rc=2 to skip with || exit 0 — matching the stated design.
• shfmt operator-at-EOL reformatting in block-dangerous-git.sh is behavior-neutral and correct.
• CHANGELOG at 0.8.0 is accurate. Version bump to 0.8.0 (minor) is justified by the behavior change on the timeout path.
• jq-absent path is safe. All five blocking guards exit 0 before calling buffer_stdin when jq is missing, so the buffer_stdin jq-validation branch is never reached in that case.