fix(service): pin elevated scheduler staging paths

src/lib/windows-elevation.ts

@@ -662,31 +662,34 @@ export function runWindowsElevated(file: string, args: string[]): Promise<number
 /**
  * A task definition staged for the elevated process.
  *
- * The bytes live in a freshly created, ACL-hardened private directory, and the digest is
- * taken over exactly those bytes by the caller that validated them. The elevated script
- * reads the file once, hashes what it read, and refuses unless the digest matches, so a
- * pathname is no longer a promise about content — it is a claim the receiver checks.
+ * Before elevation, the launcher pins the file and every ancestor with non-reparse
+ * handles that deny write/delete sharing. The elevated script additionally bounds and
+ * hashes the exact bytes it decodes.
  */
 export interface StagedWindowsTaskXml {
   /** Path inside the caller's hardened staging directory. */
   readonly path: string;
+  /** Exact byte length, checked before the elevated process allocates or reads. */
+  readonly byteLength: number;
   /** Lowercase hex SHA-256 of the staged bytes (UTF-16LE, no BOM). */
   readonly sha256: string;
 }
 
 /**
  * Read a staged payload, prove it is the one that was validated, and decode it.
  *
- * One read: the bytes that are hashed are the same array that is decoded and registered.
- * Hashing a path and then reopening it would reintroduce the swap window this check
- * exists to close.
+ * One bounded read: the bytes that are hashed are the same array that is decoded and
+ * registered. The unelevated launcher keeps the namespace and files pinned throughout.
  */
-const READ_STAGED_TASK_XML = "function Read-OcxStagedTaskXml([string]$path, [string]$expectedHash) {"
+const READ_STAGED_TASK_XML = "function Read-OcxStagedTaskXml([string]$path, [long]$expectedLength, [string]$expectedHash) {"
   // An unreadable payload is a diagnosable condition, not a generic throw: a hidden
   // elevated process has nowhere to print, so the cause has to ride the exit code.
-  + " try { $bytes = [IO.File]::ReadAllBytes($path) }"
+  + " try { $stream = [IO.File]::Open($path, 'Open', 'Read', 'Read');"
+  + " if ($stream.Length -ne $expectedLength) { throw 'Task Scheduler staged payload has an invalid length.' };"
+  + " $bytes = [byte[]]::new($expectedLength); $offset = 0;"
+  + " while ($offset -lt $bytes.Length) { $read = $stream.Read($bytes, $offset, $bytes.Length - $offset); if ($read -eq 0) { throw 'Task Scheduler staged payload ended early.' }; $offset += $read } }"
   + " catch [System.UnauthorizedAccessException] { exit " + OCX_ELEVATED_STAGING_UNREADABLE + " }"
-  + " catch [System.Security.SecurityException] { exit " + OCX_ELEVATED_STAGING_UNREADABLE + " };"
+  + " catch [System.Security.SecurityException] { exit " + OCX_ELEVATED_STAGING_UNREADABLE + " } finally { if ($null -ne $stream) { $stream.Dispose() } };"
   + " $sha = [Security.Cryptography.SHA256]::Create();"
   + " try { $actual = [BitConverter]::ToString($sha.ComputeHash($bytes)).Replace('-', '').ToLowerInvariant() } finally { $sha.Dispose() };"
   + " if ($actual -cne $expectedHash) { throw 'Task Scheduler staged payload failed its integrity check.' };"
@@ -706,12 +709,10 @@ const READ_STAGED_TASK_XML = "function Read-OcxStagedTaskXml([string]$path, [str
  * The command now carries two paths and two 64-character digests, so its length no
  * longer depends on the size of the XML at all.
  *
- * The original design goal was "immutable bytes, never a caller-writable pathname".
- * That goal is kept by different means rather than abandoned: the staging directory is
- * private and ACL-hardened, the files are created exclusively so nothing can be waiting
- * at the path, and the digest makes a same-account swap during the UAC prompt fail
- * closed instead of registering something else. An ACL alone could not do that last
- * part, because a process running as the same user has the same SID.
+ * The unelevated launcher opens every ancestor and payload with OPEN_REPARSE_POINT,
+ * validates its type, and denies write/delete sharing until the elevated process exits.
+ * Thus the privileged open cannot be redirected during UAC; the length and digest are
+ * defense in depth for the bytes read from the pinned regular file.
  *
  * The replacement precondition is unchanged: the elevated process still re-queries the
  * live registration and compares it to the captured predecessor before passing -Force.
