From 9a1461bcb60e99d51fa8daa2dfaf4528132fa09f Mon Sep 17 00:00:00 2001 From: H-Chris233 Date: Wed, 6 May 2026 10:02:59 +0800 Subject: [PATCH] Protect vault migrations from stale fallback overwrites The migration path must prefer the current legacy JSON source over older per-account vault leftovers, clean those leftovers after a valid manifest is available, and avoid write operations overwriting a damaged or unreadable vault payload with fallback defaults. Constraint: credentials.json is migration-only, but it was the previous Tauri source of truth.\nRejected: Use read fallback for mutating updates | can overwrite unreadable vault data with defaults.\nConfidence: high\nScope-risk: narrow\nDirective: Mutating credential operations must propagate chunked vault read errors instead of saving fallback roots.\nTested: cargo check --manifest-path src-tauri/Cargo.toml\nTested: cargo test --manifest-path src-tauri/Cargo.toml credential_payload_chunks_stay_under_windows_blob_limit\nTested: npm run -s build\nTested: git diff --check\nNot-tested: Manual migration with simultaneous JSON and stale per-account vault entries.\nRelated: #230 --- openless-all/app/src-tauri/src/persistence.rs | 56 +++++++++++++------ 1 file changed, 38 insertions(+), 18 deletions(-) diff --git a/openless-all/app/src-tauri/src/persistence.rs b/openless-all/app/src-tauri/src/persistence.rs index d4e10a32..a7232705 100644 --- a/openless-all/app/src-tauri/src/persistence.rs +++ b/openless-all/app/src-tauri/src/persistence.rs @@ -414,18 +414,20 @@ fn load_legacy_credentials() -> Option { .and_then(|p| read_legacy_credentials_file(&p)) } -fn load_credentials() -> CredsRoot { - match load_keyring_credentials() { - Ok(Some(root)) => return root, - Ok(None) => {} - Err(e) => { - log::warn!("[vault] system credential read failed: {e}"); - return load_legacy_credentials().unwrap_or_default(); +fn legacy_vault_has_credentials(root: &CredsRoot) -> bool { + !root.providers.asr.is_empty() || !root.providers.llm.is_empty() +} + +fn migrate_legacy_sources() -> CredsRoot { + if let Some(legacy) = load_legacy_credentials() { + if let Err(e) = save_credentials(&legacy) { + log::warn!("[vault] legacy credential migration failed: {e}"); } + return legacy; } let legacy_vault = load_legacy_keyring_credentials(); - if !legacy_vault.providers.asr.is_empty() || !legacy_vault.providers.llm.is_empty() { + if legacy_vault_has_credentials(&legacy_vault) { if let Err(e) = save_credentials(&legacy_vault) { log::warn!("[vault] legacy vault credential migration failed: {e}"); } else { @@ -434,14 +436,32 @@ fn load_credentials() -> CredsRoot { return legacy_vault; } - let Some(legacy) = load_legacy_credentials() else { - return CredsRoot::default(); - }; + CredsRoot::default() +} - if let Err(e) = save_credentials(&legacy) { - log::warn!("[vault] legacy credential migration failed: {e}"); +fn load_credentials() -> CredsRoot { + match load_keyring_credentials() { + Ok(Some(root)) => { + remove_legacy_keyring_credentials(); + root + } + Ok(None) => migrate_legacy_sources(), + Err(e) => { + log::warn!("[vault] system credential read failed: {e}"); + load_legacy_credentials().unwrap_or_default() + } + } +} + +fn load_credentials_for_update() -> Result { + match load_keyring_credentials() { + Ok(Some(root)) => { + remove_legacy_keyring_credentials(); + Ok(root) + } + Ok(None) => Ok(migrate_legacy_sources()), + Err(e) => Err(e), } - legacy } fn save_credentials(root: &CredsRoot) -> Result<()> { @@ -866,7 +886,7 @@ impl CredentialsVault { pub fn set(account: CredentialAccount, value: &str) -> Result<()> { let _guard = credentials_lock().lock(); - let mut root = load_credentials(); + let mut root = load_credentials_for_update()?; let v = if value.is_empty() { None } else { @@ -878,7 +898,7 @@ impl CredentialsVault { pub fn remove(account: CredentialAccount) -> Result<()> { let _guard = credentials_lock().lock(); - let mut root = load_credentials(); + let mut root = load_credentials_for_update()?; write_account(&mut root, account, None); save_credentials(&root) } @@ -890,14 +910,14 @@ impl CredentialsVault { pub fn set_active_asr_provider(id: &str) -> Result<()> { let _guard = credentials_lock().lock(); - let mut root = load_credentials(); + let mut root = load_credentials_for_update()?; root.active.asr = id.to_string(); save_credentials(&root) } pub fn set_active_llm_provider(id: &str) -> Result<()> { let _guard = credentials_lock().lock(); - let mut root = load_credentials(); + let mut root = load_credentials_for_update()?; root.active.llm = id.to_string(); save_credentials(&root) }