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

Fixes #1847 "Docker: Launch the php image with a non root user" #2189

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

anthony-o
Copy link

Q A
Branch? main for features / current stable version branch for bug fixes
Tickets #1847
License MIT
Doc PR api-platform/docs#...

This PR will launch all the commands as the host user (at least, the owner id of file 'composer.json') which is convenient to be able not to generate files as "root" on the host sources : the developer using the api-platform tarball will be able to modify those files without chowning them.

One should also modify the documentation at https://api-platform.com/docs/distribution/#plugging-the-persistence-system in order to change all the docker-compose exec references to docker-compose exec -T -u $(id -u) so that all the files created will be with the host's user id and not root.

Copy link
Contributor

@vincentchalamon vincentchalamon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @anthony-o,

Thanks for your contribution! You pointed something important related to security, it's always good to talk about it and improve it. Thanks!

For security reasons, it's recommended to run a Docker container using another user than root. Your approach only concerns some commands, and requires to pass an option on any docker compose exec command, which can be error prone.

I recommend to define a non-root user in the Dockerfile, by default using uid/gid 1000 (which is the most common uid/gid of the default user on many os) which can be overriden through Docker build-args, and set this user as default on the Dockerfile. Doing so will configure and run any file and command from this user, and prevent security issues.

But beware cause this can also implies some issues like permissions on volumes, etc.

Copy link
Member

@dunglas dunglas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if the files shouldn't be owned by the built-in user www-data?

# first arg is `-f` or `--some-option`
if [ "${1#-}" != "$1" ]; then
set -- php-fpm "$@"
set -- su-exec $USER_ID php-fpm "$@"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This isn't necessary. The official image is already designed to run FPM processes as non-root: docker-library/php#70

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes but I need to run the PHP script I developed as my current user ID when developing (for example to have read & write access to my folders) and by default it is running as another user id, so the files created when I execute my PHP script are not accessible by my current user ID (as a developer). This is a problem while developing... and this is why I wrote this patch.

@dunglas
Copy link
Member

dunglas commented Aug 3, 2022

See also: Sylius/Sylius-Standard#242 (comment)

The current situation seems to be the best compromise we can make.

@anthony-o
Copy link
Author

See also: Sylius/Sylius-Standard#242 (comment)

I agree with this comment and this is why I want to patch the entrypoint to take into account a given env variable containing the user id rather than defining it directly in the Dockerfile.

@thePanz
Copy link

thePanz commented Dec 6, 2022

@anthony-o I am trying to apply those changes to the dunglas/symfony-docker repository too, good progress so far: files uploaded in the application (on a mounted volume) are now owned by my 'local' user.

Some other issues I encountered:

  • group is not (yet) applied → I have some changes for that
  • CLI commands (run after loggin into the container) are still run as root → here I am stuck

Any hints on the second point? would be great to pair/collaborate on this and have a solid solution

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

Successfully merging this pull request may close these issues.

None yet

4 participants