diff --git a/node_modules/@npmcli/arborist/lib/arborist/audit.js b/node_modules/@npmcli/arborist/lib/arborist/audit.js index aee7072d02ab0..bf1c335e75363 100644 --- a/node_modules/@npmcli/arborist/lib/arborist/audit.js +++ b/node_modules/@npmcli/arborist/lib/arborist/audit.js @@ -4,6 +4,7 @@ const AuditReport = require('../audit-report.js') // shared with reify const _global = Symbol.for('global') +const _workspaces = Symbol.for('workspaces') module.exports = cls => class Auditor extends cls { async audit (options = {}) { @@ -21,8 +22,10 @@ module.exports = cls => class Auditor extends cls { process.emit('time', 'audit') const tree = await this.loadVirtual() - this.auditReport = await AuditReport.load(tree, this.options) - const ret = options.fix ? this.reify() : this.auditReport + if (this[_workspaces] && this[_workspaces].length) + options.filterSet = this.workspaceDependencySet(tree, this[_workspaces]) + this.auditReport = await AuditReport.load(tree, options) + const ret = options.fix ? this.reify(options) : this.auditReport process.emit('timeEnd', 'audit') this.finishTracker('audit') return ret diff --git a/node_modules/@npmcli/arborist/lib/arborist/build-ideal-tree.js b/node_modules/@npmcli/arborist/lib/arborist/build-ideal-tree.js index ade9bbf1a152f..73a6f667e35db 100644 --- a/node_modules/@npmcli/arborist/lib/arborist/build-ideal-tree.js +++ b/node_modules/@npmcli/arborist/lib/arborist/build-ideal-tree.js @@ -398,38 +398,14 @@ module.exports = cls => class IdealTreeBuilder extends cls { process.emit('time', 'idealTree:userRequests') const tree = this.idealTree.target || this.idealTree - if (!this[_workspaces].length) { - return this[_applyUserRequestsToNode](tree, options).then(() => - process.emit('timeEnd', 'idealTree:userRequests')) - } - - const wsMap = tree.workspaces - if (!wsMap) { - this.log.warn('idealTree', 'Workspace filter set, but no workspaces present') - return - } - - const promises = [] - for (const name of this[_workspaces]) { - const path = wsMap.get(name) - if (!path) { - this.log.warn('idealTree', `Workspace ${name} in filter set, but not in workspaces`) - continue - } - const loc = relpath(tree.realpath, path) - const node = tree.inventory.get(loc) - - /* istanbul ignore if - should be impossible */ - if (!node) { - this.log.warn('idealTree', `Workspace ${name} in filter set, but no workspace folder present`) - continue - } - - promises.push(this[_applyUserRequestsToNode](node, options)) + if (!this[_workspaces].length) + await this[_applyUserRequestsToNode](tree, options) + else { + await Promise.all(this.workspaceNodes(tree, this[_workspaces]) + .map(node => this[_applyUserRequestsToNode](node, options))) } - return Promise.all(promises).then(() => - process.emit('timeEnd', 'idealTree:userRequests')) + process.emit('timeEnd', 'idealTree:userRequests') } async [_applyUserRequestsToNode] (tree, options) { @@ -456,7 +432,7 @@ module.exports = cls => class IdealTreeBuilder extends cls { } if (this.auditReport && this.auditReport.size > 0) - this[_queueVulnDependents](options) + await this[_queueVulnDependents](options) const { add, rm } = options @@ -475,10 +451,14 @@ module.exports = cls => class IdealTreeBuilder extends cls { if (add && add.length || rm && rm.length || this[_global]) tree.package = tree.package - for (const spec of this[_resolvedAdd]) - this[_explicitRequests].add(tree.edgesOut.get(spec.name)) + for (const spec of this[_resolvedAdd]) { + if (spec.tree === tree) + this[_explicitRequests].add(tree.edgesOut.get(spec.name)) + } for (const name of globalExplicitUpdateNames) this[_explicitRequests].add(tree.edgesOut.get(name)) + + this[_depsQueue].push(tree) } // This returns a promise because we might not have the name yet, @@ -488,12 +468,14 @@ module.exports = cls => class IdealTreeBuilder extends cls { // ie, doing `foo@bar` we just return foo // but if it's a url or git, we don't know the name until we // fetch it and look in its manifest. - return Promise.all(add.map(rawSpec => { - // We do NOT provide the path here, because user-additions need - // to be resolved relative to the CWD the user is in. - return this[_retrieveSpecName](npa(rawSpec)) - .then(add => this[_updateFilePath](add)) - .then(add => this[_followSymlinkPath](add)) + return Promise.all(add.map(async rawSpec => { + // We do NOT provide the path to npa here, because user-additions + // need to be resolved relative to the CWD the user is in. + const spec = await this[_retrieveSpecName](npa(rawSpec)) + .then(spec => this[_updateFilePath](spec)) + .then(spec => this[_followSymlinkPath](spec)) + spec.tree = tree + return spec })).then(add => { this[_resolvedAdd].push(...add) // now add is a list of spec objects with names. @@ -528,7 +510,7 @@ module.exports = cls => class IdealTreeBuilder extends cls { async [_updateFilePath] (spec) { if (spec.type === 'file') - spec = this[_getRelpathSpec](spec, spec.fetchSpec) + return this[_getRelpathSpec](spec, spec.fetchSpec) return spec } @@ -541,7 +523,7 @@ module.exports = cls => class IdealTreeBuilder extends cls { .catch(/* istanbul ignore next */() => null) ) - spec = this[_getRelpathSpec](spec, real) + return this[_getRelpathSpec](spec, real) } return spec } @@ -561,9 +543,9 @@ module.exports = cls => class IdealTreeBuilder extends cls { // what's in the bundle at each published manifest. Without that, we // can't possibly fix bundled deps without breaking a ton of other stuff, // and leaving the user subject to getting it overwritten later anyway. - [_queueVulnDependents] (options) { - for (const {nodes} of this.auditReport.values()) { - for (const node of nodes) { + async [_queueVulnDependents] (options) { + for (const vuln of this.auditReport.values()) { + for (const node of vuln.nodes) { const bundler = node.getBundler() // XXX this belongs in the audit report itself, not here. @@ -595,6 +577,7 @@ module.exports = cls => class IdealTreeBuilder extends cls { if (this[_force] && this.auditReport && this.auditReport.topVulns.size) { options.add = options.add || [] options.rm = options.rm || [] + const nodesTouched = new Set() for (const [name, topVuln] of this.auditReport.topVulns.entries()) { const { simpleRange, @@ -602,7 +585,7 @@ module.exports = cls => class IdealTreeBuilder extends cls { fixAvailable, } = topVuln for (const node of topNodes) { - if (node !== this.idealTree && node !== this.idealTree.target) { + if (!node.isProjectRoot && !node.isWorkspace) { // not something we're going to fix, sorry. have to cd into // that directory and fix it yourself. this.log.warn('audit', 'Manual fix required in linked project ' + @@ -622,9 +605,13 @@ module.exports = cls => class IdealTreeBuilder extends cls { : 'outside your stated dependency range' this.log.warn('audit', `Updating ${name} to ${version},` + `which is ${breakingMessage}.`) - options.add.push(`${name}@${version}`) + + await this[_add](node, { add: [`${name}@${version}`] }) + nodesTouched.add(node) } } + for (const node of nodesTouched) + node.package = node.package } } @@ -646,7 +633,7 @@ module.exports = cls => class IdealTreeBuilder extends cls { // probably have their own project associated with them. // for every node with one of the names on the list, we add its - // dependents to the queue to be evaluated. in buildDepStem, + // dependents to the queue to be evaluated. in buildDepStep, // anything on the update names list will get refreshed, even if // it isn't a problem. @@ -1025,7 +1012,8 @@ This is a one-time fix-up, please be patient... // also skip over any nodes in the tree that failed to load, since those // will crash the install later on anyway. - const bd = node.isProjectRoot ? null : node.package.bundleDependencies + const bd = node.isProjectRoot || node.isWorkspace ? null + : node.package.bundleDependencies const bundled = new Set(bd || []) return [...node.edgesOut.values()] @@ -1061,8 +1049,8 @@ This is a one-time fix-up, please be patient... if (this[_isVulnerable](edge.to)) return true - // If the user has explicitly asked to install this package, it's a problem. - if (node.isProjectRoot && this[_explicitRequests].has(edge)) + // If the user has explicitly asked to install this package, it's a "problem". + if (this[_explicitRequests].has(edge)) return true // No problems! @@ -1358,7 +1346,7 @@ This is a one-time fix-up, please be patient... // first in x, then in the root, ending with KEEP, because we already // have it. In that case, we ought to REMOVE the nm/x/nm/y node, because // it is an unnecessary duplicate. - this[_pruneDedupable](target, true) + this[_pruneDedupable](target) return [] } @@ -1475,8 +1463,20 @@ This is a one-time fix-up, please be patient... return } if (descend) { - for (const child of node.children.values()) + // sort these so that they're deterministically ordered + // otherwise, resulting tree shape is dependent on the order + // in which they happened to be resolved. + const nodeSort = (a, b) => a.location.localeCompare(b.location, 'en') + + const children = [...node.children.values()].sort(nodeSort) + const fsChildren = [...node.fsChildren].sort(nodeSort) + for (const child of children) this[_pruneDedupable](child) + for (const topNode of fsChildren) { + const children = [...topNode.children.values()].sort(nodeSort) + for (const child of children) + this[_pruneDedupable](child) + } } } diff --git a/node_modules/@npmcli/arborist/lib/arborist/reify.js b/node_modules/@npmcli/arborist/lib/arborist/reify.js index b09a9e0fe16d7..e42c2eac012f2 100644 --- a/node_modules/@npmcli/arborist/lib/arborist/reify.js +++ b/node_modules/@npmcli/arborist/lib/arborist/reify.js @@ -133,12 +133,12 @@ module.exports = cls => class Reifier extends cls { this.addTracker('reify') process.emit('time', 'reify') await this[_validatePath]() - .then(() => this[_loadTrees](options)) - .then(() => this[_diffTrees]()) - .then(() => this[_reifyPackages]()) - .then(() => this[_saveIdealTree](options)) - .then(() => this[_copyIdealToActual]()) - .then(() => this[_awaitQuickAudit]()) + await this[_loadTrees](options) + await this[_diffTrees]() + await this[_reifyPackages]() + await this[_saveIdealTree](options) + await this[_copyIdealToActual]() + await this[_awaitQuickAudit]() this.finishTracker('reify') process.emit('timeEnd', 'reify') @@ -936,7 +936,7 @@ module.exports = cls => class Reifier extends cls { // last but not least, we save the ideal tree metadata to the package-lock // or shrinkwrap file, and any additions or removals to package.json - [_saveIdealTree] (options) { + async [_saveIdealTree] (options) { // the ideal tree is actualized now, hooray! // it still contains all the references to optional nodes that were removed // for install failures. Those still end up in the shrinkwrap, so we @@ -944,23 +944,26 @@ module.exports = cls => class Reifier extends cls { // support save=false option if (options.save === false || this[_global] || this[_dryRun]) - return + return false process.emit('time', 'reify:save') + const updatedTrees = new Set() + // resolvedAdd is the list of user add requests, but with names added // to things like git repos and tarball file/urls. However, if the // user requested 'foo@', and we have a foo@file:../foo, then we should // end up saving the spec we actually used, not whatever they gave us. if (this[_resolvedAdd].length) { - const root = this.idealTree - const pkg = root.package - for (const { name } of this[_resolvedAdd]) { - const req = npa.resolve(name, root.edgesOut.get(name).spec, root.realpath) + for (const { name, tree: addTree } of this[_resolvedAdd]) { + // addTree either the root, or a workspace + const edge = addTree.edgesOut.get(name) + const pkg = addTree.package + const req = npa.resolve(name, edge.spec, addTree.realpath) const {rawSpec, subSpec} = req const spec = subSpec ? subSpec.rawSpec : rawSpec - const child = root.children.get(name) + const child = edge.to let newSpec if (req.registry) { @@ -999,7 +1002,7 @@ module.exports = cls => class Reifier extends cls { // path initially, in which case we can end up with the wrong // thing, so just get the ultimate fetchSpec and relativize it. const p = req.fetchSpec.replace(/^file:/, '') - const rel = relpath(root.realpath, p) + const rel = relpath(addTree.realpath, p) newSpec = `file:${rel}` } else newSpec = req.saveSpec @@ -1031,10 +1034,9 @@ module.exports = cls => class Reifier extends cls { pkg.optionalDependencies[name] = newSpec } } - } - // refresh the edges so they have the correct specs - this.idealTree.package = pkg + updatedTrees.add(addTree) + } } // preserve indentation, if possible @@ -1048,10 +1050,21 @@ module.exports = cls => class Reifier extends cls { : this[_formatPackageLock], } - return Promise.all([ - this[_saveLockFile](saveOpt), - updateRootPackageJson(this.idealTree), - ]).then(() => process.emit('timeEnd', 'reify:save')) + const promises = [this[_saveLockFile](saveOpt)] + + // grab any from explicitRequests that had deps removed + for (const { from: tree } of this.explicitRequests) + updatedTrees.add(tree) + + for (const tree of updatedTrees) { + // refresh the edges so they have the correct specs + tree.package = tree.package + promises.push(updateRootPackageJson(tree)) + } + + await Promise.all(promises) + process.emit('timeEnd', 'reify:save') + return true } async [_saveLockFile] (saveOpt) { @@ -1085,11 +1098,11 @@ module.exports = cls => class Reifier extends cls { // Then we move the entire idealTree over to this.actualTree, and // save the hidden lockfile. if (this.diff && this.diff.filterSet.size) { + const reroot = new Set() + const { filterSet } = this.diff const seen = new Set() for (const [loc, ideal] of this.idealTree.inventory.entries()) { - if (seen.has(loc)) - continue seen.add(loc) // if it's an ideal node from the filter set, then skip it @@ -1113,7 +1126,7 @@ module.exports = cls => class Reifier extends cls { if (isLink && ideal.isLink && ideal.realpath === realpath) continue else - actual.root = this.idealTree + reroot.add(actual) } } @@ -1123,12 +1136,22 @@ module.exports = cls => class Reifier extends cls { if (seen.has(loc)) continue seen.add(loc) + + // we know that this is something that ISN'T in the idealTree, + // or else we will have addressed it in the previous loop. + // If it's in the filterSet, that means we intentionally removed + // it, so nothing to do here. if (filterSet.has(actual)) continue - actual.root = this.idealTree + + reroot.add(actual) } - // prune out any tops that lack a linkIn + // go through the rerooted actual nodes, and move them over. + for (const actual of reroot) + actual.root = this.idealTree + + // prune out any tops that lack a linkIn, they are no longer relevant. for (const top of this.idealTree.tops) { if (top.linksIn.size === 0) top.root = null diff --git a/node_modules/@npmcli/arborist/lib/audit-report.js b/node_modules/@npmcli/arborist/lib/audit-report.js index 76387cde1d66a..139a7aefd2489 100644 --- a/node_modules/@npmcli/arborist/lib/audit-report.js +++ b/node_modules/@npmcli/arborist/lib/audit-report.js @@ -89,7 +89,8 @@ class AuditReport extends Map { constructor (tree, opts = {}) { super() - this[_omit] = new Set(opts.omit || []) + const { omit } = opts + this[_omit] = new Set(omit || []) this.topVulns = new Map() this.calculator = new Calculator(opts) @@ -97,6 +98,7 @@ class AuditReport extends Map { this.options = opts this.log = opts.log || procLog this.tree = tree + this.filterSet = opts.filterSet } async run () { @@ -146,7 +148,7 @@ class AuditReport extends Map { const p = [] for (const node of this.tree.inventory.query('packageName', name)) { - if (shouldOmit(node, this[_omit])) + if (!shouldAudit(node, this[_omit], this.filterSet)) continue // if not vulnerable by this advisory, keep searching @@ -292,7 +294,7 @@ class AuditReport extends Map { try { try { // first try the super fast bulk advisory listing - const body = prepareBulkData(this.tree, this[_omit]) + const body = prepareBulkData(this.tree, this[_omit], this.filterSet) this.log.silly('audit', 'bulk request', body) // no sense asking if we don't have anything to audit, @@ -333,22 +335,25 @@ class AuditReport extends Map { } } -// return true if we should ignore this one -const shouldOmit = (node, omit) => - !node.version ? true - : node.isRoot ? true - : omit.size === 0 ? false - : node.dev && omit.has('dev') || +// return true if we should audit this one +const shouldAudit = (node, omit, filterSet) => + !node.version ? false + : node.isRoot ? false + : filterSet && filterSet.size !== 0 && !filterSet.has(node) ? false + : omit.size === 0 ? true + : !( // otherwise, just ensure we're not omitting this one + node.dev && omit.has('dev') || node.optional && omit.has('optional') || node.devOptional && omit.has('dev') && omit.has('optional') || node.peer && omit.has('peer') + ) -const prepareBulkData = (tree, omit) => { +const prepareBulkData = (tree, omit, filterSet) => { const payload = {} for (const name of tree.inventory.query('packageName')) { const set = new Set() for (const node of tree.inventory.query('packageName', name)) { - if (shouldOmit(node, omit)) + if (!shouldAudit(node, omit, filterSet)) continue set.add(node.version) diff --git a/node_modules/@npmcli/arborist/lib/node.js b/node_modules/@npmcli/arborist/lib/node.js index 370bfc9567d28..1bb84140c21e2 100644 --- a/node_modules/@npmcli/arborist/lib/node.js +++ b/node_modules/@npmcli/arborist/lib/node.js @@ -896,14 +896,14 @@ class Node { return false // it's a top level pkg, or a dep of one - if (!this.parent || !this.parent.parent) + if (!this.resolveParent || !this.resolveParent.resolveParent) return false // no one wants it, remove it if (this.edgesIn.size === 0) return true - const other = this.parent.parent.resolve(this.name) + const other = this.resolveParent.resolveParent.resolve(this.name) // nothing else, need this one if (!other) diff --git a/package-lock.json b/package-lock.json index a29f2471aacc8..f050ce62b66b1 100644 --- a/package-lock.json +++ b/package-lock.json @@ -78,7 +78,7 @@ ], "license": "Artistic-2.0", "dependencies": { - "@npmcli/arborist": "^2.5.0", + "@npmcli/arborist": "github:npm/arborist#isaacs/audit-workspace", "@npmcli/ci-detect": "^1.2.0", "@npmcli/config": "^2.2.0", "@npmcli/run-script": "^1.8.5", @@ -713,9 +713,9 @@ }, "node_modules/@npmcli/arborist": { "version": "2.5.0", - "resolved": "https://registry.npmjs.org/@npmcli/arborist/-/arborist-2.5.0.tgz", - "integrity": "sha512-YPSkV/8vofpbAJyeu52J12YnC5VTkYIcfcNkRoSW6qjfQG+QybgbJtCbcdx+M0YxfdzDKS6iDTjpNMoETZ8HOA==", + "resolved": "git+ssh://git@github.com/npm/arborist.git#e4861eae740ed46d7f3573c514ceb13d7a9ebf9f", "inBundle": true, + "license": "ISC", "dependencies": { "@npmcli/installed-package-contents": "^1.0.7", "@npmcli/map-workspaces": "^1.0.2", @@ -10823,9 +10823,8 @@ "dev": true }, "@npmcli/arborist": { - "version": "2.5.0", - "resolved": "https://registry.npmjs.org/@npmcli/arborist/-/arborist-2.5.0.tgz", - "integrity": "sha512-YPSkV/8vofpbAJyeu52J12YnC5VTkYIcfcNkRoSW6qjfQG+QybgbJtCbcdx+M0YxfdzDKS6iDTjpNMoETZ8HOA==", + "version": "git+ssh://git@github.com/npm/arborist.git#e4861eae740ed46d7f3573c514ceb13d7a9ebf9f", + "from": "@npmcli/arborist@github:npm/arborist#isaacs/audit-workspace", "requires": { "@npmcli/installed-package-contents": "^1.0.7", "@npmcli/map-workspaces": "^1.0.2", diff --git a/package.json b/package.json index bc2faabf06ab5..ae6bee7732943 100644 --- a/package.json +++ b/package.json @@ -42,7 +42,7 @@ "./package.json": "./package.json" }, "dependencies": { - "@npmcli/arborist": "^2.5.0", + "@npmcli/arborist": "github:npm/arborist#isaacs/audit-workspace", "@npmcli/ci-detect": "^1.2.0", "@npmcli/config": "^2.2.0", "@npmcli/run-script": "^1.8.5",