Skip to content

Commit

Permalink
feat: explicitly validate config within the cli
Browse files Browse the repository at this point in the history
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
  • Loading branch information
nlf authored and fritzy committed Oct 13, 2022
1 parent cee3fd9 commit d2963c6
Show file tree
Hide file tree
Showing 19 changed files with 68 additions and 54 deletions.
8 changes: 8 additions & 0 deletions lib/base-command.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,10 @@ class BaseCommand {
constructor (npm) {
this.wrapWidth = 80
this.npm = npm

if (!this.skipConfigValidation) {
this.npm.config.validate()
}
}

get name () {
Expand All @@ -25,6 +29,10 @@ class BaseCommand {
return this.constructor.ignoreImplicitWorkspace
}

get skipConfigValidation () {
return this.constructor.skipConfigValidation
}

get usage () {
const usage = [
`${this.constructor.description}`,
Expand Down
2 changes: 2 additions & 0 deletions lib/commands/config.js
Original file line number Diff line number Diff line change
Expand Up @@ -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') {
Expand Down
56 changes: 28 additions & 28 deletions tap-snapshots/test/lib/utils/exit-handler.js.test.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -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`] = `
Expand Down
1 change: 1 addition & 0 deletions test/fixtures/mock-npm.js
Original file line number Diff line number Diff line change
Expand Up @@ -216,6 +216,7 @@ class MockNpm {
}
},
list: [{ ...realConfig.defaults, ...config }],
validate: () => {},
}

if (t && config.loglevel) {
Expand Down
2 changes: 1 addition & 1 deletion test/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down
6 changes: 2 additions & 4 deletions test/lib/arborist-cmd.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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']),
Expand Down
8 changes: 0 additions & 8 deletions test/lib/commands/adduser.js
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down
2 changes: 1 addition & 1 deletion test/lib/commands/bugs.js
Original file line number Diff line number Diff line change
Expand Up @@ -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')
Expand Down
3 changes: 3 additions & 0 deletions test/lib/commands/explain.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,9 @@ const npm = {
output: (...args) => {
OUTPUT.push(args)
},
config: {
validate: () => {},
},
}
const { resolve } = require('path')

Expand Down
3 changes: 3 additions & 0 deletions test/lib/commands/explore.js
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,9 @@ const getExplore = (windows) => {
output: out => {
output.push(out)
},
config: {
validate: () => {},
},
}
return new Explore(npm)
}
Expand Down
1 change: 1 addition & 0 deletions test/lib/commands/help.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ const npm = {
parsedArgv: {
cooked: [],
},
validate: () => {},
},
exec: async (cmd, args) => {
if (cmd === 'help-search') {
Expand Down
3 changes: 3 additions & 0 deletions test/lib/commands/install-ci-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,9 @@ const installCITest = new InstallCITest({
testCalled = true
}
},
config: {
validate: () => {},
},
})

t.test('the install-ci-test command', t => {
Expand Down
3 changes: 3 additions & 0 deletions test/lib/commands/install-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,9 @@ const installTest = new InstallTest({
testCalled = true
}
},
config: {
validate: () => {},
},
})

t.test('the install-test command', t => {
Expand Down
8 changes: 0 additions & 8 deletions test/lib/commands/login.js
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down
6 changes: 3 additions & 3 deletions test/lib/commands/publish.js
Original file line number Diff line number Diff line change
Expand Up @@ -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)
})
Expand Down Expand Up @@ -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),
Expand All @@ -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({
Expand Down
3 changes: 3 additions & 0 deletions test/lib/commands/set.js
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,9 @@ const npm = {
configArgs = args
}
},
config: {
validate: () => {},
},
}

const Set = t.mock('../../../lib/commands/set.js')
Expand Down
2 changes: 1 addition & 1 deletion test/lib/commands/stars.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ let result = ''

const noop = () => null
const npm = {
config: { get () {} },
config: { get () {}, validate: () => {} },
flatOptions: {},
output: (...msg) => {
result = [result, ...msg].join('\n')
Expand Down
2 changes: 2 additions & 0 deletions test/lib/commands/token.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ const mocks = {
}
const npm = {
output: (...args) => mocks.output(...args),
config: { validate: () => {} },
}

const mockToken = (otherMocks) => t.mock('../../../lib/commands/token.js', {
Expand All @@ -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]
Expand Down
3 changes: 3 additions & 0 deletions test/lib/lifecycle-cmd.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,9 @@ const npm = {
return 'called the right thing'
}
},
config: {
validate: () => {},
},
}
t.test('create a lifecycle command', async t => {
t.plan(5)
Expand Down

0 comments on commit d2963c6

Please sign in to comment.