src: add cli option to preserve env vars on dr

nodejs · nodejs-github-bot · Nov 8, 2024 · Nov 2, 2024 · Nov 5, 2024 · Nov 7, 2024
commit ac4015d2f68c9a39f8c3d6e448c864abd0d39fba
diff --git a/doc/api/cli.md b/doc/api/cli.md
@@ -2058,6 +2058,15 @@ Enables report to be generated upon receiving the specified (or predefined)
 signal to the running Node.js process. The signal to trigger the report is
 specified through `--report-signal`.
 
+### `--report-preserve-env`
+
+<!-- YAML
+added: REPLACEME
+-->
+
+When `--report-preserve-env` is passed the diagnostic report generated will not
+contain the `environmentVariables` data.
+
 ### `--report-signal=signal`
 
 <!-- YAML

diff --git a/doc/api/process.md b/doc/api/process.md
@@ -3494,6 +3494,16 @@ const { report } = require('node:process');
 console.log(`Report on exception: ${report.reportOnUncaughtException}`);
 ```
 
+### `process.report.preserveEnv`
+
+<!-- YAML
+added: REPLACEME
+-->
+
+* {boolean}
+
+If `true`, a diagnostic report is generated without the environment variables.
+
 ### `process.report.signal`
 
 <!-- YAML

diff --git a/lib/internal/process/report.js b/lib/internal/process/report.js
@@ -105,6 +105,9 @@ const report = {
 
     nr.setReportOnUncaughtException(trigger);
   },
+  get preserveEnv() {
+    return !nr.shouldPreserveEnvironmentVariables();
+  },
 };
 
 function addSignalHandler(sig) {

diff --git a/src/env-inl.h b/src/env-inl.h
@@ -904,6 +904,10 @@ inline void Environment::RemoveHeapSnapshotNearHeapLimitCallback(
                                         heap_limit);
 }
 
+inline bool Environment::ShouldPreserveEnvOnReport() const {
+  return options_->report_preserve_env;
+}
+
 inline void Environment::SetAsyncResourceContextFrame(
     std::uintptr_t async_resource_handle,
     v8::Global<v8::Value>&& context_frame) {

diff --git a/src/env.h b/src/env.h
@@ -1048,6 +1048,8 @@ class Environment final : public MemoryRetainer {
 
   inline void RemoveHeapSnapshotNearHeapLimitCallback(size_t heap_limit);
 
+  inline bool ShouldPreserveEnvOnReport() const;
+
   // Field identifiers for exit_info_
   enum ExitInfoField {
     kExiting = 0,

diff --git a/src/node_options.cc b/src/node_options.cc
@@ -383,6 +383,10 @@ EnvironmentOptionsParser::EnvironmentOptionsParser() {
             " (default: current working directory)",
             &EnvironmentOptions::diagnostic_dir,
             kAllowedInEnvvar);
+  AddOption("--report-preserve-env",
+            "Preserve environment variables when generating report",
+            &EnvironmentOptions::report_preserve_env,
+            kAllowedInEnvvar);
   AddOption("--dns-result-order",
             "set default value of verbatim in dns.lookup. Options are "
             "'ipv4first' (IPv4 addresses are placed before IPv6 addresses) "

diff --git a/src/node_options.h b/src/node_options.h
@@ -177,6 +177,7 @@ class EnvironmentOptions : public Options {
 #endif  // HAVE_INSPECTOR
   std::string redirect_warnings;
   std::string diagnostic_dir;
+  bool report_preserve_env = false;
   std::string env_file;
   std::string optional_env_file;
   bool has_env_file_string = false;

diff --git a/src/node_report.cc b/src/node_report.cc
@@ -78,6 +78,7 @@ static void PrintJavaScriptErrorProperties(JSONWriter* writer,
 static void PrintNativeStack(JSONWriter* writer);
 static void PrintResourceUsage(JSONWriter* writer);
 static void PrintGCStatistics(JSONWriter* writer, Isolate* isolate);
+static void PrintEnvironmentVariables(JSONWriter* writer);
 static void PrintSystemInformation(JSONWriter* writer);
 static void PrintLoadedLibraries(JSONWriter* writer);
 static void PrintComponentVersions(JSONWriter* writer);
@@ -249,6 +250,9 @@ static void WriteNodeReport(Isolate* isolate,
   writer.json_arrayend();
 
   // Report operating system information
+  if (env->ShouldPreserveEnvOnReport()) {
+    PrintEnvironmentVariables(&writer);
+  }
   PrintSystemInformation(&writer);
 
   writer.json_objectend();
@@ -694,8 +698,7 @@ static void PrintResourceUsage(JSONWriter* writer) {
 #endif  // RUSAGE_THREAD
 }
 
-// Report operating system information.
-static void PrintSystemInformation(JSONWriter* writer) {
+static void PrintEnvironmentVariables(JSONWriter* writer) {
   uv_env_item_t* envitems;
   int envcount;
   int r;
@@ -715,7 +718,10 @@ static void PrintSystemInformation(JSONWriter* writer) {
   }
 
   writer->json_objectend();
+}
 
+// Report operating system information.
+static void PrintSystemInformation(JSONWriter* writer) {
 #ifndef _WIN32
   static struct {
     const char* description;

diff --git a/src/node_report_module.cc b/src/node_report_module.cc
@@ -170,6 +170,12 @@ static void ShouldReportOnUncaughtException(
       env->isolate_data()->options()->report_uncaught_exception);
 }
 
+static void ShouldPreserveEnvironmentVariables(
+    const FunctionCallbackInfo<Value>& info) {
+  Environment* env = Environment::GetCurrent(info);
+  info.GetReturnValue().Set(env->ShouldPreserveEnvOnReport());
+}
+
 static void SetReportOnUncaughtException(
     const FunctionCallbackInfo<Value>& info) {
   Environment* env = Environment::GetCurrent(info);
@@ -202,6 +208,10 @@ static void Initialize(Local<Object> exports,
             exports,
             "shouldReportOnUncaughtException",
             ShouldReportOnUncaughtException);
+  SetMethod(context,
+            exports,
+            "shouldPreserveEnvironmentVariables",
+            ShouldPreserveEnvironmentVariables);
   SetMethod(context,
             exports,
             "setReportOnUncaughtException",
@@ -226,6 +236,7 @@ void RegisterExternalReferences(ExternalReferenceRegistry* registry) {
   registry->Register(ShouldReportOnSignal);
   registry->Register(SetReportOnSignal);
   registry->Register(ShouldReportOnUncaughtException);
+  registry->Register(ShouldPreserveEnvironmentVariables);
   registry->Register(SetReportOnUncaughtException);
 }
 

diff --git a/test/common/report.js b/test/common/report.js
@@ -59,7 +59,11 @@ function _validateContent(report, fields = []) {
 
   // Verify that all sections are present as own properties of the report.
   const sections = ['header', 'nativeStack', 'javascriptStack', 'libuv',
-                    'environmentVariables', 'sharedObjects', 'resourceUsage', 'workers'];
+                    'sharedObjects', 'resourceUsage', 'workers'];
+
+  if (!process.report.preserveEnv)
+    sections.push('environmentVariables');
+
   if (!isWindows)
     sections.push('userLimits');
 

diff --git a/test/report/test-report-writereport-preserve-env.js b/test/report/test-report-writereport-preserve-env.js
@@ -0,0 +1,80 @@
+// Flags: --report-preserve-env
+'use strict';
+
+// Test producing a report via API call, using the no-hooks/no-signal interface.
+require('../common');
+const assert = require('assert');
+const fs = require('fs');
+const path = require('path');
+const helper = require('../common/report');
+const tmpdir = require('../common/tmpdir');
+
+tmpdir.refresh();
+process.report.directory = tmpdir.path;
+
+function validate() {
+  const reports = helper.findReports(process.pid, tmpdir.path);
+  assert.strictEqual(reports.length, 1);
+  helper.validate(reports[0], arguments[0]);
+  fs.unlinkSync(reports[0]);
+  return reports[0];
+}
+
+{
+  // Test with no arguments.
+  process.report.writeReport();
+  validate();
+}
+
+{
+  // Test with an error argument.
+  process.report.writeReport(new Error('test error'));
+  validate();
+}
+
+{
+  // Test with an error with one line stack
+  const error = new Error();
+  error.stack = 'only one line';
+  process.report.writeReport(error);
+  validate();
+}
+
+{
+  const error = new Error();
+  error.foo = 'goo';
+  process.report.writeReport(error);
+  validate([['javascriptStack.errorProperties.foo', 'goo']]);
+}
+
+{
+  // Test with a file argument.
+  const file = process.report.writeReport('custom-name-1.json');
+  const absolutePath = tmpdir.resolve(file);
+  assert.strictEqual(helper.findReports(process.pid, tmpdir.path).length, 0);
+  assert.strictEqual(file, 'custom-name-1.json');
+  helper.validate(absolutePath);
+  fs.unlinkSync(absolutePath);
+}
+
+{
+  // Test with file and error arguments.
+  const file = process.report.writeReport('custom-name-2.json',
+                                          new Error('test error'));
+  const absolutePath = tmpdir.resolve(file);
+  assert.strictEqual(helper.findReports(process.pid, tmpdir.path).length, 0);
+  assert.strictEqual(file, 'custom-name-2.json');
+  helper.validate(absolutePath);
+  fs.unlinkSync(absolutePath);
+}
+
+{
+  // Test with a filename option.
+  process.report.filename = 'custom-name-3.json';
+  const file = process.report.writeReport();
+  assert.strictEqual(helper.findReports(process.pid, tmpdir.path).length, 0);
+  const filename = path.join(process.report.directory, 'custom-name-3.json');
+  assert.strictEqual(file, process.report.filename);
+  helper.validate(filename);
+  fs.unlinkSync(filename);
+}