@@ -725,18 +726,37 @@ export function runWindowsElevatedScheduledTaskRegistration(
   if (replace && !expectedExisting) {
     throw new Error("Elevated Task Scheduler replacement requires a captured existing definition.");
   }
+  for (const payload of [xml, expectedExisting].filter((value): value is StagedWindowsTaskXml => value !== undefined)) {
+    if (!Number.isSafeInteger(payload.byteLength) || payload.byteLength < 0 || !/^[0-9a-f]{64}$/.test(payload.sha256)) {
+      throw new Error("Elevated Task Scheduler staging metadata is invalid.");
+    }
+  }
   const powerShellPath = windowsPowerShell();
   const powerShellDirectory = powerShellPath.replace(/[\\/][^\\/]+$/, "");
   const scheduledTasksModule = `${powerShellDirectory}\\Modules\\ScheduledTasks\\ScheduledTasks.psd1`;
+  const stageDirectory = xml.path.replace(/[\\/][^\\/]+$/, "");
+  // A prefix match is not containment: a `..` segment passes startsWith while the
+  // resolved path lands outside the pinned directory, so traversal segments are
+  // rejected on either separator before the prefix is compared.
+  const sharesStagingDirectory = (path: string) =>
+    !path.split(/[\\/]+/).includes("..")
+    && (path.startsWith(`${stageDirectory}\\`) || path.startsWith(`${stageDirectory}/`));
+  if (
+    stageDirectory === xml.path
+    || !sharesStagingDirectory(xml.path)
+    || (expectedExisting && !sharesStagingDirectory(expectedExisting.path))
+  ) {
+    throw new Error("Elevated Task Scheduler payloads must share one staging directory.");
+  }
   const inner = [
     `$taskName = ${psSingleQuote(taskName)}`,
     READ_STAGED_TASK_XML,
-    `$xml = Read-OcxStagedTaskXml ${psSingleQuote(xml.path)} ${psSingleQuote(xml.sha256)}`,
+    `$xml = Read-OcxStagedTaskXml ${psSingleQuote(xml.path)} ${xml.byteLength} ${psSingleQuote(xml.sha256)}`,
     `$module = Microsoft.PowerShell.Core\\Import-Module -Name ${psSingleQuote(scheduledTasksModule)} -PassThru -Force -ErrorAction Stop`,
     "$registerTask = $module.ExportedCommands['Register-ScheduledTask']",
     "if ($null -eq $registerTask) { throw 'Trusted ScheduledTasks module does not export Register-ScheduledTask.' }",
     ...(replace ? [
-      `$expectedXml = Read-OcxStagedTaskXml ${psSingleQuote(expectedExisting!.path)} ${psSingleQuote(expectedExisting!.sha256)}`,
+      `$expectedXml = Read-OcxStagedTaskXml ${psSingleQuote(expectedExisting!.path)} ${expectedExisting!.byteLength} ${psSingleQuote(expectedExisting!.sha256)}`,
       `$schtasks = ${psSingleQuote(resolveTrustedWindowsSchtasksExe())}`,
       "$currentXml = & $schtasks /query /tn $taskName /xml 2>$null | Out-String",
       "if ($LASTEXITCODE -ne 0) { throw 'Task Scheduler replacement precondition could not be read.' }",
@@ -747,6 +767,13 @@ export function runWindowsElevatedScheduledTaskRegistration(
   ].join("; ");
   const encodedCommand = Buffer.from(inner, "utf16le").toString("base64");
   const script = [
+    "Add-Type -TypeDefinition 'using System; using System.Runtime.InteropServices; public static class OcxStageLock { [StructLayout(LayoutKind.Sequential)] public struct TagInfo { public uint Attributes; public uint Tag; } [DllImport(\"kernel32.dll\", CharSet=CharSet.Unicode, SetLastError=true)] public static extern Microsoft.Win32.SafeHandles.SafeFileHandle CreateFile(string p, uint a, uint s, IntPtr q, uint c, uint f, IntPtr t); [DllImport(\"kernel32.dll\", SetLastError=true)] public static extern bool GetFileInformationByHandleEx(Microsoft.Win32.SafeHandles.SafeFileHandle h, int c, out TagInfo i, uint n); }';",
+    "$locks = @();",
+    "function Lock-OcxStage([string]$path, [bool]$directory) { $flags = 0x00200000; $share = 1; if ($directory) { $flags = $flags -bor 0x02000000; $share = 3 }; $h = [OcxStageLock]::CreateFile($path, 0x80000000, $share, [IntPtr]::Zero, 3, $flags, [IntPtr]::Zero); if ($h.IsInvalid) { throw 'Task Scheduler staging lock failed.' }; $info = [OcxStageLock+TagInfo]::new(); if (![OcxStageLock]::GetFileInformationByHandleEx($h, 9, [ref]$info, 8) -or (($info.Attributes -band 0x400) -ne 0) -or $directory -ne (($info.Attributes -band 0x10) -ne 0)) { $h.Dispose(); throw 'Task Scheduler staging path is redirected or has the wrong type.' }; $script:locks += $h };",
+    `$dir = [IO.DirectoryInfo]::new(${psSingleQuote(stageDirectory)}); $dirs = @(); while ($null -ne $dir) { $dirs += $dir.FullName; $dir = $dir.Parent }; [array]::Reverse($dirs); $dirs | ForEach-Object { Lock-OcxStage $_ $true };`,
+    `Lock-OcxStage ${psSingleQuote(xml.path)} $false;`,
+    ...(expectedExisting ? [`Lock-OcxStage ${psSingleQuote(expectedExisting.path)} $false;`] : []),
+    "try {",
     `$p = Start-Process -FilePath ${psSingleQuote(powerShellPath)}`,
     ` -ArgumentList ${psSingleQuote(buildWindowsElevatedArgumentList([
       "-NoProfile",
@@ -760,7 +787,7 @@ export function runWindowsElevatedScheduledTaskRegistration(
     `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",
+    "$code = $p.ExitCode } finally { $locks | ForEach-Object { $_.Dispose() } }; exit $code",
   ].join("");
 
   return startPowerShellCommand(script).completion.then(result => result.exitCode);

src/service/windows-ops.ts

@@ -200,11 +200,10 @@ export interface ElevatedSchedulerStagingDeps {
  *   is the atomic step here — there is no replace path to race, because every path is
  *   inside a directory that did not exist a moment ago. The explicit check is what keeps
  *   that guarantee from depending on a reading of `O_EXCL` semantics.
- * - **Tamper evidence.** The digest is taken over the exact bytes written, and the
- *   elevated script recomputes it over the bytes it reads. An ACL cannot cover this:
- *   a process running as the same user has the same SID and can rewrite the file, so
- *   the digest is the only thing that makes such a swap fail closed rather than
- *   silently register a different task definition.
+ * - **Pinned namespace and content.** Before UAC, the launcher opens every ancestor and
+ *   payload without following reparse points and without sharing write/delete access.
+ *   Those handles stay open until elevation exits. The digest then verifies the exact,
+ *   length-bounded bytes read by the elevated process.
  *
  * Payloads are UTF-16LE with no BOM, and the elevated process decodes them straight into
  * `Register-ScheduledTask`. What is hashed is therefore exactly what is registered, with
@@ -266,7 +265,7 @@ export function stageElevatedSchedulerRegistration(
         throw new Error(`Refusing to stage an elevated Task Scheduler payload through a redirected path: ${path}`);
       }
       hardenPath(path);
-      return { path, sha256: createHash("sha256").update(bytes).digest("hex") };
+      return { path, byteLength: bytes.length, sha256: createHash("sha256").update(bytes).digest("hex") };
     };
     return {
       xml: stage("register.xml", xml),

structure/runtime.md

@@ -123,7 +123,7 @@ does not perform OAuth, and runtime credential resolution rereads the owned sour
 | `src/types.ts` | Shared config, parsed request, adapter, and event types. |
 | `src/reasoning-effort.ts` | Codex reasoning-level definitions (`low`/`medium`/`high`/`xhigh`), per-model effort mapping, and catalog effort sanitization. |
 | `src/codex/shim.ts` | Codex autostart shim: replaces the `codex` binary with a wrapper that auto-starts the proxy on demand. It skips startup for management subcommands even when value-taking global flags precede the subcommand, and transactionally restores complete, stable external launcher replacements without a watcher or PATH rediscovery. |
-| `src/service.ts` | OS service manager (macOS launchd, Linux systemd, Windows schtasks): always-on proxy with crash restart. Facade over the `src/service/` leaves — `src/service/launchd.ts`, `src/service/systemd.ts`, `src/service/windows-ops.ts`, `src/service/windows-scheduler.ts`, `src/service/windows-taskxml.ts`, `src/service/state.ts`, `src/service/guards.ts`, `src/service/health.ts`, `src/service/repair.ts`, `src/service/orchestration.ts`, `src/service/diagnostics.ts`, `src/service/cli.ts`. |
+| `src/service.ts` | OS service manager (macOS launchd, Linux systemd, Windows schtasks): always-on proxy with crash restart. Facade over the `src/service/` leaves — `src/service/launchd.ts`, `src/service/systemd.ts`, `src/service/windows-ops.ts`, `src/service/windows-scheduler.ts`, `src/service/windows-taskxml.ts`, `src/service/state.ts`, `src/service/guards.ts`, `src/service/health.ts`, `src/service/repair.ts`, `src/service/orchestration.ts`, `src/service/diagnostics.ts`, `src/service/cli.ts`. Elevated Task Scheduler repair stages bounded payloads; the unelevated launcher pins every namespace ancestor and payload with non-reparse handles that deny write/delete sharing until UAC processing exits. |
 
 `src/cli/provider.ts` accepts the Google-only `--google-tool-schema-policy` creation flag and rejects
 an unknown value or non-Google effective adapter before persistence. The persisted field and default

tests/windows/windows-elevation-spawn.test.ts

@@ -86,6 +86,7 @@ describe("elevated Task Scheduler payload staging", () => {
         expect(onDisk.equals(Buffer.from(value, "utf16le"))).toBe(true);
         expect(onDisk[0]).not.toBe(0xff);
         expect(payload.sha256).toBe(createHash("sha256").update(onDisk).digest("hex"));
+        expect(payload.byteLength).toBe(onDisk.length);
         expect(payload.sha256).toMatch(/^[0-9a-f]{64}$/);
       }
       expect(staged.xml.sha256).not.toBe(staged.expectedExisting!.sha256);
@@ -300,15 +301,15 @@ describe("runWindowsElevated spawn contract", () => {
 
     await expect(runWindowsElevatedScheduledTaskRegistration(
       "opencodex-proxy",
-      { path: "C:\\Temp\\opencodex-service-stage-aaaaaa\\register.xml", sha256: "0".repeat(64) },
+      { path: "C:\\Temp\\opencodex-service-stage-aaaaaa\\register.xml", byteLength: 42, sha256: "0".repeat(64) },
     )).resolves.toBe(0);
 
     const startProcessIndex = commandScript.indexOf("Start-Process");
     const filePathIndex = commandScript.indexOf(" -FilePath ");
     const argumentListIndex = commandScript.indexOf(" -ArgumentList ");
     const verbIndex = commandScript.indexOf(" -Verb RunAs ");
     const waitIndex = commandScript.indexOf(" -Wait");
-    const firstTerminator = commandScript.indexOf(";");
+    const firstTerminator = commandScript.indexOf(";", startProcessIndex);
 
     expect(startProcessIndex).toBeGreaterThanOrEqual(0);
     expect(filePathIndex).toBeGreaterThan(startProcessIndex);
@@ -321,7 +322,7 @@ describe("runWindowsElevated spawn contract", () => {
     expect(commandScript).not.toMatch(/-ArgumentList\s+'[^']*';\s+-Verb RunAs/);
   });
 
-  test("scheduled-task registration passes staged paths and digests, never inline payloads", async () => {
+  test("scheduled-task registration locks staged paths before elevation and bounds reads", async () => {
     let commandScript = "";
     setWindowsElevationSpawnForTests(((
       _cmd: string,
@@ -344,7 +345,7 @@ describe("runWindowsElevated spawn contract", () => {
 
     const xml = "<Task><Description>fixed-definition</Description></Task>";
     const stageDir = "C:\\Temp\\opencodex-service-stage-aaaaaa";
-    const staged = { path: stageDir + "\\register.xml", sha256: "a".repeat(64) };
+    const staged = { path: stageDir + "\\register.xml", byteLength: 108, sha256: "a".repeat(64) };
     await expect(runWindowsElevatedScheduledTaskRegistration("opencodex-proxy", staged)).resolves.toBe(0);
     const match = /-EncodedCommand ([A-Za-z0-9+/=]+)/.exec(commandScript);
     expect(match).not.toBeNull();
@@ -367,7 +368,12 @@ describe("runWindowsElevated spawn contract", () => {
     // then rereading would leave the swap window this check exists to close.
     expect(elevatedScript).toContain(staged.path);
     expect(elevatedScript).toContain(staged.sha256);
-    expect(elevatedScript).toContain("[IO.File]::ReadAllBytes($path)");
+    expect(commandScript.indexOf("Lock-OcxStage")).toBeLessThan(commandScript.indexOf("Start-Process"));
+    expect(commandScript).toContain("GetFileInformationByHandleEx");
+    expect(commandScript).toContain("0x00200000");
+    expect(commandScript).toContain("0x400");
+    expect(elevatedScript).toContain("$stream.Length -ne $expectedLength");
+    expect(elevatedScript).toContain("[byte[]]::new($expectedLength)");
     expect(elevatedScript).toContain("$sha.ComputeHash($bytes)");
     expect(elevatedScript).toContain("Task Scheduler staged payload failed its integrity check.");
     // #4692 follow-up: the one failure this staging design introduces has to be readable.
@@ -393,13 +399,13 @@ describe("runWindowsElevated spawn contract", () => {
     // pinned here is independence, not one lucky measurement: the same staging shape must
     // produce the same command length no matter how large the definition behind it is.
     const smallLength = commandScript.length;
-    const largeStaged = { path: stageDir + "\\register.xml", sha256: "b".repeat(64) };
+    const largeStaged = { path: stageDir + "\\register.xml", byteLength: 20_000, sha256: "b".repeat(64) };
     await expect(runWindowsElevatedScheduledTaskRegistration("opencodex-proxy", largeStaged)).resolves.toBe(0);
-    expect(commandScript.length).toBe(smallLength);
+    expect(commandScript.length).toBeLessThanOrEqual(smallLength + 8);
     expect(commandScript.length).toBeLessThan(8192);
 
     const predecessor = "<Task><Description>captured-predecessor</Description></Task>";
-    const stagedPredecessor = { path: stageDir + "\\expected.xml", sha256: "c".repeat(64) };
+    const stagedPredecessor = { path: stageDir + "\\expected.xml", byteLength: 126, sha256: "c".repeat(64) };
     await expect(
       runWindowsElevatedScheduledTaskRegistration("opencodex-proxy", staged, true, stagedPredecessor),
     ).resolves.toBe(0);
@@ -427,11 +433,34 @@ describe("runWindowsElevated spawn contract", () => {
     // overwriting a registration somebody else changed while the prompt was open.
     expect(() => runWindowsElevatedScheduledTaskRegistration(
       "opencodex-proxy",
-      { path: "C:\\Temp\\opencodex-service-stage-aaaaaa\\register.xml", sha256: "a".repeat(64) },
+      { path: "C:\\Temp\\opencodex-service-stage-aaaaaa\\register.xml", byteLength: 42, sha256: "a".repeat(64) },
       true,
     )).toThrow("requires a captured existing definition");
   });
 
+  test("refuses a staged payload whose path escapes the pinned directory", () => {
+    const stageDir = "C:\\Temp\\opencodex-service-stage-aaaaaa";
+    const staged = { path: `${stageDir}\\register.xml`, byteLength: 42, sha256: "a".repeat(64) };
+    const digest = "c".repeat(64);
+    // `..` slips past a startsWith prefix check but resolves outside the pinned
+    // directory — on either payload, and with either separator.
+    for (const escaped of [
+      `${stageDir}\\..\\elsewhere\\expected.xml`,
+      `${stageDir}/../elsewhere/expected.xml`,
+    ]) {
+      expect(() => runWindowsElevatedScheduledTaskRegistration(
+        "opencodex-proxy",
+        staged,
+        true,
+        { path: escaped, byteLength: 42, sha256: digest },
+      )).toThrow("must share one staging directory");
+    }
+    expect(() => runWindowsElevatedScheduledTaskRegistration(
+      "opencodex-proxy",
+      { path: `${stageDir}\\sub\\..\\..\\register.xml`, byteLength: 42, sha256: "a".repeat(64) },
+    )).toThrow("must share one staging directory");
+  });
+
   test("maps exit 1223 to cancelled", async () => {
     fakeChild({ code: OCX_ELEVATED_UAC_CANCELLED });
     await expect(runWindowsElevated("schtasks.exe", ["/create"])).rejects.toMatchObject({