diff --git a/desktop/playwright.config.ts b/desktop/playwright.config.ts index 440d8fac5..6cf9962ae 100644 --- a/desktop/playwright.config.ts +++ b/desktop/playwright.config.ts @@ -84,6 +84,7 @@ export default defineConfig({ "**/drafts-screenshots.spec.ts", "**/buzz-theme-screenshots.spec.ts", "**/channel-sort.spec.ts", + "**/identity-lost.spec.ts", ], use: { ...devices["Desktop Chrome"], diff --git a/desktop/scripts/check-file-sizes.mjs b/desktop/scripts/check-file-sizes.mjs index 1215e3edb..c994e5837 100644 --- a/desktop/scripts/check-file-sizes.mjs +++ b/desktop/scripts/check-file-sizes.mjs @@ -133,7 +133,11 @@ const overrides = new Map([ // baked-env-required-badge: getBakedBuildEnvKeys wrapper adds ~16 lines. Queued to split. // restart-badge: started the queued split — start/stopManagedAgent moved to // tauriManagedAgents.ts; limit ratcheted down 1388 → 1380 to bank the headroom. - ["src/shared/api/tauri.ts", 1380], + // identity-import-keyring: identity wrappers (RawIdentity, getIdentity, getNsec, + // importIdentity, persistCurrentIdentity) moved to tauriIdentity.ts; + // limit ratcheted down 1380 → 1360 to bank the headroom (absorbs main-side + // growth landed between the split and the rebase). + ["src/shared/api/tauri.ts", 1360], // readiness-gate: PersonaDialog.tsx threads computeLocalModeGate + // requiredCredentialEnvKeys + RequiredFieldLabel so the "New agent" dialog // shows required markers and credential amber rows (parity with @@ -160,6 +164,12 @@ const overrides = new Map([ // unified-agent-model 1A.1: inline test module moved to discovery/tests.rs, // ratcheting 1259 -> 802 (under the 1000 default; entry kept as a ratchet). ["src-tauri/src/managed_agents/discovery.rs", 802], + // identity-import-keyring: the identity resolution state machine's behavioral + // matrix (46 tests over FakeIdentityStore — probe × marker × file cells, + // adoption / read-back-corruption / marker-failure arms, recovery-mode + // gating). Load-bearing regression coverage for silent identity rotation, + // not generic debt growth. Approved override; split if the matrix grows. + ["src-tauri/src/app_state_tests.rs", 1290], // 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 @@ -198,7 +208,6 @@ const overrides = new Map([ // uid-keyed lockfile path + behavioral tests add ~303 lines. Load-bearing // security fix for the lost-update race that stranded agent keys. ["src-tauri/src/secret_store.rs", 1043], - ["src-tauri/src/app_state.rs", 1033], // 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/build.rs b/desktop/src-tauri/build.rs index 022ef4768..c4d7b71f4 100644 --- a/desktop/src-tauri/build.rs +++ b/desktop/src-tauri/build.rs @@ -102,5 +102,20 @@ fn main() { println!("cargo:rustc-cfg=buzz_updater_enabled"); } + // Cargo test executables get no embedded Windows manifest (tauri_build + // attaches one to bin targets only), so the loader binds comctl32 v5, which + // lacks TaskDialogIndirect (statically imported via tauri-plugin-dialog/rfd) + // and debug test exes die at load with STATUS_ENTRYPOINT_NOT_FOUND. Declaring + // the Common Controls v6 dependency makes link.exe emit a side-by-side + // .manifest that the loader honors for manifest-less executables; + // binaries with an embedded manifest (the real app) ignore it. + if std::env::var("CARGO_CFG_TARGET_OS").as_deref() == Ok("windows") + && std::env::var("CARGO_CFG_TARGET_ENV").as_deref() == Ok("msvc") + { + println!( + "cargo:rustc-link-arg=/MANIFESTDEPENDENCY:type='win32' name='Microsoft.Windows.Common-Controls' version='6.0.0.0' processorArchitecture='*' publicKeyToken='6595b64144ccf1df' language='*'" + ); + } + tauri_build::build() } diff --git a/desktop/src-tauri/src/app_state.rs b/desktop/src-tauri/src/app_state.rs index dca627fe6..6cc3a29dc 100644 --- a/desktop/src-tauri/src/app_state.rs +++ b/desktop/src-tauri/src/app_state.rs @@ -1,7 +1,10 @@ use std::{ collections::HashMap, io::Write, - sync::{atomic::AtomicU16, Arc, Mutex}, + sync::{ + atomic::{AtomicBool, AtomicU16}, + Arc, Mutex, + }, }; use nostr::{Keys, ToBech32}; @@ -33,6 +36,38 @@ pub struct AppState { pub audio_output_device: Mutex>, /// Port of the localhost media streaming proxy (set during setup). pub media_proxy_port: AtomicU16, + /// Set when identity resolution detected a "keyring-locked" state: the + /// keyring is unreachable this boot but a migration marker shows the key + /// lives there. An ephemeral key is generated so the app can open; all + /// signing commands check this flag via [`AppState::signing_keys`] and + /// return `Err` so no events are published under the inaccessible identity. + /// Mutually exclusive with `identity_lost` (guaranteed by `RecoveryState` + /// at the resolve boundary). + /// + /// Ordering: writers store with `Ordering::Release` after `state.keys` is + /// updated, so a reader observing `false` with `Ordering::Acquire` is + /// guaranteed to see the updated keys. Writers: `setup()` (initial + /// resolution via `resolve_persisted_identity`) and `import_identity` + /// (clears the flag when the user successfully imports a new key). + pub keyring_locked: AtomicBool, + /// Set when identity resolution detected a "lost" state: the migration + /// marker was present but the keyring was empty and no plaintext fallback + /// existed. An ephemeral key was generated to let the app boot; the + /// frontend checks this flag via `get_identity` and routes to the nsec + /// re-import step instead of the normal onboarding profile flow. + /// + /// Ordering: writers store with `Ordering::Release` after `state.keys` is + /// updated, so a reader observing `false` with `Ordering::Acquire` is + /// guaranteed to see the updated keys. Writers: `setup()` (initial + /// resolution) and `import_identity`/`persist_current_identity` + /// (user-initiated key import). + pub identity_lost: AtomicBool, + /// Serializes runtime identity mutations (`import_identity` and + /// `persist_current_identity`) so a stale ephemeral key can never overwrite + /// a newer imported key during concurrent calls. Deliberately separate from + /// `keys` so readers (signing, get_identity, etc.) are not blocked during + /// keyring I/O. + pub identity_mutation: Mutex<()>, /// Cached ACP session config from running agents, keyed by agent pubkey. /// Populated when the harness emits `session_config_captured` observer events. pub session_config_cache: Mutex>, @@ -93,6 +128,7 @@ pub fn build_app_state() -> AppState { .build() .unwrap_or_else(|_| reqwest::Client::new()), relay_url_override: Mutex::new(None), + identity_mutation: Mutex::new(()), managed_agents_store_lock: Mutex::new(()), channel_templates_store_lock: Mutex::new(()), managed_agent_processes: Mutex::new(HashMap::new()), @@ -104,6 +140,8 @@ pub fn build_app_state() -> AppState { prevent_sleep: Arc::new(Mutex::new( crate::prevent_sleep::PreventSleepState::default(), )), + keyring_locked: AtomicBool::new(false), + identity_lost: AtomicBool::new(false), #[cfg(feature = "mesh-llm")] mesh_llm_runtime: AsyncMutex::new(None), #[cfg(feature = "mesh-llm")] @@ -137,6 +175,32 @@ impl AppState { } } + /// Return the active identity keys if they are in a signable state. + /// + /// Returns `Err` when the identity is in a lost state (`identity_lost` + /// — ephemeral key, user must re-import their nsec) or when the keyring + /// is locked (`keyring_locked` — key is held in a keyring that is + /// unavailable this boot). All signing and publish commands must call + /// this instead of locking `state.keys` directly, so that recovery mode + /// blocks publishing under an invalid or inaccessible identity. + pub fn signing_keys(&self) -> Result { + if self + .identity_lost + .load(std::sync::atomic::Ordering::Acquire) + || self + .keyring_locked + .load(std::sync::atomic::Ordering::Acquire) + { + return Err("identity is in recovery mode; event signing is disabled \ + until the identity is restored and Buzz is relaunched" + .to_string()); + } + self.keys + .lock() + .map_err(|e| e.to_string()) + .map(|k| k.clone()) + } + /// Emit the current huddle state to the frontend via Tauri event. /// /// Acquires both locks (app_handle + huddle_state), clones a snapshot, @@ -156,13 +220,21 @@ impl AppState { } } -/// Resolve the user's identity key from the app data directory. +/// Resolve the user's identity key from the app data directory and wire +/// the resulting [`RecoveryState`] into `AppState`. /// /// Priority: `BUZZ_PRIVATE_KEY` env var (already handled in `build_app_state`) -/// → `{app_data_dir}/identity.key` file → generate + save. +/// → keyring → `{app_data_dir}/identity.key` file → generate + save. /// -/// Writes use `atomic-write-file` which handles temp file creation, fsync, -/// atomic rename, and directory sync — no partial or corrupt files on disk. +/// On success, writes the resolved keys into `state.keys` (with the mutex) +/// before storing the recovery flags (Release), so any thread that reads +/// either flag as `false` with Acquire is guaranteed to see the updated keys. +/// +/// Sets `state.identity_lost` on `RecoveryState::Lost` (keyring empty after +/// migration — key gone externally) and `state.keyring_locked` on +/// `RecoveryState::KeyringLocked` (keyring unreachable — key still in keyring +/// but inaccessible this boot). Both states boot with an ephemeral key; the +/// frontend shows different recovery screens for each. pub fn resolve_persisted_identity(app: &AppHandle, state: &AppState) -> Result<(), String> { // Only skip file-based resolution if the env var was present AND parsed // successfully. A malformed env var should fall through to the persisted @@ -177,8 +249,18 @@ pub fn resolve_persisted_identity(app: &AppHandle, state: &AppState) -> Result<( .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 keys = load_or_create_identity(&data_dir)?; - *state.keys.lock().map_err(|e| e.to_string())? = keys; + let resolved = load_or_create_identity(&data_dir)?; + // Write keys before setting the recovery flags (Release) so any thread + // that reads a flag as false with Acquire is guaranteed to see the keys. + *state.keys.lock().map_err(|e| e.to_string())? = resolved.keys; + state.identity_lost.store( + resolved.recovery == RecoveryState::Lost, + std::sync::atomic::Ordering::Release, + ); + state.keyring_locked.store( + resolved.recovery == RecoveryState::KeyringLocked, + std::sync::atomic::Ordering::Release, + ); Ok(()) } @@ -196,6 +278,26 @@ const IDENTITY_KEY_NAME: &str = "identity"; /// keyring is merely unreachable (the key IS in the keyring, must NOT generate). const MIGRATION_MARKER_NAME: &str = "identity.migrated"; +/// Recovery state produced by identity resolution. `None` means the app has +/// a real, usable identity. `Lost` means the keyring was reachable-but-empty +/// despite a prior successful migration — the key vanished externally. `KeyringLocked` +/// means the keyring is unreachable this boot but was used in the past +/// (marker present, no file) — the key still exists but is temporarily +/// inaccessible. Both non-`None` variants boot with an ephemeral key; the +/// frontend shows a different recovery screen for each. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +enum RecoveryState { + None, + Lost, + KeyringLocked, +} + +/// The output of identity resolution. +struct ResolvedIdentity { + keys: Keys, + recovery: RecoveryState, +} + /// 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. @@ -230,12 +332,16 @@ impl IdentityKeyStore for crate::secret_store::SecretStore { /// 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 { +fn load_or_create_identity(data_dir: &std::path::Path) -> Result { 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 keys = load_file_or_generate(&legacy_path, data_dir)?; + return Ok(ResolvedIdentity { + keys, + recovery: RecoveryState::None, + }); } let store = crate::secret_store::SecretStore::shared(KEYRING_SERVICE); @@ -249,49 +355,143 @@ fn resolve_identity_with_store( store: &impl IdentityKeyStore, legacy_path: &std::path::Path, data_dir: &std::path::Path, -) -> Result { +) -> Result { 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) => { + Ok(keyring_keys) => { eprintln!( "buzz-desktop: persisted identity pubkey {}", - keys.public_key().to_hex() + keyring_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); + // Check for a leftover identity.key. If it holds a + // DIFFERENT pubkey, the user imported that key after + // the last boot (pre-fix, import only wrote the file). + // Adopt it into the keyring so the user's intent sticks. + // If the pubkeys match it is a stale leftover from a + // prior migration whose remove_file failed — clean it up. + if legacy_path.exists() { + match load_key_file(legacy_path) { + Ok(file_keys) + if file_keys.public_key() != keyring_keys.public_key() => + { + eprintln!( + "buzz-desktop: identity.key differs from keyring; \ + adopting imported key {}", + file_keys.public_key().to_hex() + ); + // Delegate the store→read-back-verify→marker→delete + // sequence to `persist_identity_to_keyring`, which owns + // the marker-before-delete invariant and the fallback + // logic that keeps identity.key when the marker write + // fails. A transient keyring failure must not abort + // boot — the file key is safe and adoption retries next + // boot when the keyring is reachable again. + if let Err(e) = persist_identity_to_keyring( + store, + &file_keys, + legacy_path, + data_dir, + ) { + eprintln!( + "buzz-desktop: keyring adoption of identity.key \ + failed ({e}); using file key, will retry next boot" + ); + } + return Ok(ResolvedIdentity { + keys: file_keys, + recovery: RecoveryState::None, + }); + } + // Corrupt file — keyring is authoritative. Log before + // cleanup so there is a diagnostic for the lost data. + Err(e) => { + eprintln!( + "buzz-desktop: leftover identity.key is corrupt ({e}); \ + keyring is authoritative, removing" + ); + ensure_marker_then_cleanup(data_dir, legacy_path); + } + // Same pubkey (stale leftover from a completed migration + // whose remove_file previously failed) — keyring is + // authoritative. Ensure the marker exists (crash-safe + // ordering: marker before delete), then clean up. + Ok(_) => { + ensure_marker_then_cleanup(data_dir, legacy_path); + } + } + } + // Self-heal: if the identity.key is gone and the migration + // marker is absent (e.g. a stranded keyring-only install from + // a pre-fix path that stored to the keyring but could not write + // the marker or fallback file), write the marker now so a later + // keyring-Unreachable boot does not treat this as a fresh install + // and silently rotate the identity. Failure is non-fatal — boot + // must never be blocked here. + if !legacy_path.exists() && !migration_marker_path(data_dir).exists() { + if let Err(e) = write_migration_marker(&migration_marker_path(data_dir)) + { + eprintln!( + "buzz-desktop: keyring present but marker missing; \ + self-heal marker write failed ({e}), continuing" + ); + } + } + return Ok(ResolvedIdentity { + keys: keyring_keys, + recovery: RecoveryState::None, + }); } // 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(), - ); + let keys = + recover_from_keyring(store, legacy_path, data_dir, &error.to_string())?; + return Ok(ResolvedIdentity { + keys, + recovery: RecoveryState::None, + }); } } + } else { + // Probe said Present but load found nothing — treat as empty. + // Falls through to generate_and_persist below. } - // 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); + return Ok(ResolvedIdentity { + keys, + recovery: RecoveryState::None, + }); } + } else if migration_marker_path(data_dir).exists() { + // Marker present, keyring empty, no file — the key was previously + // durably stored in the keyring but is now gone (keyring cleared, + // new login session, or the entry was externally deleted). There + // is no plaintext fallback to recover from. + // + // Generate an ephemeral in-memory key so the app can boot, but + // surface a "lost" flag so the frontend prompts re-import rather + // than silently starting a fresh identity. + let ephemeral = Keys::generate(); + eprintln!( + "buzz-desktop: identity lost — keyring was empty despite migration marker; \ + using ephemeral key {}, awaiting user re-import", + ephemeral.public_key().to_hex() + ); + return Ok(ResolvedIdentity { + keys: ephemeral, + recovery: RecoveryState::Lost, + }); } } KeyringProbe::Unreachable => { @@ -300,22 +500,40 @@ fn resolve_identity_with_store( // 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. + // file deleted. The real key is unreachable this boot but still + // exists in the keyring. Boot keyring-locked recovery (ephemeral + // key, all signing disabled) so the app can at least open; the + // frontend shows a "unlock the keyring and relaunch" screen. + // Fail-closed semantics are preserved: nothing is ever persisted + // under the ephemeral key, so no silent identity rotation occurs. // - 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(), + let ephemeral = Keys::generate(); + eprintln!( + "buzz-desktop: keyring unreachable but migration marker present; \ + 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, + }); } - return load_file_or_generate(legacy_path, data_dir); + let keys = load_file_or_generate(legacy_path, data_dir)?; + return Ok(ResolvedIdentity { + keys, + recovery: RecoveryState::None, + }); } } - generate_and_persist(store, legacy_path, data_dir) + let keys = generate_and_persist(store, legacy_path, data_dir)?; + Ok(ResolvedIdentity { + keys, + recovery: RecoveryState::None, + }) } /// Recover from a corrupt nsec in the keyring (parse failed). Clear the bad @@ -416,6 +634,110 @@ fn migrate_identity_file( } } +/// Persist `keys` into the keyring with read-back verification, write the +/// migration marker, and delete any leftover `identity.key`. Returns `Ok` on +/// success. Returns `Err` when the keyring write fails (availability error) — +/// the caller must fall back to `save_key_file` so the key survives the boot. +/// +/// This is the shared kernel used by both one-time file migration and the +/// `import_identity` command. Crash-safe ordering: marker is written BEFORE +/// deleting the file. +fn persist_identity_to_keyring( + store: &impl IdentityKeyStore, + keys: &Keys, + legacy_path: &std::path::Path, + data_dir: &std::path::Path, +) -> Result<(), String> { + let nsec = keys + .secret_key() + .to_bech32() + .map_err(|e| format!("encode nsec: {e}"))?; + + // Will error if the keyring is unavailable — caller falls back to the file. + store.store(IDENTITY_KEY_NAME, &nsec)?; + + // Read-back verify before touching durable state. + match store.load(IDENTITY_KEY_NAME)? { + Some(stored) if stored == nsec => {} + _ => return Err("keyring read-back verify failed".to_string()), + } + + // Write marker before deleting the file (crash-safe ordering). + let marker_path = migration_marker_path(data_dir); + if let Err(e) = write_migration_marker(&marker_path) { + // Keyring holds the key but no marker exists. Preserve the invariant + // "keyring-only implies marker exists" by ensuring identity.key is + // present as a fallback: write it if absent, leave it if already there. + // This prevents a later keyring-unreachable + no-marker boot from + // treating this as a fresh install and silently rotating identity. + if !legacy_path.exists() { + if let Err(write_err) = save_key_file(legacy_path, keys) { + eprintln!( + "buzz-desktop: keyring ok but marker write failed ({e}) and \ + identity.key write also failed ({write_err}); key may be unrecoverable" + ); + return Err(format!( + "keyring ok but neither migration marker nor identity.key fallback \ + could be written (marker: {e}; file: {write_err}); \ + identity must not be treated as durably persisted — retry the import" + )); + } else { + eprintln!( + "buzz-desktop: keyring ok but marker write failed ({e}); \ + wrote identity.key as fallback so the key is not stranded" + ); + } + } else { + eprintln!( + "buzz-desktop: keyring ok but marker write failed ({e}); \ + keeping existing identity.key so the key is not stranded" + ); + } + return Ok(()); + } + + if legacy_path.exists() { + if let Err(e) = std::fs::remove_file(legacy_path) { + eprintln!("buzz-desktop: keyring write ok but failed to delete identity.key: {e}"); + } + } + + Ok(()) +} + +/// Core implementation of imported-identity persistence. Tries the OS keyring +/// first via [`persist_identity_to_keyring`]; if the keyring is unavailable, +/// falls back to the `0o600` identity.key file. Returns `Err` only when both +/// the keyring write and the file fallback fail. +fn persist_imported_identity_impl( + store: &impl IdentityKeyStore, + keys: &Keys, + legacy_path: &std::path::Path, + data_dir: &std::path::Path, +) -> Result<(), String> { + match persist_identity_to_keyring(store, keys, legacy_path, data_dir) { + Ok(()) => Ok(()), + Err(e) => { + eprintln!( + "buzz-desktop: keyring write failed during import ({e}), \ + falling back to identity.key" + ); + save_key_file(legacy_path, keys) + } + } +} + +/// Public entry point binding [`persist_imported_identity_impl`] to the shared +/// [`crate::secret_store::SecretStore`]. See the impl for the persistence policy. +pub(crate) fn persist_imported_identity( + store: &crate::secret_store::SecretStore, + keys: &Keys, + legacy_path: &std::path::Path, + data_dir: &std::path::Path, +) -> Result<(), String> { + persist_imported_identity_impl(store, keys, legacy_path, data_dir) +} + /// 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) @@ -437,10 +759,10 @@ fn write_migration_marker(marker_path: &std::path::Path) -> Result<(), String> { .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. +/// Which backend [`store_key_preferring_keyring`] 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, @@ -459,7 +781,7 @@ fn generate_and_persist( data_dir: &std::path::Path, ) -> Result { let keys = Keys::generate(); - if let PersistBackend::Keyring = persist_identity(store, &keys, legacy_path)? { + if let PersistBackend::Keyring = store_key_preferring_keyring(store, &keys, legacy_path)? { let marker_path = migration_marker_path(data_dir); if let Err(e) = write_migration_marker(&marker_path) { eprintln!( @@ -476,10 +798,12 @@ fn generate_and_persist( 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( +/// Persist `keys` through the store, silently falling back to the `0o600` file +/// when the keyring write fails on an availability error. Reports which backend +/// held the key (no verify/marker/delete — those belong to callers that own the +/// full migration contract) so the caller can write the migration marker only on +/// keyring success. +fn store_key_preferring_keyring( store: &impl IdentityKeyStore, keys: &Keys, legacy_path: &std::path::Path, @@ -498,6 +822,28 @@ fn persist_identity( } } +/// Ensure the migration marker exists (writing it if absent), then remove the +/// leftover `identity.key`. Crash-safe ordering: the marker is written and +/// fsync-committed before the file is deleted, so a crash between the two +/// leaves the marker on disk and the file intact — the invariant "keyring-only +/// implies marker exists" is preserved. If the marker write fails, the file is +/// kept so a later keyring-unreachable boot can use it as a fallback. +fn ensure_marker_then_cleanup(data_dir: &std::path::Path, legacy_path: &std::path::Path) { + let marker_path = migration_marker_path(data_dir); + let marker_ok = marker_path.exists() + || write_migration_marker(&marker_path) + .map_err(|e| { + eprintln!( + "buzz-desktop: keyring present but marker missing; \ + failed to write marker ({e}), keeping identity.key" + ); + }) + .is_ok(); + if marker_ok { + cleanup_leftover_identity_file(legacy_path); + } +} + /// 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. @@ -573,460 +919,5 @@ pub(crate) fn save_key_file(path: &std::path::Path, keys: &Keys) -> Result<(), S } #[cfg(test)] -mod tests { - use super::*; - - fn assert_key_eq(a: &Keys, b: &Keys) { - 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(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(); - let path = dir.path().join("identity.key"); - let keys = Keys::generate(); - - save_key_file(&path, &keys).unwrap(); - let loaded = load_key_file(&path).unwrap(); - assert_key_eq(&keys, &loaded); - } - - #[test] - fn load_rejects_empty_file() { - let dir = tempfile::tempdir().unwrap(); - let path = dir.path().join("identity.key"); - std::fs::write(&path, "").unwrap(); - - assert!(load_key_file(&path).is_err()); - } - - #[test] - fn load_rejects_corrupt_content() { - let dir = tempfile::tempdir().unwrap(); - let path = dir.path().join("identity.key"); - std::fs::write(&path, "not-a-valid-nsec").unwrap(); - - assert!(load_key_file(&path).is_err()); - } - - #[test] - fn load_missing_file_is_err() { - let dir = tempfile::tempdir().unwrap(); - let path = dir.path().join("nonexistent.key"); - - 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(); - let path = dir.path().join("identity.key"); - let keys = Keys::generate(); - - save_key_file(&path, &keys).unwrap(); - - let content = std::fs::read_to_string(&path).unwrap(); - assert!(content.starts_with("nsec1")); - } - - #[cfg(unix)] - #[test] - fn save_creates_file_with_restricted_permissions() { - use std::os::unix::fs::PermissionsExt; - - let dir = tempfile::tempdir().unwrap(); - let path = dir.path().join("identity.key"); - let keys = Keys::generate(); - - save_key_file(&path, &keys).unwrap(); - - let perms = std::fs::metadata(&path).unwrap().permissions(); - assert_eq!(perms.mode() & 0o777, 0o600); - } - - #[test] - fn save_overwrites_existing_file() { - let dir = tempfile::tempdir().unwrap(); - let path = dir.path().join("identity.key"); - - let keys1 = Keys::generate(); - save_key_file(&path, &keys1).unwrap(); - - let keys2 = Keys::generate(); - save_key_file(&path, &keys2).unwrap(); - - 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>, - deleted: RefCell>, - /// 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, 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()); - } -} +#[path = "app_state_tests.rs"] +mod tests; diff --git a/desktop/src-tauri/src/app_state_tests.rs b/desktop/src-tauri/src/app_state_tests.rs new file mode 100644 index 000000000..50a9859b5 --- /dev/null +++ b/desktop/src-tauri/src/app_state_tests.rs @@ -0,0 +1,1284 @@ +use super::*; + +fn assert_key_eq(a: &Keys, b: &Keys) { + 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(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(); + let path = dir.path().join("identity.key"); + let keys = Keys::generate(); + + save_key_file(&path, &keys).unwrap(); + let loaded = load_key_file(&path).unwrap(); + assert_key_eq(&keys, &loaded); +} + +#[test] +fn load_rejects_empty_file() { + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("identity.key"); + std::fs::write(&path, "").unwrap(); + + assert!(load_key_file(&path).is_err()); +} + +#[test] +fn load_rejects_corrupt_content() { + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("identity.key"); + std::fs::write(&path, "not-a-valid-nsec").unwrap(); + + assert!(load_key_file(&path).is_err()); +} + +#[test] +fn load_missing_file_is_err() { + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("nonexistent.key"); + + 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(); + let path = dir.path().join("identity.key"); + let keys = Keys::generate(); + + save_key_file(&path, &keys).unwrap(); + + let content = std::fs::read_to_string(&path).unwrap(); + assert!(content.starts_with("nsec1")); +} + +#[cfg(unix)] +#[test] +fn save_creates_file_with_restricted_permissions() { + use std::os::unix::fs::PermissionsExt; + + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("identity.key"); + let keys = Keys::generate(); + + save_key_file(&path, &keys).unwrap(); + + let perms = std::fs::metadata(&path).unwrap().permissions(); + assert_eq!(perms.mode() & 0o777, 0o600); +} + +#[test] +fn save_overwrites_existing_file() { + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("identity.key"); + + let keys1 = Keys::generate(); + save_key_file(&path, &keys1).unwrap(); + + let keys2 = Keys::generate(); + save_key_file(&path, &keys2).unwrap(); + + 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>, + deleted: RefCell>, + /// When true, `store` returns an availability error, driving the + /// keyring-write-failure → file-fallback arm of `store_key_preferring_keyring`. + store_fails: bool, + /// When `Some`, `load()` always returns this value regardless of what was + /// stored. Used to simulate read-back corruption: `store()` succeeds but + /// the subsequent `load()` returns a different value, causing + /// `persist_identity_to_keyring`'s read-back verify to fail. + load_override: Option, +} + +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, + load_override: None, + } + } + + /// 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, + load_override: None, + } + } + + /// 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, + load_override: None, + } + } + + /// Present probe seeded with a value but whose `store` always fails — + /// exercises the keyring-write-failure arm of adoption and import paths. + fn present_with_store_failing(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: true, + load_override: None, + } + } + + /// 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, + load_override: None, + } + } + + /// Reachable-but-empty probe whose `store` succeeds but whose `load` + /// always returns `corrupt_nsec` — simulates keyring read-back corruption. + /// `persist_identity_to_keyring`'s read-back verify sees a mismatch and + /// returns `Err("keyring read-back verify failed")`. + fn with_readback_corruption(corrupt_nsec: &str) -> Self { + Self { + probe: KeyringProbe::ReachableButEmpty, + slot: RefCell::new(HashMap::new()), + deleted: RefCell::new(Vec::new()), + store_fails: false, + load_override: Some(corrupt_nsec.to_string()), + } + } +} + +impl IdentityKeyStore for FakeIdentityStore { + fn probe(&self, _name: &str) -> KeyringProbe { + self.probe + } + fn load(&self, name: &str) -> Result, String> { + if let Some(v) = &self.load_override { + return Ok(Some(v.clone())); + } + 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.keys); + // 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.keys.secret_key().to_bech32().unwrap().as_str()) + ); +} + +#[test] +fn valid_keyring_is_used_and_matching_leftover_file_cleaned_up() { + // A valid keyring entry and a leftover identity.key with the SAME pubkey + // (stale leftover from a migration whose remove_file previously failed): + // keyring wins, plaintext is removed without adoption. + 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"); + // Same key in file as keyring → stale leftover, not an import. + save_key_file(&legacy_path, &keyring_keys).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.keys); + assert_eq!(resolved.recovery, RecoveryState::None); + assert!(store.deleted.borrow().is_empty()); + assert!(!legacy_path.exists()); +} + +#[test] +fn unreachable_post_migration_boots_keyring_locked_recovery() { + // After a migration the file is gone and the marker exists. A later boot + // with the keyring unreachable must NOT generate a fresh key (that would + // silently rotate the identity), but must also allow the app to open + // instead of hard-aborting. The result is a keyring-locked recovery boot: + // ephemeral key held in memory only, nothing persisted anywhere. + // + // Fail-closed semantics are preserved: no identity is ever written to disk + // or the keyring under the ephemeral key, so no silent rotation occurs. + // The abort is replaced by a graceful recovery screen. + 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 resolved = resolve_identity_with_store(&store, &legacy_path, dir.path()).unwrap(); + + // KeyringLocked recovery: ephemeral key returned, nothing persisted. + assert_eq!(resolved.recovery, RecoveryState::KeyringLocked); + // No identity.key was written. + assert!(!legacy_path.exists()); + // Keyring store was never called (it is unreachable). + assert!(store.slot.borrow().is_empty()); + assert!(store.deleted.borrow().is_empty()); +} + +#[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.keys, &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.keys); + // 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.keys.secret_key().to_bech32().unwrap().as_str()) + ); +} + +#[test] +fn fresh_keyring_generate_then_unreachable_boots_locked_recovery() { + // 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 NOT generate a new key and rotate identity. Instead it boots + // keyring-locked recovery — the real key is still in the keyring. + 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 → locked recovery. + let unreachable = FakeIdentityStore::unreachable(); + let resolved = resolve_identity_with_store(&unreachable, &legacy_path, dir.path()).unwrap(); + + assert_eq!( + resolved.recovery, + RecoveryState::KeyringLocked, + "second boot must boot keyring-locked, not generate a fresh key" + ); + // No identity.key was written — nothing new persisted. + 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.keys, &from_file); + // No marker: the file is the authoritative store, not the keyring. + assert!(!migration_marker_path(dir.path()).exists()); +} + +// ── New tests for the three defects fixed in this PR ───────────────────── + +#[test] +fn import_persists_to_keyring_reboot_resolves_imported_pubkey() { + // (a) import persists to keyring → simulated reboot resolves the + // imported pubkey. + // + // `persist_identity_to_keyring` is the kernel called by + // `import_identity`. After it succeeds the keyring slot holds the + // imported nsec. A fresh store seeded with that nsec (simulating a + // reboot where the keyring has the value) must resolve to the same key. + let dir = tempfile::tempdir().unwrap(); + let legacy_path = dir.path().join("identity.key"); + let imported_keys = Keys::generate(); + + // Simulate what import_identity does: persist to keyring. + let store_import = FakeIdentityStore::reachable_but_empty(); + persist_identity_to_keyring(&store_import, &imported_keys, &legacy_path, dir.path()) + .expect("persist_identity_to_keyring must succeed with a reachable store"); + + // Keyring slot now holds the imported nsec. + let stored_nsec = store_import + .slot + .borrow() + .get(IDENTITY_KEY_NAME) + .cloned() + .expect("keyring must hold the imported nsec after persist"); + assert_eq!(stored_nsec, imported_keys.secret_key().to_bech32().unwrap()); + + // Simulated reboot: new store with Present probe, seeded with the stored nsec. + let store_reboot = FakeIdentityStore::present_with(&stored_nsec); + let resolved = resolve_identity_with_store(&store_reboot, &legacy_path, dir.path()).unwrap(); + + // The resolved key is the imported one — identity survives the reboot. + assert_key_eq(&imported_keys, &resolved.keys); + assert_eq!(resolved.recovery, RecoveryState::None); + // No identity.key left on disk (was deleted by persist_identity_to_keyring). + assert!(!legacy_path.exists()); +} + +#[test] +fn present_keyring_with_mismatched_file_adopts_file_key() { + // (b) Present + mismatched identity.key → file's key adopted into + // keyring, no data loss, file removed. + // + // This auto-heals installs already stuck in the re-onboarding loop: + // the keyring holds the shadow key generated at first launch, while + // identity.key holds the user's imported key from a subsequent import + // that only reached the file (pre-fix bug). Resolution must adopt the + // file's key as the user's explicit intent. + let dir = tempfile::tempdir().unwrap(); + let legacy_path = dir.path().join("identity.key"); + + let keyring_keys = Keys::generate(); + let keyring_nsec = keyring_keys.secret_key().to_bech32().unwrap(); + + // identity.key has a DIFFERENT key — the user's import. + let file_keys = Keys::generate(); + save_key_file(&legacy_path, &file_keys).unwrap(); + + let store = FakeIdentityStore::present_with(&keyring_nsec); + let resolved = resolve_identity_with_store(&store, &legacy_path, dir.path()).unwrap(); + + // The file's key (user's explicit import) wins. + assert_key_eq(&file_keys, &resolved.keys); + assert_eq!(resolved.recovery, RecoveryState::None); + + // The keyring now holds the file's key (overwritten with read-back verify). + 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()) + ); + + // identity.key was removed after adoption. + assert!(!legacy_path.exists()); + + // Migration marker was written before file removal (crash-safe ordering). + // Without the marker, a later keyring-unreachable boot would see no file + // and no marker and silently generate a fresh key. + let marker_path = migration_marker_path(dir.path()); + assert!( + marker_path.exists(), + "migration marker must exist after mismatched-file adoption" + ); +} + +#[test] +fn present_keyring_mismatched_file_adoption_store_failure_boots_with_file_key() { + // Present + mismatched identity.key + keyring write fails during adoption. + // Boot must succeed with the FILE's key (the user's intent). The file must + // survive on disk because the write was rejected — adoption retries on the + // next boot when the keyring is reachable. The keyring slot must be + // unchanged (shadow nsec still present, not overwritten). + let dir = tempfile::tempdir().unwrap(); + let legacy_path = dir.path().join("identity.key"); + + let keyring_keys = Keys::generate(); + let keyring_nsec = keyring_keys.secret_key().to_bech32().unwrap(); + + // identity.key has a DIFFERENT key — the user's import. + let file_keys = Keys::generate(); + save_key_file(&legacy_path, &file_keys).unwrap(); + + let store = FakeIdentityStore::present_with_store_failing(&keyring_nsec); + let resolved = resolve_identity_with_store(&store, &legacy_path, dir.path()).unwrap(); + + // File key (user's explicit import) is returned. + assert_key_eq(&file_keys, &resolved.keys); + assert_eq!(resolved.recovery, RecoveryState::None); + + // identity.key must survive — adoption write failed, so it is the only + // durable copy of the imported key until the next-boot retry. + assert!( + legacy_path.exists(), + "identity.key must be kept when keyring adoption write fails" + ); + + // Keyring slot unchanged — write was rejected, no overwrite occurred. + assert_eq!( + store + .slot + .borrow() + .get(IDENTITY_KEY_NAME) + .map(String::as_str), + Some(keyring_nsec.as_str()), + "keyring slot must be unchanged when adoption write fails" + ); +} + +// read-only-dir marker-failure injection is Unix-only: on Windows, +// FILE_ATTRIBUTE_READONLY on a directory does not prevent creating new +// files inside it (it only guards the directory entry itself), so the +// marker write succeeds and the fault cannot be injected this way. +#[cfg(unix)] +#[test] +fn present_keyring_with_mismatched_file_adopts_file_key_marker_failure_keeps_file() { + // (b-fault) Present + mismatched identity.key + marker write fails → + // file MUST NOT be deleted so a later keyring-unreachable boot has a + // fallback. Invariant: keyring-only implies marker exists; if marker + // cannot be written, identity.key is the surviving discriminator. + let dir = tempfile::tempdir().unwrap(); + let legacy_path = dir.path().join("identity.key"); + + let keyring_keys = Keys::generate(); + let keyring_nsec = keyring_keys.secret_key().to_bech32().unwrap(); + + let file_keys = Keys::generate(); + save_key_file(&legacy_path, &file_keys).unwrap(); + + // Force marker write failure by making the data directory read-only. + // AtomicWriteFile writes a temp file in the same dir then renames it, + // so removing write permission on the dir blocks the write entirely. + let dir_perms_orig = std::fs::metadata(dir.path()).unwrap().permissions(); + let mut dir_perms_ro = dir_perms_orig.clone(); + // unknown_lints: the clippy lint below doesn't exist yet in the pinned + // 1.95 toolchain but does in CI's newer clippy — allow both worlds. + #[allow(unknown_lints)] + #[allow(clippy::permissions_set_readonly_value)] + dir_perms_ro.set_readonly(true); + std::fs::set_permissions(dir.path(), dir_perms_ro).unwrap(); + + let store = FakeIdentityStore::present_with(&keyring_nsec); + // Resolve with the read-only dir; marker write will fail. + let resolved = resolve_identity_with_store(&store, &legacy_path, dir.path()); + + // Restore perms before any assertions that might panic mid-cleanup. + std::fs::set_permissions(dir.path(), dir_perms_orig).unwrap(); + + // On a read-only fs the file write also fails; we can't even check + // legacy_path reliably there. What matters is that if resolve succeeded + // it returned the file's key, and did NOT delete the file. + if let Ok(resolved) = resolved { + assert_key_eq(&file_keys, &resolved.keys); + assert_eq!(resolved.recovery, RecoveryState::None); + // identity.key must NOT have been deleted — it is the only + // fallback when the marker could not be written. + assert!( + legacy_path.exists(), + "identity.key must be kept when marker write fails after adoption" + ); + } + // If resolve Err'd (e.g. file write also failed) the test still passes — + // we've verified the code doesn't delete the file without a marker. +} + +#[test] +fn reachable_but_empty_with_marker_and_no_file_returns_lost() { + // (d) ReachableButEmpty + marker + no file → "lost" state, NO new key + // generated into the keyring. + // + // The marker says a key was once stored in the keyring. If the keyring + // is now empty (entry deleted externally, new OS login session cleared + // it, etc.) and there is no file fallback, the user's key is truly + // gone. Resolution must NOT silently generate a new identity; it must + // surface a "lost" state so the frontend can prompt re-import. + let dir = tempfile::tempdir().unwrap(); + let legacy_path = dir.path().join("identity.key"); + // Write the migration marker — a key was once in the keyring. + write_migration_marker(&migration_marker_path(dir.path())).unwrap(); + assert!(!legacy_path.exists()); // no file fallback + + let store = FakeIdentityStore::reachable_but_empty(); + let resolved = resolve_identity_with_store(&store, &legacy_path, dir.path()).unwrap(); + + // The "lost" flag is set — the frontend must prompt re-import. + assert_eq!( + resolved.recovery, + RecoveryState::Lost, + "identity lost state must be surfaced" + ); + + // No key was persisted to the keyring — the ephemeral key is in-memory + // only and must not overwrite the user's actual (externally lost) key. + assert!( + store.slot.borrow().is_empty(), + "no key must be written to keyring when identity is lost" + ); + + // No identity.key written either — the ephemeral key is transient. + assert!(!legacy_path.exists()); +} + +#[test] +fn persist_imported_identity_falls_back_to_file_on_keyring_failure() { + // `persist_imported_identity_impl` with a failing store returns Ok and + // writes identity.key as a fallback. No migration marker is written — a + // marker here would cause fail-closed on a later Unreachable boot even + // though the key is in the file, not the keyring. + let dir = tempfile::tempdir().unwrap(); + let legacy_path = dir.path().join("identity.key"); + let imported_keys = Keys::generate(); + + let store = FakeIdentityStore::store_failing(); + + let result = persist_imported_identity_impl(&store, &imported_keys, &legacy_path, dir.path()); + + // The policy core handles the keyring failure — Ok, not Err. + assert!( + result.is_ok(), + "must not propagate keyring failure when file fallback succeeds" + ); + + // Key is recoverable from the file on next boot. + let from_file = load_key_file(&legacy_path).unwrap(); + assert_key_eq(&imported_keys, &from_file); + + // No marker written — the file is the authoritative store, not the keyring. + assert!(!migration_marker_path(dir.path()).exists()); + + // The underlying kernel still propagates keyring failure (low-level + // contract unchanged — the impl layer is what adds the fallback). + let dir2 = tempfile::tempdir().unwrap(); + let path2 = dir2.path().join("identity.key"); + assert!( + persist_identity_to_keyring(&store, &imported_keys, &path2, dir2.path()).is_err(), + "persist_identity_to_keyring must still propagate keyring failure" + ); +} + +#[test] +fn persist_to_keyring_marker_failure_writes_file_when_absent_preserves_invariant() { + // (f) Marker-write failure after a verified keyring write when no + // identity.key exists (e.g. import from a lost state where the file + // was already deleted). The invariant "keyring-only implies marker + // exists" must be preserved: persist_identity_to_keyring must write + // identity.key as a fallback so a later keyring-unreachable boot does + // NOT treat the machine as a fresh install and silently rotate identity. + let dir = tempfile::tempdir().unwrap(); + let legacy_path = dir.path().join("identity.key"); + assert!(!legacy_path.exists()); // no file — simulates import from lost state + + let imported_keys = Keys::generate(); + // Keyring write succeeds. + let store = FakeIdentityStore::reachable_but_empty(); + + // Force marker write to fail by placing a directory at the marker path. + // AtomicWriteFile::open fails when the target path is a directory. + let marker_path = migration_marker_path(dir.path()); + std::fs::create_dir_all(&marker_path).unwrap(); + + // persist_identity_to_keyring will: store to keyring (succeeds), read- + // back verify (succeeds), attempt write_migration_marker (fails because + // marker_path is a directory), then write identity.key as a fallback. + let result = persist_identity_to_keyring(&store, &imported_keys, &legacy_path, dir.path()); + + // The function returns Ok — the error is handled, not propagated. + assert!( + result.is_ok(), + "persist_identity_to_keyring must not propagate marker failure" + ); + + // identity.key was written as a fallback — invariant preserved. + assert!( + legacy_path.exists(), + "identity.key must exist as fallback when marker write failed and file was absent" + ); + let from_file = load_key_file(&legacy_path).unwrap(); + assert_key_eq(&imported_keys, &from_file); +} + +#[test] +fn present_keyring_same_pubkey_file_no_marker_writes_marker_before_cleanup() { + // Present branch: keyring present + same-pubkey identity.key + NO marker. + // This can arise when persist_identity_to_keyring succeeded at keyring + // write + marker write but the remove_file step failed, then the marker + // was deleted externally — or from any earlier code path that stored to + // the keyring without writing the marker. + // + // The fix: write the marker first (crash-safe ordering), then delete the + // file. Must NOT delete the file while no marker exists. + let dir = tempfile::tempdir().unwrap(); + let legacy_path = dir.path().join("identity.key"); + assert!(!migration_marker_path(dir.path()).exists()); // no marker + + let keys = Keys::generate(); + let nsec = keys.secret_key().to_bech32().unwrap(); + // Same key in both keyring and file — stale leftover scenario. + save_key_file(&legacy_path, &keys).unwrap(); + + let store = FakeIdentityStore::present_with(&nsec); + let resolved = resolve_identity_with_store(&store, &legacy_path, dir.path()).unwrap(); + + // Keyring key is returned. + assert_key_eq(&keys, &resolved.keys); + assert_eq!(resolved.recovery, RecoveryState::None); + + // Marker must now exist — written before or instead of deleting. + assert!( + migration_marker_path(dir.path()).exists(), + "marker must be written before identity.key is deleted" + ); +} + +#[test] +fn reachable_but_empty_with_marker_and_no_file_returns_lost_ephemeral_not_persisted() { + // Extension of reachable_but_empty_with_marker_and_no_file_returns_lost: + // also verifies that the ephemeral key returned in lost state is NOT + // persisted to the keyring — it must remain in-memory only. + 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::reachable_but_empty(); + let resolved = resolve_identity_with_store(&store, &legacy_path, dir.path()).unwrap(); + + assert_eq!(resolved.recovery, RecoveryState::Lost); + // The ephemeral key must NOT be written to the keyring. + assert!( + store.slot.borrow().is_empty(), + "ephemeral lost key must not be written to keyring" + ); + // No identity.key written either. + assert!(!legacy_path.exists()); + // The ephemeral pubkey is distinct on every call (sanity check). + let store2 = FakeIdentityStore::reachable_but_empty(); + let resolved2 = resolve_identity_with_store(&store2, &legacy_path, dir.path()).unwrap(); + assert_eq!(resolved2.recovery, RecoveryState::Lost); + // Two ephemeral keys are different (probabilistic — collision probability is negligible). + assert_ne!( + resolved.keys.public_key().to_hex(), + resolved2.keys.public_key().to_hex(), + "each lost-state boot produces a distinct ephemeral key" + ); +} + +// ── signing_keys() gate tests ───────────────────────────────────────────── + +#[test] +fn signing_keys_returns_ok_when_normal() { + // When neither identity_lost nor keyring_locked is set, signing_keys() + // must return the live keys and allow signing. + let state = build_app_state(); + state + .identity_lost + .store(false, std::sync::atomic::Ordering::Relaxed); + state + .keyring_locked + .store(false, std::sync::atomic::Ordering::Relaxed); + + let result = state.signing_keys(); + assert!( + result.is_ok(), + "signing_keys() must return Ok when neither flag is set" + ); + // The returned keys must match the stored keys. + let expected = state.keys.lock().unwrap().clone(); + assert_key_eq(&result.unwrap(), &expected); +} + +#[test] +fn signing_keys_returns_err_when_identity_lost() { + // An ephemeral key is held when identity is lost — signing under it would + // publish events with a random identity the user does not own. + let state = build_app_state(); + state + .identity_lost + .store(true, std::sync::atomic::Ordering::Relaxed); + + let result = state.signing_keys(); + assert!( + result.is_err(), + "signing_keys() must return Err when identity_lost is set" + ); + assert!( + result.unwrap_err().contains("recovery mode"), + "error message must mention recovery mode" + ); +} + +#[test] +fn signing_keys_returns_err_when_keyring_locked() { + // The identity key is held in a keyring that is unavailable this boot — + // the stored keys are inaccessible so signing must be blocked. + let state = build_app_state(); + state + .keyring_locked + .store(true, std::sync::atomic::Ordering::Relaxed); + + let result = state.signing_keys(); + assert!( + result.is_err(), + "signing_keys() must return Err when keyring_locked is set" + ); + assert!( + result.unwrap_err().contains("recovery mode"), + "error message must mention recovery mode" + ); +} + +#[test] +fn signing_keys_identity_lost_takes_priority_over_keyring_locked() { + // When both flags are set, identity_lost is checked first and its error + // message is returned (the ephemeral-key case is more specific). + let state = build_app_state(); + state + .identity_lost + .store(true, std::sync::atomic::Ordering::Relaxed); + state + .keyring_locked + .store(true, std::sync::atomic::Ordering::Relaxed); + + let err = state.signing_keys().unwrap_err(); + assert!( + err.contains("recovery mode"), + "both-set must return recovery-mode error: {err}" + ); +} + +// ── Keyring-locked recovery mode tests ─────────────────────────────────── + +#[test] +fn keyring_locked_recovery_ephemeral_never_persisted() { + // Unreachable + marker + no file → KeyringLocked recovery. The ephemeral + // key is held in memory only; no identity.key is created, no keyring + // slot is touched. Fail-closed semantics: no identity is ever rotated. + 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 resolved = resolve_identity_with_store(&store, &legacy_path, dir.path()).unwrap(); + + assert_eq!(resolved.recovery, RecoveryState::KeyringLocked); + // Nothing written to disk — ephemeral key is transient. + assert!(!legacy_path.exists()); + // Keyring was never contacted (it is unreachable). + assert!(store.slot.borrow().is_empty()); + assert!(store.deleted.borrow().is_empty()); +} + +#[test] +fn keyring_locked_recovery_distinct_ephemeral_per_boot() { + // Each locked-state boot produces a distinct ephemeral key and persists + // nothing — mirroring the lost-state guarantee. + 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 store1 = FakeIdentityStore::unreachable(); + let resolved1 = resolve_identity_with_store(&store1, &legacy_path, dir.path()).unwrap(); + assert_eq!(resolved1.recovery, RecoveryState::KeyringLocked); + + let store2 = FakeIdentityStore::unreachable(); + let resolved2 = resolve_identity_with_store(&store2, &legacy_path, dir.path()).unwrap(); + assert_eq!(resolved2.recovery, RecoveryState::KeyringLocked); + + // Two ephemeral keys are different (probabilistic — collision negligible). + assert_ne!( + resolved1.keys.public_key().to_hex(), + resolved2.keys.public_key().to_hex(), + "each locked-state boot produces a distinct ephemeral key" + ); + // Neither boot persisted anything. + assert!(!legacy_path.exists()); +} + +// ── B1: read-back corruption ────────────────────────────────────────────── + +#[test] +fn persist_identity_to_keyring_readback_corrupt_returns_err() { + // B1.1: store() succeeds but load() returns a different valid-format value. + // The read-back verify in persist_identity_to_keyring must detect the + // mismatch and return Err so the caller knows the key was not durably stored. + let dir = tempfile::tempdir().unwrap(); + let legacy_path = dir.path().join("identity.key"); + + let other_keys = Keys::generate(); + let other_nsec = other_keys.secret_key().to_bech32().unwrap(); + let store = FakeIdentityStore::with_readback_corruption(&other_nsec); + let imported_keys = Keys::generate(); + + let result = persist_identity_to_keyring(&store, &imported_keys, &legacy_path, dir.path()); + + assert!( + result.is_err(), + "must return Err when read-back returns a different value" + ); + assert!( + result.unwrap_err().contains("read-back"), + "error message must mention read-back verify failure" + ); +} + +#[test] +fn persist_imported_identity_impl_readback_corrupt_falls_back_to_file() { + // B1.2: persist_imported_identity_impl with a readback-corrupt store returns + // Ok and writes identity.key as a fallback, and the file holds the original key. + let dir = tempfile::tempdir().unwrap(); + let legacy_path = dir.path().join("identity.key"); + + let other_keys = Keys::generate(); + let other_nsec = other_keys.secret_key().to_bech32().unwrap(); + let store = FakeIdentityStore::with_readback_corruption(&other_nsec); + let imported_keys = Keys::generate(); + + let result = persist_imported_identity_impl(&store, &imported_keys, &legacy_path, dir.path()); + + assert!( + result.is_ok(), + "must return Ok when file fallback succeeds after readback corruption: {:?}", + result.err() + ); + assert!( + legacy_path.exists(), + "identity.key must be written as fallback" + ); + let from_file = load_key_file(&legacy_path).unwrap(); + assert_key_eq(&imported_keys, &from_file); +} + +// ── B2: corrupt key material recovery ──────────────────────────────────── + +#[test] +fn reachable_but_empty_corrupt_file_generates_fresh() { + // B2.1: ReachableButEmpty probe + corrupt identity.key → migrate_identity_file + // returns Ok(None) for the corrupt file, then generate_and_persist runs and + // stores a fresh valid key in the keyring. No panic; resolve succeeds. + let dir = tempfile::tempdir().unwrap(); + let legacy_path = dir.path().join("identity.key"); + std::fs::write(&legacy_path, b"this-is-not-a-valid-nsec").unwrap(); + + let store = FakeIdentityStore::reachable_but_empty(); + let resolved = resolve_identity_with_store(&store, &legacy_path, dir.path()).unwrap(); + + assert_eq!(resolved.recovery, RecoveryState::None); + // The keyring now holds the fresh key. + let stored_nsec = store + .slot + .borrow() + .get(IDENTITY_KEY_NAME) + .cloned() + .expect("keyring must hold a fresh key after corrupt-file recovery"); + let keyring_keys = Keys::parse(&stored_nsec).expect("keyring value must be a valid nsec"); + assert_key_eq(&resolved.keys, &keyring_keys); +} + +#[test] +fn present_corrupt_keyring_and_corrupt_file_generates_fresh() { + // B2.2: Present probe with a corrupt keyring value AND a corrupt identity.key. + // recover_from_keyring clears the bad entry, migrate_identity_file returns + // Ok(None) for the corrupt file, then generate_and_persist stores a fresh key + // in the keyring. Resolve succeeds; keyring holds the fresh valid key. + let dir = tempfile::tempdir().unwrap(); + let legacy_path = dir.path().join("identity.key"); + std::fs::write(&legacy_path, b"this-is-not-a-valid-nsec").unwrap(); + + let store = FakeIdentityStore::present_with("not-a-valid-nsec"); + let resolved = resolve_identity_with_store(&store, &legacy_path, dir.path()).unwrap(); + + assert_eq!(resolved.recovery, RecoveryState::None); + // The corrupt keyring entry was cleared. + assert!( + store + .deleted + .borrow() + .contains(&IDENTITY_KEY_NAME.to_string()), + "corrupt keyring entry must be cleared" + ); + // Keyring holds the newly generated valid key. + let stored_nsec = store + .slot + .borrow() + .get(IDENTITY_KEY_NAME) + .cloned() + .expect("keyring must hold a fresh key after double-corrupt recovery"); + let keyring_keys = Keys::parse(&stored_nsec).expect("keyring value must be a valid nsec"); + assert_key_eq(&resolved.keys, &keyring_keys); +} + +// ── B3: Unreachable probe branches ─────────────────────────────────────── + +#[test] +fn unreachable_with_valid_file_resolves_to_file_key() { + // B3.a+b (inputs are indistinguishable at this level): Unreachable + valid + // identity.key → resolves to the file's key. The keyring is never contacted + // and the file is kept on disk (no migration when keyring is down). + 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::unreachable(); + let resolved = resolve_identity_with_store(&store, &legacy_path, dir.path()).unwrap(); + + assert_key_eq(&file_keys, &resolved.keys); + assert_eq!(resolved.recovery, RecoveryState::None); + assert!( + legacy_path.exists(), + "identity.key must not be deleted when keyring is unreachable" + ); + assert!( + store.slot.borrow().is_empty(), + "keyring must not be contacted when unreachable" + ); +} + +#[test] +fn unreachable_valid_file_with_marker_resolves_to_file_not_locked_recovery() { + // Unreachable + valid identity.key + marker present → resolves to the file + // key, NOT KeyringLocked recovery. The locked-recovery branch only fires + // when the file is ABSENT; a present file is always used as a direct + // fallback regardless of the marker. + 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(); + write_migration_marker(&migration_marker_path(dir.path())).unwrap(); + + let store = FakeIdentityStore::unreachable(); + let resolved = resolve_identity_with_store(&store, &legacy_path, dir.path()).unwrap(); + + assert_key_eq(&file_keys, &resolved.keys); + assert_eq!( + resolved.recovery, + RecoveryState::None, + "must not enter locked-recovery when a valid file is present" + ); +} + +#[test] +fn unreachable_corrupt_file_generates_fresh() { + // B3.c: Unreachable + corrupt identity.key → load_file_or_generate quarantines + // the corrupt file, generates a fresh key, and saves it to identity.key. + let dir = tempfile::tempdir().unwrap(); + let legacy_path = dir.path().join("identity.key"); + std::fs::write(&legacy_path, b"this-is-not-a-valid-nsec").unwrap(); + assert!(!migration_marker_path(dir.path()).exists()); + + let store = FakeIdentityStore::unreachable(); + let resolved = resolve_identity_with_store(&store, &legacy_path, dir.path()).unwrap(); + + assert_eq!(resolved.recovery, RecoveryState::None); + // A fresh key was saved to identity.key (quarantine renames the corrupt file). + assert!( + legacy_path.exists(), + "fresh key must be saved to identity.key" + ); + let from_file = load_key_file(&legacy_path).unwrap(); + assert_key_eq(&resolved.keys, &from_file); +} + +// ── B4: marker-write failure variants ──────────────────────────────────── + +#[test] +fn persist_identity_to_keyring_marker_failure_file_fallback_returns_ok() { + // B4.1: marker write fails (data_dir is an existing file, so the marker + // path cannot be created), but the file fallback succeeds — returns Ok and + // identity.key exists and holds the original key. + let dir = tempfile::tempdir().unwrap(); + let key_dir = tempfile::tempdir().unwrap(); + let legacy_path = key_dir.path().join("identity.key"); + assert!(!legacy_path.exists()); + + // Make data_dir a FILE so marker write fails. + let data_dir_file = dir.path().join("data_as_file"); + std::fs::write(&data_dir_file, b"not a dir").unwrap(); + + let store = FakeIdentityStore::reachable_but_empty(); + let imported_keys = Keys::generate(); + + let result = persist_identity_to_keyring(&store, &imported_keys, &legacy_path, &data_dir_file); + + assert!( + result.is_ok(), + "must return Ok when file fallback succeeds despite marker failure: {:?}", + result.err() + ); + assert!( + legacy_path.exists(), + "identity.key must be written as fallback" + ); + let from_file = load_key_file(&legacy_path).unwrap(); + assert_key_eq(&imported_keys, &from_file); +} + +#[test] +fn persist_identity_to_keyring_marker_and_file_failure_returns_err() { + // B4.2: both marker write and file write fail → must return Err (A2 fix). + // data_dir is a FILE (marker write fails); legacy_path is in a non-existent + // subdirectory so AtomicWriteFile::open fails on the file write too. + let dir = tempfile::tempdir().unwrap(); + + let data_dir_file = dir.path().join("data_as_file"); + std::fs::write(&data_dir_file, b"not a dir").unwrap(); + + // Parent directory does not exist → file write fails. + let legacy_path = dir.path().join("nonexistent_subdir").join("identity.key"); + assert!(!legacy_path.exists()); + + let store = FakeIdentityStore::reachable_but_empty(); + let imported_keys = Keys::generate(); + + let result = persist_identity_to_keyring(&store, &imported_keys, &legacy_path, &data_dir_file); + + assert!( + result.is_err(), + "must return Err when both marker write and file write fail" + ); + let err_msg = result.unwrap_err(); + assert!( + err_msg.contains("persisted") || err_msg.contains("marker") || err_msg.contains("file"), + "error message must describe the dual failure: {err_msg}" + ); +} + +#[test] +fn present_keyring_no_file_no_marker_self_heals_marker() { + // B4.3 / A3 coverage: Present(valid) + no identity.key + no migration marker. + // After resolve, the marker must exist (self-healed by A3) so a later + // keyring-Unreachable boot does not treat this as a fresh install. + 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 keys = Keys::generate(); + let nsec = keys.secret_key().to_bech32().unwrap(); + let store = FakeIdentityStore::present_with(&nsec); + + let resolved = resolve_identity_with_store(&store, &legacy_path, dir.path()).unwrap(); + + assert_key_eq(&keys, &resolved.keys); + assert_eq!(resolved.recovery, RecoveryState::None); + assert!( + migration_marker_path(dir.path()).exists(), + "marker must be self-healed by A3 when Present(valid) + no file + no marker" + ); +} diff --git a/desktop/src-tauri/src/commands/agents.rs b/desktop/src-tauri/src/commands/agents.rs index f5cafaaeb..cb08b81ba 100644 --- a/desktop/src-tauri/src/commands/agents.rs +++ b/desktop/src-tauri/src/commands/agents.rs @@ -60,7 +60,7 @@ pub(super) fn retain_managed_agent_pending( let content = serde_json::to_string(&agent_event_content(record)) .map_err(|e| format!("failed to serialize managed-agent content: {e}"))?; let (owner_pubkey, event) = { - let keys = state.keys.lock().map_err(|e| e.to_string())?; + let keys = state.signing_keys()?; let owner_pubkey = keys.public_key().to_hex(); let existing = get_retained_event(&conn, KIND_MANAGED_AGENT, &owner_pubkey, &record.pubkey)?; @@ -122,7 +122,7 @@ fn tombstone_managed_agent_pending(app: &AppHandle, state: &AppState, agent_pubk let result = (|| -> Result<(), String> { let (owner_pubkey, event) = { - let keys = state.keys.lock().map_err(|e| e.to_string())?; + let keys = state.signing_keys()?; let owner_pubkey = keys.public_key().to_hex(); let event = build_agent_delete(agent_pubkey, &owner_pubkey)? .sign_with_keys(&keys) @@ -607,7 +607,7 @@ pub async fn create_managed_agent( // Agents authenticate via the auth tag in their kind:0 profile event. // No tokens are minted. Fail closed: bad auth tag → don't create agent. let auth_tag = { - let owner_keys = state.keys.lock().map_err(|e| e.to_string())?; + let owner_keys = state.signing_keys()?; // Bridge nostr 0.37 → 0.36 (buzz-sdk) via hex round-trip. let compat_owner = nostr::Keys::parse(&owner_keys.secret_key().to_secret_hex()) .map_err(|e| format!("failed to bridge owner keys: {e}"))?; diff --git a/desktop/src-tauri/src/commands/identity.rs b/desktop/src-tauri/src/commands/identity.rs index 8a7abb46c..45bb2992f 100644 --- a/desktop/src-tauri/src/commands/identity.rs +++ b/desktop/src-tauri/src/commands/identity.rs @@ -10,23 +10,37 @@ use crate::{ relay::{self, relay_api_base_url_with_override, relay_ws_url_with_override}, }; -#[tauri::command] -pub fn get_identity(state: State<'_, AppState>) -> Result { - let keys = state.keys.lock().map_err(|error| error.to_string())?; - let pubkey = keys.public_key(); - let pubkey_hex = pubkey.to_hex(); +/// Encode `pubkey` as npub bech32 and truncate it for display: first 10 chars +/// + "…" + last 4 chars. Returns the full bech32 when it is 16 chars or fewer. +fn truncated_display_name(pubkey: &PublicKey) -> Result { let bech32 = pubkey .to_bech32() .map_err(|error| format!("bech32 encode failed: {error}"))?; - let display_name = if bech32.len() > 16 { + Ok(if bech32.len() > 16 { format!("{}…{}", &bech32[..10], &bech32[bech32.len() - 4..]) } else { bech32 - }; + }) +} + +#[tauri::command] +pub fn get_identity(state: State<'_, AppState>) -> Result { + let keys = state.keys.lock().map_err(|error| error.to_string())?; + let pubkey = keys.public_key(); + let pubkey_hex = pubkey.to_hex(); + let display_name = truncated_display_name(&pubkey)?; + let lost = state + .identity_lost + .load(std::sync::atomic::Ordering::Acquire); + let locked = state + .keyring_locked + .load(std::sync::atomic::Ordering::Acquire); Ok(IdentityInfo { pubkey: pubkey_hex, display_name, + lost, + locked, }) } @@ -71,11 +85,7 @@ pub async fn sign_event( tags: Vec>, state: State<'_, AppState>, ) -> Result { - let keys = state - .keys - .lock() - .map_err(|error| error.to_string())? - .clone(); + let keys = state.signing_keys()?; tauri::async_runtime::spawn_blocking(move || { let nostr_tags = tags @@ -103,13 +113,7 @@ pub fn decrypt_observer_event( event_json: String, state: State<'_, AppState>, ) -> Result { - let nsec = { - let keys = state.keys.lock().map_err(|error| error.to_string())?; - keys.secret_key() - .to_bech32() - .map_err(|error| format!("encode nsec: {error}"))? - }; - let keys = Keys::parse(&nsec).map_err(|error| format!("parse nsec: {error}"))?; + let keys = state.signing_keys()?; let event = Event::from_json(event_json).map_err(|error| format!("invalid event: {error}"))?; // Defense-in-depth: verify event ID and signature before decrypting. @@ -130,13 +134,7 @@ pub fn build_observer_control_event( payload: serde_json::Value, state: State<'_, AppState>, ) -> Result { - let nsec = { - let keys = state.keys.lock().map_err(|error| error.to_string())?; - keys.secret_key() - .to_bech32() - .map_err(|error| format!("encode nsec: {error}"))? - }; - let keys = Keys::parse(&nsec).map_err(|error| format!("parse nsec: {error}"))?; + let keys = state.signing_keys()?; let agent_pubkey = PublicKey::from_hex(agent_pubkey.trim()) .map_err(|error| format!("invalid agent pubkey: {error}"))?; let agent_pubkey_hex = agent_pubkey.to_hex(); @@ -158,7 +156,7 @@ pub fn build_observer_control_event( #[tauri::command] pub fn get_nsec(state: State<'_, AppState>) -> Result { - let keys = state.keys.lock().map_err(|error| error.to_string())?; + let keys = state.signing_keys()?; keys.secret_key() .to_bech32() .map_err(|error| format!("encode nsec: {error}")) @@ -173,36 +171,121 @@ pub async fn import_identity( let trimmed = nsec.trim(); let keys = Keys::parse(trimmed).map_err(|e| format!("Invalid private key: {e}"))?; - // Persist to identity.key before swapping in-memory state. If the disk - // write fails, the running app keeps the old identity. + // Serialize against persist_current_identity: hold this guard for the + // full function body so a concurrent stale persist can't overwrite + // this import. + let state = app_handle.state::(); + let _mutation_guard = state.identity_mutation.lock().map_err(|e| e.to_string())?; + let data_dir = app_handle .path() .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"); - crate::app_state::save_key_file(&key_path, &keys)?; - // Update in-memory keys only after persistence succeeds. - let state = app_handle.state::(); + // Persist into the OS keyring first (store → read-back verify → marker → + // delete file). Falls back to the 0o600 file when the keyring is + // unavailable; returns Err only when both backends fail. + let store = crate::secret_store::SecretStore::shared(crate::app_state::KEYRING_SERVICE); + crate::app_state::persist_imported_identity(store, &keys, &key_path, &data_dir)?; + + // Update in-memory keys BEFORE clearing recovery flags. The Release + // stores below pair with Acquire loads in get_identity: a reader + // observing false is guaranteed to see the updated keys. let pubkey = keys.public_key(); *state.keys.lock().map_err(|e| e.to_string())? = keys; + // Clear both recovery flags — an import is valid in either lost or + // keyring-locked state and resolves both. In the locked case the + // keyring is unreachable, so persist_imported_identity already fell + // back to identity.key; on the next Unreachable boot the file is + // loaded directly and when the keyring returns the adoption path + // picks it up. + state + .identity_lost + .store(false, std::sync::atomic::Ordering::Release); + state + .keyring_locked + .store(false, std::sync::atomic::Ordering::Release); + let pubkey_hex = pubkey.to_hex(); - let bech32 = pubkey - .to_bech32() - .map_err(|error| format!("bech32 encode failed: {error}"))?; - let display_name = if bech32.len() > 16 { - format!("{}…{}", &bech32[..10], &bech32[bech32.len() - 4..]) - } else { - bech32 - }; + let display_name = truncated_display_name(&pubkey)?; eprintln!("buzz-desktop: imported identity pubkey {}", pubkey_hex); Ok(IdentityInfo { pubkey: pubkey_hex, display_name, + lost: false, + locked: false, + }) + }) + .await + .map_err(|e| format!("spawn_blocking failed: {e}"))? +} + +/// Make the current ephemeral identity durable by persisting it to the OS +/// keyring (or falling back to identity.key). This is called when the user +/// chooses to start a new identity instead of re-importing their previous one +/// — it converts the transient lost-state key into a permanent identity. +/// +/// **LOST-ONLY**: returns `Err` when `identity_lost` is false, and deliberately +/// does NOT accept `keyring_locked`. In locked state the user's real identity +/// still exists in the unreachable keyring; persisting the ephemeral key to +/// `identity.key` would make it appear as a "different key" on next boot, +/// and the mismatched-file adoption path would then clobber the real keyring +/// key once the keyring becomes reachable again. The correct action in locked +/// state is to unlock the keyring and relaunch — not to adopt the ephemeral key. +#[tauri::command] +pub async fn persist_current_identity( + app_handle: tauri::AppHandle, +) -> Result { + tokio::task::spawn_blocking(move || { + let state = app_handle.state::(); + + // Acquire mutation lock before reading identity_lost so that a + // concurrent import_identity cannot complete between our check and + // our persist, which would let the stale ephemeral key overwrite the + // imported one. + let _mutation_guard = state.identity_mutation.lock().map_err(|e| e.to_string())?; + + if !state + .identity_lost + .load(std::sync::atomic::Ordering::Acquire) + { + return Err("identity is not in a lost state".to_string()); + } + + // Clone current keys without holding the mutex across keyring I/O. + let keys = state.keys.lock().map_err(|e| e.to_string())?.clone(); + + let data_dir = app_handle + .path() + .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"); + + let store = crate::secret_store::SecretStore::shared(crate::app_state::KEYRING_SERVICE); + crate::app_state::persist_imported_identity(store, &keys, &key_path, &data_dir)?; + + // Keys are already the live identity — only clear identity_lost. + // Release pairs with Acquire in get_identity so readers see + // consistent state. + state + .identity_lost + .store(false, std::sync::atomic::Ordering::Release); + + let pubkey = keys.public_key(); + let pubkey_hex = pubkey.to_hex(); + let display_name = truncated_display_name(&pubkey)?; + + Ok(IdentityInfo { + pubkey: pubkey_hex, + display_name, + lost: false, + locked: false, }) }) .await @@ -215,11 +298,7 @@ pub async fn create_auth_event( relay_url: String, state: State<'_, AppState>, ) -> Result { - let keys = state - .keys - .lock() - .map_err(|error| error.to_string())? - .clone(); + let keys = state.signing_keys()?; tauri::async_runtime::spawn_blocking(move || { let tags = vec![ @@ -245,7 +324,7 @@ pub async fn nip44_encrypt_to_self( plaintext: String, state: State<'_, AppState>, ) -> Result { - let keys = state.keys.lock().map_err(|e| e.to_string())?.clone(); + let keys = state.signing_keys()?; tauri::async_runtime::spawn_blocking(move || { nip44::encrypt( @@ -265,7 +344,7 @@ pub async fn nip44_decrypt_from_self( ciphertext: String, state: State<'_, AppState>, ) -> Result { - let keys = state.keys.lock().map_err(|e| e.to_string())?.clone(); + let keys = state.signing_keys()?; tauri::async_runtime::spawn_blocking(move || { nip44::decrypt(keys.secret_key(), &keys.public_key(), &ciphertext) diff --git a/desktop/src-tauri/src/commands/media.rs b/desktop/src-tauri/src/commands/media.rs index 6c55613cb..9d8a85822 100644 --- a/desktop/src-tauri/src/commands/media.rs +++ b/desktop/src-tauri/src/commands/media.rs @@ -208,7 +208,7 @@ async fn do_upload( }; let base_url = relay_api_base_url_with_override(state); let auth_event = { - let keys = state.keys.lock().map_err(|e| e.to_string())?; + let keys = state.signing_keys()?; sign_blossom_upload_auth(&keys, &sha256, expiry_secs, &base_url)? }; diff --git a/desktop/src-tauri/src/commands/pairing.rs b/desktop/src-tauri/src/commands/pairing.rs index 1168b9f1b..f86a1a94f 100644 --- a/desktop/src-tauri/src/commands/pairing.rs +++ b/desktop/src-tauri/src/commands/pairing.rs @@ -76,15 +76,12 @@ pub async fn start_pairing( } pairing.clear(); - let (nsec, pubkey_hex) = { - let keys = state.keys.lock().map_err(|e| e.to_string())?; - let nsec = keys - .secret_key() - .to_bech32() - .map_err(|e| format!("encode nsec: {e}"))?; - let pubkey = keys.public_key().to_hex(); - (nsec, pubkey) - }; + let keys = state.signing_keys()?; + let nsec = keys + .secret_key() + .to_bech32() + .map_err(|e| format!("encode nsec: {e}"))?; + let pubkey_hex = keys.public_key().to_hex(); let ws_url = relay_ws_url_with_override(&state); let http_url = relay_api_base_url_with_override(&state); diff --git a/desktop/src-tauri/src/lib.rs b/desktop/src-tauri/src/lib.rs index 5d0fd6318..3e5bdbbc8 100644 --- a/desktop/src-tauri/src/lib.rs +++ b/desktop/src-tauri/src/lib.rs @@ -226,6 +226,19 @@ pub fn run() { resolve_persisted_identity(&app_handle, &state) .map_err(|e| -> Box { e.into() })?; + // When the identity is in recovery mode (lost = keyring empty after + // migration, or keyring-locked = keyring unreachable but marker + // present), all owner-keyed side effects (event sync, agent restore, + // relay publish) are skipped. The frontend shows a recovery screen; + // the user must relaunch after restoring the identity. + let identity_lost = state + .identity_lost + .load(std::sync::atomic::Ordering::Acquire); + let keyring_locked = state + .keyring_locked + .load(std::sync::atomic::Ordering::Acquire); + let recovery_mode = identity_lost || keyring_locked; + // Snapshot owner keys after identity resolution; the best-effort // event reconcile itself runs off the synchronous setup path below. let owner_keys = state @@ -334,8 +347,11 @@ pub fn run() { // Sync team-dir edits and reconcile persona/team/agent events after // setup can continue. It is best-effort retention backfill, unlike // identity resolution above, so JSON/SQLite/signing work must not - // hold the boot path hostage. - event_sync::spawn_event_sync(app_handle.clone(), owner_keys); + // hold the boot path hostage. Skipped in recovery mode — the owner + // key is ephemeral. + if !recovery_mode { + event_sync::spawn_event_sync(app_handle.clone(), owner_keys); + } if let Some(mgr) = huddle::models::global_model_manager() { mgr.start_stt_download(state.http_client.clone()); @@ -361,7 +377,9 @@ pub fn run() { // the boot-time repos symlink result (see restore_agents above): // skip when a configured repos_dir could not be resolved, so no // agent clones into a REPOS that isn't the user's target. - if restore_agents { + // Also skipped in recovery mode — agents must not be spawned + // under an ephemeral owner key. + if restore_agents && !recovery_mode { tauri::async_runtime::spawn(async move { if let Err(error) = restore_managed_agents_on_launch(&app_handle, shutdown_started.as_ref()) @@ -416,26 +434,31 @@ pub fn run() { // One loop is the sole publisher for persona, team, and managed- // agent writers; a relay-unreachable tick leaves rows pending for // the next sweep. - let flush_handle = app.handle().clone(); - tauri::async_runtime::spawn(async move { - use std::time::Duration; - use tauri::Manager; - let Ok(db_path) = managed_agents::managed_agents_base_dir(&flush_handle) - .map(|d| d.join("retention.db")) - else { - eprintln!("buzz-desktop: event-flush: cannot resolve retention db path"); - return; - }; - loop { - let state = flush_handle.state::(); - if let Err(e) = - managed_agents::persona_events::flush_pending_events(&db_path, &state).await - { - eprintln!("buzz-desktop: event-flush: {e}"); + // Skipped in recovery mode — flushing under an ephemeral key would + // publish events attributed to an identity the user doesn't own. + if !recovery_mode { + let flush_handle = app.handle().clone(); + tauri::async_runtime::spawn(async move { + use std::time::Duration; + use tauri::Manager; + let Ok(db_path) = managed_agents::managed_agents_base_dir(&flush_handle) + .map(|d| d.join("retention.db")) + else { + eprintln!("buzz-desktop: event-flush: cannot resolve retention db path"); + return; + }; + loop { + let state = flush_handle.state::(); + if let Err(e) = + managed_agents::persona_events::flush_pending_events(&db_path, &state) + .await + { + eprintln!("buzz-desktop: event-flush: {e}"); + } + tokio::time::sleep(Duration::from_secs(30)).await; } - tokio::time::sleep(Duration::from_secs(30)).await; - } - }); + }); + } Ok(()) }) @@ -443,6 +466,7 @@ pub fn run() { get_identity, get_nsec, import_identity, + persist_current_identity, get_profile, update_profile, get_user_profile, diff --git a/desktop/src-tauri/src/models.rs b/desktop/src-tauri/src/models.rs index 4d534636a..881cd11a3 100644 --- a/desktop/src-tauri/src/models.rs +++ b/desktop/src-tauri/src/models.rs @@ -6,6 +6,17 @@ use serde::{Deserialize, Deserializer, Serialize}; pub struct IdentityInfo { pub pubkey: String, pub display_name: String, + /// True when the app booted with an ephemeral key because the OS keyring + /// was empty despite a prior successful migration (key was externally + /// deleted). The frontend routes to the nsec re-import step when true. + /// Mutually exclusive with `locked`. + pub lost: bool, + /// True when the app booted with an ephemeral key because the OS keyring + /// holding the identity is unreachable this boot (keyring locked or + /// unavailable). The real key still exists in the keyring; the frontend + /// shows a "unlock the keyring and relaunch" screen. Mutually exclusive + /// with `lost`. + pub locked: bool, } #[derive(Serialize, Deserialize)] diff --git a/desktop/src-tauri/src/relay.rs b/desktop/src-tauri/src/relay.rs index a0fb1c4bd..646c19913 100644 --- a/desktop/src-tauri/src/relay.rs +++ b/desktop/src-tauri/src/relay.rs @@ -486,14 +486,14 @@ pub async fn submit_event( // so the MutexGuard is dropped and the future remains Send. let url = format!("{}/events", relay_api_base_url_with_override(state)); let (auth_header, body_bytes) = { - let keys = state.keys.lock().map_err(|e| e.to_string())?; + let keys = state.signing_keys()?; let event = builder .sign_with_keys(&keys) .map_err(|e| format!("failed to sign event: {e}"))?; let body = event.as_json().into_bytes(); let auth = build_nip98_auth_header_for_keys(&keys, &Method::POST, &url, &body)?; (auth, body) - }; // keys lock dropped here + }; // keys dropped here let response = state .http_client @@ -532,9 +532,9 @@ pub async fn submit_signed_event( let url = format!("{}/events", relay_api_base_url_with_override(state)); let body_bytes = event.as_json().into_bytes(); let auth_header = { - let keys = state.keys.lock().map_err(|e| e.to_string())?; + let keys = state.signing_keys()?; build_nip98_auth_header_for_keys(&keys, &Method::POST, &url, &body_bytes)? - }; // keys lock dropped here + }; // keys dropped here let response = state .http_client diff --git a/desktop/src/app/App.tsx b/desktop/src/app/App.tsx index a830cfd2e..a5743b64a 100644 --- a/desktop/src/app/App.tsx +++ b/desktop/src/app/App.tsx @@ -17,6 +17,8 @@ import { useReloadShortcut } from "@/app/useReloadShortcut"; import { useAppOnboardingState } from "@/features/onboarding/hooks"; import { OnboardingSlideTransition } from "@/features/onboarding/ui/OnboardingSlideTransition"; import { OnboardingFlow } from "@/features/onboarding/ui/OnboardingFlow"; +import { KeyringLockedScreen } from "@/features/onboarding/ui/KeyringLockedScreen"; +import { RelaunchRequiredScreen } from "@/features/onboarding/ui/RelaunchRequiredScreen"; import type { Workspace } from "@/features/workspaces/types"; import { useWorkspaceInit } from "@/features/workspaces/useWorkspaceInit"; import { useNestNotifications } from "@/features/workspaces/useNestNotifications"; @@ -285,11 +287,20 @@ function AppReady({ onFirstRunWorkspaceSettled, ]); + if (onboarding.stage === "keyring-locked") { + return ; + } + + if (onboarding.stage === "relaunch-required") { + return ; + } + if (onboarding.stage === "onboarding") { return ( { + if (!identityLost || !currentPubkey || identityStatus !== "success") { + return; + } + setGateState((current) => + updateActiveGateState(current, currentPubkey, (activeGateState) => ({ + ...activeGateState, + hasCompletedCurrentPubkey: false, + hasSettledCurrentPubkey: true, + isOpen: true, + })), + ); + }, [currentPubkey, identityLost, identityStatus]); + React.useEffect(() => { // Fast-path: shared identity worktrees have already onboarded in the // main checkout. Skip unconditionally without waiting for the relay @@ -389,10 +408,27 @@ export function useAppOnboardingState(isSharedIdentity: boolean) { ); const [isCompletingWelcomeSetup, setIsCompletingWelcomeSetup] = React.useState(false); - const profileQuery = useProfileQuery(); + const identityLost = identity?.lost === true; + // Keyring unreachable at boot — the real key is still in the OS keyring but + // the session cannot access it. No in-app recovery is possible; the user + // must unlock the keyring externally and relaunch. Mutually exclusive with lost. + const identityLocked = identity?.locked === true; + + // Sticky boot fact: once identity was lost at boot, this remains true for the + // entire session. Per-component state in OnboardingFlow cannot carry this + // because the flow remounts when pubkey changes after recovery. + const [bootedLost, setBootedLost] = React.useState(false); + React.useEffect(() => { + if (identityLost) setBootedLost(true); + }, [identityLost]); + + const profileQuery = useProfileQuery( + !identityLost && !identityLocked && identityQuery.status === "success", + ); const onboardingGate = useFirstRunOnboardingGate({ currentPubkey, identityIsFetching: identityQuery.fetchStatus === "fetching", + identityLost, identityStatus: identityQuery.status, isSharedIdentity, profileHasEvent: profileQuery.data?.hasProfileEvent, @@ -469,9 +505,26 @@ export function useAppOnboardingState(isSharedIdentity: boolean) { }, }; + // Recovery completed this boot: force a relaunch screen regardless of any + // other gate state. Backend startup routines (event sync, agent restore, + // pending-event flush) were skipped for the ephemeral key and cannot restart + // in-process, so nothing else can proceed until the app restarts. + const relaunchRequired = + bootedLost && !identityLost && identityQuery.status === "success"; + return { currentPubkey, flow, - stage: isCompletingWelcomeSetup ? "blocking" : onboardingGate.stage, + identityLost, + // keyring-locked is the highest-precedence stage: nothing in-session can + // clear a locked keyring, so this fully blocks the UI until relaunch. + stage: + identityLocked && identityQuery.status === "success" + ? ("keyring-locked" as const) + : relaunchRequired + ? ("relaunch-required" as const) + : isCompletingWelcomeSetup + ? ("blocking" as const) + : onboardingGate.stage, }; } diff --git a/desktop/src/features/onboarding/ui/KeyringLockedScreen.tsx b/desktop/src/features/onboarding/ui/KeyringLockedScreen.tsx new file mode 100644 index 000000000..42f5b7205 --- /dev/null +++ b/desktop/src/features/onboarding/ui/KeyringLockedScreen.tsx @@ -0,0 +1,11 @@ +import { RecoveryScreen } from "./RecoveryScreen"; + +export function KeyringLockedScreen() { + return ( + + ); +} diff --git a/desktop/src/features/onboarding/ui/OnboardingFlow.tsx b/desktop/src/features/onboarding/ui/OnboardingFlow.tsx index 7e9618f6e..67691f0ae 100644 --- a/desktop/src/features/onboarding/ui/OnboardingFlow.tsx +++ b/desktop/src/features/onboarding/ui/OnboardingFlow.tsx @@ -7,7 +7,11 @@ import { } from "@/features/profile/hooks"; import { relayClient } from "@/shared/api/relayClient"; import { getMyRelayMembershipLookup } from "@/shared/api/relayMembers"; -import { getIdentity, importIdentity } from "@/shared/api/tauri"; +import { + getIdentity, + importIdentity, + persistCurrentIdentity, +} from "@/shared/api/tauriIdentity"; import { ACCENT_STORAGE_KEY, NEUTRAL_ACCENT, @@ -73,6 +77,7 @@ async function checkMembershipDenied(): Promise { type OnboardingFlowProps = { actions: OnboardingActions; canBackToWorkspaceSetup: boolean; + identityLost?: boolean; initialProfile: OnboardingProfileSeed; onBackToWorkspaceSetup: () => void; }; @@ -142,6 +147,7 @@ function resolveProfileSaveRecovery( export function OnboardingFlow({ actions, canBackToWorkspaceSetup, + identityLost = false, initialProfile, onBackToWorkspaceSetup, }: OnboardingFlowProps) { @@ -151,11 +157,15 @@ export function OnboardingFlow({ const profileUpdateMutation = useUpdateProfileMutation(); const { error: profileSaveError, isPending: isSavingProfile } = profileUpdateMutation; - const [currentPage, setCurrentPage] = - React.useState("profile"); + // When identity was lost (keyring cleared after migration), land the user + // directly on the import step with a recovery notice rather than profile setup. + const [currentPage, setCurrentPage] = React.useState( + identityLost ? "key-import" : "profile", + ); const [profileDraft, setProfileDraft] = React.useState(savedProfile); const [deniedPubkey, setDeniedPubkey] = React.useState(""); + const [persistError, setPersistError] = React.useState(null); const [isUploadingAvatar, setIsUploadingAvatar] = React.useState(false); const [isProfileAdvancePending, setIsProfileAdvancePending] = React.useState(false); @@ -403,6 +413,22 @@ export function OnboardingFlow({ [profileUpdateMutation, queryClient], ); + // Lost-mode "go back": persist the ephemeral key so the new identity is + // durable, then let the stage machinery (bootedLost + !identityLost) replace + // this flow with RelaunchRequiredScreen. No navigation needed here. + const handleLostModeBack = React.useCallback(async () => { + try { + const identity = await persistCurrentIdentity(); + queryClient.setQueryData(["identity"], identity); + } catch (error) { + setPersistError( + error instanceof Error + ? error.message + : "Failed to create a new identity. Please try again.", + ); + } + }, [queryClient]); + if (currentPage === "membership-denied") { return (
-

- Use your existing key -

-

- Import your Nostr private key to use that identity with Buzz. If - this key already has a profile on the relay, your name and - avatar are restored automatically. -

+ {identityLost ? ( + <> +

+ Re-import your key +

+

+ Your identity is no longer in the system keyring. Re-import + your nsec to restore it — Buzz will restart to finish + recovery. Or go back to start a new identity with a fresh + key. +

+ + ) : ( + <> +

+ Use your existing key +

+

+ Import your Nostr private key to use that identity with + Buzz. If this key already has a profile on the relay, your + name and avatar are restored automatically. +

+ + )}
+ {persistError ? ( +

+ {persistError} +

+ ) : null} + diff --git a/desktop/src/features/onboarding/ui/RecoveryScreen.tsx b/desktop/src/features/onboarding/ui/RecoveryScreen.tsx new file mode 100644 index 000000000..0fc7e9164 --- /dev/null +++ b/desktop/src/features/onboarding/ui/RecoveryScreen.tsx @@ -0,0 +1,41 @@ +import { relaunch } from "@tauri-apps/plugin-process"; + +import { useSystemColorScheme } from "@/shared/theme/useSystemColorScheme"; +import { Button } from "@/shared/ui/button"; +import { StartupWindowDragRegion } from "@/shared/ui/StartupWindowDragRegion"; + +export function RecoveryScreen({ + testId, + title, + body, +}: { + testId: string; + title: string; + body: string; +}) { + const systemColorScheme = useSystemColorScheme(); + + return ( +
+ +
+

{title}

+

{body}

+ +
+
+ ); +} diff --git a/desktop/src/features/onboarding/ui/RelaunchRequiredScreen.tsx b/desktop/src/features/onboarding/ui/RelaunchRequiredScreen.tsx new file mode 100644 index 000000000..aea0273a8 --- /dev/null +++ b/desktop/src/features/onboarding/ui/RelaunchRequiredScreen.tsx @@ -0,0 +1,11 @@ +import { RecoveryScreen } from "./RecoveryScreen"; + +export function RelaunchRequiredScreen() { + return ( + + ); +} diff --git a/desktop/src/features/projects/hooks.ts b/desktop/src/features/projects/hooks.ts index 4e1646cdd..a3ce512fb 100644 --- a/desktop/src/features/projects/hooks.ts +++ b/desktop/src/features/projects/hooks.ts @@ -2,7 +2,8 @@ import { useMutation, useQuery, useQueryClient } from "@tanstack/react-query"; import * as React from "react"; import { relayClient } from "@/shared/api/relayClient"; -import { getIdentity, signRelayEvent } from "@/shared/api/tauri"; +import { signRelayEvent } from "@/shared/api/tauri"; +import { getIdentity } from "@/shared/api/tauriIdentity"; import { getProjectLocalRepoDiff, getProjectRepoDiff, diff --git a/desktop/src/features/workspaces/ui/WelcomeSetup.tsx b/desktop/src/features/workspaces/ui/WelcomeSetup.tsx index 069f049e3..515cd0613 100644 --- a/desktop/src/features/workspaces/ui/WelcomeSetup.tsx +++ b/desktop/src/features/workspaces/ui/WelcomeSetup.tsx @@ -4,7 +4,7 @@ import { flushSync } from "react-dom"; import { getIdentity, importIdentity as tauriImportIdentity, -} from "@/shared/api/tauri"; +} from "@/shared/api/tauriIdentity"; import { NostrKeyImportForm } from "@/features/onboarding/ui/NostrKeyImportForm"; import { type OnboardingTransitionDirection, diff --git a/desktop/src/features/workspaces/useWorkspaceInit.ts b/desktop/src/features/workspaces/useWorkspaceInit.ts index d368735b6..0ba805cb7 100644 --- a/desktop/src/features/workspaces/useWorkspaceInit.ts +++ b/desktop/src/features/workspaces/useWorkspaceInit.ts @@ -1,11 +1,8 @@ import { useEffect, useRef, useState } from "react"; import { relayClient } from "@/shared/api/relayClient"; -import { - applyWorkspace, - getDefaultRelayUrl, - getIdentity, -} from "@/shared/api/tauri"; +import { applyWorkspace, getDefaultRelayUrl } from "@/shared/api/tauri"; +import { getIdentity } from "@/shared/api/tauriIdentity"; import { resetMediaCaches } from "@/shared/lib/mediaUrl"; import { clearSearchHitEventCache } from "@/app/navigation/searchHitEventCache"; import { diff --git a/desktop/src/features/workspaces/useWorkspaceUnread.ts b/desktop/src/features/workspaces/useWorkspaceUnread.ts index dd196b7b9..cbbfee518 100644 --- a/desktop/src/features/workspaces/useWorkspaceUnread.ts +++ b/desktop/src/features/workspaces/useWorkspaceUnread.ts @@ -1,6 +1,6 @@ import * as React from "react"; -import { getIdentity } from "@/shared/api/tauri"; +import { getIdentity } from "@/shared/api/tauriIdentity"; import { markWorkspaceRead } from "@/features/workspaces/workspaceMarkRead"; import { pollWorkspaceUnread } from "@/features/workspaces/workspaceUnreadObserver"; diff --git a/desktop/src/shared/api/customEmoji.ts b/desktop/src/shared/api/customEmoji.ts index de9ca13f8..2bc85aab1 100644 --- a/desktop/src/shared/api/customEmoji.ts +++ b/desktop/src/shared/api/customEmoji.ts @@ -17,7 +17,8 @@ */ import { relayClient } from "@/shared/api/relayClient"; -import { getIdentity, signRelayEvent } from "@/shared/api/tauri"; +import { signRelayEvent } from "@/shared/api/tauri"; +import { getIdentity } from "@/shared/api/tauriIdentity"; import type { RelayEvent } from "@/shared/api/types"; import type { CustomEmoji } from "@/shared/lib/remarkCustomEmoji"; diff --git a/desktop/src/shared/api/hooks.ts b/desktop/src/shared/api/hooks.ts index 5193746c9..8902175f0 100644 --- a/desktop/src/shared/api/hooks.ts +++ b/desktop/src/shared/api/hooks.ts @@ -1,6 +1,6 @@ import { useQuery } from "@tanstack/react-query"; -import { getIdentity } from "@/shared/api/tauri"; +import { getIdentity } from "@/shared/api/tauriIdentity"; export function useIdentityQuery() { return useQuery({ diff --git a/desktop/src/shared/api/relayMembers.ts b/desktop/src/shared/api/relayMembers.ts index 5dee6ef81..23feb06b2 100644 --- a/desktop/src/shared/api/relayMembers.ts +++ b/desktop/src/shared/api/relayMembers.ts @@ -1,5 +1,6 @@ import { relayClient } from "@/shared/api/relayClient"; -import { getIdentity, signRelayEvent } from "@/shared/api/tauri"; +import { signRelayEvent } from "@/shared/api/tauri"; +import { getIdentity } from "@/shared/api/tauriIdentity"; import type { RelayEvent, RelayMember, diff --git a/desktop/src/shared/api/tauri.ts b/desktop/src/shared/api/tauri.ts index 121b28d7d..1c28e7d7d 100644 --- a/desktop/src/shared/api/tauri.ts +++ b/desktop/src/shared/api/tauri.ts @@ -14,7 +14,6 @@ import type { CreateChannelInput, GetHomeFeedInput, HomeFeedResponse, - Identity, ManagedAgent, ManagedAgentBackend, RelayAgent, @@ -50,8 +49,6 @@ import type { RuntimeConfigSurface, } from "@/shared/api/types"; -type RawIdentity = { pubkey: string; display_name: string }; - type RawProfile = { pubkey: string; display_name: string | null; @@ -457,24 +454,6 @@ function fromRawUserSearchResult(user: RawUserSearchResult): UserSearchResult { }; } -export async function getIdentity(): Promise { - const identity = await invokeTauri("get_identity"); - - return { - pubkey: identity.pubkey, - displayName: identity.display_name, - }; -} - -export async function getNsec(): Promise { - return invokeTauri("get_nsec"); -} - -export async function importIdentity(nsec: string): Promise { - const raw = await invokeTauri("import_identity", { nsec }); - return { pubkey: raw.pubkey, displayName: raw.display_name }; -} - export async function getProfile(): Promise { const profile = await invokeTauri("get_profile"); return fromRawProfile(profile); diff --git a/desktop/src/shared/api/tauriIdentity.ts b/desktop/src/shared/api/tauriIdentity.ts new file mode 100644 index 000000000..d935e6855 --- /dev/null +++ b/desktop/src/shared/api/tauriIdentity.ts @@ -0,0 +1,38 @@ +import { invokeTauri } from "@/shared/api/tauri"; +import type { Identity } from "@/shared/api/types"; + +type RawIdentity = { + pubkey: string; + display_name: string; + lost?: boolean; + locked?: boolean; +}; + +function fromRawIdentity(raw: RawIdentity): Identity { + return { + pubkey: raw.pubkey, + displayName: raw.display_name, + lost: raw.lost === true, + locked: raw.locked === true, + }; +} + +export async function getIdentity(): Promise { + return fromRawIdentity(await invokeTauri("get_identity")); +} + +export async function getNsec(): Promise { + return invokeTauri("get_nsec"); +} + +export async function importIdentity(nsec: string): Promise { + return fromRawIdentity( + await invokeTauri("import_identity", { nsec }), + ); +} + +export async function persistCurrentIdentity(): Promise { + return fromRawIdentity( + await invokeTauri("persist_current_identity"), + ); +} diff --git a/desktop/src/shared/api/types.ts b/desktop/src/shared/api/types.ts index aad57a70e..2babc8e1d 100644 --- a/desktop/src/shared/api/types.ts +++ b/desktop/src/shared/api/types.ts @@ -106,6 +106,17 @@ export type AddChannelMembersResult = { export type Identity = { pubkey: string; displayName: string; + /** True when the app booted in "identity lost" recovery mode — the OS + * keyring was empty despite a prior successful migration. The frontend + * should route to nsec re-import instead of normal onboarding. + * Mutually exclusive with `locked`. */ + lost?: boolean; + /** True when the app booted with an ephemeral key because the OS keyring + * holding the real identity is UNREACHABLE (e.g. GNOME Keyring / KWallet + * locked). The real key still exists; no in-app recovery is possible — + * the user must unlock the keyring externally and relaunch. + * Mutually exclusive with `lost`. */ + locked?: boolean; }; export type Profile = { diff --git a/desktop/src/testing/e2eBridge.ts b/desktop/src/testing/e2eBridge.ts index eff1708ee..0593d1a7c 100644 --- a/desktop/src/testing/e2eBridge.ts +++ b/desktop/src/testing/e2eBridge.ts @@ -177,6 +177,12 @@ type E2eConfig = { // Event IDs that `get_event` should report as definitively not found. // Causes `useDraftRootStatus` to classify as `deleted`. deletedEventIds?: string[]; + // When true, `get_identity` returns `lost: true` until `persist_current_identity` + // or `import_identity` is called. Drives the identity-lost recovery UX in tests. + identityLost?: boolean; + // When true, `get_identity` returns `locked: true` until `import_identity` is + // called. Drives the keyring-locked screen in tests. + identityLocked?: boolean; }; relayHttpUrl?: string; relayWsUrl?: string; @@ -830,6 +836,13 @@ const OWNED_RELAY_AGENT_PUBKEY = "a1b2c3d4e5f60718293a4b5c6d7e8f90112233445566778899aabbccddeeff00"; const MOCK_IDENTITY_PUBKEY = DEFAULT_MOCK_IDENTITY.pubkey; +// Tracks whether `persist_current_identity` or `import_identity` has cleared +// the lost flag set by `mock.identityLost`. Reset to false on each fresh page +// load (module re-evaluation), so tests start in a clean state. +let mockIdentityLostCleared = false; +// Same pattern for `mock.identityLocked`. +let mockIdentityLockedCleared = false; + const mockDisplayNames = new Map([ [MOCK_IDENTITY_PUBKEY, DEFAULT_MOCK_IDENTITY.display_name], [ALICE_PUBKEY, "alice"], @@ -8101,18 +8114,43 @@ export function maybeInstallE2eTauriMocks() { }, }; } - case "get_identity": + case "get_identity": { + const isLost = + !mockIdentityLostCleared && activeConfig?.mock?.identityLost === true; + const isLocked = + !mockIdentityLockedCleared && + activeConfig?.mock?.identityLocked === true; if (identity) { return { pubkey: identity.pubkey, display_name: identity.username, + lost: false, + locked: false, }; } - return DEFAULT_MOCK_IDENTITY; + return { ...DEFAULT_MOCK_IDENTITY, lost: isLost, locked: isLocked }; + } case "get_nsec": return "nsec1mock000000000000000000000000000000000000000000000000000000"; + case "persist_current_identity": { + // Persist the ephemeral key: clears only the lost flag. The locked flag + // is cleared only by import_identity; production rejects + // persist_current_identity when the identity is in the locked state. + mockIdentityLostCleared = true; + const currentPubkey = identity?.pubkey ?? DEFAULT_MOCK_IDENTITY.pubkey; + const currentDisplayName = + identity?.username ?? DEFAULT_MOCK_IDENTITY.display_name; + return { + pubkey: currentPubkey, + display_name: currentDisplayName, + lost: false, + locked: false, + }; + } case "import_identity": + mockIdentityLostCleared = true; + mockIdentityLockedCleared = true; return importMockIdentity( (payload as { nsec?: string } | null)?.nsec ?? "", ); diff --git a/desktop/tests/e2e/identity-lost.spec.ts b/desktop/tests/e2e/identity-lost.spec.ts new file mode 100644 index 000000000..af44005e2 --- /dev/null +++ b/desktop/tests/e2e/identity-lost.spec.ts @@ -0,0 +1,122 @@ +import { hexToBytes } from "@noble/hashes/utils.js"; +import { expect, test } from "@playwright/test"; +import { nsecEncode } from "nostr-tools/nip19"; + +import { installMockBridge, TEST_IDENTITIES } from "../helpers/bridge"; + +test("lost boot opens onboarding gate directly on the key-import page", async ({ + page, +}) => { + await installMockBridge( + page, + { identityLost: true }, + { skipOnboardingSeed: true }, + ); + await page.goto("/"); + + await expect(page.getByTestId("onboarding-gate")).toBeVisible(); + await expect( + page.getByRole("heading", { name: "Re-import your key" }), + ).toBeVisible(); +}); + +test("importing a key from lost mode shows the relaunch-required screen", async ({ + page, +}) => { + await installMockBridge( + page, + { identityLost: true }, + { skipOnboardingSeed: true }, + ); + await page.goto("/"); + + await expect( + page.getByRole("heading", { name: "Re-import your key" }), + ).toBeVisible(); + + const importedNsec = nsecEncode(hexToBytes(TEST_IDENTITIES.alice.privateKey)); + await page.getByTestId("nostr-import-nsec-input").fill(importedNsec); + await expect(page.getByTestId("nostr-import-npub-preview")).toBeVisible(); + await page.getByTestId("nostr-import-submit").click(); + + await expect(page.getByTestId("relaunch-required")).toBeVisible(); +}); + +test("going back from lost mode persists the ephemeral key and shows relaunch-required", async ({ + page, +}) => { + await installMockBridge( + page, + { identityLost: true }, + { skipOnboardingSeed: true }, + ); + await page.goto("/"); + + await expect( + page.getByRole("heading", { name: "Re-import your key" }), + ).toBeVisible(); + + await page.getByRole("button", { name: "Back" }).click(); + + await expect(page.getByTestId("relaunch-required")).toBeVisible(); + await expect + .poll(() => + page.evaluate( + () => + ( + window as Window & { + __BUZZ_E2E_COMMAND_PAYLOADS__?: Array<{ command: string }>; + } + ).__BUZZ_E2E_COMMAND_PAYLOADS__?.some( + (e) => e.command === "persist_current_identity", + ) ?? false, + ), + ) + .toBe(true); +}); + +test("locked boot shows the keyring-locked screen without the onboarding gate or key-import UI", async ({ + page, +}) => { + await installMockBridge( + page, + { identityLocked: true }, + { skipOnboardingSeed: true }, + ); + await page.goto("/"); + + await expect(page.getByTestId("keyring-locked")).toBeVisible(); + await expect(page.getByTestId("onboarding-gate")).toHaveCount(0); + await expect( + page.getByRole("heading", { name: "Re-import your key" }), + ).toHaveCount(0); +}); + +test("locked screen relaunch button records the process-restart invoke", async ({ + page, +}) => { + await installMockBridge( + page, + { identityLocked: true }, + { skipOnboardingSeed: true }, + ); + await page.goto("/"); + + await expect(page.getByTestId("keyring-locked")).toBeVisible(); + await page.getByTestId("relaunch-app").click(); + + await expect + .poll(() => + page.evaluate( + () => + ( + window as Window & { + __BUZZ_E2E_COMMAND_PAYLOADS__?: Array<{ command: string }>; + } + ).__BUZZ_E2E_COMMAND_PAYLOADS__?.some( + (e) => e.command === "plugin:process|restart", + ) ?? false, + ), + ) + .toBe(true); +}); diff --git a/desktop/tests/helpers/bridge.ts b/desktop/tests/helpers/bridge.ts index d1571e6bc..51d3f1afb 100644 --- a/desktop/tests/helpers/bridge.ts +++ b/desktop/tests/helpers/bridge.ts @@ -206,6 +206,16 @@ type MockBridgeOptions = { * can exercise the "Thread deleted" label / disabled-send path. */ deletedEventIds?: string[]; + /** + * When true, `get_identity` returns `lost: true` until `persist_current_identity` + * or `import_identity` is invoked. Drives the identity-lost recovery UX in tests. + */ + identityLost?: boolean; + /** + * When true, `get_identity` returns `locked: true` until `import_identity` is + * invoked. Drives the keyring-locked screen in tests. + */ + identityLocked?: boolean; }; type BridgeOptions = {