From 6739299ea2a1d9ba3402ddc36fdfde2daeb85c57 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. --- 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 9544f8451f8ae..e33ac91fa5ade 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 3b590f712e783..0782c6979ca4b 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 5b6e3c85ab112..e8f817cd15f00 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 b85c981cd008b..2c0b6c0ba727e 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'],