fix(hook-utils): read hook stdin in chunks so a large payload is not blocked

[kyle-sexton]

Closes #1563

Summary

hook::buffer_stdin read the hook payload with IFS= read -r -d '' -t "$read_timeout". On a pipe
— which is exactly how Claude Code delivers hook stdin — bash consumes read -d '' one byte at a
time, measured at roughly 32 KB/s on Git Bash. The stdin_read_timeout bound (default 2s) was
therefore a ~64 KB throughput ceiling, not the stall detector it was written to be.

Past that ceiling the read returned a truncated payload and rc 2:

• Seven fail-closed guardrails guards — hardcoded-path-check, secret-pattern-detection,
  block-no-verify, block-dangerous-git, block-hook-bypass, block-noncanonical-commit,
  block-convention-violation — map rc 2 to exit 2, so a legitimate large write was blocked,
  with its content never scanned. Reported from the field: a full-file Write of an 844-line
  (~50 KB) document blocked repeatedly, forcing the author to write it in five chunks.
• Every other caller uses || exit 0, so on a large payload the formatter / audit / advisory
  hook silently did not run at all, with no diagnostic.

Reproduction (before the fix)

hook::buffer_stdin in isolation, file redirect vs. pipe:

payload	file redirect	pipe
1 KB	176 ms	221 ms
10 KB	240 ms	461 ms
50 KB	428 ms	1564 ms — 78% of the bound
100 KB	666 ms	rc 2 — BLOCKED
200 KB	1396 ms	rc 2 — BLOCKED

End-to-end, a benign Write payload (nothing in the content is a violation) piped into the real
plugins/guardrails/hooks/hardcoded-path-check.sh with CLAUDE_PROJECT_DIR set:

payload=20504    rc=0  elapsed_ms=1810
payload=50939    rc=0  elapsed_ms=2857
payload=101664   rc=2  elapsed_ms=2853   BLOCKED: hook stdin timed out before a complete JSON payload arrived.
payload=203113   rc=2  elapsed_ms=3006   BLOCKED: hook stdin timed out before a complete JSON payload arrived.

The block is purely a function of payload size. A 50 KB source file JSON-escapes to well over 50 KB
of payload, which is why the 50 KB field report and this 100 KB reproduction are the same defect —
the threshold sits in the 50–100 KB range and moves with machine load.

Fix

Two changes together, because either alone leaves the bound meaning the wrong thing.

1. Chunk the read. read -N 65536 lets bash satisfy the read in blocks rather than
byte-at-a-time. Measured: 50 KB drops from ~2100 ms to ~20 ms, 200 KB from ~6800 ms to
~85 ms. All four end-to-end payloads up to 200 KB now return rc 0.

2. Measure inactivity, not the read. read -t is a deadline for the whole requested read, not
an inactivity timer, so a producer making steady progress but slower than one chunk per window would
still trip it. read assigns whatever it received even when it times out, so any byte counts as
progress: the partial chunk is kept and the read continues. Only the absence of bytes for a whole
stdin_read_timeout is a stall.

2b. Read the bound in four slices. read -t reports only that its window expired, never when
inside it the last byte arrived — so armed as one window, a stall is declared anywhere between one
and two bounds after the pipe actually went quiet. Slicing caps that overshoot at a quarter-bound.
That residual quarter is the honest limit of the mechanism, and it errs toward waiting, never toward
declaring a live producer dead. Measured back to back at a 1.2 s bound: a partial-then-silent producer
is declared stalled at 2012 ms sliced vs 2728 ms unsliced. A shell whose read -t rejects the
fractional slice degrades to a count of 1 — exactly the unsliced behavior — rather than failing.

3. Stop reading once the buffer is already whole. Otherwise the Win32 late-EOF case — payload
complete, pipe simply never closed — waits out the rest of the bound for an EOF that is not coming.
The loop now checks completeness before continuing. Some wait is the floor, since a held-open pipe is
indistinguishable from a slow producer until a window expires. Measured at a 1.2 s bound: 719 ms
with the early stop vs 1909 ms without.

The stall verdict now comes from read's own exit status — EOF returns 1, an exceeded -t returns

128 — instead of elapsed-time arithmetic, which deletes the EPOCHREALTIME/awk heuristic and two
awk subprocesses per hook invocation.

The trade this makes, stated in the comment: a producer trickling bytes indefinitely is never cut off
here. That is deliberate — Claude Code's own default timeout for a command hook is 600 seconds
(hooks reference, fetched this session), so the harness is
the outer bound, and blocking a live producer is exactly the failure this function had.

4. Validate the configured timeout instead of passing it straight to read. stdin_read_timeout
is consumer-configurable and reached read -t directly. An unusable spec is a silent disable:
read rejects it with rc 1, the loop reads rc 1 as EOF, the payload comes back empty, every caller
skips, and a usage error prints on every hook invocation. Worse, stdin_read_timeout=0 made the
chunked loop spin — read -t 0 returns success having consumed nothing. hook::resolve_read_timeout
now falls back to the default, deciding acceptance by probing the running shell rather than a
Bash version table (the upstream changelog does not date the introduction of fractional timeouts, so
a version check would be a guess). Loop termination is additionally made structural: a successful
read that consumed nothing breaks rather than continues.

5. Never feed a hook payload to jq through a here-string. jq -e . <<< "$buf" delivers the
string through a pipe that bash fills itself before exec'ing jq, so a payload at or above the
pipe capacity — 65536 bytes here, exactly one read chunk — blocks the shell forever. Traced:
65536 hung indefinitely, 65000 returned immediately. Every such call now goes through printf | jq:
hook::json_complete, buffer_stdin's final check, hook::jq_field, and claude-ops'
skill-usage-expansion-audit. hook::jq_field is the load-bearing one — it takes the whole buffered
payload and is called by most hooks in the fleet, and the old throughput ceiling is precisely what
kept payloads that large from ever reaching it. Removing the ceiling made this deadlock reachable, so
this PR had to fix it.

