Compare commits

..
Author SHA1 Message Date
kristianwindandClaude Opus 5 539abf946c 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>
2026-09-28 21:14:10 +02:00
Kristian Wind 5151204b12 Merge pull request #44 from kristianwind/identity-check-must-not-flag-its-own-repair
CI / WordPress latest + WooCommerce latest on PHP 8.3 (push) Successful in 1m32s
CI / Static checks (push) Successful in 3m30s
Let the identity check tell a write from a read and from its own repair
2026-09-25 12:37:37 +02:00
kristianwindandClaude Opus 5 97818ec4db Let the identity check tell a write from a read and from its own repair
Widening the scope group to --[a-z-]+ was wrong in a way the previous
commit's matrix could not see, because it had no repair row: --unset is
matched as a scope flag, and user. follows it, so

    git config --local --unset user.name

is flagged as a violation. That is the remedy this check recommends in its
own failure message. A build that fails on its own repair is how a correct
check gets deleted — the failure accuses the check, and the next person
believes it. The same widening flags a read, echo "$(git config user.name)".

Two halves, both needed. Enumerate the three real scopes, and require a
VALUE after the key: user\.[a-zA-Z]+[[:space:]]+[^[:space:])]. The trailing
class is what makes a read legal — $(git config user.name) has ) where a
write has a value. Sixteen rows measured one at a time, 16/16, and eleven of
them now run in ci/selftest.sh (44 assertions).

Reported by the Nolimit Security session; three of us reached for
--[a-z-]+ independently, so the comment above the step says explicitly not
to, with the reason, rather than leaving it to be re-derived. The read row
came from nolimit-seo.

Two things found while proving it, both about the probe rather than the
check:

