4b0146
From 730dd78c897a28c3df0468ed1fc42d5817badefe Mon Sep 17 00:00:00 2001
4b0146
From: Ruy Adorno <ruyadorno@hotmail.com>
4b0146
Date: Wed, 2 Feb 2022 22:10:22 -0500
4b0146
Subject: [PATCH] fix(ci): lock file validation
4b0146
MIME-Version: 1.0
4b0146
Content-Type: text/plain; charset=UTF-8
4b0146
Content-Transfer-Encoding: 8bit
4b0146
4b0146
Make sure to validate any lock file (either package-lock.json or
4b0146
npm-shrinkwrap.json) against the current install. This will properly
4b0146
throw an error in case any of the dependencies being installed don't
4b0146
match the dependencies that are currently listed in the lock file.
4b0146
4b0146
Fixes: https://github.com/npm/cli/issues/2701
4b0146
Fixes: https://github.com/npm/cli/issues/3947
4b0146
Signed-off-by: Jan Staněk <jstanek@redhat.com>
4b0146
---
4b0146
 deps/npm/lib/commands/ci.js                   | 23 ++++++
4b0146
 deps/npm/lib/utils/validate-lockfile.js       | 29 +++++++
4b0146
 .../smoke-tests/index.js.test.cjs             | 11 +++
4b0146
 .../test/lib/commands/ci.js.test.cjs          | 13 +++
4b0146
 .../lib/utils/validate-lockfile.js.test.cjs   | 35 ++++++++
4b0146
 deps/npm/test/lib/commands/ci.js              | 82 +++++++++++++++++++
4b0146
 deps/npm/test/lib/utils/validate-lockfile.js  | 82 +++++++++++++++++++
4b0146
 7 files changed, 275 insertions(+)
4b0146
 create mode 100644 deps/npm/lib/utils/validate-lockfile.js
4b0146
 create mode 100644 deps/npm/tap-snapshots/test/lib/commands/ci.js.test.cjs
4b0146
 create mode 100644 deps/npm/tap-snapshots/test/lib/utils/validate-lockfile.js.test.cjs
4b0146
 create mode 100644 deps/npm/test/lib/utils/validate-lockfile.js
4b0146
4b0146
diff --git a/deps/npm/lib/commands/ci.js b/deps/npm/lib/commands/ci.js
4b0146
index 2c2f8da..376a85d 100644
4b0146
--- a/deps/npm/lib/commands/ci.js
4b0146
+++ b/deps/npm/lib/commands/ci.js
4b0146
@@ -6,6 +6,7 @@ const runScript = require('@npmcli/run-script')
4b0146
 const fs = require('fs')
4b0146
 const readdir = util.promisify(fs.readdir)
4b0146
 const log = require('../utils/log-shim.js')
4b0146
+const validateLockfile = require('../utils/validate-lockfile.js')
4b0146
 
4b0146
 const removeNodeModules = async where => {
4b0146
   const rimrafOpts = { glob: false }
4b0146
@@ -55,6 +56,28 @@ class CI extends ArboristWorkspaceCmd {
4b0146
       }),
4b0146
       removeNodeModules(where),
4b0146
     ])
4b0146
+
4b0146
+    // retrieves inventory of packages from loaded virtual tree (lock file)
4b0146
+    const virtualInventory = new Map(arb.virtualTree.inventory)
4b0146
+
4b0146
+    // build ideal tree step needs to come right after retrieving the virtual
4b0146
+    // inventory since it's going to erase the previous ref to virtualTree
4b0146
+    await arb.buildIdealTree()
4b0146
+
4b0146
+    // verifies that the packages from the ideal tree will match
4b0146
+    // the same versions that are present in the virtual tree (lock file)
4b0146
+    // throws a validation error in case of mismatches
4b0146
+    const errors = validateLockfile(virtualInventory, arb.idealTree.inventory)
4b0146
+    if (errors.length) {
4b0146
+      throw new Error(
4b0146
+        '`npm ci` can only install packages when your package.json and ' +
4b0146
+        'package-lock.json or npm-shrinkwrap.json are in sync. Please ' +
4b0146
+        'update your lock file with `npm install` ' +
4b0146
+        'before continuing.\n\n' +
4b0146
+        errors.join('\n') + '\n'
4b0146
+      )
4b0146
+    }
4b0146
+
4b0146
     await arb.reify(opts)
