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

add process title to npm cli #1927

Closed
wants to merge 3 commits into from
Closed

add process title to npm cli #1927

wants to merge 3 commits into from

Conversation

unphased
Copy link

@unphased unphased commented Oct 8, 2020

See
https://npm.community/t/why-does-the-cli-override-process-title/8400
tmux-plugins/tmux-resurrect#201

Code from http://broken-by.me/node-js-change-visible-process-name

Not a huge number of tmux resurrect users out there, but there are many reasons for setting the process title. For example, it would make it easier to identify an npm process inside top, htop, ps. We would simply like to give npm the ability to set the title.

This just adds simple code to report the npm command being run in the process title.

I am expecting this to be closed because I suspect that the reason why the shell command that is being run is not shown in the title is done for security reasons (in case an npm command is constructed with passwords right in the command).

I think that even if this is grounds for rejection, we could at least have a discussion about whether we could add a flag to the CLI to allow the title to be manually specified or maybe a flag that can be configured to let it run this code?

Basically this will allow npm cli to report itself as npm start or npm run lint etc. rather than just always npm.

Thanks

@unphased unphased requested a review from a team as a code owner October 8, 2020 18:13
Steven Lu (1950X) added 2 commits October 8, 2020 14:17
@darcyclarke darcyclarke added Release 7.x work is associated with a specific npm 7 release Needs Review labels Oct 9, 2020
@isaacs
Copy link
Contributor

isaacs commented Oct 9, 2020

Yes, the security concerns you bring up are exactly my objection, so good job predicting that ;)

But I think we can and should do something like this. We could maybe do something more useful by setting process.title to something like: process.title = ['npm', npm.command, ...npm.argv], so we just show positional args instead of all arguments to the process.

Also, should be done in v7, not on v6, since we're so close to v7 GA.

@darcyclarke darcyclarke added the Release 6.x work is associated with a specific npm 6 release label Nov 10, 2020
isaacs added a commit that referenced this pull request Nov 10, 2020
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
@isaacs
Copy link
Contributor

isaacs commented Nov 10, 2020

Will be addressed in npm v7 by #2154

darcyclarke pushed a commit that referenced this pull request Nov 13, 2020
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

Credit: @isaacs
Close: #2154
Reviewed-by: @ruyadorno
@isaacs isaacs closed this in fd1d7a2 Nov 13, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Release 6.x work is associated with a specific npm 6 release Release 7.x work is associated with a specific npm 7 release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants