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

[5.7] Masking the content of .env on Whoops page #26947

Closed
wants to merge 3 commits into from
Closed

[5.7] Masking the content of .env on Whoops page #26947

wants to merge 3 commits into from

Conversation

wilkinsocks
Copy link

(This PR is derived from: #21336 and laravel/ideas#1236)

In this PR, the variable names from the .env file are loaded in and passed to the Whoops handler blacklist, which prevents displaying any data such as: database credentials, service keys, etc. I feel like most of the content from the .env should be hidden by default due to their nature.

Each time a key/value is added to the .env you don't have to repeat the key in the app config blacklist. You actually have to repeat it twice currently, as it appears in $_ENV and $_SERVER.

Adding debug_env_blacklist = true to the app config automatically hides all the variables — I thought opt-in would make more sense for 5.7.

The previous debug_blacklist config key has been retained to allow other variables not in the .env to be hidden. Additionally, a debug_whitelist facility has been added to allow blacklist exceptions which overrides the functionality for that key.

image

@wilkinsocks wilkinsocks changed the title Masking the content of .env on Whoops page [5.7] Masking the content of .env on Whoops page Dec 24, 2018
@GrahamCampbell
Copy link
Member

Why do we need to mask the variables? Surely they are useful for debugging, to see if the correct configuration is there?

@xylonpsl
Copy link

xylonpsl commented Feb 5, 2019

@GrahamCampbell It's useful for some environments which are publicly exposed. An option to hide the sensitive data from being exposed. I wanted to integrate the package for our dev and stage environments but our dev ops guy isn't agreeing to install the package because the same data is exposed.

@tontonsb
Copy link
Contributor

I'll just plug my package here. Let's hope Whoops fix this behaviour some day...

@GrahamCampbell
Copy link
Member

@xylonpsl I would not recommend using debug mode on any public facing website, or even an internal facing one accessible to anyone other than the developers.

@GrahamCampbell
Copy link
Member

GrahamCampbell commented Feb 16, 2019

Perhaps instead look at integrating with error monitoring software like https://bugsnag.com/?

@tontonsb
Copy link
Contributor

@GrahamCampbell the debug mode is opt-out not opt-in. It shouldn't be the default behaviour to dump database credentials when visiting /logout.

I find PrettyPageHandler really useful and want to keep using it. I have no issues with random people seeing my scripts. I just don't want the public to see my ENV variables. And I don't need them dumped in the error page either. I have never needed to look at those for any debugging.

@wilkinsocks
Copy link
Author

I closed this because when I considered it, it isn't actually required, and probably pointless for Laravel... 😅

On a production environment, run composer install --no-dev to install dependencies. This doesn't install Whoops (and certain other packages).

It should also be noted that you can actually mask these variables already, if you so required. See: https://stackoverflow.com/a/46407010

@JohnnyWalkerDigital
Copy link
Contributor

JohnnyWalkerDigital commented Feb 28, 2021

@wilkinsocks You were wrong to close this! You were right in your first post. This is an easy, and potentially devastating, gotcha. Especially for those new to Laravel. As evidenced by the frequency of this issue appearing online:

https://stackoverflow.com/questions/46407009/how-to-hide-env-passwords-in-laravel-whoops-output
https://stackoverflow.com/questions/56741230/how-to-remove-the-environment-variables-from-laravel-debug
https://www.reddit.com/r/laravel/comments/9x9cyi/why_is_laravel_showing_all_env_variables_on_every/
filp/whoops#340

I even came across a live website with this very issue. Ironic given the content:
https://www.helplazy.com/blog/how-to-hide-env-passwords-in-laravel-whoops-output

Not revealing a database password to the public relies on the dev never forgetting to set debug to false or adding --no-dev to composer. Human error happens, especially when a developer is stuck in the office at 3am trying to debug code to meet a deadline. Laravel should have the dev's back.

Even the included blacklist feature is flawed because it requires you to remember to manually add any new .env variables to it. The current configuration is the opposite of how security is handled today, where "no access" is supposed to be the default. You were so right!

While Ignition doesn't have these problems, it just underlines how pointless it is to have it in Whoops display all the environment variables at all. Unfortunately I believe Whoops can still display in Laravel 8 in the right conditions?

Even more of a reason for you to update this PR.

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

Successfully merging this pull request may close these issues.

5 participants