4b0146
 
4b0146
     const ignoreScripts = this.npm.config.get('ignore-scripts')
4b0146
diff --git a/deps/npm/lib/utils/validate-lockfile.js b/deps/npm/lib/utils/validate-lockfile.js
4b0146
new file mode 100644
4b0146
index 0000000..29161ec
4b0146
--- /dev/null
4b0146
+++ b/deps/npm/lib/utils/validate-lockfile.js
4b0146
@@ -0,0 +1,29 @@
4b0146
+// compares the inventory of package items in the tree
4b0146
+// that is about to be installed (idealTree) with the inventory
4b0146
+// of items stored in the package-lock file (virtualTree)
4b0146
+//
4b0146
+// Returns empty array if no errors found or an array populated
4b0146
+// with an entry for each validation error found.
4b0146
+function validateLockfile (virtualTree, idealTree) {
4b0146
+  const errors = []
4b0146
+
4b0146
+  // loops through the inventory of packages resulted by ideal tree,
4b0146
+  // for each package compares the versions with the version stored in the
4b0146
+  // package-lock and adds an error to the list in case of mismatches
4b0146
+  for (const [key, entry] of idealTree.entries()) {
4b0146
+    const lock = virtualTree.get(key)
4b0146
+
4b0146
+    if (!lock) {
4b0146
+      errors.push(`Missing: ${entry.name}@${entry.version} from lock file`)
4b0146
+      continue
4b0146
+    }
4b0146
+
4b0146
+    if (entry.version !== lock.version) {
4b0146
+      errors.push(`Invalid: lock file's ${lock.name}@${lock.version} does ` +
4b0146
+      `not satisfy ${entry.name}@${entry.version}`)
4b0146
+    }
4b0146
+  }
4b0146
+  return errors
4b0146
+}
4b0146
+
4b0146
+module.exports = validateLockfile
4b0146
diff --git a/deps/npm/tap-snapshots/smoke-tests/index.js.test.cjs b/deps/npm/tap-snapshots/smoke-tests/index.js.test.cjs
4b0146
index c1316e0..5fa3977 100644
4b0146
--- a/deps/npm/tap-snapshots/smoke-tests/index.js.test.cjs
4b0146
+++ b/deps/npm/tap-snapshots/smoke-tests/index.js.test.cjs
4b0146
@@ -40,6 +40,17 @@ Configuration fields: npm help 7 config
4b0146
 
4b0146
 npm {CWD}
4b0146
 
4b0146
+`
4b0146
+
4b0146
+exports[`smoke-tests/index.js TAP npm ci > should throw mismatch deps in lock file error 1`] = `
4b0146
+npm ERR! \`npm ci\` can only install packages when your package.json and package-lock.json or npm-shrinkwrap.json are in sync. Please update your lock file with \`npm install\` before continuing.
4b0146
+npm ERR! 
4b0146
+npm ERR! Invalid: lock file's abbrev@1.0.4 does not satisfy abbrev@1.1.1
4b0146
+npm ERR! 
4b0146
+
4b0146
+npm ERR! A complete log of this run can be found in:
4b0146
+
4b0146
+
4b0146
 `
4b0146
 
4b0146
 exports[`smoke-tests/index.js TAP npm diff > should have expected diff output 1`] = `
4b0146
diff --git a/deps/npm/tap-snapshots/test/lib/commands/ci.js.test.cjs b/deps/npm/tap-snapshots/test/lib/commands/ci.js.test.cjs
4b0146
new file mode 100644
4b0146
index 0000000..d6a7471
4b0146
--- /dev/null
4b0146
+++ b/deps/npm/tap-snapshots/test/lib/commands/ci.js.test.cjs
4b0146
@@ -0,0 +1,13 @@
4b0146
+/* IMPORTANT
4b0146
+ * This snapshot file is auto-generated, but designed for humans.
4b0146
+ * It should be checked into source control and tracked carefully.
4b0146
+ * Re-generate by setting TAP_SNAPSHOT=1 and running tests.
4b0146
+ * Make sure to inspect the output below.  Do not ignore changes!
4b0146
+ */
4b0146
+'use strict'
4b0146
+exports[`test/lib/commands/ci.js TAP should throw error when ideal inventory mismatches virtual > must match snapshot 1`] = `
4b0146
+\`npm ci\` can only install packages when your package.json and package-lock.json or npm-shrinkwrap.json are in sync. Please update your lock file with \`npm install\` before continuing.
4b0146
+
4b0146
+Invalid: lock file's foo@1.0.0 does not satisfy foo@2.0.0
4b0146
+
4b0146
+`
4b0146
diff --git a/deps/npm/tap-snapshots/test/lib/utils/validate-lockfile.js.test.cjs b/deps/npm/tap-snapshots/test/lib/utils/validate-lockfile.js.test.cjs
4b0146
new file mode 100644
4b0146
index 0000000..98a5126
4b0146
--- /dev/null
4b0146
+++ b/deps/npm/tap-snapshots/test/lib/utils/validate-lockfile.js.test.cjs
4b0146
@@ -0,0 +1,35 @@
4b0146
+/* IMPORTANT
4b0146
+ * This snapshot file is auto-generated, but designed for humans.
4b0146
+ * It should be checked into source control and tracked carefully.
4b0146
+ * Re-generate by setting TAP_SNAPSHOT=1 and running tests.
4b0146
+ * Make sure to inspect the output below.  Do not ignore changes!
4b0146
+ */
4b0146
+'use strict'
4b0146
+exports[`test/lib/utils/validate-lockfile.js TAP extra inventory items on idealTree > should have missing entries error 1`] = `
4b0146
+Array [
4b0146
+  "Missing: baz@3.0.0 from lock file",
4b0146
+]
4b0146
+`
4b0146
+
4b0146
+exports[`test/lib/utils/validate-lockfile.js TAP extra inventory items on virtualTree > should have no errors if finding virtualTree extra items 1`] = `
4b0146
+Array []
4b0146
+`
4b0146
+
4b0146
+exports[`test/lib/utils/validate-lockfile.js TAP identical inventory for both idealTree and virtualTree > should have no errors on identical inventories 1`] = `
4b0146
+Array []
4b0146
+`
4b0146
+
4b0146
+exports[`test/lib/utils/validate-lockfile.js TAP mismatching versions on inventory > should have errors for each mismatching version 1`] = `
4b0146
+Array [
4b0146
+  "Invalid: lock file's foo@1.0.0 does not satisfy foo@2.0.0",
4b0146
+  "Invalid: lock file's bar@2.0.0 does not satisfy bar@3.0.0",
4b0146
+]
4b0146
+`
4b0146
+
4b0146
+exports[`test/lib/utils/validate-lockfile.js TAP missing virtualTree inventory > should have errors for each mismatching version 1`] = `
4b0146
+Array [
4b0146
+  "Missing: foo@1.0.0 from lock file",
4b0146
+  "Missing: bar@2.0.0 from lock file",
4b0146
+  "Missing: baz@3.0.0 from lock file",
4b0146
+]
4b0146
+`
4b0146
diff --git a/deps/npm/test/lib/commands/ci.js b/deps/npm/test/lib/commands/ci.js
4b0146
index 537d078..e077c99 100644
4b0146
--- a/deps/npm/test/lib/commands/ci.js
4b0146
+++ b/deps/npm/test/lib/commands/ci.js
4b0146
@@ -19,6 +19,17 @@ t.test('should ignore scripts with --ignore-scripts', async t => {
4b0146
       this.reify = () => {
4b0146
         REIFY_CALLED = true
4b0146
       }
4b0146
+      this.buildIdealTree = () => {}
4b0146
+      this.virtualTree = {
4b0146
+        inventory: new Map([
4b0146
+          ['foo', { name: 'foo', version: '1.0.0' }],
4b0146
+        ]),
4b0146
+      }
4b0146
+      this.idealTree = {
4b0146
+        inventory: new Map([
4b0146
+          ['foo', { name: 'foo', version: '1.0.0' }],
4b0146
+        ]),
4b0146
+      }
4b0146
     },
4b0146
   })
4b0146
 
