[BUG] Install via Git fails when running with sudo (no access to SSH_AUTH_SOCK)

[okdana]

What / Why

(Sorry for any bad assumptions i'm making here, i know very little about NPM or pacote)

When pacote detects that it's running as root, but the directory it's trying to clone into is owned by another user, it runs git with that user's UID. This seems to work as far as making the permissions on the clone consistent, but, when using Git+SSH, the privilege-dropped OpenSSH process is then prevented from accessing the agent auth socket (because it's still owned by root). This causes git to fail.

The obvious work-around (besides not running NPM as root in the first place, which is of course my long-term goal) is to pass -H or -i to sudo, which should avoid the privilege drop in most cases. And, if reconciling the ownerships of the clone directory and auth socket is too irritating, maybe that should be the 'official' solution — i think pip has a similar requirement.

But one big difference between pip and pacote is that pip actually tells you what's wrong and how to fix it — without spend a whole bunch of time troubleshooting it, the pacote issue simply looks like the Git clone is failing for no reason at all.

When

Always, in this configuration/scenario:

• Ubuntu 20.04 Focal
• npm 6.14.4
• pacote 9.5.12
• npm running as root
• Clone directory not owned by root (e.g. using sudo without -i or -H)
• Installing package via Git+SSH
• SSH agent needed for auth (i.e. OpenSSH can't just fall back to a default key)

Where

Using a private repository in this case, but i assume this can occur any time pacote deals with Git via SSH

How

Current Behavior

git commands fail in the above scenario, with no good explanation as to why

Steps to Reproduce

% mkdir /tmp/pacote-bug && cd /tmp/pacote-bug
% sudo sh -c '
  eval "$(ssh-agent)";
  ssh-add -q /path/to/necessary/key;
  ls -ld -- "$SSH_AUTH_SOCK";
  GIT_SSH_COMMAND="id >&2; ssh -v" npm install "git+ssh://[email protected]/foo/bar.git"
'
Agent pid 1456457
srw------- 1 root root 0 Jun 17 01:50 /tmp/ssh-W9IypxzdTXFs/agent.1456456
npm ERR! code 128
npm ERR! Command failed: git clone --mirror -q ssh://[email protected]/foo/bar.git /home/dana/.npm/_cacache/tmp/git-clone-0b3c15e8/.git
npm ERR! warning: templates not found in /tmp/pacote-git-template-tmp/git-clone-576ad45a
npm ERR! uid=1001(dana) gid=1001(dana) groups=1001(dana)
...
npm ERR! debug1: pubkey_prepare: ssh_get_authentication_socket: Permission denied
...
npm ERR! [email protected]: Permission denied (publickey).
npm ERR! fatal: Could not read from remote repository.
npm ERR! 
npm ERR! Please make sure you have the correct access rights
npm ERR! and the repository exists.
...

(The clone succeeds if i change cwdOwner() and mkOpts() in git.js so that they don't try to de-escalate)

Expected Behavior

imo, pacote should either:

• figure out how to perform the clone without breaking the SSH agent, or
• alert the user that the way they're running it may cause problems, and ideally explain what to do instead

Who

Me!

References

• Related to git fetcher fails if opts.uid is used to drop privileges zkat/pacote#105
• Related to [BUG] Some git commands are executed under destination directory's owner account #14

[okdana]

Actually, looking into this again this morning, i forgot one thing in my 'When': The SSH agent socket of course needs to be owned by root, as in my STR.

If you're running sudo as an unprivileged user, and you've got it set up to pass that user's agent socket through, when pacote drops privileges it'll (normally) drop to that user's UID, which should match the socket, and that'll be fine. In my case, sudo is being used by an unprivileged user to run an automated build script, and that script is what's setting up the agent.

(Also, in case it wasn't clear, this did work fine for several years; it only just broke now after an NPM update)

[isaacs]

So, if I'm reading this correctly, if you change the sudo sh -c '... to sudo -i sh -c '... or sudo -H sh -c '... then it works?

It's not too hard to detect the permission failure and tell the user to try that. Turning off the priv drop would be kind of hazardous, as for every one person affected by this issue, there are dozens who get stuff into a weird state with root-owned files in their cache directory.

What we might be able to do is turn off the priv drop, but then do a chown -R on the checked out files. That's somewhat less reliable, because the chown is yet another thing that can fail, but at least we'd be trying to leave stuff in a non-broken state.

[okdana]

So, if I'm reading this correctly, if you change the sudo sh -c '... to sudo -i sh -c '... or sudo -H sh -c '... then it works?

Yes. In that case, sudo will set (probably) HOME=/root, which is where it'll look for (or create) the cache directory, and since that directory will (probably) be owned by the same user that's running npm (root) it won't try to drop privileges.

Like i said, i'm not very familiar with NPM stuff, so i'm not sure what would be the best way for the tool to account for this situation, or if it should. But, speaking for myself, it would have been helpful if it had just printed a warning message like pip does telling you that it might cause problems. For reference, pip's message looks like this:

The directory '/home/foo/.cache/pip' or its parent directory is not owned by the current user and the cache has been disabled. Please check the permissions and owner of that directory. If executing pip with sudo, you may want sudo's -H flag.

[isaacs]

Yeah, I like that. We can definitely detect and print some helpful guidance. I think pacote itself should probably ultimately still do what it's currently doing, since it's usually the right thing, and more reliable than trying to chmod after the checkout.