src: don't kill own process group on failed spawn

libuv only assigns a pid to the process handle once uv_spawn() has
succeeded, so a child that never started keeps pid 0. Calling kill()
on such a child still reached uv_process_kill(), which ended up in
kill(0, signal) and signalled every process in the caller's own
process group, Node included.

Return ESRCH when the handle has no pid, and zero the pid in the
constructor so the check never reads an unassigned value.

Signed-off-by: Lazizbek Ergashev <lazerg2@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/65054
Fixes: https://github.com/nodejs/node/issues/65052
Reviewed-By: Aviv Keller <me@aviv.sh>
Reviewed-By: Robert Nagy <ronagy@icloud.com>

src/process_wrap.cc

@@ -112,6 +112,7 @@ class ProcessWrap : public HandleWrap {
                    object,
                    reinterpret_cast<uv_handle_t*>(&process_),
                    AsyncWrap::PROVIDER_PROCESSWRAP) {
+    process_.pid = 0;
     MarkAsUninitialized();
   }
 
@@ -355,7 +356,10 @@ class ProcessWrap : public HandleWrap {
       signal = SIGKILL;
     }
 #endif
-    int err = uv_process_kill(&wrap->process_, signal);
+    // uv_spawn() only assigns a pid when it succeeds, and kill(0, signal)
+    // signals every process in our own process group.
+    int err = wrap->process_.pid > 0 ? uv_process_kill(&wrap->process_, signal)
+                                     : UV_ESRCH;
     args.GetReturnValue().Set(err);
   }
 

test/fixtures/child-process-kill-spawn-error.js

@@ -0,0 +1,4 @@
+const { spawn } = require('child_process');
+const child = spawn('foo123');
+child.on('error', () => {});
+if (child.kill() !== false || child.killed !== false) process.exit(1);

test/parallel/test-child-process-kill-spawn-error.js

@@ -0,0 +1,16 @@
+'use strict';
+const common = require('../common');
+const fixtures = require('../common/fixtures');
+const assert = require('assert');
+const { spawn } = require('child_process');
+
+// Killing a child process that never spawned must not signal the process
+// group of the caller. The check runs in a detached child so that a
+// regression cannot take the test runner down with it.
+const childPath = fixtures.path('child-process-kill-spawn-error.js');
+const child = spawn(process.execPath, [childPath], { detached: true });
+
+child.on('exit', common.mustCall((code, signal) => {
+  assert.strictEqual(signal, null);
+  assert.strictEqual(code, 0);
+}));