Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 24 additions & 12 deletions workspaces/arborist/lib/arborist/reify.js
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -1152,7 +1163,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) {
Expand Down
9 changes: 1 addition & 8 deletions workspaces/arborist/lib/isolated-classes.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 = []
Expand All @@ -18,7 +11,7 @@ class IsolatedNode {
fsChildren = new Set()
hasShrinkwrap = false
integrity = null
inventory = new IsolatedInventory()
inventory = new Map()
isInStore = false
inBundle = false
isRegistryDependency = false
Expand Down
15 changes: 15 additions & 0 deletions workspaces/arborist/test/arborist/reify.js
Original file line number Diff line number Diff line change
Expand Up @@ -221,6 +221,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)

Expand Down
Loading