Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
56 changes: 56 additions & 0 deletions src/bun.js/modules/NodeModuleModule.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ JSC_DECLARE_HOST_FUNCTION(jsFunctionDebugNoop);
JSC_DECLARE_HOST_FUNCTION(jsFunctionFindPath);
JSC_DECLARE_HOST_FUNCTION(jsFunctionIsBuiltinModule);
JSC_DECLARE_HOST_FUNCTION(jsFunctionNodeModuleCreateRequire);
JSC_DECLARE_HOST_FUNCTION(jsFunctionFindPackageJSON);
JSC_DECLARE_HOST_FUNCTION(jsFunctionNodeModuleModuleConstructor);
JSC_DECLARE_HOST_FUNCTION(jsFunctionResolveFileName);
JSC_DECLARE_HOST_FUNCTION(jsFunctionResolveLookupPaths);
Expand Down Expand Up @@ -288,6 +289,60 @@ JSC_DEFINE_HOST_FUNCTION(jsFunctionNodeModuleCreateRequire,
scope, JSValue::encode(Bun::JSCommonJSModule::createBoundRequireFunction(vm, globalObject, val)));
}

extern "C" void Bun__findPackageJSON(JSC::JSGlobalObject* globalObject, BunString* path, BunString* result);

JSC_DEFINE_HOST_FUNCTION(jsFunctionFindPackageJSON,
(JSC::JSGlobalObject * globalObject,
JSC::CallFrame* callFrame))
{
auto& vm = JSC::getVM(globalObject);
auto scope = DECLARE_THROW_SCOPE(vm);

if (callFrame->argumentCount() < 1) {
return Bun::throwError(globalObject, scope,
Bun::ErrorCode::ERR_MISSING_ARGS,
"findPackageJSON() requires at least one argument"_s);
}

auto argument = callFrame->uncheckedArgument(0);
auto val = argument.toWTFString(globalObject);
RETURN_IF_EXCEPTION(scope, {});

// Convert file:// URL to path if needed
if (!isAbsolutePath(val)) {
WTF::URL url(val);
if (!url.isValid()) {
ERR::INVALID_ARG_VALUE(scope, globalObject,
"path"_s, argument,
"must be a file URL or absolute path"_s);
RELEASE_AND_RETURN(scope, {});
}
if (!url.protocolIsFile()) {
ERR::INVALID_ARG_VALUE(scope, globalObject,
"path"_s, argument,
"must be a file URL"_s);
RELEASE_AND_RETURN(scope, {});
}
val = url.fileSystemPath();
}

BunString input = Bun::toString(val);
BunString result;
Bun__findPackageJSON(globalObject, &input, &result);

if (result.tag == BunStringTag::Empty) {
return JSValue::encode(jsNull());
}

auto resultStr = result.toWTFString();
if (!resultStr.isNull()) {
ASSERT(resultStr.impl()->refCount() == 2);
resultStr.impl()->deref();
}

RELEASE_AND_RETURN(scope, JSValue::encode(jsString(vm, resultStr)));
}

JSC_DEFINE_HOST_FUNCTION(jsFunctionSyncBuiltinExports,
(JSGlobalObject * globalObject,
CallFrame* callFrame))
Expand Down Expand Up @@ -834,6 +889,7 @@ builtinModules getBuiltinModulesObject PropertyCallback
constants getConstantsObject PropertyCallback
createRequire jsFunctionNodeModuleCreateRequire Function 1
enableCompileCache jsFunctionEnableCompileCache Function 0
findPackageJSON jsFunctionFindPackageJSON Function 1
findSourceMap Bun__JSSourceMap__find Function 1
getCompileCacheDir jsFunctionGetCompileCacheDir Function 0
globalPaths getGlobalPathsObject PropertyCallback
Expand Down
72 changes: 72 additions & 0 deletions src/bun.js/node/path.zig
Original file line number Diff line number Diff line change
Expand Up @@ -2969,3 +2969,75 @@ const typeBaseNameT = bun.meta.typeBaseNameT;

const strings = bun.strings;
const L = strings.literal;

