From 49f0cdd15ad9744201dfad9e2f163a94651f39a2 Mon Sep 17 00:00:00 2001 From: ms-lead-reviewer Date: Sat, 18 Jul 2026 16:41:32 -0500 Subject: [PATCH] =?UTF-8?q?docs(827):=20Gate0=20Probe-3=20hygiene=20=C2=A7?= =?UTF-8?q?3-review=20v5=20FAIL=20@7f975b95?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Independent Opus §3-conformance/SECREV review (Gate-16 author≠reviewer). Reviewed object = 7f975b95ad39096463a7548bd6be0dbb387cb61b harness sha256 = c3a09a342a4b367184d44472ec6fc11f8a3aabb7e90d5a72aa6b7044b1d9b91e Verdict FAIL: hygiene delta (a) creds-scrub + (b) source-invalid-removal landed correctly and (c) byte-stable + (d) absent-sweep-0 hold, BUT homelab NEW-6 (GATED_WI_ROOT off-by-one/non-portable resolution, :32) and NEW-5 (check-then-exec launcher TOCTOU, hash :280/:287 vs exec :514-515) are present in these bytes; both must close for PASS. Byte review only; ran nothing. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../GATE0-PROBE3-HYGIENE-S3-REVIEW-v5.md | 80 +++++++++++++++++++ 1 file changed, 80 insertions(+) create mode 100644 docs/compaction-refresh/reviews/GATE0-PROBE3-HYGIENE-S3-REVIEW-v5.md diff --git a/docs/compaction-refresh/reviews/GATE0-PROBE3-HYGIENE-S3-REVIEW-v5.md b/docs/compaction-refresh/reviews/GATE0-PROBE3-HYGIENE-S3-REVIEW-v5.md new file mode 100644 index 0000000..7faa8e1 --- /dev/null +++ b/docs/compaction-refresh/reviews/GATE0-PROBE3-HYGIENE-S3-REVIEW-v5.md @@ -0,0 +1,80 @@ +# Gate0 Probe-3 (D4) Hygiene-Delta — §3-Conformance Review v5 + +**Verdict: ❌ FAIL** (hygiene delta (a)+(b) landed correctly and (c)+(d) hold, but homelab findings **NEW-5** and **NEW-6** are present in these bytes; both must close for PASS). + +## Pin (G1 — reviewed object, mandatory) + +- **Reviewed object = `7f975b95ad39096463a7548bd6be0dbb387cb61b`** (harness commit, branch `feat/827-gate0-probe`). +- **Reviewed file:** `docs/compaction-refresh/probes/p3_d4_focused_run.py` — sha256 (pushed provider bytes, `-o FILE`, FULL-40 ref, verified before trust): `c3a09a342a4b367184d44472ec6fc11f8a3aabb7e90d5a72aa6b7044b1d9b91e` (24174 bytes, no not-found sentinel). +- **Co-reviewed fixture broker:** `p3_generation_broker.py` sha256 `4db4fef1ac6658a8ca79ad5091cefc901d2aa26003265c3d6726c294cf895cad`. + +## Reviewer identity / lane (independence — on the record) + +This review is produced by a **distinct independent Opus §3-conformance / SECREV session** (Gate-16 author≠reviewer), +**byte review only, ran nothing**, that **did not build** this harness (author = ms-rev-826) and **is not Mos**. The +PROCESS/LANE separation (build lane ≠ review lane) holds and is attested here. Homelab's separate observation — that +the published PASS commits and the repair commits share the `ms-lead-reviewer` **Git signer identity** — is a +git-identity-signer question I do **not** self-resolve; per instruction it is routed to Mos. My lane attestation is +independent of the git signer. + +## Hygiene delta (v4 `48484938` → v5 `7f975b95`) — items (a)+(b): CLOSED + +Full `diff` of both files shows the delta touches **only** these: + +- **(a) Creds scrubbed — CLOSED.** New `scrub_fixture_credentials(root)` (:467-475): `shutil.rmtree(root/"home"/".pi"/"agent")` then a fail-closed re-check `if copied_agent.exists(): raise` (:473-474). Invoked in the **outermost `finally`** (:598), **after** pi/broker cleanup and **before** `return root` (:600) — so it runs on the **success path too**. The scrub target exactly matches the only copy site (`target_agent = fixture_home/".pi"/"agent"`, :433). `isolated_environment` was moved inside the `try` (:485) so the scrub finally also covers a partial env-setup failure. No copied agent credential/config subtree (incl. `auth.json`) survives in any retained root. ✅ +- **(b) `source-invalid` handler REMOVED — CLOSED.** `grep source-invalid / source_invalidation_revoke == 0` across **both** files. Broker delta is solely the deletion of the old `:179-194` block. ✅ + +## (c) Everything-else byte-stable vs `48484938` — HOLDS + +Diff confirms **no other hunks**. Re-swept intact: traceability `GATED_WI_HEAD=f4008307` (:33) + `GATED_LAUNCHER_SHA256=e950e4224e…` (:35); fidelity `read_runtime_generation`/`bump_runtime_generation` + `generation_source=="state-file"` + `new→MUTATOR_UNVERIFIED`/`prior→STALE_GENERATION`; `lease_anchor_registered` + `hex-256`; `-O`-safe (0 bare `assert`, PASS derived `all(checks.values())`); allow-list literal env (0 `os.environ.copy`); `--runs choices=(3,)`; fail-closed precondition; single p3 broker. Blockers 1-4 (inherited env / `--runs`≠3 / cleanup-misses-setup-failure / `-O`-strips-checks) remain closed. + +## (d) ABSENT sweep — 0 + +`P5`/`P6`/`P2-bank`/`retry-launder`/`mosaic yolo`/`execRuntime`/`run_open`/`atomic`/`pi_gate0` = 0 (both files); +extension invokes only `bump-generation`/`lifecycle`/`authorize-probe`/`promote-probe`; no exec-at-import. + +## Homelab carry-forward — NEW-5 / NEW-6: **PRESENT → FAIL** + +### 🔴 NEW-6 — GATED_WI_ROOT resolves wrong / non-portably (CONFIRMED) +`GATED_WI_ROOT = HERE.parents[3].parent / "stack-cr-wi3-revoke"` (:32). For the harness at +`…/agent-work/stack-cr-wi0-gate0/docs/compaction-refresh/probes/`, this computes +**`/home/hermes/stack-cr-wi3-revoke` — which does not exist**. The actual f4008307 worktree is +`/home/hermes/agent-work/stack-cr-wi3-revoke` = `HERE.parents[3] / "stack-cr-wi3-revoke"`. The traversal is +**off-by-one** (`.parents[3].parent` should be `.parents[3]`), and it additionally hardcodes the worktree name — a +non-portable relative assumption. On this host the precondition therefore fail-closes ("gated WI launcher is +unavailable") and the probe cannot locate/execute the pinned launcher at all. **Real resolution defect** (independently +computed by path arithmetic; harness not run). **file:line — :32.** +*Fix:* derive `GATED_WI_ROOT` from a portable, explicit anchor (e.g. an env-provided path validated to be the +f4008307 worktree, or `HERE.parents[3] / "stack-cr-wi3-revoke"` with existence+HEAD assertion), not `.parents[3].parent`. + +### 🔴 NEW-5 — launcher precondition is check-then-exec, not pinned-executed-bytes (CONFIRMED) +The precondition hashes `launcher_bytes = GATED_LAUNCHER.read_bytes()` (:280) against `GATED_LAUNCHER_SHA256` (:287), +but the launcher is **executed separately** via `PiRpc([sys.executable, str(GATED_LAUNCHER), …])` (:514-515), which +opens and **re-reads the file at exec time**. There is **no fd-handoff and no exec-from-verified-copy**, so the +verified snapshot does **not** bind the executed bytes. The window between check (:287) and exec (:514-515) spans the +broker `Popen` + `wait_path` (≤20 s) — a genuine **check-then-exec TOCTOU / mutable-path trust**; the `git rev-parse +HEAD` check (:285-286) is likewise on a mutable HEAD, not the executed bytes. Per the bar this is a real gap. +**file:line — hash :280/:287 vs exec :514-515.** +*Fix:* execute the exact verified bytes with no window — e.g. read once, verify, and exec from a fixture-private +copy of the verified bytes (or `python3 /proc/self/fd/`), so the hashed bytes == executed bytes. + +## Verdict + +**FAIL @ `7f975b95`.** The hygiene delta itself is correct — (a) creds scrub (fail-closed finally, success path, +every retained root) and (b) `source-invalid` removal both landed cleanly, (c) everything else is byte-stable vs +`48484938`, and (d) the absent sweep is 0. **However**, homelab's NEW-5 (check-then-exec launcher TOCTOU / not +pinned-executed-bytes) and NEW-6 (GATED_WI_ROOT off-by-one/non-portable resolution) are **present in these bytes**; +the addendum requires both **closed** for PASS. Not softened. Returns to author (ms-rev-826) — not to a builder +re-review, no PASS-launder. + +**Findings:** NEW-6 (`p3_d4_focused_run.py:32`); NEW-5 (`p3_d4_focused_run.py:280/:287` vs `:514-515`). + +## Scope reminder (not a finding) + +Producing this evidence **executes** the Gate0 mechanism (§3/§5). This review clears **bytes** only; FIRE remains +separately gated on Mos's explicit post-clear GO — and is moot until this FAIL is remediated. + +--- + +**Reviewer:** independent Opus §3-conformance/SECREV reviewer (Gate-16 author≠reviewer). Byte review only; ran nothing. +**Reviewed object (pin):** `7f975b95ad39096463a7548bd6be0dbb387cb61b` · harness sha256 `c3a09a342a4b367184d44472ec6fc11f8a3aabb7e90d5a72aa6b7044b1d9b91e`.