sys(windows): create dup() handles non-inheritable so children stop inheriting them

[robobun]

Problem

On Windows, every handle returned by bun_sys::dup() was created inheritable (DuplicateHandle(..., bInheritHandle = TRUE, ...)). libuv's uv_spawn calls CreateProcessW with bInheritHandles = TRUE and no handle list (vendor/libuv/src/win/process.c), so any process Bun spawned while such a duplicate was open received a copy of it, and kept the underlying file / pipe / directory open after Bun itself had closed the duplicate, for as long as the child lived.

dup() backs Bun.file(fd).stream() (FileReader.rs), a fetch() body made from Bun.file(fd), FileSink started on an fd (io/openForWriting.rs), and the shell: its duplicates of stdin/stdout/stderr and of the cwd directory handle for every subshell / pipeline (shell/interpreter.rs, shell_dup / dupe_for_subshell). The POSIX arm of the same function uses fcntl(F_DUPFD_CLOEXEC) precisely so this does not happen.

Measured on Windows x64 with the released 1.4.0-canary (da3851e): a bun -e child that prints its own GetProcessHandleCount(), spawned five times each way while the parent held 64 open fds:

parent holds 64 plain fs.openSync() fds:                  164 164 164 164 164
same, plus Bun.file(fd).stream().getReader() on each one:  228 228 228 228 228   (+64, one per dup())

Fix

src/sys/lib.rs, Windows dup(): pass FALSE for bInheritHandle. Nothing relies on these duplicates being inheritable:

• When an fd is handed to a child as stdio, libuv duplicates it again itself with bInheritHandle = TRUE (uv__duplicate_handle in vendor/libuv/src/win/process-stdio.c), regardless of the source handle's flag. This covers Bun.spawn / child_process (UV_INHERIT_FD in spawn/process.rs) and every shell command, since the shell spawns through the same path.
• Bun's other process-creation paths (--watch manager, the bin shim, the crash reporter, std::process::Command in bun_core::util) do not involve dup() at all; the first two hand the child the process's real std handles and set those inheritable themselves.
• Every other handle Bun opens on Windows (NtCreateFile paths, libuv's uv_fs_open) is already non-inheritable, which is what the 164 / 164 control above shows. dup() was the only DuplicateHandle call in the tree.

The POSIX arm is unchanged; it already has the CLOEXEC behaviour this gives Windows.

Verification

New test in test/js/bun/spawn/spawn.test.ts (Windows only, since the POSIX dup() is already close-on-exec): opens 64 fds, spawns a control child, starts Bun.file(fd).stream() on each fd, checks with GetProcessHandleCount that this process now holds at least 64 more handles, spawns a second child, and asserts that the second child did not start with (about) 64 more handles than the first.

• Windows x64, released canary (unfixed): fails with Expected: < 32, Received: 64.
• Windows x64, debug build of this branch: passes; the same probe as above reads 171 / 171. The rest of spawn.test.ts passes there as well (125 pass, 19 platform skips, 3 todo).
• Also checked by hand on the debug build that each Windows dup() caller still works: Bun.file(fd).stream().text(), fetch() with a Bun.file(fd) body, Bun.spawn with an fd as stdout, and shell commands with inherited stdio, pipelines and subshells.
• On POSIX the test is skipped and the compiled code is unchanged.

The same function is touched by #37516 (EBADF for a descriptor that is not open); the two changes are on different lines and git merge-tree reports a clean merge either way.

[coderabbitai]

Warning

Review limit reached

@robobun, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 6 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 1e10b01a-5851-41c5-8117-81f40a4e46d2

📥 Commits

Reviewing files that changed from the base of the PR and between da3851e and 589b977.

📒 Files selected for processing (2)

• src/sys/lib.rs
• test/js/bun/spawn/spawn.test.ts

---

_{Comment @coderabbitai help to get the list of available commands.}

[robobun]

Status: reproduced and fixed, waiting on CI.

Reproduced on Windows x64 with the released 1.4.0-canary (da3851e): a child spawned while 64 Bun.file(fd).stream() readers were open started with exactly 64 more handles than a child spawned before they existed (228 vs 164); the new test in test/js/bun/spawn/spawn.test.ts fails there with Expected: < 32, Received: 64. With this branch's debug build the same child count is unchanged (171 vs 171) and the test passes, along with the rest of spawn.test.ts on Windows.

The test is Windows only: the POSIX dup() already uses F_DUPFD_CLOEXEC, so there is nothing to fail there, and the changed code is inside the Windows cfg arm.

[claude]

LGTM — one-bool flip aligning Windows dup() with the POSIX arm's F_DUPFD_CLOEXEC.

What was reviewed

• Confirmed the POSIX arm at src/sys/lib.rs:2624 already uses F_DUPFD_CLOEXEC; this makes Windows match.
• Checked dup() callers (FileReader, fetch body, FileSink, shell stdio/cwd, Terminal) — none rely on the duplicate itself being inheritable; stdio handed to children goes through libuv's UV_INHERIT_FD, which re-duplicates inheritable.
• Test: dlopen("kernel32.dll") is inside the .if(isWindows) body so the top-level bun:ffi import is inert on POSIX; readers/fds are released in finally; the differential assertion (< N/2) against a control child avoids absolute-count flakiness.

Extended reasoning...

Overview

Single functional change: src/sys/lib.rs Windows dup() now passes bInheritHandle = FALSE to DuplicateHandle instead of TRUE, plus a doc comment explaining why. A Windows-only regression test in test/js/bun/spawn/spawn.test.ts opens 64 fds, starts Bun.file(fd).stream() on each (which dup()s), and uses GetProcessHandleCount via bun:ffi to assert a child spawned while those duplicates are open does not inherit ~64 extra handles compared to a control child.

Security risks

None introduced. This removes unintended handle inheritance into child processes, which is strictly a reduction in exposure. No new attack surface, no user-controlled input reaches the changed line.

Level of scrutiny

Low-to-medium. The runtime change is a one-line constant flip in a platform-gated function whose POSIX twin already has the equivalent semantics (F_DUPFD_CLOEXEC). The PR description enumerates every dup() caller and every process-creation path and explains why none need the inheritable flag on the source handle — libuv's uv__duplicate_handle re-duplicates stdio as inheritable regardless, so UV_INHERIT_FD still works. I spot-checked the caller list with grep and it matches. src/sys/lib.rs shows this is the only DuplicateHandle call in src/.

Other factors

The test follows harness conventions: tempDir with using, bunEnv, concurrent drain of stdout/stderr/exited, cleanup in finally registered before assertions, await using on subprocesses. It asserts its own precondition (ownHandleCount() - before >= N) so it can't pass vacuously if stream() stops dup()ing. The < N/2 differential margin is wide enough to tolerate a few incidental handles without admitting the +64 regression. The top-level import { dlopen } from "bun:ffi" is safe on POSIX because the dlopen("kernel32.dll", ...) call itself is inside the .if(isWindows) body. PR description reports the test fails on unfixed canary and passes on the debug build, and that the rest of spawn.test.ts still passes on Windows.