agent: lumotia — Phase B.5 close symlink-target bypass in write_text_file_cmd path scope

Phase B.5 audit of commits a2b47db/a48653c/b3da58c (Trust-1 — write
path allowlist), 9653e25 (Trust-5 — transcribe_file extension allowlist
+ size cap), and ed449cc (Trust-2 — resolve_recording_path output_folder
validation). The three Trust-1 commits were a corrective sequence that
swapped the staged file in a parallel-agent race; b3da58c is the
authoritative landing.

Existing coverage is strong:
  * commands/fs.rs (Trust-1): 6 tests — outside-allowlist, traversal,
    accepts inside, accepts nested, rejects missing parent, prefix check.
  * commands/transcription.rs (Trust-5): 7 tests — accepts wav,
    accepts MP3 case-insensitive, accepts each allowed extension,
    rejects unsupported, rejects no-extension, rejects oversize, accepts
    exactly-at-cap, rejects traversal-with-disallowed-ext.
  * commands/audio.rs (Trust-2): 7 tests including
    validate_output_folder_rejects_symlink_pointing_out — the symlink
    bypass for output folders is already covered.

One real residual found in commands/fs.rs:

Asymmetric symlink handling between Trust-1 (fs.rs) and Trust-2
(audio.rs). Trust-2 canonicalises the FULL requested path (it's a
directory that must already exist), so a symlink at the directory itself
that points outside the base is resolved before the containment check
and gets rejected. Trust-1 canonicalises only the PARENT of the
requested path, because the target file typically does not exist yet
(canonicalize() returns NotFound on missing paths). Concrete bypass:

  1. A symlink at, e.g., ~/Downloads/notes.md -> ~/.bashrc — innocently
     created by the user, or planted via another vulnerability.
  2. Compromised webview invokes
       write_text_file_cmd("/home/user/Downloads/notes.md", "<payload>").
  3. Path-scope check: parent canonicalises to /home/user/Downloads,
     file_name joins, canonical path string sits inside the Downloads
     allowlist. PASS.
  4. tokio::fs::write -> File::create -> open(2) follows the symlink and
     writes "<payload>" to ~/.bashrc, exfiltrating shell startup.

Fix: two-mode canonicalisation in resolve_export_path. If the target
exists, canonicalise the full path — this follows any symlink at the
target itself, and the subsequent containment check sees the resolved
location. Only on NotFound do we fall back to parent-canonicalise +
join-filename (the original save-dialog path). This mirrors the audio
crate's canonicalisation discipline.

Regression tests:
  * rejects_symlink_target_outside_allowlist — creates a symlink inside
    a base pointing OUT to a real outside file; resolve_export_path must
    return Err with "outside the allowed export directories".
  * accepts_symlink_target_inside_allowlist — symmetric, an in-base
    alias symlink must still resolve and pass, so legitimate uses of
    symlinks are not regressed.

Both gated #[cfg(unix)] because std::os::unix::fs::symlink is unix-only.
The Trust-1 surface ships symmetrically on Windows; the symlink class
attack does not generalise the same way on NTFS (junctions vs symlinks
have different ACL semantics), and a windows-specific test would be
duplicate-effort outside the audit scope.

Verification:
  * cargo test -p lumotia --lib commands::fs
      → 8/8 pass including the two new symlink tests.
  * cargo fmt --check → clean.
  * cargo clippy -p lumotia --all-targets -- -D warnings → clean.

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

src-tauri/src/commands/fs.rs

