src: don't overwrite environment from .env file

This commit adds a check to see if an environment variable that is
found in the .env file is already set in the environment. If it is,
then the value from the .env file is not used.

PR-URL: nodejs#49424
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Chemi Atlow <chemi@atlow.co.il>

Author: philnash

src/node_dotenv.cc

@@ -48,14 +48,19 @@ void Dotenv::SetEnvironment(node::Environment* env) {
   for (const auto& entry : store_) {
     auto key = entry.first;
     auto value = entry.second;
-    env->env_vars()->Set(
-        isolate,
-        v8::String::NewFromUtf8(
-            isolate, key.data(), NewStringType::kNormal, key.size())
-            .ToLocalChecked(),
-        v8::String::NewFromUtf8(
-            isolate, value.data(), NewStringType::kNormal, value.size())
-            .ToLocalChecked());
+
+    auto existing = env->env_vars()->Get(key.data());
+
+    if (existing.IsNothing()) {
+      env->env_vars()->Set(
+          isolate,
+          v8::String::NewFromUtf8(
+              isolate, key.data(), NewStringType::kNormal, key.size())
+              .ToLocalChecked(),
+          v8::String::NewFromUtf8(
+              isolate, value.data(), NewStringType::kNormal, value.size())
+              .ToLocalChecked());
+    }
   }
 }
 

test/fixtures/dotenv/valid.env

@@ -31,5 +31,5 @@ RETAIN_INNER_QUOTES={"foo": "bar"}
 RETAIN_INNER_QUOTES_AS_STRING='{"foo": "bar"}'
 RETAIN_INNER_QUOTES_AS_BACKTICKS=`{"foo": "bar's"}`
 TRIM_SPACE_FROM_UNQUOTED=    some spaced out string
-USERNAME=therealnerdybeast@example.tld
+EMAIL=therealnerdybeast@example.tld
     SPACED_KEY = parsed

test/parallel/test-dotenv-edge-cases.js

@@ -35,4 +35,17 @@ describe('.env supports edge cases', () => {
     assert.strictEqual(child.code, 0);
   });
 
+  it('should not override existing environment variables but introduce new vars', async () => {
+    const code = `
+      require('assert').strictEqual(process.env.BASIC, 'existing');
+      require('assert').strictEqual(process.env.AFTER_LINE, 'after_line');
+    `.trim();
+    const child = await common.spawnPromisified(
+      process.execPath,
+      [ `--env-file=${validEnvFilePath}`, '--eval', code ],
+      { cwd: __dirname, env: { ...process.env, BASIC: 'existing' } },
+    );
+    assert.strictEqual(child.stderr, '');
+    assert.strictEqual(child.code, 0);
+  });
 });

test/parallel/test-dotenv-node-options.js

@@ -48,10 +48,15 @@ describe('.env supports NODE_OPTIONS', () => {
     const code = `
       require('assert')(new Date().toString().includes('GMT-1000'))
     `.trim();
+    // Some CI environments set TZ. Since an env file doesn't override existing
+    // environment variables, we need to delete it and then pass the env object
+    // as the environment to spawnPromisified.
+    const env = { ...process.env };
+    delete env.TZ;
     const child = await common.spawnPromisified(
       process.execPath,
       [ `--env-file=${relativePath}`, '--eval', code ],
-      { cwd: __dirname },
+      { cwd: __dirname, env },
     );
     assert.strictEqual(child.stderr, '');
     assert.strictEqual(child.code, 0);

test/parallel/test-dotenv.js

@@ -65,6 +65,6 @@ assert.strictEqual(process.env.RETAIN_INNER_QUOTES_AS_BACKTICKS, '{"foo": "bar\'
 // Retains spaces in string
 assert.strictEqual(process.env.TRIM_SPACE_FROM_UNQUOTED, 'some spaced out string');
 // Parses email addresses completely
-assert.strictEqual(process.env.USERNAME, 'therealnerdybeast@example.tld');
+assert.strictEqual(process.env.EMAIL, 'therealnerdybeast@example.tld');
 // Parses keys and values surrounded by spaces
 assert.strictEqual(process.env.SPACED_KEY, 'parsed');