diff --git a/workspaces/arborist/lib/arborist/reify.js b/workspaces/arborist/lib/arborist/reify.js index 45cd7c0e46c48..47d936f67a2d4 100644 --- a/workspaces/arborist/lib/arborist/reify.js +++ b/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) { diff --git a/workspaces/arborist/lib/isolated-classes.js b/workspaces/arborist/lib/isolated-classes.js index d451b605d8ca2..c5b7abbb64a3d 100644 --- a/workspaces/arborist/lib/isolated-classes.js +++ b/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 diff --git a/workspaces/arborist/test/arborist/reify.js b/workspaces/arborist/test/arborist/reify.js index e1f67033d4581..81d91f08f1d95 100644 --- a/workspaces/arborist/test/arborist/reify.js +++ b/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)