feat(desktop): store nsec private keys in the OS keyring (#1172)

Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
Co-authored-by: npub1mn7jgtj4w2pd0g0zeuhxsa6jy6p0rewxz4kujt98my82ahfmp72sxjexk7 <dcfd242e557282d7a1e2cf2e6877522682f1e5c6156dc92ca7d90eaedd3b0f95@sprout-oss.stage.blox.sqprod.co>
This commit is contained in:
Will Pfleger
2026-06-24 17:07:34 -04:00
committed by GitHub
parent 0482288891
commit 4be43d7086
14 changed files with 1918 additions and 418 deletions

View File

@@ -67,9 +67,30 @@ unless the subscriber is a member.
### Append-Only Audit Log
All events are written to a tamper-evident audit log (`buzz-audit`). Each
log entry is chained to the previous one via an HMAC, making retroactive
modification detectable. The audit log is designed for SOX-grade compliance
and eDiscovery.
log entry is chained to the previous one via a SHA-256 hash chain. Because the
chain is keyless, it is tamper-evident but not tamper-resistant: it detects
accidental corruption or single-row edits, but an attacker with database write
access can recompute the entire chain after editing. The audit log is designed
for SOX-grade compliance and eDiscovery.
### Desktop Secret Storage — OS Keyring
The Buzz desktop app stores nsec private keys in the operating system keyring
rather than in plaintext files: macOS Keychain, Windows Credential Manager, or
the Linux Secret Service (`gnome-keyring` / `kwallet` via D-Bus). This covers
both the human identity key and every managed-agent key.
On first launch after upgrading, existing plaintext keys are migrated into the
keyring: the key is imported, read back to verify the round-trip, and only then
is the plaintext deleted. Migration runs only when the keyring is reachable —
if the backend is unavailable that session, the app keeps reading from the
plaintext file and does **not** migrate, so a transient outage cannot resurrect
a rotated key from a leftover file.
When no keyring backend is available (headless Linux with no Secret Service, for
example), keys fall back to a `0o600` owner-only file. The `BUZZ_PRIVATE_KEY`
environment variable, when set, always takes precedence over both stores — this
is how harnessed agents and CI receive their identity.
### Input Validation

View File

@@ -41,7 +41,7 @@ const overrides = new Map([
// harness-persona-sync: persona-runtime resolution threaded into the spawn
// path here. Load-bearing feature growth; queued to split in the resolver
// unify refactor followup.
["src-tauri/src/managed_agents/runtime.rs", 1966],
["src-tauri/src/managed_agents/runtime.rs", 1969],
["src-tauri/src/managed_agents/personas.rs", 1080],
["src-tauri/src/managed_agents/persona_card.rs", 1050],
// applyWorkspace reposDir parameter plus the validateReposDir binding,
@@ -53,11 +53,15 @@ const overrides = new Map([
// harness-persona-sync feature growth, queued to split in the resolver-unify
// refactor followup. discovery.rs is dominated by the new test module
// (the effective_agent_command / divergent / create-time override matrix);
// types.rs adds the persona/instance harness fields; migration_tests.rs adds
// the harness-sync migration coverage. Load-bearing, not generic debt.
// types.rs adds the persona/instance harness fields. Load-bearing, not
// generic debt.
["src-tauri/src/managed_agents/discovery.rs", 1043],
["src-tauri/src/managed_agents/types.rs", 1010],
["src-tauri/src/migration_tests.rs", 1033],
// migration_tests.rs carries the harness-sync migration coverage plus the
// patch_json_records owner-only writeback regression test (SECURITY.md:90
// crash-safe 0o600 fallback). Load-bearing security + feature coverage, not
// generic debt growth. Approved override; still queued to split.
["src-tauri/src/migration_tests.rs", 1063],
["src-tauri/src/nostr_convert.rs", 1126],
["src/shared/api/relayClientSession.ts", 1022],
["src-tauri/src/migration.rs", 1295],
@@ -70,6 +74,11 @@ const overrides = new Map([
// the Inbox nav badge — a small overage from load-bearing badge plumbing,
// not generic debt growth. Approved override; still queued to split.
["src/app/AppShell.tsx", 1008],
// PersistBackend enum + marker-on-keyring-success plumbing and its three
// fail-closed regression tests (silent identity rotation on keyring outage).
// A small overage from load-bearing security plumbing on a file already at
// 893 lines, not generic debt growth. Approved override; still queued to split.
["src-tauri/src/app_state.rs", 1012],
]);
await runFileSizeCheck({

View File

@@ -865,6 +865,7 @@ dependencies = [
"futures-util",
"hex",
"infer",
"keyring",
"libc",
"mesh-llm-host-runtime",
"mesh-llm-sdk",

View File

@@ -17,8 +17,11 @@ name = "buzz_lib"
crate-type = ["staticlib", "cdylib", "rlib"]
[features]
default = []
default = ["system-keyring"]
mesh-llm = ["dep:mesh-llm-sdk", "dep:mesh-llm-host-runtime"]
# OS keyring backing for desktop secret storage (nsec private keys). When
# disabled, secrets fall back to 0o600 files. On by default for real builds.
system-keyring = ["dep:keyring"]
[build-dependencies]
serde = { version = "1", features = ["derive"] }
@@ -31,9 +34,14 @@ ctrlc = { version = "3", features = ["termination"] }
[target.'cfg(target_os = "macos")'.dependencies]
objc2-app-kit = { version = "0.3.2", default-features = false, features = ["NSHapticFeedback"] }
keyring = { version = "3.6.3", default-features = false, features = ["apple-native", "vendored"], optional = true }
[target.'cfg(windows)'.dependencies]
windows-sys = { version = "0.61", features = ["Win32_Storage_FileSystem", "Win32_System_JobObjects", "Win32_System_Threading", "Win32_Foundation"] }
keyring = { version = "3.6.3", default-features = false, features = ["windows-native", "vendored"], optional = true }
[target.'cfg(target_os = "linux")'.dependencies]
keyring = { version = "3.6.3", default-features = false, features = ["sync-secret-service", "vendored"], optional = true }
[dependencies]
atomic-write-file = "0.3"

View File

@@ -45,30 +45,41 @@ pub struct AppState {
pub mesh_coordinator: AsyncMutex<Option<crate::mesh_llm::MeshCoordinator>>,
}
pub fn build_app_state() -> AppState {
// Env var takes precedence (dev/CI). If absent, resolve_persisted_identity()
// in setup() will replace the ephemeral placeholder with a persisted key.
let (keys, source) = match std::env::var("BUZZ_PRIVATE_KEY") {
/// Parse the `BUZZ_PRIVATE_KEY` env var into identity keys. `Some` means the
/// env var was present and valid and MUST win over any persisted/keyring key
/// (the dev/CI/harness override). `None` means absent or malformed — callers
/// fall through to persisted resolution. A malformed value is logged and
/// treated as absent rather than left on an ephemeral identity.
fn identity_from_env() -> Option<Keys> {
match std::env::var("BUZZ_PRIVATE_KEY") {
Ok(nsec) => match Keys::parse(nsec.trim()) {
Ok(keys) => (keys, "configured"),
Ok(keys) => Some(keys),
Err(error) => {
eprintln!("buzz-desktop: invalid BUZZ_PRIVATE_KEY: {error}");
(Keys::generate(), "ephemeral")
None
}
},
Err(std::env::VarError::NotUnicode(_)) => {
eprintln!("buzz-desktop: BUZZ_PRIVATE_KEY contains invalid UTF-8");
(Keys::generate(), "ephemeral")
None
}
Err(std::env::VarError::NotPresent) => (Keys::generate(), "ephemeral"),
};
if source == "configured" {
eprintln!(
"buzz-desktop: configured identity pubkey {}",
keys.public_key().to_hex()
);
Err(std::env::VarError::NotPresent) => None,
}
}
pub fn build_app_state() -> AppState {
// Env var takes precedence (dev/CI). If absent, resolve_persisted_identity()
// in setup() will replace the ephemeral placeholder with a persisted key.
let keys = match identity_from_env() {
Some(keys) => {
eprintln!(
"buzz-desktop: configured identity pubkey {}",
keys.public_key().to_hex()
);
keys
}
None => Keys::generate(),
};
AppState {
keys: Mutex::new(keys),
@@ -135,10 +146,8 @@ pub fn resolve_persisted_identity(app: &AppHandle, state: &AppState) -> Result<(
// Only skip file-based resolution if the env var was present AND parsed
// successfully. A malformed env var should fall through to the persisted
// key rather than leaving the app on an ephemeral identity.
if let Ok(nsec) = std::env::var("BUZZ_PRIVATE_KEY") {
if Keys::parse(nsec.trim()).is_ok() {
return Ok(());
}
if identity_from_env().is_some() {
return Ok(());
}
let data_dir = app
@@ -146,48 +155,357 @@ pub fn resolve_persisted_identity(app: &AppHandle, state: &AppState) -> Result<(
.app_data_dir()
.map_err(|e| format!("app data dir: {e}"))?;
std::fs::create_dir_all(&data_dir).map_err(|e| format!("create app data dir: {e}"))?;
let key_path = data_dir.join("identity.key");
// Try to load an existing key.
if key_path.exists() {
match load_key_file(&key_path) {
let keys = load_or_create_identity(&data_dir)?;
*state.keys.lock().map_err(|e| e.to_string())? = keys;
Ok(())
}
/// Service name for the desktop OS keyring. Shared by the human identity key
/// and managed-agent keys (each addressed by a distinct key name within it).
pub(crate) const KEYRING_SERVICE: &str = "buzz-desktop";
/// Keyring key name for the human identity nsec.
const IDENTITY_KEY_NAME: &str = "identity";
/// Filename of the marker written once a successful keyring migration deletes
/// the legacy `identity.key`. Its presence is the only durable signal that a
/// key once lived in the keyring — used to tell a genuine first-ever launch
/// (no key anywhere, generating is correct) from a post-migration boot whose
/// keyring is merely unreachable (the key IS in the keyring, must NOT generate).
const MIGRATION_MARKER_NAME: &str = "identity.migrated";
/// The keyring operations the identity resolution flow needs. Abstracted so the
/// corrupt-keyring recovery decision ([`recover_from_keyring`]) can be
/// unit-tested against a fake without touching the live OS keyring.
trait IdentityKeyStore {
fn probe(&self, name: &str) -> crate::secret_store::KeyringProbe;
fn load(&self, name: &str) -> Result<Option<String>, String>;
fn store(&self, name: &str, value: &str) -> Result<(), String>;
fn delete(&self, name: &str) -> Result<(), String>;
}
impl IdentityKeyStore for crate::secret_store::SecretStore {
fn probe(&self, name: &str) -> crate::secret_store::KeyringProbe {
crate::secret_store::SecretStore::probe(self, name)
}
fn load(&self, name: &str) -> Result<Option<String>, String> {
crate::secret_store::SecretStore::load(self, name)
}
fn store(&self, name: &str, value: &str) -> Result<(), String> {
crate::secret_store::SecretStore::store(self, name, value)
}
fn delete(&self, name: &str) -> Result<(), String> {
crate::secret_store::SecretStore::delete(self, name)
}
}
/// Resolve the human identity key: migrate a legacy `identity.key` into the
/// keyring when safe, otherwise load from whichever backend holds it, else
/// generate-and-save.
///
/// Migration rule (prevents stale-key resurrection): only import the plaintext
/// file when the keyring is REACHABLE-but-empty. If the keyring is UNREACHABLE
/// this boot, fall back to reading the file directly and do NOT migrate — a
/// later import from a leftover (possibly rotated) file could resurrect an old
/// key.
fn load_or_create_identity(data_dir: &std::path::Path) -> Result<Keys, String> {
let legacy_path = data_dir.join("identity.key");
// No keyring available in this build: the `0o600` file is the only store.
if !cfg!(feature = "system-keyring") {
return load_file_or_generate(&legacy_path, data_dir);
}
let store = crate::secret_store::SecretStore::keyring(KEYRING_SERVICE);
resolve_identity_with_store(&store, &legacy_path, data_dir)
}
/// Identity resolution over an [`IdentityKeyStore`] seam. Split from
/// [`load_or_create_identity`] so the probe/recover branches are testable
/// without the live OS keyring.
fn resolve_identity_with_store(
store: &impl IdentityKeyStore,
legacy_path: &std::path::Path,
data_dir: &std::path::Path,
) -> Result<Keys, String> {
use crate::secret_store::KeyringProbe;
match store.probe(IDENTITY_KEY_NAME) {
KeyringProbe::Present => {
if let Some(nsec) = store.load(IDENTITY_KEY_NAME)? {
match Keys::parse(nsec.trim()) {
Ok(keys) => {
eprintln!(
"buzz-desktop: persisted identity pubkey {}",
keys.public_key().to_hex()
);
// The key is authoritative in the keyring. A leftover
// `identity.key` means a prior migration's `remove_file`
// failed (transient AV lock, read-only mount, EPERM) and
// never retried — clean it up now so plaintext does not
// linger on disk.
cleanup_leftover_identity_file(legacy_path);
return Ok(keys);
}
// The corruption is in the KEYRING, not the file. Clear the
// bad keyring value and recover from the file (or generate
// fresh) — do NOT quarantine a valid leftover `identity.key`
// that holds the user's only good key.
Err(error) => {
return recover_from_keyring(
store,
legacy_path,
data_dir,
&error.to_string(),
);
}
}
}
// Probe said Present but load found nothing — treat as empty.
}
KeyringProbe::ReachableButEmpty => {
// One-time migration: import the legacy plaintext file, read-back
// verify, THEN delete it.
if legacy_path.exists() {
if let Some(keys) = migrate_identity_file(store, legacy_path, data_dir)? {
return Ok(keys);
}
}
}
KeyringProbe::Unreachable => {
// Keyring down this boot. If a recoverable file is present, use it
// (and do NOT migrate — re-importing later could resurrect a
// rotated key). With NO file, the marker disambiguates two states
// that are otherwise byte-identical (Unreachable + no file):
// - marker present → the key was migrated into the keyring and the
// file deleted. The real key is unreachable, not gone. Fail
// CLOSED — generating here would silently rotate the identity.
// - no marker → genuine first-ever launch with nothing to protect.
// Generate to the `0o600` file (legitimate first-run).
if !legacy_path.exists() && migration_marker_path(data_dir).exists() {
return Err(
"identity key is in the OS keyring but the keyring is unavailable this boot; \
retry once the keyring (Keychain / Credential Manager / Secret Service) is reachable"
.to_string(),
);
}
return load_file_or_generate(legacy_path, data_dir);
}
}
generate_and_persist(store, legacy_path, data_dir)
}
/// Recover from a corrupt nsec in the keyring (parse failed). Clear the bad
/// keyring value, then migrate a valid leftover `identity.key` if one exists,
/// generating fresh only as a last resort. The keyring delete is best-effort:
/// a delete failure logs and continues — it must never block startup.
fn recover_from_keyring(
store: &impl IdentityKeyStore,
legacy_path: &std::path::Path,
data_dir: &std::path::Path,
error: &str,
) -> Result<Keys, String> {
eprintln!("buzz-desktop: corrupt nsec in keyring ({error}), clearing and recovering from file");
if let Err(e) = store.delete(IDENTITY_KEY_NAME) {
eprintln!("buzz-desktop: failed to clear corrupt keyring value: {e}");
}
if legacy_path.exists() {
if let Some(keys) = migrate_identity_file(store, legacy_path, data_dir)? {
return Ok(keys);
}
}
generate_and_persist(store, legacy_path, data_dir)
}
/// Load the `0o600` identity file, quarantining corruption, else generate and
/// save a fresh key to the file. Used when no keyring is available.
fn load_file_or_generate(
legacy_path: &std::path::Path,
data_dir: &std::path::Path,
) -> Result<Keys, String> {
if legacy_path.exists() {
match load_key_file(legacy_path) {
Ok(keys) => {
eprintln!(
"buzz-desktop: persisted identity pubkey {}",
keys.public_key().to_hex()
);
*state.keys.lock().map_err(|e| e.to_string())? = keys;
return Ok(());
}
Err(error) => {
// Corrupted — quarantine with a timestamp so prior backups
// are never overwritten.
let ts = std::time::SystemTime::now()
.duration_since(std::time::UNIX_EPOCH)
.map(|d| d.as_secs())
.unwrap_or(0);
let bad_name = format!("identity.key.bad.{ts}");
eprintln!(
"buzz-desktop: corrupt identity.key ({error}), quarantining to {bad_name}"
);
let bad_path = data_dir.join(bad_name);
if std::fs::rename(&key_path, &bad_path).is_err() {
let _ = std::fs::remove_file(&key_path);
}
return Ok(keys);
}
Err(error) => quarantine_corrupt_key(legacy_path, data_dir, &error),
}
}
// First run (or recovery from corruption): generate and save.
let keys = Keys::generate();
save_key_file(&key_path, &keys)?;
save_key_file(legacy_path, &keys)?;
eprintln!(
"buzz-desktop: generated and saved identity pubkey {}",
keys.public_key().to_hex()
);
*state.keys.lock().map_err(|e| e.to_string())? = keys;
Ok(())
Ok(keys)
}
/// Import the plaintext `identity.key` into the store, verify the round-trip,
/// then delete the file. Returns `Ok(None)` if the file was corrupt (caller
/// continues to generate-and-save).
fn migrate_identity_file(
store: &impl IdentityKeyStore,
legacy_path: &std::path::Path,
data_dir: &std::path::Path,
) -> Result<Option<Keys>, String> {
let keys = match load_key_file(legacy_path) {
Ok(keys) => keys,
Err(error) => {
eprintln!("buzz-desktop: corrupt identity.key during migration ({error}), skipping");
return Ok(None);
}
};
let nsec = keys
.secret_key()
.to_bech32()
.map_err(|e| format!("encode nsec: {e}"))?;
store.store(IDENTITY_KEY_NAME, &nsec)?;
// Read-back verify before deleting the plaintext file.
match store.load(IDENTITY_KEY_NAME)? {
Some(stored) if stored == nsec => {
// Crash-safe ordering: record that the key now lives in the keyring
// (marker write + fsync) BEFORE deleting the file. A crash between
// the two must never leave "file gone, no marker" — that state is
// indistinguishable from a fresh install and would silently rotate
// the identity on the next keyring-unreachable boot. If the marker
// cannot be written, keep the file so the key is never stranded.
let marker_path = migration_marker_path(data_dir);
if let Err(e) = write_migration_marker(&marker_path) {
eprintln!(
"buzz-desktop: keyring import ok but failed to write migration marker ({e}); \
keeping identity.key so the key is not stranded"
);
return Ok(Some(keys));
}
if let Err(e) = std::fs::remove_file(legacy_path) {
eprintln!("buzz-desktop: keyring import ok but failed to delete identity.key: {e}");
} else {
eprintln!("buzz-desktop: migrated identity key into OS keyring");
}
Ok(Some(keys))
}
_ => Err("keyring read-back verify failed for identity key".to_string()),
}
}
/// Path of the migration-completed marker within `data_dir`.
fn migration_marker_path(data_dir: &std::path::Path) -> std::path::PathBuf {
data_dir.join(MIGRATION_MARKER_NAME)
}
/// Atomically write (and fsync) the migration-completed marker. The content is
/// irrelevant — only the file's durable existence is the signal — so a single
/// byte keeps it minimal. Atomicity + fsync guarantee that once this returns
/// `Ok`, the marker survives a crash, which is what makes deleting the legacy
/// file afterward safe.
fn write_migration_marker(marker_path: &std::path::Path) -> Result<(), String> {
use atomic_write_file::AtomicWriteFile;
let mut file = AtomicWriteFile::open(marker_path)
.map_err(|e| format!("open migration marker for atomic write: {e}"))?;
file.write_all(b"1")
.map_err(|e| format!("write migration marker: {e}"))?;
file.commit()
.map_err(|e| format!("commit migration marker: {e}"))
}
/// Which backend `persist_identity` wrote to. The caller writes the migration
/// marker only after a keyring success — on the file-fallback arm the key is on
/// disk and a marker would wrongly trip the next Unreachable boot into failing
/// closed.
enum PersistBackend {
Keyring,
File,
}
/// Generate a fresh identity, persist it through the store, return it.
///
/// On a keyring-backed persist no file is written, so a later
/// keyring-Unreachable boot would see "no file, no marker" (identical to a
/// fresh install) and silently rotate the identity. Writing the marker here
/// makes that boot fail closed. If the marker write fails, fall back to the
/// `0o600` file so the key is never keyring-only-without-marker.
fn generate_and_persist(
store: &impl IdentityKeyStore,
legacy_path: &std::path::Path,
data_dir: &std::path::Path,
) -> Result<Keys, String> {
let keys = Keys::generate();
if let PersistBackend::Keyring = persist_identity(store, &keys, legacy_path)? {
let marker_path = migration_marker_path(data_dir);
if let Err(e) = write_migration_marker(&marker_path) {
eprintln!(
"buzz-desktop: stored identity in keyring but failed to write migration marker \
({e}); saving identity.key fallback so the key is not stranded"
);
save_key_file(legacy_path, &keys)?;
}
}
eprintln!(
"buzz-desktop: generated and saved identity pubkey {}",
keys.public_key().to_hex()
);
Ok(keys)
}
/// Persist `keys` through the store, falling back to the `0o600` file when the
/// keyring write fails on an availability error. Reports which backend held the
/// key so the caller can write the migration marker only on keyring success.
fn persist_identity(
store: &impl IdentityKeyStore,
keys: &Keys,
legacy_path: &std::path::Path,
) -> Result<PersistBackend, String> {
let nsec = keys
.secret_key()
.to_bech32()
.map_err(|e| format!("encode nsec: {e}"))?;
match store.store(IDENTITY_KEY_NAME, &nsec) {
Ok(()) => Ok(PersistBackend::Keyring),
Err(keyring_err) => {
eprintln!("buzz-desktop: keyring write failed ({keyring_err}), using file fallback");
save_key_file(legacy_path, keys)?;
Ok(PersistBackend::File)
}
}
}
/// Best-effort removal of a leftover `identity.key` once the keyring is the
/// authoritative store. Idempotent: a missing file is success. Logs but does
/// not error on failure — a delete failure must never block startup.
fn cleanup_leftover_identity_file(legacy_path: &std::path::Path) {
if !legacy_path.exists() {
return;
}
match std::fs::remove_file(legacy_path) {
Ok(()) => eprintln!("buzz-desktop: removed leftover identity.key (key is in keyring)"),
Err(e) => eprintln!("buzz-desktop: failed to remove leftover identity.key: {e}"),
}
}
/// Quarantine a corrupt `identity.key` with a timestamp so prior backups are
/// never overwritten.
fn quarantine_corrupt_key(key_path: &std::path::Path, data_dir: &std::path::Path, error: &str) {
if !key_path.exists() {
return;
}
let ts = std::time::SystemTime::now()
.duration_since(std::time::UNIX_EPOCH)
.map(|d| d.as_secs())
.unwrap_or(0);
let bad_name = format!("identity.key.bad.{ts}");
eprintln!("buzz-desktop: corrupt identity.key ({error}), quarantining to {bad_name}");
let bad_path = data_dir.join(bad_name);
if std::fs::rename(key_path, &bad_path).is_err() {
let _ = std::fs::remove_file(key_path);
}
}
fn load_key_file(path: &std::path::Path) -> Result<Keys, String> {
@@ -241,6 +559,50 @@ mod tests {
assert_eq!(a.public_key().to_hex(), b.public_key().to_hex());
}
/// `BUZZ_PRIVATE_KEY` is process-global; serialize the env-mutating tests
/// so they don't race each other under the parallel test runner.
static ENV_LOCK: std::sync::Mutex<()> = std::sync::Mutex::new(());
/// Run `body` with `BUZZ_PRIVATE_KEY` set to `value` (or unset when `None`),
/// restoring the prior value afterward.
fn with_env_key<T>(value: Option<&str>, body: impl FnOnce() -> T) -> T {
let _guard = ENV_LOCK.lock().unwrap_or_else(|e| e.into_inner());
let prior = std::env::var("BUZZ_PRIVATE_KEY").ok();
match value {
Some(v) => std::env::set_var("BUZZ_PRIVATE_KEY", v),
None => std::env::remove_var("BUZZ_PRIVATE_KEY"),
}
let out = body();
match prior {
Some(v) => std::env::set_var("BUZZ_PRIVATE_KEY", v),
None => std::env::remove_var("BUZZ_PRIVATE_KEY"),
}
out
}
#[test]
fn identity_from_env_wins_when_valid() {
let configured = Keys::generate();
let nsec = configured.secret_key().to_bech32().unwrap();
let resolved =
with_env_key(Some(&nsec), identity_from_env).expect("valid env key must resolve");
assert_key_eq(&configured, &resolved);
}
#[test]
fn identity_from_env_none_when_absent() {
assert!(with_env_key(None, identity_from_env).is_none());
}
#[test]
fn identity_from_env_none_when_malformed() {
// A malformed env var falls through to persisted resolution rather than
// winning — otherwise a typo'd key would silently shadow the real one.
assert!(with_env_key(Some("not-a-valid-nsec"), identity_from_env).is_none());
}
#[test]
fn save_and_load_round_trip() {
let dir = tempfile::tempdir().unwrap();
@@ -278,6 +640,34 @@ mod tests {
assert!(load_key_file(&path).is_err());
}
#[test]
fn cleanup_removes_leftover_identity_file() {
// Item 1: a leftover identity.key (from a migration whose remove_file
// failed) is deleted once the keyring is authoritative, so plaintext
// does not linger on disk.
let dir = tempfile::tempdir().unwrap();
let path = dir.path().join("identity.key");
save_key_file(&path, &Keys::generate()).unwrap();
assert!(path.exists());
cleanup_leftover_identity_file(&path);
assert!(!path.exists());
}
#[test]
fn cleanup_is_noop_when_no_leftover_file() {
// Idempotent: the cleanup runs on every keyring-Present boot, so a
// missing file must be a silent success, not an error or panic.
let dir = tempfile::tempdir().unwrap();
let path = dir.path().join("identity.key");
assert!(!path.exists());
cleanup_leftover_identity_file(&path);
assert!(!path.exists());
}
#[test]
fn save_creates_file_with_valid_nsec() {
let dir = tempfile::tempdir().unwrap();
@@ -319,4 +709,303 @@ mod tests {
let loaded = load_key_file(&path).unwrap();
assert_key_eq(&keys2, &loaded);
}
use std::cell::RefCell;
use std::collections::HashMap;
use crate::secret_store::KeyringProbe;
/// In-memory [`IdentityKeyStore`] for testing identity recovery without the
/// OS keyring. Seeded with an initial value and a probe outcome; records
/// every `delete`/`store` so tests can assert the keyring was cleared and
/// rewritten. `write_and_verify` succeeds (store then load reflects it).
struct FakeIdentityStore {
probe: KeyringProbe,
slot: RefCell<HashMap<String, String>>,
deleted: RefCell<Vec<String>>,
/// When true, `store` returns an availability error, driving the
/// keyring-write-failure → file-fallback arm of `persist_identity`.
store_fails: bool,
}
impl FakeIdentityStore {
fn present_with(value: &str) -> Self {
let mut slot = HashMap::new();
slot.insert(IDENTITY_KEY_NAME.to_string(), value.to_string());
Self {
probe: KeyringProbe::Present,
slot: RefCell::new(slot),
deleted: RefCell::new(Vec::new()),
store_fails: false,
}
}
/// Backend down this boot: probe is `Unreachable` and the slot is empty
/// (the real key, if any, is in the keyring we cannot reach).
fn unreachable() -> Self {
Self {
probe: KeyringProbe::Unreachable,
slot: RefCell::new(HashMap::new()),
deleted: RefCell::new(Vec::new()),
store_fails: false,
}
}
/// Backend reachable with no entry — drives the one-time migration path.
/// `store`/`load` go through the slot, so a read-back verify succeeds.
fn reachable_but_empty() -> Self {
Self {
probe: KeyringProbe::ReachableButEmpty,
slot: RefCell::new(HashMap::new()),
deleted: RefCell::new(Vec::new()),
store_fails: false,
}
}
/// Reachable-but-empty probe whose `store` always fails — exercises the
/// keyring-write-failure → `0o600` file-fallback arm.
fn store_failing() -> Self {
Self {
probe: KeyringProbe::ReachableButEmpty,
slot: RefCell::new(HashMap::new()),
deleted: RefCell::new(Vec::new()),
store_fails: true,
}
}
}
impl IdentityKeyStore for FakeIdentityStore {
fn probe(&self, _name: &str) -> KeyringProbe {
self.probe
}
fn load(&self, name: &str) -> Result<Option<String>, String> {
Ok(self.slot.borrow().get(name).cloned())
}
fn store(&self, name: &str, value: &str) -> Result<(), String> {
if self.store_fails {
return Err("simulated keyring write failure".to_string());
}
self.slot
.borrow_mut()
.insert(name.to_string(), value.to_string());
Ok(())
}
fn delete(&self, name: &str) -> Result<(), String> {
self.deleted.borrow_mut().push(name.to_string());
self.slot.borrow_mut().remove(name);
Ok(())
}
}
#[test]
fn corrupt_keyring_recovers_valid_file_without_rotating() {
// The load-bearing regression guard. When the keyring holds a corrupt
// nsec (Present) AND a valid `identity.key` is on disk (leftover from a
// failed prior migration), recovery must RECOVER THE FILE'S identity —
// not quarantine the file and rotate to a fresh key (the original
// hazard). The corrupt keyring value must be cleared and replaced by the
// file's key (migrated in).
let dir = tempfile::tempdir().unwrap();
let legacy_path = dir.path().join("identity.key");
let file_keys = Keys::generate();
save_key_file(&legacy_path, &file_keys).unwrap();
let store = FakeIdentityStore::present_with("not-a-valid-nsec");
let resolved = resolve_identity_with_store(&store, &legacy_path, dir.path()).unwrap();
// The FILE's identity is recovered — NOT a freshly generated one.
assert_key_eq(&file_keys, &resolved);
// The corrupt keyring value was cleared.
assert_eq!(store.deleted.borrow().as_slice(), [IDENTITY_KEY_NAME]);
// The keyring now holds the file's key (migrated in, read-back verified).
let file_nsec = file_keys.secret_key().to_bech32().unwrap();
assert_eq!(
store
.slot
.borrow()
.get(IDENTITY_KEY_NAME)
.map(String::as_str),
Some(file_nsec.as_str())
);
// The valid file was migrated (deleted), not quarantined to .bad.*.
assert!(!legacy_path.exists());
assert!(std::fs::read_dir(dir.path()).unwrap().all(|e| !e
.unwrap()
.file_name()
.to_string_lossy()
.contains(".bad.")));
}
#[test]
fn corrupt_keyring_generates_fresh_only_when_no_file() {
// With a corrupt keyring value and NO file on disk, generate-fresh is
// the correct last resort — and the corrupt keyring value is cleared
// first.
let dir = tempfile::tempdir().unwrap();
let legacy_path = dir.path().join("identity.key");
assert!(!legacy_path.exists());
let store = FakeIdentityStore::present_with("not-a-valid-nsec");
let resolved = resolve_identity_with_store(&store, &legacy_path, dir.path()).unwrap();
assert_eq!(store.deleted.borrow().as_slice(), [IDENTITY_KEY_NAME]);
// A fresh, valid key was persisted to the keyring (replacing the cleared
// corrupt value).
let stored = store.slot.borrow().get(IDENTITY_KEY_NAME).cloned();
assert_eq!(
stored.as_deref(),
Some(resolved.secret_key().to_bech32().unwrap().as_str())
);
}
#[test]
fn valid_keyring_is_used_and_leftover_file_cleaned_up() {
// The happy path is unchanged: a valid keyring value is used as-is, and
// a leftover plaintext file is cleaned up (keyring is authoritative).
let keyring_keys = Keys::generate();
let nsec = keyring_keys.secret_key().to_bech32().unwrap();
let dir = tempfile::tempdir().unwrap();
let legacy_path = dir.path().join("identity.key");
save_key_file(&legacy_path, &Keys::generate()).unwrap();
let store = FakeIdentityStore::present_with(&nsec);
let resolved = resolve_identity_with_store(&store, &legacy_path, dir.path()).unwrap();
assert_key_eq(&keyring_keys, &resolved);
assert!(store.deleted.borrow().is_empty());
assert!(!legacy_path.exists());
}
#[test]
fn unreachable_post_migration_fails_closed_when_marker_present() {
// The silent-rotation hazard (Wes Comment 1). After a migration the
// file is gone and the marker exists; a later boot with the keyring
// unreachable must FAIL CLOSED — the real key is in the keyring, not
// gone, so generating a fresh one would silently rotate the identity.
let dir = tempfile::tempdir().unwrap();
let legacy_path = dir.path().join("identity.key");
write_migration_marker(&migration_marker_path(dir.path())).unwrap();
assert!(!legacy_path.exists());
let store = FakeIdentityStore::unreachable();
let result = resolve_identity_with_store(&store, &legacy_path, dir.path());
assert!(
result.is_err(),
"must fail closed, not generate a fresh key"
);
// No identity file was written — nothing was generated or persisted.
assert!(!legacy_path.exists());
}
#[test]
fn unreachable_first_run_generates_to_file_when_no_marker() {
// Genuine first-EVER launch on a machine whose keyring is down: no file,
// no marker. There is no prior identity to protect, so generating to the
// `0o600` file is correct — fail-closed here would block a legitimate
// first launch.
let dir = tempfile::tempdir().unwrap();
let legacy_path = dir.path().join("identity.key");
assert!(!legacy_path.exists());
assert!(!migration_marker_path(dir.path()).exists());
let store = FakeIdentityStore::unreachable();
let resolved = resolve_identity_with_store(&store, &legacy_path, dir.path()).unwrap();
// A fresh key was generated and persisted to the file (keyring is down).
let from_file = load_key_file(&legacy_path).unwrap();
assert_key_eq(&resolved, &from_file);
}
#[test]
fn migration_writes_marker_before_deleting_file() {
// Crash-safe ordering: a successful migration must leave the marker on
// disk AND remove the file. The marker existing while the file is gone
// is the durable post-migration signal the Unreachable arm relies on;
// "file gone, no marker" must never be the resting state.
let dir = tempfile::tempdir().unwrap();
let legacy_path = dir.path().join("identity.key");
let file_keys = Keys::generate();
save_key_file(&legacy_path, &file_keys).unwrap();
// ReachableButEmpty drives the one-time migration path.
let store = FakeIdentityStore::reachable_but_empty();
let resolved = resolve_identity_with_store(&store, &legacy_path, dir.path()).unwrap();
assert_key_eq(&file_keys, &resolved);
// Marker written, file deleted — the safe resting state.
assert!(migration_marker_path(dir.path()).exists());
assert!(!legacy_path.exists());
}
#[test]
fn fresh_keyring_generate_writes_marker() {
// Fix 1 (Pinky comment 1): a fresh install generating straight into a
// reachable-but-empty keyring must write the marker. Without it, "no
// file, no marker" matches a never-launched machine, so a later
// Unreachable boot would silently rotate the key.
let dir = tempfile::tempdir().unwrap();
let legacy_path = dir.path().join("identity.key");
assert!(!legacy_path.exists());
let store = FakeIdentityStore::reachable_but_empty();
let resolved = resolve_identity_with_store(&store, &legacy_path, dir.path()).unwrap();
// The key was stored in the keyring (not the file), and the marker marks it.
assert!(!legacy_path.exists());
assert!(migration_marker_path(dir.path()).exists());
assert_eq!(
store
.slot
.borrow()
.get(IDENTITY_KEY_NAME)
.map(String::as_str),
Some(resolved.secret_key().to_bech32().unwrap().as_str())
);
}
#[test]
fn fresh_keyring_generate_then_unreachable_fails_closed() {
// The end-to-end guard for Fix 1: after a fresh keyring-created identity
// (marker written, no file), a later boot with the keyring unreachable
// must FAIL CLOSED rather than generate a new key and rotate identity.
let dir = tempfile::tempdir().unwrap();
let legacy_path = dir.path().join("identity.key");
// First boot: fresh generate into a reachable keyring.
let reachable = FakeIdentityStore::reachable_but_empty();
resolve_identity_with_store(&reachable, &legacy_path, dir.path()).unwrap();
assert!(!legacy_path.exists());
assert!(migration_marker_path(dir.path()).exists());
// Second boot: keyring is down. No file + marker present → fail closed.
let unreachable = FakeIdentityStore::unreachable();
let result = resolve_identity_with_store(&unreachable, &legacy_path, dir.path());
assert!(
result.is_err(),
"must fail closed, not generate a fresh key"
);
assert!(!legacy_path.exists());
}
#[test]
fn fresh_generate_keyring_failure_falls_back_to_file_without_marker() {
// Fix 1 correctness on the file-fallback arm: when the keyring write
// FAILS during a fresh generate, the key must land in the `0o600` file
// and the marker must NOT be written — a marker here would wrongly trip
// the next Unreachable boot into failing closed even though the key is
// sitting in the file.
let dir = tempfile::tempdir().unwrap();
let legacy_path = dir.path().join("identity.key");
let store = FakeIdentityStore::store_failing();
let resolved = resolve_identity_with_store(&store, &legacy_path, dir.path()).unwrap();
// Key persisted to the file (fallback), and recoverable from it.
let from_file = load_key_file(&legacy_path).unwrap();
assert_key_eq(&resolved, &from_file);
// No marker: the file is the authoritative store, not the keyring.
assert!(!migration_marker_path(dir.path()).exists());
}
}

View File

@@ -8,11 +8,12 @@ use crate::{
find_managed_agent_mut, invoke_provider, load_managed_agents, load_personas,
managed_agent_avatar_url, managed_agent_log_path, managed_agents_base_dir,
normalize_agent_args, provider_deploy, read_log_tail, resolve_provider_binary,
save_managed_agents, start_managed_agent_process, stop_managed_agent_process,
sync_managed_agent_processes, try_regenerate_nest, validate_provider_config, BackendKind,
BackendProviderInfo, CreateManagedAgentRequest, CreateManagedAgentResponse,
ManagedAgentLogResponse, ManagedAgentRecord, ManagedAgentSummary, RelayMeshConfig,
DEFAULT_ACP_COMMAND, DEFAULT_AGENT_PARALLELISM, DEFAULT_AGENT_TURN_TIMEOUT_SECONDS,
save_managed_agents, spawn_key_refusal, start_managed_agent_process,
stop_managed_agent_process, sync_managed_agent_processes, try_regenerate_nest,
validate_provider_config, BackendKind, BackendProviderInfo, CreateManagedAgentRequest,
CreateManagedAgentResponse, ManagedAgentLogResponse, ManagedAgentRecord,
ManagedAgentSummary, RelayMeshConfig, DEFAULT_ACP_COMMAND, DEFAULT_AGENT_PARALLELISM,
DEFAULT_AGENT_TURN_TIMEOUT_SECONDS,
},
relay::{relay_ws_url_with_override, sync_managed_agent_profile},
util::now_iso,
@@ -148,6 +149,13 @@ fn build_deploy_payload(
state: &AppState,
record: &ManagedAgentRecord,
) -> Result<serde_json::Value, String> {
// Fail closed when the private key is unavailable (keyring outage leaves it
// empty after hydration). Without this, a provider deploy would serialize
// `"private_key_nsec": ""` and launch the agent with no identity — the same
// hazard the local spawn path already refuses via this guard.
if let Some(error) = spawn_key_refusal(record) {
return Err(error);
}
// Merge persona env_vars + agent env_vars for provider deploy. Same
// precedence as local spawn: persona first, agent overrides last. Without
// this, provider-backed agents wouldn't receive credentials saved on the
@@ -1149,6 +1157,8 @@ pub fn delete_managed_agent(
return Err(format!("agent {pubkey} not found"));
}
save_managed_agents(&app, &records)?;
// Remove the agent's nsec from the keyring after the record is gone.
crate::managed_agents::delete_agent_key(&pubkey);
}
try_regenerate_nest(&app);
Ok(())

View File

@@ -11,6 +11,7 @@ mod models;
pub mod nostr_convert;
mod prevent_sleep;
mod relay;
mod secret_store;
mod templates;
mod util;

View File

@@ -6,7 +6,7 @@ use crate::{
managed_agents::{
append_log_marker, known_acp_runtime, login_shell_path, managed_agent_log_path,
missing_command_message, normalize_agent_args, open_log_file, resolve_command,
ManagedAgentProcess, ManagedAgentRecord, ManagedAgentSummary,
spawn_key_refusal, ManagedAgentProcess, ManagedAgentRecord, ManagedAgentSummary,
},
util::now_iso,
};
@@ -1499,6 +1499,9 @@ pub fn spawn_agent_child(
record: &ManagedAgentRecord,
owner_hex: Option<&str>,
) -> Result<crate::managed_agents::ManagedAgentProcess, String> {
if let Some(error) = spawn_key_refusal(record) {
return Err(error);
}
let log_path = managed_agent_log_path(app, &record.pubkey)?;
append_log_marker(
&log_path,

View File

@@ -6,7 +6,25 @@ use std::{
use tauri::{AppHandle, Manager};
use crate::app_state::KEYRING_SERVICE;
use crate::managed_agents::ManagedAgentRecord;
use crate::secret_store::{KeyringProbe, SecretStore};
/// Keyring key name for an agent's nsec, namespaced from the human identity
/// key (`"identity"`) which shares the service.
fn agent_keyring_name(pubkey: &str) -> String {
format!("agent:{pubkey}")
}
/// The agent secret store. `None` when the build has no keyring backend, in
/// which case agent keys stay inline in the `0o600` JSON file.
fn agent_secret_store() -> Option<SecretStore> {
if cfg!(feature = "system-keyring") {
Some(SecretStore::keyring(KEYRING_SERVICE))
} else {
None
}
}
pub fn managed_agents_base_dir(app: &AppHandle) -> Result<PathBuf, String> {
let dir = app
@@ -32,6 +50,100 @@ pub fn managed_agent_log_path(app: &AppHandle, pubkey: &str) -> Result<PathBuf,
Ok(managed_agents_logs_dir(app)?.join(format!("{pubkey}.log")))
}
/// The keyring operations the migration chokepoint needs. Abstracted so the
/// migrate-and-strip decision logic ([`migrate_inline_key`]) can be unit-tested
/// against a fake without touching the live OS keyring.
trait KeyStore {
fn probe(&self, name: &str) -> KeyringProbe;
/// Read a key. `Ok(None)` is "no such entry" (absent); `Err` is a backend
/// failure (keyring unreachable) — the caller MUST NOT collapse the two.
fn load(&self, name: &str) -> Result<Option<String>, String>;
/// Write `value` and read it back to confirm before the caller strips the
/// inline copy.
fn write_and_verify(&self, name: &str, value: &str) -> Result<(), String>;
}
impl KeyStore for SecretStore {
fn probe(&self, name: &str) -> KeyringProbe {
SecretStore::probe(self, name)
}
fn load(&self, name: &str) -> Result<Option<String>, String> {
SecretStore::load(self, name)
}
fn write_and_verify(&self, name: &str, value: &str) -> Result<(), String> {
self.store(name, value)?;
match self.load(name)? {
Some(stored) if stored == value => Ok(()),
_ => Err("keyring read-back verify failed".to_string()),
}
}
}
/// Outcome of attempting to lift a record's inline key into the keyring.
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
enum KeyMigration {
/// Written to the keyring and read-back verified. Safe to drop the inline
/// copy when serializing.
Persisted,
/// Could not persist (keyring unreachable, or write/verify failed). The key
/// must stay inline (0o600 file fallback); do NOT drop it.
KeptInline,
/// The record carried no inline key, so there was nothing to migrate. Kept
/// distinct from [`KeyMigration::Persisted`] so an empty key is never
/// mistaken for "verified present in the keyring" — an empty key after a
/// keyring outage means the secret is currently unavailable, not persisted.
Nothing,
}
/// Attempt to lift one record's inline key into the keyring with read-back
/// verify. Pure decision logic — does NOT mutate the record, so the caller
/// chooses whether to strip the inline copy based on the returned outcome.
///
/// The single source of truth for the migrate-vs-keep decision, shared by the
/// load-time opportunistic re-migrate ([`hydrate_keys`]) and the save-time
/// chokepoint ([`persist_agent_keys`]). An empty key returns
/// [`KeyMigration::Nothing`] — never [`KeyMigration::Persisted`], so a record
/// left empty by a keyring outage is not mistaken for one verified present.
fn migrate_inline_key(store: &impl KeyStore, record: &ManagedAgentRecord) -> KeyMigration {
if record.private_key_nsec.is_empty() {
return KeyMigration::Nothing;
}
let name = agent_keyring_name(&record.pubkey);
match store.probe(&name) {
// Keyring down this boot: keep the key inline (file fallback), do NOT
// migrate — re-importing later could resurrect a rotated key.
KeyringProbe::Unreachable => KeyMigration::KeptInline,
KeyringProbe::Present | KeyringProbe::ReachableButEmpty => {
match store.write_and_verify(&name, &record.private_key_nsec) {
Ok(()) => KeyMigration::Persisted,
Err(e) => {
eprintln!(
"buzz-desktop: keyring write for agent {} failed ({e}), keeping inline",
record.pubkey
);
KeyMigration::KeptInline
}
}
}
}
}
/// Refuse to spawn an agent whose private key is unavailable. Returns
/// `Some(error)` when `private_key_nsec` is empty — after [`hydrate_keys`] an
/// empty key means a keyring outage or a genuinely absent secret, NOT a
/// deliberately keyless agent. Spawning anyway would inject an empty
/// `BUZZ_PRIVATE_KEY`/`NOSTR_PRIVATE_KEY`, launching with no identity. Callers
/// (the spawn path) must fail closed (Wes storage.rs:158).
pub(crate) fn spawn_key_refusal(record: &ManagedAgentRecord) -> Option<String> {
record.private_key_nsec.is_empty().then(|| {
format!(
"agent {} has no private key available — the OS keyring may be unreachable. \
Refusing to start without an identity; retry once the keyring is reachable.",
record.pubkey
)
})
}
pub fn load_managed_agents(app: &AppHandle) -> Result<Vec<ManagedAgentRecord>, String> {
let path = managed_agents_store_path(app)?;
if !path.exists() {
@@ -40,7 +152,69 @@ pub fn load_managed_agents(app: &AppHandle) -> Result<Vec<ManagedAgentRecord>, S
let content = fs::read_to_string(&path)
.map_err(|error| format!("failed to read agent store: {error}"))?;
serde_json::from_str(&content).map_err(|error| format!("failed to parse agent store: {error}"))
let mut records: Vec<ManagedAgentRecord> = serde_json::from_str(&content)
.map_err(|error| format!("failed to parse agent store: {error}"))?;
hydrate_keys(&mut records);
Ok(records)
}
/// Fill in each record's in-memory `private_key_nsec` from the keyring, and
/// opportunistically re-migrate any key that is still inline.
///
/// - Empty key → fetch it from the keyring (the normal keyring-backed case).
/// - Non-empty key → the JSON carried it inline because the keyring was
/// unreachable at its last save. Re-migrate it now ([`migrate_inline_key`]):
/// if the keyring is reachable this boot, write-verify-strip so the next save
/// writes clean JSON and plaintext stops lingering on disk; if still
/// unreachable, leave it inline. This makes the strip deterministic on the
/// next reachable boot rather than waiting for a non-deterministic save.
fn hydrate_keys(records: &mut [ManagedAgentRecord]) {
let Some(store) = agent_secret_store() else {
return;
};
hydrate_keys_with(&store, records);
}
/// Testable core of [`hydrate_keys`], generic over the [`KeyStore`] seam.
///
/// A keyring LOAD error (`Err`) is an OUTAGE — distinct from `Ok(None)`
/// (genuinely absent). On an outage the key is left empty and the record is
/// surfaced as unavailable rather than silently swallowed: callers must refuse
/// to spawn an agent whose key could not be read (see the empty-key bail in
/// `spawn_agent_child`). Empty here never means "fine" — it means "no usable
/// key this boot."
fn hydrate_keys_with(store: &impl KeyStore, records: &mut [ManagedAgentRecord]) {
for record in records.iter_mut() {
if record.private_key_nsec.is_empty() {
match store.load(&agent_keyring_name(&record.pubkey)) {
Ok(Some(nsec)) => record.private_key_nsec = nsec,
Ok(None) => {
eprintln!(
"buzz-desktop: agent {} has no key in JSON or keyring",
record.pubkey
);
}
// Outage, NOT absence: the key may exist in the keyring but is
// unreadable this boot. Leave it empty so the spawn path
// refuses rather than launching with no identity.
Err(e) => {
eprintln!(
"buzz-desktop: agent {} key unavailable — keyring read failed ({e}); \
agent will be refused until the keyring is reachable",
record.pubkey
);
}
}
} else {
// Inline residue from a prior keyring-unreachable save. Lift it
// into the keyring now (side effect) but KEEP it in memory — the
// returned record must carry the key for readers. The next save
// then strips it from JSON. Outcome is intentionally ignored:
// on failure the key simply stays inline until a later boot.
let _ = migrate_inline_key(store, record);
}
}
}
pub fn save_managed_agents(app: &AppHandle, records: &[ManagedAgentRecord]) -> Result<(), String> {
@@ -52,11 +226,50 @@ pub fn save_managed_agents(app: &AppHandle, records: &[ManagedAgentRecord]) -> R
.then_with(|| left.pubkey.cmp(&right.pubkey))
});
// Persist each key to the keyring; on success blank the inline copy so it
// is skipped from JSON (`skip_serializing_if = "String::is_empty"`). If the
// keyring is unreachable, the key stays inline.
persist_agent_keys(&mut sorted);
let path = managed_agents_store_path(app)?;
let payload = serde_json::to_vec_pretty(&sorted)
.map_err(|error| format!("failed to serialize agent store: {error}"))?;
atomic_write_json(&path, &payload)
// `managed-agents.json` carries plaintext agent nsecs in the keyringless
// fallback. Write it owner-only (`0o600`) unconditionally — harmless for the
// keyring-backed case (it is the user's own agent store) and closes the
// umask window a post-write `chmod` would leave open.
atomic_write_json_restricted(&path, &payload)
}
/// Write each record's in-memory key to the keyring and blank the inline copy
/// on success. Keys that cannot be persisted (keyring unreachable) stay inline
/// in the JSON. Mutates `records` (a save-local clone) — the caller's in-memory
/// records keep their keys.
fn persist_agent_keys(records: &mut [ManagedAgentRecord]) {
let Some(store) = agent_secret_store() else {
// No keyring backend: keys stay inline.
return;
};
for record in records.iter_mut() {
// Only a verified keyring entry lets us drop the inline copy. Both
// other outcomes keep the key inline: `KeptInline` (keyring
// unreachable) so it is not lost, and `Nothing` (empty key) because
// there is no verified entry to claim. This is a save-local clone, so
// callers keep their keys regardless.
if migrate_inline_key(&store, record) == KeyMigration::Persisted {
record.private_key_nsec.clear();
}
}
}
/// Remove an agent's key from the keyring (best-effort). Called when an agent
/// is deleted so its secret does not linger in the OS store.
pub fn delete_agent_key(pubkey: &str) {
if let Some(store) = agent_secret_store() {
if let Err(e) = store.delete(&agent_keyring_name(pubkey)) {
eprintln!("buzz-desktop: failed to delete agent {pubkey} key from keyring: {e}");
}
}
}
/// Atomic, symlink-preserving JSON write.
@@ -70,6 +283,34 @@ pub(crate) fn atomic_write_json(path: &Path, payload: &[u8]) -> Result<(), Strin
.map_err(|e| format!("failed to rename {}: {e}", resolved.display()))
}
/// Atomic, symlink-preserving JSON write that creates the file `0o600` BEFORE
/// any bytes hit disk — closing the umask window the post-write `chmod` left
/// open. Used for `managed-agents.json`, which carries plaintext agent nsecs in
/// the keyringless fallback. Mirrors [`crate::app_state::save_key_file`].
///
/// Canonicalizes `path` first so the write lands at the real target, preserving
/// any symlink at `path` exactly like [`atomic_write_json`].
pub(crate) fn atomic_write_json_restricted(path: &Path, payload: &[u8]) -> Result<(), String> {
use atomic_write_file::AtomicWriteFile;
let resolved = std::fs::canonicalize(path).unwrap_or_else(|_| path.to_path_buf());
let mut file = AtomicWriteFile::open(&resolved)
.map_err(|e| format!("open {} for atomic write: {e}", resolved.display()))?;
// Set owner-only permissions before writing the secret bytes.
#[cfg(unix)]
{
use std::os::unix::fs::PermissionsExt;
file.set_permissions(std::fs::Permissions::from_mode(0o600))
.map_err(|e| format!("set {} permissions: {e}", resolved.display()))?;
}
file.write_all(payload)
.map_err(|e| format!("write {}: {e}", resolved.display()))?;
file.commit()
.map_err(|e| format!("commit {}: {e}", resolved.display()))
}
/// Maximum log file size before rotation (10 MB).
const MAX_LOG_FILE_SIZE: u64 = 10 * 1024 * 1024;
@@ -213,16 +454,243 @@ pub fn meaningful_agent_error_from_log(path: &Path) -> Option<String> {
#[cfg(test)]
mod tests {
use std::cell::RefCell;
use std::collections::HashMap;
use std::io::Write as _;
use tempfile::NamedTempFile;
use super::{
agent_keyring_name, hydrate_keys_with, migrate_inline_key, KeyMigration, KeyStore,
KeyringProbe, ManagedAgentRecord,
};
/// In-memory [`KeyStore`] for testing the migrate decision without the OS
/// keyring. `reachable=false` simulates a backend outage; `fail_verify`
/// simulates a write whose read-back does not confirm.
struct FakeKeyStore {
reachable: bool,
fail_verify: bool,
stored: RefCell<HashMap<String, String>>,
}
impl FakeKeyStore {
fn reachable() -> Self {
Self {
reachable: true,
fail_verify: false,
stored: RefCell::new(HashMap::new()),
}
}
fn unreachable() -> Self {
Self {
reachable: false,
fail_verify: false,
stored: RefCell::new(HashMap::new()),
}
}
fn verify_fails() -> Self {
Self {
reachable: true,
fail_verify: true,
stored: RefCell::new(HashMap::new()),
}
}
/// Seed a key as already present in the keyring.
fn with_key(self, name: &str, value: &str) -> Self {
self.stored
.borrow_mut()
.insert(name.to_string(), value.to_string());
self
}
}
impl KeyStore for FakeKeyStore {
fn probe(&self, _name: &str) -> KeyringProbe {
if self.reachable {
KeyringProbe::ReachableButEmpty
} else {
KeyringProbe::Unreachable
}
}
fn load(&self, name: &str) -> Result<Option<String>, String> {
// An unreachable backend errors on read (outage), distinct from a
// reachable backend returning `Ok(None)` for an absent entry.
if !self.reachable {
return Err("keyring backend unreachable".to_string());
}
Ok(self.stored.borrow().get(name).cloned())
}
fn write_and_verify(&self, name: &str, value: &str) -> Result<(), String> {
if self.fail_verify {
return Err("read-back verify failed".to_string());
}
self.stored
.borrow_mut()
.insert(name.to_string(), value.to_string());
Ok(())
}
}
fn record_with_key(nsec: &str) -> ManagedAgentRecord {
serde_json::from_str(&format!(
r#"{{
"pubkey": "agent-pubkey",
"name": "test-agent",
"private_key_nsec": "{nsec}",
"relay_url": "wss://localhost:3000",
"acp_command": "buzz-acp",
"agent_command": "goose",
"agent_args": [],
"mcp_command": "",
"turn_timeout_seconds": 320,
"created_at": "2026-01-01T00:00:00Z",
"updated_at": "2026-01-01T00:00:00Z"
}}"#
))
.expect("sample record")
}
#[test]
fn migrate_persists_and_signals_stripping_when_keyring_reachable() {
// Item 2: an inline key (residue from a prior keyring-unreachable save)
// is written to the keyring and verified when the backend is reachable,
// so the next save can drop it from JSON.
let store = FakeKeyStore::reachable();
let record = record_with_key("nsec1realkey");
let outcome = migrate_inline_key(&store, &record);
assert_eq!(outcome, KeyMigration::Persisted);
assert_eq!(
store
.stored
.borrow()
.get(&agent_keyring_name("agent-pubkey"))
.map(String::as_str),
Some("nsec1realkey")
);
}
#[test]
fn migrate_keeps_inline_when_keyring_unreachable() {
// No-resurrection guard: a transient outage must NOT migrate; the key
// stays inline (file fallback) so it is not lost.
let store = FakeKeyStore::unreachable();
let record = record_with_key("nsec1realkey");
let outcome = migrate_inline_key(&store, &record);
assert_eq!(outcome, KeyMigration::KeptInline);
assert!(store.stored.borrow().is_empty());
}
#[test]
fn migrate_keeps_inline_when_verify_fails() {
// A write whose read-back does not confirm must keep the key inline —
// never drop plaintext on an unverified write.
let store = FakeKeyStore::verify_fails();
let record = record_with_key("nsec1realkey");
assert_eq!(
migrate_inline_key(&store, &record),
KeyMigration::KeptInline
);
}
#[test]
fn migrate_reports_nothing_for_empty_key() {
// A record whose key already lives in the keyring (empty inline) has
// nothing to migrate. It must NOT be reported as `Persisted` — an
// empty key after a keyring outage means the secret is unavailable,
// not verified present (Wes storage.rs:158).
let store = FakeKeyStore::reachable();
let record = record_with_key("");
assert_eq!(migrate_inline_key(&store, &record), KeyMigration::Nothing);
assert!(store.stored.borrow().is_empty());
}
#[test]
fn hydrate_fills_key_from_keyring_when_reachable() {
// The normal keyring-backed case: an empty inline key is filled from
// the keyring on load.
let store =
FakeKeyStore::reachable().with_key(&agent_keyring_name("agent-pubkey"), "nsec1stored");
let mut records = vec![record_with_key("")];
hydrate_keys_with(&store, &mut records);
assert_eq!(records[0].private_key_nsec, "nsec1stored");
}
#[test]
fn hydrate_leaves_key_empty_on_keyring_outage() {
// Outage edge (Wes storage.rs:158): when the keyring read ERRORS, the
// key must be left empty — never silently treated as resolved — so the
// spawn path refuses rather than launching the agent with no identity.
let store = FakeKeyStore::unreachable();
let mut records = vec![record_with_key("")];
hydrate_keys_with(&store, &mut records);
assert!(
records[0].private_key_nsec.is_empty(),
"an unreadable key must stay empty, not be fabricated"
);
}
#[test]
fn spawn_refused_when_private_key_empty() {
// The spawn path MUST refuse a record left empty by an outage/absence
// before injecting an empty BUZZ_PRIVATE_KEY / NOSTR_PRIVATE_KEY — never
// launch an agent with no identity (Wes storage.rs:158).
let record = record_with_key("");
assert!(
super::spawn_key_refusal(&record).is_some(),
"an agent with no private key must be refused"
);
}
#[test]
fn spawn_allowed_when_private_key_present() {
// A record carrying a key must not be blocked by the refusal guard.
let record = record_with_key("nsec1realkey");
assert!(super::spawn_key_refusal(&record).is_none());
}
fn write_log(content: &str) -> NamedTempFile {
let mut file = NamedTempFile::new().expect("temp log");
file.write_all(content.as_bytes()).expect("write log");
file
}
/// The keyringless fallback write must land `0o600` from the write itself —
/// not a post-write `chmod` — so a crash in the umask window can never leave
/// plaintext agent nsecs world-readable (Wes storage.rs:239, SECURITY.md:90).
#[cfg(unix)]
#[test]
fn restricted_write_lands_owner_only_without_post_write_chmod() {
use std::os::unix::fs::PermissionsExt;
let dir = tempfile::tempdir().expect("temp dir");
let path = dir.path().join("managed-agents.json");
super::atomic_write_json_restricted(&path, br#"[{"private_key_nsec":"nsec1secret"}]"#)
.expect("restricted write");
let mode = std::fs::metadata(&path)
.expect("metadata")
.permissions()
.mode()
& 0o777;
assert_eq!(mode, 0o600, "secret-bearing write must be owner-only");
assert_eq!(
std::fs::read_to_string(&path).expect("read back"),
r#"[{"private_key_nsec":"nsec1secret"}]"#
);
}
#[test]
fn meaningful_agent_error_from_log_promotes_wrapped_llm_auth() {
let file = write_log("noise\nAgent reported error: llm auth: denied\n");

View File

@@ -88,9 +88,16 @@ pub struct ManagedAgentRecord {
pub name: String,
#[serde(default)]
pub persona_id: Option<String>,
/// `#[serde(default)]` so an old build still parses a store whose inline
/// key was stripped after a keyring build migrated it into the Keychain.
#[serde(default)]
/// nsec private key. Held in memory but persisted to the OS keyring (keyed
/// by `pubkey`) rather than serialized to `managed-agents.json`. The
/// storage layer blanks this before writing JSON once the key is safely in
/// the keyring, and re-hydrates it from the keyring on load.
///
/// It is only serialized inline (the `0o600` JSON fallback) when the
/// keyring is unreachable — `skip_serializing_if` keeps it out of JSON in
/// the normal keyring-backed case. `default` also lets an old build parse a
/// store whose inline key was already migrated out and blanked.
#[serde(default, skip_serializing_if = "String::is_empty")]
pub private_key_nsec: String,
/// NIP-OA auth tag JSON. Computed at agent creation time.
///
@@ -656,354 +663,4 @@ pub fn validate_respond_to_allowlist(input: &[String]) -> Result<Vec<String>, St
}
#[cfg(test)]
mod tests {
use super::{ManagedAgentRecord, PersonaRecord};
use std::path::PathBuf;
#[test]
fn persona_record_defaults_active_when_field_is_missing() {
let record: PersonaRecord = serde_json::from_str(
r#"{
"id": "builtin:fizz",
"display_name": "Fizz",
"avatar_url": null,
"system_prompt": "Prompt",
"created_at": "2026-03-19T00:00:00Z",
"updated_at": "2026-03-19T00:00:00Z"
}"#,
)
.expect("legacy persona payload should deserialize");
assert!(record.is_active);
assert!(!record.is_builtin);
assert_eq!(record.runtime, None);
assert_eq!(record.model, None);
assert!(record.name_pool.is_empty());
}
/// Legacy agent records (created before NIP-OA) lack the `auth_tag` field.
/// `#[serde(default)]` must ensure they deserialize with `auth_tag: None`.
#[test]
fn managed_agent_record_without_auth_tag_deserializes() {
let record: ManagedAgentRecord = serde_json::from_str(
r#"{
"pubkey": "abcd1234",
"name": "test-agent",
"private_key_nsec": "nsec1fake",
"relay_url": "wss://localhost:3000",
"acp_command": "buzz-acp",
"agent_command": "goose",
"agent_args": [],
"mcp_command": "",
"turn_timeout_seconds": 320,
"system_prompt": null,
"created_at": "2026-01-01T00:00:00Z",
"updated_at": "2026-01-01T00:00:00Z",
"last_started_at": null,
"last_stopped_at": null,
"last_exit_code": null,
"last_error": null
}"#,
)
.expect("legacy agent record without auth_tag should deserialize");
assert_eq!(record.auth_tag, None);
assert_eq!(record.avatar_url, None);
assert_eq!(record.pubkey, "abcd1234");
}
/// Agent records WITH an auth_tag round-trip correctly through serde.
#[test]
fn managed_agent_record_with_auth_tag_round_trips() {
let json = r#"{
"pubkey": "abcd1234",
"name": "test-agent",
"private_key_nsec": "nsec1fake",
"auth_tag": "[\"auth\",\"deadbeef\",\"\",\"cafebabe\"]",
"relay_url": "wss://localhost:3000",
"acp_command": "buzz-acp",
"agent_command": "goose",
"agent_args": [],
"mcp_command": "",
"turn_timeout_seconds": 320,
"system_prompt": null,
"created_at": "2026-01-01T00:00:00Z",
"updated_at": "2026-01-01T00:00:00Z",
"last_started_at": null,
"last_stopped_at": null,
"last_exit_code": null,
"last_error": null
}"#;
let record: ManagedAgentRecord =
serde_json::from_str(json).expect("record with auth_tag should deserialize");
assert_eq!(
record.auth_tag.as_deref(),
Some(r#"["auth","deadbeef","","cafebabe"]"#)
);
// Round-trip: serialize and deserialize again.
let serialized = serde_json::to_string(&record).expect("should serialize");
let record2: ManagedAgentRecord =
serde_json::from_str(&serialized).expect("round-trip should deserialize");
assert_eq!(record.auth_tag, record2.auth_tag);
}
// ── Inbound author gate tests ────────────────────────────────────────
use super::{validate_respond_to_allowlist, RespondTo};
#[test]
fn respond_to_default_is_owner_only() {
assert_eq!(RespondTo::default(), RespondTo::OwnerOnly);
}
#[test]
fn respond_to_serde_is_kebab_case() {
assert_eq!(
serde_json::to_string(&RespondTo::OwnerOnly).unwrap(),
"\"owner-only\""
);
assert_eq!(
serde_json::to_string(&RespondTo::Allowlist).unwrap(),
"\"allowlist\""
);
assert_eq!(
serde_json::to_string(&RespondTo::Anyone).unwrap(),
"\"anyone\""
);
let parsed: RespondTo = serde_json::from_str("\"owner-only\"").unwrap();
assert_eq!(parsed, RespondTo::OwnerOnly);
let parsed: RespondTo = serde_json::from_str("\"allowlist\"").unwrap();
assert_eq!(parsed, RespondTo::Allowlist);
let parsed: RespondTo = serde_json::from_str("\"anyone\"").unwrap();
assert_eq!(parsed, RespondTo::Anyone);
}
#[test]
fn respond_to_rejects_unknown_modes() {
// `nobody` is a valid harness mode but intentionally not exposed
// through the desktop request types.
assert!(serde_json::from_str::<RespondTo>("\"nobody\"").is_err());
assert!(serde_json::from_str::<RespondTo>("\"OwnerOnly\"").is_err());
}
/// Records persisted before this feature must continue to load,
/// defaulting to OwnerOnly (the safe, matches-harness-default value).
#[test]
fn managed_agent_record_without_respond_to_fields_defaults_to_owner_only() {
let record: ManagedAgentRecord = serde_json::from_str(
r#"{
"pubkey": "abcd1234",
"name": "legacy-agent",
"private_key_nsec": "nsec1fake",
"relay_url": "wss://localhost:3000",
"acp_command": "buzz-acp",
"agent_command": "goose",
"agent_args": [],
"mcp_command": "",
"turn_timeout_seconds": 320,
"system_prompt": null,
"created_at": "2026-01-01T00:00:00Z",
"updated_at": "2026-01-01T00:00:00Z",
"last_started_at": null,
"last_stopped_at": null,
"last_exit_code": null,
"last_error": null
}"#,
)
.expect("legacy record without respond_to fields should deserialize");
assert_eq!(record.respond_to, RespondTo::OwnerOnly);
assert!(record.respond_to_allowlist.is_empty());
}
#[test]
fn validate_respond_to_allowlist_accepts_valid_hex_and_lowercases() {
let upper = "A".repeat(64);
let lower = "a".repeat(64);
let result = validate_respond_to_allowlist(std::slice::from_ref(&upper)).unwrap();
assert_eq!(result, vec![lower.clone()]);
}
#[test]
fn validate_respond_to_allowlist_dedups_preserving_order() {
let a = "a".repeat(64);
let b = "b".repeat(64);
let a_upper = "A".repeat(64);
let input = vec![a.clone(), b.clone(), a_upper];
let result = validate_respond_to_allowlist(&input).unwrap();
assert_eq!(result, vec![a, b]);
}
#[test]
fn validate_respond_to_allowlist_rejects_wrong_length() {
let too_short = "a".repeat(63);
assert!(validate_respond_to_allowlist(&[too_short]).is_err());
let too_long = "a".repeat(65);
assert!(validate_respond_to_allowlist(&[too_long]).is_err());
}
#[test]
fn validate_respond_to_allowlist_rejects_non_hex() {
let bad = "z".repeat(64);
assert!(validate_respond_to_allowlist(&[bad]).is_err());
// npub-style strings should not slip through.
let npub = format!("npub1{}", "a".repeat(59));
assert!(validate_respond_to_allowlist(&[npub]).is_err());
}
#[test]
fn validate_respond_to_allowlist_trims_whitespace() {
let padded = format!(" {} ", "a".repeat(64));
let result = validate_respond_to_allowlist(&[padded]).unwrap();
assert_eq!(result, vec!["a".repeat(64)]);
}
#[test]
fn validate_respond_to_allowlist_accepts_empty() {
// Empty is allowed at this layer; the boundary check
// (Allowlist mode requires ≥1 entry) is the caller's job.
let result = validate_respond_to_allowlist(&[]).unwrap();
assert!(result.is_empty());
}
use super::{CreateManagedAgentRequest, RelayMeshConfig};
/// Wire-shape test: the create request arrives from TS as camelCase
/// (`relayMesh: { modelRef }`). `rename_all = "camelCase"` on
/// `CreateManagedAgentRequest` does NOT recurse into nested structs, so
/// `RelayMeshConfig` needs its own `alias = "modelRef"`. This test pins
/// the exact JSON the frontend sends; if the alias is dropped, creating
/// a relay-mesh agent fails to deserialize at the Tauri boundary.
#[test]
fn create_request_deserializes_camel_case_relay_mesh() {
let request: CreateManagedAgentRequest = serde_json::from_str(
r#"{
"name": "mesh-agent",
"relayMesh": { "modelRef": "Qwen3" }
}"#,
)
.expect("camelCase relayMesh payload from TS should deserialize");
assert_eq!(
request.relay_mesh,
Some(RelayMeshConfig {
model_ref: "Qwen3".to_string()
})
);
}
/// Persisted records use snake_case; the camelCase alias must not break
/// the stored-record round trip.
#[test]
fn relay_mesh_config_round_trips_snake_case() {
let config = RelayMeshConfig {
model_ref: "Qwen3".to_string(),
};
let json = serde_json::to_string(&config).unwrap();
assert_eq!(json, r#"{"model_ref":"Qwen3"}"#);
let back: RelayMeshConfig = serde_json::from_str(&json).unwrap();
assert_eq!(back, config);
}
// ── Packs → Teams serde alias backward compatibility ────────────────
#[test]
fn persona_record_deserializes_old_source_pack_fields_via_alias() {
let record: PersonaRecord = serde_json::from_str(
r#"{
"id": "persona-1",
"display_name": "Test",
"avatar_url": null,
"system_prompt": "Prompt",
"source_pack": "com.example.my-pack",
"source_pack_persona_slug": "agent-one",
"created_at": "2026-01-01T00:00:00Z",
"updated_at": "2026-01-01T00:00:00Z"
}"#,
)
.expect("old-format persona with source_pack should deserialize via alias");
assert_eq!(record.source_team.as_deref(), Some("com.example.my-pack"));
assert_eq!(
record.source_team_persona_slug.as_deref(),
Some("agent-one")
);
}
#[test]
fn persona_record_serializes_new_field_names() {
let record: PersonaRecord = serde_json::from_str(
r#"{
"id": "persona-1",
"display_name": "Test",
"avatar_url": null,
"system_prompt": "Prompt",
"source_team": "com.example.my-team",
"source_team_persona_slug": "agent-one",
"created_at": "2026-01-01T00:00:00Z",
"updated_at": "2026-01-01T00:00:00Z"
}"#,
)
.unwrap();
let json = serde_json::to_string(&record).unwrap();
assert!(json.contains("source_team"));
assert!(json.contains("source_team_persona_slug"));
assert!(!json.contains("source_pack"));
}
#[test]
fn managed_agent_record_deserializes_old_pack_path_fields_via_alias() {
let record: ManagedAgentRecord = serde_json::from_str(
r#"{
"pubkey": "abcd1234",
"name": "test-agent",
"private_key_nsec": "nsec1fake",
"relay_url": "wss://localhost:3000",
"acp_command": "buzz-acp",
"agent_command": "goose",
"agent_args": [],
"mcp_command": "",
"turn_timeout_seconds": 320,
"system_prompt": null,
"persona_pack_path": "/path/to/agents/packs/my-pack",
"persona_name_in_pack": "agent-one",
"created_at": "2026-01-01T00:00:00Z",
"updated_at": "2026-01-01T00:00:00Z",
"last_started_at": null,
"last_stopped_at": null,
"last_exit_code": null,
"last_error": null
}"#,
)
.expect("old-format agent with persona_pack_path should deserialize via alias");
assert_eq!(
record.persona_team_dir,
Some(PathBuf::from("/path/to/agents/packs/my-pack"))
);
assert_eq!(record.persona_name_in_team.as_deref(), Some("agent-one"));
}
#[test]
fn team_record_deserializes_without_new_fields() {
let record: super::TeamRecord = serde_json::from_str(
r#"{
"id": "team-1",
"name": "My Team",
"description": null,
"persona_ids": ["p1", "p2"],
"is_builtin": false,
"created_at": "2026-01-01T00:00:00Z",
"updated_at": "2026-01-01T00:00:00Z"
}"#,
)
.expect("team record without new fields should deserialize with defaults");
assert_eq!(record.source_dir, None);
assert!(!record.is_symlink);
assert_eq!(record.symlink_target, None);
assert_eq!(record.version, None);
}
}
mod tests;

View File

@@ -0,0 +1,429 @@
use super::{ManagedAgentRecord, PersonaRecord};
use std::path::PathBuf;
#[test]
fn persona_record_defaults_active_when_field_is_missing() {
let record: PersonaRecord = serde_json::from_str(
r#"{
"id": "builtin:fizz",
"display_name": "Fizz",
"avatar_url": null,
"system_prompt": "Prompt",
"created_at": "2026-03-19T00:00:00Z",
"updated_at": "2026-03-19T00:00:00Z"
}"#,
)
.expect("legacy persona payload should deserialize");
assert!(record.is_active);
assert!(!record.is_builtin);
assert_eq!(record.runtime, None);
assert_eq!(record.model, None);
assert!(record.name_pool.is_empty());
}
/// Legacy agent records (created before NIP-OA) lack the `auth_tag` field.
/// `#[serde(default)]` must ensure they deserialize with `auth_tag: None`.
#[test]
fn managed_agent_record_without_auth_tag_deserializes() {
let record: ManagedAgentRecord = serde_json::from_str(
r#"{
"pubkey": "abcd1234",
"name": "test-agent",
"private_key_nsec": "nsec1fake",
"relay_url": "wss://localhost:3000",
"acp_command": "buzz-acp",
"agent_command": "goose",
"agent_args": [],
"mcp_command": "",
"turn_timeout_seconds": 320,
"system_prompt": null,
"created_at": "2026-01-01T00:00:00Z",
"updated_at": "2026-01-01T00:00:00Z",
"last_started_at": null,
"last_stopped_at": null,
"last_exit_code": null,
"last_error": null
}"#,
)
.expect("legacy agent record without auth_tag should deserialize");
assert_eq!(record.auth_tag, None);
assert_eq!(record.avatar_url, None);
assert_eq!(record.pubkey, "abcd1234");
}
/// Agent records WITH an auth_tag round-trip correctly through serde.
#[test]
fn managed_agent_record_with_auth_tag_round_trips() {
let json = r#"{
"pubkey": "abcd1234",
"name": "test-agent",
"private_key_nsec": "nsec1fake",
"auth_tag": "[\"auth\",\"deadbeef\",\"\",\"cafebabe\"]",
"relay_url": "wss://localhost:3000",
"acp_command": "buzz-acp",
"agent_command": "goose",
"agent_args": [],
"mcp_command": "",
"turn_timeout_seconds": 320,
"system_prompt": null,
"created_at": "2026-01-01T00:00:00Z",
"updated_at": "2026-01-01T00:00:00Z",
"last_started_at": null,
"last_stopped_at": null,
"last_exit_code": null,
"last_error": null
}"#;
let record: ManagedAgentRecord =
serde_json::from_str(json).expect("record with auth_tag should deserialize");
assert_eq!(
record.auth_tag.as_deref(),
Some(r#"["auth","deadbeef","","cafebabe"]"#)
);
// Round-trip: serialize and deserialize again.
let serialized = serde_json::to_string(&record).expect("should serialize");
let record2: ManagedAgentRecord =
serde_json::from_str(&serialized).expect("round-trip should deserialize");
assert_eq!(record.auth_tag, record2.auth_tag);
}
// ── Inbound author gate tests ────────────────────────────────────────
use super::{validate_respond_to_allowlist, RespondTo};
#[test]
fn respond_to_default_is_owner_only() {
assert_eq!(RespondTo::default(), RespondTo::OwnerOnly);
}
#[test]
fn respond_to_serde_is_kebab_case() {
assert_eq!(
serde_json::to_string(&RespondTo::OwnerOnly).unwrap(),
"\"owner-only\""
);
assert_eq!(
serde_json::to_string(&RespondTo::Allowlist).unwrap(),
"\"allowlist\""
);
assert_eq!(
serde_json::to_string(&RespondTo::Anyone).unwrap(),
"\"anyone\""
);
let parsed: RespondTo = serde_json::from_str("\"owner-only\"").unwrap();
assert_eq!(parsed, RespondTo::OwnerOnly);
let parsed: RespondTo = serde_json::from_str("\"allowlist\"").unwrap();
assert_eq!(parsed, RespondTo::Allowlist);
let parsed: RespondTo = serde_json::from_str("\"anyone\"").unwrap();
assert_eq!(parsed, RespondTo::Anyone);
}
#[test]
fn respond_to_rejects_unknown_modes() {
// `nobody` is a valid harness mode but intentionally not exposed
// through the desktop request types.
assert!(serde_json::from_str::<RespondTo>("\"nobody\"").is_err());
assert!(serde_json::from_str::<RespondTo>("\"OwnerOnly\"").is_err());
}
/// Records persisted before this feature must continue to load,
/// defaulting to OwnerOnly (the safe, matches-harness-default value).
#[test]
fn managed_agent_record_without_respond_to_fields_defaults_to_owner_only() {
let record: ManagedAgentRecord = serde_json::from_str(
r#"{
"pubkey": "abcd1234",
"name": "legacy-agent",
"private_key_nsec": "nsec1fake",
"relay_url": "wss://localhost:3000",
"acp_command": "buzz-acp",
"agent_command": "goose",
"agent_args": [],
"mcp_command": "",
"turn_timeout_seconds": 320,
"system_prompt": null,
"created_at": "2026-01-01T00:00:00Z",
"updated_at": "2026-01-01T00:00:00Z",
"last_started_at": null,
"last_stopped_at": null,
"last_exit_code": null,
"last_error": null
}"#,
)
.expect("legacy record without respond_to fields should deserialize");
assert_eq!(record.respond_to, RespondTo::OwnerOnly);
assert!(record.respond_to_allowlist.is_empty());
}
#[test]
fn validate_respond_to_allowlist_accepts_valid_hex_and_lowercases() {
let upper = "A".repeat(64);
let lower = "a".repeat(64);
let result = validate_respond_to_allowlist(std::slice::from_ref(&upper)).unwrap();
assert_eq!(result, vec![lower.clone()]);
}
#[test]
fn validate_respond_to_allowlist_dedups_preserving_order() {
let a = "a".repeat(64);
let b = "b".repeat(64);
let a_upper = "A".repeat(64);
let input = vec![a.clone(), b.clone(), a_upper];
let result = validate_respond_to_allowlist(&input).unwrap();
assert_eq!(result, vec![a, b]);
}
#[test]
fn validate_respond_to_allowlist_rejects_wrong_length() {
let too_short = "a".repeat(63);
assert!(validate_respond_to_allowlist(&[too_short]).is_err());
let too_long = "a".repeat(65);
assert!(validate_respond_to_allowlist(&[too_long]).is_err());
}
#[test]
fn validate_respond_to_allowlist_rejects_non_hex() {
let bad = "z".repeat(64);
assert!(validate_respond_to_allowlist(&[bad]).is_err());
// npub-style strings should not slip through.
let npub = format!("npub1{}", "a".repeat(59));
assert!(validate_respond_to_allowlist(&[npub]).is_err());
}
#[test]
fn validate_respond_to_allowlist_trims_whitespace() {
let padded = format!(" {} ", "a".repeat(64));
let result = validate_respond_to_allowlist(&[padded]).unwrap();
assert_eq!(result, vec!["a".repeat(64)]);
}
#[test]
fn validate_respond_to_allowlist_accepts_empty() {
// Empty is allowed at this layer; the boundary check
// (Allowlist mode requires ≥1 entry) is the caller's job.
let result = validate_respond_to_allowlist(&[]).unwrap();
assert!(result.is_empty());
}
use super::{CreateManagedAgentRequest, RelayMeshConfig};
/// Wire-shape test: the create request arrives from TS as camelCase
/// (`relayMesh: { modelRef }`). `rename_all = "camelCase"` on
/// `CreateManagedAgentRequest` does NOT recurse into nested structs, so
/// `RelayMeshConfig` needs its own `alias = "modelRef"`. This test pins
/// the exact JSON the frontend sends; if the alias is dropped, creating
/// a relay-mesh agent fails to deserialize at the Tauri boundary.
#[test]
fn create_request_deserializes_camel_case_relay_mesh() {
let request: CreateManagedAgentRequest = serde_json::from_str(
r#"{
"name": "mesh-agent",
"relayMesh": { "modelRef": "Qwen3" }
}"#,
)
.expect("camelCase relayMesh payload from TS should deserialize");
assert_eq!(
request.relay_mesh,
Some(RelayMeshConfig {
model_ref: "Qwen3".to_string()
})
);
}
/// Persisted records use snake_case; the camelCase alias must not break
/// the stored-record round trip.
#[test]
fn relay_mesh_config_round_trips_snake_case() {
let config = RelayMeshConfig {
model_ref: "Qwen3".to_string(),
};
let json = serde_json::to_string(&config).unwrap();
assert_eq!(json, r#"{"model_ref":"Qwen3"}"#);
let back: RelayMeshConfig = serde_json::from_str(&json).unwrap();
assert_eq!(back, config);
}
// ── Packs → Teams serde alias backward compatibility ────────────────
#[test]
fn persona_record_deserializes_old_source_pack_fields_via_alias() {
let record: PersonaRecord = serde_json::from_str(
r#"{
"id": "persona-1",
"display_name": "Test",
"avatar_url": null,
"system_prompt": "Prompt",
"source_pack": "com.example.my-pack",
"source_pack_persona_slug": "agent-one",
"created_at": "2026-01-01T00:00:00Z",
"updated_at": "2026-01-01T00:00:00Z"
}"#,
)
.expect("old-format persona with source_pack should deserialize via alias");
assert_eq!(record.source_team.as_deref(), Some("com.example.my-pack"));
assert_eq!(
record.source_team_persona_slug.as_deref(),
Some("agent-one")
);
}
#[test]
fn persona_record_serializes_new_field_names() {
let record: PersonaRecord = serde_json::from_str(
r#"{
"id": "persona-1",
"display_name": "Test",
"avatar_url": null,
"system_prompt": "Prompt",
"source_team": "com.example.my-team",
"source_team_persona_slug": "agent-one",
"created_at": "2026-01-01T00:00:00Z",
"updated_at": "2026-01-01T00:00:00Z"
}"#,
)
.unwrap();
let json = serde_json::to_string(&record).unwrap();
assert!(json.contains("source_team"));
assert!(json.contains("source_team_persona_slug"));
assert!(!json.contains("source_pack"));
}
#[test]
fn managed_agent_record_deserializes_old_pack_path_fields_via_alias() {
let record: ManagedAgentRecord = serde_json::from_str(
r#"{
"pubkey": "abcd1234",
"name": "test-agent",
"private_key_nsec": "nsec1fake",
"relay_url": "wss://localhost:3000",
"acp_command": "buzz-acp",
"agent_command": "goose",
"agent_args": [],
"mcp_command": "",
"turn_timeout_seconds": 320,
"system_prompt": null,
"persona_pack_path": "/path/to/agents/packs/my-pack",
"persona_name_in_pack": "agent-one",
"created_at": "2026-01-01T00:00:00Z",
"updated_at": "2026-01-01T00:00:00Z",
"last_started_at": null,
"last_stopped_at": null,
"last_exit_code": null,
"last_error": null
}"#,
)
.expect("old-format agent with persona_pack_path should deserialize via alias");
assert_eq!(
record.persona_team_dir,
Some(PathBuf::from("/path/to/agents/packs/my-pack"))
);
assert_eq!(record.persona_name_in_team.as_deref(), Some("agent-one"));
}
#[test]
fn team_record_deserializes_without_new_fields() {
let record: super::TeamRecord = serde_json::from_str(
r#"{
"id": "team-1",
"name": "My Team",
"description": null,
"persona_ids": ["p1", "p2"],
"is_builtin": false,
"created_at": "2026-01-01T00:00:00Z",
"updated_at": "2026-01-01T00:00:00Z"
}"#,
)
.expect("team record without new fields should deserialize with defaults");
assert_eq!(record.source_dir, None);
assert!(!record.is_symlink);
assert_eq!(record.symlink_target, None);
assert_eq!(record.version, None);
}
/// A record whose in-memory key was blanked (because it lives in the
/// keyring) must NOT serialize `private_key_nsec` into JSON.
#[test]
fn managed_agent_record_omits_empty_key_from_json() {
let mut record = sample_agent_record();
record.private_key_nsec = String::new();
let json = serde_json::to_string(&record).expect("serialize");
assert!(
!json.contains("private_key_nsec"),
"blanked key must be skipped from JSON, got: {json}"
);
}
/// A record with an inline key (the keyringless `0o600` JSON fallback)
/// serializes the key and round-trips it back.
#[test]
fn managed_agent_record_serializes_inline_key_for_fallback() {
let mut record = sample_agent_record();
record.private_key_nsec = "nsec1fallback".to_string();
let json = serde_json::to_string(&record).expect("serialize");
assert!(json.contains("nsec1fallback"));
let back: ManagedAgentRecord = serde_json::from_str(&json).expect("deserialize");
assert_eq!(back.private_key_nsec, "nsec1fallback");
}
/// A keyring-backed record on disk lacks `private_key_nsec`; it must
/// deserialize with an empty key (to be hydrated from the keyring).
#[test]
fn managed_agent_record_without_key_deserializes_empty() {
let record: ManagedAgentRecord = serde_json::from_str(
r#"{
"pubkey": "abcd1234",
"name": "test-agent",
"relay_url": "wss://localhost:3000",
"acp_command": "buzz-acp",
"agent_command": "goose",
"agent_args": [],
"mcp_command": "",
"turn_timeout_seconds": 320,
"system_prompt": null,
"created_at": "2026-01-01T00:00:00Z",
"updated_at": "2026-01-01T00:00:00Z",
"last_started_at": null,
"last_stopped_at": null,
"last_exit_code": null,
"last_error": null
}"#,
)
.expect("keyring-backed record without inline key should deserialize");
assert_eq!(record.private_key_nsec, "");
}
fn sample_agent_record() -> ManagedAgentRecord {
serde_json::from_str(
r#"{
"pubkey": "abcd1234",
"name": "test-agent",
"private_key_nsec": "nsec1fake",
"relay_url": "wss://localhost:3000",
"acp_command": "buzz-acp",
"agent_command": "goose",
"agent_args": [],
"mcp_command": "",
"turn_timeout_seconds": 320,
"system_prompt": null,
"created_at": "2026-01-01T00:00:00Z",
"updated_at": "2026-01-01T00:00:00Z",
"last_started_at": null,
"last_stopped_at": null,
"last_exit_code": null,
"last_error": null
}"#,
)
.expect("sample record")
}

View File

@@ -263,6 +263,11 @@ fn copy_file_over_generated_default(src: &Path, dst: &Path) -> std::io::Result<(
/// Read a JSON array of objects from `path`, apply `f` to each object,
/// and write back if any mutation returned `true`.
///
/// Writes back via [`crate::managed_agents::atomic_write_json_restricted`]
/// (owner-only `0o600`): the store files this rewrites can carry plaintext
/// agent nsecs on a keyringless host, so the write must not reopen the umask
/// window SECURITY.md:90 closes.
fn patch_json_records(
path: &Path,
mut f: impl FnMut(&mut serde_json::Map<String, serde_json::Value>) -> bool,
@@ -285,7 +290,7 @@ fn patch_json_records(
}
if changed {
if let Ok(bytes) = serde_json::to_vec_pretty(&records) {
if let Err(e) = crate::managed_agents::atomic_write_json(path, &bytes) {
if let Err(e) = crate::managed_agents::atomic_write_json_restricted(path, &bytes) {
eprintln!("buzz-desktop: patch-json-records: {e}");
}
}

View File

@@ -654,6 +654,36 @@ fn migrate_packs_to_teams_rewrites_agents_json() {
assert!(records[0].get("persona_name_in_pack").is_none());
}
/// `patch_json_records` rewrites `managed-agents.json`, which carries plaintext
/// agent nsecs on a keyringless host — the writeback must land `0o600` from the
/// write itself (no post-write `chmod`), or a launch-time reconcile reopens the
/// umask window SECURITY.md:90 closes (Thufir, migration.rs:288).
#[cfg(unix)]
#[test]
fn patch_json_records_rewrites_secret_store_owner_only() {
use std::os::unix::fs::PermissionsExt;
let dir = tempfile::tempdir().unwrap();
write_agents_json(
dir.path(),
&serde_json::json!([{ "private_key_nsec": "nsec1secret", "provider": "goose" }]),
);
let path = dir.path().join("agents/managed-agents.json");
// Mutate so the write actually fires (it only writes back on `changed`).
patch_json_records(&path, |obj| {
let provider = obj.remove("provider").unwrap();
obj.insert("runtime".to_string(), provider);
true
});
let mode = std::fs::metadata(&path).unwrap().permissions().mode() & 0o777;
assert_eq!(mode, 0o600, "secret-bearing rewrite must be owner-only");
let records = read_agents_json(dir.path());
assert_eq!(records[0]["private_key_nsec"], "nsec1secret");
assert_eq!(records[0]["runtime"], "goose");
}
#[test]
fn rename_provider_to_runtime_migrates_field() {
let dir = tempfile::tempdir().unwrap();

View File

@@ -0,0 +1,169 @@
//! OS keyring access for desktop nsec private keys.
//!
//! Backed by the `keyring` crate (macOS Keychain / Windows Credential Manager /
//! Linux Secret Service via D-Bus). The chosen backend is selected at compile
//! time by the per-target feature in `Cargo.toml`. The `system-keyring`
//! feature gates the whole store; when it is off, [`SecretStore`] is unusable
//! and callers fall back to their own `0o600` file storage.
//!
//! The store is deliberately NOT on any env-read path. `BUZZ_PRIVATE_KEY`
//! resolution for harnessed agents and CI is handled upstream (an env
//! short-circuit for the human key, child-process env injection for agents);
//! adding an env tier here would duplicate that precedence and create a
//! divergent-behavior trap.
/// Result of probing the keyring before a migration: distinguishes "reachable
/// but holds no entry" (safe to migrate into) from "unreachable this boot"
/// (must NOT migrate — re-importing from a leftover plaintext file could
/// resurrect a rotated/stale key).
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
pub enum KeyringProbe {
/// Keyring is reachable and an entry for the key already exists.
Present,
/// Keyring is reachable but has no entry for the key.
ReachableButEmpty,
/// Keyring backend is unavailable this boot (no Secret Service, dbus
/// failure, etc.). Migration must be skipped.
Unreachable,
}
/// An OS keyring, addressed by service name. Each logical secret is a distinct
/// key within the service (passed to each operation as the keyring "username").
pub struct SecretStore {
service: String,
}
impl SecretStore {
/// Keyring-backed store under `service`. The active platform backend
/// (apple-native / windows-native / sync-secret-service) is chosen at
/// compile time.
pub fn keyring(service: impl Into<String>) -> Self {
SecretStore {
service: service.into(),
}
}
}
/// Whether a keyring error string indicates the backend itself is unavailable
/// (vs. a per-entry error like "not found"). Mirrors goose's discriminator
/// (`crates/goose/src/config/base.rs`): treat dbus / Secret Service / platform
/// secure-storage failures as "keyring unavailable, fall back to file".
#[cfg(feature = "system-keyring")]
fn is_keyring_availability_error(error_str: &str) -> bool {
let lower = error_str.to_lowercase();
lower.contains("keyring")
|| lower.contains("dbus")
|| lower.contains("org.freedesktop.secrets")
|| lower.contains("platform secure storage")
|| lower.contains("no secret service")
}
#[cfg(feature = "system-keyring")]
fn keyring_entry(service: &str, key: &str) -> Result<keyring::Entry, keyring::Error> {
keyring::Entry::new(service, key)
}
impl SecretStore {
/// Probe whether `key` exists and whether the backend is reachable.
pub fn probe(&self, key: &str) -> KeyringProbe {
#[cfg(feature = "system-keyring")]
{
match keyring_entry(&self.service, key) {
Ok(entry) => match entry.get_password() {
Ok(_) => KeyringProbe::Present,
Err(keyring::Error::NoEntry) => KeyringProbe::ReachableButEmpty,
Err(e) if is_keyring_availability_error(&e.to_string()) => {
KeyringProbe::Unreachable
}
// A non-availability per-entry error (e.g. bad attributes)
// means the backend is reachable but the entry is unusable.
Err(_) => KeyringProbe::ReachableButEmpty,
},
Err(e) if is_keyring_availability_error(&e.to_string()) => {
KeyringProbe::Unreachable
}
Err(_) => KeyringProbe::Unreachable,
}
}
#[cfg(not(feature = "system-keyring"))]
{
let _ = key;
KeyringProbe::Unreachable
}
}
/// Load the secret for `key`. `Ok(None)` when there is no entry; `Err` only
/// when the backend errored in a way that is not "missing".
pub fn load(&self, key: &str) -> Result<Option<String>, String> {
#[cfg(feature = "system-keyring")]
{
let entry =
keyring_entry(&self.service, key).map_err(|e| format!("keyring entry: {e}"))?;
match entry.get_password() {
Ok(secret) => Ok(Some(secret)),
Err(keyring::Error::NoEntry) => Ok(None),
Err(e) => Err(format!("keyring get: {e}")),
}
}
#[cfg(not(feature = "system-keyring"))]
{
let _ = key;
Err("system-keyring feature disabled".to_string())
}
}
/// Store `value` for `key`. Reports `Err` on availability failures — callers
/// decide whether to fall back to file storage.
pub fn store(&self, key: &str, value: &str) -> Result<(), String> {
#[cfg(feature = "system-keyring")]
{
let entry =
keyring_entry(&self.service, key).map_err(|e| format!("keyring entry: {e}"))?;
entry
.set_password(value)
.map_err(|e| format!("keyring set: {e}"))
}
#[cfg(not(feature = "system-keyring"))]
{
let _ = (key, value);
Err("system-keyring feature disabled".to_string())
}
}
/// Delete the secret for `key`. A missing entry is not an error.
pub fn delete(&self, key: &str) -> Result<(), String> {
#[cfg(feature = "system-keyring")]
{
let entry =
keyring_entry(&self.service, key).map_err(|e| format!("keyring entry: {e}"))?;
match entry.delete_credential() {
Ok(()) | Err(keyring::Error::NoEntry) => Ok(()),
Err(e) => Err(format!("keyring delete: {e}")),
}
}
#[cfg(not(feature = "system-keyring"))]
{
let _ = key;
Err("system-keyring feature disabled".to_string())
}
}
}
#[cfg(all(test, feature = "system-keyring"))]
mod tests {
use super::*;
#[test]
fn availability_error_discriminator() {
assert!(is_keyring_availability_error("dbus connection failed"));
assert!(is_keyring_availability_error(
"org.freedesktop.secrets not provided"
));
assert!(is_keyring_availability_error("No Secret Service"));
assert!(is_keyring_availability_error(
"Platform secure storage failure"
));
// A plain "not found" is per-entry, not an availability failure.
assert!(!is_keyring_availability_error("entry not found"));
}
}