From 8a50cefdcc9ce3cc8fb754c388fe840c8aa1fca6 Mon Sep 17 00:00:00 2001
From: Joyee Cheung <joyeec9h3@gmail.com>
Date: Tue, 29 Oct 2024 18:17:58 +0100
Subject: [PATCH 1/5] cli: implement --trace-env and
 --trace-env-[js|native]-stack

This implements --trace-env, --trace-env-js-stack and
--trace-env-native-stack CLI options which can be used to find
out what environment variables are accessed and where they are
accessed.
---
 doc/api/cli.md                         | 41 ++++++++++++
 src/debug_utils.cc                     |  4 +-
 src/debug_utils.h                      |  4 +-
 src/env.cc                             | 15 +++--
 src/node.cc                            |  2 +-
 src/node_credentials.cc                | 38 ++++++++---
 src/node_env_var.cc                    | 74 ++++++++++++++++++++-
 src/node_internals.h                   |  7 +-
 src/node_options.cc                    | 18 ++++++
 src/node_options.h                     |  3 +
 src/path.cc                            |  2 +-
 test/fixtures/process-env/define.js    |  6 ++
 test/fixtures/process-env/delete.js    |  1 +
 test/fixtures/process-env/enumerate.js |  3 +
 test/fixtures/process-env/get.js       |  2 +
 test/fixtures/process-env/query.js     |  3 +
 test/fixtures/process-env/set.js       |  1 +
 test/parallel/test-trace-env-stack.js  | 85 ++++++++++++++++++++++++
 test/parallel/test-trace-env.js        | 89 ++++++++++++++++++++++++++
 19 files changed, 372 insertions(+), 26 deletions(-)
 create mode 100644 test/fixtures/process-env/define.js
 create mode 100644 test/fixtures/process-env/delete.js
 create mode 100644 test/fixtures/process-env/enumerate.js
 create mode 100644 test/fixtures/process-env/get.js
 create mode 100644 test/fixtures/process-env/query.js
 create mode 100644 test/fixtures/process-env/set.js
 create mode 100644 test/parallel/test-trace-env-stack.js
 create mode 100644 test/parallel/test-trace-env.js

diff --git a/doc/api/cli.md b/doc/api/cli.md
index d232c34230276a..7a459f4677aa5e 100644
--- a/doc/api/cli.md
+++ b/doc/api/cli.md
@@ -2588,6 +2588,44 @@ added: v0.8.0
 
 Print stack traces for deprecations.
 
+### `--trace-env`
+
+<!-- YAML
+added: REPLACEME
+-->
+
+Print reads and writes to environment variables.
+
+The following accesses will be printed:
+
+* 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)`.
+
+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 +3172,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`
diff --git a/src/debug_utils.cc b/src/debug_utils.cc
index 9d36082db672ce..c8b3b11ee34d7a 100644
--- a/src/debug_utils.cc
+++ b/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);
 }
 
diff --git a/src/debug_utils.h b/src/debug_utils.h
index 359b8d6b421020..d4391ac987ba5b 100644
--- a/src/debug_utils.h
+++ b/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.
diff --git a/src/env.cc b/src/env.cc
index 458fd3b5611c47..290048304d2d64 100644
--- a/src/env.cc
+++ b/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,
diff --git a/src/node.cc b/src/node.cc
index 26f94be94bae70..dd52fbffac0dee 100644
--- a/src/node.cc
+++ b/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);
diff --git a/src/node_credentials.cc b/src/node_credentials.cc
index 2a7f2e878bc953..99f92bcb7b98e7 100644
--- a/src/node_credentials.cc
+++ b/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,36 @@ 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\": ", key);
+    if (has_env) {
+      fprintf(stderr, "\"%s\"\n", text->c_str());
+    } else {
+      fprintf(stderr, "undefined\n");
+    }
+
+    PrintTraceEnvStack(options);
+  }
+
+  return has_env;
 }
 
 static void SafeGetenv(const FunctionCallbackInfo<Value>& args) {
@@ -103,7 +123,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 +137,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();
   };
 
diff --git a/src/node_env_var.cc b/src/node_env_var.cc
index d19d11dc714e08..751c6718343c82 100644
--- a/src/node_env_var.cc
+++ b/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,27 @@ 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\": ",
+            static_cast<int>(key.length()),
+            key.out());
+    if (has_env) {
+      Utf8Value value_utf8(env->isolate(), value_string.ToLocalChecked());
+      fprintf(stderr,
+              "\"%.*s\"\n",
+              static_cast<int>(value_utf8.length()),
+              value_utf8.out());
+    } else {
+      fprintf(stderr, "undefined\n");
+    }
+    PrintTraceEnvStack(env);
+  }
+
+  if (has_env) {
     info.GetReturnValue().Set(value_string.ToLocalChecked());
     return Intercepted::kYes;
   }
