Stripping environment variables from the HttpInterface #2895
Conversation
MichielDeMey
changed the title
lib/HttpInterface.js
Outdated
| @@ -46,6 +46,17 @@ function startWebServer(pm2) { | |||
| processes: list | |||
| }; | |||
|
|
|||
| if (cst.WEB_STRIP_ENV_VARS === true) { | |||
| for (var i = data.processes.length - 1; i >= 0; i--) { | |||
| var process = data.processes[i]; | |||
There was a problem hiding this comment.
can you use another variable name? This is a global nodejs variable.
There was a problem hiding this comment.
Good point, I'll use proc.
constants.js
Outdated
| @@ -80,6 +80,7 @@ var csts = { | |||
| DEBUG : process.env.PM2_DEBUG || false, | |||
| WEB_IPADDR : process.env.PM2_API_IPADDR || '0.0.0.0', | |||
| WEB_PORT : parseInt(process.env.PM2_API_PORT) || 9615, | |||
| WEB_STRIP_ENV_VARS : process.env.PM2_WEB_STRIP_ENV_VARS || true, | |||
There was a problem hiding this comment.
I'm not sure about the default value for this, might break some implementations. WDYT @vmarchaud, shouldn't we set this to false by default so that things remain unchanged?
There was a problem hiding this comment.
Not sure about some internal APIs, but yeah this might introduce a breaking change. I simply opted for the "secure by default" option, but we might want to set this to false in case it does break backwards compatibility.
There was a problem hiding this comment.
@soyuka The behavior shouldn't be changed yes, by default we should put it as false.
There was a problem hiding this comment.
Should I rename the ENV var then as well? Suggestions welcome. :)
|
CI seems to fail on Node 6, known issue? |
|
Nothing related imo. |
|
|
||
| // Strip important environment variables | ||
| if (typeof proc.pm2_env === 'undefined' && typeof proc.pm2_env.env === 'undefined') return; | ||
|
|
There was a problem hiding this comment.
Maybe change 'return' to 'break;
http request will not be ended if return from here.
* Fixes 452 * Renamed to `proc` * Default to false to keep backwards compatibility
Please always submit pull requests on the development branch.
Re-submit of #459
Let's try not to wait 3 years this time. 馃槈