Set UID and GID of untared files to zero if user is root #1837

lawliet89 · 2016-11-15T05:29:41Z

Summary

TarballFetcher uses tar to untar tarballs. tar in turn constructs a Writer object per entry in the tarball, preserving the original UID and GID in the tarball.

If Writer detects that the UID of the user is 0 (i.e. root user), it will attempt to chown the extracted file to the UID.

Usually, this is not a problem. However, as in #1750 and other issues, if the root user is not really root but inside a user namespace, and the UID is outside the subuid range of the mapped user, then it will fail. This is the same issue for GID.

This PR attempts to just set everything to UID 0 if the user is detected as root, regardless of whether the root is inside a namespace or not.

Test plan

First, configure Docker Daemon with user namespace enabled.

Consider the Dockerfile, package.json, and yarn.lock from this gist.

$ docker build -t test .
Sending build context to Docker daemon 29.34 MB
Step 1 : FROM node:7.1
7.1: Pulling from library/node
386a066cd84a: Pull complete 
75ea84187083: Pull complete 
88b459c9f665: Pull complete 
1e3ee139a577: Pull complete 
bd2400e7d880: Pull complete 
b8c6812ba469: Pull complete 
1882bd83135c: Pull complete 
Digest: sha256:873c24bdb94e9b198bbec3c298fb7b2dfc64bce7b2dea71348b62f47814e16fb
Status: Downloaded newer image for node:7.1
 ---> f5eca816b45d
Step 2 : RUN set -x     && sudo curl https://dl.yarnpkg.com/debian/pubkey.gpg | sudo apt-key add -     && echo "deb http://dl.yarnpkg.com/debian/ stable main" | sudo tee /etc/apt/sources.list.d/yarn.list     && sudo apt-get update && sudo apt-get install yarn
 ---> Running in a6b6c803751f
+ sudo curl https://dl.yarnpkg.com/debian/pubkey.gpg
/bin/sh: 1: sudo: not found
+ sudo apt-key add -
/bin/sh: 1: sudo: not found
The command '/bin/sh -c set -x     && sudo curl https://dl.yarnpkg.com/debian/pubkey.gpg | sudo apt-key add -     && echo "deb http://dl.yarnpkg.com/debian/ stable main" | sudo tee /etc/apt/sources.list.d/yarn.list     && sudo apt-get update && sudo apt-get install yarn' returned a non-zero code: 127
yongwen@yongwen-Thinkpad:~/work/scratch/yarn-ns$ docker build -t test .
Sending build context to Docker daemon 29.34 MB
Step 1 : FROM node:7.1
 ---> f5eca816b45d
Step 2 : RUN set -x     && curl https://dl.yarnpkg.com/debian/pubkey.gpg | apt-key add -     && echo "deb http://dl.yarnpkg.com/debian/ stable main" | tee /etc/apt/sources.list.d/yarn.list     && apt-get update && sudo apt-get install yarn
 ---> Running in 0e0b66e20a42
+ curl https://dl.yarnpkg.com/debian/pubkey.gpg
+ apt-key add -
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
100  7453  100  7453    0     0  19369      0 --:--:-- --:--:-- --:--:-- 19408
OK
+ tee /etc/apt/sources.list.d/yarn.list
+ echo deb http://dl.yarnpkg.com/debian/ stable main
+ apt-get update
deb http://dl.yarnpkg.com/debian/ stable main
Get:1 http://dl.yarnpkg.com stable InRelease [2464 B]
Get:2 http://dl.yarnpkg.com stable/main amd64 Packages [881 B]
Get:3 http://security.debian.org jessie/updates InRelease [63.1 kB]
Ign http://deb.debian.org jessie InRelease
Get:4 http://security.debian.org jessie/updates/main amd64 Packages [403 kB]
Get:5 http://deb.debian.org jessie-updates InRelease [145 kB]
Get:6 http://deb.debian.org jessie Release.gpg [2373 B]
Get:7 http://deb.debian.org jessie-updates/main amd64 Packages [17.6 kB]
Get:8 http://deb.debian.org jessie Release [148 kB]
Get:9 http://deb.debian.org jessie/main amd64 Packages [9064 kB]
Fetched 9847 kB in 4s (2072 kB/s)
Reading package lists...
+ sudo apt-get install yarn
/bin/sh: 1: sudo: not found
The command '/bin/sh -c set -x     && curl https://dl.yarnpkg.com/debian/pubkey.gpg | apt-key add -     && echo "deb http://dl.yarnpkg.com/debian/ stable main" | tee /etc/apt/sources.list.d/yarn.list     && apt-get update && sudo apt-get install yarn' returned a non-zero code: 127
yongwen@yongwen-Thinkpad:~/work/scratch/yarn-ns$ docker build -t test .
Sending build context to Docker daemon 29.34 MB
Step 1 : FROM node:7.1
 ---> f5eca816b45d
