fix(server): harden findExecutable + remove gratuitous realpath from spawn paths (#787)

* Phase 0: baseline tests for probeExecutable

* Phase 1: invert self node env handling

* Phase 2: refactor probeExecutable to execCommand

* Phase 3: push POSIX executable lookup to system which

* Fix probe test spawn-failure fixture

* Guard system which executable lookup

packages/server/src/utils/executable.probe.test.ts

@@ -0,0 +1,159 @@
+import {
+  chmodSync,
+  existsSync,
+  mkdirSync,
+  mkdtempSync,
+  readFileSync,
+  rmSync,
+  writeFileSync,
+} from "node:fs";
+import os from "node:os";
+import path from "node:path";
+import { performance } from "node:perf_hooks";
+import { afterEach, describe, expect, test } from "vitest";
+
+import { probeExecutable } from "./executable.js";
+
+const timeoutMs = 1000;
+const timeoutSlackMs = 500;
+const tempDirs: string[] = [];
+
+interface ProbeFixture {
+  name: string;
+  expected: boolean;
+  create: (dir: string) => { executablePath: string; pidFile?: string };
+}
+
+function makeTempDir(): string {
+  const dir = mkdtempSync(path.join(os.tmpdir(), "paseo-probe-test-"));
+  tempDirs.push(dir);
+  return dir;
+}
+
+function writeExecutable(filePath: string, content: string | Buffer): string {
+  writeFileSync(filePath, content);
+  if (process.platform !== "win32") {
+    chmodSync(filePath, 0o755);
+  }
+  return filePath;
+}
+
+function scriptPath(dir: string, name: string): string {
+  return process.platform === "win32" ? path.join(dir, `${name}.cmd`) : path.join(dir, name);
+}
+
+function createHangingFixture(dir: string): string {
+  if (process.platform === "win32") {
+    return writeExecutable(
+      scriptPath(dir, "hangs"),
+      "@echo off\r\n:loop\r\ntimeout /T 5 /NOBREAK > NUL\r\ngoto loop\r\n",
+    );
+  }
+  return writeExecutable(
+    scriptPath(dir, "hangs"),
+    `#!/bin/sh\ntrap '' TERM\necho $$ > "${path.join(dir, "hangs.pid")}"\nwhile :; do :; done\n`,
+  );
+}
+
+function createNoVersionFixture(dir: string): string {
+  if (process.platform === "win32") {
+    return writeExecutable(scriptPath(dir, "no-version"), "@echo off\r\nexit /b 0\r\n");
+  }
+  return writeExecutable(scriptPath(dir, "no-version"), "#!/bin/sh\nexit 0\n");
+}
+
+function createNonZeroFixture(dir: string): string {
+  if (process.platform === "win32") {
+    return writeExecutable(
+      scriptPath(dir, "non-zero"),
+      "@echo off\r\necho oops 1>&2\r\nexit /b 1\r\n",
+    );
+  }
+  return writeExecutable(scriptPath(dir, "non-zero"), "#!/bin/sh\necho oops 1>&2\nexit 1\n");
+}
+
+function createSlowSuccessFixture(dir: string): string {
+  if (process.platform === "win32") {
+    return writeExecutable(
+      scriptPath(dir, "slow-success"),
+      "@echo off\r\nping -n 1 127.0.0.1 > NUL\r\nexit /b 0\r\n",
+    );
+  }
+  return writeExecutable(scriptPath(dir, "slow-success"), "#!/bin/sh\nsleep 0.05\nexit 0\n");
+}
+
+function createDirectoryFixture(dir: string): string {
+  const directoryPath = path.join(dir, "candidate-directory");
+  mkdirSync(directoryPath);
+  return directoryPath;
+}
+
+function missingAbsolutePath(): string {
+  return process.platform === "win32" ? "C:\\no\\such\\path.exe" : "/no/such/path";
+}
+
+async function waitForFile(filePath: string): Promise<void> {
+  const deadline = performance.now() + timeoutSlackMs;
+  while (!existsSync(filePath) && performance.now() < deadline) {
+    await new Promise((resolve) => setTimeout(resolve, 10));
+  }
+}
+
+const fixtures: ProbeFixture[] = [
+  {
+    name: "hangs forever after starting",
+    expected: true,
+    create: (dir) => ({
+      executablePath: createHangingFixture(dir),
+      pidFile: process.platform === "win32" ? undefined : path.join(dir, "hangs.pid"),
+    }),
+  },
+  {
+    name: "does not know --version and exits zero",
+    expected: true,
+    create: (dir) => ({ executablePath: createNoVersionFixture(dir) }),
+  },
+  {
+    name: "exits non-zero immediately",
+    expected: true,
+    create: (dir) => ({ executablePath: createNonZeroFixture(dir) }),
+  },
+  {
+    name: "starts slowly and exits zero",
+    expected: true,
+    create: (dir) => ({ executablePath: createSlowSuccessFixture(dir) }),
+  },
+  {
+    name: "points at a directory",
+    expected: false,
+    create: (dir) => ({ executablePath: createDirectoryFixture(dir) }),
+  },
+  {
+    name: "does not exist at an absolute path",
+    expected: false,
+    create: () => ({ executablePath: missingAbsolutePath() }),
+  },
+];
+
+afterEach(() => {
+  for (const dir of tempDirs.splice(0)) {
+    rmSync(dir, { recursive: true, force: true });
+  }
+});
+
+describe("probeExecutable", () => {
+  test.each(fixtures)("$name", async ({ create, expected }) => {
+    const { executablePath, pidFile } = create(makeTempDir());
+    const startedAt = performance.now();
+
+    const result = await probeExecutable(executablePath, timeoutMs);
+
+    expect(result).toBe(expected);
+    expect(performance.now() - startedAt).toBeLessThanOrEqual(timeoutMs + timeoutSlackMs);
+    if (pidFile) {
+      await waitForFile(pidFile);
+      const pid = Number(readFileSync(pidFile, "utf8"));
+      expect(() => process.kill(pid, 0)).toThrow(expect.objectContaining({ code: "ESRCH" }));
+    }
+  });
+});

packages/server/src/utils/executable.ts

@@ -1,8 +1,7 @@
-import type { ChildProcess } from "node:child_process";
 import { createRequire } from "node:module";
 import { existsSync } from "node:fs";
 import { extname } from "node:path";
-import { spawnProcess } from "./spawn.js";
+import { execCommand } from "./spawn.js";
 import { isWindowsCommandScript } from "./windows-command.js";
 
 export { quoteWindowsArgument, quoteWindowsCommand } from "./windows-command.js";
@@ -18,6 +17,25 @@ function hasPathSeparator(value: string): boolean {
 }
 
 async function enumerateCandidates(name: string): Promise<string[]> {
+  if (process.platform !== "win32" && existsSync("/usr/bin/which")) {
+    return enumerateCandidatesViaSystemWhich(name);
+  }
+  return enumerateCandidatesViaLibrary(name);
+}
+
+async function enumerateCandidatesViaSystemWhich(name: string): Promise<string[]> {
+  try {
+    const { stdout } = await execCommand("/usr/bin/which", ["-a", name], {
+      timeout: 3000,
+      killSignal: "SIGKILL",
+    });
+    return Array.from(new Set(stdout.trim().split("\n").filter(Boolean)));
+  } catch {
+    return [];
+  }
+}
+
+async function enumerateCandidatesViaLibrary(name: string): Promise<string[]> {
   let candidates: string[];
   try {
     candidates = await which(name, { all: true });
@@ -39,57 +57,42 @@ async function enumerateCandidates(name: string): Promise<string[]> {
   });
 }
 
-async function probeExecutable(executablePath: string): Promise<boolean> {
-  return await new Promise((resolve) => {
-    let pendingResolve: ((result: boolean) => void) | null = resolve;
-    let started = false;
-    let timer: NodeJS.Timeout | undefined;
-
-    const settle = (result: boolean) => {
-      if (!pendingResolve) {
-        return;
-      }
-      const fn = pendingResolve;
-      pendingResolve = null;
-      if (timer) {
-        clearTimeout(timer);
-      }
-      fn(result);
-    };
-
-    let child: ChildProcess;
-    try {
-      child = spawnProcess(executablePath, ["--version"], {
-        stdio: "ignore",
-        // Windows batch shims (.cmd/.bat) require cmd.exe; native binaries do not.
-        shell: isWindowsCommandScript(executablePath),
-      });
-    } catch {
-      settle(false);
-      return;
-    }
-
-    timer = setTimeout(() => {
-      if (started) {
-        child.kill();
-        settle(true);
-        return;
-      }
-      settle(false);
-    }, PROBE_TIMEOUT_MS) as unknown as NodeJS.Timeout;
-    timer.unref();
-
-    child.once("spawn", () => {
-      started = true;
+export async function probeExecutable(
+  executablePath: string,
+  timeoutMs = PROBE_TIMEOUT_MS,
+): Promise<boolean> {
+  try {
+    await execCommand(executablePath, ["--version"], {
+      timeout: timeoutMs,
+      killSignal: "SIGKILL",
+      maxBuffer: 64 * 1024,
+      shell: isWindowsCommandScript(executablePath),
     });
-    child.once("error", () => {
-      // ENOENT/EACCES/EPERM/UNKNOWN here means the OS could not start the candidate.
-      settle(started);
-    });
-    child.once("exit", () => {
-      settle(started);
-    });
-  });
+    return true;
+  } catch (error) {
+    return classifyProbeError(error);
+  }
+}
+
+function classifyProbeError(error: unknown): boolean {
+  const err = error as NodeJS.ErrnoException & {
+    killed?: boolean;
+  };
+  if (err.killed) {
+    return true;
+  }
+  if (typeof err.code === "number") {
+    return true;
+  }
+  if (
+    err.code === "ENOENT" ||
+    err.code === "EACCES" ||
+    err.code === "ENOEXEC" ||
+    err.code === "UNKNOWN"
+  ) {
+    return false;
+  }
+  return false;
 }
 
 /**
@@ -109,18 +112,23 @@ export function executableExists(
   return null;
 }
 
-export async function findExecutable(name: string): Promise<string | null> {
+export async function findExecutable(
+  name: string,
+  probeTimeoutMs = PROBE_TIMEOUT_MS,
+): Promise<string | null> {
   const trimmed = name.trim();
   if (!trimmed) {
     return null;
   }
 
   if (hasPathSeparator(trimmed)) {
-    return (await probeExecutable(trimmed)) ? trimmed : null;
+    return (await probeExecutable(trimmed, probeTimeoutMs)) ? trimmed : null;
   }
 
   const candidates = await enumerateCandidates(trimmed);
-  const probeResults = await Promise.all(candidates.map((candidate) => probeExecutable(candidate)));
+  const probeResults = await Promise.all(
+    candidates.map((candidate) => probeExecutable(candidate, probeTimeoutMs)),
+  );
   const firstMatch = probeResults.findIndex((result) => result);
   return firstMatch === -1 ? null : candidates[firstMatch];
 }

packages/server/src/utils/spawn.test.ts

@@ -1,8 +1,10 @@
 import { mkdtempSync, realpathSync, rmSync } from "node:fs";
+import * as fs from "node:fs";
 import { tmpdir } from "node:os";
 import path from "node:path";
-import { afterEach, describe, expect, test } from "vitest";
+import { afterEach, describe, expect, test, vi } from "vitest";
 
+import { buildSelfNodeCommand } from "../server/paseo-env.js";
 import { execCommand, spawnProcess } from "./spawn.js";
 
 const printEnvScript = `
@@ -93,7 +95,7 @@ describe("execCommand", () => {
     expect(parsePrintedEnv(result.stdout)).toEqual({
       CUSTOM: "from-overlay",
       ELECTRON_NO_ATTACH_CONSOLE: null,
-      ELECTRON_RUN_AS_NODE: "1",
+      ELECTRON_RUN_AS_NODE: null,
       PASEO_DESKTOP_MANAGED: null,
       PASEO_NODE_ENV: null,
       PASEO_SUPERVISED: null,
@@ -150,7 +152,7 @@ describe("execCommand", () => {
     expect(parsePrintedEnv(Buffer.concat(stdoutChunks).toString())).toEqual({
       CUSTOM: "spawn-overlay",
       ELECTRON_NO_ATTACH_CONSOLE: null,
-      ELECTRON_RUN_AS_NODE: "1",
+      ELECTRON_RUN_AS_NODE: null,
       PASEO_DESKTOP_MANAGED: null,
       PASEO_NODE_ENV: null,
       PASEO_SUPERVISED: null,
@@ -180,4 +182,37 @@ describe("execCommand", () => {
       PASEO_SUPERVISED: "1",
     });
   });
+
+  test("does not realpath commands while finalizing external command env", async () => {
+    const realpathSpy = vi.spyOn(fs.realpathSync, "native");
+
+    await execCommand("/some/random/binary", ["--version"], {
+      env: {
+        PATH: process.env.PATH,
+      },
+      timeout: 100,
+    }).catch(() => {});
+
+    expect(realpathSpy).not.toHaveBeenCalled();
+  });
+
+  test("self node command explicitly enables Electron node mode", async () => {
+    const command = buildSelfNodeCommand(["-e", printEnvScript], {
+      CUSTOM: "from-helper",
+    });
+
+    const result = await execCommand(command.command, command.args, {
+      env: command.env,
+      envMode: "internal",
+    });
+
+    expect(parsePrintedEnv(result.stdout)).toEqual({
+      CUSTOM: "from-helper",
+      ELECTRON_NO_ATTACH_CONSOLE: null,
+      ELECTRON_RUN_AS_NODE: "1",
+      PASEO_DESKTOP_MANAGED: null,
+      PASEO_NODE_ENV: null,
+      PASEO_SUPERVISED: null,
+    });
+  });
 });

packages/server/src/utils/spawn.ts

@@ -23,8 +23,10 @@ export type SpawnProcessOptions = Omit<SpawnOptions, "env"> & ExternalEnvOptions
 interface ExecCommandOptions extends ExternalEnvOptions {
   cwd?: string;
   encoding?: BufferEncoding;
+  killSignal?: NodeJS.Signals;
   timeout?: number;
   maxBuffer?: number;
+  shell?: boolean | string;
 }
 
 interface ExecCommandResult {
@@ -87,7 +89,7 @@ export async function execCommand(
   const { baseEnv, env, envOverlay } = options ?? {};
   const resolvedBaseEnv = env ?? baseEnv ?? process.env;
   const isWindows = process.platform === "win32";
-  const shell = shouldUseWindowsShell(command);
+  const shell = shouldUseWindowsShell(command, options?.shell);
   const shouldQuoteForShell = isWindows && shell !== false;
   const resolvedCommand = shouldQuoteForShell ? quoteWindowsCommand(command) : command;
   const resolvedArgs = shouldQuoteForShell ? args.map(quoteWindowsArgument) : args;
@@ -104,6 +106,7 @@ export async function execCommand(
     cwd: options?.cwd,
     env: childEnv,
     encoding: options?.encoding ?? "utf8",
+    killSignal: options?.killSignal,
     timeout: options?.timeout,
     maxBuffer: options?.maxBuffer,
     shell,