fix(node): Prevent node:child_process from always inheriting the pare…

…nt 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

@@ -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,

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>();