@@ -386,6 +419,17 @@ 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);
+    Utf8Value value_utf8(env->isolate(), value_string);
+    fprintf(stderr,
+            "[--trace-env] set environment variable \"%.*s\" = \"%.*s\"\n",
+            static_cast<int>(key_utf8.length()),
+            key_utf8.out(),
+            static_cast<int>(value_utf8.length()),
+            value_utf8.out());
+    PrintTraceEnvStack(env);
+  }
 
   return Intercepted::kYes;
 }
@@ -396,7 +440,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 +466,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 +487,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()));
 }
diff --git a/src/node_internals.h b/src/node_internals.h
index 85b666e11f5654..000ba16303740d 100644
--- a/src/node_internals.h
+++ b/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,
diff --git a/src/node_options.cc b/src/node_options.cc
index 5ef5a7b6cf9386..a4a63d3daab31f 100644
--- a/src/node_options.cc
+++ b/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,
diff --git a/src/node_options.h b/src/node_options.h
index aa7a2ce3eba57b..7e4f506dd6423d 100644
--- a/src/node_options.h
+++ b/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;
diff --git a/src/path.cc b/src/path.cc
index 47ddba31443644..1f88e38a857171 100644
--- a/src/path.cc
+++ b/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
diff --git a/test/fixtures/process-env/define.js b/test/fixtures/process-env/define.js
new file mode 100644
index 00000000000000..59d5744157696a
--- /dev/null
+++ b/test/fixtures/process-env/define.js
@@ -0,0 +1,6 @@
+Object.defineProperty(process.env, 'FOO', {
+  configurable: true,
+  enumerable: true,
+  writable: true,
+  value: 'FOO',
+});
diff --git a/test/fixtures/process-env/delete.js b/test/fixtures/process-env/delete.js
new file mode 100644
index 00000000000000..19ea5160513f08
--- /dev/null
+++ b/test/fixtures/process-env/delete.js
@@ -0,0 +1 @@
+delete process.env.FOO;
diff --git a/test/fixtures/process-env/enumerate.js b/test/fixtures/process-env/enumerate.js
new file mode 100644
index 00000000000000..ea6f3972bf9470
--- /dev/null
+++ b/test/fixtures/process-env/enumerate.js
@@ -0,0 +1,3 @@
+Object.keys(process.env);
+
+const env = { ...process.env };
diff --git a/test/fixtures/process-env/get.js b/test/fixtures/process-env/get.js
new file mode 100644
index 00000000000000..e0257a022be4eb
--- /dev/null
+++ b/test/fixtures/process-env/get.js
@@ -0,0 +1,2 @@
+const foo = process.env.FOO;
+const bar = process.env.BAR;
diff --git a/test/fixtures/process-env/query.js b/test/fixtures/process-env/query.js
new file mode 100644
index 00000000000000..1e3fc9b79c3d04
--- /dev/null
+++ b/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');
diff --git a/test/fixtures/process-env/set.js b/test/fixtures/process-env/set.js
new file mode 100644
index 00000000000000..a9863742d187e2
--- /dev/null
+++ b/test/fixtures/process-env/set.js
@@ -0,0 +1 @@
+process.env.FOO = "FOO";
diff --git a/test/parallel/test-trace-env-stack.js b/test/parallel/test-trace-env-stack.js
new file mode 100644
index 00000000000000..fdec811f760c1c
--- /dev/null
+++ b/test/parallel/test-trace-env-stack.js
@@ -0,0 +1,85 @@
+'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/);
+      assert.match(output, /node::EnvGetter/);
+    }
+  }
+});
+
+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/);
+  }
+});
diff --git a/test/parallel/test-trace-env.js b/test/parallel/test-trace-env.js
new file mode 100644
index 00000000000000..ef7394d20b2af8
--- /dev/null
+++ b/test/parallel/test-trace-env.js
@@ -0,0 +1,89 @@
+'use strict';
+
+// This tests that --trace-env 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.
+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.
+    assert.match(output, /get environment variable "NODE_ICU_DATA":/);
+    assert.match(output, /get environment variable "OPENSSL_CONF":/);
+    assert.match(output, /get environment variable "NODE_EXTRA_CA_CERTS":/);
+    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":/);
+    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": "FOO"/);
+    assert.match(output, /get environment variable "BAR": undefined/);
+  }
+});
+
+// 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" = "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" = "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);
+  }
+});

