Write the VEX document through the shared stage + rename writer (#1144)

crates/socket-patch-cli/src/commands/vex.rs

@@ -912,7 +912,10 @@ async fn generate_vex(
     let wrote_to_file = match &params.output {
         Some(_) if params.dry_run => false,
         Some(path) => {
-            if let Err(e) = tokio::fs::write(path, format!("{serialized}\n")).await {
+            let bytes = format!("{serialized}\n");
+            if let Err(e) =
+                socket_patch_core::utils::fs::write_user_output(path, bytes.as_bytes()).await
+            {
                 // The raw io::Error names neither the file nor the operation.
                 return Err(fail(
                     common,

crates/socket-patch-cli/tests/e2e_vex.rs

@@ -1559,6 +1559,104 @@ fn compact_flag_emits_single_line_json() {
 // vexctl integration (run only when the binary is on PATH)
 // ──────────────────────────────────────────────────────────────────────
 
+/// A write that fails part-way (here `EFBIG` from `RLIMIT_FSIZE`, the
+/// same short write a full disk gives) never leaves a prefix of the new
+/// document at `--output`. The document is staged beside the destination
+/// and renamed into place, so the previous document survives the failed
+/// write and the failure cleanup then removes it: the path holds no file,
+/// never a truncated OpenVEX header (#1144). On `main` the in-place write
+/// left the first 512 bytes, which the cleanup cannot parse and kept.
+#[cfg(unix)]
+#[test]
+fn failed_output_write_leaves_no_partial_document() {
+    use std::os::unix::process::CommandExt as _;
+    let tmp = tempfile::tempdir().unwrap();
+    let cwd = tmp.path();
+    let vex_path = cwd.join("out.vex.json");
+    let run = |limit: Option<u64>| {
+        let mut cmd = cli();
+        cmd.args([
+            "vex",
+            "--cwd",
+            cwd.to_str().unwrap(),
+            "--no-verify",
+            "--json",
+            "--output",
+            vex_path.to_str().unwrap(),
+            "--product",
+            "pkg:npm/app@1.0.0",
+        ]);
+        if let Some(limit) = limit {
+            // SAFETY: only async-signal-safe libc calls between fork and exec.
+            unsafe {
+                cmd.pre_exec(move || {
+                    libc::signal(libc::SIGXFSZ, libc::SIG_IGN);
+                    let lim = libc::rlimit {
+                        rlim_cur: limit as libc::rlim_t,
+                        rlim_max: limit as libc::rlim_t,
+                    };
+                    if libc::setrlimit(libc::RLIMIT_FSIZE, &lim) != 0 {
+                        return Err(std::io::Error::last_os_error());
+                    }
+                    Ok(())
+                });
+            }
+        }
+        cmd.output().expect("invoke vex")
+    };
+
+    let mut manifest = PatchManifest::new();
+    manifest.patches.insert(
+        "pkg:npm/x@1.0.0".to_string(),
+        make_record(
+            "11111111-1111-4111-8111-111111111111",
+            "package/index.js",
+            "a".repeat(64).as_str(),
+            "b".repeat(64).as_str(),
+            "GHSA-zzzz",
+            &["CVE-9999"],
+        ),
+    );
+    write_manifest(cwd, &manifest);
+    assert!(run(None).status.success());
+    let first: Value = serde_json::from_slice(&std::fs::read(&vex_path).unwrap()).unwrap();
+    assert_eq!(first["statements"].as_array().unwrap().len(), 1);
+
+    for i in 0..30 {
+        manifest.patches.insert(
+            format!("pkg:npm/dep{i}@1.0.0"),
+            make_record(
+                &format!("22222222-2222-4222-8222-{i:012}"),
+                "package/index.js",
+                "c".repeat(64).as_str(),
+                "d".repeat(64).as_str(),
+                &format!("GHSA-dep{i}"),
+                &["CVE-2024-0001"],
+            ),
+        );
+    }
+    write_manifest(cwd, &manifest);
+    let out = run(Some(512));
+    assert!(!out.status.success(), "the write must fail under the limit");
+    let env: Value = serde_json::from_slice(&out.stdout).expect("envelope JSON");
+    assert_eq!(env["error"]["code"], "write_failed", "{env}");
+
+    match std::fs::read(&vex_path) {
+        Err(e) => assert_eq!(e.kind(), std::io::ErrorKind::NotFound),
+        Ok(bytes) => panic!(
+            "a failed write left {} bytes at --output: {:?}",
+            bytes.len(),
+            String::from_utf8_lossy(&bytes[..bytes.len().min(80)])
+        ),
+    }
+    let names: Vec<String> = std::fs::read_dir(cwd)
+        .unwrap()
+        .map(|e| e.unwrap().file_name().to_string_lossy().into_owned())
+        .filter(|n| n.starts_with(".socket-stage-"))
+        .collect();
+    assert!(names.is_empty(), "stage litter {names:?}");
+}
+
 /// Pipe the VEX text through `vexctl` if it's on `PATH`. CI installs
 /// vexctl before the test step so the validation actually runs there;
 /// local devs without Go see a skip message instead of a failure.

crates/socket-patch-core/src/utils/fs.rs

@@ -455,6 +455,36 @@ pub async fn atomic_write_bytes_preserving_mode(
     .await
 }
 
+/// Write a file the user named on the command line (`vex --output`, the
+/// embedded `--vex <path>`): a reader, or a failed write, sees the complete
+/// old or the complete new bytes, never a prefix.
+///
+/// A regular file (or a symlink to one) is replaced by stage + rename, keeping
+/// its permission bits; a symlink is written through to its target, as a
+/// plain `write` would, so the user's link stays a link. Anything else that
+/// already exists (`/dev/stdout`, a FIFO, a character device) has no inode to
+/// swap, so it is written in place. Never captured by an open group commit:
+/// the path is the user's output, not one of the run's commit points.
+pub async fn write_user_output(path: &Path, content: &[u8]) -> std::io::Result<()> {
+    match tokio::fs::metadata(path).await {
+        Ok(meta) if !meta.is_file() && !meta.is_dir() => tokio::fs::write(path, content).await,
+        _ => {
+            let target = tokio::fs::canonicalize(path)
+                .await
+                .unwrap_or_else(|_| path.to_path_buf());
+            write_atomic(
+                &target,
+                content,
+                WriteOpts {
+                    preserve_mode: true,
+                    ..WriteOpts::UNSYNCED
+                },
+            )
+            .await
+        }
+    }
+}
+
 /// Atomically write a CONTENT-VERIFIED artifact (a vendored `.tgz`, wheel,
 /// jar, marker, …) via stage + rename, without an fsync: the next durable
 /// commit point's [`super::durability::barrier`] makes it durable before
@@ -949,7 +979,7 @@ mod tests {
             m
         };
         assert_ne!(fresh_mode, 0o600, "umask must make 0600 distinguishable");
-        let cases: [(&str, bool); 8] = [
+        let cases: [(&str, bool); 9] = [
             ("bytes", false),
             ("bytes_preserving", true),
             ("artifact", false),
@@ -958,6 +988,7 @@ mod tests {
             ("unsynced_preserving", true),
             ("sync_plain", false),
             ("sync_preserving", true),
+            ("user_output", true),
         ];
         for (name, preserves) in cases {
             let dir = tmp.path().join(name);
@@ -978,6 +1009,7 @@ mod tests {
                 "unsynced_preserving" => atomic_write_unsynced(&path, content, true).await,
                 "sync_plain" => atomic_write_sync(&path, content, false),
                 "sync_preserving" => atomic_write_sync(&path, content, true),
+                "user_output" => write_user_output(&path, content).await,
                 _ => unreachable!(),
             }
             .unwrap_or_else(|e| panic!("{name}: {e}"));
@@ -993,6 +1025,52 @@ mod tests {
         }
     }
 
+    /// A user-named output path (`vex --output`) keeps what the user put
+    /// there: a symlink stays a link and its target gets the new bytes, a
+    /// character device (`/dev/null`, like `/dev/stdout`) is written in
+    /// place rather than swapped for a regular file, and an open group
+    /// commit does not hold the write back (#1144).
+    #[cfg(unix)]
+    #[tokio::test]
+    async fn user_output_writes_through_links_and_devices() {
+        let tmp = tempfile::tempdir().unwrap();
+        let target = tmp.path().join("real.vex.json");
+        std::fs::write(&target, b"old").unwrap();
+        let link = tmp.path().join("out.vex.json");
+        std::os::unix::fs::symlink(&target, &link).unwrap();
+        write_user_output(&link, b"new").await.unwrap();
+        assert!(std::fs::symlink_metadata(&link)
+            .unwrap()
+            .file_type()
+            .is_symlink());
+        assert_eq!(std::fs::read(&target).unwrap(), b"new");
+        let names: Vec<String> = std::fs::read_dir(tmp.path())
+            .unwrap()
+            .map(|e| e.unwrap().file_name().to_string_lossy().into_owned())
+            .collect();
+        assert_eq!(names.len(), 2, "stage litter {names:?}");
+
+        write_user_output(Path::new("/dev/null"), b"doc")
+            .await
+            .unwrap();
+        use std::os::unix::fs::FileTypeExt as _;
+        assert!(std::fs::metadata("/dev/null")
+            .unwrap()
+            .file_type()
+            .is_char_device());
+
+        let group = super::super::group_commit::GroupCommit::begin(tmp.path());
+        let fresh = tmp.path().join("fresh.vex.json");
+        write_user_output(&fresh, b"doc").await.unwrap();
+        assert_eq!(
+            std::fs::read(&fresh).unwrap(),
+            b"doc",
+            "written, not captured"
+        );
+        drop(group);
+        assert_eq!(std::fs::read(&fresh).unwrap(), b"doc");
+    }
+
     /// One stage-name builder for the `utils::fs` writers and the blob
     /// cache: a dot-prefixed sibling named after the destination, unique per
     /// call.