Skip to content

Add Bun.loadExtraCACerts(string) that works like $NODE_EXTRA_CA_CERTS does at startup#20386

Open
nabijaczleweli wants to merge 1 commit into
oven-sh:mainfrom
nabijaczleweli:main
Open

Add Bun.loadExtraCACerts(string) that works like $NODE_EXTRA_CA_CERTS does at startup#20386
nabijaczleweli wants to merge 1 commit into
oven-sh:mainfrom
nabijaczleweli:main

Conversation

@nabijaczleweli

@nabijaczleweli nabijaczleweli commented Jun 14, 2025

Copy link
Copy Markdown

What does this PR do?

Add Bun.loadExtraCACerts(string) to load additional certificates into the current certificate store, like $NODE_EXTRA_CA_CERTS does on start-up.

This is the first and only way to change them dynamically.

This implements/closes #13868

  • Documentation or TypeScript types (it's okay to leave the rest blank in this case)
  • Code changes

How did you verify your code works?

  • I included a test for the new code, or existing tests cover it
  • I ran my tests locally and they pass (bun-debug test test-file-name.test)

Auto-testing this would involve generating TLS certificates and making HTTP servers. I've tested this manually thusly:

// http --ssl ~/uwu/tls/ca/server.p12  -p 8000
// http --ssl ~/uwu/tls/ca2/server.p12 -p 8001
// NODE_EXTRA_CA_CERTS=/srv/tls/ca/ca.crt bun ./repro.ts
// 0. /srv/tls/ca/ca.crt is loaded from $NODE_EXTRA_CA_CERTS
// 1. request to :8000 succeeds
// 2. request to :8001 fails
// 3. /srv/tls/ca2/ca.crt is loaded from loadExtraCACerts()
// 4. request to :8001 succeeds now
async function one(port) {
	try {
		const response = await fetch(`https://tarta:${port}/util.diff`, {method: 'GET', verbose: false});
		const html = await response.text();
		console.log(html)
	} catch(e) {
		console.log(e)
	}
}

await one(8000);
await one(8001);
Bun.loadExtraCACerts("/srv/tls/ca2/ca.crt");
await one(8001);

where tls is tls.tar.gz

Comment thread packages/bun-types/bun.d.ts Outdated
@nabijaczleweli nabijaczleweli force-pushed the main branch 2 times, most recently from 35f24b9 to bb25303 Compare June 14, 2025 12:07
@nabijaczleweli nabijaczleweli changed the title Add Bun.loadAdditionalCACerts(string) that works like $NODE_EXTRA_CA_CERTS does at startup Add Bun.loadExtraCACerts(string) that works like $NODE_EXTRA_CA_CERTS does at startup Jun 14, 2025
@nabijaczleweli nabijaczleweli force-pushed the main branch 2 times, most recently from 5d2832f to a6d4384 Compare June 14, 2025 19:01
@nabijaczleweli

Copy link
Copy Markdown
Author

Rebased, should be g2g. No locking applies to store since boringssl X509_STORE_add_cert (vendor/boringssl/crypto/x509/x509_lu.cc) takes a lock on store itself.

@nabijaczleweli

Copy link
Copy Markdown
Author

Rebased, bump

@nabijaczleweli

Copy link
Copy Markdown
Author

Rebased, bump :)

@nabijaczleweli

Copy link
Copy Markdown
Author

Rebased

@coderabbitai

coderabbitai Bot commented Nov 2, 2025

Copy link
Copy Markdown
Contributor

Note

Other AI code review bot(s) detected

CodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review.

Walkthrough

Adds a runtime API Bun.loadExtraCACerts to load extra CA certificates from a file into the global CA store; includes type declarations, C/C++ loading implementation and synchronization, JS binding, and documentation update.

Changes

Cohort / File(s) Summary
Documentation
docs/runtime/bun-apis.md
Inserted a new "Cryptography" section in the API topics table and added an entry for Bun.loadExtraCACerts.
Type definitions
packages/bun-types/bun.d.ts
Added declaration function loadExtraCACerts(path: string) to the bun module (appears in two insertion points).
Crypto implementation
packages/bun-usockets/src/crypto/root_certs.cpp, packages/bun-usockets/src/crypto/root_certs_header.h
Added us_load_extra_ca_certs(const char*) C interface, a static X509_STORE *store, and logic to load/merge extra CA certs; introduced root_extra_cert_instances path, conditional system CA loading, and a new mutex us_get_root_extra_cert_instances_mutex.
JS bindings (Bun object)
src/bun.js/bindings/BunObject.cpp
Added functionBunLoadExtraCACerts host function that resolves a path and calls us_load_extra_ca_certs; exposed as Bun.loadExtraCACerts; included root certs header.
TLS synchronization
src/bun.js/bindings/NodeTLS.cpp
Added std::unique_lock acquiring us_get_root_extra_cert_instances_mutex in getExtraCACertificates to synchronize access to extra cert instances.

Possibly related PRs

Pre-merge checks

✅ Passed checks (3 passed)
Check name Status Explanation
Title Check ✅ Passed The PR title "Add Bun.loadExtraCACerts(string) that works like $NODE_EXTRA_CA_CERTS does at startup" directly and accurately describes the main change across all modified files. The title is specific and clear, referencing both the new function name and its intended behavior, without unnecessary noise or vague language. The implementation spans documentation, TypeScript types, C/C++ runtime code, and JavaScript bindings—all focused on delivering exactly what the title promises.
Linked Issues Check ✅ Passed The PR directly implements the core requirements from issue #13868 by adding a runtime API to load extra CA certificates dynamically. Issue #13868 requests the ability to set NODE_EXTRA_CA_CERTS behavior in standalone executables, proposing a hook to add certificates at runtime so custom CAs can be bundled without requiring external environment variable configuration. The implementation delivers exactly this: Bun.loadExtraCACerts(string) provides the requested runtime hook, with supporting documentation, TypeScript types, and C/C++ implementation in the certificate store handling layer.
Out of Scope Changes Check ✅ Passed All code changes are directly related to implementing the Bun.loadExtraCACerts() feature. The documentation update (bun-apis.md) documents the new API, TypeScript types (bun.d.ts) declare the new function signature, C/C++ implementation (root_certs.cpp, root_certs_header.h) provides the certificate loading mechanism and thread-safe access patterns, the JavaScript binding (BunObject.cpp) exposes the API, and the thread-safety enhancement (NodeTLS.cpp) protects the shared certificate store during concurrent access to the new functionality. No unrelated changes or scope creep is present.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

@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: 4

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)
src/bun.js/bindings/NodeTLS.cpp (1)

43-72: Reduce lock scope; copy refs under lock, convert outside.

Hold the mutex only long enough to snapshot the stack and up_ref X509*s, then release it before PEM conversion. This avoids blocking concurrent updates while doing BIO/PEM work.

Also, sk_X509_num(NULL) returns 0; the “size < 0” guard/comment is misleading.

