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

Author: wraithgar

Diff for: 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 for: tap-snapshots/test/lib/commands/config.js.test.cjs

@@ -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)
+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 for: 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/,