Redact protected config values in argv logs

Author: AI-DEV-BOT

lib/commands/config.js

@@ -8,21 +8,7 @@ const { defaults, definitions, nerfDarts, proxyEnv } = require('@npmcli/config/l
 const { log, output, input } = require('proc-log')
 const BaseCommand = require('../base-cmd.js')
 const { redact } = require('@npmcli/redact')
-
-// 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 cannot "npm config get" them, and they will not display during "npm config list"
-const protected = [
-  'auth',
-  'authToken',
-  'certfile',
-  'email',
-  'keyfile',
-  'password',
-  'username',
-]
+const { isProtected } = require('../utils/protected-config.js')
 
 // take an array of `[key, value, k2=v2, k3, v3, ...]` and turn into { key: value, k2: v2, k3: v3 }
 const keyValues = args => {
@@ -38,29 +24,6 @@ const keyValues = args => {
   return kv
 }
 
-const isProtected = (k) => {
-  // _password
-  if (k.startsWith('_')) {
-    return true
-  }
-  if (protected.includes(k)) {
-    return true
-  }
-  // //localhost:8080/:_password
-  if (k.startsWith('//')) {
-    if (k.includes(':_')) {
-      return true
-    }
-    // //registry:_authToken or //registry:authToken
-    for (const p of protected) {
-      if (k.endsWith(`:${p}`) || k.endsWith(`:_${p}`)) {
-        return true
-      }
-    }
-  }
-  return false
-}
-
 // Private fields are either protected or they can redacted info
 const isPrivate = (k, v) => isProtected(k) || redact(v) !== v
 

lib/npm.js

@@ -11,6 +11,35 @@ const { redactLog: replaceInfo } = require('@npmcli/redact')
 const pkg = require('../package.json')
 const { deref } = require('./utils/cmd-list.js')
 const { jsonError, outputError } = require('./utils/output-error.js')
+const { isProtected } = require('./utils/protected-config.js')
+
+const REDACTED = '***'
+
+const cleanArgv = (argv) => {
+  const cleaned = replaceInfo(argv)
+  for (let i = 0; i < cleaned.length; i++) {
+    const arg = cleaned[i]
+    if (typeof arg !== 'string' || !arg.startsWith('--')) {
+      continue
+    }
+
+    const raw = arg.slice(2)
+    const equalsIndex = raw.indexOf('=')
+    const key = equalsIndex === -1 ? raw : raw.slice(0, equalsIndex)
+    if (!isProtected(key)) {
+      continue
+    }
+
+    if (equalsIndex === -1) {
+      if (i + 1 < cleaned.length) {
+        cleaned[i + 1] = REDACTED
+      }
+    } else {
+      cleaned[i] = `${arg.slice(0, equalsIndex + 3)}${REDACTED}`
+    }
+  }
+  return cleaned
+}
 
 class Npm {
   static get version () {
@@ -142,8 +171,7 @@ class Npm {
       process.title = this.#title
       // The cooked argv is also logged separately for debugging purposes.
       // It is cleaned as a best effort by replacing known secrets like basic auth password and strings that look like npm tokens.
-      // XXX: for this to be safer the config should create a sanitized version of the argv as it has the full context of what each option contains.
-      this.#argvClean = replaceInfo(cooked)
+      this.#argvClean = cleanArgv(cooked)
       log.verbose('title', this.title)
       log.verbose('argv', this.#argvClean.map(JSON.stringify).join(' '))
     })

lib/utils/protected-config.js

@@ -0,0 +1,39 @@
+// These are config values whose contents should not be displayed.
+// This is intentionally broader than nerfDarts because users may have old or
+// invalid auth entries that still need to be protected when npm reports config.
+const protected = [
+  'auth',
+  'authToken',
+  'certfile',
+  'email',
+  'keyfile',
+  'password',
+  'username',
+]
+
+const isProtected = (key) => {
+  // _password
+  if (key.startsWith('_')) {
+    return true
+  }
+  if (protected.includes(key)) {
+    return true
+  }
+  // //localhost:8080/:_password
+  if (key.startsWith('//')) {
+    if (key.includes(':_')) {
+      return true
+    }
+    // //registry:authToken or //registry:_authToken
+    for (const protectedKey of protected) {
+      if (key.endsWith(`:${protectedKey}`) || key.endsWith(`:_${protectedKey}`)) {
+        return true
+      }
+    }
+  }
+  return false
+}
+
+module.exports = {
+  isProtected,
+}

test/lib/cli/entry.js

@@ -105,6 +105,28 @@ t.test('logged argv is sanitized with equals', async t => {
   t.match(logs.verbose.byTitle('argv'), ['argv "version" "--registry" "https://u:***@npmjs.org"'])
 })
 
+t.test('logged argv redacts protected config values', async t => {
+  const { logs, cli } = await cliMock(t, {
+    globals: {
+      'process.argv': [
+        'node',
+        'npm',
+        'version',
+        '--//registry.npmjs.org/:_authToken=plain-secret',
+        '--_password',
+        'hunter2',
+      ],
+    },
+  })
+  await cli(process)
+
+  const argv = logs.verbose.byTitle('argv')[0]
+  t.notMatch(argv, 'plain-secret')
+  t.notMatch(argv, 'hunter2')
+  t.match(argv, '"--//registry.npmjs.org/:_authToken" "***"')
+  t.match(argv, '"--_password" "***"')
+})
+
 t.test('print usage if no params provided', async t => {
   const { cli, outputs, exitHandlerCalled, exitHandlerNpm } = await cliMock(t, {
     globals: {