Skip to content

Support CA store selection in bunfig.toml - #30314

Open
robobun wants to merge 11 commits into
mainfrom
farm/2cabcd73/bunfig-ca-store
Open

robobun wants to merge 11 commits into
mainfrom
farm/2cabcd73/bunfig-ca-store

Conversation

@robobun

@robobun robobun commented May 6, 2026 •

Copy link
Copy Markdown
Collaborator

Fixes #30313

Related: #17325, #17108. This PR only makes the existing CA-store selection configurable in bunfig.toml. It does not extend that selection to bun install or add new TLS env var handling, so those issues stay open.

Feature

Adds a top-level CA key to bunfig.toml that accepts "system", "openssl", or "bundled":

# bunfig.toml
CA = "system"

So users can set a default once instead of passing --use-system-ca (or --use-openssl-ca / --use-bundled-ca) on every bun run.

Applied to run, auto, and test commands.

Precedence

CLI flag > NODE_USE_SYSTEM_CA env var > bunfig.toml CA > bundled (default).

The existing CLI error about "choose exactly one" still fires only when multiple CLI flags are passed together. A bunfig value that disagrees with a CLI flag or env var just gets overridden silently, matching the existing NODE_USE_SYSTEM_CA behavior.

Implementation

  • src/options_types/Context.zig: moves the BunCAStore enum here and adds runtime_options.ca_store: ?BunCAStore so bunfig can stash the parsed value without forcing a write to the exported global.
  • src/cli/bunfig.zig: new parser branch for the top-level CA / ca key (string-valued for easier validation than three booleans). Emits a diagnostic on unknown values.
  • src/cli/Arguments.zig: CA-flag block now consults ctx.runtime_options.ca_store as the lowest-precedence source after the CLI flags and NODE_USE_SYSTEM_CA.
  • src/cli/cli.zig: exposes Command.BunCAStore alongside the other context-type aliases.

Verification

Extends test/js/node/tls/test-use-system-ca.test.ts with:

  • CA = "system" in bunfig matches --use-system-ca's cert count
  • CA = "bundled" matches the plain default
  • CA = "openssl" is accepted without error
  • CLI --use-bundled-ca wins over CA = "system" in bunfig
  • NODE_USE_SYSTEM_CA=1 wins over CA = "bundled" in bunfig
  • Unknown CA values emit a diagnostic and exit non-zero
  • bun test reads the bunfig value too
$ bun bd test test/js/node/tls/test-use-system-ca.test.ts
 11 pass
 0 fail
 44 expect() calls

Against the baked bun (no fix), the two key tests fail — CA = "system" produces the bundled-only count (145) instead of 448 with system certs, and the invalid-value test exits 0 because the key is silently ignored.


[review] gate passed · iteration 16 · 7 files touched

fails on main (without fix)
ASAN without fix: 4 FAILED
$ BUN_DEBUG_QUIET_LOGS=1 bun scripts/build.ts --profile=debug --quiet test "--reporter=junit" "--reporter-outfile=/tmp/mechgate.xml" test/js/node/tls/test-use-system-ca.test.ts
bun test v1.4.0 (205cd0c5d)

test/js/node/tls/test-use-system-ca.test.ts:
(pass) --use-system-ca > flag loads system certificates [279.88ms]
(pass) --use-system-ca > NODE_USE_SYSTEM_CA=1 loads system certificates [279.36ms]
(pass) --use-system-ca > NODE_USE_SYSTEM_CA=0 doesn't load system certificates [283.47ms]
(pass) --use-system-ca > --use-system-ca overrides NODE_USE_SYSTEM_CA=0 [275.24ms]
(pass) bunfig.toml CA > CA = "openssl" in bunfig.toml matches --use-openssl-ca [2016.58ms]
(pass) bunfig.toml CA > CLI --use-bundled-ca overrides bunfig CA = "system" [2025.93ms]
(pass) bunfig.toml CA > CA = "bundled" in bunfig.toml matches bundled-only default [2104.70ms]
129 |     });
130 |     const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
131 |     expect(stderr).toBe("");
132 |     const runCount = parseInt(stdout.trim(), 10);
133 |     const flagCount = await defaultCertCount(["--use-system-ca"]);
134 |     expect(runCount).toBe(flagCou
... (truncated)

release without fix: 4 FAILED
bun test v1.4.0-canary.1 (b7a043103)

test/js/node/tls/test-use-system-ca.test.ts:
(pass) --use-system-ca > flag loads system certificates [7.81ms]
(pass) --use-system-ca > NODE_USE_SYSTEM_CA=1 loads system certificates [7.41ms]
(pass) --use-system-ca > NODE_USE_SYSTEM_CA=0 doesn't load system certificates [6.78ms]
(pass) --use-system-ca > --use-system-ca overrides NODE_USE_SYSTEM_CA=0 [7.08ms]
193 |       stdout: "pipe",
194 |       stderr: "pipe",
195 |     });
196 | 
197 |     const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
198 |     expect(stdout).not.toContain("should not run");
                             ^
error: expect(received).not.toContain(expected)

Expected to not contain: "should not run"
Received: "should not run\n"

      at <anonymous> (/workspace/bun/test/js/node/tls/test-use-system-ca.test.ts:198:24)
