recipe-maintainers/cc-ci
0
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity
Files
5ccc0d1c34922eea7b3f96ea7130fb50f0c85b2b
cc-ci/REVIEW-rcust.md

14 KiB
Raw Blame History

REVIEW-rcust.md — Adversary ledger for the recipe-customization restructure phase

SSOT for this phase: /srv/cc-ci/cc-ci-plan/recipe-custom-restructure-full-plan.md. Gates: M1 (implementation verified — branch restructure/recipe-custom, unit+concurrency+lint green on cold clone, resolved-customization diff clean for all 21 recipes, adversarial diff review) and M2 (merged + real-CI regression sweep matching baseline matrix). DONE requires fresh PASS for both with no open VETO.

I own this file and the ## Adversary findings section of BACKLOG-rcust.md only.

Standing watch items (what I will hunt at M1/M2)

  • Coverage loss (cardinal risk): for every migrated recipe, old loaders' effective customization values must equal new meta.load() values. Throwaway diff script over all 21 recipe dirs; any delta = finding.
  • Assertion weakening in tests/<recipe>/ diffs — migrations must be mechanical only (signatures, fixture/key renames, underscore prefixes). Any changed assert/expected value = VETO.
  • Deleted-code fallout — dangling refs to _recipe_meta, _load_meta, _recipe_extra_env, _recipe_meta_flag, declared_deps, is_canonical_enrolled, OIDC_AT_INSTALL, CHAOS_BASE_DEPLOY, SKIP_GENERIC, setup_custom_tests, deps_apps, deps_creds, deployed_app.
  • Validation gaps — typo'd key / wrong type / callable-on-data-key must raise MetaError, not pass.
  • R2 fixed end-to-end — orchestrator load path delivers SCREENSHOT to screenshot.py.
  • HC2 / F2-11 integrity — repo-local default-deny, requires_deps skip-report, generic floor semantics all unchanged.

Verdicts

(no GATE verdict yet — M1 is not claimed. M1 only claims after P1–P6 are all on the branch; Builder has landed P1 (472a68b) + P2 (8cd72fd) and is mid-P3. The interim pre-review below is front-loaded break-it work on the FROZEN P1/P2 commits — NOT an M1 PASS.)

Interim pre-review of frozen P1+P2 (branch @ 8cd72fd) — @2026-06-10, cold from upstream clone

Done as idle-time break-it work while no gate is pending. P1/P2 phase commits won't be rewritten (Builder adds P3+ on top), so reviewing them now is non-wasted and front-loads M1. Cold clone of origin/restructure/recipe-custom into /tmp/rcust-verify from the true upstream remote.

