Repository navigation
T35 — Audit segments rotate and the chain is checked, end to end - #2773
norrietaylor merged 14 commits into
Conversation
A test surface for the end-to-end suite: bep serve reads the variable and uses a positive integer as the segment bound, so the audit log rotates after a few requests. Any other value keeps the 8 MiB default.
One case restarts the daemon pair with BEP_AUDIT_SEGMENT_BYTES set, drives requests until the audit log rotates twice, checks that min box audit shows the box's records from every segment, edits one record and checks that min doctor exits 6, then restores it and checks that the chain verifies again.
|
Important Review skippedBot user detected. To trigger a single review, invoke the
 |
|
Approved: every review thread resolved at @norrietaylor resolved the last thread, which releases the approval this review had withheld. Nothing to do. |
There was a problem hiding this comment.
arch-review: PASS_WITH_CONDITIONS · 1 condition · the full ruling
The change conforms to the design. bep serve alone reads the test override, and the override defaults to the designed segment bound. The e2e case checks the two rules in architecture.md line 505: min box audit reads the box's records, and min doctor verifies the whole chain. That case is NOT RUN at this head, so the proof is open. C1 asks for one real run on a macOS host with gvproxy and a GitHub sign-in, with the result in the PR. Then the change is complete.
Ruled at 2d52100 against gominimal/arch@8640f85e193d by claude-opus-5-5.
A stray value made the audit log rotate fast and prune its history. The e2e and the dev stack run debug builds, so they keep the override.
|
Round 1 of 10 on the runtime's review findings, pushed as
Verify Review threads the coder judged:
With BEP_AUDIT_SEGMENT_BYTES set on a debug build, bep serve rotates its audit log at the small bound with the chain carried across segments. The e2e case then shows min box audit reads every segment and min doctor reports an edited record. A release build of bep ignores the variable and logs a warning. Notes from the agent: Revise round 1: audit_rotation now reads BEP_AUDIT_SEGMENT_BYTES only when cfg!(debug_assertions) is true. A release build ignores it and logs a warning. The e2e and the dev stack build bep in debug (justfile minvmd-build), so the e2e case keeps its override. The e2e case did not execute on this box: it needs macOS, where the Box Egress Proxy runs. The script reported NOT RUN and exited 0. The macOS lane owes that evidence. cargo clippy -p bep --release --bins is clean, so the release path has no dead code. |
There was a problem hiding this comment.
arch-review: PASS_WITH_CONDITIONS · 1 condition · the full ruling
The change conforms to the audit design in architecture line 505 and Gatehouse §6.10. The new commit makes the segment override a debug-build switch, so a release build keeps the default bound. That is the safe default. One condition stays open. The e2e case is NOT RUN on this head, and a NOT RUN leg is not a pass. Run the case on the macOS lane against this head, as C1 says.
Ruled at a855d7e against gominimal/arch@8640f85e193d by claude-opus-5-5.
The 4096-byte bound can prune old segments of the real log. The case now copies the segments aside with no proxy up, and puts them back on every exit and in the teardown.
|
Round 2 of 10 on the runtime's review findings, pushed as
Verify Review threads the coder judged:
On a macOS host with gvproxy and a GitHub sign-in, the case shows the audit log rotating with its chain carried across, Notes from the agent: This round fixes the pruning finding in scripts/session-e2e.sh only. The case first runs the live gate to check that a proxy can run on the host. Then it stops the daemon pair, checks that no |
There was a problem hiding this comment.
minimal-review — approval withheld
Verdict COMMENT — APPROVE withheld: 3 unresolved threads — minimal-arch[bot] scripts/session-e2e.sh:15498, minimal-foundry[bot] scripts/session-e2e.sh, minimal-arch[bot] scripts/session-e2e.sh:15498. No findings.
Panel: 3 of 3 personas ran (correctness, security, supply-chain).
There was a problem hiding this comment.
arch-review: PASS_WITH_CONDITIONS · 1 condition · the full ruling
The compare changes only the e2e case. The case now copies the real audit segments aside with no proxy up, and puts them back on every exit and in the teardown. That keeps the helper's hash-chained log as the case found it (architecture.md line 505). The bep change from the earlier head stands: a release build ignores BEP_AUDIT_SEGMENT_BYTES. One condition stays open. The e2e case is NOT RUN on Linux, so run it on the macOS lane at this head and record the result in the PR body (C1).
Ruled at a3e57f3 against gominimal/arch@8640f85e193d by claude-opus-5-5.
|
/revise One defect from review at a3e57f3, thread PRRT_kwDOPdfcxM6yN0Ln:
Do not change anything else. The release-build gate on |
bep has no setting that moves its audit directory, so the case renames the real one aside before the restart and back on every exit. No proxy opens the real log while the case runs, so rotation prunes none of it.
|
Round 3 of 10 on a /revise note, pushed as
Verify Review threads the coder judged:
On a macOS host with a live proxy, the rotation case rotates the audit log with its chain carried across, Notes from the agent: For the PR body: bep has no setting that moves its audit directory. |
There was a problem hiding this comment.
arch-review: PASS_WITH_CONDITIONS · 1 condition · the full ruling
The bep override and the e2e case conform to architecture.md line 505 and Gatehouse §6.10 line 836. The new script no longer rewrites the real audit log. It renames the real directory aside and gives it back after the case. The e2e case still reports NOT RUN on Linux, so the proof is open. C1 asks for a passing macOS-lane run of the case at this head.
Ruled at 6a432a0 against gominimal/arch@8640f85e193d by claude-opus-5-5.
The rotation case now stops with an error that asks for a debug build of bep when bep.log shows that a release build ignored the override.
|
Round 4 of 10 on the runtime's review findings, pushed as
Verify Review threads the coder judged:
On a macOS host with a live proxy, the e2e case drives the audit log past a small segment bound. It checks that the chain carries across the rotated segments, that Notes from the agent: Round 10BC8 4. The e2e case now looks for the release-build warning in bep.log and fails with a message that asks for a debug build of bep through MINVMD_BEP_BIN. The debug_assertions gate in crates/bep/src/main.rs stays. An earlier round added it on purpose, so a stray value in a release build cannot make the log rotate fast and prune its history. The e2e case was NOT RUN here (Linux, no macOS proxy). The macOS lane still owes a passing run. |
There was a problem hiding this comment.
arch-review: PASS_WITH_CONDITIONS · 1 condition · the full ruling
The change conforms to architecture.md line 505 and Gatehouse §6.10 line 836. The new step 1a fails closed when a release bep ignores the override. One item stays open. The e2e case is NOT RUN at this head, and NOT RUN is not a pass. Run it on the macOS lane with a debug bep and put the result in the PR body, as C1 says.
Ruled at ad359f3 against gominimal/arch@8640f85e193d by claude-opus-5-5.
There was a problem hiding this comment.
minimal-review — approval withheld
Verdict COMMENT — APPROVE withheld: 6 unresolved threads — minimal-arch[bot] scripts/session-e2e.sh:15520, minimal-arch[bot] scripts/session-e2e.sh:15520, minimal-arch[bot] scripts/session-e2e.sh:15450, minimal-arch[bot] scripts/session-e2e.sh:15485, minimal-foundry[bot] crates/bep/src/main.rs:1291, …. No findings.
Panel: 3 of 3 personas ran (correctness, security, supply-chain).
|
/revise I ran
The release-build check that thread PRRT_kwDOPdfcxM6yOFd3 asks for is already at this head. I re-checked it. Change nothing else. |
The lane's RUST_LOG filter drops bep's INFO records, so the bound line never reached bep.log. The case appends bep=info for its own restart and restores the lane's filter in ar_cleanup.
|
Round 5 of 10 on a /revise note, pushed as
Verify Review threads the coder judged:
On a Mac with a debug bep, the rotation case now finds the bound line in bep.log, sees the audit log rotate into new segments with the chain carried across, and reads the rotation lines. The min box audit step still fails because of #2462 (the proxy records the 32-hex box id, and min box audit matches the hyphenated id). So the min doctor steps and 'Audit rotation OK' cannot run until #2462 is fixed. Notes from the agent: Revise round 5. Item 1 is fixed: before the second live gate, the case saves RUST_LOG and appends ',bep=info', so minvmd passes the filter to bep serve. ar_cleanup restores the saved filter, or unsets RUST_LOG if it was not set. The default filter of the lane does not change. Item 2, evidence for the PR body Tests table, in place of the NOT RUN row: the owner ran the case live on macOS 26.7 arm64 at ad359f3. The bep was a debug build with one local, uncommitted change for #2436 (keep an exportable keychain key), which does not affect the audit code. RUST_LOG was 'warn,minimald::exec=info,bep=info', the same filter this commit sets. The case exited non-zero at the min box audit step. Its lines: 'moved the real audit log aside to ~/.minimal/bep/audit.e2e-saved, with 1 segment(s)' / 'bep serve started with the audit segment bound 4096' / 'the audit log rotated: 2 new segment(s) after 21 request(s), the active one is audit-2.jsonl' / '... bep::audit: rotated the audit log (authority="api.github.com:443" upstream_tls="valid" substitution="substituted")' / '::error::'min box audit e2e-bep-audit-rotation': the records of box 01a1907a-a9db-7434-80c7-3a25af0bc901 are in 0 segment(s), not 2 or more' / 'gave back the real audit log in ~/.minimal/bep/audit, as it was before the rotation case'. Residual: #2462 (BEP-042, id spelling) blocks the min box audit step, the min doctor steps and 'Audit rotation OK'. The step stays hard. I did not run the case live at the new head: this box is Linux, and the gate reports NOT RUN. |
… parses it
macOS /bin/bash 3.2 mis-parses a quoted heredoc inside $(...) whose body
holds an apostrophe and backticks: bash -n failed with a syntax error near
unexpected token '(' and every macOS e2e case failed (CI job 124474093151).
Reword the Python comment so it carries neither.
There was a problem hiding this comment.
Re-reviewed at ed5362e. That commit only rewords the step-3 Python comment inside the heredoc. It drops the apostrophe and the backticks that broke the parse under the macOS runner's /bin/bash 3.2 (job 124474093151), and it corrects proxy.rs to listener.rs. /bin/bash -n scripts/session-e2e.sh exits 0. The rest is unchanged from 8ccb050.
There was a problem hiding this comment.
arch-review: PASS_WITH_CONDITIONS · 1 condition · the full ruling
The compare changes only a comment in the step-3 heredoc. The comment no longer has quotes, so macOS bash can parse the heredoc in $( ). It also cites box_id_text in listener.rs correctly. The earlier ruling stands. The change conforms to architecture.md line 505 and Gatehouse §6.10 line 836. The live macOS run of the rotation case is still open. C1 keeps that run open on #2798 with every assert hard.
Ruled at ed5362e against gominimal/arch@8640f85e193d by claude-opus-5-5.
…ents-rotate-and-the-chain-is-checked-e
|
Round 9 of 10 on the runtime's review findings, pushed as
Verify Review threads the coder judged:
At this branch, a debug Notes from the agent: Round 9. bep: |
There was a problem hiding this comment.
arch-review: PASS_WITH_CONDITIONS · 1 condition · the full ruling
The compare is a clean merge of main and one small fix in crates/bep/src/main.rs. The fix logs the test override only when it applies. A bad value still keeps the default bound, which is the safe side. The audit design in architecture.md line 505 and Gatehouse §6.10 line 836 is met. One item stays open: no live run of the rotation case reached min box audit, min doctor and Audit rotation OK. C1 records that run on #2798, and the steps stay hard.
Ruled at 7bf1bb6 against gominimal/arch@8640f85e193d by claude-opus-5-5.
|
The runtime will not revise this pull request again on its own: round 11 would exceed the cap of 10; 1 base merge was not counted (the last trigger was a failed check suite). A person decides now. Remove |
|
Round 10 of 10 on the runtime's review findings, pushed as
Verify Review threads the coder judged:
With BEP_AUDIT_SEGMENT_BYTES set on a debug bep serve, the audit log rotates and carries its chain across segments, min box audit reads every segment, and min doctor reports an edited chain. The unit test now also proves that a release build keeps the default bound. The e2e case that shows this end to end still needs a live macOS run. Notes from the agent: This round: audit_rotation now delegates to audit_rotation_for(set, debug), and the T35 unit test pins the debug and release gate. The e2e case reports NOT RUN on this Linux box, because the Box Egress Proxy runs on macOS only. A macOS lane run that prints 'Audit rotation OK' is still owed on #2798. The debug-only rule is deliberate from an earlier round (a855d7e), and it is wider than the task text says. |
There was a problem hiding this comment.
arch-review: PASS_WITH_CONDITIONS · 1 condition · the full ruling
The new commit moves the release gate of BEP_AUDIT_SEGMENT_BYTES into audit_rotation_for and pins it with a unit test. The behaviour does not change, and the proof is stronger. The ruling on the earlier head stands. The design (architecture.md line 505, Gatehouse §6.10 line 836) puts the chain check over the whole log in min doctor, and the change follows it. The live e2e leg did not yet reach min box audit, min doctor and Audit rotation OK. That leg stays open on #2798 as C1.
Ruled at 32c9be0 against gominimal/arch@8640f85e193d by claude-opus-5-5.
There was a problem hiding this comment.
APPROVE withheld: 5 unresolved threads — minimal-arch[bot] scripts/session-e2e.sh:15613, minimal-foundry[bot] crates/bep/src/main.rs, minimal-arch[bot] scripts/session-e2e.sh:15681, minimal-foundry[bot] crates/bep/src/main.rs, minimal-arch[bot] scripts/session-e2e.sh:15448.
minimal-review — advisory review
Verdict COMMENT — No findings.
Panel: 2 of 3 personas ran (correctness, security); did not complete: supply-chain.
A zero or unparsable BEP_AUDIT_SEGMENT_BYTES keeps the default bound. bep serve now warns that it ignored the value, in place of the info line that said the override applied.
audit_rotation_for takes the build, so the test proves a release build keeps the default bound when BEP_AUDIT_SEGMENT_BYTES is set.
32c9be0 to
4a14c96
Compare
There was a problem hiding this comment.
Approving at 4a14c96. Above ed5362e, the branch has a merge of main (fbdcfd8) and two commits from rounds 9-10 that touch only crates/bep/src/main.rs:
audit_segment_overridesplits out the parse.bep servenow warns when a debug build ignores a zero or unparsableBEP_AUDIT_SEGMENT_BYTES, where before it logged an info line saying the override applied.audit_rotation_fortakes the build, so a test pins that a release build keeps the default bound.
A local worker rewrapped the two commit bodies for commitlint (body-max-line-length). The tree is unchanged from 32c9be0.
There was a problem hiding this comment.
arch-review: PASS_WITH_CONDITIONS · 1 condition · the full ruling
The tree is identical to 32c9be04, so the earlier ruling stands. The segment bound is debug-only and in bep, which owns the helper's log. The e2e case verifies the chain with min doctor, as architecture.md line 505 and Gatehouse §6.10 line 836 require. The live run of the case did not reach steps 3 to 5. C1 keeps that leg open, and #2798 owns it.
Ruled at 4a14c96 against gominimal/arch@8640f85e193d by claude-opus-5-5.
There was a problem hiding this comment.
minimal-review — approval withheld
Verdict COMMENT — APPROVE withheld: 6 unresolved threads — minimal-arch[bot] scripts/session-e2e.sh:15613, minimal-foundry[bot] crates/bep/src/main.rs, minimal-arch[bot] scripts/session-e2e.sh:15681, minimal-foundry[bot] crates/bep/src/main.rs, minimal-arch[bot] scripts/session-e2e.sh:15448, …. No findings.
Panel: 3 of 3 personas ran (correctness, security, supply-chain).
There was a problem hiding this comment.
minimal-review — approved
Verdict APPROVE — every review thread is resolved; @norrietaylor closed the last one. The review of 4a14c96110c5 withheld this approval on the threads open then. Posted as a comment: the App cannot approve its own pull request; the check and the label carry it.
T35 — Audit segments rotate and the chain is checked, end to end
slice: S5
kind: integration
covers: none
size: M
est: 120 code, 180 tests
executor: agent
difficulty: routine
model: standard
diagnostics: the case's last step prints the chain verified across the rotated segments
observability: the rotation log lines of the run, and one info line at
bep servestart naming the segment bound when the test override sets itlayers:
- bound:
- crates/bep/src/main.rs:
open_audit_logtakes the segment bound fromBEP_AUDIT_SEGMENT_BYTESwhen it is set to a positive integer, elseaudit::SEGMENT_BYTES; the variable is a test surface, read only bybep serve, whichminvmdspawns with its own environment (widened 2026-10-31, owner, #2295 noop: no runtime knob)test: cargo nextest run -p bep audit_segment_bound_follows_the_test_override
- e2e:
- scripts/session-e2e.sh: one case: set
BEP_AUDIT_SEGMENT_BYTESfor the restarted host daemon, drive requests past the bound, read the trail, edit a record, runmin doctortest: ./scripts/session-e2e.sh bep_audit_rotation_and_doctor_end_to_end
produces: none
consumes: none
proof: The audit log rotates with its chain carried across,
min box audit reads every segment, andmin doctorreports an edited chain.depends on:
- blocked by #2290
- blocked by #2293
Proof
Set BEP_AUDIT_SEGMENT_BYTES to a positive integer, and
bep serverotates its audit log at that size and logs the bound at start. The new e2e case bep_audit_rotation_and_doctor_end_to_end rotates the log twice and checks thatmin box auditshows the box's records from every segment. Then it edits one record and checks thatmin doctorprints the break and exits 6. It restores the record and checks thatmin doctorprints 'Audit log: chain verified.'. The unit test proves the override and the rotation with the chain intact. The e2e evidence still comes from the macOS lane, because the case is NOT RUN on Linux.Notes from the agent: The e2e case needs macOS, gvproxy and a GitHub sign-in (bep_live_gate). On this Linux box it reports NOT RUN, so its steps have not yet run against a live proxy. The case moves the real audit log under $HOME/.minimal/bep/audit aside for the run and puts it back afterwards. It stops the proxy first, so no writer races the edit. The case cleanup and the script teardown both restore the segment. With a 4096-byte bound, the run opens at least two segments, so on a host that already keeps 16 segments it can prune the oldest ones. At the end the case unsets the variable and stops the daemon pair, so no later case gets a proxy with the small bound. I added the case to the whole-lane run after bep_round_two_conditions_closed, to the dispatch arm, and to the usage list. clippy -p bep --all-targets and cargo fmt --check are clean. shellcheck -S warning is clean on the script.
Tests
cargo nextest run -p bep audit_segment_bound_follows_the_test_override./scripts/session-e2e.sh bep_audit_rotation_and_doctor_end_to_end(macOS 26.7 arm64, live, at ad359f3 withRUST_LOG=…,bep=info, the fix dc7bc11 now makes)bep serve started with the audit segment bound 4096·the audit log rotated: 2 new segment(s) after 21 request(s), the active one is audit-2.jsonl· rotation lines carryauthority="api.github.com:443" upstream_tls="valid" substitution="substituted"·::error::'min box audit e2e-bep-audit-rotation': the records of box 01a1907a-a9db-7434-80c7-3a25af0bc901 are in 0 segment(s), not 2 or more·gave back the real audit log in ~/.minimal/bep/audit, as it was before the rotation caseResidual: the live run at ad359f3 stopped at step 3 for two reasons, both now fixed. The case's own check compared
subjectwith the hyphenated id; 9d940d0 compares the proxy's 32-hex form (box_id_text).min box auditmatched only the hyphenated id; #2462 fixed that and merged in #2793. No live run has yet reachedmin box audit,min doctorandAudit rotation OK. By the owner's decision of 2026-10-31 this PR merges with that run open on #2798; the steps stay hard. Rotation, the bound, substitution and the real log's restore are shown live above. The run used a debugbepwith one local, uncommitted change that keeps an exportable keychain key (#2436, T111 #2785); it does not touch the audit code. On Linux the case still reports NOT RUN (the proxy runs on macOS only).Verify
min task run agent-verifypassed ([tasks.verify]).Closes #2295