(fail) bunfig.toml CA > invalid CA value fails with a diagnostic [8.73ms]
(pass) bunfig.toml CA > CA = "bundled" in bunfig.toml matches bundled-only default [46.78ms]
(pass) bunfig.toml CA > CA = "openssl" in bunfig.toml matches --use-openssl-ca [46.21ms]
(pass) bunfig.toml CA > CLI --use-b
... (truncated)
passes on PR (with fix)
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/node/tls/test-use-system-ca.test.ts
bun test v1.4.0 (205cd0c5d)

test/js/node/tls/test-use-system-ca.test.ts:
(pass) --use-system-ca > flag loads system certificates [277.58ms]
(pass) --use-system-ca > NODE_USE_SYSTEM_CA=1 loads system certificates [272.62ms]
(pass) --use-system-ca > NODE_USE_SYSTEM_CA=0 doesn't load system certificates [277.89ms]
(pass) --use-system-ca > --use-system-ca overrides NODE_USE_SYSTEM_CA=0 [276.97ms]
(pass) bunfig.toml CA > CA = "openssl" in bunfig.toml matches --use-openssl-ca [2064.14ms]
(pass) bunfig.toml CA > CA = "bundled" in bunfig.toml matches bundled-only default [2089.30ms]
(pass) bunfig.toml CA > CLI --use-bundled-ca overrides bunfig CA = "system" [2112.72ms]
(pass) bunfig.toml CA > invalid CA value fails with a diagnostic [126.01ms]
(pass) bunfig.toml CA > bun run <file> honors bunfig.toml CA [2521.58ms]
(pass) bunfig.toml CA > CA = "system" in bunfig.toml matches --use-system-ca [3264.45ms]
(pass) bunfig.toml CA > NODE_USE_SYSTEM_CA=1 overrides bunfig CA = "bundled" [2234.04ms]
(pass) bu
... (truncated)

release with fix: all passed
$ bun scripts/build.ts --profile=release
[configured] bun-profile → bun (stripped)
  target       linux-x64-gnu
  build type   Release
  build dir    ./build/release
  revision     205cd0c5d8
  features     baseline

22 deps, 123 codegen, 1176 objects in 847ms

