From 6b0210a5fd0cdc71a0154af5708bf41fbda2f81f Mon Sep 17 00:00:00 2001 From: Vincent Koc Date: Sun, 21 Jun 2026 09:03:40 +0200 Subject: [PATCH] fix(scripts): resolve taskkill from system32 --- scripts/lib/managed-child-process.mjs | 43 +---------- scripts/lib/windows-taskkill.mjs | 44 +++++++++++ scripts/run-with-env.mjs | 6 +- scripts/test-projects.test-support.mjs | 4 + test/scripts/run-with-env.test.ts | 101 ++++++++++++++++--------- 5 files changed, 117 insertions(+), 81 deletions(-) create mode 100644 scripts/lib/windows-taskkill.mjs diff --git a/scripts/lib/managed-child-process.mjs b/scripts/lib/managed-child-process.mjs index 6f20fb91835d..17e4732eb55d 100644 --- a/scripts/lib/managed-child-process.mjs +++ b/scripts/lib/managed-child-process.mjs @@ -1,12 +1,11 @@ // Runs child commands with process-group signal forwarding and Windows shell normalization. import { spawn, spawnSync } from "node:child_process"; import { constants as osConstants } from "node:os"; -import path from "node:path"; import { buildCmdExeCommandLine } from "../windows-cmd-helpers.mjs"; +import { resolveWindowsTaskkillPath } from "./windows-taskkill.mjs"; const FORWARDED_SIGNALS = ["SIGINT", "SIGTERM", "SIGHUP"]; const FORCE_KILL_DELAY_MS = 5_000; -const DEFAULT_WINDOWS_SYSTEM_ROOT = "C:\\Windows"; const managedChildren = new Set(); const signalHandlers = new Map(); @@ -72,46 +71,6 @@ export function terminateManagedChild( child.kill(signal); } -function getEnvValueCaseInsensitive(env, expectedKey) { - const direct = env[expectedKey]; - if (direct !== undefined) { - return direct; - } - const expected = expectedKey.toUpperCase(); - const actualKey = Object.keys(env).find((key) => key.toUpperCase() === expected); - return actualKey ? env[actualKey] : undefined; -} - -function normalizeWindowsSystemRoot(raw) { - const trimmed = raw?.trim(); - if ( - !trimmed || - trimmed.includes("\0") || - trimmed.includes("\r") || - trimmed.includes("\n") || - trimmed.includes(";") - ) { - return null; - } - const normalized = path.win32.normalize(trimmed); - if (!path.win32.isAbsolute(normalized) || normalized.startsWith("\\\\")) { - return null; - } - const parsed = path.win32.parse(normalized); - if (!/^[A-Za-z]:\\$/.test(parsed.root) || normalized.length <= parsed.root.length) { - return null; - } - return normalized.replace(/[\\/]+$/, ""); -} - -function resolveWindowsTaskkillPath(env = process.env) { - const systemRoot = - normalizeWindowsSystemRoot(getEnvValueCaseInsensitive(env, "SystemRoot")) ?? - normalizeWindowsSystemRoot(getEnvValueCaseInsensitive(env, "WINDIR")) ?? - DEFAULT_WINDOWS_SYSTEM_ROOT; - return path.win32.join(systemRoot, "System32", "taskkill.exe"); -} - /** * Run a child command while forwarding termination signals to the managed process group. * diff --git a/scripts/lib/windows-taskkill.mjs b/scripts/lib/windows-taskkill.mjs new file mode 100644 index 000000000000..b6c846c01fb2 --- /dev/null +++ b/scripts/lib/windows-taskkill.mjs @@ -0,0 +1,44 @@ +// Resolves the Windows taskkill binary without trusting PATH. +import path from "node:path"; + +const DEFAULT_WINDOWS_SYSTEM_ROOT = "C:\\Windows"; + +function getEnvValueCaseInsensitive(env, expectedKey) { + const direct = env[expectedKey]; + if (direct !== undefined) { + return direct; + } + const expected = expectedKey.toUpperCase(); + const actualKey = Object.keys(env).find((key) => key.toUpperCase() === expected); + return actualKey ? env[actualKey] : undefined; +} + +function normalizeWindowsSystemRoot(raw) { + const trimmed = raw?.trim(); + if ( + !trimmed || + trimmed.includes("\0") || + trimmed.includes("\r") || + trimmed.includes("\n") || + trimmed.includes(";") + ) { + return null; + } + const normalized = path.win32.normalize(trimmed); + if (!path.win32.isAbsolute(normalized) || normalized.startsWith("\\\\")) { + return null; + } + const parsed = path.win32.parse(normalized); + if (!/^[A-Za-z]:\\$/.test(parsed.root) || normalized.length <= parsed.root.length) { + return null; + } + return normalized.replace(/[\\/]+$/, ""); +} + +export function resolveWindowsTaskkillPath(env = process.env) { + const systemRoot = + normalizeWindowsSystemRoot(getEnvValueCaseInsensitive(env, "SystemRoot")) ?? + normalizeWindowsSystemRoot(getEnvValueCaseInsensitive(env, "WINDIR")) ?? + DEFAULT_WINDOWS_SYSTEM_ROOT; + return path.win32.join(systemRoot, "System32", "taskkill.exe"); +} diff --git a/scripts/run-with-env.mjs b/scripts/run-with-env.mjs index ea76b6a43c44..b71bfb506d71 100644 --- a/scripts/run-with-env.mjs +++ b/scripts/run-with-env.mjs @@ -1,5 +1,6 @@ // Runs a command with inline KEY=value assignments while preserving signal behavior. import { spawn, spawnSync } from "node:child_process"; +import { resolveWindowsTaskkillPath } from "./lib/windows-taskkill.mjs"; const ENV_ASSIGNMENT_RE = /^[A-Za-z_][A-Za-z0-9_]*=/u; const USAGE = "Usage: node scripts/run-with-env.mjs KEY=value [KEY=value ...] -- command [args...]"; @@ -104,16 +105,17 @@ export function signalRunWithEnvChild( } } if (platform === "win32" && typeof child.pid === "number") { + const taskkillPath = resolveWindowsTaskkillPath(); const args = ["/PID", String(child.pid), "/T"]; if (signal === "SIGKILL") { args.push("/F"); } - const result = runTaskkill("taskkill", args, { stdio: "ignore" }); + const result = runTaskkill(taskkillPath, args, { stdio: "ignore" }); if (!result?.error && result?.status === 0) { return; } if (signal !== "SIGKILL") { - const forceResult = runTaskkill("taskkill", [...args, "/F"], { stdio: "ignore" }); + const forceResult = runTaskkill(taskkillPath, [...args, "/F"], { stdio: "ignore" }); if (!forceResult?.error && forceResult?.status === 0) { return; } diff --git a/scripts/test-projects.test-support.mjs b/scripts/test-projects.test-support.mjs index 65284c108e93..b08c446516f2 100644 --- a/scripts/test-projects.test-support.mjs +++ b/scripts/test-projects.test-support.mjs @@ -1089,6 +1089,10 @@ const TOOLING_SOURCE_TEST_TARGETS = new Map([ ["scripts/lib/local-heavy-check-runtime.mjs", ["test/scripts/local-heavy-check-runtime.test.ts"]], ["scripts/lib/kova-report-gate.mjs", ["test/scripts/kova-report-gate.test.ts"]], ["scripts/lib/managed-child-process.mjs", ["test/scripts/managed-child-process.test.ts"]], + [ + "scripts/lib/windows-taskkill.mjs", + ["test/scripts/managed-child-process.test.ts", "test/scripts/run-with-env.test.ts"], + ], [ "scripts/lib/local-build-metadata.mjs", [ diff --git a/test/scripts/run-with-env.test.ts b/test/scripts/run-with-env.test.ts index 86aadfe1c285..11fe181ac1bf 100644 --- a/test/scripts/run-with-env.test.ts +++ b/test/scripts/run-with-env.test.ts @@ -12,6 +12,29 @@ import { signalRunWithEnvChild, } from "../../scripts/run-with-env.mjs"; +const taskkillPath = path.win32.join("C:\\Windows", "System32", "taskkill.exe"); + +function restoreEnvValue(key: string, value: string | undefined): void { + if (value === undefined) { + delete process.env[key]; + return; + } + process.env[key] = value; +} + +function withDefaultWindowsSystemRoot(run: () => void): void { + const originalSystemRoot = process.env.SystemRoot; + const originalWindir = process.env.WINDIR; + try { + process.env.SystemRoot = "C:\\Windows"; + delete process.env.WINDIR; + run(); + } finally { + restoreEnvValue("SystemRoot", originalSystemRoot); + restoreEnvValue("WINDIR", originalWindir); + } +} + async function waitFor(predicate: () => boolean, label: string, timeoutMs = 3_000): Promise { const startedAt = Date.now(); while (!predicate()) { @@ -160,52 +183,56 @@ describe("run-with-env", () => { }); it("signals Windows wrapped command trees with taskkill", () => { - const child = { - kill: vi.fn(), - pid: 12345, - }; - const runTaskkill = vi.fn(() => ({ error: undefined, status: 0 })); + withDefaultWindowsSystemRoot(() => { + const child = { + kill: vi.fn(), + pid: 12345, + }; + const runTaskkill = vi.fn(() => ({ error: undefined, status: 0 })); - signalRunWithEnvChild(child, "SIGTERM", { - platform: "win32", - runTaskkill, - }); - expect(runTaskkill).toHaveBeenNthCalledWith(1, "taskkill", ["/PID", "12345", "/T"], { - stdio: "ignore", - }); + signalRunWithEnvChild(child, "SIGTERM", { + platform: "win32", + runTaskkill, + }); + expect(runTaskkill).toHaveBeenNthCalledWith(1, taskkillPath, ["/PID", "12345", "/T"], { + stdio: "ignore", + }); - signalRunWithEnvChild(child, "SIGKILL", { - platform: "win32", - runTaskkill, + signalRunWithEnvChild(child, "SIGKILL", { + platform: "win32", + runTaskkill, + }); + expect(runTaskkill).toHaveBeenNthCalledWith(2, taskkillPath, ["/PID", "12345", "/T", "/F"], { + stdio: "ignore", + }); + expect(child.kill).not.toHaveBeenCalled(); }); - expect(runTaskkill).toHaveBeenNthCalledWith(2, "taskkill", ["/PID", "12345", "/T", "/F"], { - stdio: "ignore", - }); - expect(child.kill).not.toHaveBeenCalled(); }); it("force-kills Windows wrapped command trees when graceful taskkill fails", () => { - const child = { - kill: vi.fn(), - pid: 12345, - }; - const runTaskkill = vi - .fn() - .mockReturnValueOnce({ error: undefined, status: 1 }) - .mockReturnValueOnce({ error: undefined, status: 0 }); + withDefaultWindowsSystemRoot(() => { + const child = { + kill: vi.fn(), + pid: 12345, + }; + const runTaskkill = vi + .fn() + .mockReturnValueOnce({ error: undefined, status: 1 }) + .mockReturnValueOnce({ error: undefined, status: 0 }); - signalRunWithEnvChild(child, "SIGTERM", { - platform: "win32", - runTaskkill, - }); + signalRunWithEnvChild(child, "SIGTERM", { + platform: "win32", + runTaskkill, + }); - expect(runTaskkill).toHaveBeenNthCalledWith(1, "taskkill", ["/PID", "12345", "/T"], { - stdio: "ignore", + expect(runTaskkill).toHaveBeenNthCalledWith(1, taskkillPath, ["/PID", "12345", "/T"], { + stdio: "ignore", + }); + expect(runTaskkill).toHaveBeenNthCalledWith(2, taskkillPath, ["/PID", "12345", "/T", "/F"], { + stdio: "ignore", + }); + expect(child.kill).not.toHaveBeenCalled(); }); - expect(runTaskkill).toHaveBeenNthCalledWith(2, "taskkill", ["/PID", "12345", "/T", "/F"], { - stdio: "ignore", - }); - expect(child.kill).not.toHaveBeenCalled(); }); it.runIf(process.platform !== "win32").each(["SIGTERM", "SIGHUP", "SIGINT"] as const)(