Repository navigation
T111 — Accept a key the keychain made exportable in a debug build of bep, and scope the custody rule to release builds - #2790
Conversation
The keychain's generation and the CA's key check take one custody decision, keys::custody, which takes the build. A debug build keeps a key the login keychain made exportable and warns once per role at each start. A release build refuses it as before (BEP-014, BEP-059).
A debug build starts, issues its CA and warns once per role at each start. A release build refuses at the generation site and at the CA site, each with its own text (BEP-014, BEP-059).
A note on each says why a debug build keeps an exportable key with a warning: the macOS login keychain lets the process that made a key export it, and the enforcing keychain needs a Developer ID signature.
|
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: |
|
Approved: no findings at Nothing to do. Verdict APPROVE, posted as a comment: the App cannot review its own pull request; the check and the label carry it. Clobber: none seen. |
There was a problem hiding this comment.
arch-review: CONDITIONS · 1 condition · the full ruling
The debug-build scoping conforms to Gatehouse §6.10 for the root and signing keys. A release build refuses those keys at generation and at the CA. The sealing key has a gap. A debug build now leaves an exportable sealing key under the label that a release build reads. keys::load does not check custody for a key that it finds, so a release build can hold that key. Gatehouse §6.10 line 832 says that the sealing key is non-exportable. Make the change in C1. Then the change conforms.
Ruled at a918308 against gominimal/arch@8640f85e193d by claude-opus-5-5.
The CA refuses an exportable root or signing key, but a sealing key that a debug build left exportable reached open_held in a release build. load() now takes the custody decision for the sealing key before it generates anything, and the release test pins that refusal.
|
Round 1 of 10 on the runtime's review findings, pushed as
Verify Review threads the coder judged:
A debug build of bep starts and serves with keys that the login keychain made exportable, and writes one warning for each key. A release build refuses such a key where the keychain generates it, at the CA for a root or signing key, and now also at load for a sealing key that a debug build left behind. Notes from the agent: Revise round 1. load() now refuses an exportable sealing key in a release build. It uses the same custody decision (keep_generated) and the same text as the generation site, and it refuses before any key is generated. Root and signing keys still reach the CA site, so the CA refusal text stays as it was. The release test now has a third case: a store that holds an exportable sealing key with non-exportable root and signing keys. To make that case possible, the test store in main.rs now has a per-role exportable flag. cargo clippy -p bep --all-targets was clean, and the full cargo nextest run -p bep gave 236 passed, 1 skipped. |
There was a problem hiding this comment.
arch-review: PASS · the full ruling
The release build now refuses an exportable key at each site. keys::load() refuses a sealing key that a debug build left exportable. This closes the open item on a9183080. Gatehouse §6.10 lines 832 and 837 govern the shipped helper, and the release build conforms to them. The debug-build scope in BEP-014 and BEP-059 does not contradict the design. No conditions.
Ruled at 29742d3 against gominimal/arch@8640f85e193d by claude-opus-5-5.
A key's build() answer can make the custody rule stricter than the binary's build, never weaker. A release build now refuses every exportable key, whatever the key type answers.
|
Round 2 of 10 on the runtime's review findings, pushed as
Verify Review threads the coder judged:
A debug build of bep starts and serves with keys that the keychain made exportable, and writes one warning line for each role in bep.log. A release build refuses an exportable key where the keychain generates it, where load finds a sealing key, and at the CA. No key type can make a release binary keep an exportable key. Notes from the agent: Revise round 2. Threads 1 and 2 were already addressed by commit 29742d3, before this round. load() refuses an exportable sealing key in a release build before it generates anything, and a_release_build_refuses_an_exportable_key_at_both_sites has the case with a non-exportable root and signing key and an exportable sealing key. For thread 3, a cfg(test) seam does not work here: the main.rs start tests link the bep library, and Cargo builds that library without cfg(test). Changing Cargo.toml to add a feature is outside the layers. So the new keys::custody_build lets HeldKey::build() make the rule stricter but never weaker. When the binary is Build::Release, every key gets the release rule. The cfg!(debug_assertions) gate stays as it is, because the task names it. Because of this, |
There was a problem hiding this comment.
arch-review: PASS · the full ruling
The compare closes a gap. Before, a key could answer Build::Debug and pass the custody check in a release binary. Now keys::custody_build() applies the release rule to every key in a release build. Gatehouse §6.10 lines 832 and 837 require the keys non-exportable, so this change agrees with the design. The PASS on 29742d35 stands for the rest of the diff.
Ruled at 445d0fb against gominimal/arch@8640f85e193d by claude-opus-5-5.
There was a problem hiding this comment.
minimal-review — approval withheld
Verdict COMMENT — APPROVE withheld: 4 unresolved threads — minimal-foundry[bot] crates/bep/src/keys.rs:407, minimal-arch[bot] crates/bep/src/keys.rs:396, minimal-arch[bot] crates/bep/src/main.rs:3272, minimal-foundry[bot] crates/bep/src/keys.rs:122. 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 445d0fbb7442 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.
Reviewed at 445d0fb against T111's p 10BC8 lan block.
Release builds: a release build cannot keep an exportable key.
Build::CURRENT(keys.rs:157) is the only switch.custody_build(keys.rs:169) yieldsDebugonly when both the binary and the key are debug.- Every non-test custody site goes through it:
keep_generatedat generation and atload, the warn loop, andca::check_key. - Loaded root and signing keys are refused at the CA before
start()publishes the identity. - The earlier bot findings are verified, and their threads are resolved.
just test-one exportable -p beppasses 5 of 5.
Changes requested. These are small, and none is a custody defect:
keys.rs:212-215, reached fromloadatkeys.rs:331-335: the refusal misstates the cause and has no next step. Consider a releasebepthat starts on a Mac where a debugbepleft an exportable sealing key. It printsThe proxy did not start: the key store failed for the sealing key: the keychain made the key exportable.The store did not fail. The release build refused a key that is already there. AGENTS.md "User-facing text" requires a refusal to say what was refused, why, and the next step.docs/specs/24-spec-box-egress-proxy/24-spec-box-egress-proxy.md:251,257: the spec omits a site. Both notes saybep::keys::custodydecides "at the keychain's generation and at the CA's key check". The code also refuses a stored exportable sealing key atkeys::load. For the sealing key, that is the only guard.keys.rs:397-399: the comment above the warn loop is stale. It still says a release build refuses only at generation or at the CA.
|
/revise Three small fixes from review at 445d0fb. Do not change the custody logic.
Run |
|
That Nothing to type again: the command gets a rocket when its round starts, or a refusal that names the reason. No round was counted for it yet. |
A release bep that finds an exportable sealing key in the keychain now says the keychain holds it as exportable, that a release build does not use such a key, and names bep --replace-key sealing. The comments on custody and on the warn loop name the load site.
…ey-the-keychain-made-exportable-in-a-d
|
Round 3 of 10 on a review requesting changes, pushed as
Verify A debug build of bep starts and serves with keys that the login keychain made exportable, and writes one warning for each role. A release build still refuses such a key. When a release build finds an exportable sealing key that is already in the keychain, it now says what it refused and why, and names Notes from the agent: Round 3 review items. (1) A new StartError::Exportable(role) replaces the KeyError::Store refusal at keys::load. It prints: "The proxy did not start: the keychain holds the proxy's sealing key as exportable, and a release build does not use an exportable key. Run |
There was a problem hiding this comment.
arch-review: PASS · the full ruling
This head conforms to Gatehouse §6.10 lines 832 and 837. The new commits keep the release-build refusal of a stored exportable sealing key in keys::load. The refusal now has its own StartError::Exportable variant and names bep --replace-key sealing. The merge from main adds nothing to crates/bep. The PASS on 445d0fbb stands for all other parts.
Ruled at 96dcbed against gominimal/arch@8640f85e193d by claude-opus-5-5.
|
Round 4 of 10 on a /revise note: the coder changed nothing. noop: the branch already has all three items of the round-4 note. They landed in ad966b3 and bdfdbb3, after the review point 445d0fb. (1) keys::load now returns StartError::Exportable(role). In a release build that prints: "The proxy did not start: the keychain holds the proxy's sealing key as exportable, and a release build does not use an exportable key. Run |
There was a problem hiding this comment.
Re-reviewed at 96dcbed. All three changes I asked for are in.
- A stored exportable sealing key in a release build now returns
StartError::Exportable(role). The message names the key, says why a release build refuses it, and givesbep --replace-key sealingas the next step.replace_key→keys::replaceremoves and regenerates the key, so the next step is real. The third case ofa_release_build_refuses_an_exportable_key_at_both_sitespins the text. - Both spec notes now name
bep::keys::loadas the third site. - The comment above the warn loop now names all three sites.
The other changed files come from the merge ofmain.
One limit to know about: on a login keychain that makes every key exportable, the regenerated key is refused again, at generation. That is exactly what the custody probe on #2436 decides, and it does not block this PR.
|
Round 5 of 10 on a failed check suite: the coder changed nothing. No change this round. The directive gives only the failed 'tests' job and its aggregator 'ci-linux-native-success'. The log excerpt does not name a failing test. I was not able to reproduce a failure. The branch diff against main touches only crates/bep and the BEP spec. Every bep test passes under the ci profile. All tests of the crates that depend on bep (minimal, minimald, minvmd) pass under the ci profile. The package_version lint that only this lane runs also passes. origin/main is at the merged commit, so the merge brought in nothing new. I did not run the workspace doctest step (cargo test --workspace --doc), because this box has no rustdoc binary. The branch adds no code blocks to doc comments. I did not run the crates that do not depend on bep, because this diff cannot change them. The failure is probably a flaky test or a runner fault in a crate that this branch does not touch. Re-run the job, or give the name of the failing test from the full log in the next directive. |
T111 — Accept a key the keychain made exportable in a debug build of bep, and scope the custody rule to release builds
slice: S11
kind: task
covers: BEP-014, BEP-059
size: M
est: 90 code, 300 tests
executor: agent
difficulty: needs care
model: standard
diagnostics: a debug build of
bepthat keeps an exportable key writes one warn line per role tobep.log,kept the proxy's <role> key although the keychain made it exportable: this is a debug build, and a release build refuses it; a release build still prints "the keychain made the key exportable" and "the proxy root key is exportable, so the proxy does not use it for a CA", pinned by a_release_build_refuses_an_exportable_key_at_both_sitesobservability: the warn line above, once per role at each start of a debug build, in the proxy's log; a release build writes no new line
layers:
- keys:
- crates/bep/src/keys.rs: the refusal at
if key.exportable()in the keychain'sgenerate()(about line 653) goes through one custody decision that both refusal sites share; in a debug build (cfg!(debug_assertions)) the decision keeps the key and writes the warn line, and in a release build it refuses as today; the decision takes the build as an argument, so a test on every host pins both builds- crates/bep/src/ca.rs: the CA key check at
if key.exportable()(about line 721), which returnsCaError::Exportable, takes the same decision, so a debug build that kept an exportable root or signing key also starts its CA;ca_refuses_a_wrong_role_or_exportable_keypins the release refusal and the debug keeptest: cargo nextest run -p bep ca_refuses_a_wrong_role_or_exportable_key
test: cargo nextest run -p bep signing_key_is_non_exportable_and_has_no_file
test: cargo nextest run -p bep sealing_and_root_keys_persist_non_exportable
- start:
- crates/bep/src/main.rs: two start tests over keys that report
exportable(): a debug build creates its keys, builds its CA and starts serving, and a release build refuses at the generation site and at the CA site, each with its own texttest: cargo nextest run -p bep a_debug_build_starts_with_exportable_keys
test: cargo nextest run -p bep a_release_build_refuses_an_exportable_key_at_both_sites
- spec:
- docs/specs/24-spec-box-egress-proxy/24-spec-box-egress-proxy.md: BEP-014 and BEP-059 say a release build of the proxy holds its keys non-exportable; a note on each says why a debug build keeps an exportable key with a warning: the macOS login keychain lets the process that made a key export it although
kSecAttrIsExtractableis false (the probe on #2436), and the keychain that enforces it needs an entitlement only a Developer ID signature carries, so a debug build would never start on a developer's Mac (decided 2026-10-31, owner: this spec note is enough, and the architecture's key-custody rule stays as it is)produces: none
consumes: none
proof: A debug build of bep on a developer's Mac starts and serves with keys the login keychain made exportable, and writes a warning for each, while a release build still refuses such a key.
depends on:
- blocked by #2259
- blocked by #2265
- assumes OQ4 (v1 binds the macOS Keychain behind a key-store seam; the Linux host stays unverified for the signing-key rule until a later backend)
Proof
A debug build of bep starts with keys that the keychain made exportable. It issues its CA and leaves, and writes one warning for each role in its log at each start. A release build still refuses such a key. At generation it says "the keychain made the key exportable". At the CA it says "the proxy root key is exportable, so the proxy does not use it for a CA". BEP-014 and BEP-059 now scope the rule to release builds, with a note that says why.
Notes from the agent: Design: keys::custody(key, build) is the one decision. keys::keep_generated wraps it for the keychain's generate(), and ca::check_key calls it too. The build reaches the CA site through a new provided method HeldKey::build(), which defaults to Build::CURRENT (cfg!(debug_assertions)). A test key overrides it, so the tests pin both builds on every host. The warn line comes from keys::load and not from the two sites. The reason: root and signing pass the CA check up to three times in one start, and the sealing key is checked at no site on a restart. In load the line is written once for each role at each start, as the observability line asks. The main.rs tests model the keychain with a test KeyStore that calls keys::keep_generated. The macOS Keychain module does not compile on this Linux box, so its edited generate() is unverified here (the edit is small: it calls super::keep_generated and removes the key on Err). The debug start test runs start(), then Ca::with_root and a leaf that validates against the published root, as serve does. It does not call serve(), because serve reads a token on stdin and opens the audit log under the home directory. A release build still does not refuse an exportable key that load finds and that is not a CA key (the sealing key on a restart). That behaviour is unchanged from before. just lint-prose did not run, because vale is not installed on this box.
Tests
cargo nextest run -p bep ca_refuses_a_wrong_role_or_exportable_keycargo nextest run -p bep signing_key_is_non_exportable_and_has_no_filecargo nextest run -p bep sealing_and_root_keys_persist_non_exportablecargo nextest run -p bep a_debug_build_starts_with_exportable_keyscargo nextest run -p bep a_release_build_refuses_an_exportable_key_at_both_sitesVerify
min task run agent-verifypassed ([tasks.verify]).Closes #2785