fix(arborist): audit the non-isolated tree under the linked strategy (#9625)

In continuation of our exploration of using `install-strategy=linked` in
the [Gutenberg
monorepo](WordPress/gutenberg#75814), which

Under `install-strategy=linked`, `npm install --audit` reported `found 0
vulnerabilities` even with a known-vulnerable package installed, while
standalone `npm audit` reported it correctly. Only the install-time
audit was affected.

## Why

A linked reify swaps `idealTree` for the isolated tree
(`createIsolatedTree()`) before the quick audit runs, so
`_submitQuickAudit()` audited the isolated tree. That tree cannot be
audited: its inventory had a stub `query()` that always returned `[]`,
and its edges route through symlink `Link`s instead of real package
nodes. So `AuditReport.prepareBulkData()` produced an empty bulk request
and the registry was never asked about any installed version. Standalone
`npm audit` was unaffected because it audits the regular tree loaded
from the lockfile.

## How

`reify.js` stashes the original non-isolated ideal tree in
`#linkedIdealForAudit` during the linked swap, and `_submitQuickAudit()`
now audits `this.#linkedIdealForAudit || this.idealTree` — the same tree
standalone `npm audit` uses, with a queryable inventory and real package
nodes. The `_diffTrees()`/`#reifyPackages()`/orphan-sweep block is
wrapped in `try/finally` that restores `idealTree` and clears the
stashed references even if reify throws, so a reused Arborist never
audits or diffs a stale isolated tree. `isolated-classes.js` drops the
now-unused `IsolatedInventory` class (its only caller was the rerouted
audit path) in favor of a plain `Map`; the `query()` stub returning `[]`
was the silent-empty behavior behind this bug.

## References

Fixes #9609
Part of #9608

Author: manzoorwanijk

workspaces/arborist/lib/arborist/reify.js

@@ -77,6 +77,9 @@ module.exports = cls => class Reifier extends cls {
   #sparseTreeDirs = new Set()
   #sparseTreeRoots = new Set()
   #linkedActualForDiff = null
+  // Under the linked strategy the audit runs against this non-isolated ideal tree.
+  // The isolated tree's inventory has no queryable indexes and its edges route through symlinks, so auditing it reports no vulnerabilities.
+  #linkedIdealForAudit = null
 
   constructor (options) {
     super(options)
@@ -123,21 +126,29 @@ module.exports = cls => class Reifier extends cls {
           this.idealTree, this.actualTree
         )
       }
+      // Keep the non-isolated tree so the quick audit can run against it.
+      this.#linkedIdealForAudit = oldTree
     }
-    await this[_diffTrees]()
-    await this.#reifyPackages()
-    if (linked) {
-      // The sweep mutates node_modules on disk, so skip it for dry runs and lockfile-only installs (those modes also short-circuit #reifyPackages).
-      // The sweep itself scopes to in-filter workspaces when a filter is active, so it's safe to run for filtered installs too.
-      if (!this.options.dryRun && !this.options.packageLockOnly) {
-        await this.#cleanOrphanedStoreEntries()
+    try {
+      await this[_diffTrees]()
+      await this.#reifyPackages()
+      if (linked) {
+        // The sweep mutates node_modules on disk, so skip it for dry runs and lockfile-only installs (those modes also short-circuit #reifyPackages).
+        // The sweep itself scopes to in-filter workspaces when a filter is active, so it's safe to run for filtered installs too.
+        if (!this.options.dryRun && !this.options.packageLockOnly) {
+          await this.#cleanOrphanedStoreEntries()
+        }
+      }
+    } finally {
+      // Restore the non-isolated tree so the lockfile is preserved and a reused Arborist never sees the isolated tree, even if reify throws.
+      if (linked) {
+        this.idealTree = oldTree
       }
-      // swap back in the idealTree
-      // so that the lockfile is preserved
-      this.idealTree = oldTree
+      // The quick audit has captured its tree synchronously by now, so drop the stashed references even on throw.
+      this.#linkedIdealForAudit = null
+      this.#linkedActualForDiff = null
     }
     await this[_saveIdealTree](options)
-    this.#linkedActualForDiff = null
     // clean inert
     for (const node of this.idealTree.inventory.values()) {
       if (node.inert) {
@@ -1169,7 +1180,8 @@ module.exports = cls => class Reifier extends cls {
     // with the reification, and be resolved at a later time.
     const timeEnd = time.start('reify:audit')
     const options = { ...this.options }
-    const tree = this.idealTree
+    // Under the linked strategy idealTree is the isolated tree, which the audit cannot traverse; audit the non-isolated tree instead.
+    const tree = this.#linkedIdealForAudit || this.idealTree
 
     // if we're operating on a workspace, only audit the workspace deps
     if (this.options.workspaces.length) {

workspaces/arborist/lib/isolated-classes.js

@@ -2,13 +2,6 @@
 const CaseInsensitiveMap = require('./case-insensitive-map.js')
 const { resolve } = require('node:path')
 
-// fake lib/inventory.js
-class IsolatedInventory extends Map {
-  query () {
-    return []
-  }
-}
-
 // fake lib/node.js
 class IsolatedNode {
   binPaths = []
@@ -17,7 +10,7 @@ class IsolatedNode {
   edgesOut = new CaseInsensitiveMap()
   fsChildren = new Set()
   integrity = null
-  inventory = new IsolatedInventory()
+  inventory = new Map()
   isInStore = false
   inBundle = false
   isRegistryDependency = false

workspaces/arborist/test/arborist/reify.js

@@ -223,6 +223,21 @@ t.test('packageLockOnly with linked strategy in workspaces', async t => {
   t.throws(() => fs.statSync(path + '/node_modules'), { code: 'ENOENT' })
 })
 
+t.test('linked strategy audits the non-isolated tree', async t => {
+  // The isolated tree has no queryable inventory, so auditing it reports nothing. The install-time audit must run against the non-isolated tree, matching standalone npm audit. https://github.com/npm/cli/issues/9609
+  const src = resolve(fixtures, 'audit-one-vuln')
+  // Copy into a throwaway dir since packageLockOnly rewrites the lockfile.
+  const path = t.testdir({
+    'package.json': fs.readFileSync(join(src, 'package.json'), 'utf8'),
+    'package-lock.json': fs.readFileSync(join(src, 'package-lock.json'), 'utf8'),
+  })
+  const registry = createRegistry(t, true)
+  registry.audit({ convert: true, results: require(join(src, 'audit.json')) })
+  const arb = newArb({ path, audit: true, packageLockOnly: true, installStrategy: 'linked' })
+  await arb.reify()
+  t.ok(arb.auditReport.has('minimist'), 'vulnerable package reported under linked strategy')
+})
+
 t.test('malformed package.json should not be overwritten', async t => {
   t.plan(2)