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

fix(config): be more aggressive about hiding protected values #7504

Merged
merged 1 commit into from
May 10, 2024
+214 −169
Merged

fix(config): be more aggressive about hiding protected values #7504

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
39 changes: 34 additions & 5 deletions lib/commands/config.js
View file
Original file line number Diff line number Diff line change
@@ -9,15 +9,33 @@ const { log, output } = require('proc-log')
const BaseCommand = require('../base-cmd.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
// *have* config definitions so we have to explicitly validate them here.
// This is used to validate during "npm config set"
const nerfDarts = [
'_auth',
'_authToken',
'username',
'_password',
'certfile',
'email',
'keyfile',
'username',
]
// These are the config values to swap with "protected". It does not catch
// every single sensitive thing a user may put in the npmrc file but it gets
// the common ones. This is distinct from nerfDarts because that is used to
// validate valid configs during "npm config set", and folks may have old
// invalid entries lying around in a config file that we still want to protect
// when running "npm config list"
// This is a more general list of values to consider protected. You can not
// "npm config get" them, and they will not display during "npm config list"
const protected = [
'auth',
'authToken',
'certfile',
'email',
'keyfile',
'password',
'username',
]

// take an array of `[key, value, k2=v2, k3, v3, ...]` and turn into
@@ -40,10 +58,21 @@ const publicVar = k => {
if (k.startsWith('_')) {
return false
}
// //localhost:8080/:_password
if (k.startsWith('//') && k.includes(':_')) {
if (protected.includes(k)) {
return false
}
// //localhost:8080/:_password
if (k.startsWith('//')) {
if (k.includes(':_')) {
return false
}
// //registry:_authToken or //registry:authToken
for (const p of protected) {
if (k.endsWith(`:${p}`) || k.endsWith(`:_${p}`)) {
return false
}
}
}
return true
}

@@ -320,7 +349,7 @@ ${defData}
const src = this.npm.config.find(k)
const overridden = src !== where
msg.push((overridden ? '; ' : '') +
`${k} = ${v} ${overridden ? `; overridden by ${src}` : ''}`)
`${k} = ${v}${overridden ? ` ; overridden by ${src}` : ''}`)
}
msg.push('')
}
330 changes: 167 additions & 163 deletions tap-snapshots/test/lib/commands/config.js.test.cjs
View file
Original file line number Diff line number Diff line change
@@ -173,198 +173,202 @@ exports[`test/lib/commands/config.js TAP config list --json > output matches sna
exports[`test/lib/commands/config.js TAP config list --long > output matches snapshot 1`] = `
; "default" config from default values
_auth = (protected)
access = null
all = false
allow-same-version = false
also = null
audit = true
audit-level = null
auth-type = "web"
before = null
bin-links = true
browser = null
ca = null
_auth = (protected)
access = null
all = false
allow-same-version = false
also = null
audit = true
audit-level = null
auth-type = "web"
before = null
bin-links = true
browser = null
ca = null
; cache = "{CACHE}" ; overridden by cli
cache-max = null
cache-min = 0
cafile = null
call = ""
cert = null
cidr = null
cache-max = null
cache-min = 0
cafile = null
call = ""
cert = null
cidr = null
; color = {COLOR}
commit-hooks = true
cpu = null
depth = null
description = true
dev = false
diff = []
diff-dst-prefix = "b/"
diff-ignore-all-space = false
diff-name-only = false
diff-no-prefix = false
diff-src-prefix = "a/"
diff-text = false
diff-unified = 3
dry-run = false
editor = "{EDITOR}"
engine-strict = false
expect-result-count = null
expect-results = null
fetch-retries = 2
fetch-retry-factor = 10
fetch-retry-maxtimeout = 60000
fetch-retry-mintimeout = 10000
fetch-timeout = 300000
force = false
foreground-scripts = false
format-package-lock = true
fund = true
git = "git"
git-tag-version = true
global = false
global-style = false
globalconfig = "{CWD}/global/etc/npmrc"
heading = "npm"
https-proxy = null
if-present = false
ignore-scripts = false
include = []
include-staged = false
include-workspace-root = false
init-author-email = ""
init-author-name = ""
init-author-url = ""
init-license = "ISC"
init-module = "{CWD}/home/.npm-init.js"
init-version = "1.0.0"
init.author.email = ""
init.author.name = ""
init.author.url = ""
init.license = "ISC"
init.module = "{CWD}/home/.npm-init.js"
init.version = "1.0.0"
install-links = false
install-strategy = "hoisted"
json = false
key = null
legacy-bundling = false
legacy-peer-deps = false
libc = null
link = false
local-address = null
location = "user"
lockfile-version = null
loglevel = "notice"
logs-dir = null
logs-max = 10
commit-hooks = true
cpu = null
depth = null
description = true
dev = false
diff = []
diff-dst-prefix = "b/"
diff-ignore-all-space = false
diff-name-only = false
diff-no-prefix = false
diff-src-prefix = "a/"
diff-text = false
diff-unified = 3
dry-run = false
editor = "{EDITOR}"
engine-strict = false
expect-result-count = null
expect-results = null
fetch-retries = 2
fetch-retry-factor = 10
fetch-retry-maxtimeout = 60000
fetch-retry-mintimeout = 10000
fetch-timeout = 300000
force = false
foreground-scripts = false
format-package-lock = true
fund = true
git = "git"
git-tag-version = true
global = false
global-style = false
globalconfig = "{CWD}/global/etc/npmrc"
heading = "npm"
https-proxy = null
if-present = false
ignore-scripts = false
include = []
include-staged = false
include-workspace-root = false
init-author-email = ""
init-author-name = ""
init-author-url = ""
init-license = "ISC"
init-module = "{CWD}/home/.npm-init.js"
init-version = "1.0.0"
init.author.email = ""
init.author.name = ""
init.author.url = ""
init.license = "ISC"
init.module = "{CWD}/home/.npm-init.js"
init.version = "1.0.0"
install-links = false
install-strategy = "hoisted"
json = false
key = null
legacy-bundling = false
legacy-peer-deps = false
libc = null
link = false
local-address = null
location = "user"
lockfile-version = null
loglevel = "notice"
logs-dir = null
logs-max = 10
; long = false ; overridden by cli
maxsockets = 15
message = "%s"
node-options = null
noproxy = [""]
npm-version = "{NPM-VERSION}"
offline = false
omit = []
omit-lockfile-registry-resolved = false
only = null
optional = null
os = null
otp = null
pack-destination = "."
package = []
package-lock = true
package-lock-only = false
parseable = false
prefer-dedupe = false
prefer-offline = false
prefer-online = false
prefix = "{CWD}/global"
preid = ""
production = null
maxsockets = 15
message = "%s"
node-options = null
noproxy = [""]
npm-version = "{NPM-VERSION}"
offline = false
omit = []
omit-lockfile-registry-resolved = false
only = null
optional = null
os = null
otp = null
pack-destination = "."
package = []
package-lock = true
package-lock-only = false
parseable = false
prefer-dedupe = false
prefer-offline = false
prefer-online = false
prefix = "{CWD}/global"
preid = ""
production = null
progress = {PROGRESS}
provenance = false
provenance-file = null
proxy = null
read-only = false
rebuild-bundle = true
registry = "https://registry.npmjs.org/"
replace-registry-host = "npmjs"
save = true
save-bundle = false
save-dev = false
save-exact = false
save-optional = false
save-peer = false
save-prefix = "^"
save-prod = false
sbom-format = null
sbom-type = "library"
scope = ""
script-shell = null
searchexclude = ""
searchlimit = 20
searchopts = ""
searchstaleness = 900
shell = "{SHELL}"
shrinkwrap = true
sign-git-commit = false
sign-git-tag = false
strict-peer-deps = false
strict-ssl = true
tag = "latest"
tag-version-prefix = "v"
timing = false
umask = 0
unicode = false
update-notifier = true
usage = false
user-agent = "npm/{npm-version} node/{node-version} {platform} {arch} workspaces/{workspaces} {ci}"
userconfig = "{CWD}/home/.npmrc"
version = false
versions = false
viewer = "{VIEWER}"
which = null
workspace = []
workspaces = null
workspaces-update = true
yes = null
provenance = false
provenance-file = null
proxy = null
read-only = false
rebuild-bundle = true
registry = "https://registry.npmjs.org/"
replace-registry-host = "npmjs"
save = true
save-bundle = false
save-dev = false
save-exact = false
save-optional = false
save-peer = false
save-prefix = "^"
save-prod = false
sbom-format = null
sbom-type = "library"
scope = ""
script-shell = null
searchexclude = ""
searchlimit = 20
searchopts = ""
searchstaleness = 900
shell = "{SHELL}"
shrinkwrap = true
sign-git-commit = false
sign-git-tag = false
strict-peer-deps = false
strict-ssl = true
tag = "latest"
tag-version-prefix = "v"
timing = false
umask = 0
unicode = false
update-notifier = true
usage = false
user-agent = "npm/{npm-version} node/{node-version} {platform} {arch} workspaces/{workspaces} {ci}"
userconfig = "{CWD}/home/.npmrc"
version = false
versions = false
viewer = "{VIEWER}"
which = null
workspace = []
workspaces = null
workspaces-update = true
yes = null
; "global" config from {CWD}/global/etc/npmrc
globalloaded = "yes"
globalloaded = "yes"
; "user" config from {CWD}/home/.npmrc
userloaded = "yes"
userloaded = "yes"
; "project" config from {CWD}/prefix/.npmrc
projectloaded = "yes"
projectloaded = "yes"
; "cli" config from command line options
cache = "{CACHE}"
cache = "{CACHE}"
color = {COLOR}
long = true
`

exports[`test/lib/commands/config.js TAP config list > output matches snapshot 1`] = `
; "global" config from {CWD}/global/etc/npmrc
globalloaded = "yes"
globalloaded = "yes"
; "user" config from {CWD}/home/.npmrc
userloaded = "yes"
_auth = (protected)
//nerfdart:_auth = (protected)
//nerfdart:auth = (protected)
auth = (protected)
Comment on lines +359 to +362
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

here is the actual new snapshot, whitespace fix notwithstanding.

userloaded = "yes"
; "project" config from {CWD}/prefix/.npmrc
projectloaded = "yes"
projectloaded = "yes"
; "cli" config from command line options
cache = "{CACHE}"
cache = "{CACHE}"
color = {COLOR}
; node bin location = {NODE-BIN-LOCATION}
@@ -379,9 +383,9 @@ color = {COLOR}
exports[`test/lib/commands/config.js TAP config list with publishConfig global > output matches snapshot 1`] = `
; "cli" config from command line options
cache = "{CACHE}"
cache = "{CACHE}"
color = {COLOR}
global = true
global = true
; node bin location = {NODE-BIN-LOCATION}
; node version = {NODE-VERSION}
@@ -395,7 +399,7 @@ global = true
exports[`test/lib/commands/config.js TAP config list with publishConfig local > output matches snapshot 1`] = `
; "cli" config from command line options
cache = "{CACHE}"
cache = "{CACHE}"
color = {COLOR}
; node bin location = {NODE-BIN-LOCATION}
14 changes: 13 additions & 1 deletion test/lib/commands/config.js
View file
Original file line number Diff line number Diff line change
@@ -80,7 +80,13 @@ t.test('config list', async t => {
},
},
homeDir: {
'.npmrc': 'userloaded=yes',
'.npmrc': [
'userloaded=yes',
'auth=bad',
'_auth=bad',
'//nerfdart:auth=bad',
'//nerfdart:_auth=bad',
].join('\n'),
},
})

@@ -486,6 +492,12 @@ t.test('config get private key', async t => {
'rejects with protected string'
)

await t.rejects(
npm.exec('config', ['get', 'authToken']),
/authToken option is protected/,
'rejects with protected string'
)

await t.rejects(
npm.exec('config', ['get', '//localhost:8080/:_password']),
/_password option is protected/,