Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(ci): lock file validation #4363

Merged
merged 1 commit into from
Feb 3, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 23 additions & 0 deletions lib/commands/ci.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ const runScript = require('@npmcli/run-script')
const fs = require('fs')
const readdir = util.promisify(fs.readdir)
const log = require('../utils/log-shim.js')
const validateLockfile = require('../utils/validate-lockfile.js')

const removeNodeModules = async where => {
const rimrafOpts = { glob: false }
Expand Down Expand Up @@ -55,6 +56,28 @@ class CI extends ArboristWorkspaceCmd {
}),
removeNodeModules(where),
])

// retrieves inventory of packages from loaded virtual tree (lock file)
const virtualInventory = new Map(arb.virtualTree.inventory)

// build ideal tree step needs to come right after retrieving the virtual
// inventory since it's going to erase the previous ref to virtualTree
await arb.buildIdealTree()

// verifies that the packages from the ideal tree will match
// the same versions that are present in the virtual tree (lock file)
// throws a validation error in case of mismatches
const errors = validateLockfile(virtualInventory, arb.idealTree.inventory)
wraithgar marked this conversation as resolved.
Show resolved Hide resolved
if (errors.length) {
throw new Error(
'`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` ' +
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💯 having a path to resolution here explicitly laid out

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

agreed! to be fair it's not my doing 😁 I'm just reinstating the message from v6:

https://github.com/npm/libcipm/blob/9ab1a620db485c137b1c89979c80beddf7e2da42/index.js#L172-L178

'before continuing.\n\n' +
errors.join('\n') + '\n'
)
}

await arb.reify(opts)

const ignoreScripts = this.npm.config.get('ignore-scripts')
Expand Down
29 changes: 29 additions & 0 deletions lib/utils/validate-lockfile.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
// compares the inventory of package items in the tree
// that is about to be installed (idealTree) with the inventory
// of items stored in the package-lock file (virtualTree)
//
// Returns empty array if no errors found or an array populated
// with an entry for each validation error found.
function validateLockfile (virtualTree, idealTree) {
const errors = []

// loops through the inventory of packages resulted by ideal tree,
// for each package compares the versions with the version stored in the
// package-lock and adds an error to the list in case of mismatches
for (const [key, entry] of idealTree.entries()) {
const lock = virtualTree.get(key)

if (!lock) {
errors.push(`Missing: ${entry.name}@${entry.version} from lock file`)
continue
}

if (entry.version !== lock.version) {
errors.push(`Invalid: lock file's ${lock.name}@${lock.version} does ` +
`not satisfy ${entry.name}@${entry.version}`)
}
}
return errors
}

module.exports = validateLockfile
29 changes: 29 additions & 0 deletions smoke-tests/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ t.cleanSnapshot = s =>
.replace(/\r\n/g, '\n')
.replace(/ \(in a browser\)/g, '')
.replace(/^npm@.* /gm, 'npm ')
.replace(/^.*debug-[0-9]+.log$/gm, '')

// setup server
const { start, stop, registry } = require('./server.js')
Expand Down Expand Up @@ -320,3 +321,31 @@ t.test('npm update --save', async t => {
'should have expected update --save lockfile result'
)
})

t.test('npm ci', async t => {
await exec(`${npmBin} uninstall abbrev`)
await exec(`${npmBin} install [email protected] --save-exact`)

t.equal(
JSON.parse(readFile('package-lock.json')).packages['node_modules/abbrev'].version,
'1.0.4',
'should have stored exact installed version'
)

await exec(`${npmBin} pkg set "dependencies.abbrev=^1.1.1"`)

try {
const npmOpts = [
`--registry=${registry}`,
`--cache="${cacheLocation}"`,
`--userconfig="${userconfigLocation}"`,
'--no-audit',
'--no-update-notifier',
'--loglevel=error',
].join(' ')
const npmBin = `"${process.execPath}" "${npmLocation}" ${npmOpts}`
await exec(`${npmBin} ci`)
} catch (err) {
t.matchSnapshot(err.stderr, 'should throw mismatch deps in lock file error')
}
})
11 changes: 11 additions & 0 deletions tap-snapshots/smoke-tests/index.js.test.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,17 @@ Configuration fields: npm help 7 config

