From db6291036f076bf0251b74a504bd5b693c29c4bb Mon Sep 17 00:00:00 2001 From: Gar Date: Fri, 10 May 2024 08:43:22 -0700 Subject: [PATCH] fix(config): be more aggressive about hiding protected values (#7504) Err on the side of not displaying things even if they're not valid config --- lib/commands/config.js | 39 ++- .../test/lib/commands/config.js.test.cjs | 330 +++++++++--------- test/lib/commands/config.js | 14 +- 3 files changed, 214 insertions(+), 169 deletions(-) diff --git a/lib/commands/config.js b/lib/commands/config.js index 7fb8476276937..2f1f11e762a73 100644 --- a/lib/commands/config.js +++ b/lib/commands/config.js @@ -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('') } diff --git a/tap-snapshots/test/lib/commands/config.js.test.cjs b/tap-snapshots/test/lib/commands/config.js.test.cjs index ace89ec9a719d..0d62bacd45fa1 100644 --- a/tap-snapshots/test/lib/commands/config.js.test.cjs +++ b/tap-snapshots/test/lib/commands/config.js.test.cjs @@ -173,178 +173,178 @@ 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 ` @@ -352,19 +352,23 @@ 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) +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} diff --git a/test/lib/commands/config.js b/test/lib/commands/config.js index 11ac9a7477eb1..563bb97dc1612 100644 --- a/test/lib/commands/config.js +++ b/test/lib/commands/config.js @@ -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/,