From 12e1336d2a1112bf60996f4e8f6aeb5f8ac3903f Mon Sep 17 00:00:00 2001 From: autonomic-bot Date: Wed, 27 May 2026 21:08:29 +0100 Subject: [PATCH] =?UTF-8?q?review(1b):=20white-box=20=C2=A73=20pass=20#2?= =?UTF-8?q?=20(RL2=20input)=20=E2=80=94=20harness=20DRY=20PASS=20(no=20har?= =?UTF-8?q?ness=20surgery),=20architecture-matches-plan=20PASS=20(poll-pri?= =?UTF-8?q?mary=20=C2=A74.1,=20real=20traefik=20recipe=20=C2=A74.2),=20Nix?= =?UTF-8?q?=20idempotent/no-sentinels=20PASS,=20log-redaction=20real=20for?= =?UTF-8?q?=20infra=20secrets.=20No=20blocking=20findings;=202=20advisorie?= =?UTF-8?q?s=20(old=5Fapp=20copy-paste=E2=86=92IDEAS;=20generated-app-secr?= =?UTF-8?q?et=20redaction=E2=86=92RL3/D6=20watch-item)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- REVIEW-1b.md | 44 +++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 41 insertions(+), 3 deletions(-) diff --git a/REVIEW-1b.md b/REVIEW-1b.md index 329c57c..8fc069c 100644 --- a/REVIEW-1b.md +++ b/REVIEW-1b.md @@ -74,6 +74,44 @@ Advisory (not W0-blocking; re-confirm at RL3): Builder notes the Gitea→Drone * green via its exact command; I'll confirm a real push triggers the Drone lint build when I re-verify M2/D-gates at RL3 (it overlaps). Not filing a finding now — bounded phase, acceptance-as-stated is met. -## Status: W0 PASS logged. Awaiting Builder's W1 (review checklist) — I'll run my own white-box §3 -pass #2 (harness DRY · log redaction · architecture-matches-plan) as independent prep / RL2 input, then -the cold RL3 D1–D10 re-verify LAST. Cardinal rule holds: cleanup must not weaken/skip/regress any test. +## White-box §3 pass #2 @2026-05-27 (Adversary, post-W0 formatted code) — RL2 input +Remaining §3 checklist items. **No blocking findings.** + +- **Harness is DRY** — PASS. Recipe quirks live in shared harness + per-recipe *declarative* metadata + (`tests//recipe_meta.py`: HEALTH_PATH/HEALTH_OK/timeouts/EXTRA_ENV), consumed uniformly by + `tests/conftest.py` (`_recipe_meta`, `deployed`/`deployed_app` fixtures) and + `runner/harness/lifecycle.py` (`_recipe_extra_env`). **No `if recipe == "..."` branches in the shared + harness** (the M6.5 no-surgery rule holds). Recipe-specific logic is isolated to that recipe's dir + (e.g. keycloak `kc_admin.py`, cryptpad's derived SANDBOX_DOMAIN). Only smell: the ~6-8-line `old_app` + upgrade fixture is copy-pasted across recipes — thin boilerplate over shared metadata; **advisory**, + not a violation (factoring it would just add another per-recipe injection point). → IDEAS, not blocking. +- **Architecture matches plan** — PASS. §4.1 trigger is **poll-primary** (`bridge/bridge.py` `poll_loop` + runs unconditionally every ≤60s; webhook is optional + dedup'd by comment id; exact trimmed `!testme`; + commenter-auth via read-level `GET /orgs/{owner}/members/{user}` 204=allow, fail-closed). §4.2 Traefik + is the **real coop-cloud/traefik recipe via abra** (`modules/proxy.nix`: `abra recipe fetch/app new + traefik`, `WILDCARDS_ENABLED=1`, `compose.wildcard.yml`, `LETS_ENCRYPT_ENV=""` → no ACME, cert as + `ssl_cert`/`ssl_key` swarm secrets) — no hand-rolled traefik.nix. §3 layout matches. +- **Server state Nix-declared & idempotent** — PASS. `modules/proxy.nix` `deploy-proxy` is + `Type=oneshot`+`RemainAfterExit`, re-runs every activation and converges (insert secret only if + absent, deploy). No `.bootstrapped`/run-once sentinels anywhere (grep clean, pass #1). Leans on 1c's + already-proven D8 (byte-identical closure + live throwaway rebuild, no manual post-step). +- **Log redaction is real** — PASS for infra secrets; **one advisory gap to verify behaviorally at + RL3/D6.** `runner/run_recipe_ci.py` `_redact_values()` reads `/run/secrets/*` (≥8-char values) and + `run_stage_redacted()` masks them in live-streamed stage output (sorted longest-first → no partial + leak). **But class-B *generated app passwords* are NOT under `/run/secrets/*`, so they are NOT in the + `_REDACT` list** — their non-leak rests entirely on the "harness never prints them / abra doesn't echo + generated ones" assumption (code comment, run_recipe_ci.py:59-60). Also: the runner's *own* stdout + (the `cc-ci-run …` Drone step) bypasses `run_stage_redacted`. This is exactly what my behavioral D6 + leak test must catch at RL3 (grep published Drone logs **and** the dashboard for a known generated app + password). Phase-1 D6 passed that test once; recording the white-box shape so RL3 re-checks it, not a + new blocking finding. → **WATCH-ITEM for RL3/D6.** +- **Readability / docs accuracy** — advisory; defer to RL4 (docs) + the ruff/lint pass already covers + dead code / style deterministically. + +**Net of §3 white-box review (RL2 input): no blocking findings; 2 advisories** (old_app copy-paste → +IDEAS; app-secret redaction → RL3/D6 watch-item). I expect Builder's W1 to be light. I have NOT filed +`[adversary]` BACKLOG items since nothing is blocking — will file if W1/RL3 surfaces a real defect. + +## Status: W0 PASS + §3 white-box pass complete (no blockers). Awaiting Builder W1 claim (review+fixes). +RL3 (full cold D1–D10 re-verify) deferred to LAST per plan order. Cardinal rule: cleanup must not +weaken/skip/regress any test — incl. the conditional-upgrade-skip watch-item from pass #1.