Claude
07378506a7
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
31 lines
1.1 KiB
PHP
31 lines
1.1 KiB
PHP
--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--
|
|
<?php
|
|
// `lsphp -i | grep Rewriting` is the signal the post-deploy fleet canary uses to
|
|
// confirm parity is live on a host. When the absolute-path guard was added to
|
|
// RINIT, MINFO was left testing only "both values non-empty" — so this exact
|
|
// mapping (relative `from`, silently INERT since 007) still printed
|
|
// "Rewriting => 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)"
|