Skip to content
This repository has been archived by the owner. It is now read-only.

umask setting is ignored for some directories #9359

Open
int3h opened this issue Aug 20, 2015 · 11 comments

Comments

Projects
None yet
9 participants
@int3h
Copy link

commented Aug 20, 2015

Expected Behavior

All directories created by npm install should have the npm config setting umask applied to their file mode/permissions. The umask setting is clearly described as the "value to use when setting the file creation mode on files and folders."

Observed Behavior

During npm install, if directories are created by the tar module as a side-effect of extracting a file, the umask setting is ignored. For example, when tar extracts foo/bar/baz.js, it creates the foo and foo/bar directories (if they don't exist) before writing baz.js. These directories do not respect the umask setting.

Instead, npm's umask setting is ignored and process.umask() is used. However, there is a further bug/unexpected behavior where the tar module will actually do the equivalent of chmod a+x on directories created as a side-effects of file extraction, meaning the even process.umask() isn't strictly observed.

Details

  • OS: Mac OS X 10.9.5
  • npm --version: 2.13.5
  • node --version: v0.10.40
  • npm config get umask: 0077
  • umask: 0077

An example is seen if you install grunt-lib-phantomjs. The directory node_modules/phantomjs/lib (as an example) should have a mode of 0700; instead, it has a mode of 0711.

@int3h

This comment has been minimized.

Copy link
Author

commented Aug 28, 2015

Since I see this has been tagged as a feature request and support request, I want to be clear that this is definitely a bug in npm.

The umask setting in npm is being completely ignored for a seemingly arbitrary subset of operations during npm install. This is almost certainly not the intended behavior. It also explicitly disagrees with the behavior described in the documentation, and is inconsistent even within the observed behavior of npm (sometimes it respects umask, other times it doesn't, for no good reason).

The underlying cause (cited in the issue) is that the tar module isn't aware of npm's umask setting, and just does its own thing (using the process.umask() value, then overriding the execute bit on it.)

But in summary, this is 99.9% likely to be a real bug, not a new feature or user usage problem.

@Shasharala

This comment has been minimized.

Copy link

commented Nov 12, 2015

This issue still exists in npm version 3.4.0 and is most certainly a bug and not a new feature or user usage problem.

Please see #4197 which is correctly tagged as a bug.

Details

  • OS: Debian GNU/Linux 7.8 (wheezy)
  • npm --version: 3.4.0
  • node --version: v4.2.2
  • npm config get umask: 0022
  • umask: 0077

@othiym23 othiym23 added the footgun label Nov 17, 2015

@othiym23

This comment has been minimized.

Copy link
Contributor

commented Nov 18, 2015

This is marked as a feature request in part because the current behavior that npm has is underspecified. The first step to making npm's behavior here clearer is to nail down what the current behavior is, and why. Tagging this with footgun gets it onto npm's road map, and the next step for the CLI team is to unearth and document the historical reasoning for how the various pieces of npm (including node-tar) handle permissions.

@othiym23 othiym23 added bug patch-welcome and removed support labels Jan 22, 2016

@othiym23

This comment has been minimized.

Copy link
Contributor

commented Jan 22, 2016

After discussing this as a team, we think the right thing for npm to do is to ignore whatever permissions or UIDs are set in the package tarball, and explicitly squash everything to be written with the current user's user ID and umask in all cases except when npm is being run as root without --unsafe-perm being overwritten (you should never have files owned by nobody on your filesystem).

This is a mildly tricky bit of work because it requires good tests, and also because we need to make sure whatever API calls the CLI uses don't cause problems on Windows, but this is something we plan to address within the medium-term. If somebody else wants to treat the first paragraph as a rough spec and start working on a patch, that would be very welcome!

@anthraxx

This comment has been minimized.

Copy link

commented Oct 26, 2016

Did this issue get lost (honest question, no sarcasm)?
In my opinion it needs lot more love as the possible security implications could be quite catastrophic on a multi-user system and this issue is reported for over a year now.
The worst case scenario is that such directory results in the possibility to replace its content by a non-root user with evil code that may me executed by another user (including possibly root).

What I observed (even with newest npm 3.10.9) that it sometimes creates node_modules directories with permission 777. when doing the npm install multiple times the results vary, most of the time it results in 755 but sometimes in 777). This seems to have nothing to do with the source tarballs content (retrieved from registry.npmjs.org) but a more general issue. As mentioned, its not deterministic and the tarballs definitively don't contain any files/directories with such permissions.

This problem was observed while creating packages for a Linux distribution and boiled down to finding this ticket.

@npm-robot

This comment has been minimized.

Copy link

commented Jun 19, 2017

We're closing this issue as it has gone thirty days without activity. In our experience if an issue has gone thirty days without any activity then it's unlikely to be addressed. In the case of bug reports, often the underlying issue will be addressed but finding related issues is quite difficult and often incomplete.

If this was a bug report and it is still relevant then we encourage you to open it again as a new issue. If this was a feature request then you should feel free to open it again, or even better open a PR.

For more information about our new issue aging policies and why we've instituted them please see our blog post.

@npm-robot npm-robot closed this Jun 19, 2017

@anthraxx

This comment has been minimized.

Copy link

commented Jan 8, 2018

@othiym23 @isaacs @iarna
can you please reopen and revisit this issue, npm 5.6.0 still randomly ends up creating node_modules directories with 777 that contain code.
PS: This npm-robot that auto-closes a security issue because nobody replied is quite damaging

@Tsutsukakushi

This comment has been minimized.

Copy link

commented Feb 22, 2018

Aug 20, 2015

O I am laffin.

@christianjgreen

This comment has been minimized.

Copy link

commented Feb 22, 2018

Issue not fixed in 30 days?
Must be gone!

@marpuch

This comment has been minimized.

Copy link

commented Feb 23, 2018

Not treating security seriously, are we?

@npm npm locked as too heated and limited conversation to collaborators Feb 23, 2018

@npm npm deleted a comment from RedShift1 Feb 23, 2018

@zkat

This comment has been minimized.

Copy link
Member

commented Feb 23, 2018

Yeah, this seems to have gotten swept up -- the bot shouldn't have just closed the issue like that.

There was a step forward and a step back on this over the past year, and we haven't touched the issue since. The patch-welcome tag continues to apply, so if you think writing code is a more worthwhile endeavor than snarking on foss issue trackers, we super welcome your contributions!

@zkat zkat reopened this Feb 23, 2018

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
You can’t perform that action at this time.