Bash 3.2. read -N is Bash 4.1+, and nine plugin READMEs document Bash 3.2+ support (macOS
system bash), so the pre-4.1 path falls back to the delimiter read inside the same re-arming loop —
the same guard and rationale as plugins/context-guard/scripts/statusline-tee.sh. The guard is split
into its own predicate purely so that path stays reachable in tests: BASH_VERSINFO is readonly and
cannot be shadowed. It is not a consumer seam; nothing reads it from the environment.

The fail-closed posture is unchanged

This was the constraint the change had to respect — a guard that silently permits an unscanned write
is strictly worse than one that is annoying. Verified three ways against the real hook:

1. small + violation      : rc=2  Hardcoded machine-specific path(s) in …/docs/probe.md:
2. 200KB + trailing viol. : rc=2  Hardcoded machine-specific path(s) in …/docs/probe.md:
3. stalled pipe           : rc=2  BLOCKED: hook stdin timed out before a complete JSON payload arrived.

Case 2 is the improvement that matters most: a violation sitting at the very end of a 200 KB
payload is now genuinely caught, where before the whole write was swept into a content-blind
block. Case 3 is the posture proof — a truncated payload on a pipe held open still exits 2.

The jq completeness check is kept as the backstop, so the Win32 late-EOF case this function
exists for — a complete payload on a pipe that never closes — still succeeds rather than blocking.

Tests

lib/hook-utils.test.sh gains five cases alongside the existing timeout test:

• 18b — a complete JSON payload on a pipe held open past the timeout returns rc 0 with the
  payload (the Win32 late-EOF contract), and settles in one window rather than two. That second
  assertion is by comparison, not against a wall-clock constant: it times the real function and a
  variant with the completeness predicate overridden to always fail (reproducing the pre-fix
  behavior) back to back on the same host, so runner load cancels out. It fails loudly if the harness
  returns no measurement on a host that has EPOCHREALTIME.

• 18c — a 256 KB payload on a real pipe at the default timeout returns rc 0 with the content
  intact, asserted on content rather than wall-clock so a loaded runner cannot flake it. If the
  byte-at-a-time read ever returns, this fails outright.

• 18d — a payload trickled one character per 100 ms against a 300 ms timeout (far too slow to
  fill a chunk in any window) returns rc 0 with the whole payload. This is the idle-bound regression
  test; against the first commit on this branch it returned rc 2.

• 18d' — that same trickle then going silent mid-payload still returns rc 2, so re-arming on
  progress did not quietly become "never time out".

• 18e — first asserts the guard override actually flips to the pre-4.1 branch (otherwise the
  cases would be vacuous — the first attempt at this test shadowed BASH_VERSINFO, which is
  readonly, and silently tested the modern path), then that the delimiter-read fallback buffers a
  128 KB payload and still fails closed on a stalled pipe.

• 18g — a stall is declared near one bound, not two, asserted against a variant with the
  slice count forced to 1, and gated on a precondition check that the override actually engages.

• 18f — five unusable stdin_read_timeout values (abc, 0, -1, 1e3, empty) must each fall
  back to the default, still deliver the payload, and emit nothing on stderr; plus a positive case
  proving a valid non-default value is still honored rather than collapsed. The 0 case doubles as
  the loop-termination test — before the guard it hung, so a regression shows up as the suite never
  finishing.

Existing Test 18 (stalled pipe → rc 2 + BLOCKED:) is unchanged and still passes.

• 18h — a 65536-byte held-open payload (exactly one read chunk) costs no more than a
  non-boundary one, preceded by an assertion that the fixtures are exactly 65536 and 65000 bytes.
  This is also the here-string-deadlock regression test: before the fix it hung outright.

Local runs: lib/hook-utils.test.sh 114/114 (66 s), hardcoded-path-check.test.sh 72/72,
secret-pattern-detection.test.sh 42/42, markdown-format.test.sh 112/112,
typos-format.test.sh 75/75, block-hook-bypass.test.sh 203/203,
stale-path-verify.test.sh 87/87, lane-stop-gate.test.sh 26/26. shellcheck clean,
check-shell-portability.sh clean.
sync-hook-utils.sh --check / --check-bump, check-changelog-parity.sh --check / --check-bump
all pass.

Not run: a real Bash 3.2. No 3.2 host was available here, so the fallback is verified by forcing
the branch on a modern bash, not by executing under 3.2 itself.

Four tests on this branch were silently vacuous before being caught — one shadowed the readonly
BASH_VERSINFO and tested the modern path while claiming to test the 3.2 fallback; one bracketed a
whole pipeline and measured the producer's sleep; one had a missing ; in its override string, so
it exercised the unmodified function; one generated a 59578-byte "65536-byte" fixture because it
stripped newlines after truncating. Every timing, branch-forcing, and exact-size case now asserts its
own precondition first, so a broken harness fails rather than passes.

Portability lint

Two portability-ok: annotations were added for pre-existing false positives that only surfaced
because this change puts all 14 synced copies into the changed set: a bash glob bracket class
[\<\>] matching literal </>, and a literal a\b path fixture — neither is a GNU \</\b
regex construct. Annotated with the gate's own documented hatch rather than touching the synced
linter.

Secondary: stdin_read_timeout in guardrails' userConfig

Guardrails' hooks already read CLAUDE_PLUGIN_OPTION_STDIN_READ_TIMEOUT through the shared library
but never declared the option, so a consumer had no supported way to set it. actionlint and
claude-ops both declare it, and actionlint's setup skill records the convention explicitly
("hook plugins reusing the shared lib should declare it too"). Declaring it exposes the same knob
here, documented in the guardrails README.

