Skip to content

block-windows-drive-tmp: non-Windows host gate leaves large payloads undrained on stdin #3504

Description

@kyle-sexton

Summary

Follow-up from #3503, surfaced by that PR's fresh-context verifier. Not introduced as a defect by the fix — it is an accepted trade-off of the fix, recorded here rather than widened into that PR.

plugins/guardrails/hooks/block-windows-drive-tmp.sh now evaluates its non-Windows OSTYPE gate before hook::buffer_stdin, so on Linux and macOS the hook exits 0 without reading stdin. That fix was necessary: with the gate below hook::require_jq_blocking, a jq-less non-Windows host took a fail-closed exit 2 on every Write/Edit/MultiEdit/NotebookEdit call.

The cost is that the payload is left undrained on every non-Windows tool call.

Measured

Payloads piped from cat, OSTYPE=linux-gnu, comparing 158ca9e (before the reorder) with 205fd68 (after):

payload size before after (linux-gnu) after (msys)
4,205 B writer rc 0 writer rc 0 writer rc 0
65,645 B writer rc 0 writer rc 0 writer rc 0
262,253 B writer rc 0 writer rc 141 (SIGPIPE) writer rc 0
1,048,685 B writer rc 0 writer rc 141 (SIGPIPE) writer rc 0

Above roughly 64KB–256KB the writer takes SIGPIPE. The Windows lane is unaffected.

Why it was accepted rather than fixed in #3503

  • It is not proven to affect Claude Code. The measurement used cat as the writer, which dies on SIGPIPE; that is not how the harness feeds a hook.
  • The harness demonstrably tolerates an undrained hook exit already: hook::check_enabled exits before draining whenever an operator disables any guardrails guard, and that is a supported configuration.
  • Both obvious fixes are worse. A naive builtin drain loop blocks until EOF and can hang a tool call — which is precisely why hook::buffer_stdin is a bounded idle-timeout read rather than a plain read. Draining through buffer_stdin first would put its rc-2 fail-closed exit back ahead of the host gate, reintroducing the bug the reorder fixed.

What is genuinely new

The shape (exiting without draining) was already present via check_enabled. The exposure is new: check_enabled is a kill switch that fires only when a guard is explicitly disabled, whereas this gate fires unconditionally on every non-Windows tool call — the default state for every Linux and macOS user.

Suggested direction

Decide whether guardrails wants a shared bounded-drain helper in hook-utils.sh for early-exit paths, usable by any hook that gates before reading. That is a cross-hook decision, not a single-guard one, which is why it is filed rather than patched here.

Activity

  1. added
    needs-triageNot yet classified. Floor until a type and one priority tier are set.
    on Aug 31, 2026
  2. claude commented on Sep 6, 2026

    @claude
    Contributor

    Generated by Claude Code

  3. claude commented on Sep 6, 2026

    @claude
    Contributor

    This was generated by AI during triage.

    Triage verification (2026-09-06)

    Claim holds, and the code now documents the trade-off in place. The host gate still runs ahead of both the bounded stdin read and the jq requirement check, so on a non-Windows host the hook exits without draining the payload. The hook carries an in-file comment block recording exactly this, including the distinction the body draws between the established shape and the new exposure, and the reason both obvious fixes are worse. No bounded-drain helper exists in the shared hook library today, so the question the item asks is genuinely still open.

    The framing in the body is accurate: this is an accepted cost of a necessary fix, not a regression. Without the reorder, a non-Windows host lacking jq took a fail-closed refusal on every file-editing tool call.

    Triage outcome: human-gated (open cross-hook decision)

    Why not agent-ready. The item asks whether guardrails should have a shared bounded-drain helper for early-exit paths. That is a decision about a shared contract every hook would inherit, and the item deliberately does not propose a mechanism. There is no brief a cold agent could execute, because what to build has not been decided.

    Why the decision is genuinely open rather than defaultable. Doing nothing is a serious option that the body argues for on its own evidence: the effect is unproven against the real harness (the measurement used a writer that dies on a broken pipe, which is not how the harness feeds a hook), and the harness already tolerates an undrained hook exit through the existing kill-switch path. Against that, the exposure did change in kind: the kill switch fires only when an operator disables a guard, while this gate fires on every tool call for every Linux and macOS user, which is the default configuration. Weighing an unproven risk on the default path against adding a shared helper to a latency-budgeted hook library is a maintainer's call, not a default a triage lane should set.

    Constraint the decision must respect. Any helper lands in a hook library governed by the marketplace-wide always-on latency budget, and the item's own analysis rules out the two naive designs: an unbounded drain can hang a tool call, and draining through the existing bounded read would put its fail-closed refusal back ahead of the host gate, reintroducing the bug the reorder fixed.

    What a maintainer needs to decide, stated so the decision is not re-derived:

    1. Does an undrained early exit actually affect the real harness, or is the kill-switch precedent sufficient evidence that it does not? This is answerable by observation and settles the rest.
    2. If it matters: a shared bounded-drain helper for early-exit paths, or per-hook handling?
    3. If it does not: close this and record the kill-switch precedent as the standing answer, so the next verifier that notices the shape does not refile it.

    Related

    Sibling of #3507, which is a live fail-closed defect in the same guardrails stdin-handling area and is already routed as delegable. The two are related by surface, not by decision: #3507 has a definite fix, this one has an open question. Whoever answers question 1 above should look at both together, since they are the same stdin-handling contract seen from two sides.


    Generated by Claude Code

  4. added
    priority: mediumReal value, no hard deadline; normal backlog flow.
    needs-humanHuman-in-the-loop required; autonomous sessions must not resolve items carrying this.
    and removed
    needs-triageNot yet classified. Floor until a type and one priority tier are set.
    on Sep 6, 2026
  5. kyle-sexton commented on Sep 27, 2026

    @kyle-sexton
    ContributorAuthor

    This was generated by AI during attend-queue.

    Closing (operator decision, 2026-09-27): accepted trade-off. The undrained stdin on non-Windows is documented in the hook, and there is no harness evidence of harm.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    needs-humanHuman-in-the-loop required; autonomous sessions must not resolve items carrying this.priority: mediumReal value, no hard deadline; normal backlog flow.

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions