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')
   })