Bun global + crypto: remove unsafe from BunObject.rs and runtime/crypto

[Jarred-Sumner]

What

Same programme as #40055 … #40385. Stacked on #40204 (base branch claude/subprocess-zero-unsafe) and merges #40214's claude/server-zero-unsafe (needed for NewServer::init returning RefPtr / AnyServer(BackRef) under Bun.serve); retarget to main once those land.

src/runtime/api/BunObject.rs 35 → 0; src/runtime/crypto/: CryptoHasher.rs 16 → 0, HMAC.rs 12 → 0, EVP.rs 9 → 0, boringssl_jsc.rs/pwhash.rs/PasswordObject.rs/PBKDF2.rs → 0. (Terminal.rs was already at 0 via #40204.)

• crypto: new bun_boringssl_sys::digest — DigestCtx (owned EVP_MD_CTX with a digest always installed; final_ refuses short buffers), HmacCtx, digest/digest_by_name/md_size/pbkdf2_hmac; EVP_*() getters return &'static EVP_MD; engines are Option<&ENGINE>; err_{lib,func,reason}_error_string return Option<&CStr>/Cow (owned copy for ERR_LIB_SYS, whose text is strerror()). bun_sha_hmac::SHA*::hash is safe (callers in install/sql/exe_format/s3_signing updated).
• BunObject: every BunObject_callback_*/BunObject_lazyPropCb_*, Bun__CryptoHasherExtern__*, JSPasswordObject__create, Bun__reportError are HOST_EXPORTs (generator learned inline mod {} markers and lifetime-only generics); --hot server registry is a typed HotServers map in RareData (servers unregister in stop()/teardown()); bun_sys::MappedFile (owning mmap) + ArrayBuffer::create_uint8_array_from_vec/uint8_array_from_mapped_file; Resolver::custom_dir_paths, EditorContext.name owned; StandaloneModuleGraph::embedded_blob(&self, ..) replaces the per-process cached Blob* created with the first caller's global; Bun__getEnvCount/Key take the global.

Pre-existing issues noticed: EVP::hash/final passed min(len, size) as if it bounded the write (out-param; now guarded); the StandaloneModuleGraph first-global Blob* cache (replaced); crypto.createHmac("blake2b512") throws "Invalid digest" in Bun though Node supports it (unchanged).

Perf (release, taskset -c 56-63, perf stat -e instructions:u, interleaved ×8, medians): CryptoHasher("sha256").update(1KB) ×1e5 +0.26 %; Bun.hash(1KB) ×1e6 +0.0 %; crypto.createHmac ×1e5 −0.04 %. (First cut was +0.42 % from a 264-byte move in DigestCtx::new; fixed by constructing in place.)

After rebasing over #40511: PBKDF2.rs shows 2 (unsafe impl ThreadIsolatedArg + ThreadIsolated::new, main's work-pool argument API); everything else in runtime/crypto/ and BunObject.rs stays at 0.

Testing

Debug+ASAN, --timeout 60000: terminal (98), terminal-spawn (16), terminal-platform-gaps (19), inspect (80), mmap (21), password (76), sleep/sleepSync/which, bun-cryptohasher (403), hash (20), cipheriv, x25519, web-crypto (95), web-crypto-sha3, node-crypto (212), crypto.hmac (74), crypto.test (403), pbkdf2 (51), scrypt, lazyhash, oneshot, hmac-algorithm, argon2, extra-memory, zstd (92), zlib (388), open-in-editor-gc, require, module-resolve-filename-paths, node-module-module, cli/hot (12), compile-asset-bunfs (9), regression/31575 — all pass (process.test getgroups and serve.test port-1003/react-dom cases are root/sandbox environment). Hand-driven: Terminal spawn/write/resize/close, close-while-alive, child-exit with pending reads, 50 terminals + GC; every CryptoHasher algorithm + HMAC + pbkdf2 byte-identical to system bun and node; Bun.password ×20 concurrent + GC; a Worker hashing terminated ×5 — exit 0, no ASAN output. clippy clean on all touched crates; rust-check-all windows-msvc + aarch64-darwin pass.

[robobun]

^{Updated 11:23 AM PT - Sep 8th, 2026}

❌ @Jarred-Sumner, your commit 1665544 has 1 failures in Build #113066 (All Failures):

• test/js/web/fetch/fetch-backpressure.test.ts - code 1 on 🪟 11 aarch64

---

🧪   To try this PR locally:

bunx bun-pr 40399

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

bun-40399 --bun

[claude]

Thanks for the fix in d585edb — the inline-mod closing-brace matcher now handles trailing comments. This automated pass found no further issues.

Given the scope (59 files; server lifecycle moved to RefPtr/self_ref/teardown(), WeakPtr gaining Drop+Clone, RequestContext self-owning its pool slot, NodeHTTPResponse::create moved Rust-side with a changed C++ ABI, the new HotServers registry) and that it's stacked on #40204/#40214, a human pass is still warranted before merge.

What was reviewed: the new DigestCtx/HmacCtx bounds-check output before EVP_DigestFinal_ex/HMAC_Final (fixes the pre-existing min(len, size) out-param footgun the description notes); create_uint8_array_from_vec — non-empty branch hands the mimalloc allocation to MarkedArrayBuffer_deallocator, empty branch drops before returning; err_reason_error_string copies the strerror buffer for ERR_LIB_SYS before returning; the serve_with! error paths now server.deref() on both post-init exception exits.

Extended reasoning...

Overview

This PR continues the zero-unsafe programme across BunObject.rs (35→0) and runtime/crypto/ (all files → 0). It introduces owned BoringSSL context wrappers (DigestCtx, HmacCtx) in a new bun_boringssl_sys::digest module, converts EVP_*() getters to return &'static EVP_MD, and threads Option<&ENGINE> everywhere raw engine pointers used to flow. On the BunObject side, every Bun.* callback/lazy-property is migrated to the HOST_EXPORT codegen (which learned inline-mod tracking and lifetime-only generics), and several ownership models are reworked: Resolver::custom_dir_paths becomes Option<Box<[BunString]>>, EditorContext.name becomes Vec<u8>, mmap gets an owning MappedFile handed to a new uint8_array_from_mapped_file, and the --hot server registry becomes a typed HotServers map stored via dyn Any in RareData.

The largest surface area is the runtime/server/ rework carried in from #40214: NewServer becomes CellRefCounted with a self_ref held while the uWS app exists; init() returns RefPtr<Self>; listen()/teardown() replace the raw-pointer + deinit() model; RequestContext stores its own Pooled slot token so deinit drops it rather than calling back through the server; WeakPtr gains Drop/Clone and interior-mutable WeakPtrData; AnyServer is rewritten as a BackRef-based enum; and NodeHTTPResponse::create moves from C++ to Rust with a corresponding signature change to NodeHTTPServer__onRequest_{http,https} (drops any_server/upgrade_ctx/out-param, adds nodeResponseObject/hasBody).

Security risks

Crypto changes are wrapper-shaped — the underlying BoringSSL calls are unchanged, and the new DigestCtx::final_/HmacCtx::final_/digest() add explicit output-length guards that the old code lacked. The err_reason_error_string change correctly copies the strerror() buffer for ERR_LIB_SYS rather than returning a 'static borrow of thread-mutable storage. No new untrusted-input parsing. The mmap deallocator's page-rounding logic is preserved from the old inline version. I did not identify security-relevant regressions.

Level of scrutiny

High. The server-lifecycle refcounting rework is the kind of change where a missed deref() on one exit path leaks a server for the process lifetime, and an extra one is a UAF. The WeakPtr::drop addition means every existing WeakPtr field that was previously manually deref()ed now needs auditing for double-release (I could not verify all sites in the truncated diff). The NodeHTTP.cpp ABI change must be matched exactly on both sides. The RequestContext::pool_slot self-ownership pattern is subtle. This is production-critical hot-path code (Bun.serve, node:http, every hash) and the diff is ~623 KB across 59 files.

Other factors

• Stacked: base is claude/subprocess-zero-unsafe (#40204) and merges #40214; the description says to retarget once those land, so this cannot merge as-is regardless.
• Prior feedback addressed: my earlier nit on generate-host-exports.ts was fixed in d585edb exactly as suggested.
• Testing: the description lists extensive debug+ASAN test coverage and byte-identical crypto verification against Node/system-bun, plus perf measurements showing negligible instruction-count delta.
• No bugs found: the multi-agent bug hunt surfaced nothing this run; the candidate concerns I checked (Vec→Uint8Array ownership transfer, digest output-buffer sizing, ERR_LIB_SYS strerror lifetime, serve_with! error-path ref balance) all held up.

Given the scale, the lifecycle/refcount rework, and the stacked-PR status, this needs a maintainer's eyes.

[claude]

I reviewed this PR and didn't find any bugs this pass; both earlier nits (inline-mod brace matcher, stale cached_blob comments) are addressed. Given the scope — 59 files including the NewServer ref-count/lifecycle rework, RequestContext pool-slot ownership, the WeakPtr Drop/Clone addition, and the NodeHTTPServer__onRequest_* ABI change — plus the fact that this is stacked on #40204/#40214, a human look is still worthwhile.

Checked: DigestCtx/HmacCtx final_ short-buffer guards vs. all callers; create_uint8_array_from_vec empty-Vec path and mi_free deallocator pairing; NodeHTTPResponse::create's three initial refs vs. the sync/async release paths in on_node_http_request_with_upgrade_ctx; err_reason_error_string's ERR_LIB_SYS owned-copy for strerror.

Extended reasoning...

Overview

This PR continues the zero-unsafe programme across BunObject.rs (35→0) and runtime/crypto/ (all → 0). It introduces safe wrappers in bun_boringssl_sys::digest (DigestCtx, HmacCtx, one-shot digest/pbkdf2_hmac), converts EVP_*() getters to &'static EVP_MD, adds ArrayBuffer::create_uint8_array_from_vec/uint8_array_from_mapped_file and bun_sys::MappedFile, and moves every BunObject_callback_*/_lazyPropCb_* export onto the HOST_EXPORT codegen path (which learned inline-mod tracking and lifetime-only generics). It also carries the merged server-zero-unsafe work: NewServer becomes CellRefCounted with JsCell/Cell fields, init() returns RefPtr<Self>, listen()/teardown() manage a self_ref, request contexts own their pool slot via Pooled<'static, ..>, AnyServer becomes a BackRef-based enum, WeakPtr gains Cell-based interior mutability plus Drop/Clone, and NodeHTTPResponse creation moves from C++ to Rust with a changed NodeHTTPServer__onRequest_{http,https} signature.

Security risks

No new attack surface. The crypto changes are wrapper-level (BoringSSL calls unchanged) with added bounds checks in DigestCtx::final_/HmacCtx::final_ that fix a pre-existing footgun where EVP::hash/final treated the out-length as an in-bound. The err_reason_error_string change correctly copies strerror() output (ERR_LIB_SYS) rather than returning a 'static borrow of thread-local storage.

Level of scrutiny

High. The server portion rewrites ownership and teardown ordering for NewServer, RequestContext, NodeHTTPResponse, and ServePlugins — all ref-counted, all with GC-visible wrappers, several with re-entrant JS callbacks in their release paths. WeakPtr gaining Drop changes the semantics for every existing holder. The NodeHTTP.cpp ABI change must stay in lockstep with the Rust extern decls. These are exactly the areas REVIEW.md flags as most-blocked (paired acquisition/release, ref-count balance on every terminal path, re-entrance under app.close()).

Other factors

• Stacked on #40204 and merges #40214, so a substantial fraction of the server diff is inherited and presumably reviewed there; the diff shown here is against claude/subprocess-zero-unsafe, not main.
• Extensive test coverage reported (debug+ASAN across crypto, mmap, zlib/zstd, hot-reload, compile-asset), and rust-check-all on Windows/macOS.
• Both earlier automated-review nits were fixed in follow-up commits and the threads resolved.
• The diff is truncated at ~625K chars; server_body.rs, uws_sys/, webcore/{Request,Response,Blob}.rs, and the AnyServer dispatch rewrite were not fully visible in the preloaded context, which is another reason human review is appropriate.

[claude]

All four points from the last pass are addressed in c5c2e8b — unregister_hot now identity-checks before evicting (with a --hot regression test), create_uint8_array_from_vec uses as_mut_ptr(), arm_on_data reads self.as_ctx_ptr() on both H1 and H3 paths, and the shutdown-gate comment is corrected. Nothing further from this run; given the scope (server core + BoringSSL digest/HMAC wrappers, ~5.5k lines churned), a maintainer pass is still warranted.

What was reviewed: the unregister_hot fix (index lookup + AnyServer PartialEq compare, swap_remove_at only on match); the new hot.test.ts case (bounded WeakRef poll, stdout asserted before exitCode, no fixed sleeps); array_buffer.rs provenance change (mut binding, len read before pointer); arm_on_data(&self) now symmetric with prepare_js_request_context's as_ctx_ptr().

Extended reasoning...

Overview

This PR is a broad zero-unsafe refactor spanning src/runtime/server/ (mod.rs + server_body.rs, ~5.5k lines), the crypto stack (CryptoHasher, EVP, HMAC, PBKDF2, PasswordObject, pwhash, sha_hmac, plus downstream users in s3/mysql/postgres/install), BunObject.rs (all callbacks/lazy-props moved to HOST_EXPORT, typed --hot registry, owned mmap/env/editor state), new bun_boringssl_sys::digest safe wrappers, and supporting changes across uws_sys, jsc, ptr, sys, resolver, and codegen. The latest commit (c5c2e8b) responds to the four inline findings from this bot's previous pass.

Security risks

The change rewrites the FFI surface for BoringSSL digest/HMAC/PBKDF2 primitives and touches password hashing, S3 request signing, MySQL auth, and Postgres SASL. The PR description reports byte-identical output vs. system bun and Node across all CryptoHasher/HMAC/pbkdf2 algorithms, and the wrappers are thin owned-context RAII over the same EVP_* calls, but any regression here is security-relevant. The server refactor changes how request-context pointers are threaded through uWS callbacks — a provenance/lifetime bug there is a UAF vector. No new attack surface is introduced; this is a safety-hardening refactor of existing surface.

Level of scrutiny

High. Per REVIEW.md, memory-safety refactors are the most-blocked category, and this one simultaneously touches crypto FFI, request-context lifetime plumbing, refcount/weak-handle semantics, and a codegen script. The PR is stacked on two other zero-unsafe branches, so the effective diff against main is even larger. This is well outside the "simple, mechanical, or obvious" bar for bot approval.

Other factors

This bot has reviewed three prior pushes; every finding raised (codegen brace matcher, stale cached_blob comments, dead AnyServerTag, custom_dir_paths clone, the unregister_hot eviction race, two provenance nits, and a misleading comment) was addressed within hours, with the eviction race gaining a dedicated regression test. The current run found nothing new and ruled out nothing new beyond verifying those fixes. The test additions follow harness conventions (bounded polling on a WeakRef, Promise.all pipe drain, stdout/stderr asserted before exit code). Deferring so a maintainer signs off on the crypto-wrapper and server-lifetime design; nothing outstanding from the automated side.

[claude]

Code review found no issues

No high-confidence issues detected in this change.

[claude]

Nothing blocking. The comments below are optional suggestions. There is no need to push a fix for them before merging.

Additional findings (outside the current diff — GitHub can't attach inline comments there):

• 🟡 src/runtime/server/NodeHTTPResponse.rs — nit: comment on bun_vm_mut still cites "NodeHTTPResponse__createForJS disasm" as the perf rationale, but this PR deletes that extern (Rust no_mangle fn removed here; C++ decl removed from NodeHTTP.cpp) and replaces it with NodeHTTPResponse::create. Fix: update the comment to name NodeHTTPResponse::create (or drop the disasm citation) so a reader auditing the hot-path claim isn't sent to a symbol that no longer exists.

  Extended reasoning...

  NodeHTTPResponse__createForJS was removed on both sides in this diff: the #[unsafe(no_mangle)] extern "C" fn NodeHTTPResponse__createForJS block in NodeHTTPResponse.rs is deleted and replaced by an impl NodeHTTPResponse { pub(crate) fn create(...) } associated fn, and extern "C" EncodedJSValue NodeHTTPResponse__createForJS(...) plus its call site are deleted from src/jsc/bindings/NodeHTTP.cpp. Grep for the symbol now returns only the comment at NodeHTTPResponse.rs:260 ("NodeHTTPResponse__createForJS disasm showed the bunVM FFI result dropped on the floor"). Doc-accuracy only; no runtime effect. Per REVIEW.md "Delete dead code in the same PR that makes it dead" and CLAUDE.md's comment-maintenance rule, references to removed identifiers should be updated in the same PR.

  Verification: nit — The comment at src/runtime/server/NodeHTTPResponse.rs:260 still reads "NodeHTTPResponse__createForJS disasm showed the bunVM FFI result dropped on the floor", but this PR deletes that symbol on both sides: the Rust #[unsafe(no_mangle)] pub(crate) unsafe extern "C" fn NodeHTTPResponse__createForJS(...) block is removed and replaced by `impl NodeHTTPResponse { pub(crate) fn… | nit —…

[Jarred-Sumner]

Updated the bun_vm_mut comment to name NodeHTTPResponse::create (a2abe1b here, same change on #40214 where the extern was removed).

[claude]

Code review found no issues

No high-confidence issues detected in this change.

[claude]

Code review found no issues

No high-confidence issues detected in this change.

[claude]

This pull request has been reviewed before and this review found new issues. Where they share a root cause, one fix may close them together.

Findings marked 🟡 are optional suggestions and need no follow-up push.

[claude]

No new blocking issues. 1 optional suggestion (a nit or a note on pre-existing code) was found and not posted.