Fix panic when require.resolve options.paths contains non-absolute paths#32680
Fix panic when require.resolve options.paths contains non-absolute paths#32680robobun wants to merge 1 commit into
Conversation
The resolver asserted that custom_dir_paths entries passed to check_package_path were absolute. require.resolve and Module._resolveFilename pass user-supplied paths directly, so a relative string (or any value coerced to a non-absolute string) triggered a panic. Resolve relative entries against top_level_dir before looking up dir info, matching how Node.js treats relative entries in options.paths via path.resolve().
|
Updated 2:38 PM PT - Jun 24th, 2026
❌ @robobun, your commit fa253d5 has 1 failures in
🧪 To try this PR locally: bunx bun-pr 32680That installs a local version of the PR into your bun-32680 --bun |
|
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 (2)
Walkthrough
ChangesRelative options.paths resolution
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
|
Found 1 issue this PR may fix:
🤖 Generated with Claude Code |
|
This PR may be a duplicate of:
🤖 Generated with Claude Code |
|
Closing as a duplicate of #31566, which fixes the same
#31566 has already been through several review rounds and is waiting on a maintainer rerun of two flaky macOS jobs. |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
| let custom_slice = if bun_paths::is_absolute(custom_utf8.slice()) { | ||
| custom_utf8.slice() | ||
| } else { | ||
| // Node.js resolves relative entries in `options.paths` against cwd. | ||
| abs_buf = bun_paths::path_buffer_pool::get(); | ||
| match self | ||
| .fs_ref() | ||
| .abs_buf_checked(&[custom_utf8.slice()], &mut *abs_buf) | ||
| { | ||
| Some(p) => p, | ||
| None => continue, | ||
| } | ||
| }; |
There was a problem hiding this comment.
🔴 The guard uses native bun_paths::is_absolute (POSIX: leading / only), but the else branch joins via abs_buf_checked, which uses platform::Loose semantics — Loose treats Windows drive paths like C:/foo as absolute on any host and replaces the cwd base, so the joined result is still C:/foo, which then fails the native assert!(bun_paths::is_absolute(...)) in dir_info_cached_maybe_log (resolver.rs:4203). On Linux/macOS, require.resolve("pkg", { paths: ["C:/foo"] }) (or "C:\\foo") still panics after this PR. Since options.paths is arbitrary userland input — the very motivation for this fix — either gate on the Loose predicate so the guard matches the join, or re-check the joined result with the native predicate and continue if it isn't native-absolute.
Extended reasoning...
What the bug is
The new guard at resolver.rs:2095 and the join it falls through to at resolver.rs:2102 use different absolute-path predicates, so there is a class of inputs that fails the guard, gets "resolved" by the join, and yet still fails the downstream native-absolute assertion at resolver.rs:4203. Specifically, on POSIX hosts, a Windows-style drive path in options.paths (e.g. "C:/foo" or "C:\\foo") still panics the process after this PR — the exact failure mode the PR is meant to eliminate.
The code path
- Guard predicate —
bun_paths::is_absolute(paths/lib.rs:202) →bun_core::path_sep::is_absolute_native. On#[cfg(not(windows))]this is justp[0] == b'/'. Forb"C:/foo",'C' != '/'→ false → falls into theelsebranch. - Join —
self.fs_ref().abs_buf_checked(&[b"C:/foo"], buf)(resolver/lib.rs:353-355) →join_abs_string_buf_checked::<platform::Loose>(top_level_dir, buf, parts). - Inside
_join_abs_string_buf(resolve_path.rs:1730) on POSIX:- Line 1735:
P::P == Loose(notWindows) andcfg!(windows) == false→ does not take the Windows-specific impl. - Line 1783:
P::P.is_absolute(parts[0])withP::P == Loose→is_absolute_windows_t(paths/lib.rs:77-93). Forb"C:/foo":len >= 3 && p[1] == ':' && p[2] == '/'→ true. Socwdis replaced withb"C:/foo"andpartsbecomes empty (line 1784-1785). - Line 1821:
leading_separator_indexforLoosedelegates toWindowsfirst (resolve_path.rs:1322-1324) → matches'C' in 'A'..='Z' && p[1]==':' && p[2]=='/'→Some(2)(resolve_path.rs:1299-1308).leading_len = 3, prefixb"C:/"is preserved. - Final result written to
buf:b"C:/foo".
- Line 1735:
- Downstream —
check_package_path(b"C:/foo", ...)(resolver.rs:2264) immediately callsself.dir_info_cached(source_dir)→dir_info_cached_maybe_log. On POSIX: not.//.;len < MAX_PATH_BYTES; the#[cfg(windows)]normalization block (4171-4201) is skipped. Reaches:assert!(bun_paths::is_absolute(input_path), ...) // resolver.rs:4203
is_absolute_native(b"C:/foo")on POSIX →'C' != '/'→ panic. This isassert!, notdebug_assert!, so it fires in release builds.
The same trace holds for "C:\\foo" (is_absolute_windows_t accepts p[2] == '\\'; slashes_to_posix_in_place at line 1825 normalizes the prefix to C:/).
Why the existing fix doesn't catch it
The fix assumes abs_buf_checked always returns something native-absolute when given a non-native-absolute input. But abs_buf_checked uses platform::Loose, which intentionally honors Windows drive prefixes on any host (resolve_path.rs:1219, 1322-1324). When a part is Loose-absolute, it replaces the cwd base rather than being appended to it, so the function can return a value that is Loose-absolute but not native-absolute on POSIX. The guard predicate (native) and the join semantics (Loose) simply don't agree.
Step-by-step proof (Linux/macOS)
require.resolve("pkg", { paths: ["C:/foo"] });custom_utf8.slice() = b"C:/foo".bun_paths::is_absolute(b"C:/foo")→b'C' != b'/'→false→ else branch.abs_buf_checked([b"C:/foo"])→ Loose join:Loose.is_absolute(b"C:/foo") == true→ cwd ←b"C:/foo", parts ←[]→ resultb"C:/foo".check_package_path(b"C:/foo", ...)→dir_info_cached(b"C:/foo").assert!(bun_paths::is_absolute(b"C:/foo"))→false→ panic: "cannot resolve DirInfo for non-absolute path: C:/foo".
Impact
Same severity as the bug being fixed: a user-reachable process crash from arbitrary userland input to require.resolve / Module._resolveFilename on Linux and macOS. The PR description states "any relative string ... crashed the process" as the problem to solve; Windows-style drive paths are still in that crash class on POSIX after this change.
How to fix
Two straightforward options:
- Match the guard to the join: gate on the same predicate the join uses, so anything Loose considers absolute skips the join — but that just moves the problem (it would pass
b"C:/foo"directly). Better: - Verify the output, not the input: after
abs_buf_checkedreturnsSome(p), checkbun_paths::is_absolute(p)andcontinueif false (the path can't name a real directory on this host anyway). Equivalently, drop the input guard entirely and always join, then check the result:let custom_slice = match self.fs_ref().abs_buf_checked(&[custom_utf8.slice()], &mut *abs_buf) { Some(p) if bun_paths::is_absolute(p) => p, _ => continue, };
It would also be worth adding { paths: ["C:/foo"] } to the "does not crash" regression test (POSIX-only) so this input class is covered.
1 participant
What does this PR do?
Fixes a panic in the resolver when
require.resolve(),require(), orModule._resolveFilename()is called withoptions.pathscontaining a non-absolute path.The resolver passed these user-supplied paths directly to
dir_info_cached, which asserts the input is absolute. Sinceoptions.pathscomes from userland, any relative string (or any non-string value that coerces to a non-absolute string) crashed the process.Relative entries are now resolved against the top-level dir before lookup, which matches Node.js behavior (Node routes each entry through
path.resolve()viaModule._nodeModulePaths).How did you verify your code works?
Added regression tests in
test/js/node/module/module-resolve-filename-paths.test.jscoveringrequire.resolveandModule._resolveFilenamewith relative string entries, empty strings, and non-string values. Tests pass in both Bun and Node.js, and crash the unpatched Bun.