From b97a54bb89f61c1e1de189900dcedca6966542b2 Mon Sep 17 00:00:00 2001 From: H-Chris233 Date: Wed, 6 May 2026 10:25:44 +0800 Subject: [PATCH] Surface legacy plaintext cleanup failures Vault migration should not silently report success when credentials were written to the OS vault but the previous plaintext credentials file could not be removed. Propagating that cleanup error on write/update paths makes the remaining plaintext exposure visible instead of treating the migration as complete. Constraint: credentials.json is allowed only as a migration source, not a durable fallback after a successful vault write.\nRejected: Keep deletion best-effort only | can leave plaintext credentials indefinitely with no user-visible failure.\nConfidence: high\nScope-risk: narrow\nDirective: Do not hide legacy plaintext deletion failures on credential write paths.\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 permission-denied legacy file cleanup on installed apps.\nRelated: #230 --- openless-all/app/src-tauri/src/persistence.rs | 22 ++++++++++++++----- 1 file changed, 16 insertions(+), 6 deletions(-) diff --git a/openless-all/app/src-tauri/src/persistence.rs b/openless-all/app/src-tauri/src/persistence.rs index a7232705..13b660d6 100644 --- a/openless-all/app/src-tauri/src/persistence.rs +++ b/openless-all/app/src-tauri/src/persistence.rs @@ -300,12 +300,20 @@ fn read_legacy_credentials_file(path: &Path) -> Option { } } -fn remove_legacy_credentials_file() { - let Ok(path) = credentials_path() else { return }; +fn remove_legacy_credentials_file() -> Result<()> { + let Ok(path) = credentials_path() else { + return Ok(()); + }; if path.exists() { - if let Err(e) = fs::remove_file(&path) { - log::warn!("[vault] remove legacy {} failed: {}", path.display(), e); - } + fs::remove_file(&path) + .with_context(|| format!("remove legacy credentials file {}", path.display()))?; + } + Ok(()) +} + +fn remove_legacy_credentials_file_best_effort() { + if let Err(e) = remove_legacy_credentials_file() { + log::warn!("[vault] remove legacy credentials file failed: {e}"); } } @@ -443,6 +451,7 @@ fn load_credentials() -> CredsRoot { match load_keyring_credentials() { Ok(Some(root)) => { remove_legacy_keyring_credentials(); + remove_legacy_credentials_file_best_effort(); root } Ok(None) => migrate_legacy_sources(), @@ -457,6 +466,7 @@ fn load_credentials_for_update() -> Result { match load_keyring_credentials() { Ok(Some(root)) => { remove_legacy_keyring_credentials(); + remove_legacy_credentials_file()?; Ok(root) } Ok(None) => Ok(migrate_legacy_sources()), @@ -501,7 +511,7 @@ fn save_credentials(root: &CredsRoot) -> Result<()> { } } - remove_legacy_credentials_file(); + remove_legacy_credentials_file()?; Ok(()) }