fix(arborist): recognize registry-mediated tarballs

tap-snapshots/test/lib/docs.js.test.cjs

@@ -290,9 +290,9 @@ range. Please note that this could leave your tree incomplete and some
 packages may not function as intended or designed. Changing this setting
 will not remove dependencies that are already installed.
 
-As of npm 12 the default is \`none\`. Tarballs that share a hostname with the
-configured registry (the typical case for the npm registry, GitHub Packages,
-and most private registries) are still installed normally. If your registry
+As of npm 12 the default is \`none\`. Tarballs under the configured registry
+path are installed normally. npm also permits same-origin tarballs when it
+can verify their exact URL against registry metadata. If your registry
 serves tarballs from a different host, set \`replace-registry-host\` or
 override this setting. Opt in explicitly per project (in \`.npmrc\`) or per
 command (on the CLI) when you intentionally install from a URL.

test/lib/commands/ci.js

@@ -193,7 +193,7 @@ t.test('allow-remote=none blocks same-host tarball outside registry path', async
   lock.packages['node_modules/abbrev'].resolved = evilTarball
   lock.dependencies.abbrev.resolved = evilTarball
 
-  const { npm } = await loadMockNpm(t, {
+  const { npm, registry } = await loadMockNpm(t, {
     config: {
       audit: false,
       'allow-remote': 'none',
@@ -206,6 +206,8 @@ t.test('allow-remote=none blocks same-host tarball outside registry path', async
       'package-lock.json': JSON.stringify(lock),
     },
   })
+  const manifest = registry.manifest({ name: 'abbrev' })
+  await registry.package({ manifest })
 
   await t.rejects(
     npm.exec('ci', []),

workspaces/arborist/lib/arborist/isolated-reifier.js

@@ -3,6 +3,7 @@ const { depth } = require('treeverse')
 const crypto = require('node:crypto')
 const { IsolatedNode, IsolatedLink } = require('../isolated-classes.js')
 const nameFromFolder = require('@npmcli/name-from-folder')
+const { carryRegistryPackageName } = require('../registry-package-name.js')
 
 // generate short hash key based on the dependency tree starting at this node
 const getKey = (startNode) => {
@@ -59,6 +60,7 @@ module.exports = cls => class IsolatedReifier extends cls {
       resolved: node.resolved,
       root,
     })
+    carryRegistryPackageName(node, newChild)
     // XXX top is from place-dep not lib/node.js
     newChild.top = { path: this.idealGraph.localPath }
     root.children.set(newChild.location, newChild)
@@ -164,6 +166,8 @@ module.exports = cls => class IsolatedReifier extends cls {
     // Carry the source node's registry-dependency flag so the store node retains it.
     // IsolatedNode has no edges to recompute it from, and reify's registry-tarball allow-remote exemption depends on it.
     result.isRegistryDependency = node.isRegistryDependency
+    // Package metadata is not a trusted identity source, so preserve the name derived from source edges.
+    carryRegistryPackageName(node, result)
     // Same reasoning for allow-remote=root: the store node has no edgesIn, so capture from the source node whether it satisfies a valid edge from the project root or a workspace.
     result.isRootDependency = [...node.edgesIn].some(e =>
       e.valid && (e.from?.isProjectRoot || e.from?.isWorkspace)

workspaces/arborist/lib/arborist/reify.js

@@ -27,6 +27,7 @@ const relpath = require('../relpath.js')
 const { applyPatchToDir, patchIntegrity } = require('../patch.js')
 const { readFile } = require('node:fs/promises')
 const retirePath = require('../retire-path.js')
+const { getRegistryPackageName } = require('../registry-package-name.js')
 const treeCheck = require('../tree-check.js')
 const Shrinkwrap = require('../shrinkwrap.js')
 const { defaultLockfileVersion } = Shrinkwrap
@@ -697,6 +698,11 @@ module.exports = cls => class Reifier extends cls {
     await this.#validateNodeModules(nm)
 
     if (!node.isLink) {
+      const isRoot = node.isRootDependency || [...node.edgesIn].some(e =>
+        e.valid && (e.from?.isProjectRoot || e.from?.isWorkspace)
+      )
+      const allowRemote = this.options.allowRemote ?? 'all'
+      const remoteAllowed = allowRemote === 'all' || (allowRemote !== 'none' && isRoot)
       // in normal cases, node.resolved should *always* be set by now.
       // however, it is possible when a lockfile is damaged, or very old,
       // or in some other race condition bugs in npm v6, that a previously
@@ -705,10 +711,15 @@ module.exports = cls => class Reifier extends cls {
       // Do the best with what we have, or else remove it from the tree
       // entirely, since we can't possibly reify it.
       let res = null
+      let registryTarballExemption = false
       if (node.resolved) {
         const registryResolved = this.#registryResolved(node.resolved)
         if (registryResolved) {
-          res = `${node.name}@${registryResolved}`
+          const registryPackageName = !remoteAllowed && getRegistryPackageName(node)
+          registryTarballExemption = !!registryPackageName &&
+            await this.#isRegistryResolvedTarball(node, registryPackageName)
+          const packageName = registryTarballExemption ? registryPackageName : node.name
+          res = `${packageName}@${registryResolved}`
         }
       } else if (node.package.name && node.version) {
         res = `${node.package.name}@${node.version}`
@@ -744,12 +755,10 @@ module.exports = cls => class Reifier extends cls {
         // A node counts as "root" for allow-* enforcement if it satisfies at least one valid dependency edge declared by the project root or a workspace.
         // node.parent is unsafe here: after hoisting, transitive packages can have the project root as their tree parent.
         // In the linked strategy the store node has no edgesIn, so isolated-reifier precomputes isRootDependency from the source node's edges.
-        _isRoot: node.isRootDependency || [...node.edgesIn].some(e =>
-          e.valid && (e.from?.isProjectRoot || e.from?.isWorkspace)
-        ),
+        _isRoot: isRoot,
         // pacote's npa re-parses our `name@URL` spec as type=remote, so allowRemote would mis-fire on registry tarballs.
         // Override only when we can prove the URL is registry-mediated; see #isRegistryResolvedTarball.
-        ...(this.#isRegistryResolvedTarball(node) ? { allowRemote: 'all' } : {}),
+        ...(registryTarballExemption ? { allowRemote: 'all' } : {}),
       })
       // store nodes don't use Node class so node.package doesn't get updated
       if (node.isInStore) {
@@ -985,22 +994,54 @@ module.exports = cls => class Reifier extends cls {
   // When extracting a registry-resolved package, the spec we hand to pacote is name@URL.
   // pacote re-parses that with npa and gets spec.type === 'remote', so without an override the allow-remote gate would fire on every registry tarball (both =none and =root mis-fire).
   // Returns true only when we are confident this is a registry-mediated install.
-  #isRegistryResolvedTarball (node) {
-    if (!node.resolved || !node.isRegistryDependency) {
+  async #isRegistryResolvedTarball (node, packageName) {
+    // The caller only invokes this with a resolved URL and a trusted package
+    // name, but linked nodes retain an independent registry provenance flag.
+    if (!node.isRegistryDependency) {
       return false
     }
+
+    let resolvedURL
+    let registry
     try {
       // Match the effective fetch URL, not the raw lockfile value.
       // #registryResolved applies replace-registry-host, rewriting a public-registry pin to the configured proxy/mirror so it matches.
-      const resolvedURL = new URL(this.#registryResolved(node.resolved))
-      // pickRegistry only consults spec.scope, so a bare-name (tag) parse is sufficient and avoids a node.version dependency.
-      const registry = new URL(pickRegistry(npa(node.name), this.options))
-      const registryPath = registry.pathname.replace(/\/?$/, '/')
-      return resolvedURL.origin === registry.origin &&
-        (registryPath === '/' || resolvedURL.pathname.startsWith(registryPath))
+      resolvedURL = new URL(this.#registryResolved(node.resolved))
+      registry = new URL(pickRegistry(npa(packageName), this.options))
     } catch {
       return false
     }
+
+    if (resolvedURL.origin !== registry.origin) {
+      return false
+    }
+
+    const registryPath = registry.pathname.replace(/\/?$/, '/')
+    if (registryPath === '/' || resolvedURL.pathname.startsWith(registryPath)) {
+      return true
+    }
+
+    if (!node.version) {
+      return false
+    }
+
+    // Some registries advertise tarballs from a sibling path on the same
+    // origin. Verify those URLs against registry metadata rather than
+    // widening the configured registry path boundary.
+    try {
+      const manifest = await pacote.manifest(npa.resolve(packageName, node.version), {
+        ...this.options,
+        before: null,
+        fullMetadata: true,
+      })
+      const advertisedURL = new URL(this.#registryResolved(manifest._resolved))
+      advertisedURL.hash = ''
+      resolvedURL.hash = ''
+      return advertisedURL.href === resolvedURL.href
+    } catch (error) {
+      log.verbose('reify', 'unable to verify registry tarball metadata', error)
+      return false
+    }
   }
 
   #registryResolved (resolved) {

workspaces/arborist/lib/registry-package-name.js

@@ -0,0 +1,44 @@
+const npa = require('npm-package-arg')
+const { trustedSpecName } = require('./release-age-exclude.js')
+
+const carriedNames = new WeakMap()
+
+// Registry tarball exemptions widen fetch policy, so derive their package
+// identity from valid dependency specs and require every such edge to agree.
+const getRegistryPackageName = (node) => {
+  if (carriedNames.has(node)) {
+    return carriedNames.get(node)
+  }
+  if (!node.edgesIn || typeof node.edgesIn[Symbol.iterator] !== 'function') {
+    return null
+  }
+
+  const names = new Set()
+  for (const edge of node.edgesIn) {
+    if (!edge.valid) {
+      continue
+    }
+    let spec
+    try {
+      spec = npa.resolve(edge.name, edge.spec)
+    } catch {
+      return null
+    }
+    if (!spec.registry) {
+      return null
+    }
+    const name = trustedSpecName(spec)
+    if (!name) {
+      return null
+    }
+    names.add(name)
+  }
+
+  return names.size === 1 ? names.values().next().value : null
+}
+
+const carryRegistryPackageName = (from, to) => {
+  carriedNames.set(to, getRegistryPackageName(from))
+}
+
+module.exports = { carryRegistryPackageName, getRegistryPackageName }