fix(arborist): skip re-resolution when audit fix requires semver-major bump

test/lib/commands/audit.js

@@ -163,6 +163,87 @@ t.test('audit fix - bulk endpoint', async t => {
   )
 })
 
+t.test('audit fix - no spurious update when fix requires semver-major bump', async t => {
+  const semverMajorTree = {
+    'package.json': JSON.stringify({
+      name: 'test-dep',
+      version: '1.0.0',
+      dependencies: {
+        'test-dep-a': '^2.0.0',
+      },
+    }),
+    'package-lock.json': JSON.stringify({
+      name: 'test-dep',
+      version: '1.0.0',
+      lockfileVersion: 2,
+      requires: true,
+      packages: {
+        '': {
+          name: 'test-dep',
+          version: '1.0.0',
+          dependencies: {
+            'test-dep-a': '^2.0.0',
+          },
+          devDependencies: {},
+        },
+        'node_modules/test-dep-a': {
+          name: 'test-dep-a',
+          version: '2.0.0',
+        },
+      },
+      dependencies: {
+        'test-dep-a': {
+          version: '2.0.0',
+        },
+      },
+    }),
+    node_modules: {
+      'test-dep-a': {
+        'package.json': JSON.stringify({
+          name: 'test-dep-a',
+          version: '2.0.0',
+        }),
+      },
+    },
+  }
+
+  const { npm } = await loadMockNpm(t, {
+    prefixDir: semverMajorTree,
+  })
+  const registry = new MockRegistry({
+    tap: t,
+    registry: npm.config.get('registry'),
+  })
+  const manifest = registry.manifest({
+    name: 'test-dep-a',
+    packuments: [{ version: '2.0.0' }, { version: '2.0.1' }, { version: '3.0.0' }],
+  })
+  await registry.package({ manifest, times: 2 })
+  // All 2.x versions are vulnerable; fix requires major bump to 3.0.0
+  const advisory = registry.advisory({ id: 101, vulnerable_versions: '<3.0.0' })
+  registry.nock
+    .post('/-/npm/v1/security/advisories/bulk', body => {
+      const unzipped = JSON.parse(gunzip(Buffer.from(body, 'hex')))
+      return t.same(unzipped, { 'test-dep-a': ['2.0.0'] })
+    })
+    .reply(200, {
+      'test-dep-a': [advisory],
+    })
+    .post('/-/npm/v1/security/advisories/bulk', () => true)
+    .reply(200, {
+      'test-dep-a': [advisory],
+    })
+  await npm.exec('audit', ['fix'])
+  const lockfile = JSON.parse(
+    fs.readFileSync(path.join(npm.prefix, 'package-lock.json'), 'utf8')
+  )
+  t.equal(
+    lockfile.packages['node_modules/test-dep-a'].version,
+    '2.0.0',
+    'lockfile unchanged - no spurious minor update when fix requires semver-major bump'
+  )
+})
+
 t.test('audit fix no package lock', async t => {
   const { npm } = await loadMockNpm(t, {
     config: {

workspaces/arborist/lib/arborist/build-ideal-tree.js

@@ -1254,9 +1254,12 @@ This is a one-time fix-up, please be patient...
         continue
       }
 
-      // fixing a security vulnerability with this package, problem
+      // fixing a security vulnerability with this package; skip re-resolution when no fix is available within range and --force is not set
       if (this.auditReport && this.auditReport.isVulnerable(edge.to)) {
-        problems.push(edge)
+        const vuln = this.auditReport.get(edge.to.packageName)
+        if (vuln.fixAvailable === true || this.options.force) {
+          problems.push(edge)
+        }
         continue
       }
 

workspaces/arborist/test/arborist/audit.js

@@ -54,6 +54,51 @@ 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 does not update when fix requires semver-major bump', async t => {
+  const registry = createRegistry(t)
+  const advisory = registry.advisory({ id: 101, vulnerable_versions: '<3.0.0' })
+  const manifest = registry.manifest({
+    name: 'test-pkg',
+    packuments: [{ version: '2.0.0' }, { version: '2.0.1' }, { version: '3.0.0' }],
+  })
+  await registry.package({ manifest, times: 2 })
+  registry.audit({ times: 2, results: { 'test-pkg': [advisory] } })
+
+  const path = t.testdir({
+    'package.json': JSON.stringify({
+      name: 'my-project',
+      version: '1.0.0',
+      dependencies: { 'test-pkg': '^2.0.0' },
+    }),
+    'package-lock.json': JSON.stringify({
+      name: 'my-project',
+      version: '1.0.0',
+      lockfileVersion: 2,
+      requires: true,
+      packages: {
+        '': { name: 'my-project', version: '1.0.0', dependencies: { 'test-pkg': '^2.0.0' } },
+        'node_modules/test-pkg': {
+          version: '2.0.0',
+          resolved: 'https://registry.npmjs.org/test-pkg/-/test-pkg-2.0.0.tgz',
+          integrity: 'sha512-AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA==',
+        },
+      },
+    }),
+    node_modules: {
+      'test-pkg': {
+        'package.json': JSON.stringify({ name: 'test-pkg', version: '2.0.0' }),
+      },
+    },
+  })
+
+  const tree = await newArb(path).audit({ fix: true })
+  t.equal(
+    tree.children.get('test-pkg').version,
+    '2.0.0',
+    'test-pkg not updated when fix requires semver-major bump'
+  )
+})
+
 t.test('audit does not do globals', async t => {
   await t.rejects(newArb('.', { global: true }).audit(), {
     message: '`npm audit` does not support testing globals',