From 8790b027a908971960981f90c19def51027573b6 Mon Sep 17 00:00:00 2001 From: jknapp Date: Wed, 5 Aug 2026 15:23:56 -0700 Subject: [PATCH] fix(lsphp): stop SIGPIPE+pipefail reporting the parity extension as missing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `entrypoint-lsphp.sh` decided whether cac_path_parity was loaded with printf '%s\n' "$LSPHP_INFO" | grep -q '^cac_path_parity support => enabled$' under `set -euo pipefail`. `grep -q` exits on its first match; printf is still writing the remaining ~40 KB of `lsphp -i`, takes SIGPIPE, exits 141, and pipefail prefers 141 over grep's 0. The branch therefore evaluated FALSE *because the extension was present* — present early enough to stop the reader — and every affected container fell back to the auto_prepend normaliser that a customer's own .user.ini silently displaces, i.e. the exact failure the extension exists to remove. Measured on whp02 against the published cac-lsphp:php83: 5/5 runs status=141 with pipefail, 0 without. The race is decided by pipe capacity, which is why it reproduced on whp02 and not on other daemons: while the payload fits the pipe the writer never blocks and always finishes first. Forced over the limit it is deterministic — 3x the same `lsphp -i` (122100 bytes) gives 141 every time in the built image. Fixed by reading with here-strings, which are not pipelines at all, so there is no second exit status for pipefail to adopt. Same grep/awk patterns; plumbing only. Same class fixed everywhere it existed under pipefail: * entrypoint-lsphp.sh parity probe, and the SCAN_DIR awk probe * entrypoint-litespeed.sh SCAN_DIR probe (a bare assignment: 141 there does not degrade, `set -e` kills PID 1), and ols_running * entrypoint-shared-ols.sh ols_running * render-shared-ols-config.sh site.meta parsing (`sed | head -1`): measured 141 at 6000 duplicate keys, which under `set -e` aborts the whole render * fpm-parity-check.sh the `php-fpm -m` pre-flight, whose whole job is to stop a harness fault being blamed on the extension Also: the fallback used to announce "cac_path_parity extension not loadable in this image" for every reason the branch was reached, including its own plumbing breaking — a false diagnosis that sends operators to rebuild a good image whose build gate passed. Verdicts now carry the evidence they rest on, and a probe that produced nothing is reported as a probe failure that establishes nothing about the image. Fail-open posture is unchanged: no probe failure is fatal. Adds scripts/tests/lsphp-info-probe.test.sh, which runs the shipped probes (extracted verbatim, so they cannot drift from what runs in production) under `set -euo pipefail` against a realistic ~40 KB phpinfo body, and statically outlaws the shape repo-wide. Against trunk it fails, naming all 9 offending lines. Wired into CI as a new Shell-Checks job, because no existing gate ever executed the entrypoint's branch logic — the .phpt suite and the Dockerfile's own `lsphp -i | grep -q` probe (which has no pipefail) were both green for the release whose entrypoint declared that same extension missing. Verified: PHP 8.3 --no-cache build green, 10/10 .phpt, 9/9 FPM harness; the built image logs `path parity = extension` and reports `Rewriting => active` with .from/.to populated; ext-removed and probe-broken variants each produce their own honest message and still start. Co-Authored-By: Claude Opus 5 (1M context) --- .gitea/workflows/build-push.yaml | 50 +++ ext/cac-path-parity/tests/fpm-parity-check.sh | 23 +- scripts/entrypoint-litespeed.sh | 33 +- scripts/entrypoint-lsphp.sh | 92 +++++- scripts/entrypoint-shared-ols.sh | 11 +- scripts/render-shared-ols-config.sh | 40 ++- scripts/tests/lsphp-info-probe.test.sh | 308 ++++++++++++++++++ 7 files changed, 543 insertions(+), 14 deletions(-) create mode 100755 scripts/tests/lsphp-info-probe.test.sh diff --git a/.gitea/workflows/build-push.yaml b/.gitea/workflows/build-push.yaml index b290d1a..be2ce9d 100644 --- a/.gitea/workflows/build-push.yaml +++ b/.gitea/workflows/build-push.yaml @@ -6,6 +6,56 @@ on: - trunk jobs: + # Shell gate. Runs FIRST and costs seconds; the images below do not depend on + # it (a red job here does not block a push that is otherwise fine), but it is + # the only place the ENTRYPOINT logic is executed at all. The .phpt suite and + # the Dockerfile's `lsphp -i` probe both test the extension, and both were + # green for the release whose entrypoint declared that same extension + # missing — see scripts/tests/lsphp-info-probe.test.sh for what went wrong + # and why it needed a test outside the image build to catch it. + Shell-Checks: + runs-on: ubuntu-latest + steps: + - name: Checkout + uses: actions/checkout@v4 + + # Runner images differ on whether they are root and whether sudo exists. + - name: Install shellcheck + run: | + if ! command -v shellcheck >/dev/null 2>&1; then + (apt-get update && apt-get install -y shellcheck) || + (sudo apt-get update && sudo apt-get install -y shellcheck) + fi + shellcheck --version + + - name: Syntax check every shell script + run: | + set -euo pipefail + find scripts ext -name '*.sh' -print0 | xargs -0 -n1 bash -n + + # Deliberately NOT repo-wide. The older scripts (entrypoint.sh, + # entrypoint-fpm.sh, create-vhost.sh, create-php-config.sh, + # detect-memory.sh) carry pre-existing SC2154/SC2027 findings that predate + # this job; listing them here would make the gate red on arrival and + # therefore ignored. This is the set that is clean today — the scripts + # that run `set -o pipefail` plus the tests. Add files as they are fixed; + # do not add one that is not yet clean. + - name: shellcheck (warnings and above, on the clean set) + run: | + shellcheck -S warning \ + scripts/entrypoint-lsphp.sh \ + scripts/entrypoint-litespeed.sh \ + scripts/entrypoint-shared-ols.sh \ + scripts/render-shared-ols-config.sh \ + scripts/ols-htaccess-watcher.sh \ + scripts/create-vhost-litespeed.sh \ + scripts/install-lscache-wp.sh \ + scripts/tune-mpm.sh \ + scripts/tests/lsphp-info-probe.test.sh + + - name: lsphp probe regression test + run: ./scripts/tests/lsphp-info-probe.test.sh + Build-and-Push: runs-on: ubuntu-latest strategy: diff --git a/ext/cac-path-parity/tests/fpm-parity-check.sh b/ext/cac-path-parity/tests/fpm-parity-check.sh index ae27361..cfa1f20 100755 --- a/ext/cac-path-parity/tests/fpm-parity-check.sh +++ b/ext/cac-path-parity/tests/fpm-parity-check.sh @@ -60,13 +60,28 @@ command -v cgi-fcgi >/dev/null || { echo "SKIP: cgi-fcgi not installed (apt inst [ -n "$FPM_BIN" ] && [ -x "$FPM_BIN" ] || { echo "SKIP: php-fpm not found (pass it as \$2)"; exit 0; } [ -f "$EXT_SO" ] || { echo "SKIP: $EXT_SO not built (run phpize && ./configure && make)"; exit 0; } -echo "php-fpm: $FPM_BIN ($("$FPM_BIN" -n -v 2>/dev/null | head -1))" +## `${VAR%%$'\n'*}` rather than `| head -1`: same first line, no pipeline, so +## nothing here can be decided by a SIGPIPE race under the pipefail on line 39. +## This one only ever fed an echo, so it could not have misled anyone — it is +## changed so that "no pipefail script in this repo pipes into an early-exit +## reader" stays a rule with no exceptions to remember. +FPM_VERSION=$("$FPM_BIN" -n -v 2>/dev/null || true) +echo "php-fpm: $FPM_BIN (${FPM_VERSION%%$'\n'*})" echo "extension: $EXT_SO" ## Pre-flight. If the .so will not load into THIS php-fpm (PHP API mismatch is ## the usual cause) every assertion below would fail identically and blame the ## extension's logic. Say what actually happened instead. -if ! "$FPM_BIN" -n -d "extension=$EXT_SO" -m 2>/dev/null | grep -qx 'cac_path_parity'; then +## Captured into a variable and matched with a here-string, not piped into +## `grep -qx`. `grep -q` exits on its first match, and with `set -o pipefail` +## (line 39) a writer still writing at that moment dies 141 and the pipeline +## reads FALSE — announcing "cannot load the extension" *because* the extension +## was listed. `php-fpm -m` is ~1 KB and loses that race only rarely, but this +## pre-flight exists precisely to stop a harness malfunction being reported as +## an extension fault, so it must not have one of its own. (The same construct +## on 40 KB of `lsphp -i` is what broke entrypoint-lsphp.sh in production.) +FPM_MODULES=$("$FPM_BIN" -n -d "extension=$EXT_SO" -m 2>/dev/null || true) +if ! grep -qx 'cac_path_parity' <<<"$FPM_MODULES"; then echo "HARNESS FAILURE: $FPM_BIN cannot load $EXT_SO" >&2 "$FPM_BIN" -n -d "extension=$EXT_SO" -m 2>&1 | grep -i 'unable\|warning\|error' >&2 echo " The .so must be built against the same PHP as this php-fpm binary." >&2 @@ -156,6 +171,10 @@ run_case() { local pid=$! out="" for _ in $(seq 1 40); do sleep 0.15 + ## SC1007: `QUERY_STRING=` IS the intent — an empty FastCGI param in the + ## per-command environment prefix, exactly as a webserver sends it for a + ## URL with no query string. Not a truncated assignment. + # shellcheck disable=SC1007 out=$(SCRIPT_FILENAME="$DOCROOT/probe.php" DOCUMENT_ROOT="$DOCROOT" \ SCRIPT_NAME=/probe.php REQUEST_METHOD=GET QUERY_STRING= \ cgi-fcgi -bind -connect "127.0.0.1:$PORT" 2>/dev/null) diff --git a/scripts/entrypoint-litespeed.sh b/scripts/entrypoint-litespeed.sh index e9cc18f..88873bd 100644 --- a/scripts/entrypoint-litespeed.sh +++ b/scripts/entrypoint-litespeed.sh @@ -67,7 +67,19 @@ fi ## see lsphp's PHP errors in the exact same file on the new image. ## Rendered as a tiny ini in lsphp's scan dir; PHP merges it after the ## production-tuning overrides at startup. -SCAN_DIR=$(/usr/local/lsws/lsphp${PHPVER}/bin/lsphp -i 2>/dev/null | awk -F'=> ' '/^Scan this dir/ {print $2; exit}') +## Captured in two steps on purpose. As a single pipeline this was +## `lsphp -i | awk '…{print;exit}'`: awk stops at the "Scan this dir" line, +## which sits in the first few hundred bytes of ~40 KB of output, so lsphp can +## still be writing when awk closes the pipe. It then dies 141, `set -o +## pipefail` (line 12) makes that the pipeline's status, and because this is a +## bare assignment `set -e` KILLS PID 1 — the container never starts, on a +## machine where the race falls the wrong way. (Its twin in entrypoint-lsphp.sh +## chose a degraded fallback instead; this one just exits.) Reading into a +## variable first leaves awk's own status as the assignment's, and `|| true` +## keeps a genuinely failing lsphp as an empty SCAN_DIR — which the `-n` test +## below already handles — rather than as a boot failure. +LSPHP_INFO=$(/usr/local/lsws/lsphp"${PHPVER}"/bin/lsphp -i 2>/dev/null || true) +SCAN_DIR=$(awk -F'=> ' '/^Scan this dir/ {print $2; exit}' <<<"$LSPHP_INFO") if [ -n "$SCAN_DIR" ]; then cat > "$SCAN_DIR/99-user-error-log.ini" </dev/null | grep -qi 'running with pid'; } +## +## Read into a variable and match with a here-string rather than piping into +## `grep -qi`: `grep -q` closes the pipe on its first match, and under the +## `set -o pipefail` at the top of this file a writer that is still writing when +## that happens dies 141 and the pipeline reports FALSE — i.e. "OLS is down" +## precisely because the "running" line matched. (Same defect that shipped in +## entrypoint-lsphp.sh's cac_path_parity probe.) `lswsctrl status` prints one +## short line, so today it wins the race every time; the bound that makes that +## true is a vendor script's output, not something this repo controls, and the +## failure it would cause here — a spurious relaunch of a healthy OLS, five of +## which trip the crash-loop cap and exit PID 1 — is expensive enough not to +## rest on it. A non-zero `lswsctrl` still means "not running", exactly as +## pipefail made it mean before. +ols_running() { + local st + st=$(/usr/local/lsws/bin/lswsctrl status 2>/dev/null) || return 1 + grep -qi 'running with pid' <<<"$st" +} ## Crash-loop cap: if OLS can't stay up, bail out so Docker's restart policy and ## the site-health monitor escalate instead of us hot-looping forever. diff --git a/scripts/entrypoint-lsphp.sh b/scripts/entrypoint-lsphp.sh index aeaf1f4..1b17be7 100644 --- a/scripts/entrypoint-lsphp.sh +++ b/scripts/entrypoint-lsphp.sh @@ -173,9 +173,68 @@ validate_ini_num() { ## `-i` ONLY: lsphp is the LSAPI SAPI, not the CLI — it accepts just ## -[b|c|n|h|i|q|s|v|?] and answers `-m`/`-r` by printing usage and exiting 0, so ## a `lsphp -m | grep` test never matches and never errors either. +## +## ---- CAC-TEST: probe helpers BEGIN ---- +## Everything between these two markers is extracted verbatim and executed by +## scripts/tests/lsphp-info-probe.test.sh — the markers are inert comments with +## no runtime effect, and they exist so the test exercises THE SHIPPED CODE +## rather than a copy of it that can drift away from it. +## +## WHY THESE READ `$1` FROM A HERE-STRING AND NOT A PIPELINE. Both probes used +## to be `printf '%s\n' "$LSPHP_INFO" | `. `lsphp -i` is ~40 KB and both +## readers stop early — `grep -q` on first match, `awk` at `exit` — so the +## reader can close the pipe while printf is still writing to it. printf then +## takes SIGPIPE and dies 141, `set -o pipefail` (line 34) adopts 141 as the +## PIPELINE's status, and the test reads FALSE **because the thing it was +## looking for was present early enough to stop the reader**. Measured on whp02 +## against the published cac-lsphp:php83: 5/5 runs status=141 with pipefail, +## 0 without. +## +## It reproduces on some hosts and not others, and the reason is the PIPE +## CAPACITY, not the payload alone. While the writer's whole output fits in the +## pipe it never blocks and always finishes before the reader can act; once it +## does not fit, the early exit is a guaranteed SIGPIPE. Linux gives a pipe +## 64 KiB by default — 40 KB fits, which is why this same image measured 0/10 +## on the build host here — but drops NEW pipes to a single page once a user +## passes fs.pipe-user-pages-soft, which is the state a busy production host +## lives in. Forcing the payload over the limit makes it deterministic +## everywhere: 3x this output = 122100 bytes gave 141 141 141 in this very +## image. "It worked when I ran it" was never evidence about this bug. +## +## A here-string is not a pipeline at all: the shell materialises the whole +## string first (temp file, or a pipe only when it provably fits the pipe +## buffer) and the command's status is the reader's own status, so there is no +## second status for pipefail to prefer and no writer left alive to signal. +## `case`/`[[ ]]` would also avoid the pipeline, but would mean re-expressing an +## anchored line match as a glob over embedded newlines; keeping grep/awk with +## the SAME patterns makes this a plumbing change and nothing else. +## +## Both return the reader's status, so a genuinely-absent extension is still a +## clean 1 and a genuinely-missing "Scan this dir" line is still empty output. +lsphp_info_has_parity_ext() { + grep -q '^cac_path_parity support => enabled$' <<<"$1" +} +lsphp_info_scan_dir() { + awk -F'=> ' '/^Scan this dir/ {print $2; exit}' <<<"$1" +} +## Did `lsphp -i` answer at all? Separates "the extension is not there" from +## "our probe produced nothing to look in", so neither gets reported as the +## other. Keyed on the phpinfo banner, which is line 2 of every `lsphp -i` +## (verified against lsphp83 8.3.32) and is not something LSPHP_INFO could +## contain from any other source. +lsphp_info_is_usable() { + grep -q '^PHP Version => ' <<<"$1" +} +## ---- CAC-TEST: probe helpers END ---- + PATH_PARITY_MODE="none" LSPHP_INFO=$("$LSPHP_BIN" -i 2>/dev/null || true) -SCAN_DIR=$(printf '%s\n' "$LSPHP_INFO" | awk -F'=> ' '/^Scan this dir/ {print $2; exit}') +SCAN_DIR=$(lsphp_info_scan_dir "$LSPHP_INFO") +## `|| true` above is what keeps a broken probe survivable: fail-open is +## deliberate here and below — the site serves either way, only the $_SERVER +## strings differ. What the failure gets REPORTED as is handled at each of the +## two places it changes the outcome (the parity branch, and the no-scan-dir +## else at the bottom of this block). if [ -n "$SCAN_DIR" ]; then mkdir -p "$SCAN_DIR" ## Values emitted double-quoted via printf rather than interpolated into an @@ -225,7 +284,7 @@ if [ -n "$SCAN_DIR" ]; then ## the request path is unaffected. rm -f "$SCAN_DIR/99-cac-path-parity.ini" "$SCAN_DIR/99-cac-lsphp-normalize.ini" PATH_PARITY_MODE="none (user/domain rejected)" - elif printf '%s\n' "$LSPHP_INFO" | grep -q '^cac_path_parity support => enabled$'; then + elif lsphp_info_has_parity_ext "$LSPHP_INFO"; then { echo '; rendered at container start by entrypoint-lsphp.sh' printf 'cac_path_parity.from = "%s"\n' "$OLS_SITE_PATH" @@ -241,12 +300,25 @@ if [ -n "$SCAN_DIR" ]; then ## where it failed to load). Restores the old, .user.ini-defeatable ## behaviour rather than losing normalisation entirely — but say so loudly, ## because in this mode parity is NOT guaranteed. + ## + ## FAIL-OPEN, DELIBERATELY: a probe that cannot answer must never stop the + ## container. The site serves either way; only the $_SERVER strings differ. cat > "$SCAN_DIR/99-cac-lsphp-normalize.ini" <<'EOF' ; rendered at container start by entrypoint-lsphp.sh (DEGRADED FALLBACK) auto_prepend_file = /scripts/cac-lsphp-normalize.php EOF + ## ...but do not DIAGNOSE more than was established. The old wording said + ## "extension not loadable in this image" for EVERY reason this branch is + ## reached — including the probe breaking on its own, which is exactly what + ## happened (see the SIGPIPE note on the helpers above): a false verdict + ## that sent operators to rebuild an image whose extension was fine and + ## whose build gate had passed. The claim now carries its evidence, and the + ## evidence is real: reaching here at all means SCAN_DIR was parsed out of + ## this same output, so `lsphp -i` did answer and its module list is + ## authoritative. The case where it did NOT answer never gets here — it is + ## caught and reported honestly at the `lsphp_info_is_usable` check above. PATH_PARITY_MODE="auto_prepend (DEGRADED)" - echo "WARNING: entrypoint-lsphp: cac_path_parity extension not loadable in this image — falling back to the auto_prepend normaliser, which a site's own .user.ini auto_prepend_file will silently displace. Rebuild/repull cac-lsphp:php${PHPVER}." >&2 + echo "WARNING: entrypoint-lsphp: cac_path_parity extension not loadable in this image — '${LSPHP_BIN} -i' answered (${#LSPHP_INFO} bytes, scan dir ${SCAN_DIR}) and does not list it — falling back to the auto_prepend normaliser, which a site's own .user.ini auto_prepend_file will silently displace. Rebuild/repull cac-lsphp:php${PHPVER}." >&2 fi ## Per-site opcache override (panel: Advanced Tuning → OpCache size); falls ## back to the baked lsphp-overrides.ini defaults when unset. @@ -307,7 +379,19 @@ else ## No scan dir means none of the per-site ini drop-ins land — including the ## path-parity mapping. Previously this failed silently; it must not, because ## the tier's cac-fpm parity guarantee is one of the things lost. - echo "WARNING: entrypoint-lsphp: lsphp reports no additional-ini scan dir — per-site error_log, opcache and \$_SERVER path-parity settings were NOT applied." >&2 + ## + ## Two different things land here and they are not the same report. "lsphp + ## reports no additional-ini scan dir" ASSERTS that lsphp answered us, which + ## is false when the probe produced nothing at all — and that was the wrong + ## half of the same mistake the parity branch above made: describing a probe + ## that could not answer as a finding about the image. Say which one it was. + if lsphp_info_is_usable "$LSPHP_INFO"; then + echo "WARNING: entrypoint-lsphp: lsphp reports no additional-ini scan dir — per-site error_log, opcache and \$_SERVER path-parity settings were NOT applied." >&2 + PATH_PARITY_MODE="none (no scan dir)" + else + echo "WARNING: entrypoint-lsphp: '${LSPHP_BIN} -i' produced no usable phpinfo output (${#LSPHP_INFO} bytes) — per-site error_log, opcache and \$_SERVER path-parity settings were NOT applied. This is a PROBE failure and establishes nothing about what the image contains; run '${LSPHP_BIN} -i' in this container before concluding anything about it." >&2 + PATH_PARITY_MODE="none (lsphp -i unusable)" + fi fi echo "entrypoint-lsphp: \$_SERVER path parity = ${PATH_PARITY_MODE} (${OLS_SITE_PATH} -> /home/${user})" diff --git a/scripts/entrypoint-shared-ols.sh b/scripts/entrypoint-shared-ols.sh index ceb4e5e..425f026 100644 --- a/scripts/entrypoint-shared-ols.sh +++ b/scripts/entrypoint-shared-ols.sh @@ -75,7 +75,16 @@ term_handler() { } trap term_handler TERM INT -ols_running() { /usr/local/lsws/bin/lswsctrl status 2>/dev/null | grep -qi 'running with pid'; } +## Variable + here-string, not a pipe into `grep -qi` — see the long note on the +## identical function in entrypoint-litespeed.sh: `grep -q` closing the pipe on +## a match can leave the writer dying 141, and `set -o pipefail` (line 14) turns +## that into "OLS is down" *because* the running line matched. A non-zero +## `lswsctrl` still counts as not running, as pipefail made it count before. +ols_running() { + local st + st=$(/usr/local/lsws/bin/lswsctrl status 2>/dev/null) || return 1 + grep -qi 'running with pid' <<<"$st" +} MAX_STARTS=5 WINDOW=60 diff --git a/scripts/render-shared-ols-config.sh b/scripts/render-shared-ols-config.sh index 873d121..63b0fc6 100644 --- a/scripts/render-shared-ols-config.sh +++ b/scripts/render-shared-ols-config.sh @@ -82,18 +82,48 @@ awk ' } >> "$TMP" ## --- 4. emit per-site vhost stanzas + collect listener map lines --- +## +## First value of KEY= in a site.meta, as plain data. This replaces +## `sed -n 's/^KEY=//p' "$meta" | head -1`, which was a pipeline whose reader +## (`head -1`) exits after one line while the writer (`sed`) may still be +## flushing: the writer then dies 141, and `set -euo pipefail` (line 22) makes +## the whole ASSIGNMENT fail, which aborts this script mid-render. A truncated +## httpd_config.conf is never written (the render is atomic), but the effect is +## that a site the panel just provisioned silently never appears in the config +## and every subsequent render fails the same way. +## +## Measured in this image, `sed -n 's/^DOMAINS=//p' | head -1`: +## 400 matching lines (~6 KB of sed output) -> 0 0 0 0 0 +## 6000 matching lines (~90 KB of sed output) -> 141 141 141 +## The threshold is the PIPE CAPACITY, not "is the file small": while the +## writer's whole output fits, it never blocks and always finishes first; +## once it does not, the reader's early exit is a guaranteed SIGPIPE. Linux +## gives a pipe 64 KiB by default but drops NEW pipes to a single page once a +## user passes fs.pipe-user-pages-soft, which is the state a busy host gets +## into — and the reason a 40 KB probe failed 5/5 on whp02 and 0/10 here. +## So "a site.meta would never be that big" is not a bound worth resting on +## for panel-written input we do not validate. +## +## awk reads the FILE directly and stops at the first hit: no pipeline, so +## nothing for pipefail to adopt. Same semantics as before, verified against +## the old form on duplicate keys, decoy keys (`notVHNAME=`), empty values and +## missing keys: first match wins, the rest of the line is the value, verbatim. +meta_value() { + awk -v k="$1" 'index($0, k "=") == 1 { print substr($0, length(k) + 2); exit }' "$2" +} + maps="" site_count=0 for meta in "$SITES_ROOT"/*/site.meta; do [ -e "$meta" ] || continue sdir=$(dirname "$meta") - ## PARSE site.meta with sed — do NOT `source` it. The panel writes these values + ## EXTRACT from site.meta — do NOT `source` it. The panel writes these values ## (derived from DB domains), so they should be safe, but sourcing paneldata as ## shell would execute any metacharacters as root in this container if a value - ## ever slipped validation. sed extraction treats them as plain data. - VHNAME=$(sed -n 's/^VHNAME=//p' "$meta" | head -1) - VHROOT=$(sed -n 's/^VHROOT=//p' "$meta" | head -1) - DOMAINS=$(sed -n 's/^DOMAINS=//p' "$meta" | head -1) + ## ever slipped validation. meta_value treats them as plain data. + VHNAME=$(meta_value VHNAME "$meta") + VHROOT=$(meta_value VHROOT "$meta") + DOMAINS=$(meta_value DOMAINS "$meta") if [ -z "$VHNAME" ] || [ -z "$VHROOT" ] || [ -z "$DOMAINS" ] || [ ! -f "$sdir/vhconf.conf" ]; then echo "render-shared-ols: skipping $sdir (incomplete: VHNAME/VHROOT/DOMAINS/vhconf.conf)" >&2 continue diff --git a/scripts/tests/lsphp-info-probe.test.sh b/scripts/tests/lsphp-info-probe.test.sh new file mode 100755 index 0000000..74714b3 --- /dev/null +++ b/scripts/tests/lsphp-info-probe.test.sh @@ -0,0 +1,308 @@ +#!/usr/bin/env bash +## lsphp-info-probe.test.sh — regression test for the SIGPIPE-under-pipefail +## class of bug that shipped in cac-lsphp (trunk 9343a56) and silently turned +## $_SERVER path parity off on every shared-ols site whose host lost the race. +## +## THE BUG. entrypoint-lsphp.sh asked "is cac_path_parity loaded?" with +## +## printf '%s\n' "$LSPHP_INFO" | grep -q '^cac_path_parity support => enabled$' +## +## under `set -euo pipefail`. `grep -q` exits the instant it matches; printf is +## still pushing the remaining ~40 KB of `lsphp -i` into the pipe, takes +## SIGPIPE, and exits 141; pipefail prefers that over grep's 0. So the test read +## FALSE **because the extension was present** — present early enough in the +## output to stop the reader — and the container fell back to the degraded +## auto_prepend normaliser the extension exists to replace, while telling the +## operator the extension was "not loadable in this image". +## +## WHY THE EXISTING SUITE DID NOT CATCH IT. The .phpt suite and +## fpm-parity-check.sh both test the EXTENSION; nothing executed the +## ENTRYPOINT's branch logic, and the Dockerfile's own `lsphp -i | grep -q` +## build gate runs under `bash -c 'set -e'` with NO pipefail, so it reported the +## extension present in the very image whose entrypoint declared it missing. +## +## WHAT THIS ASSERTS. +## 1. behaviour — the SHIPPED probe helpers (extracted verbatim from +## entrypoint-lsphp.sh, never copied, so they cannot drift) return the +## right answer AND a clean exit status under `set -euo pipefail` with a +## realistic ~40 KB phpinfo body. +## 2. structure — no script in this repo that enables pipefail pipes into a +## reader that can exit before its writer finishes. This is the check that +## fails deterministically against trunk; assertion 1's *old* form is a +## RACE (measured 141 on 3 of 5 runs here, 5 of 5 on whp02, and 0 of 5 +## under a different Docker daemon), so no behavioural assertion about the +## broken code could be trusted to fail on every machine. Section 4 runs +## the old form anyway and prints what it did, for the record. +## +## SCOPE LIMITS, stated rather than hidden. The structural scan is a text scan, +## so it under-reports in two known ways: +## - it reads one line at a time. The repo's only multi-line pipeline +## (ols-htaccess-watcher.sh's `inotifywait … |` / `while read`) is invisible +## to it and was reviewed by hand: that reader loops until EOF, i.e. until +## the writer has already gone, so it cannot produce this failure. +## - its quote stripping is flat, so a pipe nested inside a command +## substitution inside a quoted string (`echo "x ($(a | head -1))"`) is read +## as quoted text and skipped. +## It is a guard against reintroducing the shape, not a proof of its absence. +## +## Usage: scripts/tests/lsphp-info-probe.test.sh [REPO_ROOT] +## Exit: 0 all assertions passed, 1 an assertion FAILED, 2 the test could not run. + +set -uo pipefail + +HERE="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +ROOT="${1:-$(cd "$HERE/../.." && pwd)}" +ENTRYPOINT="$ROOT/scripts/entrypoint-lsphp.sh" +TMP="$(mktemp -d)" +trap 'rm -rf "$TMP"' EXIT + +pass=0; fail=0 +ok() { pass=$((pass+1)); echo " ok — $1"; } +bad() { fail=$((fail+1)); echo " FAIL — $1" >&2; } +die() { echo "HARNESS FAILURE: $*" >&2; exit 2; } + +[ -f "$ENTRYPOINT" ] || die "no entrypoint at $ENTRYPOINT (pass REPO_ROOT as \$1)" + +## --------------------------------------------------------------------------- +## 1. Extract the real probe helpers. Not a copy: whatever the shipped +## entrypoint says between the markers is what runs below, so a future edit +## that reintroduces a pipeline is tested, not narrated. +## +## A missing block is an assertion FAILURE, not a harness error, and it does +## not stop the run: against a pre-fix checkout the probes are still inline +## pipelines, and section 5 below is what names them. Bailing out here would +## have replaced that report with "could not run". +## --------------------------------------------------------------------------- +HAVE_HELPERS=yes +awk '/^## ---- CAC-TEST: probe helpers BEGIN ----$/{f=1} f{print} /^## ---- CAC-TEST: probe helpers END ----$/{exit}' \ + "$ENTRYPOINT" > "$TMP/helpers.sh" +if [ -s "$TMP/helpers.sh" ]; then + for fn in lsphp_info_has_parity_ext lsphp_info_scan_dir lsphp_info_is_usable; do + if ! grep -q "^${fn}()" "$TMP/helpers.sh"; then + HAVE_HELPERS=no + bad "the probe-helper block in ${ENTRYPOINT#"$ROOT"/} defines no ${fn}()" + fi + done +else + HAVE_HELPERS=no + bad "no probe-helper markers in ${ENTRYPOINT#"$ROOT"/} — either they were removed, or this is a pre-fix checkout where the probes are still inline pipelines (trunk 9343a56). Section 5 says which lines." +fi + +## --------------------------------------------------------------------------- +## 2. Fixtures. Shaped like real `lsphp -i`: the two lines the probes look for +## sit near the TOP (that is what lets a reader stop early), with tens of KB +## of body after them. A fixture whose marker is near the end could not +## SIGPIPE at all and would make this whole test vacuous, so both properties +## are asserted before anything else runs. +## --------------------------------------------------------------------------- +SCAN_PATH='/usr/local/lsws/lsphp83/etc/php/8.3/mods-available' +make_fixture() { # $1=outfile $2=yes|no (include the cac_path_parity line) + { + printf 'phpinfo()\nPHP Version => 8.3.27\n\n' + printf 'System => Linux 6ecb4e0a1c1f 5.15.0 #1 SMP x86_64\n' + printf 'Server API => LiteSpeed V8.3\n' + printf 'Configuration File (php.ini) Path => /usr/local/lsws/lsphp83/etc/php/8.3/litespeed\n' + printf 'Scan this dir for additional .ini files => %s\n' "$SCAN_PATH" + printf 'PHP API => 20230831\nDebug Build => no\nThread Safety => disabled\n\n' + printf 'bcmath\n\nBCMath support => enabled\n\n' + printf 'calendar\n\nCalendar support => enabled\n\n' + [ "$2" = yes ] && printf 'cac_path_parity\n\ncac_path_parity support => enabled\nRewriting => active\n\n' + printf 'Core\n\nPHP Version => 8.3.27\n\n' + ## ~40 KB of directive rows, exactly the shape phpinfo() prints them in. + local i=0 + while [ "$i" -lt 700 ]; do + printf 'some.directive_%03d => local_value_%03d => master_value_%03d\n' "$i" "$i" "$i" + i=$((i+1)) + done + } > "$1" +} +make_fixture "$TMP/info-present.txt" yes +make_fixture "$TMP/info-absent.txt" no +: > "$TMP/info-empty.txt" + +echo "== fixture sanity ==" +FIX_BYTES=$(wc -c < "$TMP/info-present.txt") +MATCH_OFF=$(grep -b -m1 '^cac_path_parity support => enabled$' "$TMP/info-present.txt" | cut -d: -f1) +if [ "$FIX_BYTES" -ge 32768 ]; then + ok "fixture is ${FIX_BYTES} bytes (realistic 'lsphp -i' is ~40 KB)" +else + bad "fixture is only ${FIX_BYTES} bytes — too small to reproduce the failure" +fi +if [ "$MATCH_OFF" -lt $((FIX_BYTES / 4)) ]; then + ok "match sits at byte ${MATCH_OFF} of ${FIX_BYTES} — most of the body is still unwritten when a reader could stop" +else + bad "match at byte ${MATCH_OFF} of ${FIX_BYTES} leaves too small a tail; the test would be vacuous" +fi + +## --------------------------------------------------------------------------- +## 3. Behaviour, under the REAL option set (`set -euo pipefail`, as line 34 of +## the entrypoint sets it). Each case runs in its own bash process so the +## options and the exit status are the genuine article, not something this +## harness simulated. +## --------------------------------------------------------------------------- +cat > "$TMP/runner.sh" <<'RUNNER' +#!/usr/bin/env bash +set -euo pipefail +# shellcheck disable=SC1091 +source "$1" # the extracted probe helpers +LSPHP_INFO=$(cat "$2") +case "$3" in + has_ext) lsphp_info_has_parity_ext "$LSPHP_INFO" ;; + scan_dir) lsphp_info_scan_dir "$LSPHP_INFO" ;; + usable) lsphp_info_is_usable "$LSPHP_INFO" ;; + legacy) printf '%s\n' "$LSPHP_INFO" | grep -q '^cac_path_parity support => enabled$' ;; + *) echo "unknown case $3" >&2; exit 99 ;; +esac +RUNNER + +## NOT `out=$(run …)`: command substitution runs the function in a subshell, +## where an exit status assigned to RC would be thrown away with it. Run in this +## shell, capture stdout through a file, and RC is the real thing. +RC=0; out="" +run() { # $1=fixture $2=case -> sets RC and out + RC=0 + bash "$TMP/runner.sh" "$TMP/helpers.sh" "$1" "$2" >"$TMP/out" 2>&1 || RC=$? + out=$(cat "$TMP/out") +} + +echo "== the shipped probes under set -euo pipefail ==" +if [ "$HAVE_HELPERS" = no ]; then + echo " (skipped — no probe helpers to run; see the failure above)" +fi +if [ "$HAVE_HELPERS" = yes ]; then + +run "$TMP/info-present.txt" has_ext +if [ "$RC" -eq 0 ]; then + ok "extension present => branch TAKEN (exit 0)" +elif [ "$RC" -eq 141 ]; then + bad "exit 141 (SIGPIPE): the probe is a pipeline again — this is the original bug" +else + bad "extension present => exit $RC (expected 0). Output: $out" +fi + +run "$TMP/info-absent.txt" has_ext +if [ "$RC" -eq 1 ]; then + ok "extension genuinely absent => branch NOT taken (exit 1, a clean 'no')" +else + bad "extension absent => exit $RC (expected 1). Output: $out" +fi + +run "$TMP/info-present.txt" scan_dir +if [ "$RC" -eq 0 ] && [ "$out" = "$SCAN_PATH" ]; then + ok "scan-dir probe returns '$out' (exit 0)" +elif [ "$RC" -eq 141 ]; then + bad "scan-dir probe exit 141 (SIGPIPE) — as a bare assignment under set -e that KILLS PID 1" +else + bad "scan-dir probe => exit $RC, got '$out', expected '$SCAN_PATH'" +fi + +run "$TMP/info-present.txt" usable +if [ "$RC" -eq 0 ]; then ok "usability probe: real phpinfo body => usable (exit 0)" +else bad "usability probe on real body => exit $RC (expected 0)"; fi + +run "$TMP/info-empty.txt" usable +if [ "$RC" -eq 1 ]; then ok "usability probe: empty body => NOT usable (exit 1)" +else bad "usability probe on empty body => exit $RC (expected 1)"; fi + +## The two failure causes must be reported as the different things they are. +## A probe that cannot answer has established nothing about the image, and the +## old message asserted the opposite of that for every reason it fired. +## SC2016: these patterns are the entrypoint's literal text, `$` and all. +# shellcheck disable=SC2016 +if grep -q 'if lsphp_info_is_usable "$LSPHP_INFO"; then' "$ENTRYPOINT" && + grep -q 'This is a PROBE failure and establishes nothing' "$ENTRYPOINT"; then + ok "a probe that produced nothing is reported as OUR failure, not as a verdict on the image" +else + bad "entrypoint no longer reports an unusable 'lsphp -i' as a probe failure" +fi +# shellcheck disable=SC2016 +if grep -q 'not loadable in this image' "$ENTRYPOINT" && + grep -q 'answered (${#LSPHP_INFO} bytes, scan dir ${SCAN_DIR}) and does not list it' "$ENTRYPOINT"; then + ok "the 'extension not loadable' verdict now ships the evidence it rests on" +else + bad "the 'extension not loadable' message no longer states what it observed" +fi + +fi # HAVE_HELPERS + +## --------------------------------------------------------------------------- +## 4. The old form, for the record. NOT asserted: it is a race, and asserting a +## race would make this suite flap on whichever machine happens to win it. +## --------------------------------------------------------------------------- +echo "== the pre-fix pipeline form on the same input (informational) ==" +if [ "$HAVE_HELPERS" = no ]; then + echo " (skipped — the runner needs the helper block to source)" +fi +legacy_rcs="" +if [ "$HAVE_HELPERS" = yes ]; then +for _ in 1 2 3 4 5 6 7 8 9 10; do + run "$TMP/info-present.txt" legacy + legacy_rcs="$legacy_rcs $RC" +done +echo " 10 runs of 'printf | grep -q' under pipefail:${legacy_rcs}" +echo " (0 = happened to finish writing, 141 = writer SIGPIPEd and pipefail reported the" +echo " extension MISSING because it was present. Either value is expected here.)" +fi + +## --------------------------------------------------------------------------- +## 5. Structural scan — the deterministic guard, and the part of this file that +## fails against trunk on EVERY machine. Any script that turns on pipefail +## and pipes into a reader that can stop early has this bug latent in it +## whether or not today's buffer sizes expose it, so the shape is what gets +## outlawed, not the symptom. +## +## It is a text scan, so it is honest about its limits: it strips quoted +## spans and comments (that is what keeps `case a|b)`, `sed "s|x|y|"` and +## `echo "a | b"` from being reported), it requires the reader to be the +## FIRST word after a pipe, and it reads one line at a time. This file is +## skipped because section 4 runs the broken form on purpose. +## --------------------------------------------------------------------------- +echo "== structural scan: early-exit readers in pipefail scripts ==" +mapfile -t PIPEFAIL_FILES < <(grep -rl --include='*.sh' -E '^[[:space:]]*set[[:space:]]+-[a-zA-Z]*[[:space:]]*o?[[:space:]]*pipefail|^[[:space:]]*set[[:space:]]+-o[[:space:]]+pipefail' "$ROOT/scripts" "$ROOT/ext" 2>/dev/null | sort) +[ "${#PIPEFAIL_FILES[@]}" -gt 0 ] || die "found no pipefail-enabled scripts under $ROOT — the scan would be vacuous" + +cat > "$TMP/scan.awk" <<'AWKPROG' +{ + raw = $0 + l = raw + gsub(/\\"/, "X", l); gsub(/\\'/, "X", l) # escaped quotes are not delimiters + while (match(l, /'[^']*'/)) l = substr(l, 1, RSTART-1) "SQ" substr(l, RSTART+RLENGTH) + while (match(l, /"[^"]*"/)) l = substr(l, 1, RSTART-1) "DQ" substr(l, RSTART+RLENGTH) + if (l ~ /^[[:space:]]*#/) next # whole-line comment + sub(/[[:space:]]#.*/, "", l) # trailing comment + gsub(/\|\|/, " ", l) # || is not a pipeline + if (l !~ /\|/) next + n = split(l, seg, "|") + for (i = 2; i <= n; i++) { + r = seg[i] + if (r ~ /^[[:space:]]*grep[[:space:]]+-([[:alnum:]]*q|m)/) { print NR ": " raw; next } + if (r ~ /^[[:space:]]*head([[:space:]]|$)/) { print NR ": " raw; next } + if (r ~ /^[[:space:]]*(while[[:space:]]+)?read([[:space:]]|$)/) { print NR ": " raw; next } + if (r ~ /^[[:space:]]*awk([[:space:]]|$)/ && raw ~ /exit/) { print NR ": " raw; next } + if (r ~ /^[[:space:]]*sed([[:space:]]|$)/ && raw ~ /[;{\/][[:space:]]*[qQ][^[:alnum:]]/) { print NR ": " raw; next } + } +} +AWKPROG + +offenders=0 +for f in "${PIPEFAIL_FILES[@]}"; do + [ "$(basename "$f")" = "$(basename "${BASH_SOURCE[0]}")" ] && continue + while IFS= read -r hit; do + offenders=$((offenders+1)) + echo " FAIL — ${f#"$ROOT"/}:${hit}" >&2 + done < <(awk -f "$TMP/scan.awk" "$f") +done +if [ "$offenders" -eq 0 ]; then + ok "${#PIPEFAIL_FILES[@]} pipefail-enabled scripts, no pipeline whose reader can outrun its writer" +else + bad "$offenders pipeline(s) above pipe into an early-exit reader under pipefail." + echo " Read the value into a variable and match it with a here-string (see" >&2 + echo " lsphp_info_has_parity_ext in scripts/entrypoint-lsphp.sh), or give the" >&2 + echo " reader the file directly. A here-string is not a pipeline, so there is" >&2 + echo " no second exit status for pipefail to prefer." >&2 +fi + +echo +echo "passed: $pass failed: $fail" +[ "$fail" -eq 0 ] || exit 1 +exit 0