ninja: Entering directory `/workspace/bun/build/release'
[1/1238] install /workspace/bun
bun install v1.4.0-canary.1 (b7a043103)

Checked 107 installs across 153 packages (no changes) [9.00ms]
[2/1238] gen ErrorCode+*.h
[3/1238] gen bindgenv2
[4/1238] install /workspace/bun/packages/bun-error
bun install v1.4.0-canary.1 (b7a043103)

Checked 1 install across 2 packages (no changes) [2.00ms]
[5/1238] install /workspace/bun/src/node-fallbacks
bun install v1.4.0-canary.1 (b7a043103)

Checked 129 installs across 147 packages (no changes) [11.00ms]
[6/1238] fetch tinycc
[tinycc] up to date
[7/1237] gen ProcessBindingConstants.lut.h
Generating /workspace/bun/build/release/codegen/ProcessBindingConstants.lut.h from /workspace/bun/src/jsc/bindings/ProcessBindingConstants.cpp
[8/1237] fetch libjpeg-turbo
[libjpeg-turbo] up to date
[9/1237] fetch zlib
[zlib] up to date
[10/1237] gen .bind.ts → GeneratedBindings.cpp

... (truncated)
diff hotspot
docs/runtime/bunfig.mdx                     |  17 +++
 src/bunfig/bunfig.rs                        |  28 +++++
 src/options_types/context.rs                |  12 ++
 src/runtime/cli/Arguments.rs                |  45 ++++----
 src/runtime/cli/repl_command.rs             |   2 +
 src/runtime/cli/run_command.rs              |   6 +
 test/js/node/tls/test-use-system-ca.test.ts | 169 +++++++++++++++++++++++++++-
 7 files changed, 258 insertions(+), 21 deletions(-)

gate history · 1 passed · 0 rejected · iteration 16

evidence per changed file
file                                         reads  edits  tests
docs/runtime/bunfig.mdx                          2      2     37
src/bunfig/bunfig.rs                             3      1     37
src/options_types/context.rs                     4      4     37
src/runtime/cli/Arguments.rs                    10     10     37
src/runtime/cli/repl_command.rs                  2      3     37
src/runtime/cli/run_command.rs                   7      5     37
test/js/node/tls/test-use-system-ca.test.ts     11      9     37

@robobun

robobun commented May 6, 2026 •

Copy link
Copy Markdown
Collaborator Author
Updated 7:05 AM PT - Aug 14th, 2026

❌ @robobun, your commit 205cd0c has some failures in Build #95893 (All Failures)


🧪   To try this PR locally:

bunx bun-pr 30314

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

bun-30314 --bun

@github-actions github-actions Bot added the claude label May 6, 2026
@github-actions

github-actions Bot commented May 6, 2026

Copy link
Copy Markdown
Contributor

Found 3 issues this PR may fix:

  1. Limitations of bun operation in corporate networks: self-signed CA #17325 - Corporate network users need a persistent config to use system CAs for all bun commands; CA = "system" in bunfig.toml directly addresses this
  2. FR: Support Loading Certificates from Windows Certificate Store #17108 - Requests an option to load certificates from the OS store instead of bundled certs; bunfig.toml CA = "system" is exactly this feature
  3. Drop-in replacement for npm strict-ssl #4979 - User's org registry uses certs trusted by the system store but not Bun's bundled store; CA = "system" in bunfig.toml resolves this

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

Fixes #17325
Fixes #17108
Fixes #4979

🤖 Generated with Claude Code

@robobun

robobun commented May 6, 2026

Copy link
Copy Markdown
Collaborator Author

Added #17325 and #17108. Skipping #4979 — that thread is asking to disable cert verification (npm strict-ssl false / NODE_TLS_REJECT_UNAUTHORIZED=0), which is a different concern from picking which CA store to trust.

@coderabbitai

coderabbitai Bot commented May 6, 2026 •

Copy link
Copy Markdown
Contributor

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

Walkthrough

Pins and exposes a CA-store runtime option, parses a CA key from bunfig.toml, and implements a deterministic precedence (CLI flags → NODE_USE_SYSTEM_CA → bunfig.toml → runtime fallback). Adds an apply mechanism and locking to prevent later bunfig overrides; updates CLI wiring, tests, and docs.

Changes

CA Store Configuration and Resolution

Layer / File(s) Summary
Data Shape
src/options_types/Context.zig
Adds public enum BunCAStore (bundled, openssl, system) and optional ca_store: ?BunCAStore = null to RuntimeOptions.
Public API Type Exposure
src/cli/cli.zig
Adds pub const BunCAStore inside Command to expose the new type through CLI command/context types.
Public API Restructuring / Exports
src/cli/Arguments.zig
Replaces local enum with alias pub const BunCAStore = Command.BunCAStore; keeps pub export var Bun__Node__CAStore: BunCAStore = .bundled; and adds exported pub export var Bun__Node__UseSystemCA (initialized false).
Bunfig Parsing / Wiring
src/cli/bunfig.zig
Parses CA/ca for Run/Auto/Test commands (accepts system, openssl, bundled), sets ctx.runtime_options.ca_store accordingly, and emits diagnostics for invalid values.
CA Store Resolution / Locking
src/cli/Arguments.zig
Implements precedence for resolving Bun__Node__CAStore: CLI flags → NODE_USE_SYSTEM_CA env → ctx.runtime_options.ca_store (bunfig) → runtime fallback; introduces Bun__Node__CAStore_locked and applyBunfigCAStore(ctx) which applies bunfig value and updates Bun__Node__UseSystemCA.
Command Wiring (apply at load points)
src/cli/run_command.zig, src/cli/repl_command.zig
Calls bun.cli.Arguments.applyBunfigCAStore(ctx) after bunfig.toml is loaded in REPL and Run command paths to ensure bunfig CA is applied when appropriate.
Tests / Documentation
test/js/node/tls/test-use-system-ca.test.ts, docs/runtime/bunfig.mdx
Adds tests probing CA-store selection and precedence (CLI flags, NODE_USE_SYSTEM_CA, bunfig.toml, invalid values) and documents the new CA bunfig option with values and precedence.
🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed The implementation comprehensively addresses all three linked issues: #30313 (bunfig.toml CA config), #17325 (corporate network/self-signed CA support), and #17108 (OS certificate store support via system option).
Out of Scope Changes check ✅ Passed All changes are directly scoped to implementing CA store selection in bunfig.toml: core logic, test coverage, and documentation with no unrelated modifications.
Title check ✅ Passed The title clearly and concisely describes the primary change: adding CA store selection to bunfig.toml.
Description check ✅ Passed The description explains the feature, precedence, implementation scope, linked issues, and verification results in sufficient detail.

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

@coderabbitai coderabbitai 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.

Actionable comments posted: 3

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/cli/Arguments.zig`:
- Around line 1046-1056: The env-var branch treats any presence of
NODE_USE_SYSTEM_CA (including "0") as true; change the condition so it checks
the actual value string from bun.env_var.NODE_USE_SYSTEM_CA.get() (e.g., match
on .?value and verify value != "0" and optionally not "false") before setting
Bun__Node__CAStore = .system; update the branch that currently uses
bun.env_var.NODE_USE_SYSTEM_CA.get() to only take effect when the env var's
value is truthy (not "0"/"false"), keeping the other symbols/use_bundled_ca,
use_openssl_ca, use_system_ca, and ctx.runtime_options.ca_store precedence
intact.

In `@test/js/node/tls/test-use-system-ca.test.ts`:
- Around line 125-136: The test currently asserts exitCode before checking
stdout/stderr for the spawned process; update the assertion order in the
spawn-result checks (the block using spawn(...), proc,
Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]) and the
variables stdout, stderr, exitCode) so that you first assert stdout and stderr
expectations (expect(stdout.trim()).toBe("OK"); expect(stderr).toBe("");) and
only then assert expect(exitCode).toBe(0);; make the same reorder in the other
two similar blocks referenced (the blocks around the Promise.all usages at the
other spawn sites).
- Around line 183-210: The test currently only checks that
getCACertificates("default") returns an array which doesn't prove bun test
respected CA = "system"; modify the probe test (probe.test.ts) to export or
print the actual certificate count (e.g., call
getCACertificates("default").length and log it) and in the parent test compare
that numeric value against a known baseline—either the bundled CA baseline or
the value returned by defaultCertCount(["--use-system-ca"])—by spawning bun test
twice (or computing baseline separately) and asserting the spawned test's
printed count differs appropriately when the system CA is selected; update the
spawn invocation and expectations to parse the printed count from stdout and
assert the numeric comparison instead of Array.isArray.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 3765f5da-ca3e-49e6-a161-67dbd484f97e

