bug: verification_history.json writes are non-atomic, risking historical data loss on crash #213

Closed
opened 2026-09-06 12:35:25 +01:00 by gronod · 1 comment
Owner

Summary

quality_store.rs::write_history_file writes verification_history.json directly using std::fs::write(path, json). Without atomic write semantics (temporary file write followed by atomic rename), an unexpected termination, power disruption, or I/O failure can leave the file truncated or corrupted. Because load_history_file safely returns an empty vector upon JSON deserialization failure, any file corruption permanently wipes all longitudinal verification runs.

Details & Architecture Discrepancy

In src-tauri/src/quality_store.rs (lines 133–136):

let json = serde_json::to_string_pretty(&store)
    .map_err(|e| format!("Failed to serialize verification history: {}", e))?;
fs::write(path, json).map_err(|e| format!("Failed to write verification history: {}", e))?;

Contrast with AGENTS.md (line 172), which specifies:

Atomic file writes (.tmp write followed by rename) prevent data corruption.

Furthermore, load_history_file (lines 105–108):

match serde_json::from_str::<VerificationHistoryStore>(&content) {
    Ok(store) => store.records,
    Err(_) => Vec::new(),
}

If the file is partially written or corrupted, the entire store is silently treated as empty on the subsequent load and overwritten on the next verification run, permanently destroying the user's longitudinal verification history.

Proposed Remediation

  1. Update write_history_file in src-tauri/src/quality_store.rs to write the JSON data to a temporary file (<path>.tmp) within the same directory, sync/flush, and then atomically replace the target file via std::fs::rename.
  2. Add a unit test verifying atomic replacement and data persistence.
### Summary `quality_store.rs::write_history_file` writes `verification_history.json` directly using `std::fs::write(path, json)`. Without atomic write semantics (temporary file write followed by atomic rename), an unexpected termination, power disruption, or I/O failure can leave the file truncated or corrupted. Because `load_history_file` safely returns an empty vector upon JSON deserialization failure, any file corruption permanently wipes all longitudinal verification runs. ### Details & Architecture Discrepancy In `src-tauri/src/quality_store.rs` (lines 133–136): ```rust let json = serde_json::to_string_pretty(&store) .map_err(|e| format!("Failed to serialize verification history: {}", e))?; fs::write(path, json).map_err(|e| format!("Failed to write verification history: {}", e))?; ``` Contrast with `AGENTS.md` (line 172), which specifies: > Atomic file writes (`.tmp` write followed by `rename`) prevent data corruption. Furthermore, `load_history_file` (lines 105–108): ```rust match serde_json::from_str::<VerificationHistoryStore>(&content) { Ok(store) => store.records, Err(_) => Vec::new(), } ``` If the file is partially written or corrupted, the entire store is silently treated as empty on the subsequent load and overwritten on the next verification run, permanently destroying the user's longitudinal verification history. ### Proposed Remediation 1. Update `write_history_file` in `src-tauri/src/quality_store.rs` to write the JSON data to a temporary file (`<path>.tmp`) within the same directory, sync/flush, and then atomically replace the target file via `std::fs::rename`. 2. Add a unit test verifying atomic replacement and data persistence.
gronod added the Kind/Bug
Priority
Medium
3
labels 2026-09-06 12:35:25 +01:00
Author
Owner

Resolved in fix/213-atomic-history-writes (commit 5f5bf76) and merged into development (commit 54792a8).

  • Hardened quality_store.rs::write_history_file to write to <path>.tmp, sync/flush to disk, and atomically replace the destination file via std::fs::rename.
  • Added explicit log::warn! and log::error! logging in load_history_file.
  • Added unit tests verifying atomic write and cleanup of temporary file (test_atomic_write_preserves_data_and_cleans_tmp).
Resolved in `fix/213-atomic-history-writes` (commit `5f5bf76`) and merged into `development` (commit `54792a8`). - Hardened `quality_store.rs::write_history_file` to write to `<path>.tmp`, sync/flush to disk, and atomically replace the destination file via `std::fs::rename`. - Added explicit `log::warn!` and `log::error!` logging in `load_history_file`. - Added unit tests verifying atomic write and cleanup of temporary file (`test_atomic_write_preserves_data_and_cleans_tmp`).
Sign in to join this conversation.