feat: refuse to set deprecated/invalid config (#5719)

BREAKING CHANGE: `npm config set` will no longer accept deprecated or invalid config options.

Author: wraithgar

lib/commands/config.js

@@ -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 => {
@@ -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')
@@ -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}=` : ''

test/lib/commands/config.js

@@ -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 => {
@@ -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 => {
@@ -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 => {
@@ -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 => {
@@ -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')
@@ -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
@@ -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')
  })
 
@@ -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')
 
@@ -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')
  })