📥 Commits

Reviewing files that changed from the base of the PR and between 611740f and 3e75f1c.

📒 Files selected for processing (5)
  • src/cli/Arguments.zig
  • src/cli/bunfig.zig
  • src/cli/cli.zig
  • src/options_types/Context.zig
  • test/js/node/tls/test-use-system-ca.test.ts

Comment thread src/cli/Arguments.zig Outdated
Comment thread test/js/node/tls/test-use-system-ca.test.ts Outdated
Comment thread test/js/node/tls/test-use-system-ca.test.ts Outdated
Comment thread test/js/node/tls/test-use-system-ca.test.ts Outdated
Comment thread test/js/node/tls/test-use-system-ca.test.ts Outdated
Comment thread test/js/node/tls/test-use-system-ca.test.ts Outdated
Comment thread src/runtime/cli/bunfig.zig Outdated

@coderabbitai coderabbitai 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.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@docs/runtime/bunfig.mdx`:
- Around line 120-121: Update the sentence that currently reads "Applies to `bun
run` and `bun test`" so it also includes `bun auto`; locate the sentence in
docs/runtime/bunfig.mdx that mentions the CA certificate store applicability and
change it to explicitly state "Applies to `bun run`, `bun test`, and `bun auto`"
to match the implementation objective.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 1cc7ecaa-e596-46a8-923d-438cda844ae7

📥 Commits

Reviewing files that changed from the base of the PR and between 02eae5f and 666f484.

📒 Files selected for processing (1)
  • docs/runtime/bunfig.mdx

Comment thread docs/runtime/bunfig.mdx Outdated
Comment thread src/cli/Arguments.zig Outdated
Comment thread docs/runtime/bunfig.mdx Outdated

@coderabbitai coderabbitai 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.

Actionable comments posted: 3

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/cli/repl_command.zig`:
- Around line 25-29: The call to bun.cli.Arguments.applyBunfigCAStore is
currently guarded by if (!ctx.debug.loaded_bunfig), so when bunfig is already
preloaded the CA from ctx.runtime_options.ca_store never gets copied into
Bun__Node__CAStore; remove the guard or call
bun.cli.Arguments.applyBunfigCAStore(ctx) unconditionally after the existing if
block (keep bun.cli.Arguments.loadConfigPath inside the if) so that
applyBunfigCAStore always runs and ensures ctx.runtime_options.ca_store is
copied into Bun__Node__CAStore regardless of ctx.debug.loaded_bunfig.

In `@test/js/node/tls/test-use-system-ca.test.ts`:
- Around line 77-90: The helper defaultCertCount currently inherits the test
runner's cwd when cwd is omitted, which can pick up an ambient bunfig.toml;
change defaultCertCount (and its use of spawn/bunExe/probe) to run in an
isolated temporary directory when cwd is undefined by creating a fresh temp
folder (e.g., mkdtemp) and passing that path as cwd to spawn; ensure the temp
dir is cleaned up after the proc completes so all calls to defaultCertCount
without an explicit cwd become hermetic.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: e3b9c0f5-cbe4-4c0a-af8b-95f4d49e7e6e

📥 Commits

Reviewing files that changed from the base of the PR and between 666f484 and ee2d55f.

📒 Files selected for processing (5)
  • docs/runtime/bunfig.mdx
  • src/cli/Arguments.zig
  • src/cli/repl_command.zig
  • src/cli/run_command.zig
  • test/js/node/tls/test-use-system-ca.test.ts

Comment thread src/cli/repl_command.zig Outdated
Comment thread test/js/node/tls/test-use-system-ca.test.ts
Comment thread test/js/node/tls/test-use-system-ca.test.ts Outdated
Comment thread src/cli/run_command.zig Outdated

@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.

All prior feedback has been addressed (thanks!) and I found no new issues, but I'm deferring to a human reviewer since this configures TLS trust-store selection and threads a new locked-global through five deferred bunfig-load sites — worth a second pair of eyes on the precedence/ordering.

Extended reasoning...

Overview

This PR adds a top-level CA key to bunfig.toml ("system" / "openssl" / "bundled") as a persistent equivalent to the --use-*-ca CLI flags. It touches 9 files: the bunfig parser, Arguments.zig (precedence logic + new Bun__Node__CAStore_locked global + applyBunfigCAStore() helper), five call sites that invoke applyBunfigCAStore after deferred bunfig loads (run_command.zig, repl_command.zig, bun.js.zig ×2, plus inside parse itself), the Context.zig options struct, cli.zig type re-export, docs, and ~170 lines of new tests.

Over the course of review I flagged two real ordering bugs (the .RunCommand deferred-load gap in ee2d55f and the standalone-executable gap in 9467c48), plus several test-quality and docs nits — all now resolved. The current bug-hunting pass found nothing new.

Security risks

The change selects which root CA store the runtime trusts for TLS verification. The mechanism itself is sound — it only routes between three pre-existing stores (bundled Mozilla, OpenSSL default, OS trust store), all of which were already reachable via CLI flags. The precedence (CLI > env > bunfig > default) is documented and tested, and the _locked flag prevents a late bunfig load from overriding an explicit CLI/env choice. I don't see an exploitable issue, but "which CAs do we trust" is inherently security-sensitive configuration and the multi-site application pattern (five applyBunfigCAStore calls covering different boot paths) is the kind of thing where a missed call site silently degrades to the wrong store — exactly the class of bug this PR already had to fix twice during review.

