mirror of
https://github.com/block/buzz.git
synced 2026-11-12 12:22:06 +00:00
fix(desktop): store dev secrets in a 0o600 file instead of the keychain
Unsigned dev binaries get a new 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. Debug builds now default SecretStore to a file backend: secrets.<service>.json (0o600, atomic tmp+rename writes) in the app-data dir, guarded by the existing per-service flock. Release builds are keyring-only by construction (the File variant is cfg(debug_assertions)). BUZZ_DEV_USE_KEYCHAIN=1 opts a debug build back into the keychain. The file store deliberately starts empty — 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). Every legacy-keychain path short-circuits in file mode, including the prod->dev agent-key copy, so the OS keychain is never touched. First launch after this change shows the identity-recovery screen once; re-import the dev nsec or use BUZZ_PRIVATE_KEY. BUZZ_SHARE_IDENTITY now reads the secrets file first (skipped when BUZZ_DEV_USE_KEYCHAIN=1 to avoid exporting a stale identity), and the in-app reset wipes the file. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Max Lampert <maxwell@squareup.com>
This commit is contained in:
@@ -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.<service>.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.
|
||||
|
||||
---
|
||||
|
||||
|
||||
@@ -433,7 +433,13 @@ fn load_or_create_identity(data_dir: &std::path::Path) -> Result<ResolvedIdentit
|
||||
}
|
||||
|
||||
let store = crate::secret_store::SecretStore::shared(keyring_service());
|
||||
resolve_identity_with_store(store, &legacy_path, data_dir)
|
||||
let mut resolved = resolve_identity_with_store(store, &legacy_path, data_dir)?;
|
||||
// The debug file backend stores in a plain file — reporting
|
||||
// "system-keyring" to the UI would be a lie.
|
||||
if store.is_file_backed() && resolved.storage == IdentityStorage::SystemKeyring {
|
||||
resolved.storage = IdentityStorage::LocalFile;
|
||||
}
|
||||
Ok(resolved)
|
||||
}
|
||||
|
||||
/// Identity resolution over an [`IdentityKeyStore`] seam. Split from
|
||||
@@ -871,7 +877,12 @@ pub(crate) fn persist_imported_identity(
|
||||
legacy_path: &std::path::Path,
|
||||
data_dir: &std::path::Path,
|
||||
) -> Result<IdentityStorage, String> {
|
||||
persist_imported_identity_impl(store, keys, legacy_path, data_dir)
|
||||
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)
|
||||
}
|
||||
|
||||
/// Path of the migration-completed marker within `data_dir`.
|
||||
|
||||
@@ -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())
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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,
|
||||
};
|
||||
|
||||
@@ -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.<service>.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
|
||||
@@ -213,25 +228,162 @@ impl Drop for BlobLockGuard {
|
||||
|
||||
// ── 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).
|
||||
/// 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"))]
|
||||
fn read_blob_raw_file(path: &std::path::Path) -> Result<Option<Vec<u8>>, 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"))]
|
||||
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}"))
|
||||
}
|
||||
|
||||
/// Where a [`SecretStore`]'s blob physically lives. The `File` variant is
|
||||
/// debug-only so release binaries are keyring-only by construction.
|
||||
enum SecretBackend {
|
||||
Keyring,
|
||||
/// Blob at this exact path (`secrets.<service>.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<PathBuf> = std::sync::OnceLock::new();
|
||||
|
||||
/// Record the app-data dir the debug file backend stores secrets under.
|
||||
/// Must run before the first [`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)]
|
||||
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))]
|
||||
fn backend_for(_service: &str) -> SecretBackend {
|
||||
SecretBackend::Keyring
|
||||
}
|
||||
|
||||
/// 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<Option<HashMap<String, String>>>,
|
||||
}
|
||||
|
||||
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<String>) -> 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<String>) -> 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 +394,7 @@ impl SecretStore {
|
||||
pub fn shared(service: &'static str) -> &'static SecretStore {
|
||||
use std::sync::OnceLock;
|
||||
static INSTANCE: OnceLock<SecretStore> = OnceLock::new();
|
||||
INSTANCE.get_or_init(|| SecretStore::keyring(service))
|
||||
INSTANCE.get_or_init(|| SecretStore::for_service(service))
|
||||
}
|
||||
}
|
||||
|
||||
@@ -334,19 +486,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"))]
|
||||
/// 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<Option<Vec<u8>>, String> {
|
||||
self.read_blob_raw_keyring()
|
||||
}
|
||||
|
||||
#[cfg(all(feature = "system-keyring", not(target_os = "macos")))]
|
||||
fn read_blob_raw(&self) -> Result<Option<Vec<u8>>, 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 +602,14 @@ impl SecretStore {
|
||||
}
|
||||
}
|
||||
|
||||
/// Always uses the legacy keyring crate on macOS — see `read_blob_raw`.
|
||||
#[cfg(all(feature = "system-keyring", 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)
|
||||
}
|
||||
|
||||
#[cfg(all(feature = "system-keyring", not(target_os = "macos")))]
|
||||
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 +630,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 +640,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 +707,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 +717,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 +916,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<String> = match self.read_blob_raw() {
|
||||
Ok(Some(bytes)) => {
|
||||
@@ -848,6 +1020,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 +1083,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 +1114,7 @@ mod tests {
|
||||
fn with_cache(service: &str, cache: Option<HashMap<String, String>>) -> Self {
|
||||
SecretStore {
|
||||
service: service.to_string(),
|
||||
backend: SecretBackend::Keyring,
|
||||
cache: Mutex::new(cache),
|
||||
}
|
||||
}
|
||||
@@ -1266,6 +1451,185 @@ mod tests {
|
||||
let _ = store2.delete(key);
|
||||
}
|
||||
|
||||
// ── Debug file backend (no OS keychain; CI-runnable) ───────────────────
|
||||
|
||||
#[cfg(debug_assertions)]
|
||||
mod file_backend {
|
||||
use super::*;
|
||||
|
||||
// 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<String> = 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);
|
||||
}
|
||||
}
|
||||
|
||||
#[ignore = "requires real OS keychain (run locally)"]
|
||||
#[test]
|
||||
fn delete_all_with_legacy_cleanup_removes_per_key_identity() {
|
||||
|
||||
@@ -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) {
|
||||
|
||||
@@ -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
|
||||
|
||||
|
||||
Reference in New Issue
Block a user