diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index db0aea637fe..7994edda782 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -182,10 +182,18 @@ just reset # Wipe all dev state and recreate it; installed Buzz is preserved ``` Development desktop state uses separate bundle identifiers -(`xyz.block.buzz.app.dev` and per-worktree variants), a separate keyring service -(`buzz-desktop-dev`), and `~/.buzz-dev`. `just reset` removes those dev-only -locations and the local Docker volumes. It does not touch the installed app's -`xyz.block.buzz.app` data, `buzz-desktop` keyring service, or `~/.buzz` nest. +(`xyz.block.buzz.app.dev` and per-worktree variants), a separate secret-store +service (`buzz-desktop-dev`), and `~/.buzz-dev`. `just reset` removes those +dev-only locations and the local Docker volumes. It does not touch the +installed app's `xyz.block.buzz.app` data, `buzz-desktop` keyring service, or +`~/.buzz` nest. + +Debug builds keep their secrets (dev nsecs) in a `0o600` file, +`secrets..json` in the app-data dir, instead of the OS keychain — +unsigned dev binaries get a new code identity every rebuild, which would +otherwise trigger a macOS keychain password prompt on every relaunch. Set +`BUZZ_DEV_USE_KEYCHAIN=1` to opt a debug build back into the keychain. +Release builds always use the OS keychain. --- diff --git a/desktop/src-tauri/src/app_state.rs b/desktop/src-tauri/src/app_state.rs index 9cbb4444ab3..5e70b44f4a4 100644 --- a/desktop/src-tauri/src/app_state.rs +++ b/desktop/src-tauri/src/app_state.rs @@ -363,6 +363,7 @@ pub fn resolve_persisted_identity(app: &AppHandle, state: &AppState) -> Result<( #[path = "app_state_keyring.rs"] mod keyring_config; pub(crate) use keyring_config::keyring_service; +use keyring_config::{migration_marker_path, write_migration_marker}; #[path = "app_state_pending_channels.rs"] mod pending_channels; @@ -370,13 +371,6 @@ mod pending_channels; /// Keyring key name for the human identity nsec. const IDENTITY_KEY_NAME: &str = "identity"; -/// Filename of the marker written once a successful keyring migration deletes -/// the legacy `identity.key`. Its presence is the only durable signal that a -/// key once lived in the keyring — used to tell a genuine first-ever launch -/// (no key anywhere, generating is correct) from a post-migration boot whose -/// keyring is merely unreachable (the key IS in the keyring, must NOT generate). -const MIGRATION_MARKER_NAME: &str = "identity.migrated"; - /// The keyring operations the identity resolution flow needs. Abstracted so the /// corrupt-keyring recovery decision ([`recover_from_keyring`]) can be /// unit-tested against a fake without touching the live OS keyring. @@ -433,7 +427,13 @@ fn load_or_create_identity(data_dir: &std::path::Path) -> Result Result { - 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(keyring_config::migration_marker_name( - keyring_service(), - MIGRATION_MARKER_NAME, - )) -} - -/// Atomically write (and fsync) the migration-completed marker. The content is -/// irrelevant — only the file's durable existence is the signal — so a single -/// byte keeps it minimal. Atomicity + fsync guarantee that once this returns -/// `Ok`, the marker survives a crash, which is what makes deleting the legacy -/// file afterward safe. -fn write_migration_marker(marker_path: &std::path::Path) -> Result<(), String> { - use atomic_write_file::AtomicWriteFile; - - let mut file = AtomicWriteFile::open(marker_path) - .map_err(|e| format!("open migration marker for atomic write: {e}"))?; - file.write_all(b"1") - .map_err(|e| format!("write migration marker: {e}"))?; - file.commit() - .map_err(|e| format!("commit migration marker: {e}")) + let storage = persist_imported_identity_impl(store, keys, legacy_path, data_dir)?; + // See load_or_create_identity: the debug file backend reports local-file. + if store.is_file_backed() && storage == IdentityStorage::SystemKeyring { + return Ok(IdentityStorage::LocalFile); + } + Ok(storage) } /// Generate a fresh identity, persist it through the store, return it. diff --git a/desktop/src-tauri/src/app_state_keyring.rs b/desktop/src-tauri/src/app_state_keyring.rs index 68d24e87f58..cc16305d2a2 100644 --- a/desktop/src-tauri/src/app_state_keyring.rs +++ b/desktop/src-tauri/src/app_state_keyring.rs @@ -25,6 +25,38 @@ pub(super) fn migration_marker_name(service: &str, default_name: &str) -> String } } +/// Filename of the marker written once a successful keyring migration deletes +/// the legacy `identity.key`. Its presence is the only durable signal that a +/// key once lived in the keyring — used to tell a genuine first-ever launch +/// (no key anywhere, generating is correct) from a post-migration boot whose +/// keyring is merely unreachable (the key IS in the keyring, must NOT generate). +const MIGRATION_MARKER_NAME: &str = "identity.migrated"; + +/// Path of the migration-completed marker within `data_dir`. +pub(super) fn migration_marker_path(data_dir: &std::path::Path) -> std::path::PathBuf { + data_dir.join(migration_marker_name( + keyring_service(), + MIGRATION_MARKER_NAME, + )) +} + +/// Atomically write (and fsync) the migration-completed marker. The content is +/// irrelevant — only the file's durable existence is the signal — so a single +/// byte keeps it minimal. Atomicity + fsync guarantee that once this returns +/// `Ok`, the marker survives a crash, which is what makes deleting the legacy +/// file afterward safe. +pub(super) fn write_migration_marker(marker_path: &std::path::Path) -> Result<(), String> { + use atomic_write_file::AtomicWriteFile; + use std::io::Write; + + let mut file = AtomicWriteFile::open(marker_path) + .map_err(|e| format!("open migration marker for atomic write: {e}"))?; + file.write_all(b"1") + .map_err(|e| format!("write migration marker: {e}"))?; + file.commit() + .map_err(|e| format!("commit migration marker: {e}")) +} + #[cfg(test)] mod tests { use super::{dev_keyring_service, migration_marker_name}; diff --git a/desktop/src-tauri/src/lib.rs b/desktop/src-tauri/src/lib.rs index 428aa4d2a78..0d63e19b50b 100644 --- a/desktop/src-tauri/src/lib.rs +++ b/desktop/src-tauri/src/lib.rs @@ -251,6 +251,10 @@ pub fn run() { // init_nest_dir is called early here (normally it runs inside // run_boot_migrations) so reset::run_boot_reset can call nest_dir(). let reset_outcome = if let Ok(data_dir) = app_handle.path().app_data_dir() { + // Must precede the first SecretStore::shared() call (the boot + // reset below) so debug builds resolve the file backend. + #[cfg(debug_assertions)] + crate::secret_store::init_file_backend_dir(&data_dir); let is_dev_for_reset = data_dir .file_name() .and_then(|n| n.to_str()) diff --git a/desktop/src-tauri/src/managed_agents/storage.rs b/desktop/src-tauri/src/managed_agents/storage.rs index 652bb9b9ea8..6e8e6cefb26 100644 --- a/desktop/src-tauri/src/managed_agents/storage.rs +++ b/desktop/src-tauri/src/managed_agents/storage.rs @@ -461,6 +461,12 @@ pub fn migrate_agent_keys_to_dev_service(app: &tauri::AppHandle) { if !cfg!(feature = "system-keyring") || keyring_service() != "buzz-desktop-dev" { return; } + // The dev file backend deliberately starts empty (no keychain + // migration); reading the prod keychain here would reintroduce the + // password prompt that backend exists to avoid. + if crate::secret_store::SecretStore::shared(keyring_service()).is_file_backed() { + return; + } // Read the JSON store for pubkeys only — we want every instance // record without running hydrate_keys (which would try the dev diff --git a/desktop/src-tauri/src/reset.rs b/desktop/src-tauri/src/reset.rs index 18ddd80eb8d..b80e295458a 100644 --- a/desktop/src-tauri/src/reset.rs +++ b/desktop/src-tauri/src/reset.rs @@ -121,7 +121,9 @@ pub(crate) fn run_boot_reset(app_data_dir: &Path) -> ResetOutcome { .map(crate::migration::is_dev_data_dir_name) .unwrap_or(false); - let store = crate::secret_store::SecretStore::keyring(crate::app_state::keyring_service()); + // shared() (not keyring()) so the wipe targets the build's active + // backend — the debug file backend when it is in play. + let store = crate::secret_store::SecretStore::shared(crate::app_state::keyring_service()); let home_dir = dirs::home_dir(); let legacy_dir = crate::migration::legacy_app_data_dir(app_data_dir); let nest_dir = crate::managed_agents::nest_dir(); @@ -130,7 +132,7 @@ pub(crate) fn run_boot_reset(app_data_dir: &Path) -> ResetOutcome { app_data_dir, legacy_app_data_dir: legacy_dir, nest_dir, - keychain: &store, + keychain: store, home_dir, is_dev, }; diff --git a/desktop/src-tauri/src/secret_store.rs b/desktop/src-tauri/src/secret_store.rs index 43854761b50..f1ddd8f30cb 100644 --- a/desktop/src-tauri/src/secret_store.rs +++ b/desktop/src-tauri/src/secret_store.rs @@ -1,18 +1,33 @@ -//! OS keyring access for desktop nsec private keys. +//! Secret storage for desktop nsec private keys. //! -//! All secrets are stored as a single JSON blob under one keychain entry -//! (service = the store's service name, username = `"secrets"`). This means -//! exactly one OS prompt per process lifetime regardless of how many keys are -//! stored — the same pattern used by Goose. +//! All secrets are stored as a single JSON blob under one entry +//! (service = the store's service name, username = `"secrets"`). Two +//! backends share that blob format: //! -//! The chosen backend is selected at compile time by the per-target feature in -//! `Cargo.toml`. On macOS the legacy `keyring` crate (SecKeychain API) is used -//! for the blob entry so that signed release builds and unsigned dev builds -//! share the same store. DPK (Data Protection Keychain) is used only by the -//! one-time migration path that reads old per-key entries written by #1264. -//! Windows and Linux use the `keyring` crate directly. The `system-keyring` -//! feature gates the whole store; when it is off, [`SecretStore`] is unusable -//! and callers fall back to their own `0o600` file storage. +//! - **OS keyring** — release builds, and debug builds with +//! `BUZZ_DEV_USE_KEYCHAIN=1`. One keychain entry means exactly one OS +//! prompt per process lifetime regardless of how many keys are stored — +//! the same pattern used by Goose. On macOS the legacy `keyring` crate +//! (SecKeychain API) is used for the blob entry so that signed release +//! builds and keychain-opted dev builds share the same store. DPK (Data +//! Protection Keychain) is used only by the one-time migration path that +//! reads old per-key entries written by #1264. Windows and Linux use the +//! `keyring` crate directly. +//! - **Plain file** — debug builds by default: `secrets..json` +//! (0o600) in the app-data dir. Unsigned dev binaries get a fresh code +//! identity on every rebuild, which invalidates the keychain item's +//! "Always Allow" ACL and made macOS demand the login password on every +//! `tauri dev` relaunch; a file sidesteps the keychain entirely. The file +//! store deliberately starts empty — there is NO migration from the old +//! `buzz-desktop-dev` keychain item. Dev keys are cheap to re-import, and +//! a one-shot migration would live on as dead code. Old dev keychain +//! items are simply never read again (every legacy-keychain path +//! short-circuits in file mode); clean them up manually with +//! `security delete-generic-password -s buzz-desktop-dev -a secrets`. +//! +//! The `system-keyring` feature gates the whole store; when it is off, +//! [`SecretStore`] is unusable and callers fall back to their own `0o600` +//! file storage. //! //! The store is deliberately NOT on any env-read path. `BUZZ_PRIVATE_KEY` //! resolution for harnessed agents and CI is handled upstream (an env @@ -21,7 +36,6 @@ //! divergent-behavior trap. use std::collections::HashMap; -use std::path::PathBuf; use std::sync::Mutex; /// Result of probing the keyring before a migration: distinguishes "reachable @@ -43,195 +57,67 @@ pub enum KeyringProbe { /// as a JSON map under this name within the service. const BLOB_KEY: &str = "secrets"; -// ── Interprocess advisory lock ───────────────────────────────────────────── -// -// Two concurrent Buzz processes (e.g. the signed DMG build and an unsigned dev -// build via `just staging`) share the same OS keychain blob because the -// service name `"buzz-desktop"` is a constant — it does not key off the bundle -// identifier. Each process holds its own in-memory cache, so without an -// interprocess lock a warm-cache write in process A drops keys added by process -// B between A's last cache-warming read and A's write. -// -// The fix: `mutate_blob` acquires an exclusive advisory file lock, then always -// performs a fresh `read_blob_raw()` inside the lock, applies the mutation, -// writes back, and releases. The cache is still updated after a successful -// write, so same-process reads remain fast. The lock is file-based at a fixed -// per-user path `/tmp/buzz-keychain--.lock` on Unix — a path -// that is invariant to `$TMPDIR`/process environment, so both the GUI-launched -// signed DMG and a terminal-launched dev build always take the same lock. - -/// Return the path of the advisory lockfile for `service`. -/// -/// The path is `/tmp/buzz-keychain--.lock` on Unix — a -/// deterministic per-user path that is invariant to `$TMPDIR`/process -/// environment. Both a GUI-launched signed DMG (`launchd`, env-stripped) and a -/// terminal-launched dev build resolve `/tmp` to the same inode, so they -/// contend on the same lockfile and achieve mutual exclusion. -/// -/// On Windows the same name used for the kernel mutex is derived from the -/// lockfile path, so the service-keyed uniqueness is preserved. -fn blob_lockfile_path(service: &str) -> PathBuf { - #[cfg(unix)] - { - // Use the real UID so distinct users get distinct lockfiles. - // SAFETY: getuid() is always safe on Unix — it never fails. - let uid = unsafe { libc::getuid() }; - PathBuf::from(format!("/tmp/buzz-keychain-{uid}-{service}.lock")) - } - #[cfg(not(unix))] - { - // Windows: no lockfile used (named mutex instead); this path is only - // used to derive the mutex name and for test assertions. - std::env::temp_dir().join(format!("buzz-keychain-{service}.lock")) - } -} - -/// Acquire an exclusive advisory file lock for the blob identified by `service`. -/// -/// Opens (or creates) the lockfile and blocks until the lock is acquired. -/// Returns the open `File`; the lock is released when the file is dropped. -/// -/// On non-Unix/non-Windows platforms this is a no-op that returns a stub. -#[cfg(feature = "system-keyring")] -fn acquire_blob_lock(service: &str) -> Result { - let path = blob_lockfile_path(service); - BlobLockGuard::acquire(&path) -} - -/// RAII guard that holds an exclusive advisory file lock. -/// -/// On Unix, implemented via `flock(2)` on a lockfile in the system temp dir. -/// On Windows, implemented via a named kernel mutex (cross-process, no file I/O -/// needed). The Windows mutex handle is released on drop. -#[cfg(feature = "system-keyring")] -struct BlobLockGuard { - /// The open lockfile. Never read — held purely for RAII: closing the fd - /// releases the `flock(LOCK_EX)` on Unix. - #[cfg(unix)] - #[allow(dead_code)] - file: std::fs::File, - #[cfg(windows)] - mutex_handle: windows_sys::Win32::Foundation::HANDLE, -} - +#[path = "secret_store_lock.rs"] +mod lock; #[cfg(feature = "system-keyring")] -impl BlobLockGuard { - fn acquire(path: &std::path::Path) -> Result { - #[cfg(unix)] - { - let file = std::fs::OpenOptions::new() - .create(true) - .truncate(false) - .write(true) - .open(path) - .map_err(|e| format!("blob lock open {}: {e}", path.display()))?; - use std::os::unix::io::AsRawFd; - // LOCK_EX blocks until the lock is acquired (no LOCK_NB). - let ret = unsafe { libc::flock(file.as_raw_fd(), libc::LOCK_EX) }; - if ret != 0 { - let err = std::io::Error::last_os_error(); - return Err(format!("blob lock flock: {err}")); - } - return Ok(BlobLockGuard { file }); - } - - #[cfg(windows)] - { - // Named kernel mutexes are cross-process on Windows — no lockfile - // needed. Derive a unique mutex name from the lockfile path so - // distinct services get distinct mutexes. - let name_str = format!( - "Local\\BuzzKeychain-{}", - path.file_stem() - .and_then(|s| s.to_str()) - .unwrap_or("default") - ); - // Encode as null-terminated UTF-16. - let name_wide: Vec = name_str - .encode_utf16() - .chain(std::iter::once(0u16)) - .collect(); - use windows_sys::Win32::Foundation::WAIT_OBJECT_0; - use windows_sys::Win32::Security::SECURITY_ATTRIBUTES; - use windows_sys::Win32::System::Threading::{ - CreateMutexW, WaitForSingleObject, INFINITE, - }; - // CreateMutexW: lpMutexAttributes = null (default security), - // bInitialOwner = FALSE (0), lpName = our mutex name. - let handle = unsafe { - CreateMutexW( - std::ptr::null::(), - 0, - name_wide.as_ptr(), - ) - }; - // HANDLE = *mut c_void; null means creation failed. - if handle.is_null() { - let err = std::io::Error::last_os_error(); - return Err(format!("blob lock CreateMutexW: {err}")); - } - let wait_result = unsafe { WaitForSingleObject(handle, INFINITE) }; - if wait_result != WAIT_OBJECT_0 { - // Also accept WAIT_ABANDONED (0x80) — previous holder crashed; - // the mutex is still acquired and we own it. - if wait_result != windows_sys::Win32::Foundation::WAIT_ABANDONED { - let err = std::io::Error::last_os_error(); - unsafe { windows_sys::Win32::Foundation::CloseHandle(handle) }; - return Err(format!( - "blob lock WaitForSingleObject: {wait_result} / {err}" - )); - } - } - return Ok(BlobLockGuard { - mutex_handle: handle, - }); - } - - // Fallback for exotic platforms: no-op lock (only Unix/Windows ship). - #[allow(unreachable_code)] - Err("blob lock: unsupported platform".to_string()) - } -} - -#[cfg(feature = "system-keyring")] -impl Drop for BlobLockGuard { - fn drop(&mut self) { - #[cfg(unix)] - { - // Dropping `self.file` closes the fd, which releases flock on Unix. - // Nothing explicit needed. - } - #[cfg(windows)] - { - unsafe { - windows_sys::Win32::System::Threading::ReleaseMutex(self.mutex_handle); - windows_sys::Win32::Foundation::CloseHandle(self.mutex_handle); - } - } - } -} - -// ── End interprocess advisory lock ──────────────────────────────────────── - -/// An OS keyring, addressed by service name. All secrets are stored in a -/// single JSON blob entry (one OS prompt per process lifetime). +use lock::acquire_blob_lock; + +#[path = "secret_store_file.rs"] +mod file_backend; +#[cfg(debug_assertions)] +pub use file_backend::init_file_backend_dir; +use file_backend::{backend_for, SecretBackend}; +#[cfg(all(debug_assertions, feature = "system-keyring"))] +use file_backend::{read_blob_raw_file, write_blob_raw_file}; + +/// Secret storage addressed by service name. All secrets are stored in a +/// single JSON blob (one OS prompt per process lifetime on the keyring +/// backend; a `0o600` file on the debug file backend). pub struct SecretStore { service: String, + backend: SecretBackend, /// In-memory cache of the deserialized blob. `None` means "not yet loaded". cache: Mutex>>, } impl SecretStore { - /// Keyring-backed store under `service`. The active platform backend - /// (apple-native / windows-native / sync-secret-service) is chosen at - /// compile time. + /// Keyring-backed store under `service`, unconditionally — never the + /// debug file backend. For the build's default backend use + /// [`SecretStore::shared`]. The active platform keyring (apple-native / + /// windows-native / sync-secret-service) is chosen at compile time. pub fn keyring(service: impl Into) -> Self { SecretStore { service: service.into(), + backend: SecretBackend::Keyring, cache: Mutex::new(None), } } + /// Store for `service` on the build's default backend: the debug file + /// backend when active, the OS keyring otherwise. + pub fn for_service(service: impl Into) -> Self { + let service = service.into(); + let backend = backend_for(&service); + SecretStore { + service, + backend, + cache: Mutex::new(None), + } + } + + /// Whether this store is on the debug file backend. Callers use this to + /// report `local-file` storage to the UI and to skip keychain-only work. + pub(crate) fn is_file_backed(&self) -> bool { + #[cfg(debug_assertions)] + { + matches!(self.backend, SecretBackend::File(_)) + } + #[cfg(not(debug_assertions))] + { + false + } + } + /// Return a process-global `SecretStore` for `service`. All callers with /// the same service name share one instance — and therefore one in-memory /// cache and one mutex — so concurrent blob read-modify-write operations @@ -242,7 +128,7 @@ impl SecretStore { pub fn shared(service: &'static str) -> &'static SecretStore { use std::sync::OnceLock; static INSTANCE: OnceLock = OnceLock::new(); - INSTANCE.get_or_init(|| SecretStore::keyring(service)) + INSTANCE.get_or_init(|| SecretStore::for_service(service)) } } @@ -334,19 +220,18 @@ impl SecretStore { Ok(Some(map)) } - /// Read the raw blob bytes from the keychain. `Ok(None)` = not found. + /// Read the raw blob bytes from the active backend. `Ok(None)` = not found. /// - /// Always uses the legacy keyring crate on macOS so that signed and - /// unsigned (dev) builds share the same store. DPK is only used by - /// `migrate_legacy_key` to read old per-key entries written by #1264. - #[cfg(all(feature = "system-keyring", target_os = "macos"))] - fn read_blob_raw(&self) -> Result>, String> { - self.read_blob_raw_keyring() - } - - #[cfg(all(feature = "system-keyring", not(target_os = "macos")))] + /// The keyring path always uses the legacy keyring crate on macOS so that + /// signed and keychain-opted dev builds share the same store. DPK is only + /// used by `migrate_legacy_key` to read old per-key entries from #1264. + #[cfg(feature = "system-keyring")] fn read_blob_raw(&self) -> Result>, String> { - self.read_blob_raw_keyring() + match &self.backend { + SecretBackend::Keyring => self.read_blob_raw_keyring(), + #[cfg(debug_assertions)] + SecretBackend::File(path) => read_blob_raw_file(path), + } } /// Read blob via the legacy `keyring` crate (Windows, Linux, or macOS dev @@ -451,15 +336,14 @@ impl SecretStore { } } - /// Always uses the legacy keyring crate on macOS — see `read_blob_raw`. - #[cfg(all(feature = "system-keyring", target_os = "macos"))] - fn write_blob_raw(&self, bytes: &[u8]) -> Result<(), String> { - self.write_blob_raw_keyring(bytes) - } - - #[cfg(all(feature = "system-keyring", not(target_os = "macos")))] + /// Write the raw blob bytes to the active backend — see `read_blob_raw`. + #[cfg(feature = "system-keyring")] fn write_blob_raw(&self, bytes: &[u8]) -> Result<(), String> { - self.write_blob_raw_keyring(bytes) + match &self.backend { + SecretBackend::Keyring => self.write_blob_raw_keyring(bytes), + #[cfg(debug_assertions)] + SecretBackend::File(path) => write_blob_raw_file(path, bytes), + } } #[cfg(feature = "system-keyring")] @@ -480,6 +364,9 @@ impl SecretStore { Ok(Some(map)) => { if map.contains_key(key) { KeyringProbe::Present + } else if self.is_file_backed() { + // File mode never consults the legacy keychain. + KeyringProbe::ReachableButEmpty } else { // Blob exists but key absent — still check old per-key // entries so a partial migration (e.g. identity migrated @@ -487,6 +374,7 @@ impl SecretStore { self.probe_legacy_key(key) } } + Ok(None) if self.is_file_backed() => KeyringProbe::ReachableButEmpty, // No blob yet — check old per-key entries so callers that // gate `load()` on `Present` still trigger migration. Ok(None) => self.probe_legacy_key(key), @@ -553,6 +441,9 @@ impl SecretStore { Ok(Some(map)) => { if let Some(value) = map.get(key) { Ok(Some(value.clone())) + } else if self.is_file_backed() { + // File mode never consults the legacy keychain. + Ok(None) } else { // Blob exists but key absent — attempt migration from old // per-key entry. migrate_legacy_key writes the result into @@ -560,6 +451,7 @@ impl SecretStore { self.migrate_legacy_key(key) } } + Ok(None) if self.is_file_backed() => Ok(None), Ok(None) => { // No blob yet — attempt one-time migration from old per-key // DPK entry (macOS) or return Ok(None) (other platforms). @@ -758,6 +650,20 @@ impl SecretStore { { let _lock = acquire_blob_lock(&self.service)?; + // File mode: deleting the file IS the complete wipe — nothing + // reads the keychain in this mode, so legacy entries are inert. + #[cfg(debug_assertions)] + if let SecretBackend::File(path) = &self.backend { + match std::fs::remove_file(path) { + Ok(()) => {} + Err(e) if e.kind() == std::io::ErrorKind::NotFound => {} + Err(e) => return Err(format!("secrets file delete {}: {e}", path.display())), + } + let mut guard = self.cache.lock().unwrap_or_else(|e| e.into_inner()); + *guard = None; + return Ok(()); + } + // Step 1: read current blob keys (best-effort; no entry = empty set). let blob_keys: Vec = match self.read_blob_raw() { Ok(Some(bytes)) => { @@ -848,6 +754,13 @@ impl SecretStore { pub fn verify_fully_wiped(&self) -> bool { #[cfg(feature = "system-keyring")] { + // File mode: the secrets file is the only shape `load()` can + // consume — its absence is the whole proof. + #[cfg(debug_assertions)] + if let SecretBackend::File(path) = &self.backend { + return !path.exists(); + } + // 1. Main blob must be absent. match self.read_blob_raw() { Ok(None) => {} @@ -904,6 +817,11 @@ impl SecretStore { self.mutate_blob(|map| { map.remove(key); })?; + // File mode never reads the keychain, so there is nothing a + // leftover legacy entry could resurrect — skip the cleanup. + if self.is_file_backed() { + return Ok(()); + } // Best-effort: also delete any old per-key entry for this key to // prevent resurrection on the next probe/load (migration path). #[cfg(target_os = "macos")] @@ -930,6 +848,7 @@ mod tests { fn with_cache(service: &str, cache: Option>) -> Self { SecretStore { service: service.to_string(), + backend: SecretBackend::Keyring, cache: Mutex::new(cache), } } @@ -1049,64 +968,6 @@ mod tests { let _ = reader.delete("agent_b"); } - #[test] - fn test_blob_lockfile_path_is_in_tmp_with_uid() { - // The lockfile must be at a deterministic per-user path under /tmp — - // invariant to $TMPDIR — so both a GUI-launched DMG (env-stripped by - // launchd) and a terminal-launched dev build resolve the same inode and - // achieve mutual exclusion. - let path = blob_lockfile_path("buzz-desktop"); - #[cfg(unix)] - { - let uid = unsafe { libc::getuid() }; - assert!( - path.starts_with("/tmp"), - "lockfile {path:?} must start with /tmp (not $TMPDIR)" - ); - let name = path - .file_name() - .and_then(|n| n.to_str()) - .unwrap_or_default(); - assert!( - name.contains(&uid.to_string()), - "lockfile {path:?} must contain uid {uid}" - ); - assert!( - name.contains("buzz-keychain"), - "lockfile name must contain 'buzz-keychain'" - ); - } - #[cfg(not(unix))] - { - assert!( - path.file_name() - .and_then(|n| n.to_str()) - .is_some_and(|n| n.contains("buzz-keychain")), - "lockfile name must contain 'buzz-keychain'" - ); - } - } - - #[test] - fn test_blob_lock_acquire_and_release() { - // Verify the advisory lock can be acquired and released without errors. - // This exercises the real flock/mutex path on the current platform. - let guard = acquire_blob_lock("buzz-test-lock-smoke"); - assert!( - guard.is_ok(), - "advisory lock acquire must succeed: {:?}", - guard.err() - ); - // Drop the guard — lock is released. A second acquire must succeed. - drop(guard); - let guard2 = acquire_blob_lock("buzz-test-lock-smoke"); - assert!( - guard2.is_ok(), - "advisory lock re-acquire after release must succeed: {:?}", - guard2.err() - ); - } - #[ignore = "requires real OS keychain (run locally)"] #[test] fn mutate_blob_does_not_advance_cache_on_write_failure() { diff --git a/desktop/src-tauri/src/secret_store_file.rs b/desktop/src-tauri/src/secret_store_file.rs new file mode 100644 index 00000000000..deaa1659cab --- /dev/null +++ b/desktop/src-tauri/src/secret_store_file.rs @@ -0,0 +1,295 @@ +//! Debug-only file backend for [`super::SecretStore`]. +//! +//! Stores the same JSON blob the keyring backend uses in a `0o600` +//! `secrets..json` in the app-data dir. See the `secret_store` +//! module docs for why (unsigned dev binaries invalidate the keychain ACL +//! on every rebuild) and for the deliberate no-migration policy. + +use std::path::PathBuf; + +/// Where a [`super::SecretStore`]'s blob physically lives. The `File` variant +/// is debug-only so release binaries are keyring-only by construction. +pub(super) enum SecretBackend { + Keyring, + /// Blob at this exact path (`secrets..json` in the app-data dir). + #[cfg(debug_assertions)] + File(PathBuf), +} + +/// App-data dir for the debug file backend, recorded once at boot. +#[cfg(debug_assertions)] +static FILE_BACKEND_DIR: std::sync::OnceLock = std::sync::OnceLock::new(); + +/// Record the app-data dir the debug file backend stores secrets under. +/// Must run before the first [`super::SecretStore::shared`] call — `lib.rs` +/// setup does, ahead of `run_boot_reset`. Later calls are no-ops. +#[cfg(debug_assertions)] +pub fn init_file_backend_dir(dir: &std::path::Path) { + let _ = FILE_BACKEND_DIR.set(dir.to_path_buf()); +} + +/// Pure backend decision for debug builds: keyring when the user opted back +/// in via `BUZZ_DEV_USE_KEYCHAIN=1` or the file dir was never initialized, +/// otherwise the per-service secrets file (namespaced because `just dev` and +/// a main-checkout standalone share one app-data dir with different services). +#[cfg(debug_assertions)] +fn select_backend( + use_keychain_env: Option<&str>, + file_dir: Option<&std::path::Path>, + service: &str, +) -> SecretBackend { + if use_keychain_env == Some("1") { + return SecretBackend::Keyring; + } + match file_dir { + Some(dir) => SecretBackend::File(dir.join(format!("secrets.{service}.json"))), + None => SecretBackend::Keyring, + } +} + +#[cfg(debug_assertions)] +pub(super) fn backend_for(service: &str) -> SecretBackend { + let env = std::env::var("BUZZ_DEV_USE_KEYCHAIN").ok(); + let backend = select_backend( + env.as_deref(), + FILE_BACKEND_DIR.get().map(|p| p.as_path()), + service, + ); + if matches!(backend, SecretBackend::Keyring) && env.as_deref() != Some("1") { + eprintln!( + "buzz-desktop: file backend dir not initialized; \ + using OS keychain for service {service}" + ); + } + backend +} + +#[cfg(not(debug_assertions))] +pub(super) fn backend_for(_service: &str) -> SecretBackend { + SecretBackend::Keyring +} + +/// Read the file backend's blob. `Ok(None)` = no file yet (fresh store — +/// deliberately no fallback to the old dev keychain item). +#[cfg(all(debug_assertions, feature = "system-keyring"))] +pub(super) fn read_blob_raw_file(path: &std::path::Path) -> Result>, String> { + match std::fs::read(path) { + Ok(bytes) => Ok(Some(bytes)), + Err(e) if e.kind() == std::io::ErrorKind::NotFound => Ok(None), + Err(e) => Err(format!("secrets file read {}: {e}", path.display())), + } +} + +/// Atomically replace the file backend's blob: write a `0o600` sibling tmp +/// file, fsync, rename over the final path. A crash mid-write can never leave +/// a truncated secrets file. +#[cfg(all(debug_assertions, feature = "system-keyring"))] +pub(super) fn write_blob_raw_file(path: &std::path::Path, bytes: &[u8]) -> Result<(), String> { + use std::io::Write; + + if let Some(parent) = path.parent() { + std::fs::create_dir_all(parent) + .map_err(|e| format!("secrets dir create {}: {e}", parent.display()))?; + } + let file_name = path + .file_name() + .and_then(|n| n.to_str()) + .unwrap_or("secrets.json"); + let tmp = path.with_file_name(format!("{file_name}.tmp")); + + let mut opts = std::fs::OpenOptions::new(); + opts.write(true).create(true).truncate(true); + #[cfg(unix)] + { + use std::os::unix::fs::OpenOptionsExt; + opts.mode(0o600); + } + let mut file = opts + .open(&tmp) + .map_err(|e| format!("secrets tmp open {}: {e}", tmp.display()))?; + file.write_all(bytes) + .map_err(|e| format!("secrets tmp write: {e}"))?; + file.sync_all() + .map_err(|e| format!("secrets tmp fsync: {e}"))?; + drop(file); + std::fs::rename(&tmp, path).map_err(|e| format!("secrets file rename: {e}")) +} + +#[cfg(all(test, debug_assertions, feature = "system-keyring"))] +mod tests { + use super::super::{KeyringProbe, SecretStore}; + use super::*; + use std::sync::Mutex; + + // Test-only constructor: file backend at an explicit path, bypassing + // the process-global FILE_BACKEND_DIR. + impl SecretStore { + fn file_at(service: &str, path: std::path::PathBuf) -> Self { + SecretStore { + service: service.to_string(), + backend: SecretBackend::File(path), + cache: Mutex::new(None), + } + } + } + + fn tmp_store(service: &str) -> (tempfile::TempDir, SecretStore) { + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join(format!("secrets.{service}.json")); + let store = SecretStore::file_at(service, path); + (dir, store) + } + + #[test] + fn select_backend_uses_service_namespaced_file_when_dir_set() { + let dir = std::path::Path::new("/tmp/buzz-test-data"); + match select_backend(None, Some(dir), "buzz-desktop-dev.slug") { + SecretBackend::File(p) => { + assert_eq!(p, dir.join("secrets.buzz-desktop-dev.slug.json")) + } + SecretBackend::Keyring => panic!("expected file backend"), + } + } + + #[test] + fn select_backend_env_escape_hatch_forces_keyring() { + let dir = std::path::Path::new("/tmp/buzz-test-data"); + assert!(matches!( + select_backend(Some("1"), Some(dir), "buzz-desktop-dev"), + SecretBackend::Keyring + )); + // Any value other than "1" does not opt back into the keychain. + assert!(matches!( + select_backend(Some("0"), Some(dir), "buzz-desktop-dev"), + SecretBackend::File(_) + )); + } + + #[test] + fn select_backend_without_dir_falls_back_to_keyring() { + assert!(matches!( + select_backend(None, None, "buzz-desktop-dev"), + SecretBackend::Keyring + )); + } + + #[test] + fn is_file_backed_reflects_backend() { + let (_dir, store) = tmp_store("buzz-test-file-backed"); + assert!(store.is_file_backed()); + assert!(!SecretStore::keyring("buzz-test-file-backed").is_file_backed()); + } + + #[test] + fn file_roundtrip_store_load_delete() { + let (_dir, store) = tmp_store("buzz-test-file-roundtrip"); + store.store("identity", "nsec1aaa").unwrap(); + store.store("agent:abc", "nsec1bbb").unwrap(); + assert_eq!( + store.load("identity").unwrap(), + Some("nsec1aaa".to_string()) + ); + assert_eq!( + store.load("agent:abc").unwrap(), + Some("nsec1bbb".to_string()) + ); + store.delete("agent:abc").unwrap(); + assert_eq!(store.load("agent:abc").unwrap(), None); + assert_eq!( + store.load("identity").unwrap(), + Some("nsec1aaa".to_string()) + ); + } + + #[test] + fn missing_key_probe_and_load_never_consult_legacy_keychain() { + // A fresh (empty) file store must report reachable-but-empty and + // Ok(None) without falling through to the legacy keychain + // migration paths — file mode never touches the OS keychain. + let (_dir, store) = tmp_store("buzz-test-file-no-legacy"); + assert_eq!(store.probe("identity"), KeyringProbe::ReachableButEmpty); + assert_eq!(store.load("identity").unwrap(), None); + // Same when a blob exists but the key is absent. + store.store("other", "v").unwrap(); + assert_eq!(store.probe("identity"), KeyringProbe::ReachableButEmpty); + assert_eq!(store.load("identity").unwrap(), None); + assert_eq!(store.probe("other"), KeyringProbe::Present); + } + + #[cfg(unix)] + #[test] + fn file_is_created_0o600() { + use std::os::unix::fs::PermissionsExt; + let (dir, store) = tmp_store("buzz-test-file-perms"); + store.store("identity", "nsec1aaa").unwrap(); + let path = dir.path().join("secrets.buzz-test-file-perms.json"); + let mode = std::fs::metadata(&path).unwrap().permissions().mode(); + assert_eq!(mode & 0o777, 0o600, "secrets file must be 0o600"); + } + + #[test] + fn write_is_atomic_leaves_no_tmp_residue() { + let (dir, store) = tmp_store("buzz-test-file-atomic"); + store.store("identity", "nsec1aaa").unwrap(); + store.store("agent:abc", "nsec1bbb").unwrap(); + let names: Vec = std::fs::read_dir(dir.path()) + .unwrap() + .map(|e| e.unwrap().file_name().to_string_lossy().into_owned()) + .collect(); + assert_eq!( + names, + vec!["secrets.buzz-test-file-atomic.json".to_string()], + "only the final secrets file may remain: {names:?}" + ); + } + + #[test] + fn corrupt_file_fails_closed_and_is_preserved() { + let (dir, store) = tmp_store("buzz-test-file-corrupt"); + let path = dir.path().join("secrets.buzz-test-file-corrupt.json"); + std::fs::write(&path, b"not json").unwrap(); + assert!(store.load("identity").is_err()); + assert_eq!(store.probe("identity"), KeyringProbe::Unreachable); + // A store() must not clobber the corrupt file — the fresh read + // inside mutate_blob fails first. + assert!(store.store("identity", "nsec1aaa").is_err()); + assert_eq!(std::fs::read(&path).unwrap(), b"not json"); + } + + #[test] + fn two_stores_same_path_observe_each_others_writes() { + // CI-runnable port of the cross-process stale-cache race test: + // two instances (= two processes with separate caches) on one + // file must never drop each other's keys. + let dir = tempfile::tempdir().unwrap(); + let svc = "buzz-test-file-race"; + let path = dir.path().join(format!("secrets.{svc}.json")); + let store_a = SecretStore::file_at(svc, path.clone()); + store_a.store("k1", "v1").unwrap(); // warms A's cache + let store_b = SecretStore::file_at(svc, path.clone()); + store_b.store("k2", "v2").unwrap(); + store_a.store("k3", "v3").unwrap(); // must re-read, not drop k2 + let reader = SecretStore::file_at(svc, path); + for (k, v) in [("k1", "v1"), ("k2", "v2"), ("k3", "v3")] { + assert_eq!( + reader.load(k).unwrap(), + Some(v.to_string()), + "{k} must survive" + ); + } + } + + #[test] + fn delete_all_removes_file_and_verifies_wiped() { + let (dir, store) = tmp_store("buzz-test-file-wipe"); + store.store("identity", "nsec1aaa").unwrap(); + assert!(!store.verify_fully_wiped()); + store.delete_all_with_legacy_cleanup().unwrap(); + let path = dir.path().join("secrets.buzz-test-file-wipe.json"); + assert!(!path.exists(), "secrets file must be deleted"); + assert!(store.verify_fully_wiped()); + // Idempotent on an already-absent file. + store.delete_all_with_legacy_cleanup().unwrap(); + assert_eq!(store.load("identity").unwrap(), None); + } +} diff --git a/desktop/src-tauri/src/secret_store_lock.rs b/desktop/src-tauri/src/secret_store_lock.rs new file mode 100644 index 00000000000..8868afa5a4a --- /dev/null +++ b/desktop/src-tauri/src/secret_store_lock.rs @@ -0,0 +1,232 @@ +//! Interprocess advisory lock for the [`super::SecretStore`] blob. +//! +//! Two concurrent Buzz processes (e.g. the signed DMG build and an unsigned dev +//! build via `just staging`) share the same OS keychain blob because the +//! service name `"buzz-desktop"` is a constant — it does not key off the bundle +//! identifier. Each process holds its own in-memory cache, so without an +//! interprocess lock a warm-cache write in process A drops keys added by process +//! B between A's last cache-warming read and A's write. +//! +//! The fix: `mutate_blob` acquires an exclusive advisory file lock, then always +//! performs a fresh `read_blob_raw()` inside the lock, applies the mutation, +//! writes back, and releases. The cache is still updated after a successful +//! write, so same-process reads remain fast. The lock is file-based at a fixed +//! per-user path `/tmp/buzz-keychain--.lock` on Unix — a path +//! that is invariant to `$TMPDIR`/process environment, so both the GUI-launched +//! signed DMG and a terminal-launched dev build always take the same lock. + +use std::path::PathBuf; + +/// Return the path of the advisory lockfile for `service`. +/// +/// The path is `/tmp/buzz-keychain--.lock` on Unix — a +/// deterministic per-user path that is invariant to `$TMPDIR`/process +/// environment. Both a GUI-launched signed DMG (`launchd`, env-stripped) and a +/// terminal-launched dev build resolve `/tmp` to the same inode, so they +/// contend on the same lockfile and achieve mutual exclusion. +/// +/// On Windows the same name used for the kernel mutex is derived from the +/// lockfile path, so the service-keyed uniqueness is preserved. +fn blob_lockfile_path(service: &str) -> PathBuf { + #[cfg(unix)] + { + // Use the real UID so distinct users get distinct lockfiles. + // SAFETY: getuid() is always safe on Unix — it never fails. + let uid = unsafe { libc::getuid() }; + PathBuf::from(format!("/tmp/buzz-keychain-{uid}-{service}.lock")) + } + #[cfg(not(unix))] + { + // Windows: no lockfile used (named mutex instead); this path is only + // used to derive the mutex name and for test assertions. + std::env::temp_dir().join(format!("buzz-keychain-{service}.lock")) + } +} + +/// Acquire an exclusive advisory file lock for the blob identified by `service`. +/// +/// Opens (or creates) the lockfile and blocks until the lock is acquired. +/// Returns the open `File`; the lock is released when the file is dropped. +/// +/// On non-Unix/non-Windows platforms this is a no-op that returns a stub. +#[cfg(feature = "system-keyring")] +pub(super) fn acquire_blob_lock(service: &str) -> Result { + let path = blob_lockfile_path(service); + BlobLockGuard::acquire(&path) +} + +/// RAII guard that holds an exclusive advisory file lock. +/// +/// On Unix, implemented via `flock(2)` on a lockfile in the system temp dir. +/// On Windows, implemented via a named kernel mutex (cross-process, no file I/O +/// needed). The Windows mutex handle is released on drop. +#[cfg(feature = "system-keyring")] +pub(super) struct BlobLockGuard { + /// The open lockfile. Never read — held purely for RAII: closing the fd + /// releases the `flock(LOCK_EX)` on Unix. + #[cfg(unix)] + #[allow(dead_code)] + file: std::fs::File, + #[cfg(windows)] + mutex_handle: windows_sys::Win32::Foundation::HANDLE, +} + +#[cfg(feature = "system-keyring")] +impl BlobLockGuard { + fn acquire(path: &std::path::Path) -> Result { + #[cfg(unix)] + { + let file = std::fs::OpenOptions::new() + .create(true) + .truncate(false) + .write(true) + .open(path) + .map_err(|e| format!("blob lock open {}: {e}", path.display()))?; + use std::os::unix::io::AsRawFd; + // LOCK_EX blocks until the lock is acquired (no LOCK_NB). + let ret = unsafe { libc::flock(file.as_raw_fd(), libc::LOCK_EX) }; + if ret != 0 { + let err = std::io::Error::last_os_error(); + return Err(format!("blob lock flock: {err}")); + } + return Ok(BlobLockGuard { file }); + } + + #[cfg(windows)] + { + // Named kernel mutexes are cross-process on Windows — no lockfile + // needed. Derive a unique mutex name from the lockfile path so + // distinct services get distinct mutexes. + let name_str = format!( + "Local\\BuzzKeychain-{}", + path.file_stem() + .and_then(|s| s.to_str()) + .unwrap_or("default") + ); + // Encode as null-terminated UTF-16. + let name_wide: Vec = name_str + .encode_utf16() + .chain(std::iter::once(0u16)) + .collect(); + use windows_sys::Win32::Foundation::WAIT_OBJECT_0; + use windows_sys::Win32::Security::SECURITY_ATTRIBUTES; + use windows_sys::Win32::System::Threading::{ + CreateMutexW, WaitForSingleObject, INFINITE, + }; + // CreateMutexW: lpMutexAttributes = null (default security), + // bInitialOwner = FALSE (0), lpName = our mutex name. + let handle = unsafe { + CreateMutexW( + std::ptr::null::(), + 0, + name_wide.as_ptr(), + ) + }; + // HANDLE = *mut c_void; null means creation failed. + if handle.is_null() { + let err = std::io::Error::last_os_error(); + return Err(format!("blob lock CreateMutexW: {err}")); + } + let wait_result = unsafe { WaitForSingleObject(handle, INFINITE) }; + if wait_result != WAIT_OBJECT_0 { + // Also accept WAIT_ABANDONED (0x80) — previous holder crashed; + // the mutex is still acquired and we own it. + if wait_result != windows_sys::Win32::Foundation::WAIT_ABANDONED { + let err = std::io::Error::last_os_error(); + unsafe { windows_sys::Win32::Foundation::CloseHandle(handle) }; + return Err(format!( + "blob lock WaitForSingleObject: {wait_result} / {err}" + )); + } + } + return Ok(BlobLockGuard { + mutex_handle: handle, + }); + } + + // Fallback for exotic platforms: no-op lock (only Unix/Windows ship). + #[allow(unreachable_code)] + Err("blob lock: unsupported platform".to_string()) + } +} + +#[cfg(feature = "system-keyring")] +impl Drop for BlobLockGuard { + fn drop(&mut self) { + #[cfg(unix)] + { + // Dropping `self.file` closes the fd, which releases flock on Unix. + // Nothing explicit needed. + } + #[cfg(windows)] + { + unsafe { + windows_sys::Win32::System::Threading::ReleaseMutex(self.mutex_handle); + windows_sys::Win32::Foundation::CloseHandle(self.mutex_handle); + } + } + } +} + +#[cfg(all(test, feature = "system-keyring"))] +mod tests { + use super::*; + + #[test] + fn test_blob_lockfile_path_is_in_tmp_with_uid() { + // The lockfile must be at a deterministic per-user path under /tmp — + // invariant to $TMPDIR — so both a GUI-launched DMG (env-stripped by + // launchd) and a terminal-launched dev build resolve the same inode and + // achieve mutual exclusion. + let path = blob_lockfile_path("buzz-desktop"); + #[cfg(unix)] + { + let uid = unsafe { libc::getuid() }; + assert!( + path.starts_with("/tmp"), + "lockfile {path:?} must start with /tmp (not $TMPDIR)" + ); + let name = path + .file_name() + .and_then(|n| n.to_str()) + .unwrap_or_default(); + assert!( + name.contains(&uid.to_string()), + "lockfile {path:?} must contain uid {uid}" + ); + assert!( + name.contains("buzz-keychain"), + "lockfile name must contain 'buzz-keychain'" + ); + } + #[cfg(not(unix))] + { + assert!( + path.file_name() + .and_then(|n| n.to_str()) + .is_some_and(|n| n.contains("buzz-keychain")), + "lockfile name must contain 'buzz-keychain'" + ); + } + } + + #[test] + fn test_blob_lock_acquire_and_release() { + // Verify the advisory lock can be acquired and released without errors. + // This exercises the real flock/mutex path on the current platform. + let guard = acquire_blob_lock("buzz-test-lock-smoke"); + assert!( + guard.is_ok(), + "advisory lock acquire must succeed: {:?}", + guard.err() + ); + // Drop the guard — lock is released. A second acquire must succeed. + drop(guard); + let guard2 = acquire_blob_lock("buzz-test-lock-smoke"); + assert!( + guard2.is_ok(), + "advisory lock re-acquire after release must succeed: {:?}", + guard2.err() + ); + } +} diff --git a/desktop/src/features/onboarding/ui/BackupStep.tsx b/desktop/src/features/onboarding/ui/BackupStep.tsx index 8793eda7942..274c15daee1 100644 --- a/desktop/src/features/onboarding/ui/BackupStep.tsx +++ b/desktop/src/features/onboarding/ui/BackupStep.tsx @@ -162,7 +162,7 @@ export function BackupStep({ identityStorage === "system-keyring" ? "Buzz keeps your identity key in your system keychain. Your computer may ask for your password when Buzz needs to read the key." : identityStorage === "local-file" - ? "Your system keychain wasn’t available, so Buzz keeps your identity key in a private file on this device." + ? "Buzz keeps your identity key in a private file on this device." : "Buzz keeps your identity key protected on this device. Make a separate backup in case you lose access."; const storageTitle = identityStorage === "system-keyring" @@ -174,7 +174,7 @@ export function BackupStep({ identityStorage === "system-keyring" ? "Buzz keeps your identity key in your system keychain." : identityStorage === "local-file" - ? "Buzz keeps your identity key in a private file on this device because the system keychain wasn’t available." + ? "Buzz keeps your identity key in a private file on this device." : "Your identity key is protected on this device."; if (optionsExpanded) { diff --git a/scripts/instance-env.sh b/scripts/instance-env.sh index bc185d98f18..544b930eff5 100755 --- a/scripts/instance-env.sh +++ b/scripts/instance-env.sh @@ -48,19 +48,30 @@ if git rev-parse --is-inside-work-tree &>/dev/null; then # tauri-plugin-single-instance or the app data directory. if [[ "${BUZZ_SHARE_IDENTITY:-0}" == "1" ]]; then KEYRING_SERVICE="buzz-desktop-dev" + # Debug builds store secrets in a per-service file (see + # desktop/src-tauri/src/secret_store.rs); prefer it over the + # legacy OS keychain item, which dev builds no longer write. + SECRETS_FILE="" KEYRING_BLOB="" case "$(uname -s)" in Darwin) - if command -v security &>/dev/null; then + SECRETS_FILE="$HOME/Library/Application Support/xyz.block.buzz.app.dev/secrets.${KEYRING_SERVICE}.json" + if [[ ! -f "$SECRETS_FILE" ]] && command -v security &>/dev/null; then KEYRING_BLOB="$(security find-generic-password -s "$KEYRING_SERVICE" -a secrets -w 2>/dev/null || true)" fi ;; Linux) - if command -v secret-tool &>/dev/null; then + SECRETS_FILE="${XDG_DATA_HOME:-$HOME/.local/share}/xyz.block.buzz.app.dev/secrets.${KEYRING_SERVICE}.json" + if [[ ! -f "$SECRETS_FILE" ]] && command -v secret-tool &>/dev/null; then KEYRING_BLOB="$(secret-tool lookup service "$KEYRING_SERVICE" username secrets target default 2>/dev/null || true)" fi ;; esac + # BUZZ_DEV_USE_KEYCHAIN=1 puts the app on the keychain, so the + # file may hold a stale identity — only prefer it in file mode. + if [[ "${BUZZ_DEV_USE_KEYCHAIN:-0}" != "1" && -f "$SECRETS_FILE" ]]; then + KEYRING_BLOB="$(cat "$SECRETS_FILE")" + fi KEYRING_IDENTITY="$(printf '%s' "$KEYRING_BLOB" | python3 -c 'import json, sys; value = json.load(sys.stdin).get("identity", ""); print(value if isinstance(value, str) else "")' 2>/dev/null || true)" CANONICAL_KEY="$HOME/Library/Application Support/xyz.block.buzz.app.dev/identity.key" @@ -76,7 +87,7 @@ if git rev-parse --is-inside-work-tree &>/dev/null; then if [[ -n "$SHARED_IDENTITY" ]]; then export BUZZ_PRIVATE_KEY="$SHARED_IDENTITY" else - echo "⚠ BUZZ_SHARE_IDENTITY=1 but no identity found in keyring service $KEYRING_SERVICE, at $CANONICAL_KEY, or at $LEGACY_CANONICAL_KEY — run Buzz from repo root first" >&2 + echo "⚠ BUZZ_SHARE_IDENTITY=1 but no identity found in $SECRETS_FILE, keyring service $KEYRING_SERVICE, $CANONICAL_KEY, or $LEGACY_CANONICAL_KEY — run Buzz from repo root first" >&2 fi fi