Level of scrutiny

Medium-high. This is a user-facing feature in production code paths (run/auto/test/repl/standalone), not a config tweak or refactor. The locking mechanism and the need to sprinkle applyBunfigCAStore after every deferred loadConfigPath(..., .RunCommand) is a design decision a maintainer should sign off on — particularly whether there's a cleaner single-point-of-application, and whether the top-level CA key name (vs. nesting under [run] or similar) is the desired surface.

Other factors

Test coverage is thorough for the happy paths and precedence rules across bun <file>, bun run <file>, and bun test. CI shows two unrelated pre-existing flakes (s3-storage-class, http-emit-close-on-abort). All review threads — mine and CodeRabbit's — are resolved. Given the security-adjacent surface and the non-trivial cross-cutting wiring, I'm not auto-approving.

@robobun
robobun force-pushed the farm/2cabcd73/bunfig-ca-store branch from 9467c48 to 251ac98 Compare May 16, 2026 15:26
@robobun
robobun force-pushed the farm/2cabcd73/bunfig-ca-store branch from 251ac98 to 88e7ded Compare May 24, 2026 20:07

@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.

All prior feedback has been addressed and I didn't find anything new this round — deferring to a human since this is a security-adjacent feature (TLS CA store selection) with a non-trivial precedence/locking scheme spread across several deferred-load sites in both the Zig and Rust runtimes.

Extended reasoning...

Overview

Adds a top-level CA key to bunfig.toml ("system" / "openssl" / "bundled") as a persistent equivalent to the --use-*-ca flags. Touches 14 files: the bunfig parser (Zig + Rust), RuntimeOptions / BunCAStore type plumbing in options_types/, the CA precedence block in Arguments.{zig,rs}, a new applyBunfigCAStore helper plus a Bun__Node__CAStore_locked flag, call sites in run_command, repl_command, and bun.js.zig (boot / bootStandalone), docs, and tests.

Security risks

This directly controls which CA trust store the runtime uses for TLS verification. The change itself only adds a new source for an existing setting (the values map onto the same three stores already reachable via CLI flags / NODE_USE_SYSTEM_CA), so it doesn't introduce a new trust surface. The main risk is precedence/ordering: a bug here could cause a user's explicit --use-bundled-ca to be silently overridden by a project-local bunfig.toml, or vice versa. The _locked flag addresses exactly that, and the test suite covers CLI-over-bunfig and env-over-bunfig, but the number of distinct entry paths (auto, run, test, repl, standalone, preloaded --config) makes this worth a human pass.

Level of scrutiny

Medium-high. Not a trivial config tweak: it's a user-facing feature in security-adjacent code, the review surfaced two real ordering bugs (the .RunCommand deferred-load gap and the bootStandalone gap) that were fixed mid-review, and the most recent push ports the whole thing to the Rust runtime in parallel. The Zig and Rust implementations need to stay in lockstep.

Other factors

  • All five rounds of my prior inline feedback (test assertion ordering, weak bun test assertion, describe.concurrent, missing docs, dead anchors, the bun run deferred-load bug, and the bun.js.zig standalone gap) have been addressed and resolved.
  • robobun's CI comment currently shows build-rust failures on several targets for 88e7ded; worth confirming eee21cf (autofix) cleared those before merge.
  • Good test coverage for the precedence matrix (CLI > env > bunfig) and for bun run / bun test paths; no coverage for the standalone-executable path, which is harder to test but was fixed by inspection.
  • Design choices a maintainer may want to weigh in on: top-level CA key naming (vs. e.g. [run].ca), and accepting both CA and lowercase ca.

Comment thread src/runtime/cli/Arguments.zig Outdated
@robobun

robobun commented May 24, 2026

Copy link
Copy Markdown
Collaborator Author

CI status: every executed lane passed on 19cacd8. The two remaining reds on build 57692 (darwin-14-aarch64-test-bun, darwin-26-aarch64-test-bun) are both "Expired" — buildkite agents never picked up the jobs within TTL, nothing ran. Prior build 57686 was the same story: libspng tarball 502 from github during an ASAN C++ build, and a streams-leak.test.ts RSS-threshold flake already diagnosed on main in four prior commits (most recently 1346d3e3f7). I used my one CI re-roll on the previous push; per the workflow I wont push another ci: retrigger. Needs a maintainer to re-run or merge.

@robobun
robobun force-pushed the farm/2cabcd73/bunfig-ca-store branch from 19cacd8 to 2f88bcf Compare June 6, 2026 13:38
@robobun
robobun force-pushed the farm/2cabcd73/bunfig-ca-store branch from 2f88bcf to 811db1c Compare June 26, 2026 01:17
@mintlify

mintlify Bot commented Jun 26, 2026 •

Copy link
Copy Markdown

Preview deployment for your docs. Learn more about Mintlify Previews.

Project Status Preview Updated (UTC)
bun 🟢 Ready View Preview Jun 26, 2026, 1:18 AM

💡 Tip: Enable Workflows to automatically generate PRs for you.

@robobun

robobun commented Jun 26, 2026

Copy link
Copy Markdown
Collaborator Author

