-
-
Notifications
You must be signed in to change notification settings - Fork 632
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
Allow read-only mode #163
Comments
See this issue here about So the issue has 2 parts:
Removing |
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
This is about using an |
@tuupola your conclusions are wrong in a lot of ways.
Furthermore
Just my 12 cents. Thanks. |
Please do not claim I have said something which I have not. Conclusions are your own.
What part of my comment is this referring to?
OP specifically wrote about accidentaly printing / dumping info on production.
Let me try again. README has the following sentence.
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. |
Yes, there is a note about production usage, bit this is the next line:
To me, this clearly suggests the main reason not to use it in production, is for performance.
This comment says otherwise, but then it should be a lot clearer:
Yes, but in cases we don't want that 'feature', the option I suggested can be used.. |
@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. |
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. |
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. |
My two cents. TL;DR: 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
And then later when To summarize, here's how it fails on my local system:
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 I tried to add a patch in Laravel's Now I have two approaches which seem to work reliably:
While 1) option is cleaner, it would break dependencies. I went for option 2) which now works reliably on my local Windows development environment. |
@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. |
Here it is: and for Laravel to benefit from Dotenv's patch: 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 I have created a typical Laravel application and have added To force everyone to use unique app_key, in app.config I have set In 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
and set my browser to ignore cache (Chrome has |
Were this patches added to dotenv and Laravel? |
@barryvdh This is now possible in V3. Example 1The following example assumes This example will load the <?php
use Dotenv\Dotenv;
$dotenv = Dotenv::create($path);
$variables = $dotenv->load(); Example 2The following example assumes Similar to the first example, only this time we avoid calling any non-thread-safe functions. <?php
use Dotenv\Environment\Adapter\EnvConstAdapter;
use Dotenv\Environment\Adapter\ServerConstAdapter;
use Dotenv\Environment\DotenvFactory;
use Dotenv\Dotenv;
$factory = new DotenvFactory([new EnvConstAdapter(), new ServerConstAdapter()]);
Dotenv::create($path, null, $factory)->load(); EXAMPLE 3The following example assumes This example will load the <?php
use Dotenv\Environment\Adapter\ArrayAdapter;
use Dotenv\Environment\DotenvFactory;
use Dotenv\Dotenv;
$dotenv = Dotenv::create($path, null, new DotenvFactory([new ArrayAdapter()]));
$variables = $dotenv->load(); EXAMPLE 4The following example assumes Once again, this example will not mutate your actual environment, allowing you to inspect the contents of an env file in isolation. <?php
use Dotenv\Environment\Adapter\ArrayAdapter;
use Dotenv\Environment\DotenvFactory;
use Dotenv\Loader;
$loader = new Loader([], new DotenvFactory([new ArrayAdapter()]));
$variables = $loader->loadDirect($content)); |
NB In the case of laravel, this issue is circumvented by the config cache command. |
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:
dotenv('DB_PASS')
instead ofgetenv('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.
The text was updated successfully, but these errors were encountered: