diff --git a/desktop/src-tauri/src/managed_agents/reconcile.rs b/desktop/src-tauri/src/managed_agents/reconcile.rs index 2ddd311190..3f1fb11f61 100644 --- a/desktop/src-tauri/src/managed_agents/reconcile.rs +++ b/desktop/src-tauri/src/managed_agents/reconcile.rs @@ -72,13 +72,40 @@ pub(crate) fn reconcile_agents_to_events( /// Returns the number of agents (re)written to the retention store. #[cfg(test)] pub(crate) fn reconcile_agents_in_dir(base_dir: &Path, keys: &nostr::Keys) -> Result { - reconcile_agents_in_dir_at(base_dir, keys, &base_dir.join("retention.db")) + // No key store: unit tests must never touch the live OS keyring (a macOS + // Keychain ACL prompt blocks a headless test binary forever). Tests that + // exercise key hydration inject a fake via + // [`reconcile_agents_in_dir_with`]. + reconcile_agents_in_dir_with( + base_dir, + keys, + &base_dir.join("retention.db"), + None::<&crate::secret_store::SecretStore>, + ) } +/// Production entry: hydrates keys from the real agent secret store (`None` +/// on builds without a keyring backend, where keys stay inline in the JSON). fn reconcile_agents_in_dir_at( base_dir: &Path, keys: &nostr::Keys, db_path: &Path, +) -> Result { + reconcile_agents_in_dir_with( + base_dir, + keys, + db_path, + super::storage::agent_secret_store(), + ) +} + +/// Testable core, generic over the [`super::storage::KeyStore`] seam so unit +/// tests never reach the live OS keyring. +fn reconcile_agents_in_dir_with( + base_dir: &Path, + keys: &nostr::Keys, + db_path: &Path, + store: Option<&impl super::storage::KeyStore>, ) -> Result { let store_path = base_dir.join("managed-agents.json"); if !store_path.exists() { @@ -101,7 +128,9 @@ fn reconcile_agents_in_dir_at( // default `system-keyring` build lives in the keyring and NOT in the JSON. // Without this, `retain_private_agent_record`'s empty-nsec skip fires for // every untouched agent and boot reconcile publishes zero 30179s. - super::storage::hydrate_keys(&mut records); + if let Some(store) = store { + super::storage::hydrate_keys_with(store, &mut records); + } let conn = open_retention_db(db_path).map_err(|e| format!("failed to open retention db: {e}"))?; diff --git a/desktop/src-tauri/src/managed_agents/reconcile/tests.rs b/desktop/src-tauri/src/managed_agents/reconcile/tests.rs index 55599ef249..43d0845b2f 100644 --- a/desktop/src-tauri/src/managed_agents/reconcile/tests.rs +++ b/desktop/src-tauri/src/managed_agents/reconcile/tests.rs @@ -195,6 +195,69 @@ fn missing_store_is_noop() { assert_eq!(reconcile_agents_in_dir(dir.path(), &keys).unwrap(), 0); } +/// Boot reconcile on a default `system-keyring` build reads the agent nsec +/// from the keyring, not the JSON. Hydration through the [`KeyStore`] seam +/// must fill it in so the first 30179 gets published; without it, the +/// empty-nsec skip fires and only the 30177 lands. The `None`-store control +/// pins the other side: no store, no 30179 — which is also why the plain +/// [`reconcile_agents_in_dir`] test helper can never hit the live OS keyring +/// (a macOS Keychain ACL prompt blocks a headless test binary forever). +#[test] +fn keyring_resident_nsec_is_hydrated_for_private_config() { + use crate::managed_agents::storage::{agent_keyring_name, tests::FakeKeyStore}; + use nostr::ToBech32; + + let owner_keys = nostr::Keys::generate(); + let agent_keys = nostr::Keys::generate(); + let pubkey = agent_keys.public_key().to_hex(); + let nsec = agent_keys.secret_key().to_bech32().unwrap(); + // JSON carries an empty nsec — keyring-resident, as on a default build. + let record = sample_record(&pubkey, "keyring-agent"); + assert!(record.private_key_nsec.is_empty()); + + // Control: no key store → empty-nsec skip → no 30179 head. + let dir = TempDir::new().unwrap(); + write_store(&dir, std::slice::from_ref(&record)); + let db_path = dir.path().join("retention.db"); + reconcile_agents_in_dir_with( + dir.path(), + &owner_keys, + &db_path, + None::<&crate::secret_store::SecretStore>, + ) + .unwrap(); + let conn = open_retention_db(&db_path).unwrap(); + assert!(get_retained_event( + &conn, + KIND_PRIVATE_MANAGED_AGENT, + &owner_keys.public_key().to_hex(), + &pubkey, + ) + .unwrap() + .is_none()); + drop(conn); + + // With the key in the (fake) keyring, hydration fills the nsec and the + // first 30179 is retained — and decrypts back to that exact key. + let dir = TempDir::new().unwrap(); + write_store(&dir, &[record]); + let db_path = dir.path().join("retention.db"); + let store = FakeKeyStore::reachable().with_key(&agent_keyring_name(&pubkey), &nsec); + reconcile_agents_in_dir_with(dir.path(), &owner_keys, &db_path, Some(&store)).unwrap(); + let conn = open_retention_db(&db_path).unwrap(); + let row = get_retained_event( + &conn, + KIND_PRIVATE_MANAGED_AGENT, + &owner_keys.public_key().to_hex(), + &pubkey, + ) + .unwrap() + .expect("hydrated nsec must publish the first 30179"); + let event = nostr::Event::from_json(&row.raw_event).unwrap(); + let (_, payload) = private_managed_agent::validate_and_decrypt(&event, &owner_keys).unwrap(); + assert_eq!(payload.identity.private_key_nsec, nsec); +} + #[test] fn fresh_record_is_retained_pending() { let dir = TempDir::new().unwrap(); diff --git a/desktop/src-tauri/src/managed_agents/storage.rs b/desktop/src-tauri/src/managed_agents/storage.rs index c789c59c54..5ad4552bd0 100644 --- a/desktop/src-tauri/src/managed_agents/storage.rs +++ b/desktop/src-tauri/src/managed_agents/storage.rs @@ -15,7 +15,7 @@ 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 { +pub(super) fn agent_keyring_name(pubkey: &str) -> String { format!("agent:{pubkey}") } @@ -24,7 +24,7 @@ fn agent_keyring_name(pubkey: &str) -> String { /// `SecretStore::shared` so identity and agent callers share one instance — /// and therefore one in-memory cache and one mutex — preventing last-writer-wins /// races on concurrent blob writes. -fn agent_secret_store() -> Option<&'static SecretStore> { +pub(super) fn agent_secret_store() -> Option<&'static SecretStore> { if cfg!(feature = "system-keyring") { Some(SecretStore::shared(keyring_service())) } else { @@ -131,7 +131,7 @@ fn newest_agent_log_in_dir(dir: &Path, pubkey: &str) -> Option { /// 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 { +pub(super) 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. @@ -317,7 +317,7 @@ pub(crate) fn hydrate_keys(records: &mut [ManagedAgentRecord]) { /// 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]) { +pub(super) fn hydrate_keys_with(store: &impl KeyStore, records: &mut [ManagedAgentRecord]) { for record in records.iter_mut() { // A key-less definition (no pubkey yet — unified agent model) has no // keyring entry by construction; keys are minted on first start. @@ -906,4 +906,4 @@ pub fn meaningful_agent_error_from_log(path: &Path) -> Option { #[cfg(test)] #[path = "storage_tests.rs"] -mod tests; +pub(super) mod tests; diff --git a/desktop/src-tauri/src/managed_agents/storage_tests.rs b/desktop/src-tauri/src/managed_agents/storage_tests.rs index 9943c6b3ac..b755b89686 100644 --- a/desktop/src-tauri/src/managed_agents/storage_tests.rs +++ b/desktop/src-tauri/src/managed_agents/storage_tests.rs @@ -19,7 +19,11 @@ use super::{ /// 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 { +/// +/// `pub(in crate::managed_agents)` so sibling test modules (e.g. +/// `reconcile::tests`) can inject it through the same seam instead of +/// touching the live keyring. +pub(in crate::managed_agents) struct FakeKeyStore { reachable: bool, fail_verify: bool, stored: RefCell>, @@ -28,7 +32,7 @@ struct FakeKeyStore { } impl FakeKeyStore { - fn reachable() -> Self { + pub(in crate::managed_agents) fn reachable() -> Self { Self { reachable: true, fail_verify: false, @@ -56,7 +60,7 @@ impl FakeKeyStore { } } /// Seed a key as already present in the keyring. - fn with_key(self, name: &str, value: &str) -> Self { + pub(in crate::managed_agents) fn with_key(self, name: &str, value: &str) -> Self { self.stored .borrow_mut() .insert(name.to_string(), value.to_string());