Skip to content

fetch: never derive TLS SNI or certificate identity from the Host request header - #35886

Merged
Jarred-Sumner merged 2 commits into
mainfrom
farm/6e8ce95a/fetch-host-header-tls-identity
Aug 26, 2026
Merged

Jarred-Sumner merged 2 commits into
mainfrom
farm/6e8ce95a/fetch-host-header-tls-identity

Conversation

@robobun

@robobun robobun commented Jul 26, 2026 •

Copy link
Copy Markdown
Collaborator

Problem

Fix

  • get_tls_hostname() resolves to tls.servername when set, otherwise url.hostname. The Host header still goes on the wire and no longer touches TLS.
  • Remove the plumbing that carried the header to TLS, including its term in the connection pool keys (list in Notes). The proxy-tunnel inner SNI uses get_tls_hostname() too.
  • Correct because the TLS identity is the authority the client dialled (RFC 6125 / RFC 9525). Node's fetch (undici) and curl do the same. tls.servername stays the opt-in for "dial an IP, verify a name".
  • Verified: test/js/web/fetch/fetch.tls.test.ts (three cases) and test/js/bun/http/proxy.test.ts (tunnel case) fail on stock bun and pass here. Neighbouring TLS, proxy, keep-alive and h2 suites pass (Notes).

Breaking change

  • fetch("https://10.0.0.5/", { headers: { Host: "internal.example" } }) against a certificate for internal.example used to pass verification and now fails with ERR_TLS_CERT_ALTNAME_INVALID. Migration: add tls: { servername: "internal.example" }. Node's fetch already fails this case, so the migrated code is also portable.

Background

  • SNI is the hostname in the TLS ClientHello. A server with several certificates picks one by it.
  • Certificate verification compares the SAN entries to the name the client meant to reach. If SNI and that name differ, an SNI-routing server returns a certificate that cannot pass.
  • The keep-alive pool, h2 session registry and TLS session cache share one key. Requests with different Host headers now share one connection, as for plain HTTP.
  • fetch: verify certificate against URL authority (not Host header); apply per-request tls options over unix sockets #35889 changed only the verification target and kept the Host header as SNI. Against an SNI-routing server that fails every request.
Notes

Node's fetch deletes a caller-supplied Host header before dispatch (undici lib/web/fetch/index.js, httpRequest.headersList.delete('host'), present since undici 5). Bun keeps sending the header, which users rely on (#20173); only its effect on TLS changes here. Node's https.request is different: its agent derives servername from the Host header in JS (calculateServerName). Bun's node:http client is a socket-based port of that code and does not go through this HTTP client, so it is unaffected.

Removed plumbing, in full: HTTPClient.hostname, FetchOptions.hostname, FetchTasklet.hostname, async_http::Options.hostname, the init_sync hostname parameter (15 call sites), InternalStateFlags.clear_hostname_on_redirect and its set/consume sites in do_redirect, do_redirect_multiplexed and handle_response_metadata, the Host branch of proxy_auth_hash(), the direct-TLS proxy_auth_hash() terms in HTTPContext::connect, release_socket and session_cache::install (now CONNECT tunnels only, which is what the hash covers), h2::ClientSession.host_header_hash, h2::PendingConnect.host_header_hash, strip_port_from_host().

Suites run with the fix: fetch.tls, fetch-keepalive, fetch-http2-adversarial, fetch.tls.wildcard, fetch.unix, fetch-proxy-tls-intern-race, fetch-tls-abortsignal-timeout, regression/issue/27890, bun-serve-ssl, node/tls/ssl-ctx-cache, all of bun/http/proxy.test.ts. fetch.test.ts has 41 failures in my container that reproduce on stock bun too (localhost binds to ::1 there); none touch this diff.

Branch history: the first version of this PR (July) conflicted with main after the h2 PeerVerification and TLS session cache changes. This is the same change re-applied on current main, plus the tests from #35889 that fit this design (the URL-host verify cases and the checkServerIdentity hostname check) and a CONNECT tunnel case. #35889's unix-socket SSL_CTX fix is a separate bug and lives in #35885.

Fixes #26579

@robobun

robobun commented Jul 26, 2026 •

Copy link
Copy Markdown
Collaborator Author

Repro

bun bd test test/js/web/fetch/fetch.tls.test.ts -t "Host"
bun bd test test/js/bun/http/proxy.test.ts -t "caller-supplied Host header"

On main, fetch("https://127.0.0.1:<port>/", { headers: { host: "localhost" } }) is accepted by a certificate whose only SAN is DNS:localhost, and fetch("https://localhost:<port>/", { headers: { host: "whatever.invalid" } }) fails with ERR_TLS_CERT_ALTNAME_INVALID (#26579). With this branch the first is rejected and the second succeeds, with SNI localhost and the Host header sent as given.

Status

Rebased on current main (the July branch conflicted with the h2 PeerVerification and TLS session cache changes). This branch replaces #35889, which kept the Host header as SNI and changed only the verification target: against a server that selects its certificate by SNI that combination fails every request. The PR body has the comparison table.

@coderabbitai

coderabbitai Bot commented Jul 26, 2026 •

Copy link
Copy Markdown
Contributor

Warning

Review limit reached

  • Run on-demand review

On-demand reviews are free for the next 25 days. After that, they cost $0.25 per reviewed file.

Or wait 13 minutes for your next included review.

View limit details

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

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 13c71906-d0b7-4257-90b5-d9b15b8c6bff

📥 Commits

Reviewing files that changed from the base of the PR and between 731aa92 and 6f93c33.

📒 Files selected for processing (22)
  • src/http/AsyncHTTP.rs
  • src/http/HTTPContext.rs
  • src/http/InternalState.rs
  • src/http/ProxyTunnel.rs
  • src/http/h2_client/ClientSession.rs
  • src/http/h2_client/PendingConnect.rs
  • src/http/lib.rs
  • src/install/npm.rs
  • src/runtime/cli/audit_command.rs
  • src/runtime/cli/create_command.rs
  • src/runtime/cli/pm_diff_command.rs
  • src/runtime/cli/pm_view_command.rs
  • src/runtime/cli/publish_command.rs
  • src/runtime/cli/upgrade_command.rs
  • src/runtime/webcore/fetch.rs
  • src/runtime/webcore/fetch/FetchTasklet.rs
  • src/standalone_graph/StandaloneModuleGraph.rs
  • test/js/bun/http/proxy.test.ts
  • test/js/web/fetch/fetch-http2-adversarial.test.ts
  • test/js/web/fetch/fetch-keepalive.test.ts
  • test/js/web/fetch/fetch.test.ts
  • test/js/web/fetch/fetch.tls.test.ts

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

@robobun

robobun commented Jul 26, 2026 •

Copy link
Copy Markdown
Collaborator Author
Updated 1:34 PM PT - Aug 26th, 2026

✅ @robobun, your commit 6f93c335de493d9215951bd491fcfc5b33ba8ecd passed in Build #106333! 🎉


🧪   To try this PR locally:

bunx bun-pr 35886

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

bun-35886 --bun

Comment thread src/http/lib.rs Outdated
Comment thread src/http/lib.rs
Comment thread src/runtime/webcore/fetch/FetchTasklet.rs Outdated
Comment thread src/runtime/webcore/fetch/FetchTasklet.rs Outdated
Comment thread src/runtime/webcore/fetch/FetchTasklet.rs Outdated
@github-actions

Copy link
Copy Markdown
Contributor

Found 2 issues this PR may fix:

  1. fetch with HTTPS URL and host header different than URL's host causes ConnectionRefused #26579 - Exact bug: setting a Host header different from the URL hostname on an HTTPS fetch() causes ConnectionRefused because Bun used the Host header for TLS SNI/cert verification
  2. Wildcard domain cert + fetch leads to ERR_TLS_CERT_ALTNAME_INVALID #20173 - Wildcard cert + fetch fails with ERR_TLS_CERT_ALTNAME_INVALID when forwarded Host header (e.g. from Tanstack Start or Minio virtual-host-style) differs from the URL hostname

If this is helpful, copy the block below into the PR description to auto-close these issues on merge.

Fixes #26579
Fixes #20173

🤖 Generated with Claude Code

Comment thread src/http/ProxyTunnel.rs Outdated
Comment thread src/http/AsyncHTTP.rs Outdated
Comment thread src/http/AsyncHTTP.rs Outdated
Comment thread src/http/AsyncHTTP.rs

@claude claude Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No issues found; both earlier nits (proxy-tunnel inner SNI routing through get_tls_hostname, stale hostname mentions in the AsyncHTTP lifetime docs) were addressed in e3d1b56. Deferring to a human because this rewires TLS peer-identity resolution and drops the Host-header discriminator from the h1 keep-alive pool key and the h2 session/pending-connect key — the security direction is clearly right, but the pool-key change is worth a maintainer glance.

What was reviewed:

  • get_tls_hostname now resolves tls.servername → url.hostname only; direct-TLS SNI, proxy-tunnel inner SNI, and check_server_identity all route through it.
  • proxy_auth_hash / release_socket / existing_socket: dropping the Host-header term leaves proxy-header + Proxy-Authorization as the only tunnel discriminators; direct-TLS pool key is now just (host, port, ssl_config, reject_unauthorized).
  • strip_port_from_host had no remaining callers after the removal.
  • New fetch.tls.test.ts case exercises the DNS-only-SAN cert against IP host with/without Host header and with tls.servername; the inverted keep-alive/h2 pool tests match the new semantics.
Extended reasoning...

Overview

The PR removes the mechanism by which a caller-supplied Host request header influenced TLS SNI and certificate verification in fetch(). It deletes HTTPClient.hostname and the entire chain that fed it: FetchOptions.hostname, FetchTasklet.hostname, async_http::Options.hostname, the init_sync parameter (updated at ~12 call sites in CLI/install/standalone), InternalStateFlags.clear_hostname_on_redirect, the Host-header branch of proxy_auth_hash(), strip_port_from_host(), and the host_header_hash field on h2::ClientSession/h2::PendingConnect. get_tls_hostname() is simplified to tls_props.server_name → url.hostname and made pub(crate) so the proxy-tunnel inner SNI routes through it too.

Security risks

This is a security fix: previously a header-forwarding proxy (fetch(upstreamIpUrl, { headers: incomingHeaders })) let the remote client's Host header choose which certificate name the upstream TLS connection would accept. The new behavior matches Node/undici — the Host header is HTTP-layer only; tls.servername remains the explicit opt-in for a custom TLS identity.

The secondary change is that the Host header no longer partitions the h1 keep-alive pool or the h2 session key. That is consistent with the new model (the handshake identity is derived solely from URL host + tls.servername, so two requests with different Host headers to the same URL negotiate identical TLS sessions), and the pool key still covers ssl_config (which includes server_name) and reject_unauthorized. I did not find a case where dropping the discriminator weakens an identity guarantee, but pool-key changes in TLS code are the kind of thing a maintainer should confirm.

Level of scrutiny

High — this is TLS peer-identity verification and connection-pool keying in the HTTP client core. The change is a targeted removal (no new logic beyond routing one more SNI site through the shared helper), and the direction is unambiguously safer, but it is exactly the class of code the review guidelines say should not be bot-approved.

Other factors

  • Test coverage: a new fetch.tls.test.ts case with a DNS-only-SAN cert directly asserts the fixed behavior (baseline fails, Host header still fails, tls.servername succeeds). The cross-origin-redirect test now asserts checkServerIdentity receives the URL host on both hops. Two existing tests were inverted to assert the Host header no longer partitions pooling — those inversions are intentional and match the new semantics.
  • The two nits I raised on the previous revision (proxy-tunnel inner SNI not going through get_tls_hostname; stale hostname mentions in the AsyncHTTP lifetime docs) were both addressed in e3d1b56 and the threads are resolved.
  • The comment-cop bot flags on doc comments were all pre-existing comments being trimmed, not new justifications; robobun already explained each.

@robobun

robobun commented Aug 13, 2026

Copy link
Copy Markdown
Collaborator Author

Re-checked against current main (f426a8e) after #20173 was closed by #36252. #36252 only changed the certificate name matcher; get_tls_hostname() in src/http/lib.rs still prefers the Host header over the URL host, so the behavior this PR changes is still on main, and main's own fetch.tls.test.ts still asserts it (the redirect case expects the first hop to be verified against the Host value, ["localhost", "127.0.0.1"]). Applying this PR's test hunks to main, both new fetch.tls.test.ts cases fail: fetch("https://127.0.0.1:<port>", { headers: { host: "example.test" } }) is accepted by a certificate whose only SAN is DNS:example.test (expected ERR_TLS_CERT_ALTNAME_INVALID), and the redirect case verifies the first hop against the header value.

#20173 is closed, but #26579 (also referenced here) is still open, so this stays open as a proposed behavior change for a maintainer to decide on. The branch conflicts with main now and needs a rebase.

…uest header

fetch() copied a caller-supplied Host header into HTTPClient.hostname, and
get_tls_hostname() preferred that value over url.hostname for both the
ClientHello SNI and the name the peer certificate is verified against. A
request to https://127.0.0.1 with `Host: example.test` accepted a
certificate whose only SAN is DNS:example.test, and a request to
https://a.example with a foreign Host header failed verification because
the server answered for the header's name.

The Host header is an HTTP field only. SNI and certificate verification
now use tls.servername when set, otherwise url.hostname, the same as
node's fetch. The header is still written to the wire and still stripped
on cross-origin redirects.

Remove the plumbing that only existed to carry the header value to the TLS
layer: HTTPClient.hostname, FetchOptions.hostname, FetchTasklet.hostname,
async_http::Options.hostname, the init_sync hostname parameter,
InternalStateFlags.clear_hostname_on_redirect, the Host branch of
proxy_auth_hash(), the direct-TLS pool-key guards built on it,
h2 ClientSession/PendingConnect.host_header_hash, and strip_port_from_host.
The proxy-tunnel inner SNI goes through get_tls_hostname() like the direct
path, so tls.servername applies there too.

Fixes #26579
@robobun
robobun force-pushed the farm/6e8ce95a/fetch-host-header-tls-identity branch from e3d1b56 to 75f6f88 Compare August 26, 2026 20:08
Comment thread src/http/lib.rs
Comment thread src/runtime/webcore/fetch/FetchTasklet.rs
Comment thread src/runtime/webcore/fetch/FetchTasklet.rs
@robobun robobun changed the title fetch: don't let the Host request header steer TLS SNI / certificate verification fetch: never derive TLS SNI or certificate identity from the Host request header Aug 26, 2026

@claude claude Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code review found no issues

No high-confidence issues detected in this change.

Jarred-Sumner pushed a commit that referenced this pull request Sep 15, 2026
Fixes #30668

### Problem

- `fetch('https://[::1]:PORT/')` with `rejectUnauthorized: true` fails
with `ERR_TLS_CERT_ALTNAME_INVALID` even when the cert carries an
`IP:::1` SAN. A user-supplied `checkServerIdentity` callback receives
the bracketed `"[::1]"`, so forwarding it to
`node:tls.checkServerIdentity` also rejects (`net.isIP("[::1]") === 0`
falls through to CN matching).
- The URL parser keeps the surrounding `[` `]` on IPv6 hostnames. That
form reached both TLS verification paths: the native fast path
(`check_x509_server_identity` gates on `is_ip_address`, which rejects
brackets and takes the DNS-name branch) and the user callback.

### Fix

- Add `strip_ipv6_brackets` and apply it to every return path of
`get_tls_hostname` in `src/http/lib.rs` (proxy hostname,
`tls_props.server_name`, `url.hostname`).
- Node.js strips brackets in `urlToHttpOptions` before the hostname
reaches `tls.checkServerIdentity`. This mirrors that contract at the
single chokepoint, so the native cert check, the SNI `is_ip_address`
check, and the user callback all see the bare `::1`. Since #35886, the
proxy-tunnel inner handshake also routes through `get_tls_hostname`, so
it is covered too.
- The WebSocket client has the same class of bug: it stored the
bracketed URL host for its own verification and SNI. Store it
bracket-stripped at the two set sites in `WebSocketUpgradeClient.rs` and
in the proxy tunnel's `sni_hostname`, reusing the same helper (now
`pub`).
- The HTTP/3 client also passed the bracketed `url.hostname` straight to
`ClientContext::connect`, which uses it as both the QUIC dial host and
the SNI/verification name. Both call sites now strip brackets.
- Non-bracketed hostnames pass through unchanged. DNS names and IPv4
literals are unaffected.
- Verified: `test/js/web/fetch/fetch.tls.ipv6.test.ts` exercises both
fetch paths end to end against `https://[::1]:port/` and asserts the
callback receives `"::1"`.
`test/js/web/websocket/websocket.tls.ipv6.test.ts` does the same for
`wss://[::1]:port/`. Both fail without the fix and pass with it.
`fetch.tls.test.ts` and `fetch.tls.wildcard.test.ts` still pass.

