Using clear-env

[jasonmccallister]

We are currently setting PHP-FPMs clear-env to yes which limits what environment variables are exposed to php-fpm and in-turn the Craft application. This can lead to some headaches on "why is the environment variable not showing up" but this is also a good security feature as developers can sometimes leave phpinfo pages exposed to the public - which will expose all environment variables be default.

I think we should consider the following:

1. Keep clear-envas the current value of yes
2. Provide clear documentation and steps on how to add environment variables (in the README)
3. Add a startup script that checks for an env var and changes the setting (e.g. CRAFT_PHP_CLEAR_ENV=no)

The goal is to add more behaviors based on environment variables in the future (e.g. running the queue worker, running the up command ,and etc)

[timkelty]

I see both sides, but my gut says defaulting to clear-env=yes feels like a bit of a strong-arm tactic. We're basically making it harder to set env variables, because we "don't trust users". That has merits, but I think I'd lean in the other direction, and just document how exposing env variables can be dangerous, and you can toggle the behavior to set clear-env=yes if you desire.

[jasonmccallister]

feels like a bit of a strong-arm tactic. We're basically making it harder to set env variables, because we "don't trust users".

I think it's less about not trusting the users and more about providing a secure starting point to protect the users out of the box. If we look at how Alpine starts - there are no extra packages to reduce the attack surface... You then add what you want on top of the image. I know this is not quite the same thing but the approach applies.

I think leaving clear-env as yes is the best approach, just because it's not common does not mean it's not a good and secure practice. If there is a way to override the option easily using an environment variable, then its easy to build a secure image first - and remove the guard rails second.

[timkelty]

Yeah I’m mostly just playing devils advocate, it’s prob a good idea.

I’d be curious what would the process look like for setting vars you intentionally want php to get.

[timkelty]

My initial thought/argument was "it isn't a security concern if they don't enter and env vars, and if they do, they want them read by php".

But I'm realizing that's probably oversimplified, because of all the system env that exists aside from anything the user wants to inject.

[jasonmccallister]

The readme talks about how to add environment variables https://github.com/craftcms/image#environment-variables

[timkelty]

I've changed my tune, I'm on board with it.

[angrybrad]

phpinfo would be the most common, right? but this would also stop people from exposing env vars if they uploaded a test.php with dump($_ENV);?

[timkelty]

yeah, they woudn't be passed to php-fpm at all

[jasonmccallister]

We are moving forward with the clear-env option.

[jasonmccallister]

now we are not :)