Skip to content

Commit f66256e

Browse files
committed
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
1 parent 8b136ec commit f66256e

2 files changed

Lines changed: 168 additions & 2 deletions

File tree

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

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1559,6 +1559,81 @@ 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+
15621637
/// A write that fails part-way (here `EFBIG` from `RLIMIT_FSIZE`, the
15631638
/// same short write a full disk gives) never leaves a prefix of the new
15641639
/// document at `--output`. The document is staged beside the destination

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

Lines changed: 93 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -465,10 +465,16 @@ pub async fn atomic_write_bytes_preserving_mode(
465465
/// already exists (`/dev/stdout`, a FIFO, a character device) has no inode to
466466
/// swap, so it is written in place: on Windows a DOS device name (`NUL`,
467467
/// `CON`, `COM1`) or a `\\.\` device path such as a named pipe, which std
468-
/// reports as a file. Never captured by an open group commit: the path is
469-
/// the user's output, not one of the run's commit points.
468+
/// reports as a file. A path that is, or links through, `/dev/…` or
469+
/// `/proc/…` (`/dev/stdout`, `/dev/fd/1`, `/proc/self/fd/1`) is a stream
470+
/// too, even when the descriptor behind it is a regular file
471+
/// (`--output /dev/stdout > vex.json`): renaming over its target would
472+
/// detach the caller's open descriptor, and on macOS `/dev/fd` holds no
473+
/// stage. Never captured by an open group commit: the path is the user's
474+
/// output, not one of the run's commit points.
470475
pub async fn write_user_output(path: &Path, content: &[u8]) -> std::io::Result<()> {
471476
let special = (cfg!(windows) && is_windows_device_path(path))
477+
|| links_through_dev_or_proc(path)
472478
|| matches!(
473479
tokio::fs::metadata(path).await,
474480
Ok(meta) if !meta.is_file() && !meta.is_dir()
@@ -490,6 +496,30 @@ pub async fn write_user_output(path: &Path, content: &[u8]) -> std::io::Result<(
490496
.await
491497
}
492498

499+
/// Whether `path`, or any hop of the symlink chain its final component
500+
/// starts, names something under `/dev/` or `/proc/`. The chain is read link
501+
/// by link, never canonicalized: `/proc/self/fd/1` resolves to the regular
502+
/// file behind the descriptor, which is exactly what must not be renamed
503+
/// over.
504+
fn links_through_dev_or_proc(path: &Path) -> bool {
505+
let under = |p: &Path| p.starts_with("/dev") || p.starts_with("/proc");
506+
let mut hop = path.to_path_buf();
507+
// The kernel's own loop limit for symlink resolution.
508+
for _ in 0..40 {
509+
if under(&hop) {
510+
return true;
511+
}
512+
let Ok(target) = std::fs::read_link(&hop) else {
513+
return false;
514+
};
515+
hop = match hop.parent() {
516+
Some(dir) if target.is_relative() => dir.join(target),
517+
_ => target,
518+
};
519+
}
520+
false
521+
}
522+
493523
/// Whether Windows resolves `path` to a device rather than a file: the
494524
/// `\\.\` device namespace (named pipes, `\\.\COM1`), or a final
495525
/// component whose name before the first dot, trailing spaces dropped, is a
@@ -1100,6 +1130,67 @@ mod tests {
11001130
assert_eq!(std::fs::read(&fresh).unwrap(), b"doc");
11011131
}
11021132

1133+
/// `--output /dev/stdout > vex.json`: the descriptor behind a `/dev` or
1134+
/// `/proc` path is written in place even when it is a regular file, so
1135+
/// the caller's open file keeps the bytes; a user's link to such a path
1136+
/// is a stream too (review on #1262).
1137+
#[cfg(target_os = "linux")]
1138+
#[tokio::test]
1139+
async fn user_output_writes_descriptor_paths_in_place() {
1140+
use std::io::{Read as _, Seek as _};
1141+
use std::os::fd::AsRawFd as _;
1142+
let tmp = tempfile::tempdir().unwrap();
1143+
let captured = tmp.path().join("captured.json");
1144+
let mut file = std::fs::OpenOptions::new()
1145+
.read(true)
1146+
.write(true)
1147+
.create(true)
1148+
.truncate(true)
1149+
.open(&captured)
1150+
.unwrap();
1151+
let fd_path = PathBuf::from(format!("/proc/self/fd/{}", file.as_raw_fd()));
1152+
assert!(links_through_dev_or_proc(&fd_path));
1153+
write_user_output(&fd_path, b"doc").await.unwrap();
1154+
let mut seen = String::new();
1155+
file.rewind().unwrap();
1156+
file.read_to_string(&mut seen).unwrap();
1157+
assert_eq!(seen, "doc", "the open descriptor must see the bytes");
1158+
1159+
let link = tmp.path().join("out.vex.json");
1160+
std::os::unix::fs::symlink(&fd_path, &link).unwrap();
1161+
assert!(links_through_dev_or_proc(&link));
1162+
write_user_output(&link, b"doc2").await.unwrap();
1163+
assert!(std::fs::symlink_metadata(&link)
1164+
.unwrap()
1165+
.file_type()
1166+
.is_symlink());
1167+
seen.clear();
1168+
file.rewind().unwrap();
1169+
file.read_to_string(&mut seen).unwrap();
1170+
assert_eq!(seen, "doc2");
1171+
let names = std::fs::read_dir(tmp.path()).unwrap().count();
1172+
assert_eq!(names, 2, "no stage left beside the captured file");
1173+
}
1174+
1175+
#[cfg(unix)]
1176+
#[test]
1177+
fn dev_and_proc_paths_are_streams() {
1178+
for stream in ["/dev/stdout", "/dev/stderr", "/dev/fd/1", "/proc/self/fd/1"] {
1179+
assert!(links_through_dev_or_proc(Path::new(stream)), "{stream}");
1180+
}
1181+
let tmp = tempfile::tempdir().unwrap();
1182+
let file = tmp.path().join("vex.json");
1183+
std::fs::write(&file, b"{}").unwrap();
1184+
assert!(!links_through_dev_or_proc(&file));
1185+
assert!(!links_through_dev_or_proc(&tmp.path().join("missing.json")));
1186+
let relative = tmp.path().join("rel.json");
1187+
std::os::unix::fs::symlink("vex.json", &relative).unwrap();
1188+
assert!(!links_through_dev_or_proc(&relative));
1189+
let looped = tmp.path().join("loop");
1190+
std::os::unix::fs::symlink("loop", &looped).unwrap();
1191+
assert!(!links_through_dev_or_proc(&looped));
1192+
}
1193+
11031194
/// Windows device names and the `\\.\` namespace are written in place
11041195
/// (Bugbot on #1262: std reports `NUL` and named pipes as files there).
11051196
#[test]

0 commit comments

Comments
 (0)