Skip to content

node:crypto: accept key data in crypto.diffieHellman, split key prepare from import - #45335

Closed
robobun wants to merge 1 commit into
mainfrom
robobun/1899ee18/dh-stateless-key-data
Closed

robobun wants to merge 1 commit into
mainfrom
robobun/1899ee18/dh-stateless-key-data

Conversation

@robobun

@robobun robobun commented Oct 15, 2026 •

Copy link
Copy Markdown
Collaborator

Problem

  • crypto.diffieHellman() accepts only KeyObjects. Key data throws ERR_INVALID_ARG_VALUE: The property 'options.privateKey' is invalid. Node accepts it since v26.1.0 (crypto: accept key data in crypto.diffieHellman() and cleanup DH jobs nodejs/node#62527).
  • Cause: DhJobCtx::fromJS (src/jsc/bindings/node/crypto/CryptoDhJob.cpp:80) downcasts both to JSKeyObject.
  • KeyObject::prepareAsymmetricKey hardcodes the name key and returns a pointer that a second key's getter can invalidate.

Fix

  • prepareAsymmetricKey(value, mode, name) only validates and returns the owner of the key bytes. importPreparedKey reads them. diffieHellman prepares both keys, then imports both, like Node.
  • The 7 other sites call getAsymmetricKeyFromJS (both steps). Sign#sign, privateEncrypt and privateDecrypt pass Node's name privateKey.
  • The EC JWK import checks for an exception after it decodes d: a vm timeout there crashed, on main too.
  • Verified: test/js/node/crypto/node-crypto.test.js, crypto.key-objects.test.ts (74 fail on main), upstream test-crypto-dh-stateless.js, 159 vendored crypto tests. Self-reviewed: 6 concerns raised, 3 addressed.

Background

  • Key data is a key that is not a KeyObject: PEM text or { key, format }.
  • Node validates both keys before it reads either, and a getter on one key can change the other.
  • Considered copying the two-step block into DhJobCtx::fromJS: errors would say key.key, Node's test requires options.privateKey.key.

Downsides

  • Error text changes at 10 APIs (options.format becomes key.format). Incompatible KeyObjects throw ERR_CRYPTO_INCOMPATIBLE_KEY, a failed derive ERR_OSSL_EVP_*.
  • Release text grows 1,872 bytes. With KeyObjects: diffieHellman 3 to 2 mi_malloc, 2,076 to 2,007 instructions. createPublicKey 802 to 803.
  • 68 of 250 probed diffieHellman results differ from Node (230 on main): BoringSSL codes, JWK and raw import text.
Notes

Reproduction (Node v26.3.0 prints ok five times, Bun 1.4.3 prints ok once):

import crypto from "node:crypto";
const a = crypto.generateKeyPairSync("x25519");
const b = crypto.generateKeyPairSync("x25519");
const ref = crypto.diffieHellman({ privateKey: a.privateKey, publicKey: b.publicKey }).toString("hex");
const t = f => { try { return f() === ref ? "ok" : "WRONG SECRET"; } catch (e) { return `${e.code}: ${e.message.slice(0, 60)}`; } };
const dh = (privateKey, publicKey) => crypto.diffieHellman({ privateKey, publicKey }).toString("hex");
console.log(t(() => dh(a.privateKey, b.publicKey)));
console.log(t(() => dh({ key: a.privateKey.export({ format: "jwk" }), format: "jwk" }, b.publicKey)));
console.log(t(() => dh(a.privateKey, { key: b.publicKey.export({ format: "jwk" }), format: "jwk" })));
console.log(t(() => dh(a.privateKey.export({ type: "pkcs8", format: "pem" }), b.publicKey.export({ type: "spki", format: "pem" }))));
console.log(t(() => dh({ key: a.privateKey.export({ type: "pkcs8", format: "der" }), format: "der", type: "pkcs8" }, { key: b.publicKey.export({ type: "spki", format: "der" }), format: "der", type: "spki" })));

What diffieHellman does now, in Node's order

  1. options.privateKey == null, then options.publicKey == null: ERR_INVALID_ARG_VALUE.
  2. A KeyObject privateKey that is not private, or a secret KeyObject publicKey: ERR_CRYPTO_INVALID_KEY_OBJECT_TYPE.
  3. Two KeyObjects of different types, or of a type other than dh, ec, x448, x25519: ERR_CRYPTO_INCOMPATIBLE_KEY (Incompatible key types for Diffie-Hellman: ec and x25519). Two valid KeyObjects return here. No new code runs for them.
  4. Validate publicKey key data, then validate privateKey key data. Then import publicKey, then import privateKey. Each error throws synchronously, also when a callback is given.
  5. The derive. A failure reports the first OpenSSL error (ERR_OSSL_EVP_DIFFERENT_PARAMETERS, ERR_OSSL_EVP_DIFFERENT_KEY_TYPES, ERR_OSSL_EVP_OPERATION_NOT_SUPPORTED_FOR_THIS_KEYTYPE, ERR_OSSL_EVP_INVALID_PEER_KEY). These are the codes that the Node test expects for BoringSSL.

Two changes for callers that pass KeyObjects and get an error

  • Two KeyObjects of incompatible types: ERR_CRYPTO_INCOMPATIBLE_KEY, class Error. Before: ERR_INVALID_ARG_VALUE, class TypeError, The property 'options.privateKey' must be a supported key type.
  • A failed derive (for example P-256 with P-384): ERR_OSSL_EVP_DIFFERENT_PARAMETERS. Before: ERR_CRYPTO_OPERATION_FAILED: diffieHellman operation failed. If OpenSSL reports no error, the fallback is ERR_CRYPTO_OPERATION_FAILED: Deriving bits failed, which is Node's text.

Names. The second column is name. Each path is name, name.key, name.format, name.type, name.passphrase, name.asymmetricKeyType or name.namedCurve.

Function Node v26.3.0 and this PR Before
diffieHellman options.privateKey, options.publicKey key data rejected
Sign#sign, privateEncrypt, privateDecrypt privateKey key, and options.format / options.type / options.passphrase
createPublicKey, createPrivateKey, sign, verify, Verify#verify, publicEncrypt, publicDecrypt key key, and options.format / options.type / options.passphrase
KeyObject#export options.<option> same
generateKeyPair options.publicKeyEncoding.<option>, options.privateKeyEncoding.<option> same

makeOptionString(prefix, option) now renders <prefix>.<option>, which is Node's option() helper after nodejs/node#62527. #38415 makes that change with a fixed key prefix. This PR contains it with the name of the caller, so #38415 is not needed after this PR.

One text outside node:crypto changes. tls.createSecureContext({ key: [{ pem, passphrase: 1 }] }) reports The property 'key.passphrase' is invalid. Before: The property 'options.passphrase' is invalid. Node reports ERR_INVALID_ARG_TYPE for options.passphrase from its own tls check, which Bun does not have for a passphrase inside the key array.

Two guards changed on lines that this PR rewrites, so that the new diffieHellman forms do not inherit them. A function is not an options object (Node tests typeof key === 'object'). A format that is not a string is not converted to one (Node compares with ===). Sign#sign had its own check for a number or a boolean key, with a different list of types. It is gone, so that key gets the same message as in Node.

The crash in the EC JWK import. getKeyObjectHandleFromJwk decodes d and used the result with no exception check. JSC delivers a termination (a vm script timeout, worker.terminate()) at the next exception check, so the decode can return null. On main this loop ends with Segmentation fault at address 0x10 in 3 of 3 runs: vm.runInContext("for (;;) run();", context, { timeout: 1 }) with run = () => crypto.createPrivateKey({ key: ecPrivateJwk, format: "jwk" }). With this PR diffieHellman reaches the same line, so the check is part of it. #32910 reworks the OKP branch of that function and does not touch this line. After the fix, 19 key forms (every diffieHellman form, and JWK keys through sign, verify, createPrivateKey, privateDecrypt) survive 120 timeouts each on the debug build with ASAN.

The 68 probe results that still differ from Node v26.3.0. The probe is 125 inputs, each with and without a callback. None of the 68 is specific to diffieHellman. Each is the error that createPrivateKey or createPublicKey gives for the same input in Bun. No input returns a secret in one runtime and throws in the other (50 such results on main).

Not changed here, and the same on main: the order of checks at the other call sites. Sign#sign checks padding, Verify#verify checks the signature and publicEncrypt checks oaepLabel before the key, where Node checks the key first. #40550 (encapsulate and decapsulate) adds one more copy of the two-step block and can call getAsymmetricKeyFromJS. #38431, #38690, #43508 and #36166 edit lines inside prepareAsymmetricKey and need a rebase after this PR.

Measurements. Release builds of main at 408c937 with and without this change, linux-x64. Two later commits are not in these builds: getAsymmetricKeysFromJS took the two steps out of the diffieHellman key data path, and Sign#sign lost a pre-check. No measured path except diffieHellman(PEM, PEM) runs that code, and text can differ by tens of bytes. gdb counts on a fixed script, with ASLR off and the same environment for both binaries. perf and valgrind are not installed on the build machine.

  • diffieHellman(KeyObject, KeyObject), sync: 2 mi_malloc per call (base 3. Calls 20 to 59, x25519). The unused WTF::Vector in DhJobCtx::runTask is gone.
  • diffieHellman(KeyObject, KeyObject), sync: 2,007 instructions outside the derive (base 2,076. stepi, calls 20 to 23 give the same count).
  • diffieHellman(KeyObject, KeyObject, callback): 4 mi_malloc per call (base 5). DhJobCtx 48 B requested and 48 B usable (base 40 and 48). 1 work-pool job per call (base 1). 1 syscall on the JS thread per call (base 1).
  • createPublicKey(KeyObject): 803 instructions (base 802). crypto.sign(KeyObject): 159,534 (base 159,532). crypto.verify(KeyObject): 630,196 (base 630,189). createPublicKey(PEM): 5,592 (base 5,590). Fourth warm call, one fixed Ed25519 key. importPreparedKey is entered 0 times for a KeyObject argument.
  • createPublicKey({ key, format: 'pem' }): 18 mi_malloc per call (base 20). The two option names (options.format, options.type) are now built only for an error. createPublicKey(pem): 16 (base 16). sign(pem): 32 (base 32).
  • diffieHellman(PEM, PEM), sync: 42 mi_malloc and 0 syscalls per call. createPrivateKey + createPublicKey + diffieHellman(KeyObject, KeyObject) on the base: 43 and 0.
  • Release text: 105,954,359 B (base 105,952,487, +1,872 B, llvm-size). jsDiffieHellman: 1,949 B (base 1,836). The compiler now inlines DhJobCtx::runTask (138 B) into it.
  • getAsymmetricKeyFromJS is ALWAYS_INLINE. As an out-of-line function it gives text 432 B below the base, and 28 more instructions for each key argument (createPublicKey(KeyObject) 830).
  • Host functions +0, job types +0, src/js bytes +0. The binding table, node_crypto_binding.rs and src/js have no diff.
  • Key descriptor on the stack: 72 B (base 88 B). It cannot be heap allocated (WTF_FORBID_HEAP_ALLOCATION). The split adds 0 heap allocations.
  • Error text of the 10 other APIs, 120 inputs: 10 differ from Node (base 71). All 10 are the expected-type list of a raw key, which node:crypto: render prepareAsymmetricKey ERR_INVALID_ARG_TYPE lists like Node #38431 changes.
  • Getters and Proxies that change one key while the other is prepared, 754 cases: 0 differ from Node without a callback, 0 with a callback. If both keys are invalid, the same key wins as in Node in 7 of 7 cases. 0 ASAN reports on the debug build.
  • Wall clock, bench/crypto/diffie-hellman.mjs case diffieHellman X25519 - KeyObjects, median of 8 interleaved runs: base 55.64 us, PR 55.46 us. Two runs of the base differ by 11.8 %, and the 8 runs of one binary spread over 42 %. This shows no difference.

Self-review. The review did not run to its end. These are the concerns that it had raised and probed.

  • Addressed: the null dereference in the EC JWK import (the exception check, and the test with the vm timeout).
  • Addressed: a validated key must keep its Buffers alive when a GC runs in a getter of the other key. It does. The new test forces a GC in that getter. It also passes with BUN_JSC_collectContinuously=1.
  • Not changed: LeakSanitizer reports at process.exit() for the Buffer that holds a decoded PEM string, and for the results of jobs in flight. Main reports the same for crypto.sign(pem) and for KeyObject jobs.
  • Not changed: a setter on Error.prototype.reason runs when the job builds its error. It needs prototype pollution, and sign and generateKeyPair build their errors the same way.
  • Addressed: the two steps were public, and a caller had to pass the same mode to both. They are private now. getAsymmetricKeyFromJS runs them for one key and getAsymmetricKeysFromJS for two keys, in Node's order.
  • Not changed: ALWAYS_INLINE on the one-call wrapper. The two costs are in the measurements above.

Suites run on the debug build with this change

  • test/js/node/crypto/node-crypto.test.js and crypto.key-objects.test.ts: 389 pass. On main 64 and 10 of them fail.
  • test/js/node/crypto/: 0 fail (with a longer per-test timeout, for the RSA key generation tests on a debug build).
  • test/js/node/test/parallel/test-crypto-* and test-webcrypto-*: 159 of 159 exit 0. test-crypto-dh-stateless.js, test-crypto-key-objects.js and test-crypto-sign-verify.js exit 1 on main with the updated files.
  • The new diffieHellman and key name tests, and six vendored key tests, also pass with BUN_JSC_validateExceptionChecks=1.

Vendored tests. test-crypto-dh-stateless.js was a block-commented old copy (#35408 lists it). It is now the Node v26.3.0 file. The only edit is the first word of two comments, which is Note here. Its finite-field DH and X448 cases skip themselves for BoringSSL. One line each in test-crypto-key-objects.js (key.type) and test-crypto-sign-verify.js (privateKey.key) goes back to the upstream text.


no test proof · iteration 1 · platform-specific test(s) that do not run on this machine, deferring to CI, which covers all platforms: test/js/node/crypto/crypto.key-objects.test.ts

@robobun

robobun commented Oct 15, 2026

Copy link
Copy Markdown
Collaborator Author

Status: ready for review (#45335).

Reproduction: the script in the PR notes calls crypto.diffieHellman() with two KeyObjects, then with a JWK private key, a JWK public key, PEM keys and DER keys. Node v26.3.0 prints ok five times. Bun 1.4.3 prints ok once and throws ERR_INVALID_ARG_VALUE: The property 'options.privateKey' is invalid for the key data. With this PR, Bun prints ok five times.

@robobun

robobun commented Oct 15, 2026 •

Copy link
Copy Markdown
Collaborator Author
Updated 9:54 AM PT - Oct 15th, 2026

✅ @robobun, your commit 5c471e571701cc33dbe28230be189567c2c4faed passed in Build #125969! 🎉


🧪   To try this PR locally:

bunx bun-pr 45335

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

bun-45335 --bun

@coderabbitai

coderabbitai Bot commented Oct 15, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Repository: oven-sh/bun/.coderabbit.yaml
  • Review profile: ASSERTIVE
  • Plan: Advanced
  • Run ID: 084c3ff8-9ceb-4714-824a-7009b7588fdc
📥 Commits

Reviewing files that changed from the base of the PR and between 8b85476 and 5c471e5.

📒 Files selected for processing: 6
  • src/jsc/bindings/node/crypto/CryptoDhJob.cpp
  • src/jsc/bindings/node/crypto/CryptoUtil.cpp
  • src/jsc/bindings/node/crypto/JSSign.cpp
  • src/jsc/bindings/node/crypto/KeyObject.cpp
  • src/jsc/bindings/node/crypto/KeyObject.h
  • test/js/node/crypto/crypto.key-objects.test.ts
💤 Files with no reviewable changes: 1
  • src/jsc/bindings/node/crypto/JSSign.cpp

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 1 remain after this review.

Walkthrough

The change defers asymmetric-key import, routes crypto APIs through shared KeyObject conversion helpers, and expands Diffie-Hellman key-data handling and error reporting. Tests cover input formats, getter ordering, and synchronous and callback behavior. Two X25519 benchmarks compare KeyObject and PEM inputs.

Changes

Crypto Key Handling

Layer / File(s) Summary
Deferred asymmetric-key preparation and import
src/jsc/bindings/node/crypto/KeyObject.*, src/jsc/bindings/node/crypto/CryptoUtil.*
PreparedAsymmetricKey retains key-data and passphrase owners for later import. Preparation validates JWK and raw-key options, while import handles JWK, raw, and encoded formats. Option parsing builds contextual property paths and can return the passphrase owner.
Crypto API key conversion and validation paths
src/jsc/bindings/node/crypto/CryptoKeys.cpp, CryptoSignJob.cpp, JSCipher.cpp, JSSign.cpp, JSVerify.cpp, CryptoGenKeyPair.cpp, test/js/node/crypto/crypto.key-objects.test.ts, test/js/node/test/parallel/test-crypto-key-objects.js, test/js/node/test/parallel/test-crypto-sign-verify.js
Key creation, signing, verification, and RSA operations use shared key-conversion helpers. Encoding errors use contextual option paths. Tests cover key-read ordering, validation names, and encrypted PEM handling through TLS.
Diffie-Hellman inputs, derivation, and validation
src/jsc/bindings/ErrorCode.*, src/jsc/bindings/node/crypto/CryptoDhJob.*, test/js/node/crypto/node-crypto.test.js, test/js/node/test/parallel/test-crypto-dh-stateless.js, bench/crypto/diffie-hellman.mjs
Diffie-Hellman accepts key data as well as KeyObjects, and reports incompatible key types with a dedicated error. Derivation errors retain OpenSSL error codes. Tests cover key formats, validation order, and synchronous and callback behavior; benchmarks compare X25519 KeyObject and PEM inputs.

Suggested reviewers: jarred-sumner, dylan-conway

Priority: ➖ Normal

Merge Risk: ⚪ Minimal · up to 5c471

No confirmed issue remains that would block merging after normal checks.

Pre-merge checks: Passed 4
Passed Passed checks: 4 passed
Check name Status Explanation
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Title check Passed The title clearly and concisely describes the main changes: accepting key data in crypto.diffieHellman and separating key preparation from import.
Description check Passed The description provides a detailed explanation of the problem, implementation, behavior changes, testing, measurements, and known differences. It does not use the exact template headings, but it incl…
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

@robobun
robobun force-pushed the robobun/1899ee18/dh-stateless-key-data branch from 31c1ff0 to 8b85476 Compare October 15, 2026 13:07
@robobun

robobun commented Oct 15, 2026

Copy link
Copy Markdown
Collaborator Author

I worked on the same report in parallel, on top of the first commit of this branch. This PR covers the fix, so I did not open a second PR. My branch has four changes that this PR does not have. Take them here, or leave them for a follow-up.

Diff against the head of this PR: https://github.com/oven-sh/bun/compare/robobun/1899ee18/dh-stateless-key-data..robobun/dd281612/dh-stateless-key-data

1. Check order at the other call sites (cfe87b4). The PR body lists this as not changed. Sign#sign, Verify#verify, publicEncrypt, publicDecrypt, privateEncrypt and privateDecrypt call prepareAsymmetricKey first, then validate their other options (and the signature or the buffer), then call importPreparedKey. That is the order of Node's lib/internal/crypto/sig.js and cipher.js.

  • 37 calls with two invalid arguments: the same error wins as in Node v26.3.0 in 36. With the order of main it is 19.
  • The one that differs: publicEncrypt({ key: 'not a key', oaepHash: 'nope' }, buffer) reports ERR_OSSL_EVP_INVALID_DIGEST, and Node reports the key. The digest lookup stays before the import. Node reads a public key in the private RSA functions and Bun does not, so the upstream case privateDecrypt({ key: <public PEM>, oaepHash: 'Hello world' }) in test-crypto-rsa-dsa.js needs the digest error first.
  • The RSA functions convert the buffer before the import and take its bytes after it, because the getters of a JWK run during the import.
  • The !options.isCell() check in Sign#sign is gone. prepareAsymmetricKey reports that case with Node's text.
  • Cost for a KeyObject against main, in instructions before the crypto operation, release build: Sign#sign +10, Verify#verify +11, publicEncrypt +27. The allocation counts do not change.

2. No default for the name (f17964f). getAsymmetricKeyFromJS takes the name at each call site. getPrivateKeyFromJS and getPublicOrPrivateKeyFromJS are gone.

3. The prepared key carries its mode (f17964f, 01e06bd). importPreparedKey takes no mode, so the two steps cannot disagree. It tests keyData inline and calls the out-of-line importPreparedKeyData only for key data, so a KeyObject pays no call at the sites of item 1.

4. Shared validators (f17964f). The JWK and raw checks in prepareAsymmetricKey call V::validateObject(value, name, "key") and V::validateString(value, name, "asymmetricKeyType"). The new overloads build the dotted name only for the error.

Tests on the branch that this PR does not have (5711a0c):

  • crypto.diffieHellman: the util.promisify form, CryptoKeys with one DEP0203 warning, a key buffer that a getter makes shorter, both keys read before a call with a callback returns, and three key data rows in the LeakSanitizer table.
  • The other functions: the order cases of item 1, JWK and raw keys through each function, and publicEncrypt with a JWK getter that rewrites or detaches the input buffer.

On the debug build of the branch, test/js/node/crypto/node-crypto.test.js (284 tests), crypto.key-objects.test.ts (120 tests) and the 120 vendored test-crypto-* files pass.

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

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

Beyond the inline findings, I also checked the lifetime of the new PreparedAsymmetricKey across the second key's getters in dhJobCtxFromKeyData: it holds only stack-resident JSValues/cell pointers (no spans), and importPreparedKey reads the key and passphrase bytes under EnsureStillAliveScope only after both prepares have run, so a getter that detaches a buffer yields an empty span rather than a stale pointer. I also checked the two-KeyObject fast path: it is unchanged apart from the new ERR_CRYPTO_INCOMPATIBLE_KEY error.

Extended reasoning...

The change rewires key preparation across ten node:crypto entry points in the C++ bindings and adds a key-data path to crypto.diffieHellman, which is crypto-sensitive code; the inline findings already signal that a human should look, and this note only records the GC/detach lifetime concern that was examined and ruled out from the diff.

Comment thread src/jsc/bindings/node/crypto/JSSign.cpp Resolved
crypto.diffieHellman() took only KeyObjects. It now takes what Node takes
since v26.1.0 (nodejs/node#62527) for privateKey and publicKey: a
KeyObject, a CryptoKey, PEM in a string or a buffer, or a
{ key, format, type, passphrase, encoding } object.

KeyObject::prepareAsymmetricKey now only validates its argument. It
returns a PreparedAsymmetricKey, which holds the owner of the key bytes
and no pointer into them. The new KeyObject::importPreparedKey reads the
bytes. Both are private. KeyObject::getAsymmetricKeyFromJS runs them for
one key, and the seven other call sites use it.
KeyObject::getAsymmetricKeysFromJS runs them for two keys: it prepares
both keys and then imports both, which is Node's order.
crypto.diffieHellman uses it.

prepareAsymmetricKey takes the name of the key argument, as Node's
prepareAsymmetricKey(key, ctx, name) does. Error messages use it:
options.privateKey.format in diffieHellman, privateKey.key in Sign#sign,
privateEncrypt and privateDecrypt, and key.format where Bun said
options.format. Sign#sign has no separate check for a number or a
boolean key now, so that key gets Node's message too.

Two KeyObjects of incompatible types now throw
ERR_CRYPTO_INCOMPATIBLE_KEY. A failed derive reports its OpenSSL error.

The import of an EC private JWK now checks for an exception after it
decodes "d". A vm timeout or a worker termination at that point was a
null dereference, also through createPrivateKey.

test-crypto-dh-stateless.js is now the Node v26.3.0 file. The only edit
is the first word of two comments.
@robobun
robobun force-pushed the robobun/1899ee18/dh-stateless-key-data branch from 8b85476 to 5c471e5 Compare October 15, 2026 14:16

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

Still open from earlier reviews (3):

  • Unresolved: 3 minor or pre-existing.

Comment on lines 440 to +443
DSASigEnc dsaSigEnc = getDSASigEnc(lexicalGlobalObject, scope, options);
RETURN_IF_EXCEPTION(scope, {});

auto prepareResult = KeyObject::preparePrivateKey(lexicalGlobalObject, scope, options);
KeyObject keyObject = KeyObject::getPrivateKeyFromJS(lexicalGlobalObject, scope, options, "privateKey"_s);

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.

🔴 pre-existing: callers of Sign#sign with a bad key and a bad option get the option error where Node reports the key error. JSSign.cpp:433-443 runs getPadding, getSaltLength and getDSASigEnc before getPrivateKeyFromJS, while Node's Sign.prototype.sign calls preparePrivateKey(options, 'privateKey') first and reads padding afterwards. Fix: prepare the key before the padding, saltLength and dsaEncoding options in Sign#sign, and in Verify#verify prepare the key before the signature argument (JSVerify.cpp:330-333), so the error that wins on doubly-invalid input matches Node. Same pattern at 2 sites (JSSign.cpp:443, JSVerify.cpp:333).

Why this was flagged

Input: createSign('sha256').update(data).sign({ key: 1, padding: 'x' }). jsSignProtoFuncSign at src/jsc/bindings/node/crypto/JSSign.cpp:433 calls getPadding, which reads options.padding (CryptoUtil.cpp:442-458) and throws ERR_INVALID_ARG_VALUE "The property 'options.padding' is invalid" before the key is validated at JSSign.cpp:443. Node's lib/internal/crypto/sig.js Sign.prototype.sign calls preparePrivateKey(options, 'privateKey') first, so Node throws ERR_INVALID_ARG_TYPE for "privateKey.key" (or "privateKey") and never reaches getPadding. The same inversion exists in JSVerify.cpp:330-333, where the signature argument is converted before the key, while Node's Verify.prototype.verify prepares the key before getArrayBufferOnView(signature). The base branch has the same order in both files; this PR rewrites the key call at both sites without moving it ahead of the option reads, so the doubly-invalid case still diverges from Node. Only the error that wins differs; valid input is unaffected.

Verification: src/jsc/bindings/node/crypto/JSSign.cpp:433-444 runs getPadding, getSaltLength, getDSASigEnc and only then KeyObject::getPrivateKeyFromJS at line 443. getIntOption (CryptoUtil.cpp:438-466) throws ERR_INVALID_ARG_VALUE for options.padding, so for { key: 1, padding: 'x' } the padding error wins before the key is examined; Node calls preparePrivateKey first. JSVerify.cpp:330 reads the signature before the key at line 333. The base branch had the identical order.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

This order is on main too, and the PR notes list it under "Not changed here". The suggested move is not enough, so I kept it out.

Node's order in Sign#sign is: validate the key, read padding, saltLength and dsaEncoding, then import the key. Checked on v26.3.0:

  • sign({ key: 1, padding: 'x' }) reports privateKey.key.
  • sign({ key: 'garbage', padding: 'x' }) reports options.padding.
  • verify({ key: 1 }, 1) reports key.key.
  • verify('garbage', 1) reports signature.

Bun reports the option or the signature in all four cases. A call to getPrivateKeyFromJS ahead of the option reads fixes the first case of each pair and breaks the second, because that helper also imports the key.

The exact order needs the two steps around the option reads, at six call sites (Sign#sign, Verify#verify, the RSA functions, one-shot sign and verify). That changes which error wins in six functions, so it is a separate change.

Jarred-Sumner added a commit that referenced this pull request Oct 20, 2026
… and CLI (#46021)

### What does this PR do?

Hardening. This collects the open pull requests that fix a crash in Bun
into one change, and fixes about 20 more crashes that had no pull
request.

"Crash" here means a segfault, a Rust panic, an abort, a failed
assertion, memory corruption that a sanitizer reports, a native stack
overflow, or an exception that ends the process and that the program
can't catch. With this PR those cases throw a normal error, print a
normal CLI error and exit 1, or just work.

306 pull requests are included: 227 whole, and 79 cut down to the crash
fix where the rest was a refactor, a feature or a behavior change. Each
one was checked against current `main` first. 45 more turned out to be
fixed already, and 17 were duplicates. The full list is at the bottom.

#### Crashes you could hit in normal use

- **`bun build` with macros and a `.env` file crashed at random.** In a
repro with many macro modules, more than a third of the builds crashed.
Now 3,000 of 3,000 produce the right bundle. (#45960)
- **`bun run` and `bunx` aborted if a signal arrived while the child was
being spawned.** (#38926, #45965)
- **`bun build --compile ./index.js ./public/*` panicked** if an
embedded file's name wasn't valid UTF-8.
- **A truncated or corrupted compiled executable** died with SIGBUS or a
panic at startup. It now prints an error. (#31787, #38251, #41360)
- **Dev server:** several `Option::unwrap()` panics after a failed or
deleted import (#38046, #44744, #44746), a segfault when an `onLoad`
plugin throws while another bundle is queued, a request that hung and
then panicked with `bundle active`, and `server.stop()` that never
resolved after a client aborted a waiting request.
- **`Bun.serve`:** `server.fetch(url, { headers: new Headers() })`
followed by a GC segfaulted (#40888). Routes were used after free after
`server.reload()` with `tls.serverName` (#38636).
- **`text()` and `json()` on a stream of string chunks** could abort the
process, a regression in 1.4.0. (#44269)
- **`Bun.Transpiler` with `exports.eliminate` or `exports.replace`**
aborted on `export default 1` and on most re-export clauses. (#44137)
- **A decorated `declare` field in a class expression** with
`experimentalDecorators` panicked. (#38095)
- **YAML and JSON5 files with a key that isn't a string** panicked with
`--no-bundle` and in `Bun.Transpiler`. (#41827)
- **`Bun.FileSystemRouter`:** reading `.params` segfaulted when a
rejected route candidate left its parameters behind. (#39290, #39291)
- **Native addons that call `uv_async_*`, `uv_timer_*`, `uv_idle_*`,
`uv_prepare_*`, `uv_check_*`, `uv_queue_work` or `uv_cwd`** aborted with
"unsupported uv function" on Linux and macOS. They're implemented now.
(#36609, #39652, #39892)
- **`--watch` and `--hot`** panicked when the system was out of inotify
instances or file descriptors, or when the reload `exec` failed. They
print an error now. (#39629, #39975, #44837)
- **Paths longer than the path buffer** panicked in `bun install`, the
resolver, the bundler, `--compile`, `tsconfig.json` `paths` and
`baseUrl`, `package.json` `main` and `sideEffects`, `bun run --filter`,
`bun pm diff`, source maps and snapshots. They're ordinary errors now,
or they work. (#36340, #37529, #39104, #39650, #43067)
- **A Worker that creates a `ShadowRealm` and imports
`node:worker_threads` in it** segfaulted.
- **`structuredClone` or `postMessage` of an X25519 key** gave an object
that aborted the process when read. (#45624)
- **`node:stream` web adapters and `node:http2`** could end the process
with an exception that no handler could catch. (#45076, #45975)
- **Windows:** a watcher panic in a burst of file changes (#44344), a
segfault after the machine resumes from sleep (#33015), crashes in the
shell's pipe reader (#39638, #43606), and addons that import from
`node.exe` without the delay-load hook (#35618).

Most of the rest needs unusual input: strings near the 2 GiB limit,
buffers that another thread or a getter resizes or detaches in the
middle of a call, `worker.terminate()` at a bad moment, running out of
memory or file descriptors, very deep nesting, or hostile arguments to
`bun:ffi`, `node:vm` and `bun:sqlite`.

#### Behavior changes

Things that didn't crash before and behave differently now:

- `new` on native functions that aren't constructors throws a
`TypeError`, as in Node: `util.types.*`, `buffer.isAscii`, `isUtf8`,
`transcode`, `resolveObjectURL`, `bun:jsc` exports, `cc()` symbols. It
used to return the call's result. They also stop printing as `[class
hostname]`. (#34575)
- `` $`cmd` `` throws `ERR_INVALID_ARG_VALUE` if an environment
variable's name or value contains a NUL byte. It used to cut the value
at the NUL. (#44981)
- `cc()` functions throw a `TypeError` for arguments that used to reach
C as garbage, and `read.*(ptr, NaN)` throws. (#37989, #38057, #40773)
- As PID 1, `bun` exits with 128 + the signal number on Ctrl+C or
`docker stop`. (#38877)
- A `NODE_MODULE` addon that's already loaded by the main thread throws
`ERR_DLOPEN_FAILED` when a live Worker loads it too, as in Node.
(#39968)
- An exception from a `node:vm` context reaches `uncaughtException`
handlers. (#39992) `vm.constants.DONT_CONTEXTIFY` returns the real
global proxy. (#34623)
- `createPrivateKey` rejects an EC JWK whose private value doesn't match
its public point. (#33752)
- `@dec static {}` is a syntax error, as in TypeScript and esbuild.
(#44883)
- `clearTimeout` ignores 0 and negative ids, and timer ids no longer
wrap. (#44771)
- HEAD for a `Response` whose body is an S3-backed `Blob` reports the
length that GET sends. (#44734)
- `bun:sqlite` and `node:sqlite`: if `iterate()` left a write unfinished
on a connection that has a hook, a user function or an authorizer, the
write is finished on the connection's next call instead of during
garbage collection. If the database is gone too, it's rolled back.
- `bun test --bail` runs exit listeners and closes SQLite databases.
(#39010)
- Dev server: a request for a client script waits while a bundle is in
flight (#42733), query strings reach framework routes (#33232), and a
request gets a 500 instead of hanging when plugins fail (#36812).

#### File formats

- `.jsc` bytecode files get a 24-byte footer so they can be verified
before decoding. Files from an older Bun are ignored and the source is
parsed instead. (#39396)
- `node:vm` `cachedData` gets an 8-byte checksum. A blob from an older
Bun is rejected, which the API allows. (#32839)
- The built-in modules section of a compiled executable changes from
format 1 to 2 and carries an engine stamp. (#44907)

#### Performance

Measured in instructions per operation on release builds of this branch
and its base, with an A/A run for the noise floor (0.0 to 0.2 %).

No change: `Bun.serve`, `node:http`, WebSocket echo and publish, `bun
build` of 3,807 modules, `bun run` with 2,000 imports, `readdir` /
`Glob` / `fs.cp` over 23,689 entries, startup and RSS. The binary grows
by 176 KB (0.17 %).

| Faster | Instructions |
| --- | --- |
| WebSocket echo of a 1 MB non-ASCII text message | −72 % |
| `Bun.inspect` of a large non-ASCII string | −39 % |
| `Bun.markdown.html` on large non-ASCII input | −33 % |
| `` $`cmd` `` with 300 environment variables | −6 % |
| `fs.readSync` | −5 % |
| `TextEncoder.encodeInto`, short ASCII strings | −4 % |
| async `crypto.pbkdf2`, 16-byte inputs | −17 % |

| Slower | Instructions | Why |
| --- | --- | --- |
| `cc()` calls | +5 to +51 per call | Argument count and type checks,
and a check for `close()` (#37989, #38057, #43802, #44921). `dlopen()`
calls don't change |
| `process.env.X = v` | 38 → 593 per write | Writes aren't cached. Reads
don't change (#38871) |
| async `crypto.pbkdf2`, inputs of 1 KB or more | +4 to +9 % | Password
and salt are copied (#40554) |
| `bun build` with an object `--define` used thousands of times | +4.5 %
| The object is copied per use (#45969) |

#### Not included

- Fixes that need a WebKit change: #39967, #42534, #42982, #43764,
#43786, #44226. #42185 too: a typed array that has no `ArrayBuffer` yet
can't be pinned, so `view.buffer.transfer()` during an async call can
still free its bytes. Creating the `ArrayBuffer` up front costs 7 to 12
% on every async `fs` call, so the fix belongs in JSC: an `ArrayBuffer`
created while the view is borrowed should start out pinned.
- #45442 (`calc()` sums as a flat list): it changes minified output for
real stylesheets to survive a `calc()` with 25,000 terms. It deserves
its own review.
- #40023 (Windows event loop redesign), #29928 (Windows exit path),
#40534, #43963, #39571, #42904, #42834, #36355, #43266: refactors or
features whose crash is already covered or isn't a crash.
- The pull requests marked *(crash fix only)* below should stay open for
their other half.

### How did you verify your code works?

- **Tests.** Every fix has a test that fails on `main` (a release or an
ASAN build) and passes on this branch, except where that isn't
practical: code that only runs on Windows or macOS, inputs that need
several GB of memory, and a few rare races. Those were checked by hand
or by reading.
- **Stored repro scripts.** 230 crash repros ran on release and ASAN
builds of this branch and of `main`, side by side. Release: 88 no longer
crash. ASAN: 106 no longer crash. None crashes that didn't before. What
still crashes needs WebKit, is SQLite's own bug, or isn't a crash. The
ASAN number is from a later commit than the release number.
- **Dev server.** Because one fix makes requests wait, 2,171 generated
programs ran random interleavings of edits, build errors, failing
plugins, reloads, stops and disconnects with about 120,000 requests. No
request hung, was answered twice or got a stale body. `main` hung in 82
of 1,161 programs. This also found the four dev server bugs listed
above.
- **Path helpers.** 336,000 boundary cases with buffers of exactly the
computed size, and 12,003 random calls through 30 JS APIs on an ASAN
build, compared with `main`. No sanitizer report, and the results match
wherever `main` doesn't panic.
- **What worked on 1.4.3 still works.** Reviewers compared behavior with
1.4.3 and took out what changed it: for example an output directory
layout change for 9 or more entry points, a `server.reload()` change,
and a changed error message for `import("file:///50%deposit.ts")`.
- **Not run locally:** the Windows, macOS and FreeBSD code paths. CI
compiles and tests them.

### Included pull requests

**Bundler, transpiler, CSS, macros** (33)

- #33881 bundler: allocate wrapper_ref for CommonJS modules with no
non-hoistable statements
- #35055 Bun.build plugins: handle filter RegExp flags beyond i/m/v
- #37769 bundler: fail the build instead of panicking when the bundle
thread cannot create its waker
- #37784 bundler: copy inlined env values into the define arena, and
give each Bun.build its own env copy *(crash fix only)*
- #37858 bundler: fail the build instead of panicking when a plugin
answer string cannot be converted
- #38046 bun build --server-components: require the bun target and
reject directives without a framework *(crash fix only)*
- #38095 js_parser: fix panic on decorated declare/abstract fields in
class expressions with experimentalDecorators
- #38966 bundler: keep deep CSS, export star and composes graphs from
overflowing the linker's stack
- #38984 bundler: stop an export star ambiguity that loops back into a
re-export chain from overflowing the stack
- #39113 css: reject stylesheets of 2 GiB or more instead of aborting on
an int cast
- #39396 bundler: frame .jsc bytecode sidecars and verify them before
decoding
- #39855 bundler: join the work still on the pools before the bundle is
torn down *(crash fix only)*
- #41157 dev server: leave out an html link to a cached json module
instead of panicking, and mark a new placeholder file stale
- #41801 css: decode ill-formed UTF-8 to U+FFFD before tokenizing
*(crash fix only)*
- #41827 bundler: handle YAML and JSON5 keys that are not strings
instead of panicking
- #42242 js_parser: compress symbol link chains when following merged
declarations
- #42387 bundler: check receivers on the Bun.build plugin object and
freeze its filters during a build
- #42413 Bounds-check the InternalSourceMap window reader
- #42657 macros: keep visited values alive so a reused address does not
get another value's Expr
- #43330 transpiler: do not print an empty `var` for a data file with no
named export
- #43355 bundler: fix "The memory limit has been exceeded." on large
HTML entries
- #43658 react-compiler: fix skipped functions and build aborts for dead
code in a try body *(crash fix only)*
- #44137 js_parser: fix four crashes of exports.eliminate and
exports.replace
- #44467 js_parser: replace skipped expressions with E::Missing and skip
argument checks after a stack overflow
- #44662 js_parser: walk binary chains on the heap and check the stack
when rewriting decorated #private accesses
- #44712 json5: reject sources of 2 GiB or more before parsing instead
of aborting on an int cast
- #44864 js_parser: walk conditional chains in import()/require() and
define keys in a loop
- #44883 js_parser: reject a decorator on a class static block, keep
decorators out of the body of `declare global` *(crash fix only)*
- #45045 Fix a segfault in new Bun.Transpiler(): re-point options.log
after the constructor moves its Log
- #45604 css: keep a url() in an at-rule prelude as written, and never
print the resolved path of a url() *(crash fix only)*
- #45748 bundler: put --banner/--footer in the bytecode ESM module
record (flags from a parse, entries from JSC) *(crash fix only)*
- #45960 macros: give each parse-worker macro VM its own env loader
- #45969 js_parser: copy object and array defines per use, and copy the
list of a lone JSX spread object *(crash fix only)*

**`bun build --compile` and standalone executables** (3)

- #31787 Don't crash at startup when a standalone executable's embedded
graph is corrupted *(crash fix only)*
- #38251 compile: exit with an error instead of SIGBUS when a standalone
executable is truncated
- #41360 standalone: check every offset in the embedded module graph
before reading it

**Resolver, module loading, plugins, paths** (25)

- #32269 Bun.plugin: validate the namespace without a shared regex
- #33183 bun:jsc: filter internal cells out of getProtectedObjects()
- #34575 Don't register native functions as their own constructors
- #36340 paths: size the thread-local join output buffers to
MAX_PATH_BYTES on Windows
- #37037 mock.module: reject async factory results that are not objects
- #37065 bun:jsc: propagate JSONParse exceptions from
samplingProfilerStackTraces
- #37529 resolver: don't abort on a package.json sideEffects entry
longer than the join buffer
- #38179 node:module: throw instead of crashing when module._compile is
set to a non-function
- #38273 bun test: report a test file or preload the loader cannot
resolve instead of exiting; keep the JUnit report valid U… *(crash fix
only)*
- #38732 paths: compute the common path of 9+ inputs with the same loop
as fewer *(crash fix only)*
- #39228 paths: strip C:\\ down to the drive root in
without_trailing_slash_windows_path *(crash fix only)*
- #39650 bun run --filter: stop panicking on workspace paths longer than
the path buffer
- #40186 resolver: stop deeply nested exports targets from overflowing
the stack
- #40258 resolver: publish Entry.abs_path through the per-entry mutex
- #42183 node:module: reject a _resolveFilename override result that
contains a null byte
- #42401 node:module: accept any `this` in Module.prototype.require
*(crash fix only)*
- #42405 Bun.plugin: transpile a copy of typed array onLoad contents
- #42414 node:module: end the Module._stat path at the first null byte
- #42685 resolver: strip the trailing separator inside bust_dir_cache
- #42735 Throw a RangeError instead of aborting when require.resolve
paths or Worker preload outgrows its Vector
- #43067 Report a path that does not fit a path buffer instead of
aborting *(crash fix only)*
- #44410 Fix a crash when code from a node:vm context reads
require.cache or require.extensions
- #44466 Trace the value an object or data module exports from its
JSSourceCode
- #45044 Bun.plugin: give onLoad's args.path its own bytes
- #45117 cjs: do not call an `__esModule` getter during the annotation
lookup

**Dev server, `bun bake`, routers, `--watch` and `--hot`** (28)

- #28793 Stop file watcher thread before Global.exit tears down memory
*(crash fix only)*
- #32143 bake: fix "Runtime file not found" panic in production builds
of custom frameworks
- #32443 watcher: don't free the Watcher in the spawn-to-start window
*(crash fix only)*
- #33232 bake: normalize the HTTP request-target before matching
framework routes
- #36812 bake: respond 500 and release both RequestContext refs when
on_plugins_rejected / OOM drains a deferred framework r…
- #37039 bake: report per-file print failures in the dev server instead
of serving empty modules
- #37276 hot_reloader: take entries_mutex for watcher-thread directory
cache probes
- #37444 bake: unregister Bake::SourceProvider from the source map table
when it is destroyed
- #39290 FileSystemRouter: match / against a root-level optional
catch-all route *(crash fix only)*
- #39291 FileSystemRouter: order dynamic routes by per-segment
precedence like Next.js *(crash fix only)*
- #39629 watch: report a failed watcher init as an error instead of a
crash
- #41185 bake: check for exceptions in the production build's module
loader hooks and helpers
- #41340 bake: skip the directory watch when the directory cannot be
opened
- #41617 Decide a directory watch event from the watchlist, not from the
resolver's cached listing *(crash fix only)*
- #42547 bake: re-arm the memory visualizer timer when it fires
- #42733 dev server: make a client script request wait for the bundle in
flight
- #42828 bake: serve routes from a router root outside the project root
instead of aborting or answering 404
- #42831 bake: resolve app.root and router roots through one checked
join
- #42836 bake: report route scan errors instead of aborting, and fail
bun build --app on them *(crash fix only)*
- #42840 dev server: map a router type's server entry point to the root
route of that type
- #42862 Bun.serve: return before a pending app.plugins setup() settles
*(crash fix only)*
- #42920 Do not create a mimalloc heap per FileSystemRouter, reload() or
:nth-child(An-B) selector
- #42934 dev server: register a client component without evaluating it
- #43329 dev server: check the route bundle index of a client script URL
before Index::init
- #44249 fs.watch: read each directory and close it before descending
- #44344 Windows: fix a watcher panic and lost events in a burst of file
changes, and a --hot that stops watching a deleted…
- #44744 dev server: answer a cached-file lookup only for a graph that
bundled the file
- #44746 dev server: keep an importer in its own graph when an import
fails or is deleted

**`Bun.serve`, sockets, HTTP, WebSocket, HTTP/2, QUIC, DNS** (28)

- #42667 Reland "usockets(windows): keep a closed poll alive until the
outer tick and libuv are done with it" (#39643)
- #33015 fix(windows): segfault in uv__wake_all_loops after system
resume *(crash fix only)*
- #35761 node:quic: detach lsquic ctx pointers before the finalize-order
sweep at worker VM teardown
- #37082 Bun.serve: terminate FIFO/pipe file responses at EOF
- #38024 dns: keep the c-ares uv_poll_t alive while its poll callback is
on the stack (Windows)
- #38636 serve: fix route use-after-free after reload() with
tls.serverName *(crash fix only)*
- #40888 server.fetch: copy a Headers argument instead of adopting the
wrapper's pointer
- #41337 Update lsquic to 4.9.4 and free unsent stateless resets on
destroy *(crash fix only)*
- #41384 usockets: hold a tick level for the whole of close_all
- #41412 Windows: wait for close completions when closing a worker's uv
loop
- #41516 node:http2: handle an empty byte range in respondWithFile and
respondWithFD
- #41521 udp: report every binaryType from the getter, reject a dgram
bind of the other family *(crash fix only)*
- #42188 net: do not assert a two-character host in SocketAddress.parse
- #42210 libuv(windows): keep a pipe's completed read counted until its
read callback returns
- #42334 fetch, Bun.serve: throw a RangeError for Headers that total 4
GiB or more *(crash fix only)*
- #43738 usockets: close a socket whose resume the kernel refuses from
the loop, not from the caller
- #44338 node:http: response trailers travel with the end call of the
response *(crash fix only)*
- #44433 bake: make the parked-request list non-owning and look up the
DevServer when a plugin load settles
- #44533 node:quic: capture the handshake while the conn is alive, set
the application at session creation
- #44734 Bun.serve: answer HEAD for an S3-backed Blob body without an S3
stat
- #44751 node:quic: close() waits only for opened streams, and lsquic
sends no GOAWAY before HTTP/3 starts *(crash fix only)*
- #44897 node:http2: throw instead of panic when the native
setNextStreamID() misses its argument
- #45076 node:http2: let close() and destroy() win over the refusal of a
request
- #45341 uws: AsyncSocket::write takes a size_t length, so res.end()
sends a body of 2 GiB or more whole
- #45522 node:http2: throw for an ALTSVC frame that is too long and for
a pushPromise id that is not a stream id
- #45676 node:quic: do not reset a stream lsquic has already finished
- #45822 node:quic: name a stream in queued session events by a
per-session key, not by its address
- #45858 Bun.serve WebSocket: a message that does not fit in memory
fails one connection, not the server *(crash fix only)*

**Streams, Blob, S3, files, `node:fs`** (28)

- #33845 io: propagate IoRequestLoop init failure instead of aborting
*(crash fix only)*
- #36365 prompt: drop debug_assert that rejects answers containing CR
- #37090 Deliver FIFO bodies whose writer connects after the reader
opens *(crash fix only)*
- #38039 node:fs: stop fs.copyFile(src, "") from segfaulting on Windows
- #38653 shell(cp): reject copying a directory into itself; walk cp
trees without recursion
- #39207 Convert scope-object receivers in the chaining functions and
inspect.custom fallbacks
- #39390 Blob deserialize: reject over-long paths, treat size >=
MAX_SIZE as unknown, make slice() non-panicking
- #39639 fetch: drop the upload and drain hop refs at VM teardown
*(crash fix only)*
- #40008 inspect hooks: copy user options with putDirectMayBeIndex
*(crash fix only)*
- #40462 Windows: FileSink.start({ path }) on a live writer: hand the
writer a libuv fd and close the old source *(crash fix only)*
- #41440 node:fs: pass the buffer to the read/readv/write/writev
callback on error
- #41584 Walk the timer pairing heap without recursion
- #41608 native-readable: accept a stream with no native handle
- #41710 JSSink write(): convert the chunk before resolving the native
sink *(crash fix only)*
- #42711 webstreams: bound the native recursion through a chain of tees
or text streams
- #43264 fetch: reject a file: URL whose path has an encoded NUL
- #43709 s3: fix double free of a multipart part's bytes on worker
terminate
- #43950 streams: link readers and writers last, reject their promises
as handled
- #44036 PipeReader: label a chunk Eof only when the reader is at its
end
- #44181 streams: throw instead of aborting when a direct stream's text
sink cannot take a string chunk
- #44195 fs: validate POSIX directory records in one parser, report a
malformed record as EIO *(crash fix only)*
- #44269 streams: join the string chunks of a text consumer with one
allocation
- #44490 watch(windows): report a failed child spawn as an error instead
of a panic *(crash fix only)*
- #44954 process.stdout/stderr with no FileSink: write synchronously,
reset after a failed write *(crash fix only)*
- #45004 streams: leave a stream alone when a consumer fails to lock it
- #45521 FileSink: throw ERR_ILLEGAL_CONSTRUCTOR from the constructor
that script reaches by writer.constructor
- #45738 s3: own the Blob store across the calls that run user JS
- #45975 node:stream: do not touch the web controller after the web side
is done

**Node-API, V8 API, `bun:ffi`, `process.dlopen`** (15)

- #36609 napi: implement uv_cwd on POSIX
- #37989 bun:ffi: stop cc() from turning non-numeric pointer arguments
into garbage pointers
- #38057 bun:ffi: make cc() reject non-numeric i64/u64 arguments instead
of passing garbage
- #38425 bun:ffi: throw on an empty cc() source array instead of
panicking
- #39652 napi: implement uv_async_t, uv_queue_work and uv_default_loop
on posix
- #39805 Share one handle scope implementation between Node-API and the
V8 API
- #39892 napi: implement uv_idle_t, uv_prepare_t, uv_check_t and
uv_timer_t on posix
- #40144 bun:ffi: return undefined for a NULL napi_value from cc() and
throw the scheduled napi exception
- #40773 bun:ffi: add the full byteOffset in read.*, do not truncate it
to int32
- #41704 napi: hand the thrown value to
napi_get_and_clear_last_exception after napi_run_script
- #42960 process.dlopen: run a V8 addon's init function after dlopen()
returns
- #43797 Fix debug assertion on an empty error message from a V8 addon
or SQLite *(crash fix only)*
- #43802 bun:ffi: throw a TypeError when a cc() symbol is called after
its library's close()
- #43940 napi: fail with ERR_BUFFER_TOO_LARGE instead of aborting above
the ArrayBuffer limit *(crash fix only)*
- #44921 bun:ffi: pad a short call to a cc() symbol with undefined

**`node:vm`, workers, process, timers** (19)

- #34623 node:vm: run DONT_CONTEXTIFY contexts directly against the real
global
- #32839 node:vm: reject invalid cachedData instead of crashing
- #36921 error printer: clear the pending exception when formatting a
non-Error uncaught value throws
- #37270 error printer: one render per error, [Circular] under the key
that closes the cycle
- #38740 bun:test: give each VM its own fake clock *(crash fix only)*
- #39864 Report a refused HTTP, bundle, IO or process waiter thread as
an error instead of a crash *(crash fix only)*
- #39992 process: deliver exceptions reported against a node:vm context
to uncaughtException
- #41654 node:vm: give each SourceTextModule request the import
attributes of the declaration that created it *(crash fix only)*
- #41739 Do not forward the inherited IPC channel to spawned scripts and
tools
- #41769 node:vm: stop compiling accepted cachedData, report each
cachedData decode to the GC *(crash fix only)*
- #42045 bun:test fake timers: never move the fake clock backwards
- #42308 process.execve: throw instead of aborting for a string past the
string limits
- #42734 node:vm: report cachedData of 2 GiB or more as rejected instead
of aborting
- #42843 process.dlopen: reject a filename of PATH_MAX or more before
dlopen()
- #43500 process: accept SIGSTKFLT, SIGPOLL and SIGPWR, and emit every
name of a signal number *(crash fix only)*
- #44486 worker: stop the start sequence when a handler it calls stops
the worker
- #44771 timers: allocate ids from one 64-bit counter that never wraps
- #44837 --hot: defer a reload during the preloads, and report a reload
that cannot start
- #44991 Fix a use-after-free in the VM ref-string intern for a string
over the length limit

**Shell, spawn, CLI, `bun test`, `bun install`** (31)

- #30642 Fail fast at startup when simdutf has no usable implementation
for this CPU
- #34223 Fix four crashes: HTMLRewriter disturbed body, S3 locked-stream
upload, ConnectionsList closed parser, shell brace… *(crash fix only)*
- #34884 Guard the console and test-runner formatters against native
stack overflow on deeply nested values *(crash fix only)*
- #36584 process.chdir: publish an immutable cwd slice so no thread sees
a torn cwd
- #37135 bun test --parallel: hold the JSC API lock while draining the
worker's final IPC backlog
- #37527 shell(mv): report ENAMETOOLONG instead of panicking when a
source name does not fit the path buffers
- #37528 cli: stop aborting on absolute script paths longer than 1024
bytes on Windows
- #38162 shell(cp): report ENAMETOOLONG instead of panicking on operands
longer than the path buffers
- #38205 shell(windows): report ENAMETOOLONG for operands that do not
fit the path buffer
- #39010 bun test: exit through the VM teardown path when --bail stops a
run
- #39104 bun test: report over-long coverage dir, JUnit outfile and
bunfig root instead of panicking *(crash fix only)*
- #39638 shell(cat): serve a second cat on the same stdin instead of
hanging or panicking
- #39641 usockets: report an out-of-descriptors event loop init as an
error, not a crash *(crash fix only)*
- #40181 shell: stop deeply nested if clauses from overflowing the
parser stack
- #40885 shell: throw instead of panicking when a ReadableStream is used
as a redirect
- #41155 ipc: send a queue item larger than 2 GiB in capped write
requests
- #42298 bun:test: print String objects with 16-bit storage correctly
- #42577 bun:test: root the return values toHaveReturnedWith collects
- #43606 shell(windows): report a failed pipe reader start through
on_reader_error instead of leaking the subprocess *(crash fix only)*
- #43838 node:cluster: read a worker's `ack` and `addressType` in the
primary only when they are int32
- #44172 node:cluster: refuse a descriptor of the wrong kind in the
primary *(crash fix only)*
- #44185 node:cluster: one handle for each descriptor that workers name
*(crash fix only)*
- #44491 bun test: run every callback when native code leaves an
exception pending *(crash fix only)*
- #44694 shell mv: refuse to copy a directory into itself on the
cross-device fallback
- #44752 shell: take the command-substitution capture and drop its NUL
bytes
- #44981 shell: reject NUL bytes in .cwd() and .env() at the native
setters
- #45206 ini: reject a source of 2 GiB or more in Parser::parse instead
of aborting on an int cast *(crash fix only)*
- #45348 bun test: fix a segfault in .not.toThrow(Class) when
Symbol.hasInstance accepts a thrown string, symbol or BigInt
- #45649 shell: replace the output counters in mkdir, cp, ls and touch
with the OutputTask queue
- #45792 bun test: SIGKILL a spawnSync child that is still running 500
ms after the test's timeout *(crash fix only)*
- #45910 create: own the remote-template response buffers instead of
parking them in the CLI arena

**Web APIs, Buffer, encoding, URL, crypto, zlib, SQLite, Redis and the
rest** (96)

- #29709 console: guard JSX/Proxy recursion in Bun.inspect (stack
overflow SIGSEGV)
- #30817 console.takeHeapSnapshot: clear prior snapshots before building
- #30956 spawnSync: make signal-forwarding register/unregister safe for
concurrent callers *(crash fix only)*
- #31505 node:zlib: reject invalid brotli flush values at validation
*(crash fix only)*
- #31735 node:path: fix win32 drive-relative resolve on non-Windows
hosts *(crash fix only)*
- #32085 Fix crash when RedisClient onclose/onconnect is set to a
non-callable
- #32240 Fix BroadcastChannel custom inspect crash with numeric option
keys
- #33181 Reject interior null bytes in Bun.mmap, Glob.scan, and
FileSystemRouter paths *(crash fix only)*
- #33752 node:crypto: validate EC JWK private scalar matches public
point in createPrivateKey
- #34976 spyOn: install and restore through one writer that throws when
the object refuses *(crash fix only)*
- #35562 structuredClone: purify NaN in deserialized Date timestamp so
getTime never returns a forged JSValue
- #35618 napi(windows): load addons that import node.exe without
win_delay_load_hook
- #35757 Coerce later arguments before capturing input buffers in
Bun.Transpiler and Bun.RedisClient
- #37252 Report UTF-16 to UTF-8 encode counts through size_t so exactly
2^32 written bytes doesn't wrap to 0
- #37338 node:module: build require.cache without user-writable globals
and propagate its exceptions *(crash fix only)*
- #37340 redis: accept unsubscribe acks that arrive outside subscriber
mode
- #37370 Error.appendStackTrace: make appending an error's trace to
itself a no-op
- #37443 node:crypto: fix null deref when worker.terminate() lands
during an EC JWK private key import
- #37448 jsc: check for exceptions before using a stored
constructEmptyArray result *(crash fix only)*
- #38009 MessagePort: fix a heap-use-after-free in contextDestroyed(),
release ref() when the last 'message' listener is rem… *(crash fix
only)*
- #38469 napi: settle or refuse async work queued during VM teardown
instead of taking a ticket
- #38708 Handle 16-bit module sources in the error preview, the
new-position fix-up and the REPL *(crash fix only)*
- #38821 process.env: make a failed env build throw instead of aborting
(Windows 0xC0000409 on Bun.$ / Bun.sql / process.env…
- #38871 process.env: coerce to string on every execution of an
assignment, not just the first
- #38877 cli: exit 128 + signo when bun cannot die from a re-raised
signal (bun as container PID 1)
- #38926 Clear the inherited signal mask at startup and exec children
with an empty one *(crash fix only)*
- #39382 Make a failed util.inspect load throw from custom inspect
functions instead of aborting
- #39481 Always create the error when a native error message exceeds the
string length limit
- #39651 performance: keep the user timing entry counts exact after
clearMarks(name) and clearMeasures(name)
- #39775 Turn seccomp SIGSYS traps into ENOSYS so the syscall fallbacks
run on Android
- #39968 process.dlopen: replay a non-context-aware V8 addon only while
no other live VM owns it
- #39975 watch: report a failed reload exec as an error instead of a
panic *(crash fix only)*
- #40346 Give Glob's native scan methods their own private names
- #40554 node:crypto: copy password and salt for async pbkdf2 and scrypt
- #40567 url: reject URL-encoded input over the WTF decode limit instead
of aborting
- #40577 url: throw a RangeError instead of aborting when
URLSearchParams or FormData outgrows its Vector
- #41073 Store array-index property names as indexed properties in
native object builders
- #41137 Write the sampling profiler report at exit from Bun, not JSC's
atexit hook
- #41365 webview: fail only the view when WebKit lacks
initWithDirectory:
- #41574 Decode a private copy of a JS buffer in the UTF-8 decoders
*(crash fix only)*
- #41652 console: read a BigInt depth option by value in console.dir
- #41749 Startup: reopen fd 0-2 as /dev/null only when the fd is closed
- #41819 Report an incoherent BUN_JSC_* option combination as a
configuration error, not a crash *(crash fix only)*
- #42158 webview: reject malformed Chrome replies instead of crashing
- #42179 shell: clamp a `> ${buf}` redirect to the target buffer's live
length
- #42187 node:crypto: throw ERR_OUT_OF_RANGE for >2 GiB buffer arguments
instead of aborting
- #42191 URLPattern: throw instead of aborting when a pattern string
passes the string limit
- #42192 Copy WebAssembly.Memory bytes for native borrows that outlive
the call *(crash fix only)*
- #42198 Make Bun.$, Bun.sql, Bun.postgres and Bun.SQL custom value
getters so reifying the Bun object passes exception chec…
- #42203 node:fs: fs.read into a WebAssembly.Memory view writes into the
block a grow freed
- #42214 Headers: throw a RangeError instead of aborting when the
set-cookie list cannot grow
- #42218 Headers: throw instead of aborting when a value passes the
string length limit
- #42220 URLPattern: throw instead of aborting when the token list
cannot grow
- #42225 Throw ERR_STRING_TOO_LONG from Bun.wrapAnsi instead of aborting
when the output passes the string length limit
- #42226 Throw instead of aborting when a MIME string does not fit in a
string *(crash fix only)*
- #42237 Bun.Cookie: throw instead of aborting when the Set-Cookie
string passes the string length limit
- #42249 Copy a shared input before libdeflate compresses it in
Bun.gzipSync and Bun.deflateSync
- #42253 bun test: walk nested describe scopes without native recursion
- #42259 Throw instead of aborting when a domainToASCII host passes the
string length limit
- #42310 Parse a private copy of a shared or resizable buffer in
Bun.Transpiler, Bun.markdown, Bun.YAML and Bun.TOML *(crash fix only)*
- #42312 YAML, JSON5, XML stringify: throw instead of aborting when the
output passes the string length limit
- #42318 Bound libdeflate's emit path so a racing input cannot overrun
the output
- #42338 inspector: check the type of process.argv in getModuleGraph
- #42364 node:perf_hooks: do not truncate the monitorEventLoopDelay
resolution to int32
- #42412 transpiler cache: reject an entry with no sourcemap section
- #42479 worker_threads: do not abort when
markAsUntransferable/markAsUncloneable gets a WebAssembly GC reference
- #42517 EventTarget: skip a listener object that has no realm
- #42518 fs.Stats: make the date accessors define the property like
Object.defineProperty *(crash fix only)*
- #42575 Stop natives from writing onto a foreign `this` with putDirect:
process.ppid, require.cache, StringDecoder, the nat… *(crash fix only)*
- #42631 JSStringView: pin the cell that owns the borrowed characters
- #42660 bun_core: read /dev/urandom when getrandom(2) answers ENOSYS or
EPERM
- #42780 node:crypto: treat a name too long to convert to UTF-8 as
unknown instead of aborting
- #42782 node:crypto: throw ERR_OUT_OF_RANGE for a passphrase larger
than INT_MAX bytes
- #42856 url: do not abort when a host is too long for the IDNA
conversion buffers
- #42868 sqlite, console, process: do not abort on a string too long to
convert to UTF-8 *(crash fix only)*
- #42941 Bun.sliceAnsi: throw instead of aborting when the result passes
the string length limit
- #43058 Bun.serve: fix crash and cross-request 500 when inspecting a
non-Response return value throws
- #43904 JSC bindings: build property keys from a BunString through
Bun::toIdentifier
- #44124 Bun.Image: decode WebP into a Bun-owned canvas and read ICCP
without the demuxer *(crash fix only)*
- #44190 Convert scope-object receivers before Buffer read*/write* and
vm.Script describe them
- #44211 Stop getsockname(out), useFakeTimers() and the stream source
pull() from writing onto a caller's object with putDirect
- #44295 bun test --coverage: append a final newline to a text that
lacks one *(crash fix only)*
- #44360 rust-argon2: return OutOfMemory instead of aborting when the
memory cost cannot be allocated
- #44421 node:zlib: free zstd's workers before the dictionary they read
- #44521 util.parseArgs: pick the default args start from a per-VM eval
flag, as Node v26 does *(crash fix only)*
- #44740 structured clone: tolerate a zero-sized shared
WebAssembly.Memory in computeMemoryCost()
- #44759 HTMLRewriter: park a suspended handler's token before the
output flush
- #44907 Builtins section format 2: an engine stamp says which buns
compile its modules
- #45188 Reject an oversized WebCrypto cipher output instead of aborting
- #45354 Throw for a `file:` URL whose path is not UTF-8 on Linux and
macOS, through one `fileSystemPath()` accessor *(crash fix only)*
- #45608 ipc: refuse an over-long message before the receive buffer
stores it
- #45624 webcrypto: write the key type into the X25519 structured-clone
record
- #45625 node:sqlite: refuse session.close() and Symbol.dispose inside a
SQLite callback
- #45653 glob: a group with `?` or a class matches when an alternative
takes 64 bytes or more
- #45820 Bun.file, S3Client.file and Bun.write no longer keep a buffer
path alive forever
- #45832 `bun check`: fix a panic on `export *` without a module
specifier, and a false error for one in parentheses

### Already fixed on `main`, or duplicates of a pull request above

Already fixed: #27323 #30401 #31011 #31206 #31566 #31626 #31695 #31702
#31945 #32773 #34154 #34697 #34744 #34964 #35655 #35816 #36020 #36181
#36296 #36342 #36581 #36602 #36807 #36818 #36857 #37172 #37235 #37239
#37267 #37420 #37731 #37914 #38113 #38119 #38509 #38514 #38540 #38975
#39410 #40749 #41163 #41171 #42108 #43085 #43230

Duplicates: #45654 #29986 #33378 #36895 #37070 #37639 #38014 #39021
#41266 #42250 #42951 #43238 #44259 #45335 #45565 #45726 #45965

Fixes #5019
Fixes #6106
Fixes #10690
Fixes #10886
Fixes #13566
Fixes #14745
Fixes #15551
Fixes #20454
Fixes #21310
Fixes #29145
Fixes #30454
Fixes #30613
Fixes #30766
Fixes #32103
Fixes #32142
Fixes #32183
Fixes #32212
Fixes #39060
Fixes #42296
Fixes #42931
Fixes #43671
Fixes #44201
Fixes #44735
Fixes #44783
Fixes #45198
Fixes #45327
Fixes #45491
Fixes #45721

---------

Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
@Jarred-Sumner

Copy link
Copy Markdown
Collaborator

This is Claude (an AI assistant) commenting on Jarred's behalf.

Another fix for the same crash went in with #46021, which is merged. Closing as superseded.

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.

2 participants