feat: enhance safety and crash resilience in file operations; add fsync to critical writes and checks for path traversal

src/app/harness.rs

@@ -18,23 +18,31 @@ pub struct Harness;
 impl Harness {
     /// Decide whether a tool call is allowed to execute.
     ///
-    /// Flow: if the tool isn't flagged risky, allow immediately → otherwise
+    /// Flow: if the tool isn't flagged risky, allow immediately → basic
-    /// defer to `classify`.
+    /// content checks (path traversal) → defer to `classify`.
     ///
-    /// Why: `_args` and `_workspace_roots` are accepted for a future
+    /// Why: `classify` is currently a stub that always allows; the basic
-    /// content-aware classifier but currently unused — `classify` is a
+    /// checks here serve as defense-in-depth alongside the shell filters
-    /// stub that always allows.
+    /// and `resolve_path` in the tool modules.
     ///
     /// Return: `Verdict::Allow` or `Verdict::Block(reason)`.
     pub fn gate_tool_call(
         tool_name: &str,
-        _args: &serde_json::Value,
+        args: &serde_json::Value,
         _workspace_roots: &[&std::path::Path],
     ) -> Verdict {
 
         if !crate::tool::tool_is_risky(tool_name) {
             return Verdict::Allow;
         }
+        // Basic path traversal check for file-mutating tools.
+        if matches!(tool_name, "write" | "edit" | "delete") {
+            if let Some(path) = args.get("path").and_then(|v| v.as_str()) {
+                if path.contains("..") {
+                    return Verdict::Block("path traversal detected in 'path' argument".to_string());
+                }
+            }
+        }
         Self::classify(tool_name)
     }
 

src/app/subagent/engine.rs

@@ -101,9 +101,13 @@ pub fn run_subagent(ctx: SubagentContext, tx: mpsc::Sender<SubagentEvent>) -> an
     let (tools, tdefs) = build_subagent_tools(&ctx.allowed_tools);
     let tdefs_opt: Option<Vec<ToolDef>> = if tdefs.is_empty() { None } else { Some(tdefs) };
 
+    // Cache provider config once before the loop instead of re-resolving
+    // from disk on every step (Settings::load + AppConfig::load each parse
+    // JSON files, and the config cannot change between steps).
+    let (api_key, model, base_url) = resolve_provider_config();
+    let client = crate::service::provider::LlmClient::new(api_key, model, base_url);
+
     for step in 0..ctx.max_steps {
-        let (api_key, model, base_url) = resolve_provider_config();
-        let client = crate::service::provider::LlmClient::new(api_key, model, base_url);
 
         // Use the structured tool-calling API so the LLM can request tools with
         // proper arguments, exactly like the main agent does.

src/model/agent_def/global.rs

@@ -36,13 +36,16 @@ pub fn load_global_agents() -> Vec<AgentDefinition> {
     agents
 }
 
-/// Persist a global agent definition as `<store>/agents/<name>.json`.
+/// Persist a global agent definition as `<store>/agents/<name>.json`,
+/// with fsync for crash safety.
 ///
 /// Flow: ensure the `agents/` directory exists → serialize `def` to
-/// pretty JSON → write to a file named after `def.name`.
+/// pretty JSON → write to a temp file → fsync → rename into place →
+/// fsync parent directory.
 ///
 /// Why: writing by name overwrites any existing definition with the
-/// same name, acting as an upsert.
+/// same name, acting as an upsert; fsync prevents a torn write from
+/// losing the definition on crash.
 ///
 /// Return: `Ok(())` on success, or an error if directory creation,
 /// serialization, or the write fails.
@@ -51,8 +54,13 @@ pub fn save_global_agent(def: &AgentDefinition) -> anyhow::Result<()> {
     let agents_dir = store.base_dir.join("agents");
     std::fs::create_dir_all(&agents_dir)?;
     let path = agents_dir.join(format!("{}.json", def.name));
+    let tmp = agents_dir.join(format!("{}.json.tmp", def.name));
     let content = serde_json::to_string_pretty(def)?;
-    std::fs::write(path, content)?;
+    std::fs::write(&tmp, content)?;
+    let f = std::fs::File::open(&tmp)?;
+    f.sync_all()?;
+    std::fs::rename(&tmp, path)?;
+    let _ = std::fs::File::open(&agents_dir).and_then(|d| d.sync_all());
     Ok(())
 }
 

src/model/agent_def/session.rs

@@ -31,17 +31,23 @@ pub fn load_session_agents(session_dir: &Path) -> Vec<AgentDefinition> {
     }
 }
 
-/// Overwrite `<session_dir>/agents.json` with the given agent list.
+/// Overwrite `<session_dir>/agents.json` with the given agent list,
+/// with fsync for crash safety.
 ///
-/// Flow: serialize `agents` to pretty JSON → write to
+/// Flow: serialize `agents` to pretty JSON → write to a temp file →
-/// `<session_dir>/agents.json`.
+/// fsync → rename over `agents.json` → fsync parent directory.
 ///
 /// Return: `Ok(())` on success, or an error if serialization or the
 /// write fails.
 pub fn save_session_agents(session_dir: &Path, agents: &[AgentDefinition]) -> anyhow::Result<()> {
     let agents_file = session_dir.join("agents.json");
+    let tmp = session_dir.join("agents.json.tmp");
     let content = serde_json::to_string_pretty(agents)?;
-    std::fs::write(agents_file, content)?;
+    std::fs::write(&tmp, content)?;
+    let f = std::fs::File::open(&tmp)?;
+    f.sync_all()?;
+    std::fs::rename(&tmp, agents_file)?;
+    let _ = std::fs::File::open(session_dir).and_then(|d| d.sync_all());
     Ok(())
 }
 

src/model/editlog.rs

@@ -48,14 +48,16 @@ impl EditLog {
             .collect()
     }
 
-    /// Append one entry to `edits.jsonl` on disk and to the in-memory log.
+    /// Append one entry to `edits.jsonl` on disk and to the in-memory log,
+    /// with fsync for crash safety.
     ///
     /// Flow: serialize `entry` to a JSON line → ensure parent dir exists →
-    /// open the file in append mode → write the line → push into
+    /// open the file in append mode → write the line → fsync → push into
     /// `self.entries`.
     ///
     /// Why: appending (not rewriting) keeps the log durable and cheap even
-    /// as it grows across a long session.
+    /// as it grows across a long session; fsync ensures the entry survives
+    /// a crash rather than lingering in the page cache.
     ///
     /// Return: `Ok(())` on success; an `io::Error` if serialization or
     /// any filesystem operation fails.
@@ -69,6 +71,7 @@ impl EditLog {
             .open(&self.path)?;
         use std::io::Write;
         file.write_all(line.as_bytes())?;
+        file.sync_all()?;
         self.entries.push(entry);
         Ok(())
     }

src/model/memory.rs

@@ -219,7 +219,15 @@ pub fn export_lessons(memory_dir: &Path, output: &Path) -> std::io::Result<()> {
         .collect();
     let data = serde_json::to_string_pretty(&lessons)
         .map_err(std::io::Error::other)?;
-    std::fs::write(output, data)?;
+    // Write to temp, fsync, then rename for crash-safe export
+    let tmp = output.with_extension("json.tmp");
+    std::fs::write(&tmp, data)?;
+    let f = std::fs::File::open(&tmp)?;
+    f.sync_all()?;
+    std::fs::rename(&tmp, output)?;
+    if let Some(parent) = output.parent() {
+        let _ = std::fs::File::open(parent).and_then(|d| d.sync_all());
+    }
     Ok(())
 }
 /// Import memories from a JSON export file into `memory_dir`, skipping

src/model/session.rs

@@ -50,13 +50,16 @@ impl Session {
         self.session_dir(base_dir).join("conversation.json")
     }
 
-    /// Persist this session's metadata to `session.json`, atomically.
+    /// Persist this session's metadata to `session.json`, atomically
+    /// with fsync for crash safety.
     ///
     /// Flow: ensure the session directory exists → serialize to pretty
-    /// JSON → write to `session.json.tmp` → rename over `session.json`.
+    /// JSON → write to `session.json.tmp` → fsync → rename over
+    /// `session.json` → fsync parent directory.
     ///
     /// Why: write-then-rename avoids a torn/partial `session.json` if
-    /// interrupted mid-write.
+    /// interrupted mid-write; fsync before rename ensures the data is
+    /// on disk before the rename makes it visible.
     ///
     /// Return: `Ok(())` on success, or an `io::Error` from any step.
     pub fn save(&self, base_dir: &Path) -> std::io::Result<()> {
@@ -66,7 +69,10 @@ impl Session {
         let data = serde_json::to_string_pretty(self)?;
         let tmp = dir.join("session.json.tmp");
         std::fs::write(&tmp, data)?;
+        let f = std::fs::File::open(&tmp)?;
+        f.sync_all()?;
         std::fs::rename(&tmp, path)?;
+        let _ = std::fs::File::open(&dir).and_then(|d| d.sync_all());
         Ok(())
     }
 

src/model/settings.rs

@@ -78,17 +78,24 @@ impl Settings {
             .unwrap_or_default()
     }
 
-    /// Serialize and write settings to `<store_base_dir>/settings.json`.
+    /// Serialize and write settings to `<store_base_dir>/settings.json`,
+    /// using write-then-rename with fsync for crash safety.
     ///
-    /// Flow: ensure base dir exists → pretty-print JSON → write to disk.
+    /// Flow: ensure base dir exists → pretty-print JSON → write to a temp
+    /// file → sync to disk → rename over the real path → sync the directory.
     ///
     /// Return: `Err` if the directory can't be created or the write fails.
     pub fn save(&self) -> std::io::Result<()> {
         let store = super::store::Store::new();
         std::fs::create_dir_all(&store.base_dir)?;
         let path = store.base_dir.join("settings.json");
+        let tmp = store.base_dir.join("settings.json.tmp");
         let s = serde_json::to_string_pretty(self)?;
-        std::fs::write(path, s)?;
+        std::fs::write(&tmp, s)?;
+        let f = std::fs::File::open(&tmp)?;
+        f.sync_all()?;
+        std::fs::rename(&tmp, path)?;
+        let _ = std::fs::File::open(&store.base_dir).and_then(|d| d.sync_all());
         Ok(())
     }
 }

src/tool/git_worktree.rs

@@ -47,6 +47,9 @@ impl Tool for GitWorktree {
             .and_then(|v| v.as_str())
             .ok_or_else(|| anyhow!("missing required argument: name"))?
             .to_string();
+        if name.contains('/') || name.contains('\\') || name.contains("..") {
+            anyhow::bail!("worktree name must not contain path separators or '..'");
+        }
         let base_ref = args.get("base_ref")
             .and_then(|v| v.as_str())
             .ok_or_else(|| anyhow!("missing required argument: base_ref"))?