From e3bd440f353bf410cf6cd1399db9aa47ff360365 Mon Sep 17 00:00:00 2001 From: Paul Bouchon Date: Tue, 11 Aug 2026 17:25:38 -0400 Subject: [PATCH] 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: https://github.com/nodejs/node/issues/65220 Signed-off-by: Paul Bouchon --- src/node_modules.cc | 18 +++++- .../test-module-unreadable-package-json.js | 57 +++++++++++++++++++ 2 files changed, 74 insertions(+), 1 deletion(-) create mode 100644 test/parallel/test-module-unreadable-package-json.js diff --git a/src/node_modules.cc b/src/node_modules.cc index 717d84a4f89a..30780e12aed1 100644 --- a/src/node_modules.cc +++ b/src/node_modules.cc @@ -111,7 +111,23 @@ 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) { + // No file at this path, a path component that is not a directory, or a + // "package.json" that is itself a directory all mean 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 && + read_error != UV_EISDIR) { + 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}); diff --git a/test/parallel/test-module-unreadable-package-json.js b/test/parallel/test-module-unreadable-package-json.js new file mode 100644 index 000000000000..4a99bfd670cf --- /dev/null +++ b/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);