fix(ext/node): support numeric FDs in child_process stdio array

Now that `fs.openSync` returns real OS file descriptors (#33039), numeric
values in child_process stdio arrays should be treated as raw fds rather
than Deno resource IDs. This re-lands the functionality from #32959
(reverted in #33017) with the correct fd-based approach.

- Rename `StdioOrRid` to `StdioOrFd`, replace `Rid(ResourceId)` with `Fd(i32)`
- `as_stdio()` dups the fd directly (Unix: `libc::dup`, Windows: handle clone)
  instead of looking up the resource table
- `extra_stdio` now accepts `StdioOrFd` so numeric fds work beyond stdin/stdout/stderr
- Hardcoded `Rid(1)`/`Rid(2)` for inherit become `Fd(1)`/`Fd(2)`

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Author: bartlomieju

ext/node/polyfills/internal/child_process.ts

@@ -748,7 +748,7 @@ function toDenoStdio(
     return "inherit";
   }
   if (typeof pipe === "number") {
-    /* Assume it's a rid returned by fs APIs */
+    /* Real OS file descriptor, e.g. from fs.openSync() */
     return pipe;
   }
 

ext/process/lib.rs

@@ -7,6 +7,8 @@ use std::collections::HashMap;
 use std::ffi::OsString;
 use std::io::Write;
 #[cfg(unix)]
+use std::os::unix::io::FromRawFd;
+#[cfg(unix)]
 use std::os::unix::prelude::ExitStatusExt;
 #[cfg(unix)]
 use std::os::unix::process::CommandExt;
@@ -35,7 +37,6 @@ use deno_io::ChildStderrResource;
 use deno_io::ChildStdinResource;
 use deno_io::ChildStdoutResource;
 use deno_io::IntoRawIoHandle;
-use deno_io::fs::FileResource;
 use deno_os::SignalError;
 use deno_permissions::PathQueryDescriptor;
 use deno_permissions::PermissionsContainer;
@@ -79,12 +80,12 @@ impl Stdio {
 }
 
 #[derive(Copy, Clone, Eq, PartialEq)]
-pub enum StdioOrRid {
+pub enum StdioOrFd {
   Stdio(Stdio),
-  Rid(ResourceId),
+  Fd(i32),
 }
 
-impl<'de> Deserialize<'de> for StdioOrRid {
+impl<'de> Deserialize<'de> for StdioOrFd {
   fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
   where
     D: serde::Deserializer<'de>,
@@ -93,47 +94,71 @@ impl<'de> Deserialize<'de> for StdioOrRid {
     let value = Value::deserialize(deserializer)?;
     match value {
       Value::String(val) => match val.as_str() {
-        "inherit" => Ok(StdioOrRid::Stdio(Stdio::Inherit)),
-        "piped" => Ok(StdioOrRid::Stdio(Stdio::Piped)),
-        "null" => Ok(StdioOrRid::Stdio(Stdio::Null)),
+        "inherit" => Ok(StdioOrFd::Stdio(Stdio::Inherit)),
+        "piped" => Ok(StdioOrFd::Stdio(Stdio::Piped)),
+        "null" => Ok(StdioOrFd::Stdio(Stdio::Null)),
         "ipc_for_internal_use" => {
-          Ok(StdioOrRid::Stdio(Stdio::IpcForInternalUse))
+          Ok(StdioOrFd::Stdio(Stdio::IpcForInternalUse))
         }
         val => Err(serde::de::Error::unknown_variant(
           val,
           &["inherit", "piped", "null"],
         )),
       },
-      Value::Number(val) => match val.as_u64() {
-        Some(val) if val <= ResourceId::MAX as u64 => {
-          Ok(StdioOrRid::Rid(val as ResourceId))
+      Value::Number(val) => match val.as_i64() {
+        Some(val) if val >= 0 && val <= i32::MAX as i64 => {
+          Ok(StdioOrFd::Fd(val as i32))
         }
-        _ => Err(serde::de::Error::custom("Expected a positive integer")),
+        _ => Err(serde::de::Error::custom(
+          "Expected a non-negative integer file descriptor",
+        )),
       },
       _ => Err(serde::de::Error::custom(
-        r#"Expected a resource id, "inherit", "piped", or "null""#,
+        r#"Expected a file descriptor, "inherit", "piped", or "null""#,
       )),
     }
   }
 }
 
-impl StdioOrRid {
-  pub fn as_stdio(
-    &self,
-    state: &mut OpState,
-  ) -> Result<StdStdio, ProcessError> {
+impl StdioOrFd {
+  pub fn as_stdio(&self) -> Result<StdStdio, ProcessError> {
     match &self {
-      StdioOrRid::Stdio(val) => Ok(val.as_stdio()),
-      StdioOrRid::Rid(rid) => {
-        Ok(FileResource::with_file(state, *rid, |file| {
-          file.as_stdio().map_err(deno_error::JsErrorBox::from_err)
-        })?)
+      StdioOrFd::Stdio(val) => Ok(val.as_stdio()),
+      StdioOrFd::Fd(fd) => {
+        #[cfg(unix)]
+        {
+          // Safety: we dup the fd so the original remains open for the caller
+          let new_fd = unsafe { libc::dup(*fd) };
+          if new_fd < 0 {
+            return Err(ProcessError::Io(std::io::Error::last_os_error()));
+          }
+          // Safety: new_fd is a valid, freshly duplicated file descriptor
+          Ok(unsafe {
+            StdStdio::from(std::os::unix::io::OwnedFd::from_raw_fd(new_fd))
+          })
+        }
+        #[cfg(windows)]
+        {
+          // Convert CRT file descriptor to OS handle, clone it, and wrap in Stdio
+          let handle = unsafe { libc::get_osfhandle(*fd as _) };
+          if handle == -1 {
+            return Err(ProcessError::Io(std::io::Error::last_os_error()));
+          }
+          let borrowed = unsafe {
+            std::os::windows::io::BorrowedHandle::borrow_raw(
+              handle as std::os::windows::io::RawHandle,
+            )
+          };
+          let owned =
+            borrowed.try_clone_to_owned().map_err(ProcessError::Io)?;
+          Ok(StdStdio::from(owned))
+        }
       }
     }
   }
 
   pub fn is_ipc(&self) -> bool {
-    matches!(self, StdioOrRid::Stdio(Stdio::IpcForInternalUse))
+    matches!(self, StdioOrFd::Stdio(Stdio::IpcForInternalUse))
   }
 }
 
@@ -248,7 +273,7 @@ pub struct SpawnArgs {
 
   input: Option<JsBuffer>,
 
-  extra_stdio: Vec<Stdio>,
+  extra_stdio: Vec<StdioOrFd>,
   detached: bool,
   needs_npm_process_state: bool,
   #[cfg(unix)]
@@ -349,9 +374,9 @@ pub enum ProcessError {
 #[derive(Deserialize)]
 #[serde(rename_all = "camelCase")]
 pub struct ChildStdio {
-  stdin: StdioOrRid,
-  stdout: StdioOrRid,
-  stderr: StdioOrRid,
+  stdin: StdioOrFd,
+  stdout: StdioOrFd,
+  stderr: StdioOrFd,
 }
 
 #[derive(ToV8)]
@@ -515,16 +540,16 @@ fn create_command(
   } else if args.input.is_some() {
     command.stdin(StdStdio::piped());
   } else {
-    command.stdin(args.stdio.stdin.as_stdio(state)?);
+    command.stdin(args.stdio.stdin.as_stdio()?);
   }
 
   command.stdout(match args.stdio.stdout {
-    StdioOrRid::Stdio(Stdio::Inherit) => StdioOrRid::Rid(1).as_stdio(state)?,
-    value => value.as_stdio(state)?,
+    StdioOrFd::Stdio(Stdio::Inherit) => StdioOrFd::Fd(1).as_stdio()?,
+    value => value.as_stdio()?,
   });
   command.stderr(match args.stdio.stderr {
-    StdioOrRid::Stdio(Stdio::Inherit) => StdioOrRid::Rid(2).as_stdio(state)?,
-    value => value.as_stdio(state)?,
+    StdioOrFd::Stdio(Stdio::Inherit) => StdioOrFd::Fd(2).as_stdio()?,
+    value => value.as_stdio()?,
   });
 
   #[cfg(unix)]
@@ -578,27 +603,34 @@ fn create_command(
     for (i, stdio) in args.extra_stdio.into_iter().enumerate() {
       // index 0 in `extra_stdio` actually refers to fd 3
       // because we handle stdin,stdout,stderr specially
-      let fd = (i + 3) as i32;
-      // TODO(nathanwhit): handle inherited, but this relies on the parent process having
-      // fds open already. since we don't generally support dealing with raw fds,
-      // we can't properly support this
-      if matches!(stdio, Stdio::Piped) {
-        let (fd1, fd2) = deno_io::bi_pipe_pair_raw()?;
-        fds_to_dup.push((fd2, fd));
-        fds_to_close.push(fd2);
-        let rid = state.resource_table.add(
-          match deno_io::BiPipeResource::from_raw_handle(fd1) {
-            Ok(v) => v,
-            Err(e) => {
-              log::warn!("Failed to open bidirectional pipe for fd {fd}: {e}");
-              extra_pipe_rids.push(None);
-              continue;
-            }
-          },
-        );
-        extra_pipe_rids.push(Some(rid));
-      } else {
-        extra_pipe_rids.push(None);
+      let target_fd = (i + 3) as i32;
+      match stdio {
+        StdioOrFd::Stdio(Stdio::Piped) => {
+          let (fd1, fd2) = deno_io::bi_pipe_pair_raw()?;
+          fds_to_dup.push((fd2, target_fd));
+          fds_to_close.push(fd2);
+          let rid = state.resource_table.add(
+            match deno_io::BiPipeResource::from_raw_handle(fd1) {
+              Ok(v) => v,
+              Err(e) => {
+                log::warn!(
+                  "Failed to open bidirectional pipe for fd {target_fd}: {e}"
+                );
+                extra_pipe_rids.push(None);
+                continue;
+              }
+            },
+          );
+          extra_pipe_rids.push(Some(rid));
+        }
+        StdioOrFd::Fd(fd) => {
+          // Dup the caller's fd onto the target fd slot in the child
+          fds_to_dup.push((fd, target_fd));
+          extra_pipe_rids.push(None);
+        }
+        _ => {
+          extra_pipe_rids.push(None);
+        }
       }
     }
 
@@ -670,29 +702,41 @@ fn create_command(
     for (i, stdio) in args.extra_stdio.into_iter().enumerate() {
       // index 0 in `extra_stdio` actually refers to fd 3
       // because we handle stdin,stdout,stderr specially
-      let fd = (i + 3) as i32;
-      // TODO(nathanwhit): handle inherited, but this relies on the parent process having
-      // fds open already. since we don't generally support dealing with raw fds,
-      // we can't properly support this
-      if matches!(stdio, Stdio::Piped) {
-        let (fd1, fd2) = deno_io::bi_pipe_pair_raw()?;
-        handles_to_close.push(fd2);
-        let rid = state.resource_table.add(
-          match deno_io::BiPipeResource::from_raw_handle(fd1) {
-            Ok(v) => v,
-            Err(e) => {
-              log::warn!("Failed to open bidirectional pipe for fd {fd}: {e}");
-              extra_pipe_rids.push(None);
-              continue;
-            }
-          },
-        );
-        command.extra_handle(Some(fd2));
-        extra_pipe_rids.push(Some(rid));
-      } else {
-        // no handle, push an empty handle so we need get the right fds for following handles
-        command.extra_handle(None);
-        extra_pipe_rids.push(None);
+      let target_fd = (i + 3) as i32;
+      match stdio {
+        StdioOrFd::Stdio(Stdio::Piped) => {
+          let (fd1, fd2) = deno_io::bi_pipe_pair_raw()?;
+          handles_to_close.push(fd2);
+          let rid = state.resource_table.add(
+            match deno_io::BiPipeResource::from_raw_handle(fd1) {
+              Ok(v) => v,
+              Err(e) => {
+                log::warn!(
+                  "Failed to open bidirectional pipe for fd {target_fd}: {e}"
+                );
+                extra_pipe_rids.push(None);
+                continue;
+              }
+            },
+          );
+          command.extra_handle(Some(fd2));
+          extra_pipe_rids.push(Some(rid));
+        }
+        StdioOrFd::Fd(fd) => {
+          // Convert CRT fd to OS handle and pass to child
+          let handle = unsafe { libc::get_osfhandle(fd as _) };
+          if handle != -1 {
+            command.extra_handle(Some(handle as _));
+          } else {
+            command.extra_handle(None);
+          }
+          extra_pipe_rids.push(None);
+        }
+        _ => {
+          // no handle, push an empty handle so we get the right fds for following handles
+          command.extra_handle(None);
+          extra_pipe_rids.push(None);
+        }
       }
     }
 
@@ -1116,8 +1160,8 @@ fn op_spawn_sync(
   state: &mut OpState,
   #[serde] args: SpawnArgs,
 ) -> Result<SpawnOutput, ProcessError> {
-  let stdout = matches!(args.stdio.stdout, StdioOrRid::Stdio(Stdio::Piped));
-  let stderr = matches!(args.stdio.stderr, StdioOrRid::Stdio(Stdio::Piped));
+  let stdout = matches!(args.stdio.stdout, StdioOrFd::Stdio(Stdio::Piped));
+  let stderr = matches!(args.stdio.stderr, StdioOrFd::Stdio(Stdio::Piped));
   let input = args.input.clone();
   let (mut command, _, _, _) =
     create_command(state, args, "Deno.Command().outputSync()")?;
@@ -1219,11 +1263,11 @@ mod deprecated {
     cwd: Option<String>,
     env: Vec<(String, String)>,
     #[from_v8(serde)]
-    stdin: StdioOrRid,
+    stdin: StdioOrFd,
     #[from_v8(serde)]
-    stdout: StdioOrRid,
+    stdout: StdioOrFd,
     #[from_v8(serde)]
-    stderr: StdioOrRid,
+    stderr: StdioOrFd,
   }
 
   struct ChildResource {
@@ -1296,20 +1340,20 @@ mod deprecated {
     }
 
     // TODO: make this work with other resources, eg. sockets
-    c.stdin(run_args.stdin.as_stdio(state)?);
+    c.stdin(run_args.stdin.as_stdio()?);
     c.stdout(
       match run_args.stdout {
-        StdioOrRid::Stdio(Stdio::Inherit) => StdioOrRid::Rid(1),
+        StdioOrFd::Stdio(Stdio::Inherit) => StdioOrFd::Fd(1),
         value => value,
       }
-      .as_stdio(state)?,
+      .as_stdio()?,
     );
     c.stderr(
       match run_args.stderr {
-        StdioOrRid::Stdio(Stdio::Inherit) => StdioOrRid::Rid(2),
+        StdioOrFd::Stdio(Stdio::Inherit) => StdioOrFd::Fd(2),
         value => value,
       }
-      .as_stdio(state)?,
+      .as_stdio()?,
     );
 
     // We want to kill child when it's closed

tests/unit_node/child_process_test.ts

@@ -1365,3 +1365,26 @@ Deno.test(function spawnSyncReturnsPid() {
   assertEquals(typeof ret.pid, "number");
   assert(ret.pid > 0);
 });
+
+Deno.test(async function spawnWithNumericFdInStdioArray() {
+  const fs = await import("node:fs");
+  const tmpFile = Deno.makeTempFileSync();
+  try {
+    const fd = fs.openSync(tmpFile, "w");
+    const child = spawn(Deno.execPath(), [
+      "eval",
+      "/* child just exits successfully */",
+    ], {
+      stdio: ["ignore", "pipe", "pipe", fd],
+    });
+    const { promise, resolve } = Promise.withResolvers<void>();
+    child.on("close", (code: number) => {
+      assertEquals(code, 0);
+      resolve();
+    });
+    await promise;
+    fs.closeSync(fd);
+  } finally {
+    Deno.removeSync(tmpFile);
+  }
+});