fix(arborist): honor allowRemote exemption for registry bundle tarball

workspaces/arborist/lib/arborist/build-ideal-tree.js

@@ -13,6 +13,7 @@ const { lstat, readlink } = require('node:fs/promises')
 const { depth } = require('treeverse')
 const { log, time } = require('proc-log')
 const { redact } = require('@npmcli/redact')
+const { isRegistryResolvedTarball } = require('../registry-resolved-tarball.js')
 
 const {
   OK,
@@ -1028,6 +1029,9 @@ This is a one-time fix-up, please be patient...
           Arborist,
           resolved: node.resolved,
           integrity: node.integrity,
+          // pacote's npa re-parses node.resolved as type=remote, so allowRemote would mis-fire on registry tarballs.
+          // Override only when we can prove the URL is registry-mediated; see isRegistryResolvedTarball.
+          ...(isRegistryResolvedTarball(node, this) ? { allowRemote: 'all' } : {}),
         })
 
         await new Arborist({ ...this.options, path })

workspaces/arborist/lib/arborist/reify.js

@@ -1,10 +1,8 @@
 // mixin implementing the reify method
 const PackageJson = require('@npmcli/package-json')
-const hgi = require('hosted-git-info')
 const npa = require('npm-package-arg')
 const packageContents = require('@npmcli/installed-package-contents')
 const pacote = require('pacote')
-const { pickRegistry } = require('npm-registry-fetch')
 const promiseAllRejectLate = require('promise-all-reject-late')
 const runScript = require('@npmcli/run-script')
 const { callLimit: promiseCallLimit } = require('promise-call-limit')
@@ -32,6 +30,10 @@ const Shrinkwrap = require('../shrinkwrap.js')
 const { defaultLockfileVersion } = Shrinkwrap
 const { saveTypeMap, hasSubKey } = require('../add-rm-pkg-deps.js')
 const { IsolatedNode, IsolatedLink } = require('../isolated-classes.js')
+const {
+  registryResolved: getRegistryResolved,
+  isRegistryResolvedTarball,
+} = require('../registry-resolved-tarball.js')
 
 // Part of steps (steps need refactoring before we can do anything about these)
 const _retireShallowNodes = Symbol.for('retireShallowNodes')
@@ -706,9 +708,9 @@ module.exports = cls => class Reifier extends cls {
       // entirely, since we can't possibly reify it.
       let res = null
       if (node.resolved) {
-        const registryResolved = this.#registryResolved(node.resolved)
-        if (registryResolved) {
-          res = `${node.name}@${registryResolved}`
+        const resolved = getRegistryResolved(node.resolved, this)
+        if (resolved) {
+          res = `${node.name}@${resolved}`
         }
       } else if (node.package.name && node.version) {
         res = `${node.package.name}@${node.version}`
@@ -748,8 +750,8 @@ module.exports = cls => class Reifier extends cls {
           e.valid && (e.from?.isProjectRoot || e.from?.isWorkspace)
         ),
         // 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' } : {}),
+        // Override only when we can prove the URL is registry-mediated; see isRegistryResolvedTarball.
+        ...(isRegistryResolvedTarball(node, this) ? { allowRemote: 'all' } : {}),
       })
       // store nodes don't use Node class so node.package doesn't get updated
       if (node.isInStore) {
@@ -982,80 +984,6 @@ module.exports = cls => class Reifier extends cls {
     return realpathSync(child.path) !== realpathSync(child.realpath)
   }
 
-  // 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) {
-      return false
-    }
-    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))
-    } catch {
-      return false
-    }
-  }
-
-  #registryResolved (resolved) {
-    // the default registry url is a magic value meaning "the currently
-    // configured registry".
-    // `resolved` must never be falsey.
-    //
-    // XXX: use a magic string that isn't also a valid value, like
-    // ${REGISTRY} or something.  This has to be threaded through the
-    // Shrinkwrap and Node classes carefully, so for now, just treat
-    // the default reg as the magical animal that it has been.
-    try {
-      const resolvedURL = hgi.parseUrl(resolved)
-      const registryURL = new URL(this.registry)
-      const registryPath = registryURL.pathname.replace(/\/$/, '')
-
-      let matchURL = null
-      try {
-        matchURL = new URL(this.options.replaceRegistryHost)
-      } catch {
-        // keep matchURL null
-      }
-
-      const matchHost = matchURL?.hostname ?? this.options.replaceRegistryHost
-      const matchPath = matchURL?.pathname.replace(/\/$/, '') ?? null
-      const hasPathPrefix = (pathname, prefix) =>
-        pathname === prefix || pathname.startsWith(`${prefix}/`)
-
-      const hostMatches = this.options.replaceRegistryHost === 'always' || matchHost === resolvedURL.hostname
-      const pathMatches = !matchPath || hasPathPrefix(resolvedURL.pathname, matchPath)
-
-      if (!hostMatches || !pathMatches) {
-        return resolved
-      }
-
-      resolvedURL.protocol = registryURL.protocol
-      resolvedURL.hostname = registryURL.hostname
-      resolvedURL.port = registryURL.port
-
-      if (matchPath) {
-        // full-URL prefix: swap old path prefix for the registry path
-        resolvedURL.pathname = registryPath + resolvedURL.pathname.slice(matchPath.length)
-      } else if (registryPath && !hasPathPrefix(resolvedURL.pathname, registryPath)) {
-        // host-only: prepend registry path if not already present
-        resolvedURL.pathname = registryPath + resolvedURL.pathname
-      }
-
-      return resolvedURL.toString()
-    } catch {
-      // if we could not parse the url at all then returning nothing
-      // here means it will get removed from the tree in the next step
-      return undefined
-    }
-  }
-
   // bundles are *sort of* like shrinkwraps, in that the branch is defined
   // by the contents of the package.  however, in their case, rather than
   // shipping a virtual tree that must be reified, they ship an entire

