fix: allow-remote=none does not block registry tarballs

Author: owlstronaut

test/lib/commands/install.js

@@ -344,6 +344,51 @@ t.test('exec commands', async t => {
       'optional transitive git dep is silently skipped'
     )
   })
+
+  t.test('allow-remote=none does not block registry tarballs', async t => {
+    const { npm, registry } = await loadMockNpm(t, {
+      config: {
+        'allow-remote': 'none',
+        audit: false,
+      },
+      prefixDir: {
+        'package.json': JSON.stringify({
+          ...packageJson,
+          dependencies: { abbrev: '^1.0.0' },
+        }),
+        abbrev,
+      },
+    })
+    const manifest = registry.manifest({ name: 'abbrev' })
+    await registry.package({ manifest })
+    await registry.tarball({
+      manifest: manifest.versions['1.0.0'],
+      tarball: path.join(npm.prefix, 'abbrev'),
+    })
+    await npm.exec('install', [])
+    const installed = require(path.join(npm.prefix, 'node_modules', 'abbrev', 'package.json'))
+    t.equal(installed.name, 'abbrev', 'registry dep is installed despite allow-remote=none')
+  })
+
+  t.test('allow-remote=none still blocks a user-supplied remote URL', async t => {
+    const { npm } = await loadMockNpm(t, {
+      config: {
+        'allow-remote': 'none',
+      },
+      prefixDir: {
+        'package.json': JSON.stringify({
+          name: '@npmcli/test-package',
+          version: '1.0.0',
+          dependencies: { abbrev: 'https://registry.npmjs.org/abbrev/-/abbrev-2.0.0.tgz' },
+        }),
+      },
+    })
+    await t.rejects(
+      npm.exec('install', []),
+      { code: 'EALLOWREMOTE' },
+      'user-supplied remote URL is still blocked'
+    )
+  })
 })
 
 t.test('completion', async t => {

workspaces/arborist/lib/arborist/reify.js

@@ -4,6 +4,7 @@ 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')
@@ -709,6 +710,9 @@ module.exports = cls => class Reifier extends cls {
         _isRoot: [...node.edgesIn].some(e =>
           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' } : {}),
       })
       // store nodes don't use Node class so node.package doesn't get updated
       if (node.isInStore) {
@@ -832,6 +836,23 @@ module.exports = cls => class Reifier extends cls {
     return wrapper
   }
 
+  // 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: the node's inbound edges must all be registry-typed (no exotic spec smuggled the URL in) AND the resolved URL's host must match the registry npm-registry-fetch selected for this spec, so a tampered lockfile pointing at an attacker host still hits the gate.
+  #isRegistryResolvedTarball (node) {
+    if (!node.resolved || !node.isRegistryDependency) {
+      return false
+    }
+    try {
+      const resolvedHost = new URL(node.resolved).hostname
+      // pickRegistry only consults spec.scope, so a bare-name (tag) parse is sufficient and avoids a node.version dependency.
+      const registryHost = new URL(pickRegistry(npa(node.name), this.options)).hostname
+      return resolvedHost === registryHost
+    } catch {
+      return false
+    }
+  }
+
   #registryResolved (resolved) {
     // the default registry url is a magic value meaning "the currently
     // configured registry".