From 8a17f274e5538d0c591e62a1a0ec65294cabc64e Mon Sep 17 00:00:00 2001 From: Gar Date: Tue, 18 Oct 2022 14:43:08 -0700 Subject: [PATCH] feat: refuse to set deprecated/invalid config BREAKING CHANGE: `npm config set` will no longer accept deprecated or invalid config options. --- lib/commands/config.js | 24 ++++- test/lib/commands/config.js | 172 ++++++++++++++++++++++-------------- 2 files changed, 130 insertions(+), 66 deletions(-) diff --git a/lib/commands/config.js b/lib/commands/config.js index ed3946880d0b8..103fbb554e5d1 100644 --- a/lib/commands/config.js +++ b/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}=` : '' diff --git a/test/lib/commands/config.js b/test/lib/commands/config.js index c94e0df1395c8..35872e722e17e 100644 --- a/test/lib/commands/config.js +++ b/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') })