node:tls: skip checkServerIdentity on resumed sessions

[robobun]

What does this PR do?

Node's onConnectSecure (lib/internal/tls/wrap.js) gates the server-identity check on !this.isSessionReused():

// Verify that server's identity matches it's certificate's names
// Unless server has resumed our existing session
if (!verifyError && !this.isSessionReused()) {
  const cert = this.getPeerCertificate(true);
  verifyError = options.checkServerIdentity(hostname, cert);
}

Bun ran the check unconditionally in both client handshake handlers in src/js/node/net.ts, so on a resumed session:

• a custom checkServerIdentity was invoked again (call counts / side effects calibrated to full handshakes fired on every connection),
• a checkServerIdentity that returns an Error (certificate pinning) tore the resumed connection down,
• a servername absent from the certificate's SAN failed with ERR_TLS_CERT_ALTNAME_INVALID even though the server accepted the ticket.

Node connects cleanly in all three cases because the peer's identity was established on the full handshake that issued the ticket.

Reproduction

import tls from 'node:tls';
// server with cert for SAN DNS:localhost
// first connection: full handshake, capture 'session' ticket
// second connection: tls.connect({ session: ticket, checkServerIdentity: () => calls++ })

	node v26.3.0	bun before	bun after
resumed + counting checkServerIdentity	reused=true csi=0	reused=true csi=1	reused=true csi=0
resumed + servername not in cert	reused=true ok	ERR_TLS_CERT_ALTNAME_INVALID	reused=true ok
resumed + checkServerIdentity returns Error	reused=true ok	EPIN	reused=true ok

Fix

Add !socket.isSessionReused() to the identity-check gate in both handshake handlers (SocketHandlers and SocketHandlers2).

How did you verify your code works?

New test in test/js/node/tls/node-tls-connect.test.ts covering all three cases; fails on USE_SYSTEM_BUN=1 with calls: 1 vs expected 0, passes with this change. test-tls-client-resume.js, test-tls-client-resume-12.js, test-https-client-resume.js, test-https-client-checkServerIdentity.js, test-tls-check-server-identity.js, test-tls-ticket*.js and node-tls-connect.test.ts all pass.

[coderabbitai]

Warning

Review limit reached

You’ve reached a temporary PR review limit under our Fair Usage Limits Policy.

Your recent review volume is higher than typical usage, so adaptive limits are currently applied.

Next review available in: 56 minutes

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

How can I continue?

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

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

How do review limits work?

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

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

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 0769ea8a-9d5c-4fd0-a7d9-d1fed3a3fbb2

📥 Commits

Reviewing files that changed from the base of the PR and between e10d444 and c107885.

📒 Files selected for processing (2)

• src/js/node/net.ts
• test/js/node/tls/node-tls-connect.test.ts

---

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

[robobun]

^{Updated 4:19 AM PT - Jul 8th, 2026}

✅ @autofix-ci[bot], your commit c1078851a5acc9bdcef9b95918e853f544d68d8a passed in Build #70358! 🎉

---

🧪   To try this PR locally:

bunx bun-pr 33734

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

bun-33734 --bun

[claude]

I didn't find any issues and the change correctly mirrors Node's onConnectSecure gate — but since this alters when TLS server-identity verification runs, it's worth a human sign-off.

Extended reasoning...

Overview

Two-line change in src/js/node/net.ts: adds !socket.isSessionReused() to the checkServerIdentity gate in both client handshake handlers (SocketHandlers and SocketHandlers2), matching Node's lib/internal/tls/wrap.js onConnectSecure. A new test in test/js/node/tls/node-tls-connect.test.ts covers three resume scenarios (counting callback, error-returning callback, servername not in SAN).

Security risks

This removes the hostname/identity check on resumed TLS sessions. That is the standard TLS model — a successful resume proves the peer holds the session secret from the originally-verified full handshake — and is exactly what Node does. socket.isSessionReused() is a native binding (sockets.classes.ts) reporting BoringSSL's SSL_session_reused, not a user-tamperable JS property, so the gate can't be spoofed from userland. I don't see a way this weakens the security posture relative to Node, but any change to when cert verification runs warrants a second pair of eyes.

Level of scrutiny

High. TLS peer-identity verification is core security-sensitive code; even a small, well-justified change here should be reviewed by a human familiar with Bun's TLS stack.

Other factors

The fix is applied symmetrically to both handshake handlers, the PR description cites the exact Node source and includes a before/after comparison table, and the author reports the relevant Node parallel tests (test-tls-client-resume*.js, test-https-client-resume.js, test-tls-check-server-identity.js, etc.) still pass. The new test is well-constructed (waits for the ticket, wires error → resolve, asserts reused: true so it can't vacuously pass on a fallback full handshake). No prior human reviews or outstanding comments.

[robobun]

CI status for build #70358: 284/287 jobs passed, including all lanes that ran test/js/node/tls/node-tls-connect.test.ts (darwin-14-x64, darwin-26-aarch64, all linux/windows/freebsd). No annotations reference any file in this diff.

The two remaining jobs are both darwin-14-aarch64 test-bun shards that have been stuck in scheduled for 5+ hours, auto-retrying on expiry (04:53 → 06:05 → 08:05 → 10:05). This is a test-darwin queue backlog on the 14-aarch64 runners, not a test failure. A retrigger would hit the same queue.

The diff itself is green on every completed lane.

[robobun]

The !isSessionReused() gate this PR adds was landed as part of #34598. #36131 removes it again: BoringSSL stores the peer chain on the SSL_SESSION, so Bun has the real certificate on a resumed connection (Node does not), and with the gate in place a caller that resumes an app-supplied session under a different servername sees authorized: true for a name the certificate does not cover. Closing in favour of #36131.