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
229 lines
10 KiB
Bash
Executable File
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 ]]
|