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

Wrong user home set when generating pm2-init.sh #1888

Closed
binarykitchen opened this Issue Jan 14, 2016 · 24 comments

Comments

5 participants
@binarykitchen
Contributor

binarykitchen commented Jan 14, 2016

Just followed the instructions in http://pm2.keymetrics.io/docs/usage/startup/#user-permissions and ran this command $ pm2 startup ubuntu -u node

Then had a look at /etc/init.d/pm2-init.sh here are the first top lines of that fine:

NAME=pm2
PM2=/usr/lib/node_modules/pm2/bin/pm2
USER=nodejs
DEFAULT=/etc/default/$NAME

export PATH=/usr/bin:$PATH
export PM2_HOME="/home/michael.heuberger/.pm2"

USER is correct. But PM2_HOME is wrong. Should be /home/nodejs/.pm2

@estliberitas

This comment has been minimized.

estliberitas commented Jan 14, 2016

@binarykitchen use --hp option.

# pm2 --help | grep -- --hp
    --hp <home path>                     define home path when generating startup script
@binarykitchen

This comment has been minimized.

Contributor

binarykitchen commented Jan 14, 2016

@estliberitas well, i didn't know about that and think it should be documented in the advanced readme

and honestly, i think this should be default behaviour. to use the same home directory as the user.

@soyuka

This comment has been minimized.

Collaborator

soyuka commented Jan 14, 2016

You're right, -u is not handled when PM2_ROOT_PATH is defined. Just wondering why is the -u user not the same as the user who handled pm2 (when you did pm2 startup -u). They should have the same home.

If pm2 is set to resurrect with the node the user, binarykitchen won't have the same pm2 instance in memory and it might cause trouble.


Long story (or notes to myself):

export PM2_HOME="/home/michael.heuberger/.pm2"

This comes from PM2_ROOT_PATH in cli.js which is defined in constants.js.