Deliberately not claimed: that the harness materializes the default into the env var. The
plugins reference describes default as "value used when the user provides nothing" and says all
values "are exported to hook processes as CLAUDE_PLUGIN_OPTION_<KEY> environment variables"
(plugins-reference, fetched this session), but
that specific default-materialization path is not something this PR verified end-to-end. The
effective default with nothing configured remains the shell-level :-2 fallback inside
hook-utils.sh, which is code-verifiable here.

Scope

lib/hook-utils.sh is synced into all 14 carrying plugins by scripts/sync-hook-utils.sh; the
coupled sync-hook-utils / changelog-parity CI gates require every carrying plugin to bump and add
a changelog entry, so all 14 are bumped (guardrails to a minor, 0.18.0, for the new userConfig
option; the rest patch).

The supported Bash floor is unchanged at 3.2. (An earlier revision of this PR claimed the library
already required 4.0+ because of ${var^} in hook::normalize_path — that was wrong: those case
operators sit behind an OSTYPE msys/cygwin branch that never executes on macOS. The 4.1-only
read -N is now guarded rather than unconditional.)

Related

• Refs docs: migrate remaining docs.claude.com URLs to code.claude.com #1564 / docs: migrate remaining docs.claude.com URLs to code.claude.com #1567 — the other half of the same handoff item (docs domain migration), landed
  separately. This branch is rebased on top of it, which is why claude-ops lands on 0.21.6.
• Explicitly not related to the previously-closed hardcoded-path-check fail-open scope
  item: this is the contrasting fail-closed observation, not confirmation of that one.

Deliberately out of scope

hook::buffer_stdin returns 1 (skip) when stdin hits EOF with a truncated payload — a genuinely
short write rather than a stall. Arguably that should fail closed too, but it has not been observed
firing and changing it is a separate behavior change. Left as-is.

Worth knowing when reading the new control flow: a truncated payload that goes quiet and then
closes reaches that EOF branch (rc 1) rather than the stall branch, if the close lands before the
silence exceeds one full timeout window. The stall branch requires a window that delivers nothing —
which is the Win32 late-EOF shape this guard is actually aimed at, and what Test 18 and the
end-to-end stall case both exercise.

[claude]

Claude finished @kyle-sexton's task in 6m 25s —— View job

---

PR Review: fix(hook-utils): read hook stdin in chunks so a large payload is not blocked

Tasks

• Gather PR context and changed files
• Read core change: lib/hook-utils.sh
• Read tests: lib/hook-utils.test.sh
• Review guardrails plugin changes (plugin.json, README.md, CHANGELOG.md)
• Spot-check synced copies for consistency
• Review for correctness, security, and alignment with CLAUDE.md
• Post review findings

---

Summary

Solid fix for a real production defect. The root cause diagnosis is correct (read -d '' is byte-at-a-time on pipes), the repair (read -N with an idle-armed timeout) is the right approach, and the fail-closed posture is preserved for stalled pipes. Fail-closed coverage is verified against three distinct scenarios in the PR description. The three new tests cover the key contracts cleanly. All 14 synced plugins are bumped consistently, and guardrails appropriately gets a minor version bump for the new userConfig entry.

No blocking findings. Three observations below.

---

🟡 Nit — jq is now unconditionally called on every invocation; the PR description understates this

File: lib/hook-utils.sh L277–278

if command -v jq >/dev/null 2>&1; then
    jq -e . >/dev/null 2>&1 <<<"$input" || jq_rc=$?
fi

In the old code, the jq call sat inside if [[ "$read_status" -ne 0 ]] && command -v jq ... — it only ran when the read had already timed out. Now it runs on every invocation. Two effects worth calling out:

Performance: the old code spawned zero post-read subprocesses on a fast, clean-EOF read (awk only fired when both the read timed out AND jq had already returned non-zero, so on the happy path the cost was the single printf|tr CR strip). The new code spawns jq on every call. This contradicts the PR description's claim of "dropping two awk subprocesses per hook invocation" — awk was gated on read_status -ne 0 && jq_rc -ne 0, not unconditional.

Behavior: a corrupt or truncated payload arriving on a clean-closing pipe (EOF, not stall) now returns 1 (skip) instead of being passed through to the caller, which is arguably better but is a silent change the PR describes as "Left as-is" — that "left as-is" appears to refer only to keeping the return code at 1 (not bumping it to 2), but the gating of jq validation changed. Worth a comment or a note in the changelog so the intent is clear.

---

🟡 Nit — empty stalled pipe returns 1 (skip) rather than 2 (block); not explicitly called out

File: lib/hook-utils.sh L275

[[ -n "$input" ]] || return 1

If the pipe stalls before delivering any data, stalled=1 but input="", and this guard returns 1 (skip) before the stall path is ever checked. The PR calls the similar truncated-clean-EOF case "deliberately out of scope," but this specific sub-case — zero bytes on a stalled pipe → skip rather than block — is not mentioned. The pr's "fail-closed posture is unchanged" claim holds for payloads that begin arriving and then stall (tested in Test 18), but a cold-stalled empty pipe is treated as silent stdin rather than a failed probe. For an adversarial scenario where a hooker replaces the payload delivery with silence to suppress scanning, skip is weaker than block. Given existing callers map skip to exit 0, this is the one edge that produces no block.

---

🟡 Nit — IFS= is redundant with read -N

File: lib/hook-utils.sh L264

IFS= read -r -t "$read_timeout" -N 65536 chunk

The bash manual states that -N does not use IFS to split input. IFS= was meaningful for the old read -d '' (prevented word-splitting on the accumulated assignment), but has no effect here. It is harmless but could mislead a future maintainer into thinking IFS matters for -N-mode reads. A brief comment or simply dropping IFS= would clarify intent.

---

Everything else checks out

