Skip to content

Commit ec4ffeb

Browse files
committed
Write the VEX document by stage + rename
`vex --output` and the embedded `--vex <path>` of apply, vendor and scan wrote the OpenVEX document with `tokio::fs::write`, which truncates the file and writes in place. A write that failed part-way (full disk, quota, file-size limit) left a truncated document that still starts with an OpenVEX header; the failure cleanup cannot parse it, so it stayed, against the documented "a failed run leaves no document" contract. The document now goes through `utils::fs::write_user_output`, the shared stage + rename core with the destination's mode kept: a failed write leaves the previous document, which the cleanup then removes. A symlinked output is written through to its target, and /dev/stdout, a FIFO or a device is still written in place. The write is never held back by an open group commit. The `write_failed` code and message are unchanged. Fixes #1144 Assisted-by: Claude Code:claude-opus-5-5
1 parent 333ce57 commit ec4ffeb

3 files changed

Lines changed: 181 additions & 2 deletions

File tree

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

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -912,7 +912,10 @@ async fn generate_vex(
912912
let wrote_to_file = match &params.output {
913913
Some(_) if params.dry_run => false,
914914
Some(path) => {
915-
if let Err(e) = tokio::fs::write(path, format!("{serialized}\n")).await {
915+
let bytes = format!("{serialized}\n");
916+
if let Err(e) =
917+
socket_patch_core::utils::fs::write_user_output(path, bytes.as_bytes()).await
918+
{
916919
// The raw io::Error names neither the file nor the operation.
917920
return Err(fail(
918921
common,

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

Lines changed: 98 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1559,6 +1559,104 @@ fn compact_flag_emits_single_line_json() {
15591559
// vexctl integration (run only when the binary is on PATH)
15601560
// ──────────────────────────────────────────────────────────────────────
15611561

1562+
/// A write that fails part-way (here `EFBIG` from `RLIMIT_FSIZE`, the
1563+
/// same short write a full disk gives) never leaves a prefix of the new
1564+
/// document at `--output`. The document is staged beside the destination
1565+
/// and renamed into place, so the previous document survives the failed
1566+
/// write and the failure cleanup then removes it: the path holds no file,
1567+
/// never a truncated OpenVEX header (#1144). On `main` the in-place write
1568+
/// left the first 512 bytes, which the cleanup cannot parse and kept.
1569+
#[cfg(unix)]
1570+
#[test]
1571+
fn failed_output_write_leaves_no_partial_document() {
1572+
use std::os::unix::process::CommandExt as _;
1573+
let tmp = tempfile::tempdir().unwrap();
1574+
let cwd = tmp.path();
1575+
let vex_path = cwd.join("out.vex.json");
1576+
let run = |limit: Option<u64>| {
1577+
let mut cmd = cli();
1578+
cmd.args([
1579+
"vex",
1580+
"--cwd",
1581+
cwd.to_str().unwrap(),
1582+
"--no-verify",
1583+
"--json",
1584+
"--output",
1585+
vex_path.to_str().unwrap(),
1586+
"--product",
1587+
"pkg:npm/app@1.0.0",
1588+
]);
1589+
if let Some(limit) = limit {
1590+
// SAFETY: only async-signal-safe libc calls between fork and exec.
1591+
unsafe {
1592+
cmd.pre_exec(move || {
1593+
libc::signal(libc::SIGXFSZ, libc::SIG_IGN);
1594+
let lim = libc::rlimit {
1595+
rlim_cur: limit as libc::rlim_t,
1596+
rlim_max: limit as libc::rlim_t,
1597+
};
1598+
if libc::setrlimit(libc::RLIMIT_FSIZE, &lim) != 0 {
1599+
return Err(std::io::Error::last_os_error());
1600+
}
1601+
Ok(())
1602+
});
1603+
}
1604+
}
1605+
cmd.output().expect("invoke vex")
1606+
};
1607+
1608+
let mut manifest = PatchManifest::new();
1609+
manifest.patches.insert(
1610+
"pkg:npm/x@1.0.0".to_string(),
1611+
make_record(
1612+
"11111111-1111-4111-8111-111111111111",
1613+
"package/index.js",
1614+
"a".repeat(64).as_str(),
1615+
"b".repeat(64).as_str(),
1616+
"GHSA-zzzz",
1617+
&["CVE-9999"],
1618+
),
1619+
);
1620+
write_manifest(cwd, &manifest);
1621+
assert!(run(None).status.success());
1622+
let first: Value = serde_json::from_slice(&std::fs::read(&vex_path).unwrap()).unwrap();
1623+
assert_eq!(first["statements"].as_array().unwrap().len(), 1);
1624+
1625+
for i in 0..30 {
1626+
manifest.patches.insert(
1627+
format!("pkg:npm/dep{i}@1.0.0"),
1628+
make_record(
1629+
&format!("22222222-2222-4222-8222-{i:012}"),
1630+
"package/index.js",
1631+
"c".repeat(64).as_str(),
1632+
"d".repeat(64).as_str(),
1633+
&format!("GHSA-dep{i}"),
1634+
&["CVE-2024-0001"],
1635+
),
1636+
);
1637+
}
1638+
write_manifest(cwd, &manifest);
1639+
let out = run(Some(512));
1640+
assert!(!out.status.success(), "the write must fail under the limit");
1641+
let env: Value = serde_json::from_slice(&out.stdout).expect("envelope JSON");
1642+
assert_eq!(env["error"]["code"], "write_failed", "{env}");
1643+
1644+
match std::fs::read(&vex_path) {
1645+
Err(e) => assert_eq!(e.kind(), std::io::ErrorKind::NotFound),
1646+
Ok(bytes) => panic!(
1647+
"a failed write left {} bytes at --output: {:?}",
1648+
bytes.len(),
1649+
String::from_utf8_lossy(&bytes[..bytes.len().min(80)])
1650+
),
1651+
}
1652+
let names: Vec<String> = std::fs::read_dir(cwd)
1653+
.unwrap()
1654+
.map(|e| e.unwrap().file_name().to_string_lossy().into_owned())
1655+
.filter(|n| n.starts_with(".socket-stage-"))
1656+
.collect();
1657+
assert!(names.is_empty(), "stage litter {names:?}");
1658+
}
1659+
15621660
/// Pipe the VEX text through `vexctl` if it's on `PATH`. CI installs
15631661
/// vexctl before the test step so the validation actually runs there;
15641662
/// local devs without Go see a skip message instead of a failure.

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

Lines changed: 79 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -455,6 +455,36 @@ pub async fn atomic_write_bytes_preserving_mode(
455455
.await
456456
}
457457

458+
/// Write a file the user named on the command line (`vex --output`, the
459+
/// embedded `--vex <path>`): a reader, or a failed write, sees the complete
460+
/// old or the complete new bytes, never a prefix.
461+
///
462+
/// A regular file (or a symlink to one) is replaced by stage + rename, keeping
463+
/// its permission bits; a symlink is written through to its target, as a
464+
/// plain `write` would, so the user's link stays a link. Anything else that
465+
/// already exists (`/dev/stdout`, a FIFO, a character device) has no inode to
466+
/// swap, so it is written in place. Never captured by an open group commit:
467+
/// the path is the user's output, not one of the run's commit points.
468+
pub async fn write_user_output(path: &Path, content: &[u8]) -> std::io::Result<()> {
469+
match tokio::fs::metadata(path).await {
470+
Ok(meta) if !meta.is_file() && !meta.is_dir() => tokio::fs::write(path, content).await,
471+
_ => {
472+
let target = tokio::fs::canonicalize(path)
473+
.await
474+
.unwrap_or_else(|_| path.to_path_buf());
475+
write_atomic(
476+
&target,
477+
content,
478+
WriteOpts {
479+
preserve_mode: true,
480+
..WriteOpts::UNSYNCED
481+
},
482+
)
483+
.await
484+
}
485+
}
486+
}
487+
458488
/// Atomically write a CONTENT-VERIFIED artifact (a vendored `.tgz`, wheel,
459489
/// jar, marker, …) via stage + rename, without an fsync: the next durable
460490
/// commit point's [`super::durability::barrier`] makes it durable before
@@ -949,7 +979,7 @@ mod tests {
949979
m
950980
};
951981
assert_ne!(fresh_mode, 0o600, "umask must make 0600 distinguishable");
952-
let cases: [(&str, bool); 8] = [
982+
let cases: [(&str, bool); 9] = [
953983
("bytes", false),
954984
("bytes_preserving", true),
955985
("artifact", false),
@@ -958,6 +988,7 @@ mod tests {
958988
("unsynced_preserving", true),
959989
("sync_plain", false),
960990
("sync_preserving", true),
991+
("user_output", true),
961992
];
962993
for (name, preserves) in cases {
963994
let dir = tmp.path().join(name);
@@ -978,6 +1009,7 @@ mod tests {
9781009
"unsynced_preserving" => atomic_write_unsynced(&path, content, true).await,
9791010
"sync_plain" => atomic_write_sync(&path, content, false),
9801011
"sync_preserving" => atomic_write_sync(&path, content, true),
1012+
"user_output" => write_user_output(&path, content).await,
9811013
_ => unreachable!(),
9821014
}
9831015
.unwrap_or_else(|e| panic!("{name}: {e}"));
@@ -993,6 +1025,52 @@ mod tests {
9931025
}
9941026
}
9951027

1028+
/// A user-named output path (`vex --output`) keeps what the user put
1029+
/// there: a symlink stays a link and its target gets the new bytes, a
1030+
/// character device (`/dev/null`, like `/dev/stdout`) is written in
1031+
/// place rather than swapped for a regular file, and an open group
1032+
/// commit does not hold the write back (#1144).
1033+
#[cfg(unix)]
1034+
#[tokio::test]
1035+
async fn user_output_writes_through_links_and_devices() {
1036+
let tmp = tempfile::tempdir().unwrap();
1037+
let target = tmp.path().join("real.vex.json");
1038+
std::fs::write(&target, b"old").unwrap();
1039+
let link = tmp.path().join("out.vex.json");
1040+
std::os::unix::fs::symlink(&target, &link).unwrap();
1041+
write_user_output(&link, b"new").await.unwrap();
1042+
assert!(std::fs::symlink_metadata(&link)
1043+
.unwrap()
1044+
.file_type()
1045+
.is_symlink());
1046+
assert_eq!(std::fs::read(&target).unwrap(), b"new");
1047+
let names: Vec<String> = std::fs::read_dir(tmp.path())
1048+
.unwrap()
1049+
.map(|e| e.unwrap().file_name().to_string_lossy().into_owned())
1050+
.collect();
1051+
assert_eq!(names.len(), 2, "stage litter {names:?}");
1052+
1053+
write_user_output(Path::new("/dev/null"), b"doc")
1054+
.await
1055+
.unwrap();
1056+
use std::os::unix::fs::FileTypeExt as _;
1057+
assert!(std::fs::metadata("/dev/null")
1058+
.unwrap()
1059+
.file_type()
1060+
.is_char_device());
1061+
1062+
let group = super::super::group_commit::GroupCommit::begin(tmp.path());
1063+
let fresh = tmp.path().join("fresh.vex.json");
1064+
write_user_output(&fresh, b"doc").await.unwrap();
1065+
assert_eq!(
1066+
std::fs::read(&fresh).unwrap(),
1067+
b"doc",
1068+
"written, not captured"
1069+
);
1070+
drop(group);
1071+
assert_eq!(std::fs::read(&fresh).unwrap(), b"doc");
1072+
}
1073+
9961074
/// One stage-name builder for the `utils::fs` writers and the blob
9971075
/// cache: a dot-prefixed sibling named after the destination, unique per
9981076
/// call.

0 commit comments

Comments
 (0)