Robustness and input-handling pass across install, shell, TLS/QUIC, HTTP/3, SQL and crypto - #37669
Conversation
|
Updated 6:05 AM PT - Aug 12th, 2026
❌ @Jarred-Sumner, your commit f4f5e74 has some failures in 🧪 To try this PR locally: bunx bun-pr 37669That installs a local version of the PR into your bun-37669 --bun |
|
claude fix the conflicts |
cb6f01e to
4ba8325
Compare
|
Rebased onto main and resolved the conflicts. The unix-socket redirect change is dropped from this branch since #37451 settled that behavior; everything else is unchanged. |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
WalkthroughChangesThis change set updates TLS and QUIC verification, package installation and archive extraction, shell and runtime safety, SQL protocol handling, HTTP behavior, cryptography, and bundler validation. Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 20
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
src/install/PackageManager/PackageManagerLifecycle.rs (1)
482-501: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winUse an explicit work-list instead of recursion in
add_package_to_set.
add_package_to_setrecurses once per newly-visitedpackage_idin a dependency chain before thefound_existingmemoization guard can short-circuit it. The guard stops cycles, but not depth. A deep, non-cyclic transitive dependency chain under a--trust-ed package can drive recursion depth proportional to chain length, risking a stack overflow (a process crash, not a catchable error).
src/install/isolated_install.rs, added in this same PR, treats the same class of dependency-graph walk with an explicit iterative DFS specifically to avoid this risk. Apply the same pattern here: replace the recursive call with an explicitVec-backed work-list.🛡️ Proposed iterative rewrite
fn add_package_to_set( set: &mut ArrayHashMap<PackageID, ()>, lockfile: &Lockfile, package_id: PackageID, ) { - if handle_oom(set.get_or_put(package_id)).found_existing { - return; - } - let dependencies_slice = lockfile.packages.items_dependencies()[package_id as usize]; - let begin = dependencies_slice.off; - let end = begin.saturating_add(dependencies_slice.len); - let mut dep_id = begin; - while dep_id < end { - let dep_package_id = lockfile.buffers.resolutions[dep_id as usize]; - if dep_package_id != invalid_package_id { - add_package_to_set(set, lockfile, dep_package_id); - } - dep_id += 1; - } + let mut stack: Vec<PackageID> = vec![package_id]; + while let Some(current) = stack.pop() { + if handle_oom(set.get_or_put(current)).found_existing { + continue; + } + let dependencies_slice = lockfile.packages.items_dependencies()[current as usize]; + let begin = dependencies_slice.off; + let end = begin.saturating_add(dependencies_slice.len); + let mut dep_id = begin; + while dep_id < end { + let dep_package_id = lockfile.buffers.resolutions[dep_id as usize]; + if dep_package_id != invalid_package_id { + stack.push(dep_package_id); + } + dep_id += 1; + } + } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/install/PackageManager/PackageManagerLifecycle.rs` around lines 482 - 501, Replace recursive traversal in add_package_to_set with an explicit Vec-backed work-list implementing iterative DFS. Seed it with package_id, pop each package, skip processing when handle_oom(set.get_or_put(...)).found_existing is true, and push each valid dependency package ID for later processing while preserving the current dependency bounds and invalid_package_id filtering.src/libarchive/lib.rs (1)
1738-1738: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winKeep
O_NOFOLLOWfor regular-file extraction.extract_to_diskaccepts an existing caller-supplied path, andArchive.rspassesself.pathdirectly. A pre-existing symlink can therefore redirectopenatoutside the extraction directory.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/libarchive/lib.rs` at line 1738, Update the flag construction in extract_to_disk for regular-file extraction to include bun_sys::O::NOFOLLOW alongside WRONLY, CREAT, and TRUNC. Preserve the existing openat flow while ensuring caller-supplied paths cannot follow pre-existing symlinks.src/install/TarballStream.rs (1)
1433-1489: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winMove symlink-target validation into one shared Unix helper.
symlink_target_stays_insideduplicatesis_symlink_target_safe, and the implementations differ for empty targets and dirname resolution. Export one helper fromsrc/libarchive/lib.rsand call it from both extraction paths. The existing#[cfg(unix)]gate already excludes theu16Windows path.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/install/TarballStream.rs` around lines 1433 - 1489, Consolidate symlink validation by exporting a shared Unix helper from libarchive/lib.rs, based on the existing is_symlink_target_safe behavior, and remove the duplicated symlink_target_stays_inside implementation from TarballStream.rs. Update both extraction paths to call the shared helper, preserving consistent handling of empty targets and dirname resolution; rely on the existing #[cfg(unix)] gating for platform differences.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/http/lib.rs`:
- Around line 1436-1456: Update print_request to redact sensitive data in curl
verbose output: pass the URL through the existing redaction utility before
printing, and ensure Authorization and Proxy-Authorization header values expose
only their scheme while masking credentials. Preserve normal output for
non-sensitive headers and the existing request formatting.
In `@src/install/PackageManager/PackageManagerEnqueue.rs`:
- Around line 2647-2660: Extract the duplicated root workspace lookup into a
shared helper such as find_root_workspace_resolution, accepting PackageManager
and PackageNameHash and returning the matching PackageID or None. Replace the
loops in the Npm-satisfies block, DistTag fallback block, and Workspace-tag
guard with calls to this helper while preserving each caller’s existing success
and result-handling behavior.
In `@src/install/repository.rs`:
- Line 942: Update the cleanup around dir.delete_file_z so a real node_modules
directory is removed recursively while preserving the existing symlink/file
removal behavior; do not discard failures for the directory case, and keep the
symlink case covered by the current logic.
In `@src/js/internal/sql/shared.ts`:
- Around line 1805-1811: Honor explicit TLS/SSL false values across URL and
option parsing: in the URL handling branch around shared.ts lines 1805-1811, set
sslMode to SSLMode.disable for false or 0; in the alias-resolution logic around
lines 1956-1963, avoid truthiness checks so options.tls or options.ssl preserves
explicit false and disables SSL mode. Add precedence tests covering URL
parameters, option objects, and PGSSLMODE, explicitly distinguishing empty,
zero, false, and unset inputs.
In `@src/jsc/bindings/JSEnvironmentVariableMap.cpp`:
- Around line 800-805: Add a regression case in the worker_threads tests using a
SHARE_ENV worker: set NODE_TLS_REJECT_UNAUTHORIZED to "0", delete the variable,
then issue a request to an invalid-certificate endpoint and assert that it
rejects. Reuse the existing SHARE_ENV worker and TLS request/test helpers where
available.
In `@src/libarchive/lib.rs`:
- Around line 1104-1108: Validate the `dirname`/`symlink.path` relationship
before slicing in the non-empty branch of the surrounding symlink creation flow:
require the directory prefix to be followed by exactly one separator and reject
or safely handle non-normalized paths such as `a//b`. Update the logic around
`dirname_simple` and the `ZStr::from_slice_with_nul` construction so the derived
name can never begin with `/` before calling `bun_sys::symlinkat`.
In `@src/paths/lib.rs`:
- Around line 966-972: Update Path’s is_node_module method to use
case-insensitive matching only on macOS and Windows, while requiring exact
matching against crate::NODE_MODULES_NEEDLE on other targets such as Linux.
Preserve matching against self.name().dir rather than text.
In `@src/runtime/cli/publish_command.rs`:
- Line 1123: Update the browser-opening call in the publish flow to handle the
result from bun_core::spawn_sync_inherit instead of discarding it. When the
opener cannot start or exits unsuccessfully, return or propagate a recoverable
error that includes both open::OPENER and auth_url; preserve the existing
success path when the command completes successfully.
In `@test/bundler/bundler_plugin.test.ts`:
- Around line 1682-1687: Assert complete subprocess results at all three sites:
in test/bundler/bundler_plugin.test.ts:1682-1687, require empty stderr alongside
the existing stdout and exitCode assertions; in
test/bundler/transpiler/macro-test.test.ts:180-182, add the empty-stderr
assertion with the existing last-line and exit-code checks; in
test/bundler/transpiler/transpiler.test.js:4468-4476, assert the expected child
diagnostics, empty stderr as applicable, and successful exit code before opening
out.json, while preserving concurrent stdout, stderr, and exit collection.
In `@test/cli/install/bun-install-streaming-extract.test.ts`:
- Around line 385-386: Replace randomFillSync in the test’s incompressible
payload setup with deterministic 8 MiB data derived from a fixed seed, while
preserving its incompressible characteristics and the existing
streaming-threshold coverage.
In `@test/cli/install/bun-install.test.ts`:
- Around line 5382-5385: Update the `.bun-tag` assertion in the test to require
the deterministic final state produced by the `O_NOFOLLOW` failure cleanup in
`src/install/repository.rs`: assert that the entry is absent, rather than
defaulting a missing entry to a passing non-symlink result. Keep using
`lstatSync` with `throwIfNoEntry: false` and distinguish absence explicitly from
an existing symlink.
In `@test/cli/install/bun-workspaces.test.ts`:
- Around line 1957-1965: Replace the external tar invocation in the affected
workspace test with in-process tarball creation to keep the test hermetic. If
retaining spawn, capture stderr instead of ignoring it and include the captured
message in the assertion alongside the exit code.
In `@test/cli/install/symlink-path-traversal.test.ts`:
- Around line 613-646: Move the exit-code assertions before the filesystem
validation in the test flow containing the `misplaced`, `markerDirs`, and
`pkgDir` checks. After the existing stdout/stderr assertions, add the
house-style `if (exitCode !== 0) { expect(stderr).toBe(""); }` guard immediately
before `expect(exitCode).toBe(0)`, then retain the filesystem checks without the
`console.error` diagnostics.
In `@test/js/bun/http/serve-directory-routes.test.ts`:
- Around line 13-21: The canInjectOpenat2Error setup only validates strace
syntax and discards evidence of injection. Update the test’s strace invocation
and the related route-test setup to write to a temporary trace file, then verify
that the trace contains an injected openat2 EPERM record before proceeding with
route assertions; skip or disable the injection path when that proof is absent.
In `@test/js/bun/shell/bunshell.test.ts`:
- Around line 232-245: Update the affected subprocess tests to capture stderr
and assert it is empty as part of the complete result:
test/js/bun/shell/bunshell.test.ts lines 232-245 for both shell commands,
test/js/bun/util/bun-cryptohasher.test.ts lines 116-131,
test/js/node/worker_threads/worker_threads.test.ts lines 1485-1498,
test/js/web/fetch/fetch.tls.test.ts lines 711-725, and
test/js/web/workers/worker.test.ts lines 190-203. Assert stderr before parsing
stdout where applicable, while preserving concurrent stdout, stderr, and exit
handling and the existing output assertions.
In `@test/js/bun/shell/shell-pipe-read-fault.test.ts`:
- Around line 450-466: Replace the expect.any(String) matchers for stderr and
readerStderr in the shell-pipe fault test at
test/js/bun/shell/shell-pipe-read-fault.test.ts:450-466 with exact empty-string
expectations. Also replace the expect.any(String) stderr matcher in
test/js/bun/shell/yield.test.ts:94-102 with an empty-string expectation,
preserving the remaining assertions.
In `@test/js/node/quic/quic-sni.test.ts`:
- Around line 208-219: Update the subprocess result assertion around stdout and
exitCode to assert a combined object that also includes stderr matched with
expect.any(String). Preserve the exact stdout JSON expectation and exitCode
validation while retaining the drained stderr diagnostics in failure diffs.
In `@test/js/node/quic/quic-stream.test.ts`:
- Around line 219-222: Update the test flow around createBidirectionalStream so
its rejection is caught when the session is closed or closing, including the
expected ERR_INVALID_STATE case. Ensure the rejection is handled before awaiting
Promise.all so the test reaches the combined server/client assertion.
In `@test/js/sql/adapter-env-var-precedence.test.ts`:
- Around line 432-440: Add a separate test case for a URL ending in empty `tls=`
while setting PGSSLMODE=require, and assert the intended SSL mode and
startup-parameter behavior explicitly. Keep the existing explicit false-value
cases unchanged, and use the test’s SQL constructor and environment setup to
document whether empty tls is ignored or disables TLS.
In `@test/js/web/crypto/web-crypto.test.ts`:
- Around line 403-406: Update the structuredClone call in the Ed25519
verification test to clone consistent, the imported key, instead of privateKey.
Keep the subsequent crypto.subtle.sign and verify flow unchanged so the test
exercises the imported key normalization path.
---
Outside diff comments:
In `@src/install/PackageManager/PackageManagerLifecycle.rs`:
- Around line 482-501: Replace recursive traversal in add_package_to_set with an
explicit Vec-backed work-list implementing iterative DFS. Seed it with
package_id, pop each package, skip processing when
handle_oom(set.get_or_put(...)).found_existing is true, and push each valid
dependency package ID for later processing while preserving the current
dependency bounds and invalid_package_id filtering.
In `@src/install/TarballStream.rs`:
- Around line 1433-1489: Consolidate symlink validation by exporting a shared
Unix helper from libarchive/lib.rs, based on the existing is_symlink_target_safe
behavior, and remove the duplicated symlink_target_stays_inside implementation
from TarballStream.rs. Update both extraction paths to call the shared helper,
preserving consistent handling of empty targets and dirname resolution; rely on
the existing #[cfg(unix)] gating for platform differences.
In `@src/libarchive/lib.rs`:
- Line 1738: Update the flag construction in extract_to_disk for regular-file
extraction to include bun_sys::O::NOFOLLOW alongside WRONLY, CREAT, and TRUNC.
Preserve the existing openat flow while ensuring caller-supplied paths cannot
follow pre-existing symlinks.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 49d0c738-37e3-46dc-9984-ea4b4f4b1896
📒 Files selected for processing (98)
packages/bun-types/sql.d.tspackages/bun-usockets/src/crypto/openssl.cpackages/bun-usockets/src/libusockets.hpackages/bun-usockets/src/quic.csrc/ast/lib.rssrc/bun_core/fmt.rssrc/bun_core/lib.rssrc/bun_core/string/immutable/unicode.rssrc/http/AsyncHTTP.rssrc/http/InternalState.rssrc/http/lib.rssrc/install/PackageInstaller.rssrc/install/PackageManager/PackageManagerEnqueue.rssrc/install/PackageManager/PackageManagerLifecycle.rssrc/install/PackageManager/PackageManagerResolution.rssrc/install/PackageManifestMap.rssrc/install/TarballStream.rssrc/install/isolated_install.rssrc/install/isolated_install/Installer.rssrc/install/lockfile.rssrc/install/lockfile/Package.rssrc/install/npm.rssrc/install/repository.rssrc/install_types/resolver_hooks.rssrc/io/PipeReader.rssrc/io/lib.rssrc/js/builtins/ProcessObjectInternals.tssrc/js/internal/sql/shared.tssrc/js/node/_http2_upgrade.tssrc/js/node/net.tssrc/js/node/tls.tssrc/js_parser/visit/mod.rssrc/jsc/NodeCompileCache.rssrc/jsc/bindings/JSBundlerPlugin.cppsrc/jsc/bindings/JSEnvironmentVariableMap.cppsrc/jsc/bindings/webcrypto/CryptoKeyOKP.cppsrc/jsc/bindings/webcrypto/CryptoKeyOKPOpenSSL.cppsrc/jsc/web_worker.rssrc/libarchive/lib.rssrc/paths/lib.rssrc/runtime/cli/publish_command.rssrc/runtime/cli/run_command.rssrc/runtime/crypto/CryptoHasher.rssrc/runtime/ffi/ffi_body.rssrc/runtime/node/quic/session.rssrc/runtime/node/zlib/NativeZlib.rssrc/runtime/server/server_body.rssrc/runtime/shell/IOReader.rssrc/runtime/shell/builtin/mv.rssrc/runtime/shell/shell_body.rssrc/runtime/shell/subproc.rssrc/runtime/socket/socket_body.rssrc/runtime/webcore/Request.rssrc/simdutf_sys/simdutf.rssrc/sql/mysql/protocol/PacketHeader.rssrc/sql/shared/ColumnIdentifier.rssrc/sql_jsc/mysql/MySQLConnection.rssrc/sql_jsc/postgres/PostgresSQLConnection.rssrc/sys/lib.rstest/bundler/bundler_plugin.test.tstest/bundler/transpiler/macro-test.test.tstest/bundler/transpiler/transpiler.test.jstest/cli/install/bun-install-lifecycle-scripts.test.tstest/cli/install/bun-install-registry.test.tstest/cli/install/bun-install-streaming-extract.test.tstest/cli/install/bun-install.test.tstest/cli/install/bun-publish.test.tstest/cli/install/bun-run.test.tstest/cli/install/bun-workspaces.test.tstest/cli/install/redacted-config-logs.test.tstest/cli/install/symlink-path-traversal.test.tstest/js/bun/ffi/cc.test.tstest/js/bun/http/serve-directory-routes.test.tstest/js/bun/http/serve-http3.test.tstest/js/bun/net/socket.test.tstest/js/bun/shell/bunshell.test.tstest/js/bun/shell/commands/mv.test.tstest/js/bun/shell/shell-pipe-read-fault.test.tstest/js/bun/shell/yield.test.tstest/js/bun/util/bun-cryptohasher.test.tstest/js/node/buffer.test.jstest/js/node/http2/node-http2-upgrade.test.mtstest/js/node/module/node-module-module.test.jstest/js/node/quic/quic-sni.test.tstest/js/node/quic/quic-stream.test.tstest/js/node/tls/node-tls-connect-hostname-verification.test.tstest/js/node/tls/node-tls-server.test.tstest/js/node/worker_threads/worker_threads.test.tstest/js/node/zlib/zlib-handle-bounds-check.test.tstest/js/sql/adapter-env-var-precedence.test.tstest/js/sql/postgres-datarow-overrun.test.tstest/js/sql/postgres-error-then-datarow.test.tstest/js/sql/sql-mysql.test.tstest/js/web/crypto/web-crypto.test.tstest/js/web/fetch/fetch.test.tstest/js/web/fetch/fetch.tls.test.tstest/js/web/fetch/fetch.unix.test.tstest/js/web/workers/worker.test.ts
💤 Files with no reviewable changes (1)
- src/simdutf_sys/simdutf.rs
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/install/TarballStream.rs (2)
585-599: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winContinue when
Retryretains the input buffer.When
archive.nextreturnslib::Result::Retrywhilearchive_holds_readingis true, the current match enters_and fails extraction. Continue the loop in this state. ReturnOk(())only whenarchive_holds_readingis false.Proposed fix
match block.result { lib::Result::Retry if !(*this).archive_holds_reading => return Ok(()), + lib::Result::Retry => continue, lib::Result::Ok | lib::Result::Warn => {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/install/TarballStream.rs` around lines 585 - 599, Update the match handling around archive.next in TarballStream so lib::Result::Retry with archive_holds_reading true continues the processing loop instead of reaching the error branch. Preserve the existing Ok(()) return when archive_holds_reading is false, while leaving Ok/Warn writes and other errors unchanged.
631-632: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winPropagate archive setup failures.
Handle
Warn,Failed, andFatalfromread_support_format_tarandread_set_options. Logarchive.error_string()and return before opening the archive.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/install/TarballStream.rs` around lines 631 - 632, Update the archive setup in the surrounding installation flow to inspect the results of read_support_format_tar and read_set_options instead of discarding them. Handle Warn, Failed, and Fatal by logging archive.error_string() and returning before the archive is opened, while preserving the existing success path.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/install/TarballStream.rs`:
- Around line 146-149: Bound deferred symlink accumulation wherever
deferred_symlinks is populated: validate the current symlink count and combined
path/target byte size before copying or pushing the entry. Enforce both a
maximum count and total-byte limit using established extraction-limit
conventions, and return a recoverable extraction error when either limit would
be exceeded.
- Around line 551-553: Update create_deferred_symlinks and its parent-directory
traversal to walk every component from dir_fd using no-follow semantics, rather
than relying on bun_sys::open_dir_at. Reject any symlinked parent component
before performing containment or inode checks, while preserving the existing
deferred symlink creation behavior.
In `@src/runtime/ffi/ffi_body.rs`:
- Around line 2420-2421: The compiler-runtime staging flow must propagate
failures from every CompilerRtSources::NODE_HEADERS write instead of returning
success with a partial directory. Update the NODE_HEADERS loop in the
surrounding staging function to return false immediately when
write_compiler_rt_file fails, and assign COMPILER_RT_DIR and
COMPILER_RT_NODE_DIR only after both source and header loops complete
successfully.
---
Outside diff comments:
In `@src/install/TarballStream.rs`:
- Around line 585-599: Update the match handling around archive.next in
TarballStream so lib::Result::Retry with archive_holds_reading true continues
the processing loop instead of reaching the error branch. Preserve the existing
Ok(()) return when archive_holds_reading is false, while leaving Ok/Warn writes
and other errors unchanged.
- Around line 631-632: Update the archive setup in the surrounding installation
flow to inspect the results of read_support_format_tar and read_set_options
instead of discarding them. Handle Warn, Failed, and Fatal by logging
archive.error_string() and returning before the archive is opened, while
preserving the existing success path.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: a87baf39-b5ce-40cb-b891-6a4589fa2830
📒 Files selected for processing (10)
src/http/lib.rssrc/install/PackageInstaller.rssrc/install/PackageManager/PackageManagerLifecycle.rssrc/install/TarballStream.rssrc/io/lib.rssrc/runtime/cli/publish_command.rssrc/runtime/ffi/ffi_body.rssrc/runtime/server/server_body.rssrc/runtime/webcore/Request.rssrc/sys/lib.rs
| /// Symlink entries accepted so far; written to disk only after every | ||
| /// other entry. | ||
| #[cfg(unix)] | ||
| created_symlinks: Vec<Vec<u8>>, | ||
| deferred_symlinks: Vec<bun_libarchive::DeferredSymlink>, |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Bound deferred symlink storage.
Each accepted symlink copies its path and target until archive EOF. A compressed archive with many symlink headers can cause unbounded heap growth before extraction completes. Enforce a symlink count and total-byte limit before push, and return a recoverable extraction error when either limit is exceeded.
As per coding guidelines, “Validate untrusted input before processing, allocation, or side effects; enforce actual resource limits.”
Also applies to: 862-867
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/install/TarballStream.rs` around lines 146 - 149, Bound deferred symlink
accumulation wherever deferred_symlinks is populated: validate the current
symlink count and combined path/target byte size before copying or pushing the
entry. Enforce both a maximum count and total-byte limit using established
extraction-limit conventions, and return a recoverable extraction error when
either limit would be exceeded.
Source: Coding guidelines
There was a problem hiding this comment.
Left as is: each deferred entry is at most one normalized path (< PATH_MAX, checked at TarballStream.rs:802) plus one target that is_symlink_target_safe already rejects unless dirname + target fits in a PathBuffer (lib.rs:1032), and both strings are carried verbatim in the tar stream, so the retained bytes are bounded by the decoded archive size — the same order as the per-symlink created_symlinks: Vec<Vec<u8>> this replaces on main. There is no existing entry-count limit in either extractor to align with, and introducing one here would reject valid archives with many links, which is out of scope for this change.
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
ast-grep outline src/libarchive/lib.rs --items all --match 'is_symlink_target_safe|DeferredSymlink|create_deferred_symlinks' --view expanded
rg -n -A18 -B12 'fn is_symlink_target_safe|struct DeferredSymlink|impl DeferredSymlink|created_symlinks|deferred_symlinks' \
src/libarchive/lib.rs src/install/TarballStream.rs
git show HEAD^:src/install/TarballStream.rs 2>/dev/null | rg -n -A14 -B10 'created_symlinks|make_symlink|path_traverses_created_symlink' || trueLength of output: 22436
@Jarred-Sumner, the per-entry bounds are valid. is_symlink_target_safe runs before DeferredSymlink::new copies the normalized path and target.
The aggregate remains unbounded because neither extractor limits the number of accepted symlinks. The buffered extractor has the same Vec<DeferredSymlink> behavior.
I agree that adding a new archive-entry limit here would change existing extraction behavior and is outside this PR. I will not request a change in this PR.
You are interacting with an AI system.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/js/internal/sql/shared.ts (1)
1956-1962: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winRestore runtime validation for TLS option types.
A JavaScript caller can pass a truthy unsupported value such as
tls: 1. Line [1961] copies that value intotlsand returns malformed TLS configuration instead of raising$ERR_INVALID_ARG_TYPE. Validate the selectedtlsorsslvalue before assignment. Add a regression test for an invalid truthy value.The supplied library context identifies this change as removing the previous type validation and invalid-argument exception. As per coding guidelines: “Validate untrusted input before processing, allocation, or side effects.”
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/js/internal/sql/shared.ts` around lines 1956 - 1962, Restore runtime type validation in the TLS option handling around tlsOption, rejecting truthy unsupported tls or ssl values such as numbers with the existing $ERR_INVALID_ARG_TYPE behavior before assigning to tls. Preserve string normalization and valid TLS option handling, and add a regression test covering an invalid truthy value.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@src/js/internal/sql/shared.ts`:
- Around line 1956-1962: Restore runtime type validation in the TLS option
handling around tlsOption, rejecting truthy unsupported tls or ssl values such
as numbers with the existing $ERR_INVALID_ARG_TYPE behavior before assigning to
tls. Preserve string normalization and valid TLS option handling, and add a
regression test covering an invalid truthy value.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 0978a348-6362-425f-b909-8c70f16a5375
📒 Files selected for processing (2)
src/js/internal/sql/shared.tstest/js/sql/adapter-env-var-precedence.test.ts
💤 Files with no reviewable changes (1)
- test/js/sql/adapter-env-var-precedence.test.ts
|
Replies to the notes that could not be posted inline:
|
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/libarchive/lib.rs (1)
1056-1144: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winPropagate deferred symlink creation failures.
create_deferred_symlinksskips parent-verification andsymlinkatfailures, then extraction reports success. This can install a package with required symlinks missing. Returncrate::Result<()>from the helper and propagate failures through both EOF paths.
src/libarchive/lib.rs#L1056-L1144: return a recoverable error for deferred-symlink verification and creation failures.src/install/TarballStream.rs#L552-L554: propagate the helper error into streaming extraction.src/libarchive/lib.rs#L1962-L1963: propagate the helper error into buffered extraction.As per coding guidelines, “Never swallow failures or signal success after failure; propagate I/O, syscall, cleanup, and requested-operation errors explicitly.”
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/libarchive/lib.rs` around lines 1056 - 1144, Change create_deferred_symlinks in src/libarchive/lib.rs:1056-1144 to return crate::Result<()> and propagate parent-verification and symlinkat failures as recoverable errors instead of logging and continuing; update src/install/TarballStream.rs:552-554 and src/libarchive/lib.rs:1962-1963 to use ? when invoking it so both streaming and buffered extraction report deferred-symlink failures.Source: Coding guidelines
src/install/TarballStream.rs (1)
863-868: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winBound deferred symlink storage.
Each symlink copies archive-controlled path and target bytes until extraction reaches EOF. A tarball with many symlink entries can exhaust memory in both extraction paths. Enforce a shared maximum entry count and byte budget before
DeferredSymlink::new. Return an extraction error when the next entry exceeds either limit.
src/install/TarballStream.rs#L863-L868: enforce the limits before adding a streaming entry.src/libarchive/lib.rs#L1716-L1717: enforce the same limits before adding a buffered entry.As per coding guidelines, “Validate untrusted input before processing, allocation, or side effects; enforce actual resource limits.”
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/install/TarballStream.rs` around lines 863 - 868, The deferred symlink paths lack shared resource limits, allowing untrusted archives to exhaust memory. In src/install/TarballStream.rs:863-868, validate the entry count and cumulative path/target byte budget before DeferredSymlink::new and return an extraction error when either limit would be exceeded; apply the same pre-allocation checks to the buffered-entry path in src/libarchive/lib.rs:1716-1717, using shared limits and accounting so both extraction paths enforce the actual bounds.Source: Coding guidelines
♻️ Duplicate comments (1)
src/js/internal/sql/shared.ts (1)
1809-1811: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winKeep an explicit
falsevalue authoritative.For
?tls=true&ssl=false, Line 1809 changes onlysslMode.tlsremainstrue, so Line 2065 changes the mode back torequire.For
{ tls: false, ssl: true }, Line 1958 selectsssl: true. The false branch does not run.Clear
tlswhen a URL value disables TLS. Resolve the aliases byundefinedstatus, not truthiness. Add regression cases for both combinations.Proposed fix
} else if (value === "false" || value === "0") { + tls = undefined; sslMode = SSLMode.disable; } -const tlsOption = options.tls || options.ssl; +const tlsOption = options.tls === undefined ? options.ssl : options.tls; ... -} else if (!tlsOption && (options.tls === false || options.ssl === false)) { +} else if (tlsOption === false) {As per coding guidelines: “Deliberately enumerate input spaces, distinguishing empty, zero, and unset.”
Also applies to: 1962-1964
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/js/internal/sql/shared.ts` around lines 1809 - 1811, Update the URL parsing branch for sslMode so an explicit SSL-disabled value also clears tls, preventing later precedence logic from restoring require mode. In the option-resolution logic around the tls/ssl aliases, distinguish unset values using undefined checks rather than truthiness so explicit false remains authoritative while ssl: true still resolves correctly. Add regression cases covering ?tls=true&ssl=false and { tls: false, ssl: true }.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@src/install/TarballStream.rs`:
- Around line 863-868: The deferred symlink paths lack shared resource limits,
allowing untrusted archives to exhaust memory. In
src/install/TarballStream.rs:863-868, validate the entry count and cumulative
path/target byte budget before DeferredSymlink::new and return an extraction
error when either limit would be exceeded; apply the same pre-allocation checks
to the buffered-entry path in src/libarchive/lib.rs:1716-1717, using shared
limits and accounting so both extraction paths enforce the actual bounds.
In `@src/libarchive/lib.rs`:
- Around line 1056-1144: Change create_deferred_symlinks in
src/libarchive/lib.rs:1056-1144 to return crate::Result<()> and propagate
parent-verification and symlinkat failures as recoverable errors instead of
logging and continuing; update src/install/TarballStream.rs:552-554 and
src/libarchive/lib.rs:1962-1963 to use ? when invoking it so both streaming and
buffered extraction report deferred-symlink failures.
---
Duplicate comments:
In `@src/js/internal/sql/shared.ts`:
- Around line 1809-1811: Update the URL parsing branch for sslMode so an
explicit SSL-disabled value also clears tls, preventing later precedence logic
from restoring require mode. In the option-resolution logic around the tls/ssl
aliases, distinguish unset values using undefined checks rather than truthiness
so explicit false remains authoritative while ssl: true still resolves
correctly. Add regression cases covering ?tls=true&ssl=false and { tls: false,
ssl: true }.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 7ed3f5a5-947f-4975-80ec-520ed9d074f9
📒 Files selected for processing (25)
src/http/lib.rssrc/install/PackageManager/PackageManagerEnqueue.rssrc/install/PackageManager/PackageManagerLifecycle.rssrc/install/TarballStream.rssrc/install/repository.rssrc/js/internal/sql/shared.tssrc/libarchive/lib.rssrc/runtime/ffi/ffi_body.rstest/bundler/bundler_plugin.test.tstest/bundler/transpiler/macro-test.test.tstest/bundler/transpiler/transpiler.test.jstest/cli/install/bun-install-streaming-extract.test.tstest/cli/install/bun-install.test.tstest/cli/install/bun-workspaces.test.tstest/cli/install/symlink-path-traversal.test.tstest/js/bun/http/serve-directory-routes.test.tstest/js/bun/shell/bunshell.test.tstest/js/bun/shell/shell-pipe-read-fault.test.tstest/js/bun/shell/yield.test.tstest/js/bun/util/bun-cryptohasher.test.tstest/js/node/quic/quic-sni.test.tstest/js/node/worker_threads/worker_threads.test.tstest/js/sql/adapter-env-var-precedence.test.tstest/js/web/crypto/web-crypto.test.tstest/js/web/fetch/fetch.tls.test.ts
Checkpoint of the batch pass; tests follow in a later commit. No-Verification-Needed: unpushed WIP checkpoint; verification runs before push
… across the read loop No-Verification-Needed: unpushed WIP checkpoint; verification runs before push
…i changes No-Verification-Needed: unpushed WIP checkpoint; verification runs before push
No-Verification-Needed: unpushed WIP checkpoint; verification runs before push
No-Verification-Needed: reverts one hunk to origin/main behavior
No-Verification-Needed: user asked to skip /verify; code review in progress on the branch
…fi header staging, test assertions No-Verification-Needed: user asked to skip /verify; review round on an open PR
No-Verification-Needed: test-only gate restore
734d80d to
80834ec
Compare
No-Verification-Needed: test-only reorder
No-Verification-Needed: review follow-up on an open PR; behavior identical (same inode)
No-Verification-Needed: review follow-up on an open PR; covered by the precedence tests
There was a problem hiding this comment.
I've reviewed the latest revision — all four earlier inline findings (verbose-fetch header quoting, the dropped isASAN gate on the cc() tests, the mv directory-branch stat shadowing, and ?ssl=true serverName handling) have been addressed and this pass found nothing new. Given the breadth of security-sensitive changes here — TLS 1.3 resumed-session peer verification in openssl.c, QUIC client-cert enforcement, deferred-symlink tarball extraction, Ed25519 public-half validation, compile-cache directory ownership, and install trust-propagation — a human sign-off is still warranted.
Checked in this pass: the ?ssl=true fix now sets sslMode before the serverName block and the test asserts { serverName: 'h' }; the mv directory arm shadows st/mode from sst; describe.skipIf(isASAN) is restored on the cc() header-directory suite; the us_internal_verify_peer_certificate change correctly narrows the exemption to PSK ciphers only.
Extended reasoning...
Overview
This PR is a broad robustness/hardening pass touching 96 files across install (tarball symlink deferral, manifest-cache name checks, --trust scope, git-dep .bun-tag/node_modules handling, workspace-specifier confinement), the shell (cross-device mv TOCTOU, $${var} interpolation, IOReader keepalive), TLS/QUIC (TLS 1.3 resumed-session authorized, node:quic verifyPeer enforcement, HTTP/3 requestCert, tls.connect({socket, host}) identity checks), Bun.serve HTTP/3 (header validation, :authority host filtering), the HTTP client (Content-Length buffer sizing, credential redaction in verbose logs), SQL (MySQL split-packet reassembly, ssl/sslmode option parsing, Postgres statement-map identity check, invalid-UTF-8 column names), crypto (Ed25519 public-half check, CryptoHasher argument coercion order), and assorted runtime corners (worker env inlining, NODE_TLS_REJECT_UNAUTHORIZED delete, compile-cache dir ownership, Windows ffi header dir naming, zlib params buffer reset, BundlerPlugin filter type checks, macro AST-store reset nesting).
Security risks
Nearly every subsystem touched has a security angle: us_internal_verify_peer_certificate drops the TLS 1.3 session-reused exemption so a resumed session without a client cert no longer reports authorized === true; QUIC now enforces rejectUnauthorized at connection accept and validates HTTP/3 field names/values against RFC 9113 token rules; tarball extraction defers all symlinks until after file/dir entries so no later entry is created through an archive-provided link; module.enableCompileCache() refuses group/world-writable or non-owned directories with O_NOFOLLOW; Ed25519 JWK import verifies the trailing public half matches the seed-derived key via constant-time compare; Windows --shell=system rejects cmd.exe metachars in passthrough args; bun publish no longer looks up xdg-open in the package cwd. These are hardenings, but each is a behavior change on a code path where the wrong direction is a vulnerability.
Level of scrutiny
High. This is exactly the class of PR the approval guidelines say not to auto-approve: large, cross-cutting, and concentrated in TLS/crypto/permissions/extraction paths. Each individual change is small and well-tested (94 new regression tests, each in the existing test file for its module), but the aggregate surface is too wide for an automated sign-off.
Other factors
Four prior inline findings from earlier passes were all addressed in follow-up commits (184c03d3, 2b8cd791, 80834ece, and the earlier review-feedback commit). CodeRabbit's threads are all resolved or withdrawn. The remaining unresolved inline comment (?ssl=true serverName) is now fixed in the diff — the 'true' | '1' branch bumps sslMode to prefer and the test asserts the full { serverName: 'h' } shape. No new issues surfaced in this run.
| /// parsed dir component (`name.dir`, NOT `text`). | ||
| pub fn is_node_module(&self) -> bool { | ||
| crate::strings::contains(self.name().dir, crate::NODE_MODULES_NEEDLE) | ||
| crate::strings::contains_case_insensitive_ascii( |
There was a problem hiding this comment.
Let's revert this one. contains_case_insensitive_ascii is a windowed strncasecmp per byte offset (~180ns on a typical path vs ~56ns for the memmem it replaces), and is_node_module() runs per parsed file (ParseTask, HMR lowering, json) and per package resolution in the resolver. Nobody runs into a directory spelled NODE_MODULES — not worth a slower primitive on that path.
There was a problem hiding this comment.
Reverted — is_node_module() is back to the plain contains, and the test that exercised the alternate-casing path is removed.
#39445) Stacked on #36463 (the base branch is that PR's branch, so the diff here is only the additions). Merging this into #36463 adds the behavior changes listed below; #36463 itself now covers the #38333 install batch, the optional-peer correction, and the TOML / `bun init` fixes, so this PR no longer touches those. ### Problem - These 1.3 to 1.4 behavior changes are not in the guide at `701b3e2a0`: - MySQL: the first `caching_sha2_password` connection over plain TCP is refused unless `allowPublicKeyRetrieval: true` (#31129; 1.3.14 requested the key automatically, `MySQLConnection.zig` in the 1.3.14 tag). SQL `tls` / `ssl` options now require TLS instead of falling back to plaintext, and `?ssl=` / `?ssl-mode=` are read (`shared.ts` 1.3.14 only read `?sslmode=`; #37669). - Install: `~/.npmrc` fallback when `XDG_CONFIG_HOME` is set (#36289), credentials in `--registry` / env / bunfig object URLs are sent and outrank same-host `.npmrc` tokens (#38796, #38824), `bun outdated` exits 1 on fetch failures (#38809), new `dedupe` / `up` commands shadow scripts of those names and `bun feedback` is removed (#38333, #38444), `workspace:` ranges inside registry packages (#37669), isolated store entry names (#39014). - Runtime: `module.enableCompileCache()` / `NODE_COMPILE_CACHE` implemented (#34660), `require()` / `import` not-found messages (#34660), `AbortError` message without the period (#39277; 1.3.14's `BunCommonStrings.h` has the period), GCM IV length (#34092), `mkdtemp("")` (#34908), vm options (#38381), `server.reload` (#38697), ICU 75/73 to 78 (#38013), Compression stream chunking (#38695), `Bun.SQL` sqlite bindings (#35950). - Bundler: `splitting` with `cjs` / `iife` is an error (#32685), block-scoped `enum` lowers to `let` (#34249), exports emitted ascending instead of descending (#35957; `doStep5.zig` in 1.3.14 used `sortDesc`), minified `$` (#35668). - The TOML integer bullet did not say what the limit or the fix is. ### Fix - Adds a MySQL public key section (plus a summary table row), a TLS note under the `PGSSLMODE` section, an `.npmrc` / credentials addendum to the `bunfig.toml` section, a `module.enableCompileCache()` section, and the rest as bullets in the existing lists. - `docs/pm/overrides.mdx`: one-line change adding a pointer to this guide in the existing `lockfileVersion` 3 limitation. (The base branch briefly had a duplicate "Nested overrides" section; it removed that itself in `8257d01acb`, and this PR was rebased over it.) - Verification: each runtime claim was run against `1.4.0-canary.1+8326d1bd3` (22 commits behind main; contains every change referenced), and each install or bundler claim was checked against the source on main, with the 1.3 side taken from the `bun-v1.3.14` tag where the PR body did not state it. The `/runtime/sql#mysql` and `/upgrade-to-1.4` links resolve. `prettier --check` passes. ### Not included on purpose - Lifecycle scripts no longer receiving `npm_package_name` / `npm_package_version` / `npm_package_json` / `npm_config_local_prefix` during `bun install`, and transitive `"*"` ranges no longer deduplicating onto the root's version: regressions with open fixes (#36690, #38110, #38770). They need either the fixes or a guide line before release. - Postgres `sslmode=prefer` / `allow` (including `PGSSLMODE=prefer`, which 1.4 newly reads) hangs until the connection timeout against a server without SSL because nothing sends the startup message after the `N` reply. Same code in 1.3.14; filed as a bug instead of documented. <details> <summary>Commands used to verify the runtime claims</summary> ``` timers/promises setTimeout with an aborted signal # "The operation was aborted" bun req.cjs # Cannot find module ... Require stack: bun b.mjs (import() of a missing package / relative file) # Cannot find package 'x' imported from /path, ERR_MODULE_NOT_FOUND bun a_static.mjs (unhandled static import) # printed line still: Cannot find package 'x' from '/path' process.versions.icu # 78.3 createCipheriv("aes-128-gcm", key, Buffer.alloc(129)) # ERR_CRYPTO_INVALID_IV DecompressionStream of a 1 MiB gzip member # 16 chunks of 65536 bytes new SQL("sqlite://:memory:") with ${[1,2]} / ${new Date()} # Binding expected ... fs.mkdtempSync("") # EINVAL vm.runInThisContext("1", []) # ERR_INVALID_ARG_TYPE NODE_COMPILE_CACHE=/tmp/cc bun cc.cjs # creates /tmp/cc/v1.4.0-x86_64-<sha>-<uid> NODE_DISABLE_COMPILE_CACHE=1 + enableCompileCache() # status 3 (DISABLED) bun dedupe / bun up with package.json scripts of those names # built-in command runs bun feedback # Script not found "feedback" Bun.build({ splitting: true, format: "cjs" }) # Code splitting is currently only supported ... bun build of a function-scoped enum and import * as ns # let Color; exports a, m, z new SQL({ url: "postgres://...", tls: true }) on a non-TLS server # ERR_POSTGRES_TLS_NOT_AVAILABLE Bun.TOML.parse("a = 9007199254740993") # Integer cannot be losslessly represented ... ``` </details> <details> <summary>Previous revision</summary> The first revision of this PR (`3c5611454a`) also rewrote the package manager section for #38333 / #38853 (nested overrides and `lockfileVersion: 3`, the optional-peer correction, `bun update`, `bunfig.toml` over `.npmrc`, `--filter`) and fixed the TOML date and `bun init` lines. #36463 picked those up in its own commits the same day, so this PR was rebased onto its new head and reduced to the items above. </details> <!-- robobun:evidence:begin --> --- **no test proof** · iteration 0 · docs-only change; test-proof not applicable <!-- robobun:evidence:end -->
2 participants
Summary
A broad robustness and input-handling pass across install, the shell, TLS/QUIC, HTTP/3 serving, the HTTP client, SQL clients, crypto, and a few runtime corners, ahead of the release. Each item below is a small, self-contained behavior change with a regression test in the existing test file for that module. No API is removed.
Behavior notes, by area
install
workspace:specifiers are only honored in the root and workspace manifests; aworkspace:range inside a downloaded package resolves like any other unresolvable range.--trust <pkg>applies to the packages actually resolved in that package's dependency tree, and the names written totrustedDependenciesare the same identitybun installchecks on later runs.node_modulessymlink in the checkout is not carried intonode_modules/<pkg>/, and.bun-tagis written without following an existing entry. Committednode_modulesdirectories (bundleDependencies) are untouched.bun install --verbosemasksAuthorization/Proxy-Authorizationheader values and credential-bearing URLs in request logs; bunfigpasswordvalues are redacted in config diagnostics.shell
mvopens the source without following a swapped-in link and takes ownership/mode from the opened descriptor.$${value}: an interpolated value directly after a literal$is always passed as data, never joined into a variable name.cat > /dev/full, closed pipes).bun run --shell=systemon Windows rejects pass-through arguments thatcmd.exewould reinterpret.TLS / node:tls / node:quic
authorized === false(node:tls, node:https,Bun.listen), same as its original handshake.node:quic: with the defaultverifyPeer, a client whose server certificate does not validate refuses natively before any queued stream data is sent and before peer streams/datagrams are delivered;session.openedrejects with the same error as before. Servers withverifyClient: truerequire a client certificate during the handshake.tls.connect({ socket, host: <ip> })checks the certificate against that IP;Bun.connectoverunix:/fd:withtlschecks againstserverNameorlocalhost.Http2SecureServerconnections injected viaemit('connection')use the server's full TLS option set (min/max version, ciphers, crl).delete process.env.NODE_TLS_REJECT_UNAUTHORIZEDrestores the default and later assignments still take effect.Bun.serve / HTTP/3
:authoritygoes through the same host validation asHost.http3: true,tls.requestCert/rejectUnauthorizedare enforced on QUIC connections as they are on TCP.openat2resolution after an unrelatedEPERM/EINVAL.fetch / HTTP client
Content-Length.SQL
?ssl=,?ssl-mode=/sslmode=spellings andssl: "verify-full"-style strings select the correspondingsslmode;tls: { caFile }enables verification likecadoes; an explicit option object still wins over the URL.crypto
Bun.SHA*/MD5update()andBun.CryptoHasher.hash()coerce every argument before touching hasher state.runtime / bundler / misc
Buffer.write/fill/TextEncoder.encodeIntowith a string ending in a lone high surrogate size the fast path from the full input.process.envat runtime like the main thread instead of inlining values at transpile time.module.enableCompileCache()only uses a cache directory owned by the current user with private permissions.bun:fficc()creates a fresh private compiler-runtime header directory per process.zlibparams()resets stream pointers beforedeflateParams;BundlerPluginnative filter registration type-checks its arguments; destructuring a macro result with repeated keys and nested macro evaluation keep the AST store consistent.bun publishlaunches the browser opener by absolute path.Tests
93 new regression tests across 39 existing test files; each fails on the current release and passes on this branch (three are platform-gated: Windows
--shell=system, non-ASANcc(), and macOS-only extractor assertions run in reduced form elsewhere).cargo clippy --workspace,bun run rust:check-all(10/10 targets), oxlint and prettier are clean.Related
Larger items from the same pass are in their own PRs: #37590 (security scanner receives every resolution) and follow-ups for install-cache entry identity, off-thread I/O into WebAssembly memory, and TLS common-name fallback parity with Node.js.