### Background

- `get_tls_hostname` is the one place the HTTP client picks the name
used for SNI and certificate verification. Every TLS consumer of fetch
reads it.
- A certificate matches an IP literal through its IP SAN entries, not
its DNS entries. The checker picks the IP branch only when
`is_ip_address(hostname)` is true, which requires the bare address form.
- The test is gated on `isIPv6()` (Buildkite Linux agents have no IPv6)
and lives in a standalone file for this one concern, following the
`fetch.tls.wildcard.test.ts` precedent.

<!-- robobun:evidence:begin -->

---

**[review]** gate passed · iteration 12 · 5 files touched

<details><summary>fails on main (without fix)</summary>

```console
ASAN without fix: 2 FAILED
$ BUN_DEBUG_QUIET_LOGS=1 bun scripts/build.ts --profile=debug --quiet test "--reporter=junit" "--reporter-outfile=/tmp/mechgate.xml" "test/js/web/fetch/fetch.tls.ipv6.test.ts" "test/js/web/websocket/websocket.tls.ipv6.test.ts"
bun test v1.4.1 (65362b5)

test/js/web/fetch/fetch.tls.ipv6.test.ts:
87 |     stderr: "pipe",
88 |   });
89 |   const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
90 |   // Asserting stdout/stderr before exitCode produces a more useful failure
91 |   // message when the subprocess crashes unexpectedly.
92 |   expect(stdout).toBe("native: Hello World\ncallback hostname: ::1\njs: Hello World\n");
                      ^
error: expect(received).toBe(expected)

- "native: Hello World
- callback hostname: ::1
- js: Hello World
- "
+ ""

- Expected  - 4
+ Received  + 1

      at <anonymous> (/workspace/bun/test/js/web/fetch/fetch.tls.ipv6.test.ts:92:18)
(fail) fetch with IPv6 literal hostname verifies the certificate [1062.55ms]

test/js/web/websocket/websocket.tls.ipv6.test.ts:
58 |     env: cleanEnv,
59 |     stdout: "pipe",
60 |     stderr: "pipe",
61 |   });
62 |   const [stdout, stderr
... (truncated)

release without fix: all passed
bun test v1.4.1-canary.1 (bb1425b)

test/js/web/fetch/fetch.tls.ipv6.test.ts:
(pass) fetch with IPv6 literal hostname verifies the certificate [39.11ms]

test/js/web/websocket/websocket.tls.ipv6.test.ts:
(pass) wss to an IPv6 literal verifies the certificate [8.18ms]

 2 pass
 0 fail
 6 expect() calls
Ran 2 tests across 2 files. [128.00ms]
__F:0:S:0
```

