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
This commit is contained in:
1 parent
9a1461bcb6
commit
b97a54bb89
1 file changed
+16
-6
@@ -300,12 +300,20 @@ fn read_legacy_credentials_file(path: &Path) -> Option<CredsRoot> {
|
||||
}
|
||||
}
|
||||
|
||||
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<CredsRoot> {
|
||||
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(())
|
||||
}
|
||||
|
||||
|
||||
Reference in new issue
Block a user