@npmcli/arborist - npm Package Compare versions

Comparing version 9.5.0 to 9.6.0

lib/arborist/build-ideal-tree.js

@@ -31,2 +31,11 @@ // mixin implementing the buildIdealTree method
 const Link = require('../link.js')
+
+// Maps a parsed spec.type to the corresponding allow-* arborist option name.
+// Hoisted to module scope so #checkAllow doesn't re-allocate it per call.
+const ALLOW_OPTION_FOR_TYPE = {
+  git: 'allowGit',
+  remote: 'allowRemote',
+  file: 'allowFile',
+  directory: 'allowDirectory',
+}
 const addRmPkgDeps = require('../add-rm-pkg-deps.js')
@@ -653,2 +662,41 @@ const optionalSet = require('../optional-set.js')
 
+  // Enforces the allow-git / allow-file / allow-directory / allow-remote configs at the arborist resolution layer, before any branching into the symlink (Link) path or the manifest-fetch path.
+  // Pacote also enforces these inside FetcherBase.get() as defense-in-depth, but the symlink branch never reaches pacote, and the manifest cache here would bypass pacote on a cached hit.
+  // Throws the same { code: EALLOW${TYPE} } shape pacote uses, so callers and downstream consumers stay consistent.
+  #checkAllow (spec, edge) {
+    const optName = ALLOW_OPTION_FOR_TYPE[spec.type]
+    if (!optName) {
+      return
+    }
+    const allow = this.options[optName] ?? 'all'
+    if (allow === 'all') {
+      return
+    }
+    const isRoot = !!(edge?.from?.isProjectRoot || edge?.from?.isWorkspace)
+    if (allow !== 'none' && isRoot) {
+      return
+    }
+    throw Object.assign(
+      new Error(`Fetching${allow === 'root' ? ' non-root' : ''} packages of type "${spec.type}" have been disabled`),
+      {
+        code: `EALLOW${spec.type.toUpperCase()}`,
+        package: spec.toString(),
+      }
+    )
+  }
+
+  // Builds a Node representing a spec we failed to load (allow-* gate, network failure, ENOTARGET, etc.) and records it in #loadFailures so #pruneFailedOptional can later decide whether the failure is fatal or silently dropped for optional deps.
+  #failureNode (name, parent, error, edge) {
+    error.requiredBy = edge?.from?.location || '.'
+    const n = new Node({
+      name,
+      parent,
+      error,
+      installLinks: this.installLinks,
+      legacyPeerDeps: this.legacyPeerDeps,
+    })
+    this.#loadFailures.add(n)
+    return n
+  }
+
   #queueNamedUpdates () {
@@ -1045,3 +1093,3 @@ // ignore top nodes, since they are not loaded the same way, and
             promises.push(() =>
-              this.#fetchManifest(npa.resolve(e.name, e.spec, fromPath(placed, e)), parent)
+              this.#fetchManifest(npa.resolve(e.name, e.spec, fromPath(placed, e)), parent, e)
                 .catch(() => null)
@@ -1237,3 +1285,5 @@ )
 
-  async #fetchManifest (spec, parent) {
+  async #fetchManifest (spec, parent, edge) {
+    // Enforce allow-* gates before consulting the manifest cache so a cached entry from a different edge cannot bypass the policy.
+    this.#checkAllow(spec, edge)
     const options = {
@@ -1243,3 +1293,3 @@ ...this.options,
       fullMetadata: true,
-      _isRoot: parent?.isProjectRoot || parent?.isWorkspace,
+      _isRoot: !!(edge?.from?.isProjectRoot || edge?.from?.isWorkspace),
     }
@@ -1261,2 +1311,10 @@ // get the intended spec and stored metadata from yarn.lock file,
   async #nodeFromSpec (name, spec, parent, edge) {
+    // Enforce allow-git / allow-file / allow-directory / allow-remote before any branching, so the symlink (Link) path is enforced as well as the manifest-fetch path.
+    // Route the failure through #loadFailures so optional-dep semantics apply (e.g. a transitive optionalDependencies entry that resolves to a disallowed git URL is silently dropped rather than failing the install).
+    try {
+      this.#checkAllow(spec, edge)
+    } catch (error) {
+      return this.#failureNode(name, parent, error, edge)
+    }
+
     // pacote will slap integrity on its options, so we have to clone the object so it doesn't get mutated.
@@ -1316,19 +1374,22 @@ // Don't bother to load the manifest for link deps, because the target might be within another package that doesn't exist yet.
     // doesn't satisfy the edge. try to fetch a manifest and build a node from that.
