From 085be23e0859476dbda50cbb9c8dd01fd62bf2d7 Mon Sep 17 00:00:00 2001 From: Wibias <37517432+Wibias@users.noreply.github.com> Date: Fri, 31 Jul 2026 00:18:09 +0200 Subject: [PATCH 1/5] fix: address issue #764 Fixes #764 --- src/lib/winsw.ts | 6 ++++ src/service.ts | 80 ++++++++++++++++++++++++++++++++++++++----- tests/service.test.ts | 26 +++++++++++--- tests/winsw.test.ts | 7 ++++ 4 files changed, 106 insertions(+), 13 deletions(-) diff --git a/src/lib/winsw.ts b/src/lib/winsw.ts index 5c00e47aff..42a5f9553f 100644 --- a/src/lib/winsw.ts +++ b/src/lib/winsw.ts @@ -165,6 +165,12 @@ function runWinsw(args: string[]): string { /** `install /p` prompts for the service-account password on the console — stdin must be inherited. */ function runWinswInteractive(args: string[]): void { + if (!process.stdin.isTTY) { + throw new Error( + "WinSW install requires an interactive console to prompt for the service account password. " + + "Run `ocx service install --native` from an elevated Command Prompt or PowerShell window, not a hidden or piped session.", + ); + } execFileSync(winswExePath(), args, { stdio: "inherit" }); } diff --git a/src/service.ts b/src/service.ts index b38305fa11..fc64d4c4c4 100644 --- a/src/service.ts +++ b/src/service.ts @@ -6,6 +6,7 @@ * restore it via the command. */ import { execFileSync, execSync } from "node:child_process"; +import { findLiveProxy } from "./server/proxy-liveness"; import { chmodSync, existsSync, mkdirSync, readFileSync, unlinkSync, writeFileSync } from "node:fs"; import { homedir } from "node:os"; import { dirname, join, resolve } from "node:path"; @@ -1363,7 +1364,38 @@ export async function repairService(deps: RepairServiceDeps = {}): Promise * scheduler backend first; on failure the machine is left with NO service (explicitly * reported) — never a silent fallback to the scheduler. */ +/** Refuse WinSW when the interactive user is a Microsoft account (SCM cannot authenticate it). */ +export function assertWindowsNativeServiceAccountSupported(): void { + if (process.platform !== "win32") return; + const source = readWindowsPrincipalSource(); + if (source?.toLowerCase() === "microsoftaccount") { + throw new Error( + "The native (WinSW) service backend cannot run under a Microsoft-account Windows login. " + + "Keep the Task Scheduler backend (`ocx service install`) or sign in with a local/domain account before `ocx service install --native`.", + ); + } +} + +function readWindowsPrincipalSource(): string | null { + if (process.platform !== "win32") return null; + const ps = join(process.env.SystemRoot ?? "C:\\Windows", "System32", "WindowsPowerShell", "v1.0", "powershell.exe"); + if (!existsSync(ps)) return null; + try { + const out = execFileSync(ps, [ + "-NoLogo", + "-NoProfile", + "-NonInteractive", + "-Command", + "(Get-LocalUser -Name $env:USERNAME -ErrorAction SilentlyContinue).PrincipalSource", + ], { encoding: "utf8", stdio: ["ignore", "pipe", "pipe"], windowsHide: true }).trim(); + return out || null; + } catch { + return null; + } +} + async function installWindowsNative(): Promise { + assertWindowsNativeServiceAccountSupported(); recordOwnedConfigPath(getConfigDir(), serviceStatePath()); if (!existsSync(getConfigDir())) mkdirSync(getConfigDir(), { recursive: true }); writeServiceApiTokenFile(); @@ -1402,7 +1434,23 @@ function stopWindows(): void { try { schtasks(["/end", "/tn", TASK]); } catch { function statusWindows(): string { try { return schtasks(["/query", "/tn", TASK]); } catch { return ""; } } function statusWindowsXml(): string { try { return schtasks(["/query", "/tn", TASK, "/xml"]); } catch { return ""; } } function uninstallWindows(): void { - try { schtasks(["/delete", "/tn", TASK, "/f"]); } catch { /* absent */ } + const probe = probeWindowsSchedulerTask(TASK); + if (probe.status === "present") { + try { + schtasks(["/delete", "/tn", TASK, "/f"]); + } catch (error) { + throw new Error(`Failed to delete Task Scheduler task ${TASK}: ${error instanceof Error ? error.message : String(error)}`); + } + const afterDelete = probeWindowsSchedulerTask(TASK); + if (afterDelete.status === "present") { + throw new Error(`Task Scheduler task ${TASK} is still present after delete — refusing to remove service assets. Retry from an elevated shell.`); + } + if (afterDelete.status === "unknown") { + throw new Error(`Task Scheduler task ${TASK} presence could not be verified after delete — refusing to remove service assets.`); + } + } else if (probe.status === "unknown") { + throw new Error(`Task Scheduler task ${TASK} presence could not be verified — refusing to remove service assets.`); + } if (existsSync(windowsServiceScriptPath())) unlinkSync(windowsServiceScriptPath()); if (existsSync(windowsLauncherVbsPath())) unlinkSync(windowsLauncherVbsPath()); if (existsSync(windowsTaskXmlPath())) unlinkSync(windowsTaskXmlPath()); @@ -1562,17 +1610,29 @@ function platformOps(backend: ServiceBackend = "scheduler"): ServiceOps | null { type TrackedProxyCleanupResult = "none" | "stale" | "stopped"; async function stopTrackedProxyIfRunning(): Promise { + let stopped = false; const pid = readPid(); - if (!pid) return "none"; - if (!isProcessAlive(pid)) { + if (pid && isProcessAlive(pid)) { + await stopProxy(pid); removePid(pid); removeRuntimePort(pid); - return "stale"; + stopped = true; + } else if (pid) { + removePid(pid); + removeRuntimePort(pid); + } + // Orphan recovery: the pid file can be missing/stale while the service wrapper keeps + // a live proxy running — mirror `ocx stop`'s identity-checked findLiveProxy fallback. + const live = await findLiveProxy({ timeoutMs: 1500 }); + if (live?.pid) { + await stopProxy(live.pid); + removePid(live.pid); + removeRuntimePort(live.pid); + stopped = true; } - await stopProxy(pid); - removePid(pid); - removeRuntimePort(pid); - return "stopped"; + if (stopped) return "stopped"; + if (pid) return "stale"; + return "none"; } async function stopTrackedProxyForServiceCommand(): Promise { @@ -1886,6 +1946,10 @@ export async function serviceCommand(...args: (string | undefined)[]): Promise { expect(service).toContain('code !== "EBUSY" && code !== "EPERM" && code !== "EACCES"'); }); - test("Windows service uninstall removes generated task XML", async () => { + test("Windows service uninstall verifies task deletion before removing assets", async () => { const service = await readText("src/service.ts"); const uninstallWindows = service.slice(service.indexOf("function uninstallWindows()"), service.indexOf("function serviceDiagnosticsSummary()")); + expect(uninstallWindows).toContain("probeWindowsSchedulerTask(TASK)"); expect(uninstallWindows).toContain("windowsServiceScriptPath()"); expect(uninstallWindows).toContain("windowsTaskXmlPath()"); expect(uninstallWindows).toContain("unlinkSync(windowsTaskXmlPath())"); + expect(uninstallWindows).toContain("refusing to remove service assets"); }); - test("service cleanup stops gracefully first via the shared stopper and clears the pid file", async () => { + test("service cleanup falls back to findLiveProxy and clears the pid file", async () => { const service = await readText("src/service.ts"); expect(service).toContain('import { expandUserPath, getConfigDir, readPid, removePid, removeRuntimePort } from "./config";'); expect(service).toContain("removeRuntimePort(pid);"); expect(service).toContain('import { isProcessAlive, stopProxy } from "./lib/process-control";'); + expect(service).toContain('import { findLiveProxy } from "./server/proxy-liveness";'); expect(service).toContain('type TrackedProxyCleanupResult = "none" | "stale" | "stopped";'); expect(service).toContain("async function stopTrackedProxyIfRunning(): Promise"); - expect(service).toContain('if (!pid) return "none";'); - expect(service).toContain("if (!isProcessAlive(pid))"); - expect(service).toContain('return "stale";'); + expect(service).toContain("await findLiveProxy({ timeoutMs: 1500 })"); expect(service).toContain("await stopProxy(pid);"); expect(service).toContain("removePid(pid);"); expect(service).toContain('return "stopped";'); }); + test("service stop refuses success while the proxy is still live", async () => { + const service = await readText("src/service.ts"); + const stopCase = service.slice(service.indexOf('case "stop":'), service.indexOf('case "status":')); + expect(stopCase).toContain("await findLiveProxy({ timeoutMs: 1500 })"); + expect(stopCase).toContain("Service stop did not terminate the proxy"); + expect(stopCase).toContain("process.exit(1)"); + }); + + test("native install refuses Microsoft-account logins before removing the scheduler backend", async () => { + const service = await readText("src/service.ts"); + const installNative = service.slice(service.indexOf("async function installWindowsNative()"), service.indexOf("function startWindows()")); + expect(installNative.indexOf("assertWindowsNativeServiceAccountSupported()")).toBeLessThan(installNative.indexOf("uninstallWindows()")); + expect(service).toContain("Microsoft-account Windows login"); + }); + test("service command cleanup logs kill failures without skipping restore/delete", async () => { const service = await readText("src/service.ts"); diff --git a/tests/winsw.test.ts b/tests/winsw.test.ts index 2f728a8b8d..8c922d0957 100644 --- a/tests/winsw.test.ts +++ b/tests/winsw.test.ts @@ -165,6 +165,13 @@ describe("winsw install flow", () => { expect(calls).toEqual([["interactive", "install", "/p"], ["verify"], ["run", "start"]]); }); + test("install /p refuses non-interactive stdin instead of hanging", () => { + const winsw = readFileSync(new URL("../src/lib/winsw.ts", import.meta.url), "utf8"); + const fn = winsw.slice(winsw.indexOf("function runWinswInteractive"), winsw.indexOf("function scQc()")); + expect(fn).toContain("process.stdin.isTTY"); + expect(fn).toContain("interactive console"); + }); + test("repair over an existing service rewrites assets and restarts without re-prompting", async () => { const calls: string[][] = []; await installWinswService(entry, { From bb95d8c8877f12b262515d755e93e863641be552 Mon Sep 17 00:00:00 2001 From: Wibias <37517432+Wibias@users.noreply.github.com> Date: Fri, 31 Jul 2026 02:13:43 +0200 Subject: [PATCH 2/5] fix(service): harden Windows stop and verified PID kills Wait through the scheduler wrapper restart window when schtasks /end fails, and verify health-reported PIDs before stopProxy during service cleanup. --- src/server/proxy-liveness.ts | 11 ++++++-- src/service.ts | 55 ++++++++++++++++++++++++++++-------- tests/proxy-liveness.test.ts | 7 +++-- tests/service.test.ts | 20 +++++++++++-- 4 files changed, 75 insertions(+), 18 deletions(-) diff --git a/src/server/proxy-liveness.ts b/src/server/proxy-liveness.ts index 0ed7e6bbfd..913afb0577 100644 --- a/src/server/proxy-liveness.ts +++ b/src/server/proxy-liveness.ts @@ -109,6 +109,13 @@ export async function findLiveProxy(io: LivenessIo = {}): Promise { + if (reported === null) return null; + if (!Number.isSafeInteger(reported) || reported <= 0) return null; + const verified = verifyPidFn(reported); + return verified === reported ? verified : null; + }; + const pid = readPidFn(); let probedPort: number | null = null; if (pid) { @@ -136,7 +143,7 @@ export async function findLiveProxy(io: LivenessIo = {}): Promise { writeServiceInstallState("native"); } function startWindows(): void { schtasks(["/run", "/tn", TASK]); } -function stopWindows(): void { try { schtasks(["/end", "/tn", TASK]); } catch { /* not running */ } } +/** Batch wrapper cooldown after a failed child exit (`ping -n 6` ≈ 5s). */ +export const WINDOWS_SCHEDULER_WRAPPER_RESTART_MS = 6500; + +export function isWindowsSchedulerEndBenign(error: unknown): boolean { + const detail = schtasksErrorDetail(error).toLowerCase(); + return detail.includes("no running instance") + || detail.includes("not currently running") + || detail.includes("0x41330"); +} + +/** End the scheduler task; false when `/end` failed for a reason other than "not running". */ +export function stopWindows(): boolean { + try { + schtasks(["/end", "/tn", TASK]); + return true; + } catch (error) { + return isWindowsSchedulerEndBenign(error); + } +} function statusWindows(): string { try { return schtasks(["/query", "/tn", TASK]); } catch { return ""; } } function statusWindowsXml(): string { try { return schtasks(["/query", "/tn", TASK, "/xml"]); } catch { return ""; } } function uninstallWindows(): void { @@ -1609,13 +1627,20 @@ function platformOps(backend: ServiceBackend = "scheduler"): ServiceOps | null { type TrackedProxyCleanupResult = "none" | "stale" | "stopped"; +function verifiedKillTarget(pid: number | null | undefined): number | null { + if (typeof pid !== "number" || !Number.isSafeInteger(pid) || pid <= 0) return null; + const verified = verifyPidIdentity(pid); + return verified === pid ? verified : null; +} + async function stopTrackedProxyIfRunning(): Promise { let stopped = false; const pid = readPid(); - if (pid && isProcessAlive(pid)) { - await stopProxy(pid); - removePid(pid); - removeRuntimePort(pid); + const trackedKillPid = verifiedKillTarget(pid); + if (trackedKillPid !== null && isProcessAlive(trackedKillPid)) { + await stopProxy(trackedKillPid); + removePid(trackedKillPid); + removeRuntimePort(trackedKillPid); stopped = true; } else if (pid) { removePid(pid); @@ -1624,10 +1649,11 @@ async function stopTrackedProxyIfRunning(): Promise { // Orphan recovery: the pid file can be missing/stale while the service wrapper keeps // a live proxy running — mirror `ocx stop`'s identity-checked findLiveProxy fallback. const live = await findLiveProxy({ timeoutMs: 1500 }); - if (live?.pid) { - await stopProxy(live.pid); - removePid(live.pid); - removeRuntimePort(live.pid); + const liveKillPid = verifiedKillTarget(live?.pid); + if (liveKillPid !== null) { + await stopProxy(liveKillPid); + removePid(liveKillPid); + removeRuntimePort(liveKillPid); stopped = true; } if (stopped) return "stopped"; @@ -1944,8 +1970,15 @@ export async function serviceCommand(...args: (string | undefined)[]): Promise { readPidFn: () => null, readRuntimeFn: () => null, configFn: () => ({ port: 10100 }), + verifyPidFn: candidate => candidate, fetchFn: (async () => healthz(OURS)) as typeof fetch, }); @@ -98,6 +99,7 @@ describe("findLiveProxy", () => { readPidFn: () => null, readRuntimeFn: () => ({ pid: 4242, port: 58195, hostname: "::1" }), configFn: () => ({ port: 10100 }), + verifyPidFn: candidate => candidate, fetchFn: (async (url: string | URL | Request) => { urls.push(String(url)); return healthz(OURS); @@ -143,9 +145,8 @@ describe("findLiveProxy", () => { fetchFn: (async () => healthz({ ...OURS, pid: 9999 })) as typeof fetch, }); - // The runtime probe fails the pid check; the config fallback probes the same port - // without a pid expectation and adopts the reported live pid instead. - expect(live).toEqual({ pid: 9999, port: 58195, source: "config" }); + // healthz-reported pids must pass identity verification before they become kill targets. + expect(live).toEqual({ pid: null, port: 58195, source: "config" }); }); test("a pidless legacy healthz never promotes an unverified cheap pid to a kill target", async () => { diff --git a/tests/service.test.ts b/tests/service.test.ts index 63725e652a..5c4a4c7afb 100644 --- a/tests/service.test.ts +++ b/tests/service.test.ts @@ -658,18 +658,34 @@ describe("service lifecycle cleanup ordering", () => { test("service cleanup falls back to findLiveProxy and clears the pid file", async () => { const service = await readText("src/service.ts"); - expect(service).toContain('import { expandUserPath, getConfigDir, readPid, removePid, removeRuntimePort } from "./config";'); + expect(service).toContain('verifyPidIdentity'); expect(service).toContain("removeRuntimePort(pid);"); expect(service).toContain('import { isProcessAlive, stopProxy } from "./lib/process-control";'); expect(service).toContain('import { findLiveProxy } from "./server/proxy-liveness";'); expect(service).toContain('type TrackedProxyCleanupResult = "none" | "stale" | "stopped";'); expect(service).toContain("async function stopTrackedProxyIfRunning(): Promise"); expect(service).toContain("await findLiveProxy({ timeoutMs: 1500 })"); - expect(service).toContain("await stopProxy(pid);"); + expect(service).toContain("await stopProxy(trackedKillPid);"); + expect(service).toContain("await stopProxy(liveKillPid);"); expect(service).toContain("removePid(pid);"); expect(service).toContain('return "stopped";'); }); + + test("Windows scheduler stop waits through wrapper restart when schtasks /end fails", async () => { + const service = await readText("src/service.ts"); + const stopCase = service.slice(service.indexOf('case "stop":'), service.indexOf('case "status":')); + expect(stopCase).toContain("schedulerEndOk = stopWindows()"); + expect(stopCase).toContain("WINDOWS_SCHEDULER_WRAPPER_RESTART_MS"); + expect(stopCase).toContain("await Bun.sleep(WINDOWS_SCHEDULER_WRAPPER_RESTART_MS)"); + }); + + test("tracked proxy cleanup verifies health-reported pids before stopProxy", async () => { + const service = await readText("src/service.ts"); + expect(service).toContain("function verifiedKillTarget(pid: number | null | undefined): number | null"); + expect(service).toContain("const liveKillPid = verifiedKillTarget(live?.pid);"); + expect(service).toContain("const trackedKillPid = verifiedKillTarget(pid);"); + }); test("service stop refuses success while the proxy is still live", async () => { const service = await readText("src/service.ts"); const stopCase = service.slice(service.indexOf('case "stop":'), service.indexOf('case "status":')); From 5b1af43d1c06a9d5528dd6777ed8e8a644331974 Mon Sep 17 00:00:00 2001 From: Wibias <37517432+Wibias@users.noreply.github.com> Date: Fri, 31 Jul 2026 02:35:05 +0200 Subject: [PATCH 3/5] fix(service): remove duplicate findLiveProxy import after dev merge Restores cross-platform typecheck when both branches added the same proxy-liveness import. --- src/service.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/service.ts b/src/service.ts index ebf7a75623..1e9ab54a95 100644 --- a/src/service.ts +++ b/src/service.ts @@ -18,7 +18,6 @@ import { isWslRuntime } from "./codex/home"; import { durableBunPath, durableBunRuntime } from "./lib/bun-runtime"; import { isProcessAlive, stopProxy } from "./lib/process-control"; import { serviceApiTokenFilePath } from "./lib/service-secrets"; -import { findLiveProxy } from "./server/proxy-liveness"; import { randomUUID } from "node:crypto"; import { ELEVATION_REQUEST_TIMEOUT_MS, @@ -2104,3 +2103,4 @@ export async function serviceCommand(...args: (string | undefined)[]): Promise Date: Fri, 31 Jul 2026 02:43:38 +0200 Subject: [PATCH 4/5] ci: retrigger cross-platform checks From b0dee82ff9f0aef9f5ed678e54eb7e1c6b28b3ee Mon Sep 17 00:00:00 2001 From: Wibias <37517432+Wibias@users.noreply.github.com> Date: Fri, 31 Jul 2026 06:39:02 +0200 Subject: [PATCH 5/5] fix(service): drop wait keyed on schtasks /end failure Owner review: #764 is an /end that succeeds while the wrapper respawns, so waiting only when /end errors cannot catch it. Keep orphan/PID cleanup and the immediate live-proxy refuse-success guard; restart-window verification stays on the separate stop-verification path. --- src/service.ts | 29 ++++++++++++++++------------- tests/proxy-liveness.test.ts | 1 + tests/service.test.ts | 10 ++++++---- 3 files changed, 23 insertions(+), 17 deletions(-) diff --git a/src/service.ts b/src/service.ts index 1e9ab54a95..80cf868e84 100644 --- a/src/service.ts +++ b/src/service.ts @@ -1494,8 +1494,6 @@ async function installWindowsNative(): Promise { writeServiceInstallState("native"); } function startWindows(): void { schtasks(["/run", "/tn", TASK]); } -/** Batch wrapper cooldown after a failed child exit (`ping -n 6` ≈ 5s). */ -export const WINDOWS_SCHEDULER_WRAPPER_RESTART_MS = 6500; export function isWindowsSchedulerEndBenign(error: unknown): boolean { const detail = schtasksErrorDetail(error).toLowerCase(); @@ -1504,13 +1502,19 @@ export function isWindowsSchedulerEndBenign(error: unknown): boolean { || detail.includes("0x41330"); } -/** End the scheduler task; false when `/end` failed for a reason other than "not running". */ -export function stopWindows(): boolean { +/** + * End the scheduler task. "Already stopped" is success; other `/end` failures are + * swallowed so callers can still run tracked-proxy + live-proxy cleanup. + * + * Do not key a restart-window wait on `/end` failure: the #764 case is an `/end` + * that *succeeds* while the wrapper survives and respawns. That verification lives + * on the stop-verification path (poll across the restart window), not here. + */ +export function stopWindows(): void { try { schtasks(["/end", "/tn", TASK]); - return true; } catch (error) { - return isWindowsSchedulerEndBenign(error); + if (isWindowsSchedulerEndBenign(error)) return; } } function statusWindows(): string { try { return schtasks(["/query", "/tn", TASK]); } catch { return ""; } } @@ -2030,19 +2034,17 @@ export async function serviceCommand(...args: (string | undefined)[]): Promise { test("a runtime record whose healthz reports a different pid is rejected", async () => { const live = await findLiveProxy({ readPidFn: () => 1111, + verifyPidFn: () => null, readRuntimeFn: () => ({ port: 58195 }), configFn: () => ({ port: 58195 }), fetchFn: (async () => healthz({ ...OURS, pid: 9999 })) as typeof fetch, diff --git a/tests/service.test.ts b/tests/service.test.ts index 5c4a4c7afb..c07f597531 100644 --- a/tests/service.test.ts +++ b/tests/service.test.ts @@ -672,12 +672,14 @@ describe("service lifecycle cleanup ordering", () => { }); - test("Windows scheduler stop waits through wrapper restart when schtasks /end fails", async () => { + test("Windows scheduler stop does not wait on schtasks /end failure", async () => { const service = await readText("src/service.ts"); const stopCase = service.slice(service.indexOf('case "stop":'), service.indexOf('case "status":')); - expect(stopCase).toContain("schedulerEndOk = stopWindows()"); - expect(stopCase).toContain("WINDOWS_SCHEDULER_WRAPPER_RESTART_MS"); - expect(stopCase).toContain("await Bun.sleep(WINDOWS_SCHEDULER_WRAPPER_RESTART_MS)"); + // #764 is an /end that succeeds while the wrapper respawns; waiting only when + // /end errors cannot catch that path. Restart-window polling lives elsewhere. + expect(stopCase).not.toContain("WINDOWS_SCHEDULER_WRAPPER_RESTART_MS"); + expect(stopCase).not.toContain("schedulerEndOk"); + expect(stopCase).not.toContain("await Bun.sleep("); }); test("tracked proxy cleanup verifies health-reported pids before stopProxy", async () => {