fix(logging): sanitize logged argv

wraithgar · wraithgar · commit 487731cd56a2 · 2021-08-17T09:02:54.000-07:00
Wraps logged process.argv in `replaceInfo` Removes logged process.argv from EJSONPARSE warning for top level package.json merge conflicts. It is currently not even working (er.file is not being populated by the parsing library right now), and process.argv contains fullly resolved paths which isn't very nice looking. The user knows what they typed, it's enough to tell them to retry. PR-URL: #3658 Credit: @wraithgar Close: #3658 Reviewed-by: @nlf
diff --git a/lib/cli.js b/lib/cli.js
@@ -27,7 +27,8 @@ module.exports = async (process) => {
   if (process.argv[1][process.argv[1].length - 1] === 'g')
     process.argv.splice(1, 1, 'npm', '-g')
 
-  log.verbose('cli', process.argv)
+  const replaceInfo = require('../lib/utils/replace-info.js')
+  log.verbose('cli', replaceInfo(process.argv))
 
   log.info('using', 'npm@%s', npm.version)
   log.info('using', 'node@%s', process.version)
diff --git a/lib/utils/error-message.js b/lib/utils/error-message.js
@@ -109,9 +109,7 @@ module.exports = (er, npm) => {
             [
               'Merge conflict detected in your package.json.',
               '',
-              'Please resolve the package.json conflict and retry the command:',
-              '',
-              `$ ${process.argv.join(' ')}`,
+              'Please resolve the package.json conflict and retry.',
             ].join('\n'),
           ])
           break
diff --git a/tap-snapshots/test/lib/utils/error-message.js.test.cjs b/tap-snapshots/test/lib/utils/error-message.js.test.cjs
@@ -1537,9 +1537,7 @@ Object {
       String(
         Merge conflict detected in your package.json.
         
-        Please resolve the package.json conflict and retry the command:
-        
-        $ arg v
+        Please resolve the package.json conflict and retry.
       ),
     ],
   ],
diff --git a/test/lib/cli.js b/test/lib/cli.js
@@ -104,6 +104,32 @@ t.test('calling with --versions calls npm version with no args', async t => {
   t.strictSame(exitHandlerCalled, [])
 })
 
+t.test('logged argv is sanitized', async t => {
+  const proc = processMock({
+    argv: ['node', 'npm', 'testcommand', 'https://username:password@npmjs.org/test_url_with_a_password'],
+  })
+  const { npm } = mockNpm(t)
+  const cli = cliMock(npm)
+
+  npm.commands.testcommand = (args, cb) => {
+    cb()
+  }
+
+  await cli(proc)
+  t.equal(proc.title, 'npm')
+  t.strictSame(logs, [
+    'pause',
+    ['verbose', 'cli', [
+      'node',
+      'npm',
+      'testcommand',
+      'https://username:***@npmjs.org/test_url_with_a_password',
+    ]],
+    ['info', 'using', 'npm@%s', npm.version],
+    ['info', 'using', 'node@%s', process.version],
+  ])
+})
+
 t.test('print usage if no params provided', async t => {
   const proc = processMock({
     argv: ['node', 'npm'],
