nodejs / node

Node.js JavaScript runtime ✨🐢🚀✨
https://nodejs.org
Other
107.08k stars 29.32k forks source link

Issue about `os.homedir()` #5582

Closed XadillaX closed 8 years ago

XadillaX commented 8 years ago

I saw os.homedir() is directly using uv_os_homedir and it has an strange feature.

See here for the feature.

It will check the environment variable first and then check for the real home directory.

And someone discussed here for the reason.

I wonder if this should check $HOME first? It's something of a UNIX tradition to be able to change your home directory on the fly. A problem with that is that getenv("HOME") is not MT-safe.

-- @bnoordhuis

But I think there's still a problem.


Consider one situation:

If I wrote a CLI program and it will write some log file on user's home directory.

Now I run the CLI like this:

$ sudo -u foo node mycli.js

It should write log file on /home/foo. But the problem is - os.homedir() still got process.env.HOME.

One solution is to edit /etc/sudoers:

# env_keep+="HOME MAIL"
# comment env_keep += "HOME"

And another method is using -i argument.

_For further reading you can go to this article. (It was written in Chinese by @Amunu)_


But I think why don't we wrote a os.realHomedir() or removing checking HOME in realhomedir()? Not all of us (normal developers) have the permission to access configuration files on our online servers.

parox2014 commented 8 years ago

+1

cjihrig commented 8 years ago

There is work going on in https://github.com/libuv/libuv/pull/742 to get the current user's password file entry. This would bypass the HOME check.

XadillaX commented 8 years ago

I wrote a package called real-homedir.

Actrully it's a copy of uv_os_homedir() but remove checking getenv("HOME") for getting real home directory.

So what I mean is that to replace os.homedir() or write a new function like os.realHomedir() for the situation above.

@cjihrig

yorkie commented 8 years ago

@XadillaX I don't think adding the new function os.realHomedir is a flawless solution, which would cause more confusion at user-land.

Fishrock123 commented 8 years ago

IIRC there was actually requests to use $HOME if it existed..

XadillaX commented 8 years ago

@Fishrock123 Yeah I agree with $HOME.

But I'm talking about the situation above like sudo -u foo node mycli.js. It's the situation which mutually exclusive with $HOME.

How to solve it in Node.js and let developers be comfortable with it?

ngot commented 8 years ago

+1. provide warning when sudo -u to get the os.homedir() maybe nice.

cjihrig commented 8 years ago

Moving forward, you can use os.userInfo().homedir. It skips the environment variable check, and returns the information for the effective user.