</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/fetch/fetch.tls.ipv6.test.ts" "test/js/web/websocket/websocket.tls.ipv6.test.ts"
bun test v1.4.1 (65362b5)

test/js/web/fetch/fetch.tls.ipv6.test.ts:
(pass) fetch with IPv6 literal hostname verifies the certificate [1232.17ms]

test/js/web/websocket/websocket.tls.ipv6.test.ts:
(pass) wss to an IPv6 literal verifies the certificate [397.30ms]

 2 pass
 0 fail
 6 expect() calls
Ran 2 tests across 2 files. [3.68s]
__F:0:S:0

release with fix: all passed
$ bun scripts/build.ts --profile=release
[configured] bun-profile → bun (stripped) in 662ms (unchanged)
ninja: Entering directory `/workspace/bun/build/release'
[1/26] cc obj/packages/bun-usockets/src/bsd.c.o
[2/26] cc obj/packages/bun-usockets/src/context.c.o
[3/26] cc obj/packages/bun-usockets/src/eventing/epoll_kqueue.c.o
[4/26] cc obj/packages/bun-usockets/src/crypto/openssl.c.o
[5/26] cxx obj/unified/UnifiedSource-packages_bun_usockets_src_crypto-0.cpp.o
[6/26] cxx obj/unified/UnifiedSource-src_jsc_bindings_node-0.cpp.o
[7/26] cxx obj/unified/UnifiedSource-src_jsc_bindings-5.cpp.o
[8/26] cxx obj/unified/UnifiedSource-src_uws_sys-0.cpp.o
[9/26] cc obj/packages/bun-usockets/src/fault_inject.c.o
[10/26] cc obj/packages/bun-usockets/src/eventing/libuv.c.o
[11/26] cc obj/packages/bun-usockets/src/loop.c.o
[12/26] cc obj/packages/bun-usockets/src/node_quic_shim.c.o
[13/26] cc obj/packages/bun-usockets/src/socket.c.o
[14/26] cc obj/packages/bun-usockets/src/udp.c.o
[15/26] cxx obj/unified/UnifiedSource-src_jsc_bindings-4.cpp.o
[16/26] cxx obj/src/jsc/bindings/bindings.cpp.o
[17/26] cc obj/packages/bun-usockets/src/quic.c.o
[18/26] cxx obj/unified/UnifiedSource-src_j
... (truncated)
```

