fix(resolver): add PermissionDenied/EPERM/EACCES to directory read error handling - #5
Merged
Merged
Conversation
…#30389) ## Problem `std.c.Sigaction` / `std.c.sigset_t` for `.linux` assume the glibc/musl layout: `{ handler, mask[128B], flags, restorer }`. bionic LP64 is `{ int sa_flags; sa_handler; sigset_t sa_mask; sa_restorer }` where `sigset_t` is a single `unsigned long`. When Bun calls `std.posix.sigaction()` on Android, bionic reads `sa_handler` from offset 8 — which is Zig's `mask[0]`: * Every handler installed with `mask = sigemptyset()` (SIGPIPE/SIGXFSZ in `main`, the crash handler, SIGINT in repl/filter_run/multi_run/Coordinator) silently becomes `SIG_DFL`. Broken-pipe kills the process, no crash trace on SEGV, Ctrl+C isn't caught. * `WaiterThread.reloadHandlers()` sets `mask[0] = 1<<16` (SIGCHLD), so bionic installs the handler `0x10000`. When SIGCHLD fires, the kernel jumps there → `SEGV_MAPERR`, `rip=0x10000`, `rdi=17`. Reproduces 100% on a full Android emulator (not under qemu-user). ## Fix Add `bun.sys.{Sigaction, sigset_t, sigemptyset, sigaddset, sigaction}` in `src/sys/sys.zig`: * **Android**: an `extern struct` matching bionic `bits/signal_types.h` (`__LP64__`) and an `@extern` to libc `sigaction` that takes it. `sigset_t = c_ulong`. * **Everywhere else**: transparent aliases of `std.posix.*`, so nothing changes. Replace all nine callsites (`main.zig`, `crash_handler.zig`, `process.zig`, `repl.zig`, `filter_run.zig`, `multi_run.zig`, `Coordinator.zig`, `Global.zig`). A comptime tripwire fires if `@sizeOf(std.c.Sigaction)` ever shrinks to the bionic 32 bytes, so the workaround can be dropped once the Zig stdlib bug is fixed upstream. Also adds `zig build check-android[-debug]`, gated on `-Dandroid_ndk_sysroot` like `check-freebsd`, so the Android-only struct gets compile-checked. ## Verification ``` $ zig build check-android-debug -Dandroid_ndk_sysroot=…/sysroot check-android-debug success compile obj bun-debug Debug x86_64-linux-android success compile obj bun-debug Debug aarch64-linux-android success ``` `bun run zig:check-all` still passes all 16 targets. Host smoke tests (`spawn/exit-code`, `spawn/spawn-signal`, `spawn/spawn-kill-signal`, plus `BUN_FEATURE_FLAG_FORCE_WAITER_THREAD=1` to hit the modified SIGCHLD path) pass. No runtime test is included because the misbehaviour only surfaces on a real Android kernel, which CI does not run. --------- Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
…oven-sh#30408) ### What does this PR do? Brings `--use-system-ca` / `NODE_USE_SYSTEM_CA` on Windows to parity with Node.js's `ReadWindowsCertificates` (`src/crypto/crypto_context.cc`). Before this change, `root_certs_windows.cpp` only enumerated the `ROOT` store for `CURRENT_USER` and `LOCAL_MACHINE` (2 `CertOpenStore` calls). Node opens 13: `ROOT`, `CA` (intermediates), and `TrustedPeople` across `LOCAL_MACHINE`, `CURRENT_USER`, and the group-policy / enterprise variants — and filters by EKU. The most user-visible consequence of the old behavior: when a server is misconfigured and sends only the leaf cert without its intermediates (very common on intranets, the primary use case for `--use-system-ca`), Node can still build the chain from the intermediates Windows keeps in the `CA` store; Bun would fail with `unable to get local issuer certificate`. Changes, all mirroring Node: | | before | after | |---|---|---| | Store names | `ROOT` | `ROOT`, `CA`, `TrustedPeople` | | Locations | `LOCAL_MACHINE`, `CURRENT_USER` | + `GROUP_POLICY`, `ENTERPRISE` variants | | `CERT_STORE_OPEN_EXISTING_FLAG` | no | yes (don't create a missing store) | | EKU server-auth filter (`CertGetEnhancedKeyUsage`) | no | yes (skip certs restricted to e.g. code-signing only) | `IsCertTrustedForServerAuth` and `GatherCertsForLocation` are direct ports of the equivalents in Node's `crypto_context.cc`, adapted to Bun's raw-DER-blob layering (this TU is kept OpenSSL-free to avoid `wincrypt.h` / BoringSSL macro collisions; `root_certs.cpp` does the `d2i_X509` conversion). ### Related issues (context, not fixes) The issue-finder bot flagged oven-sh#17108, oven-sh#28612, and oven-sh#9365. None of them are closed by this PR because it only changes behavior when `--use-system-ca` / `NODE_USE_SYSTEM_CA=1` is set: - oven-sh#17108 asked for the base feature, which oven-sh#22441 already shipped — this PR refines which Windows stores it reads. - oven-sh#28612 reports `unable to get local issuer certificate` with **no** `--use-system-ca` set (default bundled store, public CAs, TTY-startup race) — different layer. - oven-sh#9365 reproduces on WSL/Linux too and predates `--use-system-ca` — likely a server omitting intermediates with no system-store fallback at all.
…0404) ## What does this PR do? `Bun.mmap(path, 256)` (or any non-object second argument) hit a debug assertion in `JSValue.get()` because the options value was passed straight to `getBooleanLoose` without first checking that it is an object. Now non-object, non-nullish values throw `ERR_INVALID_ARG_TYPE`, and `undefined`/`null` are treated the same as omitting the options argument. ``` panic(main thread): reached unreachable code bun.debugAssert jsc.JSValue.JSValue.get src/jsc/JSValue.zig:1534 jsc.JSValue.JSValue.getBooleanLoose src/jsc/JSValue.zig:1867 runtime.api.BunObject.mmapFile src/runtime/api/BunObject.zig:1219 ``` ## How did you verify your code works? Added a test to `test/js/bun/util/mmap.test.js` covering number/string/boolean (throw) and undefined/null (no throw). Found by Fuzzilli (fingerprint `b1832bde6df73226`). --- Co-authored-by: Alistair Smith <hi@alistair.sh>
oven-sh#30398) ## What does this PR do? Fixes a debug assertion crash when `websocket.perMessageDeflate` is set to a primitive value that isn't a boolean (e.g. a number, string, bigint, or symbol). ```js Bun.serve({ port: 0, fetch: () => new Response(), websocket: { message() {}, perMessageDeflate: 1073741824, }, }); ``` Previously this fell through the `undefined` / `boolean` / `null` checks in `WebSocketServerContext.onCreate` and called `JSValue.getTruthy("compress")` on the primitive, hitting `bun.debugAssert(target.isObject())` in `JSValue.get`. Now it throws `TypeError: websocket expects perMessageDeflate to be a boolean or an object`. ## How did you verify your code works? Added validation tests in `test/js/bun/websocket/websocket-server.test.ts` covering invalid primitives (number, string, bigint, symbol) and valid values (`true`, `false`, `null`, `undefined`, `{}`, `{ compress, decompress }`). Found by Fuzzilli. Fingerprint: `3fbce3302b421eec`
…en-sh#30376) Fixes the `bun install` hang reported in oven-sh#30325 (latest comment — still reproducing on 1.3.13). ## Repro Point `bun install` at an HTTPS registry that accepts TCP but never answers the TLS ClientHello: ```ts // raw TCP server that swallows the ClientHello and never replies net.createServer(s => s.on("data", () => {})).listen(0); ``` ```toml [install] registry = "https://127.0.0.1:<port>/" ``` `bun install` connects, the socket goes ESTABLISHED, and the process blocks in `epoll_wait` forever with no timer armed. This is the state the reporter captured in their Gitea/Kubernetes CI: three ESTABLISHED sockets to the npm CDN, zero rx/tx, 14+ minutes and counting. ## Root cause `HTTPClient.onOpen()` starts the TLS handshake but does not arm the socket's idle timer — the first `setTimeout(socket, 5)` call is in `onWritable()`, which only runs *after* the handshake completes. Freshly-connected sockets inherit `long_timeout = 255` (disabled) from the connecting socket, so a stall anywhere between TCP-connect and handshake-done has no timer at all. The `bun install` main loop then waits forever on `pendingTaskCount() == 0` because the `NetworkTask` callback never fires. The earlier fixes in oven-sh#29611 / oven-sh#29649 covered a different hang (4xx/5xx tarball responses not releasing the task slot); they didn't touch this path. ## Fix - Arm the idle timer in `onOpen()` so it covers the TLS handshake. - Wire the short-tick `onTimeout` handler in `HTTPContext.Handler` alongside the existing `onLongTimeout` — `socket.setTimeout(seconds)` picks whichever timer fits the duration, so both must dispatch. - Read the idle-timeout duration from a new `BUN_CONFIG_HTTP_IDLE_TIMEOUT` env var (seconds). Default is 300 — the previous hard-coded 5 minutes — so nothing changes for unconfigured environments except that the handshake phase is now covered. `0` disables the timer (same as `disable_timeout = true`). - Route the experimental h2 client session's `rearmTimeout` through the same value for consistency. ## Verification New test `test/cli/install/bun-install-stalled-tls.test.ts` starts a raw TCP server that accepts connections and never replies, points `bun install` at it over `https://`, sets `BUN_CONFIG_HTTP_IDLE_TIMEOUT=3` / `BUN_CONFIG_HTTP_RETRY_COUNT=0`, and asserts the install fails with a timeout error. ``` # without this change (fail) bun install times out when the registry accepts TCP but never completes the TLS handshake [60004.48ms] ^ this test timed out after 60000ms. # with this change (pass) bun install times out when the registry accepts TCP but never completes the TLS handshake [4483.87ms] ``` `fetch-http2-client.test.ts` (58 tests) and `bun-install-retry.test.ts` still pass. Fixes oven-sh#30325 --------- Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
### What does this PR do? Makes these actions only run on upstream Bun. I don't need PRs [like these](https://github.com/190n/bun/pulls?q=is%3Apr+is%3Aclosed) to be opened since I only keep my repo up for sending PRs back here, and Bun will merge those updates itself. ### How did you verify your code works?
…nger experimental (oven-sh#30473)
…n-sh#28708) ## Problem When a reused keep-alive connection gets reset (ECONNRESET), `fetch()` unconditionally retries the request — even for POST and other non-idempotent methods. This causes: 1. **Duplicate side effects** on the server (POST executed twice) 2. **Stream corruption** — the retried response body is piped into the original `ReadableStream`, mixing chunks from two different request lifecycles ## Root Cause In `src/http.zig`, `onClose` checks `client.allow_retry` (set when reusing a keep-alive socket) but never checks whether the HTTP method is safe to retry. Per RFC 7231 §4.2.2, only idempotent methods (GET, HEAD, PUT, DELETE, OPTIONS, TRACE) may be retried on connection reset. ## Fix - Add `Method.isIdempotent()` to `src/http/Method.zig` - Gate the retry in `onClose` on the method being idempotent **and** no response body data already having been received (`response_stage` not in `.body` or `.body_chunk`) ## Verification ``` USE_SYSTEM_BUN=1 bun test test/regression/issue/28706.test.ts → FAIL (requestCount 3 instead of 2) bun bd test test/regression/issue/28706.test.ts → PASS ``` Closes oven-sh#28706 --------- Co-authored-by: robobun <robobun@users.noreply.github.com> Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com> Co-authored-by: Ciro Spaciari <ciro.spaciari@gmail.com>
…lex mode (oven-sh#30377) Fixes oven-sh#13696 Fixes oven-sh#23970 Related: oven-sh#21342, oven-sh#29012, oven-sh#18982. ## Reproduction ```js const req = http.request({ method: "POST", headers: { "Transfer-Encoding": "chunked" }, ... }); req.on("response", res => res.on("data", ...)); req.write(body); // req.end() intentionally NOT called — request body stream kept open ``` In Node, the request goes out on the first `write()` and the `'response'` event fires as soon as headers arrive, while the request body stream stays open for further writes. In Bun, the request was **never sent** and `'response'` was never emitted. ## Cause In `_http_client.ts`: 1. `pushChunk` only called `startFetch()` when `writeCount > 1`, so a single `write()` without `end()`/`flushHeaders()` never dispatched the request. 2. When `startFetch()` ran with `isDuplex = true` (body still streaming), `handleResponse()` was gated on `!keepOpen` and otherwise deferred until the body generator finished — i.e. until `req.end()`. docker-modem hits both for `container.exec({ stdin: true })`: it sends a chunked POST, writes the JSON options once, and keeps the request open so it can stream stdin to the container. This is what makes testcontainers' default `HostPortWaitStrategy` (which shells into the container via `exec` to check ports) hang until timeout. ## Fix - First `write()` now schedules `startFetch()` for the next tick. If `end()` runs in the same tick, `send()` still takes the non-duplex fast path (`fetching` / `finished` guards prevent the deferred start from doing anything). - `handleResponse()` is now called unconditionally when response headers arrive, matching Node's event ordering. The existing self-clear (`handleResponse = undefined`) keeps later call sites no-ops. ## Verification `test/regression/issue/13696.test.ts` simulates docker-modem's exec pattern against a raw TCP server and a Unix-socket server (Docker daemon uses a Unix socket). Both, plus the `flushHeaders()` variant, hang/timeout on current `main` and pass with this change. Existing `write()`/`end()` combinations (same-tick, next-tick, multi-chunk, empty) match Node output unchanged. ## Related issues checked - **oven-sh#23970** (single `req.write()` without `end()` never reaches server): same root cause, verified fixed. - **oven-sh#21620** (write, then `setTimeout`, then write+end): already passes on 1.3.13, not this bug. - **oven-sh#29012** / **oven-sh#18982** (dockerode `exec.start`/`attach` with `hijack: true`): this PR gets the request out the door and delivers the 101 response, so they no longer *hang* — but Bun emits `'response'` instead of `'upgrade'` for the 101, which docker-modem treats as an unexpected status. Fully fixing those needs the separate `'upgrade'` event work (oven-sh#25278). --------- Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
## What Fixes oven-sh#30433. `YAML.stringify` emits number-like strings unquoted when they contain patterns the scanner does not recognise as numbers but the YAML parser does. The resulting YAML round-trips as the number, silently corrupting the value: ```js YAML.stringify({ id: "0e6836" }) // → `id: 0e6836` YAML.parse(`id: 0e6836`) // → { id: 0 } YAML.stringify({ id: "+1" }) // → `id: +1` YAML.parse(`id: +1`) // → { id: 1 } ``` The issue's other complaint (trailing colons like `"foo:"` emitted unquoted) was already fixed on main via oven-sh#25439. What remained were the number-like-string cases. ## Root cause In `src/runtime/api/YAMLObject.zig`: 1. `stringIsNumber` short-circuited when a leading `0` was followed by anything other than `x`/`X`/`o`/`O`/digit — so `"0e6836"` and `"0.0"` were classified as non-numbers even though the YAML parser accepts them as float literals. 2. `stringNeedsQuotes` had no `+` entry in its main loop, so strings starting with `+` never reached `stringIsNumber` at all — `"+1"`, `"+99"`, `"+1.5"`, `"+1e5"` were all emitted bare. 3. On the failure path, callers of `stringIsNumber` reused the scanner's advanced `offset` as their own `i`, then `i += 1`. That skipped whatever character made the scan fail — including flow indicators like `,`, `{`, `}` — so `"9{"`, `"9,"`, `".{"`, `".,b"` also slipped out unquoted and broke `YAML.parse`. ## Fix - `stringIsNumber`: after a leading `0`, accept `e`/`E`/`.` as float continuations so `"0e6836"` and `"0.0"` are recognised as numbers. - `stringNeedsQuotes`: new `+` case that consults `stringIsNumber`, mirroring the existing `-` handling. - The `-`, `.`, `0...9`, `+` callers pass a local scratch offset to `stringIsNumber` instead of mutating their own scan index, so the main loop still sees characters the number scanner looked at and rejected. ## Verification Added `test/js/bun/yaml/yaml.test.ts` coverage for the issue plus the flow-indicator edge cases. Updated two pre-existing assertions (`+99` and `....`) that were documenting the bug's output. Before the fix, `YAML.parse(YAML.stringify({ id: "0e6836" }))` returned `{ id: 0 }`. After, it returns `{ id: "0e6836" }`. --------- Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
…en-sh#30527) ## WebKit changes (88b2f7a2 → 5488984d) Single commit on top of the previous pin: ### `module-loader: don't double-fire moduleRegistryModuleSettled after inline sync replay` (oven-sh/WebKit#225, rebased oven-sh#217) **File:** `Source/JavaScriptCore/runtime/JSMicrotask.cpp` **What:** Adds a `modulePromise->status() != Pending` early-return guard to `moduleRegistryModuleSettled`, symmetric with the guard already present in `moduleRegistryFetchSettled`. Gated under `#if USE(BUN_JSC_ADDITIONS)`. **Why:** `require()` of an ESM whose graph contained a diamond dependency through a barrel deadlocked (release) / aborted on `ASSERTION FAILED: m_status == Status::Fetching` (debug). `hostLoadImportedModule`'s synchronous-replay branch (taken when `require(esm)` is draining the synchronous module queue) calls `fetchComplete` + fulfills `modulePromise` inline. If a `ModuleRegistryFetchSettled` reaction had already run on the *normal* microtask queue for the same entry before sync mode was entered, it left a stale `ModuleRegistryModuleSettled` reaction queued there. When the normal queue later drained, that reaction re-entered `fetchComplete` on an already-`Fetched` entry. No changes to `JSType.h`. No WebCore code-generator changes. --- **Verification:** - ✅ `test/regression/issue/30493.test.ts` fails on current `main` (assertion crash, empty stdout) - ✅ Same test passes on `bun run build:local` with the patched WebKit - ✅ Same test passes on `bun bd` with the prebuilt preview tarball - ✅ Full bun CI green against `autobuild-preview-pr-225-2b6b1c39` (build #53556 — 67 pass, 3 pre-existing main flakes also red on oven-sh#30522) Fixes oven-sh#30493 Fixes oven-sh#30281 Closes oven-sh#30283 (the dependency-free 6-file repro in this PR covers the same root cause without needing a react+MUI install) --------- Co-authored-by: robobun <117481402+robobun@users.noreply.github.com>
…error-throwing module no longer hangs (oven-sh#30537) A second `await import()` of a path whose first import threw used to hang forever instead of re-throwing. Two reported variants: - **oven-sh#23139**: same path twice - **oven-sh#22743**: path X, then a *different* error path Y, then X again (true regression, 1.2.20 → 1.2.21) While verifying oven-sh#30527 I checked whether oven-sh/WebKit#225 covered these — turns out both were **already fixed** before that bump (main + WebKit `88b2f7a2` doesn't hang either). The fix landed somewhere in the module-loader rewrite window (oven-sh#29393 → oven-sh#30262); the issues just never got re-tested. | build | oven-sh#23139 | oven-sh#22743 | |---|---|---| | 1.3.13 | hangs | hangs | | main + WebKit `88b2f7a2` (pre-oven-sh#30527) | ✅ | ✅ | | main + WebKit `5488984d` | ✅ | ✅ | Both tests fail on `USE_SYSTEM_BUN=1` (stdout missing the post-hang lines, killed by spawn timeout) and pass on `bun bd test`. Fixes oven-sh#23139 Fixes oven-sh#22743
Pins all `uses:` references to full commit SHAs with a `# vX.Y.Z` comment for readability.
## What
Renames the `Bun.serve()` `ServerConfig` HTTP/3 options from the
abbreviated `h3` / `h1` to the spelled-out `http3` / `http1`.
```ts
// before
Bun.serve({ tls, h3: true, h1: false, fetch })
// after
Bun.serve({ tls, http3: true, http1: false, fetch })
```
These options have not shipped in a release yet, so this is a clean
rename with no backwards-compat aliases.
## Changes
- **`src/runtime/server/ServerConfig.zig` / `server.zig`**: rename the
`ServerConfig` struct fields and the JS option lookups (`arg.get(global,
"http3")`, etc.); update the validation error messages and the
unix-socket warning.
- **`packages/bun-types/serve.d.ts`**: rename the type declarations,
drop the incorrect "(and HTTP/2)" from the `http1` JSDoc (Bun.serve does
not support HTTP/2), and mark both options `@experimental`.
- **`docs/runtime/http/server.mdx`**: add an "HTTP/3 (QUIC)" section
documenting `http3` / `http1`, Alt-Svc, the unix-socket limitation, and
the experimental status.
- **Tests**: update `Bun.serve({ http3, http1 })` call sites, test
names, and the validation error-message assertion across the HTTP/3
suites (`serve-http3.test.ts`, `serve-protocols.test.ts`,
`body-stream.test.ts`, `fetch-http3-client.test.ts`,
`fetch-http3-adversarial.test.ts`, `direct-readable-stream.test.tsx`,
`fetch-h3.ts`, `packages/h3blast/`).
## Notes
- The internal `uws.AnyRequest` union variants (`.h1`/`.h3`) are left
as-is — those tag a transport, not a config option.
- The `fetch()` `protocol` option (`"http1.1" | "h1" | "http2" | "h2" |
"http3" | "h3"`) is unchanged; it already accepts both spellings.
- `zig:check` and the `bun-types` `tsc` test pass locally. The full
HTTP/3 test suite will run in CI.
Follow-up to oven-sh#30583. Prettier wrapped the inline `` `http3: true` `` span across two lines inside the `<Note>`, so per CommonMark code-span rules the newline + 2-space indent become literal spaces and it renders as `http3: true`. Dropped the redundant "always" so the line fits under `printWidth: 120` and stays prettier-stable. Docs-only, no build needed.
### What does this PR do? Enables TCP keepalive (`SO_KEEPALIVE` + `TCP_KEEPIDLE=60s`) on `fetch()` client sockets. Without this, when a connection becomes half-open — the peer is gone but the FIN/RST never reached us (NAT timeout, wifi/cellular handoff, middlebox state eviction, VPN disconnect) — the kernel never discovers it. A streaming `reader.read()` on such a socket blocks forever (or until an application-level timeout). Node's fetch (undici) sets `SO_KEEPALIVE` with `TCP_KEEPIDLE=60s`, so a half-open connection is detected at ~70s (60s idle + 10 probes × 1s). This makes Bun match that behavior via the existing `socket.setKeepAlive()` → `bsd_socket_keepalive()` path, which already hardcodes `TCP_KEEPINTVL=1` and `TCP_KEEPCNT=10`. The call is placed in `onOpen()` next to `client.setTimeout(socket)` (oven-sh#30376) — socket-level, fires once per connection, inherited by keep-alive-reused requests. ### How did you verify your code works? Added `test/js/web/fetch/fetch-tcp-keepalive.test.ts` (Linux-only) that: - starts a streaming server, opens a `fetch()` to it - reads `/proc/self/net/tcp` and finds the client socket (ESTABLISHED, remote port = server port) - asserts the timer field is not `00:00000000` — i.e. the kernel's `sk_timer` (keepalive) is armed (`timer_active=02`) Without this patch the timer field is `00`; with it, `02:<jiffies>`. --------- Co-authored-by: robobun <117481402+robobun@users.noreply.github.com> Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
…en-sh#30640) Includes oven-sh#30637 (undici source link for the 60s default). Mirror undici's `options.keepAlive` guard in [`buildConnector`](https://github.com/nodejs/undici/blob/f33a6cb615e1/lib/core/connect.js#L121-L124): when `fetch(url, { keepalive: false })` is used (which already disables HTTP connection reuse), also skip the `SO_KEEPALIVE` `setsockopt` instead of applying it unconditionally. ### `node:http` / `node:https` defaults (no changes needed) `ClientRequest` already forwards `agent.keepAlive` to fetch's `keepalive` in [`_http_client.ts`](../blob/b1cc6187ab/src/js/node/_http_client.ts#L298-L302), and both global agents are constructed with `{ keepAlive: true, scheduling: "lifo", timeout: 5000 }` — matching Node 19+: | | `agent.keepAlive` | TCP `SO_KEEPALIVE` | | --- | --- | --- | | `http.globalAgent` / `https.globalAgent` | `true` | on (60s, undici default) | | `new Agent()` / `agent: false` | `false` | off | ### How did you verify your code works? `test/js/web/fetch/fetch-tcp-keepalive.test.ts` (Linux-only, reads `/proc/self/net/tcp`) extended with cases for `keepalive: false`, `node:http` `agent: false`, and `node:http` `globalAgent`. --------- Co-authored-by: robobun <117481402+robobun@users.noreply.github.com>
Blog post with details coming soon. Still some optimization work to do before this lands in non-canary version. --------- Co-authored-by: Dylan Conway <dylan.conway567@gmail.com> Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com> Co-authored-by: robobun <117481402+robobun@users.noreply.github.com>
## What Pass `-no-pie` alongside `-fno-pic -fno-pie` in the `CMAKE_C_FLAGS` we hand to WebKit's local cmake configure. ## Why 23427db added `-fno-pic -fno-pie` to `optFlags` so JSC/WTF vtables land in `.rodata`. On distros where the clang driver defaults to `-pie` (Arch, current Ubuntu/Debian), cmake's `try_compile()` then compiles the probe with `-fno-pic` but still links it `-pie`, so every probe dies with: ``` relocation R_X86_64_32S against symbol `info_compiler' can not be used when making a PIE object; recompile with -fPIE ``` `FindThreads` is `REQUIRED`, so configure aborts. CI doesn't see this because the prebuilt-WebKit path never reconfigures. `-no-pie` in `CMAKE_C_FLAGS` is ignored at `-c` time (and WebKit already prepends `-Qunused-arguments` for its own compiles) but suppresses the driver's default `-pie` when `try_compile` links the probe. Kept it in `optFlags` rather than `CMAKE_EXE_LINKER_FLAGS` because `spec.args` are appended last and would clobber source.ts's `--ld-path=${cfg.ld}`. Prebuilt mode is unaffected — `build()` returns `{kind: "none"}` before `optFlags` is constructed. ## Also `compile.ts`: widen `LinkOpts.linkerMapOutput` to `string | undefined` so the `cond ? path : undefined` call sites in `bun.ts` typecheck under `exactOptionalPropertyTypes`. Same pattern as `bun.ts:136,138`. ## Verified - Before: `bun run build:local --target=configure-WebKit` → `CMake Error ... FindThreads` - After: `Found Threads: TRUE`, `Configuring done` - `bun x tsc --noEmit -p scripts/build/tsconfig.json` clean - `build/debug/build.ninja` (prebuilt) contains no standalone `-no-pie`
## Summary - Adds `[lints]` with `workspace = true` to every workspace member crate so per-crate manifests opt into the shared `[workspace.lints]` policy. - Lifts the `unexpected_cfgs` registration for `cfg(bun_asan)` from `bun_core` and `bun_alloc` into `[workspace.lints.rust]`. Side effect: `bun_safety` (which uses `cfg(bun_asan)` without registering it locally) is now covered too. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --- Note: This allows for adding more workspace level lints in the near future. Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ven-sh#30715) First step toward `#![forbid(unsafe_code)]` on `bun_collections`. ## What - `array_list.rs`: deleted the `*_shallow` method family (`deinit_shallow`, `replace_range_shallow`, `replace_range_assume_capacity_shallow`, `resize_without_deinit`, `shrink_and_free_shallow`, `shrink_retaining_capacity_shallow`, `clear_retaining_capacity_shallow`, `clear_and_free_shallow`). These were Zig "leak-on-shrink" semantics that no Rust call site uses — in Rust, "elements already moved out" is expressed at the call site, not via a leaking container method. Module is now `#![forbid(unsafe_code)]`. - `lib.rs`: deleted `SmallList::get_last_unchecked` (zero callers). Net: **-7 unsafe blocks**, all dead code, no call-site changes. ## Verified - `cargo check -p bun_collections` ✅ - `bun bd` builds and links ✅ - `cargo test -p bun_collections` has pre-existing compile errors on `main` in `hive_array.rs`/`linear_fifo.rs` `#[cfg(test)]` code — unrelated to this change, confirmed by stash+retest. ## Follow-ups (not in this PR) The other modules I surveyed need design work, not mechanical cleanup: - **`SmallList::set_len`** (1 unsafe) — 4 callers in `src/css/`, all shrink-only. `parser.rs:68` reimplements the existing safe `to_owned_slice()`; `css_parser.rs:971` truncates `Copy` data; `builder.rs:213-214` needs the move-out path verified. Replace with `truncate`/`clear`/`to_owned_slice` and delete. - **`linear_fifo.rs`** (13 unsafe) — root cause is `LinearFifoBuffer::as_slice()` exposing `MaybeUninit<T>` storage as `&[T]`. Can't bound `T: Copy` because valkey's `Entry`/`PromisePair` aren't. Fix: rework the trait to return `&[MaybeUninit<T>]`, change `writable_slice`/`writable_with_size` return type, update callers in `websocket_client.rs`/`MiniEventLoop.rs`. - **`bit_set.rs`** (23 unsafe) — all in `DynamicBitSetUnmanaged`/`DynamicBitSetList`. The `*mut usize` with size-at-`ptr[-1]` exists because `DynamicBitSetList::at()` hands out aliasing-mutable views into a shared buffer. Fix: switch `DynamicBitSetList` to an index-based API (`set(i, j)`/`is_set(i, j)`/`union(i, j)`) so `DynamicBitSetUnmanaged` can become `Vec<usize>`-backed; update callers in `hoisted_install`/`isolated_install`/`PackageInstaller`.
### What does this PR do? Ports the TCP keepalive call from `src/http/http.zig` `onOpen` (added in oven-sh#30627, gated in oven-sh#30640) into the Rust `on_open` in `src/http/lib.rs`. The Rust HTTP client landed in oven-sh#30412 from a branch cut before oven-sh#30627 merged, so the `set_keepalive` call was never carried over. This is the cause of `test/js/web/fetch/fetch-tcp-keepalive.test.ts` failing on every Linux test job since oven-sh#30412 merged. The test reads `/proc/self/net/tcp` and expects `timer_active=02` (keepalive armed) but gets `00` because the Rust client never calls `set_keepalive`. Build #54331 (and every Linux job since): 2 pass / 2 fail; build #54099 (oven-sh#30640's PR, Zig build): 4 pass / 0 fail; build #54202 (oven-sh#30412's PR): test file not in tree, so it never ran. The change is the same shape as the Zig reference: after `self.set_timeout(socket)`, guarded by `!self.flags.disable_keepalive` so `fetch(url, { keepalive: false })` and `node:http` non-keepalive agents skip it (matching undici's `buildConnector`). `set_keep_alive` already exists on `NewSocketHandler` (`src/uws_sys/socket.rs:466`) and the `disable_keepalive` flag is already wired through. ### How did you verify your code works? I have not built or run this — opening it directly per request so CI validates. The covering test is `test/js/web/fetch/fetch-tcp-keepalive.test.ts` (Linux-only), which is the test currently failing on every PR.
…-sh#30735) Zig models Android as `os.tag == .linux, abi == .android`, so `Environment.isLinux` was true on Android. Rust splits them into two `target_os` values, so `cfg!(target_os = "linux")` is false on Android — code ported from Zig's `isLinux` checks needs `any(target_os = "linux", target_os = "android")` to preserve semantics. `IS_LINUX` in `src/bun_core/env.rs` already documents this. The most visible symptom was `StandaloneModuleGraph::from_executable()` falling through to its catch-all `unreachable!()` on every invocation, panicking → SIGILL at startup on Android. ### What changed Audited every `#[cfg(target_os = "linux")]` / `cfg!(target_os = "linux")` that doesn't already mention `android`, and added it where the gated code is a Linux-**kernel** feature (epoll, /proc, memfd, prctl, pidfd, copy_file_range, sendfile, statx, O_PATH/O_TMPFILE, ELF `link_section`, ftrace, futex, etc.) rather than a glibc/musl libc feature. Left alone: - `target_env = "gnu"` / `target_env = "musl"` gates (libc-specific by definition) - glibc-only symbols: `gnu_get_libc_version`, `backtrace`/`<execinfo.h>`, `__wrap_gettid`, `getaddrinfo_a` EAI extensions - `EAI_ADDRFAMILY` value — bionic uses BSD-style `1`, not glibc's `-9` - `MAP_SHARED_VALIDATE | MAP_SYNC` — not exposed by the libc crate for android - sites that already have a dedicated `#[cfg(target_os = "android")]` arm (e.g. `pidfd_open` raw-syscall shim, `spawn_sync_inherit` fork+execvp fallback) Three small follow-on fixes to keep `cargo check --target aarch64-linux-android` green: - `libc::RENAME_EXCHANGE` / `RENAME_NOREPLACE` are `c_int` on android vs `c_uint` on linux → `as u32` - `libc::POSIX_SPAWN_SETSID` isn't exposed by the libc crate for android (bionic value is `0x80`, same as glibc/musl) → local const at the two call sites ### Verification `cargo check --workspace` passes on `{aarch64,x86_64}-linux-android`, `x86_64-unknown-linux-{gnu,musl}`, `aarch64-apple-darwin`, `x86_64-pc-windows-msvc`, `x86_64-unknown-freebsd`.
`Bun__CallFrame__describeFrame` is only compiled into the C++ side under `#if ASSERT_ENABLED` (bindings.cpp), since `JSC::CallFrame::describeFrame()` itself is debug-only in JavaScriptCore. The Rust `extern` declared it unconditionally. With Cargo's default `lto = "fat"` the dead `pub fn` is DCE'd in release, but `scripts/build/rust.ts` disables Cargo LTO for `cfg.asan` and `cfg.crossLangLto` builds. In those configs the reference survives in `bun_jsc`'s object file, and on Windows release the link fails: ``` lld-link: error: undefined symbol: Bun__CallFrame__describeFrame ``` (Linux/macOS happen to GC it via `--gc-sections` / `-dead_strip`.) The only caller, `btjs::print_source_at_address`, is already `#[cfg(debug_assertions)]`, so gate the wrapper and FFI import the same way. Also cfg-gate the now-debug-only `ZStr` import and inline `c_char` to keep release builds warning-free.
…0743) `src/runtime/ffi/mod.rs` declares its own unconditional `extern "C" { fn tcc_delete }`, separate from `bun_tcc_sys::tcc_externs!` which correctly cfg-gates the libtcc externs on `cfg.tinycc` (= NOT android / freebsd / windows-aarch64, where `libtcc.a` isn't built). `Function::drop` calls it inside `if let Some(state)` — `state` is never `Some` on those targets so the call is dead at runtime, but with cargo LTO overridden off (asan / linker-plugin-lto path in `rust.ts`) the symbol reference survives into the staticlib and link fails: ``` lld-link: error: undefined symbol: tcc_delete ``` Gate the local extern on the same predicate as `tcc_externs!` and provide an `unreachable!()` stub on the disabled targets, mirroring what `src/tcc_sys/tcc.rs` already does.
…en-sh#30741) The Rust port of oven-sh#30583 covered the user-facing JS option names (`"http3"`/`"http1"`) and validation error messages, but left the internal `ServerConfig` struct fields as `h3`/`h1` and one stderr warning string as `"h3: true with a unix socket — HTTP/3 listener skipped"`. This finishes the rename: - `ServerConfig::{h3,h1}` → `{http3,http1}` (fields, defaults, clone, all reads in `ServerConfig.rs`/`mod.rs`) - unix-socket warning string `"h3: true ..."` → `"http3: true ..."` - doc comments in `mod.rs` and `uws_sys/quic/Context.rs` Transport-layer names (`uws_sys::h3::`, `h3_app`, `on_h3_listen`, `AnyRequest::H1`, `HTTPClient.h3`) are left as-is, matching oven-sh#30583. Adds `serve-http3.test.ts` → `validation: http3 with unix socket warns and skips H3 listener` to cover the warning text.
…ven-sh#30720) ## Reproduction ```js // repro.js import libpath from "./libhello.so" with { type: "file" }; import { dlopen, FFIType } from "bun:ffi"; const lib = dlopen(libpath, { hello: { args: [], returns: FFIType.i32 } }); console.log("loaded:", lib.symbols.hello()); ``` ```bash clang -shared -fPIC -o libhello.so hello.c bun repro.js # works → "loaded: 42" bun build --compile repro.js --outfile repro ./repro # broken on canary ``` Fails with: ``` error: Failed to open library "/$bunfs/root/libhello-XXXX.so": /$bunfs/root/libhello-XXXX.so: cannot open shared object file: No such file or directory syscall: "dlopen", code: "ERR_DLOPEN_FAILED" ``` ## Cause Regression from the Rust rewrite (1.3.14-canary, commit `23427dbc12`; last known-good `1.3.13`). The Rust port of `FFI.open` (`src/runtime/ffi/ffi_body.rs:1486`) shipped a stub where the Zig original called `jsc.ModuleLoader.resolveEmbeddedFile` to materialize the bunfs-embedded library to an on-disk tmpfile. The raw `/$bunfs/...` virtual path fell through to libc `dlopen(2)`, which can't see the bunfs virtual filesystem. `process.dlopen` (`.node` addons) was unaffected — that path still reaches the working `Bun__resolveEmbeddedNodeFile` → `resolve_embedded_node_file_hook`. The `PORT NOTE` at `ModuleLoader.rs:561` enumerated only two Zig callers being ported; it omitted `ffi.zig:1030`. ## Fix Factor the extraction body out of `resolve_embedded_node_file_hook` into `resolve_embedded_file_to_buf(input_path, extname, out_buf)`. Keep the `.node` hook thin (pass `b"node"` and clone the result into `in_out_str`). Call the helper from `ffi_body` with the platform-chosen extname (`so`/`dylib`/`dll`). Same-crate call, no new `LoaderHooks` entry needed. ## Verification - `test/regression/issue/30717.test.ts`: compiles a C fixture, embeds it with `{ type: "file" }`, runs `bun build --compile`, deletes the on-disk `.so`, runs the compiled binary from a different cwd, asserts it prints `loaded: 42` without `ERR_DLOPEN_FAILED`. Fails on pre-fix tree (`ERR_DLOPEN_FAILED /$bunfs/root/…`), passes with the fix. - Existing `test/napi/napi.test.ts` `--compile` tests still load `.node` addons correctly (refactor preserves behavior). - Existing `test/js/bun/ffi/ffi.test.js` tests unchanged. Fixes oven-sh#30717. Fixes oven-sh#11598. Fixes oven-sh#14009. --------- Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
…-sh#30682) ## Summary - Drop the Zig download + `zig fmt src` block from `.github/workflows/format.yml` and run `cargo fmt --all` instead. The codebase no longer has Zig source to format. - The pinned nightly + `rustfmt` come from `rust-toolchain.toml` (now lists `rustfmt` in `components`); `rustup` auto-installs on the first `cargo` invocation, so no separate setup step. - Run `cargo fmt --all` once against the current toolchain so the next format run is a no-op — 92 source files reformatted (~640 lines, mostly import-list reordering and over-long-line wraps). - Update `.github/workflows/CLAUDE.md` to describe the rustfmt step and toolchain-bump process. ## Test plan - [x] `cargo fmt --all -- --check` exits 0 after the formatting pass (idempotent) - [x] `cargo check -p bun_core -p bun_runtime -p bun_bin` clean after the reformat - [x] `bun run rust:check` clean - [x] `python3 -c "import yaml; yaml.safe_load(open('.github/workflows/format.yml'))"` — YAML valid - [ ] Confirm the `Format` GHA job on this PR runs `cargo fmt --all` and emits no changes --------- Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
…-Rust pointers (oven-sh#30880) ## What ### Module split The Zig→Rust port wrapped the entire resolver implementation in a single 7,664-line `pub mod __phase_a_body { ... }` inside `src/resolver/lib.rs` (lines 2609–10,273) — a port artifact ("this is the mechanically-translated block"). There's no name for a module that wraps a crate's whole body that isn't redundant, which is the tell the wrapper shouldn't exist. Split it into sibling files following the crate's existing convention (`data_url.rs` / `dir_info.rs` / `package_json.rs`): | File | Lines | Holds | |---|---|---| | `options.rs` | 357 | `BundleOptions`, `Packages`, `ExternalModules`, `Framework`, `ExtOrder`, … | | `result.rs` | 578 | `Result`, `MatchResult`, `PathPair`, `DebugLogs`, `PendingResolution`, `LoadResult`, … | | `resolver.rs` | 6,640 | `Resolver` struct + impl, threadlocal `Bufs`, local shim modules | | `standalone_module_graph.rs` | 30 | the `StandaloneModuleGraph` trait | | `allocators.rs` | 6 | re-exports referenced cross-file from `dir_info.rs` | `lib.rs` shrinks 10,273 → 2,615 lines. Public API surface is byte-identical — `bun_resolver::Resolver`, `::Result`, `::options`, … all resolve as before. ### Typed extern-Rust pointers Un-erase the `extern "Rust"` link-time pointers where the declaring crate already names the type. The port applied "type-erase across the crate boundary" uniformly to every `#[no_mangle]` upward call, but `extern "Rust"` carries full Rust types — both crates can name the parameters. Where visible, use the typed pointer with the Zig pointer shape (`NonNull<T>` for `*T`, `Option<NonNull<T>>` for `?*T`): - `__bun_resolver_init_package_manager`: `log: *mut Log` → `NonNull<Log>`, `install: *const ()` → `Option<NonNull<BunInstall>>`, `env: *mut c_void` → `NonNull<Loader<'static>>` - `BundleOptions.install`: `*const ()` → `Option<NonNull<BunInstall>>` - `Resolver.log`: `*mut Log` → `NonNull<Log>` - `__bun_jsc_enable_hot_module_reloading_for_bundler`: `*mut ()` → `NonNull<BundleV2<'static>>` The implementation-side `cast::<T>()` calls — the tell that the erasure was unnecessary — are removed. Sites where the type is genuinely not visible (`bun_event_loop` → `bun_jsc::VirtualMachine`, `bun_js_parser` → `bun_bundler::Transpiler`) are left as-is — that's the real layering boundary the pattern exists for. ## Verification - `cargo check --workspace` clean - `cargo fmt --check` clean - `bun run rust:check-all` (all 6 target triples) - `bun bd` links and runs - `grep -rn resolver_body src/` returns nothing
…ecifiers (oven-sh#30882) ## Summary - `~SourceProvider()` derefs `m_resolvedSource.specifier` and `.source_url` (introduced in c713ab5 to fix a leak), which requires every `ResolvedSource` producer to hand those `BunString`s in as +1. - The synthetic-module paths in `jsc_hooks.rs` (`bun:main`, `bun:wrap`, `macro:`, standalone-graph, embedded sqlite) stored a bitwise copy of the borrowed specifier (`*specifier`) with no extra ref, so the destructor over-derefs. The atom impl frees while a `JSString` in the worker heap still references it, and once that slot is reused, `Heap::lastChanceToFinalize` trips `RELEASE_ASSERT(wasRemoved)` in `AtomStringImpl::remove` during worker VM teardown — symptom is a SIGABRT on a random atom string. - Fix: `specifier.dupe_ref()` for both `specifier` and `source_url` on the paths whose `ResolvedSource` flows into `Zig::SourceProvider::create()`, matching `RuntimeTranspilerStore::run_from_js_thread`. ## Test plan - [x] `test/js/node/test/parallel/test-worker-console-listeners.js` — 480/480 clean (was ~5/240 SIGABRT) with `BUN_DESTRUCT_VM_ON_EXIT=1` under 8× parallel debug-build loop - [x] `test/js/node/test/parallel/test-crypto-worker-thread.js` — 400/400 clean under the same harness - [x] Diagnostic heap walk before `~VM` confirms worker heaps no longer hold a `bun:main` `JSString` whose `StringImpl` is absent from the worker's atom table (`foreign=1` → `foreign=0`)
…s, http (oven-sh#30722) Hardens 36 reachable security findings across the runtime, package manager, parsers, HTTP client/server, and SQL drivers. Three auto-applied fixes (oven-sh#61 SSL exception leak, oven-sh#68 YAML merge dedup, oven-sh#104 archive overwrite precheck) were dropped: oven-sh#61 introduced a use-after-free, oven-sh#68 stored a non-`'static` byte view in a `'static` field, and oven-sh#104 added dead gating that did not close the traversal. ### Memory safety / lifetime - #2 — Dangling proxy slice across reentrant JS getter — copy `process.env` proxy href to an owned `Vec` before reentrant getters can free the env map (`Blob.rs`) - #15 — Rollback restores dangling editor name pointer — preserve and restore `name_storage` on `detect_editor` failure (`BunObject.rs`) - oven-sh#81 — Reentrant reconnect frees live handlers — only free previous handlers when `active_connections == 0` (`Listener.rs`) - oven-sh#110 — Async randomFill uses stale resizable buffer pointer — fill a worker-owned scratch buffer; copy back on the JS thread after re-validating bounds (`node_crypto_binding.rs`) - oven-sh#119 — Null zero-length slice UB in DOMJIT fast path — use `ffi::slice` which tolerates `(null, 0)` (`Crypto.rs`) - oven-sh#67 — Raw serialization reads struct padding bytes — add explicit `_padding_*` fields with `offset_of!` proof asserts (`npm.rs`) - oven-sh#74 — TLS rejection path leaks websocket refcount — route SSL/auth failures through `self.fail()` which clears `outgoing_websocket` (`websocket_client.rs`) - oven-sh#108 — FD-backed fetch body leaks duplicated descriptor — close `opened_fd` unconditionally after `read_file` (`fetch.rs`) ### Untrusted-input bounds / panics - #10 — Invalid lockfile tag causes panic DoS — replace `unreachable!()` with logged error + `Tag::Uninitialized` (`dependency.rs`) - oven-sh#20 — Unchecked lockfile string offsets cause OOB slice — bounds-check non-inline `String` pointers against `ctx.buffer` (`dependency.rs`) - oven-sh#91 — Panic on unvalidated resolution tag — validate `ResolutionTag` discriminants on lockfile load (`Package.rs`) - oven-sh#24 — Unwrap panic on unexpected 304 response — return `UnexpectedNotModified` when no cached manifest exists (`npm.rs`) - oven-sh#44 — UDP port getter unwrap panic on transient state — return `undefined` when `socket` is `None` (`udp_socket.rs`) - oven-sh#36 — Close reason length mismatch causes panic — clamp `body_len` to 125 and bail on overlong UTF-8 transcode (`websocket_client.rs`) - oven-sh#100 — Windows pipe name length panic DoS — `debug_assert` → real bounds check (`Listener.rs`) - oven-sh#60 / oven-sh#111 — Windows shim stack buffer overflows — bounds-check argument and filename writes against `BUF1_LEN`/`BUF2_U16_LEN` before `copy_nonoverlapping` (`bun_shim_impl.rs`) - oven-sh#76 / oven-sh#101 — Unchecked bin name/entry name copies — bounds-check before slicing into `abs_dest_buf` (`bin.rs`) - oven-sh#79 — `if` keyword misclassification causes parser panic — require a delimiter token before classifying (`shell_parser/parse.rs`) - oven-sh#32 — Bounds check occurs after UTF-16 write — pre-flight key/value lengths before `convert_utf8_to_utf16_in_buffer` (`env_loader.rs`) - oven-sh#95 — PBKDF2 digest validation allows panic-only algorithm — reject digests with no `EVP_MD` (`PBKDF2.rs`) ### DoS / resource caps - oven-sh#17 — Unbounded recursion on deep TOML dotted keys — cap dotted-key segments at 512 (`toml.rs`) - oven-sh#39 — Unbounded brace expansion preallocation — cap expansion count at 65536 in `Bun.$` and `Bun.braces` (`BunObject.rs`, `Expansion.rs`) - oven-sh#31 — SCRAM PBKDF2 parameters accepted from server — clamp iteration count to `[4096, 10M]`, salt length to `[1, 1024]` (`PostgresSQLConnection.rs`) ### Auth / injection / traversal - oven-sh#19 — Cleartext password sent after TLS downgrade — require `TLSStatus::SslOk`, not just `ssl_mode != Disable` (`MySQLConnection.rs`) - oven-sh#83 — Strict TLS request reuses lax-verified pooled socket — track `established_with_reject_unauthorized` and refuse pool reuse for strict callers (`HTTPContext.rs`, `lib.rs`, `ClientSession.rs`) - oven-sh#73 — IPv6 loopback prefix auth bypass — exact-match `::1` instead of `starts_with` (`server_body.rs`) - oven-sh#56 — Unsanitized filename injects response headers — reject `\r`/`\n`/NUL/`"` in `content-disposition` filenames (`RequestContext.rs`) - oven-sh#43 — Missing CRLF checks for signed host/auth headers — also validate `region`, `access_key_id`, and `host` (`s3_signing/credentials.rs`) - oven-sh#34 — Bucket slash enables S3 host confusion — reject buckets containing `/` (`s3_signing/credentials.rs`) - oven-sh#25 — Lexical symlink check permits extraction escape — track created symlinks during extraction and refuse paths that traverse them (`libarchive/lib.rs`) - oven-sh#71 — bunx executes untrusted temp-cache binary — `lstat` cached binary; refuse symlinks and other-uid files (`bunx_command.rs`) ### Permission hygiene - #6 — Bin target chmod always sets mode 0777 — `0o777 & !umask` instead of `umask | 0o777` (`bin.rs`) - oven-sh#23 — Process umask cleared and never restored — restore umask after probing it in `ensure_umask` (`bin.rs`) ### Parser correctness - oven-sh#22 — Sign-prefixed scalar misparsed as infinity — fix Zig→Rust `&&`/`||` precedence transliteration (`yaml.rs`)
…0877) ## What Removes the ~1,750 stale "Phase A" / "Phase B" references the Zig→Rust port left across ~600 files. The port phases are complete; the references confuse what's a real TODO vs. a finished process step. Comments that encode real deferred work (e.g. `PERF(port): was X — profile in Phase B`) keep the substance and drop the phase framing (`PERF(port): was X — profile if hot.`). Comments that only describe past process steps are removed. Also fixes the trivial lint warnings cargo check surfaced along the way: unused imports, an unnecessary `unsafe` block over a safe `extern "C" fn`, `unreachable_pub` items, ambiguous glob re-exports, an unused `#[must_use]` result, and a private-type-in-public-alias. Two SAFETY/rustdoc comments that referenced API methods removed in a follow-up are rewritten to name the current entry points. ## What this is not No behavior changes. No public API changes. The hive-pool deprecation warnings (`HiveArrayFallback::get/try_get`) are not silenced here — the call sites are migrated to the safe API in a follow-up PR. ## Verification - `cargo check --workspace` clean for everything this PR touches - `cargo fmt --all` applied - `bun run rust:check-all` (all 6 target triples)
### What does this PR do? Removes unnecessary `str::from_utf8_unchecks` calls for static slices. ### How did you verify your code works? Trivial conversion.
…ror handling On Linux with SELinux/seccomp, directory reads can return EPERM or EACCES (errno 1/13) in addition to EACCES. Add PermissionDenied, AccessDenied, EPERM, and EACCES alongside the existing ENOENT/FileNotFound/ENOTDIR/NotDir handlers across the resolver so these errors are silently skipped rather than logged as 'Cannot read directory' noise. Affected locations: - resolver/lib.rs: readDirectoryError cache - resolver/fs.rs: readDirectoryError cache - resolver/resolver.rs: dir_queue_next directory scan - resolver/resolver.rs: readDirInfo directory listing - resolver/resolver.rs: tsconfig file lookup - resolver/package_json.rs: package.json file read
springmin
pushed a commit
that referenced
this pull request
May 25, 2026
…er (oven-sh#31333) ### Problem Fuzzing found a second transpiler stack overflow (`sig:SIGSEGV:nostack`): ~600 nested `{` blocks crash the process. ```js new Bun.Transpiler({ loader: "tsx", target: "bun", minifyWhitespace: true, deadCodeElimination: true }) .transformSync("{".repeat(600) + 'class Test1 { static "prop1" = 0; }' + "}".repeat(600)); ``` oven-sh#31242 guarded the **expression** recursion (`visit_expr_in_out`, `print_expr`, DCE helpers), but the **statement** recursion was left unguarded. Nested blocks stay under `MAX_STMT_DEPTH` (1000) in `parse_stmt`, then the visit pass recurses through `visit_stmts → visit_and_append_stmt → s_block → visit_stmts` with no stack check — each level stacks several multi-KB frames, so a few hundred levels exhaust the thread's stack (reproduces at depth 800 on a debug build's 8 MB main stack; smaller stacks crash at 600): ``` #5 visit_stmts src/js_parser/visit/mod.rs:1280 #6 s_block src/js_parser/visit/visit_stmt.rs:1627 #7 visit_and_append_stmt src/js_parser/visit/visit_stmt.rs:108 #8 visit_stmts src/js_parser/visit/mod.rs:1336 ... (repeats until SIGSEGV) ``` ### Fix Guard the statement recursion the same way the expression recursion already is: - `visit_and_append_stmt` now checks `stack_check.is_safe_to_recurse()` (plus the `reported_stack_overflow` fast-path) and reports "Maximum call stack size exceeded" instead of descending, mirroring `visit_expr_in_out`. - `print_stmt` and `print_if` (which self-recurses for `else if` chains without passing through `print_stmt`) get the same guard `print_expr`/`print_binding` already have, so a deep AST printed on a thread with less stack headroom errors instead of overflowing. - Removed the `MAX_STMT_DEPTH`/`parse_stmt_depth` hard cap from `parse_stmt` (review feedback): recursion depth in every phase is now governed by `StackCheck` alone, matching the Zig parser. - Guarded `hoist_symbols` the same way: it walks the scope tree before the visit pass at the full depth the parser allowed, and was only kept safe previously by the now-removed cap (the 15k-deep `lots-of-for-loop.js` fixture overflowed it in release builds otherwise). With this, every arbitrarily-nestable AST recursion (statements, expressions, bindings) is stack-checked in all three phases (parse, visit, print); deep inputs throw a catchable `Maximum call stack size exceeded` error. ### Verification New test `deeply nested statement blocks error instead of crashing the process` in `test/bundler/transpiler/transpiler.test.js` transpiles nested-block and `else if`-chain shapes at depths 600/800/990 (below the parse-time cap, deep enough to overflow an unguarded visitor) in a subprocess and asserts it exits cleanly. - Without the fix: the subprocess dies with SIGSEGV at depth 800+ (debug build), so the test fails. - With the fix: `bun bd test test/bundler/transpiler/transpiler.test.js` → 147 pass, 0 fail; the repro above now throws `Maximum call stack size exceeded`.
springmin
pushed a commit
that referenced
this pull request
Jun 18, 2026
…letes mid-read (oven-sh#31959) [publish images] Fixes a use-after-free in the HTTP client's proxy tunnel close path (Sentry BUN-2VY8, ~10 events/day on Windows release builds; reproduces deterministically under ASAN on all platforms). ## Repro `fetch()` through an HTTP CONNECT proxy to an HTTPS origin, where the origin's final response bytes and its TLS `close_notify` reach the client in a single TCP batch (origin writes the response and immediately closes). The regression test builds exactly that: a local CONNECT proxy that holds origin-to-client bytes after the handshake and flushes session tickets + response + close_notify in one write. On an unfixed ASAN build: ``` ERROR: AddressSanitizer: heap-use-after-free READ of size 8 thread T11 (HTTP Client) #0 Option<RefPtr<ProxyTunnel>>::as_ref #1 bun_http::proxy_tunnel::on_close src/http/ProxyTunnel.rs:525 #2 SSLWrapper<*mut HTTPClient>::trigger_close_callback src/uws/lib.rs:802 #3 SSLWrapper<*mut HTTPClient>::handle_reading src/uws/lib.rs:1022 #4 SSLWrapper<*mut HTTPClient>::handle_traffic #5 SSLWrapper<*mut HTTPClient>::receive_data #6 ProxyTunnel::receive src/http/ProxyTunnel.rs:751 freed by: AsyncHTTP::on_async_http_callback_raw src/http/AsyncHTTP.rs:813 HTTPClient::send_progress_update_without_stage_check src/http/lib.rs:3793 ``` ## Cause 1. `handle_reading` processes the batch: `SSL_read` returns the body bytes, the next `SSL_read` hits `close_notify` (`SSL_ERROR_ZERO_RETURN`), which sets `received_ssl_shutdown` and `sent_ssl_shutdown` before flushing the already-decrypted bytes through the data callback. 2. The data callback completes the response. The done path runs `close_proxy_tunnel(true)` -> `ProxyTunnel::shutdown()` -> `SSLWrapper::shutdown(true)`, which hits the already-shut-down early return (`sent_ssl_shutdown || fatal_error`) and returns **without setting `closed_notified`**. The result callback then frees the `ThreadlocalAsyncHTTP` embedding the `HTTPClient`, the exact pointer stored in the wrapper's `handlers.ctx`. 3. Control returns to `handle_reading`. Its liveness guard (`ssl.is_none() || closed_notified()`) passes because neither is set, so `trigger_close_callback()` invokes `on_close(handlers.ctx)` on the freed client. When the allocation has been recycled, `on_close` can ref or close a different request's tunnel instead of faulting. ## Fix `src/uws/lib.rs`: when `SSLWrapper::shutdown(fast_shutdown=true)` takes the already-shut-down early return, fire `trigger_close_callback()` (idempotent via `closed_notified`) so the wrapper is marked closed before the owner detaches and frees `handlers.ctx`. A fast shutdown is a full teardown, and the normal fast-shutdown path already fires the close callback unconditionally; this only closes the gap where the SSL-level shutdown had already happened. Graceful `shutdown(false)` (node:tls half-close via UpgradedDuplex / WindowsNamedPipe) is unchanged, so reads after a sent `close_notify` keep working. ## Verification New test in `test/js/bun/http/proxy.test.ts` (`test.skipIf(!isASAN)`, the UAF is only deterministic under ASAN): fails on an unfixed ASAN debug build with the heap-use-after-free above, passes with the fix. Full `proxy.test.ts` (46 tests) plus `node-tls-connect`, `node-tls-upgrade`, `node-tls-duplex-close-throw-uaf`, `node-tls-socket-allow-half-open-option`, `node-tls-server`, `fetch-tls-cert`, and `node-https-checkServerIdentity` suites pass. ## Note on the asan-lane CI failure (oven-sh#32144) The intermittent LeakSanitizer failure on the x64-asan shard (deferred napi finalizers parked on a never-drained cleanup-hook list at `bun test` exit) is being fixed in oven-sh#32146, which carries the same `global_exit()` drain plus a hooks-only guard that skips pending `napi_wrap` finalizers on undrained-loop exits. A subset version of that fix was briefly on this branch (e59bc1d) but without the hooks-only guard it made `test/js/third_party/duckdb/duckdb-basic-usage.test.ts` SEGV at exit on the asan lane (build 62135), exactly the failure mode oven-sh#32146's guard prevents, so it was reverted (61f9e70). This PR is scoped to the proxy-tunnel UAF; its asan lane can still intermittently hit the pre-existing oven-sh#32144 leak until oven-sh#32146 lands. ## Related PRs - oven-sh#30606 addresses the same crash signature but patches only the `.zig` reference files, which are no longer compiled; this PR fixes the shipping Rust implementation. - oven-sh#31952 fixes the same UAF by calling a new `mark_close_notified()` helper from `ProxyTunnel::shutdown` (silently setting the flag at one call site, with `close_raw` exempted). This PR instead closes the gap inside `SSLWrapper::shutdown(true)` itself, so every fast-shutdown caller (`ProxyTunnel::shutdown`, `ProxyTunnel::close_raw`, `UpgradedDuplex::close`, `WebSocketProxyTunnel::shutdown`) gets the same "no callbacks after teardown" guarantee without new wrapper API or a shutdown/close_raw asymmetry. The close callback is fired rather than suppressed, so the error teardown path keeps delivering `on_close` -> `close_and_fail` exactly once (idempotent via `closed_notified`). Test here is a deterministic single-shot repro (the test proxy reassembles TLS records and flushes tickets + response + close_notify in one write) rather than an iteration loop. --------- Co-authored-by: Ciro Spaciari MacBook <ciro@anthropic.com>
springmin
pushed a commit
that referenced
this pull request
Jun 27, 2026
…sweep (oven-sh#32729) ### Crash ``` ASSERTION FAILED: vm().currentThreadIsHoldingAPILock() => vm().heap.mutatorState() != MutatorState::Sweeping vendor/WebKit/Source/JavaScriptCore/runtime/JSCell.cpp(179) : bool JSC::JSCell::validateIsNotSweeping() const ``` Backtrace (from a release-asan build with asserts): ``` #3 JSC::JSCell::validateIsNotSweeping() #4 JSC::JSCell::classInfo() const #5 WTF::uncheckedDowncast<WebCore::JSResumableFetchSink>(JSValue const&) #6 ResumableFetchSinkPrototype__ondrainSetCachedValue #7 bun_runtime::webcore::fetch::fetch_tasklet::FetchTasklet::ignore_remaining_response_body #8 JSC::WeakBlock::sweep() <- inside GC sweep (Weak finalizer) #9 JSC::WeakSet::sweep() #10 JSC::PreciseAllocation::sweep() #12 JSC::Heap::finalize() oven-sh#21 JSC::LocalAllocator::allocateSlowCase oven-sh#23 JSC::ErrorInstance::create <- ordinary allocation kicked off GC ``` Found by the syscall fault-injection fuzzer's client-side grammar scenario (fetch/node:http with abort + transient errno on the client socket). Reproduces ~4/5 under `BUN_JSC_collectContinuously=1`. ### Cause `FetchTasklet::on_response_finalize` is the `WeakRefOwner<FetchResponse>::finalize` callback and runs inside `WeakBlock::sweep` while `MutatorState == Sweeping`. When the response body is `Locked` without a pending promise or stream it calls `ignore_remaining_response_body()`, which called: - `ResumableSink::detach_js()`: writes the sink wrapper's cached `ondrain` / `oncancel` / `stream` slots via the generated `ResumableFetchSinkPrototype__*SetCachedValue` helpers. Each does `uncheckedDowncast<JSResumableFetchSink>(thisValue)`, which reaches `JSCell::classInfo()` and then issues a write barrier on the wrapper cell. - `clear_stream_handlers()`: reaches `ReadableStreamTag__tagged` -> `object->inherits<JSReadableStream>()` (guarded today, but one boolean away). Calling `classInfo()` on any cell while the mutator is sweeping is forbidden: the cell's `Structure` may already have been swept. Assert builds catch it; release builds corrupt the heap. ### Fix Thread a `from_finalizer` flag through `ignore_remaining_response_body`. When `true` (the `on_response_finalize` caller) skip `detach_js()` and `clear_stream_handlers()`; only native state is touched. The sink's JS-side detach still happens from `clear_sink()` in `FetchTasklet::deinit()`, which runs as an event-loop `ConcurrentTask` outside any sweep, so nothing leaks. The `on_stream_cancelled_callback` caller (reader `.cancel()`, runs from JS on the event loop) passes `false` and keeps the immediate detach. Also corrects the `ResumableSink::detach_js` doc comment that claimed finalizer safety. ### Verification New test at `test/js/web/fetch/fetch-response-finalizer-sweep.test.ts`: a child process under `BUN_JSC_collectContinuously=1` does 12 iterations of `fetch()` with a user-constructed `ReadableStream` body (so the sink takes the JS route with a Strong `js_this`) against a raw TCP server that sends headers + a partial chunked body and never terminates it, then drops the `Response` unconsumed and runs `Bun.gc(true)`. Without the fix (`bun bd`, src/ stashed): ``` exitCode: 134 stderr: ASSERTION FAILED: vm().currentThreadIsHoldingAPILock() => vm().heap.mutatorState() != MutatorState::Sweeping ``` With the fix: `stdout: "ok"`, `exitCode: 0`. `test/js/web/fetch/fetch-backpressure.test.ts` (exercises the `on_stream_cancelled_callback` path) passes unchanged. --------- Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
springmin
pushed a commit
that referenced
this pull request
Jun 27, 2026
…type (oven-sh#32738) ### Repro ```js using listener = Bun.listen({ hostname: "127.0.0.1", port: 0, socket: { data() {} } }); await Bun.connect({ hostname: "127.0.0.1", port: listener.port, socket: { open(s) { s.setTypeOfService({}); } }, }); ``` On any assert build (debug, release-asan): ``` ASSERTION FAILED: isInt32() #4 JSC__JSValue__toInt32 #5 TCPSocketPrototype__setTypeOfService #6 WebCore::TCPSocketPrototype__setTypeOfServiceCallback ``` On plain release the assert compiles out and the NaN-boxed bits of the object handle are passed to `setsockopt(IP_TOS)` as the TOS byte. ### Cause `set_type_of_service` in `src/runtime/socket/socket_body.rs` called `args.ptr[0].to_int32()` on the raw argument. For non-numeric values that falls through to the C++ `JSC__JSValue__toInt32`, which is `JSC::JSValue::asInt32()` (the unchecked accessor that asserts `isInt32()`), not a coercing conversion. The `node:net` wrapper validates `tos` in JS before calling the handle, but the Bun-native `Bun.connect` socket exposes this prototype method directly with no JS validation layer. ### Fix Route the argument through `validate_integer_range` with `min: 0, max: 255, field_name: "tos"`, the same pattern the sibling `setKeepAlive` already uses for `initialDelay`. Non-numbers now throw `ERR_INVALID_ARG_TYPE`, out-of-range integers throw `ERR_OUT_OF_RANGE`, and non-integral numbers throw `ERR_INVALID_ARG_TYPE`, matching the `node:net` surface. I audited the other numeric setters on the TCPSocket/TLSSocket prototype (`timeout`, `setMaxSendFragment`, `write` offset/length) and the remaining `to_int32()` / `to_int64()` callers in `src/runtime/socket/`: each is already gated by `is_number()` / `is_any_int()` or routes through `coerce`. `setTypeOfService` was the only unguarded one. ### Verification New test in `test/js/bun/net/socket.test.ts` spawns a subprocess that calls `setTypeOfService` on a connected `Bun.connect` socket with `{}`, `"x"`, `-1`, `256`, `1.5`, and `0x10`, and asserts the error code for each plus that `getTypeOfService()` returns an integer. Without the fix the subprocess aborts (exit 134) on the first call; with the fix all seven checks pass. `test/js/node/test/parallel/test-net-socket-tos.js` continues to pass. Found by the bun-sys-fuzz API-grammar layer.
springmin
pushed a commit
that referenced
this pull request
Jun 27, 2026
…oven-sh#32742) ### What does this PR do? Fixes a use-after-free in the HTTP client's CONNECT proxy tunnel, caught by ASAN: ``` READ of size 8 at 0x61e00001fe80 thread T6 #0 Option<RefPtr<ProxyTunnel>>::as_ref #1 proxy_tunnel::on_close ProxyTunnel.rs:525 #2 SSLWrapper::trigger_close_callback uws/lib.rs:833 #3 SSLWrapper::handle_reading uws/lib.rs:1053 ... freed by thread T6 here (same stack, same `handle_reading` call): #5 AsyncHTTP::on_async_http_callback_raw AsyncHTTP.rs:819 #7 HTTPClient::send_progress_update_without_stage_check #9 proxy_tunnel::on_data ProxyTunnel.rs:350 #11 SSLWrapper::trigger_data_callback uws/lib.rs:824 #12 SSLWrapper::handle_reading uws/lib.rs:1046 ``` `SSLWrapper::handle_reading` flushes pending decrypted bytes to the data callback, then runs the close callback, guarded only by `closed_notified`: 1. The flushed data callback completes a keep-alive response through the tunnel. A fatal TLS record error sets only `fatal_error` — none of the shutdown flags — so the wrapper passed `tunnel_poolable`'s `!is_shutdown()` check and the tunnel was handed to the keep-alive pool. Nothing called `wrapper.shutdown()`, so `closed_notified` was never latched. Dispatching the final result then freed the `ThreadlocalAsyncHTTP` that embeds the `HTTPClient`. 2. The guard (`ssl.is_none() || closed_notified()`) passes. 3. `trigger_close_callback()` invokes `on_close(handlers.ctx)` with `ctx` pointing at the freed client. The pooling branch is the only terminal path that doesn't go through `close_proxy_tunnel(true)` → `wrapper.shutdown()` → `closed_notified`, which is the latch the read loop relies on. `SSLWrapper::shutdown` already special-cases the *close_notify* flavor of this for exactly that reason; the fatal-error flavor never reaches `shutdown()`. The fix is one predicate: a tunnel whose wrapper has a fatal error or pending unconsumed input/output is not poolable. That routes it through the orderly teardown that latches `closed_notified`, and the pending-I/O half closes the same hole for a tunnel pooled from a mid-loop data callback while more decrypted bytes or queued output remain. Both are also required for the pool to be correct on its own terms — a poisoned or dirty TLS session must not be handed to the next request. ### How did you verify your code works? New regression test in `test/js/bun/http/proxy.test.ts` (next to the existing close_notify sibling): an HTTPS keep-alive response through a CONNECT proxy with a corrupt TLS record appended to the same TCP burst, followed by a second request that can only complete if the HTTP client thread survived the first. Against an unfixed ASAN debug build the fixture aborts every run: ``` ==20981==ERROR: AddressSanitizer: heap-use-after-free on address 0x61e00001fe80 READ of size 8 at 0x61e00001fe80 thread T6 ... exit=134 ``` With this change it prints `4096 200 200` and exits 0 with no ASAN report. `test/js/bun/http/proxy.test.ts` (49/49), `fetch-proxy-connect-tunnel-split-envelope.test.ts`, `fetch-proxy-tls-intern-race.test.ts`, and `fetch-keepalive.test.ts` all pass.
springmin
pushed a commit
that referenced
this pull request
Jun 27, 2026
…oven-sh#32743) ## What `ReadableStream::from_pipe` (the `proc.stdout` / `proc.stderr` path for `Bun.spawn` and the shell subprocess) moves an already-registered pipe poll from the subprocess `PipeReader` into a freshly allocated `NewSource<FileReader>` and re-points the poll's owner at it. The across-read ref that keeps that box alive (`waiting_for_on_reader_done` + `increment_count()`, which upgrades `this_jsvalue` to `Strong`) was only taken in `FileReader::on_start`, i.e. the first time JS actually pulls from the stream. Between `from_pipe` and that first pull, the poll's owner points into a box whose only ref is the JS wrapper's own `Weak` back-reference. If the `Subprocess` and its cached stdout become unreachable before anyone pulls (a fire-and-forget spawn where `proc.stdout` is touched but never read, and the direct child exits while something else still holds the write end), GC sweeps the `JSFileInternalReadableStreamSource` wrapper and frees the `NewSource<FileReader>` box while the poll is still armed. The next readability or EOF event dispatches into freed memory: ``` READ of size 8 (heap-use-after-free) #0 Vec::len / is_empty (freed Vec<u8>) #2 webcore::file_reader::FileReader::on_reader_done FileReader.rs:1008 #3 bun_io::pipe_reader::read_socket{closure} PipeReader.rs:846 #4 PosixBufferedReader::read_socket PipeReader.rs:576 #5 file-poll dispatch <- posix_event_loop <- us_internal_dispatch_ready_polls freed by: JSC::JSDestructibleObjectDestroyFunc <- MarkedBlock sweep <- MarkedSpace::sweepBlocks allocated: ReadableStream::from_pipe<subprocess::PipeReader> -> NewSource<FileReader> ``` Found by a coverage-guided GC-stress fuzzer with syscall interposition (`BUN_JSC_collectContinuously=1` plus an injected `EAGAIN` to keep the read pending). In release builds this is silent heap corruption. ## Fix Take the across-read ref in `from_pipe` itself, immediately after the live reader is transferred and the JS wrapper is created, so the box is `Strong`-rooted for as long as the poll can fire. `on_reader_done` / `on_reader_error` release it exactly as before. `FileReader::on_start` now checks `waiting_for_on_reader_done` before taking the ref so the later `handle.start()` call from `lazyLoadStream` does not double-count on this path. ## How did you verify your code works? The test asserts the lifetime invariant directly via `heapStats().objectTypeCounts.FileInternalReadableStreamSource` rather than racing for the crash, since the exact UAF trigger depends on the fuzzer's syscall interposition. A detached grandchild (`sh -c 'while [ ! -e FLAG ]; do sleep 0.02; done; echo x'`) inherits the child's stdout and keeps the write end open past the direct child's exit, so the `FileReader`'s poll is still armed while we force GC with nothing in JS referencing the wrapper. - **Before** (`git stash push -- src/` + `bun bd test`): `duringLivePipe = 0` of 4; every wrapper swept while its poll owner still points into the freed box. - **After**: `duringLivePipe >= 4`; once the grandchildren exit and the pipes EOF, `afterEof <= 1` (one may remain via a conservatively-rooted final `Subprocess`, same caveat as `spawn-ipc-gc.test.ts`). Also passes `spawn-streaming-stdout.test.ts`, `spawn-unread-stdout-gc.test.ts`, `spawn-ipc-gc.test.ts`, `spawn-stdout-iterate-leak.test.ts`, and `readablestream-helpers.test.ts`. --------- Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
springmin
pushed a commit
that referenced
this pull request
Jun 29, 2026
…ll-driven read (oven-sh#32986) ## Problem Heap use-after-free in Bun Shell when `epoll_ctl` fails while re-registering a pipe's `FilePoll` from a poll-driven read. Found by syscall-fault-injection fuzzing against `origin/main`. Follow-up to oven-sh#32754, which fixed the same failure on the eager spawn-time read path. ``` ERROR: AddressSanitizer: heap-use-after-free READ of size 1, thread T0 #0 <bun_io::pipe_reader::BufferedReaderVTable>::link io/PipeReader.rs:105 #1 <bun_io::pipe_reader::BufferedReaderVTable>::on_read_chunk io/PipeReader.rs:125 #2 <bun_io::pipe_reader::PosixBufferedReader>::read_with_fn io/PipeReader.rs:890 #3 <bun_io::pipe_reader::PosixBufferedReader>::read_socket io/PipeReader.rs:576 #4 <bun_io::pipe_reader::PosixBufferedReader>::on_poll io/PipeReader.rs:529 #5 __bun_run_file_poll runtime/dispatch.rs:677 freed by: <alloc::sync::Arc<bun_runtime::shell::subproc::PipeReader>>::drop ``` ## Repro 1. `PipeReader::start` registers the poll and the eager spawn-time `read_all()` hits `EAGAIN`, so `read_with_fn`'s `EAGAIN` arm re-registers the poll and the spawn returns. 2. The child writes to stdout and the poll fires. `__bun_run_file_poll`'s `BUFFERED_READER` arm dispatches straight into `PosixBufferedReader::on_poll` with a bare `&mut *h` and no keepalive. 3. `read_with_fn` drains the chunk, `recv()` returns a real `EAGAIN`, and `register_poll()` issues another `epoll_ctl`, which fails (`ENOMEM` in the repro). 4. `register_poll` dispatches `on_reader_error`. The shell `PipeReader::on_reader_error` signals the `Cmd`, the `Readable::Pipe` `Arc` is dropped, and the callback's own `guard_from_raw` keepalive becomes the last reference. The code already documents this: "Dropping `guard` is the matching `deref()`; may free `this`." 5. Back in `read_with_fn`, the `EAGAIN` arm still delivers the drained head: `parent.vtable.on_read_chunk(.., ReadState::Drained)` reads the freed vtable. Traced with the test's `LD_PRELOAD` shim: ``` [shim] epoll_ctl(ADD fd=13) unix call#1 -> ok PipeReader::start [shim] recv(fd=13) unix call#1 -> EAGAIN eager read, inside spawn [shim] epoll_ctl(MOD fd=13) unix call#2 -> ok re-register; spawn returns [shim] recv(fd=13) unix call#2 poll fired: the child's bytes [shim] recv(fd=13) unix call#3 real EAGAIN [shim] epoll_ctl(MOD fd=13) unix call#3 -> ENOMEM register_poll fails [shell_subproc] PipeReader(0x..250) onReaderError errno: 12 [shell_subproc] PipeReader(0x..250, stdout) detach() [shell_subproc] PipeReader(0x..250, stdout) deinit() ==ERROR: AddressSanitizer: heap-use-after-free ``` ## Cause `register_poll()`'s failure path dispatches `on_reader_error`, which the `BufferedReaderParent` contract explicitly allows to free the parent, but `register_poll` gave the caller no way to know that happened. `read_with_fn`'s `EAGAIN` arm is the only call site that touches the reader afterwards; every other `register_poll()` is in tail position. The `SAFETY` comment above the `parent` rebind claimed the parent is "never freed mid-call", which holds for `on_read_chunk` re-entry but not for `on_reader_error`. oven-sh#32754 covered this exact sequence on the eager spawn-time entry by holding an `Arc<PipeReader>` across `start()` and `read_all()` in `Readable::start_pipe_reader`. The epoll dispatch has no equivalent keepalive, so the poll-driven entry was still exposed. ## Fix `PosixBufferedReader::register_poll()` now returns whether registration succeeded. `false` means `on_reader_error` was dispatched and `self` must not be touched again, so `read_with_fn`'s `EAGAIN` arm returns there instead of delivering the drained head to a possibly freed parent. The stream has already been completed with the registration error at that point, so nothing is lost. All other `register_poll()` call sites are tail calls and discard the result. ## Test Two new modes in `test/js/bun/shell/shell-pipe-read-fault.test.ts`'s `LD_PRELOAD` fault shim: - `SHELL_RECV_EAGAIN_FIRST=1`: the first `recv()` on each `AF_UNIX` socket returns `EAGAIN`, pushing the first successful read off the eager spawn-time `read_all()` and onto the epoll dispatch. - `SHELL_FAIL_EPOLL_FROM=N`: the Nth and later `epoll_ctl` `ADD`/`MOD` on each `AF_UNIX` socket fail with `ENOMEM`. `N=3` lets the initial registration and the eager read's re-registration succeed, then fails the first poll-driven one. The new test is `skipIf(!isASAN)` because the use-after-free is only reliably observable under ASAN. With `src/io/PipeReader.rs` reverted to `main` it fails in ~1.1s with the `heap-use-after-free` above; with the fix all 6 tests in the file pass. ## Out of scope Shell `PipeReader::on_read_chunk` also calls `self.reader.register_poll()` from a `&mut self` method whose stated contract is that it never frees `self`. If that inner registration fails, the same free can happen under `read_with_fn`'s mid-loop flush instead of its `EAGAIN` arm. Reaching it needs a large (>32 KB) burst in one poll wake; I have not reproduced it, so it is not changed here.
springmin
pushed a commit
that referenced
this pull request
Jun 29, 2026
…ed (oven-sh#33016) A backend message that fails the connection can share a TCP read with messages that follow it. `PostgresRequest::on_data`'s message loop had no bail-out once `fail()` had run, so the trailing messages in that read kept being dispatched against the already-failed connection. ### Repro A mock backend that answers the StartupMessage with one write carrying two messages: ``` R int32(8) int32(99) Authentication, unrecognized type Z int32(5) 'I' ReadyForQuery ``` ```ts const sql = new SQL({ url: `postgres://u@127.0.0.1:${port}/db`, max: 1, idleTimeout: 1, connectionTimeout: 5 }); await sql`select 1`.catch(() => {}); await Bun.sleep(1600); ``` ### Cause The unrecognized `Authentication` type calls `fail()`, which sets the status to `Failed`, closes the socket, and rejects the pending requests, but the message loop keeps going and dispatches the `ReadyForQuery` from the same read. That calls `set_status(Status::Connected)`, which has no guard against leaving `Failed`, so the dead connection is flipped back to `Connected` and the `on_data` epilogue re-arms its idle timer. uSockets frees a closed `us_socket_t` at the end of the event-loop iteration, so when the timer later fires, `ref_and_close` reads the freed socket: ``` ERROR: AddressSanitizer: heap-use-after-free READ of size 1 at 0x71f2125605d2 thread T0 #0 us_socket_is_closed packages/bun-usockets/src/socket.c:143:21 #4 PostgresSQLConnection::ref_and_close src/sql_jsc/postgres/PostgresSQLConnection.rs:1528:31 #5 PostgresSQLConnection::fail_with_js_value src/sql_jsc/postgres/PostgresSQLConnection.rs:726:14 #6 PostgresSQLConnection::fail_fmt src/sql_jsc/postgres/PostgresSQLConnection.rs:749:14 #7 PostgresSQLConnection::on_connection_timeout src/sql_jsc/postgres/PostgresSQLConnection.rs:557:14 #8 __bun_fire_timer src/runtime/dispatch.rs:1020:35 0x71f2125605d2 is located 18 bytes inside of 104-byte region freed by thread T0 here: #2 us_internal_free_closed_sockets packages/bun-usockets/src/loop.c:305:9 ``` ### Fix - `PostgresRequest::on_data`: the message loop returns once the connection's status is `Failed`. `fail()` is terminal; nothing after it in the same read should be handled (a `DataRow`, `CommandComplete`, or `ErrorResponse` in that position would be just as wrong as the `ReadyForQuery`). - `PostgresSQLConnection::set_status`: refuses to transition out of `Failed`. The transition function owns that invariant; every other consumer of `Status` (the timer interval, `update_has_pending_activity`, the idempotency check in `fail_with_js_value`) already assumes `Failed` is terminal. ### Verification `test/js/sql/postgres-failed-connection-resurrection.test.ts` runs a fixture against the mock backend above and lets it outlive the idle-timer window. Without the fix the fixture dies with the ASan report above; with it the fixture exits 0. Gated to ASan builds because the bug is a read of freed memory, which release lanes do not detect. The postgres fault-injection and integration suites still pass locally (90 tests across `test/js/sql/postgres-*.test.ts`, `sql*.test.ts`, `tls-sql.test.ts`). ### Related - oven-sh#32861 detaches the stored socket handle in `on_close` / `on_connect_error` so nothing can dereference the freed `us_socket_t` regardless of how the stale read is reached. It removes the last step of this chain from the other end; this PR stops the failed connection from being resurrected at all. - oven-sh#30950 guards the JS pool's `handleConnected` against the reverse ordering within one read (a legitimately queued `onconnect` microtask arriving after a synchronous `onclose`).
springmin
pushed a commit
that referenced
this pull request
Jul 2, 2026
…h#33186) ### Repro ```sh printf '{"name":"x","version":"1.0.0"}' > package.json bun pm pkg set 'contributors[0]=alice' ``` On a release build (1.4.0 and current `main`) this exits 0 and writes freed heap bytes into `package.json` as the property key: ```json { "name": "x", "version": "1.0.0", "P\x01\x00\x00\x00tors": { "\x00": "alice" } } ``` Depending on what was in the freed allocation the result is often not valid JSON at all. Any `bun pm pkg set` key path containing `[index]` hits it. Under ASAN it is a deterministic `heap-use-after-free`: ``` ERROR: AddressSanitizer: heap-use-after-free READ of size 1 #0 bun_js_printer::write_pre_quoted_string_inner src/js_printer/lib.rs:1014 #7 PmPkgCommand::save_package_json src/runtime/cli/pm_pkg_command.rs:909 freed by thread T0 here: #7 <Box<[u8]> as Drop>::drop #12 PmPkgCommand::set_value src/runtime/cli/pm_pkg_command.rs:661 previously allocated by thread T0 here: #10 <Box<[u8]> as From<&[u8]>>::from #11 PmPkgCommand::parse_key_path src/runtime/cli/pm_pkg_command.rs:583 ``` <details> <summary>full ASAN report</summary> ``` ================================================================= ==16563==ERROR: AddressSanitizer: heap-use-after-free on address 0x73423c7c0670 at pc 0x00000f583cc5 bp 0x7fff2667e950 sp 0x7fff2667e948 READ of size 1 at 0x73423c7c0670 thread T0 #0 0x00000f583cc4 in _RINvCs59Hqei94dXF_14bun_js_printer29write_pre_quoted_string_innerINtB2_16StdWriterAdapterQINtB2_6WriterNtB2_12BufferWriterEEKVNtNtB2_8Encoding4Utf8UECsgBGN0jRPILJ_11bun_bundler /workspace/bun/src/js_printer/lib.rs:1014:79 #1 0x00000ebda439 in <bun_js_printer::__gated_printer::Printer<&mut bun_js_printer::Writer<bun_js_printer::BufferWriter>, false, false, false, true, false>>::print_string_characters_utf8 /workspace/bun/src/js_printer/lib.rs:2641:21 #2 0x00000ebdb7a7 in <bun_js_printer::__gated_printer::Printer<&mut bun_js_printer::Writer<bun_js_printer::BufferWriter>, false, false, false, true, false>>::print_string_characters_e_string /workspace/bun/src/js_printer/lib.rs:4546:22 #3 0x00000ebdb238 in <bun_js_printer::__gated_printer::Printer<&mut bun_js_printer::Writer<bun_js_printer::BufferWriter>, false, false, false, true, false>>::print_string_literal_e_string /workspace/bun/src/js_printer/lib.rs:3018:18 #4 0x00000ebd2be2 in <bun_js_printer::__gated_printer::Printer<&mut bun_js_printer::Writer<bun_js_printer::BufferWriter>, false, false, false, true, false>>::print_property /workspace/bun/src/js_printer/lib.rs:4807:34 #5 0x00000ebc0166 in <bun_js_printer::__gated_printer::Printer<&mut bun_js_printer::Writer<bun_js_printer::BufferWriter>, false, false, false, true, false>>::print_expr /workspace/bun/src/js_printer/lib.rs:3962:38 #6 0x00000ee574c1 in bun_js_printer::print_json::<&mut bun_js_printer::Writer<bun_js_printer::BufferWriter>> /workspace/bun/src/js_printer/lib.rs:8071:13 #7 0x00000c05c270 in <bun_runtime::cli::pm_pkg_command::PmPkgCommand>::save_package_json /workspace/bun/src/runtime/cli/pm_pkg_command.rs:909:25 #8 0x00000c060cfb in <bun_runtime::cli::pm_pkg_command::PmPkgCommand>::exec_set /workspace/bun/src/runtime/cli/pm_pkg_command.rs:330:13 #9 0x00000c05d333 in <bun_runtime::cli::pm_pkg_command::PmPkgCommand>::exec /workspace/bun/src/runtime/cli/pm_pkg_command.rs:73:32 #10 0x00000bf919d0 in <bun_runtime::cli::package_manager_command::PackageManagerCommand>::exec /workspace/bun/src/runtime/cli/package_manager_command.rs:704:13 #11 0x00000c3fbb87 in bun_runtime::cli::command::exec_pm /workspace/bun/src/runtime/cli/mod.rs:1591:34 #12 0x00000c3f2b86 in bun_runtime::cli::command::start /workspace/bun/src/runtime/cli/mod.rs:1309:43 #13 0x00000bfad16c in bun_runtime::cli::cli::start /workspace/bun/src/runtime/cli/mod.rs:573:27 #14 0x00000bb3c034 in main /workspace/bun/src/bun_bin/lib.rs:230:5 #15 0x77223ccc7ca7 in __libc_start_call_main csu/../sysdeps/nptl/libc_start_call_main.h:58:16 oven-sh#16 0x77223ccc7d64 in __libc_start_main csu/../csu/libc-start.c:360:3 oven-sh#17 0x0000099d1d1d in __wrap___libc_start_main /workspace/bun/build/debug/../../src/jsc/bindings/workaround-missing-symbols.cpp:487:12 0x73423c7c0670 is located 0 bytes inside of 12-byte region [0x73423c7c0670,0x73423c7c067c) freed by thread T0 here: #0 0x000007ae192a in free crtstuff.c #1 0x00000bb3c5a7 in <std::alloc::System as core::alloc::global::GlobalAlloc>::dealloc /root/.rustup/toolchains/nightly-2026-05-06-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/sys/alloc/unix.rs:48:18 #2 0x00000bb3be9a in __rustc::__rust_dealloc /workspace/bun/src/bun_bin/lib.rs:56:15 #3 0x00001258b05f in alloc::alloc::dealloc_nonnull /root/.rustup/toolchains/nightly-2026-05-06-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/alloc/src/alloc.rs:128:14 #4 0x0000125872fe in <alloc::alloc::Global>::deallocate_impl_runtime /root/.rustup/toolchains/nightly-2026-05-06-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/alloc/src/alloc.rs:229:22 #5 0x000012586364 in <alloc::alloc::Global>::deallocate_impl /root/.rustup/toolchains/nightly-2026-05-06-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/alloc/src/alloc.rs:344:9 #6 0x00001258d79c in <alloc::alloc::Global as core::alloc::Allocator>::deallocate /root/.rustup/toolchains/nightly-2026-05-06-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/alloc/src/alloc.rs:462:23 #7 0x000012582946 in <alloc::boxed::Box<[u8]> as core::ops::drop::Drop>::drop /root/.rustup/toolchains/nightly-2026-05-06-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/alloc/src/boxed.rs:1956:24 #8 0x000012572e44 in core::ptr::drop_in_place::<alloc::boxed::Box<[u8]>> /root/.rustup/toolchains/nightly-2026-05-06-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/ptr/mod.rs:809:1 #9 0x000011f8d429 in core::ptr::drop_in_place::<[alloc::boxed::Box<[u8]>]> /root/.rustup/toolchains/nightly-2026-05-06-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/ptr/mod.rs:809:1 #10 0x00000ef6b73a in <alloc::vec::Vec<alloc::boxed::Box<[u8]>> as core::ops::drop::Drop>::drop /root/.rustup/toolchains/nightly-2026-05-06-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/alloc/src/vec/mod.rs:4258:13 #11 0x00000ef69e64 in core::ptr::drop_in_place::<alloc::vec::Vec<alloc::boxed::Box<[u8]>>> /root/.rustup/toolchains/nightly-2026-05-06-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/ptr/mod.rs:809:1 #12 0x00000c061846 in <bun_runtime::cli::pm_pkg_command::PmPkgCommand>::set_value /workspace/bun/src/runtime/cli/pm_pkg_command.rs:661:5 #13 0x00000c061038 in <bun_runtime::cli::pm_pkg_command::PmPkgCommand>::exec_set /workspace/bun/src/runtime/cli/pm_pkg_command.rs:325:13 #14 0x00000c05d333 in <bun_runtime::cli::pm_pkg_command::PmPkgCommand>::exec /workspace/bun/src/runtime/cli/pm_pkg_command.rs:73:32 #15 0x00000bf919d0 in <bun_runtime::cli::package_manager_command::PackageManagerCommand>::exec /workspace/bun/src/runtime/cli/package_manager_command.rs:704:13 oven-sh#16 0x00000c3fbb87 in bun_runtime::cli::command::exec_pm /workspace/bun/src/runtime/cli/mod.rs:1591:34 oven-sh#17 0x00000c3f2b86 in bun_runtime::cli::command::start /workspace/bun/src/runtime/cli/mod.rs:1309:43 oven-sh#18 0x00000bfad16c in bun_runtime::cli::cli::start /workspace/bun/src/runtime/cli/mod.rs:573:27 oven-sh#19 0x00000bb3c034 in main /workspace/bun/src/bun_bin/lib.rs:230:5 oven-sh#20 0x77223ccc7ca7 in __libc_start_call_main csu/../sysdeps/nptl/libc_start_call_main.h:58:16 previously allocated by thread T0 here: #0 0x000007ae1bc8 in malloc crtstuff.c #1 0x00000bb3c520 in <std::alloc::System as core::alloc::global::GlobalAlloc>::alloc /root/.rustup/toolchains/nightly-2026-05-06-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/sys/alloc/unix.rs:14:22 #2 0x00000bb3be30 in __rustc::__rust_alloc /workspace/bun/src/bun_bin/lib.rs:56:15 #3 0x00001258b335 in alloc::alloc::alloc /root/.rustup/toolchains/nightly-2026-05-06-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/alloc/src/alloc.rs:101:9 #4 0x000012586b81 in <alloc::alloc::Global>::alloc_impl_runtime /root/.rustup/toolchains/nightly-2026-05-06-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/alloc/src/alloc.rs:210:73 #5 0x0000125862b6 in <alloc::alloc::Global>::alloc_impl /root/.rustup/toolchains/nightly-2026-05-06-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/alloc/src/alloc.rs:332:9 #6 0x00001258d86a in <alloc::alloc::Global as core::alloc::Allocator>::allocate /root/.rustup/toolchains/nightly-2026-05-06-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/alloc/src/alloc.rs:449:14 #7 0x00001257dbd3 in <alloc::boxed::Box<[u8]>>::try_clone_from_ref_in /root/.rustup/toolchains/nightly-2026-05-06-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/alloc/src/boxed.rs:881:29 #8 0x00001257da49 in <alloc::boxed::Box<[u8]>>::clone_from_ref_in /root/.rustup/toolchains/nightly-2026-05-06-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/alloc/src/boxed.rs:840:15 #9 0x00001257d3f4 in <alloc::boxed::Box<[u8]>>::clone_from_ref /root/.rustup/toolchains/nightly-2026-05-06-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/alloc/src/boxed.rs:793:9 #10 0x000012581e34 in <alloc::boxed::Box<[u8]> as core::convert::From<&[u8]>>::from /root/.rustup/toolchains/nightly-2026-05-06-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/alloc/src/boxed/convert.rs:77:9 #11 0x00000c05a47f in <bun_runtime::cli::pm_pkg_command::PmPkgCommand>::parse_key_path /workspace/bun/src/runtime/cli/pm_pkg_command.rs:583:37 #12 0x00000c061608 in <bun_runtime::cli::pm_pkg_command::PmPkgCommand>::set_value /workspace/bun/src/runtime/cli/pm_pkg_command.rs:643:30 #13 0x00000c061038 in <bun_runtime::cli::pm_pkg_command::PmPkgCommand>::exec_set /workspace/bun/src/runtime/cli/pm_pkg_command.rs:325:13 #14 0x00000c05d333 in <bun_runtime::cli::pm_pkg_command::PmPkgCommand>::exec /workspace/bun/src/runtime/cli/pm_pkg_command.rs:73:32 ``` </details> ### Cause `parse_key_path` returned a `Vec<Box<[u8]>>`, and `set_value` / `set_nested` inserted those boxed segments into the manifest AST by reference: `E::Object::put` constructs `EString::init(key)`, whose documented contract is that `key` is arena-owned (it records the slice, it does not copy it). The vector is a local of `set_value`, so it dropped before `exec_set` reached `save_package_json`, and the JSON printer then read the dangling keys. The non-bracket path in `set_value` did not have the bug: it borrowed its segments straight out of the argv key, which outlives the whole command. The bracket path differed only by the unnecessary boxing. ### Fix `parse_key_path` now returns `Vec<&[u8]>`. Every segment is a literal sub-slice of the input key, so nothing ever needed owning. With the boxing gone, `set_value`'s separate non-bracket branch and its `set_nested_simple` helper (which existed only to avoid the allocation) were exact duplicates of the bracket path, so they are deleted and all keys route through `parse_key_path` + `set_nested`. `set_nested_simple`'s trailing `root.put(current_key, nested)` was a no-op: `ExprData::EObject` is a `StoreRef` handle, so mutating the copy returned by `root.get()` already mutates the stored object, and the put re-stores the same handle. Dropping it with the function changes nothing (and the prior bracket path, `set_nested`, never had it). Intentionally not changed here: `set 'contributors[0]=alice'` produces `"contributors": {"0": "alice"}`, an object keyed by the digit string, rather than the array npm's `pkg set` creates, and `set 'array[]=x'` still errors with `InvalidPath` instead of appending. Both are the npm compat gap tracked in oven-sh#22035, which is separate from the memory safety of the key names and is not closed by this PR. ### Verification New test in `test/cli/install/bun-pm-pkg.test.ts` reparses the written file and asserts the exact object. Without the fix it fails on release (`SyntaxError: JSON Parse error: Invalid escape character x`) and on the ASAN debug build (the child aborts on the use-after-free). With the fix the full `bun-pm-pkg.test.ts` suite passes (74 pass, 0 fail).
springmin
pushed a commit
that referenced
this pull request
Jul 4, 2026
…buffer cannot be allocated (oven-sh#33326) Fixes a `Segmentation fault at address 0x00000040` (sometimes `0x00000030`) reported from Windows x64 builds, crashing inside boringssl's record copy from uSockets' TLS read loop: ``` memcpy src/vctools/crt/vcruntime/src/string/amd64/memcpy.asm bssl::OPENSSL_memcpy vendor/boringssl/crypto/internal.h:868 SSL_peek vendor/boringssl/ssl/ssl_lib.cc:947 SSL_read vendor/boringssl/ssl/ssl_lib.cc:918 us_internal_ssl_on_data packages/bun-usockets/src/crypto/openssl.c:1797 us_internal_dispatch_ready_poll packages/bun-usockets/src/loop.c:600 uv__fast_poll_process_poll_req vendor/libuv/src/win/poll.c:208 uv_run vendor/libuv/src/win/core.c:737 ``` ## Cause `us_internal_init_loop_ssl_data` (`openssl.c:677`) allocates one 512 KiB plaintext buffer per event loop, lazily, on the loop's first TLS socket, and never checked the result: ```c loop_ssl_data->ssl_read_output = us_malloc(LIBUS_RECV_BUFFER_LENGTH + LIBUS_RECV_BUFFER_PADDING * 2); ``` With `ssl_read_output == NULL`, every later `SSL_read` hands boringssl ```c loop_ssl_data->ssl_read_output + LIBUS_RECV_BUFFER_PADDING + read ``` as its plaintext destination, so the first record of application data memcpy's to `NULL + 32`. `SSL_peek`'s `OPENSSL_memcpy(buf, ...)` at `ssl_lib.cc:947` is the write, and the access violation confirms it is a write fault. `0x30`/`0x40` rather than `0x20` is memcpy's destination-alignment preamble (`dst += VEC_SIZE; dst &= ~(VEC_SIZE - 1)`), which moves the first faulting store for copies larger than eight vector registers. Measured on the copy sizes a real TLS record produces: | memcpy variant | first faulting store for `dst = NULL + 32` | | --- | --- | | 32-byte vectors (AVX) | `0x40` | | 16-byte vectors (SSE) | `0x30` | So the two strikingly stable fault addresses are just CPU dispatch across the affected machines, and `read` is always `0`: the crash is always the connection's first record of application data. Only Windows reports it because Linux and macOS overcommit, so a 512 KiB `malloc` there effectively never returns NULL. Windows fails the commit cleanly, and the loop's much smaller `us_calloc` still succeeds out of an already-committed page, leaving exactly the observed shape: a valid `loop_ssl_data` whose `ssl_read_output` is NULL. ## Fix - Null-check the buffer allocation, the `us_calloc` of `loop_ssl_data`, and the `BIO_meth_new`/`BIO_new` calls beside them, and route the failure through Bun's out-of-memory crash path (`Bun__outOfMemory`, new C entry point next to `Bun__panic`). The process now dies with `Bun ran out of memory` and a stack trace that names the allocation, instead of faulting on the first TLS byte. - Apply the same check to the sibling site: `recv_buf`/`send_buf` in `us_internal_loop_data_init` are the same unchecked `malloc(LIBUS_RECV_BUFFER_LENGTH + LIBUS_RECV_BUFFER_PADDING * 2)`. A NULL `recv_buf` does not fault, it makes every read on the loop fail with `EFAULT` for the life of the process, which is worse to diagnose. - `us_internal_free_loop_ssl_data` left `loop->data.ssl_data` dangling, which defeats the `if (!loop->data.ssl_data)` guard the init function relies on. It now clears the field. A 512 KiB `malloc` effectively never returns NULL on an overcommitting kernel, so the failure path needs the existing socket fault injector to be reachable from a test. This adds an `ssl_loop_buffer` rule to it, which like the rest of the injector is compiled out of release builds. ## Verification The new test spawns a child that arms `ssl_loop_buffer` before its first TLS socket and asserts it reports out of memory rather than reaching a read loop. Reverting only `if (!loop_ssl_data->ssl_read_output) Bun__outOfMemory();` reproduces the reported crash exactly, on Linux, from that same fixture: same fault address, same frames, same boringssl source lines. <details> <summary>Reproduction on the unfixed build (<code>bun bd</code>, ASAN)</summary> ``` ==19592==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000040 ==19592==The signal is caused by a WRITE memory access. ==19592==Hint: address points to the zero page. #0 __memcpy_evex_unaligned_erms #1 bssl::OPENSSL_memcpy(void*, void const*, unsigned long) vendor/boringssl/crypto/internal.h:868:10 #2 SSL_peek vendor/boringssl/ssl/ssl_lib.cc:947:3 #3 SSL_read vendor/boringssl/ssl/ssl_lib.cc:918:13 #4 us_internal_ssl_on_data packages/bun-usockets/src/crypto/openssl.c:1847:21 #5 us_internal_dispatch_ready_poll packages/bun-usockets/src/loop.c:625:38 ``` With the fix: ``` panic(main thread): Bun ran out of memory Bun__outOfMemory src/bun_bin/phase_c_exports.rs:81:5 us_internal_init_loop_ssl_data packages/bun-usockets/src/crypto/openssl.c:696:42 us_internal_ssl_attach packages/bun-usockets/src/crypto/openssl.c:1275:3 ``` </details> `test/js/node/tls/tls-syscall-fault.test.ts` (11 pass), `test/js/bun/util/socket-fault-injection.test.ts` (15 pass), plus `socket-syscall-fault`, `serve-syscall-fault` and `fetch-syscall-fault` (19 pass) are green. The three failures in `test/js/node/tls/` on this machine are pre-existing: two also fail on an unmodified 1.4.0, and `tls.connect should ignore invalid NODE_EXTRA_CA_CERTS` takes 5.75s, just over the 5s local default (CI triples the per-test timeout for ASAN builds). ## Teardown audit The report also asked whether a socket can reach `us_internal_ssl_on_data` after its loop's SSL data has been freed. `us_internal_free_loop_ssl_data` is only reachable from `us_loop_free`, and the only loop Bun frees today is `SpawnSyncEventLoop`'s, which never has a TLS socket attached (its `loop->data.ssl_data` is always NULL, so the free is a no-op). So that is not the cause here. It is worth noting separately that the libuv `us_loop_free` (`eventing/libuv.c:201-206`) calls `us_internal_loop_data_free(loop)` and *then* runs `uv_run(loop->uv_loop, UV_RUN_NOWAIT)`, which is a full libuv iteration that can dispatch socket poll callbacks into the just-freed `recv_buf` and `ssl_data`. The POSIX `us_loop_free` (`eventing/epoll_kqueue.c:56-60`) has no such window. It is unreachable today for the reason above, so it is left out of this PR rather than changing loop teardown ordering without a test that can exercise it.
springmin
pushed a commit
that referenced
this pull request
Jul 20, 2026
…ven-sh#34693) ## Use-after-free in `H2FrameParser::on_native_writable` Fleet ASAN fuzz hit (p-h2c cleartext harness, seed 1, `server-conn.recv.*:A8`): ``` use-after-poison READ 8 (shadow f7 = user poison, HiveArray slot re-poison) #0 Vec::len (write_buffer) #2 has_backpressure h2_frame_parser.rs:3276 #3 on_native_writable h2_frame_parser.rs:9605 #4 NewSocket<true>::on_writable socket_body.rs:894 #8 us_internal_ssl_on_writable bun-usockets openssl.c:1851 allocated by: HiveArray Fallback<H2FrameParser,256>, H2FrameParser::constructor ``` ### Cause `on_native_writable` loops `flush()` and checks `has_backpressure()` between iterations. `flush()` re-enters JS via `flush_stream_queue` -> `dispatch_write_callback` / `onStreamEnd` / `onWantTrailers`. A callback that destroys the session reaches `detach_native_callback`, dropping the socket's `+1` on the parser. If that was the last external ref, `flush()`'s own keepalive is all that remains and drops on return, so the next `has_backpressure()` reads a HiveArray slot that was just `drop_in_place`'d and re-poisoned by `POOL.put`. `on_native_read` already takes a `keepalive()` for exactly this reason (h2_frame_parser.rs:9590); `on_native_writable` did not. In release builds there is no poison: the same ordering is a silent use-after-free in every `node:http2` server/client on a native socket. The read of a stale `write_buffer.len()` can satisfy the loop condition and send the next `flush()` into UAF writes on the freed parser. ### Fix - Take a `keepalive()` for the extent of `on_native_writable`, mirroring `on_native_read`. - `NativeCallbacks::on_data`/`on_writable`: copy the raw `*mut H2FrameParser` out of the enum before dispatching, so the `JsCell<NativeCallbacks>` borrow does not span a re-entrant `detach_native_callback` that overwrites the cell. ### Test `test/js/node/http2/node-http2-writable-destroy-fixture.ts` reproduces the exact fleet stack under ASAN by faulting `send`/`writev` to 0 (backpressure, arms WRITABLE), queuing a DATA frame whose write callback runs `session.destroy()` + `Bun.gc(true)`, then clearing the fault so the writable event drains the queue inside `on_native_writable`. Added to `node-http2-syscall-fault.test.ts` as an ASAN-gated subprocess test. <details><summary>Fail-before ASAN report (matches the fleet hit)</summary> ``` ==ERROR: AddressSanitizer: use-after-poison on address 0x... READ of size 8 at 0x... thread T0 #2 H2FrameParser::has_backpressure h2_frame_parser.rs:3276:33 #3 H2FrameParser::on_native_writable h2_frame_parser.rs:9605:21 #4 NativeCallbacks::on_writable socket_body.rs:3960:20 #5 NewSocket<false>::on_writable socket_body.rs:894:39 allocated by thread T0 here: ... Fallback<H2FrameParser, 256>::new_boxed hive_array.rs:668 ... H2FrameParser::constructor h2_frame_parser.rs:9800 SUMMARY: AddressSanitizer: use-after-poison ... Vec<u8>::len ``` </details> <!-- robobun:evidence:begin --> --- **no test proof** · iteration 2 · Platform-specific test(s) that do not run on this machine. Deferring to CI, which covers all platforms: test/js/node/http2/node-http2-syscall-fault.test.ts <!-- robobun:evidence:end --> --------- Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
springmin
pushed a commit
that referenced
this pull request
Jul 23, 2026
oven-sh#35255) `test/js/bun/http/serve-protocols.test.ts` has been going red on main (build 78445 darwin-x64 hard, 78462 debian-11-aarch64 hard, plus 78300/78419 with retries), always as ``` error: HTTP3StreamReset fetching "https://127.0.0.1:<port>/echo" ✗ Bun.serve over http/3 > POST echo 1000000 bytes [20169.37ms] ``` Reproduced on Linux by looping the file: the h3 subset alone fails about 17 of 100 runs. ## Cause The ten concurrent h3 tests share one lsquic client engine and one unconnected UDP socket. `bsd_create_udp_socket()` sets `IP_RECVERR` on every UDP socket (for `node:dgram`'s error surfacing, oven-sh#28827), including QUIC's. When a finished test `proc.kill()`s its server, the client session is still in the engine and keeps scheduling retransmits / `NEW_CONNECTION_ID` to an unbound port. With `IP_RECVERR` on, the resulting ICMP port-unreachable is queued on the shared socket and the next `sendmmsg` returns `-1 ECONNREFUSED`, even though that call is sending a datagram to a live peer. `us_quic_packets_out` reports that as a short return, lsquic clears `ENPUB_CAN_SEND` for the whole engine and only its one-second `resume_sending_at` failsafe re-enables it. With several dead sessions generating ICMPs, every failsafe retry fails the same way and the live 1MB upload never advances; the 20s in CI is two idle-timeout rounds through `retry_or_fail`. While tracing that I also found an unsigned underflow in lsquic's `send_batch` requeue loop: when the first unsent spec in a batch coalesces multiple packets (`pack_off[0] == 0`, `iovlen > 1`), `end = &batch->packets[off - 1]` indexes with `UINT_MAX` and only the last packet of the coalesced group is returned to the connection. The earlier ones are the INIT ACK and the HSK CRYPTO carrying the client Finished, so the peer can never complete the handshake. This is the same hang reached from a different direction (real EAGAIN backpressure instead of stale ICMP). ## Fix - `IP_RECVERR` is now opt-in via `LIBUS_UDP_LINUX_RECVERR`, set by `us_create_udp_socket` when a `recv_error_cb` is provided. `node:dgram` always passes one and keeps the option; QUIC passes `NULL` and no longer gets it. This matches libuv's `UV_UDP_LINUX_RECVERR` gating that `bsd.c` already cited. - `us_quic_packets_out()` retries once on a non-`EAGAIN`/`ENOBUFS` send failure before reporting a short return, so a stale `sk_err` that does surface cannot pause the engine. Both the `sendmmsg` and per-packet paths now go through `US_FAULT_CHECK(US_FAULT_SENDMSG, ...)` so the short-return path is reachable from tests. - `patches/lsquic/requeue-unsent-coalesced.patch` rewrites the requeue loop's bounds as `[off, off+count)` so every packet in an unsent coalesced datagram is returned to the connection. The same underflow is present in upstream lsquic master; I will open a PR there separately. - `serve-protocols.test.ts` now stops each fixture server gracefully on stdin close (`server.stop(true)`), matching `serve-http3.test.ts`, so the pooled client session sees `CONNECTION_CLOSE` instead of leaving the engine retransmitting to unbound ports. - `test/js/web/fetch/fetch-http3-syscall-fault.test.ts` injects `EAGAIN` on the coalesced handshake datagram (the `pack_off[0]==0`, `iovlen>1` spec the lsquic patch fixes), a one-shot `ECONNREFUSED` that the retry-once branch consumes, and a burst of `EAGAIN` that the `on_drain` path recovers from. ## Verification Release build, looped: | | before | after | | --- | --- | --- | | `serve-protocols -t "http/3"` | 17/100 fail | 2/100 fail | | `serve-protocols` (full) | 7/100 fail | 4/200 fail | Debug+ASAN: `serve-protocols`, `serve-http3` (46), `fetch-http3-client` (52), `fetch-http3-adversarial` (29), `fetch-http3-syscall-fault` (3) and `dgram.test.ts` all pass, 211 tests total. The residual ~1-2% is a separate pre-existing bug (the client's 36-byte HSK CRYPTO is buffered but never flushed when `drain_send_body` writes the whole 1MB body synchronously from `on_stream_open`); I've handed that off as its own issue. With CI's retry it is well under the flake threshold. ### Gate note The fault-injection hook that makes the new test deterministic lives in `packages/bun-usockets/src/quic.c`, so `git stash -- src/ packages/` removes it along with the fix and the fault never fires. The lsquic piece lives in `patches/` and `scripts/`, which the stash does not touch. That means a single stashed run passes (no fault, no stall) and a single unstashed run passes (fault fires, fix handles it), and the gate cannot distinguish them mechanically. The 300-iteration probe above is the evidence; the fault-injection tests pin the behavior going forward. <details> <summary>lsquic debug trace of the stall</summary> ``` engine: packets out returned 0 (out of 1) [C919…] event: unsent packet #15 ACK_FREQUENCY, size 36 [C919…] sendctl: packet #15 has been delayed engine: send_packets_out: sent 0 packets … <- no "can send again"; nothing for 1s engine: failsafe activated: resume sending packets again after timeout engine: packets out returned 0 (out of 10) <- fails again, live conn's oven-sh#207 included ``` and for the underflow, a batch with `pack_off[0]=0`, `iovlen[0]=3`: ``` engine: packets out returned 0 (out of 2) event: unsent packet #3 ACK PADDING, size 1059 event: unsent packet #4 ACK CRYPTO, size 87 event: unsent packet #5 NEW_CONNECTION_ID, size 54 event: unsent packet #6 STREAM, size 114 sendctl: packet #6 has been delayed sendctl: packet #5 has been delayed … <- #3 and #4 never requeued [WARN] sendctl: send history gap 2 - 5 ``` </details> <!-- robobun:evidence:begin --> --- **no test proof** · iteration 1 · Platform-specific test(s) that do not run on this machine. Deferring to CI, which covers all platforms: test/js/web/fetch/fetch-http3-syscall-fault.test.ts <!-- robobun:evidence:end --> --------- Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
springmin
pushed a commit
that referenced
this pull request
Jul 29, 2026
…rier (oven-sh#36337) `JSNativeStreamSourceAdapter::m_controller` was a `JSC::Weak<JSReadableStreamDefaultController>`. When the native pull promise is rejected (socket fault on a fetch body) the adapter is queued as the `onNativePullRejected` reaction context, which roots the **adapter** but not the **controller**: the adapter's only edge to it was the `Weak`. `FetchTasklet` releases both native `Strong<>`s to the body stream before that microtask drains, so a GC in between can leave the entire consumer graph (`controller -> stream -> reader -> pipe op -> destination -> writer -> readyPromise`) white. The subsequent error cascade then enqueues the pipe's writes-drained shutdown deferral against a corpse `op`, and `performPipeShutdownAction(AbortDestination)` dereferences a swept `readyPromise`: ``` ASSERTION FAILED: result JSObject.h(583) JSGlobalObject *JSC::JSObject::realm() const #5 JSC::JSObject::realm() #6 JSC::JSPromise::rejectPromise #7 JSC::JSPromise::reject #8 Bun::WebStreams::writableStreamDefaultWriterEnsureReadyPromiseRejected #9 Bun::WebStreams::writableStreamStartErroring #10 Bun::WebStreams::writableStreamAbort #11 WebCore::performPipeShutdownAction (AbortDestination) #12 WebCore::JSStreamPipeToOperation::onWritesFinishedForShutdown ``` On builds without the assert the same path is a silent write into freed/reused promise memory. ## Fix Hold `m_controller` as a visited internal field so a queued adapter roots the controller directly. The edge is cleared on every terminal path (`nativeSourcePullRejected`, `nativeSourceCallClose`, `nativeSourceCancel`); `controller->algorithmContext` is cleared by `readableStreamDefaultControllerClearAlgorithms`, so the abandoned case is an ordinary intra-heap cycle mark-sweep collects. `NewSource::this_jsvalue` is only `Strong` during FileReader I/O, where pinning the consumer graph is the correct behavior anyway. With the `Weak` gone the adapter no longer needs a destructor, so it is now a `JSInternalFieldObjectImpl<5>`: the five JSValue members (handle, pendingView, closer, drainValue, controller) are internal fields visited by the base class, with typed accessors at call sites. The scalar members (chunkSize, flag bitfield, text-decode state) stay as plain members. ## Verification `native-source-onclose-leak.test.ts` (the partial-read + `releaseLock` abandonment tests for Blob/fetch/File sources) continues to pass, confirming the cycle does not pin. `streams.test.js`, `pipeTo-signal-leak.test.ts`, `compression.test.ts`, `blob.test.ts` all pass. The crash itself is 0/1800 standalone; it reproduces ~1/3 only under a fault-injected tracer replay. `pipeTo-shutdown-gc.test.ts` exercises the shape (native body source, socket fault mid-stream, fire-and-forget `pipeTo` under `collectContinuously`, `AbortDestination` shutdown arm) as a regression surface. <!-- robobun:evidence:begin --> --- **no test proof** · iteration 2 · Platform-specific test(s) that do not run on this machine. Deferring to CI, which covers all platforms: test/js/web/streams/pipeTo-shutdown-gc.test.ts <!-- robobun:evidence:end -->
springmin
pushed a commit
that referenced
this pull request
Aug 6, 2026
…e cache (oven-sh#37034) ### Problem On the `13 x64-asan` lane, a test that exercises non-ISO Temporal calendars from a test callback can abort after a fully green run with a LeakSanitizer report. Seen in build 89504 on oven-sh#37024, whose `test/js/bun/bun-object/deep-equals-temporal.test.ts` uses `[u-ca=hebrew]`: ``` Direct leak of 624 byte(s) in 1 object(s) allocated from: #1 icu_75::HebrewCalendar::clone() const #2 icu_75::Calendar::createInstance(icu_75::TimeZone*, icu_75::Locale const&, UErrorCode&) #3 ucal_open_75 #4 JSC::TemporalCore::buildCalendarTemplate(WTF::AbstractLocker const&, unsigned int) #5 JSC::TemporalCore::withCalendar<JSC::TemporalCore::calendarYear(...)::$_0>(...) ``` The CI annotation titles this `direct leak of 624b in {closure#0} (src/jsc/JSValue.rs:1664:22)` because that is the first in-repo frame (the test-runner's `JSValue::call`); everything below it is WebKit/ICU. ### Cause `TemporalCore::withCalendar` (`vendor/WebKit/.../temporal/core/CalendarICUBridge.cpp`) keeps up to 8 open `UCalendar` templates in a process-lifetime `LazyNeverDestroyed` `TinyLRUCache`, one per calendar ID (non-ISO arithmetic, plus pure-ISO `PlainDateTime.prototype.with`, which reaches the same path unguarded); LRU eviction `ucal_close`s them, so the set is bounded. The `CalendarCacheEntry` that owns each `UCalendar` is `WTF_MAKE_TZONE_ALLOCATED` (bmalloc), which LSan does not scan, so the libc-allocated `UCalendar` (and the ICU `TimeZone` inside it) is reported as a direct leak even though it is reachable. Whether a given run aborts depends on whether some stale stack or register value still points at the ICU object when LSan scans at exit, hence the intermittence. This is the calendar twin of the already-suppressed `TemporalCore::withTimeZone` entry (same cache design, same TZone-allocated owner). ### Fix - Add a `leak:TemporalCore::buildCalendarTemplate` suppression to `test/leaksan.supp`, mirroring the `withTimeZone` entry. The pattern anchors on the template builder rather than `withCalendar` itself so that a future real leak inside one of the many op lambdas `withCalendar` runs would still be reported; every cached-template allocation carries the builder frame. (`withTimeZone` has no such builder frame, its `ucal_open` is inline, so that entry keeps its existing pattern.) - Drop the `test/no-validate-leaksan.txt` escape hatch oven-sh#37024 added for `deep-equals-temporal.test.ts`, re-enabling leak validation for it; that file exercises the suppressed path on the asan lane. ### Verification On a debug ASAN build, running `bun test test/js/bun/bun-object/deep-equals-temporal.test.ts` under the CI leak-validation env (`BUN_DESTRUCT_VM_ON_EXIT=1`, `detect_leaks=1:abort_on_error=1`, repo suppression file): - with the new entry: clean exit, 5/5 runs - without it: LSan abort with the calendar-template stacks above, 3/3 runs A standalone probe exercising 8 non-ISO calendars plus pure-ISO `PlainDateTime.with` from a timer callback shows the same split (10/10 aborts without, 10/10 clean with; `print_suppressions=1` attributes exactly the ICU template allocations to the new entry). Top-level module code cannot reproduce this: its allocation stacks carry `JSC::JSModuleLoader::evaluateNonVirtual`, which the suppression file already covers wholesale. An ASAN-gated test pinning the entry was part of an earlier revision and was dropped per review; the re-enabled `deep-equals-temporal.test.ts` covers the path in CI instead. The Expect-wrapper shutdown leak mentioned in the dropped no-validate comment is a separate issue tracked in oven-sh#32180: that is `bun test`'s own finalizer-owned memory, while this cache deliberately survives VM teardown, so oven-sh#32180 would not prevent this report. <!-- robobun:evidence:begin --> --- **no test proof** · iteration 1 · docs-only change; test-proof not applicable <!-- robobun:evidence:end --> --------- Co-authored-by: Dylan Conway <dylan.conway567@gmail.com>
springmin
pushed a commit
that referenced
this pull request
Aug 8, 2026
…ad transforms (oven-sh#37139) ### Repro `CompressionStream('brotli')` with a chunk over 128 KiB runs the codec step on a WorkPool thread. Tearing down the VM while that step is in flight frees the native coder under the pool thread: ```js // ASAN build, BUN_DESTRUCT_VM_ON_EXIT=1 (the CI test runner sets this) const s = new CompressionStream("brotli"); const w = s.writable.getWriter(); const big = new Uint8Array(6 << 20); for (let i = 0; i < big.length; i += 3) big[i] = (i * 2654435761) >>> 24; w.write(big).catch(() => {}); w.close().catch(() => {}); s.readable.getReader().read().catch(() => {}); setTimeout(() => process.exit(0), 15); ``` ``` ==ERROR: AddressSanitizer: heap-use-after-free ... thread (Bun Pool 0) #0 UpdateNodes vendor/brotli/c/enc/backward_references_hq.c:468 ... #4 BrotliEncoderCompressStream vendor/brotli/c/enc/encode.c:1661 #5 CompressionStreamCoder::transform src/runtime/webcore/CompressionStreamCoder.rs:367 freed by: BrotliEncoderDestroyInstance CompressionStreamCoder__destroy JSCompressionStream.cpp:176 (CFinalizer) JSC::Heap::CFinalizerOwner::finalize -> Heap::lastChanceToFinalize ``` The same free-under-the-pool-thread happens on `worker.terminate()` / `process.exit()` inside a worker while a large write is in flight (`WebWorker::shutdown` -> `WebWorker__teardownJSCVM` -> `lastChanceToFinalize`). Other faces of the same report: READ 1 in `BrotliEstimateBitCostsForLiterals` / `UpdateNodes`, WRITE 4 in `StoreAndFindMatchesH10`. `DecompressionStream` has the identical finalizer shape, and zstd/zlib formats share the path. ### Cause The stream cell's CFinalizer (registered in the constructor) destroys `m_coder` unconditionally. During normal operation the in-flight task's `Strong` root keeps the cell from being swept, and the eager ClearAlgorithms release already defers on `m_asyncCodecInFlight`. But `Heap::lastChanceToFinalize` at VM teardown runs every finalizer regardless of roots, so the coder (brotli ring buffer + hasher, zlib window, zstd ctx) is freed while the pool thread is still inside `transform`. ### Fix Reference-count the coder. The JS cell holds one reference, released where it released before (finalizer, or the eager ClearAlgorithms path; both already null the cell's pointer first, so `CompressionStreamCoder__destroy` keeps its signature and call sites). Each in-flight `CompressionAsyncCtx` takes its own reference when the async step is scheduled and drops it with the ctx on the JS thread. The backend is freed when the last reference drops, so teardown releases the cell's hold but can no longer free the state under the pool thread. On the teardown paths where the completion never gets delivered, the coder is abandoned with the dying process instead of freed early, which is the bounded-leak tradeoff the worker teardown path already takes elsewhere. Related: oven-sh#36983 fences `WebWorker::shutdown` on outstanding off-thread jobs, which closes the worker-terminate door from the other side (and is still needed for it: after this change, the worker repro's surviving report moves to `EventLoop::enqueue_task_concurrent` via `WorkTask::on_finish` on the freed worker loop, which is exactly the bug that PR addresses, now with a `WorkTask` stack). This change covers what the fence cannot: the main-thread `BUN_DESTRUCT_VM_ON_EXIT=1` exit path, and the coder's own lifetime independent of teardown ordering. ### Verification - New test in `test/js/web/streams/compression.test.ts` (ASAN-gated): fails on the unfixed build with the ASan report above, passes with the fix. - Main-thread repro: 5/5 clean runs with the fix (was UAF on every run before). - `test/js/web/streams/compression.test.ts` (37), `test/regression/issue/18413-all-compressions.test.ts`, `test/regression/issue/23314/zstd-large-decompression.test.ts`, and the four node webstreams compression compat tests all pass. <!-- robobun:evidence:begin --> --- **[review]** gate passed · iteration 0 · 4 files touched <details><summary>fails on main (without fix)</summary> ```console ASAN without fix: 1 FAILED $ BUN_DEBUG_QUIET_LOGS=1 bun scripts/build.ts --profile=debug --quiet test "--reporter=junit" "--reporter-outfile=/tmp/mechgate.xml" test/js/web/streams/compression.test.ts bun test v1.4.0 (38b3183) test/js/web/streams/compression.test.ts: (pass) TransformStream.prototype getters reject native transform subclasses (0) [13.39ms] (pass) TransformStream.prototype getters reject native transform subclasses (1) [2.50ms] (pass) TransformStream.prototype getters reject native transform subclasses (2) [2.66ms] (pass) TransformStream.prototype getters reject native transform subclasses (3) [2.27ms] (pass) CompressionStream and DecompressionStream > brotli > compresses data with brotli [15.13ms] (pass) CompressionStream and DecompressionStream > brotli > decompresses brotli data [21.20ms] (pass) CompressionStream and DecompressionStream > brotli > round-trip compression with brotli [51.32ms] (pass) CompressionStream and DecompressionStream > zstd > compresses data with zstd [9.46ms] (pass) CompressionStream and DecompressionStream > zstd > decompresses zstd data [18.82ms] (pass) CompressionStream and DecompressionStream > zstd > round-trip compression with zstd [36.88ms] ( ... (truncated) release without fix: 1 skipped bun test v1.4.0-canary.1 (0ac8ea9) test/js/web/streams/compression.test.ts: (pass) TransformStream.prototype getters reject native transform subclasses (0) [0.42ms] (pass) TransformStream.prototype getters reject native transform subclasses (1) [0.07ms] (pass) TransformStream.prototype getters reject native transform subclasses (2) [0.06ms] (pass) TransformStream.prototype getters reject native transform subclasses (3) [0.03ms] (pass) CompressionStream and DecompressionStream > brotli > compresses data with brotli [0.91ms] (pass) CompressionStream and DecompressionStream > brotli > decompresses brotli data [0.78ms] (pass) CompressionStream and DecompressionStream > brotli > round-trip compression with brotli [1.80ms] (pass) CompressionStream and DecompressionStream > zstd > compresses data with zstd [0.58ms] (pass) CompressionStream and DecompressionStream > zstd > decompresses zstd data [0.43ms] (pass) CompressionStream and DecompressionStream > zstd > round-trip compression with zstd [1.02ms] (pass) CompressionStream and DecompressionStream > zstd > decompresses a multi-frame zstd stream [0.28ms] (pass) CompressionStream and DecompressionStream > zstd > decompr ... (truncated) ``` </details> <details><summary>passes on PR (with fix)</summary> ```console ASAN with fix: all passed $ BUN_DEBUG_QUIET_LOGS=1 bun scripts/build.ts --profile=debug --quiet test "--reporter=junit" "--reporter-outfile=/tmp/mechgate.xml" test/js/web/streams/compression.test.ts bun test v1.4.0 (38b3183) test/js/web/streams/compression.test.ts: (pass) TransformStream.prototype getters reject native transform subclasses (0) [13.88ms] (pass) TransformStream.prototype getters reject native transform subclasses (1) [2.67ms] (pass) TransformStream.prototype getters reject native transform subclasses (2) [2.66ms] (pass) TransformStream.prototype getters reject native transform subclasses (3) [2.15ms] (pass) CompressionStream and DecompressionStream > brotli > compresses data with brotli [15.57ms] (pass) CompressionStream and DecompressionStream > brotli > decompresses brotli data [22.42ms] (pass) CompressionStream and DecompressionStream > brotli > round-trip compression with brotli [51.98ms] (pass) CompressionStream and DecompressionStream > zstd > compresses data with zstd [10.10ms] (pass) CompressionStream and DecompressionStream > zstd > decompresses zstd data [18.91ms] (pass) CompressionStream and DecompressionStream > zstd > round-trip compression with zstd [38.62ms] ... (truncated) release with fix: 1 skipped $ bun scripts/build.ts --profile=release [configured] bun-profile → bun (stripped) in 820ms (unchanged) ninja: Entering directory `/workspace/bun/build/release' [1/12] gen generated_host_exports.rs generated_host_exports.rs: 92 exports (host=3, lazy=10, generic=79, rust=0); 239 extern-C blocks audited [1/12] cargo bun_bin → libbun_rust.a (--target x86_64-unknown-linux-gnu) nightly-2026-07-20-x86_64-unknown-linux-gnu unchanged - rustc 1.99.0-nightly (9f36de775 2026-07-19) �[1m�[92m Compiling�[0m bun_core v0.0.0 (/workspace/bun/src/bun_core) �[1m�[92m Compiling�[0m bun_errno v0.0.0 (/workspace/bun/src/errno) �[1m�[92m Compiling�[0m bun_ptr v0.0.0 (/workspace/bun/src/ptr) �[1m�[92m Compiling�[0m bun_boringssl_sys v0.0.0 (/workspace/bun/src/boringssl_sys) �[1m�[92m Compiling�[0m bun_safety v0.0.0 (/workspace/bun/src/safety) �[1m�[92m Compiling�[0m bun_zlib_sys v0.0.0 (/workspace/bun/src/zlib_sys) �[1m�[92m Compiling�[0m bun_cares_sys v0.0.0 (/workspace/bun/src/cares_sys) �[1m�[92m Compiling�[0m bun_zstd v0.0.0 (/workspace/bun/src/zstd) �[1m�[92m Compiling�[0m bun_picohttp v0.0.0 (/workspace/bun/src/picohttp) �[1m�[92m Compiling�[0m bun_brotli ... (truncated) ``` </details> <details><summary>diff hotspot</summary> ``` .../bindings/webcore/streams/JSCompressionStream.h | 7 ++-- .../webcore/streams/JSCompressionStreamShared.h | 1 + src/runtime/webcore/CompressionStreamCoder.rs | 43 +++++++++++++++---- test/js/web/streams/compression.test.ts | 49 +++++++++++++++++++++- 4 files changed, 87 insertions(+), 13 deletions(-) ``` </details> **gate history** · 2 passed · 0 rejected · iteration 0 <details><summary>evidence per changed file</summary> ``` file reads edits tests src/jsc/bindings/webcore/streams/JSCompressionStream.h 2 2 0 …sc/bindings/webcore/streams/JSCompressionStreamShared.h 2 2 0 src/runtime/webcore/CompressionStreamCoder.rs 3 6 0 test/js/web/streams/compression.test.ts 1 1 0 ``` </details> <!-- robobun:evidence:end -->
springmin
pushed a commit
that referenced
this pull request
Aug 16, 2026
…wo parallel vecs (oven-sh#39145) ### Problem - `LOLHTMLContext` in `src/runtime/api/html_rewriter.rs` keeps two vecs, `selectors` and `element_handlers`, that describe one thing: entry `i` of each is the selector and the handler object from the same `rewriter.on(selector, handlers)` call. - The pairing is only held up by convention: `on_()` pushes to both, `build_settings()` zips them back together, and a doc comment plus an invariant comment explain it. The mordant `parallel_vecs` lint flags this (the one baselined finding for this file). ### Fix - Add `ElementHandlerEntry { selector, handler: Box<ElementHandler> }` and store `element_handlers: Vec<ElementHandlerEntry>`. One vec, one push in `on_()`, and `build_settings()` destructures each entry instead of zipping. - No behavior change: the same values are pushed in the same order, the handler is still boxed (the lol-html closures built in `build_settings()` hold raw pointers into the box, so it must not move when the vec reallocates), and the body of the `build_settings()` loop is unchanged. The `#[expect(clippy::vec_box)]` comes off `element_handlers` because it is no longer a `Vec<Box<_>>`; `document_handlers` keeps its own. - Remove the `parallel_vecs:src/runtime/api/html_rewriter.rs` line from `mordant-baseline.toml`. - Tests, in `test/js/workerd/html-rewriter.test.js` (`on() registrations`), pin down the two things this storage has to get right. They pass before and after this change, since it is a refactor: - Many selectors registered on one rewriter, with two rejected `on()` calls in the middle, each still run the handlers they were registered with, on two transforms of the same rewriter. - `on()` called from inside a handler, often enough to reallocate the registry while lol-html is still calling the handlers registered before the transform started: the running transform is unaffected and the next one picks the additions up. With the `Box` removed from `ElementHandlerEntry` this test fails under ASAN with a heap-use-after-free (report in the details below), so the boxing is now covered rather than only commented. - Verified: - `bun bd test` on `test/js/workerd/html-rewriter.test.js` (165 tests, including the new ones), `html-rewriter-end-error.test.ts`, `html-rewriter-leak.test.ts`, `test/js/web/html/html-rewriter-doctype.test.ts` and the HTMLRewriter regression tests: all pass. - `cargo clippy -p bun_runtime --no-deps`: clean. - `cargo dylint --all -p bun_runtime` with this baseline: nothing over the baseline. The same command with the baseline line removed but the source change stashed reports exactly the one `parallel_vecs` finding for this file, so the removed line is the one this change fixes. - Regenerating the baseline with `MORDANT_BASELINE_WRITE=1` also drops two entries this PR does not touch (`always_unwrapped_option:src/install/PackageInstall.rs`, `narrowed_two_ways:src/runtime/node/node_crypto_binding.rs`); those findings were already fixed on main by other changes and are left for a separate cleanup. ### Background - `HTMLRewriter.on(selector, handlers)` parses the CSS selector with lol-html and wraps the JS handler object in an `ElementHandler` (the protected `element`/`comments`/`text` callbacks). Nothing is handed to lol-html at that point; registrations are collected in `LOLHTMLContext`, which is shared by the rewriter and every transform it starts, because `transform()` can run more than once. - `build_settings()` runs at transform time and turns each registration into a `(selector, ElementContentHandlers)` pair for lol-html. Its closures capture a `NonNull<ElementHandler>` pointing into the heap allocation owned by the `Box`, which is why the handler has to stay boxed even though clippy would normally suggest otherwise. An `on()` call after a transform has started (for example from inside a handler) pushes onto the same vec, which is what makes the reallocation case reachable from JS. - `mordant-baseline.toml` is the ratchet for the mordant lint pack run by the Rust lints workflow: it records the accepted number of findings per (lint, file), and CI reports anything above those counts. Removing the line here means a reintroduction of the pattern in this file would be reported. <details> <summary>ASAN report from the new test with the Box removed from ElementHandlerEntry</summary> ``` ERROR: AddressSanitizer: heap-use-after-free READ of size 8 #3 <ElementHandler as HandlerLike>::global src/runtime/api/html_rewriter.rs #4 handler_callback::<ElementHandler, Element, ...> src/runtime/api/html_rewriter.rs #5 ElementHandler::on_element src/runtime/api/html_rewriter.rs #6 build_settings::{closure#0} src/runtime/api/html_rewriter.rs #8 lol_html ContentHandlersDispatcher::handle_start_tag freed by thread T0 here: #13 RawVec<ElementHandlerEntry>::grow_one #15 Vec<ElementHandlerEntry>::push oven-sh#16 HTMLRewriter::on_ src/runtime/api/html_rewriter.rs ``` </details> <!-- robobun:evidence:begin --> --- **no test proof** · iteration 2 · Platform-specific test(s) that do not run on this machine. Deferring to CI, which covers all platforms: test/js/workerd/html-rewriter.test.js <!-- robobun:evidence:end --> --------- Co-authored-by: Alistair Smith <hi@alistair.sh>
springmin
pushed a commit
that referenced
this pull request
Sep 1, 2026
…h#36545) ## Repro Under a debug/ASAN build: ```js const result = Bun.spawnSync(["cat"], { stdin: "ignore" }); const copy = { ...result }; ``` ``` ASSERTION FAILED: hasInlineStorage() JSObject.h(437) : PropertyStorage JSC::JSObject::inlineStorage() frame #4: JSC::JSObject::inlineStorage frame #5: tryCreateObjectViaCloning (ObjectConstructorInlines.h:281) frame #6: globalFuncCloneObject (JSGlobalObjectFunctions.cpp:1006) ``` The same assert fires for a spread of an `S3Client.list()` result or one of its `contents` entries, for `const x = structuredClone({}); x.a = 1; ({ ...x })`, and for `const m = new (require("node:module").Module)("x"); m.exports.a = 1; ({ ...m.exports })`. Release builds are unaffected: the pointer returned by `inlineStorage()` is fed into a copy loop bounded by `inlineCapacity`, which is zero, so nothing is dereferenced. ## Cause Several bindings call `constructEmptyObject(globalObject, objectPrototype(), 0)` (either as a literal `0` or as a computed size that can be `0`), which yields a `JSFinalObject` with `inlineCapacity() == 0` and therefore `hasInlineStorage() == false`. Every property added to such an object lands out of line in the butterfly. The object-spread fast path `tryCreateObjectViaCloning` in the current WebKit build calls `source->inlineStorage()` unconditionally to seed `createWithButterflyCopyingInlineStorage`, and `inlineStorage()` asserts `hasInlineStorage()` in debug builds. No object produced by JavaScript has zero inline capacity (`{}` gets `JSFinalObject::defaultInlineCapacity`, and `ObjectAllocationProfile` asserts `inlineCapacity > 0`), so only host-created objects with an explicit `0` hint reach this state. ## Fix Treat a zero hint as "unsized" and route it through `constructEmptyObject(globalObject)`, which produces the same `defaultInlineCapacity` structure as a JS `{}` literal and gives the first handful of properties inline slots instead of forcing a butterfly. - `JSC__JSValue__createEmptyObject` and `JSC__JSObject__create` (the two Rust FFI entry points) gain an `if (!initialCapacity)` branch. This covers every Rust caller of `JSValue::create_empty_object(_, 0)` (25 call sites on main, `Bun.spawnSync` among them) and `JSObject::create_with_initializer(_, _, 0)`. The non-zero branch now clamps the `size_t` hint to `maxInlineCapacity` before it narrows to `unsigned`, so a hint that is a multiple of 2^32 cannot narrow to zero either. - The direct C++ callers that pass `0` (literal or computed) switch to the default-capacity overload: `SQLClient.cpp` row fallback, `ZigGlobalObject.cpp` worker serialized-env when empty (Windows only on current main), `bindings.cpp` `SystemError` `info`, `NodeModuleModule.cpp` `new Module().exports`, the two `SerializedScriptValue.cpp` fast-path deserializer sites (`structuredClone({})` / `structuredClone([{}])`), the `_NativeModule.h` `INIT_NATIVE_MODULE` macro (applied to the uncached construction), the `JSEnvironmentVariableMap.cpp` `process.env` target when `count == 0`, and the `JSONRowsToJS.cpp` object builder when the parsed object has no properties (reaches user code through `Bun.JSONC.parse`, `Bun.TOML.parse`, JSON5, and XML; this file landed on main after the first audit and review caught it). Two sites from the first version of this PR are gone after the merge with main: `ProcessBindingUV.cpp` got the same fix in oven-sh#34660, and the two `NodeHTTP.cpp` `assignHeaders*` sites were removed when request headers became lazy. The remaining computed-capacity `constructEmptyObject(.*objectPrototype(),` sites in `src/` are `fromEntries` (already guarded), `JSFetchHeaders.cpp` (early-returns on `size == 0`), `JSDOMFormData.cpp` / `JSURLSearchParams.cpp` (`size + 1`), and `JSSQLStatement.cpp` (`initializeColumnNames` returns early when `count < 1`). Everything else passes a literal nonzero constant. ## Verification New spread tests guarding each confirmed-aborting surface: - `test/js/bun/spawn/spawnSync.test.ts`: subprocess spreads the `Bun.spawnSync` result. - `test/js/bun/s3/s3-list-objects.test.ts`: subprocess spreads the top-level `list()` result, a `contents` entry, its `owner`, and a `commonPrefixes` entry. - `test/js/node/module/node-module-module.test.js`: subprocess spreads `new Module().exports` after a put. - `test/js/web/structured-clone-fastpath.test.ts`: spreads `structuredClone({})` and `structuredClone([{}])[0]` after adding a property. - `test/js/node/process-binding.test.ts`: spreads `process.binding("uv")` (the fix for this one is already on main, the test pins it). - `test/js/sql/sql.test.ts`: spreads the wide-row result on the null-structure fallback path. - `test/js/bun/jsonc/jsonc.test.ts`: spreads a parsed nested empty `{}` (JSONC) and an empty TOML table after adding a property. Each aborts (or, for the subprocess tests, produces empty stdout) against main's `src/` and passes with the fix. The assertion only exists in debug builds of JavaScriptCore, so these tests cannot fail against a release binary. The remaining sites are defensive rather than confirmed aborts and ship without a dedicated test: - `SystemError` `info`: its `DontDelete` properties bail `checkStructureForClone`. - `JSC__JSObject__create`: its sole caller early-returns through the guarded helper when the count is 0. - Worker serialized-env: the object is consumed before user code can spread it. - `INIT_NATIVE_MODULE(0)` (`node:constants`): 216 default-attribute properties push the structure to `Dictionary`, which bails the fast path. - `JSEnvironmentVariableMap.cpp` `process.env` target: unconditional `CustomAccessor` puts for TZ et al. bail `checkStructureForClone`. <!-- robobun:evidence:begin --> --- **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/module/node-module-module.test.js, test/js/bun/spawn/spawnSync.test.ts <!-- robobun:evidence:end --> --------- Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
springmin
pushed a commit
that referenced
this pull request
Sep 3, 2026
…p_until (oven-sh#41149) Fuzzilli found an ASAN stack-buffer-overflow in `run_tasks` -> `Log::add_msg` -> `Vec::push` during runtime auto-install. ## What happened `enqueue_dependency_to_root` blocks in `sleep_until`. `sleep_until` ticks the JS event loop between `is_done` polls. Each poll calls `run_tasks`, which reads `manager.log` to log manifest 4xx errors. The event loop tick can run module transpilation or `AsyncModule::resume_loading_module`. Both swap the VM's log pointers (`jsc_vm.log`, `transpiler.log`, `resolver.log`, `linker.log`, `pm.log`) and restore them on exit. They do not save from the same source: - `resolve_maybe_needs_trailing_slash` saves `jsc_vm.log` and swaps every pointer except `transpiler.log`. - `transpile_source_code_inner` saves `transpiler.log` and restores `pm.log` to it. - `AsyncModule::resume_loading_module` saves `jsc_vm.log` but restores `transpiler.log` to it. When a module transpile runs during a resolve's `sleep_until` tick, its restore sets `pm.log` to the old `transpiler.log`, not the resolve's scoped log. The 404 diagnostics from `run_tasks` then land in the wrong log. With more interleaving across calls `pm.log` ends up at a dead stack `Log`, and the next `run_tasks` poll reads it. ``` READ of size 8 at 0x7ffd98a149c0 thread T0 #0 RawVecInner::capacity #1 Log::add_msg (lib.rs:2369) #3 Log::add_error_fmt (lib.rs:2032) #4 run_tasks (runTasks.rs:494) #5 Closure::is_done (PackageManagerEnqueue.rs:534) #7 AnyEventLoop::tick_raw (AnyEventLoop.rs:148) #8 PackageManager::sleep_until (PackageManager.rs:1063) #9 enqueue_dependency_to_root (PackageManagerEnqueue.rs:571) ... oven-sh#18 resolve_maybe_needs_trailing_slash (VirtualMachine.rs:4259) Address is located in stack of thread T0 in frame #0 to_js_host_call (host_fn.rs:677) [144, 152) 'scope' <== Memory access at offset 160 overflows this variable [176, 240) 'scope_storage' ``` ## Fix - Swap and restore `transpiler.log` with the other log pointers in `resolve_maybe_needs_trailing_slash`. It can no longer drift from `jsc_vm.log` across a resolve. - Snapshot `pm.log` on entry to `enqueue_dependency_to_root`'s `sleep_until`. Re-assert it before each `run_tasks` poll. `run_tasks` always sees the caller's log, whatever the event loop tick did to it. ## Test The test runs a 404 registry in the parent process on `port: 0`. The child queues `require()` calls with `setImmediate`, so they run during `sleep_until`'s event-loop tick and trigger `transpile_source_code_inner`'s log swap. Without the fix, the 404 errors land in the VM log and print to stderr at exit. With the fix, stderr is empty. Verified on current `main` (6f27257): the test fails without the source change and passes with it. Supersedes oven-sh#31120, which the stale bot closed after 90 days. Same change, rebased onto current main; moved to this branch so the fix can be tracked. <!-- robobun:evidence:begin --> --- **[human-review]** gate passed · iteration 6 · 3 files touched <details><summary>fails on main (without fix)</summary> ```console ASAN without fix: 1 FAILED $ BUN_DEBUG_QUIET_LOGS=1 bun scripts/build.ts --profile=debug --quiet test "--reporter=junit" "--reporter-outfile=/tmp/pr_gate.xml" test/js/bun/resolve/resolve-autoinstall-log-dangling.test.ts bun test v1.4.1 (a6c4cc2) test/js/bun/resolve/resolve-autoinstall-log-dangling.test.ts: (pass) repeated failing auto-install resolves at varying stack depth don't read a dangling pm.log [1075.94ms] 114 | stderr: "pipe", 115 | }); 116 | 117 | const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); 118 | 119 | expect(stderr).toBe(""); ^ error: expect(received).toBe(expected) - "" + "error: GET http://localhost:37987/autoinstall-missing-pkg-0 - 404 + + error: GET http://localhost:37987/autoinstall-missing-pkg-1 - 404 + + error: GET http://localhost:37987/autoinstall-missing-pkg-2 - 404 + + error: GET http://localhost:37987/autoinstall-missing-pkg-3 - 404 + + error: GET http://localhost:37987/autoinstall-missing-pkg-4 - 404 + + error: GET http://localhost:37987/autoinstall-missing-pkg-5 - 404 + + error: GET http://localhost:37987/autoinstall-missing-pkg-6 - 404 + + error: GET http:// ... (truncated) release without fix: 1 FAILED bun test v1.4.1-canary.1 (a6c4cc2) test/js/bun/resolve/resolve-autoinstall-log-dangling.test.ts: (pass) repeated failing auto-install resolves at varying stack depth don't read a dangling pm.log [22.63ms] 114 | stderr: "pipe", 115 | }); 116 | 117 | const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); 118 | 119 | expect(stderr).toBe(""); ^ error: expect(received).toBe(expected) - "" + "error: GET http://localhost:35697/autoinstall-missing-pkg-0 - 404 + + error: GET http://localhost:35697/autoinstall-missing-pkg-1 - 404 + + error: GET http://localhost:35697/autoinstall-missing-pkg-2 - 404 + + error: GET http://localhost:35697/autoinstall-missing-pkg-3 - 404 + + error: GET http://localhost:35697/autoinstall-missing-pkg-4 - 404 + + error: GET http://localhost:35697/autoinstall-missing-pkg-5 - 404 + + error: GET http://localhost:35697/autoinstall-missing-pkg-6 - 404 + + error: GET http://localhost:35697/autoinstall-missing-pkg-7 - 404 + + error: GET http://localhost:35697/autoinstall-missing-pkg-8 - 404 + + error: GET http://localhost:35697/autoinstall-missing-pkg-9 - ... (truncated) ``` </details> <details><summary>passes on PR (with fix)</summary> ```console ASAN with fix: all passed $ BUN_DEBUG_QUIET_LOGS=1 bun scripts/build.ts --profile=debug --quiet test "--reporter=junit" "--reporter-outfile=/tmp/pr_gate.xml" test/js/bun/resolve/resolve-autoinstall-log-dangling.test.ts bun test v1.4.1 (a6c4cc2) test/js/bun/resolve/resolve-autoinstall-log-dangling.test.ts: (pass) repeated failing auto-install resolves at varying stack depth don't read a dangling pm.log [1104.72ms] (pass) module transpile during auto-install's event-loop tick doesn't desync pm.log [689.71ms] 2 pass 0 fail 8 expect() calls Ran 2 tests across 1 file. [3.73s] __F:0:S:0 release with fix: all passed $ bun scripts/build.ts --profile=release [configured] bun-profile → bun (stripped) in 590ms (unchanged) ninja: Entering directory `/workspace/bun/build/release' [1/141] gen generated_host_exports.rs generated_host_exports.rs: 122 exports (host=5, lazy=10, generic=107, rust=0); 244 extern-C blocks audited [2/141] gen JS modules (bundle-modules) Preprocess modules (8046ms) Bundle modules (666ms) Postprocesss modules (969ms) Bundle Functions (629ms) Generate Code (40ms) [10.36s] Bundled "src/js" for production 2594 kb 197 internal modules 13 native modules 50 internal functions across 16 files [2/141] cargo bun_runtime → libbun_runtime.a �[1m�[92m Compiling�[0m bun_output_tags v0.0.0 (/workspace/bun/src/bun_output_tags) �[1m�[92m Compiling�[0m bun_core v0.0.0 (/workspace/bun/src/bun_core) �[1m�[92m Compiling�[0m bun_parsers v0.0.0 (/workspace/bun/src/parsers) �[1m�[92m Compiling�[0m bun_install v0.0.0 (/workspace/bun/src/install) �[1m�[92m Compiling�[0m bun_jsc v0.0.0 (/workspace/bun/src/jsc) �[1m�[92m Compiling�[0m bun_dispatch v0.0.0 (/workspace/bun/src/dispatch) �[1m�[92m Compiling�[0m bun_jsc_macros v0.0.0 (/workspace/bun/src/jsc_macros) ... (truncated) ``` </details> <details><summary>diff hotspot</summary> ``` .../PackageManager/PackageManagerEnqueue.rs | 6 ++ src/jsc/VirtualMachine.rs | 5 ++ .../resolve-autoinstall-log-dangling.test.ts | 65 ++++++++++++++++++++++ 3 files changed, 76 insertions(+) ``` </details> **gate history** · 1 passed · 0 rejected · iteration 6 <details><summary>evidence per changed file</summary> ``` file reads edits tests src/install/PackageManager/PackageManagerEnqueue.rs 1 2 19 src/jsc/VirtualMachine.rs 5 3 19 …js/bun/resolve/resolve-autoinstall-log-dangling.test.ts 3 9 18 ``` </details> <!-- robobun:evidence:end -->
springmin
pushed a commit
that referenced
this pull request
Sep 15, 2026
…text (oven-sh#42520) ### Problem - `class Upload extends File {}` inside a `node:vm` context crashes on `new Upload(...)`: `panic(main thread): Segmentation fault at address 0x28`. `class S extends fs.Stats {}`, `BigIntStats`, and `Reflect.construct(File, args, fnFromContext)` crash the same way. Node v26.3.0 constructs the object. - These constructors do `static_cast<Zig::GlobalObject*>(getFunctionRealm(lexicalGlobalObject, newTarget))` (`JSDOMFile.cpp:75`, `NodeFSStatBinding.cpp:843`, `NodeFSStatFSBinding.cpp:372`). For a function from a context the realm is a `NodeVMGlobalObject`, which is not a `Zig::GlobalObject`. `JSBlobStructure()` and `getStructure<isBigInt>()` then read a `LazyClassStructure` past the end of the cell. ### Fix - The three sites call `defaultGlobalObject(getFunctionRealm(...))`. It returns the realm when it is a Bun global, and the default Bun global when it is not. - The other native constructors already do this (`generate-classes.ts:2387`, `JSDOMWrapperCache.cpp:42`, `NodeDirent.cpp:177`). - The `StatFs` constructor is not reachable from JS, so that site has no test. - Verified: `test/js/node/vm/vm.test.ts`, four new cases. All four exit with SIGSEGV on 1.4.3-canary.1. Also ran the `fs.Stats`, `Blob`, and globals suites. ### Background - `newTarget` is the constructor that `new` was applied to. In `new Upload()`, `File` runs with `newTarget === Upload`. - `getFunctionRealm(newTarget)` returns the global object the function was created in. A native constructor takes its base `Structure` from that global. `createSubclassStructure` then applies `newTarget.prototype`. - A `node:vm` context has its own global, `NodeVMGlobalObject`. It and `Zig::GlobalObject` (the Bun global) are sibling subclasses of `Bun::GlobalScope`. Only `Zig::GlobalObject` holds the structures of Bun classes. - A ShadowRealm global is a `Zig::GlobalObject`. The comment at these sites names that case, and the cast was valid for it. <details><summary>Notes</summary> Repro (`bun f.js file`, `bun f.js stats`, `bun f.js bigint`): ```js const vm = require("vm"), fs = require("fs"); const ctx = vm.createContext({ File, Stats: fs.Stats }); const which = process.argv[2] || "file"; try { if (which === "file") console.log("ok", vm.runInContext(`class Upload extends File {}; new Upload(["a"], "a.txt")`, ctx).name); if (which === "stats") console.log("ok", typeof vm.runInContext(`class S extends Stats {}; new S()`, ctx).isFile); if (which === "bigint") console.log("ok", typeof Reflect.construct(fs.statSync(".", { bigint: true }).constructor, [], vm.runInContext("(function(){})", ctx))); } catch (e) { console.log("threw", e.message); } console.log("survived"); ``` | | `file` | `stats` | `bigint` | | --- | --- | --- | --- | | 1.4.3-canary.1 | segfault at 0x28 | segfault at 0x28 | segfault at 0x28 | | this branch | `ok a.txt` | `ok function` | `threw Invalid argument type in ToBigInt operation` | | node v26.3.0 | `ok a.txt` | `ok function` | `threw Cannot mix BigInt and other types` | ASAN on main with `Malloc=1` in the environment. `heap-buffer-overflow`, `READ of size 8`, "located 3776 bytes after 4184-byte region" (`File`), 896 bytes after (`Stats`), 912 bytes after (`BigIntStats`). The region is the `NodeVMGlobalObject` cell. ``` #0 JSC::LazyProperty<JSC::JSGlobalObject, JSC::Structure>::getInitializedOnMainThread LazyProperty.h:95 #1 JSC::LazyClassStructure::getInitializedOnMainThread LazyClassStructure.h:104 #2 Zig::GlobalObject::JSBlobStructure() ZigGeneratedClasses+lazyStructureHeader.h:7 #3 JSDOMFile::construct src/jsc/bindings/JSDOMFile.cpp:79 #5 llint_op_super_construct_varargs ``` ``` #1 JSC::LazyClassStructure::getInitializedOnMainThread LazyClassStructure.h:104 #2 Bun::getStructure<false>(Zig::GlobalObject*) src/jsc/bindings/NodeFSStatBinding.cpp:167 (<true>: line 164) #3 Bun::constructJSStatsObject<false>(JSGlobalObject*, CallFrame*) src/jsc/bindings/NodeFSStatBinding.cpp:847 #4 Bun::constructStats src/jsc/bindings/NodeFSStatBinding.cpp:915 ``` Without `Malloc=1` the debug build of main reads zero there and stops at `Structure.h:415: runtime error: member call on null pointer of type 'const JSC::Structure *'`. Fail-before and pass-after, same test file: - `USE_SYSTEM_BUN=1 bun test test/js/node/vm/vm.test.ts -t "belongs to a context"`: 0 pass, 4 fail (`exitCode: 139`, `signalCode: "SIGSEGV"`). - Debug ASAN build of main with only the test added: 0 pass, 4 fail (`exitCode: 1`). - Debug ASAN build of this branch: 4 pass. The whole file: 285 pass, 0 fail. Other cases checked by hand on this branch: - A newTarget that is a plain function, a bound function, or a Proxy from the context. The new test covers these. A bound function has no `prototype`, so the object gets the prototype of the constructor. - A revoked Proxy from the context throws `TypeError: Cannot get function realm from revoked Proxy`. - A newTarget whose `prototype` is not an object falls back to `File.prototype` or `fs.Stats.prototype`. - `class X extends File {}` inside a `ShadowRealm` still works. `File` and `X` both belong to the ShadowRealm global there. - An instance of the context subclass works with `FormData`, `.text()`, and a forced GC. Suites run on the debug build: `test/js/node/vm/vm.test.ts`, `test/js/node/fs/fs-stats-constructor.test.ts`, `test/js/node/fs/fs-stats-truncate.test.ts`, `test/js/node/fs/fs.test.ts -t Stats`, `test/js/bun/globals.test.js`, `test/js/web/fetch/blob.test.ts`, and the Node tests `test-fs-stat.js`, `test-fs-stat-bigint.js`, `test-fs-statfs.js`, `test-fs-watchfile.js`. Related: - oven-sh#32434 changes the `File` cast to `defaultGlobalObject` as one line of a larger `File.prototype` change. It does not touch `fs.Stats`. - oven-sh#42514 fixes the same kind of cast at another site (`UtilInspect.cpp`). - `new BigIntStats(...)` stores `atimeMs` as a Number where Node stores a BigInt, so `.atime` throws. That is a separate bug and is not part of this PR. </details> <!-- robobun:evidence:begin --> --- **no test proof** · iteration 1 · platform-specific test(s) that do not run on this machine, deferring to CI, which covers all platforms: test/js/node/vm/vm.test.ts <!-- robobun:evidence:end -->
springmin
pushed a commit
that referenced
this pull request
Sep 30, 2026
### Problem - On Windows, a worker can use the stdin `FileSink` of a `Bun.spawn` child after it is freed. It needs `stdin: "pipe"`, a live child, and a script that never read `proc.stdin`. Debug build: `panic: misaligned pointer dereference: address must be a multiple of 0x8 but is 0xdfdfdfdfdfdf`. A release build reads freed memory silently. - `FileSink::on_close` (`src/runtime/webcore/FileSink.rs:528`) calls `source.close()`. That reaches `Writable::on_close` (`src/runtime/api/bun/subprocess/Writable.rs:114`), which drops the only ref on the sink. `on_close` then continues on freed memory. ### Fix - `FileSink::on_close` holds a ref on the sink until it returns. `on_write` and `run_pending` already use this guard. - A testing hook, `subprocessInternals.closeStdinWriter(proc)`, does the same close as the Windows stop phase, on every platform. - Verified with the hook, in `test/js/bun/util/filesink.test.ts`. Without the guard, the Linux ASAN build reports `heap-use-after-free` in `settle_stream_done`, and a Windows debug build panics. With the guard both pass. The worker test in `test/js/web/workers/worker-terminate-lifetime.test.ts` can fail only on a Windows debug build. - Self-reviewed: 3 concerns raised, 2 addressed (see Notes). ### Background - A `FileSink` is a refcounted writer. For `stdin: "pipe"` the Subprocess holds one ref. The `proc.stdin` getter moves it to a JS wrapper. `source.close()` tells that owner that the sink closed, and the owner drops its ref. - The stop phase is the first step of worker teardown (oven-sh#37075). On Windows it closes each libuv pipe of the worker through its writer, which calls `FileSink::on_close`. ### Downsides - None found for the guard. `on_close` never runs while the sink is destroyed, so the guard never takes a ref from zero. - Still open: the stream-fed stdin leak in the Notes. <details><summary>Notes</summary> **Repro** (windows-x64, debug build, 5 of 5 runs on main at b7ea95a): ```js import { Worker } from "node:worker_threads"; const w = new Worker(` const { parentPort } = require("node:worker_threads"); globalThis.keep = Bun.spawn({ cmd: [process.execPath, "-e", "setTimeout(() => {}, 4000)"], stdin: "pipe", stdout: "ignore", stderr: "ignore" }); setTimeout(() => parentPort.postMessage("go"), 300); `, { eval: true }); await new Promise(r => w.once("message", r)); await w.terminate(); console.log("survived"); ``` **Symbolized stack** (llvm-symbolizer with the PDB, ASLR off, line numbers at b7ea95a): ``` bun_jsc::strong::Impl::get src/jsc/Strong.rs:179 bun_jsc::strong::Optional::get src/jsc/Strong.rs:92 webcore::streams::PipeCell::take_done src/runtime/webcore/streams.rs:908 FileSink::settle_stream_done src/runtime/webcore/FileSink.rs:552 FileSink::on_close src/runtime/webcore/FileSink.rs:530 <FileSink as WindowsStreamingWriterParent>::on_close src/io/PipeWriter.rs:2734 WindowsStreamingWriter<FileSink>::on_close_source src/io/PipeWriter.rs:2010 WindowsStreamingWriter<FileSink>::close src/io/PipeWriter.rs:1212 WindowsStreamingWriter<FileSink>::stop_for_vm_teardown src/io/PipeWriter.rs:1130 open_handles::stop_all_for_vm_teardown src/libuv_sys/open_handles.rs:172 VirtualMachine::stop_phase_sweep src/jsc/VirtualMachine.rs:2554 VirtualMachine::teardown src/jsc/VirtualMachine.rs:2401 WebWorker::shutdown / spin / thread_main src/jsc/web_worker.rs ``` The address matches. `Strong::get` masks the handle to 48 bits, and `0xdfdfdfdfdfdfdfdf & ((1 << 48) - 1)` is `0xdfdfdfdfdfdf`. `0xdf` is the fill that a debug mimalloc writes into a freed block. **Linux ASAN report** (debug ASAN build with the hook and without the guard, from the new test in `filesink.test.ts`): ``` ERROR: AddressSanitizer: heap-use-after-free ... READ of size 8 #0 bun_jsc::strong::Optional::get src/jsc/Strong.rs:91 #1 webcore::streams::PipeCell::take_done src/runtime/webcore/streams.rs:908 #2 FileSink::settle_stream_done src/runtime/webcore/FileSink.rs #3 FileSink::on_close src/runtime/webcore/FileSink.rs #5 PosixStreamingWriter<FileSink>::close src/io/PipeWriter.rs:1048 #7 PollOrFd::close_impl src/io/pipes.rs:118 #10 testing_apis::close_stdin_writer src/runtime/api/bun/subprocess.rs freed by thread T0 here: #12 <FileSink as CellRefCounted>::destroy src/ptr/ref_count.rs:448 #15 <RefPtr<FileSink> as Drop>::drop src/ptr/ref_count.rs:522 oven-sh#19 JsCell<Writable>::set src/ptr/js_cell.rs:94 oven-sh#20 Writable::on_close src/runtime/api/bun/subprocess/Writable.rs:114 oven-sh#22 SourceHandle::close src/runtime/webcore/streams.rs:1043 oven-sh#23 FileSink::on_close src/runtime/webcore/FileSink.rs ``` The use and the free are in the same `FileSink::on_close` call. For this proof I removed only the guard line and kept the hook. With the `src/` of main the new `filesink.test.ts` test also fails, but for a weaker reason: the hook does not exist there. **Why only this path.** Every other path that closes this sink keeps it alive or detaches the source first: - `Subprocess::on_process_exit` clears `source` before `on_attached_process_exit`, which also holds a ref. - `Writable::finalize` and `on_close_io` clear `source` before they drop the ref. - The `proc.stdin` getter takes the pipe out of the `stdin` slot and clears `source`, so `Writable::on_close` is not reached. - POSIX has no stop-phase close of the pipe. The sink dies in `Subprocess::finalize`. A poll HUP with an empty buffer reports `Drained`, not a close. **Variants** (windows-x64 debug, before and after the fix): | worker ends by | stdin | before | after | | --- | --- | --- | --- | | `terminate()` | `"pipe"`, never read | panic | ok | | `process.exit()` in the worker | `"pipe"`, never read | panic | ok | | loop drains (`proc.unref()`) | `"pipe"`, never read | panic | ok | | `terminate()`, `stdout: "pipe"` too | `"pipe"`, never read | panic | ok | | `terminate()` | `"pipe"`, `proc.stdin.write()` pending | ok | ok | | no worker, `closeStdinWriter(proc)` hook | `"pipe"`, never read | panic | ok | After the fix, `fileSinkInternals.liveCount()` is back at the baseline after the `exit` event of the worker in each row. **Suites run with the fix.** - windows-x64 debug: `worker-terminate-lifetime.test.ts` (24 pass, 2 skip), `worker_destruction.test.ts` (5 pass), `spawn.test.ts` (126 pass), `spawn-stdin-readable-stream.test.ts` (36 pass), `spawn-stdin-destroy.test.ts`, `spawn-stdin-pipe-fd-leak.test.ts`, `child_process.test.ts` (53 pass), `filesink.test.ts` (33 pass, 1 fail, see below). The hook test passes 5 of 5 runs and the worker test 3 of 3. - linux-x64 debug ASAN: both new tests, `filesink.test.ts` (70 pass), `spawn-stdin-readable-stream.test.ts` (37 pass), `spawn-stdin-destroy.test.ts`, `worker_destruction.test.ts` (5 pass with `--timeout 120000`). **Self-review.** - Addressed: no CI lane could fail the worker test. The Windows lanes run release builds, where the stale reads do not crash, and Linux does not reach the path. The hook test now fails on an ASAN build of any platform without the guard. The worker test stays, because it is the real trigger, and its comment says which build can fail it. - Addressed: the comment before `clear_keep_alive_ref` said that call can free the sink. With the guard it cannot, so the sentence is gone. - Not changed: the sink keeps its stale `source` after the owner is told. Nothing reads it again on this path, because a later write needs the JS wrapper, and the getter that creates the wrapper clears `source`. - Checked and fine: no caller of `on_close` touches the writer or the sink after the call returns (`on_close_source`, the tail of `close`, `stop_all_for_vm_teardown`, and `PollOrFd::close` on POSIX, where the callback is last). Both writer `Drop` impls close without a report, so `on_close` never runs while the sink is destroyed. **Not in this PR.** On Windows, a worker that ends while `Bun.spawn({ stdin: new ReadableStream({ pull(c) { c.enqueue(new Uint8Array(1024)); return new Promise(() => {}); } }) })` still pumps leaves one `FileSink` alive. `liveCount()` stays at +1, before and after this change. Linux returns to the baseline. The cause is different: the ref that `assign_to_js_stream` takes for the reactions of the pump promise is not released, because the reactions never run after script is forbidden. oven-sh#43729 adds a flag that tracks that ref. **Local failures that this diff does not cause.** - windows-x64 (Server 2019): `test/js/bun/util/filesink.test.ts` "Bun.spawn stdin pipe with an unref'd child" fails 3 of 3 runs with the `src/` of main, with this fix, and with the release canary. - linux-x64 debug ASAN in my container: "terminate() while dns.lookup() is in flight" in the same test file reports a 16 byte LeakSanitizer leak from `node_fs_binding::Binding::new`. No `FileSink` is involved. </details> <!-- robobun:evidence:begin --> --- **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/web/workers/worker-terminate-lifetime.test.ts, test/js/bun/util/filesink.test.ts <!-- robobun:evidence:end -->
springmin
pushed a commit
that referenced
this pull request
Sep 30, 2026
…#43900) ### Problem - `test/js/bun/spawn/spawn-stdio-syscall-error.test.ts` is red on alpine: `"lost": -82000`, not `0` (build 120303). oven-sh#43739 added the case, not the bug. - `read_loop` (`src/io/PipeReader.rs:766`) delivers the bytes read before a failed read, then the error. The consumer asks for more inside that delivery, and the reader reads the fd past the error. The error comes late, or never. ### Fix - `read_once` sets `PosixFlags::READ_FAILED` on a fatal error, and `begin_read` does not read while it is set. The request parks, then `on_reader_error` rejects it. Nothing clears the flag. - Correct: libuv `uv__read` clears `UV_HANDLE_READABLE` before it reports a read error. - Verified: `test/js/bun/spawn/spawn-stdio-syscall-error.test.ts`, 17 pass. The four new cases fail without the fix. Suites: Notes. - Self-reviewed: 15 concerns raised, 14 addressed. Not taken: a flag reset in `start()`, which nothing on main needs. ### Background - `PosixBufferedReader` reads the fd behind subprocess stdio, file streams and the shell. Its parent gets `on_read_chunk`, then `on_reader_done` or `on_reader_error`. - `FileReader` is the parent behind a `ReadableStream`. `on_read_chunk` resolves the parked `pull()`, and the reaction runs before it returns. - Considered: a repeating shim failure hides the lost error. An error stored in `FileReader` first needs a callback in every parent. - oven-sh#43920 is a newer PR with the same change. Its test trigger is here. ### Downsides - After a read error, a stream that ended with `'end'` now ends with `'error'`, as in Node. With no listener the process stops. - After a read error, `bun run --filter` no longer drains that pipe at exit. - Reads that do not fail pay nothing: `begin_read` tests one more bit. <details><summary>Notes</summary> **Trace without the fix** (bun 1.4.3-canary.1+367d939d9, shim logs each `recv()`, writer `dd bs=1025`). The debug build of main at 8d36bff does the same: 3 of 8 runs, two with no `'error'` and `lost` -7714150: ``` recv #5 len=262144 -> 95325 recv #6 len=166819 -> EIO (injected) same fill_scratch call as #5 JS data 95325 recv #7 len=65536 -> 65536 pull from inside the delivery, read_into recv #8 .. oven-sh#116 to EOF {"received":452025,"got":8000125,"lost":-7548100,"events":["stdout.close","close"]} ``` No `'error'` event: the reads reached EOF before `on_reader_error` ran, and a stored error is only returned by a later pull. When the reads park first, `on_reader_error` rejects that pull and `'error'` comes late. That is the CI signature: the events match and `lost` is a negative multiple of 1025. **Why alpine.** In CI the failure is injected: the shim fails only the Nth `recv()`, so a later `recv()` succeeds and the extra bytes show. The failing `recv()` must follow bytes in the same wakeup. BusyBox `head` writes 1025 bytes at a time, so a wakeup often holds a short `recv()` and then the failing one. coreutils `head` fills the buffer in one `recv()`. The same run fails on debian when the writer is slow. With a writer that copies BusyBox `head` (stdio, 1024-byte buffer), the `RECV_AT=6` case fails 7 of 40 runs on bun 1.4.3-canary (release), and 36 of 40 when the writer also spins between chunks. This branch (debug): 0 of 120, and 0 of 60 with `dd bs=1025`. **With no shim.** A child with one AF_UNIX socket as fd 0 and fd 1 writes a line to stdout and reads stdin. The peer leaves that line unread, sends 8192 bytes and closes. The kernel gives the child 8192 bytes, then `ECONNRESET`, then EOF. | Runtime | stdin events | |---|---| | Node v26.3.0 | `'error'` `ECONNRESET` after 8192 bytes | | bun 1.4.3-canary.1+367d939d9 | `'end'` after 8192 bytes, no error | | this branch | `'error'` `ECONNRESET` after 8192 bytes | **The new cases.** Each one reaches the reader from inside the delivery in a different way. Whole-file runs of the describe block: | Case | Entry | Release, no fix | Debug, no fix | Debug, guard in `read_into` only | Debug, this branch | |---|---|---|---|---|---| | `child_process`, `'data'` listener | pull, `read_into` | fails 6 of 6 | fails 5 of 5 | passes | passes | | `child_process`, `'readable'` and `read()` | `set_flowing(true)`, `read` | fails 6 of 6 | fails 5 of 5 | fails 3 of 3 | passes | | `Bun.spawn`, `lazy` | pull, `read_into` | fails 6 of 6 | fails 5 of 5 | passes | passes | | `Bun.spawn`, reader started at spawn | pull, `read_into` | fails 6 of 6 | fails 5 of 5 | passes | passes | - The first three use counts: `SPAWN_FAULT_RECV_CAP=4096` makes every `recv()` short, so `fill_scratch` calls `recv()` again in the same wakeup. `SPAWN_FAULT_RECV_EAGAIN_AT=2` ends the first read loop, so the consumer's read parks and the next read is poll-driven. `SPAWN_FAULT_RECV_AT` then fails after bytes in that wakeup. For the `'readable'` case it is 19: #3 to oven-sh#18 return 64 KiB, the highWaterMark, so the reader is stopped and `read()` starts it again. - The fourth uses state, and comes from oven-sh#43920: the writer waits for a line on stdin, so the first read is parked when the bytes arrive. `SPAWN_FAULT_RECV_MID_FILL=1` fails the `recv()` that follows one that returned bytes, and `SPAWN_FAULT_READS_AFTER` counts the `recv()` calls after it. Without the fix it is 1. - With a count-based trigger and the reader started at spawn, the case passed without the fix on a debug build: the buffered reader that runs before JS reads `.stdout` took the bytes and the error. That is why this case uses state. - With the BusyBox-like writer at four speeds, the whole file passes 20 of 20 on this branch. **With the fix**, `CAP=4096 EAGAIN_AT=2 RECV_AT=5`: ``` recv #3 -> 4096, recv #4 -> 4096, recv #5 -> EIO JS data 8192 close(fd) JS error EIO ``` **Node.** libuv `uv__read` (`src/unix/stream.c`): on a read error other than `EAGAIN` it clears `UV_HANDLE_READABLE | UV_HANDLE_WRITABLE`, calls `read_cb` with the error, then stops the watcher. It calls `read_cb` once for each `read()`, so it never holds bytes and an error from one batch. **Placement.** EOF and the `maxBuffer` stop have the same guard at this site: `close_if_final` closes the reader before the final chunk is delivered. An error cannot use it, because a closed reader with no stored error reads as a clean end. For the same reason `READ_FAILED` is not part of `is_done()`. **Parents** (14 `BufferedReaderParent` implementations, what each does in `on_reader_error`): - 3 release the fd: `FileReader`, `SubprocessPipeReader`, `Terminal`. - 2 drop the reader: `FileResponseStream`, shell `subproc.rs`. - 8 only do accounting: `filter_run.rs`, `multi_run.rs`, `lifecycle_script_runner.rs`, `security_scanner.rs`, `git_runner.rs`, both cron jobs, test `Worker.rs`. `lifecycle_script_runner.rs` and `cron.rs` build a new reader with `init()` for each spawn. - 1 is shared and can restart: the shell `IOReader`. Only two read the same reader after an error. `filter_run.rs` `drain_and_close_pipes` reads once more at exit. That read is now a no-op, and `deinit()` follows. Before, it could reach a second terminal callback and decrement `remaining_fds` twice. The shell `IOReader::start()` does not restart a reader after a failed read on main, because the fired one-shot poll still counts as registered. The Windows reader gets one libuv callback for each read, so it has no bytes-then-error batch. **Left open.** - The flag is permanent. oven-sh#39638 and oven-sh#37901 change `IOReader::start()` to restart the shell's stdin reader. After this PR they must clear `READ_FAILED` there, or a `cat` that follows a stdin read error gets no data, no EOF and no error. - Not in this PR: a batch that stops because the buffer is full is labelled `ReadState::Eof` when the poll event carries the hangup (`read_state`, `None if received_hup`). `Bun.write(file, proc.stdout)` then writes 262144 of 400000 pending bytes and resolves. It is on main and in 1.4.3-canary, and this PR does not change it. It needs its own change. - Not in this PR: release the fd on a read error once, in `PosixBufferedReader::on_error`. oven-sh#41456 names it as a follow-up. oven-sh#41420, oven-sh#41456 and oven-sh#42150 did it for one parent each. **Suites run on the debug build:** `test/js/web/streams/streams.test.js` (624 pass), `test/js/node/stream/node-stream.test.js` (112 pass), `test/js/bun/spawn/spawn-streaming-stdout.test.ts` (pass), `test/js/node/child_process/child_process.test.ts` (81 pass, 2 fail in my container for reasons outside this change: "spawn in the default shell" reads an empty `$SHELL`, and "extra stdio pipes are not double-closed on GC" needs 5.0 s in a debug build against the 5 s timeout, its script prints `OK`). With this branch's build, the test file of oven-sh#43920 passes 17 of 17 in 3 runs. oven-sh#43790 is open and edits the comment above the failing case. It changes the event order in `native-readable.ts`, not the reader. </details> <!-- robobun:evidence:begin --> --- **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/bun/spawn/spawn-stdio-syscall-error.test.ts <!-- robobun:evidence:end -->
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
9 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
On Linux with SELinux/seccomp, directory reads can return EPERM or EACCES in addition to the usual EACCES. The resolver currently only handles ENOENT/FileNotFound/ENOTDIR/NotDir, causing 'Cannot read directory' log spam for permission-denied paths.
This PR adds PermissionDenied, AccessDenied, EPERM, and EACCES to all 6 locations in the resolver where directory/file read errors are handled.
Changes