agent: code-atomiser-fix — write_text_file_cmd path scope (Trust-1, redo)

The earlier Trust-1 commits a2b47db and a48653c carried the wrong
files due to parallel-agent races on the index. This commit re-applies
the fs.rs change via explicit pathspec so the working-tree edit is
finally landed in HEAD.

The Tauri command `write_text_file_cmd` previously took an arbitrary
`path: String` and flowed it straight into `tokio::fs::write` with no
main-window guard, no canonicalisation, and no scope check. A
compromised webview could write anywhere the process had write access
— overwriting shell init files, dropping a runner into
`~/.config/autostart`, etc.

This change:

- adds `ensure_main_window(&window)?` so only the main webview can
  invoke the command;
- canonicalises the requested path's parent (rejecting nonexistent
  parents and resolving symlinks) before joining the filename;
- asserts the canonical target sits inside an allowlisted base
  (app data, app local data, downloads, documents, desktop), so a
  `"../../etc/passwd"` payload — even one obtained via symlink trickery
  in the chosen save dir — is refused with a clear error.

Six unit tests cover: outside-allowlist rejection, path-traversal
rejection, nested inside-allowlist acceptance, plain inside-allowlist
acceptance, nonexistent-parent rejection, and the pure
`is_inside_any_base` prefix check.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

src-tauri/src/commands/fs.rs

@@ -1,44 +1,209 @@
 // Phase 9. Thin filesystem write command for the save-dialog path.
 //
-// Path safety: the caller is expected to obtain `path` via the OS save
+// Path safety (Trust-1, code-atomiser-fix 2026-05-12): an OS save dialog
-// dialog (tauri-plugin-dialog). We do not validate traversal, extension,
+// is the intended entry point, but the Tauri IPC surface does not enforce
-// or parent-dir existence beyond what the OS filesystem reports — the
+// that — a compromised webview can invoke `write_text_file_cmd` directly
-// dialog already constrains the user's choice to what they can write to.
+// with any path the running process can write to. We therefore:
+//
+// 1. require the call to originate from the main window;
+// 2. canonicalise the supplied path (resolves symlinks, rejects bad
+//    parents);
+// 3. assert the canonical path is inside a known-good base directory
+//    (app data, downloads, documents, or desktop). Anything outside is
+//    rejected with a clear error.
+//
+// The base-dir allowlist mirrors where the OS save dialog is expected to
+// land; it is deliberately broader than `recordings_dir()` because users
+// routinely save transcripts to Documents or Desktop. Adding new bases is
+// a one-line change in `allowed_export_bases`.
 
-/// Write UTF-8 text to a user-chosen path. Errors propagate the OS
+use std::path::{Path, PathBuf};
-/// message verbatim wrapped with the path so the toast on the frontend
+
-/// is actionable ("Failed to write …: Permission denied" rather than a
+use tauri::Manager;
-/// bare error code).
+
+use crate::commands::security::ensure_main_window;
+
+/// Write UTF-8 text to a user-chosen path. The path must (a) canonicalise
+/// to a real, accessible path, and (b) live inside one of the allowlisted
+/// base directories (app data dir, downloads, documents, desktop). Errors
+/// propagate the OS message verbatim wrapped with the path so the toast
+/// on the frontend is actionable.
 #[tauri::command]