• read -N vs read -d '' semantics: correct — -N reads block-mode, -r still suppresses backslash-newline line continuation (meaningful for JSON with embedded backslashes), timeout returns >128 while EOF returns 1. The ((read_rc > 128)) stall gate is the right test.
• Exact-multiple-of-65536 payloads: handled — the final empty read returns 1 (EOF with 0 bytes read) and sets stalled=0, so jq validates the complete accumulation and returns it cleanly.
• Win32 late-EOF path: preserved — stall fires on the last chunk (pipe stays open after full payload), stalled=1, jq validates complete JSON, jq_rc=0 → falls through to printf '%s' "$input". Test 18b covers this contract explicitly.
• Fail-closed posture for stalled-with-partial-JSON: intact. stalled=1 && jq_rc != 0 → return 2 + BLOCKED. Test 18 unchanged and passing.
• Version bumps: guardrails → 0.18.0 (minor, new userConfig option) correct; all 13 others → patch bump, consistent with a library-only sync. Sync gate (--check-bump) passes.
• stdin_read_timeout userConfig in guardrails: declaration is accurate. "min": 1 matches the convention established by actionlint and claude-ops. The description correctly distinguishes this plugin's fail-closed stall behavior from actionlint's fail-open description. The shell-level :-2 fallback is unambiguously the effective default when nothing is configured, per the PR note.
• CLAUDE.md / plugin-form rules: no hardcoded paths, ${CLAUDE_PLUGIN_OPTION_*} used for config, no PII or secrets, plugin.json versioned.
• Security (REVIEW.md always-check criteria): no auth/object-level checks, no tenant data paths, no secrets in source, no string-concatenated process invocations built from external input, no missing audit entries. None of the blocking criteria apply to this change.

[claude]

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

---

Security Review

• Gather context and understand the request
• Fetch PR diff
• Read core changed files (lib/hook-utils.sh, lib/hook-utils.test.sh, plugins/guardrails/...)
• Analyze for security vulnerabilities (logic, injection, trust boundaries, access control)
• Post findings

---

Scope: hook::buffer_stdin rewrite in lib/hook-utils.sh (and its 14 synced copies), plus the guardrails userConfig addition. No workflow files changed.

No critical or important security issues found. One suggestion-level finding below.

---

SUGGESTION — CLAUDE_PLUGIN_OPTION_STDIN_READ_TIMEOUT is untrusted input with no runtime validation | Confidence: medium

Severity: SUGGESTION
Confidence: medium — requires env-layer access, which implies broader compromise; practical risk is low

Location: lib/hook-utils.sh line 260–264

local read_timeout="${CLAUDE_PLUGIN_OPTION_STDIN_READ_TIMEOUT:-2}"
...
IFS= read -r -t "$read_timeout" -N 65536 chunk || read_rc=$?

The plugin.json declares "min": 1 on stdin_read_timeout, but that constraint is UI-only — it is not enforced at the shell level. If CLAUDE_PLUGIN_OPTION_STDIN_READ_TIMEOUT is set directly in the environment (bypassing the plugin config picker) to a value that read -t rejects:

• Non-numeric (e.g., "foo"): bash emits read: invalid timeout specification to stderr and read returns rc=1 (not >128). stalled stays 0. The function then falls through to the jq completeness check. With a normal payload this is fine; with an incomplete payload it returns 1 (skip) instead of 2 (block). Fail-closed callers silently skip rather than block.
• Zero ("0"): read -t 0 -N 65536 polls without blocking and returns immediately (rc>128 or rc=1 depending on bash version/data available). Either stalled=1 with empty input → return 1, or stalled=0 with empty input → return 1. Advisory and blocking guards both silently skip rather than block.

Practical impact is low — exploiting this requires controlling the process environment, which implies the ability to disable the hooks entirely. The min: 1 UI constraint provides the right guardrail for the supported configuration path. But since the security promise of the blocking guards is "fail closed," it is worth noting that this promise degrades silently when the env var is invalid rather than falling back to the hardcoded default.

Possible hardening (if desired — not blocking):

local read_timeout="${CLAUDE_PLUGIN_OPTION_STDIN_READ_TIMEOUT:-2}"
# Clamp to a safe minimum; a non-positive or non-numeric value silently resets.
if ! [[ "$read_timeout" =~ ^[0-9]+(\.[0-9]+)?$ ]] || ((${read_timeout%.*} < 1)); then
  read_timeout=2
fi

Consider fixing →

---

Other surfaces reviewed

Surface	Assessment
read -t "$read_timeout" — shell expansion into bash built-in flag	Safe: bash's read does not evaluate the timeout value as a shell expression
jq -e . >/dev/null 2>&1 <<<"$input" — herestring from accumulated stdin	Safe: herestring is not subject to word-splitting or glob expansion; jq parses JSON, does not execute it
input+="$chunk" — unbounded accumulation	Acceptable: producer is Claude Code (trusted); per-chunk idle timeout limits stall time, not payload size. No practical DoS path that wouldn't also require compromising the producer.
((read_rc > 128)) arithmetic on $?	Safe: $? is always 0–255
printf '%s' "$input" return path	Safe: no command substitution on caller side
Bash 4.x: read -N -t may return rc=1 (not >128) on timeout	Not a regression — the old code's EPOCHREALTIME-based heuristic was already unavailable on bash 4.x (bash 5.0+ feature). Behavior on bash 4.x is unchanged.
min: 1 in plugin.json userConfig	Consistent with repo convention (actionlint, claude-ops use the same field name). Not a JSON Schema minimum — this is the marketplace's own schema dialect.
jq always-runs (new behavior vs. old gating on read_status != 0)	Strictly better: a clean EOF with malformed JSON now returns 1 (skip) instead of passing through to callers

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 3c7354e525

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 33f95e0266

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[claude]

Claude encountered an error after 1m 57s —— View job

---

PR Review — Updated Code (post 33f95e0)

Tasks

