Replace the two growing | grep -q checks with case
Under `set -o pipefail` a grep that MATCHES can make the pipeline exit 141: grep closes the pipe on its first hit, the producer takes SIGPIPE, and pipefail reports that as failure. A match becomes an error. Security found the shape; measured here before believing it, because the earlier pipefail lesson went the other way and I did not want to carry a second claim I had not seen: yes hittet | grep -q hittet under pipefail -> exit 141 printf hittet | grep -q hittet -> exit 0 So it needs the producer to outrun the 64 KB pipe buffer, which is why it is invisible until it is not. Two of this repo's sites can get there, and both were converted. Neither is reachable today — that is the reason to do it now rather than an argument against: ci/publish-release.sh checked the release feed for a download URL. The feed is 3.5 KB in the new repo and 13.5 KB in the old one across seven releases, so about 1.9 KB each: it reaches 64 KB around release 34. That failure would land in the publishing path and say the feed has no zip attached while it plainly has one, and nothing about the commit that tripped it would have anything to do with it. A bug on a timer is worse than a bug. ci/static.sh checked whether each plugin-relative include is in the shipped list, once per include. Same growth, plus a fork per iteration. Both now use `case`, which has no pipe, no fork and no producer to signal, and cannot be reversed by a shell option. The static.sh one wraps both sides in newlines to keep the exact-line match `grep -qx` gave. Proven both ways: thirteen includes checked on a clean tree, and excluding one from the zip still names it. The remaining `| grep -q` sites were measured rather than assumed safe: zip_entries is 1.3 KB, the shipped list 478 bytes, static.sh's own output 1.6 KB, and site-test.sh and update-smoke.sh grep a single line of wp-cli output. Left alone; converting those would be churn against producers that cannot plausibly reach the buffer. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5
parent
36c15dc05e
commit
d121005c0a
+16
-3
@@ -200,12 +200,25 @@ if [ -z "$feed" ]; then
|
|||||||
exit 1
|
exit 1
|
||||||
fi
|
fi
|
||||||
|
|
||||||
printf '%s' "$feed" | grep -q '"browser_download_url"' || {
|
# `case`, not `printf … | grep -q`. Under `set -o pipefail` a grep that
|
||||||
|
# MATCHES can make the pipeline exit 141: grep closes the pipe on the first
|
||||||
|
# hit and the producer takes SIGPIPE, which pipefail reports as failure. So a
|
||||||
|
# match becomes an error, in the publishing path, with a message saying the
|
||||||
|
# feed has no download URL when it plainly does.
|
||||||
|
#
|
||||||
|
# Not reachable today — the feed is 3.5 KB and the pipe buffer is 64 KB — but
|
||||||
|
# it grows about 1.9 KB per release, so it arrives around release 34. A bug on
|
||||||
|
# a timer is worse than a bug, because nothing about the commit that trips it
|
||||||
|
# will have anything to do with it. `case` has no pipe, no fork and no
|
||||||
|
# producer to signal.
|
||||||
|
case "$feed" in
|
||||||
|
*'"browser_download_url"'*) ;;
|
||||||
|
*)
|
||||||
echo
|
echo
|
||||||
ci_fail "The release is readable, but it has no zip attached. WordPress would be offered an update it cannot download."
|
ci_fail "The release is readable, but it has no zip attached. WordPress would be offered an update it cannot download."
|
||||||
exit 1
|
exit 1
|
||||||
}
|
;;
|
||||||
|
esac
|
||||||
# Checked at the address the PLUGIN builds, not the one the API reports.
|
# Checked at the address the PLUGIN builds, not the one the API reports.
|
||||||
#
|
#
|
||||||
# A forge fills browser_download_url from its own configured root URL, which is
|
# A forge fills browser_download_url from its own configured root URL, which is
|
||||||
|
|||||||
+9
-2
@@ -122,8 +122,15 @@ for path in $referenced; do
|
|||||||
case "$path" in */*) ;; *) continue ;; esac
|
case "$path" in */*) ;; *) continue ;; esac
|
||||||
[ -d "${path%%/*}" ] || continue
|
[ -d "${path%%/*}" ] || continue
|
||||||
checked=$((checked + 1))
|
checked=$((checked + 1))
|
||||||
printf '%s\n' "$shipped" | grep -qx "$path" \
|
# `case` rather than piping the list into grep -qx: under pipefail a grep
|
||||||
|| note_fail "The code loads '$path', but the release zip does not contain it. Committed?"
|
# that matches can exit 141 when the producer is signalled, so a HIT becomes
|
||||||
|
# a failure. The list is small today and this loop runs once per include, so
|
||||||
|
# this also drops a fork per iteration. Newlines on both sides make it an
|
||||||
|
# exact line match, which is what -qx gave.
|
||||||
|
case $'\n'"$shipped"$'\n' in
|
||||||
|
*$'\n'"$path"$'\n'*) ;;
|
||||||
|
*) note_fail "The code loads '$path', but the release zip does not contain it. Committed?" ;;
|
||||||
|
esac
|
||||||
done
|
done
|
||||||
if [ "$checked" = 0 ]; then
|
if [ "$checked" = 0 ]; then
|
||||||
# A check that verifies nothing must say so rather than reporting success.
|
# A check that verifies nothing must say so rather than reporting success.
|
||||||
|
|||||||
Reference in New Issue
Block a user