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/docs/2026-08-05-boot-posture-and-ci-gating-decision.md b/docs/2026-08-05-boot-posture-and-ci-gating-decision.md new file mode 100644 index 0000000..1d9301d --- /dev/null +++ b/docs/2026-08-05-boot-posture-and-ci-gating-decision.md @@ -0,0 +1,291 @@ +# Decision record: entrypoint boot posture on a failed `lsphp -i` probe, and CI gating for the pipeline-shape check + +Date: 2026-08-05 +Status: recommendations for sign-off; nothing here is implemented by this document. +Background: the SIGPIPE-under-pipefail fix (here-strings replacing `printf | awk` in +`scripts/entrypoint-lsphp.sh` and `scripts/entrypoint-litespeed.sh`) removed a boot-killing +141 and, as a side effect, changed what happens when `lsphp -i` *genuinely* produces +nothing. Two judgement calls were flagged for a human decision. This document is that +decision's working-out. + +--- + +## Call 1 — boot posture when `lsphp -i` genuinely fails + +### What actually happens today (post-fix, as-written in the repo) + +The two container types are **not** currently symmetric, which matters for the decision: + +- **`cac-lsphp`** (`scripts/entrypoint-lsphp.sh`): fail-open, but *not silent*. An empty + `SCAN_DIR` falls to an else-branch that distinguishes "lsphp answered but reported no + scan dir" from "`lsphp -i` produced no usable output at all" (`lsphp_info_is_usable`, + keyed on the `PHP Version =>` banner), prints a distinct `WARNING:` to stderr for each, + and every boot ends with a summary line + `entrypoint-lsphp: $_SERVER path parity = (...)` where `` becomes + `none (no scan dir)` or `none (lsphp -i unusable)`. So `docker logs` carries a + greppable verdict — but nothing on the platform consumes it. +- **`cac-litespeed`** (`scripts/entrypoint-litespeed.sh`, lines 81–102): fail-open and + **completely silent**. `if [ -n "$SCAN_DIR" ] ... fi` with no else. An empty probe means + the per-site `error_log` ini and the opcache override ini are simply never written, and + nothing says so anywhere. +- **Shared tiers** (`entrypoint-shared-ols.sh`, `entrypoint-shared-httpd.sh`): no + `lsphp -i` probe exists in them at all. The question is moot there today, but the + blast-radius argument below still applies if one is ever added. + +What degradation costs the customer, concretely: + +| Lost fragment | cac-lsphp | cac-litespeed | Customer-visible symptom | +|---|---|---|---| +| `99-user-error-log.ini` | yes | yes | PHP errors go to stderr/`docker logs` instead of `/home/$user/logs/php-fpm/error.log`. Baked `log_errors = On` still applies, so errors are *not lost* — they are somewhere the customer can't see and support doesn't look first. Ticket shape: "my error log is empty." | +| `99-user-opcache.ini` | yes | yes | Panel-set opcache overrides silently revert to baked defaults (64 MB / 8000 files). A site tuned up for a big codebase gets cache thrash; hard to attribute. | +| `99-cac-path-parity.ini` | yes | n/a | Extension gets no mapping ⇒ `$_SERVER` path parity with cac-fpm is lost entirely in the "no output" case (the auto_prepend fallback is only written in the "extension missing from module list" branch, which requires the probe to have answered). Symptom: Wordfence/path-sensitive plugins misbehave after migration; extremely hard to diagnose from the outside. | + +### The premise to examine first + +The fail-open argument assumes "the site serves either way; only ini fragments differ." +That is true when `lsphp -i` answers but lacks a `Scan this dir` line (weird packaging, +still a working interpreter). It is **much weaker** when `lsphp -i` produces *nothing*: +the same `$LSPHP_BIN` that just failed to print phpinfo is what the entrypoint `exec`s +as PID 1 to serve requests. A binary that can't run `-i` because of a missing shared +library will not serve LSAPI either — fail-open in that case doesn't buy a serving +container, it buys a container that passes `docker ps`, then wedges or crash-loops +*after* the entrypoint's diagnostic moment has passed. So the two sub-cases deserve +different treatment, and conflating them is the main framing error in a flat +"fail-open vs fail-closed" question. + +But there is a counterweight, and it is specific to this fleet: **`lsphp -i` can fail +transiently on a healthy image.** These hosts have documented fork-starvation episodes +(Committed_AS pinned against CommitLimit during backup windows; it has broken backups +before). A `fork()`/`execve` failure at boot time yields exactly "empty `LSPHP_INFO`" +through the `|| true`. Under fail-closed, a container (re)starting during a memory-pressure +window would refuse to boot **because of the host's state, not its own**, and its restart +policy would then add restart churn to a host already under pressure. The previous +incident has the same shape: the last time "the probe returned nothing useful" happened +in production, the image was *fine* — the probe itself was broken (the SIGPIPE bug), and +the fail-closed behavior converted a cosmetic probe defect into a fleet-wide +fails-to-boot on busy hosts only. Historically, probe-side failures have outnumbered +genuinely-corrupt-image failures **1–0 (or 2–0 counting fork starvation as observed +class)**. That base rate is the strongest single argument against pure fail-closed. + +### Options + +**A. Fail-open as fixed (status quo).** +- Buys: no false boot failures from probe-side defects or host memory pressure; matches + the extension's documented posture; sibling entrypoints consistent (once litespeed + gains the warning it currently lacks). +- Costs: "starts but serves degraded" is precisely the state this platform is worst at + noticing — the container watchdog's filter has historically been `exited|dead|created` + only, blind to running-but-wrong, and its post-start verify checks `State.Running`, so + it would even score a restart of a degraded container as success. Nothing alerts on the + stderr WARNINGs. Realistic outcome of a genuine failure under Option A: **weeks of + silent degradation**, surfaced eventually as a confusing customer ticket ("where are my + errors?" / plugin misbehavior), with the diagnostic clue buried in `docker logs` of a + container nobody had reason to inspect. + +**B. Fail-closed (`exit 1`) whenever `lsphp -i` yields nothing usable.** +- Buys: loud, immediate, operator-visible failure; watchdog catches `exited`; the + diagnostic (the WARNING line) is the *last* thing in `docker logs`, exactly where an + on-call looks first. A genuinely broken image cannot masquerade as healthy for weeks. +- Costs: converts transient host-side failures (fork starvation) and any future + probe-side defect into customer downtime. On a bad-image push, every container that + restarts goes down *hard* — arguably correct (they weren't going to serve anyway), but + it also means a probe regression like the one just fixed would again be a mass boot + outage rather than a mass warning. The customer experience of B's false positive is + strictly worse than A's true positive: total outage vs. degraded-but-serving. +- Note: B is not "the old behavior restored" — the old behavior also failed to boot on + *healthy* images because the probe itself was the broken part. Anyone arguing "the old + crash-loop caught real problems" should first account for the fact that the only + production firing of that crash was a false positive. + +**C. Fail-open + a machine-detectable degraded marker (middle).** +Boot and serve, but make the degraded state a *first-class, monitorable fact* rather than +a log line: +1. On any degraded branch, write a marker file (e.g. `/run/cac-degraded` containing the + mode string) in addition to the existing stderr WARNING, in **both** entrypoints — + and add the currently missing warning + summary line to `entrypoint-litespeed.sh` so + the two siblings really are consistent (today's "consistency" claim is only half true). +2. Retry the probe once or twice with a short sleep before accepting degradation. This + is nearly free and specifically absorbs the fork-starvation transient, which is the + most likely real-world cause of an empty probe on this fleet. +3. Surface the marker: either a periodic platform check (`docker exec ... test -f + /run/cac-degraded` across cac containers, or grepping the boot summary line + `path parity = none` from `docker logs --tail`) wired into existing monitoring/alerting, + or fold it into the health story. **Caution on the healthcheck route:** the + HEALTHCHECKs (all seven Dockerfiles have one) are liveness probes; making "degraded + config" report unhealthy is tempting but risky — if the watchdog ever learns to + restart unhealthy containers, a persistently-degraded-but-serving site would be + restart-looped for a condition a restart cannot fix. Keep degraded ≠ unhealthy; + alert on the marker through a channel that pages a human instead of triggering an + automated restart. + +### Recommendation + +**Option C, with one fail-closed carve-out considered and (narrowly) rejected.** + +The carve-out I weighed: fail closed *only* in the `lsphp -i is unusable` sub-case (probe +empty), fail open in the `answered but no scan dir` sub-case — on the logic that an +interpreter that can't print phpinfo probably can't serve. I recommend against it, for +two reasons. First, the fork-starvation transient lands in exactly that sub-case and +would cause real, host-pressure-correlated boot failures of healthy sites; the retry in +C(2) mitigates but can't eliminate it. Second, if the binary truly cannot execute, the +final `exec "$LSPHP_BIN" -b ...` fails on its own a few lines later and PID 1 dies +anyway — fail-closed-at-the-probe mostly duplicates a crash the container will have +regardless, while adding a new false-positive class. The genuinely dangerous residue — +binary runs but is subtly broken — is better caught by the existing HEALTHCHECK +(TCP :9000 + `pgrep lsphp`), which *does* fail in that world; the platform gap is that +the watchdog ignores unhealthy, and that gap is worth closing on its own merits. + +Per-type answer: same posture for `cac-lsphp` and `cac-litespeed` (their failure domains +are equivalent for this probe; the current behavioral gap between them is an oversight, +not a design). For **shared-tier** entrypoints the answer is *more* strongly fail-open +than for per-site containers, should a similar probe ever appear there: a fail-closed +shared tier takes down every site on the host, and the marginal diagnostic value of a +crash over a marker does not scale with the blast radius. + +Why C over A, stated so it can be disagreed with: A and C serve the customer identically; +they differ only in whether the platform can *notice*. The honest failure-likelihood +ranking on this fleet is (1) transient probe failure under memory pressure, (2) probe +logic regression, (3) genuinely broken image — and only (3) is the case where B +outperforms. C handles (1) with the retry, (2) and (3) with the marker + alert, and its +worst case is "degraded for as long as the alerting channel takes to be read" instead of +A's "degraded for weeks" or B's "down because the host was busy." If you believe a +corrupt image push is *more* likely than a probe/host transient, B becomes defensible — +but the recent incident history says otherwise. + +What a customer experiences, per option, when the failure is genuine: A/C — site up, +error log empty, opcache defaults, possible plugin path weirdness; C additionally gets it +noticed and fixed in hours. B — site down until an operator acts, but the operator is +paged immediately by existing machinery. Which failure gets silently tolerated for weeks: +A's, unambiguously. B's cannot be (crash-loops are loud); C's only if the marker alert is +never actually wired up — which is why C **is not done** until the alerting side ships, +and shipping the marker without the consumer would be A with extra steps. + +Before committing, measure: +- Fleet scan: does `path parity = none` (or an empty scan dir) occur on any live + container today? (`docker logs` grep across cac-lsphp/litespeed containers.) If it + already fires, the fix priority changes from "posture" to "why". +- Frequency of boot-time fork failures during backup windows (correlate container start + timestamps with `MemAvailable` dips) — validates or kills the retry rationale. +- Whether the watchdog's unhealthy-blindness fix (in progress platform-side) will ever + auto-restart on unhealthy; that decides whether degraded may ever map to unhealthy. + +--- + +## Call 2 — should the new CI check gate releases? + +### What actually exists (`.gitea/workflows/build-push.yaml`) + +A `Shell-Checks` job runs on every push to trunk: (1) `bash -n` over every script, +(2) `shellcheck -S warning` over an explicit allow-list of the pipefail-era scripts, +(3) `scripts/tests/lsphp-info-probe.test.sh`, whose section 5 is the structural scan +that outlaws early-exit-reader pipelines in any `pipefail`-enabled script under +`scripts/` and `ext/`. None of the six image build jobs has `needs: Shell-Checks`, and +the workflow's own comment says so: "a red job here does not block a push that is +otherwise fine." So today a red check produces a red run *annotation* while all seven +images publish anyway. + +False-positive surface, honestly assessed: +- `bash -n` and the test's sections 1–4 are deterministic (the pre-fix pipeline is run + only informationally; no pass/fail depends on the race). Near-zero FP risk. +- `shellcheck` runs against a hand-curated clean set, so it goes red only when a listed + file regresses or a new shellcheck version tightens — low FP risk, nonzero. +- The structural scan is a line-oriented awk text scan with flat quote-stripping. Its + own header is candid: it can miss multi-line pipelines (false negatives) and could + misread exotic quoting (false positives). It requires the reader to be the first word + after `|` and special-cases `case a|b)`, `sed "s|x|y|"`, `||`. FP risk is real but + bounded — and an FP is a *visible* red on a specific line, trivially triaged. + +### Options + +**A. Leave it non-blocking (status quo).** +Buys: zero risk that a scanner quirk halts a release. Costs: this platform's own history +says non-blocking signals decay into wallpaper — one-shot installers drifted, the +watchdog's blind spot sat unnoticed, and this exact bug class shipped to production +through a green build gate and **two** code reviews. A check that exists precisely +because the build gate can't see entrypoint runtime, but that cannot stop a release, +protects nothing on the day it matters; it merely timestamps the moment the regression +was ignorable. The failure it would have caught kills PID 1 on busy production hosts +only — the single most expensive place to discover it. + +**B. Hard gate now (add `needs: Shell-Checks` to all six build jobs).** +Buys: the regression class physically cannot ship again through this pipeline. Costs: an +unproven scanner acquires veto power over every release, including emergency ones. Worth +sizing that cost concretely rather than abstractly: releases here are operator-driven +pushes to trunk, not a high-frequency train; a false red costs the minutes needed to read +the offending line, either fix real sloppiness or adjust the scanner, and re-push. There +is no mechanism by which a red silently *delays* an unattended release for days, because +releases are attended. The scary version of "blocks all releases" mostly doesn't apply to +this repo's release model. The one genuinely bad scenario: an urgent security rebuild +blocked at 2 a.m. by a scanner FP — mitigable, since the operator can comment out one +`needs:` line in the same push, an escape hatch that is itself visible in the diff. + +**C. Staged: soft-fail now, hard gate after N clean runs.** +Buys: evidence before enforcement; if the scanner has an FP mode, it is discovered while +red is cheap. Costs: N pushes of window during which the bug class can ship again, and a +follow-up task of exactly the kind this platform historically forgets (see: one-shot +installer drift). If staged, the flip must be a dated calendar/scheduler entry or a +tracked ticket, not an intention. + +**D. Split the gate: hard-gate the deterministic steps now, soft-fail the scanner.** +Put `bash -n` + the probe regression test (sections 1–4) in a blocking job immediately — +they have effectively no FP surface and section 2's marker check alone prevents the +specific regression (reintroducing the inline pipelines) — and leave shellcheck + the +structural scan non-blocking until proven. Buys: immediate protection against the exact +bug that happened, zero new FP-block risk. Costs: two jobs to maintain; the structural +scan (the only piece that generalizes beyond this one probe) stays advisory. + +### Recommendation + +**B — make it a hard gate now**, with C's discipline applied only to future *additions* +to the check, and one prerequisite: run the full `Shell-Checks` job as-is against the +current trunk and the last ~5 release tags first (locally or via a manual dispatch) to +confirm it is green on everything already shipped. If that back-test is green, gate. If +it is not, fix or scope the scanner, then gate. + +Reasoning, laid out to be disagreeable-with: the decision hinges on comparing the two +error costs *as they occur in this repo's actual release process*. A false red costs an +attended operator minutes, with a one-line escape hatch, and every FP found makes the +scanner better. A false green — or a true red that doesn't block — reproduces a +production incident whose failure mode is "customer containers refuse to boot, but only +on hosts busy enough to have collapsed pipe buffers," i.e. undetectable in dev by +construction. The asymmetry is roughly minutes-of-operator-time vs. +production-boot-outage, and the probability of FP red is bounded by a back-test we can +run *today* instead of estimating. Choosing C is defensible only if you weight "new check +might be flaky in ways a back-test can't reveal" highly — plausible for a check with +runtime/timing behavior, but sections 1–4 are deterministic and section 5 is a pure text +scan over files that are fully known at back-test time. Flakiness that a back-test can't +surface would have to come from the runner environment (shellcheck version drift is the +main one — worth pinning the shellcheck version when gating). D is the fallback if the +back-test turns up scanner noise that can't be resolved same-day. + +Two obligations come with gating, or the gate rots: +1. The shellcheck allow-list must grow with the repo — a new `set -o pipefail` script + that never gets listed is invisibly exempt from step (2) (the structural scan, by + contrast, auto-discovers pipefail scripts). A one-line CI assertion that every + pipefail-enabled script appears in the shellcheck list would close that seam. +2. Green must not be oversold: the scanner's documented limits (single-line pipelines, + flat quote-stripping) mean a multi-line pipeline reintroduction passes it. The gate + raises the floor; it does not replace review. Record that limit where reviewers see + it (it already is, in the test header — keep it there). + +Before committing, measure: the back-test above (mandatory); shellcheck version on the +Gitea runner vs. pinned (drift is the likeliest future FP source); and after gating, +track red-run causes for the first month — if more than ~1 in 10 reds is a scanner FP, +revisit with D. + +--- + +## Framing notes (where the question as posed deserves push-back) + +1. "Previously it failed loudly and the watchdog caught it" is only half the history: + the *only* production firing of the loud failure was a false positive (the probe's own + SIGPIPE), not a broken image. The old posture's loudness was never demonstrated on a + genuine failure; pricing it as a proven detection mechanism overstates its record. +2. The fail-open "consistency with the sibling" claim is currently aspirational: + `entrypoint-litespeed.sh` fails open *silently* while `entrypoint-lsphp.sh` fails open + loudly with mode strings. Whatever posture is chosen, making the siblings actually + consistent (warning + summary line + marker in both) is part of the work. +3. "Fail-open vs fail-closed" flattens two distinct sub-cases — probe empty vs. probe + answered without a scan dir — that have different likely causes and different serving + prognoses. The entrypoint already distinguishes them in its reporting; the decision + should too (and above, does). diff --git a/ext/cac-path-parity/tests/fpm-parity-check.sh b/ext/cac-path-parity/tests/fpm-parity-check.sh index ae27361..c4974e6 100755 --- a/ext/cac-path-parity/tests/fpm-parity-check.sh +++ b/ext/cac-path-parity/tests/fpm-parity-check.sh @@ -60,13 +60,37 @@ 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. The reason to change it is structural, not that `php-fpm -m` is +## small: there is no payload size that makes this shape safe (41 KB SIGPIPEs +## about 11% of the time into a 64 KB pipe — see the note over the probe helpers +## in scripts/entrypoint-lsphp.sh), and 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.) +## +## A here-string, not the `[[ ]]` form those helpers use, on purpose: `<<<` +## spills to /tmp/sh-thd.XXXXXX above ~4-64 KB depending on the bash build, so +## it is a writable-temp-dir precondition, which is unacceptable on a boot path +## and irrelevant here — `php-fpm -m` is ~1 KB, and this harness has already +## created a docroot and a pool config by the time it runs. +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 +180,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..cfa3753 100644 --- a/scripts/entrypoint-litespeed.sh +++ b/scripts/entrypoint-litespeed.sh @@ -67,7 +67,46 @@ 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, then matched in the shell. As a single pipeline this was +## `lsphp -i | awk '…{print;exit}'`: awk stops at the "Scan this dir" line, +## which is near the top of the output, so lsphp can still be writing when awk +## closes the pipe. lsphp 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. (Its twin in entrypoint-lsphp.sh chose a +## degraded fallback instead; this one just exits.) That is a race on whether +## the reader closes before the writer's last write() returns, not a function of +## how big the output is: see the long note over the probe helpers in +## entrypoint-lsphp.sh for the measurements. The rule is simply that no +## `writer | early-exiting-reader` belongs in a pipefail script. +## +## A here-string would remove the pipeline, but bash spills a here-string to +## /tmp/sh-thd.XXXXXX above a build-dependent size (65536 for the bash 5.2.21 in +## this image, between 4096 and 16384 for Debian's 5.2.15) — and on this line, +## a bare assignment, a temp file it cannot create is again `set -e` killing +## PID 1: `docker run --read-only` reproduces exactly that. So the extraction is +## done with parameter expansion, which allocates nothing. +## +## Same answer as the awk it replaces: first line starting "Scan this dir", then +## the text between the FIRST and SECOND '=> ' on it (awk's $2 under -F'=> '), +## empty if the line carries no separator, empty if there is no such line. +## `|| true` on the capture keeps a genuinely failing lsphp as an empty +## SCAN_DIR — which the `-n` test below already handles — not a boot failure. +LSPHP_INFO=$(/usr/local/lsws/lsphp"${PHPVER}"/bin/lsphp -i 2>/dev/null || true) +SCAN_DIR="" +## The leading newline is what makes a match on LINE 1 behave like every other +## line, exactly as awk's `^` anchor does. +scan_rest=$'\n'"$LSPHP_INFO" +if [[ $scan_rest == *$'\nScan this dir'* ]]; then + ## `#` takes the SHORTEST prefix, i.e. the FIRST matching line — awk's `exit`. + scan_rest=${scan_rest#*$'\nScan this dir'} + scan_line="Scan this dir${scan_rest%%$'\n'*}" + if [[ $scan_line == *'=> '* ]]; then + SCAN_DIR=${scan_line#*'=> '} + SCAN_DIR=${SCAN_DIR%%'=> '*} + fi + unset scan_line +fi +unset scan_rest 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, which here means a spurious +## relaunch of a healthy OLS, five of which trip the crash-loop cap and exit +## PID 1. (Same defect that shipped in entrypoint-lsphp.sh's cac_path_parity +## probe.) The reason to change it is STRUCTURAL — a pipefail script must not +## pipe into an early-exit reader, whatever the payload — because "lswsctrl +## prints one short line so it always wins" is a size argument, and size +## arguments about this race are wrong: see the measurements over the probe +## helpers in entrypoint-lsphp.sh, where 41 KB SIGPIPEd 11% of the time into a +## 64 KB pipe. A non-zero `lswsctrl` still means "not running", exactly as +## pipefail made it mean before. +## +## A here-string is the right shape HERE, where those helpers use `[[ ]]`: the +## reason to avoid `<<<` there is that bash spills a large here-string to +## /tmp/sh-thd.XXXXXX and so makes a writable temp dir a boot precondition. The +## threshold is 65536 bytes in this image's bash 5.2.21 and no lower than 4096 +## in any bash this repo has met; `lswsctrl status` prints well under 100 bytes +## and cannot approach it, so no temp file is ever created and the case- +## insensitive match stays a plain `grep -i` instead of a hand-rolled glob. +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..ff45d47 100644 --- a/scripts/entrypoint-lsphp.sh +++ b/scripts/entrypoint-lsphp.sh @@ -173,9 +173,127 @@ 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. Keep BOTH markers: the +## extractor refuses to emit anything unless it sees the END one, so a half- +## deleted pair is reported there as a marker error instead of silently +## sourcing the rest of this file. +## +## WHY THESE MATCH `$1` IN THE SHELL AND NEVER PIPE IT INTO A READER. +## +## What broke. Both probes used to be `printf '%s\n' "$LSPHP_INFO" | `, +## and both readers stop early — `grep -q` at its first match, `awk` at `exit`. +## When the reader closes the pipe with the writer still writing, the writer +## takes SIGPIPE and dies 141; `set -o pipefail` (line 34) adopts 141 as the +## PIPELINE's status; and the branch 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. +## +## THE RULE, and it is not about size. ANY `writer | early-exiting-reader` +## under pipefail is a latent 141. PAYLOAD SIZE IS NOT A SAFETY ARGUMENT. Each +## run is decided by a race — whether the reader's close lands before the +## writer's final write() returns — and the payload only sets how many write() +## syscalls the writer has to lose. Measured here against a default +## 65536-byte pipe (confirmed with F_GETPIPE_SZ): +## 41144 bytes -> 141 in 32/300 runs (11%) — well UNDER capacity +## 65012 bytes -> 141 in 25/30 runs — not 100% even AT capacity +## 500 KB into a 1 MiB pipe -> 200/200 SIGPIPE written 4096 bytes at a +## time, 0/200 written as one 500 KB write +## and strace caught printf dying having written 12086 of 40406 bytes into a +## 65536-byte pipe, i.e. losing with 53 KB of room to spare. The reason the same +## image failed 5/5 on whp02 and 10/10 clean in a dev container is the WRITER's +## syscall size: bash <= 5.2.15 pushes ~37 KB per write, bash >= 5.2.21 pushes +## 80-160 bytes, so the newer shell needs hundreds of chances to lose the race +## and the older one needs a couple. (An earlier draft of this comment blamed +## pipe capacity and fs.pipe-user-pages-soft. Both were wrong: that soft limit +## clamps new pipes to two pages rather than one, applies only once a single uid +## holds more than 1024 pipes, and is skipped entirely for CAP_SYS_RESOURCE.) +## +## The only SOUND reasons a call site is safe are structural: +## * the file does not set pipefail; or +## * the reader provably consumes to EOF (no `q`, `-q`, `-l`, `-m`, `exit`, +## `break`); or +## * the pipeline's status is discarded. +## The reader's implementation is not a defence either: at 248 KB, mawk, gawk, +## `grep -q` and `head -1` each returned 141 on 10/10, and these images have +## already drifted between mawk 1.3.4 (cac-lsphp) and gawk 5.2.1 +## (cac-litespeed:php83) — not a property this repo controls. +## +## WHY PURE-BASH MATCHING RATHER THAN A HERE-STRING. `<<<` does remove the +## pipeline, but it is not free: above a build-dependent size bash materialises +## the string as /tmp/sh-thd.XXXXXX, so it makes a writable temp dir a +## PRECONDITION OF BOOTING. Measured in this image (bash 5.2.21) the switch is +## at exactly 65536 bytes and `lsphp -i` is 39934, so the here-string form was +## not hitting disk here — but Debian's bash 5.2.15 switches somewhere between +## 4096 and 16384, where the same payload would. What that costs is not +## theoretical: +## docker run --read-only ... 'SCAN_DIR=$(awk ... <<<"$BIG")' +## -> bash: cannot create temp file for here-document: Read-only file +## system ... and the script is dead: exit 1, PID 1 gone. +## which is the exact boot failure this branch exists to remove, re-acquired +## from a different direction and gated on which bash the base image ships. +## `[[ ]]` and `${...}` allocate nothing and cannot fail that way. Where the +## subject is a couple of hundred bytes and provably cannot approach the +## threshold, a here-string is still fine — see `ols_running` in +## entrypoint-litespeed.sh, which says so at the call site. +## +## THE PATTERNS ARE THE OLD ONES RE-EXPRESSED, NOT APPROXIMATED. +## grep -q '^cac_path_parity support => enabled$' — an anchored whole-line +## match, so the subject is wrapped in a newline at BOTH ends and the glob +## matches \n\n; the wrapping is what keeps the first line and an +## unterminated last line matching exactly as grep matched them. +## grep -q '^PHP Version => ' — anchored at the start +## only, so only a leading newline is added. +## awk -F'=> ' '/^Scan this dir/ {print $2; exit}' — first matching line, +## then the text between the FIRST and SECOND '=> ' on it ($2), or empty if +## there is no separator. `${x#*'=> '}` then `${y%%'=> '*}` is that, exactly. +## scripts/tests/lsphp-info-probe.test.sh asserts this equivalence against the +## grep/awk originals over the edge cases (match on the first line, on the last +## line with no trailing newline, decoy substrings, a second separator, an empty +## value, a missing key), so "same answer as before" is checked, not asserted. +## +## Statuses are unchanged: a genuinely-absent extension is still a clean 1, and +## a genuinely-missing "Scan this dir" line is still empty output with status 0. +lsphp_info_has_parity_ext() { + [[ $'\n'"$1"$'\n' == *$'\ncac_path_parity support => enabled\n'* ]] +} +lsphp_info_scan_dir() { + local rest line val + rest=$'\n'"$1" + [[ $rest == *$'\nScan this dir'* ]] || return 0 + ## `#` takes the SHORTEST prefix, i.e. the FIRST matching line — awk's `exit`. + rest=${rest#*$'\nScan this dir'} + line="Scan this dir${rest%%$'\n'*}" + val="" + if [[ $line == *'=> '* ]]; then + val=${line#*'=> '} + val=${val%%'=> '*} + fi + printf '%s\n' "$val" +} +## 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() { + [[ $'\n'"$1" == *$'\nPHP Version => '* ]] +} +## ---- 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 +343,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 +359,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 +438,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..d376b05 100644 --- a/scripts/entrypoint-shared-ols.sh +++ b/scripts/entrypoint-shared-ols.sh @@ -75,7 +75,20 @@ 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. The reason is +## structural (a pipefail script must not pipe into an early-exit reader), not +## that this particular output is small; and the here-string is safe here for +## the separate reason that `lswsctrl status` is far below the size at which +## bash spills a here-string to a temp file. 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..65a2d86 100644 --- a/scripts/render-shared-ols-config.sh +++ b/scripts/render-shared-ols-config.sh @@ -82,18 +82,61 @@ 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 +## Do not read a threshold into those two rows. There is no size below which +## this is safe: each run is a RACE on whether `head` closes the pipe before +## `sed`'s final write() returns, and the payload only decides how many write() +## syscalls sed has to lose. Measured against a default 65536-byte pipe +## (F_GETPIPE_SZ), 41144 bytes SIGPIPEd on 32 of 300 runs — 11%, well under +## capacity — and 65012 bytes still only on 25 of 30, so it is neither safe +## below capacity nor certain at it; strace caught a writer dying having put +## 12086 of 40406 bytes into a 65536-byte pipe. What actually separated a host +## that failed 5/5 from one that passed 10/10 was the WRITER's syscall size +## (bash <= 5.2.15 writes ~37 KB at a time, >= 5.2.21 writes 80-160 bytes), not +## the host's pipe capacity. (An earlier draft of this comment blamed +## fs.pipe-user-pages-soft; that limit clamps new pipes to two pages, not one, +## only past 1024 pipes for one uid, and never for CAP_SYS_RESOURCE.) +## +## So the rule this file follows is structural, not statistical: under pipefail, +## a writer piped into a reader that can stop early (`head`, `grep -q`/`-l`/`-m`, +## `sed q`, `awk ... exit`, `read`) is a latent 141 — full stop. A call site is +## only sound when the file does not set pipefail, or the reader provably runs +## to EOF, or the status is thrown away. "A site.meta would never be that big" +## was never one of those, least of all 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..88fc467 --- /dev/null +++ b/scripts/tests/lsphp-info-probe.test.sh @@ -0,0 +1,564 @@ +#!/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 rest 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". +## +## THE RULE THIS FILE ENFORCES, stated so nobody re-derives a wrong one: ANY +## `writer | early-exiting-reader` under pipefail is a latent 141. PAYLOAD SIZE +## IS NOT A SAFETY ARGUMENT — each run is a race on whether the reader's close +## lands before the writer's final write() returns, and size only sets how many +## write() syscalls the writer must survive. Measured against a default +## 65536-byte pipe: 41144 bytes SIGPIPEd on 32/300 runs (11%, well UNDER +## capacity) and 65012 bytes on 25/30 (not certain even AT capacity); 500 KB +## into a 1 MiB pipe was 200/200 when written 4096 bytes at a time and 0/200 as +## a single write. A call site is sound only for a STRUCTURAL reason: the file +## does not set pipefail, or the reader provably consumes to EOF (no `q`, `-q`, +## `-l`, `-m`, `exit`, `break`), or the pipeline's status is discarded. Section 4 +## below prints this race happening at ~41 KB; if the numbers there ever read as +## "small payloads are fine", the numbers are right and the reading is wrong. +## +## 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. +## 3. equivalence — the helpers' pure-bash matching answers exactly what the +## grep/awk patterns they replaced answer, checked case by case against +## those same patterns reading a FILE (a file, so the reference itself +## cannot SIGPIPE). Section 6. +## 4. the scan's own coverage — every reader shape section 5 claims to catch +## is caught, and a matched set of safe shapes is NOT flagged. Section 7. +## Without this the scan's regexes are unfalsified and can quietly stop +## matching; `grep -l` and `sed q` were both missed until section 7 existed. +## +## SCOPE LIMITS, stated rather than hidden. The structural scan is a text scan, +## so it under-reports in 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. +## - the reader list is an ENUMERATION, not a proof. It knows `grep` +## (-q/-l/-L/-m and their long forms), `head`, `read`, `awk … exit` and +## `sed` with a q/Q command; it does not know an early exit hidden in +## `perl -ne '… last'`, `python -c`, `jq`, `head -c`, or any project-local +## program that stops reading. A reader not on the list is not thereby safe. +## - it only inspects the FIRST word after a pipe, so `foo | LC_ALL=C grep -q` +## or `foo | { grep -q x; }` reads as an unknown reader and is 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". +## +## BOUNDED ON PURPOSE. The extraction buffers and emits NOTHING until it has +## seen the END marker, so deleting or mistyping that one line is a marker +## error (exit 4) rather than a slurp of every line after BEGIN into a file +## this harness then `source`s. That is not hypothetical: it happened during +## development and only failed loudly by luck — `set -u` tripped over an +## unbound variable two statements into the entrypoint's real boot code. An +## extractor whose failure mode is "execute arbitrary parts of the program +## under test" is not a safe thing to leave lying around, however careful the +## markers are today. +## exit 0 = both markers, block on stdout +## exit 3 = no BEGIN marker at all (pre-fix checkout, or block removed) +## exit 4 = BEGIN seen, END missing — refuse to emit, refuse to source +## --------------------------------------------------------------------------- +HAVE_HELPERS=yes +XRC=0 +awk ' + /^## ---- CAC-TEST: probe helpers BEGIN ----$/ { f = 1 } + f { buf = buf $0 ORS } + f && /^## ---- CAC-TEST: probe helpers END ----$/ { printf "%s", buf; found = 1; exit 0 } + END { if (found) exit 0; else if (f) exit 4; else exit 3 } +' "$ENTRYPOINT" > "$TMP/helpers.sh" || XRC=$? + +case "$XRC" in + 0) + 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 + ;; + 4) + HAVE_HELPERS=no + : > "$TMP/helpers.sh" + bad "${ENTRYPOINT#"$ROOT"/} has a 'CAC-TEST: probe helpers BEGIN' marker with no matching END marker. Nothing was extracted — an unterminated block would otherwise have pulled the whole rest of the entrypoint into a file this test sources and runs." + ;; + 3) + 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." + ;; + *) + HAVE_HELPERS=no + bad "extracting the probe-helper block from ${ENTRYPOINT#"$ROOT"/} failed (awk exit $XRC)" + ;; +esac + +## --------------------------------------------------------------------------- +## 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' +# Does this `grep …` invocation stop reading before EOF? -q/-l/-L/-m do; -c, +# -i, -v, -o, -n and the rest read the whole input and are none of our business. +# Walked option by option rather than pattern-matched in one go, because the +# letters have to be read the way grep reads them: a cluster like -im1 stops +# early, -e/-f/-A/-B/-C/-d/-D swallow the rest of their token as an ARGUMENT +# (so `grep -eq` is the pattern "q", not --quiet), and the first non-option word +# is the pattern, after which nothing is a flag any more. +function grep_stops_early(s, a, k, j, t, c, m) { + if (s !~ /^[[:space:]]*grep([[:space:];&)]|$)/) return 0 + sub(/^[[:space:]]*grep([[:space:]]+|$)/, "", s) + m = split(s, a, /[[:space:]]+/) + for (k = 1; k <= m; k++) { + t = a[k] + if (t == "") continue + if (t == "--") return 0 + if (t ~ /^--/) { + if (t ~ /^--(quiet|silent|max-count|files-with-match|files-without-match)/) return 1 + continue + } + if (t !~ /^-/) return 0 # the pattern; options are over + sub(/^-/, "", t) + for (j = 1; j <= length(t); j++) { + c = substr(t, j, 1) + if (c == "q" || c == "l" || c == "L" || c == "m") return 1 + if (c ~ /[efABCdD]/) break # rest of the token is its argument + } + } + return 0 +} +{ + 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 + + # `u` = the line with quote CHARACTERS dropped but their contents KEPT, used + # only to read a command's script argument. `l` cannot serve for that: it + # replaces a whole quoted span with a placeholder, so `sed 'q'` and + # `awk '{exit}'` lose the very token that makes them early-exit readers. + u = raw + gsub(/["']/, "", u) + sub(/[[:space:]]#.*/, "", u) + + gsub(/\|\|/, " ", l) # || is not a pipeline + if (l !~ /\|/) next + n = split(l, seg, "|") + for (i = 2; i <= n; i++) { + r = seg[i] + if (grep_stops_early(r)) { print NR ": " raw; next } + # The `[[:space:];&)]|$` tail rather than a bare `[[:space:]]|$`: a reader + # can be the last word of a compound command (`… | head; }`), which the + # whitespace-only form silently skipped. + 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:];&)]|$)/ && u ~ /exit/) { print NR ": " raw; next } + # sed: a q/Q command anywhere in the script — `/re/q`, `2q`, `$q`, `1p;q`, + # `{…;q}`, and the bare `sed q` / `sed 'q'` that the earlier pattern missed + # (it required a `;`, `{` or `/` in front of the q, which a lone script has + # none of). Anchored on what may PRECEDE the q and what may FOLLOW it, so a + # q inside a replacement — `sed s/a/q/` — is not read as the command (which + # is what the trailing `/` exclusion buys: a command q is never followed by + # another delimiter, a replacement q always is). + if (r ~ /^[[:space:]]*sed([[:space:];&)]|$)/ && + (u ~ /[;{\/][[:space:]]*[qQ]([^[:alnum:]\/]|$)/ || + u ~ /[[:space:]]([0-9]+|\$)?[qQ]([[:space:];}]|$)/)) { 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 in the shell (see" >&2 + echo " lsphp_info_has_parity_ext in scripts/entrypoint-lsphp.sh), or give the" >&2 + echo " reader the file directly. Neither is a pipeline, so there is no second" >&2 + echo " exit status for pipefail to prefer. A here-string also works, but it" >&2 + echo " spills to a temp file above a build-dependent size, so it is not the" >&2 + echo " right shape on a boot path." >&2 +fi + +## --------------------------------------------------------------------------- +## 6. Equivalence. The helpers answer with `[[ ]]` and `${…}` what they used to +## answer with grep and awk, and "same patterns, different plumbing" is a +## claim that has to be checked rather than asserted — an anchored line match +## re-expressed as a glob is exactly where an off-by-one lives. +## +## The reference runs the ORIGINAL grep/awk patterns over a FILE, so the +## reference itself cannot SIGPIPE and cannot be accused of the bug it is +## refereeing. The file is written the way the old pipeline fed them, +## `printf '%s\n' "$SUBJECT"`, so the comparison is against the pre-fix +## behaviour byte for byte and not against a tidier reading of it. +## --------------------------------------------------------------------------- +echo "== pure-bash matching vs the grep/awk patterns it replaced ==" +if [ "$HAVE_HELPERS" = no ]; then + echo " (skipped — no probe helpers to compare)" +else + # shellcheck disable=SC1091 + source "$TMP/helpers.sh" + + NL=$'\n' + eq_fail=0 + eq_case() { # $1 = label, $2 = subject + local label="$1" subj="$2" f="$TMP/eq.txt" + local ref_has ref_use new_has new_use ref_scan new_scan + printf '%s\n' "$subj" > "$f" + + ref_has=0; grep -q '^cac_path_parity support => enabled$' "$f" || ref_has=$? + new_has=0; lsphp_info_has_parity_ext "$subj" || new_has=$? + ref_use=0; grep -q '^PHP Version => ' "$f" || ref_use=$? + new_use=0; lsphp_info_is_usable "$subj" || new_use=$? + ref_scan=$(awk -F'=> ' '/^Scan this dir/ {print $2; exit}' "$f") + new_scan=$(lsphp_info_scan_dir "$subj") + + if [ "$ref_has" = "$new_has" ] && [ "$ref_use" = "$new_use" ] && [ "$ref_scan" = "$new_scan" ]; then + ok "equivalent on: $label" + else + eq_fail=$((eq_fail+1)) + bad "NOT equivalent on: $label — has_ext grep=$ref_has bash=$new_has; usable grep=$ref_use bash=$new_use; scan_dir awk='$ref_scan' bash='$new_scan'" + fi + } + + SD='Scan this dir for additional .ini files' + PARITY='cac_path_parity support => enabled' + eq_case "empty subject" "" + eq_case "match on the first line" "${PARITY}${NL}tail line" + eq_case "match on the last line" "head line${NL}${PARITY}" + eq_case "match is the only line" "${PARITY}" + eq_case "match sandwiched" "a${NL}${PARITY}${NL}b" + eq_case "not at line start (decoy)" "x ${PARITY}${NL}b" + eq_case "trailing space defeats the \$" "${PARITY} ${NL}b" + eq_case "prefix-only line (decoy)" "cac_path_parity support => enabled but no${NL}b" + eq_case "genuinely absent" "a${NL}b${NL}c" + eq_case "embedded blank lines" "a${NL}${NL}${PARITY}${NL}${NL}b" + eq_case "banner first, scan dir second" "PHP Version => 8.3.27${NL}${SD} => /a/b" + eq_case "scan dir on the first line" "${SD} => /a/b${NL}PHP Version => 8.3.27" + eq_case "scan dir on the last line" "PHP Version => 8.3.27${NL}${SD} => /a/b" + eq_case "scan dir, second separator" "${SD} => /a => /b${NL}x" + eq_case "scan dir, empty value" "${SD} => ${NL}x" + eq_case "scan dir, no separator" "Scan this dir is broken${NL}x" + eq_case "scan dir, first of two wins" "${SD} => /first${NL}${SD} => /second" + eq_case "scan dir, value has spaces" "${SD} => /a b/c ${NL}x" + eq_case "scan-dir line is a prefix" "Scan this directory => /a/b${NL}x" + eq_case "banner not at line start" "x PHP Version => 8.3.27${NL}b" + eq_case "banner without trailing space" "PHP Version =>${NL}b" + eq_case "CR-terminated lines" $'PHP Version => 8.3.27\r'"${NL}${PARITY}"$'\r' + eq_case "glob metacharacters in body" "*${NL}?${NL}[a-z]${NL}${PARITY}${NL}][" + eq_case "realistic 40 KB body" "$(cat "$TMP/info-present.txt")" + [ "$eq_fail" -eq 0 ] || echo " (an inequivalence here means the shipped probe now answers something the old grep/awk did not)" >&2 +fi + +## --------------------------------------------------------------------------- +## 7. The scan's own coverage. Section 5 only proves something if its regexes +## actually match the shapes it claims to outlaw — an unfalsified scanner +## reports "clean" just as loudly when it has stopped matching anything. +## `grep -l foo` and `sed q` were both silently missed until this section +## existed; both are asserted below, alongside the safe forms that must NOT +## be reported, because a scanner that flags everything is no better. +## --------------------------------------------------------------------------- +echo "== the structural scan catches what it claims to ==" +cat > "$TMP/scan-bad.sh" <<'BADSH' +set -euo pipefail +a() { producer | grep -q needle; } +b() { producer | grep -qx needle; } +c() { producer | grep -m1 needle; } +d() { producer | grep -im1 needle; } +e() { producer | grep -l foo; } +f() { producer | grep -L foo; } +g() { producer | grep --quiet foo; } +h() { producer | grep --files-with-matches foo; } +i() { producer | head -1; } +j() { producer | head; } +k() { producer | read -r x; } +l() { producer | while read -r x; do :; done; } +m() { producer | awk '/x/ {print; exit}'; } +n() { producer | sed q; } +o() { producer | sed 'q'; } +p() { producer | sed 2q; } +q() { producer | sed -n '1p;q'; } +r() { producer | sed -n '/x/{p;q}'; } +s() { producer | sed '$q'; } +BADSH +cat > "$TMP/scan-good.sh" <<'GOODSH' +set -euo pipefail +a() { producer | grep -c needle; } +b() { producer | grep -i needle; } +c() { producer | grep -v needle; } +d() { producer | grep -o needle; } +e() { producer | awk '{print $1}'; } +f() { producer | sed -n 's/^K=//p'; } +g() { producer | sed 's/a/q/'; } +h() { producer | sed -e 'y/abc/xqz/'; } +i() { producer | wc -l; } +j() { producer | sort -u; } +k() { producer | tail -1; } +l() { case $x in a|b) : ;; esac; } +m() { echo "a | head -1"; } +n() { echo 'x | grep -q y'; } +o() { grep -q needle <<<"$1"; } +p() { grep -q needle "$file"; } +# q() { producer | grep -q commented-out; } +r() { producer | grep -eq foo; } +GOODSH +missed=""; falsely=""; hits_bad="" +while IFS= read -r line; do + fn=${line#*: }; fn=${fn%%(*} + hits_bad="$hits_bad $fn" +done < <(awk -f "$TMP/scan.awk" "$TMP/scan-bad.sh") +for want in a b c d e f g h i j k l m n o p q r s; do + case " ${hits_bad:-} " in *" $want "*) ;; *) missed="$missed $want" ;; esac +done +mapfile -t good_hits < <(awk -f "$TMP/scan.awk" "$TMP/scan-good.sh") +if [ -z "$missed" ]; then + ok "all 19 early-exit reader shapes are reported (incl. grep -l/-L/--quiet and bare 'sed q')" +else + bad "the scan misses these shapes in scan-bad.sh:$missed" + awk -f "$TMP/scan.awk" "$TMP/scan-bad.sh" >&2 +fi +if [ "${#good_hits[@]}" -eq 0 ]; then + ok "18 read-to-EOF / quoted / non-pipeline forms are not reported" +else + falsely=$(printf '%s; ' "${good_hits[@]}") + bad "the scan false-positives on: $falsely" +fi + +echo +echo "passed: $pass failed: $fail" +[ "$fail" -eq 0 ] || exit 1 +exit 0