From 2553316d68d1399cafb68a6fdefc8bd3f688a102 Mon Sep 17 00:00:00 2001
From: Joyee Cheung <joyeec9h3@gmail.com>
Date: Tue, 19 Nov 2024 19:12:40 +0100
Subject: [PATCH 2/5] fixup! cli: implement --trace-env and
 --trace-env-[js|native]-stack

---
 test/parallel/test-trace-env-stack.js |  3 +--
 test/parallel/test-trace-env.js       | 20 +++++++++++++++-----
 2 files changed, 16 insertions(+), 7 deletions(-)

diff --git a/test/parallel/test-trace-env-stack.js b/test/parallel/test-trace-env-stack.js
index fdec811f760c1c..3b85bf7ff2bb87 100644
--- a/test/parallel/test-trace-env-stack.js
+++ b/test/parallel/test-trace-env-stack.js
@@ -17,8 +17,7 @@ const fixtures = require('../common/fixtures');
 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/);
-      assert.match(output, /node::EnvGetter/);
+      assert.match(output, /node::credentials::SafeGetenv/);  // This is usually not optimized away.
     }
   }
 });
diff --git a/test/parallel/test-trace-env.js b/test/parallel/test-trace-env.js
index ef7394d20b2af8..b95cd7ccb5a415 100644
--- a/test/parallel/test-trace-env.js
+++ b/test/parallel/test-trace-env.js
@@ -1,7 +1,7 @@
 'use strict';
 
 // This tests that --trace-env works.
-require('../common');
+const common = require('../common');
 const { spawnSyncAndAssert } = require('../common/child_process');
 const assert = require('assert');
 const fixtures = require('../common/fixtures');
