@npmcli/arborist@2.8.2

 * fix: treat top-level global packages as "top" nodes
 * fix: load global symlinks implicitly as file: deps
 * fix(reify): debug crash when extracting into symlink
 * fix: node_modules must be a directory
 * fix: make Node.children() a case-insensitive Map
 * fix(reify): verify existing deps in nm are dirs

8 files changed, 186 insertions, 36 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 679d52582..cda7f8acf 100644
--- a/node_modules/@npmcli/arborist/lib/arborist/build-ideal-tree.js
+++ b/node_modules/@npmcli/arborist/lib/arborist/build-ideal-tree.js
@@ -9,6 +9,9 @@ const { resolve, dirname } = require('path')
 const { promisify } = require('util')
 const treeCheck = require('../tree-check.js')
 const readdir = promisify(require('readdir-scoped-modules'))
+const fs = require('fs')
+const lstat = promisify(fs.lstat)
+const readlink = promisify(fs.readlink)
 const { depth } = require('treeverse')
 
 const {
@@ -407,7 +410,14 @@ module.exports = cls => class IdealTreeBuilder extends cls {
         if (this[_updateAll] || updateName) {
           if (updateName)
             globalExplicitUpdateNames.push(name)
-          tree.package.dependencies[name] = '*'
+          const dir = resolve(nm, name)
+          const st = await lstat(dir).catch(/* istanbul ignore next */ er => null)
+          if (st && st.isSymbolicLink()) {
+            const target = await readlink(dir)
+            const real = resolve(dirname(dir), target)
+            tree.package.dependencies[name] = `file:${real}`
+          } else
+            tree.package.dependencies[name] = '*'
         }
       }
     }
@@ -1015,6 +1025,15 @@ This is a one-time fix-up, please be patient...
     // keep track of the thing that caused this node to be included.
     const src = parent.sourceReference
     this[_peerSetSource].set(node, src)
+
+    // do not load the peers along with the set if this is a global top pkg
+    // otherwise we'll be tempted to put peers as other top-level installed
+    // things, potentially clobbering what's there already, which is not
+    // what we want.  the missing edges will be picked up on the next pass.
+    if (this[_global] && edge.from.isProjectRoot)
+      return node
+
+    // otherwise, we have to make sure that our peers can go along with us.
     return this[_loadPeerSet](node, required)
   }
 
diff --git a/node_modules/@npmcli/arborist/lib/arborist/load-actual.js b/node_modules/@npmcli/arborist/lib/arborist/load-actual.js
index 86856d868..0338b2cd8 100644
--- a/node_modules/@npmcli/arborist/lib/arborist/load-actual.js
+++ b/node_modules/@npmcli/arborist/lib/arborist/load-actual.js
@@ -188,8 +188,10 @@ module.exports = cls => class ActualLoader extends cls {
       const tree = this[_actualTree]
       const actualRoot = tree.isLink ? tree.target : tree
       const { dependencies = {} } = actualRoot.package
-      for (const name of actualRoot.children.keys())
-        dependencies[name] = dependencies[name] || '*'
+      for (const [name, kid] of actualRoot.children.entries()) {
+        const def = kid.isLink ? `file:${kid.realpath}` : '*'
+        dependencies[name] = dependencies[name] || def
+      }
       actualRoot.package = { ...actualRoot.package, dependencies }
     }
     return this[_actualTree]
diff --git a/node_modules/@npmcli/arborist/lib/arborist/reify.js b/node_modules/@npmcli/arborist/lib/arborist/reify.js
index 1cfa6034e..965435f84 100644
--- a/node_modules/@npmcli/arborist/lib/arborist/reify.js
+++ b/node_modules/@npmcli/arborist/lib/arborist/reify.js
@@ -6,11 +6,13 @@ const AuditReport = require('../audit-report.js')
 const {subset, intersects} = require('semver')
 const npa = require('npm-package-arg')
 const debug = require('../debug.js')
+const walkUp = require('walk-up-path')
 
 const {dirname, resolve, relative} = require('path')
 const {depth: dfwalk} = require('treeverse')
 const fs = require('fs')
 const {promisify} = require('util')
+const lstat = promisify(fs.lstat)
 const symlink = promisify(fs.symlink)
 const mkdirp = require('mkdirp-infer-owner')
 const justMkdirp = require('mkdirp')
@@ -19,6 +21,7 @@ const rimraf = promisify(require('rimraf'))
 const PackageJson = require('@npmcli/package-json')
 const packageContents = require('@npmcli/installed-package-contents')
 const { checkEngine, checkPlatform } = require('npm-install-checks')
+const _force = Symbol.for('force')
 
 const treeCheck = require('../tree-check.js')
 const relpath = require('../relpath.js')
