Repository navigation
fetch: support PKCS#12 client certificates - #41958
Open
bsreeram08 wants to merge 2 commits into
Open
bsreeram08 wants to merge 2 commits into
bsreeram08 wants to merge 2 commits into
Conversation
Add binary PFX parsing, certificate-chain handling, validation, and secure cleanup to fetch TLS configuration. Include pooling, input-form, error-path, type, and documentation coverage.\n\nTests: bun bd test test/js/web/fetch/fetch.tls.test.ts\nTests: BUN_JSC_validateExceptionChecks=1 BUN_JSC_dumpSimulatedThrows=1 bun bd test test/js/web/fetch/fetch.tls.test.ts -t PKCS#12\nTests: bun test test/integration/bun-types/bun-types.test.ts -t 'TypeScript latest'
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configuration
📒 Files selected for processing (7)
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review. WalkthroughChangesFetch TLS options now accept PKCS## Walkthrough ChangesFetch TLS options now accept PKCS#12/PFX client identities as binary data or PKCS#12 fetch TLS support
Suggested reviewers: Priority: ➖ Normal Severity of issue fixed: Medium Merge Risk: ⚪ Minimal · up to No actionable issue remains from this review; the change is ready to merge after normal checks. Security Architecture ReviewSecurity architecture risk: 🔵 Low · up to PKCS#12 credentials are normalized into the existing TLS identity configuration, with guarded cleanup and connection-pool separation. No introduced security defect was established in the inspected fetch path. Risk remains low rather than minimal because the shared conversion also serves other TLS entrypoints whose complete behavior was not assessed. Retained concerns Security review detailsSecurity Blast Radius
Security Findings and Attack Paths
Trust Boundaries and Controls
Resilience and Maintainability Implications
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
Comment |
This was referenced Sep 11, 2026
M4xymm
added a commit
to ravendb/ravendb-nodejs-client
that referenced
this pull request
Sep 15, 2026
Bun's fetch silently ignores the `tls.pfx` option (oven-sh/bun#41958, oven-sh/bun#17543), so a PFX certificate configured through authOptions was never presented and the request went out uncertified. Its node:https implementation does honour `pfx` (oven-sh/bun#14417), so route those requests there instead. BunHttpUtil installs a fetch-compatible transport backed by node:https as the request `fetcher` - the same seam conventions.customFetch uses - when a PKCS#12 archive is configured on Bun. PEM material keeps using Bun's `tls` fetch option, which works. The transport covers what the client asks of fetch: methods, headers, string/binary/FormData/stream request bodies, streamed response bodies, AbortSignal, and content-encoding decompression. It is built lazily, once per executor, and closed in dispose(). Before Bun 1.4.0 neither transport presents a client certificate (node:https ignored the certificate and the ca as well), so DocumentStore.initialize() now throws an actionable error there instead of failing per request. RavenCommand.send passes a node:stream Readable body through when the fetcher declares it accepts one, so streamed attachment uploads keep working on that path. Drops the now-obsolete console.warn from PfxCertificate.toBunTlsOptions and documents the Bun runtime, including the pre-existing Changes API gap (Bun's built-in WebSocket ignores the ws package's TLS options).
# Conflicts: # test/integration/bun-types/fixture/fetch.ts
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
fetch(url, { tls: { pfx, passphrase } })support forBufferSource,BunFile, and arraysSSLConfig, preserving connection-pool identity isolationBefore this change, Bun accepted the
tlsobject but did not consumepfx, so PKCS#12 client authentication failed during the TLS handshake.Fixes #17543.
Tests
bun bd test test/js/web/fetch/fetch.tls.test.ts— 48 passedBUN_JSC_validateExceptionChecks=1 BUN_JSC_dumpSimulatedThrows=1 bun bd test test/js/web/fetch/fetch.tls.test.ts -t PKCS#12— 14 passedbun test test/integration/bun-types/bun-types.test.ts -t "TypeScript latest"— passedUSE_SYSTEM_BUN=1 bun test test/js/web/fetch/fetch.tls.test.ts -t PKCS#12— 13 failed on the released Bun, confirming the regression coverage exercises the new behaviorThe unfiltered bun-types suite also has an unrelated existing
Blobdiagnostic mismatch infixture/24154.ts; the focused TypeScript-latest fixture check passes.