Rebased onto main and force-pushed (811db1c3ec).

Conflict resolution: main deleted the .zig reference files (Arguments.zig, bunfig.zig, cli.zig, Context.zig, repl_command.zig, run_command.zig, bun.js.zig) as part of the Zig→Rust migration cleanup. My earlier commits had modified those files, producing modify/delete conflicts. I accepted the deletions and kept the .rs implementation (src/bunfig/bunfig.rs, src/options_types/context.rs, src/runtime/cli/{Arguments,run_command,repl_command}.rs), which is what ships. This also resolves the earlier review note about adding behavior to .zig files, those files no longer exist.

Final diff is 7 files: the 5 Rust files, docs/runtime/bunfig.mdx, and test/js/node/tls/test-use-system-ca.test.ts.

Verified the full precedence matrix directly against the debug build (getCACertificates("default").length):

baseline (no flag/env/bunfig): 120   (bundled)
--use-system-ca:               423
bunfig CA="system":            423   (== flag)
bunfig CA="bundled":           120   (== baseline)
bunfig CA="openssl":           120   (== --use-openssl-ca)
bun run <file> + CA="system":  423   (deferred-load path)
NODE_USE_SYSTEM_CA=1 over CA="bundled": 423
--use-bundled-ca over CA="system":      120
invalid CA value: exit 1, "Invalid CA value \"nope\". Expected one of: ..."

Note: bun test currently aborts at startup on this main commit while loading bun:internal-for-testing (Private symbol not found: newZigFunction(..., "TestingAPIs.jsSocketFaultInjectionAvailable", 0)). That file is byte-identical to main and unrelated to this PR; the multi-line $newZigFunction call at internal-for-testing.ts:303 is not being registered by the JS2Native codegen. It reproduces on a fully clean build, so it is a pre-existing codegen issue, not introduced here. The CA feature is validated directly above in the meantime.

@robobun

robobun commented Jun 26, 2026

Copy link
Copy Markdown
Collaborator Author

Rebased onto latest main (e174a4e5a3) and force-pushed; no merge conflicts remain.

Heads-up for whoever reviews: current main HEAD does not build, independent of this PR. src/js/codegen/generate-js2native.ts now rejects the socket-fault testing bindings because src/js/internal-for-testing.ts calls $newRustFunction("runtime/socket/socket.zig", ...) with a .zig path after the $newZigFunction→$newRustFunction rename:

error: Expected filename for $rust to have .rs extension, got "runtime/socket/socket.zig"
    at resolveNativeFileId (src/codegen/generate-js2native.ts:106:15)
While processing: internal-for-testing.ts

I confirmed this reproduces with every file of this PR reverted to origin/main, so it blocks all builds on this commit, not just this branch. (On the prior main base the same area instead built but crashed bun test at startup loading bun:internal-for-testing.)

The CA feature itself compiles (cargo check -p bun_options_types -p bun_bunfig is clean; full debug+release built on the prior base) and every assertion in the test is validated directly against the debug build:

baseline:120  --use-system-ca:423
bunfig CA="system":423 (==flag)   CA="bundled":120 (==baseline)   CA="openssl":120 (==--use-openssl-ca)
bun run <file> + CA="system":423   NODE_USE_SYSTEM_CA=1 over CA="bundled":423   --use-bundled-ca over CA="system":120
invalid CA value -> exit 1 + "Invalid CA value ..."

Once main's codegen path is fixed, a re-run (or trivial rebase) should go green.

@robobun

robobun commented Jun 26, 2026

Copy link
Copy Markdown
Collaborator Author

Update: main fixed the codegen path ($newRustFunction now takes the correct .rs filename in internal-for-testing.ts), so the build blocker from my previous comment is gone. Rebased onto the fixed main and force-pushed.

bun bd test test/js/node/tls/test-use-system-ca.test.ts now runs clean: 12 pass / 0 fail. Fail-before verified — reverting my src/ changes yields 4 failures (e.g. bun test honors CA = "system" gets 120 instead of 423), confirming the tests exercise the fix.

