search

lorisleiva / laravel-docker

🐳 Generic docker image for Laravel Applications
MIT License
934 stars 314 forks source link

[Discussion] Composer security issue #29

Closed leckylao closed 4 years ago

leckylao commented 5 years ago

I propose change composer install from root to normal user as recommended by official composer https://getcomposer.org/doc/faqs/how-to-install-untrusted-packages-safely.md

From:

# Install composer ENV COMPOSER_HOME /composer ENV PATH ./vendor/bin:/composer/vendor/bin:$PATH ENV COMPOSER_ALLOW_SUPERUSER 1 RUN curl -s https://getcomposer.org/installer | php -- --install-dir=/usr/local/bin/ --filename=composer

To: # Install PHP Composer RUN curl -sS https://getcomposer.org/installer | php -- --install-dir=/usr/local/bin --filename=composer

The only impact would be PHPCS which is optional and include be included into composer.json

lorisleiva commented 5 years ago

Hi there 👋

You're suggesting 2 changes:

  1. Removing the COMPOSER_HOME /composer option that explicitly defines the global folder composer will use to install itself. I'm not sure if that part was intentional but this does not represent a security issue and is used on the official composer Dockerfile.
  2. Removing the COMPOSER_ALLOW_SUPERUSER 1 option which is recommended by the official composer documentation. However, this option is also used on the official composer Dockerfile and widely used by the Docker community when installing composer in a docker container. There should be a reason for that option to be true by default and I'm worried that turning it off will be a breaking change for the current users of Laravel Docker. If you can shine more light on the consequences of this change I might consider it.

Thanks for taking the time to create this issue.

lorisleiva commented 4 years ago

Closing this due to inactivity. Feel free to reopen.