</details>

<details><summary>diff hotspot</summary>

```
src/http/lib.rs                                    | 21 +++--
 .../websocket_client/WebSocketProxyTunnel.rs       |  4 +-
 .../websocket_client/WebSocketUpgradeClient.rs     | 10 ++-
 test/js/web/fetch/fetch.tls.ipv6.test.ts           | 95 ++++++++++++++++++++++
 test/js/web/websocket/websocket.tls.ipv6.test.ts   | 66 +++++++++++++++
 5 files changed, 188 insertions(+), 8 deletions(-)
```

</details>

**gate history** · 5 passed · 0 rejected · iteration 12

<details><summary>evidence per changed file</summary>

```
file                                                     reads  edits  tests
src/http/lib.rs                                             11     14     11
src/http_jsc/websocket_client/WebSocketProxyTunnel.rs        1      1      7
src/http_jsc/websocket_client/WebSocketUpgradeClient.rs      4      5      7
test/js/web/fetch/fetch.tls.ipv6.test.ts                     1      2      7
test/js/web/websocket/websocket.tls.ipv6.test.ts             0      1      0
```

</details>

**root cause** · written by the author bot

Bun passed IPv6 hostnames to TLS certificate verification with their URL
brackets still attached, so names like "[::1]" or "[ff:ff::ff]" never
matched the certificate's IP entries and valid connections were
rejected. The fix normalizes the hostname before verification by
stripping the surrounding brackets across the fetch, WebSocket, and
HTTP/3 TLS paths, so the bare IPv6 literal is used for SNI and
certificate checks, matching Node's behavior. Brackets are removed only
when the inner value parses as an IPv6 address, so bracketed non-IP
names such as "[example.com]" still fail validation as…

