Repository navigation
feat(bep): rotate audit segments, carry the chain and restore revocations at start (T33) - #2583
Conversation
The audit log rotates at a size bound, and each new segment's first record carries the previous segment's final hash (BEP-068). Pruning keeps a segment that holds a revocation while a value naming its box can be unexpired. bep serve restores every revocation from every retained segment at start, so a destroyed box stays refused after a restart.
min box audit reads every segment left in the log's directory. The new test writes through the rotating log, prunes the oldest segment, and checks that the rest of the trail still reads in order (BEP-068).
|
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 · 3 conditions · the full ruling
The rotation, the carried chain, the restore at start and the revocation-aware prune conform to Gatehouse §6.10 line 839. Three edits remain. C1: the doc comments in redeemer.rs still say that #2290 is open. This PR closes #2290, so rewrite them. C2: spec BEP-041 forbids the removal of any record, but §6.10 permits compaction. Amend BEP-041. C3: a revocation line that does not parse is dropped from the restore without a log line, and its segment can be pruned. Keep such a segment, and log the skipped count at start.
Outside the diff
crates/bep/src/redeemer.rs:109: C1 · This comment says the redeemer starts with no revocation in force until #2290. This PR closes #2290. Rewrite it to say thatbep servepasses the restored revocations throughwith_revocations. Do the same at line 225.docs/specs/24-spec-box-egress-proxy/24-spec-box-egress-proxy.md:565: C2 · BEP-041 forbids the removal of any record. BEP-068 retention removes whole segments, and §6.10 line 839 permits that compaction. Amend BEP-041 so that it allows the removal of a whole segment that retention no longer keeps.
Ruled at cb3dc9d against gominimal/arch@109541a48c31 by claude-opus-5-5.
There was a problem hiding this comment.
minimal-review — approval withheld
Verdict COMMENT — APPROVE withheld: 3 unresolved threads — minimal-arch[bot] crates/bep/src/audit.rs:662, minimal-arch[bot] crates/bep/src/revoke.rs:30, minimal-arch[bot] crates/bep/src/main.rs:669. No findings.
Panel: 3 of 3 personas ran (correctness, security, supply-chain).
|
/revise Round 1 of #2583, on the arch ruling at cb3dc9d (PASS_WITH_CONDITIONS, 3 conditions). No round is in flight. Do all three. They are in-scope edits, not residuals.
One commit, Conventional Commits subject, every body line at most 72 characters. Generated by Claude Code |
…it-segments-with-the-chain-carried-acr
A complete audit line that does not parse can be a revocation. prune now keeps the segment that holds one, and restore returns the count of skipped lines with the box ids. bep serve logs a warning with the count and the audit directory when it is not zero. The redeemer doc comments now say that bep serve restores revocations through with_revocations. BEP-041 now permits the BEP-068 retention prune to remove a whole segment.
…it-segments-with-the-chain-carried-acr
Spec lint
Advisory. None of this blocks the merge; the review decides. Advisory, and never a merge gate. Linter: |
|
Round 1 of 10 on a /revise note, pushed as
Verify Outside the task's
Converted to a draft: something above needs a person before review. Review threads the coder judged:
After rotation, Notes from the agent: Revise round 1, one commit 49f92fc. C1: changed doc comments in crates/bep/src/redeemer.rs. C2: changed BEP-041 in docs/specs/24-spec-box-egress-proxy/24-spec-box-egress-proxy.md to the given text. The directive asked for both files, but they are outside the task's layers. revoke::restore now returns a Restored struct with box_ids and skipped. A partial last line (no trailing newline) is a torn write. It does not keep a segment and is not counted as skipped. I did not resolve the threads, because the reviewer resolves them. I did not run just ci or clippy-strict. cargo clippy -p bep --all-targets and cargo fmt -p bep --check are clean. |
There was a problem hiding this comment.
arch-review: PASS · the full ruling
The change conforms to Gatehouse §6.10 line 839. Segments rotate with the chain carried across. bep serve restores the revocations from every retained segment. prune keeps a segment with a revocation, or with a line it cannot parse, until the ceiling passes. The new commit reports skipped lines and amends BEP-041 to permit removal of a whole segment only. There are no conditions.
Ruled at a48affc against gominimal/arch@109541a48c31 by claude-opus-5-5.
A complete audit line that does not parse can be a revocation. bep serve started without it and redeemed values for a destroyed box. It now refuses the start and names each segment that holds such a line.
|
Round 2 of 10 on the runtime's review findings, pushed as
Verify Outside the task's
Converted to a draft: something above needs a person before review. Review threads the coder judged:
After rotation, Notes from the agent: Round 2: |
There was a problem hiding this comment.
arch-review: PASS · the full ruling
The new commit makes bep serve refuse to start when an audit line does not parse. Before, it logged a warning and started without that line's revocation. Gatehouse §6.10 line 839 restores every revocation from every retained segment at start, so this refusal fails closed and conforms. bep still opens the log for append only. The PASS on a48affcc stands for the rest of the change, and this head has no conditions.
Ruled at c25f20d against gominimal/arch@109541a48c31 by claude-opus-5-5.
…it-segments-with-the-chain-carried-acr
The lock is per segment. A writer that listed the directory before another writer rotated could lock the closed segment and fork the chain there. open_dir now lists again after the lock and moves to the newest segment.
There was a problem hiding this comment.
arch-review: PASS · the full ruling
The new commit makes AuditLog::open_dir list the segments again after it takes the lock. A writer with a stale listing now moves to the newest segment, or it refuses with WouldBlock. This keeps the single hash chain that Gatehouse §6.10 line 839 requires. The merge of main is clean and is not part of this ruling. The PASS on c25f20d4 stands for the remainder of the change.
Ruled at d84c1d7 against gominimal/arch@109541a48c31 by claude-opus-5-5.
|
Round 3 of 10 on the runtime's review findings, pushed as
Verify Outside the task's
Converted to a draft: something above needs a person before review. Review threads the coder judged:
After rotation, Notes from the agent: This round fixes the one review thread. AuditLog::open_dir lists the directory again after it takes the segment lock. If a newer segment is in the second listing, it drops the stale segment unwritten and tries the newest one. It tries 8 times at most, then returns WouldBlock. The new test open_dir_relists_after_a_rotation in crates/bep/src/audit.rs pins this. The four test: lines and the new test pass. cargo clippy -p bep --all-targets shows no warnings. Known edge: a stale listing can name a segment that prune already removed. Then open creates an empty file with that number before the second listing finds the newer one. That empty file holds no record and does not change the chain. |
|
/revise Round 4 of #2583, on review finding 4373323173 (security, rated LOW), by the epic owner's decision on 2026-10-26. No round is in flight. Keep the fail-closed refusal as it is. The chain-preserving way out is #2588 and is not part of this round. The finding: each audit segment has its own lock, and
Keep to Generated by Claude Code |
…it-segments-with-the-chain-carried-acr
Each audit segment had its own lock, and a rotation released the closed segment. A second `bep serve` with a stale listing could lock that segment, append there, and fork the hash chain. `AuditLog::open_dir` now locks a `.lock` file in the directory and holds it for the life of the log, across rotations. A second open of the directory fails. `bep serve` takes the lock before it restores the revocations, and prints a refusal that names the directory and says to stop the other `bep serve` and run it again.
There was a problem hiding this comment.
arch-review: PASS · the full ruling
The compare adds one lock on the audit directory. bep serve holds this lock for the life of the log and across each rotation. This keeps one writer on the hash chain that §6.10 line 839 and architecture.md line 501 require. A second writer is refused, so the change fails closed. The baseline PASS stays. There are no conditions.
Ruled at 2899e8e against gominimal/arch@109541a48c31 by claude-opus-5-5.
|
Round 4 of 10 on a /revise note, pushed as
Verify Outside the task's
Converted to a draft: something above needs a person before review. Review threads the coder judged:
After rotation, Notes from the agent: Round 4 is in commit 2899e8e. AuditLog::open_dir locks bep serve'. In bep serve, open_audit_log maps WouldBlock to: 'bep did not start. Another bep serve writes the audit log in .' / 'Only one bep serve can write the audit log at a time.' / 'Stop the other bep serve, then run bep serve again.' serve_refuses_a_second_writer_of_the_audit_log pins this text. bep serve now opens the log before revoke::restore, so no other writer rotates segments during the read. The per-segment lock and the re-list after it stay as a guard against an older writer that takes no directory lock. The .lock file does not match the segment name pattern, so segments(), prune and min box audit ignore it. The fail-closed refusal on unreadable lines is unchanged. Directive item 4 asks me to resolve the thread. I cannot resolve threads, so the thread verdict names the commit and the reviewer can resolve it. I did not run just clippy-strict.
|
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 2899e8e51fba 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.
There was a problem hiding this comment.
Babysitter approval at 2899e8e: every merge gate passes at this head (arch PASS at 2899e8e, zero open threads, every lane aggregator green). Squash title feat(bep): rotate audit segments, carry the chain and restore revocations at start (T33) and the squashed message pass commitlint. Queued for squash merge.
Generated by Claude Code
The revocation helper that main added in #2583 builds a Decision without jti, which failed the merge-queue build with E0063.
…l#2596) * feat(bep): carry the handle jti into the audit record (T68) The forwarder asks the redeemer for the handle's jti beside its verdict, and writes it into each decision record. min box audit shows it. The jti is empty when the request carries no handle or the handle does not parse. * test(bep): expect jti in the audit record field set * docs(bep): say a refused record's jti is the handle's claim Only an admitted record's jti is verified, since decide admits only a verified handle. A refusal keeps the claimed jti so a forged handle still correlates. * fix(bep): log the handle jti escaped in the redeemer A refused handle's jti is an unverified claim, so a sender controls it. Debug formatting escapes control characters in the log line. * fix(bep): set an empty jti on the revocation test record The revocation helper that main added in gominimal#2583 builds a Decision without jti, which failed the merge-queue build with E0063. --------- Co-authored-by: minimal-foundry[bot] <327719912+minimal-foundry[bot]@users.noreply.github.com>
T33 — Rotate audit segments with the chain carried across and revocations restored
slice: S5
kind: task
covers: BEP-068
size: M
est: 250 code, 240 tests
executor: agent
difficulty: needs care
model: standard
diagnostics:
min box audit <box>reads across every retained segment, so a box's trail is whole after rotationobservability: one info log line per rotation with the closed segment and its final hash, and one at start with the revocations restored
layers:
- audit:
- crates/bep/src/audit.rs: segments rotated with the chain carried, revocation segments retained (BEP-068)
- crates/bep/src/main.rs:
bep servepasses the revocations restored from every retained segment toBoxRedeemer::with_revocations, and drops the revocation interim and its clause from the startup warning (T41)- crates/bep/src/revoke.rs: restores every revocation from every retained segment at start
test: cargo nextest run -p bep segment_rotation_continues_chain
test: cargo nextest run -p bep revocations_restored_from_every_segment_on_start
test: cargo nextest run -p bep segment_with_live_revocation_is_retained
- cli:
- crates/minimal/src/cmd/box_audit.rs: reads every retained segment
test: cargo nextest run -p minimal box_audit_reads_across_segments
produces: none
consumes:
- crates/bep/src/revoke.rs
proof: After rotation,
min box auditstill shows a box's whole trail, and a restart still refuses a destroyed box.depends on:
- blocked by #2276
- blocked by #2277
Proof
bep serve now writes the audit log as segments that rotate at 8 MiB. The first record of each new segment carries the previous segment's final hash.
min box audit <box>shows the box's records from every segment that is still in the log directory, in order, also after a rotation and a prune. At start, bep serve restores every revocation from every retained segment. So after a restart, a value for a destroyed box is still refused, because its segment is kept until every value naming the box can have expired.Notes from the agent: The size and retention bounds were not in the task, so I chose them: SEGMENT_BYTES = 8 MiB, RETAINED_SEGMENTS = 16, and REVOCATION_CEILING = ROOT_VALIDITY_SECS (10 years). The ceiling is the root CA's full validity, because a value's expiry is the root's remaining validity at the mint (BEP-005, BEP-063). Records carry no timestamp, so prune uses the segment file's mtime as the time of its revocation records. This is a safe upper bound, because no value is minted for a box after its destroy. Pruning removes whole old segments. The BEP-041 statement 'remove no existing record' does not allow for this, but BEP-068 retention requires it. A reviewer may want the spec to say this. AuditLog::open (no rotation) is unchanged, and AuditLog::open_dir is the new rotating entry point that bep serve uses. The bep crate's whole suite passed once (175 tests), and clippy -p bep and -p minimal --all-targets gave no new warnings. Outside the layers, I did not touch two doc comments in crates/bep/src/redeemer.rs (lines 110 and 225). They still say that T33 (#2290) will restore revocations, and they are now stale. A follow-up must update them.
Tests
cargo nextest run -p bep segment_rotation_continues_chaincargo nextest run -p bep revocations_restored_from_every_segment_on_startcargo nextest run -p bep segment_with_live_revocation_is_retainedcargo nextest run -p minimal box_audit_reads_across_segmentsVerify
min task run agent-verifyexited 101 under rustc 1.98.1, which the repository does not pin (1.97.0 in rust-toolchain.toml); the PR is not a draft on that alone — CI, which honours the pin, is the gate.Residuals
bep serverefuses to start on an unreadable audit line, and a downgrade past a newer record kind (epic owner's decision, 2026-10-26).Closes #2290
Generated by Claude Code