Skip to content

Commit fb512ef

Browse files
philnashUlisesGascon
authored andcommitted
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: #49424 Reviewed-By: Yagiz Nizipli <[email protected]> Reviewed-By: Geoffrey Booth <[email protected]> Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: Chemi Atlow <[email protected]>
1 parent b28b85a commit fb512ef

File tree

5 files changed

+34
-11
lines changed

5 files changed

+34
-11
lines changed

src/node_dotenv.cc

Lines changed: 13 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -48,14 +48,19 @@ void Dotenv::SetEnvironment(node::Environment* env) {
4848
for (const auto& entry : store_) {
4949
auto key = entry.first;
5050
auto value = entry.second;
51-
env->env_vars()->Set(
52-
isolate,
53-
v8::String::NewFromUtf8(
54-
isolate, key.data(), NewStringType::kNormal, key.size())
55-
.ToLocalChecked(),
56-
v8::String::NewFromUtf8(
57-
isolate, value.data(), NewStringType::kNormal, value.size())
58-
.ToLocalChecked());
51+
52+
auto existing = env->env_vars()->Get(key.data());
53+
54+
if (existing.IsNothing()) {
55+
env->env_vars()->Set(
56+
isolate,
57+
v8::String::NewFromUtf8(
58+
isolate, key.data(), NewStringType::kNormal, key.size())
59+
.ToLocalChecked(),
60+
v8::String::NewFromUtf8(
61+
isolate, value.data(), NewStringType::kNormal, value.size())
62+
.ToLocalChecked());
63+
}
5964
}
6065
}
6166

test/fixtures/dotenv/valid.env

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,5 +31,5 @@ RETAIN_INNER_QUOTES={"foo": "bar"}
3131
RETAIN_INNER_QUOTES_AS_STRING='{"foo": "bar"}'
3232
RETAIN_INNER_QUOTES_AS_BACKTICKS=`{"foo": "bar's"}`
3333
TRIM_SPACE_FROM_UNQUOTED= some spaced out string
34-
34+
3535
SPACED_KEY = parsed

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

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,4 +35,17 @@ describe('.env supports edge cases', () => {
3535
assert.strictEqual(child.code, 0);
3636
});
3737

38+
it('should not override existing environment variables but introduce new vars', async () => {
39+
const code = `
40+
require('assert').strictEqual(process.env.BASIC, 'existing');
41+
require('assert').strictEqual(process.env.AFTER_LINE, 'after_line');
42+
`.trim();
43+
const child = await common.spawnPromisified(
44+
process.execPath,
45+
[ `--env-file=${validEnvFilePath}`, '--eval', code ],
46+
{ cwd: __dirname, env: { ...process.env, BASIC: 'existing' } },
47+
);
48+
assert.strictEqual(child.stderr, '');
49+
assert.strictEqual(child.code, 0);
50+
});
3851
});

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

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,10 +48,15 @@ describe('.env supports NODE_OPTIONS', () => {
4848
const code = `
4949
require('assert')(new Date().toString().includes('Hawaii'))
5050
`.trim();
51+
// Some CI environments set TZ. Since an env file doesn't override existing
52+
// environment variables, we need to delete it and then pass the env object
53+
// as the environment to spawnPromisified.
54+
const env = { ...process.env };
55+
delete env.TZ;
5156
const child = await common.spawnPromisified(
5257
process.execPath,
5358
[ `--env-file=${relativePath}`, '--eval', code ],
54-
{ cwd: __dirname },
59+
{ cwd: __dirname, env },
5560
);
5661
assert.strictEqual(child.stderr, '');
5762
assert.strictEqual(child.code, 0);

test/parallel/test-dotenv.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,6 @@ assert.strictEqual(process.env.RETAIN_INNER_QUOTES_AS_BACKTICKS, '{"foo": "bar\'
6565
// Retains spaces in string
6666
assert.strictEqual(process.env.TRIM_SPACE_FROM_UNQUOTED, 'some spaced out string');
6767
// Parses email addresses completely
68-
assert.strictEqual(process.env.USERNAME, '[email protected]');
68+
assert.strictEqual(process.env.EMAIL, '[email protected]');
6969
// Parses keys and values surrounded by spaces
7070
assert.strictEqual(process.env.SPACED_KEY, 'parsed');

0 commit comments

Comments
 (0)