cli: implement --trace-env and --trace-env-[js|native]-stack

doc/api/cli.md

@@ -2588,6 +2588,45 @@ added: v0.8.0
 
 Print stack traces for deprecations.
 
+### `--trace-env`
+
+<!-- YAML
+added: REPLACEME
+-->
+
+Print information about any access to environment variables done in the current Node.js
+instance to stderr, including:
+
+* The environment variable reads that Node.js does internally.
+* Writes in the form of `process.env.KEY = "SOME VALUE"`.
+* Reads in the form of `process.env.KEY`.
+* Definitions in the form of `Object.defineProperty(process.env, 'KEY', {...})`.
+* Queries in the form of `Object.hasOwn(process.env, 'KEY')`,
+  `process.env.hasOwnProperty('KEY')` or `'KEY' in process.env`.
+* Deletions in the form of `delete process.env.KEY`.
+* Enumerations inf the form of `...process.env` or `Object.keys(process.env)`.
+
+Only the names of the environment variables being accessed are printed. The values are not printed.
+
+To print the stack trace of the access, use `--trace-env-js-stack` and/or
+`--trace-env-native-stack`.
+
+### `--trace-env-js-stack`
+
+<!-- YAML
+added: REPLACEME
+-->
+
+In addition to what `--trace-env` does, this prints the JavaScript stack trace of the access.
+
+### `--trace-env-native-stack`
+
+<!-- YAML
+added: REPLACEME
+-->
+
+In addition to what `--trace-env` does, this prints the native stack trace of the access.
+
 ### `--trace-event-categories`
 
 <!-- YAML
@@ -3134,6 +3173,9 @@ one is included in the list below.
 * `--tls-min-v1.2`
 * `--tls-min-v1.3`
 * `--trace-deprecation`
+* `--trace-env-js-stack`
+* `--trace-env-native-stack`
+* `--trace-env`
 * `--trace-event-categories`
 * `--trace-event-file-pattern`
 * `--trace-events-enabled`

src/debug_utils.cc

@@ -62,9 +62,9 @@ EnabledDebugList enabled_debug_list;
 using v8::Local;
 using v8::StackTrace;
 
