From a09d3bed944728bf21bd343a8992c18ab6e682d9 Mon Sep 17 00:00:00 2001 From: morluto <76467478+morluto@users.noreply.github.com> Date: Sun, 15 Nov 2026 03:37:30 +0800 Subject: [PATCH] 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 | 42 ++----- src/filesystem/StableFileHash.ts | 112 +++++++++++------- .../stableFileDescriptorHash.test.ts | 80 +++++++++++++ 3 files changed, 157 insertions(+), 77 deletions(-) create mode 100644 tests/boundary/filesystem/stableFileDescriptorHash.test.ts diff --git a/src/application/BinaryTargetResolver.ts b/src/application/BinaryTargetResolver.ts index 522adcca87..bb4787004d 100644 --- a/src/application/BinaryTargetResolver.ts +++ b/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 => { - 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 ( diff --git a/src/filesystem/StableFileHash.ts b/src/filesystem/StableFileHash.ts index 1fa92d4c7a..58f537d7b5 100644 --- a/src/filesystem/StableFileHash.ts +++ b/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 { 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 => { + 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 }; +}; diff --git a/tests/boundary/filesystem/stableFileDescriptorHash.test.ts b/tests/boundary/filesystem/stableFileDescriptorHash.test.ts new file mode 100644 index 0000000000..e8e6f42dac --- /dev/null +++ b/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(); + } + }, + ); +});