No defects found so far. Results:

  1. Deleted-code fallout — CLEAN. Grepped runner/ tests/ scripts/ for live refs to every deleted symbol (_recipe_meta, _load_meta, _recipe_extra_env, _recipe_meta_flag, declared_deps, is_canonical_enrolled, OIDC_AT_INSTALL, CHAOS_BASE_DEPLOY, SKIP_GENERIC, setup_custom_tests, deps_apps, deps_creds, deployed_app). All hits are comments/docstrings explaining the deletion, test names, or the intentionally-RETAINED CCCI_SKIP_GENERIC* env form (kept per P2c). Zero live call-sites. setup_custom_tests.sh files gone.
  2. All-recipes-load-clean (typo gate) — PASS, independently. Ran meta.load() (pure stdlib) over all 21 recipe dirs cold via plain python3 (did NOT trust the Builder's test_meta.py). All 21 load; non-default key sets sane. Every ALL-CAPS key used in any recipe_meta.py is in the 14-key registry.
  3. Coverage-loss diff (CARDINAL check) — ZERO deltas on data keys + hook presence. Throwaway harness (/tmp/diff_meta.py) reproduces main's six-loader effective resolution (_load_meta, declared_deps, is_enrolled, _recipe_extra_env) from MAIN's recipe_meta files and diffs vs the BRANCH's meta.load() for all 21 recipes. After correcting one harness artifact (EXTRA_ENV default is {} not None), 0/21 recipes show any delta for HEALTH_PATH/HEALTH_OK/DEPLOY_TIMEOUT/ HTTP_TIMEOUT/BACKUP_CAPABLE/EXPECTED_NA/UPGRADE_BASE_VERSION/DEPS/WARM_CANONICAL + presence of READY_PROBE/BACKUP_VERIFY/UPGRADE_EXTRA_ENV/EXTRA_ENV/SCREENSHOT.
  4. Validation gaps — CLOSED. Crafted tmp recipe_metas: typo'd key → MetaError (with "did you mean DEPLOY_TIMEOUT?"); wrong type (DEPLOY_TIMEOUT="str") → MetaError; callable on data key (DEPLOY_TIMEOUT=lambda ctx:...) → MetaError; _PRIVATE/lowercase-helper → loads clean (exemption works). All four behave per the locked decision.
  5. meta.py read — single exec(), frozen RecipeMeta generated from KEYS, _coerce rejects bool-as-int and callable-on-data-key; non_default compares vs registry default. No issues.

Still UNVERIFIED for M1 (do NOT treat above as M1 PASS): full pytest tests/unit -q + pytest tests/concurrency -q + scripts/lint.sh cold on the cc-ci host; R2 end-to-end through the real orchestrator screenshot path; P3 ctx-hook signature migration (assert byte-identical, legacy lambda domain: raises clear MetaError); P4/P5/P6; re-run the coverage diff on the FINAL branch (P3 changes hook signatures); recipe-test diffs are mechanical-only (no assertion weakening); HC2/F2-11/generic-floor integrity. These wait for the claim(rcust): M1.

Interim pre-review of frozen P3 (branch @ fd02d9f) — @2026-06-10, cold from upstream clone

Builder landed P3 (uniform ctx hook convention) and moved to P4, so P3 is frozen. Pre-reviewed it. No defects found.

  1. Mechanical-migration discipline — HELD (no VETO trigger). git diff 8cd72fd..fd02d9f over tests/*/ shows ZERO changed assert/expected literals. Every hook change is purely def HOOK(domain[, meta]) → def HOOK(ctx) + domain → ctx.domain in the body. Spot-checked cryptpad/mumble/ghost/lasuite-drive recipe_meta.py + lasuite-drive ops.py: seeded values, return dicts, paths, status codes, and the pre_restore assert _psql(...) in (...) are byte-identical apart from the ctx. deref.
  2. HookCtx — present + complete. meta.HookCtx frozen dataclass has all 5 documented fields (.domain, .base_url, .meta, .deps, .op); meta.hook_ctx(domain, meta, op=…) factory builds it and pulls deps from $CCCI_DEPS_FILE. All call sites migrated: run_recipe_ci pre_<op>, BACKUP_VERIFY; lifecycle extra_env + READY_PROBE; screenshot SCREENSHOT(page, ctx). (NB my first pass falsely flagged "no HookCtx" — that was a STALE WORKTREE at P2; corrected by checking out fd02d9f. Logged here for honesty.)
  3. Legacy-signature guard (P3.4) — PRESENT + works, live-probed. meta.check_hook_signature exact-matches positional params and raises a CLEAR MetaError naming the P3 migration + HookCtx fields. Wired into both load() (recipe_meta hooks; SCREENSHOT expects (page, ctx), rest (ctx)) and the orchestrator (ops.py pre_<op>). Crafted tmp metas: legacy READY_PROBE(domain), SCREENSHOT(page, domain, meta), EXTRA_ENV(domain) all → MetaError at load; READY_PROBE(ctx) loads clean. No silent mid-run TypeError path.
  4. Coverage diff re-run at P3 head — still 0/21 deltas (hook presence + all data keys unchanged).

Net: P1+P2+P3 all clean under cold adversarial probing. M1 still gated on full unit+concurrency+lint on the cc-ci host, P4–P6, R2 end-to-end via the real screenshot orchestrator path, and a final coverage re-diff. No findings filed; no VETO.

Interim pre-review of frozen P4 (branch @ 29a28e2) — @2026-06-10T18:55Z, cold from fresh host clone

Builder landed P4 (custom-test ergonomics) and moved to P5, so P4 is frozen. Pre-reviewed it cold. No defects found. NOT an M1 verdict — M1 stays gated (see "Still UNVERIFIED" below).

Cold acceptance (fresh git clone on cc-ci host at 29a28e2, my own checkout — not the Builder's):

  • cc-ci-run -m pytest tests/unit -q → 184 passed (exact match to claim; full suite, no cross-fixture pollution from the session-scoped deps fixture).
  • cc-ci-run -m pytest tests/unit/test_discovery.py test_discovery_phase2.py test_conftest_fixtures.py -q → 14 passed.
  • nix develop .#lint --command scripts/lint.sh → lint: PASS (ruff format/check, deadnix, shfmt, shellcheck, yamllint all clean).

Correctness probes:

  1. Placement-rule claim ("zero in-repo users of top-level custom tests") — HOLDS. Filesystem sweep of every tests/<recipe>/test_*.py: ALL are lifecycle names (test_{install,upgrade, backup,restore}.py). No top-level non-lifecycle custom exists in-repo, so dropping the top-level glob in discovery.custom_tests loses ZERO coverage. The lifecycle-name exclusion is retained inside functional/playwright as the double-run safety net.
  2. Discovery diff — clean. Top-level glob(test_*.py) branch removed; functional/ + playwright/ subdir globs retained with basename not in lifecycle_names guard. Docstring + module header updated to state the placement RULE.
  3. Test changes are adaptation + strengthening, NOT weakening (no VETO trigger).
    • test_discovery_phase2: renamed to ..._placement_rule_...; now ASSERTS the top-level test_sso_smoke.py is not in names (new negative assertion proving the behavior change), while functional/playwright customs are still in names and lifecycle name excluded.
    • test_discovery::test_custom_tests_repo_local_gated: repo-local custom moved from top-level into functional/; HC2 default-deny (== [] when unapproved) and approved-case (functional/test_sso.py in names, test_install.py excluded) both INTACT. HC2 integrity preserved.
  4. op_state fixture — correct. Skips with clear reason on unset env / missing file / non-JSON (except ValueError catches JSONDecodeError); reads & returns parsed dict otherwise. Tests cover 3 of 4 paths (the non-JSON skip path is untested — minor coverage gap, not a defect; the branch is trivially correct by inspection).

Net: P1+P2+P3+P4 all clean under cold adversarial probing; both halves of every phase claim (unit count + lint) reproduced cold on a fresh clone. No findings filed; no VETO.

Still UNVERIFIED for M1 (do NOT treat above as M1 PASS): P5 (manifest) + P6 (docs); pytest tests/concurrency -q cold; R2 end-to-end through the real orchestrator screenshot path; final coverage re-diff on the COMPLETE branch (P1–P6, all 21 recipes, effective customization set unchanged); recipe-test diffs mechanical-only across the whole branch; HC2/F2-11/generic-floor integrity at the final head. These wait for claim(rcust): M1.

Interim pre-review of frozen P5 (branch @ 68954be) — @2026-06-10T19:06Z, cold from fresh host clone

Builder landed P5 (customization manifest) and moved to P6, so P5 is frozen. Pre-reviewed it cold. No blocking defect; one secret-SURFACE observation raised (heads-up to Builder, NOT a VETO, NOT an M1 secret-leak failure). NOT an M1 verdict.

Cold acceptance (fresh git clone on cc-ci host at 68954be, my own checkout):

  • cc-ci-run -m pytest tests/unit -q → 191 passed (exact match to claim).
  • nix develop .#lint --command scripts/lint.sh → lint: PASS.

Primary adversarial target — SECRET LEAKAGE via the new manifest surface (D-gate: published logs + dashboard contain NO secrets, incl. generated app passwords):

  1. Generated/runtime secrets — NOT exposed (gate holds). manifest.build collects only: meta_non_default (static recipe_meta), hook NAMES (pre-ops/install_steps.sh/compose.ccci.yml), overlay FILENAMES, custom-test COUNTS, and env-override KEY names (printed KEY=1, value never rendered). It never touches deps (client_secret), op_state, abra-generated app passwords, or any env VALUE. The cardinal concern — generated app passwords on the dashboard — is structurally absent from this surface.
  2. Cold all-recipes sweep. Built+rendered the manifest for all 21 recipes on the host; grepped the rendered blocks AND the results.json customization payload for secret/password/token/key/ credential and for any 32+ char high-entropy string. The ONLY hit, across every recipe, is plausible's EXTRA_ENV.SECRET_KEY_BASE = "ccciplausibletestkeybase64charsexactlyforCIephemeral4567890123".
  3. OBSERVATION (not a leak): that value is a HARDCODED, committed, PUBLIC dummy CI constant (tests/plausible/recipe_meta.py, in the open-source repo) — not a generated or real secret. meta_non_default dumps EXTRA_ENV literal dicts verbatim into the log AND results.json (→ dashboard), so a field literally named SECRET_KEY_BASE with a value now appears on the dashboard. No real secret is exposed (it's public), so this is NOT a D-gate failure and does NOT block P5. BUT it's a standing surface: (a) a dashboard secret-scan gets a true-positive-shaped hit on a public dummy (noise that could mask a real leak), and (b) if any recipe ever set a real secret-ish literal in a meta dict, the manifest would surface it unredacted. Flagged to Builder via BUILDER-INBOX as a heads-up to consider redacting values of sensitive-named meta keys before M1. Will re-examine on the real dashboard at the M1 cold-verify.
  4. HC2-honoring — confirmed. Manifest routes ALL repo-local reads through discovery._gated (ops.py loop direct; install_steps/resolve_overlay_op/custom_tests each call _gated internally). An unapproved repo-local recipe contributes nothing to the manifest.
  5. Pure presentation — holds. build() only reads files/env and returns a dict; render() formats a string. Called at run_recipe_ci.py:889-890 (print) + embedded at :1261 into results; no state mutation, no verdict influence. _jsonable renders callables as '<hook>' (so a callable EXTRA_ENV/READY_PROBE never leaks closure internals) and tuples→lists for JSON.

Net: P1–P5 all clean under cold adversarial probing; every phase claim (unit count + lint) reproduced cold. No findings filed; no VETO. One non-blocking secret-surface heads-up sent.

Still UNVERIFIED for M1: P6 (docs); pytest tests/concurrency -q cold; R2 end-to-end via the real orchestrator screenshot path; final coverage re-diff on the COMPLETE branch (all 21 recipes, effective customization unchanged); recipe-test diffs mechanical-only across the whole branch; HC2/F2-11/generic-floor integrity at final head; AND — at the M1 dashboard check — confirm the SECRET_KEY_BASE-named field on the real dashboard is the accepted public dummy (or redacted). These wait for claim(rcust): M1.