Step 2 : RUN set -x     && curl https://dl.yarnpkg.com/debian/pubkey.gpg | apt-key add -     && echo "deb http://dl.yarnpkg.com/debian/ stable main" | tee /etc/apt/sources.list.d/yarn.list     && apt-get update && apt-get install yarn
 ---> Running in 5dc048773191
+ curl https://dl.yarnpkg.com/debian/pubkey.gpg
+ apt-key add -
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
100  7453  100  7453    0     0  95426      0 --:--:-- --:--:-- --:--:-- 95551
OK
+ echo deb http://dl.yarnpkg.com/debian/ stable main
+ tee /etc/apt/sources.list.d/yarn.list
+ apt-get update
deb http://dl.yarnpkg.com/debian/ stable main
Get:1 http://dl.yarnpkg.com stable InRelease [2464 B]
Get:2 http://dl.yarnpkg.com stable/main amd64 Packages [881 B]
Get:3 http://security.debian.org jessie/updates InRelease [63.1 kB]
Ign http://deb.debian.org jessie InRelease
Get:4 http://security.debian.org jessie/updates/main amd64 Packages [403 kB]
Get:5 http://deb.debian.org jessie-updates InRelease [145 kB]
Get:6 http://deb.debian.org jessie Release.gpg [2373 B]
Get:7 http://deb.debian.org jessie-updates/main amd64 Packages [17.6 kB]
Get:8 http://deb.debian.org jessie Release [148 kB]
Get:9 http://deb.debian.org jessie/main amd64 Packages [9064 kB]
Fetched 9847 kB in 3s (2992 kB/s)
Reading package lists...
+ apt-get install yarn
Reading package lists...
Building dependency tree...
Reading state information...
Recommended packages:
  nodejs
The following NEW packages will be installed:
  yarn
0 upgraded, 1 newly installed, 0 to remove and 3 not upgraded.
Need to get 1817 kB of archives.
After this operation, 28.1 MB of additional disk space will be used.
Get:1 http://dl.yarnpkg.com/debian/ stable/main yarn all 0.17.0-1 [1817 kB]
debconf: delaying package configuration, since apt-utils is not installed
Fetched 1817 kB in 1s (1446 kB/s)
Selecting previously unselected package yarn.
(Reading database ... 21208 files and directories currently installed.)
Preparing to unpack .../archives/yarn_0.17.0-1_all.deb ...
Unpacking yarn (0.17.0-1) ...
Setting up yarn (0.17.0-1) ...
 ---> 7d0fb09b8fc4
Removing intermediate container 5dc048773191
Step 3 : WORKDIR /src/app
 ---> Running in 445c55f260da
 ---> f4364d2bb0a4
Removing intermediate container 445c55f260da
Step 4 : COPY package.json yarn.lock ./
 ---> c97301d73473
Removing intermediate container 1217afe9a60e
Step 5 : RUN yarn install
 ---> Running in 11685338da3d
