From fd100f0e958dea74487c729d29de0340cc822da6 Mon Sep 17 00:00:00 2001
From: l0ng-ai <24760907+l0ng-ai@users.noreply.github.com>
Date: Sat, 15 Aug 2026 17:43:15 +0800
Subject: [PATCH] refactor(duplex): own the dup'd descriptors, so a failed
 redirect cannot leak one
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

`StdioDuplex::take` dup'd stdin and stdout, redirected both to /dev/null, and
only then wrapped the raw descriptors in `File`. Any of the four `?`s between
the first `dup` and the wrap returned without closing what it already held —
a descriptor leak on the error path.

`dup_fd` now hands back an `OwnedFd`. `dup` returns a fresh descriptor nothing
else holds, which is exactly that type's contract, and ownership from the
moment it exists is what makes the early returns safe. `take` drops two
`unsafe` blocks in the process: `File::from(OwnedFd)` is the safe conversion.

Immaterial in production — `take` is called once in tty7-server's main and the
`?` there exits the process — but it is unsafe-adjacent code where the correct
version is also the shorter one.

Exercised by the 55 `stdio_conformance` tests, which drive
`tty7-server --stdio` through this constructor. 2934 pass.
---
 crates/tty7-core/src/daemon/duplex.rs | 18 ++++++++++++------
 1 file changed, 12 insertions(+), 6 deletions(-)

diff --git a/crates/tty7-core/src/daemon/duplex.rs b/crates/tty7-core/src/daemon/duplex.rs
index fe82780e..9b930c4d 100644
--- a/crates/tty7-core/src/daemon/duplex.rs
+++ b/crates/tty7-core/src/daemon/duplex.rs
@@ -66,15 +66,16 @@ pub struct StdioDuplex {
 #[cfg(unix)]
 impl StdioDuplex {
     pub fn take() -> io::Result<StdioDuplex> {
-        use std::os::fd::FromRawFd as _;
-
+        // Each descriptor is owned from the moment it exists, so the four `?`s
+        // below cannot strand one: a `redirect_to_null` that fails after both
+        // `dup`s used to return without closing either.
         let stdin_fd = dup_fd(libc::STDIN_FILENO)?;
         let stdout_fd = dup_fd(libc::STDOUT_FILENO)?;
         redirect_to_null(libc::STDIN_FILENO)?;
         redirect_to_null(libc::STDOUT_FILENO)?;
 
-        let read = unsafe { std::fs::File::from_raw_fd(stdin_fd) };
-        let write = unsafe { std::fs::File::from_raw_fd(stdout_fd) };
+        let read = std::fs::File::from(stdin_fd);
+        let write = std::fs::File::from(stdout_fd);
         Ok(StdioDuplex {
             read,
             write: StdioWriter {
@@ -147,13 +148,18 @@ impl LinkShutdown for StdioWriter {
     }
 }
 
+/// Hands back an *owned* descriptor rather than a raw one, so that a caller
+/// which gives up between one `dup` and the next drops it instead of leaking
+/// it. `dup` returns a fresh descriptor that nothing else holds, which is
+/// exactly the contract `OwnedFd` wants.
 #[cfg(unix)]
-fn dup_fd(fd: libc::c_int) -> io::Result<libc::c_int> {
+fn dup_fd(fd: libc::c_int) -> io::Result<std::os::fd::OwnedFd> {
+    use std::os::fd::FromRawFd as _;
     let new = unsafe { libc::dup(fd) };
     if new < 0 {
         return Err(io::Error::last_os_error());
     }
-    Ok(new)
+    Ok(unsafe { std::os::fd::OwnedFd::from_raw_fd(new) })
 }
 
 #[cfg(unix)]