diff --git a/crates/tui/src/audit.rs b/crates/tui/src/audit.rs index 2640b6af..b4571a8a 100644 --- a/crates/tui/src/audit.rs +++ b/crates/tui/src/audit.rs @@ -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(()) } diff --git a/crates/tui/src/mcp.rs b/crates/tui/src/mcp.rs index 024b604c..52c51346 100644 --- a/crates/tui/src/mcp.rs +++ b/crates/tui/src/mcp.rs @@ -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 { 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(()) } diff --git a/crates/tui/src/runtime_threads.rs b/crates/tui/src/runtime_threads.rs index efb3032f..6bc2f258 100644 --- a/crates/tui/src/runtime_threads.rs +++ b/crates/tui/src/runtime_threads.rs @@ -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(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)] diff --git a/crates/tui/src/session_manager.rs b/crates/tui/src/session_manager.rs index f30794ca..7da3dd47 100644 --- a/crates/tui/src/session_manager.rs +++ b/crates/tui/src/session_manager.rs @@ -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 { 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) } diff --git a/crates/tui/src/task_manager.rs b/crates/tui/src/task_manager.rs index 5f896918..f1aa8507 100644 --- a/crates/tui/src/task_manager.rs +++ b/crates/tui/src/task_manager.rs @@ -1611,25 +1611,8 @@ fn write_json_atomic(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 { diff --git a/crates/tui/src/utils.rs b/crates/tui/src/utils.rs index ebd45ea3..99da8018 100644 --- a/crates/tui/src/utils.rs +++ b/crates/tui/src/utils.rs @@ -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` wraps the append handle. Call +/// `.flush()` followed by `.get_ref().sync_all()` after each batch. +pub fn open_append(path: &Path) -> std::io::Result> { + 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::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}; diff --git a/crates/tui/src/workspace_trust.rs b/crates/tui/src/workspace_trust.rs index b055d21e..14a757de 100644 --- a/crates/tui/src/workspace_trust.rs +++ b/crates/tui/src/workspace_trust.rs @@ -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(()) }