Skip to content

Fix panic when require.resolve options.paths contains non-absolute paths #32680

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
robobun wants to merge 1 commit into from
Closed

Fix panic when require.resolve options.paths contains non-absolute paths #32680

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 15 additions & 6 deletions src/resolver/resolver.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2091,12 +2091,21 @@
bun_core::hint::cold();
for custom_path in custom_paths {
let custom_utf8 = custom_path.to_utf8_without_ref();
match self.check_package_path(
custom_utf8.slice(),
import_path,
kind,
global_cache,
) {
let mut abs_buf;
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,
}
};

Check failure on line 2107 in src/resolver/resolver.rs

View check run for this annotation

Claude / Claude Code Review

Panic still reachable on POSIX with Windows-style drive paths in options.paths 

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"`)
Comment on lines +2095 to +2107

@claude claude Bot Jun 24, 2026

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.

🔴 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 just p[0] == b'/'. For b"C:/foo", 'C' != '/' → false → falls into the else branch.
  • 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 (not Windows) and cfg!(windows) == false → does not take the Windows-specific impl.
    • Line 1783: P::P.is_absolute(parts[0]) with P::P == Loose → is_absolute_windows_t (paths/lib.rs:77-93). For b"C:/foo": len >= 3 && p[1] == ':' && p[2] == '/' → true. So cwd is replaced with b"C:/foo" and parts becomes empty (line 1784-1785).
    • Line 1821: leading_separator_index for Loose delegates to Windows first (resolve_path.rs:1322-1324) → matches 'C' in 'A'..='Z' && p[1]==':' && p[2]=='/' → Some(2) (resolve_path.rs:1299-1308). leading_len = 3, prefix b"C:/" is preserved.
    • Final result written to buf: b"C:/foo".
  • Downstream — check_package_path(b"C:/foo", ...) (resolver.rs:2264) immediately calls self.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 is assert!, not debug_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"] });
  1. custom_utf8.slice() = b"C:/foo".
  2. bun_paths::is_absolute(b"C:/foo") → b'C' != b'/' → false → else branch.
  3. abs_buf_checked([b"C:/foo"]) → Loose join: Loose.is_absolute(b"C:/foo") == true → cwd ← b"C:/foo", parts ← [] → result b"C:/foo".
  4. check_package_path(b"C:/foo", ...) → dir_info_cached(b"C:/foo").
  5. 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:

  1. 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:
  2. Verify the output, not the input: after abs_buf_checked returns Some(p), check bun_paths::is_absolute(p) and continue if 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.

match self.check_package_path(custom_slice, import_path, kind, global_cache) {
ResultUnion::Success(res) => return ResultUnion::Success(res),
ResultUnion::Pending(p) => return ResultUnion::Pending(p),
ResultUnion::Failure(p) => return ResultUnion::Failure(p),
Expand Down
45 changes: 45 additions & 0 deletions test/js/node/module/module-resolve-filename-paths.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -199,3 +199,48 @@ test("Module._resolveFilename throws ERR_INVALID_ARG_TYPE if options.paths is no
Module._resolveFilename("path", __filename, false, { paths: { 0: "/some/path" } });
}).toThrow();
});

test("require.resolve resolves relative entries in options.paths against cwd", () => {
const { path: dir, cleanup } = createTempDir("require-resolve-relative-paths", {
"sub/node_modules/rel-path-pkg/package.json": JSON.stringify({ name: "rel-path-pkg", main: "index.js" }),
"sub/node_modules/rel-path-pkg/index.js": "module.exports = 'ok';",
});

const prevCwd = process.cwd();
try {
process.chdir(dir);
const resolved = require.resolve("rel-path-pkg", { paths: ["./sub"] });
expect(resolved).toBe(resolve(dir, "sub/node_modules/rel-path-pkg/index.js"));
} finally {
process.chdir(prevCwd);
cleanup();
}
});

test("require.resolve with non-absolute options.paths does not crash", () => {
let err;
try {
require.resolve("this-package-does-not-exist-anywhere", { paths: ["not-absolute-dir", ""] });
} catch (e) {
err = e;
}
expect(err).toBeDefined();
expect(err.code).toBe("MODULE_NOT_FOUND");

// Non-string entries: Node throws ERR_INVALID_ARG_TYPE, Bun coerces to string.
// Either way this must throw, not crash the process.
expect(() => {
require.resolve("this-package-does-not-exist-anywhere", { paths: [Int16Array] });
}).toThrow();
});

test("Module._resolveFilename with non-absolute options.paths does not crash", () => {
let err;
try {
Module._resolveFilename("this-package-does-not-exist-anywhere", null, false, { paths: ["not-absolute-dir"] });
} catch (e) {
err = e;
}
expect(err).toBeDefined();
expect(err.code).toBe("MODULE_NOT_FOUND");
});
Loading