git fetcher fails if opts.uid is used to drop privileges

[cjwatson]

The git fetcher goes wrong if opts.uid is used to drop privileges, which npm does by default when running under sudo.

Reproduction steps:

• Construct a clean environment where you have a non-root user with sudo privileges. (I used lxc launch ubuntu:16.04 pacote-test followed by lxc exec pacote-test su - ubuntu.)
• Install (for example) Node 8.1.3 / npm 5.0.3. (I installed nvm and used nvm install 8.1.3 followed by nvm use 8.1.3.)
• Create a new project with the following package.json:

{
  "name": "pacote-test",
  "version": "1.0.0",
  "description": "",
  "main": "index.js",
  "scripts": {
    "test": "echo \"Error: no test specified\" && exit 1"
  },
  "author": "",
  "license": "ISC",
  "dependencies": {
    "socksv5": "git://github.com/heroku/socksv5.git#v0.0.6.1"
  }
}

• Run sudo PATH="$PATH" sh -c 'npm i'. (The PATH and sh wrangling is to work around sudo's defaults so that it can find npm, and you may not need them in all setups.) This says:

npm ERR! code 1
npm ERR! Command failed: /usr/bin/git clone --depth=1 -q -b v0.0.6.1 git://github.com/heroku/socksv5.git /home/ubuntu/.npm/_cacache/tmp/git-clone-6d967b02
npm ERR! /home/ubuntu/.npm/_cacache/tmp/git-clone-6d967b02/.git: Permission denied
npm ERR!

npm ERR! A complete log of this run can be found in:
npm ERR!     /home/ubuntu/.npm/_logs/2017-07-06T10_38_44_407Z-debug.log

This happens because the fetcher makes a temporary git-clone-... directory and then tries to run git clone inside that, but it doesn't chown/chgrp the temporary directory to opts.uid and opts.gid, so git clone can't write to it. I think chown/chgrping the temporary directory would fix this.

(This came up in the context of building snaps. At the moment our builders run snapcraft as root under sudo, which is mostly fine because they're running in a throwaway container anyway; but it causes us to run into this bug. We may switch to running as non-root to work around it.)

[zkat]

Nice! This is good sleuthing.

This seems focused enough, too, that it might be a good starter issue, since it sounds like the primary fix is to make sure perms are correct on that tmp dir?

[cscalfani]

This bug has left NPM 5.x.x broken since no global installs can be made. Is this scheduled to be fixed?

[douglasmiller]

This is a major blocker for my company

[msimerson]

It's a blocker for me too.

[zkat]

update: I'm working on this now. It's important to point out that the --unsafe-perm flag is for run-script, not for installation. I'm trying to make sure I understand why npm@4 and earlier managed to drop perms safely for this, but I'm pretty sure they were doing it then. npm is really really not meant to be run as root, and you'll find it doesn't play well with it. It's strange that not even doing SUDO_UID=0 gets this working.

[Jakobud]

Well you can't run npm link on a lot of installations without root. So that's a big problem to assume one can just use npm without sudo.

[zkat]

update 2: this fell by the wayside and I'm unlikely to tackle it for a while, so if any brave soul wants to take it on, please be my guest!

[kjetilk]

Hi all! So, I'm a total noob on node.js, but since this is a blocker bug for us, I figured I might just take a look anyway... :-)

When look at cacache it looks like it should be doing this for us. Is that the correct interpretation of those docs?

Based on that, I figured it might either be that opts.uid isn't passed all the way into cacache, or it is something wrong in cacache, itself. I couldn't see where opts.uid originates, so I can't tell, but am I onto something?

[zkat]

@kjetilk I would be very surprised if cacache were causing this. I'm pretty sure the issue is between lib/fetchers/git.js and lib/util/git.js.

[kjetilk]

OK, thanks a lot for the quick response, @zkat ! That's good to know!

[agy]

I created a PR with a fix that "works for me". ~I'm running some more tests at the moment to be sure.~

I found this issue via the older issue: https://github.com/npm/npm/issues/16898

(edit): Tested with:

node: v8.11.4
npm: 5.6.0

node: v8.11.4
npm: 6.4.0

[katezaps]

💯 need this fix