Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Set process.title a bit more usefully #2154

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions lib/cli.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
// Separated out for easier unit testing
module.exports = (process) => {
// set it here so that regardless of what happens later, we don't
// leak any private CLI configs to other programs
process.title = 'npm'

const {
Expand Down
20 changes: 20 additions & 0 deletions lib/npm.js
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ const _runCmd = Symbol('_runCmd')
const _load = Symbol('_load')
const _flatOptions = Symbol('_flatOptions')
const _tmpFolder = Symbol('_tmpFolder')
const _title = Symbol('_title')
const npm = module.exports = new class extends EventEmitter {
constructor () {
super()
Expand All @@ -75,6 +76,7 @@ const npm = module.exports = new class extends EventEmitter {
defaults,
shorthands,
})
this[_title] = process.title
this.updateNotification = null
}

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

get title () {
return this[_title]
}

set title (t) {
process.title = t
this[_title] = t
}

async [_load] () {
const node = await which(process.argv[0]).catch(er => null)
if (node && node.toUpperCase() !== process.execPath.toUpperCase()) {
Expand All @@ -166,6 +177,15 @@ const npm = module.exports = new class extends EventEmitter {

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

this.color = setupLog(this.config, this)
process.env.COLOR = this.color ? '1' : '0'
Expand Down
93 changes: 92 additions & 1 deletion test/lib/npm.js
Original file line number Diff line number Diff line change
Expand Up @@ -256,7 +256,10 @@ t.test('npm.load', t => {
'--prefix', dir,
'--userconfig', `${dir}/.npmrc`,
'--usage',
'--scope=foo'
'--scope=foo',
'token',
'revoke',
'blergggg',
]

freshConfig()
Expand Down Expand Up @@ -353,3 +356,91 @@ t.test('loading as main will load the cli', t => {
t.end()
})
})

t.test('set process.title', t => {
const { execPath, argv: processArgv } = process
const { log } = console
const titleDesc = Object.getOwnPropertyDescriptor(process, 'title')
Object.defineProperty(process, 'title', {
value: '',
settable: true,
enumerable: true,
configurable: true,
})
const consoleLogs = []
console.log = (...msg) => consoleLogs.push(msg)

t.teardown(() => {
console.log = log
process.argv = processArgv
Object.defineProperty(process, 'title', titleDesc)
freshConfig()
})

t.afterEach(cb => {
consoleLogs.length = 0
cb()
})

t.test('basic title setting', async t => {
freshConfig({
argv: [
process.execPath,
process.argv[1],
'--metrics-registry', 'http://example.com',
'--usage',
'--scope=foo',
'ls',
],
})
await npm.load(er => {
if (er)
throw er
t.equal(npm.title, 'npm ls')
t.equal(process.title, 'npm ls')
})
})

t.test('do not expose token being revoked', async t => {
freshConfig({
argv: [
process.execPath,
process.argv[1],
'--metrics-registry', 'http://example.com',
'--usage',
'--scope=foo',
'token',
'revoke',
'deadbeefcafebad',
],
})
await npm.load(er => {
if (er)
throw er
t.equal(npm.title, 'npm token revoke ***')
t.equal(process.title, 'npm token revoke ***')
})
})

t.test('do show *** unless a token is actually being revoked', async t => {
freshConfig({
argv: [
process.execPath,
process.argv[1],
'--metrics-registry', 'http://example.com',
'--usage',
'--scope=foo',
'token',
'revoke',
],
})
await npm.load(er => {
if (er)
throw er
t.equal(npm.title, 'npm token revoke')
t.equal(process.title, 'npm token revoke')
})
})

t.end()
})