diff --git a/src/adb/AdbInputSnapshot.ts b/src/adb/AdbInputSnapshot.ts index bc547f2504..b060b91ace 100644 --- a/src/adb/AdbInputSnapshot.ts +++ b/src/adb/AdbInputSnapshot.ts @@ -61,12 +61,13 @@ export const snapshotAdbInput = async ( let primaryCause: unknown; try { - source = new OwnedFileHandle( - await (dependencies.openSource ?? openRegularFile)(sourcePath, { - symlinks: "follow", - signal, - }), - ); + const { handle, initial } = await ( + dependencies.openSource ?? openRegularFile + )(sourcePath, { + symlinks: "follow", + signal, + }); + source = new OwnedFileHandle(handle); const destinationPath = join(root.path, basename(sourcePath)); destination = new OwnedFileHandle( await (dependencies.openDestination ?? open)( @@ -75,7 +76,6 @@ export const snapshotAdbInput = async ( 0o600, ), ); - const initial = await source.handle.stat(); if (!Number.isSafeInteger(initial.size) || initial.size < 0) throw new RangeError( `Regular file size cannot be represented safely: ${sourcePath}`, diff --git a/src/android/AndroidTargetSnapshot.ts b/src/android/AndroidTargetSnapshot.ts index 44e7e3547b..bd0af19aa9 100644 --- a/src/android/AndroidTargetSnapshot.ts +++ b/src/android/AndroidTargetSnapshot.ts @@ -19,6 +19,7 @@ import { RegularFileAdmissionFailure, assertRegularFileActive, sameRegularFileState, + type StableRegularFileDescriptor, } from "../filesystem/RegularFile.js"; import type { AndroidOperation } from "../domain/android/androidAnalysis.js"; @@ -173,9 +174,12 @@ const copyAndroidSnapshot = async ( // Close each descriptor through its owner. A rejected close keeps the // copy outcome and stays retained until that owner is retried; the // partial file is removed only after those close attempts. - const source = adoptSnapshotHandle( - await openSnapshotSource(sourcePath, signal, retain(sourcePath)), + const { handle, initial } = await openSnapshotSource( + sourcePath, + signal, + retain(sourcePath), ); + const source = adoptSnapshotHandle(handle); return await withOwnedFileHandle( sourcePath, source.owner, @@ -184,7 +188,6 @@ const copyAndroidSnapshot = async ( ...retain(sourcePath), }, async () => { - const initial = await source.handle.stat(); assertRegularFileActive(signal); let copiedBytes = 0; const hash = createHash("sha256"); @@ -286,7 +289,7 @@ const openSnapshotSource = async ( retention: { readonly retainFailedClose?: (owner: OwnedFileHandle) => void; }, -): Promise => { +): Promise => { try { return await openRegularFile(path, { symlinks: "follow", signal }); } catch (cause: unknown) { diff --git a/src/android/JadxConfiguration.ts b/src/android/JadxConfiguration.ts index 3ff117927a..ca16c71f6b 100644 --- a/src/android/JadxConfiguration.ts +++ b/src/android/JadxConfiguration.ts @@ -341,7 +341,7 @@ const requireEngineArchive = async ( ), }, options.signal, - async (handle) => { + async ({ handle, initial }) => { try { // Metadata enumeration borrows this descriptor. The file workflow // alone owns admission, final close, cancellation and failed retry. @@ -349,7 +349,7 @@ const requireEngineArchive = async ( jar, "zip", JAR_METADATA_READ_MAX_BYTES, - { handle, initial: await handle.stat() }, + { handle, initial }, ); const missing = new Set([ "com/atxx/jhmcp/JadxSession.class", diff --git a/src/apktool/ApktoolWorkspace.lifecycle.test.ts b/src/apktool/ApktoolWorkspace.lifecycle.test.ts index 69cbffa247..1376c65535 100644 --- a/src/apktool/ApktoolWorkspace.lifecycle.test.ts +++ b/src/apktool/ApktoolWorkspace.lifecycle.test.ts @@ -192,11 +192,17 @@ it.each([false, true])( await fs.writeFile(path, Buffer.from([0xff])); vi.mocked(fs.open).mockImplementation(async (name, flags, mode) => { const handle = await actual.open(name, flags, mode); + const read = handle.read.bind(handle); + let readComplete = false; + handle.read = (async (...args: Parameters) => { + const value = await read(...args); + if (value.bytesRead === 0) readComplete = true; + return value; + }) as typeof handle.read; const stat = handle.stat.bind(handle); - let calls = 0; handle.stat = ((...args: Parameters) => stat(...args).then((value) => { - if (++calls === 3) + if (readComplete) controller.abort({ caller: "after bytes before interpretation" }); return value; })) as typeof handle.stat; diff --git a/src/apktool/ApktoolWorkspace.ts b/src/apktool/ApktoolWorkspace.ts index 111e8c08e8..693876740f 100644 --- a/src/apktool/ApktoolWorkspace.ts +++ b/src/apktool/ApktoolWorkspace.ts @@ -68,9 +68,8 @@ export class ApktoolWorkspace { path, this.#files, signal, - async (source) => { + async ({ handle: source, initial }) => { admitted = true; - const initial = await source.stat(); const output = join(this.path, "input.apk"); const file = await open(output, "wx", 0o600); await withArtifactFileHandle( @@ -180,9 +179,8 @@ export class ApktoolWorkspace { path, this.#files, signal, - async (file) => { + async ({ handle: file, initial }) => { admitted = true; - const initial = await file.stat(); const bytes = Buffer.alloc(budget + 1); let length = 0; while (length < bytes.length) { diff --git a/src/application/BinaryTargetResolver.ts b/src/application/BinaryTargetResolver.ts index 4f9c1d875f..c7a9dd7c9c 100644 --- a/src/application/BinaryTargetResolver.ts +++ b/src/application/BinaryTargetResolver.ts @@ -26,7 +26,10 @@ import { import { extname } from "node:path"; import { BinaryTargetError } from "../domain/configurationErrors.js"; -import { sameRegularFileState } from "../filesystem/RegularFile.js"; +import { + sameRegularFileState, + type StableRegularFileDescriptor, +} from "../filesystem/RegularFile.js"; import { AnalysisCancelledError } from "../domain/analysisErrorCore.js"; import type { BinaryTarget } from "../domain/binaryTargetTypes.js"; import { err, ok, type Result } from "../domain/result.js"; @@ -153,10 +156,10 @@ export const parseBinaryTarget = async ( symlinks: "reject", ...(signal === undefined ? {} : { signal }), }, - async (handle) => { + async (source) => { try { return await readOpenedBinaryTarget( - handle, + source, { path, sourcePath, ...bundle }, options, ); @@ -232,7 +235,7 @@ type TargetResolution = Result< /** Classify and hash the one descriptor admitted by the lifetime owner. */ const readOpenedBinaryTarget = async ( - handle: FileHandle, + source: StableRegularFileDescriptor, selected: { readonly path: string; readonly sourcePath: string; @@ -240,13 +243,12 @@ const readOpenedBinaryTarget = async ( }, options: BinaryTargetResolverOptions, ): Promise => { + const { handle, initial: metadata } = source; const { path, sourcePath, bundleInfoPlist } = selected; const bundle = bundleInfoPlist === undefined ? {} : { bundleInfoPlist }; const hostArchitecture = options.hostArchitecture ?? process.arch; const { targetKind, formatHint, signal, architecture, entryPoint } = options; throwIfTargetResolutionCancelled(signal); - const metadata = await handle.stat(); - throwIfTargetResolutionCancelled(signal); if (formatHint === "raw-binary") { if (targetKind !== undefined && targetKind !== "executable") return err( @@ -366,7 +368,7 @@ const readOpenedBinaryTarget = async ( const artifactFormat = targetKind === "database" ? undefined - : await detectArtifactFormat(path, handle, signal); + : await detectArtifactFormat(path, source, signal); if ( targetKind === "database" || (targetKind === undefined && path.toLowerCase().endsWith(".hop")) @@ -389,7 +391,7 @@ const readOpenedBinaryTarget = async ( : ok({ ...identity, kind: "artifact", format: artifactFormat }); } const detected = await readExecutableMetadata( - handle, + source, hostArchitecture, signal, ); @@ -616,7 +618,7 @@ const throwIfTargetResolutionCancelled = (signal?: AbortSignal): void => { const detectArtifactFormat = async ( path: string, - handle: FileHandle, + source: StableRegularFileDescriptor, signal?: AbortSignal, ): Promise< | Exclude< @@ -633,6 +635,7 @@ const detectArtifactFormat = async ( > | undefined > => { + const { handle, initial } = source; throwIfTargetResolutionCancelled(signal); const lower = path.toLowerCase(); const magic = Buffer.alloc(4096); @@ -664,8 +667,7 @@ const detectArtifactFormat = async ( return "pkg"; if (lower.endsWith(".dmg")) { throwIfTargetResolutionCancelled(signal); - const size = (await handle.stat()).size; - throwIfTargetResolutionCancelled(signal); + const size = initial.size; if (size >= 512) { const trailer = Buffer.alloc(4); const read = await handle.read(trailer, 0, trailer.length, size - 512); @@ -696,7 +698,7 @@ const detectArtifactFormat = async ( if (xmlState === "matched") return "plist"; } if (bytesRead === MAX_XML_PLIST_PROBE_BYTES) { - const { size } = await handle.stat(); + const { size } = initial; throwIfTargetResolutionCancelled(signal); if (size > bytesRead) throw new BinaryTargetError( @@ -837,10 +839,11 @@ const resolveAppBundle = async ( }; const readExecutableMetadata = async ( - handle: FileHandle, + source: StableRegularFileDescriptor, hostArchitecture: NodeJS.Architecture, signal?: AbortSignal, ): Promise> => { + const { handle, initial } = source; throwIfTargetResolutionCancelled(signal); const prefix = Buffer.alloc(4096); const prefixRead = await handle.read(prefix, 0, prefix.length, 0); @@ -848,8 +851,7 @@ const readExecutableMetadata = async ( const bytes = prefix.subarray(0, prefixRead.bytesRead); // Header commitments such as Mach-O load commands and FAT slices are // checked against the whole file, not the probed prefix. - const fileSize = (await handle.stat()).size; - throwIfTargetResolutionCancelled(signal); + const fileSize = initial.size; if (bytes.length >= 2 && bytes[0] === 0x4d && bytes[1] === 0x5a) { let evidence = parseMzHeaderEvidence(bytes, fileSize); let signatureEvidence: diff --git a/src/application/RegularFileRead.ts b/src/application/RegularFileRead.ts index fba17dce50..d8e5b82299 100644 --- a/src/application/RegularFileRead.ts +++ b/src/application/RegularFileRead.ts @@ -206,9 +206,8 @@ export const withRegularFileCompletion = async ( ...(signal === undefined ? {} : { signal }), symlinks: options.symlinks ?? "follow", }, - async (handle) => { + async ({ handle, initial: stats }) => { admitted = handle; - const stats = await handle.stat(); assertRegularFileActive(signal); const value = await read(handle, stats); assertRegularFileActive(signal); diff --git a/src/artifacts/ArtifactFileOperation.ts b/src/artifacts/ArtifactFileOperation.ts index 6b88b62d5d..679673dc6e 100644 --- a/src/artifacts/ArtifactFileOperation.ts +++ b/src/artifacts/ArtifactFileOperation.ts @@ -1,5 +1,8 @@ import type { FileHandle } from "node:fs/promises"; -import { RegularFileOperationCancelled } from "../filesystem/RegularFile.js"; +import { + RegularFileOperationCancelled, + type StableRegularFileDescriptor, +} from "../filesystem/RegularFile.js"; import { OwnedFileHandle } from "../filesystem/OwnedFileHandle.js"; import { withOwnedFileHandle, @@ -252,7 +255,7 @@ export const withArtifactRegularFile = async ( ): void; }, signal: AbortSignal | undefined, - operation: (file: FileHandle) => Promise, + operation: (source: StableRegularFileDescriptor) => Promise, symlinks: "follow" | "reject" = "reject", ): Promise => { let cleanupLease: AnalysisCleanupLease | undefined; @@ -272,9 +275,9 @@ export const withArtifactRegularFile = async ( }); }, }, - async (file) => { + async (source) => { assertArtifactReadActive(signal); - return await operation(file); + return await operation(source); }, ); return value; diff --git a/src/artifacts/ArtifactFileSource.ts b/src/artifacts/ArtifactFileSource.ts index 4af676b0a8..2711ad465b 100644 --- a/src/artifacts/ArtifactFileSource.ts +++ b/src/artifacts/ArtifactFileSource.ts @@ -156,28 +156,12 @@ export class ArtifactFileSource { handle: this.admitted.handle, initial: this.admitted.initial, }; - const handle = await openRegularFile(this.path, { + const source = await openRegularFile(this.path, { symlinks: "reject", signal, }); - const owner = new OwnedFileHandle(handle); - this.#owner = owner; - try { - return { handle, initial: await handle.stat() }; - } catch (cause: unknown) { - try { - await owner.close(); - this.#owner = undefined; - } catch (cleanupCause: unknown) { - throw new RegularFileAdmissionFailure( - this.path, - cause, - cleanupCause, - owner, - ); - } - throw cause; - } + this.#owner = new OwnedFileHandle(source.handle); + return source; } catch (cause: unknown) { if (cause instanceof RegularFileAdmissionFailure) this.#owner = cause.owner; diff --git a/src/artifacts/GodotPckArtifactReader.test.ts b/src/artifacts/GodotPckArtifactReader.test.ts index c3e82036e8..7938e82303 100644 --- a/src/artifacts/GodotPckArtifactReader.test.ts +++ b/src/artifacts/GodotPckArtifactReader.test.ts @@ -6,8 +6,8 @@ import { pckU64, type GodotPckMember, } from "../../tests/fixtures/godotPck.js"; -import { rename, writeFile } from "node:fs/promises"; -import { dirname, join } from "node:path"; +import { mkdir, rename, writeFile } from "node:fs/promises"; +import { join } from "node:path"; import { text } from "node:stream/consumers"; import { describe, expect, it } from "vitest"; @@ -241,19 +241,20 @@ describe("GodotPckArtifactReader member streams", () => { }); it("reads members from the admitted descriptor after the path is replaced", async () => { - const path = await writePck(buildGodotPck(3, members)); - const handle = await openRegularFile(path, { symlinks: "reject" }); + const root = await createTestTempDirectory("rea-godot-borrowed-source-"); + const selected = join(root, "selected"); + await mkdir(selected); + const path = join(selected, "game"); + await writeFile(path, buildGodotPck(3, members)); + const descriptor = await openRegularFile(path, { symlinks: "reject" }); + const { handle } = descriptor; try { - const replacement = join(dirname(path), "replacement"); - await writeFile( - replacement, - buildGodotPck(3, [{ path: "other.gd", data: "" }]), - ); - await rename(replacement, path); - const reader = new GodotPckArtifactReader(path, { - handle, - initial: await handle.stat(), - }); + // Replace the selected directory without changing the admitted inode's + // metadata. Moving the file itself would change its validated ctime. + await rename(selected, join(root, "original")); + await mkdir(selected); + await writeFile(path, buildGodotPck(3, [{ path: "other.gd", data: "" }])); + const reader = new GodotPckArtifactReader(path, descriptor); const entries = await list(reader); expect(entries.map(({ path: member }) => member)).toEqual([ "scenes/main.gd", diff --git a/src/artifacts/MachOSliceArtifactReader.test.ts b/src/artifacts/MachOSliceArtifactReader.test.ts index a3630479b3..85c6a2537f 100644 --- a/src/artifacts/MachOSliceArtifactReader.test.ts +++ b/src/artifacts/MachOSliceArtifactReader.test.ts @@ -306,8 +306,8 @@ describe("MachOSliceArtifactReader", () => { { cpu: CPU.arm64, bytes: arm }, ]), ); - const handle = await openRegularFile(binary, { symlinks: "reject" }); - const source = { handle, initial: await handle.stat() }; + const source = await openRegularFile(binary, { symlinks: "reject" }); + const { handle } = source; const runner: NativeCommandRunner = { run: () => Promise.resolve( diff --git a/src/artifacts/NativeDmgArtifactReader.ts b/src/artifacts/NativeDmgArtifactReader.ts index 9a3f9959a1..28a378f5ba 100644 --- a/src/artifacts/NativeDmgArtifactReader.ts +++ b/src/artifacts/NativeDmgArtifactReader.ts @@ -597,10 +597,10 @@ export class NativeDmgArtifactReader implements ArtifactReader { ): Promise { let handle: FileHandle; try { - handle = await openRegularFile(this.path, { + ({ handle } = await openRegularFile(this.path, { symlinks: "follow", ...(signal === undefined ? {} : { signal }), - }); + })); } catch (cause: unknown) { if (cause instanceof RegularFileAdmissionFailure) { this.#verificationProbeHandle = cause.owner; diff --git a/src/artifacts/RegularFileAdmission.test.ts b/src/artifacts/RegularFileAdmission.test.ts index fe1d0fb376..eb1ce843b3 100644 --- a/src/artifacts/RegularFileAdmission.test.ts +++ b/src/artifacts/RegularFileAdmission.test.ts @@ -1,6 +1,8 @@ import type { BigIntStats, Stats } from "node:fs"; import type { FileHandle } from "node:fs/promises"; import { tmpdir } from "node:os"; +import { writeFile } from "node:fs/promises"; +import { join } from "node:path"; import { afterEach, expect, it, vi } from "vitest"; import { @@ -10,6 +12,7 @@ import { RegularFileOperationCancelled, sameRegularFileState, } from "../filesystem/RegularFile.js"; +import { createTestTempDirectory } from "../../tests/fixtures/temporaryDirectory.js"; const { mockOpen } = vi.hoisted(() => ({ mockOpen: vi.fn() })); @@ -20,6 +23,44 @@ vi.mock("node:fs/promises", async (importOriginal) => ({ afterEach(() => mockOpen.mockReset()); +it("returns the original validated metadata snapshot with its actual descriptor", async () => { + const { open } = + await vi.importActual( + "node:fs/promises", + ); + const root = await createTestTempDirectory("rea-admission-snapshot-"); + const path = join(root, "selected.bin"); + const original = Buffer.from("admitted α测试\0\n"); + await writeFile(path, original); + let validated: Stats | undefined; + let opened: FileHandle | undefined; + mockOpen.mockImplementationOnce(async (...args: Parameters) => { + const handle = await open(...args); + opened = handle; + const stat = handle.stat.bind(handle); + vi.spyOn(handle, "stat").mockImplementationOnce(async () => { + validated = await stat(); + return validated; + }); + return handle; + }); + const descriptor = await openRegularFile(path, { symlinks: "reject" }); + try { + expect(descriptor.handle).toBe(opened); + expect(descriptor.initial).toBe(validated); + expect(descriptor.initial.size).toBe(original.length); + await writeFile( + path, + "a later, longer replacement extent on the same inode", + ); + expect(descriptor.initial).toBe(validated); + expect(descriptor.initial.size).toBe(original.length); + expect(descriptor.handle.stat).toHaveBeenCalledOnce(); + } finally { + await descriptor.handle.close(); + } +}); + it.each(["cancellation after open", "non-regular metadata", "stat failure"])( "retains the exact owner when close fails after %s", async (failureCase) => { diff --git a/src/artifacts/ZipArtifactReader.test.ts b/src/artifacts/ZipArtifactReader.test.ts index eb55c6b08f..be60df9835 100644 --- a/src/artifacts/ZipArtifactReader.test.ts +++ b/src/artifacts/ZipArtifactReader.test.ts @@ -62,7 +62,9 @@ const admittedZip = async () => { const writer = new ZipWriter(new Uint8ArrayWriter()); await writer.add("member.txt", new TextReader(content)); await writeFile(path, await writer.close()); - const handle = await openRegularFile(path, { symlinks: "reject" }); + const { handle, initial } = await openRegularFile(path, { + symlinks: "reject", + }); // Keep the actual descriptor, but select the positional overload used by // the ZIP adapter so the spy does not infer FileHandle.read's options form. const positional: { @@ -73,7 +75,7 @@ const admittedZip = async () => { position: number, ): Promise<{ bytesRead: number; buffer: Buffer }>; } = handle; - return { path, content, handle, positional, initial: await handle.stat() }; + return { path, content, handle, positional, initial }; }; describe("ZipArtifactReader positional reads", () => { diff --git a/src/artifacts/ZipArtifactReader.ts b/src/artifacts/ZipArtifactReader.ts index 3ddd544a6a..dc03966bcf 100644 --- a/src/artifacts/ZipArtifactReader.ts +++ b/src/artifacts/ZipArtifactReader.ts @@ -75,12 +75,13 @@ class NodeFileReader extends Reader { } async #initialize(): Promise { + let source: StableRegularFileDescriptor; try { - const handle = - this.admitted?.handle ?? + source = + this.admitted ?? (await openRegularFile(this.path, { symlinks: "reject" })); - this.#handle = handle; - if (this.#ownsHandle) this.#owner = new OwnedFileHandle(handle); + this.#handle = source.handle; + if (this.#ownsHandle) this.#owner = new OwnedFileHandle(source.handle); } catch (cause: unknown) { if (cause instanceof RegularFileAdmissionFailure) { this.#owner = cause.owner; @@ -108,17 +109,15 @@ class NodeFileReader extends Reader { ); throw cause; } - const metadata = await this.#handle.stat(); - if ( - !metadata.isFile() || - (this.admitted !== undefined && - !sameRegularFileState(this.admitted.initial, metadata)) - ) - throw new ArtifactReaderFailure( - "integrity", - `ZIP source identity changed before reading: ${this.path}`, - ); - this.size = metadata.size; + if (this.admitted !== undefined) { + const metadata = await source.handle.stat(); + if (!sameRegularFileState(source.initial, metadata)) + throw new ArtifactReaderFailure( + "integrity", + `ZIP source identity changed before reading: ${this.path}`, + ); + } + this.size = source.initial.size; } override async readUint8Array( diff --git a/src/artifacts/apple/DylibFileAccess.ts b/src/artifacts/apple/DylibFileAccess.ts index d0f0561c9f..8fcff4fe11 100644 --- a/src/artifacts/apple/DylibFileAccess.ts +++ b/src/artifacts/apple/DylibFileAccess.ts @@ -89,7 +89,7 @@ export class DylibFileAccess { path, this.resources, this.signal, - async (file) => { + async ({ handle: file }) => { const initial = await file.stat({ bigint: true }); assertArtifactReadActive(this.signal); const result = await operation(file, initial); diff --git a/src/artifacts/extraction/ArtifactExtraction.ts b/src/artifacts/extraction/ArtifactExtraction.ts index 914b6eef6c..51304d3380 100644 --- a/src/artifacts/extraction/ArtifactExtraction.ts +++ b/src/artifacts/extraction/ArtifactExtraction.ts @@ -554,8 +554,8 @@ const requireExtractableFormat = async ( path, input.resourceScope, signal, - async (handle) => { - const size = (await handle.stat()).size; + async ({ handle, initial }) => { + const size = initial.size; if (await locateGodotPck(handle, size, signal)) return true; if (format !== "elf" && format !== "pe" && format !== "mach-o") return false; diff --git a/src/artifacts/inventory/classify.ts b/src/artifacts/inventory/classify.ts index 7bcbb9d869..6bc93137b3 100644 --- a/src/artifacts/inventory/classify.ts +++ b/src/artifacts/inventory/classify.ts @@ -92,15 +92,15 @@ export const classifyAndHashRootForInventory = async ( ): Promise => { if (directory) return { format: "directory", digest: null, rootSource: undefined }; - const ownedHandle = await openRootArtifact( + const source = await openRootArtifact( path, options.resourceScope, options.signal, ); - const handle = ownedHandle.handle; + const { handle, initial } = source; const owner: ArtifactResourceOwner = { kind: "file-handle", - handle: ownedHandle, + handle: source.owner, resource: `root artifact descriptor for ${path}`, }; let outcome: @@ -110,7 +110,6 @@ export const classifyAndHashRootForInventory = async ( } | { readonly kind: "failed"; readonly cause: unknown }; try { - const initial = await handle.stat(); if (!sameRegularFileState(expectedMetadata, initial)) throw new ArtifactReaderFailure( "integrity", @@ -139,9 +138,7 @@ export const classifyAndHashRootForInventory = async ( value: { format, digest, - rootSource: retainSource - ? { handle, initial, owner: ownedHandle } - : undefined, + rootSource: retainSource ? source : undefined, }, }; } catch (cause: unknown) { diff --git a/src/artifacts/inventory/hashStableRootArtifact.ts b/src/artifacts/inventory/hashStableRootArtifact.ts index 5c5ef95334..29ac8be949 100644 --- a/src/artifacts/inventory/hashStableRootArtifact.ts +++ b/src/artifacts/inventory/hashStableRootArtifact.ts @@ -11,6 +11,7 @@ import { RegularFileAdmissionFailure, openRegularFile, sameRegularFileState, + type StableRegularFileDescriptor, } from "../../filesystem/RegularFile.js"; import { abortIfNeeded, @@ -24,11 +25,12 @@ export const openRootArtifact = async ( path: string, resourceScope: ArtifactResourceScope, signal?: AbortSignal, -): Promise => { +): Promise< + StableRegularFileDescriptor & { readonly owner: OwnedFileHandle } +> => { try { - return new OwnedFileHandle( - await openRegularFile(path, { symlinks: "reject", signal }), - ); + const source = await openRegularFile(path, { symlinks: "reject", signal }); + return { ...source, owner: new OwnedFileHandle(source.handle) }; } catch (cause: unknown) { const primary = cause instanceof RegularFileAdmissionFailure ? cause.cause : cause; @@ -69,13 +71,15 @@ export const hashStableRootArtifact = async ( resourceScope: ArtifactResourceScope, signal?: AbortSignal, ): Promise => { - const owner = await openRootArtifact(path, resourceScope, signal); - const handle = owner.handle; + const { handle, initial, owner } = await openRootArtifact( + path, + resourceScope, + signal, + ); let outcome: | { readonly kind: "completed"; readonly value: HashResult } | { readonly kind: "failed"; readonly cause: unknown }; try { - const initial = await handle.stat(); outcome = { kind: "completed", value: await hashStableRootArtifactHandle(path, handle, initial, signal), diff --git a/src/artifacts/pyinstaller/PyInstallerPyzStore.ts b/src/artifacts/pyinstaller/PyInstallerPyzStore.ts index 30ae081bae..cfc22b2425 100644 --- a/src/artifacts/pyinstaller/PyInstallerPyzStore.ts +++ b/src/artifacts/pyinstaller/PyInstallerPyzStore.ts @@ -9,6 +9,7 @@ import { openRegularFile, RegularFileAdmissionFailure, sameRegularFileState, + type StableRegularFileDescriptor, } from "../../filesystem/RegularFile.js"; import { ArtifactReaderFailure } from "../ArtifactReader.js"; import { @@ -148,8 +149,8 @@ export class PyInstallerPyzStore { if (decoded?.status === "rejected") throw decoded.reason; await this.release(writer); abortIfNeeded(signal); - const opened = await this.#admit(path, signal); - this.#images.set(opened, { path, initial: await opened.stat() }); + const { handle: opened, initial } = await this.#admit(path, signal); + this.#images.set(opened, { path, initial }); return { handle: opened, start: 0 }; } @@ -176,7 +177,7 @@ export class PyInstallerPyzStore { let verify: (() => Promise) | undefined; let close: ((primary?: unknown) => Promise) | undefined; if (image !== undefined) { - handle = await this.#admit(image.path, signal); + ({ handle } = await this.#admit(image.path, signal)); const opened = handle; verify = () => this.#verifyImage(opened, image); close = async (primary) => { @@ -231,14 +232,17 @@ export class PyInstallerPyzStore { this.#images.clear(); } - async #admit(path: string, signal?: AbortSignal): Promise { + async #admit( + path: string, + signal?: AbortSignal, + ): Promise { try { - const handle = await openRegularFile(path, { + const source = await openRegularFile(path, { symlinks: "reject", signal, }); - this.#retain(path, new OwnedFileHandle(handle)); - return handle; + this.#retain(path, new OwnedFileHandle(source.handle)); + return source; } catch (cause: unknown) { if (cause instanceof RegularFileAdmissionFailure) this.#retain(path, cause.owner); diff --git a/src/cutter/CutterBridgeClient.ts b/src/cutter/CutterBridgeClient.ts index f100bd7418..e3bca91b11 100644 --- a/src/cutter/CutterBridgeClient.ts +++ b/src/cutter/CutterBridgeClient.ts @@ -898,8 +898,7 @@ export class CutterBridgeClient { join(this.#directory, entry), this.#descriptorResources, signal, - async (file) => { - const info = await file.stat(); + async ({ handle: file, initial: info }) => { if ( info.size > MAX_DESCRIPTOR_BYTES || info.uid !== currentUid || diff --git a/src/electron/ElectronFuseReader.ts b/src/electron/ElectronFuseReader.ts index 603152e26a..b0a50cd698 100644 --- a/src/electron/ElectronFuseReader.ts +++ b/src/electron/ElectronFuseReader.ts @@ -151,7 +151,7 @@ export const readElectronFuseImage = async ( path, options?.resources ?? new ArtifactResourceScope(), signal, - async (owned) => { + async ({ handle: owned }) => { const opened = await owned.stat({ bigint: true }); if (!sameRegularFileState(before, opened)) throw new RegularFileChangedError(path); diff --git a/src/filesystem/OwnedRegularFile.ts b/src/filesystem/OwnedRegularFile.ts index f3759bba99..17fb36d8d6 100644 --- a/src/filesystem/OwnedRegularFile.ts +++ b/src/filesystem/OwnedRegularFile.ts @@ -6,6 +6,7 @@ import { RegularFileAdmissionFailure, assertRegularFileActive, RegularFileOperationCancelled, + type StableRegularFileDescriptor, } from "./RegularFile.js"; export type RegularFileOperationOutcome = @@ -145,9 +146,9 @@ export interface OwnedRegularFileOptions extends OwnedFileOperationOptions { export const withOwnedRegularFile = async ( path: string, options: OwnedRegularFileOptions, - operation: (handle: FileHandle) => Promise, + operation: (source: StableRegularFileDescriptor) => Promise, ): Promise => { - const handle = await openRegularFile(path, { + const source = await openRegularFile(path, { symlinks: options.symlinks ?? "follow", signal: options.signal, }).catch((cause: unknown) => { @@ -163,9 +164,9 @@ export const withOwnedRegularFile = async ( }); return withOwnedFileHandle( path, - new OwnedFileHandle(handle), + new OwnedFileHandle(source.handle), options, - operation, + () => operation(source), ); }; diff --git a/src/filesystem/RegularFile.ts b/src/filesystem/RegularFile.ts index 95c675d056..3d318e8318 100644 --- a/src/filesystem/RegularFile.ts +++ b/src/filesystem/RegularFile.ts @@ -90,14 +90,14 @@ export interface StableRegularFileDescriptor { readonly initial: Stats; } -/** Open and verify the selected file descriptor without blocking on pipes. */ +/** Admit a non-blocking regular-file descriptor with its validated initial facts. */ export const openRegularFile = async ( path: string, options: { readonly symlinks: "follow" | "reject"; readonly signal?: AbortSignal | undefined; }, -): Promise => { +): Promise => { assertRegularFileActive(options.signal); const handle = await open( path, @@ -131,7 +131,7 @@ export const openRegularFile = async ( throw new RegularFileChangedError(path); assertRegularFileActive(options.signal); } - return handle; + return { handle, initial: metadata }; } catch (cause: unknown) { try { await owner.close(); diff --git a/src/filesystem/StableFileHash.ts b/src/filesystem/StableFileHash.ts index 26f0f58d19..44d9a01b9b 100644 --- a/src/filesystem/StableFileHash.ts +++ b/src/filesystem/StableFileHash.ts @@ -82,55 +82,58 @@ async function readStableFile( includePrefix: boolean, ): Promise { try { - return await withOwnedRegularFile(path, options, async (handle) => { - const initial = await handle.stat(); - if (!Number.isSafeInteger(initial.size) || initial.size < 0) - throw new RangeError( - `Regular file size cannot be represented safely: ${path}`, + 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, ); - 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) { + 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); - const length = Math.min(chunk.length, initial.size - position + 1); - if (length <= 0) break; - const read = await handle.read(chunk, 0, length, position); + const [opened, currentPath] = await Promise.all([ + handle.stat(), + stat(path), + ]); if ( - position + read.bytesRead > initial.size || - (read.bytesRead === 0 && position < initial.size) + position !== initial.size || + !sameRegularFileState(initial, opened) || + !sameRegularFileState(initial, currentPath) ) 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); - const [opened, currentPath] = await Promise.all([ - handle.stat(), - stat(path), - ]); - if ( - position !== initial.size || - !sameRegularFileState(initial, opened) || - !sameRegularFileState(initial, currentPath) - ) - throw new RegularFileChangedError(path); - assertRegularFileActive(options.signal); - const fingerprint = { sha256: hash.digest("hex"), bytes: position }; - return includePrefix ? { ...fingerprint, prefix } : fingerprint; - }); + const fingerprint = { sha256: hash.digest("hex"), bytes: position }; + return includePrefix ? { ...fingerprint, prefix } : fingerprint; + }, + ); } catch (cause: unknown) { if ( cause instanceof RegularFileCleanupFailure || diff --git a/src/firmware/FirmwareFiles.ts b/src/firmware/FirmwareFiles.ts index 27128e78d9..8bf429a95b 100644 --- a/src/firmware/FirmwareFiles.ts +++ b/src/firmware/FirmwareFiles.ts @@ -93,7 +93,7 @@ export class FirmwareFileAccess { path, this.#resources, signal, - async (handle) => { + async ({ handle }) => { const hash = createHash("sha256"); for await (const chunk of readArtifactFileChunks(handle, { signal })) { assertArtifactReadActive(signal); @@ -125,8 +125,7 @@ export class FirmwareFileAccess { source, this.#resources, signal, - async (handle) => { - const stat = await handle.stat(); + async ({ handle, initial: stat }) => { sourceAdmitted = true; if (stat.size === 0 || stat.size > FIRMWARE_LIMITS.inputBytes) throw firmwareInputFailure(request.operation); @@ -179,7 +178,7 @@ export class FirmwareFileAccess { fullPath, this.#resources, signal, - async (handle) => { + async ({ handle }) => { await pipeline( readArtifactFileChunks(handle, { start: offset, @@ -213,20 +212,24 @@ export class FirmwareFileAccess { signal: AbortSignal, ) { return this.#resources.run(() => - withArtifactRegularFile(entry.path, this.#resources, signal, (handle) => - tree.write( - entry.relativePath, - // SafeOutputTree awaits destination setup before consuming. Defer - // the abort-bound descriptor stream until it has a consumer. - Readable.from( - (async function* () { - assertArtifactReadActive(signal); - yield* readArtifactFileChunks(handle, { signal }); - })(), + withArtifactRegularFile( + entry.path, + this.#resources, + signal, + ({ handle }) => + tree.write( + entry.relativePath, + // SafeOutputTree awaits destination setup before consuming. Defer + // the abort-bound descriptor stream until it has a consumer. + Readable.from( + (async function* () { + assertArtifactReadActive(signal); + yield* readArtifactFileChunks(handle, { signal }); + })(), + ), + { sha256, bytes: entry.size }, + signal, ), - { sha256, bytes: entry.size }, - signal, - ), ), ); } @@ -242,36 +245,41 @@ export class FirmwareFileAccess { signal?: AbortSignal, ): Promise { return this.#resources.run(() => - withArtifactRegularFile(path, this.#resources, signal, async (handle) => { - const bytes = await readBoundedFileBytes( - handle, - FIRMWARE_LIMITS.reportBytes, - ); - if (bytes === undefined) - throw firmwareReportFailure( - operation, - "oversized", - `Report exceeded ${FIRMWARE_LIMITS.reportBytes} bytes`, + withArtifactRegularFile( + path, + this.#resources, + signal, + async ({ handle }) => { + const bytes = await readBoundedFileBytes( + handle, + FIRMWARE_LIMITS.reportBytes, ); - assertArtifactReadActive(signal); - if (bytes.length === 0) - throw firmwareReportFailure(operation, "empty", "Report is empty"); - try { - const value: unknown = JSON.parse( - new TextDecoder("utf-8", { fatal: true, ignoreBOM: true }).decode( - bytes, - ), - ); - return value; - } catch (cause: unknown) { - throw firmwareReportFailure( - operation, - "invalid_json", - "Tool report is not valid complete JSON", - cause, - ); - } - }).catch((cause: unknown) => { + if (bytes === undefined) + throw firmwareReportFailure( + operation, + "oversized", + `Report exceeded ${FIRMWARE_LIMITS.reportBytes} bytes`, + ); + assertArtifactReadActive(signal); + if (bytes.length === 0) + throw firmwareReportFailure(operation, "empty", "Report is empty"); + try { + const value: unknown = JSON.parse( + new TextDecoder("utf-8", { fatal: true, ignoreBOM: true }).decode( + bytes, + ), + ); + return value; + } catch (cause: unknown) { + throw firmwareReportFailure( + operation, + "invalid_json", + "Tool report is not valid complete JSON", + cause, + ); + } + }, + ).catch((cause: unknown) => { throw mapArtifactFileFailure(cause, (primary) => primary instanceof NonRegularFileReadError ? firmwareReportFailure( diff --git a/src/flutter/FlutterBuildProvider.ts b/src/flutter/FlutterBuildProvider.ts index b4b5ea7fd4..6cecca4028 100644 --- a/src/flutter/FlutterBuildProvider.ts +++ b/src/flutter/FlutterBuildProvider.ts @@ -411,12 +411,11 @@ export class FlutterBuildProvider { // stops the write promptly instead of draining the whole file. let source: OwnedFileHandle; try { - source = this.ownResource( - root, - new OwnedFileHandle( - await openRegularFile(path, { symlinks: "follow", signal }), - ), - ); + const { handle } = await openRegularFile(path, { + symlinks: "follow", + signal, + }); + source = this.ownResource(root, new OwnedFileHandle(handle)); } catch (cause) { if (cause instanceof RegularFileAdmissionFailure) { this.ownResource(root, cause.owner); diff --git a/src/ghidra/GhidraClient.ts b/src/ghidra/GhidraClient.ts index 19995d0cd7..1b13410dd7 100644 --- a/src/ghidra/GhidraClient.ts +++ b/src/ghidra/GhidraClient.ts @@ -616,8 +616,7 @@ export class GhidraClient { path, this.#fileResources, signal, - async (handle) => { - const initial = await handle.stat(); + async ({ handle, initial }) => { assertRegularFileActive(signal); if (!Number.isSafeInteger(initial.size) || initial.size < 0) throw new RangeError("Snapshot size must be a safe integer"); diff --git a/src/ghidra/GhidraExecutableHeader.ts b/src/ghidra/GhidraExecutableHeader.ts index 23a8c2632d..aee59a2259 100644 --- a/src/ghidra/GhidraExecutableHeader.ts +++ b/src/ghidra/GhidraExecutableHeader.ts @@ -30,7 +30,7 @@ export const readGhidraExecutableHeader = async ( ), }, options.signal, - async (file) => { + async ({ handle: file }) => { const observed = await readStableFilePrefix( file, path, diff --git a/src/ghidra/GhidraFileArtifacts.ts b/src/ghidra/GhidraFileArtifacts.ts index fb22034b41..0e68bdbd93 100644 --- a/src/ghidra/GhidraFileArtifacts.ts +++ b/src/ghidra/GhidraFileArtifacts.ts @@ -35,7 +35,7 @@ export const readGhidraProfileArtifact = ( signal: AbortSignal | undefined, interpret: (bytes: Buffer) => Value, ): Promise => - withArtifactRegularFile(path, resources, signal, async (file) => { + withArtifactRegularFile(path, resources, signal, async ({ handle: file }) => { const bytes = await readStableFileBytes(file, path, maximumBytes, { symlinks: "reject", ...(signal === undefined ? {} : { signal }), diff --git a/src/ghidra/GhidraInstallationProperties.ts b/src/ghidra/GhidraInstallationProperties.ts index 3177da1242..830dd7c576 100644 --- a/src/ghidra/GhidraInstallationProperties.ts +++ b/src/ghidra/GhidraInstallationProperties.ts @@ -40,7 +40,7 @@ export const readGhidraInstallationProperties = async ( ), }, options.signal, - async (file) => + async ({ handle: file }) => ( await readStableFileBytes( file, diff --git a/src/ghidra/GhidraJavaOptions.ts b/src/ghidra/GhidraJavaOptions.ts index 251d2a4e1d..4dfd295aa9 100644 --- a/src/ghidra/GhidraJavaOptions.ts +++ b/src/ghidra/GhidraJavaOptions.ts @@ -141,7 +141,7 @@ const readArgumentFile = ( path, resources, signal, - async (file) => { + async ({ handle: file }) => { const data = await readBoundedFileBytes(file, remainingBytes, { signal, cancellationFailure: (reason) => diff --git a/src/ghidra/GhidraLanguageCatalog.ts b/src/ghidra/GhidraLanguageCatalog.ts index 27b6319bd2..fa0d17ea12 100644 --- a/src/ghidra/GhidraLanguageCatalog.ts +++ b/src/ghidra/GhidraLanguageCatalog.ts @@ -82,7 +82,7 @@ const readLanguageDefinitionFile = ( path, resources, signal, - async (file) => { + async ({ handle: file }) => { const bytes = await readBoundedFileBytes(file, MAX_LDEFS_BYTES, { signal, cancellationFailure: (reason) => diff --git a/src/ghidra/GhidraPspExtension.ts b/src/ghidra/GhidraPspExtension.ts index c85aeb9599..c3e9b9feb2 100644 --- a/src/ghidra/GhidraPspExtension.ts +++ b/src/ghidra/GhidraPspExtension.ts @@ -116,58 +116,62 @@ const fingerprint = async ( for (const relative of files) { assertArtifactReadActive(signal); const path = join(root, relative); - await withArtifactRegularFile(path, resources, signal, async (handle) => { - const before = await handle.stat(); - if ( - !Number.isSafeInteger(before.size) || - before.size > MAX_BYTES - bytesRead - ) - throw new Error( - "Allegrex extension bytes exceed the inspection limit.", - ); - if (relative === "extension.properties" && before.size > 65536) - throw new Error( - "Allegrex extension properties exceed the inspection limit.", - ); - hash.update(JSON.stringify([relative, before.size])); - const buffer = Buffer.alloc(Math.min(65536, before.size + 1)); - let offset = 0; - const chunks: Buffer[] = []; - // Serial reads keep final close behind every native read. One extra byte - // detects growth without a stream's read-ahead or unbounded allocation. - while (offset <= before.size) { - assertArtifactReadActive(signal); - const { bytesRead } = await handle.read( - buffer, - 0, - Math.min(buffer.length, before.size - offset + 1), - offset, - ); - assertArtifactReadActive(signal); - if (bytesRead === 0) break; - offset += bytesRead; - if (offset > before.size) + await withArtifactRegularFile( + path, + resources, + signal, + async ({ handle, initial: before }) => { + if ( + !Number.isSafeInteger(before.size) || + before.size > MAX_BYTES - bytesRead + ) + throw new Error( + "Allegrex extension bytes exceed the inspection limit.", + ); + if (relative === "extension.properties" && before.size > 65536) + throw new Error( + "Allegrex extension properties exceed the inspection limit.", + ); + hash.update(JSON.stringify([relative, before.size])); + const buffer = Buffer.alloc(Math.min(65536, before.size + 1)); + let offset = 0; + const chunks: Buffer[] = []; + // Serial reads keep final close behind every native read. One extra byte + // detects growth without a stream's read-ahead or unbounded allocation. + while (offset <= before.size) { + assertArtifactReadActive(signal); + const { bytesRead } = await handle.read( + buffer, + 0, + Math.min(buffer.length, before.size - offset + 1), + offset, + ); + assertArtifactReadActive(signal); + if (bytesRead === 0) break; + offset += bytesRead; + if (offset > before.size) + throw new Error( + `Allegrex component changed during inspection: ${relative}`, + ); + const chunk = buffer.subarray(0, bytesRead); + hash.update(chunk); + if (relative === "extension.properties") + chunks.push(Buffer.from(chunk)); + } + if (offset !== before.size) + throw new Error(`Truncated Allegrex component: ${relative}`); + if ( + !sameRegularFileState(before, await handle.stat()) || + !sameRegularFileState(before, await lstat(path)) + ) throw new Error( `Allegrex component changed during inspection: ${relative}`, ); - const chunk = buffer.subarray(0, bytesRead); - hash.update(chunk); + bytesRead += before.size; if (relative === "extension.properties") - chunks.push(Buffer.from(chunk)); - } - if (offset !== before.size) - throw new Error(`Truncated Allegrex component: ${relative}`); - if ( - !sameRegularFileState(before, await handle.stat()) || - !sameRegularFileState(before, await lstat(path)) - ) - throw new Error( - `Allegrex component changed during inspection: ${relative}`, - ); - bytesRead += before.size; - if (relative === "extension.properties") - properties = Buffer.concat(chunks); - }); + properties = Buffer.concat(chunks); + }, + ); } // Ghidra's extension.properties version is the compatible Ghidra version, // not the extension release (e.g. v21.4); content identity supplies the latter. diff --git a/src/ghidra/GhidraTargetSnapshot.ts b/src/ghidra/GhidraTargetSnapshot.ts index c6846bfc5f..0b630c8726 100644 --- a/src/ghidra/GhidraTargetSnapshot.ts +++ b/src/ghidra/GhidraTargetSnapshot.ts @@ -84,7 +84,7 @@ export const createGhidraTargetSnapshot = async ( sourcePath, resources, options.signal, - async (source) => { + async ({ handle: source }) => { const initial = await source.stat({ bigint: true }); assertArtifactReadActive(options.signal); return await withArtifactFileHandle( diff --git a/src/ghidra/GhidraTransport.ts b/src/ghidra/GhidraTransport.ts index 3fbf752efd..e02cc2f3dc 100644 --- a/src/ghidra/GhidraTransport.ts +++ b/src/ghidra/GhidraTransport.ts @@ -102,7 +102,7 @@ export const observeGhidraEndpoint = async ( endpoint.path, resources, options.signal, - async (file) => { + async ({ handle: file }) => { const initial = await file.stat({ bigint: true }); assertArtifactReadActive(options.signal); if (initial.size > BigInt(MAX_TCP_ENDPOINT_BYTES)) diff --git a/src/gpu/GpuCodeObjectsProvider.ts b/src/gpu/GpuCodeObjectsProvider.ts index 3a23dd507d..78c87e72d1 100644 --- a/src/gpu/GpuCodeObjectsProvider.ts +++ b/src/gpu/GpuCodeObjectsProvider.ts @@ -102,7 +102,7 @@ export class GpuCodeObjectsProvider implements GpuCodeObjectsPort { input.path, this.#resources, options?.signal, - (file) => + ({ handle: file }) => this.#read(file, input, options?.signal, (value) => { progress = value; }), diff --git a/src/ida/IdaConfiguration.ts b/src/ida/IdaConfiguration.ts index 400720998c..a85956c9bb 100644 --- a/src/ida/IdaConfiguration.ts +++ b/src/ida/IdaConfiguration.ts @@ -108,8 +108,7 @@ export const readIdaConfiguration = ( ); }, }, - async (handle) => { - const metadata = await handle.stat(); + async ({ handle, initial: metadata }) => { if (metadata.size > MAXIMUM_REGISTRATION_BYTES) throw registrationTooLarge(); const bytes = Buffer.alloc(MAXIMUM_REGISTRATION_BYTES + 1); diff --git a/src/javascript/recovery/RecoveryFiles.ts b/src/javascript/recovery/RecoveryFiles.ts index d5793dde9c..31cac69a0b 100644 --- a/src/javascript/recovery/RecoveryFiles.ts +++ b/src/javascript/recovery/RecoveryFiles.ts @@ -65,9 +65,8 @@ const scanRecoveryFile = async ( path, resources ?? new ArtifactResourceScope(), signal, - async (handle) => { + async ({ handle, initial: before }) => { const hash = createHash("sha256"); - const before = await handle.stat(); if (before.size > maximum) throw new RecoveryFileSizeError( `Expected a regular file no larger than ${String(maximum)} bytes: ${path}`, diff --git a/src/model/ModelArtifactProvider.ts b/src/model/ModelArtifactProvider.ts index 023219995b..9f021a3bfa 100644 --- a/src/model/ModelArtifactProvider.ts +++ b/src/model/ModelArtifactProvider.ts @@ -103,7 +103,7 @@ export class ModelArtifactProvider implements ModelArtifactPort { input.path, this.#resources, options?.signal, - (file) => + ({ handle: file }) => this.#read(file, input, options?.signal, (progress) => { partialProgress = progress; }), diff --git a/src/native/AppleDispatchMetadata.ts b/src/native/AppleDispatchMetadata.ts index ce249c1ca5..0c871d586b 100644 --- a/src/native/AppleDispatchMetadata.ts +++ b/src/native/AppleDispatchMetadata.ts @@ -364,8 +364,7 @@ export const inspectAppleDispatchMetadata = async ( target.path, resources, signal, - async (handle) => { - const before = await handle.stat(); + async ({ handle, initial: before }) => { if (before.size > maximumBytes) throw inputBudgetExceeded(admission, before.size); if (signal?.aborted) throw cancelled(); diff --git a/src/native/LldbReadbackFiles.ts b/src/native/LldbReadbackFiles.ts index 9914a893ef..2c91f82a6d 100644 --- a/src/native/LldbReadbackFiles.ts +++ b/src/native/LldbReadbackFiles.ts @@ -61,7 +61,7 @@ export class LldbReadbackFiles { path, this.#resources, undefined, - async (handle) => { + async ({ handle }) => { const value = await read(handle); completed = { value }; return value; diff --git a/src/native/NativePlistSource.ts b/src/native/NativePlistSource.ts index 4fa1d7c859..f7609d9db7 100644 --- a/src/native/NativePlistSource.ts +++ b/src/native/NativePlistSource.ts @@ -283,7 +283,7 @@ const readPlistFile = async ( input.selection.path, input.resources, input.signal, - read, + ({ handle }) => read(handle), "follow", ), ); diff --git a/src/pcap/PcapSnapshot.ts b/src/pcap/PcapSnapshot.ts index 5211d85284..3c62ef7c50 100644 --- a/src/pcap/PcapSnapshot.ts +++ b/src/pcap/PcapSnapshot.ts @@ -127,11 +127,11 @@ export class PcapSnapshot { await mkdir(this.configurationPath, { mode: 0o700 }); const source = await sourceRead(async () => { try { - return this.own( - new OwnedFileHandle( - await openRegularFile(path, { symlinks: "follow", signal }), - ), - ); + const { handle } = await openRegularFile(path, { + symlinks: "follow", + signal, + }); + return this.own(new OwnedFileHandle(handle)); } catch (cause) { if (cause instanceof RegularFileAdmissionFailure) { this.own(cause.owner); @@ -240,11 +240,11 @@ export class PcapSnapshot { signal?: AbortSignal, ): Promise { try { - const file = this.own( - new OwnedFileHandle( - await openRegularFile(this.path, { symlinks: "reject", signal }), - ), - ); + const { handle } = await openRegularFile(this.path, { + symlinks: "reject", + signal, + }); + const file = this.own(new OwnedFileHandle(handle)); const before = await file.handle.stat({ bigint: true }); const buffer = Buffer.alloc(64 * 1024); const hash = createHash("sha256"); diff --git a/src/process/ArtifactAnalysisResources.test.ts b/src/process/ArtifactAnalysisResources.test.ts index 025a8a9267..4a1648577b 100644 --- a/src/process/ArtifactAnalysisResources.test.ts +++ b/src/process/ArtifactAnalysisResources.test.ts @@ -514,14 +514,19 @@ it.each([false, true])( }); if (!failedClose) throw primary; if (scope === undefined) throw new Error("Expected retained file scope"); - return withArtifactRegularFile(path, scope, _signal, async (file) => { - const close = file.close.bind(file); - file.close = async () => { - if (!mayClose) throw new Error("injected descriptor close failure"); - await close(); - }; - throw primary; - }); + return withArtifactRegularFile( + path, + scope, + _signal, + async ({ handle: file }) => { + const close = file.close.bind(file); + file.close = async () => { + if (!mayClose) throw new Error("injected descriptor close failure"); + await close(); + }; + throw primary; + }, + ); }; const resources = new ArtifactAnalysisResources( "fixture", @@ -1017,7 +1022,7 @@ it("retains producer output when a completed read is cancelled during failed clo path, scope, caller.signal, - async (file) => { + async ({ handle: file }) => { const close = file.close.bind(file); file.close = async () => { caller.abort(reason); diff --git a/src/reference/ReferenceSourceReaderFile.ts b/src/reference/ReferenceSourceReaderFile.ts index 14ad08cddf..c4d8def722 100644 --- a/src/reference/ReferenceSourceReaderFile.ts +++ b/src/reference/ReferenceSourceReaderFile.ts @@ -248,7 +248,7 @@ const readStableFileEntry = async ( parentBefore.message, safeSize(expected.size), ); - const handle = await openRegularFile(absolute, { + const { handle } = await openRegularFile(absolute, { symlinks: "reject", signal, }); diff --git a/src/server/environment/stateIntake.ts b/src/server/environment/stateIntake.ts index f5ffeb8d3a..86112345c7 100644 --- a/src/server/environment/stateIntake.ts +++ b/src/server/environment/stateIntake.ts @@ -175,7 +175,7 @@ const readStateFile = async ( handle: owner, }), }, - (handle) => + ({ handle }) => readStableFileBytes(handle, path, STATE_INTAKE_LIMIT_BYTES, { symlinks: "follow", }), diff --git a/tests/boundary/filesystem/adbInputSnapshot.test.ts b/tests/boundary/filesystem/adbInputSnapshot.test.ts index 36bb8e82ae..2308c768a6 100644 --- a/tests/boundary/filesystem/adbInputSnapshot.test.ts +++ b/tests/boundary/filesystem/adbInputSnapshot.test.ts @@ -33,7 +33,7 @@ it("retains and retries a source handle that fails close during admission", asyn undefined, { openSource: async (path, options) => { - const handle = await openRegularFile(path, options); + const { handle } = await openRegularFile(path, options); const close = handle.close.bind(handle); const owner = new OwnedFileHandle(handle); handle.close = async () => { diff --git a/tests/boundary/filesystem/flutterBuildProvider.test.ts b/tests/boundary/filesystem/flutterBuildProvider.test.ts index b3ee5e1706..9d7176f0ca 100644 --- a/tests/boundary/filesystem/flutterBuildProvider.test.ts +++ b/tests/boundary/filesystem/flutterBuildProvider.test.ts @@ -1463,10 +1463,11 @@ it.each(["EACCES", "EPERM"] as const)( const admitted = vi .spyOn(regularFile, "openRegularFile") .mockImplementation(async (...args) => { - const handle = await original(...args); + const descriptor = await original(...args); + const { handle } = descriptor; if (stage === "read") vi.spyOn(handle, "read").mockRejectedValueOnce(cause); - return handle; + return descriptor; }); const pathStat = stage === "revalidate" @@ -1498,21 +1499,23 @@ it("detects same-size source rewrites with restored mtime through the opened des const admitted = vi .spyOn(regularFile, "openRegularFile") .mockImplementation(async (...args) => { - const handle = await nativeAdmission(...args); - // Admission returns an open descriptor; mutate its file before the final - // descriptor stat, preserving the size and restoring the visible mtime. - const nativeStat = handle.stat.bind(handle); - let calls = 0; - vi.spyOn(handle, "stat").mockImplementation(async (...statArgs) => { - calls += 1; - if (calls === 2) { + const descriptor = await nativeAdmission(...args); + const { handle } = descriptor; + // Rewrite after a real descriptor read. The retained admission snapshot + // remains the baseline for final verification, including ctime. + const nativeRead = handle.read.bind(handle); + let rewritten = false; + vi.spyOn(handle, "read").mockImplementation(async (...readArgs) => { + const result = await nativeRead(...readArgs); + if (!rewritten) { + rewritten = true; await new Promise((resolve) => setTimeout(resolve, 10)); await writeFile(path, content); await utimes(path, before.atime, before.mtime); } - return nativeStat(...statArgs); + return result; }); - return handle; + return descriptor; }); onTestFinished(() => admitted.mockRestore()); const provider = new FlutterBuildProvider(); @@ -1560,14 +1563,15 @@ it("stops a cancelled descriptor copy before writing its next chunk and releases const admitted = vi .spyOn(regularFile, "openRegularFile") .mockImplementation(async (...args) => { - const handle = await nativeAdmission(...args); + const descriptor = await nativeAdmission(...args); + const { handle } = descriptor; const nativeRead = handle.read.bind(handle); vi.spyOn(handle, "read").mockImplementation(async (...readArgs) => { const result = await nativeRead(...readArgs); controller.abort(new Error("cancelled while copy read was pending")); return result; }); - return handle; + return descriptor; }); onTestFinished(() => admitted.mockRestore()); const removeRoot = vi.fn((root: string) => diff --git a/tests/boundary/filesystem/ownedArtifactReaderCleanup.test.ts b/tests/boundary/filesystem/ownedArtifactReaderCleanup.test.ts index 8d0896a6cd..f33b1d7e2c 100644 --- a/tests/boundary/filesystem/ownedArtifactReaderCleanup.test.ts +++ b/tests/boundary/filesystem/ownedArtifactReaderCleanup.test.ts @@ -47,95 +47,108 @@ const runner: NativeCommandRunner = { }), }; -const readers = [ - ["ZIP", (path: string) => new ZipArtifactReader(path, "zip")], - [ - "Mach-O slice", - (path: string) => new MachOSliceArtifactReader(path, {}, runner), - ], +const scenarios = [ + { + reader: "ZIP", + createReader: (path: string) => new ZipArtifactReader(path, "zip"), + stage: "admission", + failOnStat: 1, + }, + { + reader: "Mach-O slice", + createReader: (path: string) => + new MachOSliceArtifactReader(path, {}, runner), + stage: "admission", + failOnStat: 1, + }, + { + reader: "Mach-O slice", + createReader: (path: string) => + new MachOSliceArtifactReader(path, {}, runner), + stage: "post-admission-verification", + failOnStat: 2, + }, ] as const; -it.each(readers)( - "retains %s reader admission and stat owners", - async (_, createReader) => { +it.each(scenarios)( + "retains $reader reader $stage owner", + async ({ createReader, stage, failOnStat: initialFailOnStat }) => { const root = await createTestTempDirectory("rea-owned-reader-cleanup-"); const path = join(root, "source"); await writeFile(path, "fixture source"); - for (const stage of ["admission", "post-open-stat"] as const) { - const statFailure = Object.assign( - new Error(`primary ${stage} stat failure`), - { code: "EIO", errno: -5, syscall: "fstat" }, - ); - const closeFailure = new Error( - `close failed after invalidation during ${stage}`, - ); - let descriptor: FileHandle | undefined; - let statCalls = 0; - let closeAttempts = 0; - let failOnStat = stage === "admission" ? 1 : 2; - injected.opened = (openedPath, handle) => { - if (openedPath !== path || descriptor !== undefined) return; - descriptor = handle; - const stat = handle.stat.bind(handle); - const close = handle.close.bind(handle); - vi.spyOn(handle, "stat").mockImplementation(async (...args) => { - statCalls += 1; - if (statCalls === failOnStat) { - failOnStat = -1; - throw statFailure; - } - return stat(...args); - }); - vi.spyOn(handle, "close").mockImplementation(async () => { - closeAttempts += 1; - await close(); - throw closeFailure; - }); - }; - - const reader = createReader(path); - const scope = new ArtifactResourceScope(); - const owner = { kind: "reader" as const, resource: path, reader }; - onTestFinished(async () => { - injected.opened = undefined; - await scope.close().catch(() => undefined); - vi.restoreAllMocks(); - }); - - const primary = await (async () => { - try { - for await (const _entry of reader.entries()) { - // Both injected failures happen before the reader can yield an entry. - } - } catch (cause: unknown) { - return cause; + const statFailure = Object.assign( + new Error(`primary ${stage} stat failure`), + { code: "EIO", errno: -5, syscall: "fstat" }, + ); + const closeFailure = new Error( + `close failed after invalidation during ${stage}`, + ); + let descriptor: FileHandle | undefined; + let statCalls = 0; + let closeAttempts = 0; + let failOnStat: number = initialFailOnStat; + injected.opened = (openedPath, handle) => { + if (openedPath !== path || descriptor !== undefined) return; + descriptor = handle; + const stat = handle.stat.bind(handle); + const close = handle.close.bind(handle); + vi.spyOn(handle, "stat").mockImplementation(async (...args) => { + statCalls += 1; + if (statCalls === failOnStat) { + failOnStat = -1; + throw statFailure; } - throw new Error("Expected reader initialization to fail"); - })(); - expect(primary).toBeInstanceOf(Error); - expect(causeChainContains(primary, statFailure)).toBe(true); + return stat(...args); + }); + vi.spyOn(handle, "close").mockImplementation(async () => { + closeAttempts += 1; + await close(); + throw closeFailure; + }); + }; - const release = await scope.release(owner); - expect(release).toMatchObject({ - kind: "failed", - cause: closeFailure, - owner, - }); - if (release.kind !== "failed") - throw new Error("Expected owned cleanup failure"); - expect(release.cause).toBe(closeFailure); - expect(release.lease.released).toBe(false); - expect(descriptor?.fd).toBe(-1); - await expect(scope.close()).rejects.toMatchObject({ - reason: "unavailable", - cleanup: { reason: closeFailure.message, resources: [path] }, - cleanupLeases: [release.lease], - }); - // Native close invalidated the fd before rejecting. Retrying the same owner - // must remain failed without another syscall or a false successful release. - expect(closeAttempts).toBe(1); - } + const reader = createReader(path); + const scope = new ArtifactResourceScope(); + const owner = { kind: "reader" as const, resource: path, reader }; + onTestFinished(async () => { + injected.opened = undefined; + await scope.close().catch(() => undefined); + vi.restoreAllMocks(); + }); + + const primary = await (async () => { + try { + for await (const _entry of reader.entries()) { + // Admission or its first fresh verification fails before any entry. + } + } catch (cause: unknown) { + return cause; + } + throw new Error("Expected reader initialization to fail"); + })(); + expect(primary).toBeInstanceOf(Error); + expect(causeChainContains(primary, statFailure)).toBe(true); + + const release = await scope.release(owner); + expect(release).toMatchObject({ + kind: "failed", + cause: closeFailure, + owner, + }); + if (release.kind !== "failed") + throw new Error("Expected owned cleanup failure"); + expect(release.cause).toBe(closeFailure); + expect(release.lease.released).toBe(false); + expect(descriptor?.fd).toBe(-1); + await expect(scope.close()).rejects.toMatchObject({ + reason: "unavailable", + cleanup: { reason: closeFailure.message, resources: [path] }, + cleanupLeases: [release.lease], + }); + // Native close invalidated the fd before rejecting. Retrying the same owner + // must remain failed without another syscall or a false successful release. + expect(closeAttempts).toBe(1); }, ); diff --git a/tests/boundary/filesystem/regularFileCancellation.test.ts b/tests/boundary/filesystem/regularFileCancellation.test.ts index 1f4f9dc2ff..4fb159d10b 100644 --- a/tests/boundary/filesystem/regularFileCancellation.test.ts +++ b/tests/boundary/filesystem/regularFileCancellation.test.ts @@ -359,7 +359,7 @@ it.each([false, true])( return withOwnedRegularFile( path, { signal: inner.signal }, - async (nested) => { + async ({ handle: nested }) => { innerHandle = nested; const value = await nested.readFile(); const close = nested.close.bind(nested); @@ -500,7 +500,7 @@ it.each(["completed", "failed"] as const)( signal: controller.signal, retainFailedClose: (owner) => retained.push(owner), }, - async (handle) => { + async ({ handle }) => { const bytes = await handle.readFile(); if (outcome === "failed") throw primary; return bytes; diff --git a/tests/boundary/filesystem/stableFileHashOwnership.test.ts b/tests/boundary/filesystem/stableFileHashOwnership.test.ts index 02cd4efba8..b4bb528fe2 100644 --- a/tests/boundary/filesystem/stableFileHashOwnership.test.ts +++ b/tests/boundary/filesystem/stableFileHashOwnership.test.ts @@ -20,6 +20,38 @@ import { import { PrivateRuntimeRoot } from "../../../src/process/PrivateRuntimeRoot.js"; import type { VerifiedLinuxHopperPackage } from "../../../src/application/LinuxHopperPublisher.js"; +it("rejects a real extent change after admission instead of replacing its baseline", async () => { + const root = await createTestTempDirectory("rea-stable-admitted-extent-"); + const path = join(root, "input.bin"); + const original = Buffer.from("admitted extent"); + await writeFile(path, original); + const nativeOpen = regularFile.openRegularFile; + let descriptor: regularFile.StableRegularFileDescriptor | undefined; + const open = vi + .spyOn(regularFile, "openRegularFile") + .mockImplementationOnce(async (...args) => { + descriptor = await nativeOpen(...args); + await writeFile(path, "a different and longer extent after admission"); + return descriptor; + }); + try { + const failure: unknown = await hashStableFile(path, { + retainFailedClose: () => { + throw new Error("Changed input descriptor must close successfully"); + }, + }).catch((cause: unknown) => cause); + expect(failure).toMatchObject({ _tag: "StableFileHashFailure" }); + if (!(failure instanceof Error)) throw new Error("Expected changed input"); + expect(failure.cause).toBeInstanceOf(regularFile.RegularFileChangedError); + expect(descriptor?.initial.size).toBe(original.length); + expect(descriptor?.handle.fd).toBe(-1); + } finally { + open.mockRestore(); + if (descriptor !== undefined && descriptor.handle.fd >= 0) + await descriptor.handle.close(); + } +}); + it("captures a two-byte prefix across short reads and buffer reuse", async () => { const root = await createTestTempDirectory("rea-stable-observation-prefix-"); const path = join(root, "script"); @@ -30,27 +62,27 @@ it("captures a two-byte prefix across short reads and buffer reuse", async () => const open = vi .spyOn(regularFile, "openRegularFile") .mockImplementation(async (...args) => { - const handle = await originalOpen(...args); - return { - get fd() { - return handle.fd; - }, - stat: (...statArgs: Parameters) => - handle.stat(...statArgs), - read: ( + const descriptor = await originalOpen(...args); + const { handle } = descriptor; + const positional: { + read( buffer: Buffer, offset: number, length: number, position: number, - ) => - handle.read( + ): Promise<{ bytesRead: number; buffer: Buffer }>; + } = handle; + const nativeRead = positional.read.bind(handle); + vi.spyOn(positional, "read").mockImplementation( + (buffer, offset, length, position) => + nativeRead( buffer, offset, position === 0 ? Math.min(length, 1) : length, position, ), - close: () => handle.close(), - } as unknown as Awaited>; + ); + return descriptor; }); try { const observation = await observeStableFile(path, { @@ -121,12 +153,13 @@ it("preserves a read cancellation cause when close also fails", async () => { const open = vi .spyOn(regularFile, "openRegularFile") .mockImplementation(async (selectedPath, options) => { - const handle = await nativeOpen(selectedPath, options); + const descriptor = await nativeOpen(selectedPath, options); + const { handle } = descriptor; vi.spyOn(handle, "stat").mockImplementationOnce(async () => { controller.abort(primary); throw primary; }); - return handle; + return descriptor; }); const originalClose = OwnedFileHandle.prototype.close; const close = vi @@ -270,15 +303,10 @@ it("distinguishes a thrown undefined primary failure from a clean hash", async ( const open = vi .spyOn(regularFile, "openRegularFile") .mockImplementation(async (selectedPath, options) => { - const handle = await openFileAtBoundary(selectedPath, options); - const originalStat = handle.stat.bind(handle); - let calls = 0; - vi.spyOn(handle, "stat").mockImplementation(async () => { - calls += 1; - if (calls === 2) throw undefined; - return originalStat(); - }); - return handle; + const descriptor = await openFileAtBoundary(selectedPath, options); + const { handle } = descriptor; + vi.spyOn(handle, "stat").mockRejectedValueOnce(undefined); + return descriptor; }); const closeCause = new Error("injected close failure after undefined"); const close = vi @@ -454,9 +482,10 @@ it("tags a physical hash failure without inferring cancellation from a later sig const open = vi .spyOn(regularFile, "openRegularFile") .mockImplementation(async (selected, options) => { - const handle = await nativeOpen(selected, options); + const descriptor = await nativeOpen(selected, options); + const { handle } = descriptor; vi.spyOn(handle, "read").mockRejectedValueOnce(primary); - return handle; + return descriptor; }); const nativeClose = OwnedFileHandle.prototype.close; const close = vi @@ -497,12 +526,13 @@ it.each([false, true])( const open = vi .spyOn(regularFile, "openRegularFile") .mockImplementation(async (selected, options) => { - const handle = await nativeOpen(selected, options); + const descriptor = await nativeOpen(selected, options); + const { handle } = descriptor; vi.spyOn(handle, "read").mockImplementationOnce(async () => { controller.abort("cancelled at EOF"); return { bytesRead: 0, buffer: Buffer.alloc(0) }; }); - return handle; + return descriptor; }); const nativeClose = OwnedFileHandle.prototype.close; const close = vi.spyOn(OwnedFileHandle.prototype, "close"); @@ -548,11 +578,14 @@ it.each( const controller = new AbortController(); const nativeOpen = regularFile.openRegularFile; let retained: OwnedFileHandle | undefined; - let handleSeen: Awaited> | undefined; + let handleSeen: + | regularFile.StableRegularFileDescriptor["handle"] + | undefined; const open = vi .spyOn(regularFile, "openRegularFile") .mockImplementation(async (selected, options) => { - const handle = await nativeOpen(selected, options); + const descriptor = await nativeOpen(selected, options); + const { handle } = descriptor; handleSeen = handle; const read = handle.read.bind(handle); vi.spyOn(handle, "read").mockImplementationOnce(async (...args) => { @@ -560,7 +593,7 @@ it.each( controller.abort(reason); return result; }); - return handle; + return descriptor; }); const nativeClose = OwnedFileHandle.prototype.close; const close = vi.spyOn(OwnedFileHandle.prototype, "close"); @@ -605,10 +638,11 @@ it.each(["physical", "completed"] as const)( const open = vi .spyOn(regularFile, "openRegularFile") .mockImplementation(async (selected, options) => { - const handle = await nativeOpen(selected, options); + const descriptor = await nativeOpen(selected, options); + const { handle } = descriptor; if (outcome === "physical") vi.spyOn(handle, "read").mockRejectedValueOnce(primary); - return handle; + return descriptor; }); const nativeClose = OwnedFileHandle.prototype.close; let ownerSeen: OwnedFileHandle | undefined; diff --git a/tests/boundary/process/artifactFileOutcomes.test.ts b/tests/boundary/process/artifactFileOutcomes.test.ts index 012a531a33..3fb9808c59 100644 --- a/tests/boundary/process/artifactFileOutcomes.test.ts +++ b/tests/boundary/process/artifactFileOutcomes.test.ts @@ -154,17 +154,23 @@ const createFileOutcomeFixture = async ({ const scope = args[4]; if (scope === undefined) throw new Error("Expected retained artifact scope"); - return withArtifactRegularFile(args[0], scope, args[2], async (file) => { - handle = file; - const close = file.close.bind(file); - file.close = async () => { - // A later abort must not replace the already observed physical failure. - caller.abort(cancellationReason); - if (!allowed) throw new Error("controlled descriptor close failure"); - await close(); - }; - throw physical; - }); + return withArtifactRegularFile( + args[0], + scope, + args[2], + async ({ handle: file }) => { + handle = file; + const close = file.close.bind(file); + file.close = async () => { + // A later abort must not replace the already observed physical failure. + caller.abort(cancellationReason); + if (!allowed) + throw new Error("controlled descriptor close failure"); + await close(); + }; + throw physical; + }, + ); }, async (limits) => ({ ...limits, file_bytes: 1048576 }), ); diff --git a/tests/boundary/providers/adb/adbHashCleanupConsumers.test.ts b/tests/boundary/providers/adb/adbHashCleanupConsumers.test.ts index 93b385b98b..c1547b38b0 100644 --- a/tests/boundary/providers/adb/adbHashCleanupConsumers.test.ts +++ b/tests/boundary/providers/adb/adbHashCleanupConsumers.test.ts @@ -183,12 +183,13 @@ for (const closeFails of [false, true]) { let opened: FileHandle | undefined; vi.spyOn(regularFile, "openRegularFile").mockImplementation( async (selected, options) => { - const file = await nativeOpen(selected, options); + const descriptor = await nativeOpen(selected, options); + const { handle: file } = descriptor; if (selected === path) { opened = file; vi.spyOn(file, "read").mockRejectedValueOnce(primary); } - return file; + return descriptor; }, ); const nativeClose = OwnedFileHandle.prototype.close; @@ -246,9 +247,10 @@ for (const closeFails of [false, true]) { let opened: FileHandle | undefined; vi.spyOn(regularFile, "openRegularFile").mockImplementation( async (selected, options) => { - const handle = await nativeOpen(selected, options); + const descriptor = await nativeOpen(selected, options); + const { handle } = descriptor; if (selected === path) opened = handle; - return handle; + return descriptor; }, ); const nativeClose = OwnedFileHandle.prototype.close; diff --git a/tests/boundary/providers/ghidra/ghidraFileFailure.test.ts b/tests/boundary/providers/ghidra/ghidraFileFailure.test.ts index fdfaa35c80..ab64340eef 100644 --- a/tests/boundary/providers/ghidra/ghidraFileFailure.test.ts +++ b/tests/boundary/providers/ghidra/ghidraFileFailure.test.ts @@ -32,16 +32,21 @@ const retainFailedDescriptor = async (path: string, primary: unknown) => { let handle: FileHandle | undefined; const failure: unknown = await resources .run(() => - withArtifactRegularFile(path, resources, undefined, async (file) => { - handle = file; - const close = file.close.bind(file); - file.close = async () => { - if (!allowClose) - throw new Error("controlled descriptor close refusal"); - await close(); - }; - throw new OwnedRegularFileOperationFailure(primary); - }), + withArtifactRegularFile( + path, + resources, + undefined, + async ({ handle: file }) => { + handle = file; + const close = file.close.bind(file); + file.close = async () => { + if (!allowClose) + throw new Error("controlled descriptor close refusal"); + await close(); + }; + throw new OwnedRegularFileOperationFailure(primary); + }, + ), ) .catch((cause: unknown) => cause); if (!(failure instanceof ArtifactFileCleanupFailure)) { diff --git a/tests/boundary/providers/kuna/kunaHashOwnerPropagation.test.ts b/tests/boundary/providers/kuna/kunaHashOwnerPropagation.test.ts index 47d6775676..28e49e25e0 100644 --- a/tests/boundary/providers/kuna/kunaHashOwnerPropagation.test.ts +++ b/tests/boundary/providers/kuna/kunaHashOwnerPropagation.test.ts @@ -42,6 +42,10 @@ const injected = vi.hoisted(() => ({ } | undefined, handles: [] as import("node:fs/promises").FileHandle[], + nativeCloses: new Map< + import("node:fs/promises").FileHandle, + () => Promise + >(), })); afterEach(async () => { @@ -56,8 +60,10 @@ afterEach(async () => { injected.failReadStage = undefined; injected.abortOnReadFailure = undefined; injected.abortOnClose = undefined; - for (const handle of injected.handles.splice(0)) - if (handle.fd >= 0) await handle.close(); + for (const [handle, close] of injected.nativeCloses) + if (handle.fd >= 0) await close(); + injected.nativeCloses.clear(); + injected.handles.length = 0; }); vi.mock("../../../../src/filesystem/RegularFile.js", async (importOriginal) => { @@ -69,7 +75,12 @@ vi.mock("../../../../src/filesystem/RegularFile.js", async (importOriginal) => { ...original, openRegularFile: vi.fn( async (...args: Parameters) => { - const handle = await original.openRegularFile(...args); + const descriptor = await original.openRegularFile(...args); + const { handle } = descriptor; + // Test teardown retains native release separately from the producer's + // injected close behavior, including when an assertion fails early. + const nativeClose = handle.close.bind(handle); + injected.nativeCloses.set(handle, nativeClose); const isInstallation = String(args[0]) === injected.installationPath; injected.handles.push(handle); const failNextClose = injected.failNextClose; @@ -83,7 +94,7 @@ vi.mock("../../../../src/filesystem/RegularFile.js", async (importOriginal) => { !failReadStage && !abortOnClose ) - return handle; + return descriptor; injected.failNextClose = false; let readStageMatched = false; let failedClose = false; @@ -103,13 +114,17 @@ vi.mock("../../../../src/filesystem/RegularFile.js", async (importOriginal) => { stage === "fingerprint" || stage === "identity", ); - return { - get fd() { - return handle.fd; - }, - stat: (...statArgs: Parameters) => - handle.stat(...statArgs), - read: ( + const positional: { + read( + buffer: Buffer, + offset: number, + length: number, + position: number, + ): Promise<{ bytesRead: number; buffer: Buffer }>; + } = handle; + const nativeRead = positional.read.bind(handle); + vi.spyOn(positional, "read").mockImplementation( + ( buffer: Buffer, offset: number, length: number, @@ -144,22 +159,23 @@ vi.mock("../../../../src/filesystem/RegularFile.js", async (importOriginal) => { failCloseReadStage === readStage ) readStageMatched = true; - return handle.read(buffer, offset, length, position); + return nativeRead(buffer, offset, length, position); }, - close: async () => { - injected.closeCalls += 1; - if (abortCloseMatched) abortOnClose?.abort(); - if ( - (failNextClose && !failedClose) || - (readStageMatched && injected.failReadClose && !failedClose) || - (injected.holdCloseFailures && !injected.releaseCloseFailures) - ) { - failedClose = true; - throw new Error("injected close fault"); - } - await handle.close(); - }, - } as unknown as typeof handle; + ); + vi.spyOn(handle, "close").mockImplementation(async () => { + injected.closeCalls += 1; + if (abortCloseMatched) abortOnClose?.abort(); + if ( + (failNextClose && !failedClose) || + (readStageMatched && injected.failReadClose && !failedClose) || + (injected.holdCloseFailures && !injected.releaseCloseFailures) + ) { + failedClose = true; + throw new Error("injected close fault"); + } + await nativeClose(); + }); + return descriptor; }, ), }; diff --git a/tests/boundary/providers/kuna/kunaInstallationLifetime.test.ts b/tests/boundary/providers/kuna/kunaInstallationLifetime.test.ts index 00e5031a42..aded1b35b9 100644 --- a/tests/boundary/providers/kuna/kunaInstallationLifetime.test.ts +++ b/tests/boundary/providers/kuna/kunaInstallationLifetime.test.ts @@ -32,31 +32,28 @@ vi.mock("../../../../src/filesystem/RegularFile.js", async (importOriginal) => { ...original, openRegularFile: vi.fn( async (...args: Parameters) => { - const handle = await original.openRegularFile(...args); + const descriptor = await original.openRegularFile(...args); + const { handle } = descriptor; const index = gates.opens++; - return { - get fd() { - return handle.fd; - }, - stat: (...statArgs: Parameters) => - handle.stat(...statArgs), - read: async (...readArgs: Parameters) => { - if (index === 1 && gates.holdIdentityRead) { - gates.identityRead.enter(); - await gates.identityRead.hold; - } - return handle.read(...readArgs); - }, - close: async () => { - if (index === 1 && gates.holdHashClose) { - gates.hashClose.enter(); - await gates.hashClose.hold; - } - if (gates.failClose) - throw new Error("controlled final close refusal"); - await handle.close(); - }, - } as unknown as typeof handle; + const read = handle.read.bind(handle); + const close = handle.close.bind(handle); + vi.spyOn(handle, "read").mockImplementation(async (...readArgs) => { + if (index === 1 && gates.holdIdentityRead) { + gates.identityRead.enter(); + await gates.identityRead.hold; + } + return read(...readArgs); + }); + vi.spyOn(handle, "close").mockImplementation(async () => { + if (index === 1 && gates.holdHashClose) { + gates.hashClose.enter(); + await gates.hashClose.hold; + } + if (gates.failClose) + throw new Error("controlled final close refusal"); + await close(); + }); + return descriptor; }, ), }; diff --git a/tests/process-global/binaryTargetAdmissionCancellation.test.ts b/tests/process-global/binaryTargetAdmissionCancellation.test.ts index 6347346f2b..898afbeea2 100644 --- a/tests/process-global/binaryTargetAdmissionCancellation.test.ts +++ b/tests/process-global/binaryTargetAdmissionCancellation.test.ts @@ -105,8 +105,12 @@ it.each([ "classifies $name cancellation at descriptor stat $checkpoint and closes the file", async ({ checkpoint, reason }) => { const { path, controller } = await fixture(checkpoint, reason); + // A valid selected load reaches the second stat during hash finalization. const resolved = await parseBinaryTarget(path, { signal: controller.signal, + formatHint: "raw-binary", + architecture: "x86", + entryPoint: "0x1000", }); expect(resolved.ok).toBe(false); if (resolved.ok) throw new Error("cancelled target was admitted"); diff --git a/tests/process-global/regularFileNoFollowFallback.test.ts b/tests/process-global/regularFileNoFollowFallback.test.ts index 44ce66c7c0..0e6e549bbd 100644 --- a/tests/process-global/regularFileNoFollowFallback.test.ts +++ b/tests/process-global/regularFileNoFollowFallback.test.ts @@ -46,7 +46,9 @@ it("rejects a symlink when O_NOFOLLOW is unavailable and still follows when requ await linkedReader.close(); } - const followed = await openRegularFile(link, { symlinks: "follow" }); + const { handle: followed } = await openRegularFile(link, { + symlinks: "follow", + }); try { await expect(readFile(followed)).resolves.toEqual(await readFile(target)); } finally { diff --git a/tests/process-global/selectedArtifactSourceLifecycle.test.ts b/tests/process-global/selectedArtifactSourceLifecycle.test.ts index cc76e8ef82..c6d2536e80 100644 --- a/tests/process-global/selectedArtifactSourceLifecycle.test.ts +++ b/tests/process-global/selectedArtifactSourceLifecycle.test.ts @@ -405,86 +405,3 @@ it.each([false, true])( } }, ); - -it.each([false, true])( - "closes an owned Mach-O source before rejecting its initial stat failure (invalidatingCloseFailure=%s)", - async (invalidatingCloseFailure) => { - const fixture = await machoCarrier(); - const statFailure = Object.assign( - new Error("post-admission initial stat failed"), - { - code: "EIO", - errno: -5, - syscall: "fstat", - }, - ); - const cleanupFailure = new Error( - "close rejected after descriptor invalidation", - ); - const originalAdmission = regularFile.openRegularFile; - vi.spyOn(regularFile, "openRegularFile").mockImplementation( - async (...args) => { - const handle = await originalAdmission(...args); - if (args[0] === fixture.path) { - // Admission has completed; the owner's next stat establishes its initial facts. - vi.spyOn(handle, "stat").mockRejectedValueOnce(statFailure); - const close = handle.close.bind(handle); - vi.spyOn(handle, "close").mockImplementation(async () => { - await close(); - if (invalidatingCloseFailure) throw cleanupFailure; - }); - } - return handle; - }, - ); - const run = vi.spyOn(fixture.runner, "run"); - const reader = new MachOSliceArtifactReader( - fixture.path, - {}, - fixture.runner, - ); - try { - const failure = await entries(reader).catch((cause: unknown) => { - const handle = openedHandle(); - expect(handle.close).toHaveBeenCalledOnce(); - expect(handle.fd).toBe(-1); - return cause; - }); - if (!(failure instanceof ArtifactReaderFailure)) - throw new Error("Expected the primary initial-stat failure"); - expect(failure.reason).toBe("io"); - const primary = invalidatingCloseFailure ? failure.cause : failure; - if (!(primary instanceof ArtifactReaderFailure)) - throw new Error("Missing initial-stat primary failure"); - expect(primary.cause).toBe(statFailure); - const handle = openedHandle(); - if (invalidatingCloseFailure) { - expect(failure.cleanup).toEqual({ - reason: cleanupFailure.message, - resources: [fixture.path], - }); - expect( - await Promise.allSettled([reader.close(), reader.close()]), - ).toEqual([ - { status: "rejected", reason: cleanupFailure }, - { status: "rejected", reason: cleanupFailure }, - ]); - await expect(reader.close()).rejects.toBe(cleanupFailure); - } else { - expect(failure.cleanup).toBeUndefined(); - await Promise.all([reader.close(), reader.close()]); - await reader.close(); - } - expect(handle.close).toHaveBeenCalledOnce(); - expect(handle.fd).toBe(-1); - expect(run).not.toHaveBeenCalled(); - expect(control.handles.size).toBe(1); - } finally { - if (invalidatingCloseFailure) - await reader.close().catch((cause: unknown) => { - expect(cause).toBe(cleanupFailure); - }); - else await reader.close(); - } - }, -);