<!-- robobun:evidence:end -->

---------

Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
usrbinkat pushed a commit to usrbinkat/bun that referenced this pull request Sep 15, 2026
Fixes oven-sh#30668

### Problem

- `fetch('https://[::1]:PORT/')` with `rejectUnauthorized: true` fails
with `ERR_TLS_CERT_ALTNAME_INVALID` even when the cert carries an
`IP:::1` SAN. A user-supplied `checkServerIdentity` callback receives
the bracketed `"[::1]"`, so forwarding it to
`node:tls.checkServerIdentity` also rejects (`net.isIP("[::1]") === 0`
falls through to CN matching).
- The URL parser keeps the surrounding `[` `]` on IPv6 hostnames. That
form reached both TLS verification paths: the native fast path
(`check_x509_server_identity` gates on `is_ip_address`, which rejects
brackets and takes the DNS-name branch) and the user callback.

### Fix

- Add `strip_ipv6_brackets` and apply it to every return path of
`get_tls_hostname` in `src/http/lib.rs` (proxy hostname,
`tls_props.server_name`, `url.hostname`).
- Node.js strips brackets in `urlToHttpOptions` before the hostname
reaches `tls.checkServerIdentity`. This mirrors that contract at the
single chokepoint, so the native cert check, the SNI `is_ip_address`
check, and the user callback all see the bare `::1`. Since oven-sh#35886, the
proxy-tunnel inner handshake also routes through `get_tls_hostname`, so
it is covered too.
- The WebSocket client has the same class of bug: it stored the
bracketed URL host for its own verification and SNI. Store it
bracket-stripped at the two set sites in `WebSocketUpgradeClient.rs` and
in the proxy tunnel's `sni_hostname`, reusing the same helper (now
`pub`).
- The HTTP/3 client also passed the bracketed `url.hostname` straight to
`ClientContext::connect`, which uses it as both the QUIC dial host and
the SNI/verification name. Both call sites now strip brackets.
- Non-bracketed hostnames pass through unchanged. DNS names and IPv4
literals are unaffected.
- Verified: `test/js/web/fetch/fetch.tls.ipv6.test.ts` exercises both
fetch paths end to end against `https://[::1]:port/` and asserts the
callback receives `"::1"`.
`test/js/web/websocket/websocket.tls.ipv6.test.ts` does the same for
`wss://[::1]:port/`. Both fail without the fix and pass with it.
`fetch.tls.test.ts` and `fetch.tls.wildcard.test.ts` still pass.

