Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Specifying USER in cypress:base image causes trouble for downstream #10

Closed
bencao opened this issue Sep 13, 2017 · 14 comments
Closed

Specifying USER in cypress:base image causes trouble for downstream #10

bencao opened this issue Sep 13, 2017 · 14 comments
Assignees

Comments

@bencao
Copy link

bencao commented Sep 13, 2017

Just noticed that recently we changed the default user to 'person' for cypress/base:8 image.

RUN chown person /usr/local/lib/node_modules
# give new user permission to install global tools
RUN chown person /usr/local/bin
# now run as the new "non-root" user
USER person
WORKDIR /home/person

The initial motivation is good but effectively creates problems downstream.
Imagine a typical Dockerfile for a cypress project:

FROM cypress/base:8

# default /home/person doesn't make sense as workdir 
# since some hidden files with dot prefix may implicitly changed user behaviors
WORKDIR /home/person/cypress

# copied file owner will be root
COPY . /home/person/cypress

# have to switch back to root user or the next chown command would fail
USER root
RUN chown -R person /home/person/cypress

# switch back
USER person

RUN npm install

CMD ["npm", "test"]

As the above Dockerfile shows, it is really chaotic with the addition of USER person.

I would propose leaving the safety considerations back to user land, giving recommendations instead of dictating a user implicitly in a base image.

Without the USER person it could be like this:

FROM cypress/base:8

# run as non-root user inside the docker container
# see https://vimeo.com/171803492 at 17:20 mark
RUN groupadd -r regular-users && useradd -m -r -g regular-users person

# give new user access to global NPM modules folder
RUN chown person /usr/local/lib/node_modules
# give new user permission to install global tools
RUN chown person /usr/local/bin

WORKDIR /home/person/cypress
COPY . /home/person/cypress
RUN chown -R person /home/person/cypress

# run as person afterwards
USER person

RUN npm install
CMD ["npm", "test"]
@bahmutov
Copy link
Contributor

What is the problem this is causing? We specifically made a separate user so we could support global installation of Cypress inside the Docker container. Are you saying we could just use the default user node provided by Node8 base image?

@bencao
Copy link
Author

bencao commented Sep 13, 2017

The main problem is the single line in current cypress/base:8 Dockerfile:

USER person

Please imagine that any cypress project which will be dockerized will have to do one thing, that is adding its own project files into the new image. So the Dockerfile for the cypress project will look like this in minimum

FROM cypress/base:8

WORKSPACE /some/where/to/store/the/project/files

COPY . /some/where/to/store/the/project/files

RUN npm install

CMD ["npm", "run", "cypress:run"]

Since right now in cypress/base:8 we already specified "person" as the user, so "RUN npm install" will be run as "person", who unfortunately has no write permission to "/some/where/to/store/the/project/files" because the directory owner is root (all directories COPYed will belongs to root user), it will fail with permission errors.

In order to overcome the permission issue, we have to switch USER back and forth between root and person, which resulted the complicated Dockerfile example

COPY . /some/where/to/store/the/project/files

# switch back to root user before chown
USER root
RUN chown -R person /some/where/to/store/the/project/files

# switch back to person
USER person

RUN npm install

Did I clearly express the problem now?

@bencao
Copy link
Author

bencao commented Sep 13, 2017

https://github.com/nodejs/docker-node/blob/2924f142789842282890f7b1736578b49b3be78f/8.5/Dockerfile

nodejs Dockerfile created "node" user, but it doesn't say "USER node", I think it is based on the same idea, to minimize impacts to downstream projects who build their images from base node images.

@bahmutov
Copy link
Contributor

I think I switched to user to install to avoid doing pretty much anything as a root inside the container, don't you agree?

@bencao
Copy link
Author

bencao commented Sep 16, 2017

That's a good intention, but the reality is (even you have specified a non-root USER), the 'COPY' command in Dockerfile will always create files which owner is 'root' user. If a copied folder is owned by root, any write operation by person will be failed for permission issue. So does the above samples have to switch back and forth to change owner of the copied folder to person.

@samtgarson
Copy link

samtgarson commented Oct 28, 2017

I'm hitting this issue as well with Drone CI. Here's an example issue (not mine) that explains the problem https://discourse.drone.io/t/cannot-create-root-netrc-permission-denied/969

Haven't had time to think enough about a solution yet but just posting to add my real world use case!

Have you guys had any more thoughts about this?

@bahmutov
Copy link
Contributor

bahmutov commented Oct 28, 2017 via email

@samtgarson
Copy link

samtgarson commented Oct 28, 2017

IMHO opinion that should be left up to the user? Increased security is good but not at the cost of causing issues for a segment of use cases, and it can still be the recommended approach from Cypress to switch User when running tests?

I'm obviously happy to build my own image if you feel you wouldn't want to change but just my 2¢ 😄

@jagregory
Copy link

Just chiming in to say this is causing issues for me too. As @bencao mentioned, any files COPYd into an image is owned by root, so trying to run Cypress in that directory fails with EACCES: permission denied.

@bahmutov
Copy link
Contributor

Ok, we will remake the images using default root user - but this means every user is responsible for ensuring the Docker image is not compromised of course.

bahmutov added a commit that referenced this issue Oct 31, 2017
@samtgarson
Copy link

Thanks! Absolutely—by all means I think that should be stated somewhere as advice 👍

@bahmutov
Copy link
Contributor

I have generated new cypress/base:8 Docker image with "simple" default settings, should be good now.

@itskingori
Copy link

If anyone finds themselves having issues trying to run cypress in Docker as a non-root user, this (cypress-io/cypress#1281) maybe of interest to you ... and possibly why it's difficult to do so.

@msparer
Copy link

msparer commented Mar 6, 2018

While I understand the decision, I'd like to add that removing the user "person" from the base image was a kind of a surprising move since building our images suddenly started to fail.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants