child_process: accept stdio entries that carry an fd (server._handle, { fd }) by robobun · Pull Request #39220 · oven-sh/bun

robobun · 2026-08-15T21:15:47Z

Problem

spawn() / spawnSync() reject a stdio entry that is an object carrying a descriptor. Passing a listening server's handle (the way Node hands a listen socket to a child as fd 3, used by its test-listen-fd-* tests) throws:
```
error: Invalid stdio option[3] "[object Listener]"
    at nodeToBun (node:child_process:937:64)
    at spawn (node:child_process:696:142)
```
A FileHandle fails the same way with a different message (Passing a stream.Writable without an underlying file descriptor as stdio[1] is not yet implemented), and so does a plain { fd } object. Only the numeric spelling (server._handle.fd) and streams (net.Socket, process.stdout, fs streams) work today.
Cause: nodeToBun() in src/js/node/child_process.ts translates numbers, node streams (via streamFdOf, which only looks at an own fd property, _handle.fd and the FileSink fast path) and the 'pipe'/'ignore'/... string table. Nothing reads fd off other objects, and FileHandle (an EventEmitter with write()) is claimed by the stream check before anything looks at its fd getter.
The two vendored tests for this, test-listen-fd-detached.js and test-listen-fd-detached-inherit.js, have been passing without testing anything: the spawn throws inside the intermediate "parent" process, the outer test's stdout callback never runs, and the file exits 0 with nothing left on the event loop.

Fix

nodeToBun(): before the stream check, any object whose fd is a number is translated to that number, which Bun.spawn dup2()s onto the slot. streamFdOf loses its now redundant own-property fd check; its _handle.fd and FileSink lookups are unchanged and still run for streams with no fd of their own (net.Socket, child.stdin).
Why this is right: it is Node's rule and Node's order. getValidStdio takes stdio.fd from any entry before it looks at handle wraps (lib/internal/child_process.js#L1058), and that branch is what server._handle hits in Node too, because a TCP wrap has a numeric fd getter. Bun's Listener, TCPSocket/TLSSocket, UDP handles and FileHandle expose fd the same way, so one clause covers all of them plus { fd }. Every Bun stream with a numeric fd (fs and tty streams, process.std*) exposes the real descriptor, so reading it first changes nothing for them; a stream whose fd is null/absent falls through to the existing stream logic as before.
The value is passed through unchanged, so the object spelling behaves exactly like writing .fd yourself. That includes a closed handle: its fd is -1, which Bun.spawn refuses (spawn() throws, spawnSync() returns error), the same as a bare -1 today. This is a deliberate difference from Node, where libuv spawns the child with the slot left closed for the negative errno a closed wrap reports (it only raises EINVAL for exactly -1); silently dropping the socket is not worth reproducing, and the test documents it.
Not changed: a bare net.Server / dgram.Socket (no fd of its own) is still rejected as before. Node rejects them too (spawn() throws stream._stdio.pause is not a function, spawnSync() aborts on the wrap type; output in details). net/cluster/child_process: IPC handle delivery, listen({fd})+SCM_RIGHTS, socket_list, cluster listen semantics (+15 tests) #34659 proposes additionally reading _handle.fd off such objects; this PR neither adds nor pins that, so either outcome there is fine. The rejection error is still the existing Invalid stdio option[i] Error (child_process: throw ERR_INVALID_ARG_VALUE for invalid stdio values #35226 is switching those to ERR_INVALID_ARG_VALUE; the tests here only match /stdio/).
Vendored test-listen-fd-detached{,-inherit}.js are left in the tree and listed in test/expectations.txt. Once the spawn works, their child runs http.createServer().listen({ fd: 3 }), and Bun's http.Server#listen ignores fd (it is built on Bun.serve, which has no fd option), so the child listens on a fresh port, the outer request to the inherited port hangs until the runner kills the file, and the detached child is orphaned holding the port (verified, details below). The entry says to drop it when http listen({ fd }) lands (socket: support listening on an inherited fd (socket activation) #36491 or net/cluster/child_process: IPC handle delivery, listen({fd})+SCM_RIGHTS, socket_list, cluster listen semantics (+15 tests) #34659); net/cluster/child_process: IPC handle delivery, listen({fd})+SCM_RIGHTS, socket_list, cluster listen semantics (+15 tests) #34659 also touches these two files, so leaving them untouched here keeps that rebase clean.
Tests: test/js/node/child_process/child-process-stdio.test.js, new describe("stdio entries carrying a file descriptor"):
- spawn with server._handle at stdio[3]: child does net.createServer().listen({ fd: 3 }), the parent closes its own copy and gets the child's reply through the original port; child.stdio[3] is null as in Node.
- spawnSync with a Listener at 3 and a connected TCPSocket handle at 4: child reports fstatSync(3|4).isSocket() as [true,true].
- { fd } and a FileHandle as stdout (these two also run on Windows; the socket cases are POSIX only, sockets cannot be inherited as stdio on Windows in Node either).
- {} and { fd: "3" } are still rejected synchronously by both spawn and spawnSync.
- A closed listener's handle fails with the same error as passing its fd directly.
- Released build: 5 failures, this branch: 14/14. Each scenario gives the same result under node v26.3.0 except the closed-handle one described above (script output in details).
Also ran with this change: child_process.test.ts, child_process_ipc_handle.test.ts, child_process-node.test.js, test/regression/issue/20321.test.ts (process streams as stdio) and the vendored test-stdio-pipe-stderr.js, test-fs-syncwritestream.js, test-child-process-{stdio,validate-stdio,bad-stdio,stdio-inherit,stdio-overlapped,fork-stdio-string-variant,flush-stdio}.js, test-net-listen-fd0.js, test-listen-fd-ebadf.js: no change in results.
Relation to net/cluster/child_process: IPC handle delivery, listen({fd})+SCM_RIGHTS, socket_list, cluster listen semantics (+15 tests) #34659: that branch (IPC handle delivery, cluster, Bun.serve({ fd })) carries an equivalent of this clause among its 148 files and is currently conflicting with main. This is the spawn-side slice on its own; whichever lands second gets a small conflict inside nodeToBun().

Background

Passing a listening socket as stdio: the parent owns a bound, listening socket; spawn duplicates its descriptor onto slot N of the child, and the child calls listen({ fd: N }) to accept on the parent's port. After the parent closes its copy, the child's copy keeps the kernel socket alive. This is plain descriptor inheritance at spawn time; it is unrelated to IPC handle passing (child.send(msg, handle) over the channel), which is a separate mechanism and already works.
server._handle: in Node, the TCP wrap behind a net.Server. In Bun, the Listener returned by Bun.listen(), whose fd getter returns the listening descriptor, or -1 once stopped. net.Socket#_handle is likewise Bun's TCPSocket, which also has fd; fs.promises FileHandle has an fd getter as in Node.
nodeToBun(): node:child_process maps each entry of Node's stdio array to what Bun.spawn accepts; a plain number there means "dup this parent fd onto the slot", which is why returning the object's fd is all that is needed.
test/expectations.txt: the CI runner's list of files to leave out of the run entirely; reserved for files that hang or crash the runner, which is what these two would do now.

Node v26.3.0 vs this branch on the tested scenarios

Same script run under both runtimes (server listening on 127.0.0.1, one connected client):

node v26.3.0
spawnSync [server._handle, client._handle] -> child fstat isSocket: ok -> [true,true] status=0
spawn server._handle -> child.stdio[3]: ok -> null
spawnSync { fd } as stdout: ok -> "written through the shared fd\n" status=0
spawnSync FileHandle as stdout: ok -> "written through the shared fd\n" status=0
spawn with {}: threw TypeError ERR_INVALID_ARG_VALUE "The argument 'stdio' is invalid. Received {}"
spawnSync with {}: threw TypeError ERR_INVALID_ARG_VALUE "The argument 'stdio' is invalid. Received {}"
spawn with { fd: "3" }: threw TypeError ERR_INVALID_ARG_VALUE "The argument 'stdio' is invalid. Received { fd: '3' }"
spawnSync with { fd: "3" }: threw TypeError ERR_INVALID_ARG_VALUE "The argument 'stdio' is invalid. Received { fd: '3' }"
closed listener handle .fd = -22
spawn with the closed listener's handle: ok                      (child spawned, slot 3 left closed)
spawnSync with the closed listener's handle: ok -> returned error: none

bun (this branch)
spawnSync [server._handle, client._handle] -> child fstat isSocket: ok -> [true,true] status=0
spawn server._handle -> child.stdio[3]: ok -> null
spawnSync { fd } as stdout: ok -> "written through the shared fd\n" status=0
spawnSync FileHandle as stdout: ok -> "written through the shared fd\n" status=0
spawn with {}: threw Error "Invalid stdio option[3] \"[object Object]\""
spawnSync with {}: threw Error "Invalid stdio option[3] \"[object Object]\""
spawn with { fd: "3" }: threw Error "Invalid stdio option[3] \"[object Object]\""
spawnSync with { fd: "3" }: threw Error "Invalid stdio option[3] \"[object Object]\""
closed listener handle .fd = -1
spawn with the closed listener's handle: threw TypeError ERR_INVALID_ARG_TYPE "file descriptor must be a positive integer"
spawnSync with the closed listener's handle: ok -> returned error: TypeError ERR_INVALID_ARG_TYPE "file descriptor must be a positive integer"

A bare listening net.Server as the entry, under node:

spawn:      TypeError: stream._stdio.pause is not a function
spawnSync:  invalid child stdio type: wrap
            node::SyncProcessRunner::ParseStdioOption(int, Local<Object>) at ../src/spawn_sync.cc:1072
            Assertion failed: "Unreachable code reached"        (process aborted)

Bun (before and after): Error: Invalid stdio option[3] "[object Object]" from both.

Why the two vendored files are quarantined: behavior with this fix applied

Released build: both files exit 0 in well under 100 ms after printing error: Invalid stdio option[3] "[object Listener]" from the intermediate process.

Debug build with this change, each file run directly with a 20 s limit:

===== test-listen-fd-detached.js =====
exit=124 after 20s (killed by timeout)
server listening on 36963
output from parent = {"pid":16407,"port":36963}
ORPHAN: detached child pid 16407 still alive after the test was killed

===== test-listen-fd-detached-inherit.js =====
exit=124 after 20s (killed by timeout)
server listening on 37185
output from parent = {"pid":16467,"port":37185}
child listening on fd=3
ORPHAN: detached child pid 16467 still alive after the test was killed

The child logs "listening on fd=3" but http.Server#listen({ fd }) actually bound a new port 0 socket; the inherited fd 3 is open in the child but never accepted from, so the outer http.get to the original port connects into the backlog and waits forever. Both files pass under node.

Earlier revision

The first push deleted the two vendored files instead of quarantining them, checked fd only after the stream branch (so FileHandle was still rejected), and pinned the rejection of a bare net.Server with a test. Review pointed out the FileHandle gap, that #34659 modifies both vendored files, and that the net.Server test pinned a policy #34659 is proposing to change; this revision addresses all three.

no test proof · iteration 0 · Platform-specific test(s) that do not run on this machine. Deferring to CI, which covers all platforms: test/js/node/child_process/child-process-stdio.test.js

… { fd }) nodeToBun() only translated numbers, streams and the string table, so spawning with a listening server's handle in stdio (the way Node hands a listen socket to a child as fd 3) threw `Invalid stdio option[3]`. Node's getValidStdio shares the descriptor of any entry with a numeric `fd` property; Bun's Listener, TCPSocket and UDP handles all expose one, so do the same for non-stream objects. Remove test-listen-fd-detached{,-inherit}.js: until now their grandchild died on this error and the files exited 0 without exercising anything. With spawn working, their child runs http.Server#listen({ fd: 3 }), which Bun still ignores (it listens on a fresh port), so the request to the inherited port hangs the test and leaks a detached child. They can come back once http listen({ fd }) is implemented.

coderabbitai · 2026-08-15T21:15:56Z

Warning

Review limit reached

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

Next review available in: 26 seconds

Limit details: You’ve used all 5 included reviews currently available under your plan.

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: 89263058-7576-4cbe-841c-bd714fc81290

📥 Commits

Reviewing files that changed from the base of the PR and between bcba472 and 77ec2fb.

📒 Files selected for processing (3)

src/js/node/child_process.ts
test/expectations.txt
test/js/node/child_process/child-process-stdio.test.js

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

robobun · 2026-08-15T21:15:56Z

^{Updated 8:56 PM PT - Aug 15th, 2026}

✅ @robobun, your commit 77ec2fb8b11f9a8d6832cf3c5bcd099dad1e7728 passed in Build #99053! 🎉

🧪 To try this PR locally:

bunx bun-pr 39220

That installs a local version of the PR into your bun-39220 executable, so you can run:

bun-39220 --bun

robobun · 2026-08-15T21:16:44Z

Status

Reproduced on the released build: cd test/js/node/test/parallel && bun test-listen-fd-detached.js prints error: Invalid stdio option[3] "[object Listener]" from the intermediate process and still exits 0; same for test-listen-fd-detached-inherit.js. Both files pass under node v26.3.0.
Fail-before / pass-after: test/js/node/child_process/child-process-stdio.test.js has 5 failures on the released build (server._handle via spawn and spawnSync, { fd }, FileHandle, closed handle) and passes 14/14 with this branch.
After review, the current revision reads fd before the stream check (so FileHandle works too, as in Node), leaves the two vendored files in the tree with a test/expectations.txt entry instead of deleting them (they would hang on http listen({ fd }), which socket: support listening on an inherited fd (socket activation) #36491 / net/cluster/child_process: IPC handle delivery, listen({fd})+SCM_RIGHTS, socket_list, cluster listen semantics (+15 tests) #34659 implement; net/cluster/child_process: IPC handle delivery, listen({fd})+SCM_RIGHTS, socket_list, cluster listen semantics (+15 tests) #34659 also edits those files), and no longer pins the rejection of a bare net.Server.
Waiting on CI.

claude

I reviewed this PR and didn't find any bugs. The 4-line nodeToBun() addition matches Node's getValidStdio typeof stdio.fd === 'number' branch exactly, is placed after the stream check so existing stream handling is unaffected, and child.stdio[i] correctly resolves to null for the new fd entries via #createStdioObject. Because the PR also deletes two vendored Node parallel tests (with a well-argued rationale and replacement coverage, but still a maintainer call on delete-vs-quarantine), a quick human sign-off would be worthwhile.

What was reviewed:

Branch ordering in nodeToBun: streams still route through streamFdOf; only non-stream objects reach the new .fd check.
net.Server has no own .fd (only ._handle.fd), so it still falls through to the rejection path as the test asserts.
Objects without a numeric fd fall through to nodeToBunLookup[item] and throw as before — no widening of accepted inputs beyond Node's rule.
Leftover entries in test/expected-durations.json for the deleted files are benign duration cache entries.

Extended reasoning...

Overview

This PR adds one branch to nodeToBun() in src/js/node/child_process.ts so that any stdio entry which is an object with a numeric .fd is passed to Bun.spawn as that fd number. This is Node's documented behavior (getValidStdio's typeof stdio.fd === 'number' clause) and is what makes stdio: [..., server._handle] work. Comment-only cleanups update the getBunStdioFromOptions translation table and remove a stale TODO in normalizeStdio. Five new tests are added to child-process-stdio.test.js, and two vendored Node tests (test-listen-fd-detached{,-inherit}.js) are removed because, once the spawn stops throwing, they hang and orphan detached children (Bun's http.Server#listen({ fd }) is not implemented).

Security risks

None identified. The change reads a numeric .fd off a caller-supplied object and hands the number to Bun.spawn's existing fd path, which already handles arbitrary user-supplied fd numbers today (the typeof item === "number" branch just above). No new syscalls, no escaping, no path handling.

Level of scrutiny

Medium. The production change is 4 lines in a Node-compat JS builtin, deliberately mirroring Node's own condition with a source link, placed after the existing stream branch so stream handling is not shadowed. It is straightforward to verify by reading. The higher-scrutiny piece is the deletion of two vendored Node parallel tests: the justification is thorough (they were vacuously passing before and would hang/orphan after), replacement coverage exists using net instead of http, and precedent (#36780) is cited — but per REVIEW.md every test deletion needs a stated reason or maintainer confirmation, and delete-vs-skip for vendored Node tests is the kind of call a maintainer typically makes.

Other factors

The new tests await observable conditions (server 'close', child stdout containing "listening", socket 'end'), wire error/exit to reject, use tempDir + try/finally cleanup, and skip socket cases on Windows with the reason stated in the describe comment — they meet the harness conventions.
I checked that net.Server in Bun does not expose a top-level .fd, so a bare server still rejects (matches Node, and the new test pins it).
test/expected-durations.json still lists the deleted files; that is a duration cache, not a manifest, so it should not break anything.
No prior human review comments on the timeline; CI is still building.

github-actions · 2026-08-15T21:32:23Z

This PR may be a duplicate of:

net/cluster/child_process: IPC handle delivery, listen({fd})+SCM_RIGHTS, socket_list, cluster listen semantics (+15 tests) #34659 - Adds the same nodeToBun() clause in src/js/node/child_process.ts for the same purpose (accepting server._handle / socket._handle / { fd } as stdio entries), and touches the same two vendored test-listen-fd-detached*.js tests this PR deletes.

🤖 Generated with Claude Code

robobun · 2026-08-15T21:33:11Z

Not a duplicate in practice, though the overlap is real and is noted in the description. #34659 is the IPC handle delivery / cluster / Bun.serve({ fd }) branch (148 files, currently conflicting with main); the nodeToBun() clause there is a few incidental lines of it. This PR is just that prerequisite on its own, so the server._handle spawn path can land independently of the native http listen({ fd }) work. If #34659 lands first, this one becomes unnecessary and can be closed; if this lands first, #34659 picks up a trivial conflict in nodeToBun() and re-adds the two test-listen-fd-detached*.js files it already modifies (they only start passing once its http listen({ fd }) exists).

…sten-fd tests instead of deleting Read `.fd` off any object before the stream branch, in node's order, so a FileHandle (an EventEmitter with write(), which the stream check claims) shares its descriptor too; streamFdOf's own-property fd check is now redundant and goes away. The value is passed through unchanged, so a closed handle behaves exactly like passing its fd number directly. Restore test-listen-fd-detached{,-inherit}.js and list them in test/expectations.txt: with the spawn working they hang on the child's http listen({ fd: 3 }), which still ignores the fd, so they stay out of the run until that lands rather than being removed from the tree.

claude

I reviewed this and didn't find any bugs. The 4-line nodeToBun addition cleanly mirrors Node's getValidStdio fd branch, and the removed own-property lookup in streamFdOf is provably subsumed by it. That said, a human look is still worthwhile: this changes spawn stdio parsing (ordering vs the stream check now lets .fd getters win), quarantines two vendored Node tests, and overlaps with #34659 in a way where merge order matters.

What was reviewed:

Confirmed the new item.fd check subsumes the removed ObjectHasOwn(item, "fd") path in streamFdOf (own-property → any numeric fd, which is the intended widening for FileHandle/Listener); streams without fd still fall through to _handle.fd / FileSink.
Checked that item == null and typeof item === "number" are handled before the new object branch, and strings still reach nodeToBunLookup.
Test structure: failure events wired to reject, skipIf(isWindows) on the socket-inheritance cases, cleanup in finally, rejection cases cover both spawn/spawnSync.
expectations.txt entry matches the file's own guidance (both tests would now hang and orphan a detached child).

Extended reasoning...

Overview

The PR adds one clause to nodeToBun() in src/js/node/child_process.ts so that any stdio entry object with a numeric fd property is passed to Bun.spawn as that number, matching Node's getValidStdio (cited by permalink). It removes the now-redundant own-property fd lookup from streamFdOf, updates two doc comments, quarantines test-listen-fd-detached{,-inherit}.js in test/expectations.txt (they would now hang on http.Server#listen({ fd }), which Bun does not implement yet), and adds a describe block with ~170 lines of tests covering server._handle, connected TCPSocket handles, { fd }, FileHandle, rejection of {} / { fd: "3" }, and closed-handle behavior.

Security risks

None identified. This widens which stdio option shapes are accepted, but the resulting fd is handed to Bun.spawn exactly as a bare number already is today; no new privilege or escape surface. The .fd read can trigger a user getter, but that is the caller's own object and matches Node.

Level of scrutiny

Medium-high. The runtime change is 4 lines, but it sits in child_process stdio option parsing — a hot Node-compat path where ordering matters (the new check must run before the stream check so FileHandle, an EventEmitter with write(), is not mis-claimed by isNodeStreamWritable). The PR reasons through this carefully and tests it, but it is not a mechanical change.

Other factors

Overlap with #34659: this is explicitly a slice of that larger branch; a maintainer should decide merge order.
Vendored test quarantine: per expectations.txt's own header, entries remove whole-file coverage. The justification is sound (both files were previously false-passing and would now hang + orphan a detached child), but it is the kind of CI-coverage change a human should sign off on.
CI not confirmed in the timeline: build #99053 was still building at the last update, and the closed-handle test and the two it.each file-fd cases run on Windows — I could not confirm those pass there.
The tests themselves follow harness conventions well (event-driven waits, tempDir, bunEnv, cleanup in finally, skipIf(isWindows) on socket-inheritance cases).

### Problem - `spawn()` and `spawnSync()` accept a `tls.TLSSocket` in `stdio` and give the child the raw TCP descriptor under the TLS session. The child reads TLS records (`17 03 03 ...`). A child that writes puts plaintext into the TLS stream. Node throws `ERR_INVALID_ARG_VALUE`. - The cause is `streamFdOf` (`src/js/node/child_process.ts:1752`). It returns `_handle.fd` for any stream. Node accepts a stream only when its handle is a pipe, TCP, TTY or UDP wrap ([child_process.js#L1063-L1083](https://github.com/nodejs/node/blob/v26.3.0/lib/internal/child_process.js#L1063-L1083)). ### Fix - `streamFdOf` throws `ERR_INVALID_ARG_VALUE` for a `tls.TLSSocket` before it reads `_handle.fd`. It reuses the check that `subprocess.send()` has for a TLS socket (`src/js/builtins/Ipc.ts:31`). - The message names only the class: `Received '[TLSSocket]'`. The error builder inspects its value in full, and an inspected `TLSSocket` reaches its key and passphrase. - Correct because a TLS socket is the only Bun stream whose `_handle.fd` does not carry the stream's own bytes. A `net.Socket` is still accepted. - Verified: `test/js/node/child_process/child_process.test.ts` (`a socket as a stdio entry`, the TLS test fails without the fix). Also `test/js/node/child_process/` and 18 vendored `test-child-process-*` scripts. ### Background - A stream in `stdio` shares its descriptor with the child: both processes hold the same kernel socket. - A `tls.TLSSocket` is a `net.Socket` whose bytes pass through a TLS session in the parent. Its kernel socket carries only TLS records. - In Bun, `socket._handle` is the native socket object. Native `TCPSocket` and `TLSSocket` expose `fd`. - Only `tls.TLSSocket` and its subclasses have the `Symbol.for("::buntls::")` method. net.ts and Ipc.ts detect a TLS socket with it. <details><summary>Notes</summary> **Repro.** A TLS server hands its accepted socket to a child as stdin. The peer sends 20,000 bytes of `0x41`. The child counts what it reads (python3, so the reader does not depend on Bun). ```js const tls = require("tls"), fs = require("fs"), os = require("os"), { spawn, execFileSync } = require("child_process"); const d = fs.mkdtempSync(os.tmpdir() + "/c-"); execFileSync("openssl", ["req", "-x509", "-newkey", "rsa:2048", "-nodes", "-keyout", d + "/k", "-out", d + "/c", "-days", "2", "-subj", "/CN=localhost"], { stdio: "ignore" }); const srv = tls.createServer({ key: fs.readFileSync(d + "/k"), cert: fs.readFileSync(d + "/c") }, (s) => { s.on("error", () => {}); let c; try { c = spawn("python3", ["-c", "import os,time\nn=0;first=b''\nwhile True:\n try: b=os.read(0,65536)\n except BlockingIOError: time.sleep(0.01); continue\n if not b: break\n first=first or b[:5]; n+=len(b)\nprint('child read',n,'bytes; first 5:',first.hex())"], { stdio: [s, "pipe", "inherit"] }); } catch (e) { console.log("spawn() throws", e.code); s.destroy(); return srv.close(); } let out = ""; c.stdout.on("data", (x) => (out += x)); c.on("close", () => { console.log(`${out.trim()}; parent socket.bytesRead=${s.bytesRead}`); s.destroy(); srv.close(); }); }); srv.listen(0, "127.0.0.1", () => { const cl = tls.connect({ port: srv.address().port, host: "127.0.0.1", rejectUnauthorized: false }, () => { let i = 0; const t = setInterval(() => { cl.write(Buffer.alloc(1000, 65)); if (++i === 20) { clearInterval(t); cl.end(); } }, 5); }); cl.on("error", () => {}); }); ``` | build | result (3 runs) | |---|---| | node v26.3.0 | `spawn() throws ERR_INVALID_ARG_VALUE` | | bun 1.4.3-canary.1 (367d939) | `child read 1022 bytes; first 5: 17030303f9; parent socket.bytesRead=5000`, then `child read 0 bytes`, then `child read 1046 bytes; first 5: 17030303f9` | | this branch (debug build) | `spawn() throws ERR_INVALID_ARG_VALUE` | **Compared with node v26.3.0, entry by entry** (Linux x64, `spawn("true", [], { stdio: [entry, "ignore", "ignore"] })`): | stdio entry | node | main | this branch | |---|---|---|---| | `TLSSocket` that a `tls.Server` accepted, at index 0, 1, 2 or 3, `spawn` and `spawnSync` | `ERR_INVALID_ARG_VALUE` | accepted | `ERR_INVALID_ARG_VALUE` | | `TLSSocket` from `tls.connect()` | `ERR_INVALID_ARG_VALUE` | accepted | `ERR_INVALID_ARG_VALUE` | | `TLSSocket` from `tls.connect({ socket })` | `ERR_INVALID_ARG_VALUE` | accepted | `ERR_INVALID_ARG_VALUE` | | the `net.Socket` under that wrap | accepted | accepted | accepted | | `session.socket` of a secure HTTP/2 session | `ERR_INVALID_ARG_VALUE` | accepted | `ERR_INVALID_ARG_VALUE` | | `new tls.TLSSocket()`, a destroyed `TLSSocket`, a subclass instance | `ERR_INVALID_ARG_VALUE` | plain `Error` ("without an underlying file descriptor") | `ERR_INVALID_ARG_VALUE` | | `TLSSocket` with an own numeric `fd` property | accepted as that fd | accepted | accepted | | `net.Socket` | accepted | accepted | accepted | Node takes a numeric `fd` property before it looks at handles, so the new check sits after the own `fd` check. **Why only TLS.** Node has an allowlist of handle wraps. Bun has no wrap classes: a stream reaches the child through `_handle.fd`, and the only handles with an `fd` are the native `TCPSocket` (TCP and unix sockets, the raw transport) and the native `TLSSocket`. fs, tty and `process.std*` streams carry their own `fd`. The HTTP/2 `session.socket` proxy forwards the `::buntls::` lookup to its `TLSSocket`, so the same check covers it. **Error message.** `$ERR_INVALID_ARG_VALUE(name, value)` inspects `value` into the message with no depth or length limit. The first commit passed the socket. With a PEM string `key` and a `passphrase`, the message was 81,943 characters for an accepted socket and 16,431 for a connected one, and it contained the key body and the passphrase. Node's message is 173 characters (node cuts the inspected value at 128) and contains neither. The second commit keeps the socket away from the builder. `inspect(item, { depth: -1 })` is the shallow form that `node:events` uses for the MaxListeners warning. It reads the constructor name and no property value: `'[TLSSocket]'`, `'[Sub]'` for a subclass, `'[ServerHttp2Session]'` for the HTTP/2 `session.socket` proxy. The quotes are there because the builder quotes a string value. The test pins the exact message. #38402 (open) adds node's 128 character cut to the builder for every caller. This PR does not depend on it. **Windows.** Checked on canary 1.4.3-canary.1+9cba9036a: any socket in `stdio` fails with `EBADF: bad file descriptor, uv_spawn` (node: `spawn ENOTSUP`). The new check runs before that, so the TLS test is not platform specific. The `net.Socket` control is POSIX only. **Related open PRs on the same mapping:** #35226 (error codes for entries with no fd), #39220 (entries that carry an `fd`), #43707 (stop the parent's reads after a hand-off). None of them rejects a TLS socket. A trial merge with #43707 conflicts in one line, the `node:net` import of `child_process.test.ts` (keep #43707's line, it is a superset of this one). #35226 and #39220 conflict in `streamFdOf`, so whichever merges later needs a small rebase. **Local runs of `test/js/node/child_process/` (debug build).** Three tests fail, none through this diff (a fourth, "should allow us to set env", takes 4.6 s to 5.1 s on my debug build and crosses the 5 s timeout in some runs): - `child_process.test.ts`, "should allow us to spawn in the default shell": also fails on the release canary in my container. - `child_process.test.ts`, "extra stdio pipes are not double-closed on GC": 5 s timeout. Its script prints `OK` after 6.2 s on a debug build (#39687). - `child-process-exec.test.ts`, "stderr > exceeding maxBuffer should throw": fails the same way on a debug build of main without this change, passes on the release canary. `child_process_ipc_handle.test.ts` hit the 5 s timeout in 2 tests on one run and passed 14 of 14 on the next. Of the 18 vendored scripts, `test-child-process-emfile.js` fails only on a debug build: it loads built-in modules from disk and cannot under `EMFILE`. It passes on the release canary. </details>  --- **no test proof** · iteration 0 · platform-specific test(s) that do not run on this machine, deferring to CI, which covers all platforms: test/js/node/child_process/child_process.test.ts

github-actions Bot added the claude label Aug 15, 2026

claude Bot reviewed Aug 15, 2026

View reviewed changes

robobun requested a review from Jarred-Sumner as a code owner August 16, 2026 03:04

github-actions Bot reviewed Aug 16, 2026

View reviewed changes

Comment thread src/js/node/child_process.ts Outdated

child_process: shorten the nodeToBun fd comment

20aa7af

github-actions Bot reviewed Aug 16, 2026

View reviewed changes

Comment thread src/js/node/child_process.ts Outdated

child_process: keep only the node permalink above the stdio fd check

77ec2fb

robobun mentioned this pull request Aug 16, 2026

net/cluster/child_process: IPC handle delivery, listen({fd})+SCM_RIGHTS, socket_list, cluster listen semantics (+15 tests) #34659

Open

claude Bot reviewed Aug 16, 2026

View reviewed changes

robobun mentioned this pull request Sep 21, 2026

child_process: reject a tls.TLSSocket in stdio like node #43722

Merged

robobun mentioned this pull request Sep 26, 2026

Bun.serve({ fd }) and node:http listen({ fd }): serve on an inherited socket #44013

Open

Navigation Menu

child_process: accept stdio entries that carry an fd (server._handle, { fd }) - #39220

child_process: accept stdio entries that carry an fd (server._handle, { fd })#39220
robobun wants to merge 4 commits into
mainfrom
farm/5ac28881/child-process-stdio-handle-fd

robobun commented Aug 15, 2026 •

edited

Loading

Uh oh!

coderabbitai Bot commented Aug 15, 2026 •

edited

Loading

Review limit reached

Uh oh!

robobun commented Aug 15, 2026 •

edited

Loading

Uh oh!

robobun commented Aug 15, 2026 •

edited

Loading

Uh oh!

claude Bot left a comment

Uh oh!

github-actions Bot commented Aug 15, 2026

Uh oh!

robobun commented Aug 15, 2026

Uh oh!

Uh oh!

Uh oh!

claude Bot left a comment

Uh oh!

This branch has not been deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

robobun commented Aug 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Problem

Fix

Background

Uh oh!

coderabbitai Bot commented Aug 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review limit reached

Uh oh!

robobun commented Aug 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

robobun commented Aug 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

claude Bot left a comment

Choose a reason for hiding this comment

Overview

Security risks

Level of scrutiny

Other factors

Uh oh!

github-actions Bot commented Aug 15, 2026

Uh oh!

robobun commented Aug 15, 2026

Uh oh!

Uh oh!

Uh oh!

claude Bot left a comment

Choose a reason for hiding this comment

Overview

Security risks

Level of scrutiny

Other factors

Uh oh!

This branch has not been deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

robobun commented Aug 15, 2026 •

edited

Loading

coderabbitai Bot commented Aug 15, 2026 •

edited

Loading

robobun commented Aug 15, 2026 •

edited

Loading

robobun commented Aug 15, 2026 •

edited

Loading