### Background

- `get_tls_hostname` is the one place the HTTP client picks the name
used for SNI and certificate verification. Every TLS consumer of fetch
reads it.
- A certificate matches an IP literal through its IP SAN entries, not
its DNS entries. The checker picks the IP branch only when
`is_ip_address(hostname)` is true, which requires the bare address form.
- The test is gated on `isIPv6()` (Buildkite Linux agents have no IPv6)
and lives in a standalone file for this one concern, following the
`fetch.tls.wildcard.test.ts` precedent.

<!-- robobun:evidence:begin -->

---

**[review]** gate passed · iteration 12 · 5 files touched

<details><summary>fails on main (without fix)</summary>

```console
ASAN without fix: 2 FAILED
$ BUN_DEBUG_QUIET_LOGS=1 bun scripts/build.ts --profile=debug --quiet test "--reporter=junit" "--reporter-outfile=/tmp/mechgate.xml" "test/js/web/fetch/fetch.tls.ipv6.test.ts" "test/js/web/websocket/websocket.tls.ipv6.test.ts"
bun test v1.4.1 (65362b5)

test/js/web/fetch/fetch.tls.ipv6.test.ts:
87 |     stderr: "pipe",
88 |   });
89 |   const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
90 |   // Asserting stdout/stderr before exitCode produces a more useful failure
91 |   // message when the subprocess crashes unexpectedly.
92 |   expect(stdout).toBe("native: Hello World\ncallback hostname: ::1\njs: Hello World\n");
                      ^
error: expect(received).toBe(expected)

- "native: Hello World
- callback hostname: ::1
- js: Hello World
- "
+ ""

- Expected  - 4
+ Received  + 1

      at <anonymous> (/workspace/bun/test/js/web/fetch/fetch.tls.ipv6.test.ts:92:18)
(fail) fetch with IPv6 literal hostname verifies the certificate [1062.55ms]

test/js/web/websocket/websocket.tls.ipv6.test.ts:
58 |     env: cleanEnv,
59 |     stdout: "pipe",
60 |     stderr: "pipe",
61 |   });
62 |   const [stdout, stderr
... (truncated)

release without fix: all passed
bun test v1.4.1-canary.1 (bb1425b)

test/js/web/fetch/fetch.tls.ipv6.test.ts:
(pass) fetch with IPv6 literal hostname verifies the certificate [39.11ms]

test/js/web/websocket/websocket.tls.ipv6.test.ts:
(pass) wss to an IPv6 literal verifies the certificate [8.18ms]

 2 pass
 0 fail
 6 expect() calls
Ran 2 tests across 2 files. [128.00ms]
__F:0:S:0
```

