fix(hook): bound stdin read so a never-closing stdin can't wedge the hook - #172
Merged
Conversation
…hook `rafter hook pretool/posttool` read stdin with a 5s timeout, but on Node the timeout only bounded OUTPUT latency, not process exit: a piped stdin with no EOF stays in flowing mode and keeps the event loop alive, so after emitting its decision at 5s the process hung indefinitely. Reproduced via `sleep 30 | rafter hook pretool` (outer guard had to SIGKILL it). The fix pauses stdin and removes listeners on the timeout/end/error paths so the loop drains and the process exits (~5s, fail-open). Python already exited (daemon reader thread) — unchanged in behavior. Adds RAFTER_HOOK_STDIN_TIMEOUT_MS (ms, default 5000) as an operator safety valve and to keep the regression tests fast. Both implementations parse it identically: finite and > 0, else fall back to the default — so Infinity / nan / 0 / negative / garbage can never re-create the unbounded wait (Python guarded with math.isfinite to match Node's Number.isFinite, preventing join(timeout=inf)). Regression tests spawn the real hook with a never-closing stdin and assert bounded exit + fail-open decision (Node), and assert the bound + env parsing including non-finite rejection (Python). CLI_SPEC.md documents the bounded read. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
rafter hook pretool/hook posttoolread stdin with a 5s timeout, but on Node that timeout only bounded output latency, not process exit. A piped stdin with no EOF stays in flowing mode and keeps the event loop alive — so after emitting its decision at 5s, the hook process hung indefinitely.Reproduced (before fix):
Any host that opens the hook's stdin but never writes/closes it wedges a zombie
nodeper tool call. (Reported viahq-ryfytfrom codegen_security_arena/crew/turing.)Fix
process.stdin.pause()so the event loop drains and the process exits (~5s, fail-open). Verified: never-closing stdin now exits at ~5.5s default / ~0.9s with a 300ms override.RAFTER_HOOK_STDIN_TIMEOUT_MS(ms, default 5000) as an operator safety valve and to keep tests fast. Both implementations parse it identically: finite and > 0, else fall back to default — soInfinity/nan/0/ negative / garbage can never re-create the unbounded wait.Security review (required gate)
Ran
rafter-code-review(CWE Top-25 walk) on the diff. One finding, fixed in this PR:Local secrets scan clean on all changed files.
rafter run(remote SAST) not executed — noRAFTER_API_KEYin this environment; recommend CI runs it.Tests
hook-stdin-timeout.test.ts): spawns the real hook with a never-closing stdin, asserts bounded exit + fail-open decision (pretool & posttool).test_hook_stdin_timeout.py): asserts the bound returns promptly on a blocking stdin, and env parsing rejects non-finite / non-positive / garbage.hook-integration.test.ts > blocks rm -rf /fails onmaintoo — pre-existing, unrelated to this diff; filed separately.)🤖 Generated with Claude Code