bedrock $_SERVER var exposes db_name/user/password and salts

[X] I've read the guidelines for Contributing to Roots Projects
[X] This request isn't a duplicate of an existing issue
[X] I've read the docs and followed them (if applicable)
[X] This is not a personal support request that should be posted on the Roots Discourse community

Description

When working on a site, I noticed that if I was to error_log() the $_SERVER variable that the following additional items would be available (name/user/password removed for bug report):

[DB_NAME] => {db_name}
[DB_USER] => {user}
[DB_PASSWORD] => {userspwd}
[WP_ENV] => production
[WP_HOME] => https://domain.test
[WP_SITEURL] => https://wp-boilerplate.test/wp
[AUTH_KEY] => generateme
[SECURE_AUTH_KEY] => generateme
[LOGGED_IN_KEY] => generateme
[NONCE_KEY] => generateme
[AUTH_SALT] => generateme
[SECURE_AUTH_SALT] => generateme
[LOGGED_IN_SALT] => generateme
[NONCE_SALT] => generateme

Steps to reproduce

Create a local install, use any environment
simply var_dump() or error_log() the $_SERVER var
See results

Expected behavior: These sensitive data shouldn't be available via that variable

Actual behavior: These sensitive data shouldn't be able to output

Reproduces how often: 100%

Versions

Bedrock Install: 1.12.8: 2019-09-05 macOS: 10.14.6 laravel/valet 2.3.3

Additional information

Basic Sage 9 with Bedrocks install on local.

Oct 21 '19 19:10 kupoback

Update 2020-08-11:

Current syntax looks like this:

$repository = Dotenv\Repository\RepositoryBuilder::createWithNoAdapters()
    ->addAdapter(Dotenv\Repository\Adapter\EnvConstAdapter::class)
    ->immutable()
    ->make();

$dotenv = Dotenv\Dotenv::create($repository, $root_dir);

2019-10-19:

Looks like it would be a pretty straightforward change if we care to do this.

 /**
  * Expose global env() function from oscarotero/env
  */
 Env::init();
 
 /**
  * Use Dotenv to set required environment variables and load .env file in root
  */
-$dotenv = Dotenv\Dotenv::create($root_dir);
+$dotenv = Dotenv\Dotenv::create($root_dir, null, new Dotenv\Environment\DotenvFactory([
+    new Dotenv\Environment\Adapter\PutenvAdapter(),
+]));
 if (file_exists($root_dir . '/.env')) {
     $dotenv->load();
     $dotenv->required(['WP_HOME', 'WP_SITEURL']);
     if (!env('DATABASE_URL')) {
         $dotenv->required(['DB_NAME', 'DB_USER', 'DB_PASSWORD']);
     }
 }

https://github.com/vlucas/phpdotenv#loader-customization

Oct 21 '19 21:10 QWp6t

Would this also do it for the WP Salts?

Oct 21 '19 22:10 kupoback

+1 can also confirm when running php -i, the credentials are also shown.

Oct 22 '19 01:10 demyxco

Tested suggested fix and it removed the SALTS from showing as well.

Oct 22 '19 01:10 kupoback

Any Updates on this serious issue?

Dec 10 '19 13:12 petersafwat-flairs

There’s code a couple comments up if you want to test it https://github.com/roots/bedrock/issues/474#issuecomment-544714647

Dec 10 '19 13:12 austinpray

The above does work, but it should be a change merged into the repo for those that are cloning this for projects. This way they don't have to make a note to copy this solution each time.

Dec 10 '19 14:12 kupoback

Well we need a PR with this change 😄

@QWp6t want to do the honours?

Dec 10 '19 20:12 swalkinshaw