diff --git a/.gitignore b/.gitignore index fa01e8d69..bb8b393d3 100644 --- a/.gitignore +++ b/.gitignore @@ -8,6 +8,9 @@ remove_dupe_nonsense_issues # Diagnostic harness output (scripts/diagnose-cef-runtime.mjs) diagnosis-*.json +# Dev-mode keyring file backend (plain-text secrets, dev artifact only) +dev-keychain.json + # Logs logs *.log diff --git a/Cargo.lock b/Cargo.lock index ee06d9145..6b0a1c4bb 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3599,6 +3599,21 @@ dependencies = [ "cpufeatures 0.2.17", ] +[[package]] +name = "keyring" +version = "3.6.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "eebcc3aff044e5944a8fbaf69eb277d11986064cba30c468730e8b9909fb551c" +dependencies = [ + "byteorder", + "linux-keyutils", + "log", + "security-framework 2.11.1", + "security-framework 3.7.0", + "windows-sys 0.60.2", + "zeroize", +] + [[package]] name = "konst" version = "0.2.20" @@ -3734,6 +3749,16 @@ dependencies = [ "vcpkg", ] +[[package]] +name = "linux-keyutils" +version = "0.2.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "83270a18e9f90d0707c41e9f35efada77b64c0e6f3f1810e71c8368a864d5590" +dependencies = [ + "bitflags 2.11.1", + "libc", +] + [[package]] name = "linux-raw-sys" version = "0.12.1" @@ -4398,7 +4423,7 @@ dependencies = [ "openssl-probe", "openssl-sys", "schannel", - "security-framework", + "security-framework 3.7.0", "security-framework-sys", "tempfile", ] @@ -4996,6 +5021,7 @@ dependencies = [ "hound", "iana-time-zone", "image", + "keyring", "landlock", "lettre", "log", @@ -6584,7 +6610,7 @@ dependencies = [ "openssl-probe", "rustls-pki-types", "schannel", - "security-framework", + "security-framework 3.7.0", ] [[package]] @@ -6612,7 +6638,7 @@ dependencies = [ "rustls-native-certs", "rustls-platform-verifier-android", "rustls-webpki", - "security-framework", + "security-framework 3.7.0", "security-framework-sys", "webpki-root-certs", "windows-sys 0.61.2", @@ -6776,6 +6802,19 @@ dependencies = [ "zeroize", ] +[[package]] +name = "security-framework" +version = "2.11.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "897b2245f0b511c87893af39b033e5ca9cce68824c4d7e7630b5a1d339658d02" +dependencies = [ + "bitflags 2.11.1", + "core-foundation 0.9.4", + "core-foundation-sys 0.8.7", + "libc", + "security-framework-sys", +] + [[package]] name = "security-framework" version = "3.7.0" diff --git a/Cargo.toml b/Cargo.toml index 4c1ef67ad..66123466e 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -115,6 +115,7 @@ rustls-pki-types = "1.14.0" tokio-rustls = "0.26.4" webpki-roots = "1.0.6" sysinfo = { version = "0.33", default-features = false, features = ["system"] } +keyring = { version = "3", features = ["apple-native", "windows-native", "linux-native"] } clap = { version = "4.5", features = ["derive"] } clap_complete = "4.5" lettre = { version = "0.11.22", default-features = false, features = ["builder", "smtp-transport", "rustls-tls"] } diff --git a/app/src-tauri/Cargo.lock b/app/src-tauri/Cargo.lock index fe06f2e72..316583830 100644 --- a/app/src-tauri/Cargo.lock +++ b/app/src-tauri/Cargo.lock @@ -703,6 +703,15 @@ dependencies = [ "hybrid-array", ] +[[package]] +name = "block-padding" +version = "0.3.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "a8894febbff9f758034a5b8e12d87918f56dfc64a8e1fe757d65e29041538d93" +dependencies = [ + "generic-array", +] + [[package]] name = "block2" version = "0.5.1" @@ -859,6 +868,15 @@ dependencies = [ "toml 0.9.12+spec-1.1.0", ] +[[package]] +name = "cbc" +version = "0.1.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "26b52a9543ae338f279b96b0b9fed9c8093744685043739079ce85cd58f289a6" +dependencies = [ + "cipher", +] + [[package]] name = "cc" version = "1.2.62" @@ -1694,6 +1712,24 @@ dependencies = [ "windows-sys 0.61.2", ] +[[package]] +name = "dbus-secret-service" +version = "4.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "708b509edf7889e53d7efb0ffadd994cc6c2345ccb62f55cfd6b0682165e4fa6" +dependencies = [ + "aes", + "block-padding", + "cbc", + "dbus", + "fastrand", + "hkdf", + "num", + "once_cell", + "sha2 0.10.9", + "zeroize", +] + [[package]] name = "debugid" version = "0.8.0" @@ -3210,6 +3246,15 @@ version = "0.4.3" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "7f24254aa9a54b5c858eaee2f5bccdb46aaf0e486a595ed5fd8f86ba55232a70" +[[package]] +name = "hkdf" +version = "0.12.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7b5f8eb2ad728638ea2c7d47a21db23b7b58a72ed6a38256b8a1849f15fbbdf7" +dependencies = [ + "hmac 0.12.1", +] + [[package]] name = "hmac" version = "0.12.1" @@ -3669,6 +3714,7 @@ version = "0.1.4" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "879f10e63c20629ecabbb64a8010319738c66a5cd0c29b02d63d272b03751d01" dependencies = [ + "block-padding", "generic-array", ] @@ -3896,6 +3942,22 @@ dependencies = [ "unicode-segmentation", ] +[[package]] +name = "keyring" +version = "3.6.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "eebcc3aff044e5944a8fbaf69eb277d11986064cba30c468730e8b9909fb551c" +dependencies = [ + "byteorder", + "dbus-secret-service", + "log", + "secret-service", + "security-framework 2.11.1", + "security-framework 3.7.0", + "windows-sys 0.60.2", + "zeroize", +] + [[package]] name = "konst" version = "0.2.20" @@ -4379,7 +4441,7 @@ dependencies = [ "openssl-probe", "openssl-sys", "schannel", - "security-framework", + "security-framework 3.7.0", "security-framework-sys", "tempfile", ] @@ -4453,6 +4515,7 @@ dependencies = [ "cfg-if", "cfg_aliases", "libc", + "memoffset", ] [[package]] @@ -4504,7 +4567,7 @@ dependencies = [ "mac-notification-sys", "serde", "tauri-winrt-notification", - "zbus", + "zbus 5.15.0", ] [[package]] @@ -4535,6 +4598,39 @@ dependencies = [ "windows-sys 0.61.2", ] +[[package]] +name = "num" +version = "0.4.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "35bd024e8b2ff75562e5f34e7f4905839deb4b22955ef5e73d2fea1b9813cb23" +dependencies = [ + "num-bigint", + "num-complex", + "num-integer", + "num-iter", + "num-rational", + "num-traits", +] + +[[package]] +name = "num-bigint" +version = "0.4.6" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "a5e44f723f1133c9deac646763579fdb3ac745e418f2a7af9cd0c431da1f20b9" +dependencies = [ + "num-integer", + "num-traits", +] + +[[package]] +name = "num-complex" +version = "0.4.6" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "73f88a1307638156682bada9d7604135552957b7818057dcef22705b4d509495" +dependencies = [ + "num-traits", +] + [[package]] name = "num-conv" version = "0.2.1" @@ -4552,6 +4648,37 @@ dependencies = [ "syn 2.0.117", ] +[[package]] +name = "num-integer" +version = "0.1.46" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7969661fd2958a5cb096e56c8e1ad0444ac2bbcd0061bd28660485a44879858f" +dependencies = [ + "num-traits", +] + +[[package]] +name = "num-iter" +version = "0.1.45" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "1429034a0490724d0075ebb2bc9e875d6503c3cf69e235a8941aa757d83ef5bf" +dependencies = [ + "autocfg", + "num-integer", + "num-traits", +] + +[[package]] +name = "num-rational" +version = "0.4.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f83d14da390562dca69fc84082e73e548e1ad308d24accdedd2720017cb37824" +dependencies = [ + "num-bigint", + "num-integer", + "num-traits", +] + [[package]] name = "num-traits" version = "0.2.19" @@ -5088,6 +5215,7 @@ dependencies = [ "hound", "iana-time-zone", "image", + "keyring", "lettre", "log", "mail-parser", @@ -6648,7 +6776,7 @@ dependencies = [ "openssl-probe", "rustls-pki-types", "schannel", - "security-framework", + "security-framework 3.7.0", ] [[package]] @@ -6676,7 +6804,7 @@ dependencies = [ "rustls-native-certs", "rustls-platform-verifier-android", "rustls-webpki", - "security-framework", + "security-framework 3.7.0", "security-framework-sys", "webpki-root-certs", "windows-sys 0.61.2", @@ -6852,6 +6980,38 @@ dependencies = [ "zeroize", ] +[[package]] +name = "secret-service" +version = "4.0.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e4d35ad99a181be0a60ffcbe85d680d98f87bdc4d7644ade319b87076b9dbfd4" +dependencies = [ + "aes", + "cbc", + "futures-util", + "generic-array", + "hkdf", + "num", + "once_cell", + "rand 0.8.6", + "serde", + "sha2 0.10.9", + "zbus 4.4.0", +] + +[[package]] +name = "security-framework" +version = "2.11.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "897b2245f0b511c87893af39b033e5ca9cce68824c4d7e7630b5a1d339658d02" +dependencies = [ + "bitflags 2.11.1", + "core-foundation 0.9.4", + "core-foundation-sys 0.8.7", + "libc", + "security-framework-sys", +] + [[package]] name = "security-framework" version = "3.7.0" @@ -8039,7 +8199,7 @@ dependencies = [ "thiserror 2.0.18", "url", "windows 0.61.3", - "zbus", + "zbus 5.15.0", ] [[package]] @@ -8054,7 +8214,7 @@ dependencies = [ "thiserror 2.0.18", "tracing", "windows-sys 0.60.2", - "zbus", + "zbus 5.15.0", ] [[package]] @@ -10473,6 +10633,16 @@ dependencies = [ "rustix", ] +[[package]] +name = "xdg-home" +version = "1.3.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ec1cdab258fb55c0da61328dc52c8764709b249011b2cad0454c72f0bf10a1f6" +dependencies = [ + "libc", + "windows-sys 0.59.0", +] + [[package]] name = "xkbcommon" version = "0.8.0" @@ -10528,6 +10698,38 @@ dependencies = [ "synstructure", ] +[[package]] +name = "zbus" +version = "4.4.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "bb97012beadd29e654708a0fdb4c84bc046f537aecfde2c3ee0a9e4b4d48c725" +dependencies = [ + "async-broadcast", + "async-process", + "async-recursion", + "async-trait", + "enumflags2", + "event-listener 5.4.1", + "futures-core", + "futures-sink", + "futures-util", + "hex", + "nix 0.29.0", + "ordered-stream", + "rand 0.8.6", + "serde", + "serde_repr", + "sha1", + "static_assertions", + "tracing", + "uds_windows", + "windows-sys 0.52.0", + "xdg-home", + "zbus_macros 4.4.0", + "zbus_names 3.0.0", + "zvariant 4.2.0", +] + [[package]] name = "zbus" version = "5.15.0" @@ -10558,9 +10760,22 @@ dependencies = [ "uuid 1.23.1", "windows-sys 0.61.2", "winnow 1.0.2", - "zbus_macros", - "zbus_names", - "zvariant", + "zbus_macros 5.15.0", + "zbus_names 4.3.2", + "zvariant 5.11.0", +] + +[[package]] +name = "zbus_macros" +version = "4.4.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "267db9407081e90bbfa46d841d3cbc60f59c0351838c4bc65199ecd79ab1983e" +dependencies = [ + "proc-macro-crate 3.5.0", + "proc-macro2", + "quote", + "syn 2.0.117", + "zvariant_utils 2.1.0", ] [[package]] @@ -10573,9 +10788,20 @@ dependencies = [ "proc-macro2", "quote", "syn 2.0.117", - "zbus_names", - "zvariant", - "zvariant_utils", + "zbus_names 4.3.2", + "zvariant 5.11.0", + "zvariant_utils 3.3.1", +] + +[[package]] +name = "zbus_names" +version = "3.0.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "4b9b1fef7d021261cc16cba64c351d291b715febe0fa10dc3a443ac5a5022e6c" +dependencies = [ + "serde", + "static_assertions", + "zvariant 4.2.0", ] [[package]] @@ -10586,7 +10812,7 @@ checksum = "7074f3e50b894eac91750142016d30d0a89be8e67dbfd9704fb875825760e52d" dependencies = [ "serde", "winnow 1.0.2", - "zvariant", + "zvariant 5.11.0", ] [[package]] @@ -10635,6 +10861,20 @@ name = "zeroize" version = "1.8.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "b97154e67e32c85465826e8bcc1c59429aaaf107c1e4a9e53c8d8ccd5eff88d0" +dependencies = [ + "zeroize_derive", +] + +[[package]] +name = "zeroize_derive" +version = "1.4.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "85a5b4158499876c763cb03bc4e49185d3cccbabb15b33c627f7884f43db852e" +dependencies = [ + "proc-macro2", + "quote", + "syn 2.0.117", +] [[package]] name = "zerotrie" @@ -10731,6 +10971,19 @@ dependencies = [ "zune-core", ] +[[package]] +name = "zvariant" +version = "4.2.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "2084290ab9a1c471c38fc524945837734fbf124487e105daec2bb57fd48c81fe" +dependencies = [ + "endi", + "enumflags2", + "serde", + "static_assertions", + "zvariant_derive 4.2.0", +] + [[package]] name = "zvariant" version = "5.11.0" @@ -10741,8 +10994,21 @@ dependencies = [ "enumflags2", "serde", "winnow 1.0.2", - "zvariant_derive", - "zvariant_utils", + "zvariant_derive 5.11.0", + "zvariant_utils 3.3.1", +] + +[[package]] +name = "zvariant_derive" +version = "4.2.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "73e2ba546bda683a90652bac4a279bc146adad1386f25379cf73200d2002c449" +dependencies = [ + "proc-macro-crate 3.5.0", + "proc-macro2", + "quote", + "syn 2.0.117", + "zvariant_utils 2.1.0", ] [[package]] @@ -10755,7 +11021,18 @@ dependencies = [ "proc-macro2", "quote", "syn 2.0.117", - "zvariant_utils", + "zvariant_utils 3.3.1", +] + +[[package]] +name = "zvariant_utils" +version = "2.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "c51bcff7cc3dbb5055396bcf774748c3dab426b4b8659046963523cee4808340" +dependencies = [ + "proc-macro2", + "quote", + "syn 2.0.117", ] [[package]] diff --git a/src/openhuman/credentials/profiles.rs b/src/openhuman/credentials/profiles.rs index bd902d4a8..4b64f5659 100644 --- a/src/openhuman/credentials/profiles.rs +++ b/src/openhuman/credentials/profiles.rs @@ -10,6 +10,19 @@ use std::thread; use std::time::{Duration, Instant}; const CURRENT_SCHEMA_VERSION: u32 = 1; + +/// Compact secret payload stored as a single keychain entry per auth profile. +#[derive(Debug, Clone, Serialize, Deserialize, Default)] +struct KeychainSecrets { + #[serde(default)] + pub token: Option, + #[serde(default)] + pub access_token: Option, + #[serde(default)] + pub refresh_token: Option, + #[serde(default)] + pub id_token: Option, +} const PROFILES_FILENAME: &str = "auth-profiles.json"; const LOCK_FILENAME: &str = "auth-profiles.lock"; const LOCK_WAIT_MS: u64 = 50; @@ -138,19 +151,135 @@ impl Default for AuthProfilesData { } } +/// Prefix used for keychain entries that store auth profile secrets. +/// Full key format (as handled by the keyring module): `"{user_id}:auth:{profile_id}"`. +const KEYCHAIN_AUTH_PREFIX: &str = "auth:"; + +/// Derive a stable keychain user-id from a state directory path. +/// +/// For a typical path like `/home/alice/.openhuman/users/uid-123` this +/// returns `"uid-123"`. Falls back to a hash of the full path string so +/// the function always returns a non-empty value even for unusual layouts. +fn user_id_from_state_dir(state_dir: &Path) -> String { + // The user directory is `{root}/users/{user_id}/` — take the last component. + if let Some(id) = state_dir.file_name().and_then(|s| s.to_str()) { + if !id.is_empty() { + return id.to_string(); + } + } + // Fallback: use a hex hash of the path so we always get a stable string. + let path_str = state_dir.to_string_lossy(); + let mut hash: u64 = 14695981039346656037u64; // FNV-1a offset basis + for b in path_str.as_bytes() { + hash ^= u64::from(*b); + hash = hash.wrapping_mul(1099511628211u64); + } + format!("path-{hash:016x}") +} + #[derive(Debug, Clone)] pub struct AuthProfilesStore { path: PathBuf, lock_path: PathBuf, secret_store: SecretStore, + /// Opaque user identifier used to namespace keychain entries. + user_id: String, + /// Whether the OS keychain is available on this machine. + /// Cached at construction time to avoid repeated probes. + use_keychain: bool, } impl AuthProfilesStore { pub fn new(state_dir: &Path, encrypt_secrets: bool) -> Self { + let user_id = user_id_from_state_dir(state_dir); + let use_keychain = crate::openhuman::keyring::is_available(); + log::debug!( + "[auth] AuthProfilesStore::new state_dir={} user_id={user_id} use_keychain={use_keychain}", + state_dir.display() + ); Self { path: state_dir.join(PROFILES_FILENAME), lock_path: state_dir.join(LOCK_FILENAME), secret_store: SecretStore::new(state_dir, encrypt_secrets), + user_id, + use_keychain, + } + } + + /// Build a keychain key for an auth profile's combined secret payload. + fn keychain_key_for_profile(&self, profile_id: &str) -> String { + format!("{KEYCHAIN_AUTH_PREFIX}{profile_id}") + } + + /// Store auth secrets for a profile in the OS keychain. + /// + /// The secrets are serialized as a compact JSON object so a single + /// keychain entry holds all token fields for the profile. + fn keychain_store_secrets(&self, profile: &AuthProfile) -> anyhow::Result<()> { + let key = self.keychain_key_for_profile(&profile.id); + let secrets = serde_json::json!({ + "token": profile.token, + "access_token": profile.token_set.as_ref().map(|ts| &ts.access_token), + "refresh_token": profile.token_set.as_ref().and_then(|ts| ts.refresh_token.as_deref()), + "id_token": profile.token_set.as_ref().and_then(|ts| ts.id_token.as_deref()), + }); + let payload = serde_json::to_string(&secrets) + .context("Failed to serialize auth secrets for keychain")?; + crate::openhuman::keyring::set(&self.user_id, &key, &payload) + .map_err(|e| anyhow::anyhow!("Keychain set failed for profile {}: {e}", profile.id))?; + log::debug!( + "[auth] keychain_store_secrets stored profile_id={} user_id={}", + profile.id, + self.user_id + ); + Ok(()) + } + + /// Load auth secrets for a profile from the OS keychain. + /// + /// Returns `None` if no keychain entry exists for the profile. + fn keychain_load_secrets(&self, profile_id: &str) -> anyhow::Result> { + let key = self.keychain_key_for_profile(profile_id); + let payload = match crate::openhuman::keyring::get(&self.user_id, &key) { + Ok(Some(p)) => p, + Ok(None) => { + log::debug!( + "[auth] keychain_load_secrets miss profile_id={profile_id} user_id={}", + self.user_id + ); + return Ok(None); + } + Err(e) => { + log::warn!( + "[auth] keychain_load_secrets error profile_id={profile_id} user_id={}: {e}", + self.user_id + ); + return Ok(None); + } + }; + let secrets: KeychainSecrets = serde_json::from_str(&payload).map_err(|e| { + anyhow::anyhow!("Keychain payload for profile {profile_id} is not valid JSON: {e}") + })?; + log::debug!( + "[auth] keychain_load_secrets hit profile_id={profile_id} user_id={}", + self.user_id + ); + Ok(Some(secrets)) + } + + /// Delete keychain secrets for a profile (called on profile removal). + fn keychain_delete_secrets(&self, profile_id: &str) { + let key = self.keychain_key_for_profile(profile_id); + if let Err(e) = crate::openhuman::keyring::delete(&self.user_id, &key) { + log::warn!( + "[auth] keychain_delete_secrets error profile_id={profile_id} user_id={}: {e}", + self.user_id + ); + } else { + log::debug!( + "[auth] keychain_delete_secrets ok profile_id={profile_id} user_id={}", + self.user_id + ); } } @@ -196,6 +325,13 @@ impl AuthProfilesStore { .retain(|_, active| active != profile_id); data.updated_at = Utc::now(); self.save_locked(&data)?; + + // Clean up keychain entry for this profile (idempotent if keychain + // is unavailable or no entry exists). + if self.use_keychain { + self.keychain_delete_secrets(profile_id); + } + Ok(true) } @@ -243,27 +379,200 @@ impl AuthProfilesStore { fn load_locked(&self) -> Result { let mut persisted = self.read_persisted_locked()?; + // `migrated` tracks enc: → enc2: XOR-cipher upgrades (original behavior). let mut migrated = false; + // `keychain_migrated` tracks enc2: → keychain promotions: when true the + // persisted JSON must be rewritten with secret fields cleared. + let mut keychain_migrated = false; let mut dropped_ids: Vec = Vec::new(); let mut profiles = BTreeMap::new(); for (id, p) in &mut persisted.profiles { - // Decrypt all four optional secret fields. A decryption - // failure here means the secret was encrypted with a - // `.secret_key` that no longer exists (manual deletion, - // partial workspace restore, key rotation across machines). - // The profile is unrecoverable — drop it from the store - // instead of poisoning every reader. The user falls back - // to a clean "logged out" state and the next login - // re-encrypts cleanly under the current key. - let decrypted = (|| -> Result<_> { - let (access_token, access_migrated) = - self.decrypt_optional(p.access_token.as_deref())?; - let (refresh_token, refresh_migrated) = - self.decrypt_optional(p.refresh_token.as_deref())?; - let (id_token, id_migrated) = self.decrypt_optional(p.id_token.as_deref())?; - let (token, token_migrated) = self.decrypt_optional(p.token.as_deref())?; - Ok(( + // ── Step 1: Resolve secrets ─────────────────────────────────────── + // + // Priority order: + // (a) OS keychain — preferred when available. + // (b) enc2:/enc: JSON fields — legacy; decrypt and optionally + // migrate to keychain on this read. + // (c) Plaintext JSON fields — oldest legacy path; pass through. + // + // A decrypt failure (wrong key / tampered data) drops the profile + // rather than poisoning every reader — the user falls back to a + // clean logged-out state and re-authenticates cleanly. + + let (access_token, refresh_token, id_token, token) = if self.use_keychain { + // ── (a) Keychain path ────────────────────────────────────── + match self.keychain_load_secrets(id) { + Ok(Some(secrets)) => { + // Keychain has the entry — use it directly. Clear the + // JSON secret fields so they're wiped on next save. + let had_enc_fields = p.access_token.is_some() + || p.refresh_token.is_some() + || p.id_token.is_some() + || p.token.is_some(); + if had_enc_fields { + log::info!( + "[auth] load: clearing legacy enc fields for profile_id={id} (already in keychain)" + ); + p.access_token = None; + p.refresh_token = None; + p.id_token = None; + p.token = None; + keychain_migrated = true; + } + ( + secrets.access_token, + secrets.refresh_token, + secrets.id_token, + secrets.token, + ) + } + Ok(None) => { + // ── (b) No keychain entry yet — decrypt JSON fields and migrate ── + let decrypted = (|| -> Result<_> { + let (access_token, access_mig) = + self.decrypt_optional(p.access_token.as_deref())?; + let (refresh_token, refresh_mig) = + self.decrypt_optional(p.refresh_token.as_deref())?; + let (id_token, id_mig) = + self.decrypt_optional(p.id_token.as_deref())?; + let (token, token_mig) = self.decrypt_optional(p.token.as_deref())?; + Ok(( + access_token, + access_mig, + refresh_token, + refresh_mig, + id_token, + id_mig, + token, + token_mig, + )) + })(); + let (at, at_mig, rt, rt_mig, it, it_mig, tok, tok_mig) = match decrypted { + Ok(v) => v, + Err(e) => { + log::warn!( + "[auth] dropping unrecoverable profile provider={}: {e}. \ + Most likely cause: .secret_key was regenerated. \ + Re-authenticate to restore the session.", + p.provider + ); + dropped_ids.push(id.clone()); + continue; + } + }; + // Track XOR→enc2 cipher upgrades (existing behavior). + if at_mig.is_some() { + p.access_token = at_mig; + migrated = true; + } + if rt_mig.is_some() { + p.refresh_token = rt_mig; + migrated = true; + } + if it_mig.is_some() { + p.id_token = it_mig; + migrated = true; + } + if tok_mig.is_some() { + p.token = tok_mig; + migrated = true; + } + + // If any secrets were found in JSON, promote them to keychain + // and clear the JSON fields so the next write is clean. + let has_secrets = + at.is_some() || rt.is_some() || it.is_some() || tok.is_some(); + if has_secrets { + log::info!( + "[auth] load: migrating enc fields to keychain profile_id={id} user_id={}", + self.user_id + ); + let dummy_profile = AuthProfile { + id: id.clone(), + provider: p.provider.clone(), + profile_name: p.profile_name.clone(), + kind: parse_profile_kind(&p.kind).unwrap_or(AuthProfileKind::Token), + account_id: p.account_id.clone(), + workspace_id: p.workspace_id.clone(), + token_set: at.clone().map(|access| TokenSet { + access_token: access, + refresh_token: rt.clone(), + id_token: it.clone(), + expires_at: None, + token_type: None, + scope: None, + }), + token: tok.clone(), + metadata: Default::default(), + created_at: Utc::now(), + updated_at: Utc::now(), + }; + if let Err(e) = self.keychain_store_secrets(&dummy_profile) { + // Non-fatal: keep the enc2: fields in JSON so the + // next load can try again. + log::warn!( + "[auth] load: keychain migration failed profile_id={id}: {e}; \ + keeping enc fields in JSON" + ); + } else { + // Wipe JSON secret fields now that keychain has them. + p.access_token = None; + p.refresh_token = None; + p.id_token = None; + p.token = None; + keychain_migrated = true; + } + } + (at, rt, it, tok) + } + Err(_e) => { + // Keychain I/O error — fall through to JSON decrypt path. + log::warn!( + "[auth] keychain error for profile_id={id}; falling back to JSON" + ); + let decrypted = (|| -> Result<_> { + let (at, _) = self.decrypt_optional(p.access_token.as_deref())?; + let (rt, _) = self.decrypt_optional(p.refresh_token.as_deref())?; + let (it, _) = self.decrypt_optional(p.id_token.as_deref())?; + let (tok, _) = self.decrypt_optional(p.token.as_deref())?; + Ok((at, rt, it, tok)) + })(); + match decrypted { + Ok(v) => v, + Err(e) => { + log::warn!( + "[auth] dropping unrecoverable profile provider={}: {e}", + p.provider + ); + dropped_ids.push(id.clone()); + continue; + } + } + } + } + } else { + // ── (b/c) No keychain — use existing JSON decrypt path ──────── + let decrypted = (|| -> Result<_> { + let (access_token, access_migrated) = + self.decrypt_optional(p.access_token.as_deref())?; + let (refresh_token, refresh_migrated) = + self.decrypt_optional(p.refresh_token.as_deref())?; + let (id_token, id_migrated) = self.decrypt_optional(p.id_token.as_deref())?; + let (token, token_migrated) = self.decrypt_optional(p.token.as_deref())?; + Ok(( + access_token, + access_migrated, + refresh_token, + refresh_migrated, + id_token, + id_migrated, + token, + token_migrated, + )) + })(); + + let ( access_token, access_migrated, refresh_token, @@ -272,50 +581,40 @@ impl AuthProfilesStore { id_migrated, token, token_migrated, - )) - })(); + ) = match decrypted { + Ok(v) => v, + Err(e) => { + log::warn!( + "[auth] dropping unrecoverable profile provider={}: {e}. \ + Most likely cause: .secret_key was regenerated after this profile \ + was stored. The store will be rewritten without this entry; \ + re-authenticate to restore the session.", + p.provider + ); + dropped_ids.push(id.clone()); + continue; + } + }; - let ( - access_token, - access_migrated, - refresh_token, - refresh_migrated, - id_token, - id_migrated, - token, - token_migrated, - ) = match decrypted { - Ok(v) => v, - Err(e) => { - log::warn!( - "[auth] dropping unrecoverable profile provider={}: {e}. \ - Most likely cause: .secret_key was regenerated after this profile \ - was stored. The store will be rewritten without this entry; \ - re-authenticate to restore the session.", - p.provider - ); - dropped_ids.push(id.clone()); - continue; + if let Some(value) = access_migrated { + p.access_token = Some(value); + migrated = true; } + if let Some(value) = refresh_migrated { + p.refresh_token = Some(value); + migrated = true; + } + if let Some(value) = id_migrated { + p.id_token = Some(value); + migrated = true; + } + if let Some(value) = token_migrated { + p.token = Some(value); + migrated = true; + } + (access_token, refresh_token, id_token, token) }; - if let Some(value) = access_migrated { - p.access_token = Some(value); - migrated = true; - } - if let Some(value) = refresh_migrated { - p.refresh_token = Some(value); - migrated = true; - } - if let Some(value) = id_migrated { - p.id_token = Some(value); - migrated = true; - } - if let Some(value) = token_migrated { - p.token = Some(value); - migrated = true; - } - let kind = match parse_profile_kind(&p.kind) { Ok(k) => k, Err(e) => { @@ -390,7 +689,7 @@ impl AuthProfilesStore { self.path.display(), ); self.write_persisted_locked(&persisted)?; - } else if migrated { + } else if migrated || keychain_migrated { self.write_persisted_locked(&persisted)?; } @@ -411,21 +710,40 @@ impl AuthProfilesStore { }; for (id, profile) in &data.profiles { - let (access_token, refresh_token, id_token, expires_at, token_type, scope) = - match (&profile.kind, &profile.token_set) { - (AuthProfileKind::OAuth, Some(token_set)) => ( - self.encrypt_optional(Some(&token_set.access_token))?, - self.encrypt_optional(token_set.refresh_token.as_deref())?, - self.encrypt_optional(token_set.id_token.as_deref())?, - token_set.expires_at.as_ref().map(DateTime::to_rfc3339), - token_set.token_type.clone(), - token_set.scope.clone(), - ), - _ => (None, None, None, None, None, None), + // When the OS keychain is available, store all secret fields there and + // leave them absent from the JSON file. This is the preferred path on + // macOS / Windows / Linux-with-Secret-Service. + // + // When the keychain is unavailable (Linux headless / CI), fall back to + // the existing ChaCha20-Poly1305 encrypted JSON fields. + let (access_token, refresh_token, id_token, token, expires_at, token_type, scope) = + if self.use_keychain { + // Store secrets in the OS keychain — JSON gets no secret fields. + if let Err(e) = self.keychain_store_secrets(profile) { + // Non-fatal: fall back to encrypted JSON so data is not lost. + log::warn!( + "[auth] save: keychain store failed for profile_id={id}: {e}; \ + falling back to encrypted JSON" + ); + self.encrypt_for_json(profile)? + } else { + log::debug!("[auth] save: secrets stored in keychain profile_id={id}"); + let (expires_at, token_type, scope) = match &profile.token_set { + Some(ts) => ( + ts.expires_at.as_ref().map(DateTime::to_rfc3339), + ts.token_type.clone(), + ts.scope.clone(), + ), + None => (None, None, None), + }; + // Secret fields deliberately omitted from JSON. + (None, None, None, None, expires_at, token_type, scope) + } + } else { + // Headless / no keychain — encrypt and store in JSON. + self.encrypt_for_json(profile)? }; - let token = self.encrypt_optional(profile.token.as_deref())?; - persisted.profiles.insert( id.clone(), PersistedAuthProfile { @@ -451,6 +769,43 @@ impl AuthProfilesStore { self.write_persisted_locked(&persisted) } + /// Encrypt a profile's secret fields for JSON storage (keychain-unavailable path). + fn encrypt_for_json( + &self, + profile: &AuthProfile, + ) -> Result<( + Option, + Option, + Option, + Option, + Option, + Option, + Option, + )> { + let (access_token, refresh_token, id_token, expires_at, token_type, scope) = + match (&profile.kind, &profile.token_set) { + (AuthProfileKind::OAuth, Some(token_set)) => ( + self.encrypt_optional(Some(&token_set.access_token))?, + self.encrypt_optional(token_set.refresh_token.as_deref())?, + self.encrypt_optional(token_set.id_token.as_deref())?, + token_set.expires_at.as_ref().map(DateTime::to_rfc3339), + token_set.token_type.clone(), + token_set.scope.clone(), + ), + _ => (None, None, None, None, None, None), + }; + let token = self.encrypt_optional(profile.token.as_deref())?; + Ok(( + access_token, + refresh_token, + id_token, + token, + expires_at, + token_type, + scope, + )) + } + fn read_persisted_locked(&self) -> Result { if !self.path.exists() { return Ok(PersistedAuthProfiles::default()); diff --git a/src/openhuman/credentials/profiles_tests.rs b/src/openhuman/credentials/profiles_tests.rs index 16a63cebf..5f8857f8b 100644 --- a/src/openhuman/credentials/profiles_tests.rs +++ b/src/openhuman/credentials/profiles_tests.rs @@ -59,8 +59,11 @@ async fn store_roundtrip_with_encryption() { Some("refresh-123") ); + // Under the keychain-backed model (FileBackend in debug builds, real OS + // keychain in release), secret fields are stored in the keychain and + // omitted from the JSON file entirely. The on-disk JSON must not leak + // the plaintext secrets in any form. let raw = tokio::fs::read_to_string(store.path()).await.unwrap(); - assert!(raw.contains("enc2:")); assert!(!raw.contains("refresh-123")); assert!(!raw.contains("access-123")); } @@ -179,10 +182,17 @@ fn load_drops_profiles_whose_decryption_fails_under_rotated_key() { let doomed = AuthProfile::new_token("app-session", "default", "real-jwt-payload".into()); store.upsert_profile(doomed.clone(), true).unwrap(); - // Manually corrupt the persisted token: rewrite it as a syntactically - // valid enc2: hex blob that the *current* key cannot decrypt. - // (Easier than rotating the key file because the SecretStore caches - // by canonical path.) + // Under the keychain-backed model the secret was just stored in the + // keychain (FileBackend in debug builds) and not in the JSON file. To + // exercise the legacy enc2: decrypt-failure → drop path that this test + // covers, delete the keychain entry so the load falls back to the JSON + // decrypt path, then plant a syntactically valid enc2: blob in the JSON + // that the current key cannot decrypt. + let user_id = user_id_from_state_dir(tmp.path()); + let keychain_key = format!("{KEYCHAIN_AUTH_PREFIX}{}", doomed.id); + crate::openhuman::keyring::delete(&user_id, &keychain_key) + .expect("delete keychain entry for test setup"); + let path = store.path().to_path_buf(); let mut data: serde_json::Value = serde_json::from_str(&std::fs::read_to_string(&path).unwrap()).unwrap(); diff --git a/src/openhuman/keyring/backend.rs b/src/openhuman/keyring/backend.rs new file mode 100644 index 000000000..3a06abcb4 --- /dev/null +++ b/src/openhuman/keyring/backend.rs @@ -0,0 +1,274 @@ +//! Backend implementations for the keyring module. +//! +//! Two concrete backends are provided: +//! +//! - [`OsBackend`]: Wraps the `keyring` crate to use the native OS credential +//! store (macOS Keychain, Windows Credential Manager, Linux Secret Service). +//! This is the production backend. +//! +//! - [`FileBackend`]: Stores secrets in a plain JSON file at +//! `{workspace}/dev-keychain.json`. **This file is NOT encrypted** — it is a +//! development artifact only and must never be used in production. It exists +//! solely to avoid the "different binary signature → macOS Keychain permission +//! prompt on every `cargo run`" problem that plagues dev workflows. +//! +//! Backend selection happens once at first use (see [`super::selected_backend`]). + +use std::collections::HashMap; +use std::path::{Path, PathBuf}; + +use parking_lot::Mutex; + +use crate::openhuman::keyring::error::KeyringError; + +// ── Trait ───────────────────────────────────────────────────────────────────── + +/// Abstraction over a secret-storage backend. +/// +/// All implementations must be `Send + Sync` so they can live inside a +/// `OnceLock>`. +pub trait KeyringBackend: Send + Sync { + /// Retrieve a secret. Returns `Ok(None)` when no entry exists. + fn get(&self, namespaced_key: &str) -> Result, KeyringError>; + /// Store (or overwrite) a secret. + fn set(&self, namespaced_key: &str, value: &str) -> Result<(), KeyringError>; + /// Delete a secret. Must be idempotent (no error if the entry is absent). + fn delete(&self, namespaced_key: &str) -> Result<(), KeyringError>; + /// Human-readable name used in log lines. + fn name(&self) -> &'static str; +} + +// ── OsBackend ───────────────────────────────────────────────────────────────── + +/// Production backend: native OS credential store via the `keyring` crate. +pub struct OsBackend; + +const SERVICE_NAME: &str = "openhuman"; + +impl KeyringBackend for OsBackend { + fn get(&self, namespaced_key: &str) -> Result, KeyringError> { + let entry = + keyring::Entry::new(SERVICE_NAME, namespaced_key).map_err(|e| KeyringError::Os { + key: namespaced_key.to_string(), + source: e, + })?; + match entry.get_password() { + Ok(v) => Ok(Some(v)), + Err(keyring::Error::NoEntry) => Ok(None), + Err(keyring::Error::NoStorageAccess(_)) => Ok(None), + Err(e) => Err(KeyringError::Os { + key: namespaced_key.to_string(), + source: e, + }), + } + } + + fn set(&self, namespaced_key: &str, value: &str) -> Result<(), KeyringError> { + let entry = + keyring::Entry::new(SERVICE_NAME, namespaced_key).map_err(|e| KeyringError::Os { + key: namespaced_key.to_string(), + source: e, + })?; + entry.set_password(value).map_err(|e| KeyringError::Os { + key: namespaced_key.to_string(), + source: e, + }) + } + + fn delete(&self, namespaced_key: &str) -> Result<(), KeyringError> { + let entry = + keyring::Entry::new(SERVICE_NAME, namespaced_key).map_err(|e| KeyringError::Os { + key: namespaced_key.to_string(), + source: e, + })?; + match entry.delete_credential() { + Ok(()) => Ok(()), + Err(keyring::Error::NoEntry) => Ok(()), + Err(keyring::Error::NoStorageAccess(_)) => Ok(()), + Err(e) => Err(KeyringError::Os { + key: namespaced_key.to_string(), + source: e, + }), + } + } + + fn name(&self) -> &'static str { + "os" + } +} + +// ── FileBackend ─────────────────────────────────────────────────────────────── + +/// Dev-only backend: plain JSON file at `{workspace}/dev-keychain.json`. +/// +/// # WARNING — NOT FOR PRODUCTION +/// +/// Secrets stored here are **not encrypted**. This backend exists only to +/// eliminate macOS Keychain permission prompts during development (where the +/// binary signature changes on every `cargo build`). It is selected +/// automatically in debug builds and when `OPENHUMAN_APP_ENV=dev|staging`. +/// Never use it in a production deployment. +/// +/// # Thread safety +/// +/// The `mutex` field serializes in-process read-modify-write operations on +/// `set` and `delete`. Cross-process safety relies on the atomic rename in +/// `write_map`. +pub struct FileBackend { + path: PathBuf, + /// In-process lock covering the read→modify→write cycle in mutating ops. + mutex: Mutex<()>, +} + +impl FileBackend { + /// Create a `FileBackend` that reads/writes `{workspace_dir}/dev-keychain.json`. + pub fn new(workspace_dir: &Path) -> Self { + Self { + path: workspace_dir.join("dev-keychain.json"), + mutex: Mutex::new(()), + } + } + + /// Path to the backing file (exposed for logging). + pub fn path(&self) -> &Path { + &self.path + } + + fn read_map(&self) -> Result, KeyringError> { + if !self.path.exists() { + return Ok(HashMap::new()); + } + let bytes = std::fs::read(&self.path).map_err(|e| KeyringError::MigrationReadFailed { + path: self.path.display().to_string(), + source: e, + })?; + if bytes.is_empty() { + return Ok(HashMap::new()); + } + serde_json::from_slice::>(&bytes) + .map_err(|e| { + // Treat a corrupt file as empty so we degrade gracefully. + log::warn!( + "[keyring] dev-keychain.json at {} is corrupt ({e}); treating as empty", + self.path.display() + ); + // Return empty map by converting to a no-source variant. + drop(e); + KeyringError::VerifyFailed { + key: "".to_string(), + } + }) + .or_else(|_| Ok(HashMap::new())) + } + + fn write_map(&self, map: &HashMap) -> Result<(), KeyringError> { + if let Some(parent) = self.path.parent() { + std::fs::create_dir_all(parent).map_err(|e| KeyringError::MigrationReadFailed { + path: parent.display().to_string(), + source: e, + })?; + } + + // Serialize the map to pretty JSON. Propagate serialization failure so + // callers are not silently fed empty data on a write error. + let json = serde_json::to_vec_pretty(map).map_err(|e| { + KeyringError::Backend(format!("failed to serialize dev keychain map: {e}")) + })?; + + // Atomic write: temp file + rename. + let tmp_path = self.path.with_extension("tmp"); + std::fs::write(&tmp_path, &json).map_err(|e| KeyringError::MigrationDeleteFailed { + path: tmp_path.display().to_string(), + source: e, + })?; + + // Set mode 0600 on Unix before moving into place. + #[cfg(unix)] + { + use std::os::unix::fs::PermissionsExt; + let perms = std::fs::Permissions::from_mode(0o600); + if let Err(e) = std::fs::set_permissions(&tmp_path, perms) { + log::warn!("[keyring] could not set 0600 on dev-keychain.json tmp file: {e}"); + } + } + + std::fs::rename(&tmp_path, &self.path).map_err(|e| { + KeyringError::MigrationDeleteFailed { + path: self.path.display().to_string(), + source: e, + } + })?; + + Ok(()) + } +} + +impl KeyringBackend for FileBackend { + fn get(&self, namespaced_key: &str) -> Result, KeyringError> { + let map = self.read_map()?; + Ok(map.get(namespaced_key).cloned()) + } + + fn set(&self, namespaced_key: &str, value: &str) -> Result<(), KeyringError> { + // Hold the in-process lock for the full read→modify→write cycle. + let _guard = self.mutex.lock(); + let mut map = self.read_map()?; + map.insert(namespaced_key.to_string(), value.to_string()); + self.write_map(&map) + } + + fn delete(&self, namespaced_key: &str) -> Result<(), KeyringError> { + // Hold the in-process lock for the full read→modify→write cycle. + let _guard = self.mutex.lock(); + let mut map = self.read_map()?; + if map.remove(namespaced_key).is_some() { + self.write_map(&map)?; + } + Ok(()) + } + + fn name(&self) -> &'static str { + "file" + } +} + +// ── MockBackend (test only) ─────────────────────────────────────────────────── + +/// In-memory backend used in tests and when `OPENHUMAN_KEYRING_BACKEND=mock`. +#[cfg(test)] +pub struct MockBackend { + store: std::sync::Mutex>, +} + +#[cfg(test)] +impl MockBackend { + pub fn new() -> Self { + Self { + store: std::sync::Mutex::new(HashMap::new()), + } + } +} + +#[cfg(test)] +impl KeyringBackend for MockBackend { + fn get(&self, namespaced_key: &str) -> Result, KeyringError> { + Ok(self.store.lock().unwrap().get(namespaced_key).cloned()) + } + + fn set(&self, namespaced_key: &str, value: &str) -> Result<(), KeyringError> { + self.store + .lock() + .unwrap() + .insert(namespaced_key.to_string(), value.to_string()); + Ok(()) + } + + fn delete(&self, namespaced_key: &str) -> Result<(), KeyringError> { + self.store.lock().unwrap().remove(namespaced_key); + Ok(()) + } + + fn name(&self) -> &'static str { + "mock" + } +} diff --git a/src/openhuman/keyring/error.rs b/src/openhuman/keyring/error.rs new file mode 100644 index 000000000..de6cec742 --- /dev/null +++ b/src/openhuman/keyring/error.rs @@ -0,0 +1,51 @@ +use thiserror::Error; + +/// Errors that can occur during OS keychain operations. +#[derive(Debug, Error)] +pub enum KeyringError { + /// The underlying OS keychain returned an error. + #[error("OS keychain error for key '{key}': {source}")] + Os { + key: String, + #[source] + source: keyring::Error, + }, + + /// The keychain returned a value but it was not valid UTF-8. + #[error("Keychain value for key '{key}' is not valid UTF-8: {source}")] + InvalidUtf8 { + key: String, + #[source] + source: std::string::FromUtf8Error, + }, + + /// Reading the source file for migration failed. + #[error("Failed to read migration source file '{path}': {source}")] + MigrationReadFailed { + path: String, + #[source] + source: std::io::Error, + }, + + /// Writing to keychain succeeded but read-back verification failed. + #[error( + "Keychain write verification failed for key '{key}': wrote value did not match read-back" + )] + VerifyFailed { key: String }, + + /// Deleting the source file after migration failed. + #[error("Migration succeeded but failed to delete source file '{path}': {source}")] + MigrationDeleteFailed { + path: String, + #[source] + source: std::io::Error, + }, + + /// Random bytes generation failed. + #[error("Failed to generate random bytes: {0}")] + RandomGeneration(String), + + /// A backend-internal operation failed (e.g. serialization). + #[error("Keyring backend error: {0}")] + Backend(String), +} diff --git a/src/openhuman/keyring/mod.rs b/src/openhuman/keyring/mod.rs new file mode 100644 index 000000000..a6ef70ccf --- /dev/null +++ b/src/openhuman/keyring/mod.rs @@ -0,0 +1,51 @@ +//! OS-keychain backed secret storage with a dev-mode file backend. +//! +//! Wraps the [`keyring`] crate (or a file-based backend in dev) to provide a +//! namespaced, user-scoped interface to secret storage: +//! - **macOS**: Keychain (prod) +//! - **Windows**: Credential Manager (prod) +//! - **Linux**: Secret Service / libsecret (prod) +//! - **Any platform — dev**: JSON file at `{workspace}/dev-keychain.json` +//! +//! All keys are scoped under a `user_id` parameter so multiple users can +//! coexist without collision. The backend entry key format is: +//! `"{user_id}:{logical_key}"`. +//! +//! # Backend selection +//! +//! The backend is chosen **once** at first use, in this priority order: +//! +//! 1. `OPENHUMAN_KEYRING_BACKEND` env var: `"os"` | `"file"` | `"mock"`. +//! 2. `cfg!(debug_assertions)` → `file`. +//! 3. `OPENHUMAN_APP_ENV` == `"dev"` or `"staging"` → `file`. +//! 4. Otherwise → `os` (production). +//! +//! The selected backend is logged once with `[keyring] backend= ...`. +//! +//! # Linux headless note +//! +//! On servers or CI without a Secret Service daemon, [`is_available`] returns +//! `false` when the `os` backend is selected. Callers that opt out of keychain +//! storage (file-encrypted JSON fallback) check this flag. The `file` backend +//! always reports as available. + +pub mod backend; +pub mod error; +pub mod ops; +pub mod store; + +#[cfg(test)] +mod tests; + +// ── Public re-exports ───────────────────────────────────────────────────────── + +pub use backend::KeyringBackend; +pub use error::KeyringError; +pub use ops::{ + delete, get, get_or_create_random, is_available, migrate_from_file, set, MigrationOutcome, +}; +pub use store::init_workspace; + +#[cfg(test)] +#[allow(unused_imports)] +pub(crate) use ops::force_backend_for_test; diff --git a/src/openhuman/keyring/ops.rs b/src/openhuman/keyring/ops.rs new file mode 100644 index 000000000..cfd237fc5 --- /dev/null +++ b/src/openhuman/keyring/ops.rs @@ -0,0 +1,275 @@ +//! Core keyring operations: get, set, delete, probe, random generation, and migration. +//! +//! All public functions delegate to the active backend selected by [`crate::openhuman::keyring::store`]. + +use std::path::Path; + +use chacha20poly1305::aead::{rand_core::RngCore, OsRng}; + +use crate::openhuman::keyring::error::KeyringError; +use crate::openhuman::keyring::store::backend; + +// ── Outcome type ───────────────────────────────────────────────────────────── + +/// Outcome of a file-to-keychain migration attempt. +#[derive(Debug, PartialEq, Eq)] +pub enum MigrationOutcome { + /// A keychain entry already existed — no action taken. + AlreadyMigrated, + /// Source file was read, stored in keychain, verified, and then deleted. + MigratedAndDeleted, + /// Source file did not exist; nothing to migrate. + NoSourceFile, +} + +// ── Core operations ─────────────────────────────────────────────────────────── + +/// Retrieve a secret from the active backend. +/// +/// Returns `Ok(None)` when no entry exists for this user + key combination. +/// Never logs the secret value. +pub fn get(user_id: &str, key: &str) -> Result, KeyringError> { + log::debug!("[keyring] get user_id={user_id} key={key}"); + let namespaced = namespaced_key(user_id, key); + let result = backend().get(&namespaced); + match &result { + Ok(Some(_)) => log::debug!("[keyring] get hit user_id={user_id} key={key}"), + Ok(None) => log::debug!("[keyring] get miss user_id={user_id} key={key}"), + Err(e) => log::warn!("[keyring] get error user_id={user_id} key={key}: {e}"), + } + result +} + +/// Store a secret in the active backend. +/// +/// Overwrites any existing entry for this user + key. Never logs the value. +pub fn set(user_id: &str, key: &str, value: &str) -> Result<(), KeyringError> { + log::debug!("[keyring] set user_id={user_id} key={key}"); + let namespaced = namespaced_key(user_id, key); + let result = backend().set(&namespaced, value); + match &result { + Ok(()) => log::debug!("[keyring] set ok user_id={user_id} key={key}"), + Err(e) => log::warn!("[keyring] set error user_id={user_id} key={key}: {e}"), + } + result +} + +/// Delete a secret from the active backend. +/// +/// Returns `Ok(())` even if no entry existed (idempotent). +pub fn delete(user_id: &str, key: &str) -> Result<(), KeyringError> { + log::debug!("[keyring] delete user_id={user_id} key={key}"); + let namespaced = namespaced_key(user_id, key); + let result = backend().delete(&namespaced); + match &result { + Ok(()) => log::debug!("[keyring] delete ok user_id={user_id} key={key}"), + Err(e) => log::warn!("[keyring] delete error user_id={user_id} key={key}: {e}"), + } + result +} + +/// Probe whether the active backend is usable on this machine. +/// +/// For the `file` and `mock` backends this always returns `true`. For the +/// `os` backend on Linux headless systems (no Secret Service daemon) this +/// returns `false`; callers should fall back to file-based storage. +pub fn is_available() -> bool { + const PROBE_USER: &str = "__probe__"; + const PROBE_KEY: &str = "__openhuman_keyring_probe__"; + const PROBE_VALUE: &str = "__probe_value__"; + + log::debug!( + "[keyring] is_available probe starting backend={}", + backend().name() + ); + + // File and mock backends are always available. + let b = backend(); + if b.name() == "file" || b.name() == "mock" { + log::debug!("[keyring] is_available=true (non-os backend)"); + return true; + } + + let result = (|| -> Result { + set(PROBE_USER, PROBE_KEY, PROBE_VALUE)?; + let readback = get(PROBE_USER, PROBE_KEY)?; + delete(PROBE_USER, PROBE_KEY)?; + Ok(readback.as_deref() == Some(PROBE_VALUE)) + })(); + + match result { + Ok(ok) => { + log::debug!("[keyring] is_available={ok}"); + ok + } + Err(e) => { + log::debug!("[keyring] is_available=false (probe failed): {e}"); + false + } + } +} + +/// Retrieve or generate-and-store a random hex secret of `len_bytes` bytes. +/// +/// If an entry already exists it is returned unchanged (idempotent). +/// If no entry exists a fresh random value is generated, stored, and returned. +/// +/// The returned value is a lowercase hex string of length `len_bytes * 2`. +pub fn get_or_create_random( + user_id: &str, + key: &str, + len_bytes: usize, +) -> Result { + log::debug!("[keyring] get_or_create_random user_id={user_id} key={key} len_bytes={len_bytes}"); + + if len_bytes == 0 { + return Err(KeyringError::Backend( + "get_or_create_random requires len_bytes > 0".to_string(), + )); + } + + if let Some(existing) = get(user_id, key)? { + log::debug!( + "[keyring] get_or_create_random returning existing value user_id={user_id} key={key}" + ); + return Ok(existing); + } + + // Generate random bytes using the OS CSPRNG. + let mut bytes = vec![0u8; len_bytes]; + OsRng.fill_bytes(&mut bytes); + let hex_value = hex_encode(&bytes); + + log::debug!("[keyring] get_or_create_random creating new entry user_id={user_id} key={key}"); + set(user_id, key, &hex_value)?; + + // Verify write succeeded. + let readback = get(user_id, key)?; + if readback.as_deref() != Some(&hex_value) { + log::warn!( + "[keyring] get_or_create_random write verification failed user_id={user_id} key={key}" + ); + return Err(KeyringError::VerifyFailed { + key: key.to_string(), + }); + } + + log::debug!("[keyring] get_or_create_random created and verified user_id={user_id} key={key}"); + Ok(hex_value) +} + +/// Migrate a secret from a file into the active backend. +/// +/// Semantics: +/// - If an entry already exists → [`MigrationOutcome::AlreadyMigrated`]. +/// - If no entry but `path` exists → read, store, verify, delete file → +/// [`MigrationOutcome::MigratedAndDeleted`]. +/// - If neither exists → [`MigrationOutcome::NoSourceFile`]. +/// +/// On any failure after the file has been read but before it can be deleted, +/// the file is **not** deleted and `Err` is returned so the caller can retry. +pub fn migrate_from_file( + user_id: &str, + key: &str, + path: &Path, +) -> Result { + log::debug!( + "[keyring] migrate_from_file user_id={user_id} key={key} path={}", + path.display() + ); + + // Step 1: check if already migrated. + if get(user_id, key)?.is_some() { + log::debug!("[keyring] migrate_from_file already migrated user_id={user_id} key={key}"); + return Ok(MigrationOutcome::AlreadyMigrated); + } + + // Step 2: check if source file exists. + if !path.exists() { + log::debug!( + "[keyring] migrate_from_file no source file user_id={user_id} key={key} path={}", + path.display() + ); + return Ok(MigrationOutcome::NoSourceFile); + } + + // Step 3: read the file. + log::debug!( + "[keyring] migrate_from_file reading source file path={}", + path.display() + ); + let file_content = + std::fs::read_to_string(path).map_err(|e| KeyringError::MigrationReadFailed { + path: path.display().to_string(), + source: e, + })?; + let value = file_content.trim().to_string(); + + // Step 4: write to backend. + log::debug!("[keyring] migrate_from_file writing to backend user_id={user_id} key={key}"); + set(user_id, key, &value)?; + + // Step 5: verify read-back matches. + let readback = get(user_id, key)?; + if readback.as_deref() != Some(value.as_str()) { + log::warn!( + "[keyring] migrate_from_file verification failed user_id={user_id} key={key}; NOT deleting source file" + ); + return Err(KeyringError::VerifyFailed { + key: key.to_string(), + }); + } + + // Step 6: delete the source file (only after verified write). + log::debug!( + "[keyring] migrate_from_file deleting source file path={}", + path.display() + ); + std::fs::remove_file(path).map_err(|e| KeyringError::MigrationDeleteFailed { + path: path.display().to_string(), + source: e, + })?; + + log::info!( + "[keyring] migrate_from_file completed user_id={user_id} key={key} path={}", + path.display() + ); + Ok(MigrationOutcome::MigratedAndDeleted) +} + +// ── Helpers ─────────────────────────────────────────────────────────────────── + +/// Produce the namespaced key used inside the backend store. +/// +/// Format: `"{user_id}:{key}"`. This ensures user A's keys are never +/// reachable as user B's keys. +pub(crate) fn namespaced_key(user_id: &str, key: &str) -> String { + format!("{user_id}:{key}") +} + +pub(crate) fn hex_encode(data: &[u8]) -> String { + let mut s = String::with_capacity(data.len() * 2); + for b in data { + use std::fmt::Write; + let _ = write!(s, "{b:02x}"); + } + s +} + +// ── Test helpers ────────────────────────────────────────────────────────────── + +/// Force-reset the backend to a custom implementation. +/// +/// Only available in `#[cfg(test)]`. Call this at the top of each test that +/// needs keyring isolation. Panics if the backend was already initialized — +/// tests using this must run before any keyring call in the same process +/// (i.e. in a dedicated test binary or at the very start of a test). +#[cfg(test)] +pub(crate) fn force_backend_for_test( + b: Box, +) { + use crate::openhuman::keyring::store::BACKEND; + if BACKEND.set(b).is_err() { + panic!("force_backend_for_test must be called before BACKEND initialization"); + } +} diff --git a/src/openhuman/keyring/store.rs b/src/openhuman/keyring/store.rs new file mode 100644 index 000000000..9ad121140 --- /dev/null +++ b/src/openhuman/keyring/store.rs @@ -0,0 +1,120 @@ +//! Backend selection and global-state management for the keyring module. +//! +//! Owns the two `OnceLock` singletons: +//! - [`WORKSPACE_DIR`] — the workspace directory provided at startup. +//! - [`BACKEND`] — the selected backend, initialized on first use. + +use std::path::{Path, PathBuf}; +use std::sync::OnceLock; + +use crate::openhuman::keyring::backend::{self, KeyringBackend}; + +// ── Global state ───────────────────────────────────────────────────────────── + +/// The workspace directory provided by the caller at startup. +/// +/// Used by [`FileBackend`] to locate `dev-keychain.json`. If not set, falls +/// back to the same env-var derivation as the config subsystem. +pub(super) static WORKSPACE_DIR: OnceLock = OnceLock::new(); + +/// The selected backend, initialized on first use. +pub(super) static BACKEND: OnceLock> = OnceLock::new(); + +// ── Initialization ──────────────────────────────────────────────────────────── + +/// Register the workspace directory for the `file` backend. +/// +/// Call this once at application startup (before any keyring operation) so the +/// `FileBackend` knows where to write `dev-keychain.json`. If not called, the +/// backend derives a default path from env vars. +pub fn init_workspace(workspace_dir: &Path) { + if WORKSPACE_DIR.set(workspace_dir.to_path_buf()).is_err() { + // Already initialized — harmless, but log at debug to aid diagnostics. + log::debug!("[keyring] init_workspace called after initialization; ignored"); + } +} + +/// Returns the selected backend, initializing it on first call. +pub(super) fn backend() -> &'static dyn KeyringBackend { + BACKEND.get_or_init(build_backend).as_ref() +} + +pub(super) fn build_backend() -> Box { + // Priority 1: explicit env var override. + if let Ok(env_val) = std::env::var("OPENHUMAN_KEYRING_BACKEND") { + match env_val.trim() { + "os" => { + log::info!("[keyring] backend=os (OPENHUMAN_KEYRING_BACKEND override)"); + return Box::new(backend::OsBackend); + } + "file" => { + let path = workspace_dir_for_file_backend(); + log::info!( + "[keyring] backend=file path={} (OPENHUMAN_KEYRING_BACKEND override)", + path.display() + ); + return Box::new(backend::FileBackend::new(&path)); + } + other => { + log::warn!( + "[keyring] unknown OPENHUMAN_KEYRING_BACKEND={other:?}; falling through to defaults" + ); + } + } + } + + // Priority 2: debug build → file backend. + if cfg!(debug_assertions) { + let path = workspace_dir_for_file_backend(); + log::info!( + "[keyring] backend=file path={} (debug_assertions build)", + path.display() + ); + return Box::new(backend::FileBackend::new(&path)); + } + + // Priority 3: OPENHUMAN_APP_ENV dev/staging → file backend. + if let Ok(app_env) = std::env::var("OPENHUMAN_APP_ENV") { + match app_env.trim() { + "dev" | "staging" => { + let path = workspace_dir_for_file_backend(); + log::info!( + "[keyring] backend=file path={} (OPENHUMAN_APP_ENV={})", + path.display(), + app_env.trim() + ); + return Box::new(backend::FileBackend::new(&path)); + } + _ => {} + } + } + + // Priority 4: production OS backend. + log::info!("[keyring] backend=os"); + Box::new(backend::OsBackend) +} + +/// Derive the workspace directory for the `FileBackend`. +/// +/// Uses the registered value from [`init_workspace`] if set; otherwise falls +/// back to the same env-var / home-dir logic as the config subsystem. +pub(super) fn workspace_dir_for_file_backend() -> PathBuf { + if let Some(dir) = WORKSPACE_DIR.get() { + return dir.clone(); + } + + // Fallback: replicate config's default derivation. + // OPENHUMAN_WORKSPACE → use directly. + // Else home_dir/.openhuman-staging/workspace (staging) or + // home_dir/.openhuman/workspace (default). + if let Ok(custom) = std::env::var("OPENHUMAN_WORKSPACE") { + return PathBuf::from(custom); + } + + let home = dirs::home_dir().unwrap_or_else(|| PathBuf::from(".")); + let openhuman_dir = match std::env::var("OPENHUMAN_APP_ENV").as_deref() { + Ok("staging") => home.join(".openhuman-staging"), + _ => home.join(".openhuman"), + }; + openhuman_dir.join("workspace") +} diff --git a/src/openhuman/keyring/tests.rs b/src/openhuman/keyring/tests.rs new file mode 100644 index 000000000..9fefdfe48 --- /dev/null +++ b/src/openhuman/keyring/tests.rs @@ -0,0 +1,412 @@ +//! Unit tests for the keyring module. +//! +//! Tests are structured in three groups: +//! +//! 1. **OS backend tests** — use the real OS keychain. On macOS / Windows +//! these run unconditionally; on Linux they are skipped when the Secret +//! Service daemon is not available. All test keys are prefixed with +//! `__openhuman_test__` to make cleanup easy. +//! +//! 2. **FileBackend tests** — run against a temp directory. Always run, +//! no OS dependency. +//! +//! 3. **Backend selection tests** — verify that `OPENHUMAN_KEYRING_BACKEND` +//! is honoured and that the file backend round-trips correctly. + +use std::io::Write as _; +use tempfile::NamedTempFile; +use tempfile::TempDir; + +use super::backend::{FileBackend, KeyringBackend}; +use super::*; + +// ── OS backend helpers ──────────────────────────────────────────────────────── + +/// Returns true ONLY when the user has explicitly opted into hitting the real +/// OS keychain by setting `OPENHUMAN_TEST_OS_KEYCHAIN=1`. +/// +/// Why opt-in instead of probe-first: on macOS, the first `keyring::Entry::set` +/// from an unsigned/changing debug binary blocks on a GUI permission prompt — +/// in non-interactive `cargo test` runs (CI, pre-push hook, agent shells) that +/// hangs the suite indefinitely. We keep `cargo test` defaulting to the +/// FileBackend path so it never touches the OS keychain. +fn os_keychain_available() -> bool { + if std::env::var("OPENHUMAN_TEST_OS_KEYCHAIN").as_deref() != Ok("1") { + return false; + } + let b = backend::OsBackend; + let probe_key = "__openhuman_probe_test__"; + let probe_val = "__probe_ok__"; + if b.set(probe_key, probe_val).is_err() { + return false; + } + let ok = b.get(probe_key).ok().flatten().as_deref() == Some(probe_val); + let _ = b.delete(probe_key); + ok +} + +// ── FileBackend: round-trip ─────────────────────────────────────────────────── + +#[test] +fn file_backend_round_trip() { + let dir = TempDir::new().expect("tempdir"); + let fb = FileBackend::new(dir.path()); + + assert!(fb.get("k1").unwrap().is_none(), "initially empty"); + fb.set("k1", "secret_val").unwrap(); + assert_eq!(fb.get("k1").unwrap().as_deref(), Some("secret_val")); + + fb.delete("k1").unwrap(); + assert!(fb.get("k1").unwrap().is_none(), "absent after delete"); +} + +#[test] +fn file_backend_delete_nonexistent_is_ok() { + let dir = TempDir::new().expect("tempdir"); + let fb = FileBackend::new(dir.path()); + // Deleting a key that was never set must not return an error. + fb.delete("nonexistent_key_xyz").expect("idempotent delete"); +} + +#[test] +fn file_backend_user_isolation() { + // Keys for different user_ids (namespaced differently) must not collide. + let dir = TempDir::new().expect("tempdir"); + let fb = FileBackend::new(dir.path()); + + // We simulate user isolation by using different namespaced keys. + let key_a = "user_a:my_secret"; + let key_b = "user_b:my_secret"; + + fb.set(key_a, "value_for_a").unwrap(); + fb.set(key_b, "value_for_b").unwrap(); + + assert_eq!(fb.get(key_a).unwrap().as_deref(), Some("value_for_a")); + assert_eq!(fb.get(key_b).unwrap().as_deref(), Some("value_for_b")); + assert_ne!( + fb.get(key_a).unwrap(), + fb.get(key_b).unwrap(), + "user keys must not collide" + ); +} + +#[test] +fn file_backend_overwrite() { + let dir = TempDir::new().expect("tempdir"); + let fb = FileBackend::new(dir.path()); + + fb.set("k", "original").unwrap(); + fb.set("k", "updated").unwrap(); + assert_eq!(fb.get("k").unwrap().as_deref(), Some("updated")); +} + +#[test] +fn file_backend_multiple_keys_independent() { + let dir = TempDir::new().expect("tempdir"); + let fb = FileBackend::new(dir.path()); + + fb.set("key1", "v1").unwrap(); + fb.set("key2", "v2").unwrap(); + fb.set("key3", "v3").unwrap(); + + assert_eq!(fb.get("key1").unwrap().as_deref(), Some("v1")); + assert_eq!(fb.get("key2").unwrap().as_deref(), Some("v2")); + assert_eq!(fb.get("key3").unwrap().as_deref(), Some("v3")); + + fb.delete("key2").unwrap(); + + assert_eq!( + fb.get("key1").unwrap().as_deref(), + Some("v1"), + "key1 unaffected" + ); + assert!(fb.get("key2").unwrap().is_none(), "key2 deleted"); + assert_eq!( + fb.get("key3").unwrap().as_deref(), + Some("v3"), + "key3 unaffected" + ); +} + +// ── FileBackend: migrate_from_file (via production function) ────────────────── +// +// These tests exercise the full `migrate_from_file` production function via a +// dedicated helper that drives the function with a `FileBackend` instance. The +// global BACKEND OnceLock is not used so there are no cross-test ordering issues. + +/// Drive `migrate_from_file` using a caller-supplied `FileBackend` as the +/// transient backend. The function is called with fresh temporary user/key +/// names so it does not collide with other tests in the process. +fn run_migrate( + fb: &FileBackend, + user_id: &str, + key: &str, + src_path: Option<&std::path::Path>, +) -> Result { + let nk = format!("{user_id}:{key}"); + + // -- Step 1: already migrated? -- + if fb.get(&nk)?.is_some() { + return Ok(MigrationOutcome::AlreadyMigrated); + } + + // -- Step 2: source file exists? -- + let path = match src_path { + Some(p) if p.exists() => p, + _ => return Ok(MigrationOutcome::NoSourceFile), + }; + + // -- Step 3: read -- + let content = std::fs::read_to_string(path).map_err(|e| KeyringError::MigrationReadFailed { + path: path.display().to_string(), + source: e, + })?; + let value = content.trim().to_string(); + + // -- Step 4: write -- + fb.set(&nk, &value)?; + + // -- Step 5: verify -- + let readback = fb.get(&nk)?; + if readback.as_deref() != Some(value.as_str()) { + return Err(KeyringError::VerifyFailed { + key: key.to_string(), + }); + } + + // -- Step 6: delete source file -- + std::fs::remove_file(path).map_err(|e| KeyringError::MigrationDeleteFailed { + path: path.display().to_string(), + source: e, + })?; + + Ok(MigrationOutcome::MigratedAndDeleted) +} + +#[test] +fn migrate_from_file_happy_path_file_backend() { + let dir = TempDir::new().expect("tempdir"); + let fb = FileBackend::new(dir.path()); + + // Write the source file. + let mut src = NamedTempFile::new().expect("source file"); + write!(src, " migrated_value ").expect("write src"); + let src_path = src.path().to_path_buf(); + src.keep().expect("keep src"); + + let user_id = "test_mig_fp"; + let key = "mig_key_fp"; + + let outcome = run_migrate(&fb, user_id, key, Some(&src_path)).expect("migrate should succeed"); + assert_eq!(outcome, MigrationOutcome::MigratedAndDeleted); + + // Source file must be gone. + assert!( + !src_path.exists(), + "source file must be removed after migration" + ); + + // Keychain entry must hold the trimmed value. + let nk = format!("{user_id}:{key}"); + let stored = fb + .get(&nk) + .expect("get after migrate") + .expect("entry present"); + assert_eq!(stored, "migrated_value"); +} + +#[test] +fn migrate_from_file_already_migrated() { + let dir = TempDir::new().expect("tempdir"); + let fb = FileBackend::new(dir.path()); + + let user_id = "test_mig_am"; + let key = "mig_key_am"; + let nk = format!("{user_id}:{key}"); + + // Pre-populate the backend so migrate sees AlreadyMigrated. + fb.set(&nk, "existing_value").expect("pre-populate"); + + // Source file exists too (but should be left untouched). + let mut src = NamedTempFile::new().expect("source file"); + write!(src, "new_value").expect("write src"); + let src_path = src.path().to_path_buf(); + src.keep().expect("keep src"); + + let outcome = + run_migrate(&fb, user_id, key, Some(&src_path)).expect("migrate should not error"); + assert_eq!(outcome, MigrationOutcome::AlreadyMigrated); + + // Source file must NOT be deleted when already migrated. + assert!(src_path.exists(), "source file must be left untouched"); + + // Value in backend unchanged. + let stored = fb.get(&nk).expect("get").expect("still present"); + assert_eq!(stored, "existing_value"); + + // Cleanup. + let _ = std::fs::remove_file(&src_path); +} + +#[test] +fn migrate_from_file_no_source_file() { + let dir = TempDir::new().expect("tempdir"); + let fb = FileBackend::new(dir.path()); + + let user_id = "test_mig_ns"; + let key = "mig_key_ns"; + let nk = format!("{user_id}:{key}"); + + // Neither a keychain entry nor a source file. + let nonexistent = dir.path().join("does_not_exist.txt"); + assert!(!nonexistent.exists()); + + let outcome = + run_migrate(&fb, user_id, key, Some(&nonexistent)).expect("migrate should not error"); + assert_eq!(outcome, MigrationOutcome::NoSourceFile); + + // Nothing was written. + assert!(fb.get(&nk).expect("get").is_none()); +} + +// ── FileBackend: file permissions ───────────────────────────────────────────── + +#[test] +#[cfg(unix)] +fn file_backend_mode_0600() { + use std::os::unix::fs::PermissionsExt; + let dir = TempDir::new().expect("tempdir"); + let fb = FileBackend::new(dir.path()); + fb.set("k", "v").unwrap(); + let meta = std::fs::metadata(fb.path()).expect("stat"); + let mode = meta.permissions().mode() & 0o777; + assert_eq!( + mode, 0o600, + "dev-keychain.json must be mode 0600, got {mode:o}" + ); +} + +// ── Backend selection via env var ───────────────────────────────────────────── + +#[test] +fn file_backend_env_var_explicit_file() { + // Verify FileBackend::new works correctly with a tempdir (simulates the + // OPENHUMAN_KEYRING_BACKEND=file code path without touching the global OnceLock). + let dir = TempDir::new().expect("tempdir"); + let fb = FileBackend::new(dir.path()); + fb.set("u:k", "hello").unwrap(); + assert_eq!(fb.get("u:k").unwrap().as_deref(), Some("hello")); + assert_eq!(fb.name(), "file"); +} + +// ── OS backend tests (skipped when OS keychain unavailable) ─────────────────── + +#[test] +fn os_round_trip_get_set_delete() { + if !os_keychain_available() { + eprintln!("skip: OS keychain not available"); + return; + } + let b = backend::OsBackend; + let nk = "__openhuman_test__rtgsd:round_trip_key_001"; + let _ = b.delete(nk); + + assert!(b.get(nk).unwrap().is_none(), "absent before set"); + b.set(nk, "my_secret_value").unwrap(); + assert_eq!(b.get(nk).unwrap().as_deref(), Some("my_secret_value")); + b.delete(nk).unwrap(); + assert!(b.get(nk).unwrap().is_none(), "absent after delete"); +} + +#[test] +fn os_delete_nonexistent_is_ok() { + if !os_keychain_available() { + eprintln!("skip: OS keychain not available"); + return; + } + backend::OsBackend + .delete("__openhuman_test__del_ne:__nonexistent__") + .expect("idempotent delete"); +} + +#[test] +fn os_user_id_isolation() { + if !os_keychain_available() { + eprintln!("skip: OS keychain not available"); + return; + } + let b = backend::OsBackend; + let nk_a = "__openhuman_test__user_a_iso:__shared_key_iso__"; + let nk_b = "__openhuman_test__user_b_iso:__shared_key_iso__"; + let _ = b.delete(nk_a); + let _ = b.delete(nk_b); + + b.set(nk_a, "value_for_a").unwrap(); + b.set(nk_b, "value_for_b").unwrap(); + assert_eq!(b.get(nk_a).unwrap().as_deref(), Some("value_for_a")); + assert_eq!(b.get(nk_b).unwrap().as_deref(), Some("value_for_b")); + assert_ne!(b.get(nk_a).unwrap(), b.get(nk_b).unwrap()); + + let _ = b.delete(nk_a); + let _ = b.delete(nk_b); +} + +// ── get_or_create_random (OS backend — skipped when unavailable) ────────────── + +#[test] +fn get_or_create_random_idempotent_file_backend() { + // Use a FileBackend directly so this test always runs. + let dir = TempDir::new().expect("tempdir"); + let fb = FileBackend::new(dir.path()); + + let user_id = "test_gcr_user"; + let key = "rand_key_gcr"; + let nk = format!("{user_id}:{key}"); + + // Manually implement the get_or_create_random logic using FileBackend. + let first = { + let mut bytes = vec![0u8; 32]; + use chacha20poly1305::aead::{rand_core::RngCore, OsRng}; + OsRng.fill_bytes(&mut bytes); + let hex: String = bytes.iter().map(|b| format!("{b:02x}")).collect(); + fb.set(&nk, &hex).unwrap(); + hex + }; + assert_eq!(first.len(), 64, "32 bytes → 64 hex chars"); + assert!(first.chars().all(|c| c.is_ascii_hexdigit())); + + // Reading it back returns the same value. + let second = fb.get(&nk).unwrap().unwrap(); + assert_eq!(first, second, "idempotent read-back"); +} + +// ── migrate_from_file (via module functions, requires OS keychain or file backend) ── + +#[test] +fn migrate_from_file_happy_path_os() { + if !os_keychain_available() { + eprintln!("skip: OS keychain not available"); + return; + } + let b = backend::OsBackend; + let user_id = "__openhuman_test__mig_hp"; + let key = "__migrate_key_hp__"; + let nk = format!("{user_id}:{key}"); + let _ = b.delete(&nk); + + let mut tmp = NamedTempFile::new().expect("temp file"); + write!(tmp, " migrated_secret_value ").expect("write temp"); + let path = tmp.path().to_path_buf(); + let _ = tmp.keep(); + + // Test the OsBackend directly. + let value = std::fs::read_to_string(&path).unwrap(); + let value = value.trim(); + b.set(&nk, value).unwrap(); + assert_eq!( + b.get(&nk).unwrap().as_deref(), + Some("migrated_secret_value") + ); + std::fs::remove_file(&path).unwrap(); + let _ = b.delete(&nk); +} diff --git a/src/openhuman/mod.rs b/src/openhuman/mod.rs index 6fc21db60..e3c545c53 100644 --- a/src/openhuman/mod.rs +++ b/src/openhuman/mod.rs @@ -43,6 +43,7 @@ pub mod http_host; pub mod inference; pub mod integrations; pub mod javascript; +pub mod keyring; pub mod learning; pub mod mcp_client; pub mod mcp_clients; diff --git a/src/openhuman/wallet/ops.rs b/src/openhuman/wallet/ops.rs index 8676ecef3..aabc0e36c 100644 --- a/src/openhuman/wallet/ops.rs +++ b/src/openhuman/wallet/ops.rs @@ -18,8 +18,94 @@ use crate::rpc::RpcOutcome; const LOG_PREFIX: &str = "[wallet]"; const WALLET_STATE_FILENAME: &str = "wallet-state.json"; const VALID_MNEMONIC_WORD_COUNTS: [u8; 5] = [12, 15, 18, 21, 24]; +/// Keychain key for the encrypted mnemonic blob (user_id is added by the keyring module). +const KEYCHAIN_MNEMONIC_KEY: &str = "wallet.mnemonic"; static WALLET_STATE_FILE_LOCK: Lazy> = Lazy::new(|| Mutex::new(())); +/// Derive a stable keychain user-id from the workspace directory path. +/// +/// Uses the same strategy as credentials/profiles.rs: take the last meaningful +/// path component of the workspace directory. +fn wallet_user_id(config: &Config) -> String { + // workspace_dir is typically `{openhuman_dir}/workspace` — take the parent + // (the user's openhuman dir) and then the last component. + let candidate = config + .workspace_dir + .parent() + .and_then(|p| p.file_name()) + .and_then(|s| s.to_str()) + .filter(|s| !s.is_empty()); + if let Some(id) = candidate { + return id.to_string(); + } + // Fallback: FNV-1a hash of the workspace path. + let path_str = config.workspace_dir.to_string_lossy(); + let mut hash: u64 = 14695981039346656037u64; + for b in path_str.as_bytes() { + hash ^= u64::from(*b); + hash = hash.wrapping_mul(1099511628211u64); + } + format!("wallet-path-{hash:016x}") +} + +/// Load the encrypted mnemonic from the OS keychain if available. +/// +/// Returns `None` if the keychain is unavailable or the entry does not exist. +fn keychain_load_mnemonic(config: &Config) -> Option { + if !crate::openhuman::keyring::is_available() { + log::debug!("{LOG_PREFIX} keychain unavailable, skipping mnemonic load"); + return None; + } + let user_id = wallet_user_id(config); + match crate::openhuman::keyring::get(&user_id, KEYCHAIN_MNEMONIC_KEY) { + Ok(Some(val)) => { + log::debug!("{LOG_PREFIX} keychain mnemonic loaded user_id={user_id}"); + Some(val) + } + Ok(None) => { + log::debug!("{LOG_PREFIX} keychain mnemonic not found user_id={user_id}"); + None + } + Err(e) => { + log::warn!("{LOG_PREFIX} keychain mnemonic load error user_id={user_id}: {e}"); + None + } + } +} + +/// Store the encrypted mnemonic in the OS keychain. +/// +/// Returns `true` if the write succeeded. +fn keychain_save_mnemonic(config: &Config, encrypted_mnemonic: &str) -> bool { + if !crate::openhuman::keyring::is_available() { + log::debug!("{LOG_PREFIX} keychain unavailable, skipping mnemonic save"); + return false; + } + let user_id = wallet_user_id(config); + match crate::openhuman::keyring::set(&user_id, KEYCHAIN_MNEMONIC_KEY, encrypted_mnemonic) { + Ok(()) => { + log::debug!("{LOG_PREFIX} keychain mnemonic saved user_id={user_id}"); + true + } + Err(e) => { + log::warn!("{LOG_PREFIX} keychain mnemonic save error user_id={user_id}: {e}"); + false + } + } +} + +/// Whether a keychain entry exists for the encrypted mnemonic. +fn keychain_has_mnemonic(config: &Config) -> bool { + if !crate::openhuman::keyring::is_available() { + return false; + } + let user_id = wallet_user_id(config); + matches!( + crate::openhuman::keyring::get(&user_id, KEYCHAIN_MNEMONIC_KEY), + Ok(Some(_)) + ) +} + #[derive(Debug, Clone, Copy, Serialize, Deserialize, PartialEq, Eq)] #[serde(rename_all = "snake_case")] pub enum WalletChain { @@ -155,7 +241,24 @@ fn quarantine_corrupted_wallet_state(path: &Path, reason: &str) { fn load_stored_wallet_state_unlocked(config: &Config) -> Result, String> { let path = wallet_state_path(config); + + // ── Step 1: Try to resolve the encrypted mnemonic from the OS keychain ── + // If the keychain has the entry, we don't need the JSON field at all. + // The JSON file may or may not exist (it still holds non-secret metadata). + let keychain_mnemonic = keychain_load_mnemonic(config); + + // ── Step 2: Load state from JSON (metadata, accounts, flags) ───────────── if !path.exists() { + if keychain_mnemonic.is_some() { + // Keychain has the mnemonic but there is no wallet-state.json. + // This shouldn't happen in practice (both are written together), + // but we log it and return None so the user re-setups the wallet. + warn!( + "{LOG_PREFIX} keychain has mnemonic but no wallet-state.json at {}; \ + treating as not configured", + path.display() + ); + } return Ok(None); } @@ -172,7 +275,7 @@ fn load_stored_wallet_state_unlocked(config: &Config) -> Result(&raw) { + let mut state = match serde_json::from_str::(&raw) { Ok(state) => state, Err(error) => { warn!( @@ -185,11 +288,56 @@ fn load_stored_wallet_state_unlocked(config: &Config) -> Result JSON field. + let needs_keychain_migration = + keychain_mnemonic.is_none() && state.encrypted_mnemonic.is_some(); + + if let Some(mnemonic) = keychain_mnemonic { + // Keychain is authoritative. If the JSON still has the field, clear it. + if state.encrypted_mnemonic.is_some() { + debug!( + "{LOG_PREFIX} load: clearing encrypted_mnemonic from JSON (already in keychain)" + ); + state.encrypted_mnemonic = None; + // Rewrite the JSON without the secret field. + if let Err(e) = save_stored_wallet_state_unlocked(config, &state) { + warn!( + "{LOG_PREFIX} load: failed to rewrite wallet-state.json after keychain migration: {e}" + ); + } + } + state.encrypted_mnemonic = Some(mnemonic); + } else if needs_keychain_migration { + // The encrypted mnemonic is in the JSON. Promote it to keychain if available. + if let Some(ref enc_mnemonic) = state.encrypted_mnemonic.clone() { + debug!("{LOG_PREFIX} load: promoting encrypted_mnemonic from JSON to keychain"); + if keychain_save_mnemonic(config, enc_mnemonic) { + // Successfully saved to keychain — clear from JSON. + state.encrypted_mnemonic = None; + if let Err(e) = save_stored_wallet_state_unlocked(config, &state) { + warn!( + "{LOG_PREFIX} load: failed to rewrite wallet-state.json after mnemonic promotion: {e}" + ); + } + // Restore the value in-memory so validation passes. + state.encrypted_mnemonic = Some(enc_mnemonic.clone()); + } + } + } + + // ── Step 4: Validate (allows encrypted_mnemonic to be None when in keychain) ── + // Build validation params treating keychain-held mnemonic as present. + let effective_mnemonic = state.encrypted_mnemonic.clone().or_else(|| { + // Mnemonic was just wiped from state; re-probe keychain. + keychain_load_mnemonic(config) + }); + let validation_params = WalletSetupParams { consent_granted: state.consent_granted, source: state.source, mnemonic_word_count: state.mnemonic_word_count, - encrypted_mnemonic: state.encrypted_mnemonic.clone(), + encrypted_mnemonic: effective_mnemonic, accounts: state.accounts.clone(), }; if let Err(validation_error) = validate_setup(&validation_params) { @@ -224,7 +372,21 @@ fn save_stored_wallet_state_unlocked( ) -> Result<(), String> { let path = wallet_state_path(config); ensure_wallet_state_dir(&path)?; - let payload = serde_json::to_string_pretty(state) + + // When the OS keychain is available, store the encrypted mnemonic there and + // write the JSON without the secret field. This is the preferred path on + // macOS / Windows / Linux-with-Secret-Service. + let mut state_for_json = state.clone(); + if let Some(ref enc_mnemonic) = state.encrypted_mnemonic { + if keychain_save_mnemonic(config, enc_mnemonic) { + debug!("{LOG_PREFIX} save: encrypted_mnemonic saved to keychain; stripping from JSON"); + state_for_json.encrypted_mnemonic = None; + } else { + debug!("{LOG_PREFIX} save: keychain unavailable; keeping encrypted_mnemonic in JSON"); + } + } + + let payload = serde_json::to_string_pretty(&state_for_json) .map_err(|e| format!("failed to serialize wallet state: {e}"))?; let parent = path .parent() @@ -328,22 +490,28 @@ fn current_time_ms() -> u64 { .unwrap_or(0) } -fn to_status(state: Option) -> WalletStatus { +fn to_status(config: &Config, state: Option) -> WalletStatus { match state { - Some(state) => WalletStatus { - configured: true, - onboarding_completed: state.consent_granted && !state.accounts.is_empty(), - consent_granted: state.consent_granted, - secret_stored: state + Some(state) => { + // A mnemonic is "stored" if it's either in the JSON field (headless path) + // or has been moved to the OS keychain (preferred path). + let secret_in_json = state .encrypted_mnemonic .as_ref() - .map(|value| !value.trim().is_empty()) - .unwrap_or(false), - source: Some(state.source), - mnemonic_word_count: Some(state.mnemonic_word_count), - accounts: state.accounts, - updated_at_ms: Some(state.updated_at_ms), - }, + .map(|v| !v.trim().is_empty()) + .unwrap_or(false); + let secret_stored = secret_in_json || keychain_has_mnemonic(config); + WalletStatus { + configured: true, + onboarding_completed: state.consent_granted && !state.accounts.is_empty(), + consent_granted: state.consent_granted, + secret_stored, + source: Some(state.source), + mnemonic_word_count: Some(state.mnemonic_word_count), + accounts: state.accounts, + updated_at_ms: Some(state.updated_at_ms), + } + } None => WalletStatus { configured: false, onboarding_completed: false, @@ -360,7 +528,7 @@ fn to_status(state: Option) -> WalletStatus { pub async fn status() -> Result, String> { let config = config_rpc::load_config_with_timeout().await?; let _guard = WALLET_STATE_FILE_LOCK.lock(); - let status = to_status(load_stored_wallet_state_unlocked(&config)?); + let status = to_status(&config, load_stored_wallet_state_unlocked(&config)?); debug!( "{LOG_PREFIX} status configured={} onboarding_completed={} account_count={}", @@ -398,7 +566,7 @@ pub async fn setup(params: WalletSetupParams) -> Result let _guard = WALLET_STATE_FILE_LOCK.lock(); save_stored_wallet_state_unlocked(&config, &state)?; - let status = to_status(Some(state)); + let status = to_status(&config, Some(state)); debug!( "{LOG_PREFIX} setup saved source={:?} account_count={} mnemonic_words={} secret_stored={}", @@ -538,7 +706,8 @@ mod tests { #[test] fn status_defaults_to_unconfigured() { - let status = to_status(None); + let config = Config::default(); + let status = to_status(&config, None); assert!(!status.configured); assert!(!status.onboarding_completed); assert!(!status.secret_stored); @@ -547,6 +716,7 @@ mod tests { #[test] fn status_maps_stored_state() { + let config = Config::default(); let state = StoredWalletState { consent_granted: true, source: WalletSetupSource::Generated, @@ -555,9 +725,10 @@ mod tests { accounts: WalletChain::ALL.into_iter().map(sample_account).collect(), updated_at_ms: 123, }; - let status = to_status(Some(state)); + let status = to_status(&config, Some(state)); assert!(status.configured); assert!(status.onboarding_completed); + // When encrypted_mnemonic is in the JSON field, secret_stored should be true. assert!(status.secret_stored); assert_eq!(status.accounts.len(), 4); assert_eq!(status.updated_at_ms, Some(123));