From 099c22a403da4a65b234a6a80158fd9d5aaa7e75 Mon Sep 17 00:00:00 2001 From: AI-DEV-BOT Date: Fri, 29 May 2026 21:40:03 +0900 Subject: [PATCH 1/2] Redact protected config values in argv logs --- lib/commands/config.js | 39 +------------- lib/npm.js | 4 +- lib/utils/clean-argv.js | 35 ++++++++++++ lib/utils/error-message.js | 3 +- lib/utils/protected-config.js | 39 ++++++++++++++ .../test/lib/utils/error-message.js.test.cjs | 2 + test/lib/cli/entry.js | 25 +++++++++ test/lib/utils/clean-argv.js | 53 +++++++++++++++++++ test/lib/utils/error-message.js | 10 +++- 9 files changed, 168 insertions(+), 42 deletions(-) create mode 100644 lib/utils/clean-argv.js create mode 100644 lib/utils/protected-config.js create mode 100644 test/lib/utils/clean-argv.js diff --git a/lib/commands/config.js b/lib/commands/config.js index 0a8b84aba2666..b8eb01fdadbe6 100644 --- a/lib/commands/config.js +++ b/lib/commands/config.js @@ -8,21 +8,7 @@ const { defaults, definitions, nerfDarts, proxyEnv } = require('@npmcli/config/l const { log, output, input } = require('proc-log') const BaseCommand = require('../base-cmd.js') const { redact } = require('@npmcli/redact') - -// These are the config values to swap with "protected". -// It does not catch every single sensitive thing a user may put in the npmrc file but it gets the common ones. -// This is distinct from nerfDarts because that is used to validate valid configs during "npm config set", and folks may have old invalid entries lying around in a config file that we still want to protect when running "npm config list" -// This is a more general list of values to consider protected. -// You cannot "npm config get" them, and they will not display during "npm config list" -const protected = [ - 'auth', - 'authToken', - 'certfile', - 'email', - 'keyfile', - 'password', - 'username', -] +const { isProtected } = require('../utils/protected-config.js') // take an array of `[key, value, k2=v2, k3, v3, ...]` and turn into { key: value, k2: v2, k3: v3 } const keyValues = args => { @@ -38,29 +24,6 @@ const keyValues = args => { return kv } -const isProtected = (k) => { - // _password - if (k.startsWith('_')) { - return true - } - if (protected.includes(k)) { - return true - } - // //localhost:8080/:_password - if (k.startsWith('//')) { - if (k.includes(':_')) { - return true - } - // //registry:_authToken or //registry:authToken - for (const p of protected) { - if (k.endsWith(`:${p}`) || k.endsWith(`:_${p}`)) { - return true - } - } - } - return false -} - // Private fields are either protected or they can redacted info const isPrivate = (k, v) => isProtected(k) || redact(v) !== v diff --git a/lib/npm.js b/lib/npm.js index b7317970771d1..13b28c29badef 100644 --- a/lib/npm.js +++ b/lib/npm.js @@ -10,6 +10,7 @@ const { log, time, output, META } = require('proc-log') const { redactLog: replaceInfo } = require('@npmcli/redact') const pkg = require('../package.json') const { deref } = require('./utils/cmd-list.js') +const { cleanArgv } = require('./utils/clean-argv.js') const { jsonError, outputError } = require('./utils/output-error.js') class Npm { @@ -142,8 +143,7 @@ class Npm { process.title = this.#title // The cooked argv is also logged separately for debugging purposes. // It is cleaned as a best effort by replacing known secrets like basic auth password and strings that look like npm tokens. - // XXX: for this to be safer the config should create a sanitized version of the argv as it has the full context of what each option contains. - this.#argvClean = replaceInfo(cooked) + this.#argvClean = cleanArgv(cooked) log.verbose('title', this.title) log.verbose('argv', this.#argvClean.map(JSON.stringify).join(' ')) }) diff --git a/lib/utils/clean-argv.js b/lib/utils/clean-argv.js new file mode 100644 index 0000000000000..f4cbb6d06adc4 --- /dev/null +++ b/lib/utils/clean-argv.js @@ -0,0 +1,35 @@ +const { redactLog: replaceInfo } = require('@npmcli/redact') +const { isProtected } = require('./protected-config.js') + +const REDACTED = '***' +const isProtectedArg = key => isProtected(key) || key === 'otp' + +const cleanArgv = (argv) => { + const cleaned = replaceInfo(argv) + for (let i = 0; i < cleaned.length; i++) { + const arg = cleaned[i] + if (typeof arg !== 'string' || !arg.startsWith('--')) { + continue + } + + const raw = arg.slice(2) + const equalsIndex = raw.indexOf('=') + const key = equalsIndex === -1 ? raw : raw.slice(0, equalsIndex) + if (!isProtectedArg(key)) { + continue + } + + if (equalsIndex === -1) { + if (i + 1 < cleaned.length) { + cleaned[i + 1] = REDACTED + } + } else { + cleaned[i] = `${arg.slice(0, equalsIndex + 3)}${REDACTED}` + } + } + return cleaned +} + +module.exports = { + cleanArgv, +} diff --git a/lib/utils/error-message.js b/lib/utils/error-message.js index f32d5764d5edb..2b9c8450bf844 100644 --- a/lib/utils/error-message.js +++ b/lib/utils/error-message.js @@ -1,6 +1,7 @@ const { format } = require('node:util') const { resolve } = require('node:path') const { redactLog: replaceInfo } = require('@npmcli/redact') +const { cleanArgv } = require('./clean-argv.js') const { log } = require('proc-log') const errorMessage = (er, npm) => { @@ -352,7 +353,7 @@ const errorMessage = (er, npm) => { detail.push(['signal', er.signal]) } if (er.cmd && Array.isArray(er.args)) { - detail.push(['command', ...[er.cmd, ...er.args.map(replaceInfo)]]) + detail.push(['command', ...[er.cmd, ...cleanArgv(er.args)]]) } if (er.stdout) { detail.push(['', er.stdout.trim()]) diff --git a/lib/utils/protected-config.js b/lib/utils/protected-config.js new file mode 100644 index 0000000000000..c63350435658d --- /dev/null +++ b/lib/utils/protected-config.js @@ -0,0 +1,39 @@ +// These are config values whose contents should not be displayed. +// This is intentionally broader than nerfDarts because users may have old or +// invalid auth entries that still need to be protected when npm reports config. +const protected = [ + 'auth', + 'authToken', + 'certfile', + 'email', + 'keyfile', + 'password', + 'username', +] + +const isProtected = (key) => { + // _password + if (key.startsWith('_')) { + return true + } + if (protected.includes(key)) { + return true + } + // //localhost:8080/:_password + if (key.startsWith('//')) { + if (key.includes(':_')) { + return true + } + // //registry:authToken or //registry:_authToken + for (const protectedKey of protected) { + if (key.endsWith(`:${protectedKey}`) || key.endsWith(`:_${protectedKey}`)) { + return true + } + } + } + return false +} + +module.exports = { + isProtected, +} 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 a63412c96ea4a..38ad27de2fa86 100644 --- a/tap-snapshots/test/lib/utils/error-message.js.test.cjs +++ b/tap-snapshots/test/lib/utils/error-message.js.test.cjs @@ -195,6 +195,8 @@ Object { "g", "s", "https://evil:***@npmjs.org", + "--//registry.npmjs.org/:_authToken", + "***", ], Array [ "", diff --git a/test/lib/cli/entry.js b/test/lib/cli/entry.js index 369927dace0f2..a7253e66537fb 100644 --- a/test/lib/cli/entry.js +++ b/test/lib/cli/entry.js @@ -105,6 +105,31 @@ t.test('logged argv is sanitized with equals', async t => { t.match(logs.verbose.byTitle('argv'), ['argv "version" "--registry" "https://u:***@npmjs.org"']) }) +t.test('logged argv redacts protected config values', async t => { + const { logs, cli } = await cliMock(t, { + globals: { + 'process.argv': [ + 'node', + 'npm', + 'version', + '--//registry.npmjs.org/:_authToken=plain-secret', + '--_password', + 'hunter2', + '--otp=123456', + ], + }, + }) + await cli(process) + + const argv = logs.verbose.byTitle('argv')[0] + t.notMatch(argv, 'plain-secret') + t.notMatch(argv, 'hunter2') + t.notMatch(argv, '123456') + t.match(argv, '"--//registry.npmjs.org/:_authToken" "***"') + t.match(argv, '"--_password" "***"') + t.match(argv, '"--otp" "***"') +}) + t.test('print usage if no params provided', async t => { const { cli, outputs, exitHandlerCalled, exitHandlerNpm } = await cliMock(t, { globals: { diff --git a/test/lib/utils/clean-argv.js b/test/lib/utils/clean-argv.js new file mode 100644 index 0000000000000..83b2683a0bf56 --- /dev/null +++ b/test/lib/utils/clean-argv.js @@ -0,0 +1,53 @@ +const t = require('tap') +const { cleanArgv } = require('../../../lib/utils/clean-argv.js') + +t.test('redacts protected config values', t => { + t.strictSame(cleanArgv([ + '--password=secret', + '--otp', + '123456', + '--//registry.npmjs.org/:_authToken', + 'plain-secret', + '--registry', + 'https://user:pass@registry.npmjs.org', + ]), [ + '--password=***', + '--otp', + '***', + '--//registry.npmjs.org/:_authToken', + '***', + '--registry', + 'https://user:***@registry.npmjs.org', + ]) + t.end() +}) + +t.test('redacts protected config values with equals', t => { + t.strictSame(cleanArgv([ + '--otp=123456', + '--_authToken=plain-secret', + '--//registry.npmjs.org/:_password=hunter2', + ]), [ + '--otp=***', + '--_authToken=***', + '--//registry.npmjs.org/:_password=***', + ]) + t.end() +}) + +t.test('leaves non-protected config values intact', t => { + t.strictSame(cleanArgv([ + '--registry', + 'https://registry.npmjs.org', + '--auth-type', + 'legacy', + '--scope=@npmcli', + ]), [ + '--registry', + 'https://registry.npmjs.org', + '--auth-type', + 'legacy', + '--scope=@npmcli', + ]) + t.end() +}) diff --git a/test/lib/utils/error-message.js b/test/lib/utils/error-message.js index 44a57eca645d7..3bcf36bf5c9ac 100644 --- a/test/lib/utils/error-message.js +++ b/test/lib/utils/error-message.js @@ -167,7 +167,15 @@ t.test('args are cleaned', async t => { t.matchSnapshot(errorMessage(Object.assign(new Error('cmd err'), { cmd: 'some command', signal: 'SIGYOLO', - args: ['a', 'r', 'g', 's', 'https://evil:password@npmjs.org'], + args: [ + 'a', + 'r', + 'g', + 's', + 'https://evil:password@npmjs.org', + '--//registry.npmjs.org/:_authToken', + 'plain-secret', + ], stdout: 'stdout', stderr: 'stderr', }))) From 9b5a8a9683d224a7391de7444d5e0c5ecb95fc16 Mon Sep 17 00:00:00 2001 From: AI-DEV-BOT Date: Thu, 4 Jun 2026 22:46:28 +0900 Subject: [PATCH 2/2] Revert spawn error args to log redaction --- lib/utils/error-message.js | 3 +-- .../test/lib/utils/error-message.js.test.cjs | 2 +- test/lib/utils/error-message.js | 22 +++++++++++++++++++ 3 files changed, 24 insertions(+), 3 deletions(-) diff --git a/lib/utils/error-message.js b/lib/utils/error-message.js index 2b9c8450bf844..f32d5764d5edb 100644 --- a/lib/utils/error-message.js +++ b/lib/utils/error-message.js @@ -1,7 +1,6 @@ const { format } = require('node:util') const { resolve } = require('node:path') const { redactLog: replaceInfo } = require('@npmcli/redact') -const { cleanArgv } = require('./clean-argv.js') const { log } = require('proc-log') const errorMessage = (er, npm) => { @@ -353,7 +352,7 @@ const errorMessage = (er, npm) => { detail.push(['signal', er.signal]) } if (er.cmd && Array.isArray(er.args)) { - detail.push(['command', ...[er.cmd, ...cleanArgv(er.args)]]) + detail.push(['command', ...[er.cmd, ...er.args.map(replaceInfo)]]) } if (er.stdout) { detail.push(['', er.stdout.trim()]) 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 38ad27de2fa86..1c28da0d935ff 100644 --- a/tap-snapshots/test/lib/utils/error-message.js.test.cjs +++ b/tap-snapshots/test/lib/utils/error-message.js.test.cjs @@ -196,7 +196,7 @@ Object { "s", "https://evil:***@npmjs.org", "--//registry.npmjs.org/:_authToken", - "***", + "plain-secret", ], Array [ "", diff --git a/test/lib/utils/error-message.js b/test/lib/utils/error-message.js index 3bcf36bf5c9ac..0eaf766afb7aa 100644 --- a/test/lib/utils/error-message.js +++ b/test/lib/utils/error-message.js @@ -181,6 +181,28 @@ t.test('args are cleaned', async t => { }))) }) +t.test('child process args keep protected-looking flags', async t => { + const { errorMessage } = await loadMockNpm(t) + const msg = errorMessage(Object.assign(new Error('cmd err'), { + cmd: 'some command', + args: [ + '--password', + 'hunter2', + '--registry', + 'https://evil:password@npmjs.org', + ], + })) + + t.strictSame(msg.detail, [[ + 'command', + 'some command', + '--password', + 'hunter2', + '--registry', + 'https://evil:***@npmjs.org', + ]]) +}) + t.test('eacces/eperm', async t => { const runTest = (windows, loaded, cachePath, cacheDest) => async t => { const { errorMessage, logs, cache } = await loadMockNpm(t, {