From 5a39f2f09675f4f2ea0198bd8b8e59c9ade703dc Mon Sep 17 00:00:00 2001
From: TateKennington <TateKennington@users.noreply.github.com>
Date: Tue, 14 Jan 2025 10:46:56 +1300
Subject: [PATCH] fix(node): Prevent node:child_process from always inheriting
 the parent environment (#27343) (#27340)

Fixes #27343

Currently the node:child_process polyfill is always passing the full
parent environment to all spawned subprocesses. In the case where
`options.env` is provided those keys are overridden but the rest of the
parent environment is still passed through.

On Node the behaviour is for child processes to only inherit the parent
environment when `options.env` isn't specified. When `options.env` is
specified the child process inherits only those keys.

This PR updates the internal node child_process polyfill so that the
`clearEnv` argument is set to true when spawning the subprocess to
prevent the parent environment always being inherited by default. It
also fixes an issue where `normalizeSpawnArguments` wasn't returning the
`env` option if `options.env` was unset.
---
 ext/node/polyfills/internal/child_process.ts |  2 +
 tests/unit_node/child_process_test.ts        | 67 ++++++++++++++++++++
 2 files changed, 69 insertions(+)

diff --git a/ext/node/polyfills/internal/child_process.ts b/ext/node/polyfills/internal/child_process.ts
index 17809cc559..569ee7d328 100644
--- a/ext/node/polyfills/internal/child_process.ts
+++ b/ext/node/polyfills/internal/child_process.ts
@@ -277,6 +277,7 @@ export class ChildProcess extends EventEmitter {
     try {
       this.#process = new Deno.Command(cmd, {
         args: cmdArgs,
+        clearEnv: true,
         cwd,
         env: stringEnv,
         stdin: toDenoStdio(stdin),
@@ -839,6 +840,7 @@ export function normalizeSpawnArguments(
     args,
     cwd,
     detached: !!options.detached,
+    env,
     envPairs,
     file,
     windowsHide: !!options.windowsHide,
diff --git a/tests/unit_node/child_process_test.ts b/tests/unit_node/child_process_test.ts
index bd875ad419..1732b9d2bf 100644
--- a/tests/unit_node/child_process_test.ts
+++ b/tests/unit_node/child_process_test.ts
@@ -656,6 +656,73 @@ Deno.test({
   },
 });
 
+Deno.test({
+  name:
+    "[node/child_process spawn] child inherits Deno.env when options.env is not provided",
+  async fn() {
+    const deferred = withTimeout<string>();
+    Deno.env.set("BAR", "BAR");
+    const env = spawn(
+      `"${Deno.execPath()}" eval -p "Deno.env.toObject().BAR"`,
+      {
+        shell: true,
+      },
+    );
+    try {
+      let envOutput = "";
+
+      assert(env.stdout);
+      env.on("error", (err: Error) => deferred.reject(err));
+      env.stdout.on("data", (data) => {
+        envOutput += data;
+      });
+      env.on("close", () => {
+        deferred.resolve(envOutput.trim());
+      });
+      await deferred.promise;
+    } finally {
+      env.kill();
+      Deno.env.delete("BAR");
+    }
+    const value = await deferred.promise;
+    assertEquals(value, "BAR");
+  },
+});
+
+Deno.test({
+  name:
+    "[node/child_process spawn] child doesn't inherit Deno.env when options.env is provided",
+  async fn() {
+    const deferred = withTimeout<string>();
+    Deno.env.set("BAZ", "BAZ");
+    const env = spawn(
+      `"${Deno.execPath()}" eval -p "Deno.env.toObject().BAZ"`,
+      {
+        env: {},
+        shell: true,
+      },
+    );
+    try {
+      let envOutput = "";
+
+      assert(env.stdout);
+      env.on("error", (err: Error) => deferred.reject(err));
+      env.stdout.on("data", (data) => {
+        envOutput += data;
+      });
+      env.on("close", () => {
+        deferred.resolve(envOutput.trim());
+      });
+      await deferred.promise;
+    } finally {
+      env.kill();
+      Deno.env.delete("BAZ");
+    }
+    const value = await deferred.promise;
+    assertEquals(value, "undefined");
+  },
+});
+
 // Regression test for https://github.com/denoland/deno/issues/20373
 Deno.test(async function undefinedValueInEnvVar() {
   const deferred = withTimeout<string>();