Commit Graph
4 Commits
Author SHA1 Message Date
shadowdaoandClaude Opus 5 e28c3dcce4 fix(lsphp): correct the SIGPIPE explanation, drop the temp-dir boot precondition, close two scanner blind spots
Three non-blocking findings from the review of 8790b02. The fix itself is
unchanged in intent; this makes the reasoning around it true, removes a
regression the fix introduced on the boot path, and stops the new test from
under-reporting.

F1 — the shipped comments explained the bug wrongly, and a wrong rule is what
the next maintainer reasons from. entrypoint-lsphp.sh and
render-shared-ols-config.sh both said the race is decided by PIPE CAPACITY:
"while the output fits the pipe the writer always wins; once it doesn't, SIGPIPE
is guaranteed." Both halves are refuted by measurement against a default
65536-byte pipe (F_GETPIPE_SZ):

    41144 bytes -> 141 in  32/300 runs (11%)  — well UNDER capacity
    65012 bytes -> 141 in  25/30  runs        — not certain even AT capacity
    500 KB into a 1 MiB pipe -> 200/200 with 4096-byte writes, 0/200 with one
                                500 KB write

and strace caught printf dying having written 12086 of 40406 bytes into a
65536-byte pipe. The mechanism is a race on whether the reader closes before the
writer's final write() returns; capacity only modulates how many syscalls the
writer needs. What actually separated whp02 (5/5 failures) from a dev container
(10/10 clean) is the WRITER's syscall size: bash <= 5.2.15 writes ~37 KB at a
time, bash >= 5.2.21 writes 80-160 bytes. The fs.pipe-user-pages-soft aside was
also wrong: it clamps to two pages not one, needs one uid holding >1024 pipes,
and is skipped for CAP_SYS_RESOURCE.

Both blocks now state the rule that is actually true — any
`writer | early-exiting-reader` under pipefail is a latent 141; payload size is
not a safety argument; the only sound reasons a call site is safe are structural
(no pipefail, reader provably reads to EOF, or the status is discarded) — and
the same correction is applied to the three other comments that leaned on size
(`ols_running` x2, fpm-parity-check.sh's pre-flight). Nor is the reader's
implementation a defence: at 248 KB, mawk, gawk, `grep -q` and `head -1` all
gave 141 on 10/10, and these images already differ (mawk 1.3.4 vs gawk 5.2.1).
Comment-only; the test file's own section-4 output no longer contradicts the
prose next to it.

F2 — `<<<` added a writable-temp-dir precondition to the boot path. Above a
build-dependent size bash materialises a here-string as /tmp/sh-thd.XXXXXX
(measured switch: 65536 in this image's bash 5.2.21, and Debian's 5.2.15
switches between 4096 and 16384, where a ~40 KB `lsphp -i` WOULD spill). On a
bare assignment a temp file it cannot create is `set -e` killing PID 1 — the
exact failure this branch exists to remove, re-acquired from a different
direction and gated on which bash the base image ships. In cac-lsphp:f1f2f3
under `docker run --read-only`, same payload, same statement shape:

    OLD (here-string) : bash: cannot create temp file for here-document
                        -> exit 1, script dead
    NEW (pure bash)   : REACHED NEXT STATEMENT, SCAN=[…/mods-available/], exit 0

So the boot-critical sites — the three probe helpers in entrypoint-lsphp.sh and
the SCAN_DIR extraction in entrypoint-litespeed.sh — now match with `[[ ]]` and
parameter expansion, which allocate nothing. The non-boot sites keep their
here-strings and say why at the call site: `ols_running` in both OLS entrypoints
(`lswsctrl status` is under 100 bytes, orders below any spill threshold) and
fpm-parity-check.sh's `php-fpm -m` pre-flight (~1 KB, in a harness that has
already written a docroot and a pool config).