We might handle arguments in constants.js and treat conditions like this in the constants.js file directly.
But the -u option is only used with the startup script, and defines the system username that launches pm2. IMO, we could stick with process.getuid() and remove the -u option. (can't work see below)

@Unitech

@soyuka soyuka self-assigned this Jan 14, 2016

@estliberitas

This comment has been minimized.

estliberitas commented Jan 14, 2016

@binarykitchen well, I'm not in charge a bit, but had the same thought when ran into same problem. 😉

@binarykitchen

This comment has been minimized.

Contributor

binarykitchen commented Jan 14, 2016

see ... it is about user friendliness. we are expecting this behaviour, so ...

@estliberitas

This comment has been minimized.

estliberitas commented Jan 14, 2016

@binarykitchen well, @soyuka is on this, so I guess user friendliness soon to be here. Or you can always submit a PR.

@soyuka

This comment has been minimized.

Collaborator

soyuka commented Jan 15, 2016

@binarykitchen :

Just wondering why is the -u user not the same as the user who handled pm2 (when you did pm2 startup -u). They should have the same home.

Could you give me a use case where they are different?

I want to remove the -u option to avoid such issues but this would mean that the user that issues pm2 startup will have to be the same as the one that will launch pm2 on startup.

@estliberitas

This comment has been minimized.

estliberitas commented Jan 15, 2016

@soyuka Em... What if user running pm2 startup... does not have privileges to create init script? Or I don't get something ? 😕 As for me, I'm for leaving both options and just documenting --hp because it allows some kind of customization. Maybe it would be better to use the strategy by default and otherwise let those parameters work?

Just wondering why is the -u user not the same as the user who handled pm2 (when you did pm2 startup -u). They should have the same home.

I usually create separate user, say nodeapps or whatever. So PM2 and Node.js applications are to be run by this user. Also this user can't write to /etc/init.d. It has his own home directory. And I wish all the Node apps not be run by privileged user but in some kind of sandbox which can't do anything very bad to let's say /etc/shadow.

P.S.: if you do not like --hp and -u option, possible workaround is forcing users to do sudo -E pm2 startup ..., so HOME & USER will be exactly the same. Windows platforms are a different case though.
P.P.S.: maybe I don't get something, excuse me if so.

@soyuka

This comment has been minimized.

Collaborator

soyuka commented Jan 15, 2016

@estliberitas You're right my mistake!

Actually when you launch pm2 startup without arguments you'll have an error:

[PM2] You have to run this command as root. Execute the following command:
sudo su -c "env PATH=$PATH:/home/abluchet/.nvm/versions/node/v5.3.0/bin pm2 startup linux -u abluchet --hp /home/abluchet"

We say : launch the same pm2 instance with root so that we have correct permissions to write in /etc/init.d. Actually, I'm wrong (thanks for making me realize this), we can't remove the -u option or we won't be able to set the correct user, my mistake. But, we might remove the --hp option and try to resolve the home from the user (-u). A portable nodejs solution would be something like this:

1. `pm2 startup` has to be run as root with a `-u user` argument
2. fork a process that runs as user -u
3. print os.homedir() (https://nodejs.org/api/os.html#os_os_homedir)
4. get back stdout, this is the correct home directory for user -u

I think that this would actually work on windows :p, not pretty though.

About --hp I think it exists because the PM2_ROOT_PATH might be different from the HOME variable.

@estliberitas

This comment has been minimized.

estliberitas commented Jan 15, 2016

@soyuka Pretty simple and looks good, I had the same in mind

@estliberitas

This comment has been minimized.

estliberitas commented Jan 15, 2016

@soyuka Another thing. I was wondering how could I start multiple PM2 for diff. users on a machine? Typical case - I run node apps as user A, non-node apps as user B. There is a difference in privileges between A & B - that's why I can't run all-in-one. So I'd like to run both PM2 instances on startup. 😉

Can it be another option like init script path / name?

@soyuka

This comment has been minimized.

Collaborator

soyuka commented Jan 15, 2016

Story:
In fact, pm2 was first built to run as root and we had then an option --run-as-user that could run the script as a specified user.
There were questions about security, and we thought it would be better if pm2 was built to run as an unprivilege user.

So, if you want to run pm2 with different users you'd have to run multiple instances of pm2.

Now the issue, if I understand, is that the startup command will only create one init script. I think that I could add the -$user to the init script name so that multiple instances (one per user) can be launched on startup.
Thoughts?

@estliberitas

This comment has been minimized.

estliberitas commented Jan 15, 2016

@soyuka Sounds nice. I could make a PR for this if it would help.

@soyuka

This comment has been minimized.

Collaborator

soyuka commented Jan 15, 2016

Would be great!

Everything is here: https://github.com/Unitech/pm2/blob/master/lib/CLI.js#L575

Just a though, as Cli.js gets bigger, it'd be nice if you could move the content of the startup command to a new file in lib/CLI/Startup.js for example.

@estliberitas

This comment has been minimized.

estliberitas commented Jan 15, 2016

I'll look into this during weekend and will send PR on Sun-Mon, OK?

Just a though, as Cli.js gets bigger, it'd be nice if you could move the content of the startup command to a new file in lib/CLI/Startup.js for example.

Yep, noticed that already

@soyuka

This comment has been minimized.

Collaborator

soyuka commented Jan 15, 2016

Ok np
Le ven. 15 janv. 2016 à 18:07, Alexander Makarenko notifications@github.com
a écrit :

I'll look into this during weekend and will send PR on Sun-Mon, OK?


Reply to this email directly or view it on GitHub
#1888 (comment).

@mauron85

This comment has been minimized.

mauron85 commented Jan 17, 2016

+1
What is the status of this?

In my case PM2_HOME is set into root's home dir instead if nodejs user home dir (because I ran command with sudo)

NAME=pm2
PM2=/usr/lib/node_modules/pm2/bin/pm2
USER=nodejs
DEFAULT=/etc/default/$NAME

export PATH=/usr/bin:$PATH
export PM2_HOME="/root/.pm2"

Edit: Normally this would be not a problem, as it can be change manually. But i'm trying to auto provising box using vagrant and now it's real issue, as I had to write sed awks to correct /etc/init.d/pm2-init.sh file

@estliberitas

This comment has been minimized.

estliberitas commented Jan 17, 2016

@mauron85 I'm working on it, tomorrow will send PR. And I'm re-writing startup command a bit, it's gonna be fully async due to the needs. Any additions?

Btw, what vagrant box do you use? Meaning OS.

@soyuka

This comment has been minimized.

Collaborator

soyuka commented Jan 17, 2016

@mauron85 can't you use the --hp option?

@mauron85

This comment has been minimized.

mauron85 commented Jan 17, 2016

@soyuka actually I can. :-) Didn't know about that. Thanks.

@mauron85

This comment has been minimized.

mauron85 commented Jan 17, 2016

@estliberitas my vagrant box is ubuntu 14.04 64bit.

@estliberitas

This comment has been minimized.

estliberitas commented Jan 19, 2016

Guys, I'm postponing my PR with refueled startup action till tomorrow. Sorry.

estliberitas added a commit to estliberitas/PM2 that referenced this issue Jan 19, 2016

Rework startup command code
This commit brings several things:
* Move command code to separate script
* Refactoring of code in order to be async and provide better error-handling
* Support for future integration with Windows OS
* Print & handle errors in a more clean way
* Detect user home directory

Fixes Unitech#1888

estliberitas added a commit to estliberitas/PM2 that referenced this issue Jan 20, 2016

Rework startup command code
This commit brings several things:
* Move command code to separate script
* Refactoring of code in order to be async and provide better error-handling
* Support for future integration with Windows OS
* Print & handle errors in a more clean way
* Detect user home directory

Fixes Unitech#1888

estliberitas added a commit to estliberitas/PM2 that referenced this issue Jan 20, 2016

Rework startup command code
This commit brings several things:
* Move command code to separate script
* Refactoring of code in order to be async and provide better error-handling
* Support for future integration with Windows OS
* Print & handle errors in a more clean way
* Detect user home directory

Fixes Unitech#1888

estliberitas added a commit to estliberitas/PM2 that referenced this issue Jan 20, 2016

Rework startup command code
This commit brings several things:
* Move command code to separate script
* Refactoring of code in order to be async and provide better error-handling
* Support for future integration with Windows OS
* Print & handle errors in a more clean way
* Detect user home directory

Fixes Unitech#1888

soyuka pushed a commit to soyuka/pm2 that referenced this issue Jan 21, 2016

Antoine Bluchet
Rework startup command code
This commit brings several things:
* Move command code to separate script
* Refactoring of code in order to be async and provide better error-handling
* Support for future integration with Windows OS
* Print & handle errors in a more clean way
* Detect user home directory

Fixes Unitech#1888
@soyuka

This comment has been minimized.

Collaborator

soyuka commented Jan 22, 2016

Some users to test the development branch?

@Unitech

This comment has been minimized.

Owner

Unitech commented Dec 6, 2016

closed in favor of #2559

@Unitech Unitech closed this Dec 6, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment