Skip to content

fix(guardrails): block-hook-bypass wrongly blocks echo/printf ... >&N as a file-write bypass #1680

Description

@kyle-sexton

Summary

block-hook-bypass.sh (PreToolUse, guardrails v0.17.3) blocks the extremely common, completely safe shell idiom echo "message" >&2 (and printf ... >&2, and any bare >&N redirect with no explicit leading fd digit) as a "Write/Edit bypass," even though it never touches the filesystem. Root cause identified and a working, test-suite-clean fix verified below — independently re-verified twice (once by a fresh audit subagent, once again while drafting this issue) rather than taken on a single pass's word.

Reproduction (verified, runnable)

This only fires through the actual hook — piping a bare shell command at a terminal does nothing (echo x >&2 just prints x). It requires feeding the hook the same JSON-on-stdin PreToolUse payload Claude Code sends it:

#!/usr/bin/env bash
HOOK="<install>/hooks/block-hook-bypass.sh"
export CLAUDE_PLUGIN_ROOT="<install>"   # the hook sources lib/ relative to this

run_case() {
  local cmd="$1"
  local payload rc
  payload=$(jq -n --arg cmd "$cmd" '{tool_name:"Bash", tool_input:{command:$cmd}}')
  printf '%s' "$payload" | bash "$HOOK" >/tmp/out 2>&1
  rc=$?
  printf '%s (rc=%s): %s\n' "$cmd" "$rc" "$( [[ $rc -eq 2 ]] && echo BLOCKED || echo allowed )"
}

run_case 'echo x >&2'                  # BLOCKED — false positive, never touches a file
run_case 'echo "hello world" >&2'      # BLOCKED — same
run_case "printf '%s' x >&2"           # BLOCKED — same, printf variant
run_case 'echo x 1>&2'                 # allowed — explicit leading fd digit sidesteps a DIFFERENT, unrelated check
run_case 'echo x 2>&1'                 # allowed — same reason
run_case 'echo x > realfile.txt'       # BLOCKED — correct, real write
run_case 'echo x &> realfile.txt'      # BLOCKED — correct, real write (both streams to a file)

Independently reproduced against the installed guardrails 0.17.3 hook, this exact result set, twice.

Root cause

producer_redirect_bypass() neutralizes a real >& redirect before splitting the command into ;/\n/|/&/(/)-delimited segments (so the redirect's own & isn't mistaken for a separator), using a \x01 (SOH) sentinel:

normalized="${normalized//>&/>$soh}"
# ... split into segments on separators ...
normalized="${normalized//"$soh"/&}"   # restore SOH back to a literal &

