diff --git a/Dockerfile.lsphp b/Dockerfile.lsphp index 19f0e8e..9171365 100644 --- a/Dockerfile.lsphp +++ b/Dockerfile.lsphp @@ -26,26 +26,100 @@ 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" + +## 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; \ + /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)"; \ + 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 ------------------------------------ @@ -57,12 +131,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 +191,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 diff --git a/ext/cac-path-parity/cac_path_parity.c b/ext/cac-path-parity/cac_path_parity.c index b59da77..5a77121 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,21 @@ * 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. + * + * 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 @@ -142,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) { @@ -180,12 +223,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); @@ -210,7 +266,17 @@ 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; + } + + /* + * 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 (!cacpp_mapping_active(from, to)) { return SUCCESS; } @@ -257,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/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/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" 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/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" diff --git a/scripts/entrypoint-lsphp.sh b/scripts/entrypoint-lsphp.sh index dffd56d..aeaf1f4 100644 --- a/scripts/entrypoint-lsphp.sh +++ b/scripts/entrypoint-lsphp.sh @@ -68,6 +68,26 @@ 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 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 @@ -112,6 +132,40 @@ 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, 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="" + 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. @@ -124,11 +178,33 @@ 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" + 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 @@ -142,12 +218,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. @@ -167,13 +250,58 @@ 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 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 + 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. 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 ## No scan dir means none of the per-site ini drop-ins land — including the