fix(tools): reject unresolved parent traversal (C-03)

src/tools/file_edit.rs

@@ -1,4 +1,3 @@
-use std::fs;
 use super::registry::Tool;
 
 pub struct FileEditTool;
@@ -20,8 +19,8 @@ impl Tool for FileEditTool {
         let new_string = obj.get("new_string").and_then(|v| v.as_str()).unwrap_or("");
         let replace_all = obj.get("replace_all").and_then(|v| v.as_bool()).unwrap_or(false);
 
-        let resolved = super::workspace::resolve_in_workspace(file_path)?;
-        let content = fs::read_to_string(&resolved).map_err(|e| format!("read error: {e}"))?;
+        let secured = super::workspace::acquire(file_path, false, true)?;
+        let content = secured.previous.clone().expect("required existing file");
 
         // Exact match first; if that fails, retry tolerant of CRLF/LF
         // differences between the file and a model-generated old_string
@@ -82,15 +81,23 @@ impl Tool for FileEditTool {
             1
         };
 
-        super::file_history::record_snapshot(resolved.clone(), file_path, content);
-        fs::write(&resolved, &new_content).map_err(|e| format!("write error: {e}"))?;
+        super::workspace::atomic_replace(&secured, &new_content)?;
+        super::file_history::record_snapshot(secured.relative, file_path, Some(content));
         Ok(format!("Applied {count} edit(s) to {file_path}"))
     }
 }
 
 #[cfg(test)]
 mod tests {
     use super::*;
+    use std::fs;
+    use std::sync::atomic::{AtomicU64, Ordering};
+
+    static TEST_COUNTER: AtomicU64 = AtomicU64::new(0);
+
+    fn unique_path(label: &str) -> String {
+        format!("target/cairn_{label}_{}_{}", std::process::id(), TEST_COUNTER.fetch_add(1, Ordering::Relaxed))
+    }
 
     #[test]
     fn test_workspace_escape_is_rejected() {
@@ -100,27 +107,62 @@ mod tests {
         assert!(err.contains("outside the workspace"), "unexpected error: {err}");
     }
 
+    #[test]
+    fn test_workspace_escape_after_nonexistent_prefix_is_rejected() {
+        let prefix = unique_path("missing_edit_prefix");
+        let victim = format!(
+            "outside_cairn_edit_{}_{}.txt",
+            std::process::id(),
+            TEST_COUNTER.fetch_add(1, Ordering::Relaxed)
+        );
+        let outside = std::env::current_dir().unwrap().parent().unwrap().join(&victim);
+        assert!(!outside.exists(), "unique outside victim unexpectedly exists");
+        fs::write(&outside, "original").unwrap();
+        let tool = FileEditTool;
+        let input = format!(r#"{{"file_path":"{prefix}/../../../{victim}","old_string":"original","new_string":"changed"}}"#);
+        let err = tool.execute(&input).unwrap_err();
+        assert!(err.contains("outside the workspace"), "unexpected error: {err}");
+        assert_eq!(fs::read_to_string(&outside).unwrap(), "original");
+        assert!(!std::path::Path::new(&prefix).exists());
+        fs::remove_file(outside).unwrap();
+    }
+
     #[test]
     fn test_exact_match_replaces_content() {
-        let path = "target/cairn_file_edit_test_exact.txt";
-        fs::write(path, "hello world").unwrap();
+        let path = unique_path("file_edit_exact");
+        fs::write(&path, "hello world").unwrap();
         let tool = FileEditTool;
         let input = format!(r#"{{"file_path":"{path}","old_string":"world","new_string":"rust"}}"#);
         tool.execute(&input).unwrap();
-        assert_eq!(fs::read_to_string(path).unwrap(), "hello rust");
+        assert_eq!(fs::read_to_string(&path).unwrap(), "hello rust");
         let _ = fs::remove_file(path);
     }
 
     #[test]
     fn test_crlf_fallback_match_preserves_crlf() {
-        let path = "target/cairn_file_edit_test_crlf.txt";
-        fs::write(path, "line1\r\nline2\r\nline3").unwrap();
+        let path = unique_path("file_edit_crlf");
+        fs::write(&path, "line1\r\nline2\r\nline3").unwrap();
         let tool = FileEditTool;
         // old_string uses bare \n, the file uses \r\n: exact match fails, the
         // CRLF-tolerant fallback should still find and apply it.
         let input = format!(r#"{{"file_path":"{path}","old_string":"line1\nline2","new_string":"REPLACED"}}"#);
         tool.execute(&input).unwrap();
-        assert_eq!(fs::read_to_string(path).unwrap(), "REPLACED\r\nline3");
+        assert_eq!(fs::read_to_string(&path).unwrap(), "REPLACED\r\nline3");
         let _ = fs::remove_file(path);
     }
+
+    #[cfg(unix)]
+    #[test]
+    fn test_final_symlink_cannot_modify_outside_file() {
+        use std::os::unix::fs::symlink;
+        let link = unique_path("edit_link");
+        let outside = std::env::temp_dir().join(format!("cairn-outside-edit-{}-{}", std::process::id(), TEST_COUNTER.fetch_add(1, Ordering::Relaxed)));
+        fs::write(&outside, "original").unwrap();
+        symlink(&outside, &link).unwrap();
+        let input = format!(r#"{{"file_path":"{link}","old_string":"original","new_string":"changed"}}"#);
+        assert!(FileEditTool.execute(&input).is_err());
+        assert_eq!(fs::read_to_string(&outside).unwrap(), "original");
+        fs::remove_file(link).unwrap();
+        fs::remove_file(outside).unwrap();
+    }
 }

src/tools/file_history.rs

@@ -1,5 +1,4 @@
 use std::cell::RefCell;
-use std::fs;
 use std::path::PathBuf;
 
 /// In-process stack of file snapshots taken before mutating writes
@@ -31,28 +30,13 @@ fn push(entry: Entry) {
     });
 }
 
-/// Snapshot a path by reading it from disk (or recording that it is new).
-pub fn record_before_write(path: PathBuf, label: &str) -> Result<(), String> {
-    let previous = if path.exists() {
-        Some(fs::read_to_string(&path).map_err(|e| format!("read error before write: {e}"))?)
-    } else {
-        None
-    };
-    push(Entry {
-        path,
-        label: label.to_string(),
-        previous,
-    });
-    Ok(())
-}
-
 /// Snapshot known previous content (avoids a second disk read when the
 /// caller already loaded the file, e.g. `file_edit`).
-pub fn record_snapshot(path: PathBuf, label: &str, previous: String) {
+pub fn record_snapshot(path: PathBuf, label: &str, previous: Option<String>) {
     push(Entry {
         path,
         label: label.to_string(),
-        previous: Some(previous),
+        previous,
     });
 }
 
@@ -62,21 +46,16 @@ pub fn undo_last() -> Result<String, String> {
         "Nothing to undo. Only changes made with file_edit/file_write in this process can be undone.".to_string()
     })?;
 
-    match entry.previous {
-        Some(content) => {
-            if let Some(parent) = entry.path.parent() {
-                fs::create_dir_all(parent).map_err(|e| format!("undo mkdir failed: {e}"))?;
-            }
-            fs::write(&entry.path, content).map_err(|e| format!("undo write failed: {e}"))?;
-            Ok(format!("Restored previous contents of {}", entry.label))
-        }
-        None => {
-            if entry.path.exists() {
-                fs::remove_file(&entry.path).map_err(|e| format!("undo remove failed: {e}"))?;
-            }
-            Ok(format!("Removed newly created file {}", entry.label))
-        }
+    let result = match entry.previous.as_deref() {
+        Some(content) => super::workspace::restore(&entry.path, Some(content))
+            .map(|()| format!("Restored previous contents of {}", entry.label)),
+        None => super::workspace::restore(&entry.path, None)
+            .map(|()| format!("Removed newly created file {}", entry.label)),
+    };
+    if result.is_err() {
+        push(entry);
     }
+    result
 }
 
 #[cfg(test)]

src/tools/file_undo.rs

@@ -25,12 +25,14 @@ mod tests {
     use crate::tools::file_edit::FileEditTool;
     use crate::tools::file_write::FileWriteTool;
     use std::fs;
+    use std::sync::atomic::{AtomicU64, Ordering};
+
+    static TEST_COUNTER: AtomicU64 = AtomicU64::new(0);
 
     #[test]
     fn undo_restores_file_edit() {
         crate::tools::file_history::clear();
-        // Stay inside the workspace (cwd) so resolve_in_workspace accepts the path.
-        // Unique subdir avoids parallel test collisions; do not touch process cwd.
+        // A unique workspace-relative subdirectory avoids parallel collisions.
         let rel = format!(
             "target/cairn_undo_edit_{}/f.txt",
             std::time::SystemTime::now()
@@ -84,6 +86,55 @@ mod tests {
         }
     }
 
+    #[test]
+    fn undo_restores_preexisting_empty_file() {
+        crate::tools::file_history::clear();
+        let rel = format!("target/cairn_undo_empty_{}/f.txt", std::process::id());
+        fs::create_dir_all(std::path::Path::new(&rel).parent().unwrap()).unwrap();
+        fs::write(&rel, "").unwrap();
+        let input = format!(r#"{{"file_path":"{rel}","content":"changed"}}"#);
+        FileWriteTool.execute(&input).unwrap();
+        FileUndoTool.execute("{}").unwrap();
+        assert!(std::path::Path::new(&rel).is_file());
+        assert_eq!(fs::read_to_string(&rel).unwrap(), "");
+        fs::remove_dir_all(std::path::Path::new(&rel).parent().unwrap()).unwrap();
+    }
+
+    #[test]
+    fn failed_undo_cannot_follow_symlink_and_remains_retryable() {
+        crate::tools::file_history::clear();
+        let sequence = TEST_COUNTER.fetch_add(1, Ordering::Relaxed);
+        let rel = format!("target/cairn_undo_link_{}_{sequence}.txt", std::process::id());
+        let outside = std::env::temp_dir().join(format!(
+            "cairn-undo-outside-{}-{sequence}.txt",
+            std::process::id()
+        ));
+        assert!(!outside.exists(), "unique outside victim unexpectedly exists");
+        fs::write(&outside, "outside").unwrap();
+
+        let input = format!(r#"{{"file_path":"{rel}","content":"created"}}"#);
+        FileWriteTool.execute(&input).unwrap();
+        fs::remove_file(&rel).unwrap();
+        #[cfg(unix)]
+        std::os::unix::fs::symlink(&outside, &rel).unwrap();
+        #[cfg(windows)]
+        if let Err(e) = std::os::windows::fs::symlink_file(&outside, &rel) {
+            if e.kind() == std::io::ErrorKind::PermissionDenied {
+                fs::remove_file(outside).unwrap();
+                return;
+            }
+            panic!("symlink creation failed: {e}");
+        }
+
+        assert!(FileUndoTool.execute("{}").is_err());
+        assert_eq!(fs::read_to_string(&outside).unwrap(), "outside");
+
+        fs::remove_file(&rel).unwrap();
+        FileUndoTool.execute("{}").unwrap();
+        assert!(!std::path::Path::new(&rel).exists());
+        fs::remove_file(outside).unwrap();
+    }
+
     #[test]
     fn undo_empty_stack_errors_clearly() {
         crate::tools::file_history::clear();

src/tools/file_write.rs

@@ -1,4 +1,3 @@
-use std::fs;
 use super::registry::Tool;
 
 pub struct FileWriteTool;
@@ -18,21 +17,25 @@ impl Tool for FileWriteTool {
         let file_path = obj.get("file_path").and_then(|v| v.as_str()).ok_or("file_path required")?;
         let content = obj.get("content").and_then(|v| v.as_str()).unwrap_or("");
 
-        let resolved = super::workspace::resolve_in_workspace(file_path)?;
-
-        if let Some(parent) = resolved.parent() {
-            fs::create_dir_all(parent).map_err(|e| format!("mkdir: {e}"))?;
-        }
-
-        super::file_history::record_before_write(resolved.clone(), file_path)?;
-        fs::write(&resolved, content).map_err(|e| format!("write error: {e}"))?;
+        let secured = super::workspace::acquire(file_path, true, false)?;
+        let snapshot = secured.previous.clone();
+        super::workspace::atomic_replace(&secured, content)?;
+        super::file_history::record_snapshot(secured.relative, file_path, snapshot);
         Ok(format!("Written {} bytes to {}", content.len(), file_path))
     }
 }
 
 #[cfg(test)]
 mod tests {
     use super::*;
+    use std::fs;
+    use std::sync::atomic::{AtomicU64, Ordering};
+
+    static TEST_COUNTER: AtomicU64 = AtomicU64::new(0);
+
+    fn unique_path(label: &str) -> String {
+        format!("target/cairn_{label}_{}_{}", std::process::id(), TEST_COUNTER.fetch_add(1, Ordering::Relaxed))
+    }
 
     #[test]
     fn test_workspace_escape_is_rejected() {
@@ -42,13 +45,103 @@ mod tests {
         assert!(err.contains("outside the workspace"), "unexpected error: {err}");
     }
 
+    #[test]
+    fn test_workspace_escape_after_nonexistent_prefix_is_rejected() {
+        let prefix = unique_path("missing_write_prefix");
+        let victim = format!(
+            "outside_cairn_write_{}_{}.txt",
+            std::process::id(),
+            TEST_COUNTER.fetch_add(1, Ordering::Relaxed)
+        );
+        let outside = std::env::current_dir().unwrap().parent().unwrap().join(&victim);
+        assert!(!outside.exists(), "unique outside victim unexpectedly exists");
+        let tool = FileWriteTool;
+        let input = format!(r#"{{"file_path":"{prefix}/../../../{victim}","content":"x"}}"#);
+        let err = tool.execute(&input).unwrap_err();
+        assert!(err.contains("outside the workspace"), "unexpected error: {err}");
+        assert!(!outside.exists());
+        assert!(!std::path::Path::new(&prefix).exists());
+    }
+
     #[test]
     fn test_write_creates_file_with_content() {
-        let path = "target/cairn_file_write_test.txt";
+        let path = unique_path("file_write");
         let tool = FileWriteTool;
         let input = format!(r#"{{"file_path":"{path}","content":"hello"}}"#);
         tool.execute(&input).unwrap();
-        assert_eq!(fs::read_to_string(path).unwrap(), "hello");
+        assert_eq!(fs::read_to_string(&path).unwrap(), "hello");
         let _ = fs::remove_file(path);
     }
+
+    #[test]
+    fn test_write_creates_missing_nested_parents() {
+        let base = unique_path("nested_write");
+        let path = format!("{base}/one/two/file.txt");
+        let input = format!(r#"{{"file_path":"{path}","content":"nested"}}"#);
+        FileWriteTool.execute(&input).unwrap();
+        assert_eq!(fs::read_to_string(&path).unwrap(), "nested");
+        fs::remove_dir_all(base).unwrap();
+    }
+
+    #[test]
+    fn test_final_symlink_cannot_modify_outside_file() {
+        let link = unique_path("write_link");
+        let outside = std::env::temp_dir().join(format!("cairn-outside-write-{}-{}", std::process::id(), TEST_COUNTER.fetch_add(1, Ordering::Relaxed)));
+        fs::write(&outside, "original").unwrap();
+        #[cfg(unix)]
+        std::os::unix::fs::symlink(&outside, &link).unwrap();
+        #[cfg(windows)]
+        if let Err(e) = std::os::windows::fs::symlink_file(&outside, &link) {
+            if e.kind() == std::io::ErrorKind::PermissionDenied { fs::remove_file(outside).unwrap(); return; }
+            panic!("symlink creation failed: {e}");
+        }
+        let input = format!(r#"{{"file_path":"{link}","content":"changed"}}"#);
+        assert!(FileWriteTool.execute(&input).is_err());
+        assert_eq!(fs::read_to_string(&outside).unwrap(), "original");
+        fs::remove_file(link).unwrap();
+        fs::remove_file(outside).unwrap();
+    }
+
+    #[cfg(unix)]
+    #[test]
+    fn test_dangling_directory_symlink_cannot_escape() {
+        use std::os::unix::fs::symlink;
+        let prefix = unique_path("dangling_write");
+        let outside = std::env::temp_dir().join(format!("cairn-outside-{}-{}", std::process::id(), TEST_COUNTER.fetch_add(1, Ordering::Relaxed)));
+        symlink(&outside, &prefix).unwrap();
+        let input = format!(r#"{{"file_path":"{prefix}/created.txt","content":"x"}}"#);
+        assert!(FileWriteTool.execute(&input).is_err());
+        assert!(!outside.join("created.txt").exists());
+        assert!(!outside.exists());
+        fs::remove_file(prefix).unwrap();
+    }
+
+    #[test]
+    fn test_directory_symlink_cannot_escape() {
+        let link = unique_path("write_dir_link");
+        let outside = std::env::temp_dir().join(format!(
+            "cairn-outside-dir-{}-{}",
+            std::process::id(),
+            TEST_COUNTER.fetch_add(1, Ordering::Relaxed)
+        ));
+        fs::create_dir(&outside).unwrap();
+        #[cfg(unix)]
+        std::os::unix::fs::symlink(&outside, &link).unwrap();
+        #[cfg(windows)]
+        if let Err(e) = std::os::windows::fs::symlink_dir(&outside, &link) {
+            if e.kind() == std::io::ErrorKind::PermissionDenied {
+                fs::remove_dir(outside).unwrap();
+                return;
+            }
+            panic!("directory symlink creation failed: {e}");
+        }
+        let input = format!(r#"{{"file_path":"{link}/created.txt","content":"x"}}"#);
+        assert!(FileWriteTool.execute(&input).is_err());
+        assert!(!outside.join("created.txt").exists());
+        #[cfg(unix)]
+        fs::remove_file(link).unwrap();
+        #[cfg(windows)]
+        fs::remove_dir(link).unwrap();
+        fs::remove_dir(outside).unwrap();
+    }
 }