Files
holocron/tests/test-no-pipefail-early-exit-grep.sh
Defame1297 384756b343 test(gates): pin hook wiring and enforce the bats TAP plan
The repo's gates were not pinned to their wiring. Deleting the
check-skill-version-bump block from .pre-commit-config.yaml left the
whole suite green; deleting eight blocks at once, run-tests among them,
also left it green. Only 4 of 20 hook ids had their wiring pinned
anywhere, so a merge conflict resolved badly could stop the suite
running at pre-push forever while every test still reported green.

test-adr0020-contract now derives the repo-authored hooks from the
repo: local entries and pins each one's id, entry and stages against an
explicit expected set, both directions, with the same non-vacuity
guards the file already applies to its own fixtures. Upstream hooks and
their rev: values are untouched, so a rev bump does not churn the test.
Mutation-checked: a removed block, a repointed entry and a hook moved
off pre-push each go red; a rev bump, a comment edit and reordering
stay green. 29 -> 44 assertions.

run-bats computed each file's TAP plan and then discarded it, so a
process printing "1..10", three ok lines and exit 0 was counted as
"3 tests, 0 failures" with seven tests silently gone. That is exactly
the wrapper-swallows-the-status case the runner's own comment puts in
its threat model, and the plan was the only surviving signal. The plan
is now enforced in both directions when a file emits exactly one.

Also: test-no-pipefail-early-exit-grep's live-tree floor goes from 20 to
50 against an actual 57, matching test-vale-wrap's per-glob discipline,
and test-vale-wrap's header names the real path to vale-wrap.sh.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NwD8Egs5r4ndqeFLmhusX2
2026-09-20 12:34:07 +00:00

229 lines
10 KiB
Bash
Executable File

