From 487731cd56a22272c6ff72ef2fa7822368bf63e3 Mon Sep 17 00:00:00 2001 From: Gar Date: Tue, 17 Aug 2021 07:33:39 -0700 Subject: [PATCH] fix(logging): sanitize logged argv 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: https://github.com/npm/cli/pull/3658 Credit: @wraithgar Close: #3658 Reviewed-by: @nlf --- lib/cli.js | 3 ++- lib/utils/error-message.js | 4 +-- .../test/lib/utils/error-message.js.test.cjs | 4 +-- test/lib/cli.js | 26 +++++++++++++++++++ 4 files changed, 30 insertions(+), 7 deletions(-) diff --git a/lib/cli.js b/lib/cli.js index 9544f8451f8a..e33ac91fa5ad 100644 --- 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 index 3b590f712e78..0782c6979ca4 100644 --- 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 index 5b6e3c85ab11..e8f817cd15f0 100644 --- 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 index b85c981cd008..2c0b6c0ba727 100644 --- 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'],