Repository navigation
Conversation
…ng markdown images prefetch_remote_images built every image download with default options, so `bun file.md` in a Kitty terminal connected to image hosts directly even when http_proxy/https_proxy were set, and always rejected self-signed certificates regardless of NODE_TLS_REJECT_UNAUTHORIZED. Load the process env into a Loader (arena-backed, since the downloads hold AsyncHTTP<'static>) and resolve the proxy per image URL with get_http_proxy_for, which picks http_proxy or https_proxy by scheme and applies NO_PROXY, the same way the other CLI download sites do.
|
Warning Review limit reached
Next review available in: 27 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (2)
Comment |
|
Status: reproduced and fixed; current head is 422f8fc.
|
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
LGTM — small, well-scoped fix that brings the last CLI AsyncHTTP site in line with the shared proxy/TLS env handling.
What was reviewed:
- The arena-backed
&'static LoadersatisfiesAsyncHTTP<'static>for the borrowed proxyURL; caller isrender_markdown_file_and_exit() -> !, so the arena is never reset and the non-Dropparking matches existing uses in this file. get_http_proxy_for/get_tls_reject_unauthorizedare the same helpersbun upgrade/bun createuse; per-URL resolution and theNone/truedefaults preserve prior behavior when no env is set.- Tests are hermetic (local
Bun.serve/netservers,port: 0, pty viaterminal), assert both what each server saw and what was staged, coverhttp_proxy/https_proxy/NO_PROXY/NODE_TLS_REJECT_UNAUTHORIZEDplus the negative default, and clear ambient proxy env for the pre-existing staging test.
Extended reasoning...
Overview
The PR touches two files: a ~15-line net change in src/runtime/cli/run_command.rs::prefetch_remote_images and ~180 lines of new tests in test/cli/run/markdown-entrypoint.test.ts. The Rust change replaces Options::default() in the markdown image prefetch's AsyncHTTP::init with an Options { http_proxy, reject_unauthorized, ..Default::default() }, sourced from a freshly loaded DotEnv::Loader. The loader is parked in runner_arena() so the borrowed proxy URL<'_> it returns is 'static, matching the AsyncHTTP<'static> stored in each download.
Security risks
None introduced. Honoring NODE_TLS_REJECT_UNAUTHORIZED=0 for these downloads is user-opt-in and matches every other CLI download path (bun install, bun upgrade, bun create, fetch). The proxy resolution goes through the shared get_http_proxy_for, which already handles NO_PROXY and scheme selection. The prefetch itself is best-effort and silent on failure, so no new error surface is exposed.
Level of scrutiny
Low-to-medium. This is a mechanical alignment of one call site with an established pattern — the PR description correctly identifies it as the last AsyncHTTP site in the CLI not routing through get_http_proxy_for, and grepping confirms create_command.rs and upgrade_command.rs do exactly the same Loader::init() + load_process() + get_http_proxy_for sequence for their one-shot downloads. The only non-obvious part is the lifetime handling: get_http_proxy_for(&self, ...) -> Option<URL<'_>> borrows from the loader's map, so the loader must outlive the AsyncHTTP<'static>. Parking it in runner_arena() (already used throughout run_command.rs for process-lifetime values) makes the borrow 'static; the sole caller render_markdown_file_and_exit returns -> !, so the arena is never reset and the Loader's owned heap allocations not running Drop is intentional and consistent with the file's other arena parkings.
Other factors
The tests are thorough and follow the repo's harness conventions closely: tempDir, bunEnv spread with explicit proxy-env clearing, port: 0, using/await using for server cleanup (server.stop(true) / socket destroy + close), describe.concurrent for the independent subprocess tests, and POSIX-gated because the pty (terminal option) is required to trigger the TTY-only prefetch. Each test asserts the full observable state (proxy log, origin log, staged bytes) in one toEqual on a combined object, then exit code last. The CONNECT recorder buffers to \r\n before asserting the request line, and the withoutProxyEnv spread on the pre-existing staging test prevents ambient CI proxy vars from breaking it now that they're read. The PR description documents USE_SYSTEM_BUN=1 failing 3/5 new tests and the debug build passing all 35, satisfying the fails-for-the-right-reason requirement.
|
Updated 8:05 PM PT - Aug 13th, 2026
❌ @robobun, your commit 422f8fc has some failures in 🧪 To try this PR locally: bunx bun-pr 38088That installs a local version of the PR into your bun-38088 --bun |
Parking it in the CLI arena made LeakSanitizer report the loader's map as leaked (the arena is not scanned as a root), which aborted every image-prefetching run on the asan lane. The function blocks until every download has settled, so the loader can simply live on its stack frame; the proxy URL views are lifetime-erased the same way create_command does it.
Bun.serve does dispatch CONNECT to fetch() with the authority as req.url, so the raw net server is not needed; refusing the tunnel with a 403 is enough to show where the request went.
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
I reviewed this PR and didn't find any bugs. Because it introduces an unsafe erase_lifetime() on a proxy URL borrowed from a stack-local env loader (memory-safety-sensitive per REVIEW.md), and the comment-cop flags on that pattern haven't been resolved yet, a human look would still be worthwhile. Also: the PR description still references runner_arena(), but ee4e395 moved the loader to the stack.
Checked: env is declared before downloads so it drops after every AsyncHTTP holding a proxy view; get_http_proxy_for takes &self so the map cannot reallocate mid-loop; the channel loop drains all downloads before the function returns.
Checked: load_process() returns Result<(), AllocError>, so .unwrap_or_oom() is the right sink.
Checked: the loader-init pattern matches create_command.rs / upgrade_command.rs / package_manager_command.rs.
Tests cover http_proxy, https_proxy CONNECT, NO_PROXY bypass, and both polarities of NODE_TLS_REJECT_UNAUTHORIZED, each asserting what the other server did not see.
Extended reasoning...
Overview
The PR wires http_proxy/https_proxy/NO_PROXY and NODE_TLS_REJECT_UNAUTHORIZED into the markdown remote-image prefetch in src/runtime/cli/run_command.rs:prefetch_remote_images. It creates a stack-local bun_dotenv::Loader, calls load_process(), resolves get_http_proxy_for(&url) per image, and passes http_proxy + reject_unauthorized into AsyncHTTP::init via Options. Because each download is stored as AsyncHTTP<'static>, the borrowed proxy URL is widened via unsafe { proxy.erase_lifetime() }. Five new tests exercise proxy routing and TLS-rejection behaviour under a pty; the pre-existing staging test now clears ambient proxy env.
Security risks
The change reads standard proxy/TLS env vars through the same Loader::get_http_proxy_for / get_tls_reject_unauthorized helpers every other CLI download path already uses, so no new trust surface is introduced. reject_unauthorized defaults to true when the env var is absent (verified in env_loader.rs:315), so certificate verification is unchanged unless the user opts out. No injection or auth surface.
Level of scrutiny
Higher than a config tweak: the fix itself is small and follows established patterns, but it adds an unsafe lifetime-erasure on a borrow that crosses into the HTTP thread. REVIEW.md calls memory safety the most-blocked category and requires being able to name the owner and release path in one sentence. I traced the drop order (env before downloads → dropped after) and confirmed get_http_proxy_for is &self-only, so the borrowed map is never reallocated while views are outstanding, and the channel loop joins all in-flight downloads before either drops. The argument holds, but a maintainer should confirm they prefer stack + erase_lifetime() over the original runner_arena() approach (the PR flipped between them mid-review, and the description still describes the arena version).
Other factors
- Two
comment-copgithub-actions inline comments (lines 3083, 3206) fired after the latest commit and have not been addressed. They flag the lifetime-explanation comments as paragraph-long workaround justifications; whether that's actionable or a false positive on required SAFETY-adjacent commentary is a maintainer call. load_process()'s only error type isAllocError, so.unwrap_or_oom()is correct (not swallowing a real error).- Tests are well-constructed: hermetic (
port: 0, local servers,tempDir), clear ambient proxy vars viawithoutProxyEnv, useusingfor cleanup, run underdescribe.concurrent, and assert the negative contract (what each server did not receive, what was not staged). The PR body reports they fail underUSE_SYSTEM_BUN=1and pass on the debug build.
The AsyncHTTP already borrows its URL from the download that owns it; give the proxy href the same owner instead of borrowing the env loader, so the struct's one self-referential invariant covers both and the loader is just a local.
|
Revised in 76ea85c, following up on the review notes above:
Verified again with |
There was a problem hiding this comment.
I reviewed this PR and didn't find any bugs. Because it extends an unsafe self-referential struct (RemoteImageDownload) with a new borrowed-into field and detached lifetimes, a human look at the memory-safety reasoning would still be worthwhile.
What was reviewed:
- The new
proxy: Option<Box<[u8]>>field mirrors the existingurlpattern: written into theMaybeUninitslot beforeAsyncHTTP, borrowed viadetach_lifetime, and dropped afterasync_http(field order) — looks sound. get_http_proxy_forreturns aURLborrowing from the stackenvloader, butBox::from(proxy.href)copies the bytes, so nothing borrowsenvpast the loop iteration.- Tests: local servers on
port: 0,usingcleanup, per-test isolated tempdirs sodescribe.concurrentis safe, negative cases (NO_PROXY, default TLS reject) included; existing prefetch test now clears ambient proxy env. - Note: the PR description still describes the earlier
runner_arena()/&'static Loaderapproach; the final code (76ea85c) copies the proxy href per-download instead. The comment-cop hit on line 3232 is on a pre-existing comment that was only reflowed to mentionproxy.
Extended reasoning...
Overview
This PR makes prefetch_remote_images in src/runtime/cli/run_command.rs honor http_proxy/https_proxy/NO_PROXY and NODE_TLS_REJECT_UNAUTHORIZED when bun <file>.md downloads remote images for Kitty inline rendering. It creates a stack DotEnv::Loader, resolves the proxy per image URL via get_http_proxy_for, copies the proxy href into a new proxy: Option<Box<[u8]>> field on each heap-boxed RemoteImageDownload, and passes http_proxy + reject_unauthorized in AsyncHTTP::init's Options. Five new tests in test/cli/run/markdown-entrypoint.test.ts cover http_proxy, https_proxy CONNECT, NO_PROXY bypass, and both polarities of NODE_TLS_REJECT_UNAUTHORIZED.
Security risks
None introduced. reject_unauthorized is only relaxed when the user sets NODE_TLS_REJECT_UNAUTHORIZED=0, matching every other CLI download path. Proxy resolution goes through the shared get_http_proxy_for helper, so NO_PROXY is respected per-URL.
Level of scrutiny
High. The change edits an unsafe block that hand-initializes a self-referential struct via MaybeUninit + ptr::addr_of_mut!().write() and then detaches lifetimes to 'static. REVIEW.md flags native memory safety as the most-blocked category. The new field follows the existing url field's pattern exactly (heap-owned Box<[u8]>, written before async_http, freed when downloads drops after the channel drains, and field-declaration order guarantees async_http drops before url/proxy), which is why I believe it's correct — but a maintainer should confirm the SAFETY reasoning rather than have it auto-approved.
Other factors
- The PR description is stale: it describes placing the loader in
runner_arena()and borrowing the proxy URL as&'staticfrom it, but commits ee4e395 and 76ea85c moved to a stack loader with the proxy href copied into each download's ownBox<[u8]>. The final approach is arguably simpler and the code looks right, but the description should be updated. - The comment-cop bot fired on the final commit at
run_command.rs:3232. That 5-line comment is pre-existing (only reflowed to add "proxy" to the field list) and explains whymem::zeroed()would be UB — the kind of "why not the alternative" note REVIEW.md asks for. Likely a false positive, but it's an unaddressed bot comment on the PR. - Tests are well-constructed (hermetic,
port: 0,usingcleanup, concurrent-safe, both positive and negative cases, and the existing prefetch test now clears ambient proxy env so it can't be poisoned by CI env). The bug hunting system found no issues.
There was a problem hiding this comment.
I reviewed this PR and didn't find any bugs. Because it extends unsafe self-referential Rust (MaybeUninit + ptr::write + detach_lifetime on a new borrowed field of RemoteImageDownload) and the latest CI status on this branch is still ❌ (Build #94419, with 043bb7b being only a retrigger), a human look is still worthwhile.
What was reviewed:
- The new
proxy: Option<Box<[u8]>>field isptr::writen beforeasync_httpand participates in the same heap-stable self-borrow invariant asurl; all five fields are initialized beforeassume_init. get_http_proxy_forreturns aURL<'_>borrowing the loader's map, so copyingproxy.hrefinto an ownedBox<[u8]>correctly decouples the request from the stack-local loader.- Matches the established pattern in
create_command.rs/upgrade_command.rs/StandaloneModuleGraph.rs; defaults (Noneproxy,reject_unauthorized = true) are unchanged when the env vars are unset. - Tests cover http_proxy, https_proxy CONNECT, NO_PROXY bypass, and both polarities of
NODE_TLS_REJECT_UNAUTHORIZED; the pre-existing staging test now clears ambient proxy env.
Extended reasoning...
Overview
The PR wires http_proxy/https_proxy/NO_PROXY and NODE_TLS_REJECT_UNAUTHORIZED into prefetch_remote_images in src/runtime/cli/run_command.rs, the code path that downloads remote images when bun <file>.md renders to a Kitty-capable terminal. It adds a proxy: Option<Box<[u8]>> field to RemoteImageDownload, creates a stack-local bun_dotenv::Loader, resolves the proxy per image URL via get_http_proxy_for, copies the href into the download, and passes http_proxy + reject_unauthorized in the AsyncHTTP Options. Five new tests in test/cli/run/markdown-entrypoint.test.ts cover the variant matrix.
Security risks
None identified. The change reads well-known proxy/TLS env vars through the same Loader::get_http_proxy_for / get_tls_reject_unauthorized helpers every other CLI download path uses. reject_unauthorized defaults to true when the var is unset, so the default TLS posture is unchanged. The proxy href is copied into an owned buffer rather than borrowed from the loader, so there is no dangling-slice hazard from the loader dropping.
Level of scrutiny
High. This is unsafe Rust in the memory-safety-critical category REVIEW.md flags most: it extends a self-referential struct built via MaybeUninit + field-by-field ptr::write, and adds a second detach_lifetime'd borrow that the AsyncHTTP<'static> holds across a thread-pool dispatch. The pattern mirrors the existing url field exactly (Box heap data has a stable address; the outer Box<RemoteImageDownload> keeps both buffers alive until after the channel loop has drained every callback), and I traced all five fields through to assume_init. But per the approval guidelines, changes to unsafe self-referential lifetime-erased code should get human eyes even when the pattern looks right.
Other factors
- CI is not green. The robobun status comment reports ❌ on Build #94419 for commit 76ea85c, and the head commit 043bb7b is a bare "ci: retrigger" with no updated passing status visible in the timeline. I don't want to approve over a red build.
- The comment-cop bot flagged three long comments in earlier revisions; all threads are resolved and the current diff only extends pre-existing comments by one word/field name each.
- No custom
DroponRemoteImageDownload, so the newOption<Box<[u8]>>is freed by the derived drop whendownloadsgoes out of scope — no leak. - Test quality is good: local
Bun.serverecorders onport: 0,usingfor cleanup, asserts on both what each server saw and what was staged, exit code asserted last, ambient proxy env explicitly cleared.
The terminal option is ConPTY-backed on Windows and the prefetch path is reachable there, so there is no reason to gate these to POSIX. Verified on Windows x64: the same three tests fail on the released binary and pass with the debug build.
|
422f8fc, after another pass over the diff:
Previous build (#94882) had every staffed lane green; the only open jobs were the darwin 14 aarch64 test lane waiting for an agent. |
There was a problem hiding this comment.
I reviewed this PR and didn't find any bugs. Because the change extends the unsafe self-referential MaybeUninit construction in prefetch_remote_images (adding a second detach_lifetime'd buffer that the cross-thread AsyncHTTP borrows from), a human look at the lifetime reasoning would still be worthwhile.
What was reviewed:
- Traced the new
proxy: Option<Box<[u8]>>field's lifetime — it's heap-owned by the download and freed withdownloadsafter the channel drains, mirroring the existingurlfield exactly; the env loader is not borrowed pastBox::from(proxy.href). - Confirmed the
Loader::init()+load_process()+get_http_proxy_for/get_tls_reject_unauthorizedpattern matchesupgrade_command.rs,create_command.rs, andStandaloneModuleGraph.rs;unwrap_or_oom()fits sinceload_processonly fails onAllocErrorand this function is silent-failure. - Checked the 5 new tests: hermetic (local
Bun.serveonport: 0),usingfor cleanup,withoutProxyEnvisolates ambient proxy vars, each asserts both servers' logs and the staged bytes; the two negative tests correctly pass on the released binary per the description.
Extended reasoning...
Overview
The PR makes bun <file>.md's Kitty-terminal image prefetch honor http_proxy/https_proxy/NO_PROXY and NODE_TLS_REJECT_UNAUTHORIZED, aligning it with bun install, bun upgrade, bun create, and bun build --compile. Two files: src/runtime/cli/run_command.rs (~30 net lines in prefetch_remote_images and RemoteImageDownload) and test/cli/run/markdown-entrypoint.test.ts (5 new tests in a describe.concurrent, plus a withoutProxyEnv helper spread into the pre-existing staging test).
Security risks
None new. The change wires existing, well-known opt-in env vars into a code path that was ignoring them. NODE_TLS_REJECT_UNAUTHORIZED=0 disabling verification is user-driven and already honored by the rest of the CLI's HTTP clients; the default (get_tls_reject_unauthorized() → true when unset) is unchanged, and the PR includes a negative test proving self-signed origins are still rejected without the env var.
Level of scrutiny
Higher than a config tweak: the Rust side lives inside an unsafe block that builds a self-referential RemoteImageDownload via MaybeUninit + ptr::write, then hands 'static-erased slices to an AsyncHTTP whose callback runs on the HTTP worker thread. The new proxy buffer is a mechanical mirror of the existing url buffer (same ownership, same detach_lifetime, same drop point after the channel loop), and I traced the invariant end-to-end and found it sound. But per the repo's review guidance, native memory safety around borrowed slices crossing thread boundaries is the most-blocked category, so a maintainer familiar with AsyncHTTP's borrow contract should sign off rather than an automated approval.
Other factors
- The env-loader creation (
DotEnv::Loader::init()+load_process().unwrap_or_oom()) matches five other CLI sites;unwrap_or_oomis correct here becauseload_processreturnsResult<(), AllocError>andprefetch_remote_imageshas no error return (failures are silent by design). - The comment-cop bot flagged earlier revisions three times for paragraph-long justifying comments; all were resolved in 76ea85c by moving proxy-href ownership onto the download itself, eliminating the
erase_lifetime()on a loader borrow. The current diff reflects that resolved state and all inline threads are marked resolved. - Test coverage is thorough (http_proxy absolute-URL GET, https_proxy CONNECT, NO_PROXY bypass, TLS-reject on and off), hermetic, cross-platform via
Bun.spawn'sterminaloption, and verified to fail on the released binary and pass with the fix on both Linux (incl. ASAN leak detection) and Windows per the PR description.
…ps, WebSocket, SQL) (#44618) ### What does this PR do? Consolidates the open TLS pull requests into one. Each was reproduced on `main` and, for `node:*` behavior, on Node v26.3.0 first. About a third are ported as written, the rest are rewritten smaller or merged into one fix where several PRs patched the same cause. One commit per fix, so it can be read commit by commit. Fixes #43520, fixes #31396, fixes #43635, fixes #37193, fixes #43846, fixes #17932, fixes #41061, fixes #36887, fixes #31810, fixes #35240, fixes #32234, fixes #44365, fixes #43807, fixes #42280, fixes #44517. Addresses #41856 (SNI and `servername`; not `checkServerIdentity` for SQL), #24845 (the spin is gone, shown with fault injection on Linux; not run on macOS), #19754 (node-fetch forwards the agent's TLS options; the Kubernetes client itself was not run). #### The ones that matter most | | On `main` | PRs | |---|---|---| | Client certificate disclosure | `https.request()` with a client certificate sends it to a server it then refuses (wrong name, `checkServerIdentity`, `destroy()` in `'secureConnect'`, `terminate()` in `handshake`). A server can force it with a junk record behind its Finished | #43946 | | False `authorized` | Over a Duplex, `secureConnect` with `authorized === true` for a peer that failed the key proof; `secureConnect` for a plaintext peer with `rejectUnauthorized: false` | #44422, #32929 | | Cleartext https | `https.createServer()` without a usable key/cert answers plain HTTP | #41672, #33539 | | Revoked client certificates | An https mTLS server never sees `crl`, so a revoked client is `authorized` | #41641 | | Pooled sockets | Requests with different client certificates or CAs share an `https.Agent` socket and session | #42498 | | Silent plaintext | `tls: [...]` given to `Bun.listen` / `Bun.connect` is plain TCP | #41490 | | Server weakened by a client knob | `NODE_TLS_REJECT_UNAUTHORIZED=0` turns off a server's client-certificate enforcement | #35245 | | Pins never checked | `WebSocket` never calls `tls.checkServerIdentity` and ignores `tls.serverName` | #41648 | | `verify-full` dropped | `PGSSLMODE=verify-*` is lost next to a `TLS_*` URL variable; `tls: true` sends no SNI | #44498 | | Crashes | use-after-free from `destroy()` in `ALPNCallback` over a Duplex; `abort()` on a late `setSession()`; SIGABRT in `fetch` with an https proxy from the environment and a `Bun.file()` body | #44462, #41671, #44458 | | Stream corruption | A TLS `write()` can lose 16 KiB it reported as written while another socket on the loop is stalled | #44529 | | Hangs and spins | 100% CPU on a failing `send()`; a fatal `SSL_write` leaves the socket open forever; `idleTimeout` never sheds a TLS client that ignores `close_notify` | #34510, #38176, #42336 | | Wrong certificate (regression since 1.3.14) | Connections accepted before `stop()` / `close()` get the default certificate and skip their entry's `requestCert` / `ca` | #42355 | | Quadratic Duplex / proxy tunnel | Reading one chunk over a Duplex, CPU: 8 MB 0.88 s → 0.14 s, 16 MB 3.18 s → 0.23 s, 32 MB 11.75 s → 0.39 s; `fetch` upload through CONNECT: 1.7 s → 0.18 s (debug build) | #44464 | #### By area - **fd engine, write path** (`openssl.c`, `socket.c`): #42352, #34510 + #38176 + #42336 as one change, #44529, #44458, #44192. A rejected `send()` ends the write side only and closes at the next writable event unless the peer's bytes are still queued (a 413 sent before a reset is still read). No new per-socket state. Also, on kqueue, **a FIN no longer ends a socket that waits in the low-priority queue** (`loop.c`): with more than 5 TLS handshakes at once, a client that ended right after its handshake could be reset and its server socket report `socket hang up`, because the eof that the sentinel read knote reports was acted on ahead of the unread Finished. That is on `main` too (the macOS entry for `node-tls-server.test.ts` in `test/flaky-tests.txt`: 7 of 48 recent builds of other branches), and this branch made it likelier (6 of 8 builds), since Finished now leaves in one segment with the close_notify. - **Error reporting, both engines**: #44422, #32929, #44516, #37094, #41272 + #42324 + #44223 as one change, #44021, #37472, #43946, #33630. One channel: a fatal error on an established session is reported, then **the engine closes the connection itself**, whatever the owner does with the report. `test/js/bun/net/tls-fatal-error-closes.test.ts` asserts closed-and-nothing-delivered for every owner (node:tls, `Bun.connect`, `Bun.listen`, `fetch` direct and through CONNECT, `Bun.serve`, `WebSocket` direct and through a proxy, Postgres, MySQL, Valkey, Duplex). - **Duplex engine** (`SSLWrapper`, `UpgradedDuplex`): #44462, #43529, #42332, #44464. #43877 + #44394 were in and are **out again**, see "Worth a look" 5. - **node:tls wrap lifecycle** (`net.ts`, `tls.ts`): #38007, #38058, #38028 + #38122 + #38076 as one change (six copies of the attach code become two helpers), #38311, #39008, #38154, #42340 + #42343 + #42339 + #42453 as one change, #43791, #42425, #44085, #42683, #39088, #39040, #40375, and what was still real of #36534. - **SNI, ALPN, server contexts**: #43080, #42050, #37195 + #43849 as one change (**one** SNI matcher for TCP and HTTP/3), #42355, #42285, #33253, part of #37896, part of #37013. A `tls.Server` has one `SSL_CTX`. - **Verification and options**: #44738, #41490, #37005 + the cwd pin of #40984, #31811, #43982, #33483 + #35245, #41810, #32235, #44441, #38092. - **node:tls API and CA store**: #41671, #38145, #32824, #43594, #39997, #41696, #33534, #34748, #42991, #42996, #42970. - **node:https, Agent, `ws`, node-fetch**: #41672 (https half), #41641, #38261, #42498, #44346, #35609, #31397, #42325. - **WebSocket client**: #41648, #37487 + #43048 as one change. - **SQL, Redis**: #33666, #41711, #44498, part of #42054. - **Tests only**: #41426, #40040, #44395, #44016, #37860, #40591, #44440, #41424. Found on the way and fixed here: an upload that a TLS 1.2 server interrupts with a renegotiation never completes on `main` (0 of 32 runs over `https.request`, `fetch`, `node:tls` and `Bun.connect`: the renegotiation ClientHello lands inside an application record that is still unsent, or the socket gets no `drain` again) and completes here, with two tests from robobun; the fix for #40653 (final flight and first write in one segment) stopped working whenever another TLS socket on the loop was stalled, on `main` too; the `tls.Server` prototype pinned the last server constructed and every `SSL_CTX` it owned; `Object.create(process.env).NODE_TLS_REJECT_UNAUTHORIZED = "0"` turned verification off process-wide once a `SHARE_ENV` worker existed; two debug panics when wrapping a shut-down or still-connecting socket; a `fetch` POST through a proxy sent its headers twice when the origin renegotiated; `BlockList` ignored IPv6 zone ids; a test now ties `root_certs.der` to `certdata.txt`. #### Behavior changes - **A server's `ca` without `requestCert` no longer asks for a client certificate** (`Bun.serve`, `Bun.listen`, HTTP/3, node:tls). It matches the docs and Node. On `main` such a server refused clients with no certificate but served any unrelated self-signed one, so it was never authentication. **Set `requestCert: true` to require a certificate.** A matrix test pins that `requestCert: true` still refuses no certificate and an untrusted one on 8 kinds of server, TLS 1.2 and 1.3, with `NODE_TLS_REJECT_UNAUTHORIZED` unset and `0`. - `NODE_TLS_REJECT_UNAUTHORIZED=0` no longer relaxes a server. - `Bun.connect` / `Bun.listen` hear of a fatal TLS error after the handshake through `error(socket, err)`. With no `error` handler the socket just closes. - HTTP/3 server names match like TCP: `*.` covers exactly one label, case is ignored, a trailing dot is ignored, the last registration of a name wins. - `requestCert` on node:https is `=== true`, as in Node. - An array where a generated options dictionary is expected throws (`tls: []`, `jest.useFakeTimers([])`). - `key` / `cert` arrays serve every identity. A client that can use both gets ECDSA, where `main` served whichever pair came last. - `ecdhCurve` is forwarded by node:https, `ws` and node-fetch now, so a group BoringSSL lacks (`X448`) throws there as it already does in `tls.createServer`. - A wrapped socket's error is re-emitted on the TLS socket as in Node, so `raw.destroy(err)` with a listener on `raw` only is uncaught, as in Node. - `sql.options.tls` is always an object, never `true`. `RedisClient` sends SNI. - `tls: { secureContext }` alone asks for TLS on `Bun.listen` / `Bun.connect` (it was plain TCP), and a value that is not a `SecureContext` throws. The context is served as it is: the `requestCert` / `rejectUnauthorized` it was created with hold whatever the options next to it say, and `requestCert` in the options over a context that does not ask throws at `listen()`. - `tls.DEFAULT_CIPHERS` reaches every client once assigned (`fetch`, `WebSocket`, `Bun.connect`, `RedisClient`, `Bun.SQL`, `S3Client`, proxy tunnels) and servers again. A list that selects no cipher throws `ERR_SSL_NO_CIPHER_MATCH` at the assignment. `fetch.preconnect()` dials nothing after an assignment. - The warning for an unreadable `NODE_EXTRA_CA_CERTS` is Node's one line, without the `warn:` prefix. - `BUN_CONFIG_WS_CLOSE_TIMEOUT` (default 30 s): how long a `WebSocket` client waits for the server to close the connection after the closing handshake. #### Worth a look in review 1. **#44529**: the kernel-refused remainder of a TLS write moves from the loop's one slot onto the connection (in the existing rare struct), so the write BIO never refuses a sealed record. Nothing is allocated on an unstalled path (200 writes: 0 appends, same `send()` count as `main`), memory with 16 stalled writers is lower than on `main` (276 KB vs 340 KB, which `main` holds inside BoringSSL's buffers), `us_socket_t` stays 80 bytes. It needs a bound on how long a deferred close waits, or a peer that stops reading pins the fd past `destroy()`: `US_SSL_CLOSE_AFTER_SPILL_TIMEOUT` is a fixed 10 s, not re-armed on progress. Separate commits, but the fix that keeps the client certificate off the wire beside a stalled socket builds on them. 2. **The default name check of node:tls also runs inside the handshake**, so a wrong-name server gets no client certificate on TLS 1.2 either. JS still runs it after every successful handshake, so a difference between the two matchers can only refuse. Error objects are byte-identical. 3. **#44441** widens trust by design: a self-issued leaf whose `keyUsage` lacks `keyCertSign` (`dotnet dev-certs`) is its own anchor when the store holds a byte-identical copy. No BoringSSL change. Expired pin, same subject with another key, wrong EKU and a pinned intermediate are tested to fail. 4. **#32235** only adds Ed25519 and ECDSA P-521 to the verify list. A captured ClientHello shows `main`'s list with the two inserted; `rsa_pkcs1_sha1` stays. 5. **A stream that a TLS socket wraps, when that TLS socket closes.** An earlier state of this branch lost data here while CI was green (found by #44709's report): with the peer closing first, 4 of 8 MiB arrived with TLS in TLS, 4 of 32 MiB on the http2 `emit("connection")` path, and a `write()` with no `'error'` listener ended the process. Three Node-parity changes only hold together: destroying the wrapped stream at the close (#38028 + #38122 + #38076, #38154) is safe only if every write has really completed (#43877), which in turn needs Node's handling of the peer's close_notify, which needs half-open sockets that the GC can collect. So: - #43877 + #44394 are reverted and reopened. A write over a stream completes once the stream has taken the ciphertext, as on `main`. - Until the verdict on the peer lets the session through, the application cannot have written over it. There the wrapped stream is destroyed as in Node, with the sessions below it. That keeps the release of the connection after a failed handshake, a rejected certificate and an early `destroy()`. The same for an http2 socket the application never got, and for `resetAndDestroy()`. - After that it is `main`'s teardown: a `net.Socket` only gets the engine's `end()`, closes at its peer's FIN, keeps its own timeout and reports its own errors. Any other stream is destroyed with the TLS socket. The regular suites cannot see any of this (999 files were green on every broken variant), so it was steered by eleven seeded differential fuzzers run on this build, `main`, Node v26.3.0 and the earlier state: close, `end()`, `destroy()`, `destroySoon()`, resets, hung and half-open peers, paused writers, timeouts, two and three sessions deep, over TCP and over Duplexes, before, at and after the handshake, and http2 requests. See "How did you verify". #### Known limits - `fetch` with a `checkServerIdentity` function still sends the client certificate (not the request) to a server the function refuses. On TLS 1.2 any verdict a JS callback gives is too late, as in Node. - `addContext()` / `SNICallback` still do not apply to a server-side socket on the stream engine (`emit("connection", duplex)`, TLS in TLS, unflushed writes, named pipes), as on `main`. - A CA bundled in a pfx extends an explicit `ca` only, for `ws` / node-fetch / `WebSocket`: the native `ca` can only replace the default store, and that store keeps `SSL_CERT_FILE` / `SSL_CERT_DIR`. - P-521 leaves work on TLS 1.3 only. TLS 1.2 needs secp521r1 in every ClientHello (`it.todo`). - Once `tls.DEFAULT_CIPHERS` is assigned, `fetch(url, { protocol: "http3" })` is `HTTP3Unsupported`, as with an explicit `ciphers`. - `addCACert()` by hand does not extend the chains of a context with several identities. - A throwing `ALPNCallback` sends `no_application_protocol` on both engines. Node sends nothing and its client sees `ECONNRESET`. - TLS in TLS, peer FIN while the outer handshake runs: the inner socket gets one `write EPIPE`, where Node gives `ECONNRESET` (`main` gives it no error at all). - `@SECLEVEL` in `ciphers` is dropped by the `ws` / node-fetch shims, which used to ignore `ciphers`. node:tls keeps throwing `ERR_SSL_INVALID_COMMAND`. - Beside a stalled TLS socket only the first record (16 KiB) of the first write leaves with the handshake flight. The rest goes record by record, which is what bounds the memory of stalled writers. - After a fatal error on an established session the socket emits `'error'` and then `'close'`. Node emits `'error'` and leaves the socket open. - A paused reader whose own write the kernel rejects loses what it had not read yet, with an `EPIPE`, as on Node. `main` reports no error there and delivers it. - On `main` too: a `Bun.listen` socket without `allowHalfOpen` that has unsent ciphertext when the client's `shutdown()` arrives loses that ciphertext (32 KiB), and over plain TCP `end()` with the peer still sending is a close over unread input, so a reset. - Differences from both `main` and Node that the differential runs below found and that stay, all with a peer that aborts: `ECONNRESET` instead of a clean `'end'` after the socket's own `'finish'` when the peer destroyed with unread data; under TLS 1.2, a zero-length `write()` followed by `destroy()` in `'secureConnection'` leaves the client without `'secureConnect'` (a plain `destroy()` there matches Node); a TLS 1.2 client that destroys in `'secureConnect'` gets no `'session'`; a `ClientRequest` whose handshake fails with an alert emits `'error'` and `'close'` but no `'finish'` (`writableFinished` is true). - Once `tls.DEFAULT_CIPHERS` is assigned, `fetch.preconnect()` opens nothing: `fetch()` then uses a context of its own, and a socket warmed under the default one would never be picked up. - A TLS `send()` that the kernel refuses outside a `write()` call (the drain of unsent ciphertext) is reported with the close, as `read EPIPE` / `read ECONNRESET`. Node says `write EPIPE`. `main` does not report it at all. - Once the application has a session over a `net.Socket` (TLS in TLS, http2 `emit("connection")`), a peer that never sends its FIN holds that socket after the TLS socket closed, as on `main`. Node destroys it. Two tests of #38154 are `todo` for this. Closing it any earlier (at its `'finish'`, say) makes the kernel drop what it has not sent yet as soon as the peer's close_notify arrives. - Plaintext that was queued on a socket before it was wrapped (STARTTLS with a backlog) is dropped when the TLS socket is destroyed, or its handshake fails, before the session is accepted. Node drops it too, except on `destroySoon()`. `main` sends it. - Over a stream that is no `net.Socket`, `end()` can still cut what that stream has buffered, and there is no backpressure, both as on `main` (#43877). - `tls.secureContext` (the undocumented door node:tls uses) is not read by a Windows named pipe listener, which builds its context from the options. On `upgradeTLS({ isServer: true })` the options next to it are the policy, as with Node's `SetVerifyMode`. - `selectServerName()` rebuilds the name tree per ClientHello for injected sockets of a server with `addContext()` entries: 0.4 µs for 1 entry, 3.7 µs for 10, 41 µs for 100, against 631–1111 µs for a handshake. #### Not included Left open, because they need a decision or are not TLS: #43877 + #44394 (see "Worth a look" 5; #43874 stays open with them), #38548, #38591 (both shrink who is trusted), #41589 (`verify-full` vs `NODE_TLS_REJECT_UNAUTHORIZED=0`), #37197, #41706, #43216, #33487, #33545, #36707, #32435, #37255, #28691, #40275, #30314 (features), #38120 (needs the BoringSSL fork, as did #33517, which the stale bot has closed since), #38529 (needs a Windows measurement), #34342, #38232, #43089, #44454, #40451, #42710, #44527, #38088, #38093, #41898. #37896, #42054 and #37013 stay open for the halves not taken. `http.createServer({ key, cert })` keeps serving TLS on purpose. One open question: `tls: {}` (an object that names no TLS option) is plain TCP on `Bun.listen` / `Bun.connect`, here and on `main`. It is the same trap as `tls: []`, but changing it changes a Bun default, so it is left alone. ### How did you verify your code works? - Every new test fails on `main` for the stated reason and passes here, except guards that pin existing behavior, each shown to fail when its clause is removed. `node:*` tests also pass on Node v26.3.0; the few that cannot say which Node version has the behavior. - 212 test files that touch TLS, sockets, http, http2, fetch, WebSocket, SQL, Valkey and workers: 6154 pass, 2 fail. Both are seen on `main` too: `serve.test.ts` "root range port" (the box runs as root), and `worker_threads.test.ts` "terminate(): nothing of the worker's runs after the request", which is flaky there and passed in the run below. - 58 of those files the way the ASAN lane runs them (LeakSanitizer + `BUN_JSC_validateExceptionChecks`): 58 files, 48 of them with leak checking, 4132 pass, 3 fail. All three also fail on `main`: `serve.test.ts` "root range port", `node-net.test.ts` "should not leak when connect({path}) fails synchronously on a reused handle" (times out under this environment), `worker_threads.test.ts` "process.exit() with a shell cp in flight" (a `ShellCpTask` leak). - 647 vendored `test-tls-*`, `test-https-*`, `test-net-*`, `test-http2-*`: the only two failures also fail on `main`. - The SNI matcher was diffed against both old matchers: 3 seeds × 1.23 M lookups × 3 registration flavours, every difference in one of the intended classes, TCP and HTTP/3 identical on every lookup. - The headline rows were also driven by hand with scripts against this build, `main` and Node v26.3.0: cleartext https, `crl`, `tls: []` / `{ secureContext }`, the `ca` / `requestCert` matrix, the client certificate on a wrong-name server, late `setSession()`, `destroy()` in `ALPNCallback`, `[rsa, ec]` identities with an intermediate from `ca`, `WebSocket` `checkServerIdentity`, a corrupted record, the Duplex read above, `tls.DEFAULT_CIPHERS`. - The `setSession()` guard was checked against the real `abort()` at 43 handshake states. - `bun run rust:check-all`: 12 of 12 targets. `tsc`, oxlint, source lints, prettier, rustfmt, mordant clean. - usockets' `_Nonnull` is compiled out of debug builds, so 105 of those files were also run on a local release ASAN build with the CI runner's environment (92 with leak checking): 4595 pass, 1 fail, `child_process.test.ts` "spawn reports EPERM after dropping privileges", which cannot pass as root and fails on `main` too. - The close of a TLS socket over another stream ("Worth a look" 5): eleven seeded differential fuzzers, 8,424 scenarios compared, each run on a release ASAN build of this branch, on `main`, on Node v26.3.0 and on the earlier state of the branch. Against `main`: - Data that `main` delivers in full is cut in 5 scenarios, and about 150 that `main` cuts arrive in full. Of the 5, in 2 `main` never notices the peer's close and keeps the socket for good, 2 call `end()` on the middle one of three sessions over an in-memory Duplex, and 1 does the same on Node. - No dead timeout, no silent reset and no uncaught error that `main` does not have (4 uncaught errors fewer). - A socket stays open where `main` closes it in 109, and closes where `main` keeps it in 295. 92 of the 109 do the same on Node or on the earlier state (a `destroy()` that an in-memory Duplex does not show its peer, half-open peers). 14 wait for a peer that paused reading and so does not read the FIN (#42332's backpressure, as in Node); the socket's own timeout fires there. 3 are left: one on a 5 ms timer, two with three sessions over an in-memory Duplex. - The earlier state of the branch cut data in 173 of the 400 scenarios of one of them, where `main` cuts none and this cuts none. - 23 new tests pin what they found. Each earlier attempt at this fix fails the ones that describe it, the earlier state of the branch fails 7, and all pass on Node. - After that change: 999 test files on the release ASAN build (20,246 pass; the 11 files that fail need a database, Docker, DNS or a non-root user, or share a temp directory with a parallel run and pass alone), 61 on the debug build. - TLS over a file descriptor (`openssl.c`, the path of `fetch`, `Bun.serve`, `tls.connect`, `Bun.connect`) got the same treatment after the rebase: seeded differential fuzzers on CI's release build of this branch, on `main` and, for `node:*`, on Node v26.3.0. Every runtime also against itself for the noise floor, injected faults and known bugs of `main` as positive controls, and a difference counts only if it shows in 5 of 5 fresh processes. - `node:tls` over TCP: 11,500 scenarios (one connection with Node as the oracle line by line; 2 to 60 connections beside stalled neighbours; raw peers that break the handshake). HTTPS: about 136,000 runs over `Bun.serve` + `fetch`, `node:https`, `node:http2` and `wss://`, also with the two ends in different runtimes. `Bun.connect` / `Bun.listen` / `upgradeTLS`: 11,500 scenarios and 720 slow connections, with writers driven by what `write()` returns, beside up to 6 stalled, dripping, closing or resetting neighbours, and plain TCP as a second oracle. No crash, hang, duplication, reordering or silent truncation, and no change in time or in connection reuse. - They found six things that `main` does better, none of which any test showed. All are fixed, each with a test that fails on the build before: what the peer sent lost behind a rejected `send()` (23 scenarios, and an early HTTPS response lost with only `EPIPE`), the same silently for a paused reader, `server.close()` never calling back on a half-open server after a ClientHello and a reset (17), `closeAllConnections()` taking 12 s with a stalled client, `end()` losing up to 1.3 of 4 MiB that `write()` had reported while the peer still uploads, and `end()` a little after a stall never closing beside other stalled TLS sockets. The last two fixes also deliver the 1 to 2 MiB that `main` loses there, and close the socket that `main` keeps for good without such neighbours. - All of them again after every fix, on CI's release build of it. That caught one regression of a fix itself (a reader stopped for backpressure lost 86,385 bytes, 1 of 6,000 scenarios), fixed too. On the last build: scenarios that lose data where `main` does not 23 → 2, and Node loses it in both, with the same `EPIPE`; `server.close()` that never calls back 17 → 0; connections held 4 → 0; requests that end in an error only where `main` has a response 6 → 0. With a Node server in another process, a request ends in an error only in 8 and 10 of 1,500 scenarios here, 5 and 3 on `main`, 10 with Node as the client. - `Bun.connect` / `Bun.listen` on the last build against `main`, in scenarios: hangs 0 against 1,031, sockets and fds never released 0 against 965, corrupted data 0 against 345, `abort()` 0 against 26 (`setSession()` after the handshake), writers that never close 0 against 101 of 720 connections. No kind of failure shows here and not on `main`. About a third of the slow connections close later than on `main`, in 1 to 16 s instead of at once, waiting for unsent ciphertext or for the peer's close_notify, and 79 more of them deliver all that `write()` reported. RSS and time with 16 to 256 stalled writers are the same. - What they found that `main` does worse: a `WebSocket` that calls `close()` with sends pending loses messages in 81 of 999 scenarios (0 here), 37 server sockets left open, 10 `server.close()` that never call back, 20 write callbacks that never run. - The kqueue fix cannot be run on Linux. The `connectionListener` count test now says what became of a missing connection, which is how the cause was found (`'tlsClientError'` "socket hang up", then `read ECONNRESET` at the client of the same port, after its `'secureConnect'`). On macOS x64 it failed every attempt of the three builds before the fix and passed at the first attempt of the build with it. - Windows and macOS were only run by CI. Four new tests asserted what only the Linux kernel does (a FIN read ahead of a reset, unread bytes surviving a reset, loopback buffer sizes, `fstat()` on a socket) and now say so per platform. --------- Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com> Co-authored-by: robobun <117481402+robobun@users.noreply.github.com>
This branch has not been deployed
1 participant
Problem
bun README.mdin a Kitty-compatible terminal downloads the document'shttp(s)images so they can be drawn inline. Withhttp_proxy/https_proxyset (a network that is only reachable through the proxy), every one of those downloads connects to the image host directly and fails. The prefetch is silent by design (the image falls back to its alt text), so the user just never sees images and nothing says the proxy env was ignored.NODE_TLS_REJECT_UNAUTHORIZED=0is ignored here as well, unlikebun install,bun pm view,bun upgrade,bun createandbun build --compile.prefetch_remote_images(src/runtime/cli/run_command.rs) builds each download withAsyncHTTP::init(..., Options::default()), sohttp_proxyisNoneandreject_unauthorizedstays at its default. No env loader is available to it (it is reached fromboot_and_handle_error, before the VM and its env loading exist), so nothing was reading the variables.bun install,bun audit,bun pm view,bun upgrade,bun createand thebun build --compiletarget download all resolve the proxy throughLoader::get_http_proxy_for.bun publish/bun pm whoamiget the same proxy treatment in publish, pm whoami: route registry requests through http_proxy #38075, andbun audit,bun publishandbun pm whoamialso never setreject_unauthorized(audit_command.rs,publish_command.rs,install/npm.rs); that is left for a follow-up, since each of those sites reads its settings from aPackageManagerrather than from a loader of its own.bun1.4.0 (details below): withhttp_proxypointing at a local recording proxy andNO_PROXYunset, the origin logsGET /img.pngand the proxy logs nothing.Fix
prefetch_remote_imagesloads the process environment into a localbun_dotenv::Loader(only once it knows there is at least one remote image, next to the existinghttp_thread::init). For each image it copies the href returned byenv.get_http_proxy_for(&url)into a newproxy: Option<Box<[u8]>>field on that image'sRemoteImageDownload, and builds the request withhttp_proxyparsed from that field andreject_unauthorized: Some(env.get_tls_reject_unauthorized()).urlbuffer owned by the same download (the struct is documented as self-referential for that reason); the proxy href gets the same owner, so the one existing invariant (the buffers are freed when the download is, after the channel loop has waited for every request) covers both, and nothing borrows the loader. An earlier revision parked the loader in the CLI arena instead; LeakSanitizer does not scan the arena, so the asan lane reported the loader's map as leaked and aborted every prefetching run (build 94315).get_http_proxy_foris the one definition of how the proxy env applies to a CLI download (http_proxyforhttp://targets,https_proxyforhttps://, nothing for hosts matchingNO_PROXY), and it is resolved per image URL because a document can reference several hosts. Process env is the right source: the markdown renderer never loads.envfiles and should not start to because of images. With no proxy variables set,get_http_proxy_forreturnsNoneandget_tls_reject_unauthorizedreturnstrue, so behaviour is unchanged from before.test/cli/run/markdown-entrypoint.test.ts(a newdescribe; the prefetch only runs when stdout is a TTY, so the child is spawned withBun.spawn'sterminaloption, which is a pty on POSIX and ConPTY on Windows, so the tests run on every lane): anhttpimage is requested from thehttp_proxyasGET http://127.0.0.1:<origin>/img.pngand the proxy's body is what gets staged; anhttpsimage reaches thehttps_proxyasCONNECT 127.0.0.1:<origin>; a host listed inNO_PROXYis fetched directly; a self-signedhttpsimage is fetched withNODE_TLS_REJECT_UNAUTHORIZED=0and still rejected without it. Each test also asserts what the other server did not see. The existing staging test now clears the ambient proxy variables, since the prefetch reads them from this change on.USE_SYSTEM_BUN=1 bun test test/cli/run/markdown-entrypoint.test.ts: thehttp_proxy,https_proxyandNODE_TLS_REJECT_UNAUTHORIZED=0tests fail (origin gets the request, proxy sees nothing, nothing fetched from the self-signed server), the two negative tests pass.bun bd test test/cli/run/markdown-entrypoint.test.ts: 35 pass on Linux, also with the asan lane'sASAN_OPTIONS=detect_leaks=1:abort_on_error=1andLSAN_OPTIONSexported the wayscripts/runner.node.mjssets them.Background
bun <file>.mdrenders to a terminal that supports it (KITTY_WINDOW_IDset, a Kitty-likeTERM, or a probe answers),prefetch_remote_imagesdownloads everyhttp(s)image in the document into temp files before rendering; every failure is silent and the image is rendered as its alt text instead. This is why the symptom is simply "no images".AsyncHTTP(src/http/AsyncHTTP.rs) is the request object behindfetch()and the CLI's downloads. ItsOptions::http_proxyis the only way a request learns about a proxy: for anhttp://target the client connects to the proxy and sends the request with an absolute URL (GET http://host:port/path), for anhttps://target it asks the proxy for aCONNECT host:porttunnel.Options::reject_unauthorizedis the certificate verification switch,trueby default.bun_url::URL<'a>is a borrowed view (every component is a slice of the bytes it was parsed from, andhrefis the whole input), which is why the prefetch keeps the bytes of both URLs alive on the download for as long as the request runs.bun_dotenv::Loaderis Bun's env map.load_process()copies the process environment into it;get_http_proxy_for(&url)readshttp_proxy/HTTP_PROXYorhttps_proxy/HTTPS_PROXYdepending on the URL's scheme and returnsNoneif the URL's host matchesno_proxy/NO_PROXY;get_tls_reject_unauthorized()isNODE_TLS_REJECT_UNAUTHORIZED != "0".bun upgrade,bun createandbun pm cache rmcreate a loader this same way for their own one-shot commands.Probe against the released binary (bun 1.4.0)
Recording proxy in
http_proxy, origin on127.0.0.1counting direct hits,NO_PROXYunset,bun ./doc.mdspawned under a pty withKITTY_WINDOW_ID=1wheredoc.mdcontains one image pointing at the origin:[review] gate passed · iteration 1 · 2 files touched
fails on main (without fix)
passes on PR (with fix)
diff hotspot
gate history · 1 passed · 0 rejected · iteration 1
evidence per changed file