Bugfix: Stop Volume::create_file from clobbering existing files

The IPC New File path expects `Volume::create_file` to fail with
`AlreadyExists` when the destination already exists, so the dialog
can surface a friendly "already exists" error. Pre-fix, every backend
silently truncated:

- `LocalPosixVolume` used `std::fs::write`. Now uses
  `OpenOptions::create_new(true)`; the resulting `EEXIST` round-trips
  to `VolumeError::AlreadyExists`.
- `SmbVolume` delegated to `tree.write_file` (`FileOverwriteIf`
  disposition). smb2 0.11's public API has no exclusive-create writer,
  so this lands a `tree.stat` pre-check followed by the existing write.
  The pre-check leaves a microsecond-scale TOCTOU window; the truly
  atomic fix needs an upstream smb2 release exposing `FileCreate` for
  files.
- `InMemoryVolume` now rejects collisions so the test backend can't
  mask the bug in unit tests.

Regression tests pinned for local (unit), in-memory (unit), and SMB
(Docker integration, `#[ignore]`).

Author: vdavid

apps/desktop/src-tauri/src/file_system/volume/backends/in_memory.rs

@@ -287,6 +287,10 @@ impl Volume for InMemoryVolume {
 
             let normalized = self.normalize(path);
 
+            if entries.contains_key(&normalized) {
+                return Err(VolumeError::AlreadyExists(normalized.display().to_string()));
+            }
+
             let name = normalized
                 .file_name()
                 .map(|s| s.to_string_lossy().to_string())

apps/desktop/src-tauri/src/file_system/volume/backends/in_memory_test.rs

@@ -135,6 +135,29 @@ async fn test_create_file_then_exists() {
     assert!(!metadata.is_directory);
 }
 
+#[tokio::test]
+async fn test_create_file_does_not_clobber_existing() {
+    // Regression for the high-severity audit finding: `create_file` is a
+    // no-overwrite contract. The InMemoryVolume must reject collisions so
+    // tests that stand in for real backends don't mask the bug.
+    let volume = InMemoryVolume::new("Test");
+
+    volume
+        .create_file(Path::new("/notes.txt"), b"important user data")
+        .await
+        .unwrap();
+
+    let result = volume.create_file(Path::new("/notes.txt"), b"").await;
+
+    assert!(
+        matches!(result, Err(VolumeError::AlreadyExists(_))),
+        "expected AlreadyExists, got {:?}",
+        result
+    );
+    let metadata = volume.get_metadata(Path::new("/notes.txt")).await.unwrap();
+    assert_eq!(metadata.size, Some(19), "original file bytes must survive");
+}
+
 #[tokio::test]
 async fn test_create_directory_then_exists() {
     let volume = InMemoryVolume::new("Test");

apps/desktop/src-tauri/src/file_system/volume/backends/local_posix.rs

@@ -287,8 +287,18 @@ impl Volume for LocalPosixVolume {
         }
         let content = content.to_vec();
         Box::pin(async move {
-            spawn_blocking(move || {
-                std::fs::write(&abs_path, content)?;
+            spawn_blocking(move || -> Result<(), VolumeError> {
+                use std::io::Write;
+                // `create_new(true)` is the no-clobber contract the IPC layer
+                // and frontend assume: an `AlreadyExists` errno surfaces as
+                // `VolumeError::AlreadyExists`, which the New File command
+                // maps to a friendly "already exists" error. A plain
+                // `std::fs::write` would silently truncate the user's file.
+                let mut file = std::fs::OpenOptions::new()
+                    .write(true)
+                    .create_new(true)
+                    .open(&abs_path)?;
+                file.write_all(&content)?;
                 Ok(())
             })
             .await

apps/desktop/src-tauri/src/file_system/volume/backends/local_posix_test.rs

@@ -203,6 +203,39 @@ async fn test_rename_force_overwrites() {
     let _ = fs::remove_dir_all(&test_dir);
 }
 
+#[tokio::test]
+async fn test_create_file_does_not_clobber_existing() {
+    // Regression for the high-severity audit finding: `create_file` is a
+    // no-overwrite contract; the IPC layer maps `AlreadyExists` to a friendly
+    // "file already exists" error. Switching to a truncating write here loses
+    // the user's data when the listing-cache pre-check race fails.
+    use std::fs;
+
+    let test_dir = std::env::temp_dir().join("cmdr_create_file_no_clobber_test");
+    let _ = fs::remove_dir_all(&test_dir);
+    fs::create_dir_all(&test_dir).unwrap();
+
+    let volume = LocalPosixVolume::new("Test", &test_dir);
+    let target = test_dir.join("notes.txt");
+
+    fs::write(&target, "important user data").unwrap();
+
+    let result = volume.create_file(Path::new("notes.txt"), b"").await;
+
+    assert!(
+        matches!(result, Err(VolumeError::AlreadyExists(_))),
+        "expected AlreadyExists, got {:?}",
+        result
+    );
+    assert_eq!(
+        fs::read_to_string(&target).unwrap(),
+        "important user data",
+        "original file contents must survive a colliding create_file"
+    );
+
+    let _ = fs::remove_dir_all(&test_dir);
+}
+
 // ============================================================================
 // Symlink edge case tests
 // ============================================================================

apps/desktop/src-tauri/src/file_system/volume/backends/smb.rs

@@ -1314,6 +1314,27 @@ impl Volume for SmbVolume {
 
             {
                 let (tree, mut conn) = self.clone_session().await?;
+                // No-clobber contract. The smb2 0.11 public API only exposes
+                // file writers backed by `FileOverwriteIf` disposition (no
+                // exclusive `FileCreate` for files), so we do a pre-flight
+                // stat. There's a brief TOCTOU window between stat and write
+                // where a third party can create the same path; we accept it
+                // because the alternative (silently truncating the user's
+                // file when the listing-cache pre-check races) is worse, and
+                // because the realistic attack window is microseconds on the
+                // same SMB session. The truly atomic fix needs an smb2 crate
+                // release that exposes an exclusive-create writer.
+                match tree.stat(&mut conn, &smb_path).await {
+                    Ok(_) => {
+                        return Err(VolumeError::AlreadyExists(path.display().to_string()));
+                    }
+                    Err(e) if e.kind() == smb2::ErrorKind::NotFound => {
+                        // Expected: file doesn't exist, fall through to write.
+                    }
+                    Err(e) => {
+                        return Err(map_smb_error(e));
+                    }
+                }
                 let result = tree.write_file(&mut conn, &smb_path, &data).await;
                 self.handle_smb_result("create_file", result)?;
             }
@@ -2832,6 +2853,41 @@ mod tests {
         vol.delete(Path::new(&dir)).await.unwrap();
     }
 
+    /// Regression for the high-severity audit finding: `create_file` is a
+    /// no-overwrite contract. Pre-fix, SMB delegated to `tree.write_file`
+    /// which uses `FileOverwriteIf` disposition and silently truncated.
+    #[tokio::test]
+    #[ignore = "Requires Docker SMB containers (./apps/desktop/test/smb-servers/start.sh)"]
+    async fn smb_integration_create_file_does_not_clobber_existing() {
+        let vol = make_docker_volume().await;
+        let dir = test_dir_name();
+        ensure_clean(&vol, &dir).await;
+
+        vol.create_directory(Path::new(&dir)).await.unwrap();
+        let file_path = format!("{}/notes.txt", dir);
+        let original = b"important user data";
+        vol.create_file(Path::new(&file_path), original).await.unwrap();
+
+        // Second create on the same path must fail with AlreadyExists;
+        // bytes on the wire must be unchanged.
+        let result = vol.create_file(Path::new(&file_path), b"junk").await;
+        assert!(
+            matches!(result, Err(VolumeError::AlreadyExists(_))),
+            "expected AlreadyExists, got {:?}",
+            result
+        );
+
+        let mut readback = vol.open_read_stream(Path::new(&file_path)).await.unwrap();
+        let mut bytes = Vec::new();
+        while let Some(Ok(chunk)) = readback.next_chunk().await {
+            bytes.extend_from_slice(&chunk);
+        }
+        assert_eq!(bytes, original, "original bytes must survive a colliding create_file");
+
+        vol.delete(Path::new(&file_path)).await.unwrap();
+        vol.delete(Path::new(&dir)).await.unwrap();
+    }
+
     /// Regression test for a unit-mismatch bug where SMB returned `modified_at` in
     /// milliseconds while the rest of cmdr (and the frontend formatter) expects seconds.
     /// That caused displayed years like 58247 on real shares. Asserts the mtime of a