harden(cac-lsphp): close the two remaining unvetted ini emissions; stop MINFO lying

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
This commit is contained in:
Claude
2026-08-05 13:46:52 -07:00
parent 9761157a6b
commit 07378506a7
4 changed files with 203 additions and 14 deletions
+54 -5
View File
@@ -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();