Skip to content

Commit

Permalink
feat: refuse to set deprecated/invalid config (npm#5719)
Browse files Browse the repository at this point in the history 
BREAKING CHANGE: `npm config set` will no longer accept deprecated or
invalid config options.
  • Loading branch information
@wraithgar @lukekarrys
wraithgar authored and lukekarrys committed Oct 19, 2022
1 parent 6ffa5b7 commit be642c6
Show file tree
Hide file tree
Showing 2 changed files with 130 additions and 66 deletions.
24 changes: 23 additions & 1 deletion lib/commands/config.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,18 @@ const localeCompare = require('@isaacs/string-locale-compare')('en')
const rpj = require('read-package-json-fast')
const log = require('../utils/log-shim.js')

// These are the configs that we can nerf-dart. Not all of them currently even
// *have* config definitions so we have to explicitly validate them here
const nerfDarts = [
'_auth',
'_authToken',
'username',
'_password',
'email',
'certfile',
'keyfile',
]

// take an array of `[key, value, k2=v2, k3, v3, ...]` and turn into
// { key: value, k2: v2, k3: v3 }
const keyValues = args => {
Expand Down Expand Up @@ -146,6 +158,16 @@ class Config extends BaseCommand {
const where = this.npm.flatOptions.location
for (const [key, val] of Object.entries(keyValues(args))) {
log.info('config', 'set %j %j', key, val)
const baseKey = key.split(':').pop()
if (!this.npm.config.definitions[baseKey] && !nerfDarts.includes(baseKey)) {
throw new Error(`\`${baseKey}\` is not a valid npm option`)
}
const deprecated = this.npm.config.definitions[baseKey]?.deprecated
if (deprecated) {
throw new Error(
`The \`${baseKey}\` option is deprecated, and can not be set in this way${deprecated}`
)
}
this.npm.config.set(key, val || '', where)
if (!this.npm.config.validate(where)) {
log.warn('config', 'omitting invalid config values')
Expand All @@ -163,7 +185,7 @@ class Config extends BaseCommand {
const out = []
for (const key of keys) {
if (!publicVar(key)) {
throw new Error(`The ${key} option is protected, and cannot be retrieved in this way`)
throw new Error(`The ${key} option is protected, and can not be retrieved in this way`)
}

const pref = keys.length > 1 ? `${key}=` : ''
Expand Down
172 changes: 107 additions & 65 deletions test/lib/commands/config.js
View file
Original file line number Diff line number Diff line change
@@ -1,13 +1,11 @@
const { join } = require('path')
const { promisify } = require('util')
const fs = require('fs')
const fs = require('fs/promises')
const ini = require('ini')
const tspawk = require('../../fixtures/tspawk')
const t = require('tap')

const spawk = tspawk(t)

const readFile = promisify(fs.readFile)

const Sandbox = require('../../fixtures/sandbox.js')

t.test('config no args', async t => {
Expand Down Expand Up @@ -142,60 +140,100 @@ t.test('config delete no args', async t => {
t.test('config delete single key', async t => {
// location defaults to user, so we work with a userconfig
const home = t.testdir({
'.npmrc': 'foo=bar\nbar=baz',
'.npmrc': 'access=public\nall=true',
})

const sandbox = new Sandbox(t)
await sandbox.run('config', ['delete', 'foo'], { home })
await sandbox.run('config', ['delete', 'access'], { home })

t.equal(sandbox.config.get('foo'), undefined, 'foo should no longer be set')
t.equal(sandbox.config.get('access'), null, 'acces should be defaulted')

const contents = await readFile(join(home, '.npmrc'), { encoding: 'utf8' })
t.not(contents.includes('foo='), 'foo was removed on disk')
const contents = await fs.readFile(join(home, '.npmrc'), { encoding: 'utf8' })
const rc = ini.parse(contents)
t.not(rc.access, 'access is not set')
})

t.test('config delete multiple keys', async t => {
const home = t.testdir({
'.npmrc': 'foo=bar\nbar=baz\nbaz=buz',
'.npmrc': 'access=public\nall=true\naudit=false',
})

const sandbox = new Sandbox(t)
await sandbox.run('config', ['delete', 'foo', 'bar'], { home })
await sandbox.run('config', ['delete', 'access', 'all'], { home })

t.equal(sandbox.config.get('foo'), undefined, 'foo should no longer be set')
t.equal(sandbox.config.get('bar'), undefined, 'bar should no longer be set')
t.equal(sandbox.config.get('access'), null, 'access should be defaulted')
t.equal(sandbox.config.get('all'), false, 'all should be defaulted')

const contents = await readFile(join(home, '.npmrc'), { encoding: 'utf8' })
t.not(contents.includes('foo='), 'foo was removed on disk')
t.not(contents.includes('bar='), 'bar was removed on disk')
const contents = await fs.readFile(join(home, '.npmrc'), { encoding: 'utf8' })
const rc = ini.parse(contents)
t.not(rc.access, 'access is not set')
t.not(rc.all, 'all is not set')
})

t.test('config delete key --location=global', async t => {
const global = t.testdir({
npmrc: 'foo=bar\nbar=baz',
npmrc: 'access=public\nall=true',
})

const sandbox = new Sandbox(t)
await sandbox.run('config', ['delete', 'foo', '--location=global'], { global })
await sandbox.run('config', ['delete', 'access', '--location=global'], { global })

t.equal(sandbox.config.get('foo', 'global'), undefined, 'foo should no longer be set')
t.equal(sandbox.config.get('access', 'global'), undefined, 'access should be defaulted')

const contents = await readFile(join(global, 'npmrc'), { encoding: 'utf8' })
t.not(contents.includes('foo='), 'foo was removed on disk')
const contents = await fs.readFile(join(global, 'npmrc'), { encoding: 'utf8' })
const rc = ini.parse(contents)
t.not(rc.access, 'access is not set')
})

t.test('config delete key --global', async t => {
const global = t.testdir({
npmrc: 'foo=bar\nbar=baz',
npmrc: 'access=public\nall=true',
})

const sandbox = new Sandbox(t)
await sandbox.run('config', ['delete', 'foo', '--global'], { global })
await sandbox.run('config', ['delete', 'access', '--global'], { global })

t.equal(sandbox.config.get('access', 'global'), undefined, 'access should no longer be set')

const contents = await fs.readFile(join(global, 'npmrc'), { encoding: 'utf8' })
const rc = ini.parse(contents)
t.not(rc.access, 'access is not set')
})

t.test('config set invalid option', async t => {
const sandbox = new Sandbox(t)
await t.rejects(
sandbox.run('config', ['set', 'nonexistantconfigoption', 'something']),
/not a valid npm option/
)
})

t.test('config set deprecated option', async t => {
const sandbox = new Sandbox(t)
await t.rejects(
sandbox.run('config', ['set', 'shrinkwrap', 'true']),
/deprecated/
)
})

t.equal(sandbox.config.get('foo', 'global'), undefined, 'foo should no longer be set')
t.test('config set nerf-darted option', async t => {
const sandbox = new Sandbox(t)
await sandbox.run('config', ['set', '//npm.pkg.github.com/:_authToken', '0xdeadbeef'])
t.equal(
sandbox.config.get('//npm.pkg.github.com/:_authToken'),
'0xdeadbeef',
'nerf-darted config is set'
)
})

const contents = await readFile(join(global, 'npmrc'), { encoding: 'utf8' })
t.not(contents.includes('foo='), 'foo was removed on disk')
t.test('config set scoped optoin', async t => {
const sandbox = new Sandbox(t)
await sandbox.run('config', ['set', '@npm:registry', 'https://registry.npmjs.org'])
t.equal(
sandbox.config.get('@npm:registry'),
'https://registry.npmjs.org',
'scoped config is set'
)
})

t.test('config set no args', async t => {
Expand All @@ -212,65 +250,67 @@ t.test('config set no args', async t => {

t.test('config set key', async t => {
const home = t.testdir({
'.npmrc': 'foo=bar',
'.npmrc': 'access=public',
})

const sandbox = new Sandbox(t, { home })

await sandbox.run('config', ['set', 'foo'])
await sandbox.run('config', ['set', 'access'])

t.equal(sandbox.config.get('foo'), '', 'set the value for foo')
t.equal(sandbox.config.get('access'), null, 'set the value for access')

const contents = await readFile(join(home, '.npmrc'), { encoding: 'utf8' })
t.ok(contents.includes('foo='), 'wrote foo to disk')
await t.rejects(fs.stat(join(home, '.npmrc'), { encoding: 'utf8' }), 'removed empty config')
})

t.test('config set key value', async t => {
const home = t.testdir({
'.npmrc': 'foo=bar',
'.npmrc': 'access=public',
})

const sandbox = new Sandbox(t, { home })

await sandbox.run('config', ['set', 'foo', 'baz'])
await sandbox.run('config', ['set', 'access', 'restricted'])

t.equal(sandbox.config.get('foo'), 'baz', 'set the value for foo')
t.equal(sandbox.config.get('access'), 'restricted', 'set the value for access')

const contents = await readFile(join(home, '.npmrc'), { encoding: 'utf8' })
t.ok(contents.includes('foo=baz'), 'wrote foo to disk')
const contents = await fs.readFile(join(home, '.npmrc'), { encoding: 'utf8' })
const rc = ini.parse(contents)
t.equal(rc.access, 'restricted', 'access is set to restricted')
})

t.test('config set key=value', async t => {
const home = t.testdir({
'.npmrc': 'foo=bar',
'.npmrc': 'access=public',
})

const sandbox = new Sandbox(t, { home })

await sandbox.run('config', ['set', 'foo=baz'])
await sandbox.run('config', ['set', 'access=restricted'])

t.equal(sandbox.config.get('foo'), 'baz', 'set the value for foo')
t.equal(sandbox.config.get('access'), 'restricted', 'set the value for access')

const contents = await readFile(join(home, '.npmrc'), { encoding: 'utf8' })
t.ok(contents.includes('foo=baz'), 'wrote foo to disk')
const contents = await fs.readFile(join(home, '.npmrc'), { encoding: 'utf8' })
const rc = ini.parse(contents)
t.equal(rc.access, 'restricted', 'access is set to restricted')
})

t.test('config set key1 value1 key2=value2 key3', async t => {
const home = t.testdir({
'.npmrc': 'foo=bar\nbar=baz\nbaz=foo',
'.npmrc': 'access=public\nall=true\naudit=true',
})

const sandbox = new Sandbox(t, { home })
await sandbox.run('config', ['set', 'foo', 'oof', 'bar=rab', 'baz'])
await sandbox.run('config', ['set', 'access', 'restricted', 'all=false', 'audit'])

t.equal(sandbox.config.get('foo'), 'oof', 'foo was set')
t.equal(sandbox.config.get('bar'), 'rab', 'bar was set')
t.equal(sandbox.config.get('baz'), '', 'baz was set')
t.equal(sandbox.config.get('access'), 'restricted', 'access was set')
t.equal(sandbox.config.get('all'), false, 'all was set')
t.equal(sandbox.config.get('audit'), false, 'audit was set')

const contents = await readFile(join(home, '.npmrc'), { encoding: 'utf8' })
t.ok(contents.includes('foo=oof'), 'foo was written to disk')
t.ok(contents.includes('bar=rab'), 'bar was written to disk')
t.ok(contents.includes('baz='), 'baz was written to disk')
const contents = await fs.readFile(join(home, '.npmrc'), { encoding: 'utf8' })
const rc = ini.parse(contents)
t.equal(rc.access, 'restricted', 'access is set to restricted')
t.equal(rc.all, false, 'all is set to false')
t.equal(rc.audit, false, 'audit is set to false')
})

t.test('config set invalid key logs warning', async t => {
Expand All @@ -287,30 +327,32 @@ t.test('config set invalid key logs warning', async t => {

t.test('config set key=value --location=global', async t => {
const global = t.testdir({
npmrc: 'foo=bar\nbar=baz',
npmrc: 'access=public\nall=true',
})

const sandbox = new Sandbox(t, { global })
await sandbox.run('config', ['set', 'foo=buzz', '--location=global'])
await sandbox.run('config', ['set', 'access=restricted', '--location=global'])

t.equal(sandbox.config.get('foo', 'global'), 'buzz', 'foo should be set')
t.equal(sandbox.config.get('access', 'global'), 'restricted', 'foo should be set')

const contents = await readFile(join(global, 'npmrc'), { encoding: 'utf8' })
t.not(contents.includes('foo=buzz'), 'foo was saved on disk')
const contents = await fs.readFile(join(global, 'npmrc'), { encoding: 'utf8' })
const rc = ini.parse(contents)
t.equal(rc.access, 'restricted', 'access is set to restricted')
})

t.test('config set key=value --global', async t => {
const global = t.testdir({
npmrc: 'foo=bar\nbar=baz',
npmrc: 'access=public\nall=true',
})

const sandbox = new Sandbox(t, { global })
await sandbox.run('config', ['set', 'foo=buzz', '--global'])
await sandbox.run('config', ['set', 'access=restricted', '--global'])

t.equal(sandbox.config.get('foo', 'global'), 'buzz', 'foo should be set')
t.equal(sandbox.config.get('access', 'global'), 'restricted', 'access should be set')

const contents = await readFile(join(global, 'npmrc'), { encoding: 'utf8' })
t.not(contents.includes('foo=buzz'), 'foo was saved on disk')
const contents = await fs.readFile(join(global, 'npmrc'), { encoding: 'utf8' })
const rc = ini.parse(contents)
t.equal(rc.access, 'restricted', 'access is set to restricted')
})

t.test('config get no args', async t => {
Expand Down Expand Up @@ -383,7 +425,7 @@ t.test('config edit', async t => {
'editor opened the user config file'
)

const contents = await readFile(join(home, '.npmrc'), { encoding: 'utf8' })
const contents = await fs.readFile(join(home, '.npmrc'), { encoding: 'utf8' })
t.ok(contents.includes('foo=bar'), 'kept foo')
t.ok(contents.includes('bar=baz'), 'kept bar')
t.ok(contents.includes('shown below with default values'), 'appends defaults to file')
Expand Down Expand Up @@ -448,7 +490,7 @@ t.test('config fix', (t) => {
t.not(sandbox.config.get('_authToken', 'global'), '_authToken is not set globally')
t.equal(sandbox.config.get(`${registry}:_authToken`, 'global'), 'afaketoken',
'global _authToken was scoped')
const globalConfig = await readFile(join(root, 'global', 'npmrc'), { encoding: 'utf8' })
const globalConfig = await fs.readFile(join(root, 'global', 'npmrc'), { encoding: 'utf8' })
t.equal(globalConfig, `${registry}:_authToken=afaketoken\n`, 'global config was written')

// user config fixes
Expand All @@ -459,7 +501,7 @@ t.test('config fix', (t) => {
t.not(sandbox.config.get('_authtoken', 'user'), '_authtoken is not set in user config')
t.not(sandbox.config.get('_auth'), '_auth is not set in user config')
t.equal(sandbox.config.get(`${registry}:_auth`, 'user'), 'beef', 'user _auth was scoped')
const userConfig = await readFile(join(root, 'home', '.npmrc'), { encoding: 'utf8' })
const userConfig = await fs.readFile(join(root, 'home', '.npmrc'), { encoding: 'utf8' })
t.equal(userConfig, `${registry}:_auth=beef\n`, 'user config was written')
})

Expand Down Expand Up @@ -488,7 +530,7 @@ t.test('config fix', (t) => {
t.equal(sandbox.config.get('_authtoken', 'global'), 'notatoken', 'global _authtoken untouched')
t.equal(sandbox.config.get('_authToken', 'global'), 'afaketoken', 'global _authToken untouched')
t.not(sandbox.config.get(`${registry}:_authToken`, 'global'), 'global _authToken not scoped')
const globalConfig = await readFile(join(root, 'global', 'npmrc'), { encoding: 'utf8' })
const globalConfig = await fs.readFile(join(root, 'global', 'npmrc'), { encoding: 'utf8' })
t.equal(globalConfig, '_authtoken=notatoken\n_authToken=afaketoken',
'global config was not written')

Expand All @@ -500,7 +542,7 @@ t.test('config fix', (t) => {
t.not(sandbox.config.get('_authtoken', 'user'), '_authtoken is not set in user config')
t.not(sandbox.config.get('_auth', 'user'), '_auth is not set in user config')
t.equal(sandbox.config.get(`${registry}:_auth`, 'user'), 'beef', 'user _auth was scoped')
const userConfig = await readFile(join(root, 'home', '.npmrc'), { encoding: 'utf8' })
const userConfig = await fs.readFile(join(root, 'home', '.npmrc'), { encoding: 'utf8' })
t.equal(userConfig, `${registry}:_auth=beef\n`, 'user config was written')
})

Expand Down

0 comments on commit be642c6

Please sign in to comment.