consider load-and-delete option for environment variables

[markstos]

Environment Variables Considered Harmful for Your Secrets raises some good points about using environment variables for configuration. You've likely seen an error page, log or alert email which contains a full environment dump.

What about having an option to delete environment variables after loading them into node-config? This would be safe for values that are exclusively accessed through node config after that, and not directly through the environment. It seems sensible to standardized on exclusively using node-config for configuration access if it is being used in most places.

If some tool or library required configuration-by-environment, the specific environment variable could be set just before calling it and deleted just after the call completed.

A global option for this could be useful, but per-variable inclusions or exclusions might also be required. On the other hand, if a variable is being set in the environment because something requires it there, then node-config need not be involved and thus no exception would need to be made in node-config.

[lorenwest]

This is a good proposal. Variables designed for node-config would be the ones that node-config would remove, and in v.1.x there could be an opt-in variable for this, while in 2.x the variable could be opt out.

[markstos]

There are a couple layers of environment variables to consider here. First are the ones that are likely for exclusive use of node-config:

• NODE_CONFIG_DIR
• NODE_CONFIG
• ALLOW_CONFIG_MUTATIONS
• NODE_CONFIG_STRICT_MODE
• SUPPRESS_NO_CONFIG_WARNING

These all seem fairly safe to scrub, with the NODE_CONFIG variable being the most important due the increased likelihood of it containing sensitive data.

A second layer of related environment variables are defined in custom-environment-variables.json.
These could also be really valuable to scrub, perhaps containing a database connection string or an API key.

On the other hand, someone may be using environment variables and hooking them up precisely because the environment variables need to be accessed from another location besides node-config and we have no awareness of whether that is happening or not.

However, to be clear: We are not deleting the environment variables everywhere, we simply can't delete from the parent process. The deletion affects only the current JavaScript process.

If for some reason node-config can't be used directly by code in the current process, a helper function can be used to set the environment variable for a minimal scope. mock-env does this for a general case, but we could have a helper that integrates with node-config and the custom-environment-variables.json file. Maybe something like:

    // Run with full original un-scrubbed environment.
     config.runWithEnv(my.function);

     // Run with just the env vars corresponding to these variables:
     config.runwithEnv(my.function. [ 'mongo.connectStr','s3.apiKey']);

[lorenwest]

Scrubbing node-config related environment variables after loading them into config seems harmless. If someone puts something in $NODE_CONFIG or any variable defined in environment-variables.json, they're doing it for the purposes of loading them into config.

Making this behavior opt-in prevents accidental scrubbing, and allowing opt-out in v2.0 is appropriate for those that require access to those variables outside the scope of config.

Adding support such as runWithEnv seems un-necessary since they can use mock-env, or just set the environment variables themselves and get the same results.

[markstos]

That all sounds good, thanks for the feedback.

[willsr]

+1

[markstos]

Thanks for interest, @willsr We're happy to consider a PR if you are motivated to work on one.

[markstos]

@lorenwest the security benefit of this is weaker than I first realized, after finding the that the complete environment used to launch the process is available in /proc/$pid/environ on Linux. If the process deletes one of the variables from it's environment, it will not be reflected there. This is discussed more here:

http://serverfault.com/a/79463/63268

There is still /some/ benefit to the load-and-delete pattern of environment variables. It prevents accidentally exposing them further when humans or tools dump the Node.js process.env structure.

[lorenwest]

You're welcome to close this. If your code isn't secure this is the least of your concerns.

[gtramontina]

Did this move any further?

[markstos]

@gtramontina No. The benefits are are less clear, since /proc/$pid/environ still contains the original environment. Perhaps is some container contexts this file isn't made available, so there could still be some security benefit. I'm not opposed to the feature, I'm just not motivated to work on it. :)

[markstos]

Related: #602 proposing masking sensitive values.