Normalize permissions when extracting tarfiles

[chlunde]

Summary

Tar-files from the registry might have unwanted file permissions, for example:

• files and directories the user can't read
• world-writable files
• inconsistent permissions

This addresses the issues by ignoring the permissions in the input file. It will leave group writable files/directories if the users umask allows that (such as umask 0002 or 0007), but it will never leave them world writable even with umask 0000.

Fixes #1143, #961, #872, and #713

For example, in #961 a tarball is missing access rights for a directory:

dir$ rm -Rf node_modules/ package.json ~/.yarn-cache/npm-@types/; yarn init --yes; yarn add @types/react-router@2.0.37
yarn init v0.16.2
...
[2/4] Fetching packages...
error An unexpected error occurred, please open a bug report with the information provided in "/home/chlunde/tmp/dir/yarn-error.log".
...
dir$ tail -n3 yarn-error.log 
Trace: 
  Error: https://registry.yarnpkg.com/@types/react-router/-/react-router-2.0.37.tgz: EACCES: permission denied, open '/home/chlunde/.yarn-cache/npm-@types/react-router-2.0.37/lib/History.d.ts'
      at Error (native)

This is caused by incorrect permissions in the tarfile:

dir$ tar -tvzf react-router-2.0.37.tgz  | grep /$
drw-rw-rw- 0/0               0 2016-10-03 17:23 react-router/lib/

Test plan

I have verified that I can install @types/react-router@2.0.37 after the change:

$ mkdir dir; cd dir
$ rm -Rf ~/.yarn-cache/npm-@types/; yarn init --yes; yarn add @types/react-router@2.0.37
yarn init v0.16.2
warning The yes flag has been set. This will automatically answer yes to all questions which may have security implications.
success Saved package.json
Done in 0.06s.
yarn add v0.16.2
info No lockfile found.
[1/4] Resolving packages...
[2/4] Fetching packages...
[3/4] Linking dependencies...
[4/4] Building fresh packages...
success Saved lockfile.
success Saved 3 new dependencies.
├─ @types/history@2.0.39
├─ @types/react-router@2.0.37
└─ @types/react@0.14.43
Done in 3.01s.

I have also verified that umask is applied:

dir$ umask
0022
dir$ ls -l node_modules/@types/react-router
total 20
drwxr-xr-x. 2 chlunde chlunde 4096 Oct 26 23:05 lib/
-rw-r--r--. 1 chlunde chlunde  815 Oct  3 17:23 package.json
...
chlunde@localhost.localdomain dir$ umask 0002
chlunde@localhost.localdomain dir$ rm -Rf node_modules/ package.json ~/.yarn-cache/npm-\@types/; yarn init --yes; yarn add @types/react-router@2.0.37
...
Done in 1.13s.
chlunde@localhost.localdomain dir$ ls -l node_modules/@types/react-router
total 20
drwxrwxr-x. 2 chlunde chlunde 4096 Oct 26 23:07 lib/
-rw-rw-r--. 1 chlunde chlunde  815 Oct  3 17:23 package.json
...

TODO

• Are there other file types in tarballs that need special handling
• Do you think this code needs integration tests?

[samccone]

I am not sure the correct thing to do here. Obviously we want to normalize but maybe we might want to limit to just the current user account.

[samccone]

related reading
npm/npm#9359

[samccone]

We prob should follow this approach also
https://docs.npmjs.com/misc/config#umask

[bestander]

@samccone, what action do you think should be next?

[bestander]

This needs more discussion before merging.
@chlunde, would be great if you could summarise the alternatives and suggest the better one

[gtirloni]

npm 3.10.8 seems to fix the file permissions but leaves the user/group ownership untouched (or uses a pattern I cannot recognize). I have files owned by root:root root:$user and nobody:$user in /usr/lib/node_modues, when they are installed through sudo npm install -g.

[neurotech]

Is there any chance this PR will be accepted? I'd love to be able to move to yarn completely but this issue holding me back:

ibmdb/node-ibm_db#216

[bestander]

@neurotech, hardcoding a permission for all npm dependencies sounds a bit too invasive.
Are you interested in extending this PR as suggested by @samccone?
Just read umask from config.

[chlunde]

Sorry, I forgot about this PR. What is the use case for having umask in the config, when the OS has umask support?

If the user wants special permissions, they can be applied to the top level yarn cache directory.

[bestander]

Just want to be on the safe side and not change the current default behavior.

[chlunde]

I believe the current behaviour is using "random" permissions set at packaging time from the user and program publishing to the registry, possibly including that persons umask, and then applying the end user umask on that. Or are we talking about npm behaviour?

Anyway, if anyone would like to take over this PR, notify me and I can close this (unless github has a way of "handing over" a PR).

[bestander]

I like this PR but would like to have this thing configurable.
Feel free to open a new PR if you need this feature in Yarn