4b0146
@@ -99,6 +110,17 @@ t.test('should use Arborist and run-script', async t => {
4b0146
       this.reify = () => {
4b0146
         t.ok(true, 'reify is called')
4b0146
       }
4b0146
+      this.buildIdealTree = () => {}
4b0146
+      this.virtualTree = {
4b0146
+        inventory: new Map([
4b0146
+          ['foo', { name: 'foo', version: '1.0.0' }],
4b0146
+        ]),
4b0146
+      }
4b0146
+      this.idealTree = {
4b0146
+        inventory: new Map([
4b0146
+          ['foo', { name: 'foo', version: '1.0.0' }],
4b0146
+        ]),
4b0146
+      }
4b0146
     },
4b0146
     rimraf: (path, ...args) => {
4b0146
       actualRimrafs++
4b0146
@@ -138,6 +160,17 @@ t.test('should pass flatOptions to Arborist.reify', async t => {
4b0146
       this.reify = async (options) => {
4b0146
         t.equal(options.production, true, 'should pass flatOptions to Arborist.reify')
4b0146
       }
4b0146
+      this.buildIdealTree = () => {}
4b0146
+      this.virtualTree = {
4b0146
+        inventory: new Map([
4b0146
+          ['foo', { name: 'foo', version: '1.0.0' }],
4b0146
+        ]),
4b0146
+      }
4b0146
+      this.idealTree = {
4b0146
+        inventory: new Map([
4b0146
+          ['foo', { name: 'foo', version: '1.0.0' }],
4b0146
+        ]),
4b0146
+      }
4b0146
     },
4b0146
   })
4b0146
   const npm = mockNpm({
4b0146
@@ -218,6 +251,17 @@ t.test('should remove existing node_modules before installing', async t => {
4b0146
         const nodeModules = contents.filter((path) => path.startsWith('node_modules'))
4b0146
         t.same(nodeModules, ['node_modules'], 'should only have the node_modules directory')
4b0146
       }
4b0146
+      this.buildIdealTree = () => {}
4b0146
+      this.virtualTree = {
4b0146
+        inventory: new Map([
4b0146
+          ['foo', { name: 'foo', version: '1.0.0' }],
4b0146
+        ]),
4b0146
+      }
4b0146
+      this.idealTree = {
4b0146
+        inventory: new Map([
4b0146
+          ['foo', { name: 'foo', version: '1.0.0' }],
4b0146
+        ]),
4b0146
+      }
4b0146
     },
4b0146
   })
4b0146
 
4b0146
@@ -231,3 +275,41 @@ t.test('should remove existing node_modules before installing', async t => {
4b0146
 
4b0146
   await ci.exec(null)
4b0146
 })
4b0146
+
4b0146
+t.test('should throw error when ideal inventory mismatches virtual', async t => {
4b0146
+  const CI = t.mock('../../../lib/commands/ci.js', {
4b0146
+    '../../../lib/utils/reify-finish.js': async () => {},
4b0146
+    '@npmcli/run-script': ({ event }) => {},
4b0146
+    '@npmcli/arborist': function () {
4b0146
+      this.loadVirtual = async () => {}
4b0146
+      this.reify = () => {}
4b0146
+      this.buildIdealTree = () => {}
4b0146
+      this.virtualTree = {
4b0146
+        inventory: new Map([
4b0146
+          ['foo', { name: 'foo', version: '1.0.0' }],
4b0146
+        ]),
4b0146
+      }
4b0146
+      this.idealTree = {
4b0146
+        inventory: new Map([
4b0146
+          ['foo', { name: 'foo', version: '2.0.0' }],
4b0146
+        ]),
4b0146
+      }
4b0146
+    },
4b0146
+  })
4b0146
+
4b0146
+  const npm = mockNpm({
4b0146
+    globalDir: 'path/to/node_modules/',
4b0146
+    prefix: 'foo',
4b0146
+    config: {
4b0146
+      global: false,
4b0146
+      'ignore-scripts': true,
4b0146
+    },
4b0146
+  })
4b0146
+  const ci = new CI(npm)
4b0146
+
4b0146
+  try {
4b0146
+    await ci.exec([])
4b0146
+  } catch (err) {
4b0146
+    t.matchSnapshot(err.message)
4b0146
+  }
4b0146
+})
4b0146
diff --git a/deps/npm/test/lib/utils/validate-lockfile.js b/deps/npm/test/lib/utils/validate-lockfile.js
4b0146
new file mode 100644
4b0146
index 0000000..25939c5
4b0146
--- /dev/null
4b0146
+++ b/deps/npm/test/lib/utils/validate-lockfile.js
4b0146
@@ -0,0 +1,82 @@
4b0146
+const t = require('tap')
4b0146
+const validateLockfile = require('../../../lib/utils/validate-lockfile.js')
4b0146
+
4b0146
+t.test('identical inventory for both idealTree and virtualTree', async t => {
4b0146
+  t.matchSnapshot(
4b0146
+    validateLockfile(
4b0146
+      new Map([
4b0146
+        ['foo', { name: 'foo', version: '1.0.0' }],
4b0146
+        ['bar', { name: 'bar', version: '2.0.0' }],
4b0146
+      ]),
4b0146
+      new Map([
4b0146
+        ['foo', { name: 'foo', version: '1.0.0' }],
4b0146
+        ['bar', { name: 'bar', version: '2.0.0' }],
4b0146
+      ])
4b0146
+    ),
4b0146
+    'should have no errors on identical inventories'
4b0146
+  )
4b0146
+})
4b0146
+
4b0146
+t.test('extra inventory items on idealTree', async t => {
4b0146
+  t.matchSnapshot(
4b0146
+    validateLockfile(
4b0146
+      new Map([
4b0146
+        ['foo', { name: 'foo', version: '1.0.0' }],
4b0146
+        ['bar', { name: 'bar', version: '2.0.0' }],
4b0146
+      ]),
4b0146
+      new Map([
4b0146
+        ['foo', { name: 'foo', version: '1.0.0' }],
4b0146
+        ['bar', { name: 'bar', version: '2.0.0' }],
4b0146
+        ['baz', { name: 'baz', version: '3.0.0' }],
4b0146
+      ])
4b0146
+    ),
4b0146
+    'should have missing entries error'
4b0146
+  )
4b0146
+})
4b0146
+
4b0146
+t.test('extra inventory items on virtualTree', async t => {
4b0146
+  t.matchSnapshot(
4b0146
+    validateLockfile(
4b0146
+      new Map([
4b0146
+        ['foo', { name: 'foo', version: '1.0.0' }],
4b0146
+        ['bar', { name: 'bar', version: '2.0.0' }],
4b0146
+        ['baz', { name: 'baz', version: '3.0.0' }],
4b0146
+      ]),
4b0146
+      new Map([
4b0146
+        ['foo', { name: 'foo', version: '1.0.0' }],
4b0146
+        ['bar', { name: 'bar', version: '2.0.0' }],
4b0146
+      ])
4b0146
+    ),
4b0146
+    'should have no errors if finding virtualTree extra items'
4b0146
+  )
4b0146
+})
4b0146
+
4b0146
+t.test('mismatching versions on inventory', async t => {
4b0146
+  t.matchSnapshot(
4b0146
+    validateLockfile(
4b0146
+      new Map([
4b0146
+        ['foo', { name: 'foo', version: '1.0.0' }],
4b0146
+        ['bar', { name: 'bar', version: '2.0.0' }],
4b0146
+      ]),
4b0146
+      new Map([
4b0146
+        ['foo', { name: 'foo', version: '2.0.0' }],
4b0146
+        ['bar', { name: 'bar', version: '3.0.0' }],
4b0146
+      ])
4b0146
+    ),
4b0146
+    'should have errors for each mismatching version'
4b0146
+  )
4b0146
+})
4b0146
+
4b0146
+t.test('missing virtualTree inventory', async t => {
4b0146
+  t.matchSnapshot(
4b0146
+    validateLockfile(
4b0146
+      new Map([]),
4b0146
+      new Map([
4b0146
+        ['foo', { name: 'foo', version: '1.0.0' }],
4b0146
+        ['bar', { name: 'bar', version: '2.0.0' }],
4b0146
+        ['baz', { name: 'baz', version: '3.0.0' }],
4b0146
+      ])
4b0146
+    ),
4b0146
+    'should have errors for each mismatching version'
4b0146
+  )
4b0146
+})
4b0146
-- 
4b0146
2.35.1
4b0146