fix: close coverage evidence races

scripts/lib/risk-coverage-files.ts

@@ -35,6 +35,18 @@ type WriterDependencies = Readonly<{
   fileSystem?: RiskCoverageArtifactFileSystem;
 }>;
 
+type ReadableHandle = Readonly<{
+  stat(): Promise<Stats>;
+  readFile(encoding: "utf8"): Promise<string>;
+  close(): Promise<unknown>;
+}>;
+
+type InputDependencies = Readonly<{
+  lstatPath?: (target: string) => Promise<Stats>;
+  realpathPath?: (target: string) => Promise<string>;
+  openFile?: (target: string, flags: number) => Promise<ReadableHandle>;
+}>;
+
 const defaultFileSystem: RiskCoverageArtifactFileSystem = Object.freeze({
   openFile: async (target, flags, mode) => {
     const handle = await open(target, flags, mode);
@@ -74,6 +86,45 @@ function isWithin(root: string, target: string): boolean {
   );
 }
 
+function hasStableIdentity(metadata: Stats): boolean {
+  return (
+    Number.isSafeInteger(metadata.dev) &&
+    Number.isSafeInteger(metadata.ino) &&
+    (metadata.dev !== 0 || metadata.ino !== 0)
+  );
+}
+
+function sameFileIdentity(before: Stats, after: Stats): boolean {
+  return (
+    !hasStableIdentity(before) ||
+    !hasStableIdentity(after) ||
+    (before.dev === after.dev && before.ino === after.ino)
+  );
+}
+
+async function rejectSymlinkAncestors(
+  repositoryRoot: string,
+  relativePath: string,
+  label: string,
+  lstatPath: (target: string) => Promise<Stats>,
+): Promise<void> {
+  let current = repositoryRoot;
+  let currentRelative = "";
+  const directory = path.posix.dirname(relativePath);
+  if (directory === ".") return;
+  for (const segment of directory.split("/")) {
+    current = path.join(current, segment);
+    currentRelative = currentRelative ? `${currentRelative}/${segment}` : segment;
+    const metadata = await lstatPath(current);
+    if (metadata.isSymbolicLink()) {
+      throw new TypeError(`${label} ancestor is a symlink: ${currentRelative}`);
+    }
+    if (!metadata.isDirectory()) {
+      throw new TypeError(`${label} ancestor is not a directory: ${currentRelative}`);
+    }
+  }
+}
+
 function assertDirectory(metadata: Stats, relativePath: string): void {
   if (metadata.isSymbolicLink()) {
     throw new TypeError(`artifact ancestor is a symlink: ${relativePath}`);
@@ -87,27 +138,41 @@ export async function readRiskCoverageInput(input: Readonly<{
   repositoryRoot: string;
   relativePath: string;
   label: string;
-}>): Promise<Readonly<{ relativePath: string; absolutePath: string; text: string }>> {
+}>, dependencies: InputDependencies = {}): Promise<Readonly<{
+  relativePath: string;
+  absolutePath: string;
+  text: string;
+}>> {
+  const lstatPath = dependencies.lstatPath ?? lstat;
+  const realpathPath = dependencies.realpathPath ?? realpath;
+  const openFile = dependencies.openFile ??
+    ((target: string, flags: number) => open(target, flags));
   const relativePath = normalizeRepositoryRelativePath(
     input.relativePath,
     `${input.label} path`,
   );
   const repositoryRoot = path.resolve(input.repositoryRoot);
-  const repositoryRealpath = await realpath(repositoryRoot);
+  const repositoryRealpath = await realpathPath(repositoryRoot);
   const absolutePath = path.resolve(repositoryRoot, relativePath);
-  const metadata = await lstat(absolutePath);
+  await rejectSymlinkAncestors(
+    repositoryRoot,
+    relativePath,
+    input.label,
+    lstatPath,
+  );
+  const metadata = await lstatPath(absolutePath);
   if (metadata.isSymbolicLink()) {
     throw new TypeError(`${input.label} path is a symlink: ${relativePath}`);
   }
   if (!metadata.isFile()) {
     throw new TypeError(`${input.label} path is not a regular file: ${relativePath}`);
   }
-  const resolvedPath = await realpath(absolutePath);
+  const resolvedPath = await realpathPath(absolutePath);
   if (!isWithin(repositoryRealpath, resolvedPath)) {
     throw new TypeError(`${input.label} path is outside repository: ${relativePath}`);
   }
 
-  const handle = await open(
+  const handle = await openFile(
     absolutePath,
     constants.O_RDONLY | constants.O_NOFOLLOW,
   );
@@ -116,6 +181,9 @@ export async function readRiskCoverageInput(input: Readonly<{
     if (!openedMetadata.isFile()) {
       throw new TypeError(`${input.label} path is not a regular file: ${relativePath}`);
     }
+    if (!sameFileIdentity(metadata, openedMetadata)) {
+      throw new TypeError(`${input.label} path changed during validation: ${relativePath}`);
+    }
     const text = await handle.readFile("utf8");
     return Object.freeze({ relativePath, absolutePath, text });
   } finally {
@@ -170,17 +238,35 @@ export async function resolveRiskCoverageArtifactPath(input: Readonly<{
   }
 
   const absolutePath = path.resolve(repositoryRoot, relativePath);
+  let destinationMetadata: Stats | undefined;
   try {
-    const metadata = await lstat(absolutePath);
-    if (metadata.isSymbolicLink()) {
+    destinationMetadata = await lstat(absolutePath);
+    if (destinationMetadata.isSymbolicLink()) {
       throw new TypeError(`artifact path is a symlink: ${relativePath}`);
     }
-    if (!metadata.isFile()) {
+    if (!destinationMetadata.isFile()) {
       throw new TypeError(`artifact path is not a regular file: ${relativePath}`);
     }
   } catch (error) {
     if (!hasErrorCode(error, "ENOENT")) throw error;
   }
+  if (destinationMetadata) {
+    const destinationRealpath = await realpath(absolutePath);
+    for (const inputPath of normalizedInputs) {
+      const inputAbsolutePath = path.resolve(repositoryRoot, inputPath);
+      const inputRealpath = await realpath(inputAbsolutePath);
+      const inputMetadata = await lstat(inputAbsolutePath);
+      if (
+        inputRealpath === destinationRealpath ||
+        (hasStableIdentity(inputMetadata) &&
+          hasStableIdentity(destinationMetadata) &&
+          inputMetadata.dev === destinationMetadata.dev &&
+          inputMetadata.ino === destinationMetadata.ino)
+      ) {
+        throw new TypeError(`artifact path is the same file as an input: ${inputPath}`);
+      }
+    }
+  }
   return absolutePath;
 }
 
@@ -232,7 +318,14 @@ export async function writeRiskCoverageArtifactAtomic(
     ownsTemporaryFile = false;
     const directoryHandle = await fileSystem.openDirectory(path.dirname(destination));
     try {
-      await directoryHandle.sync();
+      try {
+        await directoryHandle.sync();
+      } catch (error) {
+        // Windows and some filesystems do not support fsync on directory handles.
+        if (!hasErrorCode(error, "EINVAL") && !hasErrorCode(error, "ENOTSUP")) {
+          throw error;
+        }
+      }
     } finally {
       await directoryHandle.close();
     }

scripts/lib/risk-coverage.ts

@@ -96,6 +96,8 @@ type InventoryOptions = Readonly<{
   openFile?: (target: string, flags: number) => Promise<ReadableFileHandle>;
 }>;
 
+class FileIdentityChangedError extends Error {}
+
 function isRecord(value: unknown): value is Record<string, unknown> {
   return Boolean(value) && typeof value === "object" && !Array.isArray(value);
 }
@@ -327,6 +329,22 @@ function isWithin(root: string, target: string): boolean {
   );
 }
 
+function hasStableIdentity(metadata: Stats): boolean {
+  return (
+    Number.isSafeInteger(metadata.dev) &&
+    Number.isSafeInteger(metadata.ino) &&
+    (metadata.dev !== 0 || metadata.ino !== 0)
+  );
+}
+
+function sameFileIdentity(before: Stats, after: Stats): boolean {
+  return (
+    !hasStableIdentity(before) ||
+    !hasStableIdentity(after) ||
+    (before.dev === after.dev && before.ino === after.ino)
+  );
+}
+
 export function isProductionModulePath(relativePath: string): boolean {
   return (
     /\.tsx?$/u.test(relativePath) &&
@@ -394,7 +412,16 @@ export async function buildProductionModuleInventory(
         if (!openedMetadata.isFile()) {
           throw new TypeError("opened target is not a regular file");
         }
+        if (!sameFileIdentity(metadata, openedMetadata)) {
+          throw new FileIdentityChangedError("opened file identity changed");
+        }
       } catch (error) {
+        if (error instanceof FileIdentityChangedError) {
+          throw new Error(
+            `production inventory file changed during validation: ${relativeTarget}`,
+            { cause: error },
+          );
+        }
         throw new Error(`production inventory file is unreadable: ${relativeTarget}`, {
           cause: error,
         });