Allow read-only mode

[barryvdh]

There has been some discussion about using putenv() and setting the $_ENV/$_SERVER vars, related to security and threads, eg #76 and #124, laravel/framework#8187 and Twitter: https://twitter.com/weltling/status/700036432761659392 etc.

Those commonly revolve about setting (sensitive) data in the Environment variables. While usage in production is discouraged (through a small note in the end of the readme), it doesn't mention anything about security issues (like leaking data to other processes or turning up in dumps)

My guess is that this library is mainly used to READ files from both environment vars and the .env file, but for many cases doesn't really care about putting stuff back. So why not add a 'read only' mode:

• Loading vars loads all data in an internal cache, instead of putting them in Env vars
• Finding a var will use the cache as first source (eg dotenv('DB_PASS') instead of getenv('DB_PASS'))

(Similar to what @progmars suggested in #76 (comment) but also not using the putenv anymore).

This could be an optional flag, just like immutable.

[barryvdh]

See this issue here about putenv: https://bugs.php.net/bug.php?id=71607 and the commenter here: http://php.net/manual/en/function.putenv.php#8496

So the issue has 2 parts:

• putenv leaks vars to system processess, which could be unwanted.
• $_ENV/$_SERVER can be printed by accident on wrongly configured systems.

Removing putenv() would prevent the first issue, but if you want to avoid the second case also, we need to remove that as well.

[tuupola]

putenv leaks vars to system processess, which could be unwanted.

This is how environment works on *NIX systems. It is a feature, not a bug.

There is nothing especially dangerous in storing sensitive information into environment than compared storing it somewhere else. Yes, if you dump your environment the information can be seen. In the same way if you dump stack trace, for example PDOException, the database credentials will be visible. It does not matter where you originally store them.

While usage in production is discouraged

This is about using an .env file in production because it causes extra disk access, and not about using environment variables in production. Latter is quite common and is actually encouraged.

[weltling]

@tuupola your conclusions are wrong in a lot of ways.

• stable system is not about a potentially compromised system
• dumps you mention are not relevant in production
• "lib to use in dev only" is not about a "factor III in the 12 factor concept" and for sure is not suitable for use in a general purpose lib, or the doc is wrong

Furthermore

• positive thinking in security is a dead end
• ignoring portability issues is at least not far-seeing, even it's nice in theory
• performance must not have impact on usability

Just my 12 cents.

Thanks.

[tuupola]

Please do not claim I have said something which I have not. Conclusions are your own.

stable system is not about a potentially compromised system

What part of my comment is this referring to?

dumps you mention are not relevant in production

OP specifically wrote about accidentaly printing / dumping info on production.

"lib to use in dev only" is not about a "factor III in the 12 factor concept" and for sure is not suitable for use in a general purpose lib, or the doc is wrong

Let me try again. README has the following sentence.

phpdotenv is made for development environments, and generally should not be used in production.

This is not to imply that using environment variables in production is not recommended. It implies that using the phpdotenv library itself in production is not recommended. Using environment variables to store configuration is quite common practice and sometimes even recommended such as in The Twelve-Factor App.

[barryvdh]

Yes, there is a note about production usage, bit this is the next line:

In production, the actual environment variables should be set so that there is no overhead of loading the .env file on each request.

To me, this clearly suggests the main reason not to use it in production, is for performance.

#76 (comment)

Dotenv was never meant to be used in production. I even say this explicitly in the README, but many many people continue to use it in production anyways. The entire point of dotenv is to emulate set environment variables for local development and testing, to get them out of your source control system. This is why you don't check-in the file - it's not even supposed to exist or be parsed in production. These environment variables should already be set and ready to be used on your server (in the Apache/nginx process, for instance). I have never supported or recommended using dotenv in production, and I myself do not use it in production. On production, I set my ENV vars in either in my nginx virtual host config, or define them in the control panel of a cloud hosting provider like Heroku, etc. This is the way it was meant to be done.

This comment says otherwise, but then it should be a lot clearer:

This is how environment works on *NIX systems. It is a feature, not a bug.

Yes, but in cases we don't want that 'feature', the option I suggested can be used..

[weltling]

@tuupola there is no claim, let me reword it. You're making a positive assumption that if you "always" do putenv, it is "always" safe. It is simply not true. It might be a concept of Xfactors that are suitable "always" in some programming language like say Java or C#, or suitable "always" in some environment like partiall in non thread safe PHP SAPI, or in some particular application. But when it comes to practice in a general PHP usage - here you are.

This ticket is exactly about this, because other libraries indeed use Dotenv in production, and doing so equals using putenv in production, and using putenv in a PHP generic library the way it's used in Dotenv is not flawless. Now, the concept of 12 factors is nice and that's not the point to be discussed. But in the exact point III of it, it is not fully safe in PHP and there is no point to defend the opposite.

Dotenv was never meant for production. Implementing this ticket it probably could get suitable for production, if the Dotenv project wants to. That's the only point which not necessarily meats the goals of the authors and their willingness to support and maintain such a feature. As far as @barryvdh's quote tells, it doesn't look like the case.

Thanks.

[tuupola]

Again please do not claim I have said something which I have not. I have used exactly zero (0) times words "putenv" and "always". I have been talking about environment variables and better explained the wording of README sentence about production warning.

Apparently there is some kind of language barrier.

[weltling]

@tuupola

putenv leaks vars to system processess, which could be unwanted.

This is how environment works on *NIX systems. It is a feature, not a bug.

Well, that's is a word game :) I just stop here, as seems instead of being factual you're just trying to suppress the issue. The ticket exists, it's up to the library owners whether to implement it.

Thanks.

[progmars]

My two cents.

TL;DR:
if Dotenv is meant only for local development machines, I want it to work reliably on every typical dev machine (including Windows) out there.

---

Yes, Dotenv was not meant to be used in production, but currently it doesn't work reliably even on local development machines when using some prepackaged WAMP stacks, which often come with PHP running as Apache module.

Debugging showed that if I load an app with lots of concurrent requests, in setEnvironmentVariables at the point:

if ($this->immutable && $this->getEnvironmentVariable($name) !== null) {

getEnvironmentVariable is actually returning a value although it was not yet populated into all three env, $_ENV and $_SERVER.

And then later when getenv() is called from some other code (e.g. Laravel's config files and env helper), the value is suddenly gone. Why? Most probably, because it was set from another request, and when that concurrent request ended, the env variable also was gone. Not sure, when how and why PHP clears the leaked variables away, but obviously it does so. I tried multiple error_logs in all the places imaginable, and it clearly showed that while the variable was available in getenv in getEnvironmentVariable call, sometimes it wasn't there anymore at the time when Laravel loaded config files.

To summarize, here's how it fails on my local system:

• getEnvironmentVariable tells the variable was found and as we are immutable, Dotenv skips the update of local putenv, $_ENV and $_SERVER values
• the concurrent thread finishes its job, PHP clears away some stuff
• Laravel loads config, calls getenv - no variables anymore, defaults assumed
• default config values are mostly empty to be environment-agnostic, so server crashes with 500 because some class receives empty config value.

On my and some of my colleagues' development machines it is enough to have Laravel's Debugbar enabled - when it loads its stylesheet and javascript files, every 50th or so (depending on multiple factors) request ends up with error 500 because Laravel failed to getenv() the correct APP_KEY from .env file.

I tried to add a patch in Laravel's env() helper to fetch the value also from $_ENV and $_SERVER if it fails to find it through getenv, but this patch also turned to be unreliable because Dotenv doesn't even try to update $_ENV and $_SERVER when it detects the (presumably, leaked from another thread) variable when reading it through getenv() in getEnvironmentVariable, because of $this->immutable check.

Now I have two approaches which seem to work reliably:

1. ignore getenv(), $_SERVER, $_ENV values and rely only on Dotenv's local cache array. Of course, this would be a breaking change and every user of Dotenv should be warned that Dotenv Loader's getEnvironmentVariable should be used instead of getenv.

2. try to sync Dotenv's local cache array to getenv, $_SERVER, $_ENV values. In cases if the values are leaked from other threads, we are propagating the leak into Dotenv. But at least we try to make Dotenv consistent with this flaky but somewhat known PHP behavior.

While 1) option is cleaner, it would break dependencies. I went for option 2) which now works reliably on my local Windows development environment.
I don't feel competent enough for forking and pull requests. I already had a patch on Gist for Dotenv 1.1.0 and Laravel 5.0, however it doesn't work since Dotenv 2 and Laravel 5.2, therefore I'll publish a new quick-n-dirty diff patch in upcoming days.

[vlucas]

@progmars Thanks for digging into this and providing the full diagnosis of what is going on. Sucks to have to patch around what seems like a mainly PHP issue, but it is what it is. I look forward to your patch, and will look into this some myself as well over the next few days.

[progmars]

Here it is:
for Dotenv:
https://gist.github.com/progmars/1e545d96dd48676a2aa7

and for Laravel to benefit from Dotenv's patch:
https://gist.github.com/progmars/5c18235360c04b621ed7

I still don't have a 100% recipe to reproduce the issue reliably, but I can tell how I can reproduce it a few times in a row on my dev machine.

At first, I have Windows 10 64 bit machine with a pretty old XAMPP installation which I have later manually updated to Apache 2.4 and PHP 5.6 (32 bit). Also I have XDebug enabled (using Netbeans to debug).

Here are some relevant fragments from my phpinfo:
https://gist.github.com/progmars/f0d222239043e9fbc639

I have created a typical Laravel application and have added "barryvdh/laravel-debugbar" "version": "v2.1.1"

To force everyone to use unique app_key, in app.config I have set
'key' => env('APP_KEY', 'YouForgotToSetAppKey')

In .env file I have specified valid APP_KEY value.

In debug mode, we have lots of unconcatenated, unminified Javascript and CSS files included from vendors folders and our own assets.

The best way for me to reproduce the issue is to add the following code to my resources/views/layouts/master.blade.php:

        <script type="text/javascript">
            // Dotenv stability test when multiple concurrent requests
            // run with disabled cache
            setInterval(function(){

                $.ajax("_debugbar/assets/stylesheets?1455396160");

            }, 250);
        </script> 

and set my browser to ignore cache (Chrome has Disable cache checkbox in its F12 dev console).
Without my patches, in one minute I receive tens of failed requests with 500 and 404 errors. If I put a breakpoint in exception handler, I see that it's Laravel's Encrypter failing because it received YouForgotToSetAppKey instead of the real key from .env file. With patches applied, I can leave the test running for hours without any failures.

[mdurao]

Were this patches added to dotenv and Laravel?