src: honor NODE_EXTRA_CA_CERTS set in an env file

doc/api/cli.md

@@ -4132,6 +4132,11 @@ Disable the [module compile cache][] for the Node.js instance. See the documenta
 
 <!-- YAML
 added: v7.3.0
+changes:
+  - version: REPLACEME
+    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`.
 -->
 
 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

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

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();
@@ -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
   }

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,16 +72,33 @@ 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()) ||
       getuid() != geteuid() || getgid() != getegid())
 #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<std::string> 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.

src/node_dotenv.cc

@@ -86,6 +86,20 @@ Maybe<void> Dotenv::SetEnvironment(node::Environment* env) {
   return JustVoid();
 }
 
+Dotenv Dotenv::ForProcessEnvironment() {
+#ifdef _WIN32
+  return Dotenv(EnvKeyLess{.ignore_case = true});
+#else
+  return Dotenv();
+#endif
+}
+
+std::optional<std::string> Dotenv::Get(const std::string& key) const {
+  auto match = store_.find(key);
+  if (match == store_.end()) return std::nullopt;
+  return match->second;
+}
+
 std::vector<std::string> Dotenv::GetKeys() const {
   std::vector<std::string> keys;
   keys.reserve(store_.size());

src/node_dotenv.h

@@ -6,7 +6,9 @@
 #include "util-inl.h"
 #include "v8.h"
 
+#include <algorithm>
 #include <map>
+#include <optional>
 
 namespace node {
 
@@ -32,12 +34,31 @@ class Dotenv {
   v8::MaybeLocal<v8::Object> ToObject(Environment* env) const;
   // The names of the variables parsed from the env files.
   std::vector<std::string> GetKeys() const;
+  // The value of a variable parsed from the env files, if it is defined there.
+  std::optional<std::string> Get(const std::string& key) const;
 
   static std::vector<env_file_data> GetDataFromArgs(
       const std::vector<std::string>& args);
 
+  // For the --env-file store: keys are compared like environment variable
+  // names, ignoring case on Windows.
+  static Dotenv ForProcessEnvironment();
+
  private:
-  std::map<std::string, std::string> 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<std::string, std::string, EnvKeyLess> store_;
 };
 
 namespace per_process {

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);

test/fixtures/dotenv/mixed-case-keys.env

@@ -0,0 +1,2 @@
+FOO=one
+foo=two

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)});

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);
+  }));
+}));

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(() => {