Skip to content

Commit 59fd059

Browse files
committed
Set process.title a bit more usefully
This includes all positional args (ie, not anything that is a config), except for the third positional arg when running `npm token revoke ...`, since that is also a secret. Makes the `ps` entry a bit more useful than just "npm", so users can at least get some clue what npm is doing, while minimizing the degree to which we leak anything secret. Closes: #1927 Fixes: https://npm.community/t/8400 Related: tmux-plugins/tmux-resurrect#201
1 parent a28aff7 commit 59fd059

File tree

3 files changed

+114
-1
lines changed

3 files changed

+114
-1
lines changed

lib/cli.js

+2
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
// Separated out for easier unit testing
22
module.exports = (process) => {
3+
// set it here so that regardless of what happens later, we don't
4+
// leak any private CLI configs to other programs
35
process.title = 'npm'
46

57
const {

lib/npm.js

+20
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,7 @@ const _runCmd = Symbol('_runCmd')
5454
const _load = Symbol('_load')
5555
const _flatOptions = Symbol('_flatOptions')
5656
const _tmpFolder = Symbol('_tmpFolder')
57+
const _title = Symbol('_title')
5758
const npm = module.exports = new class extends EventEmitter {
5859
constructor () {
5960
super()
@@ -75,6 +76,7 @@ const npm = module.exports = new class extends EventEmitter {
7576
defaults,
7677
shorthands,
7778
})
79+
this[_title] = process.title
7880
this.updateNotification = null
7981
}
8082

@@ -156,6 +158,15 @@ const npm = module.exports = new class extends EventEmitter {
156158
return this.config.loaded
157159
}
158160

161+
get title () {
162+
return this[_title]
163+
}
164+
165+
set title (t) {
166+
process.title = t
167+
this[_title] = t
168+
}
169+
159170
async [_load] () {
160171
const node = await which(process.argv[0]).catch(er => null)
161172
if (node && node.toUpperCase() !== process.execPath.toUpperCase()) {
@@ -166,6 +177,15 @@ const npm = module.exports = new class extends EventEmitter {
166177

167178
await this.config.load()
168179
this.argv = this.config.parsedArgv.remain
180+
// note: this MUST be shorter than the actual argv length, because it
181+
// uses the same memory, so node will truncate it if it's too long.
182+
// if it's a token revocation, then the argv contains a secret, so
183+
// don't show that. (Regrettable historical choice to put it there.)
184+
// Any other secrets are configs only, so showing only the positional
185+
// args keeps those from being leaked.
186+
const tokrev = deref(this.argv[0]) === 'token' && this.argv[1] === 'revoke'
187+
this.title = tokrev ? 'npm token revoke' + (this.argv[2] ? ' ***' : '')
188+
: ['npm', ...this.argv].join(' ')
169189

170190
this.color = setupLog(this.config, this)
171191
process.env.COLOR = this.color ? '1' : '0'

test/lib/npm.js

+92-1
Original file line numberDiff line numberDiff line change
@@ -256,7 +256,10 @@ t.test('npm.load', t => {
256256
'--prefix', dir,
257257
'--userconfig', `${dir}/.npmrc`,
258258
'--usage',
259-
'--scope=foo'
259+
'--scope=foo',
260+
'token',
261+
'revoke',
262+
'blergggg',
260263
]
261264

262265
freshConfig()
@@ -353,3 +356,91 @@ t.test('loading as main will load the cli', t => {
353356
t.end()
354357
})
355358
})
359+
360+
t.test('set process.title', t => {
361+
const { execPath, argv: processArgv } = process
362+
const { log } = console
363+
const titleDesc = Object.getOwnPropertyDescriptor(process, 'title')
364+
Object.defineProperty(process, 'title', {
365+
value: '',
366+
settable: true,
367+
enumerable: true,
368+
configurable: true,
369+
})
370+
const consoleLogs = []
371+
console.log = (...msg) => consoleLogs.push(msg)
372+
373+
t.teardown(() => {
374+
console.log = log
375+
process.argv = processArgv
376+
Object.defineProperty(process, 'title', titleDesc)
377+
freshConfig()
378+
})
379+
380+
t.afterEach(cb => {
381+
consoleLogs.length = 0
382+
cb()
383+
})
384+
385+
t.test('basic title setting', async t => {
386+
freshConfig({
387+
argv: [
388+
process.execPath,
389+
process.argv[1],
390+
'--metrics-registry', 'http://example.com',
391+
'--usage',
392+
'--scope=foo',
393+
'ls',
394+
],
395+
})
396+
await npm.load(er => {
397+
if (er)
398+
throw er
399+
t.equal(npm.title, 'npm ls')
400+
t.equal(process.title, 'npm ls')
401+
})
402+
})
403+
404+
t.test('do not expose token being revoked', async t => {
405+
freshConfig({
406+
argv: [
407+
process.execPath,
408+
process.argv[1],
409+
'--metrics-registry', 'http://example.com',
410+
'--usage',
411+
'--scope=foo',
412+
'token',
413+
'revoke',
414+
'deadbeefcafebad',
415+
],
416+
})
417+
await npm.load(er => {
418+
if (er)
419+
throw er
420+
t.equal(npm.title, 'npm token revoke ***')
421+
t.equal(process.title, 'npm token revoke ***')
422+
})
423+
})
424+
425+
t.test('do show *** unless a token is actually being revoked', async t => {
426+
freshConfig({
427+
argv: [
428+
process.execPath,
429+
process.argv[1],
430+
'--metrics-registry', 'http://example.com',
431+
'--usage',
432+
'--scope=foo',
433+
'token',
434+
'revoke',
435+
],
436+
})
437+
await npm.load(er => {
438+
if (er)
439+
throw er
440+
t.equal(npm.title, 'npm token revoke')
441+
t.equal(process.title, 'npm token revoke')
442+
})
443+
})
444+
445+
t.end()
446+
})

0 commit comments

Comments
 (0)