ci/selftest.sh is itself a ci/*.sh file, so its fixtures trip the check they
test. Spelled out in full, the first version of this block reported six
false positives and no count, while the same rows measured standalone were
16/16 correct — a failing probe is evidence about the probe until you have
looked at it. Fixed the way the credential fixtures three hundred lines up
in the same file already were: assemble the value through $gc so the source
matches nothing while the written file matches fully. The labels need it
too, so they name the shape instead of quoting the line.

And a plant that lands in a SOURCED file is not inert text. nolimit-seo
planted into ci/lib.sh, which ci/static.sh sources, so the line ran and set
the clone's identity for real. The target here is ci/build-zip.sh and only
ci/lib.sh is sourced — verified rather than assumed — and a final assertion
checks the clone still has no local identity, because if a fixture had
executed then every verdict above it was measured against a tree nobody
intended.

static=0 selftest=0 site=0; site still reports 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>
2026-09-25 12:35:45 +02:00
Kristian Wind 7b1d667b5a Merge pull request #43 from kristianwind/scope-git-identity-to-its-own-commit
CI / WordPress latest + WooCommerce latest on PHP 8.3 (push) Successful in 1m26s
CI / Static checks (push) Successful in 2m4s
Scope the compatibility bump's git identity to its own commit
2026-09-25 12:21:20 +02:00
2 changed files with 214 additions and 48 deletions
+110 -34
View File
@@ -206,52 +206,128 @@ ann=$(ci_php ci/check-textdomain.php "$TEXT_DOMAIN" selftest-dotted 2>&1 \
|| bad "the annotated path is not a file: '$ann'"
# ------------------------------------------------------- the clone's git identity
# 🔴 Grade 2 evidence: the exact line ci/bump-tested-up-to.sh carried until
# 2026-09-25, not an arbitrary `git config`. That line writes .git/config, so
# every later commit in the clone — a release, by a person — is authored by the
# robot with no trailer. Six such commits in one sibling plugin, two in
# 🔴 Grade 2 evidence: row 1 is the exact line ci/bump-tested-up-to.sh carried
# until 2026-09-25, not an arbitrary `git config`. That line writes .git/config,
# so every later commit in the clone — a release, by a person — is authored by
# the robot with no trailer. Six such commits in one sibling plugin, two in
# another. git warns about an identity it cannot FIND and never about a wrong
# one, so nothing in the repository ever says so.
#
# The controls are the larger half here, and three of them were live faults in
# earlier versions of this check: `--[a-z-]+` as the scope group reads `--unset`
# as a scope and flags the REPAIR, and without a trailing-value requirement a
# `$(git config user.name)` READ is a violation. A build that fails on its own
# remedy is how a correct check gets deleted — the failure accuses the check,
# and the next person believes it.
#
# 🔴 The fixtures are assembled through $gc rather than written out, because
# THIS file is a ci/*.sh file and the check reads every one of them. Spelled in
# full, its own rows match: the first version of this block reported six false
# positives and no count, while the same rows measured standalone were 16/16
# correct. A failing probe is evidence about the probe until you have looked at
# it — and the labels have to dodge it too, which is why they name the shape
# instead of quoting the line. Identical to the credential fixtures three
# hundred lines up, one file over.
#
# The plant goes into ci/build-zip.sh because nothing here SOURCES it: static.sh
# and this script source ci/lib.sh only. A plant in a sourced file is not inert
# text — it runs, sets the clone's identity for real, and the commit fixing the
# fault is authored by the plant. nolimit-seo lost a run to exactly that. So the
# last assertion checks the clone rather than the output.
echo
echo "→ ci/static.sh must catch a ci/ script that configures the clone"
echo "→ ci/static.sh must catch a written identity and ignore reads and repairs"
cd "$work/repo"
cp ci/build-zip.sh "$work/build-zip.orig"
for scope in '' '--local' '--global'; do
cp "$work/build-zip.orig" ci/build-zip.sh
line="git config ${scope:+$scope }user.name \"compatibility check\""
printf '%s\n' "$line" >> ci/build-zip.sh
# The plant must BE there before any verdict is believed. A `printf` that
# silently went somewhere else leaves the control state, and every assertion
# below then passes while proving nothing — a hollow probe wearing the
# costume of the most rigorous thing in the suite.
if [ "$(grep -cF "$line" ci/build-zip.sh)" -lt 1 ]; then
bad "could not stage the git-identity fixture for '${scope:-no flag}'"
continue
fi
id_out=$(./ci/static.sh 2>&1 || true)
if printf '%s' "$id_out" | grep -q 'sets a git identity that outlives it'; then
ok "flagged git config ${scope:-(no flag)} user.name"
else
bad "NOT FLAGGED: git config ${scope:-(no flag)} user.name — the clone keeps the robot's name"
fi
done
# The correct form must NOT fire, or the fix trips the check that exists to
# protect it and the next person deletes one of the two.
identity_row() { # $1 = RED|green (want), $2 = label, $3 = line to plant
cp "$work/build-zip.orig" ci/build-zip.sh
printf '%s\n' "$3" >> ci/build-zip.sh
# The plant must BE there before any verdict is believed. A printf that went
# somewhere else leaves the control state, and then every row passes while
# proving nothing — and the all-green rows are exactly the ones whose zeroes
# you want to believe.
if [ "$(grep -cF "$3" ci/build-zip.sh)" -lt 1 ]; then
bad "could not stage the git-identity fixture: $2"
return
fi
row_out=$(./ci/static.sh 2>&1 || true)
# 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
fi
if [ "$got" = "$1" ]; then
ok "$2 -> $got"
elif [ "$1" = RED ]; then
bad "NOT FLAGGED: $2 — the clone keeps the robot's name"
else
bad "FALSE POSITIVE: $2 — the check goes red on something it must allow"
fi
}
gc="git config" # never spelled out below: see the note above
identity_row RED 'WRITE no flag, quoted value' "$gc user.name \"compatibility check\""
identity_row RED 'WRITE --local scope' "$gc --local user.name \"x\""
identity_row RED 'WRITE --global scope' "$gc --global user.email \"x@y\""
identity_row RED 'WRITE --system scope' "$gc --system user.name \"x\""
identity_row RED 'WRITE unquoted value' "$gc user.email noreply@localhost"
identity_row green 'REPAIR --local --unset' "$gc --local --unset user.name"
identity_row green 'REPAIR --unset, no scope' "$gc --unset user.email"
identity_row green 'READ inside a command substitution' "echo \"\$($gc user.name)\""
identity_row green 'FIX git -c ... commit' 'git -c user.name="x" -c user.email=y commit -m z'
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
printf '%s\n' 'git -c user.name="compatibility check" -c user.email=x commit -m y' >> ci/build-zip.sh
scoped_out=$(./ci/static.sh 2>&1 || true)
printf '%s' "$scoped_out" | grep -q 'sets a git identity that outlives it' \
&& bad "FALSE POSITIVE on 'git -c user.name=… commit' — that is the fix, not the fault" \
|| ok "ignored the scoped 'git -c … commit' form"
# 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
# above was measured against a tree nobody intended.
if git config --local --get-regexp '^user\.' >/dev/null 2>&1; then
bad "a git-identity fixture RAN — the clone has a local identity and the rows above are void"
else
ok "no fixture executed: the clone still has no local identity"
fi
# ------------------------------------------------------------------- plugin.conf
echo
+104 -14
View File
@@ -19,6 +19,51 @@ shipped_files() {
./ci/dist-files.sh | grep '\.php$' || true
}
# 🔴 scan_tracked: the only way this file greps a set of tracked files.
#
# The form it replaces — `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 and `if` reads that as clean. 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
#
# Harmless only while the whole list fits in one exec. git ls-files hands over
# enough paths to split the moment a tree grows or a path gets long, and then
# the check passes over a violation it has already printed. Nothing about the
# output says which case you are in. Reported by the Nolimit Security session
# 2026-09-28, who hit it after widening their own scan from 13 files to 102.
#
# So: no xargs, and the verdict is the OUTPUT rather than any exit status. The
# loop also yields the count for free, which is the other thing every one of
# these checks was missing — "none" over zero files and "none" over nineteen
# are the same green.
#
# Sets: scan_hits (matches, one "file:line:text" per line), scan_files (how
# many files were read). A grep failure (exit > 1) is reported, never swallowed.
scan_tracked() { # $1 = -E|-P, $2 = pattern, rest = git pathspecs
local mode=$1 pattern=$2 f m rc
shift 2
scan_hits=''
scan_files=0
# -z: git QUOTES a non-ASCII path, and the quoted form is not a path that
# exists, so a line-based read silently skips the file it meant to check.
while IFS= read -r -d '' f; do
[ -n "$f" ] || continue
scan_files=$((scan_files + 1))
m=$(grep -n "$mode" -e "$pattern" -- "$f")
rc=$?
if [ "$rc" -gt 1 ]; then
note_fail "grep failed on $f (exit $rc) — this scan did not examine it."
elif [ "$rc" = 0 ]; then
scan_hits="${scan_hits}$(printf '%s\n' "$m" | sed "s|^|$f:|")
"
fi
done < <(git ls-files -z "$@")
}
ci_step "PHP syntax"
# The header promises a minimum PHP version; without it in the loop a piece of
# newer syntax passes here and fatals on the customer's server instead.
@@ -73,10 +118,14 @@ else
fi
ci_step "no debug leftovers"
if git ls-files -z '*.php' | xargs -0 grep -nE '\b(var_dump|print_r|dd)[[:space:]]*\(' ; then
scan_tracked -E '\b(var_dump|print_r|dd)[[:space:]]*\(' '*.php'
if [ "$scan_files" -lt 1 ]; then
note_fail "The debug-leftover scan read no PHP files at all, so it proved nothing."
elif [ -n "$scan_hits" ]; then
printf '%s' "$scan_hits"
note_fail "Debug functions must not ship."
else
echo " none"
echo " none in $scan_files tracked PHP files"
fi
ci_step "no short open tags, no closing ?>"
@@ -84,9 +133,14 @@ before=$fails
# With short_open_tag off the code is printed to the visitor as text instead of
# being run; whitespace after a closing tag is sent to the browser and breaks
# headers and redirects.
if git ls-files -z '*.php' | xargs -0 grep -nP '<\?(?!php|=|xml)' ; then
scan_tracked -P '<\?(?!php|=|xml)' '*.php'
if [ "$scan_files" -lt 1 ]; then
note_fail "The open-tag scan read no PHP files at all, so it proved nothing."
elif [ -n "$scan_hits" ]; then
printf '%s' "$scan_hits"
note_fail "Use <?php, not short open tags."
fi
open_tag_files=$scan_files
# -z: git quotes non-ASCII paths, and the quoted form is not a path that
# exists, so this loop would silently skip the file it was meant to check.
while IFS= read -r -d '' f; do
@@ -94,7 +148,7 @@ while IFS= read -r -d '' f; do
[ "$(tail -c 3 "$f" | tr -d '[:space:]')" = "?>" ] \
&& { ci_annotate error "$f" 1 "Remove the trailing ?> at the end of the file."; fails=$((fails + 1)); }
done < <(git ls-files -z '*.php')
[ "$fails" = "$before" ] && echo " clean"
[ "$fails" = "$before" ] && echo " $open_tag_files tracked PHP files, no short tags and no trailing ?>"
ci_step "text domain"
# --min is deliberately far below the real count. It is not an assertion about
@@ -307,7 +361,7 @@ elif [ "$fails" = "$before" ]; then
echo " $secrets_scanned tracked files read, nothing that looks like a key or a private key"
fi
ci_step "no ci/ script configures the clone it was handed"
ci_step "no tracked file configures the clone it was handed"
# `git config user.name` writes .git/config and OUTLIVES the script. A helper
# that sets an identity before committing therefore renames every later commit
# in that working copy, including a release made by a person days afterwards
@@ -322,19 +376,55 @@ ci_step "no ci/ script configures the clone it was handed"
# misattributed hand commits in one sibling, two in another, including
# releases. This tree was clean and one run away.
#
# Any flag, not just --local: --global sets the whole machine's identity, which
# is the same fault one scope wider and reaches every repository on the box.
# The pattern has to separate a WRITE from a read and from the repair, and the
# obvious widening gets all three wrong. Measured row by row:
#
# git config user.name "compatibility check" WRITE caught
# git config --local user.name "x" WRITE caught
# git config --global user.email "x@y" WRITE caught <- --local only: missed
# git config --system user.name "x" WRITE caught
# git config --local --unset user.name REPAIR ignored <- --[a-z-]+: flagged
# git config --unset user.email REPAIR ignored <- --[a-z-]+: flagged
# echo "$(git config user.name)" READ ignored <- --[a-z-]+: flagged
# git -c user.name="x" … commit -m y FIX ignored
# git config core.autocrlf false other ignored
# git config --global core.pager cat other ignored
# # git config user.name "x" (a comment) prose ignored
# (nothing planted) clean ignored
#
# ⚠️ Do NOT widen the scope group to `--[a-z-]+`. It reads `--unset` as a scope
# and then finds `user.` right after it, so the check goes red on the very
# repair this message recommends — and a build that fails on the remedy is how
# a correct check gets deleted: the failure accuses the check, and the next
# person believes it. Three sessions reached for `--[a-z-]+` independently on
# 2026-09-25, which is why it is written here rather than re-derived.
#
# Enumerating the three real scopes is one half. The other is requiring a VALUE
# after the key — `user\.[a-zA-Z]+[[:space:]]+[^[:space:])]` — which is what
# makes a read not a violation: `$(git config user.name)` has `)` where a write
# has a value. Found by nolimit-seo, whose first version failed on it.
#
# The rule: a script may configure ITSELF, never the working copy it was
# handed. Reported by the Nolimit Security session 2026-09-25.
scripts_scanned=$(git ls-files 'ci/*.sh' | wc -l)
if [ "$scripts_scanned" -lt 1 ]; then
note_fail "The git-identity scan found no ci/*.sh files at all, so it proved nothing."
elif git ls-files -z 'ci/*.sh' \
| xargs -0 grep -nE '^[^#]*git config([[:space:]]+--[a-z-]+)*[[:space:]]+user\.'; then
note_fail "A ci/ script sets a git identity that outlives it. Scope it to the one commit with 'git -c user.name=… commit', which does not write .git/config."
#
# Writing the fault in prose inside a tracked file will trip this, by design.
# To name it in documentation without going red, write the key as user.<key> —
# the pattern needs [a-zA-Z] after the dot, so an angle-bracket placeholder is
# prose and a real key is a violation.
# Every tracked file, not just ci/*.sh. A workflow step, a hook, a Makefile or
# a docs snippet somebody pastes can write an identity 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.
# Nolimit Security widened theirs from 13 files to 102 after the thing that
# actually ran on this box turned out to be outside that glob.
scan_tracked -E '^[^#]*git config([[:space:]]+--(local|global|system))*[[:space:]]+user\.[a-zA-Z]+[[:space:]]+[^[:space:])]' .
if [ "$scan_files" -lt 1 ]; then
note_fail "The git-identity scan read no tracked files at all, so it proved nothing."
elif [ -n "$scan_hits" ]; then
printf '%s' "$scan_hits"
note_fail "A tracked file writes a git identity that outlives it. Scope it to the one command with 'git -c user.name=… commit', which does not write .git/config."
else
echo " $scripts_scanned scripts in ci/ leave the clone's git identity alone"
echo " $scan_files tracked files leave the clone's git identity alone"
fi
ci_step "no placeholders left in the plugin header"