/// Find the nearest package.json file starting from a given path using the resolver cache
/// Returns the absolute path to the package.json, or an empty string if not found
export fn Bun__findPackageJSON(globalObject: *jsc.JSGlobalObject, input_path: *bun.String, result: *bun.String) void {
var slice = input_path.toUTF8(bun.default_allocator);
defer slice.deinit();

var current_dir = slice.slice();
Comment thread
coderabbitai[bot] marked this conversation as resolved.
if (current_dir.len == 0) {
result.* = bun.String.empty;
return;
}
Comment on lines +2975 to +2983

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.

⚠️ Potential issue | 🟠 Major

Validate that the input path is absolute.

The function documentation and PR objectives state that findPackageJSON should accept "file URLs or absolute paths." However, there is no validation that input_path is absolute before processing. If a relative path is passed (e.g., "src/file.js"), the resolver's readDirInfo may not function correctly, leading to undefined behavior or incorrect results.

Consider adding validation after line 2983:

     var current_dir = slice.slice();
     if (current_dir.len == 0) {
         result.* = bun.String.empty;
         return;
     }
+    
+    // Validate that the input is an absolute path
+    if (!isAbsolutePosixT(u8, current_dir) and !isAbsoluteWindowsT(u8, current_dir)) {
+        result.* = bun.String.empty;
+        return;
+    }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
export fn Bun__findPackageJSON(globalObject: *jsc.JSGlobalObject, input_path: *bun.String, result: *bun.String) void {
var slice = input_path.toUTF8(bun.default_allocator);
defer slice.deinit();
var current_dir = slice.slice();
if (current_dir.len == 0) {
result.* = bun.String.empty;
return;
}
export fn Bun__findPackageJSON(globalObject: *jsc.JSGlobalObject, input_path: *bun.String, result: *bun.String) void {
var slice = input_path.toUTF8(bun.default_allocator);
defer slice.deinit();
var current_dir = slice.slice();
if (current_dir.len == 0) {
result.* = bun.String.empty;
return;
}
// Validate that the input is an absolute path
if (!isAbsolutePosixT(u8, current_dir) and !isAbsoluteWindowsT(u8, current_dir)) {
result.* = bun.String.empty;
return;
}
🤖 Prompt for AI Agents
In src/bun.js/node/path.zig around lines 2975 to 2983, the code does not
validate that input_path is an absolute path or a file URL before proceeding;
add a validation immediately after line 2983 that accepts either a "file://"
prefix or an absolute filesystem path (e.g., starts with '/' on POSIX or matches
a Windows absolute form like /^[A-Za-z]:\\|\/]/), and if the check fails set
result.* = bun.String.empty and return (or raise the same JS error/handling
convention used elsewhere in this function) so relative paths are rejected early
and the resolver is not invoked with invalid input.


// If the input is a file, start from its directory
// Check if it's a regular file by trying to get parent directory
var path_buf_z: [bun.MAX_PATH_BYTES:0]u8 = undefined;
if (current_dir.len >= path_buf_z.len) {
// Path too long to process
result.* = bun.String.empty;
return;
} else {
@memcpy(path_buf_z[0..current_dir.len], current_dir);
path_buf_z[current_dir.len] = 0;
const current_dir_z = path_buf_z[0..current_dir.len :0];

const stat_result = bun.sys.stat(current_dir_z);
if (stat_result == .result) {
const mode = stat_result.result.mode;
const S = bun.S;
// Check if it's a regular file (not a directory)
if ((mode & S.IFMT) == S.IFREG) {
current_dir = if (Environment.isWindows)
dirnameWindowsT(u8, current_dir)
else
dirnamePosixT(u8, current_dir);
}
} else {
// Heuristic: if path doesn't exist and doesn't end with a separator,
// treat it as a file path and start from its dirname.
if (current_dir.len > 0) {
const last = current_dir[current_dir.len - 1];
if ((Environment.isWindows and (last != CHAR_BACKWARD_SLASH and last != CHAR_FORWARD_SLASH)) or
(!Environment.isWindows and last != CHAR_FORWARD_SLASH))
{
current_dir = if (Environment.isWindows)
dirnameWindowsT(u8, current_dir)
else
dirnamePosixT(u8, current_dir);
}
}
}
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.

// Use the resolver's DirInfo cache to find package.json
const bun_vm = globalObject.bunVM();
const resolver = &bun_vm.transpiler.resolver;

// Walk up the directory tree using the resolver cache
// Use std.fs.path.dirname to get null once we reach the root
var search_dir: ?[]const u8 = current_dir;
while (search_dir) |dir| : (search_dir = std.fs.path.dirname(dir)) {
if (resolver.dirInfoCached(dir) catch null) |dir_info| {
if (dir_info.package_json) |pkg_json| {
result.* = bun.String.cloneUTF8(pkg_json.source.path.text);
return;
}
}
}

// Not found
result.* = bun.String.empty;
}
2 changes: 1 addition & 1 deletion src/resolver/resolver.zig
Original file line number Diff line number Diff line change
Expand Up @@ -2612,7 +2612,7 @@ pub const Resolver = struct {
return PackageJSON.new(pkg);
}

fn dirInfoCached(r: *ThisResolver, path: string) !?*DirInfo {
pub fn dirInfoCached(r: *ThisResolver, path: string) !?*DirInfo {
return try r.dirInfoCachedMaybeLog(path, true, true);
}

Expand Down
62 changes: 62 additions & 0 deletions test/js/node/module/node-module-findPackageJSON.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
import { describe, test, expect } from "bun:test";
import { findPackageJSON } from "node:module";
import { pathToFileURL } from "node:url";
import path from "node:path";

describe.concurrent("Module.findPackageJSON", () => {
test.concurrent("finds package.json from file URL", () => {
const fileUrl = pathToFileURL(__filename).href;
const result = findPackageJSON(fileUrl);

expect(typeof result).toBe("string");
expect(path.basename(result!)).toBe("package.json");
expect(path.isAbsolute(result!)).toBe(true);
});

test.concurrent("finds package.json from directory path", () => {
const dirUrl = pathToFileURL(import.meta.dir).href;
const result = findPackageJSON(dirUrl);

expect(typeof result).toBe("string");
expect(path.basename(result!)).toBe("package.json");
expect(path.isAbsolute(result!)).toBe(true);
});

test.concurrent("finds package.json from nested file", () => {
const nestedPath = path.join(import.meta.dir, "../../..");
const fileUrl = pathToFileURL(path.join(nestedPath, "some-file.js")).href;
const result = findPackageJSON(fileUrl);

expect(typeof result).toBe("string");
expect(path.basename(result!)).toBe("package.json");
expect(path.isAbsolute(result!)).toBe(true);
});

test.concurrent("returns null when no package.json found", () => {
// Use a path that's unlikely to have a package.json
const rootPath = path.parse(import.meta.dir).root;
const deepPath = path.join(rootPath, "nonexistent", "deep", "path", "file.js");
const fileUrl = pathToFileURL(deepPath).href;
const result = findPackageJSON(fileUrl);

// Should return null when not found
expect(result).toBeNull();
});

test.concurrent("works with absolute paths as file URLs", () => {
const absolutePath = path.resolve(import.meta.dir, "node-module-findPackageJSON.test.ts");
const fileUrl = pathToFileURL(absolutePath).href;
const result = findPackageJSON(fileUrl);

expect(typeof result).toBe("string");
expect(path.basename(result!)).toBe("package.json");
expect(path.isAbsolute(result!)).toBe(true);
});

test.concurrent("accepts absolute path string", () => {
const result = findPackageJSON(import.meta.dir);
expect(typeof result).toBe("string");
expect(path.basename(result!)).toBe("package.json");
expect(path.isAbsolute(result!)).toBe(true);
});
});
Comment on lines +6 to +62

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.

🧹 Nitpick | 🔵 Trivial

Consider adding edge case tests (optional).

The current test suite provides solid coverage of typical use cases. To further improve robustness, consider adding tests for:

  • Relative path inputs (e.g., "./some/file.js") to verify they're handled appropriately
  • Paths with symbolic links to ensure resolution works correctly
  • Invalid inputs (empty string, malformed URLs) to verify error handling

These are optional enhancements and not blockers for this PR.

🤖 Prompt for AI Agents
In test/js/node/module/node-module-findPackageJSON.test.ts around lines 6 to 62,
the test suite lacks coverage for several edge cases; add new concurrent tests
that (1) pass a relative path string (e.g., "./some/file.js") and assert correct
package.json resolution or null, (2) simulate a path with a symbolic link
(create a temp dir, create a symlink to it, call findPackageJSON on a file path
through the symlink and assert resolution), and (3) verify invalid inputs (empty
string and malformed URL) return null or throw a well-defined error per function
contract; keep tests isolated (use temp dirs and cleanup) and assert expected
types/values consistent with existing tests.