Repository navigation
feat(bep): keep a replaced client key verifying until its boxes end (T84) - #2615
norrietaylor merged 9 commits into
Conversation
register_client_key can name the box it signs for, and forget_box ends that use. A replacement retires a key in use rather than revoking it, and the proxy revokes it when the last such box is forgotten. The bound drops only a key no running box uses, and refuses a new key when every key is in use.
The mint names its box when it registers the client key, so the proxy
keeps a replaced key verifying until that box ends. A registration
that the proxy's full bound refuses fails the creation with text that
says why.
The replace and bound sub-requirements now keep a key that a running box uses, name the tests that prove it, and drop the known-gap notes. The pending-revoke note says the client removes the file.
…laced-client-key-verifying-until-the-b
|
Important Review skippedBot user detected. To trigger a single review, invoke the
 |
Spec lint
Advisory. None of this blocks the merge; the review decides. Advisory, and never a merge gate. Linter: |
The inventory test failed CI because the branch renamed one mint test and added two that serve a proxy control socket.
|
Round 1 of 10 on a failed check suite, pushed as
Verify Outside the task's
Converted to a draft: something above needs a person before review. After a client key replacement, a box created under the old key keeps redeeming its store handles and sign-in references until it ends. The registry bound never drops a key that a running box uses. The CI tests job no longer fails on the daemon connect inventory. Notes from the agent: The CI log excerpt did not name the failing test. A local run of the minimal crate found it: tests::every_daemon_connection_is_classified in crates/minimal/src/tests.rs. That test lists each connect site by name, and the branch's mint.rs tests changed the list. The branch renamed a_replaced_client_key_is_revoked_at_the_proxy to a_replaced_client_key_keeps_verifying_until_its_boxes_end, which has three connect sites, and added a_replacement_past_a_bound_of_keys_in_use_fails_the_creation. crates/minimal/src/tests.rs is outside the task's layers. I changed it because it is a registry table that must list the connect sites in mint.rs, and no change inside the layers can make it pass. ci-linux-native-success is the lane aggregator. It failed only because the tests job failed. |
|
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: CONDITIONS · 1 condition · the full ruling
The keep and the in-use bound conform to Gatehouse §6.10 line 839. A replaced key keeps verifying for the life of the boxes minted under it. One item is open. The PR body names a known limit with no issue: a box that ends without forget_box keeps its key in use across restarts. With 16 such boxes, the bound refuses every new key. Rule 6 does not accept a residual with no issue. C1 files the issue and names the gap in the BEP-063 bound comment.
Ruled at d1addbb against gominimal/arch@109541a48c31 by claude-opus-5-5.
A box that ends without forget_box stays in use across restarts, so sixteen such boxes make the bound refuse every new key. Also explain why release updates the registry before it writes the file.
|
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 a client key replacement, a box created under the old key keeps redeeming its store handles and sign-in references until it ends, and the registry bound never drops a key that a running box uses. The spec now also names the known gap of a box that ends with no forget_box. Notes from the agent: Thread C1 asks for a filed issue and its link. This session has no credential to file an issue, so the known-gap clause says 'issue not yet filed'. A person must file the issue and replace that text with its link. just lint-prose did not run: vale is not installed on this box. |
There was a problem hiding this comment.
arch-review: CONDITIONS · 1 condition · the full ruling
The keep, the in-use bound and the refusal text conform to Gatehouse §6.10 lines 839 and 842. The spec now states a known gap: a box with no forget_box holds its key forever, and 16 such boxes block every new key. The spec marks it "(issue not yet filed)". A residual with no issue has no owner. C1 tells you to file the issue, link it from the spec, and add the failed release write as a second cause.
Ruled at 78a1700 against gominimal/arch@109541a48c31 by claude-opus-5-5.
There was a problem hiding this comment.
minimal-review — approval withheld
Verdict COMMENT — APPROVE withheld: 4 unresolved threads — minimal-arch[bot] docs/specs/24-spec-box-egress-proxy/24-spec-box-egress-proxy.md:502, minimal-foundry[bot] crates/bep/src/control.rs:877, minimal-arch[bot] docs/specs/24-spec-box-egress-proxy/24-spec-box-egress-proxy.md:494, minimal-arch[bot] crates/bep/src/control.rs:870. No findings.
Panel: 3 of 3 personas ran (correctness, security, supply-chain).
|
/revise Round 3 of #2615, on arch condition C1 at 78a1700. No round is in flight.
Generated by Claude Code |
Name the second cause of the gap: a failed write of the registrations file at a box's end leaves the ended box in the file across a restart. Correct the release comment to match, and say in the control module docs that bep serve --max-client-keys sets the bound.
|
Round 3 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 a client key replacement, a box created under the old key keeps redeeming its store handles and sign-in references until it ends, and the registry bound never drops a key that a running box uses. This round only changes text: the spec gap names #2617 and both causes, and the control module docs name bep serve --max-client-keys. Notes from the agent: Item 1 done: the spec gap links #2617 and adds the failed-write cause; the release comment now says a failed write leaves the ended box in the file until a later successful write, and a restart before it restores the retired key as in use (#2617). Item 3 done: the module docs say bep serve --max-client-keys sets the count, 16 by default. Item 2 not done: the inventory in crates/minimal/src/tests.rs lists one entry for each connection site, and a_replaced_client_key_keeps_verifying_until_its_boxes_end has three sites (as store_handle_signed_and_key_registered has two entries). With one entry, every_daemon_connection_is_classified fails, so I reverted that edit. just lint-prose did not run: vale is not installed on this box. |
There was a problem hiding this comment.
arch-review: PASS · the full ruling
The compare to this head changes only comments and the BEP-063 spec note. It answers the open points of the ruling on 78a1700. The stranded-box gap now links to #2617, which is the residual that owns it. The KeyFile::release comment and the module doc on --max-client-keys now match the code. The keep of a replaced key until it
10BC8
s boxes end conforms to Gatehouse §6.10 line 839.
Ruled at 660385e against gominimal/arch@109541a48c31 by claude-opus-5-5.
A box that ends with no forget keeps its retired client key verifying with no time limit. The BEP-063 known gap now says so, and says why neither a maximum age nor a reconcile at restore fits this task (#2617).
There was a problem hiding this comment.
arch-review: PASS · the full ruling
The new commit changes only the known-gap comment of the BEP-063 replace sub-requirement. It states the stranded-box gap more exactly and keeps the link to #2617. The design keeps handles working for the life of the box (Gatehouse §6.10 line 839). Each handle still carries its own exp (line 842). The PASS on 660385e stands. No condition is open.
Ruled at dfd737c against gominimal/arch@109541a48c31 by claude-opus-5-5.
|
Round 4 of 10 on the runtime's review findings, pushed as
Verify Review threads the coder judged:
After a client key replacement, a box created under the old key keeps redeeming its store handles and sign-in references until it ends. The registry bound never drops a key that a running box uses. The BEP-063 known gap now also says that a box which ends with no forget keeps its retired key verifying with no time limit (#2617). Notes from the agent: Round 4 changes only the BEP-063 known-gap comment in the spec. The code needs no change this round. A maximum age for a retired key conflicts with the gominimal/arch#144 ruling, because a box that runs longer than that age loses its keep. A reconcile at restore needs a list of live boxes, and the proxy does not hold one. That work is outside this task's layers and #2617 tracks it. Vale is not installed on this box, so just lint-prose did not run on the changed line. |
A key that a running box uses is never dropped, so it no longer keeps a known key's registration off the fast path. A failed write at a box's end makes the next registration write the file again.
There was a problem hiding this comment.
arch-review: PASS · the full ruling
The compare conforms, and the PASS on dfd737cb stands for the rest of the change. within() now skips the kept key and keys in use, as enforce() does. So the fast path writes nothing only when the bound drops nothing. A failed write at the end of a box now makes the next registration write the file again. This keeps handles working for the life of the box, as Gatehouse §6.10 line 839 requires.
Ruled at 9432d20 against gominimal/arch@109541a48c31 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] docs/specs/24-spec-box-egress-proxy/24-spec-box-egress-proxy.md:502, minimal-arch[bot] docs/specs/24-spec-box-egress-proxy/24-spec-box-egress-proxy.md, minimal-arch[bot] crates/bep/src/control.rs:873, minimal-foundry[bot] crates/bep/src/control.rs:764, minimal-foundry[bot] crates/bep/src/control.rs, …. No findings.
Panel: 3 of 3 personas ran (correctness, security, supply-chain).
|
Round 5 of 10 on the runtime's review findings, pushed as
Verify Review threads the coder judged:
After a client key replacement, a box created under the old key keeps redeeming its store handles and sign-in references until it ends. The registry bound never drops a key that a running box uses, and a key in use no longer keeps every later registration of a known key off the fast path. Notes from the agent: Round 5. |
There was a problem hiding this comment.
Own review at 9432d20. replace_client_key keeps a key while a running box uses it and retires it when the last such box ends. The bound drops only an unused key and refuses with the diagnostics text when every key is in use. within and enforce agree on what can be dropped, and the spec states the keep. The stranded-box gap is #2617. Two security threads (4377402594 and 4377485218) wait on the epic owner.
Generated by Claude Code
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 9432d2098da5 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 9432d20: every merge gate passes at this head (arch PASS at 9432d20, zero open threads, every lane aggregator green). Squash title feat(bep): keep a replaced client key verifying until its boxes end (T84) and the squashed message pass commitlint. Queued for squash merge.
Generated by Claude Code
T84 — Keep a replaced client key verifying until the boxes minted under it end
slice: S10
kind: task
covers: BEP-063
size: M
est: 160 code, 160 tests
executor: agent
difficulty: unknown ground
model: strongest
diagnostics: when every registered client key is in use and the bound refuses a registration, the box creation fails with this text on stderr: "min did not create the box: the Box Egress Proxy already holds 16 client keys, and running boxes use each one. A key is freed when the last box created under it ends."
observability: one info log line when a kept key is retired, naming its
kidlayers:
- keep:
- crates/bep/src/control.rs:
replace_client_keykeeps a key registered while a running box's store handles or sign-in references are signed under it, and revokes it when the last such box ends; the bound drops the least recently registered key that no running box uses, and refuses the registration when every key is in use (owner ruling on gominimal/arch#144, 2026-10-26; #2605)- crates/minimal/src/mint.rs: the replacement keeps its pending-revoke file and lock, and a bound refusal fails the creation with the diagnostics text
- docs/specs/24-spec-box-egress-proxy/24-spec-box-egress-proxy.md: the BEP-063 replace and bound sub-requirements state the keep and the in-use bound, name the two verify tests below, drop their known-gap notes, and the pending-revoke comment says the client removes the file (arch C1 on #2594)
test: cargo nextest run -p minimal a_replaced_client_key_keeps_verifying_until_its_boxes_end
test: cargo nextest run -p bep the_registry_bound_never_drops_a_key_a_running_box_uses
produces: none
consumes: none
proof: After a client key replacement, a box created under the old key keeps redeeming its store handles and sign-in references until it ends, and the registry bound never drops a key that a running box uses.
depends on:
- assumes OQ3 (the mint reads the root certificate's own expiry, and the root is generated with a fixed default validity in one constant that gominimal/arch#85's ruling replaces)
- blocked by #2543
- blocked by #2578
Proof
A client key is replaced after a refusal. A store handle minted under the old key for a running box still verifies at the proxy, also after a proxy restart. A box created under the old key with a GitHub sign-in reference keeps the old key registered. When the last of those boxes is forgotten, the proxy revokes the old key. The bound drops only a key that no running box uses. When running boxes use every key, the bound refuses the new key, and the creation fails with the fixed diagnostics text.
Notes from the agent: Wire change: register_client_key takes an optional box_id. The mint and the store handle mint send their box id, and forget_box ends the use. The proxy keeps each key's boxes, and a 'retired' flag, in the .client-keys file. A line with neither keeps the old 'key seen' format. A replaced key that a running box uses is retired and not revoked. The forget of its last box revokes it, with an info log line that names its kid. The bound refusal is the error text KEYS_IN_USE plus the bound. The client parses it with bep::control::keys_in_use_bound and maps it to MintError::KeysInUse, which prints the diagnostics text with the proxy's bound. Outside the layers: crates/bep/src/main.rs. Two test struct literals of Request::RegisterClientKey needed
box_id: Noneto compile. There is no other change in that file. I replaced the old test a_replaced_client_key_is_revoked_at_the_proxy with the new verify test, as the spec planned. I also added a_replacement_past_a_bound_of_keys_in_use_fails_the_creation in mint.rs and a_replaced_key_stays_until_its_last_box_is_forgotten in control.rs. Known limit: a box that ends without a forget_box (for example a Ctrl-C before minvmd withdraws it) keeps its key in use until something forgets that box id. The pending-revoke file and lock needed no code change. I changed only the doc comment. just lint-prose did not run because vale is not installed on this box. The full bep suite passed (213 tests). The minimal mint and m…[truncated]Tests
cargo nextest run -p minimal a_replaced_client_key_keeps_verifying_until_its_boxes_endcargo nextest run -p bep the_registry_bound_never_drops_a_key_a_running_box_usesVerify
min task run agent-verifyexited 101 under rustc 1.98.1, which the repository does not pin (1.99.0 in rust-toolchain.toml); the PR is not a draft on that alone — CI, which honours the pin, is the gate.Base
The base branch moved while the coder worked; it was merged in before the tests ran.
Out of scope
These files are outside the task's
layers:, so this PR is a draft:crates/bep/src/main.rsLayer exception
Round 1 (d1addbb) changed two files outside the task's layers, both required by the change:
crates/bep/src/main.rs: two call sites set the newbox_id: Nonefield that the registration record gains incontrol.rs.crates/minimal/src/tests.rs: the test inventory names the new verify tests (a_replaced_client_key_keeps_verifying_until_its_boxes_end,a_replacement_past_a_bound_of_keys_in_use_fails_the_creation) in place of the old one.The local verify ran under rustc 1.98, which the repository does not pin. CI on the pinned toolchain is the gate.
Closes #2605
Generated by Claude Code