module: report unreadable package.json

A package.json that exists but cannot be read was treated the same as
one that is not there: the read failure returned no config and
resolution continued as if the package had none. Fields such as
"exports" and "type" silently disappear, so a specifier can resolve to
a different file than the package declares, while an unparsable
package.json already throws ERR_INVALID_PACKAGE_CONFIG.

Keep treating ENOENT and ENOTDIR as "no package config here", and
report any other read failure with the underlying error.

Fixes: #65220
Signed-off-by: Paul Bouchon <mail@bitpshr.net>

Author: bitpshr

src/node_modules.cc

@@ -111,7 +111,22 @@ const BindingData::PackageConfig* BindingData::GetPackageJSON(
   PackageConfig package_config{};
   package_config.file_path = path;
   // No need to exclude BOM since simdjson will skip it.
-  if (ReadFileSync(&package_config.raw_json, path.data()) < 0) {
+  int read_error = ReadFileSync(&package_config.raw_json, path.data());
+  if (read_error < 0) {
+    // A file that is not there, or a path that turns out not to be a
+    // directory, simply means there is no package config here. Any other
+    // failure means a package.json is present but could not be read.
+    // Treating that as absent silently drops fields such as "exports" and
+    // "type", which can resolve a specifier to a different file, so surface
+    // the read error instead of continuing.
+    if (read_error != UV_ENOENT && read_error != UV_ENOTDIR) {
+      THROW_ERR_INVALID_PACKAGE_CONFIG(realm->isolate(),
+                                       "Cannot read package config %s: %s.",
+                                       path.data(),
+                                       uv_strerror(read_error));
+      return nullptr;
+    }
+
     // Add `nullopt` to the package config cache so that we don't
     // need to open and attempt to read this path again
     binding_data->package_configs_.insert({std::string(path), std::nullopt});

test/parallel/test-module-unreadable-package-json.js

@@ -0,0 +1,57 @@
+'use strict';
+
+// A package.json that exists but cannot be read must not be treated as
+// absent. Doing so silently drops fields such as "exports", which can resolve
+// a specifier to a different file than the one the package declares.
+// Refs: https://github.com/nodejs/node/issues/65220
+
+const common = require('../common');
+
+if (common.isWindows) {
+  common.skip('chmod does not restrict reads on Windows');
+}
+if (process.getuid?.() === 0) {
+  common.skip('cannot make a file unreadable as root');
+}
+
+const assert = require('assert');
+const fs = require('fs');
+const path = require('path');
+const { spawnSync } = require('child_process');
+const tmpdir = require('../common/tmpdir');
+
+tmpdir.refresh();
+
+const depDir = tmpdir.resolve('node_modules/dep');
+fs.mkdirSync(path.join(depDir, 'lib'), { recursive: true });
+const depPackageJson = path.join(depDir, 'package.json');
+fs.writeFileSync(
+  depPackageJson,
+  '{"name":"dep","exports":{".":"./lib/real.js"}}',
+);
+fs.writeFileSync(path.join(depDir, 'lib', 'real.js'), 'export const which = "real";');
+// If the package config is ignored, resolution falls back to this file.
+fs.writeFileSync(path.join(depDir, 'index.js'), 'export const which = "decoy";');
+
+fs.writeFileSync(tmpdir.resolve('package.json'), '{"type":"module"}');
+const entry = tmpdir.resolve('main.mjs');
+fs.writeFileSync(entry, 'import { which } from "dep"; console.log(which);');
+
+// Sanity check: the export resolves while the package config is readable.
+{
+  const child = spawnSync(process.execPath, [entry], { encoding: 'utf8' });
+  assert.strictEqual(child.status, 0, child.stderr);
+  assert.strictEqual(child.stdout.trim(), 'real');
+}
+
+fs.chmodSync(depPackageJson, 0o000);
+
+{
+  const child = spawnSync(process.execPath, [entry], { encoding: 'utf8' });
+  // The read failure must be reported rather than resolving to index.js.
+  assert.notStrictEqual(child.status, 0);
+  assert.doesNotMatch(child.stdout, /decoy/);
+  assert.match(child.stderr, /Cannot read package config/);
+}
+
+fs.chmodSync(depPackageJson, 0o644);