From 04a3e8c10c3f38e1c7a35976d77c2929bdc39868 Mon Sep 17 00:00:00 2001 From: isaacs Date: Tue, 10 Nov 2020 11:23:38 -0800 Subject: [PATCH] @npmcli/arborist@1.0.10 * prevent self-assignment of parent/fsParent * Support update options in global package space Fix: #1962 --- .../arborist/lib/arborist/build-ideal-tree.js | 51 +++++++++++-------- .../arborist/lib/arborist/load-virtual.js | 7 +++ .../@npmcli/arborist/lib/arborist/reify.js | 6 ++- node_modules/@npmcli/arborist/lib/node.js | 28 ++++++++-- node_modules/@npmcli/arborist/package.json | 2 +- package-lock.json | 14 ++--- package.json | 2 +- 7 files changed, 76 insertions(+), 34 deletions(-) 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 582575ebfad29..5fa8023bdbd67 100644 --- a/node_modules/@npmcli/arborist/lib/arborist/build-ideal-tree.js +++ b/node_modules/@npmcli/arborist/lib/arborist/build-ideal-tree.js @@ -7,6 +7,10 @@ const semver = require('semver') const promiseCallLimit = require('promise-call-limit') const getPeerSet = require('../peer-set.js') const realpath = require('../../lib/realpath.js') +const walkUpPath = require('walk-up-path') +const { dirname, resolve } = require('path') +const { promisify } = require('util') +const readdir = promisify(require('readdir-scoped-modules')) const debug = require('../debug.js') const fromPath = require('../from-path.js') @@ -182,8 +186,10 @@ module.exports = cls => class IdealTreeBuilder extends cls { process.emit('time', 'idealTree') - if (!options.add && !options.rm && this[_global]) - return Promise.reject(new Error('global requires an add or rm option')) + if (!options.add && !options.rm && !options.update && this[_global]) { + const er = new Error('global requires add, rm, or update option') + return Promise.reject(er) + } // first get the virtual tree, if possible. If there's a lockfile, then // that defines the ideal tree, unless the root package.json is not @@ -305,7 +311,6 @@ module.exports = cls => class IdealTreeBuilder extends cls { // cases we don't use a lockfile anyway. // Load on a new Arborist object, so the Nodes aren't the same, // or else it'll get super confusing when we change them! - // Only have to mapWorkspaces if we didn't get it from actual or virtual .then(async root => { if (!this[_updateAll] && !this[_global] && !root.meta.loadedFromDisk) await new this.constructor(this.options).loadActual({ root }) @@ -322,10 +327,10 @@ module.exports = cls => class IdealTreeBuilder extends cls { } [_globalRootNode] () { - const root = this[_rootNodeFromPackage]({}) + const root = this[_rootNodeFromPackage]({ dependencies: {} }) // this is a gross kludge to handle the fact that we don't save // metadata on the root node in global installs, because the "root" - // node is something like /usr/local/lib/node_modules. + // node is something like /usr/local/lib. const meta = new Shrinkwrap({ path: this.path }) meta.reset() root.meta = meta @@ -353,9 +358,19 @@ module.exports = cls => class IdealTreeBuilder extends cls { // If we have a list of package names to update, and we know it's // going to update them wherever they are, add any paths into those // named nodes to the buildIdealTree queue. - if (this[_updateNames].length) + if (!this[_global] && this[_updateNames].length) this[_queueNamedUpdates]() + // global updates only update the globalTop nodes, but we need to know + // that they're there, and not reinstall the world unnecessarily. + if (this[_global] && (this[_updateAll] || this[_updateNames].length)) { + const nm = resolve(this.path, 'node_modules') + for (const name of await readdir(nm)) { + if (this[_updateAll] || this[_updateNames].includes(name)) + this.idealTree.package.dependencies[name] = '*' + } + } + if (this.auditReport && this.auditReport.size > 0) this[_queueVulnDependents](options) @@ -1519,31 +1534,27 @@ This is a one-time fix-up, please be patient... const external = /^\.\.(\/|$)/.test(loc) - if (external && !this[_follow]) { - // outside the root, somebody else's problem, ignore it - continue - } - if (!link.target.parent && !link.target.fsParent) { - // the fsParent MUST be some node in the tree, possibly the root. - // find it by walking up. Note that this is where its deps may - // end up being installed, if possible. - const parts = loc.split('/') - for (let p = parts.length - 1; p > -1; p--) { - const path = parts.slice(0, p).join('/') - if (!path && external) - break + // the fsParent likely some node in the tree, possibly the root, + // unless it is external. find it by walking up. Note that this + // is where its deps may end up being installed, if possible. + for (const p of walkUpPath(dirname(realpath))) { + const path = relpath(this.path, p) const node = !path ? this.idealTree : this.idealTree.inventory.get(path) if (node) { link.target.fsParent = node this.addTracker('idealTree', link.target.name, link.target.location) this[_depsQueue].push(link.target) - p = -1 + break } } } + // outside the root, somebody else's problem, ignore it + if (external && !this[_follow]) + continue + // didn't find a parent for it or it has not been seen yet // so go ahead and process it. const unseenLink = (link.target.parent || link.target.fsParent) diff --git a/node_modules/@npmcli/arborist/lib/arborist/load-virtual.js b/node_modules/@npmcli/arborist/lib/arborist/load-virtual.js index 924d720de752c..e335bdadd4541 100644 --- a/node_modules/@npmcli/arborist/lib/arborist/load-virtual.js +++ b/node_modules/@npmcli/arborist/lib/arborist/load-virtual.js @@ -204,12 +204,19 @@ module.exports = cls => class VirtualLoader extends cls { [assignParentage] (nodes) { for (const [location, node] of nodes) { + // Skip assignment of parentage for the root package + if (!location) + continue const { path, name } = node for (const p of walkUp(dirname(path))) { const ploc = relpath(this.path, p) const parent = nodes.get(ploc) if (!parent) continue + // Safety check: avoid self-assigning nodes as their own parents + /* istanbul ignore if - should be obviated by parentage skip check */ + if (parent === node) + continue const locTest = `${ploc}/node_modules/${name}`.replace(/^\//, '') const ptype = location === locTest diff --git a/node_modules/@npmcli/arborist/lib/arborist/reify.js b/node_modules/@npmcli/arborist/lib/arborist/reify.js index 8711f8d298534..b10637bd05543 100644 --- a/node_modules/@npmcli/arborist/lib/arborist/reify.js +++ b/node_modules/@npmcli/arborist/lib/arborist/reify.js @@ -172,7 +172,7 @@ module.exports = cls => class Reifier extends cls { ignoreMissing: true, global: true, filter: (node, kid) => !node.isRoot ? true - : this[_explicitRequests].has(kid), + : (node.edgesOut.has(kid) || this[_explicitRequests].has(kid)), } : { ignoreMissing: true } if (!this[_global]) { @@ -182,7 +182,9 @@ module.exports = cls => class Reifier extends cls { // the global install space tends to have a lot of stuff in it. don't // load all of it, just what we care about. we won't be saving a - // hidden lockfile in there anyway. + // hidden lockfile in there anyway. Note that we have to load ideal + // BEFORE loading actual, so that the actualOpt can use the + // explicitRequests which is set during buildIdealTree return this.buildIdealTree(bitOpt) .then(() => this.loadActual(actualOpt)) .then(() => process.emit('timeEnd', 'reify:loadTrees')) diff --git a/node_modules/@npmcli/arborist/lib/node.js b/node_modules/@npmcli/arborist/lib/node.js index a783ce9c97572..5671a033e3413 100644 --- a/node_modules/@npmcli/arborist/lib/node.js +++ b/node_modules/@npmcli/arborist/lib/node.js @@ -566,13 +566,32 @@ class Node { } debug(() => { - if (!this.fsParent && this.realpath.indexOf(fsParent.realpath) !== 0) - throw new Error('attempting to set fsParent improperly') + if (fsParent === this) + throw new Error('setting node to its own fsParent') + + if (fsParent.realpath === this.realpath) + throw new Error('setting fsParent to same path') + + // the initial set MUST be an actual walk-up from the realpath + // subsequent sets will re-root on the new fsParent's path. + if (!this[_fsParent] && this.realpath.indexOf(fsParent.realpath) !== 0) { + throw Object.assign(new Error('setting fsParent improperly'), { + path: this.path, + realpath: this.realpath, + fsParent: { + path: fsParent.path, + realpath: fsParent.realpath, + }, + }) + } if (fsParent.isLink) - throw new Error('attempting to set fsParent to link node') + throw new Error('setting fsParent to link node') }) + if (this === fsParent || fsParent.realpath === this.realpath) + return + // prune off the original location, so we don't leave edges lying around if (current) this.fsParent = null @@ -731,6 +750,9 @@ class Node { set parent (parent) { const oldParent = this[_parent] + if (this === parent) + return + // link nodes can't contain children directly. // children go under the link target. if (parent) { diff --git a/node_modules/@npmcli/arborist/package.json b/node_modules/@npmcli/arborist/package.json index 99522745cb303..44c176ec75e5d 100644 --- a/node_modules/@npmcli/arborist/package.json +++ b/node_modules/@npmcli/arborist/package.json @@ -1,6 +1,6 @@ { "name": "@npmcli/arborist", - "version": "1.0.9", + "version": "1.0.10", "description": "Manage node_modules trees", "dependencies": { "@npmcli/installed-package-contents": "^1.0.5", diff --git a/package-lock.json b/package-lock.json index 5c5b78a5580ad..03275f6b1bd35 100644 --- a/package-lock.json +++ b/package-lock.json @@ -78,7 +78,7 @@ ], "license": "Artistic-2.0", "dependencies": { - "@npmcli/arborist": "^1.0.9", + "@npmcli/arborist": "^1.0.10", "@npmcli/ci-detect": "^1.2.0", "@npmcli/config": "^1.2.1", "@npmcli/run-script": "^1.7.5", @@ -386,9 +386,9 @@ } }, "node_modules/@npmcli/arborist": { - "version": "1.0.9", - "resolved": "https://registry.npmjs.org/@npmcli/arborist/-/arborist-1.0.9.tgz", - "integrity": "sha512-bvaPKdlT4F0U2JT0eV1fprv9dWdjFxY+O7WbXMvABvyO81kUPTJTBe1HhsGTIqXTrt2cOqXtLJI31Xs7iugptg==", + "version": "1.0.10", + "resolved": "https://registry.npmjs.org/@npmcli/arborist/-/arborist-1.0.10.tgz", + "integrity": "sha512-k5HwMlztD7clml2PJJLMS01QWUolzw6MXOyibhipmTHtKjsh5d2WtQNvPMxNYWLyACpJu9xIfK2OGaJpuNwbjA==", "inBundle": true, "dependencies": { "@npmcli/installed-package-contents": "^1.0.5", @@ -9094,9 +9094,9 @@ } }, "@npmcli/arborist": { - "version": "1.0.9", - "resolved": "https://registry.npmjs.org/@npmcli/arborist/-/arborist-1.0.9.tgz", - "integrity": "sha512-bvaPKdlT4F0U2JT0eV1fprv9dWdjFxY+O7WbXMvABvyO81kUPTJTBe1HhsGTIqXTrt2cOqXtLJI31Xs7iugptg==", + "version": "1.0.10", + "resolved": "https://registry.npmjs.org/@npmcli/arborist/-/arborist-1.0.10.tgz", + "integrity": "sha512-k5HwMlztD7clml2PJJLMS01QWUolzw6MXOyibhipmTHtKjsh5d2WtQNvPMxNYWLyACpJu9xIfK2OGaJpuNwbjA==", "requires": { "@npmcli/installed-package-contents": "^1.0.5", "@npmcli/map-workspaces": "^1.0.1", diff --git a/package.json b/package.json index 5ac2f12538324..ebcdfc4b5b9d2 100644 --- a/package.json +++ b/package.json @@ -42,7 +42,7 @@ "./package.json": "./package.json" }, "dependencies": { - "@npmcli/arborist": "^1.0.9", + "@npmcli/arborist": "^1.0.10", "@npmcli/ci-detect": "^1.2.0", "@npmcli/config": "^1.2.1", "@npmcli/run-script": "^1.7.5",