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

joyeecheung · nodejs-github-bot · commit ab14dedc0533 · 2024-12-04T19:05:50.000Z
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. PR-URL: nodejs#55604 Reviewed-By: Chengzhong Wu <legendecas@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
diff --git a/doc/api/cli.md b/doc/api/cli.md
@@ -2572,6 +2572,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
@@ -3118,6 +3157,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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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();
   };
 
diff --git 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,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()));
 }
diff --git 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
@@ -759,6 +759,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
@@ -208,6 +208,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
@@ -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
@@ -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
@@ -0,0 +1 @@
+delete process.env.FOO;
diff --git a/test/fixtures/process-env/enumerate.js 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
@@ -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
@@ -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
@@ -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
diff --git a/test/parallel/test-trace-env.js b/test/parallel/test-trace-env.js

Original file line number	Diff line number	Diff line change
`@@ -993,7 +993,7 @@ InitializeOncePerProcessInternal(const std::vector<std::string>& args,`
`993`	`993`	`if (!(flags & ProcessInitializationFlags::kNoParseGlobalDebugVariables)) {`
`994`	`994`	`// Initialized the enabled list for Debug() calls with system`
`995`	`995`	`// environment variables.`
`996`		`- per_process::enabled_debug_list.Parse(per_process::system_environment);`
	`996`	`+ per_process::enabled_debug_list.Parse(nullptr);`
`997`	`997`	`}`
`998`	`998`
`999`	`999`	`PlatformInit(flags);`