fix(config): batch Windows migration ACL copies

src/cli/migrate-config.ts

@@ -7,7 +7,7 @@ import { planConfigMigration, type ConfigMigrationPlan } from "../config/migrate
 import { validateConfig } from "../config/validate-config.js";
 import { MiftahError } from "../utils/errors.js";
 import {
-  copyWindowsConfigSecurityDescriptor,
+  copyWindowsConfigSecurityDescriptors,
   createWindowsPrivateMigrationDirectory,
   secureWindowsConfigFile
 } from "./windows-config-acl.js";
@@ -61,6 +61,19 @@ class MigrationTransactionError extends Error {
   }
 }
 
+class MigrationFilePreparationError extends Error {
+  readonly code: string | undefined;
+
+  constructor(
+    readonly phase: "backup" | "candidate" | "descriptor",
+    cause: unknown
+  ) {
+    super("The migration files could not be prepared.");
+    this.name = "MigrationFilePreparationError";
+    this.code = errorCode(cause);
+  }
+}
+
 function errorCode(error: unknown): string | undefined {
   return typeof error === "object" && error !== null && "code" in error && typeof error.code === "string"
     ? error.code
@@ -180,13 +193,16 @@ async function writeSyncedExclusive(
   path: string,
   content: string | Uint8Array,
   mode: number,
-  beforeWrite?: (path: string) => Promise<void>
+  beforeWrite?: (path: string) => Promise<void>,
+  preopenedHandle?: Awaited<ReturnType<typeof open>>
 ): Promise<void> {
-  let handle: Awaited<ReturnType<typeof open>> | undefined;
-  let created = false;
+  let handle = preopenedHandle;
+  let created = preopenedHandle !== undefined;
   try {
-    handle = await open(path, "wx", mode);
-    created = true;
+    if (handle === undefined) {
+      handle = await open(path, "wx", mode);
+      created = true;
+    }
     if (beforeWrite !== undefined) await beforeWrite(path);
     await handle.writeFile(content);
     await handle.sync();
@@ -218,17 +234,46 @@ export async function writeNewConfigFile(path: string, content: string): Promise
   });
 }
 
