Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

strict umask and created node_modules directories #2078

Closed
inimino opened this Issue · 5 comments

2 participants

@inimino

When node_modules directories are created, they seem to use the umask of the user running the command, rather than having the permissions they need.

Even when the node_modules directory already exists, the permissions are reset:

# umask
0077
# ls -ld /usr/local/lib/node_modules/
drwxr-xr-x 5 root root 4096 Jan 23 00:42 /usr/local/lib/node_modules/
# npm install -g df
# ls -ld /usr/local/lib/node_modules/
drwx------ 5 root root 4096 Jan 23 00:43 /usr/local/lib/node_modules/
@isaacs
Owner

So, you'd prefer that existing node_modules folders are left alone, and not chmod'ed? Seems reasonable.

@inimino

Even if it doesn't exist, I'd expect the permissions to be the same regardless of umask.

Otherwise, using df as an example:

# umask
0077
# npm install -g df
# which df-json
/usr/local/bin/df-json
# su inimino
$ which df-json
which: no df-json in [...]
$ exit
# ls -l $(which df-json)
/usr/local/bin/df-json -> ../lib/node_modules/df/index.js

The unprivileged user can't access it because it is a symlink into a directory that is not user-readable.

@isaacs
Owner

The unprivileged user also can't run it because it's a symlink to a file that is not user-runnable.

It is a cardinal sin for a program to create files and folders that violate the umask by default. In fact, if you're not root, the system won't even allow it. If you want npm to use a different umask, then you can specify it by either setting the --umask config to something like 022, or of course by setting the process umask.

@inimino

The permissions on the file itself allow execution, as they should. It is not user-runnable, but only because of the missing exec bit on node_modules/.

# rm -rf /usr/local/lib/node_modules/df
# umask
0077
# npm install -g df
# ls -l /usr/local/lib/node_modules/df/index.js 
-rwxr-xr-x 1 nobody root 1808 Jan 22 22:01 /usr/local/lib/node_modules/df/index.js
# ls -ld /usr/local/lib/node_modules/
drwx------ 6 root root 4096 Jan 24 21:02 /usr/local/lib/node_modules/

It's not a cardinal sin. Violating umask by default is exactly what an installer is supposed to do. It's the way apt-get et al. operate, and it's the reason install(1) exists and is used (the example below does not require root). My umask should affect the files I personally create directly, but if apt-get install foo did anything other than install foo so that everyone on the machine can use it, regardless of my umask, I'd be very disappointed.

# umask
0077
# install -d foo/bar/baz
# find foo -ls
481817    4 drwxr-xr-x   3 root     root         4096 Jan 24 20:57 foo
481818    4 drwxr-xr-x   3 root     root         4096 Jan 24 20:57 foo/bar
481819    4 drwxr-xr-x   2 root     root         4096 Jan 24 20:57 foo/bar/baz

npm install -g foo should install foo such that it works, regardless of what my umask happens to be.

@inimino

Looking at the code, npm is already careful to set correct permissions on files it installs, regardless of umask. This particular behavior was just a bug.

There are two issues: what happens if /usr/lib/node_modules already exists, and what happens if it is created by npm.

If it already exists, it is chmod'd when extracting the tarball into /usr/lib/node_modules/___foo.npm/ due to overzealous chmod'ing in the mkdirp module which is already fixed: substack/node-mkdirp@ba450d6

The mkdirp bundled with npm is version 0.1.0, but if you update it to the current 0.3.0 my issue will go away.

The other issue is what happens when /usr/lib/node_modules does not exist already, which I have not looked at, but I would guess it will be created by npm and probably will get the right permissions to begin with in that case.

@isaacs isaacs closed this issue from a commit
@isaacs isaacs Update fstream and mkdirp
The hope that this will fix #2078
cb262a3
@isaacs isaacs closed this in cb262a3
@dtiersch dtiersch referenced this issue in keybase/keybase-issues
Open

keybase-installer and strict umask #673

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.