Add deployment ceremony evidence bundle#145
Conversation
|
@coderabbitai review |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (10)
✅ Files skipped from review due to trivial changes (5)
🚧 Files skipped from review as they are similar to previous changes (5)
📝 WalkthroughWalkthroughThis PR adds a machine-verifiable JSON schema for deployment ceremony evidence bundles, implements a no-secret validator with unit tests, provides a local Anvil example bundle, and wires validation checks into the build system, release manifest generation, checksum bundles, CI, and documentation. ChangesCeremony Evidence Schema & Validation
🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly Related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Claude Code Review
This repository is configured for manual code reviews. Comment @claude review to trigger a review and subscribe this PR to future pushes, or @claude review once for a one-time review.
Tip: disable this comment in your organization's Code Review settings.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (4)
.github/workflows/ci.yml (1)
187-193: ⚡ Quick winAdd the new scripts to the fast
py_compilepass.These entrypoints are now part of the smoke gate, but they aren't covered by the repository-hygiene compile list yet. Catching syntax errors here keeps failures in the cheap check instead of the long Foundry job.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/ci.yml around lines 187 - 193, The fast py_compile pass is missing the new entrypoints used by the "Ceremony evidence" step; add scripts/test_ceremony_evidence.py and scripts/check_ceremony_evidence.py to the py_compile check so their syntax is validated in the cheap gate. Update the py_compile list or command (the job that currently compiles existing scripts) to include these two paths or run python -m py_compile against them, ensuring the same format/array used for other checked scripts is followed.docs/tooling.md (1)
177-184: ⚡ Quick winInclude the test step in the manual release-artifact sequence.
This section currently lists only
check_ceremony_evidence.py, but the new gate is wired everywhere else astest+check. Adding the test command keeps the docs aligned with CI/makeand avoids skipping the negative-path coverage.Proposed fix
python scripts/generate_address_books.py +python scripts/test_ceremony_evidence.py python scripts/check_ceremony_evidence.py python scripts/generate_release_manifest.py🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/tooling.md` around lines 177 - 184, Update the manual release-artifact sequence to include the test step before the check step: add the test command (e.g., run your test suite with `python -m pytest` or the repo's `make test` equivalent) immediately before `python scripts/check_ceremony_evidence.py` so the sequence follows the new gate convention of `test` + `check` and aligns with CI; locate the insertion near the existing `python scripts/check_ceremony_evidence.py` entry in the list of commands.scripts/test_ceremony_evidence.py (1)
200-209: 💤 Low valueMinor style inconsistency: deepcopy usage.
Line 203 uses
copy.deepcopy(valid_evidence(root))while other tests (lines 137, 146, 157, 168, 179, 192) callvalid_evidence(root)directly and mutate the returned dict. Sincevalid_evidence()creates a fresh dict each time, the deepcopy is unnecessary.This is a minor style inconsistency with no functional impact.
♻️ Optional: Remove unnecessary deepcopy for consistency
- evidence = copy.deepcopy(valid_evidence(root)) + evidence = valid_evidence(root)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/test_ceremony_evidence.py` around lines 200 - 209, In test_secret_like_keys_fail replace the unnecessary use of copy.deepcopy(valid_evidence(root)) with a direct call to valid_evidence(root) (the helper creates a fresh dict), i.e. in the test_secret_like_keys_fail function remove the copy.deepcopy wrapper so you mutate the returned dict directly before writing it and calling checker.validate_evidence; no other changes needed.scripts/check_ceremony_evidence.py (1)
282-291: 💤 Low valueSecret detection checks keys only, not values.
The
reject_secret_like_keysfunction scans dictionary keys for secret-like patterns but does not inspect values. An operator could theoretically include a secret in a value like{"notes": "password=abc123"}.However, this is acceptable as a best-effort heuristic check, backed by the explicit
redaction_policy.no_secrets=trueassertion. The design assumes operators review evidence before committing.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/check_ceremony_evidence.py` around lines 282 - 291, The current reject_secret_like_keys function only inspects dict keys and skips string values; update reject_secret_like_keys to also examine string (and possibly bytes) values: when value is a str, lower-case it and raise CeremonyEvidenceError if it contains any token from SECRET_KEY_PARTS or matches common secret patterns (e.g., "password=...", "secret: ...", "api_key=..."); keep the existing path formatting and error message style; reference the existing symbols reject_secret_like_keys, SECRET_KEY_PARTS, and CeremonyEvidenceError when making the change.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/known-blockers.md`:
- Around line 140-147: Reword the two bullets so they clearly state that
detailed retained broadcast/verification artifacts are future non-local ceremony
requirements rather than missing parts of the current local Anvil bundle;
reference the checker’s current enforcement of the no-secret contract and
retained_artifacts contract (e.g., mention "no-secret / retained_artifacts" and
"local Anvil bundle") and change phrasing like "remain missing" to something
like "are planned for future non-local ceremonies" so readers don’t infer the
local bundle or current checker is incomplete.
In `@ops/ROADMAP.md`:
- Around line 277-278: Reword the roadmap line that currently reads "Local
ceremony evidence bundle is generated and checked; fork/testnet/live ceremony
evidence bundles are retained before public beta." to explicitly scope the
current gate to only the local, non-secret bundle (e.g., "Local ceremony
evidence bundle is generated and checked; only local/no-secret evidence is
retained for this release"), and remove or extract any mention of
fork/testnet/live retention into a separate follow-up gate entry; apply the same
change to the related lines referenced at 2161-2163 so all mentions of
fork/testnet/live retention are moved out of the immediate-release gate and into
a distinct future-facing gate description.
---
Nitpick comments:
In @.github/workflows/ci.yml:
- Around line 187-193: The fast py_compile pass is missing the new entrypoints
used by the "Ceremony evidence" step; add scripts/test_ceremony_evidence.py and
scripts/check_ceremony_evidence.py to the py_compile check so their syntax is
validated in the cheap gate. Update the py_compile list or command (the job that
currently compiles existing scripts) to include these two paths or run python -m
py_compile against them, ensuring the same format/array used for other checked
scripts is followed.
In `@docs/tooling.md`:
- Around line 177-184: Update the manual release-artifact sequence to include
the test step before the check step: add the test command (e.g., run your test
suite with `python -m pytest` or the repo's `make test` equivalent) immediately
before `python scripts/check_ceremony_evidence.py` so the sequence follows the
new gate convention of `test` + `check` and aligns with CI; locate the insertion
near the existing `python scripts/check_ceremony_evidence.py` entry in the list
of commands.
In `@scripts/check_ceremony_evidence.py`:
- Around line 282-291: The current reject_secret_like_keys function only
inspects dict keys and skips string values; update reject_secret_like_keys to
also examine string (and possibly bytes) values: when value is a str, lower-case
it and raise CeremonyEvidenceError if it contains any token from
SECRET_KEY_PARTS or matches common secret patterns (e.g., "password=...",
"secret: ...", "api_key=..."); keep the existing path formatting and error
message style; reference the existing symbols reject_secret_like_keys,
SECRET_KEY_PARTS, and CeremonyEvidenceError when making the change.
In `@scripts/test_ceremony_evidence.py`:
- Around line 200-209: In test_secret_like_keys_fail replace the unnecessary use
of copy.deepcopy(valid_evidence(root)) with a direct call to
valid_evidence(root) (the helper creates a fresh dict), i.e. in the
test_secret_like_keys_fail function remove the copy.deepcopy wrapper so you
mutate the returned dict directly before writing it and calling
checker.validate_evidence; no other changes needed.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 23478a7d-46d5-4400-96bb-ac08bd0f122e
📒 Files selected for processing (23)
.github/workflows/ci.ymlCHANGELOG.mdMakefiledeployments/README.mddeployments/ceremony-evidence/anvil-6529stream-v0.1.0-001-local.jsondeployments/schema/ceremony-evidence.schema.jsondocs/deployment.mddocs/known-blockers.mddocs/status.mddocs/tooling.mdops/AUTONOMOUS_RUN.mdops/ROADMAP.mdrelease-artifacts/README.mdrelease-artifacts/latest/SHA256SUMSrelease-artifacts/latest/release-checksums.jsonrelease-artifacts/latest/release-manifest.jsonscripts/check.ps1scripts/check.shscripts/check_ceremony_evidence.pyscripts/generate_release_checksums.pyscripts/generate_release_manifest.pyscripts/test_ceremony_evidence.pyscripts/test_release_manifest.py
Summary
Closes #144
Validation
python scripts\test_ceremony_evidence.pypython scripts\check_ceremony_evidence.pypython scripts\test_release_manifest.pypython scripts\generate_release_manifest.pypython scripts\generate_release_checksums.pypython scripts\generate_release_manifest.py --checkpython scripts\test_release_checksums.pypython scripts\generate_release_checksums.py --checkpython scripts\test_changelog_check.pypython scripts\check_changelog.pypython -m py_compile scripts\check_ceremony_evidence.py scripts\test_ceremony_evidence.py scripts\generate_release_manifest.py scripts\test_release_manifest.py scripts\generate_release_checksums.pybash -n scripts/check.sh scripts/bootstrap-ec2.shmake checkscripts\check.ps1andscripts\bootstrap-windows.ps1powershell -NoProfile -ExecutionPolicy Bypass -File scripts\check.ps1Summary by CodeRabbit
New Features
Documentation
Tests
Chores