From badeac28faf9fde5f8c05d235219be840999a646 Mon Sep 17 00:00:00 2001
From: Luke Karrys <luke@lukekarrys.com>
Date: Tue, 14 May 2024 13:49:23 -0700
Subject: [PATCH] fix(config): use redact on config output (#7521)

Fixes #3867
---
 lib/commands/config.js      | 50 +++++++++++++++++++++----------------
 test/lib/commands/config.js | 19 ++++++++++++++
 2 files changed, 48 insertions(+), 21 deletions(-)

diff --git a/lib/commands/config.js b/lib/commands/config.js
index 2f1f11e762a73..e8da195204c00 100644
--- a/lib/commands/config.js
+++ b/lib/commands/config.js
@@ -7,6 +7,7 @@ const pkgJson = require('@npmcli/package-json')
 const { defaults, definitions } = require('@npmcli/config/lib/definitions')
 const { log, output } = require('proc-log')
 const BaseCommand = require('../base-cmd.js')
+const { redact } = require('@npmcli/redact')
 
 // 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.
@@ -53,29 +54,35 @@ const keyValues = args => {
   return kv
 }
 
-const publicVar = k => {
+const isProtected = (k) => {
   // _password
   if (k.startsWith('_')) {
-    return false
+    return true
   }
   if (protected.includes(k)) {
-    return false
+    return true
   }
   // //localhost:8080/:_password
   if (k.startsWith('//')) {
     if (k.includes(':_')) {
-      return false
+      return true
     }
     // //registry:_authToken or //registry:authToken
     for (const p of protected) {
       if (k.endsWith(`:${p}`) || k.endsWith(`:_${p}`)) {
-        return false
+        return true
       }
     }
   }
-  return true
+  return false
 }
 
+// Private fields are either protected or they can redacted info
+const isPrivate = (k, v) => isProtected(k) || redact(v) !== v
+
+const displayVar = (k, v) =>
+  `${k} = ${isProtected(k, v) ? '(protected)' : JSON.stringify(redact(v))}`
+
 class Config extends BaseCommand {
   static description = 'Manage the npm configuration files'
   static name = 'config'
@@ -206,12 +213,13 @@ class Config extends BaseCommand {
 
     const out = []
     for (const key of keys) {
-      if (!publicVar(key)) {
+      const val = this.npm.config.get(key)
+      if (isPrivate(key, val)) {
         throw new Error(`The ${key} option is protected, and can not be retrieved in this way`)
       }
 
       const pref = keys.length > 1 ? `${key}=` : ''
-      out.push(pref + this.npm.config.get(key))
+      out.push(pref + val)
     }
     output.standard(out.join('\n'))
   }
@@ -338,18 +346,17 @@ ${defData}
         continue
       }
 
-      const keys = Object.keys(data).sort(localeCompare)
-      if (!keys.length) {
+      const entries = Object.entries(data).sort(([a], [b]) => localeCompare(a, b))
+      if (!entries.length) {
         continue
       }
 
       msg.push(`; "${where}" config from ${source}`, '')
-      for (const k of keys) {
-        const v = publicVar(k) ? JSON.stringify(data[k]) : '(protected)'
+      for (const [k, v] of entries) {
+        const display = displayVar(k, v)
         const src = this.npm.config.find(k)
-        const overridden = src !== where
-        msg.push((overridden ? '; ' : '') +
-          `${k} = ${v}${overridden ? ` ; overridden by ${src}` : ''}`)
+        msg.push(src === where ? display : `; ${display} ; overridden by ${src}`)
+        msg.push()
       }
       msg.push('')
     }
@@ -374,10 +381,10 @@ ${defData}
         const pkgPath = resolve(this.npm.prefix, 'package.json')
         msg.push(`; "publishConfig" from ${pkgPath}`)
         msg.push('; This set of config values will be used at publish-time.', '')
-        const pkgKeys = Object.keys(content.publishConfig).sort(localeCompare)
-        for (const k of pkgKeys) {
-          const v = publicVar(k) ? JSON.stringify(content.publishConfig[k]) : '(protected)'
-          msg.push(`${k} = ${v}`)
+        const entries = Object.entries(content.publishConfig)
+          .sort(([a], [b]) => localeCompare(a, b))
+        for (const [k, value] of entries) {
+          msg.push(displayVar(k, value))
         }
         msg.push('')
       }
@@ -389,11 +396,12 @@ ${defData}
   async listJson () {
     const publicConf = {}
     for (const key in this.npm.config.list[0]) {
-      if (!publicVar(key)) {
+      const value = this.npm.config.get(key)
+      if (isPrivate(key, value)) {
         continue
       }
 
-      publicConf[key] = this.npm.config.get(key)
+      publicConf[key] = value
     }
     output.standard(JSON.stringify(publicConf, null, 2))
   }
diff --git a/test/lib/commands/config.js b/test/lib/commands/config.js
index 563bb97dc1612..1a3644a07623b 100644
--- a/test/lib/commands/config.js
+++ b/test/lib/commands/config.js
@@ -505,6 +505,25 @@ t.test('config get private key', async t => {
   )
 })
 
+t.test('config redacted values', async t => {
+  const { npm, joinedOutput, clearOutput } = await loadMockNpm(t)
+
+  await npm.exec('config', ['set', 'proxy', 'https://proxy.npmjs.org/'])
+  await npm.exec('config', ['get', 'proxy'])
+
+  t.equal(joinedOutput(), 'https://proxy.npmjs.org/')
+  clearOutput()
+
+  await npm.exec('config', ['set', 'proxy', 'https://u:password@proxy.npmjs.org/'])
+
+  await t.rejects(npm.exec('config', ['get', 'proxy']), /proxy option is protected/)
+
+  await npm.exec('config', ['ls'])
+
+  t.match(joinedOutput(), 'proxy = "https://u:***@proxy.npmjs.org/"')
+  clearOutput()
+})
+
 t.test('config edit', async t => {
   const EDITOR = 'vim'
   const editor = spawk.spawn(EDITOR).exit(0)