• Gather PR context and review history
• Read updated lib/hook-utils.sh (post Bash 3.2 fix)
• Evaluate Codex P2: extra full-window delay on partial-progress re-arm
• Review Bash 3.2 fallback path and test coverage (Test 18e)
• Check hook::read_supports_nchars predicate isolation
• Verify all previous nit findings are addressed
• Post review findings

[claude]

Claude encountered an error after 2m 22s —— View job

---

Security Review: fix(hook-utils): read hook stdin in chunks so a large payload is not blocked

Tasks

• Gather PR context
• Fetch PR diff
• Read core changed files (lib/hook-utils.sh, lib/hook-utils.test.sh)
• Read guardrails and plugin changes
• Hunt for logic flaws, injection surfaces, trust-boundary issues
• Post security review findings

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 52650f41ea

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[claude]

Claude encountered an error after 0s —— View job

---

I'll analyze this and get back to you.

[claude]

Claude encountered an error after 0s —— View job

---

I'll analyze this and get back to you.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 316ae52b2e

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[claude]

Claude encountered an error after 0s —— View job

---

I'll analyze this and get back to you.

[claude]

Claude encountered an error after 1s —— View job

---

I'll analyze this and get back to you.

[claude]

Claude encountered an error after 0s —— View job

---

I'll analyze this and get back to you.

[claude]

Claude encountered an error after 0s —— View job

---

I'll analyze this and get back to you.

[claude]

Claude encountered an error after 0s —— View job

---

I'll analyze this and get back to you.

[claude]

Claude encountered an error after 1s —— View job

---

I'll analyze this and get back to you.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: ef12f55e7a

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[claude]

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

---

PR Review (post-merge, HEAD 0cad910)

Tasks

• Gather PR context and review history
• Read current lib/hook-utils.sh (post all Codex/Claude rounds)
• Review hook::resolve_read_timeout and hook::read_supports_nchars
• Check slice-based stall detection logic
• Check jq / here-string fix (hook::json_complete, hook::jq_field)
• Review tests (new cases 18b–18h)
• Check outstanding nits from previous Claude review
• Post findings

---

Summary

This PR has been through five rounds of substantive review (two Codex passes, a security review, and two prior Claude passes), and every raised issue was addressed. The current state is clean. No blocking issues.

The core mechanism is correct: chunked read -N, per-slice idle detection, probe-based timeout validation, early-completion break, and printf | jq everywhere a here-string would deadlock. The fail-closed posture is intact and verified from three angles in the PR description.

---

🟡 Nit — ((idle_slices++)) returns rc 1 on the first increment

File: lib/hook-utils.sh L440

((idle_slices++))

((expr)) in bash returns rc 1 when expr evaluates to zero. Post-increment when idle_slices=0 returns the old value (0), so this statement exits with rc 1 on the very first idle slice. Under set -uo pipefail (which all hooks use) this is harmless — bash only exits from an arithmetic command on rc 1 if set -e is active, and no hook sets it. The statement is also not on the left side of && or ||, so the exit code is ignored. No live bug.

The latent trap is if a future hook or test harness runs the function under set -e or set -euo pipefail (common outside this repo). It would exit buffer_stdin early on the first empty slice, causing every stall scenario to return with whatever input holds rather than the expected rc 2.

Pre-increment (((++idle_slices))) avoids this cleanly — the expression evaluates to 1 (the post-increment result), which is truthy, so rc 0. Alternatively, idle_slices=$((idle_slices + 1)) sidesteps the arithmetic-command exit-code question entirely.

Consider fixing →

---

Everything else checks out

Core mechanism:

• Version gate in hook::read_supports_nchars is arithmetically correct for all bash versions (3.2, 4.0, 4.1, 5.x).
• hook::resolve_read_timeout probe pattern (probe=$(read -r -t "$t" discard </dev/null 2>&1)) correctly captures read's stderr; valid timeouts produce no stderr so probe="", invalid specs produce the usage error. The regex + zero-check + probe is belt-and-suspenders and handles all five test cases (abc, 0, -1, 1e3, empty). This was the security review's suggestion and it's now implemented. ✓
• hook::resolve_read_slice falls back to "$t 1" (unsliced) when the computed fractional slice is rejected by this shell's read -t — correct degradation for bash 3.2 or absent awk. ✓
• idle_slices reset on any non-empty chunk is in the right place (before the read_rc branch), so progress always cancels an in-progress idle accumulation. ✓
• The completeness check fires only on the first empty slice of a quiet period (((idle_slices == 0)) &&), keeping jq off the hot path for large or slow reads. ✓
• Exact-multiple-of-65536 payloads: completed by a full-chunk read (rc 0, continue), then the FIRST empty slice reaches the ((idle_slices == 0)) && hook::json_complete check and breaks — one slice of wait rather than a full bound. ✓

jq / here-string deadlock fix:

• hook::json_complete, buffer_stdin's final check, hook::jq_field, and claude-ops's skill-usage-expansion-audit.sh all use printf '%s' "$1" | jq rather than a here-string. No remaining here-string paths that take the buffered payload. ✓
• hook::json_complete's structural pre-filter ([[ "${1: -4}" == *"}"* ]]) is O(1) and can only produce false negatives (costs one extra read, never wrong verdict). ✓

Tests:

• All eight test cases (18, 18b–18h) assert their preconditions before the measurement, which is the lesson this branch learned the hard way — three vacuous tests were caught mid-development by exactly this discipline. ✓
• The timing comparisons are relative (real function vs forced-variant on the same host), not against wall-clock constants, so runner load cannot flake them. ✓
• Test 18f's "0" case doubles as the loop-termination regression test: a spin would show as the suite never finishing. ✓
• Test 18e asserts the hook::read_supports_nchars override actually flips the branch before testing the fallback — prevents the vacuous test that shadowed BASH_VERSINFO (which is readonly). ✓

