Node: `Object.freeze` can't be used with `process.env`

[johnhamelink]

• Version: 10.17.0 & 13.3.0
• Platform: Linux john-hamelink-thinkpad 5.0.0-37-generic #40~18.04.1-Ubuntu SMP Thu Nov 14 12:06:39 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux
• Subsystem: process

I'm inheriting a legacy project where process.env was abused and effectively used in place of global. As part of my remedy for this problem, I'm looking to make it impossible for devs to use this pattern in future without coming across an error. Looking through the documentation, it seemed like Object.freeze() was the best solution for this, as it would make process.env readonly, however process.env seems to be incompatible with it:

Object.freeze(process.env);
// -> Object.freeze(process.env); // TypeError: Cannot freeze

After speaking with some people on Freenode's #Node.js channel, I'm left with a few extra remarks:

• Using the spread operator to clone process.env, freeze it, and then reassign it works around the issue (but this doesn't seem like a good pattern?)
• Shouldn't process.env either have a two-way data binding with the environment the process runs in, or else be readonly by default? I realise this is a breaking change, but it seems odd that its possible to use it as a global object in this way.

[addaleax]

• Using the spread operator to clone process.env, freeze it, and then reassign it works around the issue (but this doesn't seem like a good pattern?)

I think it’s just fine for your use case.

• Shouldn't process.env either have a two-way data binding with the environment the process runs in, or else be readonly by default? I realise this is a breaking change, but it seems odd that its possible to use it as a global object in this way.

I’m not sure that I understand – it does have the former property?

[johnhamelink]

I’m not sure that I understand – it does have the former property?

Sorry I misspoke - I meant the shell that the process is executed in.

If I do the following:

• Login to a shell
• Run export TEST=abc
• Run env | grep TEST to see the value of the variable has been set correctly
• Execute a Nodejs file containing process.env.TEST = “def”
• Run the env | grep TEST again to see what’s changed

The sort of two-way binding I’m referring to would update test from the perspective of env whereas this currently isn’t the case. Based on my conversations in IRC, this is what python does.

[targos]

AFAIK Python doesn't do it, because it's impossible for a process to change the environment variables of its parent.

[addaleax]

Yes – every process has its own set of environment variables. export only works because it’s not a separate process, but rather a built-in shell command that modifies the shell process’s environment variables.

[johnhamelink]

So in that case, surely it shouldn’t be possible to mutate the process.env object? Again, I know this is a breaking change, but is there really any reason at all to mutate it? And shouldn’t Object.freeze() allow you to do this in the meantime?

[vorticalbox]

You can use proxy to make proocess.env immutable

process.env = new Proxy({...process.env}, {
  set: function(obj, prop, value) {
    throw Error('nope')
  }
});
process.env.test = 1 // throws error nope

[addaleax]

@johnhamelink The reason to mutate it is basically the same as in bash – by default, child processes use a copy of the parent’s process.env, so mutating process.env sets these defaults.

And shouldn’t Object.freeze() allow you to do this in the meantime?

I don’t know if that’s expected – it would require some changes to Node.js, given the special nature of process.env. https://github.com/nodejs/node/pull/28006 might also affect this, I haven’t tried.

[joyeecheung]

So in that case, surely it shouldn’t be possible to mutate the process.env object? Again, I know this is a breaking change, but is there really any reason at all to mutate it?

There are several use cases of a mutable process.env according to our tests:

• Changing process.env.TZ to change the time zone used in e.g. Date.prototype.toString
• Changing process.env.NODE_DISABLE_COLORS, process.env.TERM, process.env.NO_COLOR etc. to overwrite color settings of the console
• Propagating environment variables to child processes/worker threads that inherit process.env.
• Changing the behavior of libraries that read from process.env dynamically (somewhat hacky)

I think the error was caused by the fact that process.env is implemented with interceptors. It might be possible to implement it differently? But that might be be somehow slower.

[johnhamelink]

@addaleax your bash explanation was great, the thought process behind this makes way more sense to me now that I understand this point :)

@joyeecheung Thank you for this list, I will whitelist these variables so that they are still able to be mutated for compatibility reasons.

I think I'll write an NPM package which renders process.env immutable in the meantime, making use of @vorticalbox's proxy-based solution so that I can:

1. Take into account @joyeecheung's exceptions,
2. Allow us to integrate a Sentry exception report; and
3. Allow us to provide a verbose, dev friendly error that explains why this is bad.

Once it's done I'll edit this post to include a link for future reference.

Thanks for your help folks!

[markbirbeck]

At the top you say that the following "doesn't seem like a good pattern":

process.env = Object.freeze({...process.env})

I'm just wondering if you could say a bit more about why?

This approach doesn't throw an error when trying to modify an environment variable, whereas the proxy approach does; is that the reason you prefer to use proxy?