From a19796db20b82d4a5ed5d1613f08f1792e94c057 Mon Sep 17 00:00:00 2001 From: marcopiraccini Date: Sat, 10 Oct 2026 10:11:35 +0200 Subject: [PATCH 1/3] src: honor NODE_EXTRA_CA_CERTS set in an env file NODE_EXTRA_CA_CERTS is read once at startup, before the files passed with --env-file are applied to the environment. A value set in an env file showed up in process.env but the certificates were never loaded. Look the variable up in the parsed env files when it is not in the environment. The environment keeps precedence, and the env files are ignored under the same conditions as the environment: when node runs setuid, setgid or with Linux file capabilities. Fixes: https://github.com/nodejs/node/issues/51426 Refs: https://github.com/nodejs/node/pull/51497 Signed-off-by: marcopiraccini --- doc/api/cli.md | 10 ++- doc/node.1 | 4 +- src/node.cc | 7 +- src/node_credentials.cc | 22 ++++- src/node_dotenv.cc | 15 ++++ src/node_dotenv.h | 3 + src/node_internals.h | 4 + .../test-tls-env-extra-ca-env-file.js | 84 +++++++++++++++++++ 8 files changed, 144 insertions(+), 5 deletions(-) create mode 100644 test/parallel/test-tls-env-extra-ca-env-file.js diff --git a/doc/api/cli.md b/doc/api/cli.md index 3822685d3d50..3fafaefc125e 100644 --- a/doc/api/cli.md +++ b/doc/api/cli.md @@ -4132,6 +4132,11 @@ Disable the [module compile cache][] for the Node.js instance. See the documenta When set, the well known "root" CAs (like VeriSign) will be extended with the @@ -4148,7 +4153,9 @@ has Linux file capabilities set. The `NODE_EXTRA_CA_CERTS` environment variable is only read when the Node.js process is first launched. Changing the value at runtime using -`process.env.NODE_EXTRA_CA_CERTS` has no effect on the current process. +`process.env.NODE_EXTRA_CA_CERTS` has no effect on the current process. This +includes [`process.loadEnvFile()`][]. The variable can be set in a file loaded +with [`--env-file`][] or [`--env-file-if-exists`][], which are read at startup. ### `NODE_ICU_DATA=file` @@ -4940,6 +4947,7 @@ node --stack-trace-limit=12 -p -e "Error.stackTraceLimit" # prints 12 [`node:stream/iter`]: stream_iter.md [`node:vfs`]: vfs.md [`permission.drop()`]: permissions.md#permissiondropscope-reference +[`process.loadEnvFile()`]: process.md#processloadenvfilepath [`process.setUncaughtExceptionCaptureCallback()`]: process.md#processsetuncaughtexceptioncapturecallbackfn [`session.connectToMainThread()`]: inspector.md#sessionconnecttomainthread [`tls.DEFAULT_MAX_VERSION`]: tls.md#tlsdefault_max_version diff --git a/doc/node.1 b/doc/node.1 index 863c468eed10..122b064c2fac 100644 --- a/doc/node.1 +++ b/doc/node.1 @@ -2115,7 +2115,9 @@ This environment variable is ignored when \fBnode\fR runs as setuid root or has Linux file capabilities set. The \fBNODE_EXTRA_CA_CERTS\fR environment variable is only read when the Node.js process is first launched. Changing the value at runtime using -\fBprocess.env.NODE_EXTRA_CA_CERTS\fR has no effect on the current process. +\fBprocess.env.NODE_EXTRA_CA_CERTS\fR has no effect on the current process. This +includes \fBprocess.loadEnvFile()\fR. The variable can be set in a file loaded +with \fB--env-file\fR or \fB--env-file-if-exists\fR, which are read at startup. . .It Ev NODE_ICU_DATA Ar file Data path for ICU (\fBIntl\fR object) data. Will extend linked-in data when compiled diff --git a/src/node.cc b/src/node.cc index d0b5310626c3..67dbd172c386 100644 --- a/src/node.cc +++ b/src/node.cc @@ -1427,9 +1427,14 @@ InitializeOncePerProcessInternal( }); #endif // !defined(OPENSSL_IS_BORINGSSL) { + // The env files are not applied to the environment yet at this point. + // A variable from the real environment takes precedence over them. std::string extra_ca_certs; - if (credentials::SafeGetenv("NODE_EXTRA_CA_CERTS", &extra_ca_certs)) + if (credentials::SafeGetenv("NODE_EXTRA_CA_CERTS", &extra_ca_certs) || + credentials::SafeGetenvFromEnvFile("NODE_EXTRA_CA_CERTS", + &extra_ca_certs)) { crypto::UseExtraCaCerts(extra_ca_certs); + } } #endif // HAVE_OPENSSL } diff --git a/src/node_credentials.cc b/src/node_credentials.cc index d1f7c6e2a801..2ba5e2e09247 100644 --- a/src/node_credentials.cc +++ b/src/node_credentials.cc @@ -1,4 +1,5 @@ #include "env-inl.h" +#include "node_dotenv.h" #include "node_errors.h" #include "node_external_reference.h" #include "node_internals.h" @@ -71,7 +72,10 @@ static bool HasOnly(int capability) { // process only has the capability CAP_NET_BIND_SERVICE set. If the current // process does not have any capabilities set and the process is running as // setuid root then lookup will not be allowed. -bool SafeGetenv(const char* key, std::string* text, Environment* env) { +// Whether the process runs with privileges it was not started with: setuid, +// setgid or, on Linux, file capabilities. Configuration that comes from the +// environment is not trusted in that case. +static bool HasElevatedPrivileges() { #if !defined(__CloudABI__) && !defined(_WIN32) #if defined(__linux__) if ((!HasOnly(CAP_NET_BIND_SERVICE) && linux_at_secure()) || @@ -79,8 +83,22 @@ bool SafeGetenv(const char* key, std::string* text, Environment* env) { #else if (linux_at_secure() || getuid() != geteuid() || getgid() != getegid()) #endif - return false; + return true; #endif + return false; +} + +bool SafeGetenvFromEnvFile(const char* key, std::string* text) { + if (HasElevatedPrivileges()) return false; + + std::optional value = per_process::dotenv_file.Get(key); + if (!value.has_value()) return false; + *text = *value; + return true; +} + +bool SafeGetenv(const char* key, std::string* text, Environment* env) { + if (HasElevatedPrivileges()) return false; // Fallback to system environment which reads the real environment variable // through uv_os_getenv. diff --git a/src/node_dotenv.cc b/src/node_dotenv.cc index 0c6fa30a41d7..eddc64822e8a 100644 --- a/src/node_dotenv.cc +++ b/src/node_dotenv.cc @@ -86,6 +86,21 @@ Maybe Dotenv::SetEnvironment(node::Environment* env) { return JustVoid(); } +std::optional Dotenv::Get(const std::string& key) const { +#ifdef _WIN32 + // Environment variable names are case-insensitive on Windows, and so is + // the lookup in process.env once the env files are applied. + for (const auto& [name, value] : store_) { + if (StringEqualNoCase(name.c_str(), key.c_str())) return value; + } + return std::nullopt; +#else + auto match = store_.find(key); + if (match == store_.end()) return std::nullopt; + return match->second; +#endif +} + std::vector Dotenv::GetKeys() const { std::vector keys; keys.reserve(store_.size()); diff --git a/src/node_dotenv.h b/src/node_dotenv.h index e0069bb08ce8..4f8d3d0a8154 100644 --- a/src/node_dotenv.h +++ b/src/node_dotenv.h @@ -7,6 +7,7 @@ #include "v8.h" #include +#include namespace node { @@ -32,6 +33,8 @@ class Dotenv { v8::MaybeLocal ToObject(Environment* env) const; // The names of the variables parsed from the env files. std::vector GetKeys() const; + // The value of a variable parsed from the env files, if it is defined there. + std::optional Get(const std::string& key) const; static std::vector GetDataFromArgs( const std::vector& args); diff --git a/src/node_internals.h b/src/node_internals.h index bb45d73e9090..3ce3ee26d56a 100644 --- a/src/node_internals.h +++ b/src/node_internals.h @@ -333,6 +333,10 @@ class ThreadPoolWork { namespace credentials { bool SafeGetenv(const char* key, std::string* text, Environment* env = nullptr); +// Looks up a variable in the env files passed with --env-file, with the same +// privilege check as SafeGetenv(). For variables that are read before the env +// files are applied to the environment. +bool SafeGetenvFromEnvFile(const char* key, std::string* text); } // namespace credentials void TraceEnvVar(Environment* env, const char* message); diff --git a/test/parallel/test-tls-env-extra-ca-env-file.js b/test/parallel/test-tls-env-extra-ca-env-file.js new file mode 100644 index 000000000000..b03dfa124646 --- /dev/null +++ b/test/parallel/test-tls-env-extra-ca-env-file.js @@ -0,0 +1,84 @@ +'use strict'; + +// NODE_EXTRA_CA_CERTS is honored when it is set in an env file. +// Refs: https://github.com/nodejs/node/issues/51426 + +const common = require('../common'); + +if (!common.hasCrypto) + common.skip('missing crypto'); + +const assert = require('assert'); +const fs = require('fs'); +const tls = require('tls'); +const { fork, spawnSync } = require('child_process'); +const fixtures = require('../common/fixtures'); +const tmpdir = require('../common/tmpdir'); + +if (process.env.CHILD) { + const client = tls.connect({ + port: process.env.PORT, + checkServerIdentity: common.mustCall(), + }, common.mustCall(() => { + client.end('hi'); + })); + return; +} + +tmpdir.refresh(); +const envFile = tmpdir.resolve('extra-ca.env'); +fs.writeFileSync(envFile, + `NODE_EXTRA_CA_CERTS=${fixtures.path('keys', 'ca1-cert.pem')}\n`); + +const env = { ...process.env }; +delete env.NODE_EXTRA_CA_CERTS; + +function extraCertificates(args, extraEnv) { + const child = spawnSync(process.execPath, [ + ...args, + '-p', + 'JSON.stringify(require("tls").getCACertificates("extra"))', + ], { env: { ...env, ...extraEnv }, encoding: 'utf8' }); + assert.strictEqual(child.status, 0, child.stderr); + return JSON.parse(child.stdout); +} + +const ca1 = [fixtures.readKey('ca1-cert.pem', 'utf8').trim()]; +const ca2 = [fixtures.readKey('ca2-cert.pem', 'utf8').trim()]; +const trim = (certs) => certs.map((cert) => cert.trim()); + +assert.deepStrictEqual(extraCertificates([]), []); +assert.deepStrictEqual(trim(extraCertificates([`--env-file=${envFile}`])), ca1); +assert.deepStrictEqual( + trim(extraCertificates([`--env-file-if-exists=${envFile}`])), ca1); + +if (common.isWindows) { + // Environment variable names are case-insensitive on Windows. + const lowerCaseFile = tmpdir.resolve('extra-ca-lower-case.env'); + fs.writeFileSync(lowerCaseFile, + `node_extra_ca_certs=${fixtures.path('keys', 'ca1-cert.pem')}\n`); + assert.deepStrictEqual( + trim(extraCertificates([`--env-file=${lowerCaseFile}`])), ca1); +} + +// The environment takes precedence over the env file. +assert.deepStrictEqual( + trim(extraCertificates([`--env-file=${envFile}`], { + NODE_EXTRA_CA_CERTS: fixtures.path('keys', 'ca2-cert.pem'), + })), ca2); + +// The certificate is used for TLS peer validation. +const server = tls.createServer({ + key: fixtures.readKey('agent1-key.pem'), + cert: fixtures.readKey('agent1-cert.pem'), +}, common.mustCall((socket) => { + socket.end('bye'); + server.close(); +})).listen(0, common.mustCall(() => { + fork(__filename, { + env: { ...env, CHILD: 'yes', PORT: server.address().port }, + execArgv: [`--env-file=${envFile}`], + }).on('exit', common.mustCall((status) => { + assert.strictEqual(status, 0); + })); +})); From 4f1d36eb98294ab3bed2c93d1e7be19b5dc39d54 Mon Sep 17 00:00:00 2001 From: marcopiraccini Date: Sat, 10 Oct 2026 10:53:40 +0200 Subject: [PATCH 2/3] doc: replace env-file extra CA PR URL placeholder Signed-off-by: marcopiraccini --- doc/api/cli.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/api/cli.md b/doc/api/cli.md index 3fafaefc125e..68ba8778014a 100644 --- a/doc/api/cli.md +++ b/doc/api/cli.md @@ -4134,7 +4134,7 @@ Disable the [module compile cache][] for the Node.js instance. See the documenta added: v7.3.0 changes: - version: REPLACEME - pr-url: https://github.com/nodejs/node/pull/00000 + pr-url: https://github.com/nodejs/node/pull/66638 description: The variable is now honored when set in a file loaded with `--env-file` or `--env-file-if-exists`. --> From 8c120dd305037ff0c0e5a9fd9c0dfb57f2a8a700 Mon Sep 17 00:00:00 2001 From: marcopiraccini Date: Sat, 31 Oct 2026 07:03:57 +0100 Subject: [PATCH 3/3] src: compare env file keys like the environment does Give the Dotenv store a comparator and let the instance that holds the --env-file variables ignore case on Windows, where environment variable names are case-insensitive, instead of scanning the store in Dotenv::Get(). NODE_OPTIONS read from an env file follows the same rule. util.parseEnv() and process.loadEnvFile() keep comparing keys case-sensitively on every platform: their keys also end up in case-sensitive stores, such as the environment of a Worker. Refs: https://github.com/nodejs/node/pull/66638#discussion_r4425074008 Signed-off-by: marcopiraccini --- src/node.cc | 2 +- src/node_dotenv.cc | 15 +++++++------- src/node_dotenv.h | 20 ++++++++++++++++++- test/fixtures/dotenv/mixed-case-keys.env | 2 ++ test/parallel/test-process-load-env-file.js | 22 +++++++++++++++++++++ test/parallel/test-util-parse-env.js | 2 ++ 6 files changed, 53 insertions(+), 10 deletions(-) create mode 100644 test/fixtures/dotenv/mixed-case-keys.env diff --git a/src/node.cc b/src/node.cc index 67dbd172c386..3a64be7b931e 100644 --- a/src/node.cc +++ b/src/node.cc @@ -158,7 +158,7 @@ namespace per_process { // node_dotenv.h // Instance is used to store environment variables including NODE_OPTIONS. -node::Dotenv dotenv_file = Dotenv(); +node::Dotenv dotenv_file = Dotenv::ForProcessEnvironment(); // node_config_file.h node::ConfigReader config_reader = ConfigReader(); diff --git a/src/node_dotenv.cc b/src/node_dotenv.cc index eddc64822e8a..637af64a3588 100644 --- a/src/node_dotenv.cc +++ b/src/node_dotenv.cc @@ -86,19 +86,18 @@ Maybe Dotenv::SetEnvironment(node::Environment* env) { return JustVoid(); } -std::optional Dotenv::Get(const std::string& key) const { +Dotenv Dotenv::ForProcessEnvironment() { #ifdef _WIN32 - // Environment variable names are case-insensitive on Windows, and so is - // the lookup in process.env once the env files are applied. - for (const auto& [name, value] : store_) { - if (StringEqualNoCase(name.c_str(), key.c_str())) return value; - } - return std::nullopt; + return Dotenv(EnvKeyLess{.ignore_case = true}); #else + return Dotenv(); +#endif +} + +std::optional Dotenv::Get(const std::string& key) const { auto match = store_.find(key); if (match == store_.end()) return std::nullopt; return match->second; -#endif } std::vector Dotenv::GetKeys() const { diff --git a/src/node_dotenv.h b/src/node_dotenv.h index 4f8d3d0a8154..da25d3504644 100644 --- a/src/node_dotenv.h +++ b/src/node_dotenv.h @@ -6,6 +6,7 @@ #include "util-inl.h" #include "v8.h" +#include #include #include @@ -39,8 +40,25 @@ class Dotenv { static std::vector GetDataFromArgs( const std::vector& args); + // For the --env-file store: keys are compared like environment variable + // names, ignoring case on Windows. + static Dotenv ForProcessEnvironment(); + private: - std::map store_; + struct EnvKeyLess { + bool ignore_case = false; + bool operator()(const std::string& a, const std::string& b) const { + if (!ignore_case) return a < b; + return std::lexicographical_compare( + a.begin(), a.end(), b.begin(), b.end(), [](char x, char y) { + return ToLower(x) < ToLower(y); + }); + } + }; + + explicit Dotenv(EnvKeyLess less) : store_(less) {} + + std::map store_; }; namespace per_process { diff --git a/test/fixtures/dotenv/mixed-case-keys.env b/test/fixtures/dotenv/mixed-case-keys.env new file mode 100644 index 000000000000..e2f76f77dabb --- /dev/null +++ b/test/fixtures/dotenv/mixed-case-keys.env @@ -0,0 +1,2 @@ +FOO=one +foo=two diff --git a/test/parallel/test-process-load-env-file.js b/test/parallel/test-process-load-env-file.js index ec99c099d11b..5e725e94e6b0 100644 --- a/test/parallel/test-process-load-env-file.js +++ b/test/parallel/test-process-load-env-file.js @@ -9,6 +9,7 @@ const { isMainThread } = require('worker_threads'); const basicValidEnvFilePath = fixtures.path('dotenv/basic-valid.env'); const validEnvFilePath = fixtures.path('dotenv/valid.env'); +const mixedCaseEnvFilePath = fixtures.path('dotenv/mixed-case-keys.env'); const missingEnvFile = fixtures.path('dir%20with unusual"chars \'åß∂ƒ©∆¬…`/non-existent-file.env'); describe('process.loadEnvFile()', () => { @@ -95,6 +96,27 @@ describe('process.loadEnvFile()', () => { assert.strictEqual(child.code, 1); }); + it('keeps keys that differ only in case distinct', async () => { + // process.env in a Worker is case-sensitive on every platform. + const code = ` + const { Worker } = require('worker_threads'); + new Worker(\` + const { workerData } = require('worker_threads'); + process.loadEnvFile(workerData); + const assert = require('assert'); + assert.strictEqual(process.env.FOO, 'one'); + assert.strictEqual(process.env.foo, 'two'); + \`, { eval: true, workerData: ${JSON.stringify(mixedCaseEnvFilePath)} }); + `.trim(); + const child = await common.spawnPromisified( + process.execPath, + [ '--eval', code ], + { cwd: __dirname }, + ); + assert.strictEqual(child.stderr, ''); + assert.strictEqual(child.code, 0); + }); + it('loadEnvFile does not mutate --env-file output', async () => { const code = ` process.loadEnvFile(${JSON.stringify(basicValidEnvFilePath)}); diff --git a/test/parallel/test-util-parse-env.js b/test/parallel/test-util-parse-env.js index e932a31f683e..8c00e4fdee9c 100644 --- a/test/parallel/test-util-parse-env.js +++ b/test/parallel/test-util-parse-env.js @@ -65,6 +65,8 @@ const fs = require('node:fs'); assert.deepStrictEqual(util.parseEnv(''), { __proto__: null }); assert.deepStrictEqual(util.parseEnv('FOO=bar\nFOO=baz\n'), { __proto__: null, FOO: 'baz' }); +assert.deepStrictEqual(util.parseEnv('FOO=one\nfoo=two\n'), + { __proto__: null, FOO: 'one', foo: 'two' }); // Test for invalid input. assert.throws(() => {