fix: write registry allowScripts keys under install-strategy=linked

lib/utils/allow-scripts-cmd.js

@@ -221,13 +221,14 @@ class AllowScriptsCmd extends BaseCommand {
     // edge name. A version or range on the arg narrows the match to installed
     // versions that satisfy it. Bundled deps are excluded for the same reason
     // as --all. Args that match nothing are returned in `unmatched`.
+    // Links are skipped like collectUnreviewedScripts and prune do: scripts only run on the target, and keying policy off a Link's resolved spec writes store paths under install-strategy=linked (npm/cli#9939).
     const matched = []
     const unmatched = []
     for (const arg of args) {
       const { name: wantName, range } = parsePositional(arg)
       const found = []
       for (const node of arb.actualTree.inventory.values()) {
-        if (node.isProjectRoot || node.isWorkspace || node.inBundle) {
+        if (node.isProjectRoot || node.isWorkspace || node.isLink || node.inBundle) {
           continue
         }
         const { name, version } = trustedDisplay(node)

test/lib/commands/approve-scripts.js

@@ -859,3 +859,131 @@ t.test('approve-scripts --all with only bundled deps has nothing to review', asy
   const pkg = JSON.parse(fs.readFileSync(resolve(prefix, 'package.json'), 'utf8'))
   t.notOk(pkg.allowScripts, 'no allowScripts written')
 })
+
+// Layout produced by install-strategy=linked: the real package lives in the store, node_modules holds a symlink to it, and the hidden lockfile records that isolated layout (npm/cli#9939).
+const setupLinkedProject = (t, { allowScripts, noResolved = false } = {}) => {
+  const storeDir = 'canvas@1.0.0-c2FsdHNhbHRzYWx0c2FsdA'
+  const storeLoc = `node_modules/.store/${storeDir}/node_modules/canvas`
+  const tarUrl = noResolved
+    ? undefined
+    : 'https://registry.npmjs.org/canvas/-/canvas-1.0.0.tgz'
+  const pkg = {
+    name: 'host',
+    version: '1.0.0',
+    dependencies: { canvas: '^1.0.0' },
+  }
+  if (allowScripts !== undefined) {
+    pkg.allowScripts = allowScripts
+  }
+  return {
+    'package.json': JSON.stringify(pkg, null, 2),
+    'package-lock.json': JSON.stringify({
+      name: pkg.name,
+      version: pkg.version,
+      lockfileVersion: 3,
+      requires: true,
+      packages: {
+        '': pkg,
+        'node_modules/canvas': {
+          version: '1.0.0',
+          resolved: tarUrl,
+          hasInstallScript: true,
+        },
+      },
+    }),
+    node_modules: {
+      '.package-lock.json': JSON.stringify({
+        lockfileVersion: 3,
+        requires: true,
+        packages: {
+          [`node_modules/.store/${storeDir}`]: {},
+          [storeLoc]: {
+            version: '1.0.0',
+            resolved: tarUrl,
+            hasInstallScript: true,
+          },
+          'node_modules/canvas': {
+            resolved: storeLoc,
+            link: true,
+          },
+        },
+      }),
+      '.store': {
+        [storeDir]: {
+          node_modules: {
+            canvas: {
+              'package.json': JSON.stringify({
+                name: 'canvas',
+                version: '1.0.0',
+                scripts: { install: 'echo install' },
+              }),
+            },
+          },
+        },
+      },
+      canvas: t.fixture('symlink', `.store/${storeDir}/node_modules/canvas`),
+    },
+  }
+}
+
+// loadActual only trusts the hidden lockfile when it is newer than every dir under node_modules (assertNoNewer allows 10ms), and fixture creation order makes that racy, so bump its mtime past the fixture writes.
+const trustHiddenLockfile = (prefix) => {
+  const future = new Date(Date.now() + 60_000)
+  fs.utimesSync(resolve(prefix, 'node_modules', '.package-lock.json'), future, future)
+}
+
+t.test('approve-scripts <pkg> under linked strategy writes a registry pin, not store paths', async t => {
+  const { npm, prefix } = await mockNpm(t, {
+    prefixDir: setupLinkedProject(t),
+  })
+  trustHiddenLockfile(prefix)
+  await npm.exec('approve-scripts', ['canvas'])
+
+  const pkg = JSON.parse(fs.readFileSync(resolve(prefix, 'package.json'), 'utf8'))
+  t.strictSame(pkg.allowScripts, { 'canvas@1.0.0': true })
+})
+
+t.test('approve-scripts --all under linked strategy writes a registry pin, not store paths', async t => {
+  const { npm, prefix } = await mockNpm(t, {
+    prefixDir: setupLinkedProject(t),
+    config: { all: true },
+  })
+  trustHiddenLockfile(prefix)
+  await npm.exec('approve-scripts', [])
+
+  const pkg = JSON.parse(fs.readFileSync(resolve(prefix, 'package.json'), 'utf8'))
+  t.strictSame(pkg.allowScripts, { 'canvas@1.0.0': true })
+})
+
+t.test('approve-scripts --pending is empty when the registry pin covers a linked store package', async t => {
+  const { npm, prefix, joinedOutput } = await mockNpm(t, {
+    prefixDir: setupLinkedProject(t, { allowScripts: { 'canvas@1.0.0': true } }),
+    config: { 'allow-scripts-pending': true },
+  })
+  trustHiddenLockfile(prefix)
+  await npm.exec('approve-scripts', [])
+  t.match(joinedOutput(), /No packages with unreviewed install scripts/)
+})
+
+t.test('deny-scripts under linked strategy writes a name-only deny', async t => {
+  const { npm, prefix } = await mockNpm(t, {
+    prefixDir: setupLinkedProject(t),
+  })
+  trustHiddenLockfile(prefix)
+  await npm.exec('deny-scripts', ['canvas'])
+
+  const pkg = JSON.parse(fs.readFileSync(resolve(prefix, 'package.json'), 'utf8'))
+  t.strictSame(pkg.allowScripts, { canvas: false })
+})
+
+t.test('approve-scripts <pkg> under linked strategy without resolved URLs approves by name', async t => {
+  // omit-lockfile-registry-resolved: the store package has no resolved URL, so it cannot be pinned but must still be approved by name via the incoming Link's edge.
+  const { npm, prefix } = await mockNpm(t, {
+    prefixDir: setupLinkedProject(t, { noResolved: true }),
+  })
+  trustHiddenLockfile(prefix)
+  await npm.exec('approve-scripts', ['canvas'])
+
+  const pkg = JSON.parse(fs.readFileSync(resolve(prefix, 'package.json'), 'utf8'))
+  t.strictSame(pkg.allowScripts, { canvas: true })
+})

test/lib/commands/install-scripts.js

@@ -334,3 +334,64 @@ t.test('install-scripts prune fails for global installs', async t => {
     { code: 'EGLOBAL' }
   )
 })
+
+t.test('install-scripts prune under linked strategy keeps the registry pin, drops store paths', async t => {
+  // Regression for npm/cli#9939: the buggy writer produced file:.store keys; prune must drop those and keep the valid registry pin covering the store package.
+  const storeDir = 'canvas@1.0.0-c2FsdHNhbHRzYWx0c2FsdA'
+  const storeLoc = `node_modules/.store/${storeDir}/node_modules/canvas`
+  const tarUrl = 'https://registry.npmjs.org/canvas/-/canvas-1.0.0.tgz'
+  const pkg = {
+    name: 'host',
+    version: '1.0.0',
+    dependencies: { canvas: '^1.0.0' },
+    allowScripts: {
+      'canvas@1.0.0': true,
+      [`file:.store/${storeDir}/node_modules/canvas`]: true,
+    },
+  }
+  const { npm, prefix, joinedOutput } = await mockNpm(t, {
+    prefixDir: {
+      'package.json': JSON.stringify(pkg, null, 2),
+      node_modules: {
+        '.package-lock.json': JSON.stringify({
+          lockfileVersion: 3,
+          requires: true,
+          packages: {
+            [`node_modules/.store/${storeDir}`]: {},
+            [storeLoc]: {
+              version: '1.0.0',
+              resolved: tarUrl,
+              hasInstallScript: true,
+            },
+            'node_modules/canvas': {
+              resolved: storeLoc,
+              link: true,
+            },
+          },
+        }),
+        '.store': {
+          [storeDir]: {
+            node_modules: {
+              canvas: {
+                'package.json': JSON.stringify({
+                  name: 'canvas',
+                  version: '1.0.0',
+                  scripts: { install: 'echo install' },
+                }),
+              },
+            },
+          },
+        },
+        canvas: t.fixture('symlink', `.store/${storeDir}/node_modules/canvas`),
+      },
+    },
+  })
+  // loadActual only trusts the hidden lockfile when it is newer than every dir under node_modules (assertNoNewer allows 10ms), and fixture creation order makes that racy, so bump its mtime past the fixture writes.
+  const future = new Date(Date.now() + 60_000)
+  fs.utimesSync(resolve(prefix, 'node_modules', '.package-lock.json'), future, future)
+  await npm.exec('install-scripts', ['prune'])
+
+  const updated = JSON.parse(fs.readFileSync(resolve(prefix, 'package.json'), 'utf8'))
+  t.strictSame(updated.allowScripts, { 'canvas@1.0.0': true })
+  t.match(joinedOutput(), /file:\.store.*\(package not installed\)/)
+})

workspaces/arborist/lib/script-allowed.js

@@ -1,3 +1,4 @@
+const { sep } = require('node:path')
 const npa = require('npm-package-arg')
 const semver = require('semver')
 const versionFromTgz = require('./version-from-tgz.js')
@@ -99,6 +100,15 @@ const matches = (node, key, failClosed) => {
   }
 }
 
+// True when the node lives in the linked install strategy's `node_modules/.store` directory; such registry-managed packages must never derive identity from the store's internal `file:` link specs.
+const isStoreBacked = (node) => {
+  if (node?.isInStore) {
+    return true
+  }
+  const real = node?.realpath || node?.path
+  return typeof real === 'string' && real.includes(`${sep}node_modules${sep}.store${sep}`)
+}
+
 const resolvedSourceSpecs = (node) => {
   const specs = []
   const seen = new Set()
@@ -112,7 +122,8 @@ const resolvedSourceSpecs = (node) => {
 
   add(node?.resolved)
 
-  if (!node?.resolved && node?.linksIn && typeof node.linksIn[Symbol.iterator] === 'function') {
+  if (!node?.resolved && !isStoreBacked(node) &&
+      node?.linksIn && typeof node.linksIn[Symbol.iterator] === 'function') {
     let hasIncomingLink = false
     for (const link of node.linksIn) {
       hasIncomingLink = true
@@ -235,10 +246,32 @@ const getTrustedRegistryIdentity = (node) => {
 }
 
 const nameFromEdges = (node) => {
-  if (!node.edgesIn || typeof node.edgesIn[Symbol.iterator] !== 'function') {
+  const name = nameFromEdgeSet(node?.edgesIn)
+  if (name) {
+    return name
+  }
+  // A link target carries no edges of its own; they land on the incoming Links (e.g. the linked strategy's store packages), so consult their edges too, failing closed when the Links disagree on the registry name rather than trusting insertion order.
+  let linkName = null
+  if (node?.linksIn && typeof node.linksIn[Symbol.iterator] === 'function') {
+    for (const link of node.linksIn) {
+      const name = nameFromEdgeSet(link.edgesIn)
+      if (!name) {
+        continue
+      }
+      if (linkName && linkName !== name) {
+        return null
+      }
+      linkName = name
+    }
+  }
+  return linkName
+}
+
+const nameFromEdgeSet = (edgesIn) => {
+  if (!edgesIn || typeof edgesIn[Symbol.iterator] !== 'function') {
     return null
   }
-  for (const edge of node.edgesIn) {
+  for (const edge of edgesIn) {
     let parsed
     try {
       parsed = npa.resolve(edge.name, edge.spec)
@@ -350,7 +383,14 @@ const isRegistryNode = (node) => {
   // edge resolves to a registry spec, which is much harder to spoof than
   // the URL.
   if (typeof node.isRegistryDependency === 'boolean') {
-    return node.isRegistryDependency
+    if (node.isRegistryDependency) {
+      return true
+    }
+    // A link target carries no edges of its own; they land on the incoming Links (e.g. the linked strategy's store packages, npm/cli#9939), so delegate the edge-based check to them.
+    if (node.edgesIn?.size === 0 && node.linksIn?.size > 0) {
+      return [...node.linksIn].every(link => link.isRegistryDependency)
+    }
+    return false
   }
   // Fall back to URL parsing for nodes without the arborist getter
   // (e.g. test fixtures, lockfiles with omit-lockfile-registry-resolved).