fixup: throw on relative statLocation with remediation instructions

nodejs · JakobJingleheimer · Sep 30, 2024 · Oct 1, 2024 · Oct 1, 2024 · Oct 1, 2024
commit b5314979498275d7cea1e3cc1c88c2f6645d6a15
diff --git a/lib/internal/modules/package_json_reader.js b/lib/internal/modules/package_json_reader.js
@@ -2,11 +2,18 @@
 
 const {
   ArrayIsArray,
+  ArrayPrototypeJoin,
   JSONParse,
   ObjectDefineProperty,
+  StringPrototypeEndsWith,
 } = primordials;
+const {
+  codes: {
+    ERR_INVALID_ARG_VALUE,
+  },
+} = require('internal/errors');
 const modulesBinding = internalBinding('modules');
-const { resolve } = require('path');
+const path = require('path');
 const { kEmptyObject } = require('internal/util');
 const { fileURLToPath, URL } = require('internal/url');
 const {
@@ -111,7 +118,7 @@ function read(jsonPath, { base, specifier, isESM } = kEmptyObject) {
  */
 function readPackage(requestPath) {
   // TODO(@anonrig): Remove this function.
-  return read(resolve(requestPath, 'package.json'));
+  return read(path.resolve(requestPath, 'package.json'));
 }
 
 /**
@@ -122,10 +129,26 @@ function readPackage(requestPath) {
  * @returns {undefined | DeserializedPackageConfig<everything>}
  */
 function getNearestParentPackageJSON(startLocation, everything = false) {
-  const startPath = URL.canParse(startLocation) ? fileURLToPath(startLocation) : startLocation;
+  let startPath = URL.canParse(startLocation) ? fileURLToPath(startLocation) : startLocation;
+
   validateString(startPath, 'startPath');
   validateBoolean(everything, 'everything');
 
+  if (!path.isAbsolute(startPath)) {
+    throw new ERR_INVALID_ARG_VALUE(
+      'startLocation',
+      startLocation,
+      ArrayPrototypeJoin([
+        'must be a fully resolved location. To use a relative location, first wrap with',
+        '`import.meta.resolve(startLocation)` in ESM',
+        'or',
+        '`path.resolve(__dirname, startLocation) in CJS',
+      ], ' '),
+    );
+  }
+
+  if (!StringPrototypeEndsWith(startPath, path.sep)) { startPath += path.sep; }
+
   if (everything) {
     const result = modulesBinding.getNearestRawParentPackageJSON(startPath);
 

diff --git a/test/fixtures/packages/nested/package.json b/test/fixtures/packages/nested/package.json
@@ -0,0 +1 @@
+{"name": "package-with-sub-package"}
diff --git a/test/fixtures/packages/nested/sub-pkg-cjs/index.js b/test/fixtures/packages/nested/sub-pkg-cjs/index.js
@@ -0,0 +1,4 @@
+const { getPackageJSON } = require('node:module');
+const { resolve } = require('node:path');
+
+module.exports = getPackageJSON(resolve(__dirname, '..'));
diff --git a/test/fixtures/packages/nested/sub-pkg-cjs/package.json b/test/fixtures/packages/nested/sub-pkg-cjs/package.json
@@ -0,0 +1 @@
+{"name": "sub-package", "type": "commonjs"}
diff --git a/test/fixtures/packages/nested/sub-pkg-mjs/index.js b/test/fixtures/packages/nested/sub-pkg-mjs/index.js
@@ -0,0 +1,3 @@
+import { getPackageJSON } from 'node:module';
+
+export default getPackageJSON(import.meta.resolve('..'));
diff --git a/test/fixtures/packages/nested/sub-pkg-mjs/package.json b/test/fixtures/packages/nested/sub-pkg-mjs/package.json
@@ -0,0 +1 @@
+{"name": "sub-package", "type": "module"}
diff --git a/test/parallel/test-get-package-json.js b/test/parallel/test-get-package-json.js
@@ -104,3 +104,36 @@ const pkgType = 'module'; // a non-default value
     },
   });
 }
+
+{ // Throws on unresolved location
+  let err;
+  try {
+    getPackageJSON('..');
+  } catch (e) {
+    err = e;
+  }
+
+  assert.strictEqual(err.code, 'ERR_INVALID_ARG_VALUE');
+  assert.match(err.message, /fully resolved/);
+  assert.match(err.message, /relative/);
+  assert.match(err.message, /import\.meta\.resolve/);
+  assert.match(err.message, /path\.resolve\(__dirname/);
+}
+
+{ // Can crawl up (CJS)
+  const pathToMod = fixtures.path('packages/nested/sub-pkg-cjs/index.js');
+  const parentPkg = require(pathToMod);
+
+  assert.strictEqual(parentPkg.data.name, 'package-with-sub-package');
+  const pathToParent = fixtures.path('packages/nested/package.json');
+  assert.strictEqual(parentPkg.path, pathToParent);
+}
+
+{ // Can crawl up (ESM)
+  const pathToMod = fixtures.path('packages/nested/sub-pkg-mjs/index.js');
+  const parentPkg = require(pathToMod).default;
+
+  assert.strictEqual(parentPkg.data.name, 'package-with-sub-package');
+  const pathToParent = fixtures.path('packages/nested/package.json');
+  assert.strictEqual(parentPkg.path, pathToParent);
+}
Original file line number	Diff line number	Diff line change
		@@ -0,0 +1,3 @@
		import { getPackageJSON } from 'node:module';

		export default getPackageJSON(import.meta.resolve('..'));