Skip to content

test scanner: skip paths exceeding MAX_PATH_BYTES instead of panicking - #32412

Closed
robobun wants to merge 3 commits into
mainfrom
farm/34634166/test-scanner-deep-path-panic
Closed

test scanner: skip paths exceeding MAX_PATH_BYTES instead of panicking#32412
robobun wants to merge 3 commits into
mainfrom
farm/34634166/test-scanner-deep-path-panic

Conversation

@robobun

@robobun robobun commented Jun 16, 2026

Copy link
Copy Markdown
Collaborator

Fixes Sentry BUN-3JDD (Panic: range end index 1041 out of range for slice of length 1023).

Repro

d=$(mktemp -d); cd "$d"
seg=$(printf 'a%.0s' {1..200})
# 22 levels → ~4400 byte abs path on Linux; 6 levels → ~1400 on macOS
for i in $(seq 1 22); do mkdir "$seg" && cd "$seg"; done
cd "$d" && bun test
panic: range end index 4236 out of range for slice of length 4095

Stack: bun testScanner::scanabs_bufjoin_abs_string_buf<Loose>_join_abs_string_bufnormalize_string_bufnormalize_string_generic_tzcore::slice::index::slice_index_fail.

Cause

The test scanner walks the tree by joining [parent_abs_path, entry_name] into a fixed PathBuffer ([u8; MAX_PATH_BYTES]: 1024 on macOS, 4096 on Linux). _join_abs_string_buf concatenates the parts into an unbounded JoinScratch (heap-grows past MAX_PATH_BYTES), then calls normalize_string_buf with the fixed output buffer sliced to buf[leading_len..] (1023 / 4095 bytes after the leading /). normalize_string_generic_tz copies each segment with no bounds check at resolve_path.rs:1119, so the first segment that crosses the limit panics.

The Zig reference (normalizeStringGenericTZ) used an unchecked @memcpy here, so this was pre-existing UB in the Zig build that Rust's bounds checking now catches as a panic.

Fix

Switch every path join in Scanner.rs to the existing join_abs_string_buf_checked / abs_buf_checked variant, which returns None when the normalized result does not fit. On overflow the scanner now skips that entry (directories are not descended into, files are not added) instead of aborting the whole bun test run. The OS would reject such paths with ENAMETOOLONG anyway.

The initial scan path (bun test <huge-arg>) returns DoesNotExist on overflow, matching what the user would see from the failed open.

Verification

$ USE_SYSTEM_BUN=1 bun test test/cli/test/bun-test.test.ts -t "skips directories whose absolute"
(fail) ... panic: range end index 4249 out of range for slice of length 4095

$ bun bd test test/cli/test/bun-test.test.ts -t "skips directories whose absolute"
(pass) test file discovery (scanner) > skips directories whose absolute path exceeds MAX_PATH_BYTES instead of panicking

Existing scanner / path-ignore-pattern tests all pass.

The test file scanner walks the directory tree by repeatedly joining
parent directory paths with entry names into a fixed-size PathBuffer
(MAX_PATH_BYTES: 1024 macOS, 4096 Linux). When a tree is deep enough
that the absolute path exceeds this limit, the unchecked join panics
in normalize_string_generic_tz with a slice index error.

Use the existing checked join variant at each join site in the scanner
and skip entries whose joined path would overflow. The OS would reject
such paths with ENAMETOOLONG anyway, so skipping is the correct
behavior rather than aborting the entire test run.

Fixes Sentry BUN-3JDD.
@robobun

robobun commented Jun 16, 2026

Copy link
Copy Markdown
Collaborator Author
Updated 1:01 PM PT - Jun 16th, 2026

@robobun, your commit f596e11ba7616727c772a2e44e84ad5f37529795 passed in Build #62850! 🎉


🧪   To try this PR locally:

bunx bun-pr 32412

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

bun-32412 --bun

@coderabbitai

coderabbitai Bot commented Jun 16, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: a778fa25-5a3a-432e-9124-c9f254a9f41f

📥 Commits

Reviewing files that changed from the base of the PR and between 1c57c41 and f596e11.

📒 Files selected for processing (1)
  • test/cli/test/bun-test.test.ts

Walkthrough

Scanner.rs replaces all unchecked absolute-path join calls with checked variants that return Option, mapping failures to ScanError::DoesNotExist in scan() or skipping entries during subdirectory traversal and next(). A regression test verifies that directory trees with paths exceeding MAX_PATH_BYTES are skipped gracefully without panicking.

Changes

Scanner checked path construction and regression test

