fix: use atomic write in APIKeyManager.save() to prevent credential data loss (#4591) (#4597)

* fix: use atomic write in APIKeyManager.save() to prevent data loss

Opening api_keys.json with 'w' truncates the file before writing, so a
crash, disk-full, or mid-write error leaves all stored provider API keys
corrupted. Switch to atomic write (temp file + fsync + os.replace) so
the original file is always intact on any failure.

Fixes #4591

* chore: trigger CI re-run

* chore: update PR description

* chore: fix how-to-test section for description check

---------

Co-authored-by: michaelxer <michaelxer@users.noreply.github.com>
This commit is contained in:
Michael
2026-06-24 04:28:53 +07:00
committed by GitHub
parent 2e16394b41
commit 72c0bde8a9
2 changed files with 96 additions and 2 deletions
+17 -2
View File
@@ -81,11 +81,26 @@ class APIKeyManager:
keys stay encrypted. Loading via load() first would decrypt them and
write them back as plaintext, which then fails to decrypt on the next
load() and silently drops those providers.
Uses atomic write (temp file + os.replace) so a crash, disk-full, or
mid-write error never truncates the existing keys file.
"""
keys = self._load_raw()
keys[provider] = self.encrypt_api_key(api_key)
with open(self.api_keys_file, 'w', encoding="utf-8") as f:
json.dump(keys, f)
tmp_file = self.api_keys_file + ".tmp"
try:
with open(tmp_file, 'w', encoding="utf-8") as f:
json.dump(keys, f)
f.flush()
os.fsync(f.fileno())
os.replace(tmp_file, self.api_keys_file)
except OSError:
# Clean up temp file on failure; re-raise so callers see the error
try:
os.remove(tmp_file)
except OSError:
pass
raise
def load(self) -> Dict[str, str]:
"""Load and decrypt API keys"""