mirror of
https://github.com/block/buzz.git
synced 2026-10-27 12:40:30 +00:00
perf(desktop): consolidate keychain secrets into a single blob entry (#1267)
Signed-off-by: Will Pfleger <pfleger.will@gmail.com> Co-authored-by: npub1mn7jgtj4w2pd0g0zeuhxsa6jy6p0rewxz4kujt98my82ahfmp72sxjexk7 <dcfd242e557282d7a1e2cf2e6877522682f1e5c6156dc92ca7d90eaedd3b0f95@sprout-oss.stage.blox.sqprod.co>
This commit is contained in:
1
desktop/src-tauri/Cargo.lock
generated
1
desktop/src-tauri/Cargo.lock
generated
@@ -879,6 +879,7 @@ dependencies = [
|
||||
"rodio",
|
||||
"rubato",
|
||||
"rusqlite",
|
||||
"security-framework 3.7.0",
|
||||
"serde",
|
||||
"serde_json",
|
||||
"serde_yaml",
|
||||
|
||||
@@ -17,13 +17,20 @@ fn agent_keyring_name(pubkey: &str) -> String {
|
||||
}
|
||||
|
||||
/// The agent secret store. `None` when the build has no keyring backend, in
|
||||
/// which case agent keys stay inline in the `0o600` JSON file.
|
||||
fn agent_secret_store() -> Option<SecretStore> {
|
||||
if cfg!(feature = "system-keyring") {
|
||||
Some(SecretStore::keyring(KEYRING_SERVICE))
|
||||
} else {
|
||||
None
|
||||
}
|
||||
/// which case agent keys stay inline in the `0o600` JSON file. Cached via
|
||||
/// `OnceLock` so the in-memory blob cache survives across call sites.
|
||||
fn agent_secret_store() -> Option<&'static SecretStore> {
|
||||
use std::sync::OnceLock;
|
||||
static STORE: OnceLock<Option<SecretStore>> = OnceLock::new();
|
||||
STORE
|
||||
.get_or_init(|| {
|
||||
if cfg!(feature = "system-keyring") {
|
||||
Some(SecretStore::keyring(KEYRING_SERVICE))
|
||||
} else {
|
||||
None
|
||||
}
|
||||
})
|
||||
.as_ref()
|
||||
}
|
||||
|
||||
pub fn managed_agents_base_dir(app: &AppHandle) -> Result<PathBuf, String> {
|
||||
@@ -173,7 +180,7 @@ fn hydrate_keys(records: &mut [ManagedAgentRecord]) {
|
||||
let Some(store) = agent_secret_store() else {
|
||||
return;
|
||||
};
|
||||
hydrate_keys_with(&store, records);
|
||||
hydrate_keys_with(store, records);
|
||||
}
|
||||
|
||||
/// Testable core of [`hydrate_keys`], generic over the [`KeyStore`] seam.
|
||||
@@ -257,7 +264,7 @@ fn persist_agent_keys(records: &mut [ManagedAgentRecord]) {
|
||||
// unreachable) so it is not lost, and `Nothing` (empty key) because
|
||||
// there is no verified entry to claim. This is a save-local clone, so
|
||||
// callers keep their keys regardless.
|
||||
if migrate_inline_key(&store, record) == KeyMigration::Persisted {
|
||||
if migrate_inline_key(store, record) == KeyMigration::Persisted {
|
||||
record.private_key_nsec.clear();
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1,10 +1,17 @@
|
||||
//! OS keyring access for desktop nsec private keys.
|
||||
//!
|
||||
//! Backed by the `keyring` crate (macOS Keychain / Windows Credential Manager /
|
||||
//! Linux Secret Service via D-Bus). The chosen backend is selected at compile
|
||||
//! time by the per-target feature in `Cargo.toml`. 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.
|
||||
//! 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.
|
||||
//!
|
||||
//! The chosen backend is selected at compile time by the per-target feature in
|
||||
//! `Cargo.toml`. On macOS the modern Data Protection Keychain API is used;
|
||||
//! unsigned dev builds (which lack the hardened-runtime entitlement) fall back
|
||||
//! to the legacy `keyring` crate automatically. 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.
|
||||
//!
|
||||
//! The store is deliberately NOT on any env-read path. `BUZZ_PRIVATE_KEY`
|
||||
//! resolution for harnessed agents and CI is handled upstream (an env
|
||||
@@ -12,6 +19,9 @@
|
||||
//! adding an env tier here would duplicate that precedence and create a
|
||||
//! divergent-behavior trap.
|
||||
|
||||
use std::collections::HashMap;
|
||||
use std::sync::Mutex;
|
||||
|
||||
/// Result of probing the keyring before a migration: distinguishes "reachable
|
||||
/// but holds no entry" (safe to migrate into) from "unreachable this boot"
|
||||
/// (must NOT migrate — re-importing from a leftover plaintext file could
|
||||
@@ -27,10 +37,16 @@ pub enum KeyringProbe {
|
||||
Unreachable,
|
||||
}
|
||||
|
||||
/// An OS keyring, addressed by service name. Each logical secret is a distinct
|
||||
/// key within the service (passed to each operation as the keyring "username").
|
||||
/// Username used for the single blob keychain entry. All secrets are stored
|
||||
/// as a JSON map under this name within the service.
|
||||
const BLOB_KEY: &str = "secrets";
|
||||
|
||||
/// An OS keyring, addressed by service name. All secrets are stored in a
|
||||
/// single JSON blob entry (one OS prompt per process lifetime).
|
||||
pub struct SecretStore {
|
||||
service: String,
|
||||
/// In-memory cache of the deserialized blob. `None` means "not yet loaded".
|
||||
cache: Mutex<Option<HashMap<String, String>>>,
|
||||
}
|
||||
|
||||
impl SecretStore {
|
||||
@@ -40,6 +56,7 @@ impl SecretStore {
|
||||
pub fn keyring(service: impl Into<String>) -> Self {
|
||||
SecretStore {
|
||||
service: service.into(),
|
||||
cache: Mutex::new(None),
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -97,56 +114,122 @@ fn dpk_opts(service: &str, key: &str) -> PasswordOptions {
|
||||
}
|
||||
|
||||
impl SecretStore {
|
||||
/// Probe whether `key` exists and whether the backend is reachable.
|
||||
pub fn probe(&self, key: &str) -> KeyringProbe {
|
||||
// macOS: probe the Data Protection Keychain first. If DPK is
|
||||
// unavailable (unsigned dev build), fall back to the legacy keyring
|
||||
// crate path. Items still in the old keychain will be migrated on the
|
||||
// first `load` call.
|
||||
#[cfg(all(feature = "system-keyring", target_os = "macos"))]
|
||||
/// Read the blob from the keychain and return the deserialized map.
|
||||
///
|
||||
/// Returns `Ok(None)` when no blob entry exists yet (first launch or
|
||||
/// fresh install). Returns `Err` when the backend is unavailable or the
|
||||
/// stored JSON is corrupt.
|
||||
///
|
||||
/// On success the result is stored in `self.cache` so subsequent calls
|
||||
/// within the same process return immediately without a keychain round-trip.
|
||||
#[cfg(feature = "system-keyring")]
|
||||
fn load_blob(&self) -> Result<Option<HashMap<String, String>>, String> {
|
||||
{
|
||||
match generic_password(dpk_opts(&self.service, key)) {
|
||||
Ok(_) => KeyringProbe::Present,
|
||||
Err(ref e) if is_not_found(e) => KeyringProbe::ReachableButEmpty,
|
||||
Err(ref e) if is_dpk_unavailable(e) => {
|
||||
// DPK unavailable (unsigned build) — fall back to keyring.
|
||||
match keyring_entry(&self.service, key) {
|
||||
Ok(entry) => match entry.get_password() {
|
||||
Ok(_) => KeyringProbe::Present,
|
||||
Err(keyring::Error::NoEntry) => KeyringProbe::ReachableButEmpty,
|
||||
Err(e) if is_keyring_availability_error(&e.to_string()) => {
|
||||
KeyringProbe::Unreachable
|
||||
}
|
||||
Err(_) => KeyringProbe::ReachableButEmpty,
|
||||
},
|
||||
Err(e) if is_keyring_availability_error(&e.to_string()) => {
|
||||
KeyringProbe::Unreachable
|
||||
}
|
||||
Err(_) => KeyringProbe::Unreachable,
|
||||
}
|
||||
}
|
||||
Err(ref e) if is_keyring_availability_error(&e.to_string()) => {
|
||||
KeyringProbe::Unreachable
|
||||
}
|
||||
Err(_) => KeyringProbe::ReachableButEmpty,
|
||||
let guard = self.cache.lock().unwrap_or_else(|e| e.into_inner());
|
||||
if let Some(ref map) = *guard {
|
||||
return Ok(Some(map.clone()));
|
||||
}
|
||||
}
|
||||
// Non-macOS system-keyring path (Windows, Linux).
|
||||
#[cfg(all(feature = "system-keyring", not(target_os = "macos")))]
|
||||
|
||||
let raw = self.read_blob_raw()?;
|
||||
let map = match raw {
|
||||
None => return Ok(None),
|
||||
Some(bytes) => {
|
||||
let json = String::from_utf8(bytes).map_err(|e| format!("blob utf8: {e}"))?;
|
||||
serde_json::from_str::<HashMap<String, String>>(&json)
|
||||
.map_err(|e| format!("blob json: {e}"))?
|
||||
}
|
||||
};
|
||||
|
||||
*self.cache.lock().unwrap_or_else(|e| e.into_inner()) = Some(map.clone());
|
||||
Ok(Some(map))
|
||||
}
|
||||
|
||||
/// Read the raw blob bytes from the keychain. `Ok(None)` = not found.
|
||||
#[cfg(all(feature = "system-keyring", target_os = "macos"))]
|
||||
fn read_blob_raw(&self) -> Result<Option<Vec<u8>>, String> {
|
||||
match generic_password(dpk_opts(&self.service, BLOB_KEY)) {
|
||||
Ok(bytes) => Ok(Some(bytes)),
|
||||
Err(ref e) if is_not_found(e) => Ok(None),
|
||||
Err(ref e) if is_dpk_unavailable(e) => {
|
||||
// Unsigned dev build — fall back to legacy keyring crate.
|
||||
self.read_blob_raw_keyring()
|
||||
}
|
||||
Err(e) => Err(format!("keyring read: {e}")),
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(all(feature = "system-keyring", not(target_os = "macos")))]
|
||||
fn read_blob_raw(&self) -> Result<Option<Vec<u8>>, String> {
|
||||
self.read_blob_raw_keyring()
|
||||
}
|
||||
|
||||
/// Read blob via the legacy `keyring` crate (Windows, Linux, or macOS dev
|
||||
/// builds that lack hardened-runtime entitlements).
|
||||
#[cfg(feature = "system-keyring")]
|
||||
fn read_blob_raw_keyring(&self) -> Result<Option<Vec<u8>>, String> {
|
||||
let entry =
|
||||
keyring_entry(&self.service, BLOB_KEY).map_err(|e| format!("keyring entry: {e}"))?;
|
||||
match entry.get_password() {
|
||||
Ok(s) => Ok(Some(s.into_bytes())),
|
||||
Err(keyring::Error::NoEntry) => Ok(None),
|
||||
Err(e) if is_keyring_availability_error(&e.to_string()) => {
|
||||
Err(format!("keyring unavailable: {e}"))
|
||||
}
|
||||
Err(e) => Err(format!("keyring read: {e}")),
|
||||
}
|
||||
}
|
||||
|
||||
/// Serialize `map` to JSON and write it as the single blob keychain entry.
|
||||
#[cfg(feature = "system-keyring")]
|
||||
fn save_blob(&self, map: &HashMap<String, String>) -> Result<(), String> {
|
||||
let json = serde_json::to_string(map).map_err(|e| format!("blob serialize: {e}"))?;
|
||||
self.write_blob_raw(json.as_bytes())?;
|
||||
*self.cache.lock().unwrap_or_else(|e| e.into_inner()) = Some(map.clone());
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[cfg(all(feature = "system-keyring", target_os = "macos"))]
|
||||
fn write_blob_raw(&self, bytes: &[u8]) -> Result<(), String> {
|
||||
match set_generic_password_options(bytes, dpk_opts(&self.service, BLOB_KEY)) {
|
||||
Ok(()) => Ok(()),
|
||||
Err(ref e) if is_dpk_unavailable(e) => self.write_blob_raw_keyring(bytes),
|
||||
Err(e) => Err(format!("keyring write: {e}")),
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(all(feature = "system-keyring", not(target_os = "macos")))]
|
||||
fn write_blob_raw(&self, bytes: &[u8]) -> Result<(), String> {
|
||||
self.write_blob_raw_keyring(bytes)
|
||||
}
|
||||
|
||||
#[cfg(feature = "system-keyring")]
|
||||
fn write_blob_raw_keyring(&self, bytes: &[u8]) -> Result<(), String> {
|
||||
let value = std::str::from_utf8(bytes).map_err(|e| format!("blob utf8 encode: {e}"))?;
|
||||
let entry =
|
||||
keyring_entry(&self.service, BLOB_KEY).map_err(|e| format!("keyring entry: {e}"))?;
|
||||
entry
|
||||
.set_password(value)
|
||||
.map_err(|e| format!("keyring write: {e}"))
|
||||
}
|
||||
|
||||
/// Probe whether `key` exists and whether the backend is reachable.
|
||||
pub fn probe(&self, key: &str) -> KeyringProbe {
|
||||
#[cfg(feature = "system-keyring")]
|
||||
{
|
||||
match keyring_entry(&self.service, key) {
|
||||
Ok(entry) => match entry.get_password() {
|
||||
Ok(_) => KeyringProbe::Present,
|
||||
Err(keyring::Error::NoEntry) => KeyringProbe::ReachableButEmpty,
|
||||
Err(e) if is_keyring_availability_error(&e.to_string()) => {
|
||||
KeyringProbe::Unreachable
|
||||
match self.load_blob() {
|
||||
Ok(Some(map)) => {
|
||||
if map.contains_key(key) {
|
||||
KeyringProbe::Present
|
||||
} else {
|
||||
KeyringProbe::ReachableButEmpty
|
||||
}
|
||||
Err(_) => KeyringProbe::ReachableButEmpty,
|
||||
},
|
||||
Err(e) if is_keyring_availability_error(&e.to_string()) => {
|
||||
KeyringProbe::Unreachable
|
||||
}
|
||||
Err(_) => KeyringProbe::Unreachable,
|
||||
// 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),
|
||||
Err(e) if is_keyring_availability_error(&e) => KeyringProbe::Unreachable,
|
||||
Err(_) => KeyringProbe::ReachableButEmpty,
|
||||
}
|
||||
}
|
||||
#[cfg(not(feature = "system-keyring"))]
|
||||
@@ -156,57 +239,60 @@ impl SecretStore {
|
||||
}
|
||||
}
|
||||
|
||||
/// Check old per-key DPK/keyring entries for `key`. Used by `probe()` when
|
||||
/// the blob doesn't exist yet (first launch after upgrade).
|
||||
#[cfg(all(feature = "system-keyring", target_os = "macos"))]
|
||||
fn probe_legacy_key(&self, key: &str) -> KeyringProbe {
|
||||
match generic_password(dpk_opts(&self.service, key)) {
|
||||
Ok(_) => KeyringProbe::Present,
|
||||
Err(ref e) if is_not_found(e) => self.probe_legacy_key_keyring(key),
|
||||
Err(ref e) if is_dpk_unavailable(e) => self.probe_legacy_key_keyring(key),
|
||||
Err(ref e) if is_keyring_availability_error(&e.to_string()) => {
|
||||
KeyringProbe::Unreachable
|
||||
}
|
||||
Err(_) => KeyringProbe::ReachableButEmpty,
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(all(feature = "system-keyring", not(target_os = "macos")))]
|
||||
fn probe_legacy_key(&self, key: &str) -> KeyringProbe {
|
||||
self.probe_legacy_key_keyring(key)
|
||||
}
|
||||
|
||||
#[cfg(feature = "system-keyring")]
|
||||
fn probe_legacy_key_keyring(&self, key: &str) -> KeyringProbe {
|
||||
match keyring_entry(&self.service, key) {
|
||||
Ok(entry) => match entry.get_password() {
|
||||
Ok(_) => KeyringProbe::Present,
|
||||
Err(keyring::Error::NoEntry) => KeyringProbe::ReachableButEmpty,
|
||||
Err(e) if is_keyring_availability_error(&e.to_string()) => {
|
||||
KeyringProbe::Unreachable
|
||||
}
|
||||
Err(_) => KeyringProbe::ReachableButEmpty,
|
||||
},
|
||||
Err(e) if is_keyring_availability_error(&e.to_string()) => KeyringProbe::Unreachable,
|
||||
Err(_) => KeyringProbe::Unreachable,
|
||||
}
|
||||
}
|
||||
|
||||
/// Load the secret for `key`. `Ok(None)` when there is no entry; `Err` only
|
||||
/// when the backend errored in a way that is not "missing".
|
||||
///
|
||||
/// On first launch after an upgrade from the per-key DPK format, the blob
|
||||
/// will not exist yet. In that case the macOS path falls back to reading the
|
||||
/// old per-key DPK entry for `key` specifically, writes it into a new blob,
|
||||
/// and deletes the old item — a one-time migration per key.
|
||||
pub fn load(&self, key: &str) -> Result<Option<String>, String> {
|
||||
// macOS: try Data Protection Keychain first; fall back to old keychain
|
||||
// and migrate on a miss (one-time per item). If DPK is unavailable
|
||||
// (unsigned dev build, errSecMissingEntitlement), use the legacy
|
||||
// keyring crate path directly — no migration needed in that case.
|
||||
#[cfg(all(feature = "system-keyring", target_os = "macos"))]
|
||||
#[cfg(feature = "system-keyring")]
|
||||
{
|
||||
match generic_password(dpk_opts(&self.service, key)) {
|
||||
Ok(bytes) => String::from_utf8(bytes)
|
||||
.map(Some)
|
||||
.map_err(|e| format!("keyring utf8: {e}")),
|
||||
Err(ref e) if is_not_found(e) => {
|
||||
// Not in DPK — check old keychain and migrate if found.
|
||||
let entry = keyring_entry(&self.service, key)
|
||||
.map_err(|e| format!("keyring entry: {e}"))?;
|
||||
match entry.get_password() {
|
||||
Ok(old_val) => {
|
||||
// Migrate to DPK.
|
||||
self.store(key, &old_val)?;
|
||||
// Best-effort cleanup from old keychain.
|
||||
let _ = entry.delete_credential();
|
||||
Ok(Some(old_val))
|
||||
}
|
||||
Err(keyring::Error::NoEntry) => Ok(None),
|
||||
Err(e) => Err(format!("keyring get: {e}")),
|
||||
}
|
||||
match self.load_blob() {
|
||||
Ok(Some(map)) => Ok(map.get(key).cloned()),
|
||||
Ok(None) => {
|
||||
// No blob yet — attempt one-time migration from old per-key
|
||||
// DPK entry (macOS) or return Ok(None) (other platforms).
|
||||
self.migrate_legacy_key(key)
|
||||
}
|
||||
Err(ref e) if is_dpk_unavailable(e) => {
|
||||
// DPK unavailable (unsigned build) — use keyring directly.
|
||||
let entry = keyring_entry(&self.service, key)
|
||||
.map_err(|e| format!("keyring entry: {e}"))?;
|
||||
match entry.get_password() {
|
||||
Ok(secret) => Ok(Some(secret)),
|
||||
Err(keyring::Error::NoEntry) => Ok(None),
|
||||
Err(e) => Err(format!("keyring get: {e}")),
|
||||
}
|
||||
}
|
||||
Err(e) => Err(format!("keyring get: {e}")),
|
||||
}
|
||||
}
|
||||
// Non-macOS system-keyring path (Windows, Linux).
|
||||
#[cfg(all(feature = "system-keyring", not(target_os = "macos")))]
|
||||
{
|
||||
let entry =
|
||||
keyring_entry(&self.service, key).map_err(|e| format!("keyring entry: {e}"))?;
|
||||
match entry.get_password() {
|
||||
Ok(secret) => Ok(Some(secret)),
|
||||
Err(keyring::Error::NoEntry) => Ok(None),
|
||||
Err(e) => Err(format!("keyring get: {e}")),
|
||||
Err(e) => Err(e),
|
||||
}
|
||||
}
|
||||
#[cfg(not(feature = "system-keyring"))]
|
||||
@@ -216,35 +302,63 @@ impl SecretStore {
|
||||
}
|
||||
}
|
||||
|
||||
/// On first launch after upgrading from the per-key DPK format, read the
|
||||
/// old DPK entry for `key`, write it into a new blob, and delete the old
|
||||
/// item. Returns `Ok(None)` when no old entry exists.
|
||||
#[cfg(all(feature = "system-keyring", target_os = "macos"))]
|
||||
fn migrate_legacy_key(&self, key: &str) -> Result<Option<String>, String> {
|
||||
// Try the old per-key DPK entry.
|
||||
match generic_password(dpk_opts(&self.service, key)) {
|
||||
Ok(bytes) => {
|
||||
let value = String::from_utf8(bytes).map_err(|e| format!("keyring utf8: {e}"))?;
|
||||
// Write into blob (creates the blob if it doesn't exist).
|
||||
self.store(key, &value)?;
|
||||
// Best-effort cleanup of the old per-key entry.
|
||||
let _ = delete_generic_password_options(dpk_opts(&self.service, key));
|
||||
Ok(Some(value))
|
||||
}
|
||||
Err(ref e) if is_not_found(e) => {
|
||||
// Also check the old keyring-crate entry (pre-#1264 installs).
|
||||
self.migrate_legacy_key_keyring(key)
|
||||
}
|
||||
Err(ref e) if is_dpk_unavailable(e) => {
|
||||
// Unsigned dev build — check old keyring-crate entry only.
|
||||
self.migrate_legacy_key_keyring(key)
|
||||
}
|
||||
Err(e) => Err(format!("keyring get: {e}")),
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(all(feature = "system-keyring", not(target_os = "macos")))]
|
||||
fn migrate_legacy_key(&self, key: &str) -> Result<Option<String>, String> {
|
||||
// Non-macOS: no DPK, just check the old keyring-crate per-key entry.
|
||||
self.migrate_legacy_key_keyring(key)
|
||||
}
|
||||
|
||||
/// Check the old per-key `keyring` crate entry (pre-#1264 format) and
|
||||
/// migrate it into the blob if found.
|
||||
#[cfg(feature = "system-keyring")]
|
||||
fn migrate_legacy_key_keyring(&self, key: &str) -> Result<Option<String>, String> {
|
||||
let entry = keyring_entry(&self.service, key).map_err(|e| format!("keyring entry: {e}"))?;
|
||||
match entry.get_password() {
|
||||
Ok(value) => {
|
||||
self.store(key, &value)?;
|
||||
let _ = entry.delete_credential();
|
||||
Ok(Some(value))
|
||||
}
|
||||
Err(keyring::Error::NoEntry) => Ok(None),
|
||||
Err(e) => Err(format!("keyring get: {e}")),
|
||||
}
|
||||
}
|
||||
|
||||
/// Store `value` for `key`. Reports `Err` on availability failures — callers
|
||||
/// decide whether to fall back to file storage.
|
||||
pub fn store(&self, key: &str, value: &str) -> Result<(), String> {
|
||||
// macOS: write directly to the Data Protection Keychain. If DPK is
|
||||
// unavailable (unsigned dev build), fall back to the legacy keyring
|
||||
// crate path.
|
||||
#[cfg(all(feature = "system-keyring", target_os = "macos"))]
|
||||
#[cfg(feature = "system-keyring")]
|
||||
{
|
||||
match set_generic_password_options(value.as_bytes(), dpk_opts(&self.service, key)) {
|
||||
Ok(()) => Ok(()),
|
||||
Err(ref e) if is_dpk_unavailable(e) => {
|
||||
// DPK unavailable (unsigned build) — use keyring directly.
|
||||
let entry = keyring_entry(&self.service, key)
|
||||
.map_err(|e| format!("keyring entry: {e}"))?;
|
||||
entry
|
||||
.set_password(value)
|
||||
.map_err(|e| format!("keyring set: {e}"))
|
||||
}
|
||||
Err(e) => Err(format!("keyring set: {e}")),
|
||||
}
|
||||
}
|
||||
// Non-macOS system-keyring path (Windows, Linux).
|
||||
#[cfg(all(feature = "system-keyring", not(target_os = "macos")))]
|
||||
{
|
||||
let entry =
|
||||
keyring_entry(&self.service, key).map_err(|e| format!("keyring entry: {e}"))?;
|
||||
entry
|
||||
.set_password(value)
|
||||
.map_err(|e| format!("keyring set: {e}"))
|
||||
let mut map = self.load_blob()?.unwrap_or_default();
|
||||
map.insert(key.to_string(), value.to_string());
|
||||
self.save_blob(&map)
|
||||
}
|
||||
#[cfg(not(feature = "system-keyring"))]
|
||||
{
|
||||
@@ -255,45 +369,15 @@ impl SecretStore {
|
||||
|
||||
/// Delete the secret for `key`. A missing entry is not an error.
|
||||
pub fn delete(&self, key: &str) -> Result<(), String> {
|
||||
// macOS: delete from both DPK and old keychain (best-effort on old).
|
||||
// If DPK is unavailable (unsigned dev build), fall back to the legacy
|
||||
// keyring crate path.
|
||||
#[cfg(all(feature = "system-keyring", target_os = "macos"))]
|
||||
#[cfg(feature = "system-keyring")]
|
||||
{
|
||||
// Delete from Data Protection Keychain; missing is fine.
|
||||
match delete_generic_password_options(dpk_opts(&self.service, key)) {
|
||||
Ok(()) => {}
|
||||
Err(ref e) if is_not_found(e) => {}
|
||||
Err(ref e) if is_dpk_unavailable(e) => {
|
||||
// DPK unavailable (unsigned build) — use keyring directly.
|
||||
let entry = keyring_entry(&self.service, key)
|
||||
.map_err(|e| format!("keyring entry: {e}"))?;
|
||||
return match entry.delete_credential() {
|
||||
Ok(()) | Err(keyring::Error::NoEntry) => Ok(()),
|
||||
Err(e) => Err(format!("keyring delete: {e}")),
|
||||
};
|
||||
match self.load_blob()? {
|
||||
Some(mut map) => {
|
||||
map.remove(key);
|
||||
self.save_blob(&map)
|
||||
}
|
||||
Err(e) => return Err(format!("keyring delete: {e}")),
|
||||
}
|
||||
// Best-effort cleanup from old keychain.
|
||||
if let Ok(entry) = keyring_entry(&self.service, key) {
|
||||
match entry.delete_credential() {
|
||||
Ok(()) | Err(keyring::Error::NoEntry) => {}
|
||||
Err(e) => {
|
||||
eprintln!("buzz-desktop: old-keychain delete for {key}: {e}");
|
||||
}
|
||||
}
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
// Non-macOS system-keyring path (Windows, Linux).
|
||||
#[cfg(all(feature = "system-keyring", not(target_os = "macos")))]
|
||||
{
|
||||
let entry =
|
||||
keyring_entry(&self.service, key).map_err(|e| format!("keyring entry: {e}"))?;
|
||||
match entry.delete_credential() {
|
||||
Ok(()) | Err(keyring::Error::NoEntry) => Ok(()),
|
||||
Err(e) => Err(format!("keyring delete: {e}")),
|
||||
// No blob — nothing to delete.
|
||||
None => Ok(()),
|
||||
}
|
||||
}
|
||||
#[cfg(not(feature = "system-keyring"))]
|
||||
@@ -324,15 +408,95 @@ mod tests {
|
||||
|
||||
#[cfg(target_os = "macos")]
|
||||
#[test]
|
||||
fn dpk_unavailable_discriminator() {
|
||||
fn dpk_error_discriminators() {
|
||||
// errSecMissingEntitlement = -34018 signals unsigned dev build.
|
||||
let e = SFError::from_code(-34018);
|
||||
assert!(is_dpk_unavailable(&e));
|
||||
assert!(!is_not_found(&e));
|
||||
// errSecItemNotFound = -25300 is not a DPK-unavailable error.
|
||||
let e = SFError::from_code(-25300);
|
||||
assert!(!is_dpk_unavailable(&e));
|
||||
// errSecDuplicateItem = -25299 is not a DPK-unavailable error.
|
||||
let e = SFError::from_code(-25299);
|
||||
assert!(is_not_found(&e));
|
||||
assert!(!is_dpk_unavailable(&e));
|
||||
}
|
||||
|
||||
// Integration tests that exercise the real OS keychain. Skipped in CI
|
||||
// (unsigned builds lack keychain entitlements); run locally with:
|
||||
// cargo test -p buzz-desktop -- --ignored blob_
|
||||
//
|
||||
// Each test uses a unique service name to avoid cross-test pollution.
|
||||
|
||||
#[ignore = "requires real OS keychain (run locally)"]
|
||||
#[test]
|
||||
fn blob_stores_and_retrieves_multiple_keys() {
|
||||
let store = SecretStore::keyring("buzz-test-blob-multi");
|
||||
store.store("key_a", "val_a").unwrap();
|
||||
store.store("key_b", "val_b").unwrap();
|
||||
assert_eq!(store.load("key_a").unwrap(), Some("val_a".to_string()));
|
||||
assert_eq!(store.load("key_b").unwrap(), Some("val_b".to_string()));
|
||||
assert_eq!(store.load("key_c").unwrap(), None);
|
||||
// Cleanup.
|
||||
let _ = store.delete("key_a");
|
||||
let _ = store.delete("key_b");
|
||||
}
|
||||
|
||||
#[ignore = "requires real OS keychain (run locally)"]
|
||||
#[test]
|
||||
fn blob_probe_present_absent_unreachable() {
|
||||
let store = SecretStore::keyring("buzz-test-blob-probe");
|
||||
// No blob yet — key absent, backend reachable.
|
||||
assert_eq!(store.probe("identity"), KeyringProbe::ReachableButEmpty);
|
||||
store.store("identity", "nsec1test").unwrap();
|
||||
// Key now present.
|
||||
assert_eq!(store.probe("identity"), KeyringProbe::Present);
|
||||
// Different key — blob exists but key absent.
|
||||
assert_eq!(store.probe("other"), KeyringProbe::ReachableButEmpty);
|
||||
// Cleanup.
|
||||
let _ = store.delete("identity");
|
||||
}
|
||||
|
||||
#[ignore = "requires real OS keychain (run locally)"]
|
||||
#[test]
|
||||
fn blob_delete_removes_key_not_others() {
|
||||
let store = SecretStore::keyring("buzz-test-blob-delete");
|
||||
store.store("keep", "keep_val").unwrap();
|
||||
store.store("remove", "remove_val").unwrap();
|
||||
store.delete("remove").unwrap();
|
||||
assert_eq!(store.load("keep").unwrap(), Some("keep_val".to_string()));
|
||||
assert_eq!(store.load("remove").unwrap(), None);
|
||||
// Cleanup.
|
||||
let _ = store.delete("keep");
|
||||
}
|
||||
|
||||
#[ignore = "requires real OS keychain (run locally)"]
|
||||
#[test]
|
||||
fn blob_migration_from_per_key_entry() {
|
||||
let svc = "buzz-test-blob-migration";
|
||||
let key = "identity";
|
||||
let value = "nsec1migrationtest";
|
||||
|
||||
// Seed a per-key entry (old format) — no blob exists.
|
||||
let entry = keyring_entry(svc, key).unwrap();
|
||||
entry.set_password(value).unwrap();
|
||||
|
||||
// Fresh store — no blob in the keychain yet.
|
||||
let store = SecretStore::keyring(svc);
|
||||
|
||||
// probe should find the legacy key.
|
||||
assert_eq!(store.probe(key), KeyringProbe::Present);
|
||||
|
||||
// load should migrate it into the blob and return the value.
|
||||
assert_eq!(store.load(key).unwrap(), Some(value.to_string()));
|
||||
|
||||
// Old per-key entry should be cleaned up.
|
||||
let entry = keyring_entry(svc, key).unwrap();
|
||||
assert!(matches!(entry.get_password(), Err(keyring::Error::NoEntry)));
|
||||
|
||||
// Key is now in the blob — probe confirms.
|
||||
let store2 = SecretStore::keyring(svc);
|
||||
assert_eq!(store2.probe(key), KeyringProbe::Present);
|
||||
assert_eq!(store2.load(key).unwrap(), Some(value.to_string()));
|
||||
|
||||
// Cleanup.
|
||||
let _ = store2.delete(key);
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user