[moeghassi]

Summary

GetPackageJSON() in src/node_modules.cc treated every negative return
from ReadFileSync() identically: it negative-cached the path and reported the
manifest as absent. As a result a denied read (EACCES/EPERM, or an
anti-malware/EDR block that surfaces as a failed open) is indistinguishable from
ENOENT, so resolution silently falls back to index.js and the package loads
anyway.

This means an on-endpoint scanner cannot stop a require()/import of a
quarantined package by denying/quarantining its package.json — Node treats the
denied manifest as "no manifest" and runs the code.

Change

• Only ENOENT and ENOTDIR now mean "there is no package.json here"
  (unchanged behavior, still negative-cached).
• Any other read error is treated as a security-relevant signal and throws
  ERR_ACCESS_DENIED instead of falling back — so a denied/quarantined manifest
  aborts resolution for every package style (default index.js, "main", and
  "exports"), not just exports-based ones. The failure is intentionally
  not negative-cached so the deny isn't latched for the process lifetime.
• TraverseParent and GetPackageScopeConfig now stop walking parent scopes
  when an exception is pending, so the thrown error propagates instead of being
  swallowed by continuing up the tree.

Adds a POSIX regression test (chmod(0) the manifest → asserts
ERR_ACCESS_DENIED and that the index.js fallback did not run; skipped on
Windows and when running as root).

Notes for reviewers

• This is a behavior change to module resolution (fail-open → fail-closed on
  non-ENOENT read errors). It may warrant a semver-major label: users with
  a legitimately unreadable ancestor directory (EACCES while walking up for
  package-type detection) would now get a thrown error instead of silent
  fallback. I'm happy to scope the throw more narrowly (e.g. only the target
  package's own manifest, not ancestor type walks) if preferred.
• cc @nodejs/loaders @nodejs/security-wg — feedback on the desired posture and
  scope is very welcome.

Local validation

Validated on Windows x64 on October 27, 2026, after rebasing onto
nodejs/node main at 75191c45afebca2e633eeb94ab95ccfdd7cfae76.

• Build: release node.exe and cctest.exe built successfully with
  user-local LLVM/clang-cl 21.1.8, Visual Studio 2022 libraries, and the Windows SDK.
• JavaScript tests: 500 passed, 14 skipped, 0 failed across the
  es-module and module-hooks suites and the parallel module, require, and
  permission tests. Skips reflect platform/environment constraints, including
  the POSIX-only chmod(0) case.
• Native C++ tests: 296 passed, 0 failed.
• Actual Windows access-denial checks: additional local ACL-based checks
  passed for default index.js, "main", and "exports" packages. Both
  require() and ESM import rejected denied manifests with ERR_ACCESS_DENIED,
  no fallback code executed, and loading recovered after access was restored.
  These supplemental checks are separate from the JavaScript test count above.
• Lint and formatting: changed JavaScript tests passed ESLint;
  src/node_modules.cc passed the repository-pinned clang-format 15,
  cpplint, and checkimports checks; git diff --check passed.

The added portable regression test covers malformed and directory manifests,
preservation of the first error, repeated failed lookups, and recovery after
repair. It exercises parent traversal, private-package root traversal, and both
ESM package-scope lookup variants.

Before the rebase, a separate LLVM-instrumented build confirmed execution of all
three previously uncovered exception-return paths and both outcomes of their
guards, including both ESM template variants. The new regression test was rerun
successfully on the rebased build; no refreshed Codecov percentage is claimed.

[mcollina]

lgtm

I’m unsure about the semversiness of this, as it’s a substantial behavior change.

[mcollina]

unfortunately it needs a rebase now

[moeghassi]

@mcollina rebased.

[mcollina]

lgtm

[mcollina]

@nodejs/tsc ptal

[codecov]

Codecov Report

❌ Patch coverage is 45.45455% with 6 lines in your changes missing coverage. Please review.
✅ Project coverage is 90.49%. Comparing base (ae22341) to head (9c6c2a4).
⚠️ Report is 106 commits behind head on main.

Files with missing lines	Patch %	Lines
src/node_modules.cc	45.45%	3 Missing and 3 partials ⚠️

Additional details and impacted files

@@           Coverage Diff           @@
##             main   #65030   +/-   ##
=======================================
  Coverage   90.49%   90.49%           
=======================================
  Files         797      797           
  Lines      278362   278367    +5     
  Branches    53491    53489    -2     
=======================================
+ Hits       251906   251916   +10     
+ Misses      16815    16812    -3     
+ Partials     9641     9639    -2     

Files with missing lines	Coverage Δ
src/node_modules.cc	78.75% <45.45%> (-0.73%)	⬇️

... and 22 files with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[moeghassi]

@mcollina fixed the outdated error assertion and C++ formatting, and added tests for the uncovered exception paths and recovery behavior.