diff --git a/desktop/scripts/check-file-sizes.mjs b/desktop/scripts/check-file-sizes.mjs index a2bd3823b22..cd6081d4a5c 100644 --- a/desktop/scripts/check-file-sizes.mjs +++ b/desktop/scripts/check-file-sizes.mjs @@ -414,7 +414,11 @@ const overrides = new Map([ // +4 (1081 -> 1085): mesh recovery keeps one app-scoped state object beside // the embedded runtime and coordinator. Probe/re-arm logic lives in // mesh_llm/recovery.rs rather than growing AppState or command modules. - ["src-tauri/src/app_state.rs", 1085], + // +27 (1085 -> 1112): installed-canary keychain cancel-loop fix handles the + // Present-but-unreadable macOS keychain branch without rotating identity; + // review delta keeps corrupt fallback-file failures inside locked recovery. + // The dedicated regression tests live in app_state_keyring_read_failure_tests.rs. + ["src-tauri/src/app_state.rs", 1112], // multi-slot splitting + no-op suppression (#1309): the ReadStateManager // class grew from ~700 lines to ~1019 with the addition of // splitContextsIntoBudgetedSlots (pure fn + 5 tests), publishSplitSlots, diff --git a/desktop/src-tauri/src/app_state.rs b/desktop/src-tauri/src/app_state.rs index ab3fbb808c9..b65069de00d 100644 --- a/desktop/src-tauri/src/app_state.rs +++ b/desktop/src-tauri/src/app_state.rs @@ -473,7 +473,41 @@ fn resolve_identity_with_store( match store.probe(IDENTITY_KEY_NAME) { KeyringProbe::Present => { - if let Some(nsec) = store.load(IDENTITY_KEY_NAME)? { + let loaded = match store.load(IDENTITY_KEY_NAME) { + Ok(value) => value, + Err(error) => { + if legacy_path.exists() { + match load_key_file(legacy_path) { + Ok(keys) => { + eprintln!( + "buzz-desktop: keyring identity present but unreadable \ + ({error}); using identity.key fallback for this boot" + ); + return Ok(ResolvedIdentity { + keys, + recovery: RecoveryState::None, + }); + } + Err(file_error) => eprintln!( + "buzz-desktop: keyring identity present but unreadable \ + ({error}); identity.key fallback failed ({file_error})" + ), + } + } + let ephemeral = Keys::generate(); + eprintln!( + "buzz-desktop: keyring identity present but unreadable ({error}); \ + booting keyring-locked recovery with ephemeral key {} — \ + unlock the keyring and relaunch", + ephemeral.public_key().to_hex() + ); + return Ok(ResolvedIdentity { + keys: ephemeral, + recovery: RecoveryState::KeyringLocked, + }); + } + }; + if let Some(nsec) = loaded { match Keys::parse(nsec.trim()) { Ok(keyring_keys) => { eprintln!( @@ -1069,6 +1103,9 @@ pub(crate) fn save_key_file(path: &std::path::Path, keys: &Keys) -> Result<(), S .map_err(|e| format!("commit identity.key: {e}")) } +#[cfg(test)] +#[path = "app_state_keyring_read_failure_tests.rs"] +mod keyring_read_failure_tests; #[cfg(test)] #[path = "app_state_tests.rs"] mod tests; diff --git a/desktop/src-tauri/src/app_state_keyring_read_failure_tests.rs b/desktop/src-tauri/src/app_state_keyring_read_failure_tests.rs new file mode 100644 index 00000000000..defbaccb87a --- /dev/null +++ b/desktop/src-tauri/src/app_state_keyring_read_failure_tests.rs @@ -0,0 +1,122 @@ +use super::*; + +use std::cell::RefCell; +use std::collections::HashMap; + +use crate::secret_store::KeyringProbe; + +struct FailingLoadStore { + slot: RefCell>, + deleted: RefCell>, +} + +impl FailingLoadStore { + fn new() -> Self { + Self { + slot: RefCell::new(HashMap::new()), + deleted: RefCell::new(Vec::new()), + } + } +} + +impl IdentityKeyStore for FailingLoadStore { + fn probe(&self, _name: &str) -> KeyringProbe { + KeyringProbe::Present + } + + fn load(&self, _name: &str) -> Result, String> { + Err("simulated keyring read failure".to_string()) + } + + fn store(&self, name: &str, value: &str) -> Result<(), 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(()) + } + + fn verify_stored(&self, name: &str, expected: &str) -> Result { + Ok(self.slot.borrow().get(name).is_some_and(|v| v == expected)) + } +} + +#[test] +fn present_keyring_read_failure_boots_keyring_locked_recovery_without_rotating() { + let dir = tempfile::tempdir().unwrap(); + let legacy_path = dir.path().join("identity.key"); + write_migration_marker(&migration_marker_path(dir.path())).unwrap(); + + let store = FailingLoadStore::new(); + let resolved = resolve_identity_with_store(&store, &legacy_path, dir.path()).unwrap(); + + assert_eq!(resolved.recovery, RecoveryState::KeyringLocked); + assert!(!legacy_path.exists(), "ephemeral key must not be persisted"); + assert!( + store.slot.borrow().is_empty(), + "keyring must not be rewritten" + ); + assert!( + store.deleted.borrow().is_empty(), + "keyring entry must remain intact" + ); +} + +#[test] +fn present_keyring_read_failure_uses_legacy_file_when_no_marker_exists() { + 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(); + assert!(!migration_marker_path(dir.path()).exists()); + + let store = FailingLoadStore::new(); + let resolved = resolve_identity_with_store(&store, &legacy_path, dir.path()).unwrap(); + + assert_eq!( + file_keys.public_key().to_hex(), + resolved.keys.public_key().to_hex() + ); + assert_eq!(resolved.recovery, RecoveryState::None); + assert!( + legacy_path.exists(), + "fallback file must remain authoritative" + ); + assert!( + store.slot.borrow().is_empty(), + "keyring must not be rewritten" + ); + assert!( + store.deleted.borrow().is_empty(), + "keyring entry must remain intact" + ); +} + +#[test] +fn present_keyring_read_failure_keeps_corrupt_legacy_file_locked() { + let dir = tempfile::tempdir().unwrap(); + let legacy_path = dir.path().join("identity.key"); + std::fs::write(&legacy_path, "not-a-valid-nsec").unwrap(); + + let store = FailingLoadStore::new(); + let resolved = resolve_identity_with_store(&store, &legacy_path, dir.path()).unwrap(); + + assert_eq!(resolved.recovery, RecoveryState::KeyringLocked); + assert!( + legacy_path.exists(), + "corrupt fallback must be left untouched" + ); + assert!( + store.slot.borrow().is_empty(), + "keyring must not be rewritten" + ); + assert!( + store.deleted.borrow().is_empty(), + "keyring entry must remain intact" + ); +}