diff --git a/docs-site/src/content/docs/reference/cli/lifecycle.md b/docs-site/src/content/docs/reference/cli/lifecycle.md index 26c904188..f45414da3 100644 --- a/docs-site/src/content/docs/reference/cli/lifecycle.md +++ b/docs-site/src/content/docs/reference/cli/lifecycle.md @@ -288,11 +288,15 @@ error. Foreign tasks and operations can never emit the automatic-elevation marke dashboard UAC prompt or rerun `ocx service install` in an elevated PowerShell window. For a fresh install where the OpenCodex scheduler task is confirmed absent, UAC approval now -happens before the installer stops any existing proxy. The task is registered without being run; -only after registration succeeds does OpenCodex stop the old listener, publish the service assets, -and start the scheduled task. Cancelling or denying UAC therefore leaves the working proxy and its -Codex routing in place. Existing or conflicting scheduler registrations continue to fail closed -rather than being deleted as an unsafe best-effort rollback. +happens before the installer stops any existing proxy. Its unique registration XML is staged in +an ACL-hardened private directory outside the OpenCodex config root, and the task is registered +without being run. Only after +registration succeeds does OpenCodex remove that XML, require ownership metadata for a genuinely +new config root, stop the old listener, remove and boundedly re-verify any native WinSW +registration, publish the service assets, and start the scheduled task. Cancelling or denying UAC, +or failing to claim a new root safely, therefore leaves the working proxy and its Codex routing in +place. Existing or conflicting scheduler registrations continue to fail closed rather than being +deleted as an unsafe best-effort rollback. ### `ocx codex-shim ` diff --git a/src/lib/windows-elevation.ts b/src/lib/windows-elevation.ts index 1003d1ce4..36dae55bd 100644 --- a/src/lib/windows-elevation.ts +++ b/src/lib/windows-elevation.ts @@ -612,6 +612,43 @@ export function runWindowsElevated(file: string, args: string[]): Promise result.exitCode); } +/** + * Register one scheduled-task definition without exposing a mutable XML pathname to + * the elevated process. The XML bytes are fixed in the encoded PowerShell command + * before UAC; Register-ScheduledTask receives that string directly after elevation. + */ +export function runWindowsElevatedScheduledTaskRegistration( + taskName: string, + xml: string, +): Promise { + const xmlBase64 = Buffer.from(xml, "utf16le").toString("base64"); + const inner = [ + `$taskName = ${psSingleQuote(taskName)}`, + `$xmlBase64 = ${psSingleQuote(xmlBase64)}`, + "$xml = [Text.Encoding]::Unicode.GetString([Convert]::FromBase64String($xmlBase64))", + "Register-ScheduledTask -TaskName $taskName -Xml $xml -Force -ErrorAction Stop | Out-Null", + ].join("; "); + const encodedCommand = Buffer.from(inner, "utf16le").toString("base64"); + const script = [ + `$p = Start-Process -FilePath ${psSingleQuote(windowsPowerShell())}`, + ` -ArgumentList ${psSingleQuote(buildWindowsElevatedArgumentList([ + "-NoProfile", + "-NonInteractive", + "-ExecutionPolicy", + "Bypass", + "-EncodedCommand", + encodedCommand, + ]))}`, + " -Verb RunAs -WindowStyle Hidden -PassThru -Wait", + `if ($null -eq $p) { exit ${OCX_ELEVATED_UAC_CANCELLED} }`, + "$null = $p.Handle", + `if ($null -eq $p.ExitCode) { exit ${OCX_ELEVATED_PROTOCOL_FAILED} }`, + "exit $p.ExitCode", + ].join("; "); + + return startPowerShellCommand(script).completion.then(result => result.exitCode); +} + /** * Build the elevated (post-UAC) script: create → run → optional delete rollback. * Returns only OpenCodex protocol exit codes (never raw schtasks codes, never 1223). diff --git a/src/lib/windows-secret-acl.ts b/src/lib/windows-secret-acl.ts index 0ac11927a..424a0f7b0 100644 --- a/src/lib/windows-secret-acl.ts +++ b/src/lib/windows-secret-acl.ts @@ -388,6 +388,13 @@ export function forgetEphemeralSecretPath(tempPath: string): void { timedOutPaths.delete(`optional:${tempPath}`); } +/** Directory counterpart for a proven-absent ephemeral staging root. */ +export function forgetEphemeralSecretDir(tempPath: string): void { + hardenedDirectories.delete(tempPath); + timedOutPaths.delete(`required:${tempPath}`); + timedOutPaths.delete(`optional:${tempPath}`); +} + /** Test seam: timeout memo sets return to baseline after ephemeral cleanup. */ export function timedOutSecretPathCountForTests(): number { return timedOutPaths.size; diff --git a/src/service.ts b/src/service.ts index 668f16520..b33b244e0 100644 --- a/src/service.ts +++ b/src/service.ts @@ -7,8 +7,8 @@ */ import { execFileSync, execSync, spawnSync } from "node:child_process"; import { findLiveProxy, proxyIdentityAt, SERVICE_STOP_LIVENESS } from "./server/proxy-liveness"; -import { chmodSync, existsSync, mkdirSync, readFileSync, unlinkSync, writeFileSync } from "node:fs"; -import { homedir } from "node:os"; +import { chmodSync, existsSync, mkdirSync, mkdtempSync, readFileSync, rmdirSync, unlinkSync, writeFileSync } from "node:fs"; +import { homedir, tmpdir } from "node:os"; import { dirname, join, resolve, win32 } from "node:path"; import { expandUserPath, getConfigDir, readPid, removePid, removeRuntimePort, verifyPidIdentity } from "./config"; import { loadConfig } from "./config"; @@ -27,6 +27,7 @@ import { resolveTrustedWindowsSchtasksExe, startElevatedSchtasksCreateAndRun, runWindowsElevated, + runWindowsElevatedScheduledTaskRegistration, toWindowsSchtasksError, WindowsElevationError, WindowsSchtasksError, @@ -35,7 +36,12 @@ import { type ElevatedSchtasksCreateAndRunResult, } from "./lib/windows-elevation"; import { defaultWinswEntry, installWinswService, startWinswService, stopWinswService, statusWinswRaw, uninstallWinswService, winswStatusSummary, winswXmlPath, WINSW_SERVICE_ID, WINSW_SHA256, WINSW_VERSION, type WinswStatus } from "./lib/winsw"; -import { hardenSecretDir, hardenSecretPath } from "./lib/windows-secret-acl"; +import { + forgetEphemeralSecretDir, + forgetEphemeralSecretPath, + hardenSecretDir, + hardenSecretPath, +} from "./lib/windows-secret-acl"; import { windowsEnvIndirectBatchPathList, windowsEnvIndirectBatchValue } from "./lib/win-paths"; import { recordOwnedConfigPath } from "./lib/config-ownership"; import { maybeShowStarPrompt } from "./cli/star-prompt"; @@ -1896,22 +1902,115 @@ function writeWindowsSchedulerAssets(): void { writeServiceAssetWithRetry(windowsTaskXmlPath(), `\uFEFF${buildWindowsTaskXml(script)}`, "utf16le"); } -function stageWindowsSchedulerRegistrationXml(attemptNonce: string): string { - if (!existsSync(getConfigDir())) mkdirSync(getConfigDir(), { recursive: true, mode: 0o700 }); - const path = join(getConfigDir(), `.opencodex-service-task.${randomUUID()}.xml`); - // This document points at the canonical launcher but does not publish or rewrite that - // launcher. UAC can therefore be refused while the current proxy still owns its port. - writeServiceAssetWithRetry( - path, - `\uFEFF${buildWindowsTaskXml(windowsServiceScriptPath(), windowsLauncherVbsPath(), attemptNonce)}`, - "utf16le", +const WINDOWS_SCHEDULER_STAGE_PREFIX = "opencodex-service-stage-"; +const ownedWindowsSchedulerStages = new Set(); + +export interface WindowsSchedulerRegistrationStageDeps { + createStageDir?: () => string; + hardenDir?: (path: string) => void; + writeXml?: (path: string, contents: string) => void; + hardenPath?: (path: string) => void; + removeStageDir?: (path: string) => void; +} + +function cleanupWindowsSchedulerStage( + stageDir: string, + xmlPath: string, + removeStageDir: (path: string) => void, +): void { + let cleanupError: unknown; + try { + unlinkSync(xmlPath); + forgetEphemeralSecretPath(xmlPath); + } catch (error) { + if ((error as NodeJS.ErrnoException | undefined)?.code === "ENOENT") { + forgetEphemeralSecretPath(xmlPath); + } else { + cleanupError = error; + } + } + try { + removeStageDir(stageDir); + forgetEphemeralSecretDir(stageDir); + } catch (error) { + if ((error as NodeJS.ErrnoException | undefined)?.code === "ENOENT") { + forgetEphemeralSecretDir(stageDir); + } else if (cleanupError) { + throw new AggregateError([cleanupError, error], "Task Scheduler staging cleanup failed."); + } else { + cleanupError = error; + } + } + if (cleanupError) throw cleanupError; +} + +export function stageWindowsSchedulerRegistrationXml( + attemptNonce: string, + deps: WindowsSchedulerRegistrationStageDeps = {}, +): string { + const createStageDir = deps.createStageDir + ?? (() => mkdtempSync(join(tmpdir(), WINDOWS_SCHEDULER_STAGE_PREFIX))); + const hardenDir = deps.hardenDir + ?? ((path: string) => { hardenSecretDir(path, { required: true }); }); + const writeXml = deps.writeXml ?? ((path: string, contents: string) => { + writeFileSync(path, contents, { encoding: "utf16le", flag: "wx", mode: 0o600 }); + }); + const hardenPath = deps.hardenPath + ?? ((path: string) => { hardenSecretPath(path, { required: true }); }); + const removeStageDir = deps.removeStageDir + ?? ((path: string) => { rmdirSync(path); }); + + let stageDir: string | null = null; + let xmlPath: string | null = null; + try { + stageDir = createStageDir(); + try { chmodSync(stageDir, 0o700); } catch { /* required Windows ACL is authoritative */ } + hardenDir(stageDir); + xmlPath = join(stageDir, "task.xml"); + // This document points at the canonical launcher but does not publish or rewrite it. + // The hardened private directory prevents another local account from replacing the + // document while UAC is pending; the file harden independently proves its identity. + writeXml( + xmlPath, + `\uFEFF${buildWindowsTaskXml(windowsServiceScriptPath(), windowsLauncherVbsPath(), attemptNonce)}`, + ); + hardenPath(xmlPath); + ownedWindowsSchedulerStages.add(xmlPath); + return xmlPath; + } catch (error) { + if (stageDir) { + try { + cleanupWindowsSchedulerStage(stageDir, xmlPath ?? join(stageDir, "task.xml"), removeStageDir); + } catch (cleanupError) { + throw new AggregateError( + [error, cleanupError], + "Task Scheduler staging failed and its private temporary directory could not be removed.", + ); + } + } + throw error; + } +} + +function removeWindowsSchedulerRegistrationStage(xmlPath: string): void { + if (!ownedWindowsSchedulerStages.has(xmlPath)) { + throw new Error("Refusing to remove an unrecognized Task Scheduler staging path."); + } + const stageDir = dirname(xmlPath); + cleanupWindowsSchedulerStage( + stageDir, + xmlPath, + path => { rmdirSync(path); }, ); - return path; + if (existsSync(stageDir)) { + throw new Error("The private Task Scheduler staging directory still exists after cleanup."); + } + ownedWindowsSchedulerStages.delete(xmlPath); } export interface FreshWindowsSchedulerRegistrationDeps { create?: (args: string[]) => void; - elevate?: (args: string[]) => Promise; + elevate?: (taskName: string, xml: string) => Promise; probe?: () => WindowsSchedulerTaskProbe; queryXml?: () => string; rollback?: () => Promise; @@ -1923,6 +2022,16 @@ export async function registerFreshWindowsSchedulerTask( deps: FreshWindowsSchedulerRegistrationDeps = {}, ): Promise { const args = buildWindowsSchtasksCreateArgsForXml(xmlPath); + // Capture and validate the exact definition before an access-denied attempt can + // cross the UAC boundary. The elevated fallback receives these immutable bytes, + // never the caller-writable staging pathname. + const expectedXml = decodeSchtasksOutput(readFileSync(xmlPath)); + if ( + !windowsTaskRegistrationHealthy(expectedXml) + || !windowsTaskRegistrationOwnedByAttempt(expectedXml, attemptNonce) + ) { + throw new Error("The staged Task Scheduler registration failed OpenCodex ownership or shape validation."); + } try { (deps.create ?? schtasks)(args); } catch (error) { @@ -1933,9 +2042,13 @@ export async function registerFreshWindowsSchedulerTask( ) { throw error; } - // The elevated command is still the fixed trusted schtasks executable plus the - // owned create shape. It registers only; the task is not run until cleanup commits. - await (deps.elevate ?? elevateSchtasks)(args); + // Register from the captured XML string inside the elevated process. Another + // same-user process can mutate its own temp files, but cannot change this command. + const elevate = deps.elevate ?? (async (taskName: string, xml: string) => { + const exitCode = await runWindowsElevatedScheduledTaskRegistration(taskName, xml); + if (exitCode !== 0) throw new Error(`Background service install failed with exit code ${exitCode}.`); + }); + await elevate(TASK, expectedXml); } const rollbackTask = deps.rollback ?? (() => rollbackWindowsSchedulerTaskOwnedByAttempt(attemptNonce, TASK)); @@ -1978,21 +2091,47 @@ export async function registerFreshWindowsSchedulerTask( } } -function installWindows(): void { - recordOwnedConfigPath(getConfigDir(), serviceStatePath()); +function recordWindowsSchedulerOwnership(): boolean { + // Ownership claiming is deliberately conservative: a legacy non-empty config root + // without metadata stays unclaimed, but that must not turn a service reinstall into + // an outage after prepareServiceInstall has stopped the previous manager. + return recordOwnedConfigPath(getConfigDir(), serviceStatePath()); +} + +export interface RemoveNativeWindowsServiceDeps { + status?: () => WinswStatus; + uninstall?: () => void; + sleep?: (ms: number) => void; + settleChecks?: number; +} + +export function removeNativeWindowsServiceForScheduler( + deps: RemoveNativeWindowsServiceDeps = {}, +): void { + const status = deps.status ?? statusWinswRaw; + const uninstall = deps.uninstall ?? uninstallWinswService; + const sleep = deps.sleep ?? Bun.sleepSync; + const settleChecks = Math.max(1, deps.settleChecks ?? 20); // Transactional backend switch: installing the scheduler backend removes a native // service first — two live managers would both respawn the proxy (conflict). - if (statusWinswRaw() !== "nonexistent") { + if (status() !== "nonexistent") { console.log("🔁 Removing the native (WinSW) service before installing the Task Scheduler backend..."); try { - uninstallWinswService(); + uninstall(); } catch (err) { throw new Error(`Cannot remove the native service before switching to Task Scheduler: ${err instanceof Error ? err.message : String(err)}. Remove it manually with 'sc delete ${WINSW_SERVICE_ID}' or retry.`); } - if (statusWinswRaw() !== "nonexistent") { - throw new Error(`Native service registration could not be re-verified after the removal attempt — aborting switch. Check 'sc.exe query ${WINSW_SERVICE_ID}' and remove it manually if present.`); + for (let check = 0; check < settleChecks; check++) { + if (status() === "nonexistent") return; + if (check + 1 < settleChecks) sleep(250); } + throw new Error(`Native service registration could not be re-verified after the removal attempt — aborting switch. Check 'sc.exe query ${WINSW_SERVICE_ID}' and remove it manually if present.`); } +} + +function installWindows(): void { + recordWindowsSchedulerOwnership(); + removeNativeWindowsServiceForScheduler(); // End a running task BEFORE rewriting the assets it is executing — cmd.exe reading the // script mid-rewrite runs a torn batch file, and its open handle can fail the write. try { stopWindows(); } catch { /* not running */ } @@ -2595,7 +2734,9 @@ export async function installServiceSafely( export interface FreshWindowsSchedulerInstallDeps { stageRegistrationXml?: (attemptNonce: string) => string; register?: (xmlPath: string, attemptNonce: string) => Promise; + recordOwnership?: () => boolean; prepare?: () => Promise; + removeNativeService?: () => void; publishAssets?: () => void; runTask?: () => void; writeState?: () => void; @@ -2616,7 +2757,9 @@ export async function installFreshWindowsSchedulerSafely( ): Promise { const stage = deps.stageRegistrationXml ?? stageWindowsSchedulerRegistrationXml; const register = deps.register ?? registerFreshWindowsSchedulerTask; + const recordOwnership = deps.recordOwnership ?? recordWindowsSchedulerOwnership; const prepare = deps.prepare ?? (() => prepareServiceInstall("scheduler")); + const removeNativeService = deps.removeNativeService ?? removeNativeWindowsServiceForScheduler; const publishAssets = deps.publishAssets ?? writeWindowsSchedulerAssets; const runTask = deps.runTask ?? startWindows; const writeState = deps.writeState ?? (() => writeServiceInstallState("scheduler")); @@ -2624,11 +2767,12 @@ export async function installFreshWindowsSchedulerSafely( rollbackWindowsSchedulerTaskOwnedByAttempt(attemptNonce, TASK) )); const removeStagedXml = deps.removeStagedXml ?? ((path: string) => { - if (existsSync(path)) unlinkSync(path); + removeWindowsSchedulerRegistrationStage(path); }); let stagedXml: string | null = null; const attemptNonce = randomUUID(); + const configRootWasAbsent = !existsSync(getConfigDir()); let registered = false; let started = false; try { @@ -2637,7 +2781,19 @@ export async function installFreshWindowsSchedulerSafely( registered = true; // The destructive boundary begins only after Task Scheduler accepted the definition. + // The registration has consumed its temporary XML. Remove it before claiming a newly + // created config root, because ownership initialization intentionally requires emptiness. + removeStagedXml(stagedXml); + stagedXml = null; + const ownershipRecorded = recordOwnership(); + if (!ownershipRecorded && configRootWasAbsent) { + throw new Error( + "The fresh OpenCodex config root could not be claimed for safe uninstall; " + + "aborting before service-manager cleanup or asset publication.", + ); + } await prepare(); + removeNativeService(); publishAssets(); runTask(); started = true; @@ -2663,8 +2819,11 @@ export async function installFreshWindowsSchedulerSafely( } finally { if (stagedXml) { try { removeStagedXml(stagedXml); } catch (error) { + const code = error && typeof error === "object" && "code" in error + ? String((error as NodeJS.ErrnoException).code) + : ""; console.error( - `⚠️ Failed to remove temporary Task Scheduler XML ${stagedXml}: ${error instanceof Error ? error.message : String(error)}`, + `⚠️ Failed to remove the private Task Scheduler staging directory${code ? ` (${code})` : ""}.`, ); } } diff --git a/structure/05_gui-and-management-api.md b/structure/05_gui-and-management-api.md index 7646bd3c4..dc05fb046 100644 --- a/structure/05_gui-and-management-api.md +++ b/structure/05_gui-and-management-api.md @@ -211,9 +211,15 @@ to be non-elevated. An unavailable probe remains `other` and cannot trigger UAC. native-service, file-write, and foreign task failures never use this fallback. For a fresh scheduler install whose task is proven absent, registration is the non-destructive -first phase. OpenCodex writes a unique temporary XML definition and asks Task Scheduler to create -the owned task without running it. Only after that succeeds may service-manager cleanup stop the -existing proxy, publish the canonical scheduler assets, run the task, and write install state. +first phase. OpenCodex writes a unique temporary XML definition in an ACL-hardened private directory +outside its config root and asks Task Scheduler to create the owned task without running it. Only +after that succeeds may it discard +the consumed staging XML, require scheduler ownership for a config root that was absent at entry, +stop existing service managers and the proxy, remove and boundedly re-verify any native WinSW +registration, publish the canonical scheduler assets, run the task, and write install state. A +legacy non-empty unowned root remains conservatively unclaimed. This prevents the fresh path from +leaving either an unowned new installation or two registered managers that can both respawn the +proxy. UAC cancellation or create failure removes the temporary XML before any manager/proxy stop, so the working proxy's shutdown cleanup cannot strip Codex routing merely because elevation was refused. The Dashboard does not apply its ordinary 60-second child timeout to this Windows service command: diff --git a/tests/service.test.ts b/tests/service.test.ts index 161304244..41c9cc77d 100644 --- a/tests/service.test.ts +++ b/tests/service.test.ts @@ -1,12 +1,14 @@ import { afterEach, describe, expect, spyOn, test } from "bun:test"; -import { existsSync, mkdirSync, readFileSync, rmSync, writeFileSync } from "node:fs"; +import { existsSync, mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; import { join } from "node:path"; import * as serviceModule from "../src/service"; import { saveConfig } from "../src/config"; import { windowsEnvIndirectBatchValue } from "../src/lib/win-paths"; -import { assertServiceAuthEnvironment, assertServiceEnvironmentMatchesInstall, bakedServicePathsDiagnostic, confirmServiceServing, launchdListenPort, systemdListenPort, buildPlist, buildUnit, buildWindowsLauncherVbs, buildWindowsSchtasksCreateArgs, buildWindowsSchtasksCreateArgsForXml, buildWindowsServiceScript, buildWindowsTaskXml, deriveWindowsServiceDiagnostic, installFreshWindowsSchedulerSafely, installServiceSafely, launchctlLoadFailed, launchdJobMatchesPlist, normalizeServiceSubcommand, parseServiceInstallState, prepareServiceInstall, readWindowsSchedulerXmlState, registerFreshWindowsSchedulerTask, repairService, resolveServiceListenPort, runLaunchctl, serviceLogPath, serviceStartableFromTray, serviceStatusReport, serviceRetryCommand, serviceStatusSummary, systemdNeedsDaemonReload, windowsListenPort, winswListenPort, startLaunchd, windowsTaskRegistrationHealthy } from "../src/service"; +import { assertServiceAuthEnvironment, assertServiceEnvironmentMatchesInstall, bakedServicePathsDiagnostic, confirmServiceServing, launchdListenPort, systemdListenPort, buildPlist, buildUnit, buildWindowsLauncherVbs, buildWindowsSchtasksCreateArgs, buildWindowsSchtasksCreateArgsForXml, buildWindowsServiceScript, buildWindowsTaskXml, deriveWindowsServiceDiagnostic, installFreshWindowsSchedulerSafely, installServiceSafely, launchctlLoadFailed, launchdJobMatchesPlist, normalizeServiceSubcommand, parseServiceInstallState, prepareServiceInstall, readWindowsSchedulerXmlState, registerFreshWindowsSchedulerTask, removeNativeWindowsServiceForScheduler, repairService, resolveServiceListenPort, runLaunchctl, serviceLogPath, serviceStartableFromTray, serviceStatusReport, serviceRetryCommand, serviceStatusSummary, systemdNeedsDaemonReload, windowsListenPort, winswListenPort, startLaunchd, windowsTaskRegistrationHealthy } from "../src/service"; import type { ServiceDiagnostic } from "../src/service"; import { buildWinswXml } from "../src/lib/winsw"; +import { CONFIG_OWNER_FILE, CONFIG_UNINSTALL_MANIFEST, recordOwnedConfigPath, removeOwnedConfigState } from "../src/lib/config-ownership"; import { serviceApiTokenFilePath } from "../src/lib/service-secrets"; import { WindowsSchtasksError } from "../src/lib/windows-elevation"; import type { OcxConfig } from "../src/types"; @@ -664,6 +666,40 @@ describe("launchd service plist", () => { }); describe("service lifecycle cleanup ordering", () => { + test("native service switch treats unknown as installed and requires confirmed absence", () => { + const calls: string[] = []; + const statuses: Array<"unknown" | "stopped" | "nonexistent"> = [ + "unknown", + "stopped", + "unknown", + "nonexistent", + ]; + removeNativeWindowsServiceForScheduler({ + status: () => { + calls.push("status"); + return statuses.shift() ?? "nonexistent"; + }, + uninstall: () => { calls.push("uninstall"); }, + sleep: () => { calls.push("sleep"); }, + }); + expect(calls).toEqual([ + "status", + "uninstall", + "status", + "sleep", + "status", + "sleep", + "status", + ]); + + expect(() => removeNativeWindowsServiceForScheduler({ + status: () => "stopped", + uninstall: () => {}, + sleep: () => {}, + settleChecks: 3, + })).toThrow(/could not be re-verified/); + }); + const registrationAttemptNonce = "service-test-attempt"; test("rollback preserves a task owned by another install attempt and reports residual state", async () => { @@ -717,77 +753,168 @@ describe("service lifecycle cleanup ordering", () => { test("fresh registration elevates only the fixed create after a structured denial", async () => { const calls: string[] = []; - const stagedXml = "C:\\Users\\x\\.opencodex\\attempt.xml"; + const parent = mkdtempSync(join(tmpdir(), "ocx-service-fixed-create-")); + const stagedXml = join(parent, "attempt.xml"); const expectedArgs = buildWindowsSchtasksCreateArgsForXml(stagedXml); - await registerFreshWindowsSchedulerTask(stagedXml, registrationAttemptNonce, { - create: args => { - calls.push(`create:${args.join(" ")}`); - throw new WindowsSchtasksError("create", "access-denied", "denied"); - }, - elevate: async args => { calls.push(`elevate:${args.join(" ")}`); }, - probe: () => ({ status: "present", detail: "present" }), - queryXml: () => buildWindowsTaskXml(undefined, undefined, registrationAttemptNonce), - rollback: async () => { calls.push("rollback"); return null; }, - }); + const expectedXml = buildWindowsTaskXml(undefined, undefined, registrationAttemptNonce); + try { + writeFileSync(stagedXml, `\uFEFF${expectedXml}`, "utf16le"); + await registerFreshWindowsSchedulerTask(stagedXml, registrationAttemptNonce, { + create: args => { + calls.push(`create:${args.join(" ")}`); + throw new WindowsSchtasksError("create", "access-denied", "denied"); + }, + elevate: async (taskName, xml) => { + calls.push(`elevate:${taskName}`); + expect(xml).toBe(expectedXml.trimEnd()); + }, + probe: () => ({ status: "present", detail: "present" }), + queryXml: () => expectedXml, + rollback: async () => { calls.push("rollback"); return null; }, + }); - expect(calls).toEqual([ - `create:${expectedArgs.join(" ")}`, - `elevate:${expectedArgs.join(" ")}`, - ]); + expect(calls).toEqual([ + `create:${expectedArgs.join(" ")}`, + "elevate:opencodex-proxy", + ]); + } finally { + rmSync(parent, { recursive: true, force: true }); + } }); test("fresh registration UAC denial returns before task probing or cleanup", async () => { const calls: string[] = []; - await expect(registerFreshWindowsSchedulerTask("attempt.xml", registrationAttemptNonce, { - create: () => { - calls.push("create"); - throw new WindowsSchtasksError("create", "access-denied", "denied"); - }, - elevate: async () => { calls.push("elevate"); throw new Error("UAC cancelled"); }, - probe: () => { calls.push("probe"); return { status: "present", detail: "present" }; }, - queryXml: () => { calls.push("query"); return buildWindowsTaskXml(undefined, undefined, registrationAttemptNonce); }, - rollback: async () => { calls.push("rollback"); return null; }, - })).rejects.toThrow("UAC cancelled"); + const parent = mkdtempSync(join(tmpdir(), "ocx-service-uac-denial-")); + const stagedXml = join(parent, "attempt.xml"); + try { + writeFileSync(stagedXml, `\uFEFF${buildWindowsTaskXml(undefined, undefined, registrationAttemptNonce)}`, "utf16le"); + await expect(registerFreshWindowsSchedulerTask(stagedXml, registrationAttemptNonce, { + create: () => { + calls.push("create"); + throw new WindowsSchtasksError("create", "access-denied", "denied"); + }, + elevate: async () => { calls.push("elevate"); throw new Error("UAC cancelled"); }, + probe: () => { calls.push("probe"); return { status: "present", detail: "present" }; }, + queryXml: () => { calls.push("query"); return buildWindowsTaskXml(undefined, undefined, registrationAttemptNonce); }, + rollback: async () => { calls.push("rollback"); return null; }, + })).rejects.toThrow("UAC cancelled"); - expect(calls).toEqual(["create", "elevate"]); + expect(calls).toEqual(["create", "elevate"]); + } finally { + rmSync(parent, { recursive: true, force: true }); + } }); test("fresh registration never elevates an unstructured scheduler failure", async () => { const calls: string[] = []; - await expect(registerFreshWindowsSchedulerTask("attempt.xml", registrationAttemptNonce, { - create: () => { calls.push("create"); throw new Error("scheduler unavailable"); }, - elevate: async () => { calls.push("elevate"); }, - probe: () => { calls.push("probe"); return { status: "present", detail: "present" }; }, - queryXml: () => buildWindowsTaskXml(undefined, undefined, registrationAttemptNonce), - rollback: async () => { calls.push("rollback"); return null; }, - })).rejects.toThrow("scheduler unavailable"); - - expect(calls).toEqual(["create"]); + const parent = mkdtempSync(join(tmpdir(), "ocx-service-unstructured-")); + const stagedXml = join(parent, "attempt.xml"); + try { + writeFileSync(stagedXml, `\uFEFF${buildWindowsTaskXml(undefined, undefined, registrationAttemptNonce)}`, "utf16le"); + await expect(registerFreshWindowsSchedulerTask(stagedXml, registrationAttemptNonce, { + create: () => { calls.push("create"); throw new Error("scheduler unavailable"); }, + elevate: async () => { calls.push("elevate"); }, + probe: () => { calls.push("probe"); return { status: "present", detail: "present" }; }, + queryXml: () => buildWindowsTaskXml(undefined, undefined, registrationAttemptNonce), + rollback: async () => { calls.push("rollback"); return null; }, + })).rejects.toThrow("scheduler unavailable"); + + expect(calls).toEqual(["create"]); + } finally { + rmSync(parent, { recursive: true, force: true }); + } }); test("create success followed by proven absence does not request a pointless rollback UAC", async () => { const calls: string[] = []; - await expect(registerFreshWindowsSchedulerTask("attempt.xml", registrationAttemptNonce, { - create: () => { calls.push("create"); }, - elevate: async () => { calls.push("elevate"); }, - probe: () => { calls.push("probe"); return { status: "absent", detail: "absent" }; }, - queryXml: () => { calls.push("query"); return buildWindowsTaskXml(undefined, undefined, registrationAttemptNonce); }, - rollback: async () => { calls.push("rollback"); return null; }, - })).rejects.toThrow(/registration is absent/); - - expect(calls).toEqual(["create", "probe"]); + const parent = mkdtempSync(join(tmpdir(), "ocx-service-proven-absence-")); + const stagedXml = join(parent, "attempt.xml"); + try { + writeFileSync(stagedXml, `\uFEFF${buildWindowsTaskXml(undefined, undefined, registrationAttemptNonce)}`, "utf16le"); + await expect(registerFreshWindowsSchedulerTask(stagedXml, registrationAttemptNonce, { + create: () => { calls.push("create"); }, + elevate: async () => { calls.push("elevate"); }, + probe: () => { calls.push("probe"); return { status: "absent", detail: "absent" }; }, + queryXml: () => { calls.push("query"); return buildWindowsTaskXml(undefined, undefined, registrationAttemptNonce); }, + rollback: async () => { calls.push("rollback"); return null; }, + })).rejects.toThrow(/registration is absent/); + + expect(calls).toEqual(["create", "probe"]); + } finally { + rmSync(parent, { recursive: true, force: true }); + } }); test("fresh registration requires the live Task Scheduler XML before cleanup can begin", async () => { const calls: string[] = []; - await expect(registerFreshWindowsSchedulerTask("attempt.xml", registrationAttemptNonce, { - create: () => { calls.push("create"); }, - probe: () => { calls.push("probe"); return { status: "present", detail: "present" }; }, - queryXml: () => { calls.push("query"); throw new Error("query denied"); }, - rollback: async () => { calls.push("rollback"); return null; }, - })).rejects.toThrow(/live XML could not be verified/); + const parent = mkdtempSync(join(tmpdir(), "ocx-service-live-xml-")); + const xml = join(parent, "attempt.xml"); + try { + writeFileSync(xml, `\uFEFF${buildWindowsTaskXml(undefined, undefined, registrationAttemptNonce)}`, "utf16le"); + await expect(registerFreshWindowsSchedulerTask(xml, registrationAttemptNonce, { + create: () => { calls.push("create"); }, + probe: () => { calls.push("probe"); return { status: "present", detail: "present" }; }, + queryXml: () => { calls.push("query"); throw new Error("query denied"); }, + rollback: async () => { calls.push("rollback"); return null; }, + })).rejects.toThrow(/live XML could not be verified/); + + expect(calls).toEqual(["create", "probe", "query", "rollback"]); + } finally { + rmSync(parent, { recursive: true, force: true }); + } + }); + + test("fresh registration elevation uses captured XML bytes after the staged file changes", async () => { + const parent = mkdtempSync(join(tmpdir(), "ocx-service-elevated-xml-")); + const xmlPath = join(parent, "attempt.xml"); + const originalXml = buildWindowsTaskXml(undefined, undefined, registrationAttemptNonce); + const foreignXml = buildWindowsTaskXml("C:\\foreign.cmd", undefined, "foreign-attempt"); + let elevatedXml = ""; + try { + writeFileSync(xmlPath, `\uFEFF${originalXml}`, "utf16le"); + await registerFreshWindowsSchedulerTask(xmlPath, registrationAttemptNonce, { + create: () => { + writeFileSync(xmlPath, `\uFEFF${foreignXml}`, "utf16le"); + throw new WindowsSchtasksError("create", "access-denied", "denied"); + }, + elevate: async (taskName, xml) => { + expect(taskName).toBe("opencodex-proxy"); + elevatedXml = xml; + }, + probe: () => ({ status: "present", detail: "present" }), + queryXml: () => originalXml, + rollback: async () => null, + }); - expect(calls).toEqual(["create", "probe", "query", "rollback"]); + expect(elevatedXml).toContain(`install-attempt=${registrationAttemptNonce}`); + expect(elevatedXml).not.toContain("foreign-attempt"); + } finally { + rmSync(parent, { recursive: true, force: true }); + } + }); + + test("fresh registration rejects a staged definition owned by another attempt before create", async () => { + const parent = mkdtempSync(join(tmpdir(), "ocx-service-foreign-stage-")); + const xmlPath = join(parent, "attempt.xml"); + const calls: string[] = []; + try { + writeFileSync( + xmlPath, + `\uFEFF${buildWindowsTaskXml(undefined, undefined, "foreign-attempt")}`, + "utf16le", + ); + await expect(registerFreshWindowsSchedulerTask(xmlPath, registrationAttemptNonce, { + create: () => { calls.push("create"); }, + elevate: async () => { calls.push("elevate"); }, + probe: () => { calls.push("probe"); return { status: "present", detail: "present" }; }, + queryXml: () => { calls.push("query"); return ""; }, + rollback: async () => { calls.push("rollback"); return null; }, + })).rejects.toThrow(/ownership or shape validation/); + + expect(calls).toEqual([]); + } finally { + rmSync(parent, { recursive: true, force: true }); + } }); test("fresh Windows scheduler install gets registration approval before destructive cleanup", async () => { @@ -799,7 +926,9 @@ describe("service lifecycle cleanup ordering", () => { expect(nonce).toBe(stagedNonce); calls.push(`register:${path}`); }, + recordOwnership: () => { calls.push("record-ownership"); return true; }, prepare: async () => { calls.push("prepare:stop-managers-and-proxy"); }, + removeNativeService: () => { calls.push("remove-native-service"); }, publishAssets: () => { calls.push("publish-assets"); }, runTask: () => { calls.push("run-task"); }, writeState: () => { calls.push("write-state"); }, @@ -810,15 +939,84 @@ describe("service lifecycle cleanup ordering", () => { expect(calls).toEqual([ "stage", "register:attempt.xml", + "remove:attempt.xml", + "record-ownership", "prepare:stop-managers-and-proxy", + "remove-native-service", "publish-assets", "run-task", "write-state", - "remove:attempt.xml", ]); expect(stagedNonce).not.toBe(""); }); + test("fresh scheduler staging hardens its private directory and XML before registration", async () => { + const parent = mkdtempSync(join(tmpdir(), "ocx-service-stage-order-")); + const stageDir = join(parent, "private-stage"); + const calls: string[] = []; + try { + await installFreshWindowsSchedulerSafely({ + stageRegistrationXml: nonce => serviceModule.stageWindowsSchedulerRegistrationXml(nonce, { + createStageDir: () => { + mkdirSync(stageDir, { mode: 0o700 }); + calls.push("create-stage-dir"); + return stageDir; + }, + hardenDir: () => { calls.push("harden-dir"); }, + writeXml: (path, contents) => { + calls.push("write-xml"); + writeFileSync(path, contents, { encoding: "utf16le", flag: "wx" }); + }, + hardenPath: () => { calls.push("harden-xml"); }, + }), + register: async path => { + calls.push("register"); + expect(existsSync(path)).toBe(true); + }, + recordOwnership: () => true, + prepare: async () => {}, + removeNativeService: () => {}, + publishAssets: () => {}, + runTask: () => {}, + writeState: () => {}, + rollbackTask: async () => null, + }); + + expect(calls).toEqual([ + "create-stage-dir", + "harden-dir", + "write-xml", + "harden-xml", + "register", + ]); + expect(existsSync(stageDir)).toBe(false); + } finally { + rmSync(parent, { recursive: true, force: true }); + } + }); + + test("fresh scheduler staging removes a partially-written private directory on failure", () => { + const parent = mkdtempSync(join(tmpdir(), "ocx-service-stage-failure-")); + const stageDir = join(parent, "private-stage"); + try { + expect(() => serviceModule.stageWindowsSchedulerRegistrationXml("attempt", { + createStageDir: () => { + mkdirSync(stageDir, { mode: 0o700 }); + return stageDir; + }, + hardenDir: () => {}, + writeXml: (path, contents) => { + writeFileSync(path, contents, "utf16le"); + throw new Error("synthetic partial write failure"); + }, + hardenPath: () => { throw new Error("must not harden after write failure"); }, + })).toThrow("synthetic partial write failure"); + expect(existsSync(stageDir)).toBe(false); + } finally { + rmSync(parent, { recursive: true, force: true }); + } + }); + test("UAC cancellation removes only staged XML and never enters cleanup or asset publication", async () => { const calls: string[] = []; mkdirSync(TEST_DIR, { recursive: true }); @@ -831,7 +1029,9 @@ describe("service lifecycle cleanup ordering", () => { calls.push(`register:${path}`); throw new Error("UAC prompt was cancelled"); }, + recordOwnership: () => { calls.push("record-ownership"); return true; }, prepare: async () => { calls.push("prepare"); }, + removeNativeService: () => { calls.push("remove-native-service"); }, publishAssets: () => { calls.push("publish-assets"); }, runTask: () => { calls.push("run-task"); }, writeState: () => { calls.push("write-state"); }, @@ -852,7 +1052,9 @@ describe("service lifecycle cleanup ordering", () => { await expect(installFreshWindowsSchedulerSafely({ stageRegistrationXml: () => "attempt.xml", register: async () => { calls.push("register"); }, + recordOwnership: () => { calls.push("record-ownership"); return true; }, prepare: async () => { calls.push("prepare"); throw new Error("standalone stop failed"); }, + removeNativeService: () => { calls.push("remove-native-service"); }, publishAssets: () => { calls.push("publish-assets"); }, runTask: () => { calls.push("run-task"); }, writeState: () => { calls.push("write-state"); }, @@ -860,7 +1062,114 @@ describe("service lifecycle cleanup ordering", () => { removeStagedXml: () => { calls.push("remove-stage"); }, })).rejects.toThrow(/previous proxy\/routing state was not assumed restored/); - expect(calls).toEqual(["register", "prepare", "rollback-task", "remove-stage"]); + expect(calls).toEqual(["register", "remove-stage", "record-ownership", "prepare", "rollback-task"]); + }); + + test("fresh scheduler install rolls back before publication when native service removal fails", async () => { + const calls: string[] = []; + await expect(installFreshWindowsSchedulerSafely({ + stageRegistrationXml: () => "attempt.xml", + register: async () => { calls.push("register"); }, + recordOwnership: () => { calls.push("record-ownership"); return true; }, + prepare: async () => { calls.push("prepare"); }, + removeNativeService: () => { calls.push("remove-native-service"); throw new Error("native service remains"); }, + publishAssets: () => { calls.push("publish-assets"); }, + runTask: () => { calls.push("run-task"); }, + writeState: () => { calls.push("write-state"); }, + rollbackTask: async () => { calls.push("rollback-task"); return null; }, + removeStagedXml: () => { calls.push("remove-stage"); }, + })).rejects.toThrow("native service remains"); + + expect(calls).toEqual([ + "register", + "remove-stage", + "record-ownership", + "prepare", + "remove-native-service", + "rollback-task", + ]); + }); + + test("fresh scheduler install removes staging before initializing config ownership", async () => { + const parent = mkdtempSync(join(tmpdir(), "ocx-service-fresh-ownership-")); + const home = join(parent, "config"); + const previousHome = process.env.OPENCODEX_HOME; + process.env.OPENCODEX_HOME = home; + let stagedPath = ""; + try { + // Seed the exact stale-null lifecycle: a conservative legacy refusal is cached, + // then the same root is deleted before this long-lived process installs fresh. + mkdirSync(home, { recursive: true }); + writeFileSync(join(home, "legacy.txt"), "keep", "utf8"); + expect(recordOwnedConfigPath(home, join(home, "service-state.json"))).toBe(false); + rmSync(home, { recursive: true, force: true }); + + await installFreshWindowsSchedulerSafely({ + register: async path => { + stagedPath = path; + expect(existsSync(path)).toBe(true); + expect(path.startsWith(tmpdir())).toBe(true); + expect(existsSync(home)).toBe(false); + }, + prepare: async () => {}, + removeNativeService: () => {}, + publishAssets: () => {}, + runTask: () => {}, + writeState: () => {}, + rollbackTask: async () => null, + }); + + expect(stagedPath.startsWith(home)).toBe(false); + expect(existsSync(stagedPath)).toBe(false); + expect(existsSync(join(stagedPath, ".."))).toBe(false); + expect(JSON.parse(readFileSync(join(home, CONFIG_OWNER_FILE), "utf8"))).toMatchObject({ version: 1 }); + const manifest = JSON.parse(readFileSync(join(home, CONFIG_UNINSTALL_MANIFEST), "utf8")) as { paths: string[] }; + expect(manifest.paths).toContain("service-state.json"); + expect(removeOwnedConfigState(home)).toEqual({ status: "removed", residualPaths: [] }); + expect(existsSync(home)).toBe(false); + } finally { + if (previousHome === undefined) delete process.env.OPENCODEX_HOME; + else process.env.OPENCODEX_HOME = previousHome; + removeOwnedConfigState(home); + rmSync(parent, { recursive: true, force: true }); + } + }); + + test("fresh scheduler install rolls back before cleanup when a new config root cannot be claimed", async () => { + const parent = mkdtempSync(join(tmpdir(), "ocx-service-ownership-race-")); + const home = join(parent, "config"); + const foreign = join(home, "foreign.txt"); + const calls: string[] = []; + const previousHome = process.env.OPENCODEX_HOME; + process.env.OPENCODEX_HOME = home; + let stagedPath = ""; + try { + await expect(installFreshWindowsSchedulerSafely({ + register: async path => { + stagedPath = path; + calls.push("register"); + mkdirSync(home, { recursive: true }); + writeFileSync(foreign, "keep", "utf8"); + }, + prepare: async () => { calls.push("prepare"); }, + removeNativeService: () => { calls.push("remove-native-service"); }, + publishAssets: () => { calls.push("publish-assets"); }, + runTask: () => { calls.push("run-task"); }, + writeState: () => { calls.push("write-state"); }, + rollbackTask: async () => { calls.push("rollback-task"); return null; }, + })).rejects.toThrow(/fresh OpenCodex config root could not be claimed/); + + expect(calls).toEqual(["register", "rollback-task"]); + expect(readFileSync(foreign, "utf8")).toBe("keep"); + expect(existsSync(join(home, CONFIG_OWNER_FILE))).toBe(false); + expect(existsSync(join(home, CONFIG_UNINSTALL_MANIFEST))).toBe(false); + expect(existsSync(stagedPath)).toBe(false); + } finally { + if (previousHome === undefined) delete process.env.OPENCODEX_HOME; + else process.env.OPENCODEX_HOME = previousHome; + removeOwnedConfigState(home); + rmSync(parent, { recursive: true, force: true }); + } }); test("a state-write failure leaves the already-started task for explicit diagnosis", async () => { @@ -868,7 +1177,9 @@ describe("service lifecycle cleanup ordering", () => { await expect(installFreshWindowsSchedulerSafely({ stageRegistrationXml: () => "attempt.xml", register: async () => { calls.push("register"); }, + recordOwnership: () => { calls.push("record-ownership"); return true; }, prepare: async () => { calls.push("prepare"); }, + removeNativeService: () => { calls.push("remove-native-service"); }, publishAssets: () => { calls.push("publish-assets"); }, runTask: () => { calls.push("run-task"); }, writeState: () => { calls.push("write-state"); throw new Error("state write failed"); }, @@ -878,11 +1189,13 @@ describe("service lifecycle cleanup ordering", () => { expect(calls).toEqual([ "register", + "remove-stage", + "record-ownership", "prepare", + "remove-native-service", "publish-assets", "run-task", "write-state", - "remove-stage", ]); }); diff --git a/tests/windows-elevation-spawn.test.ts b/tests/windows-elevation-spawn.test.ts index 0c7bab727..1eff2dddb 100644 --- a/tests/windows-elevation-spawn.test.ts +++ b/tests/windows-elevation-spawn.test.ts @@ -14,6 +14,7 @@ import { raceWithTimeout, runElevatedSchtasksCreateAndRun, runWindowsElevated, + runWindowsElevatedScheduledTaskRegistration, setWindowsElevationSpawnForTests, setTrustedWindowsElevationExecutablesForTests, startElevatedSchtasksCreateAndRun, @@ -119,6 +120,38 @@ describe("runWindowsElevated spawn contract", () => { await expect(runWindowsElevated("schtasks.exe", ["/create"])).resolves.toBe(1); }); + test("scheduled-task registration embeds immutable XML bytes instead of a file path", async () => { + let commandScript = ""; + setWindowsElevationSpawnForTests((( + _cmd: string, + args: ReadonlyArray, + ) => { + commandScript = String(args[args.length - 1] ?? ""); + const child = new EventEmitter() as EventEmitter & { + stdout: EventEmitter & { setEncoding?: (enc: string) => void }; + stderr: EventEmitter & { setEncoding?: (enc: string) => void }; + kill: ReturnType; + }; + child.stdout = new EventEmitter(); + child.stderr = new EventEmitter(); + child.stdout.setEncoding = () => undefined; + child.stderr.setEncoding = () => undefined; + child.kill = mock(() => true); + queueMicrotask(() => child.emit("close", 0, null)); + return child as never; + }) as never); + + const xml = "fixed-definition"; + await expect(runWindowsElevatedScheduledTaskRegistration("opencodex-proxy", xml)).resolves.toBe(0); + const match = /-EncodedCommand ([A-Za-z0-9+/=]+)/.exec(commandScript); + expect(match).not.toBeNull(); + const elevatedScript = Buffer.from(match![1]!, "base64").toString("utf16le"); + expect(elevatedScript).toContain("Register-ScheduledTask -TaskName $taskName -Xml $xml -Force"); + expect(elevatedScript).toContain(Buffer.from(xml, "utf16le").toString("base64")); + expect(commandScript).not.toContain("/xml"); + expect(commandScript).not.toContain("task.xml"); + }); + test("maps exit 1223 to cancelled", async () => { fakeChild({ code: OCX_ELEVATED_UAC_CANCELLED }); await expect(runWindowsElevated("schtasks.exe", ["/create"])).rejects.toMatchObject({ diff --git a/tests/winsw.test.ts b/tests/winsw.test.ts index f42ed31c4..f3f5cce06 100644 --- a/tests/winsw.test.ts +++ b/tests/winsw.test.ts @@ -138,14 +138,6 @@ describe("winsw fail-closed lifecycle", () => { ).rejects.toThrow(/Could not query the native service state/); }); - test("a failed status query is treated as possibly-installed by lifecycle consumers", () => { - // stopServiceIfInstalled/installWindows gate on `!== "nonexistent"` — "unknown" - // must therefore route INTO stop/uninstall attempts, never skip them. - const service = readFileSync(new URL("../src/service.ts", import.meta.url), "utf8"); - expect(service).not.toContain('statusWinswRaw() === "unknown"'); - expect((service.match(/statusWinswRaw\(\) !== "nonexistent"/g) ?? []).length).toBeGreaterThanOrEqual(3); - }); - test("exe missing + non-Windows is confirmed absence; on Windows the SCM is queried", () => { // This test host has no WinSW binary installed, so the missing-exe branch runs: // off-Windows it must short-circuit to "nonexistent" (no sc.exe exists here).