Skip to content

Harden account and auth persistence with atomic writes#14

Open
thatdaveguy1 wants to merge 1 commit intoLampese:mainfrom
thatdaveguy1:codex/atomic-storage-writes
Open

Harden account and auth persistence with atomic writes#14
thatdaveguy1 wants to merge 1 commit intoLampese:mainfrom
thatdaveguy1:codex/atomic-storage-writes

Conversation

@thatdaveguy1
Copy link
Copy Markdown
Contributor

Why

The account database and active auth file are written directly today. If the app crashes mid-write or multiple operations overlap, that can leave accounts.json or auth.json in a partially written state.

This PR hardens those persistence paths with atomic writes, simple locking, and backup files so storage updates are more crash-resistant.

What changed

  • add shared filesystem helpers in fs_utils.rs for locked atomic writes
  • update storage.rs to use the new write path for account persistence
  • update switcher.rs to write active auth data atomically and emit .bak backups
  • ensure lockfile parent directories are created for first-run paths

Reviewer notes

  • this PR is intentionally limited to persistence safety
  • backup security mode selection and process restart behavior are handled in separate PRs

Verification

  • cargo check in src-tauri passes on this branch

@Lampese
Copy link
Copy Markdown
Owner

Lampese commented Mar 7, 2026

plz resolve the conflicts

Copy link
Copy Markdown
Owner

@Lampese Lampese left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Requesting changes for two behavioral issues:

  1. The new .lock file approach can deadlock future writes after a crash. FileLock creates a sibling lock file with create_new(true) and only removes it in Drop. If the process is killed or crashes after acquiring the lock, the stale .lock file remains behind permanently, and every later write path will just time out waiting for it. That means the crash-hardening path introduces a new failure mode exactly in the scenario this PR is trying to improve.

  2. The new .bak files are written, but none of the read paths actually use them for recovery. accounts.json.bak and auth.json.bak are created, but loading still reads only the primary file and fails immediately if it is corrupted or partially unreadable. So the backup mechanism currently increases file churn without providing an actual recovery path.

I think both issues should be addressed before merging, otherwise this change improves write atomicity but still leaves crash recovery incomplete and can also introduce stale-lock write failures.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants