Skip to content

Commit c50be52

Browse files
Write the VEX document through the shared stage + rename writer (#1144) (#1262)
* Start refactor for #1144 Assisted-by: Claude Code:claude-opus-5-5 * 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 * Write Windows device outputs in place On Windows, std reports NUL, CON and named pipes as files, so `--vex NUL` would have staged a regular file and failed to rename it over the device with write_failed. A DOS device name or a \\.\ path is now written in place, as /dev/stdout and FIFOs are on Unix. Assisted-by: Claude Code:claude-opus-5-5 * Keep the size-limited VEX test out of coverage The regression test runs the CLI under RLIMIT_FSIZE = 512 bytes. In the coverage job that limit also truncated the child's exit-time .profraw, and llvm-profdata refused to merge the corrupt profile, so the coverage job failed. The limited run now writes its profile to /dev/null, which the limit does not cover. Assisted-by: Claude Code:claude-opus-5-5 * Write /dev and /proc output paths in place `vex --output /dev/stdout > vex.json` follows /dev/stdout to the file fd 1 points at, which is regular, so the document was staged and renamed over it: the caller's open descriptor kept the old empty inode, macOS failed because /dev/fd holds no stage, and with fd 1 on a deleted file a root run could stage in /dev and replace /dev/stdout. A path that is, or links through, anything under /dev or /proc (/dev/stdout, /dev/fd/1, /proc/self/fd/1, a user's link to one) is now written in place, as before this branch. The link chain is read hop by hop, never canonicalized, since canonicalizing /proc/self/fd/1 yields the regular file behind it. Assisted-by: Claude Code:claude-opus-5-5 * Treat only descriptor paths as output streams The in-place rule matched every path under /dev or /proc, so a regular file on /dev/shm or reached through /proc/self/cwd skipped stage + rename and could again be left truncated by a failed write. Only /dev/std{in,out,err}, /dev/fd/<n> and /proc/<pid>/fd/<n> (or a link to one) are written in place now. Assisted-by: Claude Code:claude-opus-5-5 --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 6c01c7f commit c50be52

3 files changed

Lines changed: 434 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: 177 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1559,6 +1559,183 @@ fn compact_flag_emits_single_line_json() {
15591559
// vexctl integration (run only when the binary is on PATH)
15601560
// ──────────────────────────────────────────────────────────────────────
15611561

1562+
/// `vex --output /dev/stdout > vex.json`: the document reaches the file the
1563+
/// shell redirected stdout to, through the caller's own descriptor.
1564+
/// `/dev/stdout` is written in place, never staged and renamed over the
1565+
/// descriptor's target, which would leave the caller's open file without
1566+
/// the document (review on #1262). Read through a kept handle, not the
1567+
/// path, so a rename shows. The human summary line also goes to fd 1 and
1568+
/// overwrites the head of the document, as on `main`; only the body is
1569+
/// asserted.
1570+
#[cfg(unix)]
1571+
#[test]
1572+
fn output_to_dev_stdout_reaches_the_redirected_file() {
1573+
let tmp = tempfile::tempdir().unwrap();
1574+
let cwd = tmp.path();
1575+
let mut manifest = PatchManifest::new();
1576+
manifest.patches.insert(
1577+
"pkg:npm/x@1.0.0".to_string(),
1578+
make_record(
1579+
"11111111-1111-4111-8111-111111111111",
1580+
"package/index.js",
1581+
"a".repeat(64).as_str(),
1582+
"b".repeat(64).as_str(),
1583+
"GHSA-zzzz",
1584+
&["CVE-9999"],
1585+
),
1586+
);
1587+
write_manifest(cwd, &manifest);
1588+
use std::io::{Read as _, Seek as _};
1589+
let captured = cwd.join("captured.vex.json");
1590+
let stdout = std::fs::OpenOptions::new()
1591+
.read(true)
1592+
.write(true)
1593+
.create(true)
1594+
.truncate(true)
1595+
.open(&captured)
1596+
.unwrap();
1597+
let mut kept = stdout.try_clone().unwrap();
1598+
let out = cli()
1599+
.args([
1600+
"vex",
1601+
"--cwd",
1602+
cwd.to_str().unwrap(),
1603+
"--no-verify",
1604+
"--output",
1605+
"/dev/stdout",
1606+
"--product",
1607+
"pkg:npm/app@1.0.0",
1608+
])
1609+
.stdout(stdout)
1610+
.output()
1611+
.expect("invoke vex");
1612+
assert!(
1613+
out.status.success(),
1614+
"stderr:\n{}",
1615+
String::from_utf8_lossy(&out.stderr)
1616+
);
1617+
let mut text = String::new();
1618+
kept.rewind().unwrap();
1619+
kept.read_to_string(&mut text).unwrap();
1620+
assert!(
1621+
text.contains("\"author\": \"Socket\"") && text.contains("GHSA-zzzz"),
1622+
"the redirected file must hold the document; got {text:?}"
1623+
);
1624+
let stages = std::fs::read_dir(cwd)
1625+
.unwrap()
1626+
.filter(|e| {
1627+
e.as_ref()
1628+
.unwrap()
1629+
.file_name()
1630+
.to_string_lossy()
1631+
.starts_with(".socket-stage-")
1632+
})
1633+
.count();
1634+
assert_eq!(stages, 0);
1635+
}
1636+
1637+
/// A write that fails part-way (here `EFBIG` from `RLIMIT_FSIZE`, the
1638+
/// same short write a full disk gives) never leaves a prefix of the new
1639+
/// document at `--output`. The document is staged beside the destination
1640+
/// and renamed into place, so the previous document survives the failed
1641+
/// write and the failure cleanup then removes it: the path holds no file,
1642+
/// never a truncated OpenVEX header (#1144). On `main` the in-place write
1643+
/// left the first 512 bytes, which the cleanup cannot parse and kept.
1644+
#[cfg(unix)]
1645+
#[test]
1646+
fn failed_output_write_leaves_no_partial_document() {
1647+
use std::os::unix::process::CommandExt as _;
1648+
let tmp = tempfile::tempdir().unwrap();
1649+
let cwd = tmp.path();
1650+
let vex_path = cwd.join("out.vex.json");
1651+
let run = |limit: Option<u64>| {
1652+
let mut cmd = cli();
1653+
cmd.args([
1654+
"vex",
1655+
"--cwd",
1656+
cwd.to_str().unwrap(),
1657+
"--no-verify",
1658+
"--json",
1659+
"--output",
1660+
vex_path.to_str().unwrap(),
1661+
"--product",
1662+
"pkg:npm/app@1.0.0",
1663+
]);
1664+
if let Some(limit) = limit {
1665+
// The limit also caps a coverage build's exit-time `.profraw`
1666+
// write, and a truncated profile fails the coverage merge: send
1667+
// this run's profile to a device, which the limit does not cover.
1668+
cmd.env("LLVM_PROFILE_FILE", "/dev/null");
1669+
// SAFETY: only async-signal-safe libc calls between fork and exec.
1670+
unsafe {
1671+
cmd.pre_exec(move || {
1672+
libc::signal(libc::SIGXFSZ, libc::SIG_IGN);
1673+
let lim = libc::rlimit {
1674+
rlim_cur: limit as libc::rlim_t,
1675+
rlim_max: limit as libc::rlim_t,
1676+
};
1677+
if libc::setrlimit(libc::RLIMIT_FSIZE, &lim) != 0 {
1678+
return Err(std::io::Error::last_os_error());
1679+
}
1680+
Ok(())
1681+
});
1682+
}
1683+
}
1684+
cmd.output().expect("invoke vex")
1685+
};
1686+
1687+
let mut manifest = PatchManifest::new();
1688+
manifest.patches.insert(
1689+
"pkg:npm/x@1.0.0".to_string(),
1690+
make_record(
1691+
"11111111-1111-4111-8111-111111111111",
1692+
"package/index.js",
1693+
"a".repeat(64).as_str(),
1694+
"b".repeat(64).as_str(),
1695+
"GHSA-zzzz",
1696+
&["CVE-9999"],
1697+
),
1698+
);
1699+
write_manifest(cwd, &manifest);
1700+
assert!(run(None).status.success());
1701+
let first: Value = serde_json::from_slice(&std::fs::read(&vex_path).unwrap()).unwrap();
1702+
assert_eq!(first["statements"].as_array().unwrap().len(), 1);
1703+
1704+
for i in 0..30 {
1705+
manifest.patches.insert(
1706+
format!("pkg:npm/dep{i}@1.0.0"),
1707+
make_record(
1708+
&format!("22222222-2222-4222-8222-{i:012}"),
1709+
"package/index.js",
1710+
"c".repeat(64).as_str(),
1711+
"d".repeat(64).as_str(),
1712+
&format!("GHSA-dep{i}"),
1713+
&["CVE-2024-0001"],
1714+
),
1715+
);
1716+
}
1717+
write_manifest(cwd, &manifest);
1718+
let out = run(Some(512));
1719+
assert!(!out.status.success(), "the write must fail under the limit");
1720+
let env: Value = serde_json::from_slice(&out.stdout).expect("envelope JSON");
1721+
assert_eq!(env["error"]["code"], "write_failed", "{env}");
1722+
1723+
match std::fs::read(&vex_path) {
1724+
Err(e) => assert_eq!(e.kind(), std::io::ErrorKind::NotFound),
1725+
Ok(bytes) => panic!(
1726+
"a failed write left {} bytes at --output: {:?}",
1727+
bytes.len(),
1728+
String::from_utf8_lossy(&bytes[..bytes.len().min(80)])
1729+
),
1730+
}
1731+
let names: Vec<String> = std::fs::read_dir(cwd)
1732+
.unwrap()
1733+
.map(|e| e.unwrap().file_name().to_string_lossy().into_owned())
1734+
.filter(|n| n.starts_with(".socket-stage-"))
1735+
.collect();
1736+
assert!(names.is_empty(), "stage litter {names:?}");
1737+
}
1738+
15621739
/// Pipe the VEX text through `vexctl` if it's on `PATH`. CI installs
15631740
/// vexctl before the test step so the validation actually runs there;
15641741
/// local devs without Go see a skip message instead of a failure.

0 commit comments

Comments
 (0)