workspaces/arborist/lib/registry-resolved-tarball.js

@@ -0,0 +1,85 @@
+// Shared by reify.js (#extractOrLink) and build-ideal-tree.js (the
+// --complete/--dry-run "crack open" path): both hand pacote a `name@URL` or
+// bare URL spec for a registry-hosted tarball. pacote's npa re-parses that
+// as spec.type === 'remote', so without an override the allow-remote gate
+// would mis-fire on every registry tarball (both allowRemote=none and
+// allowRemote=root). isRegistryResolvedTarball() tells the caller when it is
+// safe to pass `allowRemote: 'all'` to bypass that false positive.
+const hgi = require('hosted-git-info')
+const npa = require('npm-package-arg')
+const { pickRegistry } = require('npm-registry-fetch')
+
+// the default registry url is a magic value meaning "the currently
+// configured registry".
+// `resolved` must never be falsey.
+//
+// XXX: use a magic string that isn't also a valid value, like
+// ${REGISTRY} or something.  This has to be threaded through the
+// Shrinkwrap and Node classes carefully, so for now, just treat
+// the default reg as the magical animal that it has been.
+const registryResolved = (resolved, arb) => {
+  try {
+    const resolvedURL = hgi.parseUrl(resolved)
+    const registryURL = new URL(arb.registry)
+    const registryPath = registryURL.pathname.replace(/\/$/, '')
+
+    let matchURL = null
+    try {
+      matchURL = new URL(arb.options.replaceRegistryHost)
+    } catch {
+      // keep matchURL null
+    }
+
+    const matchHost = matchURL?.hostname ?? arb.options.replaceRegistryHost
+    const matchPath = matchURL?.pathname.replace(/\/$/, '') ?? null
+    const hasPathPrefix = (pathname, prefix) =>
+      pathname === prefix || pathname.startsWith(`${prefix}/`)
+
+    const hostMatches = arb.options.replaceRegistryHost === 'always' || matchHost === resolvedURL.hostname
+    const pathMatches = !matchPath || hasPathPrefix(resolvedURL.pathname, matchPath)
+
+    if (!hostMatches || !pathMatches) {
+      return resolved
+    }
+
+    resolvedURL.protocol = registryURL.protocol
+    resolvedURL.hostname = registryURL.hostname
+    resolvedURL.port = registryURL.port
+
+    if (matchPath) {
+      // full-URL prefix: swap old path prefix for the registry path
+      resolvedURL.pathname = registryPath + resolvedURL.pathname.slice(matchPath.length)
+    } else if (registryPath && !hasPathPrefix(resolvedURL.pathname, registryPath)) {
+      // host-only: prepend registry path if not already present
+      resolvedURL.pathname = registryPath + resolvedURL.pathname
+    }
+
+    return resolvedURL.toString()
+  } catch {
+    // if we could not parse the url at all then returning nothing
+    // here means it will get removed from the tree in the next step
+    return undefined
+  }
+}
+
+// Returns true only when we are confident this is a registry-mediated
+// install, i.e. it is safe to pass `allowRemote: 'all'` to pacote.
+const isRegistryResolvedTarball = (node, arb) => {
+  if (!node.resolved || !node.isRegistryDependency) {
+    return false
+  }
+  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(registryResolved(node.resolved, arb))
+    // 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), arb.options))
+    const registryPath = registry.pathname.replace(/\/?$/, '/')
+    return resolvedURL.origin === registry.origin &&
+      (registryPath === '/' || resolvedURL.pathname.startsWith(registryPath))
+  } catch {
+    return false
+  }
+}
+
+module.exports = { registryResolved, isRegistryResolvedTarball }

workspaces/arborist/test/arborist/build-ideal-tree.js

@@ -395,6 +395,22 @@ t.test('bundle deps example 1, complete:true', async t => {
   }), 'no missing deps, because complete: true, add dep, save bundled')
 })
 
+t.test('complete:true cracks open a registry-hosted bundle tarball under allowRemote=none', async t => {
+  // complete:true is what buildIdealTree runs under for --dry-run and
+  // --package-lock-only (see reify.js, `complete: this.options.packageLockOnly || this.options.dryRun`).
+  // The tarball URL handed to pacote.extract here is registry-hosted, but pacote's
+  // npa re-parses a bare URL as spec.type === 'remote', so without the same
+  // allowRemote override reify.js applies for its own extract call, this
+  // mis-fired EALLOWREMOTE on ordinary registry installs (npm/cli#9800).
+  const path = resolve(fixtures, 'testing-bundledeps-empty')
+  createRegistry(t, true)
+
+  await t.resolves(buildIdeal(path, {
+    complete: true,
+    allowRemote: 'none',
+  }), 'registry-hosted bundle tarball is allowed under allowRemote=none')
+})
+
 t.test('bundle deps example 2', async t => {
   // bundled deps at the root level are NOT ignored when building ideal trees
   const path = resolve(fixtures, 'testing-bundledeps-2')