Note on test structure: The PR body describes a separate "Test 18h" (chunk-boundary / here-string deadlock regression), but in the code this lives inside the # --- Test 18g --- comment block. Functionally identical coverage — just a minor comment-numbering discrepancy between the description and the code.

Previously flagged nits (still present, intentionally accepted):

• jq runs unconditionally on every invocation (intentional — a corrupt clean-EOF payload now returns 1 instead of passing through; documented as strictly better behavior).
• Empty stalled pipe (zero bytes delivered before stall) → rc 1 (skip) not rc 2 (block) via [[ -n "$input" ]] || return 1. Unchanged from old behavior; PR explicitly marks as out of scope.
• IFS= before read -N is redundant per the bash manual. Harmless; consistent with the -d '' branch where it was meaningful.

CLAUDE.md / plugin-form rules: No hardcoded paths, ${CLAUDE_PLUGIN_OPTION_*} used for config, no PII or secrets, plugin.json versioned. All 14 synced copies are consistent. Guardrails at 0.18.0 (minor, correct for new userConfig option); 13 others at patch. ✓

REVIEW.md always-check criteria: No object-level auth, no tenant data paths, no secrets in source, no string-concatenated process invocations from external input. None apply. ✓

[claude]

Claude finished @kyle-sexton's task in 3m 34s —— View job

---

Security Review: fix(hook-utils): read hook stdin in chunks so a large payload is not blocked

Tasks

• Load tools and set up
• Fetch PR diff and read core changed files
• Analyze hook::resolve_read_timeout for trust-boundary and injection issues
• Analyze hook::json_complete and hook::jq_field for injection surfaces
• Analyze hook::buffer_stdin control flow for logic/security flaws
• Review Bash 3.2 fallback path for security regressions
• Check skill-usage-expansion-audit.sh changes
• Post security findings

---

Scope reviewed: lib/hook-utils.sh (+224/-16), plugins/claude-ops/hooks/skill-usage-expansion-audit.sh (+3/-1), and the 14 synced hook-utils.sh copies (identical). No workflow files changed; nothing for zizmor's lane.

No security issues found. The full analysis is below.

---

Surfaces reviewed

hook::resolve_read_timeout — CLAUDE_PLUGIN_OPTION_STDIN_READ_TIMEOUT handling

Code

The env var is an untrusted external input. Three concerns traced:

Shell injection through read -t "$t": read is a bash builtin. "$t" is a fully-expanded, single-quoted word before it reaches the builtin's argument parser. An env-var value of $(rm -rf /) is never re-interpreted by bash — "$t" substitutes the literal string, not a command. No injection vector.

Probe running on unvalidated input: The probe probe=$(read -r -t "$t" discard </dev/null 2>&1) runs before the regex check. If $t is abc, bash's read emits an error message to stderr and returns rc 1; the subshell captures that stderr as probe, [[ -n "$probe" ]] fires, and t falls back to 2. The probe is the primary acceptance gate; the regex is belt-and-suspenders for the 0/0.0 spin case (where read -t 0 succeeds silently, producing no stderr, so the probe alone would pass it). Validation is complete.

Addresses the previous review's SUGGESTION: The prior security review flagged that an invalid CLAUDE_PLUGIN_OPTION_STDIN_READ_TIMEOUT silently disabled fail-closed guards. This PR's hook::resolve_read_timeout directly addresses that finding — an invalid spec now falls back to the hardcoded default of 2 rather than cascading to an empty payload and a skip.

---

hook::resolve_read_slice — awk with unvalidated input

Code

$t here is the already-validated output of hook::resolve_read_timeout — always ^[0-9]+(\.[0-9]+)?$. awk -v t="$t" passes it as an awk variable value; the awk program is a hardcoded BEGIN block performing only division. No awk code injection path. HOOK_STDIN_READ_SLICES=4 is assigned unconditionally at script scope, so it cannot be pre-seeded to 0 by an environment variable (the script-level assignment overwrites any inherited value).

---

hook::json_complete — jq called on buffered payload

Code

printf '%s' "$1" | jq -e . passes the accumulated payload as stdin to jq with a hardcoded filter .. $1 is the raw hook payload (data, not a filter). jq -e . only validates/parses the JSON — it does not evaluate embedded expressions as code. printf '%s' treats $1 as a format argument, not a format string. No injection surface.

The structural pre-filter [[ "${1: -4}" == *"}"* ]] can only produce false negatives (causing the read to continue), never false positives that bypass jq. The jq call is the authoritative gate.

---

hook::jq_field — jq filter from $2

Code

field=$(printf '%s' "$1" | jq -r "(${2} // empty)"' | gsub("\r";"")' 2>/dev/null)

${2} is a jq path expression embedded in the filter. Every call-site in the fleet passes a hardcoded string literal as $2 (.tool_input.file_path, .tool_name, .trigger, etc.) — none derives $2 from the runtime hook payload $1. Verified by grep across all 14 plugin directories. The change from jq <<< "$1" to printf '%s' "$1" | jq is a security improvement (eliminates the here-string deadlock at pipe-capacity boundary); the filter injection surface is unchanged and not reachable from untrusted input.

---

hook::buffer_stdin loop — control flow and fail-closed posture

Code

The read_opts array is built from: hardcoded flags (-r), a validated positive-decimal slice (-t "$read_slice"), and either the hardcoded -N 65536 or the hardcoded -d ''. No runtime-input-derived value enters the option array. "${read_opts[@]}" expands each element as one word — no word-splitting or glob injection.

Fail-closed posture: A stalled pipe (pipe goes silent before delivering valid JSON) still reaches stalled=1 → ((jq_rc != 0 && jq_rc != 127)) → return 2 (BLOCKED). The condition is equivalent to the old read_status -ne 0 && jq_rc -ne 0 && jq_rc -ne 127 for the stall case; the non-stalled truncated-EOF path returns 1 (skip), which was also the pre-existing behavior. The PR's three end-to-end verification scenarios (small+violation, 200KB+trailing violation, stalled pipe) confirm the posture in both directions.