-async function writeMigrationFile(
-  path: string,
-  content: string | Uint8Array,
+interface PreparedMigrationFiles {
+  readonly backupHandle: Awaited<ReturnType<typeof open>>;
+  readonly candidateHandle: Awaited<ReturnType<typeof open>>;
+}
+
+/**
+ * Creates both migration targets exclusively before one descriptor copy. The
+ * files remain empty until the descriptor copy has been verified.
+ */
+async function prepareMigrationFiles(
+  transaction: MigrationTransaction,
   mode: number,
   sourcePath: string
-): Promise<void> {
-  await writeSyncedExclusive(path, content, mode, async (targetPath) => {
-    if (!(await copyWindowsConfigSecurityDescriptor(sourcePath, targetPath))) {
+): Promise<PreparedMigrationFiles> {
+  let backupHandle: Awaited<ReturnType<typeof open>> | undefined;
+  let candidateHandle: Awaited<ReturnType<typeof open>> | undefined;
+  let backupCreated = false;
+  let candidateCreated = false;
+  let phase: MigrationFilePreparationError["phase"] = "backup";
+  try {
+    backupHandle = await open(transaction.backupPath, "wx", mode);
+    backupCreated = true;
+    phase = "candidate";
+    candidateHandle = await open(transaction.candidatePath, "wx", mode);
+    candidateCreated = true;
+    phase = "descriptor";
+    if (!(await copyWindowsConfigSecurityDescriptors(sourcePath, [transaction.backupPath, transaction.candidatePath]))) {
       throw migrationWriteError("could not preserve and verify the source Windows security descriptor");
     }
-  });
+    return { backupHandle, candidateHandle };
+  } catch (error) {
+    const cleanup: Promise<void>[] = [];
+    if (candidateCreated) cleanup.push(closeAndRemove(candidateHandle, transaction.candidatePath));
+    if (backupCreated) cleanup.push(closeAndRemove(backupHandle, transaction.backupPath));
+    const cleanupResults = await Promise.allSettled(cleanup);
+    if (cleanupResults.some((result) => result.status === "rejected")) {
+      throw migrationWriteError("could not clean up an incomplete migration file");
+    }
+    throw new MigrationFilePreparationError(phase, error);
+  }
 }
 
 interface MigrationTransaction {
@@ -411,8 +456,16 @@ async function installWithoutOverwriting(
     await restoreSourceChangedOrEscalate(transaction, path);
   }
 
+  let preparedFiles: PreparedMigrationFiles | undefined;
   try {
-    await writeMigrationFile(transaction.backupPath, source.originalBytes, source.fingerprint.mode, transaction.holdingPath);
+    preparedFiles = await prepareMigrationFiles(transaction, source.fingerprint.mode, transaction.holdingPath);
+    await writeSyncedExclusive(
+      transaction.backupPath,
+      source.originalBytes,
+      source.fingerprint.mode,
+      undefined,
+      preparedFiles.backupHandle
+    );
     await link(transaction.backupPath, publishedBackupPath);
     const [privateBackup, publishedBackup] = await Promise.all([
       lstat(transaction.backupPath, { bigint: true }),
@@ -422,6 +475,31 @@ async function installWithoutOverwriting(
       throw new Error("migration backup publication did not retain the private backup file");
     }
   } catch (error) {
+    if (preparedFiles !== undefined) {
+      try {
+        await closeAndRemove(preparedFiles.candidateHandle, transaction.candidatePath);
+      } catch {
+        await restoreAndEscalate(
+          transaction,
+          () => restoreHeldSource(transaction, path, heldFingerprint),
+          migrationWriteError("could not clean up an incomplete migration file")
+        );
+      }
+    }
+    if (error instanceof MigrationFilePreparationError && error.phase === "candidate") {
+      await restoreAndEscalate(
+        transaction,
+        () => restoreHeldSource(transaction, path, heldFingerprint),
+        migrationWriteError("could not create the synced migration candidate; the original configuration was restored")
+      );
+    }
+    if (error instanceof MigrationFilePreparationError && error.phase === "descriptor") {
+      await restoreAndEscalate(
+        transaction,
+        () => restoreHeldSource(transaction, path, heldFingerprint),
+        migrationWriteError("could not preserve and verify the source Windows security descriptor; the original configuration was restored")
+      );
+    }
     const restoredError = errorCode(error) === "EEXIST"
       ? new MiftahError(
         "CONFIG_MIGRATION_BACKUP_EXISTS",
@@ -432,7 +510,16 @@ async function installWithoutOverwriting(
   }
 
   try {
-    await writeMigrationFile(transaction.candidatePath, candidateContent, source.fingerprint.mode, transaction.holdingPath);
+    if (preparedFiles === undefined) {
+      throw new Error("migration files were not prepared");
+    }
+    await writeSyncedExclusive(
+      transaction.candidatePath,
+      candidateContent,
+      source.fingerprint.mode,
+      undefined,
+      preparedFiles.candidateHandle
+    );
   } catch {
     await restoreAndEscalate(
       transaction,

src/cli/windows-config-acl.ts

@@ -16,6 +16,12 @@ interface CopyFileSecurityRequest {
   readonly target: string;
 }
 
+interface CopyFileSecurityBatchRequest {
+  readonly operation: "copy-file-security-batch";
+  readonly source: string;
+  readonly targets: readonly [string, string];
+}
+
 interface CreatePrivateDirectoryRequest {
   readonly operation: "create-private-directory";
   readonly directory: string;
@@ -34,6 +40,7 @@ interface VerifyPrivatePathRequest {
 
 type WindowsConfigAclRequest =
   | CopyFileSecurityRequest
+  | CopyFileSecurityBatchRequest
   | CreatePrivateDirectoryRequest
   | SecurePrivateFileRequest
   | VerifyPrivatePathRequest;
@@ -46,6 +53,17 @@ export async function copyWindowsConfigSecurityDescriptor(source: string, target
   return runWindowsAclRequest({ operation: "copy-file-security", source, target });
 }
 
+/**
+ * Copies and verifies the source file's non-null owner, group, and DACL onto
+ * a fixed pair of exclusively created migration files in one trusted helper.
+ */
+export async function copyWindowsConfigSecurityDescriptors(
+  source: string,
+  targets: readonly [string, string]
+): Promise<boolean> {
+  return runWindowsAclRequest({ operation: "copy-file-security-batch", source, targets });
+}
+
 /** Creates a current-user-only directory with its DACL applied at creation time. */
 export async function createWindowsPrivateDirectory(directory: string): Promise<boolean> {
   return runWindowsAclRequest({ operation: "create-private-directory", directory });
@@ -86,13 +104,18 @@ function requestHasNul(request: WindowsConfigAclRequest): boolean {
   if (request.operation === "copy-file-security") {
     return request.source.includes("\u0000") || request.target.includes("\u0000");
   }
+  if (request.operation === "copy-file-security-batch") {
+    return request.source.includes("\u0000") || request.targets.some((target) => target.includes("\u0000"));
+  }
   if (request.operation === "create-private-directory") return request.directory.includes("\u0000");
   return request.path.includes("\u0000");
 }
 
 function encodeRequest(request: WindowsConfigAclRequest): string | undefined {
   const fields = request.operation === "copy-file-security"
     ? [request.operation, request.source, request.target]
+    : request.operation === "copy-file-security-batch"
+      ? [request.operation, request.source, ...request.targets]
     : request.operation === "create-private-directory"
       ? [request.operation, request.directory]
       : request.operation === "secure-private-file"
@@ -251,30 +274,36 @@ try {
   $fields = [Text.Encoding]::UTF8.GetString([Convert]::FromBase64String($encoded)).Split([char]0)
   [Environment]::SetEnvironmentVariable($requestName, $null, [EnvironmentVariableTarget]::Process)
 
-  if ($fields.Count -eq 3 -and $fields[0] -eq 'copy-file-security') {
+  if (
+    ($fields.Count -eq 3 -and $fields[0] -eq 'copy-file-security') -or
+    ($fields.Count -eq 4 -and $fields[0] -eq 'copy-file-security-batch')
+  ) {
     $sourceAcl = [System.IO.File]::GetAccessControl($fields[1], $accessSections)
     $sourceDescriptor = $sourceAcl.GetSecurityDescriptorBinaryForm()
     $sourceRaw = [System.Security.AccessControl.RawSecurityDescriptor]::new($sourceDescriptor, 0)
     if ($null -eq $sourceRaw.DiscretionaryAcl) { exit 1 }
-    $targetAcl = [System.IO.File]::GetAccessControl($fields[2], $accessSections)
-    $targetAcl.SetSecurityDescriptorBinaryForm($sourceDescriptor, $accessSections)
-    [System.IO.File]::SetAccessControl($fields[2], $targetAcl)
-    $verifiedAcl = [System.IO.File]::GetAccessControl($fields[2], $accessSections)
-    if ($null -eq $verifiedAcl) { exit 1 }
     $sourceRules = @($sourceAcl.GetAccessRules($true, $true, [System.Security.Principal.SecurityIdentifier]))
-    $verifiedRules = @($verifiedAcl.GetAccessRules($true, $true, [System.Security.Principal.SecurityIdentifier]))
-    if ($sourceRules.Count -ne $verifiedRules.Count) { exit 1 }
-    for ($index = 0; $index -lt $sourceRules.Count; $index++) {
-      $sourceRule = $sourceRules[$index]
-      $verifiedRule = $verifiedRules[$index]
-      if (
-        $sourceRule.IdentityReference.Value -cne $verifiedRule.IdentityReference.Value -or
-        ([int]$sourceRule.FileSystemRights) -ne ([int]$verifiedRule.FileSystemRights) -or
-        ([int]$sourceRule.AccessControlType) -ne ([int]$verifiedRule.AccessControlType) -or
-        $sourceRule.IsInherited -ne $verifiedRule.IsInherited -or
-        ([int]$sourceRule.InheritanceFlags) -ne ([int]$verifiedRule.InheritanceFlags) -or
-        ([int]$sourceRule.PropagationFlags) -ne ([int]$verifiedRule.PropagationFlags)
-      ) { exit 1 }
+    $targets = if ($fields[0] -eq 'copy-file-security') { @($fields[2]) } else { @($fields[2], $fields[3]) }
+    foreach ($target in $targets) {
+      $targetAcl = [System.IO.File]::GetAccessControl($target, $accessSections)
+      $targetAcl.SetSecurityDescriptorBinaryForm($sourceDescriptor, $accessSections)
+      [System.IO.File]::SetAccessControl($target, $targetAcl)
+      $verifiedAcl = [System.IO.File]::GetAccessControl($target, $accessSections)
+      if ($null -eq $verifiedAcl) { exit 1 }
+      $verifiedRules = @($verifiedAcl.GetAccessRules($true, $true, [System.Security.Principal.SecurityIdentifier]))
+      if ($sourceRules.Count -ne $verifiedRules.Count) { exit 1 }
+      for ($index = 0; $index -lt $sourceRules.Count; $index++) {
+        $sourceRule = $sourceRules[$index]
+        $verifiedRule = $verifiedRules[$index]
+        if (
+          $sourceRule.IdentityReference.Value -cne $verifiedRule.IdentityReference.Value -or
+          ([int]$sourceRule.FileSystemRights) -ne ([int]$verifiedRule.FileSystemRights) -or
+          ([int]$sourceRule.AccessControlType) -ne ([int]$verifiedRule.AccessControlType) -or
+          $sourceRule.IsInherited -ne $verifiedRule.IsInherited -or
+          ([int]$sourceRule.InheritanceFlags) -ne ([int]$verifiedRule.InheritanceFlags) -or
+          ([int]$sourceRule.PropagationFlags) -ne ([int]$verifiedRule.PropagationFlags)
+        ) { exit 1 }
+      }
     }
     exit 0
   }

tests/config-migration.test.ts

@@ -9,6 +9,8 @@ const migrationRace = vi.hoisted(() => ({
   concurrentBytes: undefined as Buffer | undefined,
   sourceBytesBeforeMove: undefined as Buffer | undefined,
   sourceMtimeBeforeMove: undefined as Date | undefined,
+  failCandidateOpen: false,
+  failedCandidateOpen: false,
   failCandidateLink: false,
   failedCandidateLink: false,
   backupLinked: false,
@@ -47,6 +49,19 @@ vi.mock("node:fs/promises", async (importOriginal) => {
         isSymbolicLink: stats.isSymbolicLink.bind(stats)
       });
     },
+    open: async (...args: Parameters<typeof actual.open>) => {
+      const [path, flags] = args;
+      if (
+        migrationRace.failCandidateOpen &&
+        typeof path === "string" &&
+        path.endsWith("candidate.miftah-migrate.tmp") &&
+        flags === "wx"
+      ) {
+        migrationRace.failedCandidateOpen = true;
+        throw Object.assign(new Error("simulated candidate setup collision"), { code: "EEXIST" });
+      }
+      return actual.open(...args);
+    },
     rename: async (...args: Parameters<typeof actual.rename>): Promise<void> => {
       const [from, to] = args;
       const isSourceMove =
@@ -294,6 +309,8 @@ afterEach(async () => {
   migrationRace.concurrentBytes = undefined;
   migrationRace.sourceBytesBeforeMove = undefined;
   migrationRace.sourceMtimeBeforeMove = undefined;
+  migrationRace.failCandidateOpen = false;
+  migrationRace.failedCandidateOpen = false;
   migrationRace.failCandidateLink = false;
   migrationRace.failedCandidateLink = false;
   migrationRace.backupLinked = false;
@@ -390,6 +407,25 @@ describe("migrate-config command", () => {
     expect(await readFile(`${configPath}.bak`, "utf8")).toBe("existing backup");
   });
 
+  it("reports a candidate setup collision without treating it as an existing published backup", async () => {
+    const directory = await mkdtemp(join(tmpdir(), "miftah-config-migration-"));
+    temporaryDirectories.push(directory);
+    const configPath = join(directory, "miftah.json");
+    const original = `${JSON.stringify(legacyConfig(), null, 2)}\n`;
+    await writeFile(configPath, original, "utf8");
+    migrationRace.failCandidateOpen = true;
+
+    await expect(runMigrateConfigCommand({ configPath, write: true })).rejects.toMatchObject({
+      code: "CONFIG_MIGRATION_WRITE_FAILED",
+      message: expect.stringContaining("synced migration candidate")
+    });
+
+    expect(migrationRace.failedCandidateOpen).toBe(true);
+    expect(await readFile(configPath, "utf8")).toBe(original);
+    await expect(access(`${configPath}.bak`)).rejects.toMatchObject({ code: "ENOENT" });
+    expect((await readdir(directory)).some((entry) => entry.startsWith(".miftah.json.miftah-migrate-"))).toBe(false);
+  });
+
   it("migrates a canonical v2 configuration to v3 without synthesizing OAuth state", async () => {
     const directory = await mkdtemp(join(tmpdir(), "miftah-config-migration-"));
     temporaryDirectories.push(directory);