#!/usr/bin/env bash
set -euo pipefail
# Static guard: no `echo|printf ... | grep -q` pipeline in a script that runs
# under `set -o pipefail`.
#
# The defect. grep -q (and -m, -l, -L, --quiet, --silent) exits as soon as it
# has an answer. If the writer on the left of the pipe still has output to
# write, that next write hits a closed pipe and the writer dies of SIGPIPE.
# Under pipefail the pipeline then reports the WRITER's failure, so output that
# matched reads as "no match". Whether it happens depends on scheduling: bash's
# echo can hand a multi-line value to the pipe a line at a time, and a reader
# that matched on an early line exits before the later lines arrive. Forced with
# a pause before the last line, the pipe form failed 50 of 50 runs; each of a
# here-string, a match on the last line, and the same pipe without pipefail
# passed 50 of 50. Unforced it surfaced about once per 670 suite runs, which is
# why it read as a flaky gate rather than as a bug.
#
# The fix is `grep -q PATTERN <<< "$VAR"`: a here-string has no separate writer
# process, so there is nothing to race. `printf '%s\n' "$X" | grep` and
# `<<< "$X"` feed grep the same bytes; `printf '%s'` and `echo` differ only by a
# trailing newline, which no -q verdict on a non-empty pattern depends on.
#
# Scope. Only echo/printf writers are flagged: they are the shell builtins that
# can split a write, and they are always replaceable by a here-string. A
# command writer (`run_wrap ... | grep -q`) is not flagged; those sites either
# absorb the writer's status (`|| true`) or write once at exit. Files without
# pipefail are not flagged, because without it the pipeline's status is grep's.
# Comment-only lines are skipped so the pattern can be named in prose.
#
# Remit. This scanner models early-exiting GREP readers only — grep, egrep and
# fgrep, however they are spelled (a path prefix, a `command` prefix, env-var
# assignments), anywhere in the pipeline. They are not the only readers that
# exit early: `head`, `sed -n 1p` and a bare `read` do too, and an echo/printf
# writer feeding any of them is the same race. Those sites are guarded by
# convention instead — absorb the writer's status with `|| true` (or take the
# verdict from a here-string) — and deliberately not by this test, because most
# legitimate uses of them in this tree are already absorbed and the scanner
# cannot see the absorption from the pipeline text alone. Flagging them would be
# noise; the grep readers are flagged because a here-string is always available.
#
# Known limitation: heredoc bodies are scanned as code, so a `cat <<'EOF'` body
# containing a vulnerable-looking line reads as a real site. There are none in
# the tree today.
#
# Case 1 proves the scanner can fail before case 2 trusts its clean verdict on
# the live tree.
REPO_ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)"
SELF="$REPO_ROOT/tests/$(basename "${BASH_SOURCE[0]}")"
PASS=0
FAIL=0
pass() { echo " PASS: $1"; PASS=$((PASS + 1)); }
fail() { echo " FAIL: $1"; FAIL=$((FAIL + 1)); }
RUN_TMP="$(mktemp -d)"
trap 'rm -rf "$RUN_TMP"' EXIT
# A pipeline-safe run of characters: anything but `|`, `;` or `&`, except that a
# `&` immediately followed by a digit is kept so `2>&1` does not end a segment.
# Excluding a bare `&` is what stops `echo ok && other | grep -q x` — a pipeline
# whose writer is `other`, not the echo — from being read as one site.
SAFE='([^|;&]|&[[:digit:]])'
# Writer, then any number of intermediate stages, then an early-exiting grep.
# The reader may be spelled `grep`, `egrep` or `fgrep`, behind a path prefix
# (`/bin/grep`), a `command` prefix, or env-var assignments (`LC_ALL=C grep`).
SITE_RE="(^|[^[:alnum:]_])(echo|printf)[[:space:]]${SAFE}*[|][[:space:]]*"
SITE_RE+="([^|;&[:space:]]${SAFE}*[|][[:space:]]*)*"
SITE_RE+="((command|[A-Za-z_][A-Za-z0-9_]*=[^[:space:]]*)[[:space:]]+)*"
SITE_RE+="([^[:space:];&|]*/)?(grep|egrep|fgrep)[[:space:]]"
SITE_RE+="(${SAFE}*[[:space:]])?(-[[:alpha:]]*[qmlL][[:alnum:]]*|--quiet|--silent|--max-count|--files-with)"
# `pipefail` need not follow the first `-o`: `set -o errexit -o pipefail` and
# `set -o posix -o pipefail` are the same setting. A file-level miss here skips
# every site in the file, so this is deliberately loose — the cost of scanning a
# file that does not set pipefail is nil, the cost of skipping one is total.
PIPEFAIL_RE='(^|[^[:alnum:]_-])set[[:space:]]+-[^;#]*pipefail'
# Prints `path:line: text` for every vulnerable pipeline in the given files.
# Continued lines are joined first — both a trailing backslash and a trailing
# `|`, which is equally legal as a line continuation in a pipeline — so a reader
# on the next physical line is still seen; the reported line is where the
# command starts.
scan() {
local f
for f in "$@"; do
grep -Eq "$PIPEFAIL_RE" "$f" || continue
awk -v file="$f" -v re="$SITE_RE" '
function flush( b) {
if (buf == "") return
if (buf !~ /^[[:space:]]*#/ && buf ~ re) print file ":" start ": " buf
buf = ""
}
{
if (buf == "") start = NR
line = $0
if (line ~ /\\$/) { buf = buf substr(line, 1, length(line) - 1) " "; next }
if (line ~ /[|][[:space:]]*$/) { buf = buf line " "; next }
buf = buf line
flush()
}
END { flush() }
' "$f"
done
}
# ---------------------------------------------------------------------------
echo "--- the scanner flags each vulnerable spelling and nothing else ---"
# ---------------------------------------------------------------------------
BAD="$RUN_TMP/bad.sh"
cat > "$BAD" <<'EOF_BAD'
#!/usr/bin/env bash
set -euo pipefail
if echo "$OUT" | grep -q "needle"; then :; fi
! printf '%s\n' "$OUT" | grep -qF "needle" && :
if printf '%s' "$OUT" | grep -E -q "a|b"; then :; fi
if printf '%s\n' "$OUT" \
| grep -m1 "needle"; then :; fi
if echo "$OUT" | tr -d ' ' | grep -q "needle"; then :; fi
echo "$OUT" |
grep -q "needle" && :
if echo "$OUT" | egrep -q "needle"; then :; fi
if printf '%s\n' "$OUT" | fgrep -q "needle"; then :; fi
if echo "$OUT" | /bin/grep -q "needle"; then :; fi
if echo "$OUT" | command grep -q "needle"; then :; fi
if echo "$OUT" | LC_ALL=C grep -q "needle"; then :; fi
if echo "$OUT" | sed -n '1,$p' | command /usr/bin/grep --quiet "needle"; then :; fi
EOF_BAD
ALTSET="$RUN_TMP/alt-pipefail.sh"
cat > "$ALTSET" <<'EOF_ALT'
#!/usr/bin/env bash
set -o errexit -o pipefail
if echo "$OUT" | grep -q "needle"; then :; fi
EOF_ALT
GOOD="$RUN_TMP/good.sh"
cat > "$GOOD" <<'EOF_GOOD'
#!/usr/bin/env bash
set -euo pipefail
# A comment naming the bad form: echo "$OUT" | grep -q needle
if grep -q "needle" <<< "$OUT"; then :; fi
COUNT="$(echo "$OUT" | grep -c "needle" || true)"
echo "$OUT" || grep -q "needle" <<< "$OUT"
run_wrap "$DIR" file.md | grep -q "needle" || :
printf '%s\n' "$OUT" | grep -c "needle"
echo "$OUT" | tail -n 1
echo "$OUT" | tr -d ' ' | tail -n 1
echo "$OUT" | mygrep -q "needle"
echo "$OUT" | command tail -n 1
echo "$OUT" | LC_ALL=C sort
echo "$OUT" |
tail -n 1
echo ok && run_wrap "$DIR" | grep -q "needle"
EOF_GOOD
NOPIPEFAIL="$RUN_TMP/no-pipefail.sh"
cat > "$NOPIPEFAIL" <<'EOF_NOPF'
#!/usr/bin/env bash
set -eu
if echo "$OUT" | grep -q "needle"; then :; fi
EOF_NOPF
BAD_HITS="$(scan "$BAD")"
BAD_COUNT="$(grep -c . <<< "$BAD_HITS" || true)"
if [[ "$BAD_COUNT" -eq 12 ]]; then
pass "all 12 vulnerable spellings are flagged (echo, negated printf, split options, both continuations, an intermediate stage, egrep/fgrep, and path/command/env-assignment reader prefixes)"
else
fail "expected 12 hits in the bad fixture, got $BAD_COUNT: $BAD_HITS"
fi
if grep -q "^$BAD:6: " <<< "$BAD_HITS"; then
pass "a backslash-continued pipe is reported at the line the command starts"
else
fail "the backslash-continued site was not reported at line 6: $BAD_HITS"
fi
if grep -q "^$BAD:9: " <<< "$BAD_HITS"; then
pass "a pipeline wrapped after a trailing '|' is reported at the line the command starts"
else
fail "the trailing-pipe continuation site was not reported at line 9: $BAD_HITS"
fi
ALT_HITS="$(scan "$ALTSET")"
if [[ -n "$ALT_HITS" ]]; then
pass "pipefail set as 'set -o errexit -o pipefail' is recognised, so its sites are scanned"
else
fail "a file setting pipefail after another -o option was skipped entirely"
fi
GOOD_HITS="$(scan "$GOOD")"
if [[ -z "$GOOD_HITS" ]]; then
pass "here-strings, comments, grep -c, '||', '&&', command writers and non-grep readers are left alone"
else
fail "the clean fixture was flagged: $GOOD_HITS"
fi
if [[ -z "$(scan "$NOPIPEFAIL")" ]]; then
pass "a file without pipefail is not flagged — grep's own status is the pipeline's"
else
fail "a file without pipefail was flagged"
fi
# ---------------------------------------------------------------------------
echo "--- no tracked shell script carries the race ---"
# ---------------------------------------------------------------------------
FILES=()
while IFS= read -r rel; do
[[ "$REPO_ROOT/$rel" == "$SELF" ]] && continue
[[ -f "$REPO_ROOT/$rel" ]] && FILES+=("$REPO_ROOT/$rel")
done < <(git -C "$REPO_ROOT" ls-files -- '*.sh' '*.bats' '*.bash')
# Floored at 50 against a real 57 (58 tracked `*.sh`/`*.bats`/`*.bash` files, less
# this one, which excludes itself above). Same discipline as the per-glob floors in
# tests/test-vale-wrap.sh: a guard against a broken `git ls-files` invocation or a
# moved search root resolving to a fraction of the tree, not a headcount to keep in
# step. Seven files of slack is one deliberate multi-file deletion -- a plugin's
# whole tests/ directory is three or four .bats files -- and nowhere near enough to
# absorb a discovery that degraded to a single directory.
if [[ "${#FILES[@]}" -lt 50 ]]; then
fail "only ${#FILES[@]} tracked shell files found — the scan is looking in the wrong place"
else
LIVE_HITS="$(scan ${FILES[@]+"${FILES[@]}"})"
if [[ -z "$LIVE_HITS" ]]; then
pass "none of ${#FILES[@]} tracked shell files pipes echo/printf into an early-exit grep under pipefail"
else
fail "use \`grep -q PATTERN <<< \"\$VAR\"\` instead at:"
echo "${LIVE_HITS//"$REPO_ROOT/"/ }"
fi
fi
echo ""
echo "Results: $PASS passed, $FAIL failed"
[[ "$FAIL" -eq 0 ]]