Layer / File(s) Summary
Import updates and abs_buf_projected checked helper
src/runtime/cli/test/Scanner.rs
ZStr import made unconditional; join import switched to join_abs_string_buf_checked; abs_buf_projected changed to return Option<&[u8]> using the checked join.
scan() and subdirectory traversal with checked joins
src/runtime/cli/test/Scanner.rs
scan() maps checked join failure to ScanError::DoesNotExist; both non-windows and windows subdirectory loops reserve a NUL byte and skip entries whose checked join fails.
next() directory pruning and file filtering
src/runtime/cli/test/Scanner.rs
matches_path_ignore_pattern and file candidate processing both call checked abs_buf_projected; None results in early return or the candidate being ignored.
Regression test: deep path exceeding MAX_PATH_BYTES
test/cli/test/bun-test.test.ts
Adds isWindows import and a new test.skipIf(isWindows) block that builds a deep directory chain, runs bun test, and asserts exit code 0 with 1 pass and the shallow test output.

Possibly related PRs

  • oven-sh/bun#31850: Directly modifies the same abs_buf_projected usage and next()/scan() path-projection logic in Scanner.rs, making it a close predecessor to this change.
🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and specifically describes the main change: switching the scanner from panicking on oversized paths to skipping them.
Description check ✅ Passed The description thoroughly addresses both required template sections with detailed problem analysis, root cause explanation, fix approach, and verification results.
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.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


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

Comment thread test/cli/test/bun-test.test.ts Outdated
On musl (Alpine), getcwd(3) fails once the cumulative path exceeds
PATH_MAX. bash prints a cd warning for each subsequent level but
mkdir/cd still succeed and exit 0. Filter the known warning before
asserting stderr is clean, matching the existing handling in
test/js/bun/glob/path-length.test.ts.

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

LGTM — the musl stderr filter was applied in f596e11, and the checked-join swap looks correct.

Extended reasoning...

Overview

This PR fixes a Sentry-reported panic (range end index N out of range for slice of length 1023/4095) in the bun test file scanner. It touches two files: src/runtime/cli/test/Scanner.rs swaps every unchecked abs_buf / join_abs_string_buf call for the existing _checked variants that return Option, and handles None by skipping the entry (or returning ScanError::DoesNotExist for the initial scan argument). test/cli/test/bun-test.test.ts adds a regression test that builds a directory chain past MAX_PATH_BYTES and asserts bun test runs the shallow test instead of panicking.

Security risks

None. This is a defensive bounds-check change in the test-file discovery scanner. No auth, crypto, network, or permission logic is touched; the only behavioral change is that pathologically deep directory entries are silently skipped (the OS would have rejected them with ENAMETOOLONG anyway) instead of aborting the whole process.

Level of scrutiny

Low–medium. The transformation is mechanical: each call site goes from foo(...).len() / foo(...) to a let Some(...) = foo_checked(...) else { skip } pattern. The Windows branch was refactored to inline what abs_buf_z did (checked join + manual NUL byte + ZStr::from_raw), mirroring the non-Windows branch directly above it; reserving buf_len - 1 bytes for the join so the NUL fits at open_dir_buf[path2_len] is correct. abs_buf_checked and join_abs_string_buf_checked are pre-existing helpers in resolver/lib.rs and paths/resolve_path.rs, not new code.

Other factors

I previously flagged that the new test's setup-stderr assertion would fail on musl CI due to bash getcwd warnings; the author applied the same filter used in test/js/bun/glob/path-length.test.ts in f596e11 and resolved the thread. The bug-hunting pass on the latest revision found no issues. The Buildkite musl failure in the robobun comment is an LTO/llvm-link data-layout build infrastructure error, unrelated to the source changes here. No CODEOWNERS apply to these paths.

@robobun

robobun commented Jul 25, 2026

Copy link
Copy Markdown
Collaborator Author

Issue #35728 reports the same panic ("range end index N out of range for slice of length 1023") reached from a different direction: a single bun test positional argument of 997+ bytes on macOS (PATH_MAX 1024), since Scanner::scan joins argv into the fixed buffer unchecked. The scan() change in this PR covers that case too.

Branch farm/29eadf7b/test-scanner-long-positional has a minimal version of the same fix (checked join in scan() only, overflow mapped to DoesNotExist so it prints "had no matches" and exits 1) plus an argv-based test that runs on all platforms, in case it is useful when rebasing: a 5000-byte ./xaaa...test.ts positional, asserting "had no matches" and exit code 1. Verified failing on current main and passing with the fix.

@robobun

robobun commented Jul 26, 2026

Copy link
Copy Markdown
Collaborator Author

Superseded by #35863, which carries the same Scanner changes rebased onto current main and additionally bounds _join_abs_string_buf itself so the ~40 other callers no longer panic on over-length input.

@robobun

robobun commented Aug 15, 2026

Copy link
Copy Markdown
Collaborator Author

Closing this one. #35863 now carries exactly this Scanner change rebased onto current main (the extra join_abs_string_buf hardening it used to add on top has been dropped), and the deep-tree scenario from this PR is covered there by two directory-walk cases in test/cli/test/bun-test.test.ts, next to the over-long positional argument cases from #35728.

@robobun robobun closed this Aug 15, 2026
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.

1 participant