</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/fetch/fetch.tls.ipv6.test.ts" "test/js/web/websocket/websocket.tls.ipv6.test.ts"
bun test v1.4.1 (65362b5)

test/js/web/fetch/fetch.tls.ipv6.test.ts:
(pass) fetch with IPv6 literal hostname verifies the certificate [1232.17ms]

test/js/web/websocket/websocket.tls.ipv6.test.ts:
(pass) wss to an IPv6 literal verifies the certificate [397.30ms]

 2 pass
 0 fail
 6 expect() calls
Ran 2 tests across 2 files. [3.68s]
__F:0:S:0

release with fix: all passed
$ bun scripts/build.ts --profile=release
[configured] bun-profile → bun (stripped) in 662ms (unchanged)
ninja: Entering directory `/workspace/bun/build/release'
[1/26] cc obj/packages/bun-usockets/src/bsd.c.o
[2/26] cc obj/packages/bun-usockets/src/context.c.o
[3/26] cc obj/packages/bun-usockets/src/eventing/epoll_kqueue.c.o
[4/26] cc obj/packages/bun-usockets/src/crypto/openssl.c.o
[5/26] cxx obj/unified/UnifiedSource-packages_bun_usockets_src_crypto-0.cpp.o
[6/26] cxx obj/unified/UnifiedSource-src_jsc_bindings_node-0.cpp.o
[7/26] cxx obj/unified/UnifiedSource-src_jsc_bindings-5.cpp.o
[8/26] cxx obj/unified/UnifiedSource-src_uws_sys-0.cpp.o
[9/26] cc obj/packages/bun-usockets/src/fault_inject.c.o
[10/26] cc obj/packages/bun-usockets/src/eventing/libuv.c.o
[11/26] cc obj/packages/bun-usockets/src/loop.c.o
[12/26] cc obj/packages/bun-usockets/src/node_quic_shim.c.o
[13/26] cc obj/packages/bun-usockets/src/socket.c.o
[14/26] cc obj/packages/bun-usockets/src/udp.c.o
[15/26] cxx obj/unified/UnifiedSource-src_jsc_bindings-4.cpp.o
[16/26] cxx obj/src/jsc/bindings/bindings.cpp.o
[17/26] cc obj/packages/bun-usockets/src/quic.c.o
[18/26] cxx obj/unified/UnifiedSource-src_j
... (truncated)
```

</details>

<details><summary>diff hotspot</summary>

```
src/http/lib.rs                                    | 21 +++--
 .../websocket_client/WebSocketProxyTunnel.rs       |  4 +-
 .../websocket_client/WebSocketUpgradeClient.rs     | 10 ++-
 test/js/web/fetch/fetch.tls.ipv6.test.ts           | 95 ++++++++++++++++++++++
 test/js/web/websocket/websocket.tls.ipv6.test.ts   | 66 +++++++++++++++
 5 files changed, 188 insertions(+), 8 deletions(-)
```

</details>

**gate history** · 5 passed · 0 rejected · iteration 12

<details><summary>evidence per changed file</summary>

```
file                                                     reads  edits  tests
src/http/lib.rs                                             11     14     11
src/http_jsc/websocket_client/WebSocketProxyTunnel.rs        1      1      7
src/http_jsc/websocket_client/WebSocketUpgradeClient.rs      4      5      7
test/js/web/fetch/fetch.tls.ipv6.test.ts                     1      2      7
test/js/web/websocket/websocket.tls.ipv6.test.ts             0      1      0
```

</details>

**root cause** · written by the author bot

Bun passed IPv6 hostnames to TLS certificate verification with their URL
brackets still attached, so names like "[::1]" or "[ff:ff::ff]" never
matched the certificate's IP entries and valid connections were
rejected. The fix normalizes the hostname before verification by
stripping the surrounding brackets across the fetch, WebSocket, and
HTTP/3 TLS paths, so the bare IPv6 literal is used for SNI and
certificate checks, matching Node's behavior. Brackets are removed only
when the inner value parses as an IPv6 address, so bracketed non-IP
names such as "[example.com]" still fail validation as…

<!-- robobun:evidence:end -->

---------

Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fetch with HTTPS URL and host header different than URL's host causes ConnectionRefused

2 participants