-    return this.#fetchManifest(spec, parent)
-      .then(pkg => new Node({ name, pkg, parent, installLinks, legacyPeerDeps }), error => {
-        error.requiredBy = edge.from.location || '.'
-
-        // failed to load the spec, either because of enotarget or
-        // fetch failure of some other sort.  save it so we can verify
-        // later that it's optional; otherwise, the error is fatal.
-        const n = new Node({
-          name,
-          parent,
-          error,
-          installLinks,
-          legacyPeerDeps,
-        })
-        this.#loadFailures.add(n)
-        return n
-      })
+    return this.#fetchManifest(spec, parent, edge)
+      .then(
+        pkg => {
+          // When a proxy/upstream registry returns an incomplete manifest
+          // (e.g. missing version field for platform-specific packages it
+          // hasn't cached), treat it as a load failure so that optional deps
+          // are properly pruned instead of written to the lockfile without
+          // version metadata.  Only apply to registry specs — file: deps
+          // legitimately omit version.
+          if (!pkg.version && spec.registry) {
+            const error = Object.assign(
+              new Error(`incomplete manifest for ${name}, missing version`),
+              { code: 'EINCOMPLETEMANIFEST' }
+            )
+            return this.#failureNode(name, parent, error, edge)
+          }
+          return new Node({ name, pkg, parent, installLinks, legacyPeerDeps })
+        },
+        error => this.#failureNode(name, parent, error, edge)
+      )
   }
@@ -1335,0 +1396,0 @@

lib/arborist/isolated-reifier.js

@@ -100,3 +100,5 @@ const { mkdirSync } = require('node:fs')
 
-    this.idealGraph.workspaces = await Promise.all(Array.from(idealTree.fsChildren.values(), w => this.#workspaceProxy(w)))
+    // Skip extraneous fsChildren: workspaces removed from the root manifest can linger in fsChildren via the lockfile, and re-materializing them here would re-create a directory the user just deleted.
+    const fsChildren = Array.from(idealTree.fsChildren.values()).filter(w => !w.extraneous)
+    this.idealGraph.workspaces = await Promise.all(fsChildren.map(w => this.#workspaceProxy(w)))
     const processed = new Set()
@@ -103,0 +105,0 @@ const queue = [idealTree, ...idealTree.fsChildren]

lib/link.js

@@ -112,8 +112,20 @@ const relpath = require('./relpath.js')
 
-  // When a Link receives overrides (via edgesIn), forward them to the target node which holds the actual edgesOut.
-  // Without this, overrides stop at the Link and never reach the target's dependency edges.
+  // When a Link receives overrides (via edgesIn), forward them to the target node which holds the actual edgesOut — but only when the OverrideSet has at least one rule that names a dep the target actually depends on.
+  // Without this scope, the link forwards a generic ancestor OverrideSet that has no real effect on the target's edges, but still flips the target to "has overrides", which changes downstream `canReplaceWith` / placement decisions and causes `npm ci` to re-resolve lockfile-pinned edges from the registry.
+  // See npm/cli#9357.
   recalculateOutEdgesOverrides () {
-    if (this.target) {
-      this.target.updateOverridesEdgeInAdded(this.overrides)
+    if (!this.target || !this.overrides) {
+      return
     }
+    let hasMatchingRule = false
+    for (const rule of this.overrides.ruleset.values()) {
+      if (this.target.edgesOut.has(rule.name)) {
+        hasMatchingRule = true
+        break
+      }
+    }
+    if (!hasMatchingRule) {
+      return
+    }
+    this.target.updateOverridesEdgeInAdded(this.overrides)
   }
@@ -120,0 +132,0 @@

lib/shrinkwrap.js

@@ -932,6 +932,20 @@ // a module that manages a shrinkwrap file (npm-shrinkwrap.json or
         const loc = relpath(this.path, node.path)
-        this.data.packages[loc] = Shrinkwrap.metaFromNode(
+        // Drop lockfile entries for extraneous nodes outside node_modules. These are stale workspace entries: the workspace was removed from package.json or its directory was deleted, so it should not be tracked in package-lock.json.
+        if (node.extraneous && !/(^|\/)node_modules\//.test(loc) && loc !== 'node_modules') {
+          continue
+        }
+        const meta = Shrinkwrap.metaFromNode(
           node,
           this.path,
           this.resolveOptions)
+        // Skip inert nodes — these are optional deps that failed to load
+        // (e.g. 404 from a proxy registry that hasn't cached the package,
+        // or incomplete manifest missing version field).
+        // #pruneFailedOptional marks them inert so they won't be reified;
+        // writing them to the lockfile produces invalid entries like
+        // {"optional": true} that cause "Invalid Version:" errors.
+        if (node.inert && !node.package.version) {
+          continue
+        }
+        this.data.packages[loc] = meta
       }
@@ -938,0 +952,0 @@ } else if (this.#awaitingUpdate.size > 0) {

package.json

 {
   "name": "@npmcli/arborist",
-  "version": "9.5.0",
+  "version": "9.6.0",
   "description": "Manage node_modules trees",
@@ -5,0 +5,0 @@ "dependencies": {

New alerts

AI-detected potential code anomaly

Supply chain risk

AI has identified unusual behaviors that may pose a security risk.

Found 1 instance in 1 package

Fixed alerts

AI-detected potential code anomaly

Supply chain risk

AI has identified unusual behaviors that may pose a security risk.

Found 1 instance in 1 package

Improved metrics

Total package byte prevSize

502312

1.35%

Lines of code

13107

0.87%

No dependency changes