diff --git a/doc/api/cli.md b/doc/api/cli.md index 3b1cf0b22401eb..8126e662725630 100644 --- a/doc/api/cli.md +++ b/doc/api/cli.md @@ -2604,6 +2604,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 @@ -3150,6 +3189,9 @@ one is included in the list below. * `--tls-min-v1.3` * `--trace-atomics-wait` * `--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 130524cad713e8..db7c539e5ad97e 100644 --- a/src/env.cc +++ b/src/env.cc @@ -875,9 +875,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 @@ -887,6 +884,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); @@ -1115,8 +1119,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; } @@ -1128,7 +1131,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 1a2a43bdd37441..0501003ebc1dde 100644 --- a/src/node.cc +++ b/src/node.cc @@ -1045,7 +1045,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..0152f6269c83ef 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,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 index d19d11dc714e08..82b8231c435775 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,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 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 c7af48e0015b07..d9a69480e3906e 100644 --- a/src/node_options.cc +++ b/src/node_options.cc @@ -775,6 +775,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 b7c340399475e2..d1a2ce76a4c2e3 100644 --- a/src/node_options.h +++ b/src/node_options.h @@ -210,6 +210,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 4068e1d892d6b7..5fb745832e492d 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..3b85bf7ff2bb87 --- /dev/null +++ b/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/); + } +}); diff --git a/test/parallel/test-trace-env.js b/test/parallel/test-trace-env.js new file mode 100644 index 00000000000000..c67924e5c610d3 --- /dev/null +++ b/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); + } +});