-    std::unique_lock root_extra_cert_instances_lock { us_get_root_extra_cert_instances_mutex };
-    STACK_OF(X509)* root_extra_cert_instances = us_get_root_extra_cert_instances();
-
-    auto size = sk_X509_num(root_extra_cert_instances);
-    if (size < 0) size = 0; // root_extra_cert_instances is nullptr
-    JSC::MarkedArgumentBuffer args;
-    for (auto i = 0; i < size; i++) {
+    // Snapshot under lock, then process outside the critical section
+    Vector<X509*> localCerts;
+    {
+        std::unique_lock lock{us_get_root_extra_cert_instances_mutex};
+        if (auto* stk = us_get_root_extra_cert_instances()) {
+            const int n = sk_X509_num(stk);
+            localCerts.reserveInitialCapacity(n);
+            for (int i = 0; i < n; i++) {
+                if (X509* c = sk_X509_value(stk, i)) {
+                    X509_up_ref(c);
+                    localCerts.append(c);
+                }
+            }
+        }
+    }
+    JSC::MarkedArgumentBuffer args;
+    for (auto* cert : localCerts) {
         BIO* bio = BIO_new(BIO_s_mem());
         if (!bio) {
             throwOutOfMemoryError(globalObject, scope);
             return {};
         }
-
-        if (PEM_write_bio_X509(bio, sk_X509_value(root_extra_cert_instances, i)) != 1) {
+        if (PEM_write_bio_X509(bio, cert) != 1) {
             BIO_free(bio);
             return throwError(globalObject, scope, ErrorCode::ERR_CRYPTO_OPERATION_FAILED, "X509 to PEM conversion"_str);
         }
@@
         args.append(JSC::jsString(vm, str));
         BIO_free(bio);
     }
+    for (auto* cert : localCerts) {
+        X509_free(cert); // drop our ref
+    }

Please confirm no other code mutates the same stack without using us_get_root_extra_cert_instances_mutex.

Also applies to: 46-61

packages/bun-usockets/src/crypto/root_certs.cpp (1)

241-287: Race condition exists but proposed fix is incomplete. us_load_extra_ca_certs also needs mutex protection during store mutations.

The race condition is real: us_get_default_ca_store() (lines 242-283) and us_load_extra_ca_certs() (line 226) both mutate the global store variable without synchronization. Additionally, us_load_extra_ca_certs() at line 226 calls X509_STORE_add_cert(store, cert) before acquiring the mutex at line 229—the mutex acquisition comes too late.

Wrapping us_get_default_ca_store() with the mutex is necessary, but us_load_extra_ca_certs() must also hold the mutex during its store mutations (lines 221–226), not after them. The current code acquires the lock only to update root_extra_cert_instances, leaving the X509_STORE_add_cert calls unprotected.

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Disabled knowledge base sources:

  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 7978476 and 66cb337155707320ec867c671739217fe6233ed6.

📒 Files selected for processing (6)
  • docs/runtime/bun-apis.md (1 hunks)
  • packages/bun-types/bun.d.ts (1 hunks)
  • packages/bun-usockets/src/crypto/root_certs.cpp (5 hunks)
  • packages/bun-usockets/src/crypto/root_certs_header.h (1 hunks)
  • src/bun.js/bindings/BunObject.cpp (3 hunks)
  • src/bun.js/bindings/NodeTLS.cpp (1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{cpp,h}

📄 CodeRabbit inference engine (.cursor/rules/javascriptcore-class.mdc)

**/*.{cpp,h}: When exposing a JS class with public Constructor and Prototype, define three C++ types: class Foo : public JSC::DestructibleObject (if it has C++ fields), class FooPrototype : public JSC::JSNonFinalObject, and class FooConstructor : public JSC::InternalFunction
If the class has C++ data members, inherit from JSC::DestructibleObject and provide proper destruction; if it has no C++ fields (only JS properties), avoid a class and use JSC::constructEmptyObject(vm, structure) with putDirectOffset
Prefer placing the subspaceFor implementation in the .cpp file rather than the header when possible

Files:

  • packages/bun-usockets/src/crypto/root_certs_header.h
  • src/bun.js/bindings/BunObject.cpp
  • packages/bun-usockets/src/crypto/root_certs.cpp
  • src/bun.js/bindings/NodeTLS.cpp
**/*.cpp

📄 CodeRabbit inference engine (.cursor/rules/javascriptcore-class.mdc)

**/*.cpp: Include "root.h" at the top of C++ binding files to satisfy lints
Define prototype properties using a const HashTableValue array and declare accessors/functions with JSC_DECLARE_* macros
Prototype classes should subclass JSC::JSNonFinalObject, provide create/createStructure, DECLARE_INFO, finishCreation that reifies static properties, and set mayBePrototype on the Structure
Custom getters should use JSC_DEFINE_CUSTOM_GETTER, jsDynamicCast to validate this, and throwThisTypeError on mismatch
Custom setters should use JSC_DEFINE_CUSTOM_SETTER, validate this via jsDynamicCast, and store via WriteBarrier/set semantics
Prototype functions should use JSC_DEFINE_HOST_FUNCTION, validate this with jsDynamicCast, and return encoded JSValue
Constructors should subclass JSC::InternalFunction, return internalFunctionSpace in subspaceFor, set the prototype property as non-configurable/non-writable, and provide create/createStructure
Provide a setup function that builds the Prototype, Constructor, and Structure, and assigns them to the LazyClassStructure initializer
Use the cached Structure via globalObject->m_.get(globalObject) when constructing instances
Expose constructors to Zig via an extern "C" function that returns the constructor from the LazyClassStructure
Provide an extern "C" Bun____toJS function that creates an instance using the cached Structure and returns an EncodedJSValue

Files:

  • src/bun.js/bindings/BunObject.cpp
  • packages/bun-usockets/src/crypto/root_certs.cpp
  • src/bun.js/bindings/NodeTLS.cpp
src/bun.js/bindings/**/*.cpp

📄 CodeRabbit inference engine (CLAUDE.md)

src/bun.js/bindings/**/*.cpp: When implementing a JavaScript class with a public constructor in C++, create Foo, FooPrototype, and FooConstructor classes
Define class properties using HashTableValue arrays in C++ bindings
Add iso subspaces for classes with C++ fields in JavaScriptCore bindings

Files:

  • src/bun.js/bindings/BunObject.cpp
  • src/bun.js/bindings/NodeTLS.cpp
🧠 Learnings (23)
📚 Learning: 2025-10-19T02:52:37.412Z
Learnt from: theshadow27
Repo: oven-sh/bun PR: 23798
File: packages/bun-otel/tsconfig.json:1-15
Timestamp: 2025-10-19T02:52:37.412Z
Learning: In the Bun repository, packages under packages/ (e.g., bun-otel) can follow a TypeScript-first pattern where package.json exports point directly to .ts files (not compiled .js files). Bun natively runs TypeScript, so consumers import .ts sources directly and receive full type information without needing compiled .d.ts declaration files. For such packages, adding "declaration": true or "outDir" in tsconfig.json is unnecessary and would break the export structure.
<!-- [remove_learning]
ceedde95-980e-4898-a2c6-40ff73913664

Applied to files:

  • packages/bun-types/bun.d.ts
📚 Learning: 2025-08-30T00:12:56.803Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: .cursor/rules/writing-tests.mdc:0-0
Timestamp: 2025-08-30T00:12:56.803Z
Learning: Applies to test/js/bun/**/*.{js,ts} : Place Bun API tests under test/js/bun/, separated by category (e.g., test/js/bun/glob/)

Applied to files:

  • packages/bun-types/bun.d.ts
📚 Learning: 2025-10-04T09:52:49.414Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/js/CLAUDE.md:0-0
Timestamp: 2025-10-04T09:52:49.414Z
Learning: Applies to src/js/bun/**/*.{ts,js} : Place Bun-specific modules (e.g., bun:ffi, bun:sqlite) under bun/

Applied to files:

  • packages/bun-types/bun.d.ts
  • docs/runtime/bun-apis.md
📚 Learning: 2025-10-19T04:38:37.720Z
Learnt from: theshadow27
Repo: oven-sh/bun PR: 23798
File: packages/bun-otel/examples/basic.ts:10-10
Timestamp: 2025-10-19T04:38:37.720Z
Learning: In packages/bun-otel/bun-sdk.ts, BunSDK.start() is intentionally synchronous (returns void) unlike NodeSDK.start() which is async. This is because BunSDK performs resource detection synchronously in the constructor using detectResourcesSync, while NodeSDK performs async resource detection during start(). The start() method only performs synchronous operations (setting context manager, propagator, creating tracer provider, calling installBunNativeTracing), so an async signature would be misleading.

Applied to files:

  • packages/bun-types/bun.d.ts
📚 Learning: 2025-08-30T00:13:36.815Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: .cursor/rules/zig-javascriptcore-classes.mdc:0-0
Timestamp: 2025-08-30T00:13:36.815Z
Learning: Applies to src/bun.js/bindings/generated_classes_list.zig : Update src/bun.js/bindings/generated_classes_list.zig to include new classes

Applied to files:

  • packages/bun-types/bun.d.ts
📚 Learning: 2025-09-12T18:16:50.754Z
Learnt from: RiskyMH
Repo: oven-sh/bun PR: 22606
File: src/glob/GlobWalker.zig:449-452
Timestamp: 2025-09-12T18:16:50.754Z
Learning: For Bun codebase: prefer using `std.fs.path.sep` over manual platform separator detection, and use `bun.strings.lastIndexOfChar` instead of `std.mem.lastIndexOfScalar` for string operations.

Applied to files:

  • packages/bun-types/bun.d.ts
📚 Learning: 2025-10-19T02:44:46.354Z
Learnt from: theshadow27
Repo: oven-sh/bun PR: 23798
File: packages/bun-otel/context-propagation.test.ts:1-1
Timestamp: 2025-10-19T02:44:46.354Z
Learning: In the Bun repository, standalone packages under packages/ (e.g., bun-vscode, bun-inspector-protocol, bun-plugin-yaml, bun-plugin-svelte, bun-debug-adapter-protocol, bun-otel) co-locate their tests with package source code using *.test.ts files. This follows standard npm/monorepo patterns. The test/ directory hierarchy (test/js/bun/, test/cli/, test/js/node/) is reserved for testing Bun's core runtime APIs and built-in functionality, not standalone packages.

Applied to files:

  • packages/bun-types/bun.d.ts
📚 Learning: 2025-09-08T00:41:12.052Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/bun.js/bindings/v8/CLAUDE.md:0-0
Timestamp: 2025-09-08T00:41:12.052Z
Learning: Applies to src/bun.js/bindings/v8/V8*.h : Ensure all public V8 API functions are marked with BUN_EXPORT for symbol visibility

Applied to files:

  • src/bun.js/bindings/BunObject.cpp
📚 Learning: 2025-09-08T00:41:12.052Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/bun.js/bindings/v8/CLAUDE.md:0-0
Timestamp: 2025-09-08T00:41:12.052Z
Learning: Applies to src/bun.js/bindings/v8/src/symbols.txt : Add new V8 API mangled symbols (without leading underscore) to src/symbols.txt

Applied to files:

  • src/bun.js/bindings/BunObject.cpp
📚 Learning: 2025-10-26T05:04:50.692Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-26T05:04:50.692Z
Learning: Applies to src/bun.js/bindings/**/*.cpp : Add iso subspaces for classes with C++ fields in JavaScriptCore bindings

Applied to files:

  • src/bun.js/bindings/BunObject.cpp
  • src/bun.js/bindings/NodeTLS.cpp
📚 Learning: 2025-09-08T00:41:12.052Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/bun.js/bindings/v8/CLAUDE.md:0-0
Timestamp: 2025-09-08T00:41:12.052Z
Learning: Applies to src/bun.js/bindings/v8/test/v8/v8-module/main.cpp : Add a C++ test function in test/v8/v8-module/main.cpp and register it in Init using NODE_SET_METHOD

Applied to files:

  • src/bun.js/bindings/BunObject.cpp
📚 Learning: 2025-08-30T00:13:36.815Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: .cursor/rules/zig-javascriptcore-classes.mdc:0-0
Timestamp: 2025-08-30T00:13:36.815Z
Learning: Applies to src/bun.js/**/*.zig : Use parameter name globalObject (not ctx) and accept (*JSC.JSGlobalObject, *JSC.CallFrame) in binding methods/constructors

Applied to files:

  • src/bun.js/bindings/BunObject.cpp
📚 Learning: 2025-09-08T00:41:12.052Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/bun.js/bindings/v8/CLAUDE.md:0-0
Timestamp: 2025-09-08T00:41:12.052Z
Learning: Applies to src/bun.js/bindings/v8/V8*.h : Place new V8 API class headers in src/bun.js/bindings/v8/ named V8<Class>.h with declarations in namespace v8 and BUN_EXPORT on public methods

Applied to files:

  • src/bun.js/bindings/BunObject.cpp
📚 Learning: 2025-09-08T00:41:12.052Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/bun.js/bindings/v8/CLAUDE.md:0-0
Timestamp: 2025-09-08T00:41:12.052Z
Learning: Applies to src/bun.js/bindings/v8/src/symbols.dyn : Add new V8 API mangled symbols (with leading underscore and semicolons) to src/symbols.dyn

Applied to files:

  • src/bun.js/bindings/BunObject.cpp
📚 Learning: 2025-09-20T05:35:57.318Z
Learnt from: pfgithub
Repo: oven-sh/bun PR: 22534
File: src/bun.js/bindings/headers.h:729-731
Timestamp: 2025-09-20T05:35:57.318Z
Learning: symbols.txt in the Bun codebase is specifically for V8 API mangled symbols (without leading underscore), not for general Bun host functions declared with BUN_DECLARE_HOST_FUNCTION. Host functions are handled through different build mechanisms.

Applied to files:

  • src/bun.js/bindings/BunObject.cpp
📚 Learning: 2025-08-30T00:11:00.890Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: .cursor/rules/javascriptcore-class.mdc:0-0
Timestamp: 2025-08-30T00:11:00.890Z
Learning: Applies to **/*.cpp : Provide an extern "C" Bun__<Type>__toJS function that creates an instance using the cached Structure and returns an EncodedJSValue

Applied to files:

  • src/bun.js/bindings/BunObject.cpp
📚 Learning: 2025-10-26T05:04:50.692Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-26T05:04:50.692Z
Learning: Applies to src/bun.js/bindings/**/*.cpp : Define class properties using HashTableValue arrays in C++ bindings

Applied to files:

  • src/bun.js/bindings/BunObject.cpp
📚 Learning: 2025-08-30T00:11:00.890Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: .cursor/rules/javascriptcore-class.mdc:0-0
Timestamp: 2025-08-30T00:11:00.890Z
Learning: Applies to **/*.cpp : Prototype functions should use JSC_DEFINE_HOST_FUNCTION, validate this with jsDynamicCast, and return encoded JSValue

Applied to files:

  • src/bun.js/bindings/BunObject.cpp
📚 Learning: 2025-08-30T00:11:00.890Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: .cursor/rules/javascriptcore-class.mdc:0-0
Timestamp: 2025-08-30T00:11:00.890Z
Learning: Applies to **/*.zig : Wrap the Bun__<Type>__toJS extern in a Zig method that takes a JSGlobalObject and returns JSC.JSValue

Applied to files:

  • src/bun.js/bindings/BunObject.cpp
📚 Learning: 2025-08-30T00:13:36.815Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: .cursor/rules/zig-javascriptcore-classes.mdc:0-0
Timestamp: 2025-08-30T00:13:36.815Z
Learning: Applies to src/bun.js/**/*.zig : Access JS call data via CallFrame (argument(i), argumentCount(), thisValue()) and throw errors with globalObject.throw(...)

Applied to files:

  • src/bun.js/bindings/BunObject.cpp
📚 Learning: 2025-09-08T00:41:12.052Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/bun.js/bindings/v8/CLAUDE.md:0-0
Timestamp: 2025-09-08T00:41:12.052Z
Learning: Applies to src/bun.js/bindings/v8/**/*.{h,cpp} : Use localToJSValue() when converting V8 handles to JSC values before performing JSC operations

Applied to files:

  • src/bun.js/bindings/BunObject.cpp
📚 Learning: 2025-10-04T09:52:49.414Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/js/CLAUDE.md:0-0
Timestamp: 2025-10-04T09:52:49.414Z
Learning: Applies to src/js/builtins/**/*.{cpp,cc,cxx} : Register builtin functions in C++ using object->putDirectBuiltinFunction(..., <name>CodeGenerator(vm), ...)

Applied to files:

  • src/bun.js/bindings/BunObject.cpp
📚 Learning: 2025-09-08T00:41:12.052Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/bun.js/bindings/v8/CLAUDE.md:0-0
Timestamp: 2025-09-08T00:41:12.052Z
Learning: Applies to src/bun.js/bindings/v8/**/*.cpp : Create V8 handles using isolate->currentHandleScope()->createLocal<T>() within an active handle scope

Applied to files:

  • src/bun.js/bindings/NodeTLS.cpp
🧬 Code graph analysis (3)
packages/bun-usockets/src/crypto/root_certs_header.h (1)
packages/bun-usockets/src/crypto/root_certs.cpp (2)
  • us_load_extra_ca_certs (217-239)
  • us_load_extra_ca_certs (217-217)
src/bun.js/bindings/BunObject.cpp (1)
packages/bun-usockets/src/crypto/root_certs.cpp (2)
  • us_load_extra_ca_certs (217-239)
  • us_load_extra_ca_certs (217-217)
packages/bun-usockets/src/crypto/root_certs.cpp (1)
packages/bun-usockets/src/crypto/root_certs_windows.cpp (2)
  • us_load_system_certificates_windows_raw (46-51)
  • us_load_system_certificates_windows_raw (46-47)
🔇 Additional comments (1)
packages/bun-usockets/src/crypto/root_certs_header.h (1)

6-12: Header shape LGTM.

C++-only mutex and C++ declarations for stack getters are fine; default store remains extern "C". No issues.

Comment thread docs/runtime/bun-apis.md
Comment on lines +82 to +86
- Cryptography
- `Bun.loadExtraCACerts`

---

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.

🧹 Nitpick | 🔵 Trivial

Add link/clarity for Bun.loadExtraCACerts.

Recommend linking to a section (e.g., “#bun-loadextracacerts”) and noting:

  • Input must be PEM; multiple certs allowed in one file.
  • Relative paths resolve against process.cwd().
  • Affects new TLS connections; existing handshakes are unaffected.
🤖 Prompt for AI Agents
In docs/runtime/bun-apis.md around lines 82 to 86, add a linked subsection
anchor for Bun.loadExtraCACerts (e.g., “#bun-loadextracacerts”) and expand the
brief cryptography line into a short description that states: the input must be
PEM format (multiple certificates can be concatenated in one file), relative
paths are resolved against process.cwd(), and the setting only affects new TLS
connections (existing handshakes remain unaffected). Ensure the new text
contains the anchor link and these three bullet points converted to a concise
prose or inline list.

Comment thread packages/bun-types/bun.d.ts
Comment thread packages/bun-usockets/src/crypto/root_certs.cpp
Comment thread src/bun.js/bindings/BunObject.cpp

@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: 2

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
packages/bun-usockets/src/crypto/root_certs.cpp (1)

241-287: Critical race condition and memory leak on store initialization.

The unconditional assignment store = X509_STORE_new() at line 242 creates multiple critical issues:

  1. Race condition: Concurrent calls to us_get_default_ca_store() from multiple threads can result in:

    • Multiple X509_STORE instances being created
    • Non-deterministic assignment to the global store variable
    • Inconsistent state between us_load_extra_ca_certs() and us_get_default_ca_store()
  2. Memory leak: Each call creates a new store and overwrites the global pointer without freeing the previous instance. If this function is called multiple times (which is possible since it's extern "C"), memory is leaked.

  3. Behavioral inconsistency: Repeated calls return different store instances, each potentially with different certificate sets, breaking the singleton pattern.

Solution: Use atomic initialization to ensure the store is created exactly once:

 extern "C" X509_STORE *us_get_default_ca_store() {
-  store = X509_STORE_new();
+  // Use static local with atomic initialization (C++11 guarantees thread-safety)
+  static std::atomic_flag store_initialized = ATOMIC_FLAG_INIT;
+  static bool initialization_done = false;
+  
+  if (!initialization_done) {
+    while (atomic_flag_test_and_set_explicit(&store_initialized,
+                                             std::memory_order_acquire))
+      ;
+    
+    if (!initialization_done) {
+      store = X509_STORE_new();
-  if (store == NULL) {
-    return NULL;
-  }
+      if (store == NULL) {
+        atomic_flag_clear_explicit(&store_initialized,
+                                   std::memory_order_release);
+        return NULL;
+      }

-  // Only load system default paths when NODE_USE_SYSTEM_CA=1
-  // Otherwise, rely on bundled certificates only (like Node.js behavior)
-  if (us_should_use_system_ca()) {
-    if (!X509_STORE_set_default_paths(store)) {
-      X509_STORE_free(store);
-      return NULL;
+      // Only load system default paths when NODE_USE_SYSTEM_CA=1
+      if (us_should_use_system_ca()) {
+        if (!X509_STORE_set_default_paths(store)) {
+          X509_STORE_free(store);
+          store = NULL;
+          atomic_flag_clear_explicit(&store_initialized,
+                                     std::memory_order_release);
+          return NULL;
+        }
+      }
+
+      us_default_ca_certificates *default_ca_certificates = us_get_default_ca_certificates();
+      X509** root_cert_instances = default_ca_certificates->root_cert_instances;
+      STACK_OF(X509) *root_extra_cert_instances = default_ca_certificates->root_extra_cert_instances;
+      STACK_OF(X509) *root_system_cert_instances = default_ca_certificates->root_system_cert_instances;
+
+      // load all root_cert_instances on the default ca store
+      for (size_t i = 0; i < root_certs_size; i++) {
+        X509 *cert = root_cert_instances[i];
+        if (cert == NULL)
+          continue;
+        X509_up_ref(cert);
+        X509_STORE_add_cert(store, cert);
+      }
+
+      if (root_extra_cert_instances) {
+        for (int i = 0; i < sk_X509_num(root_extra_cert_instances); i++) {
+          X509 *cert = sk_X509_value(root_extra_cert_instances, i);
+          X509_up_ref(cert);
+          X509_STORE_add_cert(store, cert);
+        }
+      }
+
+      if (us_should_use_system_ca() && root_system_cert_instances) {
+        for (int i = 0; i < sk_X509_num(root_system_cert_instances); i++) {
+          X509 *cert = sk_X509_value(root_system_cert_instances, i);
+          X509_up_ref(cert);
+          X509_STORE_add_cert(store, cert);
+        }
+      }
+      
+      initialization_done = true;
     }
+    
+    atomic_flag_clear_explicit(&store_initialized,
+                               std::memory_order_release);
   }
-
-  us_default_ca_certificates *default_ca_certificates = us_get_default_ca_certificates();
-  X509** root_cert_instances = default_ca_certificates->root_cert_instances;
-  STACK_OF(X509) *root_extra_cert_instances = default_ca_certificates->root_extra_cert_instances;
-  STACK_OF(X509) *root_system_cert_instances = default_ca_certificates->root_system_cert_instances;
-
-  // load all root_cert_instances on the default ca store
-  for (size_t i = 0; i < root_certs_size; i++) {
-    X509 *cert = root_cert_instances[i];
-    if (cert == NULL)
-      continue;
-    X509_up_ref(cert);
-    X509_STORE_add_cert(store, cert);
-  }
-
-  if (root_extra_cert_instances) {
-    for (int i = 0; i < sk_X509_num(root_extra_cert_instances); i++) {
-      X509 *cert = sk_X509_value(root_extra_cert_instances, i);
-      X509_up_ref(cert);
-      X509_STORE_add_cert(store, cert);
-    }
-  }
-
-  if (us_should_use_system_ca() && root_system_cert_instances) {
-    for (int i = 0; i < sk_X509_num(root_system_cert_instances); i++) {
-      X509 *cert = sk_X509_value(root_system_cert_instances, i);
-      X509_up_ref(cert);
-      X509_STORE_add_cert(store, cert);
-    }
-  }
-
+  
   return store;
 }

Alternative (simpler, using existing pattern): Reuse the double-checked locking pattern already used in us_internal_init_root_certs (lines 138-178):

 extern "C" X509_STORE *us_get_default_ca_store() {
+  static std::atomic_flag store_lock = ATOMIC_FLAG_INIT;
+  static std::atomic_bool store_initialized = 0;
+  
+  if (std::atomic_load(&store_initialized) == 1)
+    return store;
+  
+  while (atomic_flag_test_and_set_explicit(&store_lock,
+                                           std::memory_order_acquire))
+    ;
+  
+  if (!atomic_exchange(&store_initialized, 1)) {
-  store = X509_STORE_new();
+    store = X509_STORE_new();
     // ... rest of initialization as-is ...
+  }
+  
+  atomic_flag_clear_explicit(&store_lock, std::memory_order_release);
   return store;
 }

This ensures store is initialized exactly once and resolves both the race condition and memory leak.

♻️ Duplicate comments (2)
docs/runtime/bun-apis.md (1)

82-86: Link the API and add one‑line semantics (PEM, cwd, new connections).

Turn the table cell into a link (e.g., [...](/docs/api/tls#bun-loadextracacerts)) and note PEM file, relative to process.cwd(), and that it affects only new TLS connections.

- - `Bun.loadExtraCACerts`
+ - [`Bun.loadExtraCACerts`](https://bun.com/docs/api/tls#bun-loadextracacerts)
src/bun.js/bindings/BunObject.cpp (1)

571-587: Optional: validate argument is a non-empty string (UX consistency).

Current behavior converts any value to string and silently no-ops on invalid paths. If you prefer throwing TypeError for wrong types/empty strings, add guards before toWTFString(). Otherwise, keep as-is.

-    auto pathValue = callFrame->argument(0);
+    if (callFrame->argumentCount() < 1 || !callFrame->uncheckedArgument(0).isString()) {
+        throwTypeError(&globalObject, throwScope, "Bun.loadExtraCACerts(path): path must be a string"_s);
+        return {};
+    }
+    auto pathValue = callFrame->uncheckedArgument(0);
📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Disabled knowledge base sources:

  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 66cb337155707320ec867c671739217fe6233ed6 and bb33cf0.

📒 Files selected for processing (6)
  • docs/runtime/bun-apis.md (1 hunks)
  • packages/bun-types/bun.d.ts (1 hunks)
  • packages/bun-usockets/src/crypto/root_certs.cpp (5 hunks)
  • packages/bun-usockets/src/crypto/root_certs_header.h (1 hunks)
  • src/bun.js/bindings/BunObject.cpp (3 hunks)
  • src/bun.js/bindings/NodeTLS.cpp (1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{cpp,h}

📄 CodeRabbit inference engine (.cursor/rules/javascriptcore-class.mdc)

**/*.{cpp,h}: When exposing a JS class with public Constructor and Prototype, define three C++ types: class Foo : public JSC::DestructibleObject (if it has C++ fields), class FooPrototype : public JSC::JSNonFinalObject, and class FooConstructor : public JSC::InternalFunction
If the class has C++ data members, inherit from JSC::DestructibleObject and provide proper destruction; if it has no C++ fields (only JS properties), avoid a class and use JSC::constructEmptyObject(vm, structure) with putDirectOffset
Prefer placing the subspaceFor implementation in the .cpp file rather than the header when possible

Files:

  • packages/bun-usockets/src/crypto/root_certs_header.h
  • packages/bun-usockets/src/crypto/root_certs.cpp
  • src/bun.js/bindings/NodeTLS.cpp
  • src/bun.js/bindings/BunObject.cpp
**/*.cpp

📄 CodeRabbit inference engine (.cursor/rules/javascriptcore-class.mdc)

**/*.cpp: Include "root.h" at the top of C++ binding files to satisfy lints
Define prototype properties using a const HashTableValue array and declare accessors/functions with JSC_DECLARE_* macros
Prototype classes should subclass JSC::JSNonFinalObject, provide create/createStructure, DECLARE_INFO, finishCreation that reifies static properties, and set mayBePrototype on the Structure
Custom getters should use JSC_DEFINE_CUSTOM_GETTER, jsDynamicCast to validate this, and throwThisTypeError on mismatch
Custom setters should use JSC_DEFINE_CUSTOM_SETTER, validate this via jsDynamicCast, and store via WriteBarrier/set semantics
Prototype functions should use JSC_DEFINE_HOST_FUNCTION, validate this with jsDynamicCast, and return encoded JSValue
Constructors should subclass JSC::InternalFunction, return internalFunctionSpace in subspaceFor, set the prototype property as non-configurable/non-writable, and provide create/createStructure
Provide a setup function that builds the Prototype, Constructor, and Structure, and assigns them to the LazyClassStructure initializer
Use the cached Structure via globalObject->m_.get(globalObject) when constructing instances
Expose constructors to Zig via an extern "C" function that returns the constructor from the LazyClassStructure
Provide an extern "C" Bun____toJS function that creates an instance using the cached Structure and returns an EncodedJSValue

Files:

  • packages/bun-usockets/src/crypto/root_certs.cpp
  • src/bun.js/bindings/NodeTLS.cpp
  • src/bun.js/bindings/BunObject.cpp
src/bun.js/bindings/**/*.cpp

📄 CodeRabbit inference engine (CLAUDE.md)

src/bun.js/bindings/**/*.cpp: When implementing a JavaScript class with a public constructor in C++, create Foo, FooPrototype, and FooConstructor classes
Define class properties using HashTableValue arrays in C++ bindings
Add iso subspaces for classes with C++ fields in JavaScriptCore bindings

Files:

  • src/bun.js/bindings/NodeTLS.cpp
  • src/bun.js/bindings/BunObject.cpp
🧠 Learnings (29)
📓 Common learnings
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/bun.js/bindings/v8/CLAUDE.md:0-0
Timestamp: 2025-09-08T00:41:12.052Z
Learning: Applies to src/bun.js/bindings/v8/src/symbols.txt : Add new V8 API mangled symbols (without leading underscore) to src/symbols.txt
📚 Learning: 2025-10-04T09:52:49.414Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/js/CLAUDE.md:0-0
Timestamp: 2025-10-04T09:52:49.414Z
Learning: Applies to src/js/bun/**/*.{ts,js} : Place Bun-specific modules (e.g., bun:ffi, bun:sqlite) under bun/

Applied to files:

  • docs/runtime/bun-apis.md
  • packages/bun-types/bun.d.ts
📚 Learning: 2025-10-26T05:04:50.692Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-26T05:04:50.692Z
Learning: Applies to src/bun.js/bindings/**/*.cpp : Add iso subspaces for classes with C++ fields in JavaScriptCore bindings

Applied to files:

  • src/bun.js/bindings/NodeTLS.cpp
  • src/bun.js/bindings/BunObject.cpp
📚 Learning: 2025-09-08T00:41:12.052Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/bun.js/bindings/v8/CLAUDE.md:0-0
Timestamp: 2025-09-08T00:41:12.052Z
Learning: Applies to src/bun.js/bindings/v8/V8*.h : Ensure all public V8 API functions are marked with BUN_EXPORT for symbol visibility

Applied to files:

  • src/bun.js/bindings/BunObject.cpp
📚 Learning: 2025-09-08T00:41:12.052Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/bun.js/bindings/v8/CLAUDE.md:0-0
Timestamp: 2025-09-08T00:41:12.052Z
Learning: Applies to src/bun.js/bindings/v8/src/symbols.txt : Add new V8 API mangled symbols (without leading underscore) to src/symbols.txt

Applied to files:

  • src/bun.js/bindings/BunObject.cpp
  • packages/bun-types/bun.d.ts
📚 Learning: 2025-09-08T00:41:12.052Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/bun.js/bindings/v8/CLAUDE.md:0-0
Timestamp: 2025-09-08T00:41:12.052Z
Learning: Applies to src/bun.js/bindings/v8/V8*.h : Place new V8 API class headers in src/bun.js/bindings/v8/ named V8<Class>.h with declarations in namespace v8 and BUN_EXPORT on public methods

Applied to files:

  • src/bun.js/bindings/BunObject.cpp
📚 Learning: 2025-09-08T00:41:12.052Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/bun.js/bindings/v8/CLAUDE.md:0-0
Timestamp: 2025-09-08T00:41:12.052Z
Learning: Applies to src/bun.js/bindings/v8/test/v8/v8-module/main.cpp : Add a C++ test function in test/v8/v8-module/main.cpp and register it in Init using NODE_SET_METHOD

Applied to files:

  • src/bun.js/bindings/BunObject.cpp
📚 Learning: 2025-09-20T05:35:57.318Z
Learnt from: pfgithub
Repo: oven-sh/bun PR: 22534
File: src/bun.js/bindings/headers.h:729-731
Timestamp: 2025-09-20T05:35:57.318Z
Learning: symbols.txt in the Bun codebase is specifically for V8 API mangled symbols (without leading underscore), not for general Bun host functions declared with BUN_DECLARE_HOST_FUNCTION. Host functions are handled through different build mechanisms.

Applied to files:

  • src/bun.js/bindings/BunObject.cpp
📚 Learning: 2025-09-08T00:41:12.052Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/bun.js/bindings/v8/CLAUDE.md:0-0
Timestamp: 2025-09-08T00:41:12.052Z
Learning: Applies to src/bun.js/bindings/v8/src/symbols.dyn : Add new V8 API mangled symbols (with leading underscore and semicolons) to src/symbols.dyn

Applied to files:

  • src/bun.js/bindings/BunObject.cpp
📚 Learning: 2025-08-30T00:13:36.815Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: .cursor/rules/zig-javascriptcore-classes.mdc:0-0
Timestamp: 2025-08-30T00:13:36.815Z
Learning: Applies to src/bun.js/**/*.zig : Use parameter name globalObject (not ctx) and accept (*JSC.JSGlobalObject, *JSC.CallFrame) in binding methods/constructors

Applied to files:

  • src/bun.js/bindings/BunObject.cpp
📚 Learning: 2025-08-30T00:13:36.815Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: .cursor/rules/zig-javascriptcore-classes.mdc:0-0
Timestamp: 2025-08-30T00:13:36.815Z
Learning: Applies to src/bun.js/bindings/generated_classes_list.zig : Update src/bun.js/bindings/generated_classes_list.zig to include new classes

Applied to files:

  • src/bun.js/bindings/BunObject.cpp
  • packages/bun-types/bun.d.ts
📚 Learning: 2025-10-26T05:04:50.692Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-26T05:04:50.692Z
Learning: Applies to src/bun.js/bindings/**/*.cpp : Define class properties using HashTableValue arrays in C++ bindings

Applied to files:

  • src/bun.js/bindings/BunObject.cpp
📚 Learning: 2025-09-12T18:16:50.754Z
Learnt from: RiskyMH
Repo: oven-sh/bun PR: 22606
File: src/glob/GlobWalker.zig:449-452
Timestamp: 2025-09-12T18:16:50.754Z
Learning: For Bun codebase: prefer using `std.fs.path.sep` over manual platform separator detection, and use `bun.strings.lastIndexOfChar` instead of `std.mem.lastIndexOfScalar` for string operations.

Applied to files:

  • src/bun.js/bindings/BunObject.cpp
  • packages/bun-types/bun.d.ts
📚 Learning: 2025-08-30T00:13:36.815Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: .cursor/rules/zig-javascriptcore-classes.mdc:0-0
Timestamp: 2025-08-30T00:13:36.815Z
Learning: Applies to src/bun.js/**/*.zig : Access JS call data via CallFrame (argument(i), argumentCount(), thisValue()) and throw errors with globalObject.throw(...)

Applied to files:

  • src/bun.js/bindings/BunObject.cpp
📚 Learning: 2025-09-08T00:41:12.052Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/bun.js/bindings/v8/CLAUDE.md:0-0
Timestamp: 2025-09-08T00:41:12.052Z
Learning: Applies to src/bun.js/bindings/v8/**/*.{h,cpp} : Use localToJSValue() when converting V8 handles to JSC values before performing JSC operations

Applied to files:

  • src/bun.js/bindings/BunObject.cpp
📚 Learning: 2025-10-01T21:49:27.862Z
Learnt from: taylordotfish
Repo: oven-sh/bun PR: 23169
File: src/bun.js/bindings/BunIDLConvert.h:29-42
Timestamp: 2025-10-01T21:49:27.862Z
Learning: In Bun's IDL bindings (src/bun.js/bindings/BunIDLConvert.h), IDLStrictNull intentionally treats both undefined and null as null (using isUndefinedOrNull()), matching WebKit's IDLNull & IDLNullable behavior. This is the correct implementation and should not be changed to only accept null.

Applied to files:

  • src/bun.js/bindings/BunObject.cpp
📚 Learning: 2025-08-30T00:13:36.815Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: .cursor/rules/zig-javascriptcore-classes.mdc:0-0
Timestamp: 2025-08-30T00:13:36.815Z
Learning: Applies to src/bun.js/**/*.zig : Constructors and prototype methods should return bun.JSError!JSC.JSValue to integrate Zig error handling with JS exceptions

Applied to files:

  • src/bun.js/bindings/BunObject.cpp
📚 Learning: 2025-08-30T00:11:00.890Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: .cursor/rules/javascriptcore-class.mdc:0-0
Timestamp: 2025-08-30T00:11:00.890Z
Learning: Applies to **/*.cpp : Prototype functions should use JSC_DEFINE_HOST_FUNCTION, validate this with jsDynamicCast, and return encoded JSValue

Applied to files:

  • src/bun.js/bindings/BunObject.cpp
📚 Learning: 2025-10-08T13:56:00.875Z
Learnt from: Jarred-Sumner
Repo: oven-sh/bun PR: 23373
File: src/bun.js/api/BunObject.zig:2514-2521
Timestamp: 2025-10-08T13:56:00.875Z
Learning: For Bun codebase: prefer using `bun.path` utilities (e.g., `bun.path.joinAbsStringBuf`, `bun.path.join`) over `std.fs.path` functions for path operations.

Applied to files:

  • src/bun.js/bindings/BunObject.cpp
📚 Learning: 2025-10-01T21:59:54.571Z
Learnt from: taylordotfish
Repo: oven-sh/bun PR: 23169
File: src/bun.js/bindings/webcore/JSDOMConvertEnumeration.h:47-74
Timestamp: 2025-10-01T21:59:54.571Z
Learning: In the new bindings generator (bindgenv2) for `src/bun.js/bindings/webcore/JSDOMConvertEnumeration.h`, the context-aware enumeration conversion overloads intentionally use stricter validation (requiring `value.isString()` without ToString coercion), diverging from Web IDL semantics. This is a design decision documented in comments.

Applied to files:

  • src/bun.js/bindings/BunObject.cpp
📚 Learning: 2025-08-30T00:11:57.076Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: .cursor/rules/registering-bun-modules.mdc:0-0
Timestamp: 2025-08-30T00:11:57.076Z
Learning: Applies to src/{**/js_*.zig,bun.js/api/**/*.zig} : Use bun.JSError!JSValue for proper error propagation in JS-exposed Zig functions

Applied to files:

  • src/bun.js/bindings/BunObject.cpp
📚 Learning: 2025-08-30T00:13:36.815Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: .cursor/rules/zig-javascriptcore-classes.mdc:0-0
Timestamp: 2025-08-30T00:13:36.815Z
Learning: Applies to src/bun.js/**/*.zig : In Zig binding structs, expose generated bindings via pub const js = JSC.Codegen.JS<ClassName> and re-export toJS/fromJS/fromJSDirect

Applied to files:

  • src/bun.js/bindings/BunObject.cpp
📚 Learning: 2025-10-18T20:50:47.750Z
Learnt from: theshadow27
Repo: oven-sh/bun PR: 23798
File: src/bun.js/telemetry.zig:366-373
Timestamp: 2025-10-18T20:50:47.750Z
Learning: In Bun's Zig codebase (src/bun.js/bindings/JSValue.zig), the JSValue enum uses `.null` (not `.js_null`) for JavaScript's null value. Only `js_undefined` has the `js_` prefix to avoid collision with Zig's built-in `undefined` keyword. The correct enum fields are: `js_undefined`, `null`, `true`, `false`, and `zero`.

Applied to files:

  • src/bun.js/bindings/BunObject.cpp
📚 Learning: 2025-08-30T00:11:00.890Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: .cursor/rules/javascriptcore-class.mdc:0-0
Timestamp: 2025-08-30T00:11:00.890Z
Learning: Applies to **/*.cpp : Provide an extern "C" Bun__<Type>__toJS function that creates an instance using the cached Structure and returns an EncodedJSValue

Applied to files:

  • src/bun.js/bindings/BunObject.cpp
📚 Learning: 2025-08-30T00:11:00.890Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: .cursor/rules/javascriptcore-class.mdc:0-0
Timestamp: 2025-08-30T00:11:00.890Z
Learning: Applies to **/*.zig : Wrap the Bun__<Type>__toJS extern in a Zig method that takes a JSGlobalObject and returns JSC.JSValue

Applied to files:

  • src/bun.js/bindings/BunObject.cpp
📚 Learning: 2025-10-04T09:52:49.414Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/js/CLAUDE.md:0-0
Timestamp: 2025-10-04T09:52:49.414Z
Learning: Applies to src/js/builtins/**/*.{cpp,cc,cxx} : Register builtin functions in C++ using object->putDirectBuiltinFunction(..., <name>CodeGenerator(vm), ...)

Applied to files:

  • src/bun.js/bindings/BunObject.cpp
📚 Learning: 2025-10-19T02:52:37.412Z
Learnt from: theshadow27
Repo: oven-sh/bun PR: 23798
File: packages/bun-otel/tsconfig.json:1-15
Timestamp: 2025-10-19T02:52:37.412Z
Learning: In the Bun repository, packages under packages/ (e.g., bun-otel) can follow a TypeScript-first pattern where package.json exports point directly to .ts files (not compiled .js files). Bun natively runs TypeScript, so consumers import .ts sources directly and receive full type information without needing compiled .d.ts declaration files. For such packages, adding "declaration": true or "outDir" in tsconfig.json is unnecessary and would break the export structure.
<!-- [remove_learning]
ceedde95-980e-4898-a2c6-40ff73913664

Applied to files:

  • packages/bun-types/bun.d.ts
📚 Learning: 2025-08-30T00:12:56.803Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: .cursor/rules/writing-tests.mdc:0-0
Timestamp: 2025-08-30T00:12:56.803Z
Learning: Applies to test/js/bun/**/*.{js,ts} : Place Bun API tests under test/js/bun/, separated by category (e.g., test/js/bun/glob/)

Applied to files:

  • packages/bun-types/bun.d.ts
📚 Learning: 2025-10-18T01:49:31.037Z
Learnt from: taylordotfish
Repo: oven-sh/bun PR: 23755
File: src/bun.js/api/bun/socket/SocketConfig.bindv2.ts:58-58
Timestamp: 2025-10-18T01:49:31.037Z
Learning: In Bun's bindgenv2 TypeScript bindings (e.g., src/bun.js/api/bun/socket/SocketConfig.bindv2.ts), the pattern `b.String.loose.nullable.loose` is intentional and not a duplicate. The first `.loose` applies to the String type (loose string conversion), while the second `.loose` applies to the nullable (loose nullable, treating all falsy values as null rather than just null/undefined).

Applied to files:

  • packages/bun-types/bun.d.ts
🧬 Code graph analysis (3)
packages/bun-usockets/src/crypto/root_certs_header.h (1)
packages/bun-usockets/src/crypto/root_certs.cpp (2)
  • us_load_extra_ca_certs (217-239)
  • us_load_extra_ca_certs (217-217)
packages/bun-usockets/src/crypto/root_certs.cpp (1)
packages/bun-usockets/src/crypto/root_certs_windows.cpp (2)
  • us_load_system_certificates_windows_raw (46-51)
  • us_load_system_certificates_windows_raw (46-47)
src/bun.js/bindings/BunObject.cpp (1)
packages/bun-usockets/src/crypto/root_certs.cpp (2)
  • us_load_extra_ca_certs (217-239)
  • us_load_extra_ca_certs (217-217)
🔇 Additional comments (5)
packages/bun-usockets/src/crypto/root_certs_header.h (1)

6-11: Confirm linkage: C++-only or C‑linkable?

us_load_extra_ca_certs is declared only in the C++ section without CPPDECL/extern "C". If you intend this to be callable from Zig/C later (like other us_* APIs), expose it via CPPDECL and match the definition. If C++‑only is intentional, consider adding a comment to make that clear to avoid accidental ABI mismatches.

src/bun.js/bindings/NodeTLS.cpp (1)

43-47: LGTM: thread-safe access to extra CA stack.

Scoped unique_lock correctly guards reads of root_extra_cert_instances.

packages/bun-types/bun.d.ts (1)

959-972: No duplicate declarations found.

Verification confirms loadExtraCACerts is declared only once in the file (line 972). The other occurrence on line 969 is a code example within JSDoc comments, not a declaration.

src/bun.js/bindings/BunObject.cpp (1)

44-44: Include path appears off by one (breaks build in this directory).

Other bindings include the usockets header with ../../packages/.... From src/bun.js/bindings/, triple .. escapes the repo root. Align with NodeTLS.

-#include "../../../packages/bun-usockets/src/crypto/root_certs_header.h"
+#include "../../packages/bun-usockets/src/crypto/root_certs_header.h"
⛔ Skipped due to learnings
Learnt from: RiskyMH
Repo: oven-sh/bun PR: 22606
File: src/glob/GlobWalker.zig:449-452
Timestamp: 2025-09-12T18:16:50.754Z
Learning: For Bun codebase: prefer using `std.fs.path.sep` over manual platform separator detection, and use `bun.strings.lastIndexOfChar` instead of `std.mem.lastIndexOfScalar` for string operations.
Learnt from: Jarred-Sumner
Repo: oven-sh/bun PR: 23373
File: src/bun.js/api/BunObject.zig:2514-2521
Timestamp: 2025-10-08T13:56:00.875Z
Learning: For Bun codebase: prefer using `bun.path` utilities (e.g., `bun.path.joinAbsStringBuf`, `bun.path.join`) over `std.fs.path` functions for path operations.
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/bun.js/bindings/v8/CLAUDE.md:0-0
Timestamp: 2025-09-08T00:41:12.052Z
Learning: Applies to src/bun.js/bindings/v8/V8*.h : Ensure all public V8 API functions are marked with BUN_EXPORT for symbol visibility
Learnt from: taylordotfish
Repo: oven-sh/bun PR: 23169
File: src/bun.js/bindings/Bindgen/IDLTypes.h:1-3
Timestamp: 2025-10-01T21:48:38.278Z
Learning: In the Bun codebase, for `BunIDL*` and `Bindgen*` headers (e.g., BunIDLTypes.h, Bindgen/IDLTypes.h), it's acceptable to rely on transitive includes for standard library headers like <type_traits> and <utility> rather than including them explicitly.
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/js/CLAUDE.md:0-0
Timestamp: 2025-10-04T09:52:49.414Z
Learning: Applies to src/js/bun/**/*.{ts,js} : Place Bun-specific modules (e.g., bun:ffi, bun:sqlite) under bun/
Learnt from: taylordotfish
Repo: oven-sh/bun PR: 23755
File: src/bun.js/api/bun/socket/SocketConfig.bindv2.ts:58-58
Timestamp: 2025-10-18T01:49:31.037Z
Learning: In Bun's bindgenv2 TypeScript bindings (e.g., src/bun.js/api/bun/socket/SocketConfig.bindv2.ts), the pattern `b.String.loose.nullable.loose` is intentional and not a duplicate. The first `.loose` applies to the String type (loose string conversion), while the second `.loose` applies to the nullable (loose nullable, treating all falsy values as null rather than just null/undefined).
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/bun.js/bindings/v8/CLAUDE.md:0-0
Timestamp: 2025-09-08T00:41:12.052Z
Learning: Applies to src/bun.js/bindings/v8/src/symbols.txt : Add new V8 API mangled symbols (without leading underscore) to src/symbols.txt
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-26T05:04:50.692Z
Learning: Applies to src/bun.js/bindings/**/*.cpp : Add iso subspaces for classes with C++ fields in JavaScriptCore bindings
packages/bun-usockets/src/crypto/root_certs.cpp (1)

201-202: LGTM: Mutex declaration is correct.

The global std::mutex is properly default-constructed at program initialization and used correctly with std::unique_lock in us_load_extra_ca_certs.

Comment thread packages/bun-types/bun.d.ts
Comment thread packages/bun-usockets/src/crypto/root_certs.cpp
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Ability to set NODE_EXTRA_CA_CERTS in standalone executable

1 participant