@@ -76,8 +79,10 @@ const _copyIdealToActual = Symbol('copyIdealToActual')
 const _addOmitsToTrashList = Symbol('addOmitsToTrashList')
 const _packageLockOnly = Symbol('packageLockOnly')
 const _dryRun = Symbol('dryRun')
+const _validateNodeModules = Symbol('validateNodeModules')
+const _nmValidated = Symbol('nmValidated')
 const _validatePath = Symbol('validatePath')
-const _reifyPackages = Symbol('reifyPackages')
+const _reifyPackages = Symbol.for('reifyPackages')
 
 const _omitDev = Symbol('omitDev')
 const _omitOptional = Symbol('omitOptional')
@@ -119,6 +124,7 @@ module.exports = cls => class Reifier extends cls {
     this[_bundleUnpacked] = new Set()
     // child nodes we'd EXPECT to be included in a bundle, but aren't
     this[_bundleMissing] = new Set()
+    this[_nmValidated] = new Set()
   }
 
   // public method
@@ -159,6 +165,9 @@ module.exports = cls => class Reifier extends cls {
     // recursively, because it can have other side effects to do that
     // in a project directory.  We just want to make it if it's missing.
     await justMkdirp(resolve(this.path))
+
+    // do not allow the top-level node_modules to be a symlink
+    await this[_validateNodeModules](resolve(this.path, 'node_modules'))
   }
 
   async [_reifyPackages] () {
@@ -328,12 +337,13 @@ module.exports = cls => class Reifier extends cls {
       ideal: this.idealTree,
     })
 
-    for (const node of this.diff.removed) {
-      // a node in a dep bundle will only be removed if its bundling dep
-      // is removed as well.  in which case, we don't have to delete it!
-      if (!node.inDepBundle)
-        this[_addNodeToTrashList](node)
-    }
+    // we don't have to add 'removed' folders to the trashlist, because
+    // they'll be moved aside to a retirement folder, and then the retired
+    // folder will be deleted at the end.  This is important when we have
+    // a folder like FOO being "removed" in favor of a folder like "foo",
+    // because if we remove node_modules/FOO on case-insensitive systems,
+    // it will remove the dep that we *want* at node_modules/foo.
+
     process.emit('timeEnd', 'reify:diffTrees')
   }
 
@@ -429,19 +439,40 @@ module.exports = cls => class Reifier extends cls {
     process.emit('time', 'reify:createSparse')
     // if we call this fn again, we look for the previous list
     // so that we can avoid making the same directory multiple times
-    const dirs = this.diff.leaves
+    const leaves = this.diff.leaves
       .filter(diff => {
         return (diff.action === 'ADD' || diff.action === 'CHANGE') &&
           !this[_sparseTreeDirs].has(diff.ideal.path) &&
           !diff.ideal.isLink
       })
-      .map(diff => diff.ideal.path)
-
-    return promiseAllRejectLate(dirs.map(d => mkdirp(d)))
-      .then(made => {
-        made.forEach(made => this[_sparseTreeRoots].add(made))
-        dirs.forEach(dir => this[_sparseTreeDirs].add(dir))
-      })
+      .map(diff => diff.ideal)
+
+    // we check this in parallel, so guard against multiple attempts to
+    // retire the same path at the same time.
+    const dirsChecked = new Set()
+    return promiseAllRejectLate(leaves.map(async node => {
+      for (const d of walkUp(node.path)) {
+        if (d === node.top.path)
+          break
+        if (dirsChecked.has(d))
+          continue
+        dirsChecked.add(d)
+        const st = await lstat(d).catch(er => null)
+        // this can happen if we have a link to a package with a name
+        // that the filesystem treats as if it is the same thing.
+        // would be nice to have conditional istanbul ignores here...
+        /* istanbul ignore next - defense in depth */
+        if (st && !st.isDirectory()) {
+          const retired = retirePath(d)
+          this[_retiredPaths][d] = retired
+          this[_trashList].add(retired)
+          await this[_renamePath](d, retired)
+        }
+      }
+      const made = await mkdirp(node.path)
+      this[_sparseTreeDirs].add(node.path)
+      this[_sparseTreeRoots].add(made)
+    }))
       .then(() => process.emit('timeEnd', 'reify:createSparse'))
   }
 
@@ -536,7 +567,20 @@ module.exports = cls => class Reifier extends cls {
       })
   }
 
