Skip to content

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

Open
anthony-o wants to merge 1 commit intoapi-platform:mainfrom
anthony-o:issue-1847
Open

Fixes #1847 "Docker: Launch the php image with a non root user"#2189
anthony-o wants to merge 1 commit intoapi-platform:mainfrom
anthony-o:issue-1847

Conversation

@anthony-o
Copy link
Copy Markdown

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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown

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.

4 participants