@npmcli/arborist - npm Package Compare versions

Comparing version 0.0.0-pre.20 to 0.0.0-pre.21

lib/arborist/load-virtual.js

@@ -128,4 +128,9 @@ // mixin providing the loadVirtual method
         // actually a v2 lockfile metadata entry.
-        if (ptype === 'parent' && node.package.inBundle && parent.edgesOut.has(name)) {
-          const ppkg = parent.package
+        // If the *parent* is also bundled, though, then we assume
+        // that it's being pulled in just by virtue of that.
+        const {inBundle} = node.package
+        const ppkg = parent.package
+        const {inBundle: parentBundled} = ppkg
+        const hasEdge = parent.edgesOut.has(name)
+        if (ptype === 'parent' && inBundle && hasEdge && !parentBundled) {
           if (!ppkg.bundleDependencies)
@@ -132,0 +137,0 @@ ppkg.bundleDependencies = [name]

lib/audit-report.js

@@ -6,2 +6,3 @@ // an object representing the set of vulnerabilities in a tree
 const pacote = require('pacote')
+const semver = require('semver')
 
@@ -115,12 +116,12 @@ const Vuln = require('./vuln.js')
     process.emit('time', 'auditReport:init')
+
     const promises = []
-    for (const advisory of Object.values(this.report.advisories)) {
-      const {
-        module_name: name,
-        vulnerable_versions: range,
-      } = advisory
-      promises.push(this[_addVulnerability](name, range, advisory))
+    for (const [name, advisories] of Object.entries(this.report)) {
+      for (const advisory of advisories) {
+        const { vulnerable_versions: range } = advisory
+        promises.push(this[_addVulnerability](name, range, advisory))
+      }
     }
+    await Promise.all(promises)
 
-    await Promise.all(promises)
     process.emit('timeEnd', 'auditReport:init')
@@ -166,3 +167,3 @@ }
   // returns true if every satisfying version is vulnerable.
-  [_specVulnerable] (paku, spec, avoid) {
+  [_specVulnerable] (paku, spec, avoid, bundled) {
     spec = this[_specVulnCheck](paku, spec)
@@ -172,2 +173,8 @@ if (spec === false)
 
+    // if it's a bundle dep, then we must avoid it if the vulnerable
+    // range and the dep range intersect, since we can't update in-place.
+    if (bundled) {
+      return semver.intersects(spec, avoid, { includePrerelease: true })
+    }
+
     // if we can't avoid the vulnerable version range within the spec
@@ -219,2 +226,3 @@ // required, then the dep range is entirely vulnerable.
     this[_vulnDependents].delete(p)
+    const bd = p.package.bundleDependencies || []
     for (const edge of p.edgesOut.values()) {
@@ -227,5 +235,6 @@ if (!this.isVulnerable(edge.to))
       const vuln = this.get(name)
+      const bundled = bd.includes(name)
       const {packument, range: avoid} = vuln
 
-      if (this[_specVulnerable](packument, spec, avoid)) {
+      if (this[_specVulnerable](packument, spec, avoid, bundled)) {
         // whether it's the root, or just something we symlinked to a
@@ -259,3 +268,5 @@ // random place on disk, we aren't going to update it by looking
                 continue
-              const specVuln = this[_specVulnerable](packument, spec, avoid)
+              const bd = pmani.bundleDependencies || []
+              const bundled = bd.includes(name)
+              const specVuln = this[_specVulnerable](packument, spec, avoid, bundled)
               if (specVuln)
@@ -350,20 +361,60 @@ metaVuln.push(version)
 
+  // convert a quick-audit into a bulk advisory listing
+  static auditToBulk (report) {
+    if (!report.advisories) {
+      // tack on the report json where the response body would go
+      throw Object.assign(new Error('Invalid advisory report'), {
+        body: JSON.stringify(report),
+      })
+    }
+
+    const bulk = {}
+    const {advisories} = report
+    for (const advisory of Object.values(advisories)) {
+      const {
+        id,
+        url,
+        title,
+        severity,
+        vulnerable_versions,
+        module_name: name,
+      } = advisory
+      bulk[name] = bulk[name] || []
+      bulk[name].push({id, url, title, severity, vulnerable_versions})
+    }
+
+    return bulk
+  }
+
   async [_getReport] () {
+    // if we're not auditing, just return false
+    if (this.options.audit === false || this.tree.inventory.size === 0)
+      return null
+
     process.emit('time', 'auditReport:getReport')
     try {
-      // if we're not auditing, just return false
-      if (this.options.audit === false || this.tree.inventory.size === 0)
-        return null
+      try {
+        // first try the super fast bulk advisory listing
+        const res = await fetch('/-/npm/v1/security/advisories/bulk', {
+          ...this.options,
+          registry: this.options.auditRegistry || this.options.registry,
+          method: 'POST',
+          gzip: true,
+          body: prepareBulkData(this.tree, this.options),
+        })
 
-      // we always hit the quick endpoint, because we calculate remediations
-      // locally anyway, to handle meta-vulnerabilities.
-      const res = await fetch('/-/npm/v1/security/audits/quick', {
-        ...this.options,
-        registry: this.options.auditRegistry || this.options.registry,
-        method: 'POST',
-        gzip: true,
-        body: prepareData(this.tree, this.options),
-      })
+        return await res.json()
+      } catch (_) {
+        // that failed, try the quick audit endpoint
 
-      return await res.json()
+        const res = await fetch('/-/npm/v1/security/audits/quick', {
+          ...this.options,
+          registry: this.options.auditRegistry || this.options.registry,
+          method: 'POST',
+          gzip: true,
+          body: prepareData(this.tree, this.options),
+        })
+
+        return await res.json().then(report => AuditReport.auditToBulk(report))
+      }
     } catch (er) {
@@ -380,2 +431,14 @@ this.log.verbose('audit error', er)
 
+const prepareBulkData = (tree, opts) => {
+  const payload = {}
+  for (const name of tree.inventory.query('name')) {
+    const set = new Set()
+    for (const node of tree.inventory.query('name', name)) {
+      set.add(node.package.version)
+    }
+    payload[name] = [...set]
+  }
+  return payload
+}
+
 const prepareData = (tree, opts) => {
@@ -382,0 +445,0 @@ const { npmVersion: npm_version } = opts

lib/yarn-lock.js

@@ -34,3 +34,11 @@ // parse a yarn lock file
 const {dirname} = require('path')
+const {breadth} = require('treeverse')
 
+// for checking against previous entries
+const match = (p, n) =>
+  p.integrity && n.integrity ? p.integrity === n.integrity
+  : p.resolved && n.resolved ? p.resolved === n.resolved
+  : p.version && n.version ? p.version === n.version
+  : true
+
 const prefix =
@@ -154,3 +162,3 @@ `# THIS IS AN AUTOGENERATED FILE. DO NOT EDIT THIS FILE DIRECTLY.
   toString () {
-    return prefix + [...this.entries.values()]
+    return prefix + [...new Set([...this.entries.values()])]
       .map(e => e.toString())
@@ -162,27 +170,122 @@ .sort((a, b) => a.localeCompare(b)).join('\n\n') + '\n'
     this.entries = new Map()
-    for (const node of tree.inventory.values()) {
-      const specs = [...node.edgesIn]
-        .map(e => `${node.name}@${e.spec}`)
-        .sort((a, b) => a.localeCompare(b))
-      this.current = new YarnLockEntry(specs)
-      if (node.package.dependencies)
-        this.current.dependencies = node.package.dependencies
-      if (node.package.optionalDependencies)
-        this.current.optionalDependencies = node.package.optionalDependencies
-      if (node.package.version)
-        this.current.version = node.package.version
-      if (node.resolved)
-        this.current.resolved = consistentResolve(
-          node.resolved,
-          node.isLink ? dirname(node.path) : node.path,
-          node.root.path,
-          true
-        )
-      if (node.integrity)
-        this.current.integrity = node.integrity
-      specs.forEach(spec => this.entries.set(spec, this.current))
-    }
+    // walk the tree in a deterministic order, breadth-first, alphabetical
+    breadth({
+      tree,
+      visit: node => this.addEntryFromNode(node),
+      getChildren: node => [...node.children.values(), ...node.fsChildren]
+        .sort((a, b) => a.depth - b.depth || a.name.localeCompare(b.name)),
+    })
     return this
   }
 
+  addEntryFromNode (node) {
+    const specs = [...node.edgesIn]
+      .map(e => `${node.name}@${e.spec}`)
+      .sort((a, b) => a.localeCompare(b))
+
+    // Note:
+    // yarn will do excessive duplication in a case like this:
+    // root -> (x@1.x, y@1.x, z@1.x)
+    // y@1.x -> (x@1.1, z@2.x)
+    // z@1.x -> ()
+    // z@2.x -> (x@1.x)
+    //
+    // where x@1.2 exists, because the "x@1.x" spec will *always* resolve
+    // to x@1.2, which doesn't work for y's dep on x@1.1, so you'll get this:
+    //
+    // root
+    // +-- x@1.2.0
+    // +-- y
+    // |   +-- x@1.1.0
+    // |   +-- z@2
+    // |       +-- x@1.2.0
+    // +-- z@1
+    //
+    // instead of this more deduped tree that arborist builds by default:
+    //
+    // root
+    // +-- x@1.2.0 (dep is x@1.x, from root)
+    // +-- y
+    // |   +-- x@1.1.0
+    // |   +-- z@2 (dep on x@1.x deduped to x@1.1.0 under y)
+    // +-- z@1
+    //
+    // In order to not create an invalid yarn.lock file with conflicting
+    // entries, AND not tell yarn to create an invalid tree, we need to
+    // ignore the x@1.x spec coming from z, since it's already in the entries.
+    //
+    // So, if the integrity and resolved don't match a previous entry, skip it.
+    // We call this method on shallower nodes first, so this is fine.
+    const n = this.entryDataFromNode(node)
+    let priorEntry = null
+    const newSpecs = []
+    for (const s of specs) {
+      const prev = this.entries.get(s)
+      // no previous entry for this spec at all, so it's new
+      if (!prev) {
+        // if we saw a match already, then assign this spec to it as well
+        if (priorEntry)
+          priorEntry.addSpec(s)
+        else
+          newSpecs.push(s)
+        continue
+      }
+
+      const m = match(prev, n)
+      // there was a prior entry, but a different thing.  skip this one
+      if (!m)
+        continue
+
+      // previous matches, but first time seeing it, so already has this spec.
+      // go ahead and add all the previously unseen specs, though
+      if (!priorEntry) {
+        priorEntry = prev
+        for (const s of newSpecs) {
+          priorEntry.addSpec(s)
+          this.entries.set(s, priorEntry)
+        }
+        newSpecs.length = 0
+        continue
+      }
+
+      // have a prior entry matching n, and matching the prev we just saw
+      // add the spec to it
+      priorEntry.addSpec(s)
+      this.entries.set(s, priorEntry)
+    }
+
+    // if we never found a matching prior, then this is a whole new thing
+    if (!priorEntry) {
+      const entry = Object.assign(new YarnLockEntry(newSpecs), n)
+      for (const s of newSpecs) {
+        this.entries.set(s, entry)
+      }
+    } else {
+      // pick up any new info that we got for this node, so that we can
+      // decorate with integrity/resolved/etc.
+      Object.assign(priorEntry, n)
+    }
+  }
+
+  entryDataFromNode (node) {
+    const n = {}
+    if (node.package.dependencies)
+      n.dependencies = node.package.dependencies
+    if (node.package.optionalDependencies)
+      n.optionalDependencies = node.package.optionalDependencies
+    if (node.package.version)
+      n.version = node.package.version
+    if (node.resolved)
+      n.resolved = consistentResolve(
+        node.resolved,
+        node.isLink ? dirname(node.path) : node.path,
+        node.root.path,
+        true
+      )
+    if (node.integrity)
+      n.integrity = node.integrity
+
+    return n
+  }
+
   static get Entry () {
@@ -206,3 +309,6 @@ return YarnLockEntry
     // sort objects to the bottom, then alphabetical
-    return ([...this[_specs]].map(JSON.stringify).join(', ') + ':\n' +
+    return ([...this[_specs]]
+        .sort((a, b) => a.localeCompare(b))
+        .map(JSON.stringify).join(', ') +
+      ':\n' +
       Object.getOwnPropertyNames(this)
@@ -226,4 +332,8 @@ .filter(prop => this[prop] !== null)
   }
+
+  addSpec (spec) {
+    this[_specs].add(spec)
+  }
 }
 
 module.exports = YarnLock

package.json

 {
   "name": "@npmcli/arborist",
-  "version": "0.0.0-pre.20",
+  "version": "0.0.0-pre.21",
   "description": "Manage node_modules trees",
@@ -5,0 +5,0 @@ "dependencies": {

No alert changes

Improved metrics

Total package byte prevSize

250104

increased by2.19%

Lines of code

6070

increased by2.71%

Worsened metrics

Number of medium supply chain risk alerts

2

increased by100%

No dependency changes