Matching semantics are preserved, not approximated: the anchored whole-line
grep becomes a glob over a subject wrapped in newlines at BOTH ends (so first
and unterminated-last lines still match), and awk's `-F'=> ' {print $2; exit}`
becomes first-matching-line then the text between the FIRST and SECOND
separator. Section 6 of the test asserts that against the original grep/awk
patterns reading a FILE — 24 cases incl. trailing-space, prefix decoys, CRLF,
a second separator, an empty value, two candidate lines, glob metacharacters in
the body, and the full 40 KB fixture. Mutations verify the assertions bite:
dropping the trailing-newline wrap fails 3 cases, taking the whole rest of the
line fails "second separator", `##` instead of `#` fails "first of two wins",
dropping the `^` anchor on the banner fails "banner not at line start".

F3 — the structural scan missed shapes it implied it caught, and the extractor
was unbounded.

  * `grep -l`/`-L`/`--quiet`/`--files-with-matches`, `-im1`-style clusters, a
    bare `head` before `;`, and `sed q` / `sed 'q'` / `sed 2q` / `sed '$q'` were
    all invisible. grep is now walked option by option the way grep reads them
    (so `grep -eq foo` stays the pattern "q", not --quiet), and the sed test
    reads the script with quote characters stripped but their contents kept.
    Replaying the old regexes against the new fixtures: 10 shapes missed and 2
    false positives (`sed s/a/q/`, `grep -eq foo`) — both now correct.
  * new section 7 pins that coverage from both sides: 19 early-exit shapes must
    be reported, 18 read-to-EOF / quoted / non-pipeline forms must not. Without
    it the scan's regexes are unfalsified and can quietly stop matching, which
    is precisely how `grep -l` and `sed q` stayed missing.
  * the helper extraction is bounded. It buffers and emits nothing until it has
    seen the END marker (exit 4 = BEGIN without END, exit 3 = no markers), so a
    half-deleted pair is a marker error instead of a slurp. Measured on this
    entrypoint with the END marker removed: the old extractor produced 301 lines
    including `mkdir -p "$SCAN_DIR"` and three `rm -f "$SCAN_DIR/…"` — which the
    harness then sourced and ran. It failed loudly last time only because `set
    -u` happened to trip two statements in. The new one emits 0 bytes and says
    what is wrong.
  * the stated scope limits now include what remains: the reader list is an
    enumeration, not a proof (nothing knows about `perl -ne … last`, `jq`,
    `head -c`), and only the first word after a pipe is inspected.