jq absent: jq_rc stays 0 when jq is missing; the condition jq_rc != 0 is false; the function returns the stalled payload (rc 0). This is the pre-existing fail-open-without-jq behavior, unchanged.

---

skill-usage-expansion-audit.sh

Code

printf '%s' "$INPUT" | jq -r '...' — same pattern as the library fix. $INPUT is data to jq; the filter is a hardcoded literal. No injection surface.

---

Summary

This PR closes one class of security-relevant behavior (large hook payloads triggering the stall branch and silently disabling scans) without opening new injection surfaces. The hook::resolve_read_timeout function directly addresses the previous security review's SUGGESTION about unvalidated timeout config. All data-to-jq paths use printf | jq (writer-process pattern) rather than here-strings, eliminating the pipe-capacity deadlock. The fail-closed posture for stalled pipes with partial JSON is preserved and verified.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 0cad9102ea

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[kyle-sexton]

🤖 Claimed by autonomous babysit lane (fable-autopilot). Expires 2026-07-26T22:26:52Z. Will release or renew.

[claude]

Claude finished @kyle-sexton's task in 2m 7s —— View job

---

PR Review (HEAD 395708b)

Tasks

• Gather PR context and review history
• Read current lib/hook-utils.sh (latest HEAD)
• Check the docs qualification commit (quarter-bound claim on Bash 3.2)
• Verify ((idle_slices++)) finding from prior round
• Review for any remaining correctness or security gaps
• Post findings

---

Summary

The current commit (395708b) addresses the last Codex P2 finding: all three plugins that expose stdin_read_timeout (actionlint, claude-ops, guardrails) now document the Bash 3.2 / one-window fallback accurately. The qualification is correct — on a shell where hook::resolve_read_slice falls back to printf '%s 1' "$t" (fractional read -t rejected), slice_count=1 and one empty window after a non-empty window triggers the stall declaration, which can land up to two full intervals after the pipe went quiet. "Up to two intervals" matches the code.

No new findings. One pre-existing nit from the previous round remains unaddressed.

---

🟡 Carry-forward nit — ((idle_slices++)) returns rc 1 on first increment (from prior round, still present)

File: lib/hook-utils.sh L440

((idle_slices++))

Post-increment when idle_slices=0 evaluates to 0, so ((expr)) returns rc 1 on the first idle slice. Under set -e or set -euo pipefail (not currently used in any hook, but common in consumer harnesses) this would exit buffer_stdin early on the very first empty slice, causing every stall scenario to produce whatever partial input holds rather than the expected rc 2.

((++idle_slices)) or idle_slices=$((idle_slices + 1)) avoids the issue cleanly. Not blocking — the current calling context is safe — but worth fixing before set -e becomes a consumer expectation.

Consider fixing →

---

Everything else checks out