@@ -72,36 +72,58 @@ fn allowed_export_bases(app: &tauri::AppHandle) -> Vec<PathBuf> {
     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`.
+/// Resolve the requested write target. 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.
+/// Two-mode canonicalisation:
+///
+/// - **If the target already exists**, canonicalise the WHOLE path. This
+///   follows any symlink at the target itself, so a symlink that sits
+///   inside an allowlisted base but points outside it gets rejected
+///   here. `tokio::fs::write` follows symlinks on open(2), so without
+///   this branch a symlink at `~/Downloads/notes.md -> ~/.bashrc` would
+///   pass the containment check (the path string is inside Downloads)
+///   and the subsequent write would silently land in `~/.bashrc`.
+///   Trust-1 audit residual closed in Phase B.5 (2026-05-14); parallels
+///   `validate_output_folder`'s full-path-canonicalize in
+///   `commands/audio.rs` (Trust-2).
+///
+/// - **If the target does not yet exist** (the typical save-dialog
+///   case), canonicalise the parent and re-join the filename. 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_path = match std::fs::canonicalize(path) {
+        Ok(canon) => canon,
+        Err(e) if e.kind() == std::io::ErrorKind::NotFound => {
+            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);
+            let canon_parent = std::fs::canonicalize(parent).map_err(|e| {
+                format!(
+                    "Refusing to write {}: cannot resolve parent dir ({e}).",
+                    path.display()
+                )
+            })?;
+            canon_parent.join(file_name)
+        }
+        Err(e) => {
+            return Err(format!(
+                "Refusing to write {}: cannot resolve path ({e}).",
+                path.display()
+            ));
+        }
+    };
 
     if !is_inside_any_base(&canon_path, bases) {
         return Err(format!(
@@ -209,4 +231,65 @@ mod tests {
         assert!(is_inside_any_base(&a.join("nested/x.md"), &bases));
         assert!(!is_inside_any_base(Path::new("/tmp/other/x.md"), &bases));
     }
+
+    /// Phase B.5 audit regression (2026-05-14). The Trust-1 fix
+    /// canonicalised only the parent of the target path because the file
+    /// typically does not exist yet. But if a symlink ALREADY exists at
+    /// the target path pointing outside the allowlisted base,
+    /// `tokio::fs::write` follows it on `File::create -> open(2)` and
+    /// silently writes through to the outside target. The path-string
+    /// containment check passed because the SYMLINK lives inside the
+    /// base, even though the symlink's target does not.
+    ///
+    /// The fix canonicalises the full path when it exists (so the
+    /// symlink resolves before the containment check) and only falls
+    /// back to parent-canonicalize on NotFound. This regression test
+    /// pins that contract.
+    #[cfg(unix)]
+    #[test]
+    fn rejects_symlink_target_outside_allowlist() {
+        let (_keep_base, base) = tempdir_canon("base");
+        let (_keep_out, outside) = tempdir_canon("outside");
+        // Real, writable file outside the base — the symlink target.
+        let outside_target = outside.join("victim.txt");
+        std::fs::write(&outside_target, b"original").expect("seed outside target");
+
+        // Inside-base symlink pointing to the outside file.
+        let symlink_path = base.join("notes.md");
+        std::os::unix::fs::symlink(&outside_target, &symlink_path)
+            .expect("create symlink inside base pointing outside");
+
+        let bases = vec![base.clone()];
+        let result = resolve_export_path(&symlink_path, &bases);
+        assert!(
+            result.is_err(),
+            "symlink target outside allowlist must be rejected, got {result:?}"
+        );
+        let err = result.err().unwrap();
+        assert!(
+            err.contains("outside the allowed export directories"),
+            "unexpected error shape: {err}"
+        );
+    }
+
+    /// Symmetric to the rejection above: a symlink whose target stays
+    /// inside the same base must still be accepted, so legitimate users
+    /// of in-base symlinks (e.g. a tilde-expansion alias inside the base)
+    /// are not regressed.
+    #[cfg(unix)]
+    #[test]
+    fn accepts_symlink_target_inside_allowlist() {
+        let (_keep, base) = tempdir_canon("base");
+        let real_path = base.join("real.md");
+        std::fs::write(&real_path, b"original").expect("seed real target");
+
+        let symlink_path = base.join("alias.md");
+        std::os::unix::fs::symlink(&real_path, &symlink_path)
+            .expect("create in-base alias symlink");
+
+        let bases = vec![base.clone()];
+        let resolved = resolve_export_path(&symlink_path, &bases)
+            .expect("in-base symlink must resolve and pass");
+        assert!(resolved.starts_with(&base));
+    }
 }