robobun and others added 7 commits August 14, 2026 10:42
`bun run <file>` defers its bunfig.toml load to RunCommand.exec (it needs
to resolve the script's directory first), so the CA precedence block in
Arguments.parse runs before ctx.runtime_options.ca_store is populated,
leaving `CA = "system"` silently ignored.

Extract the precedence step into `applyBunfigCAStore()`, call it once from
Arguments.parse (to cover .AutoCommand/.TestCommand which load bunfig
eagerly), and again from RunCommand.exec/repl_command after the deferred
load. A CLI flag or NODE_USE_SYSTEM_CA env var pins the store via a lock
flag so the later call cannot downgrade it.

Also fix two docs nits from the same review: say the CA key applies to
`bun run`, `bun test`, and implicit `bun file.ts` invocations, and
point the cross-reference at `install.ca and install.cafile`'s actual
combined anchor.
Three review findings on ee2d55f:

1. Standalone `bun build --compile` binaries call `bun.js.Run.bootStandalone`
   which defers the bunfig load but never called `applyBunfigCAStore`, so
   `CA = "system"` was silently ignored in compiled executables. Same
   for `Run.boot` in bun.js.zig. Both sites now apply the bunfig value.

2. In `RunCommand.exec` / `repl_command` the apply call sat inside the
   `if (!loaded_bunfig)` guard — if bunfig was preloaded (e.g. via
   `--config`), the CA value was parsed but never copied into
   `Bun__Node__CAStore`. Call `applyBunfigCAStore` unconditionally;
   the lock flag still keeps a CLI flag / env var authoritative.

3. The test file: make `defaultCertCount`'s default cwd an empty
   temp dir so callers without an explicit `cwd` can't pick up an
   ambient `bunfig.toml`. Tighten the openssl test from "it parses"
   to "matches `--use-openssl-ca`", mirroring the system/bundled
   parity checks.
The Bun-in-Rust rewrite (#30412) landed between my PR and main — the
CLI-argument parsing, bunfig.toml parsing, and the run/repl/standalone
boot sites are now in Rust, and the Rust side didn't carry over the
bunfig `CA = "system"` parsing or the deferred-load `apply` hook
I added on the Zig side.

Port the same shape 1:1:

- `options_types/context.rs`: add `pub enum BunCAStore` and
  `runtime_options.ca_store: Option<BunCAStore>`, mirroring Context.zig.
- `bunfig.rs`: parse top-level `CA` / `ca` for Run/Auto/Test
  commands into `ctx.runtime_options.ca_store`.
- `cli/Arguments.rs`: replace local `BunCAStore` with a re-export
  from `bun_options_types::context`. Add `Bun__Node__CAStore_locked`
  and `apply_bunfig_ca_store(ctx)` with the same precedence contract
  as the Zig version (CLI flag > NODE_USE_SYSTEM_CA > bunfig > bundled).
  CLI/env branches pin the lock flag so a later bunfig load can't
  downgrade them.
- `run_command.rs` (boot / boot_standalone / the fast-run bunfig
  reload site), `repl_command.rs`: call `apply_bunfig_ca_store(ctx)`
  after each deferred `load_config_path`. The Zig-side copies of the
  same sites still exist as dead code but match for consistency.

All 12 tests in `test/js/node/tls/test-use-system-ca.test.ts` pass
under bun bd; the two load-bearing tests (`CA = "system"` matches
`--use-system-ca`, invalid-value diagnostic) fail under stashed src.
Both prior CI failures were infra/flake (libspng GitHub 502 during
vendor fetch on one ASAN C++ build; streams-leak.test.ts known RSS
threshold flake on debian-13-x64, same flake fired on 5 other
platforms in the same run per the retry annotation). Neither touches
this PR's diff.
@robobun
robobun force-pushed the farm/2cabcd73/bunfig-ca-store branch from c752c36 to 2f69650 Compare August 14, 2026 10:46
Comment thread src/options_types/context.rs Outdated
Comment thread src/options_types/context.rs Outdated
Comment thread src/runtime/cli/Arguments.rs Outdated
Comment thread src/runtime/cli/Arguments.rs Outdated
Comment thread src/runtime/cli/Arguments.rs Outdated
Comment thread src/runtime/cli/Arguments.rs Outdated
Comment thread src/runtime/cli/repl_command.rs Outdated
Comment thread src/runtime/cli/run_command.rs Outdated
Comment thread src/runtime/cli/run_command.rs Outdated
Comment thread src/runtime/cli/run_command.rs Outdated
Comment thread src/runtime/cli/Arguments.rs Outdated
Comment thread src/runtime/cli/Arguments.rs Outdated
Comment thread src/runtime/cli/run_command.rs Outdated

@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.

Re-reviewed after 76d5580 / 205cd0c — the call-site comments are now one line each, so the earlier nit (and the comment-cop warnings) are resolved. No issues found this pass.

Deferring final sign-off to a maintainer since this adds a new user-facing bunfig key (CA) that selects the runtime TLS trust store — the key name, accepted values, and precedence are API surface worth a human confirming.

What was reviewed

  • Precedence logic in Arguments.rs: CLI flag / NODE_USE_SYSTEM_CA set the lock before apply_bunfig_ca_store, and Bun__Node__UseSystemCA is still written on every path.
  • All deferred load_config_path(.., RunCommand) sites (run_command.rs ×3, repl_command.rs) call apply_bunfig_ca_store outside the loaded_bunfig guard.
  • BunCAStore move to context.rs keeps #[repr(u8)] and the same discriminant order the C++ side reads via Bun__Node__CAStore.
  • Bunfig parser branch: invalid values error via add_error_format (non-zero exit), and the top-level CA/ca key doesn't collide with [install].ca.
Extended reasoning...

Overview

Adds a top-level CA = "system" | "openssl" | "bundled" key to bunfig.toml that selects the runtime TLS CA store, equivalent to the existing --use-*-ca CLI flags. Touches: bunfig.rs (parser branch), context.rs (moves BunCAStore enum + adds runtime_options.ca_store), Arguments.rs (precedence block + apply_bunfig_ca_store helper + Bun__Node__CAStore_locked static), three run_command.rs call sites and one repl_command.rs call site (deferred bunfig loads), docs, and tests.

Since my last comment (11:05 UTC), commits 76d5580 and 205cd0c trimmed every multi-line call-site/doc comment to a single line, addressing both my nit and the comment-cop bot's inline warnings. No logic changed.

Security risks

The change routes to one of three CA stores that are already selectable via CLI flags and NODE_USE_SYSTEM_CA; it does not introduce a new trust source or weaken verification. The precedence (CLI > env > bunfig > bundled) means an ambient bunfig.toml cannot override an explicit CLI/env choice — the lock flag is set before apply_bunfig_ca_store runs. The parser rejects unknown values with a hard error rather than silently falling through. I don't see a way this expands the attack surface beyond what the existing flags already permit.

Level of scrutiny

Medium-high. The implementation is mechanical plumbing of an existing capability, but (a) it's a new user-facing config key whose name and value set become permanent API, and (b) it's TLS-adjacent. Per the repo's review guidance on API design and security-sensitive paths, a maintainer should confirm the surface (uppercase CA with lowercase alias, string enum values, documented precedence) is what they want before it ships.

Other factors

This PR has been through several review rounds since May: the missed deferred-load sites (my earlier finding), the non-hermetic test cwd, the openssl parity assertion, the REPL guard placement, and the .zig dual-tracking have all been fixed or mooted by rebase. Test coverage is thorough — the full precedence matrix plus bun run, bun test, and invalid-value paths, verified to fail on baseline. All comment-cop and CodeRabbit threads are resolved. The remaining open item is my own nit thread, which the latest two commits address; nothing substantive is outstanding.

@robobun

robobun commented Aug 14, 2026

Copy link
Copy Markdown
Collaborator Author

CI for 205cd0c is effectively green: 177 of 179 jobs passed, and the build only reports failed because the two darwin 14 aarch64 test jobs expired waiting for an agent. The two flaky-test warnings (inspect-error-leak timeout, napi test_buffer) passed on retry and are unrelated to this change. Ready for maintainer review.

@anru anru left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This PR doesn't fix #17108 issue. Bun must take into account NODE_USE_SYSTEM_CA, NODE_TLS_REJECT_UNAUTHORIZED or other net/tls related ENV for network operations.

Bun should take this ENVs into account regardless of the command used. These environment variables should act as runtime-wide environment properties, not just for specific commands.

Comment thread docs/runtime/bunfig.mdx
| `"system"` | Bundled CAs merged with the OS trust store (Keychain on macOS, Windows Certificate Store on Windows, system PEM bundle on Linux). |
| `"openssl"` | OpenSSL's default CA store. |

Precedence: CLI flag > `NODE_USE_SYSTEM_CA` env var > `bunfig.toml` `CA` > `"bundled"` (default). This does not affect `bun install`'s registry client — for that, see [`install.ca` and `install.cafile`](#install-ca-and-install-cafile).

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This does not affect bun install's registry client — for that, see install.ca and install.cafile.

This is flawed logic. See here #17325 for why.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Ok, @robobun here is additional context for you why this is flawed logic (from issue #17325 ):

As a user who want adopt bun in SDLC - I don't want to commit the certificate itself or the path to it. I want to be able to set a single certificate for all operations performed through bun, like the NODE_EXTRA_CA_CERTS variable for a node. Or in other words, I want to configure my environment, via a variable once, and thus allow all bun commands to work correctly.

Bun must take into account NODE_USE_SYSTEM_CA, NODE_TLS_REJECT_UNAUTHORIZED or other net/tls related ENV for network operations.

Bun should take this ENVs into account regardless of the command used. These environment variables should act as runtime-wide environment properties, not just for specific commands.

I propose to expand the scope of this MR so that the bun actually takes these environmental variables into account at the proper level. In that case this will fix issue #17325

@Jarred-Sumner what do you think about this ?

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Also, can you reopen issue #17325 ? This issue is not resolved on main branch:

shell session with Bun v1.4.0 (revision 34cbb9a):

buni *1 ❯  NODE_EXTRA_CA_CERTS=/etc/ssl/certs/internal-net.crt NODE_USE_SYSTEM_CA=1 bun install
bun install v1.4.0 (34cbb9a40)
HTTPThread: the CA is invalid
buni *1 ❯ echo $?
1
buni *1 ❯ NODE_EXTRA_CA_CERTS=/etc/ssl/certs/internal-net.crt yarn install --silent --pure-lockfile
(node:15252) Warning: Setting the NODE_TLS_REJECT_UNAUTHORIZED environment variable to '0' makes TLS connections and HTTPS requests insecure by disabling certificate verification.
(Use `node --trace-warnings ...` to show where the warning was created)
buni *1 ❯ echo $?                                                                                                                                                                                                                        31s
0

yarn install still works in this case.

@robobun

robobun commented Aug 21, 2026

Copy link
Copy Markdown
Collaborator Author

@anru You are right that this PR does not resolve #17108, and I removed the closing references to #17108 and #17325 from the PR body so they stay open.

The scope here is #30313 only: make the existing CA-store selection configurable in bunfig.toml. The selection mechanism itself is unchanged. Today Bun reads the --use-*-ca flags and NODE_USE_SYSTEM_CA only for runtime commands (bun run, bun test, bun file.ts, and node compat mode). The bun install registry client has its own trust config (install.ca and install.cafile). That split predates this PR.

Making the CA selection apply runtime-wide, including bun install, and honoring more TLS env vars is a real request. It needs its own change and review, and #17108 and #17325 now remain open to track it.

Jarred-Sumner added a commit that referenced this pull request Oct 10, 2026
…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 was successfully deployed

1 active (outdated) deployment
staging - docs — 2f69650b Deployed Aug 14, 2026 by mintlify[bot]
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.

[request] runtime option use-system-ca in bunfig.toml

2 participants