From 38edd28336bfeb6106b4a7b88da0f8ab807dd3cd Mon Sep 17 00:00:00 2001 From: Taylor Ho Date: Wed, 29 Jul 2026 17:08:46 -0700 Subject: [PATCH] feat(desktop): explain system keychain protection during onboarding - Track whether the active identity is stored in the system keyring, local fallback file, environment, or ephemeral memory. - Expose storage metadata through the Tauri identity response and shared TypeScript identity model. - Add a responsive first information card beside the two backup actions without revealing boot-time identity creation. - Show keychain password education only when secure storage succeeded and accurate neutral copy for fallback states. - Extend identity persistence tests and split storage types into focused modules to preserve file-size limits. Co-authored-by: Taylor Ho Signed-off-by: Taylor Ho --- desktop/src-tauri/src/app_state.rs | 97 +++++++++---------- desktop/src-tauri/src/app_state_tests.rs | 12 +-- desktop/src-tauri/src/commands/identity.rs | 28 ++++-- .../src/commands/identity_key_backup_tests.rs | 18 ++-- desktop/src-tauri/src/identity_storage.rs | 62 ++++++++++++ desktop/src-tauri/src/lib.rs | 1 + desktop/src-tauri/src/models.rs | 2 + .../src/features/onboarding/ui/BackupStep.tsx | 53 ++++++++-- .../onboarding/ui/MachineOnboardingFlow.tsx | 7 ++ desktop/src/shared/api/identityTypes.ts | 28 ++++++ desktop/src/shared/api/tauriIdentity.ts | 4 +- desktop/src/shared/api/types.ts | 20 +--- 12 files changed, 231 insertions(+), 101 deletions(-) create mode 100644 desktop/src-tauri/src/identity_storage.rs create mode 100644 desktop/src/shared/api/identityTypes.ts diff --git a/desktop/src-tauri/src/app_state.rs b/desktop/src-tauri/src/app_state.rs index 94d162e62..abce86202 100644 --- a/desktop/src-tauri/src/app_state.rs +++ b/desktop/src-tauri/src/app_state.rs @@ -2,7 +2,7 @@ use std::{ collections::HashMap, io::Write, sync::{ - atomic::{AtomicBool, AtomicU16}, + atomic::{AtomicBool, AtomicU16, AtomicU8}, Arc, Mutex, }, }; @@ -13,10 +13,15 @@ use tauri::{AppHandle, Manager}; use tokio::sync::Mutex as AsyncMutex; use crate::huddle::HuddleState; +pub(crate) use crate::identity_storage::{IdentityStorage, RecoveryState, ResolvedIdentity}; use crate::managed_agents::config_bridge::SessionConfigCache; use crate::managed_agents::{ManagedAgentPairRuntime, ManagedAgentRuntimeKey}; + pub struct AppState { pub keys: Mutex, + /// Durable backend holding `keys`. Updated after the key write and before + /// recovery flags are cleared so `get_identity` reports a consistent state. + pub(crate) identity_storage: AtomicU8, pub http_client: reqwest::Client, /// A no-redirect client for authenticated relay media fetches (download, /// clipboard copy, snapshot, editor). Every caller pre-validates the URL @@ -178,19 +183,20 @@ pub fn build_media_fetch_client() -> reqwest::Result { pub fn build_app_state() -> AppState { // Env var takes precedence (dev/CI). If absent, resolve_persisted_identity() // in setup() will replace the ephemeral placeholder with a persisted key. - let keys = match identity_from_env() { + let (keys, identity_storage) = match identity_from_env() { Some(keys) => { eprintln!( "buzz-desktop: configured identity pubkey {}", keys.public_key().to_hex() ); - keys + (keys, IdentityStorage::Environment) } - None => Keys::generate(), + None => (Keys::generate(), IdentityStorage::Ephemeral), }; AppState { keys: Mutex::new(keys), + identity_storage: AtomicU8::new(identity_storage as u8), http_client: reqwest::Client::builder() .resolve("localhost", std::net::SocketAddr::from(([127, 0, 0, 1], 0))) .pool_idle_timeout(std::time::Duration::from_secs(10)) @@ -366,9 +372,13 @@ pub fn resolve_persisted_identity(app: &AppHandle, state: &AppState) -> Result<( std::fs::create_dir_all(&data_dir).map_err(|e| format!("create app data dir: {e}"))?; 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; + // Write keys and storage before setting the recovery flags (Release) so + // any thread that reads a flag as false with Acquire sees consistent data. + { + let mut active_keys = state.keys.lock().map_err(|e| e.to_string())?; + *active_keys = resolved.keys; + state.set_identity_storage(resolved.storage); + } state.identity_lost.store( resolved.recovery == RecoveryState::Lost, std::sync::atomic::Ordering::Release, @@ -394,26 +404,6 @@ 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. @@ -465,6 +455,7 @@ fn load_or_create_identity(data_dir: &std::path::Path) -> Result Result<(), String> { +) -> Result { match persist_identity_to_keyring(store, keys, legacy_path, data_dir) { - Ok(()) => Ok(()), + Ok(()) => Ok(IdentityStorage::SystemKeyring), Err(e) => { eprintln!( "buzz-desktop: keyring write failed during import ({e}), \ falling back to identity.key" ); - save_key_file(legacy_path, keys) + save_key_file(legacy_path, keys)?; + Ok(IdentityStorage::LocalFile) } } } @@ -892,7 +897,7 @@ pub(crate) fn persist_imported_identity( keys: &Keys, legacy_path: &std::path::Path, data_dir: &std::path::Path, -) -> Result<(), String> { +) -> Result { persist_imported_identity_impl(store, keys, legacy_path, data_dir) } @@ -920,15 +925,6 @@ fn write_migration_marker(marker_path: &std::path::Path) -> Result<(), String> { .map_err(|e| format!("commit migration marker: {e}")) } -/// 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, -} - /// Generate a fresh identity, persist it through the store, return it. /// /// On a keyring-backed persist no file is written, so a later @@ -940,9 +936,10 @@ fn generate_and_persist( store: &impl IdentityKeyStore, legacy_path: &std::path::Path, data_dir: &std::path::Path, -) -> Result { +) -> Result<(Keys, IdentityStorage), String> { let keys = Keys::generate(); - if let PersistBackend::Keyring = store_key_preferring_keyring(store, &keys, legacy_path)? { + let storage = store_key_preferring_keyring(store, &keys, legacy_path)?; + if storage == IdentityStorage::SystemKeyring { let marker_path = migration_marker_path(data_dir); if let Err(e) = write_migration_marker(&marker_path) { eprintln!( @@ -956,7 +953,7 @@ fn generate_and_persist( "buzz-desktop: generated and saved identity pubkey {}", keys.public_key().to_hex() ); - Ok(keys) + Ok((keys, storage)) } /// Persist `keys` through the store, silently falling back to the `0o600` file @@ -968,17 +965,17 @@ fn store_key_preferring_keyring( store: &impl IdentityKeyStore, keys: &Keys, legacy_path: &std::path::Path, -) -> Result { +) -> Result { let nsec = keys .secret_key() .to_bech32() .map_err(|e| format!("encode nsec: {e}"))?; match store.store(IDENTITY_KEY_NAME, &nsec) { - Ok(()) => Ok(PersistBackend::Keyring), + Ok(()) => Ok(IdentityStorage::SystemKeyring), Err(keyring_err) => { eprintln!("buzz-desktop: keyring write failed ({keyring_err}), using file fallback"); save_key_file(legacy_path, keys)?; - Ok(PersistBackend::File) + Ok(IdentityStorage::LocalFile) } } } diff --git a/desktop/src-tauri/src/app_state_tests.rs b/desktop/src-tauri/src/app_state_tests.rs index 485dfaea1..751bcf22e 100644 --- a/desktop/src-tauri/src/app_state_tests.rs +++ b/desktop/src-tauri/src/app_state_tests.rs @@ -484,7 +484,7 @@ fn fresh_keyring_generate_writes_marker() { 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!(!legacy_path.exists() && resolved.storage == IdentityStorage::SystemKeyring); assert!(migration_marker_path(dir.path()).exists()); assert_eq!( store @@ -541,7 +541,10 @@ fn fresh_generate_keyring_failure_falls_back_to_file_without_marker() { 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()); + assert!( + !migration_marker_path(dir.path()).exists() + && resolved.storage == IdentityStorage::LocalFile + ); } // ── New tests for the three defects fixed in this PR ───────────────────── @@ -786,10 +789,7 @@ fn persist_imported_identity_falls_back_to_file_on_keyring_failure() { 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" - ); + assert_eq!(result.unwrap(), IdentityStorage::LocalFile); // Key is recoverable from the file on next boot. let from_file = load_key_file(&legacy_path).unwrap(); diff --git a/desktop/src-tauri/src/commands/identity.rs b/desktop/src-tauri/src/commands/identity.rs index 94e23f744..8c5cc6b3b 100644 --- a/desktop/src-tauri/src/commands/identity.rs +++ b/desktop/src-tauri/src/commands/identity.rs @@ -43,6 +43,7 @@ pub fn get_identity(state: State<'_, AppState>) -> Result Ok(IdentityInfo { pubkey: pubkey_hex, display_name, + storage: state.identity_storage().as_str().to_string(), lost, locked, reset_failed, @@ -380,7 +381,7 @@ pub async fn import_identity( 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 pubkey = commit_imported_identity(&state, &data_dir, keys, |keys| { + let (pubkey, storage) = commit_imported_identity(&state, &data_dir, keys, |keys| { // 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. @@ -397,6 +398,7 @@ pub async fn import_identity( Ok(IdentityInfo { pubkey: pubkey_hex, display_name, + storage: storage.as_str().to_string(), lost: false, locked: false, reset_failed: false, @@ -426,18 +428,22 @@ fn commit_imported_identity( state: &AppState, data_dir: &std::path::Path, keys: nostr::Keys, - persist: impl FnOnce(&nostr::Keys) -> Result<(), String>, -) -> Result { + persist: impl FnOnce(&nostr::Keys) -> Result, +) -> Result<(nostr::PublicKey, crate::app_state::IdentityStorage), String> { // Capture the previous pubkey up front for post-commit cleanup. let previous_pubkey = state.keys.lock().map_err(|e| e.to_string())?.public_key(); - persist(&keys)?; + let storage = persist(&keys)?; // 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; + { + let mut active_keys = state.keys.lock().map_err(|e| e.to_string())?; + *active_keys = keys; + state.set_identity_storage(storage); + } // Clear both recovery flags — an import is valid in either lost or // keyring-locked state and resolves both. In the locked case the @@ -462,7 +468,7 @@ fn commit_imported_identity( ); } - Ok(pubkey) + Ok((pubkey, storage)) } /// Make the current ephemeral identity durable by persisting it to the OS @@ -508,11 +514,12 @@ pub async fn persist_current_identity( 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)?; + let storage = + 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. + // Keys are already the live identity. Record where the durable write + // landed before clearing identity_lost. + state.set_identity_storage(storage); state .identity_lost .store(false, std::sync::atomic::Ordering::Release); @@ -524,6 +531,7 @@ pub async fn persist_current_identity( Ok(IdentityInfo { pubkey: pubkey_hex, display_name, + storage: storage.as_str().to_string(), lost: false, locked: false, reset_failed: false, diff --git a/desktop/src-tauri/src/commands/identity_key_backup_tests.rs b/desktop/src-tauri/src/commands/identity_key_backup_tests.rs index 0b31f44ba..90fb4e1f9 100644 --- a/desktop/src-tauri/src/commands/identity_key_backup_tests.rs +++ b/desktop/src-tauri/src/commands/identity_key_backup_tests.rs @@ -1,5 +1,5 @@ use super::{create_and_persist_backup_with_log_n, verify_ncryptsec_backup_inner}; -use crate::app_state::build_app_state; +use crate::app_state::{build_app_state, IdentityStorage}; use nostr::Keys; /// Fast scrypt tier for tests; production uses BACKUP_LOG_N (18), covered @@ -174,19 +174,21 @@ fn successful_import_removes_stale_backup_after_commit() { let new_keys = Keys::generate(); let backup_present_at_persist = std::cell::Cell::new(false); let _guard = state.identity_mutation.lock().unwrap(); - let pubkey = super::commit_imported_identity(&state, dir.path(), new_keys.clone(), |_| { - // Ordering probe: the old backup must still exist while - // persistence is running (cleanup has not happened yet). - backup_present_at_persist.set(backup_path.exists()); - Ok(()) - }) - .unwrap(); + let (pubkey, storage) = + super::commit_imported_identity(&state, dir.path(), new_keys.clone(), |_| { + // Ordering probe: the old backup must still exist while + // persistence is running (cleanup has not happened yet). + backup_present_at_persist.set(backup_path.exists()); + Ok(IdentityStorage::SystemKeyring) + }) + .unwrap(); assert!( backup_present_at_persist.get(), "cleanup must not precede persist" ); assert_eq!(pubkey, new_keys.public_key()); + assert_eq!(storage, IdentityStorage::SystemKeyring); assert_eq!( state.keys.lock().unwrap().public_key(), new_keys.public_key() diff --git a/desktop/src-tauri/src/identity_storage.rs b/desktop/src-tauri/src/identity_storage.rs new file mode 100644 index 000000000..b39c1a033 --- /dev/null +++ b/desktop/src-tauri/src/identity_storage.rs @@ -0,0 +1,62 @@ +use nostr::Keys; + +use crate::app_state::AppState; + +/// Durable location of the active human identity. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +#[repr(u8)] +pub(crate) enum IdentityStorage { + Ephemeral = 0, + SystemKeyring = 1, + LocalFile = 2, + Environment = 3, +} + +impl IdentityStorage { + pub(crate) fn as_str(self) -> &'static str { + match self { + Self::Ephemeral => "ephemeral", + Self::SystemKeyring => "system-keyring", + Self::LocalFile => "local-file", + Self::Environment => "environment", + } + } + + fn from_u8(value: u8) -> Self { + match value { + 1 => Self::SystemKeyring, + 2 => Self::LocalFile, + 3 => Self::Environment, + _ => Self::Ephemeral, + } + } +} + +impl AppState { + pub(crate) fn identity_storage(&self) -> IdentityStorage { + IdentityStorage::from_u8( + self.identity_storage + .load(std::sync::atomic::Ordering::Acquire), + ) + } + + pub(crate) fn set_identity_storage(&self, storage: IdentityStorage) { + self.identity_storage + .store(storage as u8, std::sync::atomic::Ordering::Release); + } +} + +/// Recovery state produced by identity resolution. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub(crate) enum RecoveryState { + None, + Lost, + KeyringLocked, +} + +/// Identity and persistence metadata produced by startup resolution. +pub(crate) struct ResolvedIdentity { + pub(crate) keys: Keys, + pub(crate) recovery: RecoveryState, + pub(crate) storage: IdentityStorage, +} diff --git a/desktop/src-tauri/src/lib.rs b/desktop/src-tauri/src/lib.rs index 783298c91..cc80719b8 100644 --- a/desktop/src-tauri/src/lib.rs +++ b/desktop/src-tauri/src/lib.rs @@ -8,6 +8,7 @@ mod egress_guard; mod event_sync; mod events; mod huddle; +mod identity_storage; mod key_backup; mod managed_agents; mod media_proxy; diff --git a/desktop/src-tauri/src/models.rs b/desktop/src-tauri/src/models.rs index 1d9747bc2..3f04d3d7a 100644 --- a/desktop/src-tauri/src/models.rs +++ b/desktop/src-tauri/src/models.rs @@ -6,6 +6,8 @@ use serde::{Deserialize, Deserializer, Serialize}; pub struct IdentityInfo { pub pubkey: String, pub display_name: String, + /// Durable location of the active identity key. + pub storage: 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. diff --git a/desktop/src/features/onboarding/ui/BackupStep.tsx b/desktop/src/features/onboarding/ui/BackupStep.tsx index 5b9b6c7a0..86127525f 100644 --- a/desktop/src/features/onboarding/ui/BackupStep.tsx +++ b/desktop/src/features/onboarding/ui/BackupStep.tsx @@ -10,6 +10,7 @@ import { import * as React from "react"; import { getNsec } from "@/shared/api/tauriIdentity"; +import type { IdentityStorage } from "@/shared/api/types"; import { cn } from "@/shared/lib/cn"; import { writeTextToClipboard } from "@/shared/lib/clipboard"; import { Button } from "@/shared/ui/button"; @@ -48,6 +49,7 @@ export function backupNextDisabled(): boolean { type BackupStepProps = { direction: OnboardingTransitionDirection; + identityStorage?: IdentityStorage; onBack: () => void; onNext: () => void; onOpenPasswordBackup: () => void; @@ -64,6 +66,7 @@ type BackupStepProps = { */ export function BackupStep({ direction, + identityStorage, onBack, onNext, onOpenPasswordBackup, @@ -152,6 +155,24 @@ export function BackupStep({ () => Array.from({ length: nsec?.length ?? 63 }, () => "•").join("\u200b"), [nsec], ); + const storageMessage = + identityStorage === "system-keyring" + ? { + title: "Protected by your system keychain", + description: + "Buzz uses your system keychain to protect your identity key. Your system may ask for your computer password when Buzz accesses it.", + } + : identityStorage === "local-file" + ? { + title: "Stored on this device", + description: + "Your system keychain wasn’t available, so Buzz stores your identity key in a private file on this device.", + } + : { + title: "Protected on this device", + description: + "Keep a separate backup so you can restore your identity if you lose access to this device.", + }; if (optionsExpanded) { return ( @@ -171,18 +192,37 @@ export function BackupStep({

-
+
+
+
+