yarn install v0.17.0
[1/4] Resolving packages...
[2/4] Fetching packages...
error An unexpected error occurred: "https://registry.yarnpkg.com/immutable/-/immutable-3.8.1.tgz: EINVAL: invalid argument, chown '/root/.cache/yarn/npm-immutable-3.8.1-200807f11ab0f72710ea485542de088075f68cd2/package.json'".
info If you think this is a bug, please open a bug report with the information provided in "/src/app/yarn-error.log".
info Visit https://yarnpkg.com/en/docs/cli/install for documentation about this command.
The command '/bin/sh -c yarn install' returned a non-zero code: 1

With this patch, the build will be successful.

bestander · 2016-11-15T15:05:32Z

src/constants.js

+export const ROOT_USER = isRootUser(process.platform, getUid());
+
+export function isRootUser(platform: string, uid: ?number): boolean {
+  return platform !== 'win32' && uid === 0;


do you need a second check for platform !== 'win32' here?
Would not null return false anyway?

I will remove the extraneous check.

bestander · 2016-11-15T15:06:16Z

Looks more elegant than cmodding all files after they are extracted.
Can you just address one comment? And let's merge it.

* Set UID and GID of untared files to zero if user is root * Remove second platform check

andrewmclagan · 2016-11-18T13:10:31Z

ah cool, going to run this on CircleCI on Monday as a number of users experience this issue.

* Set UID and GID of untared files to zero if user is root * Remove second platform check

amasad · 2016-12-01T23:27:47Z

Tried this on CircleCI and it works! (Thanks to @Daniel15) I hope this lands on stable soon.

Set UID and GID of untared files to zero if user is root

0a1bcb7

bestander reviewed Nov 15, 2016

View reviewed changes

bestander self-assigned this Nov 15, 2016

Remove second platform check

5b53597

bestander merged commit 92399c4 into yarnpkg:master Nov 16, 2016

ide mentioned this pull request Nov 16, 2016

Installing in a Docker container on CircleCI fails with "error <package url>: EINVAL: invalid argument, chown '/root/.yarn-cache/<package>/package.json'" #918

Closed

normanrz mentioned this pull request Nov 16, 2016

Yarn: new node package manager scalableminds/webknossos#1489

Merged

1 task

lawliet89 added a commit to lawliet89/yarn that referenced this pull request Nov 17, 2016

Set UID and GID of untared files to zero if user is root (yarnpkg#1837)

37cb230

* Set UID and GID of untared files to zero if user is root * Remove second platform check

daveisfera pushed a commit to daveisfera/yarn that referenced this pull request Dec 1, 2016

Set UID and GID of untared files to zero if user is root (yarnpkg#1837)

b10c4f0

* Set UID and GID of untared files to zero if user is root * Remove second platform check

frsyuki mentioned this pull request Dec 8, 2016

UI: Switch to Yarn treasure-data/digdag#411

Closed

fbarl mentioned this pull request Dec 14, 2016

Upgrade to npm v4.0.5 or switch to using yarn weaveworks/scope#2090

Closed

This was referenced Mar 8, 2017

yarn-e2e/label=docker,os=ubuntu-14.04 #123 failed #2866

Closed

removed entry.props.uid=0 for root after we switched from node-tar to tar-fs #2885

Closed

lawliet89 deleted the tar-root branch March 10, 2017 13:51

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Set UID and GID of untared files to zero if user is root #1837

Set UID and GID of untared files to zero if user is root #1837

lawliet89 commented Nov 15, 2016 •

edited

Loading

bestander Nov 15, 2016

lawliet89 Nov 16, 2016

bestander commented Nov 15, 2016

andrewmclagan commented Nov 18, 2016

amasad commented Dec 1, 2016

Set UID and GID of untared files to zero if user is root #1837

Set UID and GID of untared files to zero if user is root #1837

Conversation

lawliet89 commented Nov 15, 2016 • edited Loading

bestander Nov 15, 2016

Choose a reason for hiding this comment

lawliet89 Nov 16, 2016

Choose a reason for hiding this comment

bestander commented Nov 15, 2016

andrewmclagan commented Nov 18, 2016

amasad commented Dec 1, 2016

lawliet89 commented Nov 15, 2016 •

edited

Loading