From 706c397ffd1f7d1c5103ed75018b3f5096062135 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 17809cc5599cba..569ee7d328556d 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 bd875ad4195c13..1732b9d2bf48f0 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>();