diff --git a/apps/native/src-tauri/src/storage/credential_store.rs b/apps/native/src-tauri/src/storage/credential_store.rs index 2e3ec9de..9b60358d 100644 --- a/apps/native/src-tauri/src/storage/credential_store.rs +++ b/apps/native/src-tauri/src/storage/credential_store.rs @@ -1,26 +1,24 @@ -use std::sync::Arc; +//! macOS Keychain-backed credential storage. +//! +//! API keys and other secrets are stored in the macOS Keychain via +//! `tauri-plugin-keyring` rather than in the plaintext JSON settings file. +//! The `CredentialStore` trait abstracts get/set so the rest of the codebase +//! doesn't need to know the storage mechanism. A future migration could swap +//! `KeychainStore` for a Linux Secret Service implementation without changing +//! callers. + use tauri::{AppHandle, Runtime}; use tauri_plugin_keyring::KeyringExt; -#[cfg(test)] -use std::sync::Mutex; - #[derive(Debug, thiserror::Error)] pub enum CredentialStoreError { #[error("keychain operation failed: {0}")] Keychain(String), - #[error("credential store operation failed: {0}")] - Storage(String), - #[error("legacy settings store is read-only for writes")] - LegacyReadOnly, - #[error("failed to remove legacy plaintext credential: {0}")] - LegacyCleanup(String), } pub trait CredentialStore { fn get(&self) -> Result, CredentialStoreError>; fn set(&self, value: &str) -> Result<(), CredentialStoreError>; - fn delete(&self) -> Result<(), CredentialStoreError>; } pub struct KeychainStore { @@ -58,250 +56,4 @@ impl CredentialStore for KeychainStore { .set_password(&self.service, &self.account, value) .map_err(|e| CredentialStoreError::Keychain(e.to_string())) } - - fn delete(&self) -> Result<(), CredentialStoreError> { - match self - .app - .keyring() - .delete_password(&self.service, &self.account) - { - Ok(()) => Ok(()), - Err(e) if is_not_found_keyring_error(&e) => Ok(()), - Err(e) => Err(CredentialStoreError::Keychain(e.to_string())), - } - } -} - -fn is_not_found_keyring_error(err: &E) -> bool { - let msg = err.to_string().to_ascii_lowercase(); - if msg.contains("no matching entry") || msg.contains("not found") { - return true; - } - - format!("{err:?}").to_ascii_lowercase().contains("noentry") -} - -pub struct SettingsFileStore { - name: String, - getter: Arc Result, CredentialStoreError> + Send + Sync>, - deleter: Arc Result<(), CredentialStoreError> + Send + Sync>, -} - -impl SettingsFileStore { - pub fn new(name: impl Into, getter: G, deleter: D) -> Self - where - G: Fn() -> Result, CredentialStoreError> + Send + Sync + 'static, - D: Fn() -> Result<(), CredentialStoreError> + Send + Sync + 'static, - { - Self { - name: name.into(), - getter: Arc::new(getter), - deleter: Arc::new(deleter), - } - } -} - -impl CredentialStore for SettingsFileStore { - fn get(&self) -> Result, CredentialStoreError> { - let res = (self.getter)(); - if let Ok(Some(_)) = &res { - log::debug!("credential accessed from legacy settings: {}", self.name); - } - res - } - - fn set(&self, _value: &str) -> Result<(), CredentialStoreError> { - Err(CredentialStoreError::LegacyReadOnly) - } - - fn delete(&self) -> Result<(), CredentialStoreError> { - log::debug!("deleting legacy credential from settings: {}", self.name); - (self.deleter)() - } -} - -#[cfg(test)] -#[derive(Default)] -pub struct InMemoryStore { - value: Mutex>, -} - -#[cfg(test)] -impl InMemoryStore { - pub fn with_value(value: Option) -> Self { - Self { - value: Mutex::new(value), - } - } -} - -#[cfg(test)] -impl CredentialStore for InMemoryStore { - fn get(&self) -> Result, CredentialStoreError> { - self.value - .lock() - .map(|value| value.clone()) - .map_err(|_| CredentialStoreError::Storage("in-memory store lock poisoned".to_string())) - } - - fn set(&self, value: &str) -> Result<(), CredentialStoreError> { - self.value - .lock() - .map(|mut current| { - *current = Some(value.to_string()); - }) - .map_err(|_| CredentialStoreError::Storage("in-memory store lock poisoned".to_string())) - } - - fn delete(&self) -> Result<(), CredentialStoreError> { - self.value - .lock() - .map(|mut current| { - *current = None; - }) - .map_err(|_| CredentialStoreError::Storage("in-memory store lock poisoned".to_string())) - } -} - -pub fn get_with_lazy_migration( - keychain: &K, - legacy: &L, -) -> Result, CredentialStoreError> -where - K: CredentialStore, - L: CredentialStore, -{ - let keychain_get_err = match keychain.get() { - Ok(Some(value)) => { - // Keychain already has the credential. Clean up any stale plaintext - // copy in legacy storage (e.g. from a previous run that wrote to - // keychain but failed to delete the settings.json entry). - match legacy.get() { - Ok(Some(_)) => { - if let Err(err) = legacy.delete() { - log::warn!( - "Failed to clean up stale plaintext credential from settings: {}", - err - ); - } else { - log::info!("Cleaned up stale plaintext credential from settings (already in keychain)"); - } - } - Ok(None) => {} - Err(err) => { - log::warn!("Could not check legacy store during cleanup: {}", err); - } - } - return Ok(Some(value)); - } - Ok(None) => None, - Err(err) => Some(err), - }; - - let Some(legacy_value) = legacy.get()? else { - return match keychain_get_err { - Some(err) => Err(err), - None => Ok(None), - }; - }; - - match keychain.set(&legacy_value) { - Ok(()) => { - if let Err(err) = legacy.delete() { - log::warn!( - "Credential migrated to keychain but failed to clean up plaintext settings value: {}", - err - ); - } - } - Err(err) => { - log::warn!( - "Credential migration to keychain failed, keeping legacy settings value in place: {}", - err - ); - } - } - - Ok(Some(legacy_value)) -} - -pub fn set_with_cleanup( - keychain: &K, - legacy: &L, - value: &str, -) -> Result<(), CredentialStoreError> -where - K: CredentialStore, - L: CredentialStore, -{ - keychain.set(value)?; - legacy - .delete() - .map_err(|err| CredentialStoreError::LegacyCleanup(err.to_string())) -} - -#[cfg(test)] -mod tests { - use super::*; - - struct FailingSetStore; - - impl CredentialStore for FailingSetStore { - fn get(&self) -> Result, CredentialStoreError> { - Ok(None) - } - - fn set(&self, _value: &str) -> Result<(), CredentialStoreError> { - Err(CredentialStoreError::Storage("set failed".to_string())) - } - - fn delete(&self) -> Result<(), CredentialStoreError> { - Ok(()) - } - } - - #[test] - fn migrates_legacy_value_to_keychain_and_cleans_plaintext() { - let keychain = InMemoryStore::default(); - let legacy = InMemoryStore::with_value(Some("legacy-secret".to_string())); - - let value = get_with_lazy_migration(&keychain, &legacy).unwrap(); - - assert_eq!(value.as_deref(), Some("legacy-secret")); - assert_eq!(keychain.get().unwrap().as_deref(), Some("legacy-secret")); - assert_eq!(legacy.get().unwrap(), None); - } - - #[test] - fn returns_legacy_value_if_keychain_migration_write_fails() { - let keychain = FailingSetStore; - let legacy = InMemoryStore::with_value(Some("legacy-secret".to_string())); - - let value = get_with_lazy_migration(&keychain, &legacy).unwrap(); - - assert_eq!(value.as_deref(), Some("legacy-secret")); - assert_eq!(legacy.get().unwrap().as_deref(), Some("legacy-secret")); - } - - #[test] - fn write_goes_to_keychain_and_removes_legacy_plaintext() { - let keychain = InMemoryStore::default(); - let legacy = InMemoryStore::with_value(Some("stale".to_string())); - - set_with_cleanup(&keychain, &legacy, "new-secret").unwrap(); - - assert_eq!(keychain.get().unwrap().as_deref(), Some("new-secret")); - assert_eq!(legacy.get().unwrap(), None); - } - - #[test] - fn write_failure_does_not_remove_legacy_plaintext() { - let keychain = FailingSetStore; - let legacy = InMemoryStore::with_value(Some("legacy-secret".to_string())); - - let result = set_with_cleanup(&keychain, &legacy, "new-secret"); - - assert!(result.is_err()); - assert_eq!(legacy.get().unwrap().as_deref(), Some("legacy-secret")); - } }