Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: refuse to set deprecated/invalid config #5719

Merged
merged 1 commit into from
Oct 19, 2022
Merged

feat: refuse to set deprecated/invalid config #5719

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 23 additions & 1 deletion lib/commands/config.js
View file
Open in desktop
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
Open in desktop
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