Stop reading a verdict out of xargs' exit status

`if git ls-files -z … | xargs -0 grep -nE …; then` has a false green in it.
xargs exits 123 when ANY of its invocations exited 1-125, and grep exits 1 on
no-match, so a run where one batch matched and another did not exits 123 —
which `if` reads as clean, after grep has already printed the hit. Measured:

  one batch, a match       xargs exit 0    -> caught
  one batch, no match      xargs exit 123  -> clean, correct
  two batches, one match   xargs exit 123  -> CLEAN, and it printed the hit

Three checks here used that form: debug leftovers, short open tags, and the
git-identity scan added two days ago. All three are one batch at this tree's
size, so none of them is wrong today — this is a latent false green that
arrives with the file list, not a bug anyone would see. Nothing in the output
distinguishes the cases.

scan_tracked() replaces it: one grep per file, verdict from the OUTPUT rather
than any exit status, and a grep failure (exit > 1) reported instead of
swallowed. Batching stops existing. The loop also yields the file count, which
is the other thing all three were missing — "none" over zero files and "none"
over nineteen were the same green. They now read "none in 19 tracked PHP
files" and fail below a floor of one.

The identity scan is also widened from ci/*.sh to every tracked file. A
workflow step writes an identity into the runner's clone just as permanently,
and ci/*.sh was scope chosen from where the first instance happened to live —
the same "what are your checks pointed at" question as the contrast check.
13 files to 48. The new selftest row plants a write into a workflow file,
which the old scope could not see at all.

Reported by the Nolimit Security session, who hit the xargs case after
widening their own scan from 13 files to 102.

Three of my own probes were wrong before they were right, all the same class:
the assertions grep for the check's message text, and widening the scope
changed the wording from "A ci/ script sets …" to "A tracked file writes …".
Five rows reported NOT FLAGGED while the check was working perfectly. They now
match the stable half of the message, with the reason written beside them. And
the form check matched the comment in static.sh that quotes the bad form in
order to explain it, so it is anchored with ^[^#]* — the paragraph documenting
a rule is text the rule applies to.

That form check is a drift check on the shape and says so: it asserts the
pattern is gone, not that the replacement is correct, because this tree's 48
files are one batch and nothing here can measure the fault.

46 selftest assertions. static=0 selftest=0 site=0; site still 69 assertions
on WordPress 7.1.2 with WooCommerce 11.1.2. ci/ ships nothing, so no version
bump.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
kristianwind
2026-09-28 21:14:10 +02:00
co-authored by Claude Opus 5
parent 5151204b12
commit 539abf946c
2 changed files with 115 additions and 16 deletions
+38 -4
View File
@@ -251,7 +251,11 @@ identity_row() { # $1 = RED|green (want), $2 = label, $3 = line to plant
return
fi
row_out=$(./ci/static.sh 2>&1 || true)
if printf '%s' "$row_out" | grep -q 'sets a git identity that outlives it'; then
# The stable half of the message. The wider phrasing moved from "A ci/ script
# sets …" to "A tracked file writes …" when the scan was widened, and these
# rows all reported NOT FLAGGED while the check was working perfectly — an
# assertion on a log line is coupled to its wording.
if printf '%s' "$row_out" | grep -q 'git identity that outlives it'; then
got=RED
else
got=green
@@ -279,12 +283,42 @@ identity_row green 'other a non-identity key' "$gc core.autocrlf false"
identity_row green 'prose a commented-out mention' "# $gc user.name \"x\" is the fault"
cp "$work/build-zip.orig" ci/build-zip.sh
# A violation OUTSIDE ci/*.sh. This is the row the old scope missed entirely:
# a workflow step writes an identity into the runner's clone just as
# permanently, and the glob the check started with could not see it.
cp "$work/build-zip.orig" ci/build-zip.sh
wf=.github/workflows/selftest-identity.yml
printf 'name: x\njobs:\n y:\n steps:\n - run: %s user.email "x@y"\n' "$gc" > "$wf"
git add -N "$wf" >/dev/null # untracked files are invisible to git ls-files
wf_out=$(./ci/static.sh 2>&1 || true)
printf '%s' "$wf_out" | grep -q 'writes a git identity that outlives it' \
&& ok "flagged a write outside ci/ (a workflow step)" \
|| bad "NOT FLAGGED: a workflow step writing an identity — the scan is still scoped to ci/"
rm -f "$wf"
git rm --cached -q --ignore-unmatch "$wf" >/dev/null 2>&1
# 🔴 A drift check on the FORM, and only that: `if … | xargs -0 grep …; then`
# has a false green in it, because xargs exits 123 when any batch found nothing
# and `if` reads 123 as no-match. It is invisible while the list fits one exec,
# which is why nothing here can measure it — this tree's 48 files are one batch.
# So this asserts the shape is gone rather than that the bug is absent, and it
# cannot tell you the replacement is correct. scan_tracked greps per file and
# reads its verdict from the output, which is what makes batching irrelevant.
# ^[^#]* — the first version of this matched the COMMENT in static.sh that
# quotes the bad form in order to explain it, which is the stale-docblock trap
# in its own right: the paragraph documenting a rule is text the rule applies to.
if grep -nE '^[^#]*(if|elif).*\| *xargs.*grep' ci/static.sh; then
bad "a check above consumes xargs' exit status — 123 on a partial match reads as clean"
else
ok "no check in ci/static.sh consumes an xargs exit status"
fi
# And the passing line must carry a number: "every script is clean" reads the
# same over zero files as over eleven.
clean_out=$(./ci/static.sh 2>&1 || true)
printf '%s' "$clean_out" | grep -qE "^ [0-9]+ scripts in ci/ leave the clone's git identity alone" \
&& ok "the git-identity check reports how many scripts it read" \
|| bad "the git-identity check printed no count — it cannot tell 11 scripts from 0"
printf '%s' "$clean_out" | grep -qE "^ [0-9]+ tracked files leave the clone's git identity alone" \
&& ok "the git-identity check reports how many files it read" \
|| bad "the git-identity check printed no count — it cannot tell 48 files from 0"
# The fixtures are lines that SET an identity. If any of them executed rather
# than being read, this clone now has one — which would also mean every verdict