refactor(filesystem): hash admitted target descriptors with the shared stable hasher (#10573)

BinaryTargetResolver kept a private copy of StableFileHash's descriptor read
loop because it must hash the same descriptor it classified, admitted with
symlinks rejected, while hashStableFile opens its own descriptor and follows
symlinks. The loop is now hashStableRegularFileDescriptor, which takes the
admitted descriptor and the caller's symlink setting; readStableFile and the
resolver both use it. The resolver keeps symlinks: reject and maps a change
to its existing BinaryTargetError and cancellation to AnalysisCancelledError.

src/application/BinaryTargetResolver.ts

@@ -25,7 +25,6 @@ import { ArtifactReaderFailure } from "../artifacts/ArtifactReader.js";
 import { jsonValueSchema } from "../domain/jsonValue.js";
 import { childFailureDiagnostics } from "../domain/analysisErrorProjection.js";
 import { constants, type Stats } from "node:fs";
-import { createHash } from "node:crypto";
 import { access, realpath, stat, type FileHandle } from "node:fs/promises";
 import { extname } from "node:path";
 
@@ -35,6 +34,7 @@ import {
   RegularFileChangedError,
   type StableRegularFileDescriptor,
 } from "../filesystem/RegularFile.js";
+import { hashStableRegularFileDescriptor } from "../filesystem/StableFileHash.js";
 import { AnalysisCancelledError } from "../domain/analysisErrorCore.js";
 import type {
   BinaryTarget,
@@ -326,7 +326,7 @@ const readOpenedBinaryTarget = async (
     path,
     sourcePath,
     ...bundle,
-    sha256: await sha256Handle(handle, path, metadata, signal),
+    sha256: await sha256Handle(source, path, signal),
   });
   const hostArchitecture = options.hostArchitecture ?? process.arch;
   const { targetKind, formatHint, signal, architecture, entryPoint } = options;
@@ -971,36 +971,20 @@ const readLeadingBytes = async (
   return bytes;
 };
 
+/** The admitted target's SHA-256 from the descriptor that was classified. */
 const sha256Handle = async (
-  handle: FileHandle,
+  source: StableRegularFileDescriptor,
   path: string,
-  initial: Stats,
   signal?: AbortSignal,
 ): Promise<string> => {
-  const hash = createHash("sha256");
-  const chunk = Buffer.allocUnsafe(64 * 1024);
-  let position = 0;
-  while (true) {
-    throwIfTargetResolutionCancelled(signal);
-    const observed = await handle.read(
-      chunk,
-      0,
-      Math.min(chunk.length, initial.size - position + 1),
-      position,
-    );
-    throwIfTargetResolutionCancelled(signal);
-    if (observed.bytesRead === 0) break;
-    hash.update(chunk.subarray(0, observed.bytesRead));
-    position += observed.bytesRead;
-    if (position > initial.size)
-      throw new BinaryTargetError(path, "target grew while being read");
-  }
-  throwIfTargetResolutionCancelled(signal);
   try {
-    await assertStableRegularFileIdentity(handle, path, initial, {
-      symlinks: "reject",
-      ...(signal === undefined ? {} : { signal }),
-    });
+    return (
+      await hashStableRegularFileDescriptor(source, path, {
+        // Target admission refuses a symbolic-link final component.
+        symlinks: "reject",
+        ...(signal === undefined ? {} : { signal }),
+      })
+    ).sha256;
   } catch (cause: unknown) {
     throwIfTargetResolutionCancelled(signal);
     if (cause instanceof RegularFileChangedError)
@@ -1009,10 +993,6 @@ const sha256Handle = async (
       });
     throw cause;
   }
-  throwIfTargetResolutionCancelled(signal);
-  if (position !== initial.size)
-    throw new BinaryTargetError(path, "target changed while being read");
-  return hash.digest("hex");
 };
 
 const resolveAppBundle = async (

src/filesystem/StableFileHash.ts

@@ -12,6 +12,7 @@ import {
   assertStableRegularFileIdentity,
   regularFileState,
   type RegularFileState,
+  type StableRegularFileDescriptor,
 } from "./RegularFile.js";
 import type { OwnedFileHandle } from "./OwnedFileHandle.js";
 
@@ -84,54 +85,20 @@ async function readStableFile(
   includePrefix: boolean,
 ): Promise<StableFileFingerprint | StableFileObservation> {
   try {
-    return await withOwnedRegularFile(
-      path,
-      options,
-      async ({ handle, initial }) => {
-        if (!Number.isSafeInteger(initial.size) || initial.size < 0)
-          throw new RangeError(
-            `Regular file size cannot be represented safely: ${path}`,
-          );
-        const hash = createHash("sha256");
-        const chunk = Buffer.allocUnsafe(64 * 1024);
-        const prefix = Buffer.alloc(
-          includePrefix ? Math.min(STABLE_FILE_PREFIX_BYTES, initial.size) : 0,
-        );
-        let position = 0;
-        while (position <= initial.size) {
-          assertRegularFileActive(options.signal);
-          const length = Math.min(chunk.length, initial.size - position + 1);
-          if (length <= 0) break;
-          const read = await handle.read(chunk, 0, length, position);
-          if (
-            position + read.bytesRead > initial.size ||
-            (read.bytesRead === 0 && position < initial.size)
-          )
-            throw new RegularFileChangedError(path);
-          assertRegularFileActive(options.signal);
-          if (read.bytesRead === 0) break;
-          if (position < prefix.length) {
-            const prefixBytes = Math.min(
-              read.bytesRead,
-              prefix.length - position,
-            );
-            chunk.copy(prefix, position, 0, prefixBytes);
-          }
-          hash.update(chunk.subarray(0, read.bytesRead));
-          position += read.bytesRead;
-        }
-        assertRegularFileActive(options.signal);
-        if (position !== initial.size) throw new RegularFileChangedError(path);
-        await assertStableRegularFileIdentity(handle, path, initial, {
+    return await withOwnedRegularFile(path, options, async (source) => {
+      const { prefix, ...fingerprint } = await hashStableRegularFileDescriptor(
+        source,
+        path,
+        {
           symlinks: "follow",
+          prefixBytes: includePrefix ? STABLE_FILE_PREFIX_BYTES : 0,
           ...(options.signal === undefined ? {} : { signal: options.signal }),
-        });
-        const fingerprint = { sha256: hash.digest("hex"), bytes: position };
-        return includePrefix
-          ? { ...fingerprint, prefix, state: regularFileState(initial) }
-          : fingerprint;
-      },
-    );
+        },
+      );
+      return includePrefix
+        ? { ...fingerprint, prefix, state: regularFileState(source.initial) }
+        : fingerprint;
+    });
   } catch (cause: unknown) {
     if (
       cause instanceof RegularFileCleanupFailure ||
@@ -142,3 +109,56 @@ async function readStableFile(
     throw new StableFileHashFailure(path, cause);
   }
 }
+
+/**
+ * Hash an admitted descriptor from offset 0 to its admitted size, then verify
+ * that `path` still names the same unchanged file. `symlinks` must match the
+ * admission that opened the descriptor. The first `prefixBytes` bytes are
+ * captured from the same read. A file that changes, grows or shrinks raises
+ * `RegularFileChangedError`; an aborted signal raises
+ * `RegularFileOperationCancelled`.
+ */
+export const hashStableRegularFileDescriptor = async (
+  { handle, initial }: StableRegularFileDescriptor,
+  path: string,
+  options: {
+    readonly symlinks: "follow" | "reject";
+    readonly signal?: AbortSignal;
+    readonly prefixBytes?: number;
+  },
+): Promise<StableFileFingerprint & { readonly prefix: Buffer }> => {
+  if (!Number.isSafeInteger(initial.size) || initial.size < 0)
+    throw new RangeError(
+      `Regular file size cannot be represented safely: ${path}`,
+    );
+  const hash = createHash("sha256");
+  const chunk = Buffer.allocUnsafe(64 * 1024);
+  const prefix = Buffer.alloc(Math.min(options.prefixBytes ?? 0, initial.size));
+  let position = 0;
+  while (position <= initial.size) {
+    assertRegularFileActive(options.signal);
+    const length = Math.min(chunk.length, initial.size - position + 1);
+    if (length <= 0) break;
+    const read = await handle.read(chunk, 0, length, position);
+    if (
+      position + read.bytesRead > initial.size ||
+      (read.bytesRead === 0 && position < initial.size)
+    )
+      throw new RegularFileChangedError(path);
+    assertRegularFileActive(options.signal);
+    if (read.bytesRead === 0) break;
+    if (position < prefix.length) {
+      const prefixBytes = Math.min(read.bytesRead, prefix.length - position);
+      chunk.copy(prefix, position, 0, prefixBytes);
+    }
+    hash.update(chunk.subarray(0, read.bytesRead));
+    position += read.bytesRead;
+  }
+  assertRegularFileActive(options.signal);
+  if (position !== initial.size) throw new RegularFileChangedError(path);
+  await assertStableRegularFileIdentity(handle, path, initial, {
+    symlinks: options.symlinks,
+    ...(options.signal === undefined ? {} : { signal: options.signal }),
+  });
+  return { sha256: hash.digest("hex"), bytes: position, prefix };
+};

tests/boundary/filesystem/stableFileDescriptorHash.test.ts

@@ -0,0 +1,80 @@
+import { createHash } from "node:crypto";
+import { appendFile, symlink, truncate, writeFile } from "node:fs/promises";
+import { join } from "node:path";
+import { describe, expect, it } from "vitest";
+
+import { createTestTempDirectory } from "../../fixtures/temporaryDirectory.js";
+import {
+  RegularFileChangedError,
+  openRegularFile,
+} from "../../../src/filesystem/RegularFile.js";
+import { hashStableRegularFileDescriptor } from "../../../src/filesystem/StableFileHash.js";
+
+const admitted = async (contents: string) => {
+  const directory = await createTestTempDirectory("rea-stable-descriptor-");
+  const path = join(directory, "target.bin");
+  await writeFile(path, contents);
+  const source = await openRegularFile(path, { symlinks: "reject" });
+  return { directory, path, source };
+};
+
+describe("hashStableRegularFileDescriptor", () => {
+  it("hashes the admitted bytes and captures the requested prefix", async () => {
+    const { path, source } = await admitted("abcdef");
+    try {
+      expect(
+        await hashStableRegularFileDescriptor(source, path, {
+          symlinks: "reject",
+          prefixBytes: 2,
+        }),
+      ).toEqual({
+        sha256: createHash("sha256").update("abcdef").digest("hex"),
+        bytes: 6,
+        prefix: Buffer.from("ab"),
+      });
+    } finally {
+      await source.handle.close();
+    }
+  });
+
+  it.each([
+    ["grows", (path: string) => appendFile(path, "more")],
+    ["shrinks", (path: string) => truncate(path, 2)],
+  ])("refuses a file that %s after admission", async (_change, mutate) => {
+    const { path, source } = await admitted("abcdef");
+    try {
+      await mutate(path);
+      await expect(
+        hashStableRegularFileDescriptor(source, path, {
+          symlinks: "reject",
+        }),
+      ).rejects.toBeInstanceOf(RegularFileChangedError);
+    } finally {
+      await source.handle.close();
+    }
+  });
+
+  // Target admission refuses a symbolic-link final component, while generic
+  // hashing follows one; the descriptor owner applies the caller's setting.
+  it.skipIf(process.platform === "win32")(
+    "applies the caller's symlink setting to a path that names a link",
+    async () => {
+      const admittedFile = await admitted("abcdef");
+      await admittedFile.source.handle.close();
+      const { directory, path } = admittedFile;
+      const link = join(directory, "link.bin");
+      await symlink(path, link);
+      const source = await openRegularFile(link, { symlinks: "follow" });
+      try {
+        await expect(
+          hashStableRegularFileDescriptor(source, link, { symlinks: "follow" }),
+        ).resolves.toMatchObject({ bytes: 6 });
+        await expect(
+          hashStableRegularFileDescriptor(source, link, { symlinks: "reject" }),
+        ).rejects.toBeInstanceOf(RegularFileChangedError);
+      } finally {
+        await source.handle.close();
+      }
+    },
+  );
+});