From 1ad0938243110d983284e8763da41a57b561563d Mon Sep 17 00:00:00 2001 From: Gar Date: Thu, 9 Sep 2021 11:14:04 -0700 Subject: [PATCH] fix(error-message): clean urls from 404 error If the package being installed is a url it needs to be cleaned before logging so passwords aren't potentially logged. PR-URL: https://github.com/npm/cli/pull/3732 Credit: @wraithgar Close: #3732 Reviewed-by: @nlf --- lib/utils/error-message.js | 2 +- .../test/lib/utils/error-message.js.test.cjs | 48 +++++++++++++++++-- test/lib/utils/error-message.js | 8 ++++ 3 files changed, 54 insertions(+), 4 deletions(-) diff --git a/lib/utils/error-message.js b/lib/utils/error-message.js index da97195dd04f0..6e12bcb918eef 100644 --- a/lib/utils/error-message.js +++ b/lib/utils/error-message.js @@ -181,7 +181,7 @@ module.exports = (er, npm) => { const pkg = er.pkgid.replace(/(?!^)@.*$/, '') detail.push(['404', '']) - detail.push(['404', '', "'" + er.pkgid + "' is not in the npm registry."]) + detail.push(['404', '', `'${replaceInfo(er.pkgid)}' is not in this registry.`]) const valResult = nameValidator(pkg) 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 e8f817cd15f00..1f73361c48589 100644 --- a/tap-snapshots/test/lib/utils/error-message.js.test.cjs +++ b/tap-snapshots/test/lib/utils/error-message.js.test.cjs @@ -5,6 +5,48 @@ * Make sure to inspect the output below. Do not ignore changes! */ 'use strict' +exports[`test/lib/utils/error-message.js TAP 404 cleans sensitive info from package id > must match snapshot 1`] = ` +Object { + "detail": Array [ + Array [ + "404", + "", + ], + Array [ + "404", + "", + "'http://evil:***@npmjs.org/not-found' is not in this registry.", + ], + Array [ + "404", + "This package name is not valid, because", + "", + ], + Array [ + "404", + " 1. name can only contain URL-friendly characters", + ], + Array [ + "404", + String( + + Note that you can also install from a + ), + ], + Array [ + "404", + "tarball, folder, http url, or git url.", + ], + ], + "summary": Array [ + Array [ + "404", + "not found", + ], + ], +} +` + exports[`test/lib/utils/error-message.js TAP 404 name with error > must match snapshot 1`] = ` Object { "detail": Array [ @@ -15,7 +57,7 @@ Object { Array [ "404", "", - "'node_modules' is not in the npm registry.", + "'node_modules' is not in this registry.", ], Array [ "404", @@ -57,7 +99,7 @@ Object { Array [ "404", "", - "'xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx' is not in the npm registry.", + "'xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx' is not in this registry.", ], Array [ "404", @@ -111,7 +153,7 @@ Object { Array [ "404", "", - "'yolo' is not in the npm registry.", + "'yolo' is not in this registry.", ], Array [ "404", diff --git a/test/lib/utils/error-message.js b/test/lib/utils/error-message.js index 07328d588759b..d1c67a95137c4 100644 --- a/test/lib/utils/error-message.js +++ b/test/lib/utils/error-message.js @@ -423,6 +423,14 @@ t.test('404', t => { t.matchSnapshot(errorMessage(er, npm)) t.end() }) + t.test('cleans sensitive info from package id', t => { + const er = Object.assign(new Error('404 not found'), { + pkgid: 'http://evil:password@npmjs.org/not-found', + code: 'E404', + }) + t.matchSnapshot(errorMessage(er, npm)) + t.end() + }) t.end() })