types: declare allowPartialTrustChain, sessionTimeout, sigalgs and ecdhCurve on Bun.TLSOptions - #38134
types: declare allowPartialTrustChain, sessionTimeout, sigalgs and ecdhCurve on Bun.TLSOptions#38134robobun wants to merge 1 commit into
Conversation
…dhCurve on Bun.TLSOptions The runtime has accepted these four members of a `tls` object since the SSLConfig converter gained them, on every API that builds a TLS context through it (Bun.serve, Bun.listen, Bun.connect, fetch, WebSocket, and the SQL/Redis clients). The TypeScript interface never gained them, so passing any of them is a TS2353 excess-property error. Declare them with the value types the converter enforces (strict boolean, integer number, string) and document what each one does, including the current TLS 1.2-only scope of sessionTimeout. List them in the server docs' TLSOptions reference. The bun-types integration test gains a single-file tsc case that runs on debug builds too (the Bun.mmap case now shares the helper), and the serve, tcp and fetch fixtures exercise the members so the release-lane checks with and without lib.dom, and tsgo, cover them as well.
|
Warning Review limit reached
Next review available in: 7 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 (6)
Comment |
|
Updated 9:32 AM PT - Aug 13th, 2026
❌ @robobun, your commit 6fd6f13 has 2 failures in
🧪 To try this PR locally: bunx bun-pr 38134That installs a local version of the PR into your bun-38134 --bun |
|
Status: ready for review. Reproduced on main by type-checking While doing that I found that |
There was a problem hiding this comment.
I reviewed this PR and found no bugs. The four new TLSOptions members match the runtime converter in SSLConfig.bindv2.ts, and the fixture additions are appended so the hardcoded lib.dom.d.ts diagnostic line numbers are unaffected. Since packages/bun-types/ has a CODEOWNER, a human sign-off would still be worthwhile.
What was reviewed
- Verified
allowPartialTrustChain: b.bool,sessionTimeout: b.i32,sigalgs/ecdhCurve: b.String.nullableinsrc/runtime/socket/SSLConfig.bindv2.ts:100-114match the declared types. - Checked the
expectSingleFileToTypeCheckrefactor preserves theBun.mmaptest's behavior (same tsconfig, same tsc invocation). - Confirmed new fixture lines in
fetch.ts/serve.ts/tcp.tsland after existing content and notcp.ts/serve.tsentries exist in thechecks with lib.dom.d.tsdiagnostics array.
Extended reasoning...
Overview
This PR adds four optional members — allowPartialTrustChain, sessionTimeout, sigalgs, ecdhCurve — to interface TLSOptions in packages/bun-types/bun.d.ts, plus matching entries in the docs/runtime/http/server.mdx reference block. The runtime already accepts these (declared in src/runtime/socket/SSLConfig.bindv2.ts:100-114); only the type declarations lagged. Test coverage is added via a new single-file tsc case in bun-types.test.ts (which also refactors the existing Bun.mmap case into a shared expectSingleFileToTypeCheck helper) and appended usage lines in the fetch.ts/serve.ts/tcp.ts fixtures.
Security risks
None. This is a .d.ts-only change with no runtime effect; the docs and test fixtures are compile-time-only. The declared types match the strict validation the runtime already performs.
Level of scrutiny
Low. The change is mechanical: it mirrors an existing runtime schema into the type declarations. I verified each declared type against the bindv2 source (b.bool → boolean, b.i32 → number, b.String.nullable → string | undefined). The | undefined suffix on optional members follows the pattern used elsewhere in the same interface. The refactored test helper is byte-for-byte equivalent to the inlined version it replaces. The one thing worth an owner's eye is the JSDoc wording (particularly the TLS 1.2-only caveat on sessionTimeout), which the PR description says was empirically verified but which sets user-facing documentation.
Other factors
packages/bun-types/ and *.d.ts are listed in .github/CODEOWNERS, which per the review policy means I should not auto-approve. The change is otherwise straightforward, thoroughly tested (both a debug-build-runnable single-file check and release-only whole-fixture checks including lib.dom and tsgo), and the PR description documents empirical verification of every JSDoc claim against the released binary. There are no prior human review comments to address.
|
On the one point flagged above, the TLS 1.2 wording in the |
|
Heads-up on the |
Problem
allowPartialTrustChain,sessionTimeout,sigalgsorecdhCurvein atlsobject toBun.serve,Bun.listen,Bun.connect,fetchornew WebSocketfails to type-check:error TS2353: Object literal may only specify known properties, and 'sessionTimeout' does not exist in type 'TLSOptions'(TS2769on the overloaded APIs).src/runtime/socket/SSLConfig.bindv2.ts:100-114declares them (the first three since node:tls: sync the test suite to Node v26.3.0 and fix the gaps it surfaces (+24 tests, 155→179 of 221 upstream passing) #32630,ecdhCurvesince tls: close_notify on end(), injected-socket upgrades, reject-handshake wire fix, duplex data-loss, SNI, ALPN (+14 tests, tls 81%→86%) #34598),src/runtime/socket/SSLConfig.rs:179-194copies them into the sharedSSLConfig,src/http/ssl_config.rs:216-223hands them to usockets, andpackages/bun-usockets/src/crypto/openssl.c:1325-1354applies them (SSL_CTX_set_timeout,X509_V_FLAG_PARTIAL_CHAIN,SSL_CTX_set1_sigalgs_list,SSL_CTX_set1_groups_list).tlsobject with that one converter (SSLConfig::from_js), so they all honor the options. Onlyinterface TLSOptionsinpackages/bun-types/bun.d.tsnever gained the members.crl, from the same sync, is types: declare crl on Bun.TLSOptions #38092; this PR covers the rest.Fix
packages/bun-types/bun.d.ts: add the four members toBun.TLSOptions.BunFetchRequestInitTLSextends it and the serve/listen/connect/WebSocket option types reference it, so one declaration covers every API.allowPartialTrustChainis a strict boolean (1throwsERR_INVALID_ARG_TYPE),sessionTimeoutan integer number (1.5throwsERR_OUT_OF_RANGE,"300"throwsERR_INVALID_ARG_TYPE),sigalgsandecdhCurvestrings (42/7throwERR_INVALID_ARG_TYPE); explicitundefinedis accepted for all four, hence| undefined.fetch,Bun.connectand arequestCertBun.serveonly with the flag;sigalgsaccepts both TLS 1.3 names andRSA-PSS+SHA256style pairs and fails the handshake without an overlap on either side;ecdhCurveaccepts single names, aliases, colon lists and"auto", and fails the handshake without a shared group;sessionTimeout: 7makes the server's TLS 1.2 tickets advertise a 7 second lifetime.sessionTimeoutis documented as applying to TLS 1.2 sessions because that is what the runtime does today:SSL_CTX_set_timeoutonly sets the TLS <= 1.2 lifetime in BoringSSL, and TLS 1.3 tickets keep the 2 day default (172800below, where Node prints7). That runtime gap is a separate fix in usockets; when it lands the sentence in the JSDoc goes away. Declaring the member now is still right: the option exists, is validated, and does what the JSDoc says.docs/runtime/http/server.mdx: list the four options in theTLSOptionsreference block.test/integration/bun-types/bun-types.test.ts, newBun.TLSOptionscase: type-checks the four members (and rejects the wrong value types) onBun.TLSOptionsitself, onBun.serve(single and per-serverNamearray),Bun.listen,Bun.connect,fetchandWebSocket. It spawnstscover one file, so it runs on debug builds too; the existingBun.mmapcase now shares that helper. Without thebun.d.tshunk it fails with 16 diagnostics;bun bd test test/integration/bun-types/bun-types.test.tspasses with it.test/integration/bun-types/fixture/{serve,tcp,fetch}.ts: usage lines so the release-only whole-fixture checks cover the members too. With a release build the full file passes (16/16, includingchecks with lib.dom.d.tsand tsgo); without thebun.d.tshunk those cases fail with 17 diagnostics across the three fixtures.Background
Bun.TLSOptionsis the single TypeScript interface behind thetlsoption ofBun.serve,Bun.listen,Bun.connect,fetch, the WebSocket client and the SQL/Redis clients. At runtime all of them convert that object withSSLConfig::from_js, generated fromSSLConfig.bindv2.ts, so the interface is meant to mirror that file's member list.UNABLE_TO_GET_ISSUER_CERTotherwise);X509_V_FLAG_PARTIAL_CHAINlets any certificate in the store act as the anchor. Node added the same option in v22.9.sigalgsconfigures the TLS signature algorithms: BoringSSL'sSSL_CTX_set1_sigalgs_listsets both the list this side signs with and the list it accepts from the peer, which is why a server whose key is RSA but whosesigalgsonly names ECDSA schemes fails every handshake.ecdhCurveis Node's name for the list of key agreement groups (SSL_CTX_set1_groups_list); the handshake needs one group both sides list."auto"is Node's documented default andSSLConfig.rs:187-193maps it to "leave BoringSSL's default list alone".test/integration/bun-typespackspackages/bun-typesand type-checks thefixture/directory against it with and withoutlib.dom. Those whole-fixture cases run on release builds only (the in-process TypeScript service is too slow under a debug build), which is why the new coverage also includes a single-filetsccase that runs everywhere.Runtime probes on the released binary (bun 1.4.0, Node key fixtures; agent6 is issued by the ca3 intermediate, ca3 by the ca1 root, agent1 has an RSA key)
allowPartialTrustChain, trusting onlyca3:sigalgs(server key is RSA):ecdhCurve(server pinned toP-384):sessionTimeout(ticket lifetime hint seen byopenssl s_client, server configured withsessionTimeout: 7):Bun.listen+Bun.connectandnew WebSocket(url, { tls })with all four options set: handshake completes / socket opens.Type-check failure on main, the new test's
tscoutput (trimmed):