From d2963c67b992b9b3b9dd32f6f41cbbe4bcc580c8 Mon Sep 17 00:00:00 2001 From: nlf Date: Thu, 6 Oct 2022 12:48:39 -0700 Subject: [PATCH] feat: explicitly validate config within the cli BREAKING CHANGE: the presence of auth related settings that are not scoped to a specific registry found in a config file is no longer supported and will throw errors --- lib/base-command.js | 8 +++ lib/commands/config.js | 2 + .../test/lib/utils/exit-handler.js.test.cjs | 56 +++++++++---------- test/fixtures/mock-npm.js | 1 + test/index.js | 2 +- test/lib/arborist-cmd.js | 6 +- test/lib/commands/adduser.js | 8 --- test/lib/commands/bugs.js | 2 +- test/lib/commands/explain.js | 3 + test/lib/commands/explore.js | 3 + test/lib/commands/help.js | 1 + test/lib/commands/install-ci-test.js | 3 + test/lib/commands/install-test.js | 3 + test/lib/commands/login.js | 8 --- test/lib/commands/publish.js | 6 +- test/lib/commands/set.js | 3 + test/lib/commands/stars.js | 2 +- test/lib/commands/token.js | 2 + test/lib/lifecycle-cmd.js | 3 + 19 files changed, 68 insertions(+), 54 deletions(-) diff --git a/lib/base-command.js b/lib/base-command.js index 3ab800adbfd98..b57b7474a5efb 100644 --- a/lib/base-command.js +++ b/lib/base-command.js @@ -11,6 +11,10 @@ class BaseCommand { constructor (npm) { this.wrapWidth = 80 this.npm = npm + + if (!this.skipConfigValidation) { + this.npm.config.validate() + } } get name () { @@ -25,6 +29,10 @@ class BaseCommand { return this.constructor.ignoreImplicitWorkspace } + get skipConfigValidation () { + return this.constructor.skipConfigValidation + } + get usage () { const usage = [ `${this.constructor.description}`, diff --git a/lib/commands/config.js b/lib/commands/config.js index 96dd4abcaf4af..8d74b8d2741e7 100644 --- a/lib/commands/config.js +++ b/lib/commands/config.js @@ -63,6 +63,8 @@ class Config extends BaseCommand { static ignoreImplicitWorkspace = false + static skipConfigValidation = true + async completion (opts) { const argv = opts.conf.argv.remain if (argv[1] !== 'config') { diff --git a/tap-snapshots/test/lib/utils/exit-handler.js.test.cjs b/tap-snapshots/test/lib/utils/exit-handler.js.test.cjs index 4d52c02d97dd3..1c0bb1c38c811 100644 --- a/tap-snapshots/test/lib/utils/exit-handler.js.test.cjs +++ b/tap-snapshots/test/lib/utils/exit-handler.js.test.cjs @@ -7,34 +7,34 @@ 'use strict' exports[`test/lib/utils/exit-handler.js TAP handles unknown error with logs and debug file > debug file contents 1`] = ` 0 timing npm:load:whichnode Completed in {TIME}ms -15 timing config:load Completed in {TIME}ms -16 timing npm:load:configload Completed in {TIME}ms -17 timing npm:load:mkdirpcache Completed in {TIME}ms -18 timing npm:load:mkdirplogs Completed in {TIME}ms -19 verbose title npm -20 verbose argv -21 timing npm:load:setTitle Completed in {TIME}ms -23 timing npm:load:display Completed in {TIME}ms -24 verbose logfile logs-max:10 dir:{CWD}/test/lib/utils/tap-testdir-exit-handler-handles-unknown-error-with-logs-and-debug-file/cache/_logs/{DATE}- -25 verbose logfile {CWD}/test/lib/utils/tap-testdir-exit-handler-handles-unknown-error-with-logs-and-debug-file/cache/_logs/{DATE}-debug-0.log -26 timing npm:load:logFile Completed in {TIME}ms -27 timing npm:load:timers Completed in {TIME}ms -28 timing npm:load:configScope Completed in {TIME}ms -29 timing npm:load Completed in {TIME}ms -30 silly logfile done cleaning log files -31 verbose stack Error: Unknown error -32 verbose cwd {CWD} -33 verbose Foo 1.0.0 -34 verbose node v1.0.0 -35 verbose npm v1.0.0 -36 error code ECODE -37 error ERR SUMMARY Unknown error -38 error ERR DETAIL Unknown error -39 verbose exit 1 -41 timing npm Completed in {TIME}ms -42 verbose code 1 -43 error A complete log of this run can be found in: -43 error {CWD}/test/lib/utils/tap-testdir-exit-handler-handles-unknown-error-with-logs-and-debug-file/cache/_logs/{DATE}-debug-0.log +13 timing config:load Completed in {TIME}ms +14 timing npm:load:configload Completed in {TIME}ms +15 timing npm:load:mkdirpcache Completed in {TIME}ms +16 timing npm:load:mkdirplogs Completed in {TIME}ms +17 verbose title npm +18 verbose argv +19 timing npm:load:setTitle Completed in {TIME}ms +21 timing npm:load:display Completed in {TIME}ms +22 verbose logfile logs-max:10 dir:{CWD}/test/lib/utils/tap-testdir-exit-handler-handles-unknown-error-with-logs-and-debug-file/cache/_logs/{DATE}- +23 verbose logfile {CWD}/test/lib/utils/tap-testdir-exit-handler-handles-unknown-error-with-logs-and-debug-file/cache/_logs/{DATE}-debug-0.log +24 timing npm:load:logFile Completed in {TIME}ms +25 timing npm:load:timers Completed in {TIME}ms +26 timing npm:load:configScope Completed in {TIME}ms +27 timing npm:load Completed in {TIME}ms +28 silly logfile done cleaning log files +29 verbose stack Error: Unknown error +30 verbose cwd {CWD} +31 verbose Foo 1.0.0 +32 verbose node v1.0.0 +33 verbose npm v1.0.0 +34 error code ECODE +35 error ERR SUMMARY Unknown error +36 error ERR DETAIL Unknown error +37 verbose exit 1 +39 timing npm Completed in {TIME}ms +40 verbose code 1 +41 error A complete log of this run can be found in: +41 error {CWD}/test/lib/utils/tap-testdir-exit-handler-handles-unknown-error-with-logs-and-debug-file/cache/_logs/{DATE}-debug-0.log ` exports[`test/lib/utils/exit-handler.js TAP handles unknown error with logs and debug file > logs 1`] = ` diff --git a/test/fixtures/mock-npm.js b/test/fixtures/mock-npm.js index 72cfa4b0c4beb..0318eadebafe6 100644 --- a/test/fixtures/mock-npm.js +++ b/test/fixtures/mock-npm.js @@ -216,6 +216,7 @@ class MockNpm { } }, list: [{ ...realConfig.defaults, ...config }], + validate: () => {}, } if (t && config.loglevel) { diff --git a/test/index.js b/test/index.js index 081c89cee9c70..5075597b5d93c 100644 --- a/test/index.js +++ b/test/index.js @@ -11,7 +11,7 @@ t.test('loading as main module will load the cli', t => { const cwd = t.testdir() const { spawn } = require('child_process') const LS = require('../lib/commands/ls.js') - const ls = new LS({}) + const ls = new LS({ config: { validate: () => {} } }) const p = spawn(process.execPath, [index, 'ls', '-h', '--cache', cwd]) const out = [] p.stdout.on('data', c => out.push(c)) diff --git a/test/lib/arborist-cmd.js b/test/lib/arborist-cmd.js index 91d8a7b333bf9..d2497efe129dc 100644 --- a/test/lib/arborist-cmd.js +++ b/test/lib/arborist-cmd.js @@ -44,8 +44,7 @@ t.test('arborist-cmd', async t => { class TestCmd extends ArboristCmd {} - const cmd = new TestCmd() - cmd.npm = { localPrefix: path } + const cmd = new TestCmd({ localPrefix: path, config: { validate: () => {} } }) // check filtering for a single workspace name cmd.exec = async function (args) { @@ -97,8 +96,7 @@ t.test('handle getWorkspaces raising an error', async t => { }, }) class TestCmd extends ArboristCmd {} - const cmd = new TestCmd() - cmd.npm = { localPrefix: t.testdir() } + const cmd = new TestCmd({ localPrefix: t.testdir(), config: { validate: () => {} } }) await t.rejects( cmd.execWorkspaces(['foo'], ['a']), diff --git a/test/lib/commands/adduser.js b/test/lib/commands/adduser.js index 04fe8dc8c3147..73e446309ef3b 100644 --- a/test/lib/commands/adduser.js +++ b/test/lib/commands/adduser.js @@ -27,15 +27,7 @@ t.test('legacy', async t => { const { npm, home } = await loadMockNpm(t, { config: { 'auth-type': 'legacy' }, homeDir: { - // These all get cleaned up by config.setCredentialsByURI '.npmrc': [ - '_token=user', - '_password=user', - 'username=user', - '_auth=user', - '_authtoken=user', - '-authtoken=user', - '_authToken=user', '//registry.npmjs.org/:_authToken=user', '//registry.npmjs.org/:always-auth=user', '//registry.npmjs.org/:email=test-email-old@npmjs.org', diff --git a/test/lib/commands/bugs.js b/test/lib/commands/bugs.js index dbd618b0848a4..91d144b6bdc97 100644 --- a/test/lib/commands/bugs.js +++ b/test/lib/commands/bugs.js @@ -62,7 +62,7 @@ const Bugs = t.mock('../../../lib/commands/bugs.js', { '../../../lib/utils/open-url.js': openUrl, }) -const bugs = new Bugs({ flatOptions: {} }) +const bugs = new Bugs({ flatOptions: {}, config: { validate: () => {} } }) t.test('usage', (t) => { t.match(bugs.usage, 'bugs', 'usage has command name in it') diff --git a/test/lib/commands/explain.js b/test/lib/commands/explain.js index 63deb8bc78a4c..c92732e904e60 100644 --- a/test/lib/commands/explain.js +++ b/test/lib/commands/explain.js @@ -6,6 +6,9 @@ const npm = { output: (...args) => { OUTPUT.push(args) }, + config: { + validate: () => {}, + }, } const { resolve } = require('path') diff --git a/test/lib/commands/explore.js b/test/lib/commands/explore.js index 5bb211e4503c3..af6f4df908677 100644 --- a/test/lib/commands/explore.js +++ b/test/lib/commands/explore.js @@ -67,6 +67,9 @@ const getExplore = (windows) => { output: out => { output.push(out) }, + config: { + validate: () => {}, + }, } return new Explore(npm) } diff --git a/test/lib/commands/help.js b/test/lib/commands/help.js index f84f94ad2f8d9..1e623dab9386e 100644 --- a/test/lib/commands/help.js +++ b/test/lib/commands/help.js @@ -19,6 +19,7 @@ const npm = { parsedArgv: { cooked: [], }, + validate: () => {}, }, exec: async (cmd, args) => { if (cmd === 'help-search') { diff --git a/test/lib/commands/install-ci-test.js b/test/lib/commands/install-ci-test.js index 0828d2b24ed97..68b86be43a30f 100644 --- a/test/lib/commands/install-ci-test.js +++ b/test/lib/commands/install-ci-test.js @@ -23,6 +23,9 @@ const installCITest = new InstallCITest({ testCalled = true } }, + config: { + validate: () => {}, + }, }) t.test('the install-ci-test command', t => { diff --git a/test/lib/commands/install-test.js b/test/lib/commands/install-test.js index 223bbe106aec7..0e0cf47521c4e 100644 --- a/test/lib/commands/install-test.js +++ b/test/lib/commands/install-test.js @@ -23,6 +23,9 @@ const installTest = new InstallTest({ testCalled = true } }, + config: { + validate: () => {}, + }, }) t.test('the install-test command', t => { diff --git a/test/lib/commands/login.js b/test/lib/commands/login.js index 8d27421313406..6c1d40c0d6edb 100644 --- a/test/lib/commands/login.js +++ b/test/lib/commands/login.js @@ -26,15 +26,7 @@ t.test('legacy', t => { const { npm, home } = await loadMockNpm(t, { config: { 'auth-type': 'legacy' }, homeDir: { - // These all get cleaned up by config.setCredentialsByURI '.npmrc': [ - '_token=user', - '_password=user', - 'username=user', - '_auth=user', - '_authtoken=user', - '-authtoken=user', - '_authToken=user', '//registry.npmjs.org/:_authToken=user', '//registry.npmjs.org/:always-auth=user', '//registry.npmjs.org/:email=test-email-old@npmjs.org', diff --git a/test/lib/commands/publish.js b/test/lib/commands/publish.js index 00fba9ef218e0..dfcc7204daca5 100644 --- a/test/lib/commands/publish.js +++ b/test/lib/commands/publish.js @@ -190,7 +190,7 @@ t.test('dry-run', async t => { t.test('shows usage with wrong set of arguments', async t => { t.plan(1) const Publish = t.mock('../../../lib/commands/publish.js') - const publish = new Publish({}) + const publish = new Publish({ config: { validate: () => {} } }) await t.rejects(publish.exec(['a', 'b', 'c']), publish.usage) }) @@ -637,7 +637,7 @@ t.test('ignore-scripts', async t => { t.test('_auth config default registry', async t => { const { npm, joinedOutput } = await loadMockNpm(t, { config: { - _auth: basic, + '//registry.npmjs.org/:_auth': basic, }, prefixDir: { 'package.json': JSON.stringify(pkgJson), @@ -661,7 +661,7 @@ t.test('bare _auth and registry config', async t => { const { npm, joinedOutput } = await loadMockNpm(t, { config: { registry: alternateRegistry, - _auth: basic, + '//other.registry.npmjs.org/:_auth': basic, }, prefixDir: { 'package.json': JSON.stringify({ diff --git a/test/lib/commands/set.js b/test/lib/commands/set.js index feeb901571768..9a68eaf32d457 100644 --- a/test/lib/commands/set.js +++ b/test/lib/commands/set.js @@ -40,6 +40,9 @@ const npm = { configArgs = args } }, + config: { + validate: () => {}, + }, } const Set = t.mock('../../../lib/commands/set.js') diff --git a/test/lib/commands/stars.js b/test/lib/commands/stars.js index 094b530d8c610..44de6ba1fb960 100644 --- a/test/lib/commands/stars.js +++ b/test/lib/commands/stars.js @@ -4,7 +4,7 @@ let result = '' const noop = () => null const npm = { - config: { get () {} }, + config: { get () {}, validate: () => {} }, flatOptions: {}, output: (...msg) => { result = [result, ...msg].join('\n') diff --git a/test/lib/commands/token.js b/test/lib/commands/token.js index c32c0b74a96f4..af53f49a130f5 100644 --- a/test/lib/commands/token.js +++ b/test/lib/commands/token.js @@ -7,6 +7,7 @@ const mocks = { } const npm = { output: (...args) => mocks.output(...args), + config: { validate: () => {} }, } const mockToken = (otherMocks) => t.mock('../../../lib/commands/token.js', { @@ -21,6 +22,7 @@ const tokenWithMocks = (options = {}) => { for (const mod in mockRequests) { if (mod === 'npm') { mockRequests.npm = { ...npm, ...mockRequests.npm } + mockRequests.npm.config.validate = () => {} } else { if (typeof mockRequests[mod] === 'function') { mocks[mod] = mockRequests[mod] diff --git a/test/lib/lifecycle-cmd.js b/test/lib/lifecycle-cmd.js index eb03f19270be0..22011197ead54 100644 --- a/test/lib/lifecycle-cmd.js +++ b/test/lib/lifecycle-cmd.js @@ -8,6 +8,9 @@ const npm = { return 'called the right thing' } }, + config: { + validate: () => {}, + }, } t.test('create a lifecycle command', async t => { t.plan(5)