fix(tui): atomic file writes for ~/.deepseek/ persistence (#355)
Add write_atomic helper (NamedTempFile + fsync + rename) in utils.rs. Convert all non-append fs::write sites: - session_manager.rs: save_session/save_checkpoint/save_offline_queue_state - workspace_trust.rs: write_trust_file_at - task_manager.rs: write_json_atomic → delegates to write_atomic - runtime_threads.rs: write_json_atomic → delegates to write_atomic - mcp.rs: save_config/init_config/save_legacy - audit.rs: buffered append with flush_and_sync after each event - runtime_threads append_event: add sync_all after flush
This commit is contained in:
+12
-5
@@ -1,12 +1,13 @@
|
||||
//! Lightweight audit logging for sensitive operations.
|
||||
|
||||
use std::fs::{self, OpenOptions};
|
||||
use std::io::Write;
|
||||
use std::fs;
|
||||
use std::path::PathBuf;
|
||||
|
||||
use chrono::Utc;
|
||||
use serde_json::{Value, json};
|
||||
|
||||
use crate::utils::{open_append, flush_and_sync};
|
||||
|
||||
/// Append an audit event to `~/.deepseek/audit.log`.
|
||||
///
|
||||
/// This helper is best-effort by design: callers should not fail critical flows
|
||||
@@ -19,16 +20,22 @@ pub fn log_sensitive_event(event: &str, details: Value) {
|
||||
|
||||
fn append_event(event: &str, details: Value) -> anyhow::Result<()> {
|
||||
let path = default_audit_path()?;
|
||||
if let Some(parent) = path.parent() {
|
||||
let parent = path.parent().map(|p| p.to_path_buf());
|
||||
if let Some(ref parent) = parent {
|
||||
fs::create_dir_all(parent)?;
|
||||
}
|
||||
let mut file = OpenOptions::new().create(true).append(true).open(path)?;
|
||||
// Open for append with a BufWriter for buffered I/O, then flush + fsync
|
||||
// after each event so the record is durably on disk.
|
||||
let mut writer = open_append(&path)?;
|
||||
let record = json!({
|
||||
"ts": Utc::now().to_rfc3339(),
|
||||
"event": event,
|
||||
"details": details,
|
||||
});
|
||||
writeln!(file, "{}", serde_json::to_string(&record)?)?;
|
||||
let line = serde_json::to_string(&record)?;
|
||||
use std::io::Write;
|
||||
writeln!(writer, "{line}")?;
|
||||
flush_and_sync(&mut writer)?;
|
||||
Ok(())
|
||||
}
|
||||
|
||||
|
||||
@@ -21,6 +21,7 @@ use tokio::io::{AsyncBufReadExt, AsyncWriteExt};
|
||||
use tokio::process::{Child, ChildStdin, ChildStdout};
|
||||
|
||||
use crate::network_policy::{Decision, NetworkPolicyDecider, host_from_url};
|
||||
use crate::utils::write_atomic;
|
||||
|
||||
// === Error diagnostics helpers (#71) ===
|
||||
|
||||
@@ -1488,7 +1489,7 @@ pub fn save_config(path: &Path, cfg: &McpConfig) -> Result<()> {
|
||||
})?;
|
||||
}
|
||||
let rendered = serde_json::to_string_pretty(cfg).context("Failed to serialize MCP config")?;
|
||||
fs::write(path, rendered)
|
||||
write_atomic(path, rendered.as_bytes())
|
||||
.with_context(|| format!("Failed to write MCP config {}", path.display()))?;
|
||||
Ok(())
|
||||
}
|
||||
@@ -1529,7 +1530,8 @@ pub fn init_config(path: &Path, force: bool) -> Result<McpWriteStatus> {
|
||||
format!("Failed to create MCP config directory {}", parent.display())
|
||||
})?;
|
||||
}
|
||||
fs::write(path, mcp_template_json()?)
|
||||
let template = mcp_template_json()?;
|
||||
write_atomic(path, template.as_bytes())
|
||||
.with_context(|| format!("Failed to write MCP config {}", path.display()))?;
|
||||
Ok(status)
|
||||
}
|
||||
@@ -1981,7 +1983,7 @@ fn save_legacy(path: &Path, config: &LegacyMcpConfig) -> Result<()> {
|
||||
fs::create_dir_all(parent)?;
|
||||
}
|
||||
let contents = serde_json::to_string_pretty(config)?;
|
||||
fs::write(path, contents)?;
|
||||
write_atomic(path, contents.as_bytes())?;
|
||||
Ok(())
|
||||
}
|
||||
|
||||
|
||||
@@ -434,6 +434,8 @@ impl RuntimeThreadStore {
|
||||
writeln!(file, "{line}").with_context(|| format!("Failed to append {}", path.display()))?;
|
||||
file.flush()
|
||||
.with_context(|| format!("Failed to flush {}", path.display()))?;
|
||||
file.sync_all()
|
||||
.with_context(|| format!("Failed to fsync {}", path.display()))?;
|
||||
Ok(record)
|
||||
}
|
||||
|
||||
@@ -2638,25 +2640,8 @@ fn write_json_atomic<T: Serialize>(path: &Path, value: &T) -> Result<()> {
|
||||
.with_context(|| format!("Failed to create directory {}", parent.display()))?;
|
||||
}
|
||||
let payload = serde_json::to_string_pretty(value)?;
|
||||
let tmp_name = format!(
|
||||
".{}.tmp",
|
||||
path.file_name()
|
||||
.and_then(|s| s.to_str())
|
||||
.unwrap_or("runtime_state")
|
||||
);
|
||||
let tmp_path = path
|
||||
.parent()
|
||||
.unwrap_or_else(|| Path::new("."))
|
||||
.join(tmp_name);
|
||||
fs::write(&tmp_path, payload)
|
||||
.with_context(|| format!("Failed to write temp file {}", tmp_path.display()))?;
|
||||
fs::rename(&tmp_path, path).with_context(|| {
|
||||
format!(
|
||||
"Failed to rename {} -> {}",
|
||||
tmp_path.display(),
|
||||
path.display()
|
||||
)
|
||||
})
|
||||
crate::utils::write_atomic(path, payload.as_bytes())
|
||||
.with_context(|| format!("Failed to write {}", path.display()))
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
|
||||
@@ -8,6 +8,7 @@
|
||||
|
||||
use crate::models::{ContentBlock, Message, SystemPrompt};
|
||||
use crate::tui::file_mention::ContextReference;
|
||||
use crate::utils::write_atomic;
|
||||
use chrono::{DateTime, Utc};
|
||||
use serde::{Deserialize, Serialize};
|
||||
use std::fs;
|
||||
@@ -144,18 +145,15 @@ impl SessionManager {
|
||||
Self::new(default_sessions_dir()?)
|
||||
}
|
||||
|
||||
/// Save a session to disk using atomic write (temp file + rename).
|
||||
/// Save a session to disk using atomic write (temp file + fsync + rename).
|
||||
pub fn save_session(&self, session: &SavedSession) -> std::io::Result<PathBuf> {
|
||||
let path = self.validated_session_path(&session.metadata.id)?;
|
||||
|
||||
let content = serde_json::to_string_pretty(session)
|
||||
.map_err(|e| std::io::Error::new(std::io::ErrorKind::InvalidData, e))?;
|
||||
|
||||
// Atomic write: write to temp file then rename to avoid corruption
|
||||
let tmp_filename = format!(".{}.tmp", session.metadata.id.trim());
|
||||
let tmp_path = self.sessions_dir.join(&tmp_filename);
|
||||
fs::write(&tmp_path, &content)?;
|
||||
fs::rename(&tmp_path, &path)?;
|
||||
// Atomic write via write_atomic (NamedTempFile + fsync + persist)
|
||||
write_atomic(&path, content.as_bytes())?;
|
||||
|
||||
// Clean up old sessions if we have too many
|
||||
self.cleanup_old_sessions()?;
|
||||
@@ -170,9 +168,7 @@ impl SessionManager {
|
||||
let path = checkpoints.join("latest.json");
|
||||
let content = serde_json::to_string_pretty(session)
|
||||
.map_err(|e| std::io::Error::new(std::io::ErrorKind::InvalidData, e))?;
|
||||
let tmp_path = checkpoints.join(".latest.tmp");
|
||||
fs::write(&tmp_path, &content)?;
|
||||
fs::rename(&tmp_path, &path)?;
|
||||
write_atomic(&path, content.as_bytes())?;
|
||||
Ok(path)
|
||||
}
|
||||
|
||||
@@ -214,9 +210,7 @@ impl SessionManager {
|
||||
let path = checkpoints.join("offline_queue.json");
|
||||
let content = serde_json::to_string_pretty(state)
|
||||
.map_err(|e| std::io::Error::new(std::io::ErrorKind::InvalidData, e))?;
|
||||
let tmp_path = checkpoints.join(".offline_queue.tmp");
|
||||
fs::write(&tmp_path, &content)?;
|
||||
fs::rename(&tmp_path, &path)?;
|
||||
write_atomic(&path, content.as_bytes())?;
|
||||
Ok(path)
|
||||
}
|
||||
|
||||
|
||||
@@ -1611,25 +1611,8 @@ fn write_json_atomic<T: Serialize>(path: &Path, value: &T) -> Result<()> {
|
||||
.with_context(|| format!("Failed to create directory {}", parent.display()))?;
|
||||
}
|
||||
let payload = serde_json::to_string_pretty(value)?;
|
||||
let tmp_name = format!(
|
||||
".{}.tmp",
|
||||
path.file_name()
|
||||
.and_then(|s| s.to_str())
|
||||
.unwrap_or("task_state")
|
||||
);
|
||||
let tmp_path = path
|
||||
.parent()
|
||||
.unwrap_or_else(|| Path::new("."))
|
||||
.join(tmp_name);
|
||||
fs::write(&tmp_path, payload)
|
||||
.with_context(|| format!("Failed to write temp file {}", tmp_path.display()))?;
|
||||
fs::rename(&tmp_path, path).with_context(|| {
|
||||
format!(
|
||||
"Failed to rename {} -> {}",
|
||||
tmp_path.display(),
|
||||
path.display()
|
||||
)
|
||||
})
|
||||
crate::utils::write_atomic(path, payload.as_bytes())
|
||||
.with_context(|| format!("Failed to write {}", path.display()))
|
||||
}
|
||||
|
||||
fn default_auto_approve() -> bool {
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
//! Utility helpers shared across the `DeepSeek` CLI.
|
||||
|
||||
use std::fs;
|
||||
use std::io::Write;
|
||||
use std::path::{Path, PathBuf};
|
||||
|
||||
use crate::models::{ContentBlock, Message};
|
||||
@@ -149,6 +150,58 @@ pub fn project_tree(root: &Path, max_depth: usize) -> String {
|
||||
|
||||
// === Filesystem Helpers ===
|
||||
|
||||
/// Atomically write `contents` to `path` using a temporary file + fsync + rename.
|
||||
///
|
||||
/// 1. Creates a `NamedTempFile` in the same directory as `path` (same filesystem).
|
||||
/// 2. Writes `contents` to the temp file.
|
||||
/// 3. Calls `sync_all()` on the temp file for durability.
|
||||
/// 4. Atomically renames (persists) the temp file over `path`.
|
||||
///
|
||||
/// On filesystems that support it (`ext4`, `apfs`, `ntfs`), the rename is
|
||||
/// atomic — a concurrent reader sees either the old content or the new, never
|
||||
/// a partial write. `sync_all` ensures the data is on stable storage before
|
||||
/// the metadata change so an OS crash mid-rename doesn't lose data.
|
||||
///
|
||||
/// # Errors
|
||||
/// Returns `io::Error` if the parent directory cannot be determined, the temp
|
||||
/// file cannot be created, the write fails, or the rename fails.
|
||||
pub fn write_atomic(path: &Path, contents: &[u8]) -> std::io::Result<()> {
|
||||
let parent = path.parent().ok_or_else(|| {
|
||||
std::io::Error::new(
|
||||
std::io::ErrorKind::InvalidInput,
|
||||
format!("path has no parent directory: {}", path.display()),
|
||||
)
|
||||
})?;
|
||||
// Use parent directory so the rename is on the same filesystem.
|
||||
let mut tmp = tempfile::NamedTempFile::new_in(parent)?;
|
||||
std::io::Write::write_all(&mut tmp, contents)?;
|
||||
tmp.as_file().sync_all()?;
|
||||
tmp.persist(path)?;
|
||||
Ok(())
|
||||
}
|
||||
|
||||
/// Open or create a file for appending at `path`, optionally syncing after
|
||||
/// every write. Use this for append-only logs like `audit.log`.
|
||||
///
|
||||
/// The returned `BufWriter<fs::File>` wraps the append handle. Call
|
||||
/// `.flush()` followed by `.get_ref().sync_all()` after each batch.
|
||||
pub fn open_append(path: &Path) -> std::io::Result<std::io::BufWriter<std::fs::File>> {
|
||||
if let Some(parent) = path.parent() {
|
||||
std::fs::create_dir_all(parent)?;
|
||||
}
|
||||
let file = std::fs::OpenOptions::new()
|
||||
.create(true)
|
||||
.append(true)
|
||||
.open(path)?;
|
||||
Ok(std::io::BufWriter::new(file))
|
||||
}
|
||||
|
||||
/// Flush a `BufWriter` wrapping a `File`, then `fsync` the underlying file.
|
||||
pub fn flush_and_sync(writer: &mut std::io::BufWriter<std::fs::File>) -> std::io::Result<()> {
|
||||
writer.flush()?;
|
||||
writer.get_ref().sync_all()
|
||||
}
|
||||
|
||||
#[allow(dead_code)]
|
||||
pub fn ensure_dir(path: &Path) -> Result<()> {
|
||||
fs::create_dir_all(path)
|
||||
@@ -325,6 +378,74 @@ mod tests {
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod atomic_write_tests {
|
||||
use super::*;
|
||||
use std::fs;
|
||||
use tempfile::tempdir;
|
||||
|
||||
#[test]
|
||||
fn write_atomic_writes_content() {
|
||||
let tmp = tempdir().expect("tempdir");
|
||||
let path = tmp.path().join("test.json");
|
||||
let content = b"hello atomic world";
|
||||
|
||||
write_atomic(&path, content).expect("write_atomic");
|
||||
assert!(path.exists());
|
||||
let read = fs::read_to_string(&path).expect("read");
|
||||
assert_eq!(read.as_bytes(), content);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn write_atomic_replaces_existing_file() {
|
||||
let tmp = tempdir().expect("tempdir");
|
||||
let path = tmp.path().join("existing.json");
|
||||
fs::write(&path, b"old content").expect("write old");
|
||||
write_atomic(&path, b"new content").expect("write_atomic");
|
||||
let read = fs::read_to_string(&path).expect("read");
|
||||
assert_eq!(read, "new content");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn write_atomic_no_temp_left_behind_on_success() {
|
||||
let tmp = tempdir().expect("tempdir");
|
||||
let path = tmp.path().join("clean.json");
|
||||
write_atomic(&path, b"clean").expect("write_atomic");
|
||||
// List files in dir — there should be no .tmp files left
|
||||
let entries: Vec<_> = fs::read_dir(tmp.path())
|
||||
.expect("read_dir")
|
||||
.filter_map(|e| e.ok())
|
||||
.collect();
|
||||
let tmp_files: Vec<_> = entries
|
||||
.iter()
|
||||
.filter(|e| {
|
||||
e.file_name()
|
||||
.to_str()
|
||||
.is_some_and(|n| n.starts_with('.'))
|
||||
})
|
||||
.collect();
|
||||
assert!(
|
||||
tmp_files.is_empty(),
|
||||
"temp files left behind: {tmp_files:?}"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn flush_and_sync_writes_and_syncs() {
|
||||
let tmp = tempdir().expect("tempdir");
|
||||
let path = tmp.path().join("append.log");
|
||||
{
|
||||
let mut writer = open_append(&path).expect("open_append");
|
||||
writeln!(writer, "line 1").expect("write");
|
||||
flush_and_sync(&mut writer).expect("flush_and_sync");
|
||||
writeln!(writer, "line 2").expect("write");
|
||||
flush_and_sync(&mut writer).expect("flush_and_sync");
|
||||
}
|
||||
let content = fs::read_to_string(&path).expect("read");
|
||||
assert_eq!(content, "line 1\nline 2\n");
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod project_mapping_tests {
|
||||
use super::{project_tree, summarize_project};
|
||||
|
||||
@@ -19,6 +19,8 @@ use std::path::{Path, PathBuf};
|
||||
use anyhow::{Context, Result};
|
||||
use serde::{Deserialize, Serialize};
|
||||
|
||||
use crate::utils::write_atomic;
|
||||
|
||||
const TRUST_FILE_NAME: &str = "workspace-trust.json";
|
||||
|
||||
#[derive(Debug, Default, Clone, Serialize, Deserialize)]
|
||||
@@ -173,7 +175,8 @@ fn write_trust_file_at(file: &TrustFile, path: &Path) -> Result<()> {
|
||||
.with_context(|| format!("create dir {}", parent.display()))?;
|
||||
}
|
||||
let json = serde_json::to_string_pretty(file).context("serialize trust file")?;
|
||||
std::fs::write(path, json).with_context(|| format!("write {}", path.display()))?;
|
||||
write_atomic(path, json.as_bytes())
|
||||
.with_context(|| format!("write {}", path.display()))?;
|
||||
Ok(())
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user