-void EnabledDebugList::Parse(std::shared_ptr<KVStore> env_vars) {
+void EnabledDebugList::Parse(Environment* env) {
   std::string cats;
-  credentials::SafeGetenv("NODE_DEBUG_NATIVE", &cats, env_vars);
+  credentials::SafeGetenv("NODE_DEBUG_NATIVE", &cats, env);
   Parse(cats);
 }
 

src/debug_utils.h

@@ -74,10 +74,10 @@ class NODE_EXTERN_PRIVATE EnabledDebugList {
     return enabled_[static_cast<unsigned int>(category)];
   }
 
-  // Uses NODE_DEBUG_NATIVE to initialize the categories. The env_vars variable
+  // Uses NODE_DEBUG_NATIVE to initialize the categories. env->env_vars()
   // is parsed if it is not a nullptr, otherwise the system environment
   // variables are parsed.
-  void Parse(std::shared_ptr<KVStore> env_vars);
+  void Parse(Environment* env);
 
  private:
   // Enable all categories matching cats.

src/env.cc

@@ -864,9 +864,6 @@ Environment::Environment(IsolateData* isolate_data,
         EnvironmentFlags::kOwnsInspector;
   }
 
-  set_env_vars(per_process::system_environment);
-  enabled_debug_list_.Parse(env_vars());
-
   // We create new copies of the per-Environment option sets, so that it is
   // easier to modify them after Environment creation. The defaults are
   // part of the per-Isolate option set, for which in turn the defaults are
@@ -876,6 +873,13 @@ Environment::Environment(IsolateData* isolate_data,
   inspector_host_port_ = std::make_shared<ExclusiveAccess<HostPort>>(
       options_->debug_options().host_port);
 
+  set_env_vars(per_process::system_environment);
+  // This should be done after options is created, so that --trace-env can be
+  // checked when parsing NODE_DEBUG_NATIVE. It should also be done after
+  // env_vars() is set so that the parser uses values from env->env_vars()
+  // which may or may not be the system environment variable store.
+  enabled_debug_list_.Parse(this);
+
   heap_snapshot_near_heap_limit_ =
       static_cast<uint32_t>(options_->heap_snapshot_near_heap_limit);
 
@@ -1104,8 +1108,7 @@ void Environment::InitializeLibuv() {
 
 void Environment::InitializeCompileCache() {
   std::string dir_from_env;
-  if (!credentials::SafeGetenv(
-          "NODE_COMPILE_CACHE", &dir_from_env, env_vars()) ||
+  if (!credentials::SafeGetenv("NODE_COMPILE_CACHE", &dir_from_env, this) ||
       dir_from_env.empty()) {
     return;
   }
@@ -1117,7 +1120,7 @@ CompileCacheEnableResult Environment::EnableCompileCache(
   CompileCacheEnableResult result;
   std::string disable_env;
   if (credentials::SafeGetenv(
-          "NODE_DISABLE_COMPILE_CACHE", &disable_env, env_vars())) {
+          "NODE_DISABLE_COMPILE_CACHE", &disable_env, this)) {
     result.status = CompileCacheEnableStatus::DISABLED;
     result.message = "Disabled by NODE_DISABLE_COMPILE_CACHE";
     Debug(this,

src/node.cc

@@ -993,7 +993,7 @@ InitializeOncePerProcessInternal(const std::vector<std::string>& args,
   if (!(flags & ProcessInitializationFlags::kNoParseGlobalDebugVariables)) {
     // Initialized the enabled list for Debug() calls with system
     // environment variables.
-    per_process::enabled_debug_list.Parse(per_process::system_environment);
+    per_process::enabled_debug_list.Parse(nullptr);
   }
 
   PlatformInit(flags);

src/node_credentials.cc

@@ -72,9 +72,7 @@ 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,
-                std::shared_ptr<KVStore> env_vars) {
+bool SafeGetenv(const char* key, std::string* text, Environment* env) {
 #if !defined(__CloudABI__) && !defined(_WIN32)
 #if defined(__linux__)
   if ((!HasOnly(CAP_NET_BIND_SERVICE) && linux_at_secure()) ||
@@ -87,14 +85,31 @@ bool SafeGetenv(const char* key,
 
   // Fallback to system environment which reads the real environment variable
   // through uv_os_getenv.
-  if (env_vars == nullptr) {
+  std::shared_ptr<KVStore> env_vars;
+  if (env == nullptr) {
     env_vars = per_process::system_environment;
+  } else {
+    env_vars = env->env_vars();
   }
 
   std::optional<std::string> value = env_vars->Get(key);
-  if (!value.has_value()) return false;
-  *text = value.value();
-  return true;
+
+  bool has_env = value.has_value();
+  if (has_env) {
+    *text = value.value();
+  }
+
+  auto options =
+      (env != nullptr ? env->options()
+                      : per_process::cli_options->per_isolate->per_env);
+
+  if (options->trace_env) {
+    fprintf(stderr, "[--trace-env] get environment variable \"%s\"\n", key);
+
+    PrintTraceEnvStack(options);
+  }
+
+  return has_env;
 }
 
 static void SafeGetenv(const FunctionCallbackInfo<Value>& args) {
@@ -103,7 +118,7 @@ static void SafeGetenv(const FunctionCallbackInfo<Value>& args) {
   Isolate* isolate = env->isolate();
   Utf8Value strenvtag(isolate, args[0]);
   std::string text;
-  if (!SafeGetenv(*strenvtag, &text, env->env_vars())) return;
+  if (!SafeGetenv(*strenvtag, &text, env)) return;
   Local<Value> result =
       ToV8Value(isolate->GetCurrentContext(), text).ToLocalChecked();
   args.GetReturnValue().Set(result);
@@ -117,7 +132,7 @@ static void GetTempDir(const FunctionCallbackInfo<Value>& args) {
 
   // Let's wrap SafeGetEnv since it returns true for empty string.
   auto get_env = [&dir, &env](std::string_view key) {
-    USE(SafeGetenv(key.data(), &dir, env->env_vars()));
+    USE(SafeGetenv(key.data(), &dir, env));
     return !dir.empty();
   };
 

src/node_env_var.cc

@@ -337,6 +337,19 @@ Maybe<void> KVStore::AssignToObject(v8::Isolate* isolate,
   return JustVoid();
 }
 
+void PrintTraceEnvStack(Environment* env) {
+  PrintTraceEnvStack(env->options());
+}
+
+void PrintTraceEnvStack(std::shared_ptr<EnvironmentOptions> options) {
+  if (options->trace_env_native_stack) {
+    DumpNativeBacktrace(stderr);
+  }
+  if (options->trace_env_js_stack) {
+    DumpJavaScriptBacktrace(stderr);
+  }
+}
+
 static Intercepted EnvGetter(Local<Name> property,
                              const PropertyCallbackInfo<Value>& info) {
   Environment* env = Environment::GetCurrent(info);
@@ -348,7 +361,18 @@ static Intercepted EnvGetter(Local<Name> property,
   CHECK(property->IsString());
   MaybeLocal<String> value_string =
       env->env_vars()->Get(env->isolate(), property.As<String>());
-  if (!value_string.IsEmpty()) {
+
+  bool has_env = !value_string.IsEmpty();
+  if (env->options()->trace_env) {
+    Utf8Value key(env->isolate(), property.As<String>());
+    fprintf(stderr,
+            "[--trace-env] get environment variable \"%.*s\"\n",
+            static_cast<int>(key.length()),
+            key.out());
+    PrintTraceEnvStack(env);
+  }
+
+  if (has_env) {
     info.GetReturnValue().Set(value_string.ToLocalChecked());
     return Intercepted::kYes;
   }
@@ -386,6 +410,14 @@ static Intercepted EnvSetter(Local<Name> property,
   }
 
   env->env_vars()->Set(env->isolate(), key, value_string);
+  if (env->options()->trace_env) {
+    Utf8Value key_utf8(env->isolate(), key);
+    fprintf(stderr,
+            "[--trace-env] set environment variable \"%.*s\"\n",
+            static_cast<int>(key_utf8.length()),
+            key_utf8.out());
+    PrintTraceEnvStack(env);
+  }
 
   return Intercepted::kYes;
 }
@@ -396,7 +428,18 @@ static Intercepted EnvQuery(Local<Name> property,
   CHECK(env->has_run_bootstrapping_code());
   if (property->IsString()) {
     int32_t rc = env->env_vars()->Query(env->isolate(), property.As<String>());
-    if (rc != -1) {
+    bool has_env = (rc != -1);
+
+    if (env->options()->trace_env) {
+      Utf8Value key_utf8(env->isolate(), property.As<String>());
+      fprintf(stderr,
+              "[--trace-env] query environment variable \"%.*s\": %s\n",
+              static_cast<int>(key_utf8.length()),
+              key_utf8.out(),
+              has_env ? "is set" : "is not set");
+      PrintTraceEnvStack(env);
+    }
+    if (has_env) {
       // Return attributes for the property.
       info.GetReturnValue().Set(v8::None);
       return Intercepted::kYes;
@@ -411,6 +454,15 @@ static Intercepted EnvDeleter(Local<Name> property,
   CHECK(env->has_run_bootstrapping_code());
   if (property->IsString()) {
     env->env_vars()->Delete(env->isolate(), property.As<String>());
+
+    if (env->options()->trace_env) {
+      Utf8Value key_utf8(env->isolate(), property.As<String>());
+      fprintf(stderr,
+              "[--trace-env] delete environment variable \"%.*s\"\n",
+              static_cast<int>(key_utf8.length()),
+              key_utf8.out());
+      PrintTraceEnvStack(env);
+    }
   }
 
   // process.env never has non-configurable properties, so always
@@ -423,6 +475,12 @@ static void EnvEnumerator(const PropertyCallbackInfo<Array>& info) {
   Environment* env = Environment::GetCurrent(info);
   CHECK(env->has_run_bootstrapping_code());
 
+  if (env->options()->trace_env) {
+    fprintf(stderr, "[--trace-env] enumerate environment variables\n");
+
+    PrintTraceEnvStack(env);
+  }
+
   info.GetReturnValue().Set(
       env->env_vars()->Enumerate(env->isolate()));
 }

src/node_internals.h

@@ -321,11 +321,12 @@ class ThreadPoolWork {
 #endif  // defined(__POSIX__) && !defined(__ANDROID__) && !defined(__CloudABI__)
 
 namespace credentials {
-bool SafeGetenv(const char* key,
-                std::string* text,
-                std::shared_ptr<KVStore> env_vars = nullptr);
+bool SafeGetenv(const char* key, std::string* text, Environment* env = nullptr);
 }  // namespace credentials
 
+void PrintTraceEnvStack(Environment* env);
+void PrintTraceEnvStack(std::shared_ptr<EnvironmentOptions> options);
+
 void DefineZlibConstants(v8::Local<v8::Object> target);
 v8::Isolate* NewIsolate(v8::Isolate::CreateParams* params,
                         uv_loop_t* event_loop,

src/node_options.cc

@@ -762,6 +762,24 @@ EnvironmentOptionsParser::EnvironmentOptionsParser() {
             "show stack traces on promise initialization and resolution",
             &EnvironmentOptions::trace_promises,
             kAllowedInEnvvar);
+
+  AddOption("--trace-env",
+            "Print accesses to the environment variables",
+            &EnvironmentOptions::trace_env,
+            kAllowedInEnvvar);
+  Implies("--trace-env-js-stack", "--trace-env");
+  Implies("--trace-env-native-stack", "--trace-env");
+  AddOption("--trace-env-js-stack",
+            "Print accesses to the environment variables and the JavaScript "
+            "stack trace",
+            &EnvironmentOptions::trace_env_js_stack,
+            kAllowedInEnvvar);
+  AddOption(
+      "--trace-env-native-stack",
+      "Print accesses to the environment variables and the native stack trace",
+      &EnvironmentOptions::trace_env_native_stack,
+      kAllowedInEnvvar);
+
   AddOption("--experimental-default-type",
             "set module system to use by default",
             &EnvironmentOptions::type,

src/node_options.h

@@ -209,6 +209,9 @@ class EnvironmentOptions : public Options {
   bool trace_uncaught = false;
   bool trace_warnings = false;
   bool trace_promises = false;
+  bool trace_env = false;
+  bool trace_env_js_stack = false;
+  bool trace_env_native_stack = false;
   bool extra_info_on_fatal_exception = true;
   std::string unhandled_rejections;
   std::vector<std::string> userland_loaders;

src/path.cc

@@ -114,7 +114,7 @@ std::string PathResolve(Environment* env,
       // a UNC path at this points, because UNC paths are always absolute.
       std::string resolvedDevicePath;
       const std::string envvar = "=" + resolvedDevice;
-      credentials::SafeGetenv(envvar.c_str(), &resolvedDevicePath);
+      credentials::SafeGetenv(envvar.c_str(), &resolvedDevicePath, env);
       path = resolvedDevicePath.empty() ? cwd : resolvedDevicePath;
 
       // Verify that a cwd was found and that it actually points

test/fixtures/process-env/define.js

@@ -0,0 +1,6 @@
+Object.defineProperty(process.env, 'FOO', {
+  configurable: true,
+  enumerable: true,
+  writable: true,
+  value: 'FOO',
+});

test/fixtures/process-env/delete.js

@@ -0,0 +1 @@
+delete process.env.FOO;

test/fixtures/process-env/enumerate.js

@@ -0,0 +1,3 @@
+Object.keys(process.env);
+
+const env = { ...process.env };

test/fixtures/process-env/get.js

@@ -0,0 +1,2 @@
+const foo = process.env.FOO;
+const bar = process.env.BAR;

test/fixtures/process-env/query.js

@@ -0,0 +1,3 @@
+const foo = 'FOO' in process.env;
+const bar = Object.hasOwn(process.env, 'BAR');
+const baz = process.env.hasOwnProperty('BAZ');

test/fixtures/process-env/set.js

@@ -0,0 +1 @@
+process.env.FOO = "FOO";

test/parallel/test-trace-env-stack.js

@@ -0,0 +1,84 @@
+'use strict';
+
+// This tests that --trace-env-js-stack and --trace-env-native-stack works.
+require('../common');
+const { spawnSyncAndAssert } = require('../common/child_process');
+const assert = require('assert');
+const fixtures = require('../common/fixtures');
+
+// Check reads from the Node.js internals.
+// The stack trace may vary depending on internal refactorings, but it's relatively
+// stable that:
+// 1. Some C++ code would check the environment variables using
+//    node::credentials::SafeGetenv.
+// 2. Some JavaScript code would access process.env which goes to node::EnvGetter
+// 3. Some JavaScript code would access process.env from pre_execution where
+//    the run time states are used to finish bootstrap.
+spawnSyncAndAssert(process.execPath, ['--trace-env-native-stack', fixtures.path('empty.js')], {
+  stderr(output) {
+    if (output.includes('PrintTraceEnvStack')) {  // Some platforms do not support back traces.
+      assert.match(output, /node::credentials::SafeGetenv/);  // This is usually not optimized away.
+    }
+  }
+});
+
+spawnSyncAndAssert(process.execPath, ['--trace-env-js-stack', fixtures.path('empty.js')], {
+  stderr: /internal\/process\/pre_execution/
+});
+
+// Check get from user land.
+spawnSyncAndAssert(process.execPath, ['--trace-env-js-stack', fixtures.path('process-env', 'get.js')], {
+  env: {
+    ...process.env,
+    FOO: 'FOO',
+    BAR: undefined,
+  }
+}, {
+  stderr(output) {
+    assert.match(output, /get\.js:1:25/);
+    assert.match(output, /get\.js:2:25/);
+  }
+});
+
+// Check set from user land.
+spawnSyncAndAssert(process.execPath, ['--trace-env-js-stack', fixtures.path('process-env', 'set.js')], {
+  stderr(output) {
+    assert.match(output, /set\.js:1:17/);
+  }
+});
+
+// // Check define from user land.
+spawnSyncAndAssert(process.execPath, ['--trace-env-js-stack', fixtures.path('process-env', 'define.js')], {
+  stderr(output) {
+    assert.match(output, /define\.js:1:8/);
+  }
+});
+
+// Check query from user land.
+spawnSyncAndAssert(process.execPath, ['--trace-env-js-stack', fixtures.path('process-env', 'query.js')], {
+  ...process.env,
+  FOO: 'FOO',
+  BAR: undefined,
+  BAZ: undefined,
+}, {
+  stderr(output) {
+    assert.match(output, /query\.js:1:19/);
+    assert.match(output, /query\.js:2:20/);
+    assert.match(output, /query\.js:3:25/);
+  }
+});
+
+// Check delete from user land.
+spawnSyncAndAssert(process.execPath, ['--trace-env-js-stack', fixtures.path('process-env', 'delete.js')], {
+  stderr(output) {
+    assert.match(output, /delete\.js:1:16/);
+  }
+});
+
+// Check enumeration from user land.
+spawnSyncAndAssert(process.execPath, ['--trace-env-js-stack', fixtures.path('process-env', 'enumerate.js') ], {
+  stderr(output) {
+    assert.match(output, /enumerate\.js:1:8/);
+    assert.match(output, /enumerate\.js:3:26/);
+  }
+});

test/parallel/test-trace-env.js

@@ -0,0 +1,99 @@
+'use strict';
+
+// This tests that --trace-env works.
+const common = require('../common');
+const { spawnSyncAndAssert } = require('../common/child_process');
+const assert = require('assert');
+const fixtures = require('../common/fixtures');
+
+// Check reads from the Node.js internals.
+spawnSyncAndAssert(process.execPath, ['--trace-env', fixtures.path('empty.js')], {
+  stderr(output) {
+    // This is a non-exhaustive list of the environment variables checked
+    // during startup of an empty script at the time this test was written.
+    // If the internals remove any one of them, the checks here can be updated
+    // accordingly.
+    if (common.hasIntl) {
+      assert.match(output, /get environment variable "NODE_ICU_DATA"/);
+    }
+    if (common.hasCrypto) {
+      assert.match(output, /get environment variable "NODE_EXTRA_CA_CERTS"/);
+    }
+    if (common.hasOpenSSL3) {
+      assert.match(output, /get environment variable "OPENSSL_CONF"/);
+    }
+    assert.match(output, /get environment variable "NODE_DEBUG_NATIVE"/);
+    assert.match(output, /get environment variable "NODE_COMPILE_CACHE"/);
+    assert.match(output, /get environment variable "NODE_NO_WARNINGS"/);
+    assert.match(output, /get environment variable "NODE_V8_COVERAGE"/);
+    assert.match(output, /get environment variable "NODE_DEBUG"/);
+    assert.match(output, /get environment variable "NODE_CHANNEL_FD"/);
+    assert.match(output, /get environment variable "NODE_UNIQUE_ID"/);
+    if (common.isWindows) {
+      assert.match(output, /get environment variable "USERPROFILE"/);
+    } else {
+      assert.match(output, /get environment variable "HOME"/);
+    }
+    assert.match(output, /get environment variable "NODE_PATH"/);
+    assert.match(output, /get environment variable "WATCH_REPORT_DEPENDENCIES"/);
+  }
+});
+
+// Check get from user land.
+spawnSyncAndAssert(process.execPath, ['--trace-env', fixtures.path('process-env', 'get.js') ], {
+  env: {
+    ...process.env,
+    FOO: 'FOO',
+    BAR: undefined,
+  }
+}, {
+  stderr(output) {
+    assert.match(output, /get environment variable "FOO"/);
+    assert.match(output, /get environment variable "BAR"/);
+  }
+});
+
+// Check set from user land.
+spawnSyncAndAssert(process.execPath, ['--trace-env', fixtures.path('process-env', 'set.js') ], {
+  stderr(output) {
+    assert.match(output, /set environment variable "FOO"/);
+  }
+});
+
+// Check define from user land.
+spawnSyncAndAssert(process.execPath, ['--trace-env', fixtures.path('process-env', 'define.js') ], {
+  stderr(output) {
+    assert.match(output, /set environment variable "FOO"/);
+  }
+});
+
+// Check query from user land.
+spawnSyncAndAssert(process.execPath, ['--trace-env', fixtures.path('process-env', 'query.js') ], {
+  env: {
+    ...process.env,
+    FOO: 'FOO',
+    BAR: undefined,
+    BAZ: undefined,
+  }
+}, {
+  stderr(output) {
+    assert.match(output, /query environment variable "FOO": is set/);
+    assert.match(output, /query environment variable "BAR": is not set/);
+    assert.match(output, /query environment variable "BAZ": is not set/);
+  }
+});
+
+// Check delete from user land.
+spawnSyncAndAssert(process.execPath, ['--trace-env', fixtures.path('process-env', 'delete.js') ], {
+  stderr(output) {
+    assert.match(output, /delete environment variable "FOO"/);
+  }
+});
+
+// Check enumeration from user land.
+spawnSyncAndAssert(process.execPath, ['--trace-env', fixtures.path('process-env', 'enumerate.js') ], {
+  stderr(output) {
+    const matches = output.match(/enumerate environment variables/g);
+    assert.strictEqual(matches.length, 2);
+  }
+});