[BUG] Some git commands are executed under destination directory's owner account

[x-yuri]

What / Why

It might so happen that root installs packages into a directory owned by another user. Consider a case where you launch an app with docker-compose and bind mount the host directory with source code (.) into the container (/app) for changes to source code to automatically propagate into the container. Under this circumstances we get root user and /app owned by uid 1000. During fetch-package-metadata phase it succeeds, since nothing says it has to impersonate:

[email protected] started to run commands under cwd's owner. But some files are to be created in ~/.npm/_cacache/tmp (e.g. cloning repositories when installing from github). When a process (npm) user doesn't match the cwd user, process user's cache is still used. So git clone is executed under cwd user to clone to the process user's tmp dir, which apparently fails. That can happen under docker when a non-root's directory is bind-mounted into a container. It's not owned by root in the container either, but the processes in the container are running under root (unless explicitly started as another user).

The issue doesn't reveal itself on the fetch-package-metadata phase:

https://github.com/npm/cli/blob/v6.12.1/lib/fetch-package-metadata.js#L59-L65
https://github.com/npm/pacote/blob/v9.5.8/manifest.js#L25
https://github.com/npm/pacote/blob/v9.5.8/lib/finalize-manifest.js#L49
https://github.com/npm/pacote/blob/v9.5.8/lib/finalize-manifest.js#L154
https://github.com/npm/pacote/blob/v9.5.8/lib/fetch.js#L33
https://github.com/npm/pacote/blob/v9.5.8/lib/fetchers/git.js#L71-L73
https://github.com/npm/pacote/blob/v9.5.8/lib/fetchers/git.js#L176

But on the extract phase it switches to non-root and fails:

https://github.com/npm/cli/blob/v6.12.1/lib/install/action/extract.js#L90
https://github.com/npm/pacote/blob/v9.5.8/extract.js#L42
https://github.com/npm/pacote/blob/v9.5.8/lib/with-tarball-stream.js#L96
https://github.com/npm/pacote/blob/v9.5.8/lib/fetch.js#L28
https://github.com/npm/pacote/blob/v9.5.8/lib/fetchers/git.js#L44
https://github.com/npm/pacote/blob/v9.5.8/lib/fetchers/git.js#L71-L73
https://github.com/npm/pacote/blob/v9.5.8/lib/fetchers/git.js#L176

When

• When installing packages from GitHub.

Where

• n/a

How

Current Behavior

• An error occurs:

npm ERR! code 128
npm ERR! Command failed: git clone --mirror -q git://github.com/kevva/is-positive.git /root/.npm/_cacache/tmp/git-clone-d80b8730/.git
npm ERR! fatal: could not create leading directories of '/root/.npm/_cacache/tmp/git-clone-d80b8730/.git'

Steps to Reproduce

Under non-root user:

1.sh:

#!/bin/sh
set -eux
npm --version
apk add git
npm i kevva/is-positive || cat /root/.npm/_logs/*.log

$ docker run --rm -itv $PWD:/app -w /app node:10.17.0-alpine3.10 ./1.sh
+ npm --version
6.11.3
...
+ npm i kevva/is-positive
...
npm ERR! Command failed: git clone --mirror -q git://github.com/kevva/is-positive.git /root/.npm/_cac
ache/tmp/git-clone-87fa5e82/.git
npm ERR! fatal: could not create leading directories of '/root/.npm/_cacache/tmp/git-clone-87fa5e82/.
git'
...

Or under root in a non-root dir:

$ npm i kevva/is-positive

Expected Behavior

• It either succeeds in both cases (extract, fetch-package-metadata), or fails. Preferably the former.

Who

• n/a

References

• Related to Infer ownership of _all_ unpacked files (like cacache does for cache) #2

More info

Introduced in npm-6.11.0, pacote-9.5.5. Fixed in npm-6.13.6, pacote-9.5.12.
Affects node@^10.17.0 (npm-6.11.3), node >= 12.11.0 (npm-6.11.3), node < 13.7.0 (npm-6.13.6). More on it here.

Old steps to reproduce

Create files based on the following gist and do docker-compose up:

docker-compose.yml:

version: '3'

services:
  app:
    build: .
    # entrypoint: sleep 10000000
    # ports:
      # - 9229:9229
    volumes:
      - ./:/app

Dockerfile:

FROM node:12.13.0-alpine

RUN apk add git vim
WORKDIR /app

ENTRYPOINT ["./entrypoint.sh"]

entrypoint.sh:

#!/bin/sh
set -eu
npm i || true
cat /root/.npm/_logs/*.log

$ chmod u+x entrypoint.sh
$ docker-compose up

As you might guess the issue was discovered in a docker container. Alternatively,

# useradd -m u1
# cd /home/u1
# echo '{"dependencies": {"is-positive": "kevva/is-positive"}}' > package.json
# npm i

[isaacs]

Fixed in 9.5.10 and 10.2.1.

9.5.10 will be included in the next npm v6 release. 10.2.1 (or some later v10) will be included in npm v7.

[x-yuri]

I've improved the description since it was pretty bad. And wanted to add that the following commit doesn't solve the issue, because it's not that a non-root tries to switch users, but that root switches users when it shouldn't. The decision must depend on where the files are to be created. Which is handled by the following commit. So supposedly this issue doesn't affect [email protected].

It was fixed in npm-6.13.6, pacote-9.5.12. It was resolved in nodejs >= 13.7.0 (npm >= 6.13.6), but not in nodejs 10, nodejs 12 yet. But updating npm should resolve the issue where it exists by default.

[isaacs]

@x-yuri So, iiuc, it is fixed, just not in the commit that we thought fixed it?

[x-yuri]

Yes, it is fixed in [email protected] (not in 9.5.10 as you most likely supposed). And judging from the related issue, it's not an issue in [email protected]. Although I didn't try to confirm the latter.