fix(arborist): allow npm audit fix to downgrade to a patched older version

workspaces/arborist/lib/can-place-dep.js

@@ -62,6 +62,7 @@ class CanPlaceDep {
       parent = null,
       peerPath = [],
       explicitRequest = false,
+      auditReport = null,
     } = options
 
     debug(() => {
@@ -92,6 +93,7 @@ class CanPlaceDep {
     this.target = target
     this.edge = edge
     this.explicitRequest = explicitRequest
+    this.auditReport = auditReport
 
     // preventing cycles when we check peer sets
     this.peerPath = peerPath
@@ -170,7 +172,12 @@ class CanPlaceDep {
 
     const { version: curVer } = current
     const { version: newVer } = dep
-    const tryReplace = curVer && newVer && semver.gte(newVer, curVer)
+    const securityDowngrade = !!(
+      this.auditReport &&
+      this.auditReport.isVulnerable(current) &&
+      !this.auditReport.isVulnerable(dep)
+    )
+    const tryReplace = curVer && newVer && (semver.gte(newVer, curVer) || securityDowngrade)
     if (tryReplace && dep.canReplace(current)) {
       // It's extremely rare that a replaceable node would be a conflict, if
       // the current one wasn't a conflict, but it is theoretically possible
@@ -383,6 +390,7 @@ class CanPlaceDep {
         peerPath,
         // always place peers in preferDedupe mode
         preferDedupe: true,
+        auditReport: this.auditReport,
       })
       /* istanbul ignore next */
       debug(() => {

workspaces/arborist/lib/place-dep.js

@@ -96,6 +96,7 @@ class PlaceDep {
         target,
         preferDedupe: this.preferDedupe,
         explicitRequest: this.explicitRequest,
+        auditReport: this.auditReport,
       })
       this.checks.set(target, cpd)
 

workspaces/arborist/test/arborist/audit.js

@@ -54,6 +54,63 @@ t.test('audit fix reifies out the bad deps', async t => {
   t.matchSnapshot(tree, 'reified out the bad mkdirp and minimist')
 })
 
+t.test('audit fix downgrades to a patched older version when the vulnerable range excludes it', async t => {
+  // Regression test for npm/cli#9557: `npm audit fix` reported a fix was
+  // available but did nothing. This happens when the only non-vulnerable
+  // version that still satisfies the declared semver range is *older* than
+  // what's installed (e.g. a vuln was introduced partway through a range,
+  // and no newer patched version has been published yet).
+  const registry = new MockRegistry({ tap: t, registry: 'https://registry.npmjs.org' })
+
+  const manifest = registry.manifest({ name: 'vuln-pkg', versions: ['1.0.0', '1.0.1', '1.0.2'] })
+  // the packument may be fetched more than once (once for the audit report,
+  // once while building the ideal tree for the fix), allow any number of
+  // fetches instead of guessing an exact count.
+  registry.nock = registry.nock.get(registry.fullPath('/vuln-pkg')).reply(200, manifest).persist()
+
+  const tarballSrc = t.testdir({
+    'package.json': JSON.stringify({ name: 'vuln-pkg', version: '1.0.1' }),
+  })
+  await registry.tarball({ manifest: manifest.versions['1.0.1'], tarball: tarballSrc })
+
+  registry.audit({
+    results: {
+      'vuln-pkg': [{
+        id: 1,
+        url: 'https://example.com/advisories/1',
+        title: 'test vulnerability',
+        severity: 'moderate',
+        vulnerable_versions: '1.0.2',
+      }],
+    },
+  })
+
+  // author the project state directly on disk instead of installing via a
+  // live reify- `reify({ add: [...] })` would overwrite the declared
+  // range with the exact version added, defeating the repro.
+  const path = t.testdir({
+    'package.json': JSON.stringify({
+      name: 'project',
+      version: '1.0.0',
+      dependencies: { 'vuln-pkg': '^1.0.0' },
+    }),
+    'package-lock.json': JSON.stringify({
+      requires: true,
+      lockfileVersion: 1,
+      dependencies: {
+        'vuln-pkg': {
+          version: '1.0.2',
+          resolved: 'https://registry.npmjs.org/vuln-pkg/-/vuln-pkg-1.0.2.tgz',
+          integrity: 'sha512-8VwMnyGCONIs6cWue2IdpHxHnAjzxnw2Zr7MkVxB2vjmQ2ivqGFb4LEG3SMnv0Gb2F/G/2yA8zUaiL1gywDCCg==',
+        },
+      },
+    }),
+  })
+
+  const fixed = await newArb(path).audit({ fix: true })
+  t.equal(fixed.children.get('vuln-pkg').version, '1.0.1', 'downgraded to the patched older version')
+})
+
 t.test('audit does not do globals', async t => {
   await t.rejects(newArb('.', { global: true }).audit(), {
     message: '`npm audit` does not support testing globals',

workspaces/arborist/test/can-place-dep.js

@@ -1409,3 +1409,75 @@ t.test('constructor debug throws', t => {
 
   t.end()
 })
+
+t.test('audit fix replaces a vulnerable current node with an older patched one', t => {
+  const path = '/some/path'
+
+  const makeTree = () => new Node({
+    path,
+    pkg: { name: 'project', version: '1.0.0', dependencies: { a: '^1.0.0' } },
+    children: [{ pkg: { name: 'a', version: '1.0.3' } }],
+  })
+
+  const fakeAuditReport = (vulnerableVersions) => ({
+    isVulnerable (node) {
+      return node.name === 'a' && vulnerableVersions.includes(node.version)
+    },
+  })
+
+  t.test('replaces when current is vulnerable and dep is not', t => {
+    const tree = makeTree()
+    const target = tree
+    const edge = tree.edgesOut.get('a')
+    const dep = new Node({ pkg: { name: 'a', version: '1.0.0' } })
+    const vr = new Node({ sourceReference: tree, path, pkg: { ...tree.package } })
+    dep.parent = vr
+
+    const cpd = new CanPlaceDep({
+      target,
+      edge,
+      dep,
+      auditReport: fakeAuditReport(['1.0.3']),
+    })
+    t.equal(cpd.canPlace, REPLACE, 'downgrades to the patched older version')
+    t.end()
+  })
+
+  t.test('keeps current when neither current nor dep is flagged vulnerable', t => {
+    const tree = makeTree()
+    const target = tree
+    const edge = tree.edgesOut.get('a')
+    const dep = new Node({ pkg: { name: 'a', version: '1.0.0' } })
+    const vr = new Node({ sourceReference: tree, path, pkg: { ...tree.package } })
+    dep.parent = vr
+
+    const cpd = new CanPlaceDep({
+      target,
+      edge,
+      dep,
+      auditReport: fakeAuditReport([]),
+    })
+    t.equal(cpd.canPlace, KEEP, 'no audit reason to prefer the older version')
+    t.end()
+  })
+
+  t.test('does not downgrade to another vulnerable version', t => {
+    const tree = makeTree()
+    const target = tree
+    const edge = tree.edgesOut.get('a')
+    const dep = new Node({ pkg: { name: 'a', version: '1.0.0' } })
+    const vr = new Node({ sourceReference: tree, path, pkg: { ...tree.package } })
+    dep.parent = vr
+
+    const cpd = new CanPlaceDep({
+      target,
+      edge,
+      dep,
+      auditReport: fakeAuditReport(['1.0.3', '1.0.0']),
+    })
+    t.equal(cpd.canPlace, KEEP, 'no fix available, so current node is kept')
+    t.end()
+  })
+
+  t.end()
+})