-  [_extractOrLink] (node) {
+  // do not allow node_modules to be a symlink
+  async [_validateNodeModules] (nm) {
+    if (this[_force] || this[_nmValidated].has(nm))
+      return
+    const st = await lstat(nm).catch(() => null)
+    if (!st || st.isDirectory()) {
+      this[_nmValidated].add(nm)
+      return
+    }
+    this.log.warn('reify', 'Removing non-directory', nm)
+    await rimraf(nm)
+  }
+
+  async [_extractOrLink] (node) {
     // in normal cases, node.resolved should *always* be set by now.
     // however, it is possible when a lockfile is damaged, or very old,
     // or in some other race condition bugs in npm v6, that a previously
@@ -563,13 +607,29 @@ module.exports = cls => class Reifier extends cls {
       return
     }
 
-    return node.isLink
-      ? rimraf(node.path).then(() => this[_symlink](node))
-      : pacote.extract(res, node.path, {
+    const nm = resolve(node.parent.path, 'node_modules')
+    await this[_validateNodeModules](nm)
+
+    if (node.isLink) {
+      await rimraf(node.path)
+      await this[_symlink](node)
+    } else {
+      await debug(async () => {
+        const st = await lstat(node.path).catch(e => null)
+        if (st && !st.isDirectory()) {
+          debug.log('unpacking into a non-directory', node)
+          throw Object.assign(new Error('ENOTDIR: not a directory'), {
+            code: 'ENOTDIR',
+            path: node.path,
+          })
+        }
+      })
+      await pacote.extract(res, node.path, {
         ...this.options,
         resolved: node.resolved,
         integrity: node.integrity,
       })
+    }
   }
 
   async [_symlink] (node) {
diff --git a/node_modules/@npmcli/arborist/lib/case-insensitive-map.js b/node_modules/@npmcli/arborist/lib/case-insensitive-map.js
new file mode 100644
index 000000000..8254c3f7a
--- /dev/null
+++ b/node_modules/@npmcli/arborist/lib/case-insensitive-map.js
@@ -0,0 +1,48 @@
+// package children are represented with a Map object, but many file systems
+// are case-insensitive and unicode-normalizing, so we need to treat
+// node.children.get('FOO') and node.children.get('foo') as the same thing.
+
+const _keys = Symbol('keys')
+const _normKey = Symbol('normKey')
+const normalize = s => s.normalize('NFKD').toLowerCase()
+const OGMap = Map
+module.exports = class Map extends OGMap {
+  constructor (items = []) {
+    super()
+    this[_keys] = new OGMap()
+    for (const [key, val] of items)
+      this.set(key, val)
+  }
+
+  [_normKey] (key) {
+    return typeof key === 'string' ? normalize(key) : key
+  }
+
+  get (key) {
+    const normKey = this[_normKey](key)
+    return this[_keys].has(normKey) ? super.get(this[_keys].get(normKey))
+      : undefined
+  }
+
+  set (key, val) {
+    const normKey = this[_normKey](key)
+    if (this[_keys].has(normKey))
+      super.delete(this[_keys].get(normKey))
+    this[_keys].set(normKey, key)
+    return super.set(key, val)
+  }
+
+  delete (key) {
+    const normKey = this[_normKey](key)
+    if (this[_keys].has(normKey)) {
+      const prevKey = this[_keys].get(normKey)
+      this[_keys].delete(normKey)
+      return super.delete(prevKey)
+    }
+  }
+
+  has (key) {
+    const normKey = this[_normKey](key)
+    return this[_keys].has(normKey) && super.has(this[_keys].get(normKey))
+  }
+}
diff --git a/node_modules/@npmcli/arborist/lib/deepest-nesting-target.js b/node_modules/@npmcli/arborist/lib/deepest-nesting-target.js
index cbaa396f3..9c433a765 100644
--- a/node_modules/@npmcli/arborist/lib/deepest-nesting-target.js
+++ b/node_modules/@npmcli/arborist/lib/deepest-nesting-target.js
@@ -5,7 +5,7 @@
 const deepestNestingTarget = (start, name) => {
   for (const target of start.ancestry()) {
     // note: this will skip past the first target if edge is peer
-    if (target.isProjectRoot || !target.resolveParent)
+    if (target.isProjectRoot || !target.resolveParent || target.globalTop)
       return target
     const targetEdge = target.edgesOut.get(name)
     if (!targetEdge || !targetEdge.peer)
diff --git a/node_modules/@npmcli/arborist/lib/edge.js b/node_modules/@npmcli/arborist/lib/edge.js
index 0bd9021d5..9d5ece40e 100644
--- a/node_modules/@npmcli/arborist/lib/edge.js
+++ b/node_modules/@npmcli/arborist/lib/edge.js
@@ -77,7 +77,7 @@ class Edge {
   }
 
   satisfiedBy (node) {
-    return depValid(node, this.spec, this.accept, this.from)
+    return node.name === this.name && depValid(node, this.spec, this.accept, this.from)
   }
 
   explain (seen = []) {
@@ -167,7 +167,7 @@ class Edge {
   [_loadError] () {
     return !this[_to] ? (this.optional ? null : 'MISSING')
       : this.peer && this.from === this.to.parent && !this.from.isTop ? 'PEER LOCAL'
-      : !depValid(this.to, this.spec, this.accept, this.from) ? 'INVALID'
+      : !this.satisfiedBy(this.to) ? 'INVALID'
       : 'OK'
   }
 
diff --git a/node_modules/@npmcli/arborist/lib/node.js b/node_modules/@npmcli/arborist/lib/node.js
index d77b18355..5616019dd 100644
--- a/node_modules/@npmcli/arborist/lib/node.js
+++ b/node_modules/@npmcli/arborist/lib/node.js
@@ -66,6 +66,7 @@ const relpath = require('./relpath.js')
 const consistentResolve = require('./consistent-resolve.js')
 
 const printableTree = require('./printable.js')
+const CaseInsensitiveMap = require('./case-insensitive-map.js')
 
 class Node {
   constructor (options) {
@@ -148,7 +149,7 @@ class Node {
     this.hasShrinkwrap = hasShrinkwrap || pkg._hasShrinkwrap || false
     this.legacyPeerDeps = legacyPeerDeps
 
-    this.children = new Map()
+    this.children = new CaseInsensitiveMap()
     this.fsChildren = new Set()
     this.inventory = new Inventory({})
     this.tops = new Set()
@@ -181,7 +182,7 @@ class Node {
     }
 
     this.edgesIn = new Set()
-    this.edgesOut = new Map()
+    this.edgesOut = new CaseInsensitiveMap()
 
     // have to set the internal package ref before assigning the parent,
     // because this.package is read when adding to inventory
@@ -248,7 +249,7 @@ class Node {
 
   // true for packages installed directly in the global node_modules folder
   get globalTop () {
-    return this.global && this.parent.isProjectRoot
+    return this.global && this.parent && this.parent.isProjectRoot
   }
 
   get workspaces () {
@@ -478,6 +479,9 @@ class Node {
   }
 
   get isProjectRoot () {
+    // only treat as project root if it's the actual link that is the root,
+    // or the target of the root link, but NOT if it's another link to the
+    // same root that happens to be somewhere else.
     return this === this.root || this === this.root.target
   }
 
@@ -772,9 +776,15 @@ class Node {
     this[_loadDepType](this.package.dependencies, 'prod')
     this[_loadDepType](this.package.optionalDependencies, 'optional')
 
-    const { isTop, path, sourceReference } = this
-    const { isTop: srcTop, path: srcPath } = sourceReference || {}
-    if (isTop && path && (!sourceReference || srcTop && srcPath))
+    const { globalTop, isTop, path, sourceReference } = this
+    const {
+      globalTop: srcGlobalTop,
+      isTop: srcTop,
+      path: srcPath,
+    } = sourceReference || {}
+    const thisDev = isTop && !globalTop && path
+    const srcDev = !sourceReference || srcTop && !srcGlobalTop && srcPath
+    if (thisDev && srcDev)
       this[_loadDepType](this.package.devDependencies, 'dev')
   }
 
@@ -1014,8 +1024,20 @@ class Node {
 
   replace (node) {
     this[_delistFromMeta]()
-    this.path = node.path
-    this.name = node.name
+
+    // if the name matches, but is not identical, we are intending to clobber
+    // something case-insensitively, so merely setting name and path won't
+    // have the desired effect.  just set the path so it'll collide in the
+    // parent's children map, and leave it at that.
+    const nameMatch = node.parent &&
+      node.parent.children.get(this.name) === node
+    if (nameMatch)
+      this.path = resolve(node.parent.path, 'node_modules', this.name)
+    else {
+      this.path = node.path
+      this.name = node.name
+    }
+
     if (!this.isLink)
       this.realpath = this.path
     this[_refreshLocation]()
@@ -1210,7 +1232,7 @@ class Node {
   }
 
   get isTop () {
-    return !this.parent
+    return !this.parent || this.globalTop
   }
 
   get top () {
diff --git a/node_modules/@npmcli/arborist/package.json b/node_modules/@npmcli/arborist/package.json
index 01f018a62..42ec2eca3 100644
--- a/node_modules/@npmcli/arborist/package.json
+++ b/node_modules/@npmcli/arborist/package.json
@@ -1,6 +1,6 @@
 {
   "name": "@npmcli/arborist",
-  "version": "2.8.1",
+  "version": "2.8.2",
   "description": "Manage node_modules trees",
   "dependencies": {
     "@npmcli/installed-package-contents": "^1.0.7",
@@ -32,7 +32,6 @@
     "rimraf": "^3.0.2",
     "semver": "^7.3.5",
     "ssri": "^8.0.1",
-    "tar": "^6.1.0",
     "treeverse": "^1.0.4",
     "walk-up-path": "^1.0.0"
   },