feat(desktop): store nsec private keys in the OS keyring#1172
Merged
Conversation
wesbillman
requested changes
Jun 22, 2026
wesbillman
left a comment
Collaborator
There was a problem hiding this comment.
Thanks for moving desktop nsec storage toward the OS keyring — that fits Buzz's identity/security direction. I found one blocking availability edge case: after a successful migration, a later keyring outage can silently rotate the human identity or leave managed-agent secrets empty. Please fail closed or add a safe fallback/recovery story for post-migration keyring-unavailable boots.
322ab4e to
0e980dc
Compare
wesbillman
requested changes
Jun 23, 2026
wesbillman
left a comment
Collaborator
There was a problem hiding this comment.
Pinky adversarial review: requesting changes for two keyring outage safety gaps. Narf!
23d3cec to
f6e9893
Compare
wesbillman
reviewed
Jun 24, 2026
bd72e94 to
59cf70c
Compare
wesbillman
approved these changes
Jun 24, 2026
Desktop nsec private keys (the human identity and every managed-agent key) lived in plaintext on disk — identity.key and inline in managed-agents.json. Move them into the OS keyring (macOS Keychain, Windows Credential Manager, Linux Secret Service) behind a default-on system-keyring feature, mirroring goose's backend matrix. A new SecretStore wraps the keyring with a KeyringProbe that distinguishes reachable-but-empty (safe to migrate into) from unreachable (must not migrate). Migration imports the plaintext key, read-back-verifies it, then deletes the plaintext — and is skipped entirely when the keyring is unreachable, so a transient outage cannot resurrect a rotated key from a leftover file. Keyringless environments fall back to a 0o600 file. Agent keys become an in-memory #[serde(skip)] field hydrated in load_managed_agents and written-back in save_managed_agents, so the nine existing read sites are untouched and the creation path persists through the same chokepoint (no silent key loss on restart). The BUZZ_PRIVATE_KEY env override is left on its upstream path and never routed through SecretStore, preserving env-first precedence for agents and CI. SECURITY.md gains a keyring note and the audit-log description is corrected from HMAC to keyless SHA-256 hash chain. Co-authored-by: Will Pfleger <pfleger.will@gmail.com> Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
Two narrow failure paths left a plaintext nsec on disk after migration, defeating the guarantee this series ships (neither lost a key): - Human identity: a migration whose remove_file failed (AV lock, read-only mount, EPERM) left identity.key on disk, and the keyring-Present boot arm never retried the delete. The Present arm now best-effort removes a leftover file once the keyring is authoritative. - Agent keys: a save during a transient keyring outage re-inlines the key into managed-agents.json, but hydrate_keys skipped non-empty records and so never re-stripped it until a later event-driven save. hydrate_keys now opportunistically re-migrates an inline key when the keyring is reachable, keeping the key in memory for readers while the next save writes clean JSON. The migrate-vs-keep decision is extracted into migrate_inline_key, the single source of truth shared by the load-time re-migrate and the save chokepoint, behind a KeyStore trait so the decision is unit-tested without the live OS keyring. The no-resurrection guard (keyring-unreachable -> keep inline, never migrate) holds on both paths. Co-authored-by: Will Pfleger <pfleger.will@gmail.com> Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
The inline test module pushed types.rs to 1075 lines, over the 1000-line file-size limit enforced by check:file-sizes. Extract the #[cfg(test)] mod into a sibling types/tests.rs, matching the existing convention used by env_vars.rs, personas.rs, and runtime.rs in this crate. Production code is unchanged; all 22 type tests still execute by name. Co-authored-by: Will Pfleger <pfleger.will@gmail.com> Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
…entity The keyring-Present-but-corrupt path quarantined identity.key (the file) and generated a fresh identity, but the corruption is in the keyring, not the file. A valid leftover identity.key (from a prior migration whose remove_file failed) would be destroyed and the user silently rotated to a new key. Now clear the corrupt keyring value, migrate a valid file if one exists, and generate fresh only as a last resort. Adds an IdentityKeyStore seam (mirroring managed_agents::storage's KeyStore) so the recovery decision is unit-tested without a live keyring. Co-authored-by: Will Pfleger <pfleger.will@gmail.com> Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
A keyring outage after migration was indistinguishable from a fresh install (both probe Unreachable with no identity file), so the human identity could be silently regenerated, and an agent whose key failed to load was spawned with an empty BUZZ_PRIVATE_KEY/NOSTR_PRIVATE_KEY. Identity: a migration-completed marker, written and fsynced before the legacy file is deleted, is the durable signal that a key lives in the keyring. Unreachable + no file + marker now fails closed rather than generating; first-ever launch (no marker) still generates to the 0o600 file. Agent keys: a keyring LOAD error is treated as an outage (key left empty), distinct from a genuinely absent entry, and an empty key now reports KeyMigration::Nothing rather than Persisted so it is never mistaken for a verified entry. The spawn path refuses to start an agent whose key is unavailable. Co-authored-by: Will Pfleger <pfleger.will@gmail.com> Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
A fresh install that generated straight into a reachable keyring stored the key but never wrote the migration marker, so a later keyring- unreachable boot saw "no file, no marker" (indistinguishable from a never-launched machine) and silently rotated the identity. Write the marker after a keyring-success persist only; on the file-fallback arm the key is on disk and a marker would wrongly fail closed. Provider deploy serialized private_key_nsec unconditionally, so an empty key left by a keyring outage could deploy an agent with no identity. The local spawn path already refuses this via spawn_key_refusal; reuse the same guard at the top of build_deploy_payload so all backends fail closed. Co-authored-by: Will Pfleger <pfleger.will@gmail.com> Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
… disk The keyringless fallback wrote the agent store via std::fs::write + rename (process umask, typically 0644) and only chmod'd it 0o600 after the rename. A crash in that window could leave managed-agents.json with plaintext agent nsecs world-readable, contradicting the SECURITY.md owner-only fallback guarantee. The new atomic_write_json_restricted mirrors save_key_file: it sets 0o600 on the temp file before any bytes are written, then commits. The managed-agents write is now always owner-only, which retires the any_inline_key plumbing and the restrict_json_permissions chmod helper. Co-authored-by: Will Pfleger <pfleger.will@gmail.com> Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
…ile path patch_json_records rewrote the whole store via atomic_write_json (raw write+rename, umask ~0o644, no chmod), reopening the SECURITY.md:90 owner-only window on keyringless hosts whenever a launch-time field reconcile touches managed-agents.json. Route its writeback through atomic_write_json_restricted unconditionally — all targets live in the single-user agents/ dir, so 0o600 on personas/teams is harmless and avoids reintroducing the route-by-filename guard that caused the bug. Co-authored-by: Will Pfleger <pfleger.will@gmail.com> Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
59cf70c to
fb539bf
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Desktop nsec private keys move from plaintext on disk into the OS keyring, matching goose's model. Previously the human identity lived in
identity.keyand every managed-agent key was inlined inmanaged-agents.json— both world-readable plaintext. A newsecret_store.rs(keyring3.6.3, vendored, target-gatedapple-native/windows-native/sync-secret-service) backs them with the macOS Keychain, Windows Credential Manager, or the Linux Secret Service.The
system-keyringcargo feature is on by default. When no keyring backend is reachable (headless Linux with no Secret Service), keys fall back to a0o600owner-only file. TheBUZZ_PRIVATE_KEYenv var still takes precedence over both stores everywhere it did before, so harnessed agents and CI are unaffected.Migration
On first launch after upgrade, existing plaintext keys are imported into the keyring, read back to verify the round-trip, and only then is the plaintext destroyed (
identity.keydeleted; agent keys stripped frommanaged-agents.json). Migration runs only when the keyring is reachable — if the backend is unavailable that session, the app keeps the plaintext authoritative and does not migrate, so a transient outage can never resurrect a rotated key from a leftover file.The
private_key_nsecfield becomes#[serde(skip)]-style in-memory-only and is hydrated from the keyring on load, so it stops landing in JSON. A single write chokepoint insave_managed_agentscovers both migration and fresh agent creation — a newly created agent's key persists to the keyring through the same path, surviving restart.Plaintext-never-lingers guarantees
Presentboot arm cleans up a leftoveridentity.keyonly after a successful keyring load and parse — never on a failed read, so a delete can never race ahead of a confirmed key. Closes the window where a prior migration'sremove_filefailed (AV lock, read-only mount, EPERM) and left plaintext on disk indefinitely.managed-agents.jsonfor survival; the next reachable boot deterministically re-migrates it via the non-mutatingmigrate_inline_keyhelper and the following save writes clean JSON. The no-resurrection guard holds — keyringUnreachablekeeps the key inline and never migrates.Fail-closed on keyring outage
A keyring that is reachable at write time but unreachable on a later boot must never silently rotate or deploy a keyless identity:
identity.migratedmarker on keyring-success, so a later keyring-Unreachableboot sees the marker (not "no file, no marker", which is indistinguishable from a never-launched machine) and fails closed rather than generating a new key. On the keyring-write-failure → file-fallback arm the marker is deliberately not written, since the key is then authoritative in the0o600file.build_deploy_payloadreuses the samespawn_key_refusalguard that fronts the local spawn path, so a provider deploy with an emptyprivate_key_nsec(left by a keyring outage after hydration) fails closed instead of launching an agent with no identity. Create-deploy records the refusal in the agent'slast_error; start-deploy propagates it.Docs
SECURITY.mdgains the keyring storage section and corrects the audit-log description: the chain is a keyless SHA-256 hash chain (buzz-audit/src/hash.rs), tamper-evident not tamper-resistant — the prior "HMAC-chained" claim was false.