On bash ≥5.2 (the patsub_replacement shell option, enabled by default since bash 5.2 per bash's own NEWS file and the GNU manual — confirmed on this session's Git-Bash/Cygwin bash 5.3.9 on Windows, and independently confirmed on Ubuntu 24.04's actual bash 5.2.21 — this is not a Windows-only bug), an unquoted, unescaped bare & in the replacement position of ${var//pattern/replacement} means "the text the pattern matched," the same convention sed uses. So ${normalized//"$soh"/&} doesn't insert a literal ampersand — it re-inserts whatever "$soh" matched, i.e. the SOH byte itself. The restore is a no-op by construction, not a silent failure to substitute; the sentinel simply survives, invisibly, into the later regex scan.

Net effect: echo x >&2 becomes, internally, echo x > + (inert SOH byte) + 2 — which the scanner then reads as a bare >2 redirect target, passing the "is this a real file write" check. Quoting ("&") or backslash-escaping (\&) the replacement restores literal-ampersand semantics and fixes it; an unquoted variable holding only & (e.g. ${normalized//"$soh"/$amp} where amp='&') is equally broken — the same-meaning rule applies to the expanded value, not just a bare literal, so "assign it to a variable first" is not a workaround.

Blast radius, precisely scoped: only the bare >&N form (implicit fd 1) is affected. N>&M forms with an explicit leading digit (1>&2, 2>&1) are unaffected by this specific bug, because a separate, unrelated exclusion (the leading-digit check in _echo_file_out/_echo_devnull) already protects them regardless.

Verified fix — exact diff, tested clean against the existing suite

A quote-only fix is NOT sufficient on its own and was verified to reopen a real bypass. Quoting just the sentinel-restore line correctly stops the false positive, but then echo/printf x &>real-file (a genuine file write via bash's both-streams redirect) is silently allowed — verified empirically. The tested, working fix touches 3 lines total:

--- a/hooks/block-hook-bypass.sh
+++ b/hooks/block-hook-bypass.sh
@@ -307,2 +307,2 @@
-_echo_file_out='(^|[^0-9&])>>?[[:space:]]*[^|&>[:space:]]'
-_echo_devnull='(^|[^0-9&])>>?[[:space:]]*/dev/null'
+_echo_file_out='(^|[^0-9])>>?[[:space:]]*[^|&>[:space:]]'
+_echo_devnull='(^|[^0-9])>>?[[:space:]]*/dev/null'
@@ -375,1 +375,1 @@
-  normalized="${normalized//"$soh"/&}"
+  normalized="${normalized//"$soh"/"&"}"

(The $esc-restore line a few lines below, which quotes a space rather than &, was checked and is not affected by patsub_replacement — bare vs. quoted space substitution is byte-identical on this bash. No change needed there; leave it alone.)

Removing & from the two regexes' leading-exclusion class is necessary because the sentinel restore now correctly produces a literal & again — without that change, the now-correctly-restored & in >&file would itself get excluded by the old class, silently reopening the exact bypass the quote fix alone was shown to reopen. Verified: dropping only the _echo_devnull half of this change (leaving _echo_file_out alone) creates a NEW false positive on echo x &>/dev/null — both regexes need the same change together.

Test result: the fixed hook passes the plugin's own existing suite clean, 203/203, plus reproduces every case in the Reproduction section above correctly (false positives now allowed, all genuine-write cases still blocked).

One accepted, honest trade-off — not swept under "pre-existing"

The fix has one narrow, real cost: echo x >&2>file — a glued redirect-then-re-redirect where the trailing >file is a real write to a real file — is blocked by the CURRENT (buggy) hook but is NOT blocked by the fixed hook. Verified directly (both hooks tested side by side against this exact command). This is a regression the fix introduces, not a pre-existing gap it merely fails to close — a maintainer applying this diff should know that one specific glued form goes from over-blocked-by-accident to under-blocked. The already-existing, separate 2>&1>file glued-form gap is unaffected either way (present before and after). Whether this narrow trade-off is acceptable (the fix removes a much more common, much more disruptive false positive at the cost of one uncommon glued form) is a judgment call for whoever owns this hook, not something this report should decide unilaterally.

Test-suite gap

The existing 203-case suite is green today only because no fixture isolates the exact bare >&N-alone shape — adjacent near-miss tests create false confidence that this path is covered. Recommend adding explicit regression fixtures for: echo x >&2 / printf x >&2 (expect: allowed), echo x &>file / printf x &>file (expect: blocked), and — now that it's a known, accepted trade-off — echo x >&2>file (expect: allowed, with a comment noting why).

Suggestion (lower priority)

The block message ("BLOCKED: echo/printf > file write bypasses Write/Edit hooks") discards the form/segment data the hook already computed internally, forcing manual byte-level tracing to diagnose why a specific command was blocked. Including the matched segment/form in the stderr message (not just the telemetry envelope) would make future false-positive triage much faster than what this investigation required.

Severity

CRITICAL — false positive on an extremely common, safe shell idiom, shipping since v0.9.5 (confirmed: the commit that introduced producer_redirect_bypass and the unquoted & restore together is the same commit that bumped to 0.9.5). The fix must be applied as the full 3-line diff, not partially — a partial (quote-only) fix reopens a real bypass, verified.

Provenance

Surfaced by /plugin-quality:audit. Root cause and initial fix diagnosed by a dispatched auditor subagent; independently re-verified against the actual installed hook (both the false-positive claim and the fix's regression-free-ness) by a second, fresh review pass before filing — including catching and correcting the auditor's own draft errors (a malformed/duplicated diff, an overstated "≥5.1" bash-version claim corrected to the verified ≥5.2, and an initial "pre-existing" framing of the >&2>file trade-off corrected to state it plainly as a fix-introduced regression).

Metadata

Metadata

Assignees

No one assigned

    Labels

    area: securitySecurity-relevant: vulnerability, hardening, or disclosure follow-up.needs-humanHuman-in-the-loop required; autonomous sessions must not resolve items carrying this.priority: criticalProduction broken, data loss, active exploit, or release blocker.status: needs-decisionAwaiting a human or maintainer judgment call.work-class: structuralRefactors, migrations, contract changes; cross-cutting and hard to reverse.

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions