From 0dc996409a93f323481deb66acf300c8d401e779 Mon Sep 17 00:00:00 2001 From: Philip Harrison Date: Tue, 24 May 2022 09:38:44 +0100 Subject: [PATCH] Update console output and add some tests --- lib/commands/audit.js | 60 +++++---- .../test/lib/commands/audit.js.test.cjs | 43 +++--- test/lib/commands/audit.js | 124 +++++++++++++++++- 3 files changed, 183 insertions(+), 44 deletions(-) diff --git a/lib/commands/audit.js b/lib/commands/audit.js index 7a1be0d4a1be5..520aeb8a8445b 100644 --- a/lib/commands/audit.js +++ b/lib/commands/audit.js @@ -79,45 +79,45 @@ class VerifySignatures { this.appendOutput(`${verifiedPrefix}${timing}\n`) if (this.verified && !verified) { - this.appendOutput( - `${this.verified} packages have ${this.npm.color ? chalk.bold('verified') : 'verified'}` + - ` registry signatures\n` - ) + const verifiedClr = this.npm.color ? chalk.bold('verified') : 'verified' + const msg = this.verified === 1 ? + `${this.verified} package has a ${verifiedClr} registry signature\n` : + `${this.verified} packages have ${verifiedClr} registry signatures\n` + this.appendOutput(msg) } if (missing.length) { + const logMissing = this.npm.config.get('log-missing-names') + const missingClr = this.npm.color ? chalk.bold(chalk.magenta('missing')) : 'missing' const msg = missing.length === 1 ? - `package has a ${this.npm.color ? chalk.bold(chalk.magenta('missing')) : 'missing'}` + - ` registry signature` : - `packages have ${this.npm.color ? chalk.bold(chalk.magenta('missing')) : 'missing'}` + - ` registry signatures` + `package has a ${missingClr} registry signature` : + `packages have ${missingClr} registry signatures` this.appendOutput( `${missing.length} ${msg} but the registry is ` + - `providing signing keys${this.npm.config.get('missing') ? ':\n' : ''}` + `providing signing keys${logMissing ? ':\n' : ''}` ) - // TODO: This might not be the right option for this - if (this.npm.config.get('missing')) { + if (logMissing) { this.appendOutput(this.humanOutput(missing)) } else { - this.appendOutput(` run \`npm audit signatures --missing\` for details`) + this.appendOutput(` run \`npm audit signatures --log-missing-names\` for details`) } } if (invalid.length) { + const invalidClr = this.npm.color ? chalk.bold(chalk.red('invalid')) : 'invalid' const msg = invalid.length === 1 ? - `package has an ${this.npm.color ? chalk.bold(chalk.red('invalid')) : 'invalid'}` + - ` registry signature` : - `packages have ${this.npm.color ? chalk.bold(chalk.red('invalid')) : 'invalid'}` + - ` registry signatures` + `${invalid.length} package has an ${invalidClr} registry signature:\n` : + `${invalid.length} packages have ${invalidClr} registry signatures:\n` this.appendOutput( - `${missing.length ? '\n' : ''}${invalid.length} ${msg}:\n` + `${missing.length ? '\n' : ''}${msg}` ) this.appendOutput(this.humanOutput(invalid)) - const invPlural = invalid.length === 1 ? '' : 's' - this.appendOutput( - `\nSomeone might have tampered with the package${invPlural} ` + - `since it was published on the registry (monster-in-the-middle attack)!\n` - ) + const tamperMsg = invalid.length === 1 ? + `\nSomeone might have tampered with this package since it was ` + + `published on the registry!\n` : + `\nSomeone might have tampered with these packages since they where ` + + `published on the registry!\n` + this.appendOutput(tamperMsg) } } } @@ -204,7 +204,7 @@ class VerifySignatures { const parsedRegistry = new URL(registry) const regKey = `//${parsedRegistry.host}${parsedRegistry.pathname}` return { - `${regKey}:_keys`: keys + [`${regKey}:_keys`]: keys, } } @@ -369,7 +369,9 @@ class Audit extends ArboristWorkspaceCmd { case 'fix': return [] default: - throw new Error(argv[2] + ' not recognized') + throw Object.assign(new Error(argv[2] + ' not recognized'), { + code: 'EUSAGE', + }) } } @@ -406,7 +408,15 @@ class Audit extends ArboristWorkspaceCmd { } async auditSignatures () { - log.newItem('loading intalled packages') + if (this.npm.config.get('global')) { + throw Object.assign( + new Error('`npm audit signatures` does not support global packages'), { + code: 'EAUDITGLOBAL', + } + ) + } + + log.newItem('loading intalled dependencies') const reporter = this.npm.config.get('json') ? 'json' : 'detail' const opts = { ...this.npm.flatOptions, diff --git a/tap-snapshots/test/lib/commands/audit.js.test.cjs b/tap-snapshots/test/lib/commands/audit.js.test.cjs index 99a602d761a4e..6625483416c4c 100644 --- a/tap-snapshots/test/lib/commands/audit.js.test.cjs +++ b/tap-snapshots/test/lib/commands/audit.js.test.cjs @@ -109,7 +109,7 @@ audited 1 package in 0s @npmcli/arborist@1.0.14 (https://verdaccio-clone.org) -Someone might have tampered with the package since it was published on the registry (monster-in-the-middle attack)! +Someone might have tampered with this package since it was published on the registry! ` @@ -117,7 +117,7 @@ exports[`test/lib/commands/audit.js TAP audit signatures third-party registry wi audited 1 package in 0s 1 package has a missing registry signature but the registry is providing signing keys - run \`npm audit signatures --missing\` for details + run \`npm audit signatures --log-missing-names\` for details ` exports[`test/lib/commands/audit.js TAP audit signatures third-party registry with keys and signatures > must match snapshot 1`] = ` @@ -133,13 +133,13 @@ exports[`test/lib/commands/audit.js TAP audit signatures with both invalid and m audited 2 packages in xxx 1 package has a missing registry signature but the registry is providing signing keys - run \`npm audit signatures --missing\` for details + run \`npm audit signatures --log-missing-names\` for details 1 package has an invalid registry signature: kms-demo@1.0.0 -Someone might have tampered with the package since it was published on the registry (monster-in-the-middle attack)! +Someone might have tampered with this package since it was published on the registry! ` @@ -151,10 +151,10 @@ verified registry signatures, audited 1 package in 0s exports[`test/lib/commands/audit.js TAP audit signatures with color output enabled with both valid and missing signatures > must match snapshot 1`] = ` audited 2 packages in xxx -1 packages have verified registry signatures +1 package has a verified registry signature 1 package has a missing registry signature but the registry is providing signing keys - run \`npm audit signatures --missing\` for details + run \`npm audit signatures --log-missing-names\` for details ` exports[`test/lib/commands/audit.js TAP audit signatures with color output enabled with invalid signatures > must match snapshot 1`] = ` @@ -164,7 +164,7 @@ audited 1 package in 0s kms-demo@1.0.0 -Someone might have tampered with the package since it was published on the registry (monster-in-the-middle attack)! +Someone might have tampered with this package since it was published on the registry! ` @@ -176,7 +176,7 @@ audited 2 packages in xxx async@1.1.1 kms-demo@1.0.0 -Someone might have tampered with the packages since it was published on the registry (monster-in-the-middle attack)! +Someone might have tampered with these packages since they where published on the registry! ` @@ -184,7 +184,7 @@ exports[`test/lib/commands/audit.js TAP audit signatures with color output enabl audited 2 packages in xxx 2 packages have missing registry signatures but the registry is providing signing keys - run \`npm audit signatures --missing\` for details + run \`npm audit signatures --log-missing-names\` for details ` exports[`test/lib/commands/audit.js TAP audit signatures with invalid signatures > must match snapshot 1`] = ` @@ -194,7 +194,7 @@ audited 1 package in 0s kms-demo@1.0.0 -Someone might have tampered with the package since it was published on the registry (monster-in-the-middle attack)! +Someone might have tampered with this package since it was published on the registry! ` @@ -202,7 +202,7 @@ exports[`test/lib/commands/audit.js TAP audit signatures with keys but missing s audited 1 package in 0s 1 package has a missing registry signature but the registry is providing signing keys - run \`npm audit signatures --missing\` for details + run \`npm audit signatures --log-missing-names\` for details ` exports[`test/lib/commands/audit.js TAP audit signatures with multiple invalid signatures > must match snapshot 1`] = ` @@ -213,7 +213,7 @@ audited 2 packages in xxx async@1.1.1 kms-demo@1.0.0 -Someone might have tampered with the packages since it was published on the registry (monster-in-the-middle attack)! +Someone might have tampered with these packages since they where published on the registry! ` @@ -221,16 +221,29 @@ exports[`test/lib/commands/audit.js TAP audit signatures with multiple missing s audited 2 packages in xxx 2 packages have missing registry signatures but the registry is providing signing keys - run \`npm audit signatures --missing\` for details + run \`npm audit signatures --log-missing-names\` for details +` + +exports[`test/lib/commands/audit.js TAP audit signatures with multiple valid signatures and one invalid > must match snapshot 1`] = ` +audited 3 packages in xxx + +2 packages have verified registry signatures + +1 package has an invalid registry signature: + +node-fetch@1.6.0 + +Someone might have tampered with this package since it was published on the registry! + ` exports[`test/lib/commands/audit.js TAP audit signatures with valid and missing signatures > must match snapshot 1`] = ` audited 2 packages in xxx -1 packages have verified registry signatures +1 package has a verified registry signature 1 package has a missing registry signature but the registry is providing signing keys - run \`npm audit signatures --missing\` for details + run \`npm audit signatures --log-missing-names\` for details ` exports[`test/lib/commands/audit.js TAP audit signatures with valid signatures > must match snapshot 1`] = ` diff --git a/test/lib/commands/audit.js b/test/lib/commands/audit.js index 84dbfeac2b848..4b6fc04d8bbf1 100644 --- a/test/lib/commands/audit.js +++ b/test/lib/commands/audit.js @@ -257,7 +257,7 @@ t.test('audit signatures', async t => { color: false, config: { global: false, - missing: false, + 'log-missing-names': false, json: false, omit: [], }, @@ -361,7 +361,7 @@ t.test('audit signatures', async t => { }, dependencies: { get: { - version: 'npm:kms-demo@1.7.1', + version: 'npm:node-fetch@1.7.1', }, }, }), @@ -858,6 +858,112 @@ t.test('audit signatures', async t => { t.matchSnapshot(joinedOutput()) }) + t.test('with multiple valid signatures and one invalid', async t => { + npm.prefix = t.testdir({ + 'package.json': JSON.stringify({ + name: 'test-dep', + version: '1.0.0', + dependencies: { + 'kms-demo': '^1.0.0', + 'node-fetch': '^1.6.0', + }, + devDependencies: { + async: '~2.1.0', + }, + }), + node_modules: { + 'kms-demo': { + 'package.json': JSON.stringify({ + name: 'kms-demo', + version: '1.0.0', + }), + }, + async: { + 'package.json': JSON.stringify({ + name: 'async', + version: '2.5.0', + }), + }, + 'node-fetch': { + 'package.json': JSON.stringify({ + name: 'node-fetch', + version: '1.6.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: { + 'kms-demo': '^1.0.0', + 'node-fetch': '^1.6.0', + }, + devDependencies: { + async: '~2.1.0', + }, + }, + 'node_modules/kms-demo': { + version: '1.0.0', + }, + 'node_modules/async': { + version: '2.5.0', + }, + 'node_modules/node-fetch': { + version: '1.6.0', + }, + }, + dependencies: { + 'kms-demo': { + version: '1.0.0', + }, + 'node-fetch': { + version: '1.6.0', + }, + async: { + version: '2.5.0', + }, + }, + }), + }) + await manifestWithValidSigs() + const asyncManifest = registry.manifest({ + name: 'async', + packuments: [{ + version: '2.5.0', + dist: { + tarball: 'https://registry.npmjs.org/async/-/async-2.5.0.tgz', + integrity: 'sha512-e+lJAJeNWuPCNyxZKOBdaJGyLGHugXVQtrAwtuAe2vhxTYxFT' + + 'KE73p8JuTmdH0qdQZtDvI4dhJwjZc5zsfIsYw==', + signatures: [ + { + keyid: 'SHA256:jl3bwswu80PjjokCgh0o2w5c2U4LhQAE57gj9cz1kzA', + sig: 'MEUCIQCM8cX2U3IVZKKhzQx1w5AlNSDUI+fVf4857K1qT0NTNgIgdT4qwEl' + + '/kg2vU1uIWUI0bGikRvVHCHlRs1rgjPMpRFA=', + }, + ], + }, + }], + }) + await registry.package({ manifest: asyncManifest }) + await manifestWithInvalidSigs('node-fetch', '1.6.0') + validKeys() + + await audit.exec(['signatures']) + + t.equal(process.exitCode, 1, 'should exit with error') + process.exitCode = 0 + t.match(joinedOutput(), /audited 3 packages/) + t.match(joinedOutput(), /2 packages have verified registry signatures/) + t.match(joinedOutput(), /1 package has an invalid registry signature/) + t.matchSnapshot(joinedOutput()) + }) + t.test('with bundled and peer deps and no signatures', async t => { npm.prefix = installWithPeerDeps() await manifestWithValidSigs() @@ -896,7 +1002,7 @@ t.test('audit signatures', async t => { t.equal(process.exitCode, 1, 'should exit with error') process.exitCode = 0 t.match(joinedOutput(), /audited 2 packages/) - t.match(joinedOutput(), /verified registry signatures/) + t.match(joinedOutput(), /verified registry signature/) t.match(joinedOutput(), /missing registry signature/) t.matchSnapshot(joinedOutput()) }) @@ -998,7 +1104,7 @@ t.test('audit signatures', async t => { t.test('output details about missing signatures', async t => { npm.prefix = validInstall() - npm.config.set('missing', true) + npm.config.set('log-missing-names', true) await manifestWithoutSigs() validKeys() @@ -1394,6 +1500,16 @@ t.test('audit signatures', async t => { ) }) + t.test('errors for global packages', async t => { + npm.config.set('global', true) + + await t.rejects( + audit.exec(['signatures']), + /`npm audit signatures` does not support global packages/, + { code: 'ECIGLOBAL' } + ) + }) + t.test('with color output enabled', async t => { t.test('with invalid signatures', async t => { npm.prefix = validInstall()