npm {CWD}

`

exports[`smoke-tests/index.js TAP npm ci > should throw mismatch deps in lock file error 1`] = `
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.
npm ERR!
npm ERR! Invalid: lock file's [email protected] does not satisfy [email protected]
npm ERR!

npm ERR! A complete log of this run can be found in:


`

exports[`smoke-tests/index.js TAP npm diff > should have expected diff output 1`] = `
Expand Down
13 changes: 13 additions & 0 deletions tap-snapshots/test/lib/commands/ci.js.test.cjs
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
/* IMPORTANT
* This snapshot file is auto-generated, but designed for humans.
* It should be checked into source control and tracked carefully.
* Re-generate by setting TAP_SNAPSHOT=1 and running tests.
* Make sure to inspect the output below. Do not ignore changes!
*/
'use strict'
exports[`test/lib/commands/ci.js TAP should throw error when ideal inventory mismatches virtual > must match snapshot 1`] = `
\`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.

Invalid: lock file's [email protected] does not satisfy [email protected]

`
35 changes: 35 additions & 0 deletions tap-snapshots/test/lib/utils/validate-lockfile.js.test.cjs
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
/* IMPORTANT
* This snapshot file is auto-generated, but designed for humans.
* It should be checked into source control and tracked carefully.
* Re-generate by setting TAP_SNAPSHOT=1 and running tests.
* Make sure to inspect the output below. Do not ignore changes!
*/
'use strict'
exports[`test/lib/utils/validate-lockfile.js TAP extra inventory items on idealTree > should have missing entries error 1`] = `
Array [
"Missing: [email protected] from lock file",
]
`

exports[`test/lib/utils/validate-lockfile.js TAP extra inventory items on virtualTree > should have no errors if finding virtualTree extra items 1`] = `
Array []
`

exports[`test/lib/utils/validate-lockfile.js TAP identical inventory for both idealTree and virtualTree > should have no errors on identical inventories 1`] = `
Array []
`

exports[`test/lib/utils/validate-lockfile.js TAP mismatching versions on inventory > should have errors for each mismatching version 1`] = `
Array [
"Invalid: lock file's [email protected] does not satisfy [email protected]",
"Invalid: lock file's [email protected] does not satisfy [email protected]",
]
`

exports[`test/lib/utils/validate-lockfile.js TAP missing virtualTree inventory > should have errors for each mismatching version 1`] = `
Array [
"Missing: [email protected] from lock file",
"Missing: [email protected] from lock file",
"Missing: [email protected] from lock file",
]
`
82 changes: 82 additions & 0 deletions test/lib/commands/ci.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,17 @@ t.test('should ignore scripts with --ignore-scripts', async t => {
this.reify = () => {
REIFY_CALLED = true
}
this.buildIdealTree = () => {}
this.virtualTree = {
inventory: new Map([
['foo', { name: 'foo', version: '1.0.0' }],
]),
}
this.idealTree = {
inventory: new Map([
['foo', { name: 'foo', version: '1.0.0' }],
]),
}
},
})

Expand Down Expand Up @@ -99,6 +110,17 @@ t.test('should use Arborist and run-script', async t => {
this.reify = () => {
t.ok(true, 'reify is called')
}
this.buildIdealTree = () => {}
this.virtualTree = {
inventory: new Map([
['foo', { name: 'foo', version: '1.0.0' }],
]),
}
this.idealTree = {
inventory: new Map([
['foo', { name: 'foo', version: '1.0.0' }],
]),
}
},
rimraf: (path, ...args) => {
actualRimrafs++
Expand Down Expand Up @@ -138,6 +160,17 @@ t.test('should pass flatOptions to Arborist.reify', async t => {
this.reify = async (options) => {
t.equal(options.production, true, 'should pass flatOptions to Arborist.reify')
}
this.buildIdealTree = () => {}
this.virtualTree = {
inventory: new Map([
['foo', { name: 'foo', version: '1.0.0' }],
]),
}
this.idealTree = {
inventory: new Map([
['foo', { name: 'foo', version: '1.0.0' }],
]),
}
},
})
const npm = mockNpm({
Expand Down Expand Up @@ -218,6 +251,17 @@ t.test('should remove existing node_modules before installing', async t => {
const nodeModules = contents.filter((path) => path.startsWith('node_modules'))
t.same(nodeModules, ['node_modules'], 'should only have the node_modules directory')
}
this.buildIdealTree = () => {}
this.virtualTree = {
inventory: new Map([
['foo', { name: 'foo', version: '1.0.0' }],
]),
}
this.idealTree = {
inventory: new Map([
['foo', { name: 'foo', version: '1.0.0' }],
]),
}
},
})

Expand All @@ -231,3 +275,41 @@ t.test('should remove existing node_modules before installing', async t => {

await ci.exec(null)
})

t.test('should throw error when ideal inventory mismatches virtual', async t => {
const CI = t.mock('../../../lib/commands/ci.js', {
'../../../lib/utils/reify-finish.js': async () => {},
'@npmcli/run-script': ({ event }) => {},
'@npmcli/arborist': function () {
this.loadVirtual = async () => {}
this.reify = () => {}
this.buildIdealTree = () => {}
this.virtualTree = {
inventory: new Map([
['foo', { name: 'foo', version: '1.0.0' }],
]),
}
this.idealTree = {
inventory: new Map([
['foo', { name: 'foo', version: '2.0.0' }],
]),
}
},
})

const npm = mockNpm({
globalDir: 'path/to/node_modules/',
prefix: 'foo',
config: {
global: false,
'ignore-scripts': true,
},
})
const ci = new CI(npm)

try {
await ci.exec([])
} catch (err) {
t.matchSnapshot(err.message)
}
})
82 changes: 82 additions & 0 deletions test/lib/utils/validate-lockfile.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,82 @@
const t = require('tap')
const validateLockfile = require('../../../lib/utils/validate-lockfile.js')

t.test('identical inventory for both idealTree and virtualTree', async t => {
t.matchSnapshot(
validateLockfile(
new Map([
['foo', { name: 'foo', version: '1.0.0' }],
['bar', { name: 'bar', version: '2.0.0' }],
]),
new Map([
['foo', { name: 'foo', version: '1.0.0' }],
['bar', { name: 'bar', version: '2.0.0' }],
])
),
'should have no errors on identical inventories'
)
})

t.test('extra inventory items on idealTree', async t => {
t.matchSnapshot(
validateLockfile(
new Map([
['foo', { name: 'foo', version: '1.0.0' }],
['bar', { name: 'bar', version: '2.0.0' }],
]),
new Map([
['foo', { name: 'foo', version: '1.0.0' }],
['bar', { name: 'bar', version: '2.0.0' }],
['baz', { name: 'baz', version: '3.0.0' }],
])
),
'should have missing entries error'
)
})

t.test('extra inventory items on virtualTree', async t => {
t.matchSnapshot(
validateLockfile(
new Map([
['foo', { name: 'foo', version: '1.0.0' }],
['bar', { name: 'bar', version: '2.0.0' }],
['baz', { name: 'baz', version: '3.0.0' }],
]),
new Map([
['foo', { name: 'foo', version: '1.0.0' }],
['bar', { name: 'bar', version: '2.0.0' }],
])
),
'should have no errors if finding virtualTree extra items'
)
})

t.test('mismatching versions on inventory', async t => {
t.matchSnapshot(
validateLockfile(
new Map([
['foo', { name: 'foo', version: '1.0.0' }],
['bar', { name: 'bar', version: '2.0.0' }],
]),
new Map([
['foo', { name: 'foo', version: '2.0.0' }],
['bar', { name: 'bar', version: '3.0.0' }],
])
),
'should have errors for each mismatching version'
)
})

t.test('missing virtualTree inventory', async t => {
t.matchSnapshot(
validateLockfile(
new Map([]),
new Map([
['foo', { name: 'foo', version: '1.0.0' }],
['bar', { name: 'bar', version: '2.0.0' }],
['baz', { name: 'baz', version: '3.0.0' }],
])
),
'should have errors for each mismatching version'
)
})