Verified: PHP 8.3 `--no-cache` build exit 0, 10/10 .phpt; cac-lsphp boots and
logs `path parity = extension` with `Rewriting => active` and .from/.to
populated from the rendered ini; cac-litespeed boots, resolves SCAN_DIR and
writes 99-user-error-log.ini, OLS reports "running with PID", /healthz 200. The
FPM parity harness — never executed by the previous review because no cac-fpm
image existed locally — was built (Dockerfile.fpm, PHPVER=83), the extension
compiled inside it, and it reports 9/9 ALL PASS, exit 0. The new test exits 0
here and exits 1 against a `git archive 9343a56` export naming all 9 offending
lines. `bash -n` clean repo-wide; `shellcheck -S warning` clean on the CI set;
`-S style` is byte-identical to before this commit (5 pre-existing info-level
findings, 0 added — the earlier report's claim of `-S style` clean was wrong).
No `.c`/`.h` file touched and the C fail-open invariant grep is still empty.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-05 16:03:08 -07:00
shadowdaoandClaude Opus 5 8790b027a9 fix(lsphp): stop SIGPIPE+pipefail reporting the parity extension as missing
`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) <noreply@anthropic.com>
2026-08-05 15:23:56 -07:00
shadowdaoandClaude Opus 5 fb4946641a fix(cac-path-parity): make the FPM proof harness actually runnable as shipped
The artifact cited as the web-SAPI evidence could not have been run as it stood.
Measured in an official php:8.3-fpm container with the extension built in place:

  - as shipped, no args:            "SKIP: php-fpm not found", exit 0.
    The default was `php-fpm8.3`, which matches neither the official images
    (`php-fpm`) nor this repo's images.
  - with the binary supplied by hand: 9 FAIL, every one with an empty `got:`.
    The generated pool had no user/group, so php-fpm refused to start as root
    ("please specify user and group other than root"). A startup failure was
    wearing the costume of nine parity bugs.

Changes:
  - auto-detect the binary (php-fpm, php-fpm8.N, /usr/local/sbin, /usr/sbin) and
    print which one was chosen plus its version;
  - pre-flight the extension with `php-fpm -m`, so a .so that will not load into
    THIS php-fpm reports as a harness failure naming the ABI mismatch rather
    than as nine wrong paths;
  - emit user/group in the pool when running as root, resolved from accounts
    that actually exist (www-data / nobody / daemon), and chmod the fixture tmpdir
    so the non-root worker can read it;
  - run_case() now returns non-zero when php-fpm never answered, and every call
    site routes that to die_startup(), which prints the php-fpm output and the
    pool error_log and exits 2 — an exit code deliberately distinct from 1
    (assertion failure).

After: 9/9 ALL PASS from a clean checkout with no arguments and no environment
fixing, running as root in php:8.3-fpm. Mutation-tested both new paths: a pool
user that does not exist reports "HARNESS FAILURE ... STARTUP/environment
failure" with the real php-fpm error and exit 2; an EXT_SO that is not a loadable
extension is caught by the pre-flight, also exit 2.

Also fixes doc drift: 001-rewrite.phpt pointed at tests/web-sapi-parity-check.sh,
which has never existed. The file it means is tests/fpm-parity-check.sh.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-05 13:06:10 -07:00
shadowdaoandClaude Opus 5 da16faaff5 feat(cac-lsphp): guarantee $_SERVER path parity via a PHP extension
A site moved from cac-fpm to cac-lsphp must see byte-identical
$_SERVER['DOCUMENT_ROOT'] and ['SCRIPT_FILENAME'] (/home/<user>/...).
The auto_prepend_file normaliser that did this was PHP_INI_PERDIR, so
any site with its own .user.ini auto_prepend_file silently displaced it
— the state 7 live shared_ols sites (Wordfence, cPanel imports) are
actually in. Hardening the hook was not an option either: making our
prepend win would have disabled those Wordfence WAFs.

Replace it with cac_path_parity, a small PHP extension that rewrites the
filesystem-path $_SERVER keys from RINIT. RINIT cannot be displaced by
.user.ini, and it occupies no userland hook, so the customer's own
auto_prepend_file stays the only prepend in play and keeps working. The
mapping lives in two PHP_INI_SYSTEM settings, which .user.ini (PERDIR /
USER only) and ini_set() cannot reach.

Mechanism is a path-component-bounded string prefix swap, not realpath():
byte-identical to cac-fpm by construction (realpath would resolve a
customer's own symlinked public_html to some third path), no syscall, and
no failure path. Every guard fails open and leaves $_SERVER untouched;
nothing here can warn, throw or 500 a site. Unconfigured it is fully
inert, so cac-fpm and cac-litespeed are unaffected.

Built in a separate Dockerfile stage keyed off the existing ARG PHPVER —
gcc/phpize/headers never reach the shipped image (verified absent; the
image grows ~155kB), and a base-image PHP bump recompiles with no human
step. A `lsphp -i | grep` assertion fails the build if the .so does not
load, so an image can never ship having silently lost parity.

The entrypoint selects the extension when present and removes any stale
prepend ini left by an older image; if the extension is somehow not
loadable it falls back to the old normaliser and logs a WARNING rather
than losing normalisation entirely. It also now logs the active parity
mode, and warns when lsphp reports no ini scan dir (previously silent).

Probe lsphp with `-i` only: it is the LSAPI SAPI, not the CLI, and
answers `-m`/`-r` by printing usage and exiting 0 — a `lsphp -m | grep`
check never matches and never errors, which is the exact class of silent
always-false assertion this change exists to remove.

Verified: 6 .phpt tests; tests/fpm-parity-check.sh proves under the FPM
SAPI that with a customer .user.ini auto_prepend_file present both keys
are still corrected AND the customer's prepend still runs, and that the
old mechanism does not; and in a real built cac-lsphp:php83 container
that SCRIPT_FILENAME is rewritten, the customer prepend still fires, and
another tenant's path is left untouched.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-05 11:38:19 -07:00