@@ -13,9 +13,15 @@ spawnSyncAndAssert(process.execPath, ['--trace-env', fixtures.path('empty.js')],
     // 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.
-    assert.match(output, /get environment variable "NODE_ICU_DATA":/);
-    assert.match(output, /get environment variable "OPENSSL_CONF":/);
-    assert.match(output, /get environment variable "NODE_EXTRA_CA_CERTS":/);
+    if (common.hasIntl) {
+      assert.match(output, /get environment variable "NODE_ICU_DATA":/);
+    }
+    if (common.hasOpenSSL) {
+      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":/);
@@ -23,7 +29,11 @@ spawnSyncAndAssert(process.execPath, ['--trace-env', fixtures.path('empty.js')],
     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":/);
-    assert.match(output, /get environment variable "HOME":/);
+    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":/);
   }

From 418bb0aa7cd431a1b2a7e1273b6596fc462e74fb Mon Sep 17 00:00:00 2001
From: Joyee Cheung <joyeec9h3@gmail.com>
Date: Thu, 21 Nov 2024 12:20:56 +0100
Subject: [PATCH 3/5] fixup! fixup! cli: implement --trace-env and
 --trace-env-[js|native]-stack

---
 test/parallel/test-trace-env.js | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/test/parallel/test-trace-env.js b/test/parallel/test-trace-env.js
index b95cd7ccb5a415..d38fcd7d50b9e1 100644
--- a/test/parallel/test-trace-env.js
+++ b/test/parallel/test-trace-env.js
@@ -16,7 +16,7 @@ spawnSyncAndAssert(process.execPath, ['--trace-env', fixtures.path('empty.js')],
     if (common.hasIntl) {
       assert.match(output, /get environment variable "NODE_ICU_DATA":/);
     }
-    if (common.hasOpenSSL) {
+    if (common.hasCrypto) {
       assert.match(output, /get environment variable "NODE_EXTRA_CA_CERTS":/);
     }
     if (common.hasOpenSSL3) {

From 8c1ff6bfbcb281bdd3fee081d6b0b58c8aab7284 Mon Sep 17 00:00:00 2001
From: Joyee Cheung <joyeec9h3@gmail.com>
Date: Tue, 26 Nov 2024 15:42:07 +0100
Subject: [PATCH 4/5] fixup! fixup! fixup! cli: implement --trace-env and
 --trace-env-[js|native]-stack

---
 src/node_credentials.cc         |  7 +------
 src/node_env_var.cc             | 18 +++--------------
 test/parallel/test-trace-env.js | 36 ++++++++++++++++-----------------
 3 files changed, 22 insertions(+), 39 deletions(-)

diff --git a/src/node_credentials.cc b/src/node_credentials.cc
index 99f92bcb7b98e7..0152f6269c83ef 100644
--- a/src/node_credentials.cc
+++ b/src/node_credentials.cc
@@ -104,12 +104,7 @@ bool SafeGetenv(const char* key, std::string* text, Environment* env) {
                       : per_process::cli_options->per_isolate->per_env);
 
   if (options->trace_env) {
-    fprintf(stderr, "[--trace-env] get environment variable \"%s\": ", key);
-    if (has_env) {
-      fprintf(stderr, "\"%s\"\n", text->c_str());
-    } else {
-      fprintf(stderr, "undefined\n");
-    }
+    fprintf(stderr, "[--trace-env] get environment variable \"%s\"\n", key);
 
     PrintTraceEnvStack(options);
   }
diff --git a/src/node_env_var.cc b/src/node_env_var.cc
index 751c6718343c82..82b8231c435775 100644
--- a/src/node_env_var.cc
+++ b/src/node_env_var.cc
@@ -366,18 +366,9 @@ static Intercepted EnvGetter(Local<Name> property,
   if (env->options()->trace_env) {
     Utf8Value key(env->isolate(), property.As<String>());
     fprintf(stderr,
-            "[--trace-env] get environment variable \"%.*s\": ",
+            "[--trace-env] get environment variable \"%.*s\"\n",
             static_cast<int>(key.length()),
             key.out());
-    if (has_env) {
-      Utf8Value value_utf8(env->isolate(), value_string.ToLocalChecked());
-      fprintf(stderr,
-              "\"%.*s\"\n",
-              static_cast<int>(value_utf8.length()),
-              value_utf8.out());
-    } else {
-      fprintf(stderr, "undefined\n");
-    }
     PrintTraceEnvStack(env);
   }
 
@@ -421,13 +412,10 @@ 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);
-    Utf8Value value_utf8(env->isolate(), value_string);
     fprintf(stderr,
-            "[--trace-env] set environment variable \"%.*s\" = \"%.*s\"\n",
+            "[--trace-env] set environment variable \"%.*s\"\n",
             static_cast<int>(key_utf8.length()),
-            key_utf8.out(),
-            static_cast<int>(value_utf8.length()),
-            value_utf8.out());
+            key_utf8.out());
     PrintTraceEnvStack(env);
   }
 
diff --git a/test/parallel/test-trace-env.js b/test/parallel/test-trace-env.js
index d38fcd7d50b9e1..c67924e5c610d3 100644
--- a/test/parallel/test-trace-env.js
+++ b/test/parallel/test-trace-env.js
@@ -14,28 +14,28 @@ spawnSyncAndAssert(process.execPath, ['--trace-env', fixtures.path('empty.js')],
     // 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":/);
+      assert.match(output, /get environment variable "NODE_ICU_DATA"/);
     }
     if (common.hasCrypto) {
-      assert.match(output, /get environment variable "NODE_EXTRA_CA_CERTS":/);
+      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 "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":/);
+    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":/);
+      assert.match(output, /get environment variable "USERPROFILE"/);
     } else {
-      assert.match(output, /get environment variable "HOME":/);
+      assert.match(output, /get environment variable "HOME"/);
     }
-    assert.match(output, /get environment variable "NODE_PATH":/);
-    assert.match(output, /get environment variable "WATCH_REPORT_DEPENDENCIES":/);
+    assert.match(output, /get environment variable "NODE_PATH"/);
+    assert.match(output, /get environment variable "WATCH_REPORT_DEPENDENCIES"/);
   }
 });
 
@@ -48,22 +48,22 @@ spawnSyncAndAssert(process.execPath, ['--trace-env', fixtures.path('process-env'
   }
 }, {
   stderr(output) {
-    assert.match(output, /get environment variable "FOO": "FOO"/);
-    assert.match(output, /get environment variable "BAR": undefined/);
+    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" = "FOO"/);
+    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" = "FOO"/);
+    assert.match(output, /set environment variable "FOO"/);
   }
 });
 

From 81713b58e64a72a18ce6bb6adf1bc4a1e2f4fc71 Mon Sep 17 00:00:00 2001
From: Joyee Cheung <joyeec9h3@gmail.com>
Date: Tue, 26 Nov 2024 15:44:33 +0100
Subject: [PATCH 5/5] fixup! fixup! fixup! fixup! cli: implement --trace-env
 and --trace-env-[js|native]-stack

---
 doc/api/cli.md | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/doc/api/cli.md b/doc/api/cli.md
index 7a459f4677aa5e..f3a4b20016e890 100644
--- a/doc/api/cli.md
+++ b/doc/api/cli.md
@@ -2594,9 +2594,8 @@ Print stack traces for deprecations.
 added: REPLACEME
 -->
 
-Print reads and writes to environment variables.
-
-The following accesses will be printed:
+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"`.
@@ -2607,6 +2606,8 @@ The following accesses will be printed:
 * 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`.