Bash 3.2 documentation (this commit's change):

• The "up to two intervals" characterization is verified against hook::resolve_read_slice: slice_count=1 → one empty slice after a non-empty one triggers stall, and that empty window spans one full timeout → total silence duration before declaration ≈ 2× configured timeout. Accurate.
• All three surfaces updated: README and CHANGELOG for actionlint, claude-ops, and guardrails. The wording is consistent across them. ✓
• plugins/actionlint/skills/setup/SKILL.md (step 5b) does not mention the Bash 3.2 caveat, but it also does not claim quarter-bound precision — it describes the idle-bound semantics only. The README is the reference doc for precision. Acceptable. ✓
• The Bash 3.2 claim ("macOS system shell") is accurate: macOS ships /bin/bash at version 3.2.57, which is the system shell users encounter without explicitly installing a modern bash. ✓

Previously accepted nits (still intentionally present):

• IFS= before read -N — redundant per bash manual, harmless. ✓
• Empty stalled pipe (zero bytes before stall) → rc 1 (skip) not rc 2 (block) — explicitly out of scope per PR body. ✓
• jq runs unconditionally on every invocation — intentional; documented as strictly better behavior for corrupt clean-EOF payloads. ✓

[claude]

Claude finished @kyle-sexton's task in 3m 35s —— View job

---

Security Review: fix(hook-utils): read hook stdin in chunks so a large payload is not blocked

Tasks

• Gather PR context and review history
• Fetch PR diff and read core changed files
• Analyze hook::buffer_stdin control flow and loop termination
• Analyze hook::resolve_read_timeout trust boundary and probe safety
• Analyze hook::resolve_read_slice / hook::read_supports_nchars
• Analyze hook::json_complete, hook::jq_field, and injection surfaces
• Review skill-usage-expansion-audit.sh changes
• Analyze Bash 3.2 fallback path for security regressions
• Check fail-closed posture under adversarial conditions
• Post findings

---

Scope reviewed: lib/hook-utils.sh (+224/-16), plugins/claude-ops/hooks/skill-usage-expansion-audit.sh (+3/-1), plugins/guardrails/.claude-plugin/plugin.json (+8/-1), and the 14 synced hook-utils.sh copies (identical). No workflow files changed.

No security issues found. Full analysis below.

---

Surfaces reviewed

hook::resolve_read_timeout — CLAUDE_PLUGIN_OPTION_STDIN_READ_TIMEOUT as untrusted input

lib/hook-utils.sh L339–350

The env var is external untrusted input. Three injection vectors traced:

Shell injection via read -t "$t": read is a bash builtin; "$t" is a fully-expanded, single-quoted word before reaching the builtin's argument parser. An env-var value of $(rm -rf /) is stored as a literal string (no re-evaluation on variable assignment), and "$t" passes it as a single positional argument to -t — bash does not re-interpret it as a command. No injection.

Probe running before regex validation (probe=$(read -r -t "$t" discard </dev/null 2>&1) runs first, then the regex and zero-checks fire): this is intentional. The probe IS the primary gate for fractional-timeout acceptance; the regex is belt-and-suspenders against the 0/0.0 spin case (where read -t 0 succeeds silently, leaving probe empty). An invalid spec causes read to emit a usage message to stderr; the subshell captures it as probe; [[ -n "$probe" ]] fires and t falls back to 2. The combined guard handles all rejection cases in any order they fire.

$t as a flag-injection vector (e.g., $t = "-n 0"): passed as read -r -t "$t" discard — "$t" is the value of the -t option, not a new option-like word. Bash argument parsing already consumed the -t token; "$t" goes into its value slot. The builtin sees a single non-numeric value, emits an error, probe becomes non-empty, fallback fires. No injection.

This directly addresses the previous security review's SUGGESTION about the CLAUDE_PLUGIN_OPTION_STDIN_READ_TIMEOUT silent-disable path. ✓

---

hook::resolve_read_slice — awk with validated input

lib/hook-utils.sh L367–381

$t here is already validated output from hook::resolve_read_timeout — always matches ^[0-9]+(\.[0-9]+)?$. awk -v t="$t" passes it as an awk variable value (not interpreted as awk code); the program is a hardcoded BEGIN block. HOOK_STDIN_READ_SLICES=4 is assigned unconditionally at file scope — it overwrites any inherited env value before any function runs. Even if awk received n=0 (which can't happen here), division-by-zero produces empty output, the probe check fails, and the fallback to unsliced behavior fires. No injection surface; correct degradation under all failure modes.

---

hook::json_complete — pre-filter and jq pipeline

lib/hook-utils.sh L303–319

Pre-filter [[ "${1: -4}" == *"}"* ]]: can only produce false negatives (costs one extra read, never a wrong verdict). False positives would require the last four characters to contain } — those still proceed to jq validation, which is authoritative. Input is passed as printf '%s' "$1" | jq -e .: '%s' is the format string; $1 is the format argument — not the format string — so format-string injection is impossible. The printf | jq pattern (rather than a here-string) eliminates the pipe-capacity deadlock that was the stated motivation.

---

hook::jq_field — filter argument from $2

lib/hook-utils.sh L474–479

$2 is embedded as a jq filter fragment: jq -r "(${2} // empty)". Every call site across all 14 plugin directories (verified by grep) passes a hardcoded literal string (.tool_name, .tool_input.file_path, .trigger, .source, etc.) — no call site derives $2 from the hook payload $1 or from any external input. The injection surface is pre-existing and not reachable from untrusted input in the current codebase. The change from jq <<< "$1" to printf '%s' "$1" | jq is a security improvement; the filter surface is unchanged.

---

hook::buffer_stdin — loop termination and fail-closed posture

lib/hook-utils.sh L383–463

The read_opts array is built from: hardcoded flags (-r), a probed-and-validated slice (-t "$read_slice"), and either hardcoded -N 65536 or hardcoded -d ''. No runtime-input-derived value enters the option array. "${read_opts[@]}" expands each element as one word with full word separation — no word-splitting or glob injection on the array expansion.

Adversarial paths traced:

Scenario	Outcome
STDIN_READ_TIMEOUT=0 to spin the loop	hook::resolve_read_timeout detects ^0+(\.0+)?$, falls back to 2. Loop normal.
STDIN_READ_TIMEOUT=abc to produce empty payload	Probe emits stderr; [[ -n "$probe" ]] fires; fallback to 2. Loop normal.
Payload ending in } but invalid JSON, then stall	hook::json_complete pre-filter fires but jq fails → loop continues; stall accumulates; stalled=1; final jq fails → return 2 (BLOCKED).
Exactly 65536-byte complete payload, pipe held open	First empty slice: idle_slices==0, hook::json_complete succeeds → break. Returns 0 with payload intact.
jq absent, partial payload then stall	jq_rc stays 0; falls through to printf '%s' "$input". Fail-open without jq — pre-existing behavior, unchanged.
Unexpected read_rc value (1–128, not EOF-1 or timeout->128)	Falls through to final break; stalled=0; jq validates; incomplete payload → return 1 (skip). Consistent with truncated-clean-EOF case, which PR explicitly marks out of scope.

The stall verdict comes cleanly from read's exit status (> 128) rather than elapsed-time arithmetic, removing the EPOCHREALTIME/awk heuristic that could be bypassed on hosts without EPOCHREALTIME.

---

skill-usage-expansion-audit.sh change

plugins/claude-ops/hooks/skill-usage-expansion-audit.sh L44–49

printf '%s' "$INPUT" | jq -r '...' — $INPUT is data to jq; the filter is a hardcoded literal. printf '%s' treats $INPUT as an argument, not the format string. No injection surface. Pure security improvement over the here-string.

---

guardrails plugin.json — new stdin_read_timeout userConfig entry

"min": 1 is a UI constraint in the marketplace schema dialect; it is not enforced at the shell level. The shell-level enforcement is now handled by hook::resolve_read_timeout, which validates and falls back unconditionally regardless of how CLAUDE_PLUGIN_OPTION_STDIN_READ_TIMEOUT is set. The "default": 2 matches the shell-level :-2 fallback. No security gap.

---

Summary

This PR closes the previous security review's SUGGESTION (unvalidated STDIN_READ_TIMEOUT silently disabling fail-closed guards) via hook::resolve_read_timeout. All data-to-jq paths now use printf | jq (separate writer process), eliminating the pipe-capacity deadlock class entirely. The fail-closed posture for stalled pipes is preserved and verified from three angles. No new injection surfaces, trust-boundary violations, or authorization gaps introduced.

[kyle-sexton]

🤖 Claim released — PR merged by another lane while my claim was active. No conflict occurred.