Repository navigation
feat(bep): hold the proxy keys in the host keychain and publish their identity - #2363
Conversation
Add a key-store seam with a macOS Keychain backend that generates the sealing, root and signing keys non-exportable, loads them at each start and replaces one only on `bep --replace-key <role>`. Publish each key's public half and fingerprint, and seal from that file alone.
|
Important Review skippedBot user detected. To trigger a single review, invoke the ⚙️ Run configuration
You can disable this status message by setting the Use the checkbox below for a quick retry:
Comment |
|
Reviewed: 1 finding, none blocking at Address the comments on the diff. Clobber: none found: the |
There was a problem hiding this comment.
arch-review: CONDITIONS · 3 conditions · the full ruling
The change puts the keys in the right process and holds them non-exportable, as Gatehouse §6.10 lines 831 and 837 require. But keys::load regenerates any key that the store no longer holds on restart. Line 839 says the keys are never regenerated silently, and a replacement must not be a silent failure. C1 makes that start refuse and name bep --replace-key. No CI lane compiles the macOS keychain backend, and C2 adds it to the macOS scope. C3 files issues for the three items of work this change leaves open.
Ruled at 5e778c9 against gominimal/arch@3dab1097cda1 by claude-opus-5-5.
A keychain search error that is not an access denial no longer advises bep --replace-key. --replace-key publishes the new public key, and says when the old key was deleted but no new key was made. The identity file is written 0600 through a new staged file, and a file that other users can change is refused.
|
Round 1 of 10 on the runtime's review findings, pushed as
Verify Review threads the coder judged:
The proxy's keys survive a restart and appear in no file, and replacing the sealing key refuses every member sealed to the old one. After this round, Notes from the agent: Revise round 1. All five findings were valid and are fixed in one commit. This box is Linux, so the macOS Keychain backend (the find error mapping and the non-key result guard) was not compiled or run here. A macOS lane must build it. The denial codes are local constants (errSecAuthFailed -25293, errSecNoAccessForItem -25243, errSecUserCanceled -128) because the security-framework-sys source was not available to confirm its exported names. A probe failure (key use refused at start) still maps to a refused key, as before. New tests: a_store_failure_is_not_a_refused_key, a_failed_generation_reports_the_old_key_removed, the_identity_file_is_the_owners_alone. A full |
There was a problem hiding this comment.
arch-review: CONDITIONS · 2 conditions · the full ruling
The compare fixes the search path. A search error that is not a denial no longer tells the operator to replace a key. The probe path still does this. load() treats every probe failure as a refused key, and the keychain probe and agree call every error a refusal. Section 6.10 line 839 says a replacement kills every outstanding member, so a locked keychain must not lead to one. Make the change in C1. BEP-072 also asks for the root certificate, and this change publishes only the root public key. Name the issue that owns the certificate, as C2 says.
Ruled at 6ffef6c against gominimal/arch@3dab1097cda1 by claude-opus-5-5.
bep reads its published identity before it loads the keys. A role the identity names and the keychain no longer holds stops the start and names `bep --replace-key <role>`, so a later start never makes a key again silently. A missing file is a first start, and any other read error refuses the start. `bep --replace-key` publishes the new key beside the other roles' public halves from the store or the old file, so a second refused key does not keep the deleted key published. The identity file must be owned by the current user, and its directory must not be writable by other users.
|
Round 2 of 10 on the runtime's review findings, pushed as
Verify Review threads the coder judged:
The proxy's keys stay in the key store and survive a restart. The published identity holds only their public halves, and no file holds a private key. A restart that finds a published key missing refuses to start and names Notes from the agent: This round: keys::load takes the roles the published identity names. A named role that the store does not hold refuses the start (StartError::Unusable lists refused and missing roles). Generation runs only after every role is usable. main.rs reads the identity before load. A missing file is a first start, and any other read error refuses the start. --replace-key builds the new identity with Identity::after_replacement and does not do a full load. Identity::read also checks that the current user owns the file, and that other users cannot write its directory (a sticky directory is allowed). publish checks the directory too. This adds libc (workspace dep, cfg(unix)) to crates/bep/Cargo.toml and Cargo.lock, both in layers. Printed text: the refused-key line keeps the task's fixed wording, which has a semicolon. The new lost-key line uses two sentences. Not done: (1) the justfile |
There was a problem hiding this comment.
arch-review: CONDITIONS · 2 conditions · the full ruling
The compare fixes the lost-key start, the republish after --replace-key, and the identity-file ownership checks. One silent-regeneration path is left. load() decides a first start from identity.json alone. A missing, moved or unreadable file lets the next start make a new sealing or root key with no operator command. Gatehouse §6.10 line 839 says these keys are never regenerated silently. C1 makes a store that holds some roles but not others refuse the start. C2 files the seal::open gap from the PR body as a residual with an issue.
Ruled at f797864 against gominimal/arch@3dab1097cda1 by claude-opus-5-5.
A probe or key use that fails for any reason other than an access denial is now a store failure, so a locked keychain no longer advises a replacement of a healthy key. A replacement no longer overwrites an identity file it cannot read, and the root entry documents that the root certificate is still to come.
|
Round 3 of 10 on the runtime's review findings, pushed as
Verify Review threads the coder judged:
The proxy's keys survive a restart and appear in no file. Replacing the sealing key refuses every member sealed to the old one. A start that finds the keychain locked now fails without the advice to replace a healthy key. Notes from the agent: The macOS Keychain code (keychain module in keys.rs) is not compiled on this Linux box, and no CI lane compiles it (see thread PRRT_kwDOPdfcxM6uGVed). The new use_error helper assumes that SecKey::key_exchange and create_signature return CFError, and that CFError::code() gives the OSStatus. Both are true in security-framework 3.x, but a macOS build must confirm it. The DENIED codes for a key use are the same codes that find() uses. A real refused key at use time can return a different code. Nobody checked this against a rebuilt binary. A person must still file two issues: a test against keys::Keychain itself (C3), and adding -p bep to the macOS scope and strict-scope in the justfile (outside the layers). The publish.rs docs point to the task that covers BEP-013 for the root certificate. They do not give an issue number, because this session did not know it. A person can add the number to the PR body. |
There was a problem hiding this comment.
arch-review: PASS_WITH_CONDITIONS · 2 conditions · the full ruling
The compare fixes the two start-path faults. A locked keychain is now a store failure and not a refused key. A replacement no longer writes an identity that drops a role. The code conforms to Gatehouse §6.10 line 839. Two pieces of work stay undone and have no issue: the root certificate for BEP-072 and the seal.rs open variant over HeldKey::agree. File an issue for each and link it, as C1 and C2 say.
Ruled at 8c44588 against gominimal/arch@3dab1097cda1 by claude-opus-5-5.
A start with no identity file, or a file that does not name a role, generated that role's key again when the store still held the other keys. A first start finds no key at all, so a missing key beside held keys was lost. The start now refuses it and names --replace-key.
There was a problem hiding this comment.
arch-review: PASS_WITH_CONDITIONS · 3 conditions · the full ruling
The compare closes the silent-regeneration path. A lost key beside held keys now refuses the start, so BEP-060 conforms to Gatehouse §6.10 line 839. Three items stay open, and no issue tracks them. C1 and C2 tell you to open the issues for the root certificate and for the keychain-key seal::open variant, and to link them. C3 adds bep to the macOS just scope. Today no lane compiles the keychain backend that line 839 depends on.
Ruled at 7ad0c35 against gominimal/arch@3dab1097cda1 by claude-opus-5-5.
|
Round 4 of 10 on the runtime's review findings, pushed as
Verify Review threads the coder judged:
The proxy's keys survive a restart and appear in no file. Replacing the sealing key refuses every member sealed to the old one. A start now never makes a lost key again, even with no identity file: when the keychain holds the other keys, bep refuses and names Notes from the agent: Round 4. keys::load now treats a role the store does not hold as lost when the store holds or refuses a key for any other role. The identity file no longer decides this alone. The whole bep suite passed (27 tests). These items stay open for a person, because this session has no GitHub access and they are outside the layers: (1) file an issue for the BEP-072 root certificate (BEP-013 work) and put its URL in publish.rs and on Identity::root. (2) file an issue for a seal.rs open variant that takes a key agreement, and put its URL at keys.rs HeldKey::agree. (3) file an issue for a non-exportability test against keys::Keychain. (4) add -p bep to the macOS scope and strict-scope in justfile. The macOS Keychain backend is still not compiled by any lane that this box can run. |
|
/revise minimal-arch ruled PASS_WITH_CONDITIONS on 7ad0c35 (check run 118318117390). One condition is code and is yours this round; the other two wait on issue numbers that a person has to file, so leave them. C3, in C1 and C2 (issue URLs in the doc comments on Generated by Claude Code |
The macOS lane runs `just test` and `just clippy` at the `scope` variable, which named only minvmd and boxes. No lane compiled the macOS Keychain backend in crates/bep/src/keys.rs or ran bep's tests on macOS. Add `-p bep` to `scope` and `strict-scope`. The justfile is outside the task's layers. The architecture ruling on 7ad0c35 (condition C3) requires this one-line scope change.
On a first start, load generated the three keys in turn. When a later generation failed, the earlier keys stayed in the store. The next start found one role and not the others, and refused them as lost keys with advice to re-create boxes that never existed. Remove the keys that the failed start generated, so the next start is a first start again.
There was a problem hiding this comment.
arch-review: PASS_WITH_CONDITIONS · 2 conditions · the full ruling
The compare conforms to §6.10 line 839. roll_back() removes only the keys that a failed first start generated, before any key is published. That is not a silent regeneration. A failed removal fails closed. The macOS scope now compiles the Keychain backend, and that meets C3 of the last ruling. Two stated residuals have no issue: the open path for a keychain-held sealing key (C1), and a proof on a real keychain (C2). File each issue and name it in the PR body.
Ruled at 909b8a7 against gominimal/arch@3dab1097cda1 by claude-opus-5-5.
|
Round 5 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:
The proxy's keys survive a restart and are in no file. Replacing the sealing key refuses every member sealed to the old key. A first start that fails part way leaves no key behind, so the next start is a first start and not a refused lost key. On macOS, Notes from the agent: C3: justfile lines 36 and 40 now say |
|
/revise The epic owner decided (2026-10-19) to fix in this PR the one of minimal-arch's residuals that fits, and filed the other two. This round:
Then resolve the arch threads on Generated by Claude Code |
Add seal::open_held, which opens a value with the key agreement of a key the store holds, so the keychain sealing key opens sealed values without its private half. It shares the KDF and AEAD path with open. Remove a keychain key whose public half does not read, so a failed first start leaves no key behind.
There was a problem hiding this comment.
arch-review: PASS_WITH_CONDITIONS · 3 conditions · the full ruling
The compare conforms to Gatehouse §6.10. seal::open_held opens a sealed value through the held key's agreement, so the sealing key stays non-exportable (line 837). The keychain cleanup fails closed, and the root certificate deferral now names #2437. C1 and C2 are the filed residuals #2436 and #2437. C3 asks you to record seal.rs as a layer exception and to correct agent note 2 in the PR body.
Ruled at f8aae06 against gominimal/arch@3dab1097cda1 by claude-opus-5-5.
|
Round 6 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:
The proxy's keys survive a restart and appear in no file. Replacing the sealing key refuses every member sealed to the old one. The proxy can now also open a sealed value through its held sealing key's agreement (seal::open_held), so the private half never leaves the store. Notes from the agent: Revise round 6. The directive asked for a change in crates/bep/src/seal.rs, which is not in the task's layers. The task lists it under consumes. The change adds seal::open_held and an OpenError::Key variant. It moves the shared KDF and AEAD path of open into a private open_with. MemoryKey::open in the test store now opens through open_held and does not rebuild a software key. Tests: a value sealed to a MemoryStore key opens through the agreement. A held key that reports the right kid but agrees under another key fails the tag (NotOpenable). A key that the store refuses gives OpenError::Key(Refused). publish.rs now links issue 2437 in the module doc and on Identity::root. The Keychain::generate fix (remove the stored key when KeychainKey::new fails) is in the macOS-only module. This Linux box did not compile that module, so the macOS lane must compile it. I did not run just clippy-strict. |
There was a problem hiding this comment.
Babysitter review of f8aae06 (T2: the proxy's keys in the host keychain, the published identity, bep --replace-key, and seal::open_held). Read as a COMMENT review so it starts no coder round.
What I checked. keys::load finds each role, probes a found key so a refusal shows at start, treats a lost key as lost whenever the store holds or refuses any other role (whatever the identity file says), generates only after every role is known usable, and rolls back a partial first start. A locked store is a Store error, never replacement advice. replace removes before it generates, so no value sealed to the old key opens after it returns, and a failed generation leaves the identity naming the role so the next start refuses rather than regenerates. On macOS the private attrs set kSecAttrIsPermanent and kSecAttrIsExtractable = false, generate removes a key the keychain made exportable, find and the use paths classify only errSecAuthFailed, errSecNoAccessForItem and errSecUserCanceled as refusals. publish writes the identity 0600 under a 0700 directory through a create_new staged file, and read refuses a file another user owns or can write and a directory others can change without the sticky bit. Identity::key checks the kid against the point. seal::open_held keeps the kid check before the agreement, maps a store refusal to OpenError::Key, and the mismatched-agreement test pins the tag failure as NotOpenable. The macOS backend is unproven on a real keychain, which is filed as #2436, and the root certificate is #2437. No blocking finding from me.
Findings, none blocking
-
Identity::after_replacement(publish.rs): the comment "The next start treats this role as a first start" is wrong since round 4;keys::loadrefuses a lost role whenever the store holds any other key. The runtime raised this as LOW. One comment line; it rides the next push if one comes. -
replace_keyin main.rs: when the identity file exists butIdentity::readfails (writable, open directory, malformed), the key has already been replaced and the message tells the operator to runbepafter fixing the file. That is the right order (the old key must go first), but the message should say the new key is already in use and only the published copy is stale, so nobody runs--replace-keya second time and kills a second generation of members. -
publish()creates the directory beforecheck_dirruns, so a pre-existing world-writable directory gets a file written into it? No:createon an existing directory is a no-op andcheck_dirthen refuses before the staged file is created. Fine as is; noting that I checked it.
Merge. With #2436 and #2437 filed and the body recording the seal.rs layer exception, I read the PASS_WITH_CONDITIONS ruling 118366526304 the way the epic owner read T1's: conditions met, residuals open at merge.
Generated by Claude Code
There was a problem hiding this comment.
Babysitter approval of f8aae06 for the epic owner. Gate: minimal-arch PASS_WITH_CONDITIONS 118366526304 with its conditions met the way the epic owner read T1's (C1 and C2 are the filed residuals #2436 and #2437, which the ruling says may stay open at merge; C3, the body, is done), runtime review concluded with no blocking finding, every review thread resolved and re-verified, all lane aggregators and the CLA status green on the head, squash subject lints as Conventional Commits.
Generated by Claude Code
T2 — Hold the proxy's keys non-exportable in the host keychain and publish their public halves
slice: S0
kind: task
covers: BEP-014, BEP-059, BEP-060, BEP-072
size: L
est: 400 code, 420 tests
executor: agent
difficulty: unknown ground
model: strongest
diagnostics: when the keychain refuses a key,
beprefuses to start and prints "The keychain refused the proxy its key. Runbep --replace-key <role>to make a new one; boxes created before then must be re-created." on stderr, pinned by refused_key_names_the_replacement_commandobservability: one info log line at start naming each key role and its fingerprint, and one warn line per replacement naming the role
layers:
- keys:
- crates/bep/src/keys.rs (new): a key-store seam; the macOS Keychain backend holds the sealing, root and signing keys non-exportable
- crates/bep/src/publish.rs (new): publishes and parses by role the sealing public key, fingerprints and root (BEP-072)
- crates/bep/src/main.rs (new): the
bepbinary with--replace-key <role>, which runs without the files a serving proxy reads- crates/bep/src/lib.rs: module lines
- crates/bep/Cargo.toml:
security-frameworkon macOS- Cargo.lock: the keychain dependency
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
test: cargo nextest run -p bep a_value_sealed_to_the_published_identity_unseals_under_the_keys
test: cargo nextest run -p bep a_published_identity_carrying_no_key_is_refused_by_role
test: cargo nextest run -p bep keys_never_regenerated_on_restart
test: cargo nextest run -p bep key_replacement_kills_outstanding_members
test: cargo nextest run -p bep refused_key_names_the_replacement_command
produces:
- crates/bep/src/keys.rs
- crates/bep/src/publish.rs
- crates/bep/src/main.rs (entry point)
consumes:
- crates/bep/src/seal.rs
proof: The proxy's keys survive a restart and appear in no file, and replacing the sealing key refuses every member sealed to the old one.
depends on:
- blocked by #2258
- 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
The proxy loads its sealing, root and signing keys from the key store at each start and generates a key only when the store holds none. A restart gives the same fingerprints, and no file holds a private key. Only the public halves and fingerprints are published, to ~/.minimal/bep/identity.json. After
bep --replace-key sealing, a value sealed to the old key no longer opens. The tests show this on this Linux box against an in-memory store that models the keychain. The macOS Keychain backend is written but was not compiled or run here.Notes from the agent: 1. The macOS Keychain backend (keys.rs, module
keychain, cfg(target_os = "macos")) was NOT compiled. This box has no darwin std. I checked each call by hand against security-framework 3.7.0, security-framework-sys 2.17.0 and core-foundation 0.10.1. The first macOS CI run is its first compile. It declares kSecAttrIsExtractable through its own extern block because security-framework-sys does not export it. After generation it checks that the key is not exportable, and it removes the key if the check fails.2.
seal::opentakes a softwareProxyKey(a p256 scalar), which a non-exportable keychain key cannot become. Round 6 addedseal::open_held(withopen_withandOpenError::Key) incrates/bep/src/seal.rs: it opens a sealed value throughHeldKey::agree(raw ECDH,SecKeyCopyKeyExchangeResulton macOS), checkskidbefore the agreement, and fails closed on a store refusal.seal.rsis listed underconsumes:only; the change is recorded under Layer exception below. The BEP-072 test still opens through the test-only in-memory key, which delegates toseal::open; the newseal.rstests coveropen_heldagainst the memory store.3. All seven tests run against keys::memory::MemoryStore (cfg(test)). This store models the keychain: it outlives a load, it can refuse a key, and it gives no export. On a real keychain, nothing tests non-exportability or the refusal path (OQ4). On Linux,
bepprints that it needs the macOS Keychain and exits 1.4. The root certificate …[truncated]
Tests
cargo 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_value_sealed_to_the_published_identity_unseals_under_the_keyscargo nextest run -p bep a_published_identity_carrying_no_key_is_refused_by_rolecargo nextest run -p bep keys_never_regenerated_on_restartcargo nextest run -p bep key_replacement_kills_outstanding_memberscargo nextest run -p bep refused_key_names_the_replacement_commandVerify
min task run agent-verifypassed ([tasks.verify]).Closes #2259
Layer exception
Accepted by the epic owner on 2026-10-19: the
justfilemacOSscopeandstrict-scopeedit (-p bep) is outside the task's planned layers; minimal-arch condition C3 on 7ad0c35 required it so the macOS lane compileskeys::keychain. Round 6 also changedcrates/bep/src/seal.rs(open_held,open_with,OpenError::Key), which the task lists underconsumes:only; minimal-arch condition C3 on f8aae06 asked for it to be recorded here, and the epic owner's 23:12Z decision to fix the HeldKey open variant in this PR covers it.Residuals
Generated by Claude Code