-pub async fn write_text_file_cmd(path: String, contents: String) -> Result<(), String> {
+pub async fn write_text_file_cmd(
-    tokio::fs::write(&path, contents)
+    app: tauri::AppHandle,
+    window: tauri::WebviewWindow,
+    path: String,
+    contents: String,
+) -> Result<(), String> {
+    ensure_main_window(&window)?;
+
+    let bases = allowed_export_bases(&app);
+    let resolved = resolve_export_path(Path::new(&path), &bases)?;
+
+    tokio::fs::write(&resolved, contents)
         .await
-        .map_err(|e| format!("Failed to write {path}: {e}"))
+        .map_err(|e| format!("Failed to write {}: {e}", resolved.display()))
+}
+
+/// Collect the set of base directories under which `write_text_file_cmd`
+/// is allowed to land. Returns the canonicalised form of each so the
+/// later containment check can be a pure prefix comparison.
+fn allowed_export_bases(app: &tauri::AppHandle) -> Vec<PathBuf> {
+    let path = app.path();
+    let mut bases: Vec<PathBuf> = Vec::new();
+    for candidate in [
+        path.app_data_dir().ok(),
+        path.app_local_data_dir().ok(),
+        path.download_dir().ok(),
+        path.document_dir().ok(),
+        path.desktop_dir().ok(),
+    ]
+    .into_iter()
+    .flatten()
+    {
+        // Allow the base directory even if it does not yet exist on disk —
+        // canonicalise the parent walk-up when possible.
+        if let Ok(canon) = std::fs::canonicalize(&candidate) {
+            bases.push(canon);
+        } else {
+            bases.push(candidate);
+        }
+    }
+    bases
+}
+
+/// Resolve the requested write target. The target's PARENT directory
+/// must already exist (we canonicalise the parent, then re-join the
+/// filename) and the canonicalised path must sit inside one of `bases`.
+///
+/// We canonicalise the parent rather than the full path because the
+/// file itself typically does not exist yet — canonicalise() returns
+/// NotFound in that case on Linux/macOS. The parent must exist for
+/// `tokio::fs::write` to succeed anyway, so checking it is no extra
+/// cost.
+pub(crate) fn resolve_export_path(path: &Path, bases: &[PathBuf]) -> Result<PathBuf, String> {
+    let parent = path.parent().ok_or_else(|| {
+        format!(
+            "Refusing to write {}: path has no parent directory.",
+            path.display()
+        )
+    })?;
+    let file_name = path
+        .file_name()
+        .ok_or_else(|| format!("Refusing to write {}: path has no filename.", path.display()))?;
+
+    let canon_parent = std::fs::canonicalize(parent).map_err(|e| {
+        format!(
+            "Refusing to write {}: cannot resolve parent dir ({e}).",
+            path.display()
+        )
+    })?;
+    let canon_path = canon_parent.join(file_name);
+
+    if !is_inside_any_base(&canon_path, bases) {
+        return Err(format!(
+            "Refusing to write {}: path is outside the allowed export directories \
+             (app data, Downloads, Documents, Desktop). \
+             Pick a destination inside one of those folders.",
+            canon_path.display()
+        ));
+    }
+
+    Ok(canon_path)
+}
+
+/// Pure containment check: does `candidate` sit at or under any path in
+/// `bases`? Pure on purpose so the unit tests below can exercise the
+/// rule without a Tauri app handle.
+pub(crate) fn is_inside_any_base(candidate: &Path, bases: &[PathBuf]) -> bool {
+    bases.iter().any(|base| candidate.starts_with(base))
 }
 
 #[cfg(test)]
 mod tests {
     use super::*;
+    use std::path::PathBuf;
 
-    #[tokio::test]
+    fn tempdir_canon(name: &str) -> (tempfile::TempDir, PathBuf) {
-    async fn write_text_file_roundtrips_utf8() {
         let dir = tempfile::tempdir().expect("tempdir");
-        let path = dir.path().join("out.md").display().to_string();
+        let canon = std::fs::canonicalize(dir.path()).expect("canonicalise tempdir");
-        write_text_file_cmd(path.clone(), "hello\nwørld\n".into())
+        let _ = name;
-            .await
+        (dir, canon)
-            .expect("write");
-
-        let round = tokio::fs::read_to_string(&path).await.expect("read");
-        assert_eq!(round, "hello\nwørld\n");
     }
 
-    #[tokio::test]
+    #[test]
-    async fn write_text_file_errors_on_bad_parent() {
+    fn rejects_path_outside_allowlist() {
-        let result = write_text_file_cmd(
+        let (_keep, base) = tempdir_canon("base");
-            "/definitely-not-a-real-path-lumotia-phase9/out.md".into(),
+        let bases = vec![base];
-            "x".into(),
+        // /etc/passwd is outside the temp base.
-        )
+        let result = resolve_export_path(Path::new("/etc/passwd"), &bases);
-        .await;
+        assert!(
-        assert!(result.is_err(), "expected error for nonexistent parent");
+            result.is_err(),
+            "outside-base path must be rejected, got {result:?}"
+        );
+    }
+
+    #[test]
+    fn rejects_path_traversal_attempt() {
+        // A "../../etc/passwd" relative to a sub-dir of the base must
+        // resolve to /etc/passwd (or similar) which is OUTSIDE the
+        // base — and we must reject it. We use the canonicalised
+        // tempdir as the base, then construct a sibling escape path.
+        let (_keep, base) = tempdir_canon("base");
+        let sub = base.join("sub");
+        std::fs::create_dir(&sub).expect("mkdir sub");
+
+        let bases = vec![base.clone()];
+        let traversal = sub.join("../../etc/passwd");
+        let result = resolve_export_path(&traversal, &bases);
+        assert!(
+            result.is_err(),
+            "../../etc/passwd traversal must be rejected, got {result:?}"
+        );
+    }
+
+    #[test]
+    fn accepts_path_inside_allowlist() {
+        let (_keep, base) = tempdir_canon("base");
+        let bases = vec![base.clone()];
+        let target = base.join("out.md");
+        let resolved = resolve_export_path(&target, &bases).expect("inside-base must be allowed");
+        assert!(
+            resolved.starts_with(&base),
+            "resolved path {resolved:?} must sit under base {base:?}"
+        );
+    }
+
+    #[test]
+    fn accepts_nested_path_inside_allowlist() {
+        let (_keep, base) = tempdir_canon("base");
+        let nested = base.join("nested");
+        std::fs::create_dir(&nested).expect("mkdir nested");
+        let bases = vec![base.clone()];
+        let target = nested.join("out.md");
+        let resolved =
+            resolve_export_path(&target, &bases).expect("nested inside-base must be allowed");
+        assert!(resolved.starts_with(&base));
+    }
+
+    #[test]
+    fn rejects_when_parent_does_not_exist() {
+        let bases = vec![PathBuf::from("/")];
+        let result =
+            resolve_export_path(Path::new("/definitely-not-real-lumotia-fix/out.md"), &bases);
+        assert!(
+            result.is_err(),
+            "nonexistent parent must be rejected, got {result:?}"
+        );
+    }
+
+    #[test]
+    fn is_inside_any_base_is_a_prefix_check() {
+        let a = PathBuf::from("/tmp/a");
+        let b = PathBuf::from("/tmp/b");
+        let bases = vec![a.clone(), b];
+        assert!(is_inside_any_base(&a.join("x.md"), &bases));
+        assert!(is_inside_any_base(&a.join("nested/x.md"), &bases));
+        assert!(!is_inside_any_base(Path::new("/tmp/other/x.md"), &bases));
     }
 }