From 06df1c410b301f45ff7bfc30859db8238d76b7f4 Mon Sep 17 00:00:00 2001 From: jknapp Date: Wed, 5 Aug 2026 13:05:36 -0700 Subject: [PATCH 1/7] fix(cac-lsphp): pin the shipped lsphp to the version the .so was built against MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The comment claimed the extension was "built against THIS image's own lsphp so the API/ABI always match". It was not, and could not be: `lsphp-dev` is absent from the LiteSpeed apt repo at the version every prebuilt OLS base image ships, because that repo carries only the current release. Measured on OLS 1.8.4 (2026-08-05), base image vs repo candidate: lsphp81 8.1.33-5+noble -> 8.1.34-1+noble lsphp83 8.3.28-1+noble -> 8.3.32-1+noble lsphp85 8.5.0-3+noble -> 8.5.8-1+noble and the literal fix — `apt-get install lsphp-dev="$(dpkg-query lsphp)"` — fails on all three with `E: Version '' for 'lsphp-dev' was not found` (apt exit 100). So installing -dev necessarily upgrades lsphp in the build stage; the only satisfiable direction is to move the runtime to meet it. The skew the reviewer measured (a .so built on 8.3.32 shipped beside an 8.3.30 runtime, lsphp83-common at 8.3.31) was not vendor randomness: BOTH stages resolve "repo latest" independently and Docker caches them independently. `COPY ./ext` sits at the top of the ext-build stage, so editing the extension invalidated that stage's apt layer while stage 2's stayed cached — i.e. every extension edit rebuilt the .so against fresh headers and shipped it next to a stale runtime. Fixed by making them one system: - the toolchain layer moves ABOVE the source COPY, so editing the extension no longer re-resolves the PHP version; - it records the resolved version to /build-out/lsphp.version and asserts lsphp == lsphp-dev in that stage; - stage 2 COPYs that file in BEFORE its apt layer (so the version is part of that layer's cache key) and pins lsphp/-common/-ldap to it, then asserts the installed versions match. Unsatisfiable pin => loud apt failure with the remediation, never a silent fallback. Verified: builds clean on PHP 8.1/8.3/8.5; the shipped php83 image now reports a uniform lsphp83 family at 8.3.32 (the previous image shipped lsphp83 8.3.30 / -common 8.3.31 / -ldap 8.3.30). Mutation-tested by recording a version the repo no longer has: build fails at stage 2 rather than shipping the skew. The `lsphp -i | grep` build assertion is kept but its comment now says what it does and does not prove: it catches a .so that will not LOAD, never silent struct-layout drift. Co-Authored-By: Claude Opus 5 (1M context) --- Dockerfile.lsphp | 102 +++++++++++++++++++++++++++++++++++++++-------- 1 file changed, 85 insertions(+), 17 deletions(-) diff --git a/Dockerfile.lsphp b/Dockerfile.lsphp index 19f0e8e..16420d6 100644 --- a/Dockerfile.lsphp +++ b/Dockerfile.lsphp @@ -26,26 +26,63 @@ ARG PHPVER=83 ## $_SERVER['DOCUMENT_ROOT']/['SCRIPT_FILENAME'] parity with cac-fpm, enforced ## from RINIT so a customer's .user.ini cannot displace it — see ## ext/cac-path-parity/cac_path_parity.c for why this is an extension and not an -## auto_prepend_file. Built against THIS image's own lsphp so the API/ABI -## (`PHP API` / extension_dir) always match; a PHP version bump in the base -## image therefore recompiles rather than silently loading a stale .so. +## auto_prepend_file. +## +## WHICH lsphp THE .so IS BUILT AGAINST — read this before touching the apt lines. +## `lsphp${PHPVER}-dev` is NOT available at the version the base image ships: +## the LiteSpeed apt repo carries only the CURRENT release, and every prebuilt +## OLS base image is behind it (measured 2026-08-05 on OLS 1.8.4: +## lsphp81 8.1.33 base / 8.1.34 repo, lsphp83 8.3.28 / 8.3.32, +## lsphp85 8.5.0 / 8.5.8). +## Pinning -dev to the base version fails on all three with +## `E: Version '' for 'lsphp-dev' was not found`. +## +## So installing -dev necessarily UPGRADES lsphp in this stage. The parity we can +## have — and the one this file now guarantees — is the other direction: the +## shipped runtime is pinned to whatever version this stage compiled against. +## That version is recorded here and consumed by stage 2, so the two apt layers +## are cache-locked to each other. Without this, `COPY ./ext` invalidating only +## THIS stage while stage 2's apt layer stayed cached produced a real, repeatable +## skew (reviewer measured a .so built on 8.3.32 shipped next to an 8.3.30 +## runtime, with lsphp83-common at 8.3.31 — the vendor family is not always +## uniformly versioned either). +## +## Benign in practice (PHP holds ABI stable across a patch series) but it is the +## riskier direction — headers NEWER than the runtime — and the runtime assertion +## in stage 2 catches only load failure, never silent struct-layout drift. ## ## Separate stage on purpose: the compiler + headers (~400MB) stay out of the ## shipped image, which gains only the ~40KB .so. Costs ~1-2 min of CI per PHP ## version; both stages share the same base layer, so no extra pull. FROM litespeedtech/openlitespeed:${OLS_VERSION}-lsphp${PHPVER} AS ext-build ARG PHPVER=83 -COPY ./ext/cac-path-parity /usr/src/cac-path-parity -RUN apt-get update && \ + +## Toolchain layer, deliberately BEFORE the source COPY so editing the extension +## does not re-resolve the PHP version (which is what caused the skew above). +## Records the exact lsphp version the headers belong to; stage 2 pins to it. +RUN set -e; \ + apt-get update; \ DEBIAN_FRONTEND=noninteractive apt-get install -y --no-install-recommends \ build-essential autoconf pkg-config \ - lsphp${PHPVER}-dev && \ - cd /usr/src/cac-path-parity && \ - /usr/local/lsws/lsphp${PHPVER}/bin/phpize && \ + lsphp${PHPVER}-dev; \ + RTV=$(dpkg-query -W -f='${Version}' lsphp${PHPVER}); \ + DEVV=$(dpkg-query -W -f='${Version}' lsphp${PHPVER}-dev); \ + if [ "$RTV" != "$DEVV" ]; then \ + echo "FATAL: lsphp${PHPVER}=$RTV but lsphp${PHPVER}-dev=$DEVV — the headers" >&2; \ + echo " do not belong to the PHP in this stage. Refusing to build." >&2; \ + exit 1; \ + fi; \ + mkdir -p /build-out; \ + printf '%s' "$RTV" > /build-out/lsphp.version; \ + echo "cac_path_parity will be compiled against lsphp${PHPVER} $RTV" + +COPY ./ext/cac-path-parity /usr/src/cac-path-parity +RUN set -e; \ + cd /usr/src/cac-path-parity; \ + /usr/local/lsws/lsphp${PHPVER}/bin/phpize; \ ./configure --enable-cac-path-parity \ - --with-php-config=/usr/local/lsws/lsphp${PHPVER}/bin/php-config && \ - make -j"$(nproc)" && \ - mkdir -p /build-out && \ + --with-php-config=/usr/local/lsws/lsphp${PHPVER}/bin/php-config; \ + make -j"$(nproc)"; \ cp modules/cac_path_parity.so /build-out/ ## ---- stage 2: the shipped sidecar image ------------------------------------ @@ -57,12 +94,40 @@ ENV PHPVER=${PHPVER} ## base lacks is lsphpNN-ldap. setpriv (util-linux) is already on the Ubuntu ## base; we add nothing else the sidecar doesn't need. All apt cache cleaned in ## the same layer to keep the image small. -RUN apt-get update && \ - DEBIAN_FRONTEND=noninteractive apt-get install -y --no-install-recommends \ +## +## VERSION LOCKSTEP: `apt-get install lsphpNN-ldap` pulls lsphpNN-common forward, +## which drags the whole lsphpNN family to the repo's current release — the same +## upgrade the ext-build stage gets. Left implicit, the two stages resolve that +## independently and Docker caches them independently, so they drift apart (see +## the long comment on stage 1). Copying stage 1's recorded version in BEFORE +## this layer makes the version part of this layer's cache key: same version => +## cache hit, new version => this layer re-runs and lands on the same one. The +## explicit `=$V` pins then make a mid-build repo roll a LOUD apt failure instead +## of a silent skew. Verified satisfiable on PHP 8.1/8.3/8.5 (2026-08-05). +COPY --from=ext-build /build-out/lsphp.version /etc/cac-lsphp-build.version +RUN set -e; \ + V=$(cat /etc/cac-lsphp-build.version); \ + apt-get update; \ + if ! DEBIAN_FRONTEND=noninteractive apt-get install -y --no-install-recommends \ ca-certificates \ - lsphp${PHPVER}-ldap && \ - apt-get clean && \ - rm -rf /var/lib/apt/lists/* /var/cache/apt/archives/* + lsphp${PHPVER}="$V" lsphp${PHPVER}-common="$V" lsphp${PHPVER}-ldap="$V"; then \ + echo "FATAL: lsphp${PHPVER} $V is what cac_path_parity was compiled against," >&2; \ + echo " but the LiteSpeed repo no longer offers it (it keeps only the" >&2; \ + echo " current release). The ext-build stage is almost certainly a stale" >&2; \ + echo " cache hit — rebuild with --no-cache." >&2; \ + exit 1; \ + fi; \ + apt-get clean; \ + rm -rf /var/lib/apt/lists/* /var/cache/apt/archives/*; \ + RTV=$(dpkg-query -W -f='${Version}' lsphp${PHPVER}); \ + CMV=$(dpkg-query -W -f='${Version}' lsphp${PHPVER}-common); \ + if [ "$RTV" != "$V" ] || [ "$CMV" != "$V" ]; then \ + echo "FATAL: cac_path_parity.so was compiled against lsphp${PHPVER} $V but this" >&2; \ + echo " image would ship lsphp${PHPVER}=$RTV / -common=$CMV." >&2; \ + echo " Rebuild with --no-cache so both stages resolve the same release." >&2; \ + exit 1; \ + fi; \ + echo "runtime lsphp${PHPVER} pinned to $V (the version cac_path_parity was built against)" ## Scripts + the SHARED production lsphp ini (reused verbatim from the litespeed ## image — same runtime, same tuning). Scripts layer last (they change most). @@ -89,7 +154,10 @@ RUN bash -c 'set -e; \ ## ## The trailing `lsphp -i | grep` is a BUILD-TIME ASSERTION: if the .so fails to ## load (ABI drift after a base-image PHP bump, bad build) the image build fails -## here rather than shipping a sidecar that silently lost path parity. +## here rather than shipping a sidecar that silently lost path parity. Note its +## limit: it proves the .so LOADS, not that it was built against these exact +## structs — silent layout drift would sail straight through. The version lockstep +## above is what actually removes that possibility; this stays as the backstop. ## NOTE: probe lsphp with `-i` ONLY. The lsphp binary is the LSAPI SAPI, not the ## CLI — it accepts just -[b|c|n|h|i|q|s|v|?] and answers anything else (`-m`, ## `-r`) by printing its usage text and exiting 0. A `lsphp -m | grep` check From 690ff8738d6ee81fe81c26227b2fb60adb7eb167 Mon Sep 17 00:00:00 2001 From: jknapp Date: Wed, 5 Aug 2026 13:05:57 -0700 Subject: [PATCH 2/7] fix(cac-lsphp): stop generating php.ini fragments from unquoted interpolation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The two generated ini drop-ins interpolated $user/$domain into an unquoted heredoc. Measured against the pre-fix script in a real cac-lsphp:php83 container with domain=$'evil.com\nprecision = 7\n; ': 99-cac-path-parity.ini contained the injected line and lsphp reported `precision => 7 => 7` — an arbitrary ini directive supplied through the domain env var and applied to every request. The `from` value was silently truncated at the newline too, so the site also got a wrong (but "active") mapping. Both values are panel-validated and both already feed `ln -sfn` and the shared-ols vhost config, so this is defense-in-depth rather than a live hole. It is worth closing anyway because the OTHER two hostile inputs the reviewer measured — `$(...)` (ini parse error) and `"` (empty value) — leave the parity extension INERT, which is precisely the silent failure this whole change set exists to eliminate. Two layers, neither of which can fatal a request: - values are emitted double-quoted via printf instead of heredoc interpolation. php.ini double-quoted values may span newlines, so a newline is data, not a new directive. - $user/$SAFE_DOMAIN are checked against [A-Za-z0-9._-]+ first, because quoting does NOT stop php.ini's own ${VAR} interpolation. A rejected value logs a WARNING, writes no mapping at all (not even the degraded auto_prepend fallback, which would not be right for such a site either) and reports `path parity = none (user/domain rejected)` on the startup line. After: same container, same hostile domain — no 99-cac-path-parity.ini is written, `precision => 14` (default), and the warning names the rejected values. Happy path re-verified for domain=site.com and domain=*.site.com: mapping written, `Rewriting => active`, from/to parse back byte-identical. Co-Authored-By: Claude Opus 5 (1M context) --- scripts/entrypoint-lsphp.sh | 50 +++++++++++++++++++++++++++++-------- 1 file changed, 39 insertions(+), 11 deletions(-) diff --git a/scripts/entrypoint-lsphp.sh b/scripts/entrypoint-lsphp.sh index dffd56d..a794714 100644 --- a/scripts/entrypoint-lsphp.sh +++ b/scripts/entrypoint-lsphp.sh @@ -68,6 +68,25 @@ SAFE_DOMAIN="$domain" case "$domain" in \*.*) SAFE_DOMAIN="wildcard.${domain#\*.}" ;; esac + +## Both of these get interpolated into generated php.ini fragments below. They +## are panel-validated and both already feed `ln -sfn` and the shared-ols vhost +## config, so a hostile value is not reachable today — this is the belt to that +## brace. A newline in $domain is an INI-DIRECTIVE INJECTION into the generated +## fragment (measured against the pre-fix script: domain=$'evil.com\nprecision = +## 7\n; ' put that directive in 99-cac-path-parity.ini and lsphp reported +## `precision => 7`); `$(...)` yields an ini parse error and `"` an empty value, +## and BOTH of those leave the +## path-parity extension INERT — the exact silent parity loss this whole change +## exists to eliminate. Quoting the emitted values (done below) neutralises +## newlines and quotes; it does NOT neutralise php.ini's own `${VAR}` +## interpolation, which is why the character class is checked as well. +INI_TOKENS_OK=yes +case "$user" in ''|*[!A-Za-z0-9._-]*) INI_TOKENS_OK=no ;; esac +case "$SAFE_DOMAIN" in ''|*[!A-Za-z0-9._-]*) INI_TOKENS_OK=no ;; esac +if [ "$INI_TOKENS_OK" != yes ]; then + echo "WARNING: entrypoint-lsphp: user/domain contain characters outside [A-Za-z0-9._-] — refusing to write the \$_SERVER path-parity mapping (the extension stays inert; requests are unaffected). user=$(printf '%q' "$user") domain=$(printf '%q' "$domain")" >&2 +fi ## The exact path prefix the shared-ols container serves this site from — the ## string OLS puts in SCRIPT_FILENAME/DOCUMENT_ROOT. Used twice: for the symlink ## that makes it RESOLVE, and for the cac_path_parity mapping that makes it READ @@ -124,11 +143,13 @@ LSPHP_INFO=$("$LSPHP_BIN" -i 2>/dev/null || true) SCAN_DIR=$(printf '%s\n' "$LSPHP_INFO" | awk -F'=> ' '/^Scan this dir/ {print $2; exit}') if [ -n "$SCAN_DIR" ]; then mkdir -p "$SCAN_DIR" - cat > "$SCAN_DIR/99-user-error-log.ini" < "$SCAN_DIR/99-user-error-log.ini" ## ---- $_SERVER path parity with cac-fpm ---- ## Point the cac_path_parity extension at THIS site's mapping. Same two ## values the compatibility symlink above is built from, so the rewrite and @@ -142,12 +163,19 @@ EOF ## normaliser was itself PHP_INI_PERDIR and any site with its own prepend ## silently displaced it, while making OUR prepend win would have disabled ## THEIRS. See ext/cac-path-parity/cac_path_parity.c. - if printf '%s\n' "$LSPHP_INFO" | grep -q '^cac_path_parity support => enabled$'; then - cat > "$SCAN_DIR/99-cac-path-parity.ini" < enabled$'; then + { + echo '; rendered at container start by entrypoint-lsphp.sh' + printf 'cac_path_parity.from = "%s"\n' "$OLS_SITE_PATH" + printf 'cac_path_parity.to = "%s"\n' "/home/$user" + } > "$SCAN_DIR/99-cac-path-parity.ini" ## Drop the pre-extension fallback if an older image left one here — the ## container filesystem survives a "docker restart", so an in-place upgrade ## must not keep a stale auto_prepend pointing at the old normaliser. From fb4946641a4becdf47bd8d74f3c1ed0494423ec3 Mon Sep 17 00:00:00 2001 From: jknapp Date: Wed, 5 Aug 2026 13:06:10 -0700 Subject: [PATCH 3/7] fix(cac-path-parity): make the FPM proof harness actually runnable as shipped MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) --- ext/cac-path-parity/tests/001-rewrite.phpt | 2 +- ext/cac-path-parity/tests/fpm-parity-check.sh | 117 +++++++++++++++--- 2 files changed, 102 insertions(+), 17 deletions(-) diff --git a/ext/cac-path-parity/tests/001-rewrite.phpt b/ext/cac-path-parity/tests/001-rewrite.phpt index cf38688..e7f641c 100644 --- a/ext/cac-path-parity/tests/001-rewrite.phpt +++ b/ext/cac-path-parity/tests/001-rewrite.phpt @@ -15,7 +15,7 @@ HTTP_HOST=site.com // PATH_TRANSLATED (to the script path) AFTER the env import, so those three // cannot be driven from --ENV-- here. They go through the identical code path // as CONTEXT_DOCUMENT_ROOT (one loop over one key table); the real web-SAPI -// proof for them is tests/web-sapi-parity-check.sh. +// proof for them is tests/fpm-parity-check.sh. var_dump($_SERVER['CONTEXT_DOCUMENT_ROOT']); // Non-path vars must be untouched. var_dump($_SERVER['HTTP_HOST']); diff --git a/ext/cac-path-parity/tests/fpm-parity-check.sh b/ext/cac-path-parity/tests/fpm-parity-check.sh index cde05ec..ae27361 100755 --- a/ext/cac-path-parity/tests/fpm-parity-check.sh +++ b/ext/cac-path-parity/tests/fpm-parity-check.sh @@ -23,20 +23,56 @@ ## same customer .user.ini, does NOT run. This is the evidence ## that hardening the prepend hook could not have worked. ## +## Exit codes: 0 = all assertions passed, 1 = an assertion FAILED, 2 = the +## harness could not run (missing binary, php-fpm refused to start, .so would not +## load). 2 is deliberately distinct from 1: a startup problem previously +## surfaced as all nine assertions failing with an empty `got:`, which reads like +## nine parity bugs and is the opposite of the truth. +## ## Usage: ./fpm-parity-check.sh [ROOT] [PHP_FPM_BIN] [EXT_SO] ## ROOT defaults to /mnt/users (falls back to a temp dir if not creatable). +## PHP_FPM_BIN is auto-detected; every packaging of php-fpm this repo touches +## uses a different name (`php-fpm` in the official docker images, +## `php-fpm8.N` on Debian/Ubuntu, /usr/sbin/... unlinked from PATH), so a +## single hardcoded default is guaranteed to be wrong somewhere and its only +## symptom was a silent `SKIP`. set -uo pipefail HERE="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" + +find_fpm() { + local c + for c in php-fpm php-fpm8.5 php-fpm8.4 php-fpm8.3 php-fpm8.2 php-fpm8.1; do + if command -v "$c" >/dev/null 2>&1; then command -v "$c"; return 0; fi + done + for c in /usr/local/sbin/php-fpm /usr/sbin/php-fpm /usr/sbin/php-fpm8.*; do + if [ -x "$c" ]; then echo "$c"; return 0; fi + done + return 1 +} + ROOT="${1:-/mnt/users}" -FPM_BIN="${2:-$(command -v php-fpm8.3 || echo /usr/sbin/php-fpm8.3)}" +FPM_BIN="${2:-$(find_fpm || true)}" EXT_SO="${3:-$HERE/../modules/cac_path_parity.so}" PORT="${PORT:-9001}" command -v cgi-fcgi >/dev/null || { echo "SKIP: cgi-fcgi not installed (apt install libfcgi-bin)"; exit 0; } -[ -x "$FPM_BIN" ] || { echo "SKIP: php-fpm not found"; exit 0; } +[ -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))" +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 + 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 + exit 2 +fi + mkdir -p "$ROOT" 2>/dev/null || ROOT="$(mktemp -d)/mnt/users" USER_NAME=bob SITE="$ROOT/$USER_NAME/site.com" @@ -45,7 +81,29 @@ HOME_PATH="/home/$USER_NAME" TMP="$(mktemp -d)" fail=0 +## php-fpm REFUSES to start as root unless the pool names a non-root user/group, +## and the pool this script generates had neither — so as shipped it never got +## past startup in any root context (which is every container in this repo). +## Resolve a real unprivileged account rather than assuming www-data exists. +POOL_USER="" +POOL_GROUP="" +if [ "$(id -u)" -eq 0 ]; then + for u in www-data nobody daemon; do + if id -u "$u" >/dev/null 2>&1; then POOL_USER="$u"; break; fi + done + for g in www-data nogroup nobody daemon; do + if getent group "$g" >/dev/null 2>&1; then POOL_GROUP="$g"; break; fi + done + [ -n "$POOL_USER" ] && [ -n "$POOL_GROUP" ] || { + echo "HARNESS FAILURE: running as root but found no unprivileged user/group for the pool" >&2 + exit 2 + } +fi + mkdir -p "$DOCROOT" || { echo "cannot create $DOCROOT"; exit 1; } +## The pool worker is not root: it has to be able to read the fixtures under +## $TMP (mktemp -d is 0700) and walk down to $DOCROOT. +chmod 755 "$TMP" trap 'rm -rf "$TMP"; rm -f "$DOCROOT/.user.ini"' EXIT cat > "$DOCROOT/probe.php" <<'PHP' @@ -72,17 +130,27 @@ foreach (array('DOCUMENT_ROOT', 'SCRIPT_FILENAME') as $k) { } PHP -cat > "$TMP/fpm.conf" < "$TMP/fpm.conf" +## Returns non-zero when php-fpm never answered. Callers MUST distinguish that +## from an assertion failure — an unstarted php-fpm makes every expect() below +## fail with an empty `got:`, which looks like nine parity bugs. run_case() { + : > "$TMP/fpm.out" "$FPM_BIN" -n -y "$TMP/fpm.conf" -F -d user_ini.cache_ttl=0 "$@" \ >"$TMP/fpm.out" 2>&1 & local pid=$! out="" @@ -92,9 +160,26 @@ run_case() { SCRIPT_NAME=/probe.php REQUEST_METHOD=GET QUERY_STRING= \ cgi-fcgi -bind -connect "127.0.0.1:$PORT" 2>/dev/null) [ -n "$out" ] && break + ## Master already gone => it will never answer; stop waiting 6s for it. + kill -0 "$pid" 2>/dev/null || break done kill "$pid" 2>/dev/null; wait "$pid" 2>/dev/null printf '%s' "$out" + [ -n "$out" ] +} + +die_startup() { + echo + echo "HARNESS FAILURE: php-fpm never answered for case '$1'." >&2 + echo " This is a STARTUP/environment failure, NOT a parity assertion failure." >&2 + echo " php-fpm: $FPM_BIN" >&2 + echo " pool user/group: ${POOL_USER:-}/${POOL_GROUP:-}" >&2 + echo " --- php-fpm output ---" >&2 + sed 's/^/ /' "$TMP/fpm.out" >&2 + echo " --- pool error_log ---" >&2 + [ -s "$TMP/fpm-error.log" ] && sed 's/^/ /' "$TMP/fpm-error.log" >&2 + echo " ----------------------" >&2 + exit 2 } expect() { @@ -116,24 +201,24 @@ USERINI_LINE="auto_prepend_file = $SITE/customer-waf.php" echo "== 1. CONTROL: extension loaded, no mapping (reproduces the bug) ==" rm -f "$DOCROOT/.user.ini" -out=$(run_case "${EXT[@]}") +out=$(run_case "${EXT[@]}") || die_startup "1. CONTROL" expect "DOCUMENT_ROOT is the raw OLS path" "$(field "$out" DOCUMENT_ROOT)" "$DOCROOT" expect "SCRIPT_FILENAME is the raw OLS path" "$(field "$out" SCRIPT_FILENAME)" "$DOCROOT/probe.php" echo "== 2. FIX: mapping configured ==" -out=$(run_case "${EXT[@]}" "${MAP[@]}") +out=$(run_case "${EXT[@]}" "${MAP[@]}") || die_startup "2. FIX" expect "DOCUMENT_ROOT == cac-fpm value" "$(field "$out" DOCUMENT_ROOT)" "$HOME_PATH/public_html" expect "SCRIPT_FILENAME == cac-fpm value" "$(field "$out" SCRIPT_FILENAME)" "$HOME_PATH/public_html/probe.php" echo "== 3. WORDFENCE: customer .user.ini auto_prepend_file present ==" printf '%s\n' "$USERINI_LINE" > "$DOCROOT/.user.ini" -out=$(run_case "${EXT[@]}" "${MAP[@]}") -expect "DOCUMENT_ROOT still corrected" "$(field "$out" DOCUMENT_ROOT)" "$HOME_PATH/public_html" +out=$(run_case "${EXT[@]}" "${MAP[@]}") || die_startup "3. WORDFENCE" +expect "DOCUMENT_ROOT still corrected" "$(field "$out" DOCUMENT_ROOT)" "$HOME_PATH/public_html" expect "SCRIPT_FILENAME still corrected" "$(field "$out" SCRIPT_FILENAME)" "$HOME_PATH/public_html/probe.php" expect "customer auto_prepend_file still ran" "$(field "$out" PREPEND_RAN)" "yes" echo "== 4. OLD MECHANISM (why the prepend hook could not be hardened) ==" -out=$(run_case -d "auto_prepend_file=$TMP/old-normalize.php") +out=$(run_case -d "auto_prepend_file=$TMP/old-normalize.php") || die_startup "4. OLD MECHANISM" expect "auto_prepend normaliser is displaced by the customer's .user.ini" \ "$(field "$out" DOCUMENT_ROOT)" "$DOCROOT" expect "customer's prepend is the one that ran" "$(field "$out" PREPEND_RAN)" "yes" From 61bfdcfaf922094ab70a4a7fde94dfba95956424 Mon Sep 17 00:00:00 2001 From: jknapp Date: Wed, 5 Aug 2026 13:06:23 -0700 Subject: [PATCH 4/7] test(cac-path-parity): run the .phpt suite as a build gate MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Nothing executed ext/cac-path-parity/tests/ — not Dockerfile.lsphp, not .gitea/workflows/build-push.yaml. The suite passed, but as shipped it was documentation, not a gate. `make test` now runs in the ext-build stage, against the same lsphp build the .so ships next to. It costs ~1s per PHP version. The lsphp packages turn out to include a real CLI binary (php-config --php-binary => /usr/local/lsws/lsphpNN/bin/phpN.N), so run-tests.php works with no extra tooling. Guarded twice, because `make test` fails silently by default: - if PHP_EXECUTABLE is missing, the Makefile prints "Cannot run tests without CLI sapi." and EXITS 0. Asserted rather than assumed. - a run that executes zero tests also exits 0, so the summary is checked against the number of .phpt files on disk, plus "Tests failed : 0". Same reasoning as the `lsphp -i` probe: an assertion that cannot fail is worse than no assertion. Verified 8/8 on PHP 8.1/8.3/8.5. Mutation-tested both guards: breaking 001-rewrite.phpt's expectation fails the build ("FATAL: cac_path_parity .phpt suite FAILED"); adding a test that always SKIPs makes run-tests.php still exit 0 but the build fails on "expected all 9 .phpt tests to run" (summary read "Number of tests : 9 8"). Co-Authored-By: Claude Opus 5 (1M context) --- Dockerfile.lsphp | 37 +++++++++++++++++++++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/Dockerfile.lsphp b/Dockerfile.lsphp index 16420d6..9171365 100644 --- a/Dockerfile.lsphp +++ b/Dockerfile.lsphp @@ -76,6 +76,20 @@ RUN set -e; \ printf '%s' "$RTV" > /build-out/lsphp.version; \ echo "cac_path_parity will be compiled against lsphp${PHPVER} $RTV" +## Build, then RUN THE .phpt SUITE as a build gate. Before this, ext/…/tests/ +## existed but nothing ever executed it — neither this Dockerfile nor +## .gitea/workflows/build-push.yaml — so six green tests were documentation. It +## costs ~1s per PHP version and it runs against the SAME lsphp build the .so +## will ship next to. +## +## Two guards around `make test`, because its default failure mode is silence: +## - if PHP_EXECUTABLE is missing the Makefile prints "Cannot run tests without +## CLI sapi." and EXITS 0. The lsphp packages do ship a real CLI +## (php-config --php-binary => .../bin/phpN.N), but assert it rather than +## trusting it. +## - a run that executes ZERO tests also exits 0, so assert the summary shows +## every .phpt in the directory both ran and passed. Same reasoning as the +## `lsphp -i` probe below: an assertion that cannot fail is worse than none. COPY ./ext/cac-path-parity /usr/src/cac-path-parity RUN set -e; \ cd /usr/src/cac-path-parity; \ @@ -83,6 +97,29 @@ RUN set -e; \ ./configure --enable-cac-path-parity \ --with-php-config=/usr/local/lsws/lsphp${PHPVER}/bin/php-config; \ make -j"$(nproc)"; \ + PHP_BIN=$(/usr/local/lsws/lsphp${PHPVER}/bin/php-config --php-binary); \ + if [ ! -x "$PHP_BIN" ]; then \ + echo "FATAL: no CLI php at '$PHP_BIN' — \`make test\` would print" >&2; \ + echo " 'Cannot run tests without CLI sapi.' and exit 0." >&2; \ + exit 1; \ + fi; \ + EXPECTED=$(ls tests/*.phpt | wc -l); \ + if [ "$EXPECTED" -lt 1 ]; then echo "FATAL: no .phpt tests found" >&2; exit 1; fi; \ + if ! NO_INTERACTION=1 REPORT_EXIT_STATUS=1 make test >/tmp/make-test.log 2>&1; then \ + cat /tmp/make-test.log >&2; \ + echo "FATAL: cac_path_parity .phpt suite FAILED — not shipping this .so." >&2; \ + exit 1; \ + fi; \ + cat /tmp/make-test.log; \ + if ! grep -Eq "^Number of tests : +${EXPECTED} +${EXPECTED} *$" /tmp/make-test.log; then \ + echo "FATAL: expected all ${EXPECTED} .phpt tests to run; the summary above disagrees." >&2; \ + exit 1; \ + fi; \ + if ! grep -Eq "^Tests failed +: +0 " /tmp/make-test.log; then \ + echo "FATAL: run-tests.php reported failures." >&2; \ + exit 1; \ + fi; \ + echo "cac_path_parity: ${EXPECTED}/${EXPECTED} .phpt tests passed"; \ cp modules/cac_path_parity.so /build-out/ ## ---- stage 2: the shipped sidecar image ------------------------------------ From 9761157a6b611204361a664a9263ec639e764e59 Mon Sep 17 00:00:00 2001 From: jknapp Date: Wed, 5 Aug 2026 13:06:34 -0700 Subject: [PATCH 5/7] harden(cac-path-parity): make degenerate mappings inert instead of subtly wrong MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three loose ends from the review, none reachable from entrypoint-lsphp.sh today. The rewrite semantics and the prefix-boundary logic are untouched; both new guards only NARROW the set of configurations that do anything, and neither adds an error path — fail-open is unchanged. - to="/" produced "//public_html": cacpp_trim() keeps a lone separator, and the tail already starts with one. Collapse the prefix when there is a tail, keep it when there is not (value == from exactly, where "/" is correct). A doubled leading slash is not the same string as the cac-fpm value, which is the entire point of the extension. - a non-absolute `from`/`to` was accepted and applied. Both are now required to start with '/', otherwise RINIT returns exactly as it does for an absent mapping: inert, no diagnostic, request proceeds. - a well-formed but WRONG mapping stays undetectable, and now the FAILURE MODES block says so explicitly rather than leaving it as an unlisted gap, along with why that is acceptable (the entrypoint derives from/to from the same two variables it builds the compatibility symlink from, so a wrong mapping means the symlink is wrong too and the site is already broken more loudly) and where the only runtime signal is (`lsphp -i`). Two tests added, both non-vacuous — 007 rewrites without the absolute-path guard, 008 returns "//public_html" without the collapse. 8/8 pass on PHP 8.1/8.3/8.5, and the FPM harness still reports 9/9 against the changed .so. Co-Authored-By: Claude Opus 5 (1M context) --- ext/cac-path-parity/cac_path_parity.c | 45 ++++++++++++++++--- .../007-inert-when-mapping-relative.phpt | 20 +++++++++ .../tests/008-to-root-no-double-slash.phpt | 20 +++++++++ 3 files changed, 80 insertions(+), 5 deletions(-) create mode 100644 ext/cac-path-parity/tests/007-inert-when-mapping-relative.phpt create mode 100644 ext/cac-path-parity/tests/008-to-root-no-double-slash.phpt diff --git a/ext/cac-path-parity/cac_path_parity.c b/ext/cac-path-parity/cac_path_parity.c index b59da77..00aea48 100644 --- a/ext/cac-path-parity/cac_path_parity.c +++ b/ext/cac-path-parity/cac_path_parity.c @@ -68,6 +68,10 @@ * request proceed. Nothing here can warn, throw, or 500 a customer site: * - mapping unset/empty (any tier that is not shared-ols) -> RINIT returns * immediately, extension is inert. + * - either side of the mapping not an ABSOLUTE path -> inert. Nothing + * the entrypoint writes is anything else, and a relative prefix cannot + * usefully match a SAPI-supplied path, so a malformed mapping is treated + * exactly like an absent one. * - $_SERVER absent or not an array -> return. * - key absent from $_SERVER -> skip that key. * - key present but not a string -> skip that key. @@ -76,6 +80,14 @@ * There is no error path, no userland-visible diagnostic, and no dependency on * the filesystem being readable. * + * The one thing this CANNOT detect is a well-formed but WRONG mapping: it will + * confidently rewrite to a wrong path and say nothing. That is accepted by + * construction rather than overlooked — entrypoint-lsphp.sh derives from/to from + * the same two variables it builds the compatibility symlink from, so a wrong + * mapping means the symlink is wrong too and the site is already broken in a far + * louder way. The only runtime signal is `lsphp -i`, which prints + * "Rewriting => active" alongside the live from/to values. + * * SCOPE / KNOWN LIMITS * -------------------- * Only $_SERVER is rewritten. LSAPI also answers getenv('DOCUMENT_ROOT') from @@ -180,12 +192,25 @@ static void cacpp_rewrite_key(zval *server, const char *key, size_t key_len, return; } - size_t tail_len = len - from_len; - zend_string *out = zend_string_alloc(to_len + tail_len, 0); + size_t tail_len = len - from_len; - memcpy(ZSTR_VAL(out), to, to_len); - memcpy(ZSTR_VAL(out) + to_len, s + from_len, tail_len); - ZSTR_VAL(out)[to_len + tail_len] = '\0'; + /* + * to="/" is the one absolute prefix that survives cacpp_trim() as a bare + * separator, and the tail always starts with one — splicing both would give + * "//public_html". Drop it when there IS a tail; keep it when there is not + * (value == from exactly, where "/" is the correct answer). Unreachable from + * the entrypoint, which always writes to=/home/. + */ + size_t eff_to_len = to_len; + if (tail_len > 0 && eff_to_len == 1 && to[0] == '/') { + eff_to_len = 0; + } + + zend_string *out = zend_string_alloc(eff_to_len + tail_len, 0); + + memcpy(ZSTR_VAL(out), to, eff_to_len); + memcpy(ZSTR_VAL(out) + eff_to_len, s + from_len, tail_len); + ZSTR_VAL(out)[eff_to_len + tail_len] = '\0'; zval nv; ZVAL_STR(&nv, out); @@ -214,6 +239,16 @@ PHP_RINIT_FUNCTION(cac_path_parity) return SUCCESS; } + /* + * Both sides must be ABSOLUTE. The entrypoint only ever writes absolute + * paths; a relative prefix would be a typo or a mangled ini, and matching it + * against a SAPI-supplied path could only ever produce nonsense. Treat it + * like an absent mapping — inert, no diagnostic, request proceeds. + */ + if (*from != '/' || *to != '/') { + return SUCCESS; + } + /* * With auto_globals_jit=On (the default) $_SERVER is not built yet at * RINIT — php_hash_environment() only MARKED it for lazy creation. Reading diff --git a/ext/cac-path-parity/tests/007-inert-when-mapping-relative.phpt b/ext/cac-path-parity/tests/007-inert-when-mapping-relative.phpt new file mode 100644 index 0000000..6fcefed --- /dev/null +++ b/ext/cac-path-parity/tests/007-inert-when-mapping-relative.phpt @@ -0,0 +1,20 @@ +--TEST-- +cac_path_parity: a non-absolute mapping is inert, not applied +--EXTENSIONS-- +cac_path_parity +--INI-- +cac_path_parity.from=mnt/users/bob/site.com +cac_path_parity.to=/home/bob +variables_order=EGPCS +--ENV-- +CONTEXT_DOCUMENT_ROOT=mnt/users/bob/site.com/public_html +--FILE-- + +--EXPECT-- +string(34) "mnt/users/bob/site.com/public_html" diff --git a/ext/cac-path-parity/tests/008-to-root-no-double-slash.phpt b/ext/cac-path-parity/tests/008-to-root-no-double-slash.phpt new file mode 100644 index 0000000..8f8adb0 --- /dev/null +++ b/ext/cac-path-parity/tests/008-to-root-no-double-slash.phpt @@ -0,0 +1,20 @@ +--TEST-- +cac_path_parity: to=/ does not produce a doubled separator +--EXTENSIONS-- +cac_path_parity +--INI-- +cac_path_parity.from=/mnt/users/bob/site.com +cac_path_parity.to=/ +variables_order=EGPCS +--ENV-- +CONTEXT_DOCUMENT_ROOT=/mnt/users/bob/site.com/public_html +--FILE-- +). Before the eff_to_len collapse this returned +// "//public_html". A path with a doubled leading slash is not the same string as +// the cac-fpm value, which is the entire point of this extension. +var_dump($_SERVER['CONTEXT_DOCUMENT_ROOT']); +?> +--EXPECT-- +string(12) "/public_html" From 07378506a7fc1339ca7d4d030a302c1996079c9a Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 5 Aug 2026 13:46:52 -0700 Subject: [PATCH 6/7] harden(cac-lsphp): close the two remaining unvetted ini emissions; stop MINFO lying MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three non-blocking findings from the re-review of this branch. No design change: the extension's fail-open-absolute invariant is untouched (still zero error emitters, every RINIT return is SUCCESS) and both RINIT guards stay pure narrowing. 1. entrypoint-lsphp.sh: 99-user-opcache.ini was still emitted unquoted ------------------------------------------------------------------- Twenty-five lines below the mapping fix, the opcache override block interpolated the raw env into an unquoted `echo` — the same injection class the mapping fix closed. Measured on this branch's image, before this commit: OPCACHE_MEMORY_MB=$'128\nprecision = 7\n; ' -> 99-user-opcache.ini gained a `precision = 7` line -> lsphp -i reported precision => 7 => 7 WHP casts (int) and clamps 32-512 / 2000-32000 (site-pool-env.php), so this is not exploitable today — but "the panel validates it" is precisely the argument this branch already rejected for `domain`, and the panel is a different repo on a different release cadence. Both siblings in the block are now validated at the point of use (digits only, length-capped, range-checked) and emitted double-quoted. A rejected value is dropped with a WARNING and the image default applies; nothing here is ever fatal. The accepted ranges are PHP's own limits for these directives (>= 8 MB; [200, 1000000] files), deliberately a strict SUPERSET of the panel's clamps, so widening a panel clamp later cannot start silently rejecting real sites. The block now also removes a stale fragment when it has nothing valid to write: the container filesystem outlives `docker restart`, so without that an override that is later cleared — or rejected — would keep applying from the previous boot's file. 2. 99-user-error-log.ini was written from an unvetted $user -------------------------------------------------------- It was emitted before the INI_TOKENS_OK branch. Contained in practice (a newline is inert inside the quotes, and a `${`-bearing user cannot exist because useradd would have failed under `set -euo pipefail`), but "this particular unvetted value happens to be contained" is the reasoning this branch rejected one screenful up. Now gated identically. Costs a rejected user nothing it needs: `log_errors = On` is already baked in by 99-prod-overrides.ini, so PHP still logs — to stderr, i.e. `docker logs`, which is more visible than a per-site file, not less. Verified fleet-wide that no legitimate user reaches the branch (30 shared_ols sites, 4 hosts). 3. MINFO reported "active" for mappings RINIT ignores --------------------------------------------------- The absolute-path guard was added to RINIT and MINFO kept testing only "both values non-empty", so: from=mnt/users/bob/site.com (relative -> INERT since the guard landed) lsphp -i -> Rewriting => active That row is what the post-deploy fleet canary greps to confirm parity is live, so the diagnostic would have masked exactly the failure the canary exists to find — and the C comment added by this branch documents it as the only runtime signal. RINIT and MINFO now share one predicate pair (cacpp_mapping_configured / cacpp_mapping_active) rather than two longhand copies, which is what drifted. MINFO now distinguishes "inactive (mapping not absolute)" from "inactive (unconfigured)" — different operational problems. Pure reporting change: the predicates are side-effect-free and cannot fail, so MINFO gains no error path. Tests: two new .phpt cover both directions of the MINFO fix (009 relative mapping must report inactive, 010 well-formed mapping must still report active), so tightening it cannot overshoot into the opposite lie. The build gate's EXPECTED count is derived from `ls tests/*.phpt`, so it picked them up: 10/10. Non-vacuity, all five demonstrated by mutation: - opcache quoting reverted -> injection lands, `precision => 7` observed - error-log gate removed -> fragment written from the unvetted user - MINFO reverted to non-empty -> 009 FAILS, build gate exits 1 - MINFO forced always-inactive -> 010 FAILS, build gate exits 1 - all restored -> 10/10, build exit 0 --- ext/cac-path-parity/cac_path_parity.c | 59 +++++++++- .../009-minfo-reports-inert-mapping.phpt | 30 +++++ .../010-minfo-reports-active-mapping.phpt | 24 ++++ scripts/entrypoint-lsphp.sh | 104 ++++++++++++++++-- 4 files changed, 203 insertions(+), 14 deletions(-) create mode 100644 ext/cac-path-parity/tests/009-minfo-reports-inert-mapping.phpt create mode 100644 ext/cac-path-parity/tests/010-minfo-reports-active-mapping.phpt diff --git a/ext/cac-path-parity/cac_path_parity.c b/ext/cac-path-parity/cac_path_parity.c index 00aea48..5a77121 100644 --- a/ext/cac-path-parity/cac_path_parity.c +++ b/ext/cac-path-parity/cac_path_parity.c @@ -88,6 +88,13 @@ * louder way. The only runtime signal is `lsphp -i`, which prints * "Rewriting => active" alongside the live from/to values. * + * That row is what the post-deploy fleet canary greps, so MINFO's "active" test + * must stay a mirror of the conditions RINIT actually rewrites under — see + * PHP_MINFO_FUNCTION below, which shares cacpp_mapping_active() with RINIT + * precisely so the two cannot drift. A MINFO that reported "active" for a + * mapping RINIT treats as inert would mask exactly the failure the canary + * exists to catch. + * * SCOPE / KNOWN LIMITS * -------------------- * Only $_SERVER is rewritten. LSAPI also answers getenv('DOCUMENT_ROOT') from @@ -154,6 +161,30 @@ static PHP_GINIT_FUNCTION(cac_path_parity) cac_path_parity_globals->to = NULL; } +/* + * THE MAPPING PREDICATE — one definition, two callers. + * + * RINIT uses it to decide whether to rewrite; MINFO uses it to REPORT whether + * rewriting is live. Those two tests were written out longhand in two places + * and promptly drifted: the absolute-path guard was added to RINIT only, so + * `lsphp -i` went on printing "Rewriting => active" for a mapping RINIT had + * already decided to ignore. That row is the fleet canary's signal, so the lie + * masked precisely the failure the canary looks for. Keep them sharing this. + * + * Pure predicates over two NUL-terminated strings: no allocation, no side + * effect, no way to fail — MINFO gains no error path by calling them, and the + * fail-open invariant is untouched. + */ +static int cacpp_mapping_configured(const char *from, const char *to) +{ + return from != NULL && *from != '\0' && to != NULL && *to != '\0'; +} + +static int cacpp_mapping_active(const char *from, const char *to) +{ + return cacpp_mapping_configured(from, to) && *from == '/' && *to == '/'; +} + /* Trailing slashes would defeat the component-boundary test below. */ static size_t cacpp_trim(const char *s, size_t len) { @@ -235,7 +266,7 @@ PHP_RINIT_FUNCTION(cac_path_parity) const char *to = CACPP_G(to); /* Unconfigured (any tier that isn't shared-ols) => completely inert. */ - if (from == NULL || *from == '\0' || to == NULL || *to == '\0') { + if (!cacpp_mapping_configured(from, to)) { return SUCCESS; } @@ -245,7 +276,7 @@ PHP_RINIT_FUNCTION(cac_path_parity) * against a SAPI-supplied path could only ever produce nonsense. Treat it * like an absent mapping — inert, no diagnostic, request proceeds. */ - if (*from != '/' || *to != '/') { + if (!cacpp_mapping_active(from, to)) { return SUCCESS; } @@ -292,13 +323,31 @@ PHP_MINFO_FUNCTION(cac_path_parity) { const char *from = CACPP_G(from); const char *to = CACPP_G(to); - int active = (from && *from && to && *to); + + /* + * Report what RINIT would ACTUALLY do, by asking the same predicates RINIT + * asks — never a longhand copy of them (see cacpp_mapping_active above for + * what that cost last time). Three distinct answers, because "configured but + * ignored" is a different operational problem from "not configured" and the + * canary must be able to tell them apart. + */ + const char *state; + if (cacpp_mapping_active(from, to)) { + state = "active"; + } else if (cacpp_mapping_configured(from, to)) { + state = "inactive (mapping not absolute)"; + } else { + state = "inactive (unconfigured)"; + } php_info_print_table_start(); php_info_print_table_header(2, "cac_path_parity support", "enabled"); php_info_print_table_row(2, "Version", PHP_CAC_PATH_PARITY_VERSION); - /* The canary greps for this row: "active" proves the mapping is live. */ - php_info_print_table_row(2, "Rewriting", active ? "active" : "inactive (unconfigured)"); + /* + * The canary greps for this row: "active" proves the mapping is live — and, + * since the predicate is shared with RINIT, proves the request path agrees. + */ + php_info_print_table_row(2, "Rewriting", state); php_info_print_table_end(); DISPLAY_INI_ENTRIES(); diff --git a/ext/cac-path-parity/tests/009-minfo-reports-inert-mapping.phpt b/ext/cac-path-parity/tests/009-minfo-reports-inert-mapping.phpt new file mode 100644 index 0000000..683c3f3 --- /dev/null +++ b/ext/cac-path-parity/tests/009-minfo-reports-inert-mapping.phpt @@ -0,0 +1,30 @@ +--TEST-- +cac_path_parity: MINFO reports a non-absolute mapping as INACTIVE, not active +--EXTENSIONS-- +cac_path_parity +--INI-- +cac_path_parity.from=mnt/users/bob/site.com +cac_path_parity.to=/home/bob +--FILE-- + active". A canary that reports healthy for a dead mapping hides +// precisely the failure it was deployed to find. +// +// MINFO and RINIT now share cacpp_mapping_active(); revert MINFO to the +// non-empty test and this prints "active". +ob_start(); +phpinfo(INFO_MODULES); +$info = ob_get_clean(); + +// Also assert the row is unique, so the match below cannot be some other +// module's identically-named row. +var_dump(preg_match_all('/^Rewriting => (.+)$/m', $info, $m)); +var_dump(rtrim($m[1][0])); +?> +--EXPECT-- +int(1) +string(31) "inactive (mapping not absolute)" diff --git a/ext/cac-path-parity/tests/010-minfo-reports-active-mapping.phpt b/ext/cac-path-parity/tests/010-minfo-reports-active-mapping.phpt new file mode 100644 index 0000000..82bc40c --- /dev/null +++ b/ext/cac-path-parity/tests/010-minfo-reports-active-mapping.phpt @@ -0,0 +1,24 @@ +--TEST-- +cac_path_parity: MINFO reports a well-formed mapping as ACTIVE +--EXTENSIONS-- +cac_path_parity +--INI-- +cac_path_parity.from=/mnt/users/bob/site.com +cac_path_parity.to=/home/bob +--FILE-- +/ -> +// /home/), and 001 proves RINIT really does rewrite under it. +ob_start(); +phpinfo(INFO_MODULES); +$info = ob_get_clean(); + +var_dump(preg_match_all('/^Rewriting => (.+)$/m', $info, $m)); +var_dump(rtrim($m[1][0])); +?> +--EXPECT-- +int(1) +string(6) "active" diff --git a/scripts/entrypoint-lsphp.sh b/scripts/entrypoint-lsphp.sh index a794714..348d358 100644 --- a/scripts/entrypoint-lsphp.sh +++ b/scripts/entrypoint-lsphp.sh @@ -85,8 +85,9 @@ INI_TOKENS_OK=yes case "$user" in ''|*[!A-Za-z0-9._-]*) INI_TOKENS_OK=no ;; esac case "$SAFE_DOMAIN" in ''|*[!A-Za-z0-9._-]*) INI_TOKENS_OK=no ;; esac if [ "$INI_TOKENS_OK" != yes ]; then - echo "WARNING: entrypoint-lsphp: user/domain contain characters outside [A-Za-z0-9._-] — refusing to write the \$_SERVER path-parity mapping (the extension stays inert; requests are unaffected). user=$(printf '%q' "$user") domain=$(printf '%q' "$domain")" >&2 + echo "WARNING: entrypoint-lsphp: user/domain contain characters outside [A-Za-z0-9._-] — refusing to generate php.ini fragments from them, so the \$_SERVER path-parity mapping and the per-site error_log are BOTH skipped (the extension stays inert, log_errors stays On from the image defaults and PHP logs to stderr i.e. \`docker logs\`; requests are unaffected). user=$(printf '%q' "$user") domain=$(printf '%q' "$domain")" >&2 fi + ## The exact path prefix the shared-ols container serves this site from — the ## string OLS puts in SCRIPT_FILENAME/DOCUMENT_ROOT. Used twice: for the symlink ## that makes it RESOLVE, and for the cac_path_parity mapping that makes it READ @@ -131,6 +132,34 @@ export LSPHP_ENABLE_USER_INI="${LSPHP_ENABLE_USER_INI:-on}" echo "Container memory: ${CONTAINER_MEMORY_MB}MB | PHP_LSAPI_CHILDREN=${PHP_LSAPI_CHILDREN} | LSAPI_MAX_IDLE=${LSAPI_MAX_IDLE} | PHPVER=${PHPVER} | bind=${LSPHP_BIND} | user_ini=${LSPHP_ENABLE_USER_INI}" +## Validate a numeric value destined for a generated php.ini fragment. +## Sets INI_NUM to the value when it is acceptable, and to "" (plus a WARNING) +## when it is not. Never fatal: a rejected override just leaves the image +## default in place, and the site serves either way. +## +## Digits-only is what closes the injection: no newline, quote, `$` or `{` can +## survive it, so neither an ini-directive injection nor php.ini's `${VAR}` +## interpolation is reachable regardless of what the caller sent. The range +## bound is a separate concern — it stops a typo'd value from making opcache +## fail its shared-memory allocation at startup. +validate_ini_num() { + local name="$1" val="$2" min="$3" max="$4" + INI_NUM="" + case "$val" in + ''|*[!0-9]*) ;; + *) + ## Length-cap first: `[ -lt ]` on a 25-digit string is an arithmetic + ## error, not a comparison. 7 digits covers every max below. + if [ "${#val}" -le 7 ] && [ "$val" -ge "$min" ] && [ "$val" -le "$max" ]; then + INI_NUM="$val" + return 0 + fi + ;; + esac + echo "WARNING: entrypoint-lsphp: ${name}=$(printf '%q' "$val") is not a plain integer in ${min}-${max} — ignoring it; the image default from 99-prod-overrides.ini applies." >&2 + return 0 +} + ## ---- per-site ini drop-ins (identical mechanism to entrypoint-litespeed.sh) ---- ## error_log → the same customer-visible path cac:phpNN / cac-litespeed use, so ## "where's my PHP error log?" is answered identically across all site types. @@ -145,11 +174,31 @@ if [ -n "$SCAN_DIR" ]; then mkdir -p "$SCAN_DIR" ## Values emitted double-quoted via printf rather than interpolated into an ## unquoted heredoc — see the INI_TOKENS_OK note above for what that prevents. - { - echo '; rendered at container start by entrypoint-lsphp.sh' - printf 'error_log = "%s"\n' "/home/$user/logs/php-fpm/error.log" - echo 'log_errors = On' - } > "$SCAN_DIR/99-user-error-log.ini" + ## + ## Gated on INI_TOKENS_OK for the same reason the mapping below is: this + ## fragment interpolates $user into generated ini too. Leaving it ungated was + ## an INCONSISTENCY, not a live hole — a newline is inert inside the quotes, + ## and a `${`-bearing $user cannot exist because the useradd above would have + ## failed under `set -euo pipefail`. But "this particular unvetted value + ## happens to be contained" is the reasoning this branch already rejected one + ## screenful up, so it is not the reasoning that guards this line either. + ## + ## Rejecting costs such a user nothing it needs: `log_errors = On` is already + ## baked in by 99-prod-overrides.ini, so PHP still logs — to stderr, i.e. + ## `docker logs`, which is MORE visible than a per-site file, not less. No + ## legitimate user reaches this branch (verified fleet-wide: 30 shared_ols + ## sites across 4 hosts, none rejected by the charset check). + if [ "$INI_TOKENS_OK" = yes ]; then + { + echo '; rendered at container start by entrypoint-lsphp.sh' + printf 'error_log = "%s"\n' "/home/$user/logs/php-fpm/error.log" + echo 'log_errors = On' + } > "$SCAN_DIR/99-user-error-log.ini" + else + ## The container filesystem survives `docker restart`, so a fragment an + ## earlier boot wrote from a different env must not outlive the rejection. + rm -f "$SCAN_DIR/99-user-error-log.ini" + fi ## ---- $_SERVER path parity with cac-fpm ---- ## Point the cac_path_parity extension at THIS site's mapping. Same two ## values the compatibility symlink above is built from, so the rewrite and @@ -195,13 +244,50 @@ EOF fi ## Per-site opcache override (panel: Advanced Tuning → OpCache size); falls ## back to the baked lsphp-overrides.ini defaults when unset. - if [ -n "${OPCACHE_MEMORY_MB:-}" ] || [ -n "${OPCACHE_MAX_FILES:-}" ]; then + ## + ## SAME INJECTION CLASS AS THE MAPPING ABOVE, and it was left open when that + ## one was closed. These two lines interpolated the raw env into an UNQUOTED + ## `echo`, so (measured against the pre-fix script on this branch's image) + ## OPCACHE_MEMORY_MB=$'128\nprecision = 7\n; ' put `precision = 7` into + ## 99-user-opcache.ini and lsphp duly reported `precision => 7`. + ## + ## WHP does cast (int) and clamp these before setting the env + ## (web-files/libs/site-pool-env.php: 32-512 MB, 2000-32000 files) — but + ## "the panel validates it" is exactly the argument this branch rejected for + ## `domain`, and the panel is a different repo on a different release cadence. + ## Validate at the point of use, where the ini is actually generated. + ## + ## The accepted ranges below are PHP's OWN limits for these directives + ## (opcache refuses memory_consumption under 8 MB and clamps + ## max_accelerated_files into [200, 1000000]), deliberately a strict SUPERSET + ## of the panel's clamps: a value outside them could not have taken effect + ## anyway, and widening a panel clamp later can never start silently + ## rejecting real sites here. + OPCACHE_LINES=() + if [ -n "${OPCACHE_MEMORY_MB:-}" ]; then + validate_ini_num OPCACHE_MEMORY_MB "$OPCACHE_MEMORY_MB" 8 4096 + if [ -n "$INI_NUM" ]; then + OPCACHE_LINES+=("$(printf 'opcache.memory_consumption = "%s"' "$INI_NUM")") + fi + fi + if [ -n "${OPCACHE_MAX_FILES:-}" ]; then + validate_ini_num OPCACHE_MAX_FILES "$OPCACHE_MAX_FILES" 200 1000000 + if [ -n "$INI_NUM" ]; then + OPCACHE_LINES+=("$(printf 'opcache.max_accelerated_files = "%s"' "$INI_NUM")") + fi + fi + if [ "${#OPCACHE_LINES[@]}" -gt 0 ]; then { echo "; rendered at container start by entrypoint-lsphp.sh" echo "; per-site override from WHP whp.sites.opcache_*_override" - [ -n "${OPCACHE_MEMORY_MB:-}" ] && echo "opcache.memory_consumption = ${OPCACHE_MEMORY_MB}" - [ -n "${OPCACHE_MAX_FILES:-}" ] && echo "opcache.max_accelerated_files = ${OPCACHE_MAX_FILES}" + printf '%s\n' "${OPCACHE_LINES[@]}" } > "$SCAN_DIR/99-user-opcache.ini" + else + ## Nothing valid to say. Remove rather than leave whatever a previous boot + ## wrote — the env is the source of truth and the container filesystem + ## outlives a `docker restart`. Without this, an override that is later + ## cleared (or rejected) would keep applying from the stale fragment. + rm -f "$SCAN_DIR/99-user-opcache.ini" fi else ## No scan dir means none of the per-site ini drop-ins land — including the From 3047123f2b94544e05a6e866167a7e894de798a0 Mon Sep 17 00:00:00 2001 From: jknapp Date: Wed, 5 Aug 2026 14:08:09 -0700 Subject: [PATCH 7/7] docs(lsphp): correct three comments that overstated what the code does MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review found all three describing behaviour the code does not have: - The range bound does NOT prevent opcache's shared-memory startup failure. With 99-prod-overrides setting interned_strings_buffer=16, a memory_consumption of 8 or 16 is accepted here and still aborts opcache. Documented rather than raising the floor, which would forfeit the superset property. - memory_consumption's 4096 ceiling is ours, not PHP's — PHP imposes no upper bound on that directive. Only the max_accelerated_files range is a vendor clamp. Also records that an out-of-range value resets to PHP's COMPILED default, discarding the image's own override. - The stale-fragment rm -f is defensive, not a bug fix: changing these env vars requires a recreate, which starts from a fresh layer, so the scenario the comment described is not reachable via docker restart. Comments only; no behaviour change. Co-Authored-By: Claude Opus 5 (1M context) --- scripts/entrypoint-lsphp.sh | 38 +++++++++++++++++++++++++------------ 1 file changed, 26 insertions(+), 12 deletions(-) diff --git a/scripts/entrypoint-lsphp.sh b/scripts/entrypoint-lsphp.sh index 348d358..aeaf1f4 100644 --- a/scripts/entrypoint-lsphp.sh +++ b/scripts/entrypoint-lsphp.sh @@ -139,9 +139,15 @@ echo "Container memory: ${CONTAINER_MEMORY_MB}MB | PHP_LSAPI_CHILDREN=${PHP_LSAP ## ## Digits-only is what closes the injection: no newline, quote, `$` or `{` can ## survive it, so neither an ini-directive injection nor php.ini's `${VAR}` -## interpolation is reachable regardless of what the caller sent. The range -## bound is a separate concern — it stops a typo'd value from making opcache -## fail its shared-memory allocation at startup. +## interpolation is reachable regardless of what the caller sent. +## +## The range bound is a separate, weaker concern: it is a sanity check, NOT a +## guarantee that the value works. Measured — with `99-prod-overrides.ini` +## setting `opcache.interned_strings_buffer = 16`, a memory_consumption of 8 or +## 16 is ACCEPTED here and still aborts opcache at startup ("Insufficient shared +## memory for interned strings buffer"), loading no opcache at all. The floor is +## not raised to cover that because doing so would forfeit the superset property +## below; the panel clamps at 32, well clear of it. validate_ini_num() { local name="$1" val="$2" min="$3" max="$4" INI_NUM="" @@ -257,12 +263,17 @@ EOF ## `domain`, and the panel is a different repo on a different release cadence. ## Validate at the point of use, where the ini is actually generated. ## - ## The accepted ranges below are PHP's OWN limits for these directives - ## (opcache refuses memory_consumption under 8 MB and clamps - ## max_accelerated_files into [200, 1000000]), deliberately a strict SUPERSET - ## of the panel's clamps: a value outside them could not have taken effect - ## anyway, and widening a panel clamp later can never start silently - ## rejecting real sites here. + ## The accepted ranges below are deliberately a strict SUPERSET of the panel's + ## clamps (32-512 and 2000-32000), so widening a panel clamp later can never + ## start silently rejecting real sites here. + ## + ## Provenance, stated honestly: max_accelerated_files [200, 1000000] IS PHP's + ## own clamp. For memory_consumption, 8 is PHP's documented floor but 4096 is + ## OURS — PHP imposes no upper bound on that directive. It is a typo guard, not + ## a vendor limit. An out-of-range value is not merely ignored: PHP resets the + ## directive to its COMPILED default, discarding the image's own + ## `99-prod-overrides` value, which is a further reason to reject rather than + ## pass such a value through. OPCACHE_LINES=() if [ -n "${OPCACHE_MEMORY_MB:-}" ]; then validate_ini_num OPCACHE_MEMORY_MB "$OPCACHE_MEMORY_MB" 8 4096 @@ -284,9 +295,12 @@ EOF } > "$SCAN_DIR/99-user-opcache.ini" else ## Nothing valid to say. Remove rather than leave whatever a previous boot - ## wrote — the env is the source of truth and the container filesystem - ## outlives a `docker restart`. Without this, an override that is later - ## cleared (or rejected) would keep applying from the stale fragment. + ## wrote. Defensive only — do not read this as fixing a reachable bug: the + ## writable layer does outlive a `docker restart`, but so does the + ## environment, and changing these vars requires a RECREATE, which starts + ## from a fresh layer with no stale fragment. Kept because it is free, and + ## because it makes "no valid override" mean the same thing on every boot + ## regardless of how the container got here. rm -f "$SCAN_DIR/99-user-opcache.ini" fi else