diff --git a/src/artifacts/pyinstaller/PyInstallerFileAdmission.ts b/src/artifacts/ArtifactFileAdmission.ts similarity index 58% rename from src/artifacts/pyinstaller/PyInstallerFileAdmission.ts rename to src/artifacts/ArtifactFileAdmission.ts index dc3c503998..fa18c998e0 100644 --- a/src/artifacts/pyinstaller/PyInstallerFileAdmission.ts +++ b/src/artifacts/ArtifactFileAdmission.ts @@ -3,41 +3,44 @@ import { RegularFileAdmissionFailure, RegularFileChangedError, RegularFileOperationCancelled, -} from "../../filesystem/RegularFile.js"; -import { ArtifactReaderFailure } from "../ArtifactReader.js"; +} from "../filesystem/RegularFile.js"; +import { ArtifactReaderFailure } from "./ArtifactReader.js"; -/** Both carrier and private-image admission preserve cancellation and cleanup. */ -export const pyInstallerFileAdmissionFailure = ( +/** Project physical admission failures without taking the caller's descriptor ownership. */ +export const artifactFileAdmissionFailure = ( path: string, + diagnosticLabel: string, cause: unknown, ): ArtifactReaderFailure => { if (cause instanceof ArtifactReaderFailure) return cause; if (cause instanceof RegularFileOperationCancelled) return new ArtifactReaderFailure( "cancelled", - "PyInstaller file admission cancelled", + `${diagnosticLabel} admission cancelled`, { cause: cause.reason }, ); if (cause instanceof RegularFileAdmissionFailure) return ArtifactReaderFailure.withCleanup( - pyInstallerFileAdmissionFailure(path, cause.cause), + artifactFileAdmissionFailure(path, diagnosticLabel, cause.cause), ArtifactReaderFailure.cleanupObservation(cause.cleanupCause, path), ); if (cause instanceof RegularFileChangedError) return new ArtifactReaderFailure( "integrity", - `PyInstaller source changed during admission: ${path}`, + `${diagnosticLabel} source changed during admission: ${path}`, { cause }, ); if (cause instanceof NonRegularFileReadError) return new ArtifactReaderFailure( "format", - `PyInstaller source is not a regular file: ${path}`, + `${diagnosticLabel} source is not a regular file: ${path}`, { cause }, ); return new ArtifactReaderFailure( "io", - "PyInstaller archive could not be opened", + cause instanceof Error + ? cause.message + : `Cannot admit ${diagnosticLabel} source: ${path}`, { cause }, ); }; diff --git a/src/artifacts/ArtifactFileSource.ts b/src/artifacts/ArtifactFileSource.ts index 4eccd7f0dc..4af676b0a8 100644 --- a/src/artifacts/ArtifactFileSource.ts +++ b/src/artifacts/ArtifactFileSource.ts @@ -1,15 +1,15 @@ +import { lstat } from "node:fs/promises"; import { artifactIntegrityDetails } from "../domain/artifactIntegrity.js"; import { OwnedFileHandle } from "../filesystem/OwnedFileHandle.js"; import { readFileHandleChunks } from "../filesystem/readFileHandleChunks.js"; import { - NonRegularFileReadError, RegularFileAdmissionFailure, - RegularFileOperationCancelled, openRegularFile, sameRegularFileState, type StableRegularFileDescriptor, } from "../filesystem/RegularFile.js"; import { assertArtifactReadActive } from "./ArtifactFileOperation.js"; +import { artifactFileAdmissionFailure } from "./ArtifactFileAdmission.js"; import { hashReadable } from "./ArtifactHash.js"; import { ArtifactReaderFailure } from "./ArtifactReader.js"; import { ArtifactResourceScope } from "./ArtifactResourceScope.js"; @@ -41,41 +41,31 @@ export class ArtifactFileSource { return this.#scope.run(() => this.#verify(signal)); } + /** Verify both the descriptor and the file currently at the caller-selected path. */ + readSelectedPath(signal?: AbortSignal): Promise { + return this.#scope.run(() => this.#verifySelectedPath(signal)); + } + /** Bind verified bytes to the inventory's expected whole-carrier identity. */ bindSha256(expectedSha256: string, signal?: AbortSignal): Promise { - return this.#scope.run(async () => { - const { handle } = await this.#verify(signal); - const observed = - this.#boundSha256 ?? - ( - await hashReadable( - readFileHandleChunks(handle, { - signal, - cancellationFailure: (reason) => - new ArtifactReaderFailure( - "cancelled", - `${this.diagnosticLabel} verification cancelled`, - { cause: reason }, - ), - }), - signal, - ) - ).sha256; - await this.#verify(signal); - if (observed !== expectedSha256) - throw new ArtifactReaderFailure( - "integrity", - `${this.diagnosticLabel} carrier changed after inventory`, - undefined, - artifactIntegrityDetails( - ".", - { algorithm: "sha256", digest: expectedSha256 }, - observed, - false, - ), - ); - this.#boundSha256 = observed; - }); + return this.#scope.run(() => + this.#bind(expectedSha256, signal, () => this.#verify(signal)), + ); + } + + /** Bind carrier bytes while the selected pathname continues to name the same source. */ + bindSelectedPathSha256( + expectedSha256: string, + signal?: AbortSignal, + ): Promise { + return this.#scope.run(() => + this.#bind( + expectedSha256, + signal, + () => this.#verifySelectedPath(signal), + () => this.#verifySelectedPath(), + ), + ); } /** Join admitted operations before closing; failed descriptor ownership stays retained. */ @@ -83,6 +73,46 @@ export class ArtifactFileSource { return this.#scope.close(); } + async #bind( + expectedSha256: string, + signal: AbortSignal | undefined, + verify: () => Promise, + verifyCompletion: () => Promise = verify, + ): Promise { + const { handle } = await verify(); + const observed = + this.#boundSha256 ?? + ( + await hashReadable( + readFileHandleChunks(handle, { + signal, + cancellationFailure: (reason) => + new ArtifactReaderFailure( + "cancelled", + `${this.diagnosticLabel} verification cancelled`, + { cause: reason }, + ), + }), + signal, + ) + ).sha256; + await verifyCompletion(); + assertArtifactReadActive(signal); + if (observed !== expectedSha256) + throw new ArtifactReaderFailure( + "integrity", + `${this.diagnosticLabel} carrier changed after inventory`, + undefined, + artifactIntegrityDetails( + ".", + { algorithm: "sha256", digest: expectedSha256 }, + observed, + false, + ), + ); + this.#boundSha256 = observed; + } + async #verify(signal?: AbortSignal): Promise { assertArtifactReadActive(signal); this.#opening ??= this.#admit(signal); @@ -97,50 +127,65 @@ export class ArtifactFileSource { return source; } - async #admit(signal?: AbortSignal): Promise { - try { - const handle = - this.admitted?.handle ?? - (await openRegularFile(this.path, { symlinks: "reject", signal })); - if (this.admitted === undefined) - this.#owner = new OwnedFileHandle(handle); - const initial = this.admitted?.initial ?? (await handle.stat()); - return { handle, initial }; - } catch (cause: unknown) { - if (cause instanceof RegularFileAdmissionFailure) { - this.#owner = cause.owner; - throw ArtifactReaderFailure.withCleanup( - this.#admissionFailure(cause.cause), - ArtifactReaderFailure.cleanupObservation( - cause.cleanupCause, - this.path, - ), - ); - } - throw this.#admissionFailure(cause); - } + async #verifySelectedPath( + signal?: AbortSignal, + ): Promise { + assertArtifactReadActive(signal); + this.#opening ??= this.#admit(signal); + const source = await this.#opening; + const [observed, selected] = await Promise.all([ + source.handle.stat(), + lstat(this.path), + ]); + if ( + !sameRegularFileState(source.initial, observed) || + !sameRegularFileState(source.initial, selected) + ) + throw new ArtifactReaderFailure( + "integrity", + `${this.diagnosticLabel} source identity changed: ${this.path}`, + ); + assertArtifactReadActive(signal); + return source; } - #admissionFailure(cause: unknown): ArtifactReaderFailure { - if (cause instanceof ArtifactReaderFailure) return cause; - if (cause instanceof RegularFileOperationCancelled) - return new ArtifactReaderFailure( - "cancelled", - `${this.diagnosticLabel} admission cancelled`, - { cause: cause.reason }, + async #admit(signal?: AbortSignal): Promise { + try { + if (this.admitted !== undefined) + return { + handle: this.admitted.handle, + initial: this.admitted.initial, + }; + const handle = 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; + } + } catch (cause: unknown) { + if (cause instanceof RegularFileAdmissionFailure) + this.#owner = cause.owner; + throw artifactFileAdmissionFailure( + this.path, + this.diagnosticLabel, + cause, ); - if (cause instanceof NonRegularFileReadError) - return new ArtifactReaderFailure( - "format", - `${this.diagnosticLabel} source is not a regular file: ${this.path}`, - { cause }, - ); - return new ArtifactReaderFailure( - "io", - cause instanceof Error - ? cause.message - : `Cannot admit ${this.diagnosticLabel} source: ${this.path}`, - { cause }, - ); + } } } diff --git a/src/artifacts/MachOSliceArtifactReader.ts b/src/artifacts/MachOSliceArtifactReader.ts index 791e8a5978..510c0a071d 100644 --- a/src/artifacts/MachOSliceArtifactReader.ts +++ b/src/artifacts/MachOSliceArtifactReader.ts @@ -1,5 +1,4 @@ import { assertArtifactReadActive } from "./ArtifactFileOperation.js"; -import { lstat } from "node:fs/promises"; import type { FileHandle } from "node:fs/promises"; import { Readable } from "node:stream"; @@ -17,7 +16,6 @@ import { machoArchitectureFamily, machoCpuSubtypeBase, } from "../domain/apple/machoCpuIdentity.js"; -import { OwnedFileHandle } from "../filesystem/OwnedFileHandle.js"; import { readMachoImage, type ReadAt } from "./apple/MachoLoadCommandReader.js"; import { ArtifactReaderFailure, @@ -27,25 +25,17 @@ import { type ArtifactReader, } from "./ArtifactReader.js"; import { streamChunkToBuffer } from "./StreamBytes.js"; -import { - NonRegularFileReadError, - RegularFileAdmissionFailure, - openRegularFile, - sameRegularFileState, - type StableRegularFileDescriptor, -} from "../filesystem/RegularFile.js"; +import type { StableRegularFileDescriptor } from "../filesystem/RegularFile.js"; import { readArtifactFileChunks } from "./readArtifactFileChunks.js"; +import { ArtifactFileSource } from "./ArtifactFileSource.js"; /** Read-only universal Mach-O slice reader backed by native lipo metadata. */ export class MachOSliceArtifactReader implements ArtifactReader { readonly format = "file" as const; #command: ArtifactCommand | undefined; - #source: StableRegularFileDescriptor | undefined; - #ownedSource: OwnedFileHandle | undefined; - #pendingSource: Promise | undefined; + readonly #source: ArtifactFileSource; readonly #entries = new Map(); readonly #pendingCommands = new Set>(); - readonly #ownsSource: boolean; #closed = false; #closePromise: Promise | undefined; @@ -57,14 +47,13 @@ export class MachOSliceArtifactReader implements ArtifactReader { ), source?: StableRegularFileDescriptor, ) { - this.#source = source; - this.#ownsSource = source === undefined; + this.#source = new ArtifactFileSource(path, "Universal Mach-O", source); } async *entries(signal?: AbortSignal): AsyncIterable { this.#entries.clear(); const source = await this.#ensureSource(); - await this.#verifySource(source); + await this.#source.readSelectedPath(); if (this.#closed) throw new ArtifactReaderFailure( "unavailable", @@ -116,7 +105,7 @@ export class MachOSliceArtifactReader implements ArtifactReader { exit_code: captured.value.exitCode, effects: ["read"], }; - await this.#verifySource(source); + await this.#source.readSelectedPath(); const fileSize = source.initial.size; const parsed = parseLipoOutputResult(captured.value.stdout, this.path); if (!parsed.ok) @@ -128,7 +117,7 @@ export class MachOSliceArtifactReader implements ArtifactReader { const lipo = parsed.value; const { architectures } = lipo; const structural = await readStructuralSlices(source.handle, fileSize); - await this.#verifySource(source); + await this.#source.readSelectedPath(); if (structural.status === "malformed") throw new ArtifactReaderFailure( "integrity", @@ -306,7 +295,7 @@ export class MachOSliceArtifactReader implements ArtifactReader { "Invalid Mach-O slice byte range", ); const source = await this.#ensureSource(); - await this.#verifySource(source); + await this.#source.readSelectedPath(); if ( !Number.isSafeInteger(offset + size) || offset + size > source.initial.size @@ -320,7 +309,7 @@ export class MachOSliceArtifactReader implements ArtifactReader { end: offset + size - 1, ...(signal === undefined ? {} : { signal }), }); - const verifySource = () => this.#verifySource(source); + const verifySource = () => this.#source.readSelectedPath(); let validatingSource = false; const completionFailure = (): ArtifactReaderFailure | undefined => { const coverage = sourceStream.readCoverage(); @@ -456,7 +445,7 @@ export class MachOSliceArtifactReader implements ArtifactReader { ); } try { - await this.#closeSource(); + await this.#source.close(); } catch (cause: unknown) { if (cleanup === undefined) throw cause; throw ArtifactReaderFailure.withCleanup(cause, cleanup); @@ -475,9 +464,7 @@ export class MachOSliceArtifactReader implements ArtifactReader { "unavailable", "Universal Mach-O reader is closed", ); - if (this.#source !== undefined) return this.#source; - this.#pendingSource ??= this.#acquireSource(); - const source = await this.#pendingSource; + const source = await this.#source.read(); if (this.#closed) throw new ArtifactReaderFailure( "unavailable", @@ -485,70 +472,6 @@ export class MachOSliceArtifactReader implements ArtifactReader { ); return source; } - - async #acquireSource(): Promise { - let handle: FileHandle; - try { - handle = await openRegularFile(this.path, { symlinks: "reject" }); - } catch (cause: unknown) { - if (!(cause instanceof RegularFileAdmissionFailure)) throw cause; - this.#ownedSource = cause.owner; - const primary = - cause.cause instanceof NonRegularFileReadError - ? new ArtifactReaderFailure( - "format", - `Universal Mach-O source is not a regular file: ${this.path}`, - { cause: cause.cause }, - ) - : cause.cause; - throw ArtifactReaderFailure.withCleanup( - primary, - ArtifactReaderFailure.cleanupObservation(cause.cleanupCause, this.path), - ); - } - const owner = new OwnedFileHandle(handle); - this.#ownedSource = owner; - try { - const initial = await handle.stat(); - const source = { handle, initial }; - this.#source = source; - return source; - } catch (cause: unknown) { - try { - await owner.close(); - this.#ownedSource = undefined; - } catch (cleanupCause: unknown) { - throw ArtifactReaderFailure.withCleanup( - cause, - ArtifactReaderFailure.cleanupObservation(cleanupCause, this.path), - ); - } - throw cause; - } - } - - async #closeSource(): Promise { - await this.#pendingSource?.catch(() => undefined); - if (!this.#ownsSource || this.#ownedSource === undefined) return; - await this.#ownedSource.close(); - this.#ownedSource = undefined; - this.#source = undefined; - } - - async #verifySource(source: StableRegularFileDescriptor): Promise { - const [opened, currentPath] = await Promise.all([ - source.handle.stat(), - lstat(this.path), - ]); - if ( - !sameRegularFileState(source.initial, opened) || - !sameRegularFileState(source.initial, currentPath) - ) - throw new ArtifactReaderFailure( - "integrity", - `Universal Mach-O source changed during inventory: ${this.path}`, - ); - } } /** Compare lipo's independent ranges with structurally parsed slice identity. */ diff --git a/src/artifacts/PyInstallerArtifactReader.ts b/src/artifacts/PyInstallerArtifactReader.ts index 0607105bd7..ea37310538 100644 --- a/src/artifacts/PyInstallerArtifactReader.ts +++ b/src/artifacts/PyInstallerArtifactReader.ts @@ -4,7 +4,7 @@ import { setImmediate } from "node:timers/promises"; import { jsonValueByteLength } from "../domain/jsonValueByteLength.js"; import type { JsonValue } from "../domain/jsonValue.js"; import { normalizeArtifactPath } from "./ArtifactPaths.js"; -import { lstat, type FileHandle } from "node:fs/promises"; +import type { FileHandle } from "node:fs/promises"; import type { PyInstallerMetadata } from "../domain/artifactFormatMetadata.js"; import { @@ -12,13 +12,7 @@ import { scriptPythonMagic, } from "./pyinstaller/PyInstallerMetadata.js"; import { artifactExpansionBudgetBytes } from "../domain/artifactExpansionBudget.js"; -import { OwnedFileHandle } from "../filesystem/OwnedFileHandle.js"; -import { - RegularFileAdmissionFailure, - openRegularFile, - sameRegularFileState, - type StableRegularFileDescriptor, -} from "../filesystem/RegularFile.js"; +import type { StableRegularFileDescriptor } from "../filesystem/RegularFile.js"; import { ArtifactReaderFailure, sameArtifactEntry, @@ -27,10 +21,9 @@ import { type ArtifactReaderTraversal, type ArtifactTraversalOptions, } from "./ArtifactReader.js"; -import { abortIfNeeded, hashReadable } from "./ArtifactHash.js"; -import { readArtifactFileChunks } from "./readArtifactFileChunks.js"; +import { abortIfNeeded } from "./ArtifactHash.js"; +import { ArtifactFileSource } from "./ArtifactFileSource.js"; import { ArtifactResourceScope } from "./ArtifactResourceScope.js"; -import { artifactIntegrityDetails } from "../domain/artifactIntegrity.js"; import { parseMzHeaderEvidence } from "../domain/dosMz.js"; import { readFatContainerHeader, @@ -57,7 +50,6 @@ import { } from "./pyinstaller/CArchiveMembers.js"; import { readPyzArchive } from "./pyinstaller/PyzArchive.js"; import type { PyInstallerFailure } from "./pyinstaller/PyInstallerFailure.js"; -import { pyInstallerFileAdmissionFailure } from "./pyinstaller/PyInstallerFileAdmission.js"; import { readPyInstallerBytes } from "./pyinstaller/PyInstallerBytes.js"; const MAX_PARSED_FACT_BYTES = 32 * 1024 * 1024; @@ -117,13 +109,11 @@ interface LoadedArchive { */ export class PyInstallerArtifactReader implements ArtifactReader { readonly format = "pyinstaller" as const; - #source: StableRegularFileDescriptor | undefined; - #opening: Promise | undefined; + readonly #source: ArtifactFileSource; #closed = false; readonly #scope = new ArtifactResourceScope(); readonly #cleanup = new ArtifactResourceScope(); readonly #lifecycle = new AbortController(); - #boundSha256: string | undefined; #loaded: Promise | undefined; #budget = 0; #charged = 0; @@ -134,16 +124,18 @@ export class PyInstallerArtifactReader implements ArtifactReader { readonly #members = new Map(); readonly #openedPyzSizes = new Map(); - constructor( - private readonly path: string, - source?: StableRegularFileDescriptor, - ) { - this.#source = source; + constructor(path: string, source?: StableRegularFileDescriptor) { + this.#source = new ArtifactFileSource(path, "PyInstaller", source); this.#cleanup.retain({ kind: "lease", resource: "PyInstaller private PYZ images", lease: { release: () => this.#pyzStore.close() }, }); + this.#cleanup.retain({ + kind: "lease", + resource: path, + lease: { release: () => this.#source.close() }, + }); } formatMetadata(): PyInstallerMetadata | undefined { @@ -156,31 +148,8 @@ export class PyInstallerArtifactReader implements ArtifactReader { ): Promise { const active = this.#signal(signal); await this.#scope.run(async () => { - const handle = await this.#openHandle(active); - await this.#verifySource(); - const observed = - this.#boundSha256 ?? - ( - await hashReadable( - readArtifactFileChunks(handle, { signal: active }), - active, - ) - ).sha256; - await this.#verifySource(); - abortIfNeeded(active); - if (observed !== expectedSha256) - throw new ArtifactReaderFailure( - "integrity", - "PyInstaller carrier changed after inventory", - undefined, - artifactIntegrityDetails( - ".", - { algorithm: "sha256", digest: expectedSha256 }, - observed, - false, - ), - ); - this.#boundSha256 = observed; + await this.#openHandle(active); + await this.#source.bindSelectedPathSha256(expectedSha256, active); }); } @@ -191,7 +160,7 @@ export class PyInstallerArtifactReader implements ArtifactReader { const active = this.#signal(signal); const loaded = await this.#scope.run(async () => { const value = await this.#load(active); - await this.#verifySource(); + await this.#source.readSelectedPath(); this.#members.clear(); return value; }); @@ -229,7 +198,7 @@ export class PyInstallerArtifactReader implements ArtifactReader { "format", `PyInstaller entry has no readable bytes: ${entry.path}`, ); - await this.#verifySource(); + const { handle } = await this.#source.readSelectedPath(); const knownSize = read.kind === "pyz" ? (read.uncompressedSize ?? @@ -237,11 +206,7 @@ export class PyInstallerArtifactReader implements ArtifactReader { : undefined; if (read.kind === "pyz" && knownSize === undefined) this.#charge(read.header?.length ?? 0); - const parts = await this.#pyzStore.memberSource( - read, - this.#source?.handle, - active, - ); + const parts = await this.#pyzStore.memberSource(read, handle, active); if (active.aborted) { parts.source.destroy(); await finished(parts.source, { cleanup: true }).catch(() => undefined); @@ -273,7 +238,7 @@ export class PyInstallerArtifactReader implements ArtifactReader { complete: async (bytes) => { try { await parts.verify?.(); - await this.#verifySource(); + await this.#source.readSelectedPath(); abortIfNeeded(active); } catch (cause: unknown) { await closeSource(cause); @@ -320,7 +285,6 @@ export class PyInstallerArtifactReader implements ArtifactReader { this.#lifecycle.abort(closing); await this.#scope.close(); await this.#cleanup.close(); - this.#source = undefined; this.#closed = true; this.#loaded = undefined; this.#members.clear(); @@ -345,9 +309,9 @@ export class PyInstallerArtifactReader implements ArtifactReader { } async #parse(signal?: AbortSignal): Promise { - const handle = await this.#openHandle(signal); - await this.#verifySource(); - const size = (await handle.stat()).size; + await this.#openHandle(signal); + const { handle, initial } = await this.#source.readSelectedPath(); + const size = initial.size; const pathDialect = await carrierPathDialect(handle, size, signal); const cookie = await this.#cookie(handle, size, signal); this.#budget = artifactExpansionBudgetBytes(cookie.packageLength); @@ -374,7 +338,7 @@ export class PyInstallerArtifactReader implements ArtifactReader { for (const member of members) if (member.read?.kind === "carchive") this.#charge(member.read.header?.length ?? 0); - await this.#verifySource(); + await this.#source.readSelectedPath(); this.#metadata = cookieMetadata(cookie, toc, encrypted); let opaqueImages = 0; for (const image of images.values()) @@ -400,13 +364,11 @@ export class PyInstallerArtifactReader implements ArtifactReader { "unavailable", "PyInstaller archive is closed", ); - if (this.#source !== undefined) return this.#source.handle; // Admission belongs to the reader. A departing caller cannot cancel a peer // or replace a late descriptor owner; every caller joins the same admission. - this.#opening ??= this.#admit(); let handle: FileHandle; try { - handle = await this.#opening; + ({ handle } = await this.#source.read(this.#lifecycle.signal)); } catch (cause: unknown) { if ( cause instanceof ArtifactReaderFailure && @@ -424,26 +386,6 @@ export class PyInstallerArtifactReader implements ArtifactReader { return handle; } - async #admit(): Promise { - try { - const handle = await openRegularFile(this.path, { - symlinks: "reject", - signal: this.#lifecycle.signal, - }); - this.#retainCarrier(new OwnedFileHandle(handle)); - this.#source = { handle, initial: await handle.stat() }; - return handle; - } catch (cause: unknown) { - if (cause instanceof RegularFileAdmissionFailure) - this.#retainCarrier(cause.owner); - throw pyInstallerFileAdmissionFailure(this.path, cause); - } - } - - #retainCarrier(handle: OwnedFileHandle): void { - this.#cleanup.retain({ kind: "file-handle", resource: this.path, handle }); - } - async #cookie( handle: FileHandle, size: number, @@ -606,27 +548,6 @@ export class PyInstallerArtifactReader implements ArtifactReader { return members; } - async #verifySource(): Promise { - const source = this.#source; - if (source === undefined) - throw new ArtifactReaderFailure( - "unavailable", - "PyInstaller archive is closed", - ); - const [opened, currentPath] = await Promise.all([ - source.handle.stat(), - lstat(this.path), - ]); - if ( - !sameRegularFileState(source.initial, opened) || - !sameRegularFileState(source.initial, currentPath) - ) - throw new ArtifactReaderFailure( - "integrity", - `PyInstaller source changed during reading: ${this.path}`, - ); - } - #reservePathComponents(path: string): void { const remaining = MAX_RETAINED_ENTRY_BYTES - this.#retainedEntryBytes; // Case annotations can carry the path and two prefixes at every component. diff --git a/src/artifacts/pyinstaller/PyInstallerPyzStore.ts b/src/artifacts/pyinstaller/PyInstallerPyzStore.ts index 4b5b053706..30ae081bae 100644 --- a/src/artifacts/pyinstaller/PyInstallerPyzStore.ts +++ b/src/artifacts/pyinstaller/PyInstallerPyzStore.ts @@ -23,7 +23,7 @@ import { } from "./PyInstallerMemberStream.js"; import type { CArchiveEntry } from "./CArchiveLayout.js"; import type { PyInstallerMember } from "./CArchiveMembers.js"; -import { pyInstallerFileAdmissionFailure } from "./PyInstallerFileAdmission.js"; +import { artifactFileAdmissionFailure } from "../ArtifactFileAdmission.js"; import { reconstructedPycHeader, isHeaderlessPythonCode, @@ -242,7 +242,7 @@ export class PyInstallerPyzStore { } catch (cause: unknown) { if (cause instanceof RegularFileAdmissionFailure) this.#retain(path, cause.owner); - throw pyInstallerFileAdmissionFailure(path, cause); + throw artifactFileAdmissionFailure(path, "PyInstaller", cause); } } diff --git a/tests/fixtures/lifecycleAcknowledgement.ts b/tests/fixtures/lifecycleAcknowledgement.ts new file mode 100644 index 0000000000..0789185325 --- /dev/null +++ b/tests/fixtures/lifecycleAcknowledgement.ts @@ -0,0 +1,28 @@ +/** Fail promptly if an operation never reaches its controlled lifecycle boundary. */ +export const awaitAcknowledgement = async ( + acknowledgement: Promise, + operation: Promise, + boundary: string, +): Promise => { + let deadline: ReturnType | undefined; + try { + const observed = await Promise.race([ + acknowledgement.then(() => ({ kind: "acknowledged" }) as const), + operation.then( + (value) => ({ kind: "settled", status: "fulfilled", value }) as const, + (cause: unknown) => + ({ kind: "settled", status: "rejected", cause }) as const, + ), + new Promise<{ readonly kind: "deadline" }>((resolve) => { + deadline = setTimeout(() => resolve({ kind: "deadline" }), 2_000); + }), + ]); + if (observed.kind !== "acknowledged") + throw new Error( + `${boundary} was not acknowledged before ${observed.kind === "deadline" ? "the gate deadline" : "the triggering operation settled"}`, + { cause: observed }, + ); + } finally { + if (deadline !== undefined) clearTimeout(deadline); + } +}; diff --git a/tests/process-global/artifactFileSourceLifecycle.test.ts b/tests/process-global/artifactFileSourceLifecycle.test.ts index 7bf58a85a4..e06e680368 100644 --- a/tests/process-global/artifactFileSourceLifecycle.test.ts +++ b/tests/process-global/artifactFileSourceLifecycle.test.ts @@ -23,6 +23,7 @@ import { nodeSeaElf } from "../../src/artifacts/nodeSea/NodeSea.fixture.js"; import type { StableRegularFileDescriptor } from "../../src/filesystem/RegularFile.js"; import { createDeferred } from "../fixtures/binarySession.js"; import { buildGodotPck } from "../fixtures/godotPck.js"; +import { awaitAcknowledgement } from "../fixtures/lifecycleAcknowledgement.js"; import { createTestTempDirectory } from "../fixtures/temporaryDirectory.js"; interface OpenControl { @@ -101,34 +102,6 @@ const openedHandle = (): FileHandle => { return handle; }; -const awaitAcknowledgement = async ( - acknowledgement: Promise, - operation: Promise, - boundary: string, -): Promise => { - let deadline: ReturnType | undefined; - try { - const observed = await Promise.race([ - acknowledgement.then(() => ({ kind: "acknowledged" }) as const), - operation.then( - (value) => ({ kind: "settled", status: "fulfilled", value }) as const, - (cause: unknown) => - ({ kind: "settled", status: "rejected", cause }) as const, - ), - new Promise<{ readonly kind: "deadline" }>((resolve) => { - deadline = setTimeout(() => resolve({ kind: "deadline" }), 2_000); - }), - ]); - if (observed.kind !== "acknowledged") - throw new Error( - `${boundary} was not acknowledged before ${observed.kind === "deadline" ? "the gate deadline" : "the triggering operation settled"}`, - { cause: observed }, - ); - } finally { - if (deadline !== undefined) clearTimeout(deadline); - } -}; - it.each(carriers)( "$name keeps the caller's expected digest and rechecks identity after binding", async (carrier) => { diff --git a/tests/process-global/selectedArtifactSourceLifecycle.test.ts b/tests/process-global/selectedArtifactSourceLifecycle.test.ts new file mode 100644 index 0000000000..cc76e8ef82 --- /dev/null +++ b/tests/process-global/selectedArtifactSourceLifecycle.test.ts @@ -0,0 +1,490 @@ +import { createHash } from "node:crypto"; +import type { Stats } from "node:fs"; +import { + mkdir, + open, + rename, + writeFile, + type FileHandle, +} from "node:fs/promises"; +import { join } from "node:path"; +import { buffer } from "node:stream/consumers"; +import { setImmediate } from "node:timers/promises"; +import { afterEach, expect, it, vi } from "vitest"; + +import { ArtifactFileSource } from "../../src/artifacts/ArtifactFileSource.js"; +import * as artifactHash from "../../src/artifacts/ArtifactHash.js"; +import { + ArtifactReaderFailure, + type ArtifactEntry, +} from "../../src/artifacts/ArtifactReader.js"; +import { MachOSliceArtifactReader } from "../../src/artifacts/MachOSliceArtifactReader.js"; +import { + CPU, + fatImage, + machoImage, +} from "../../src/artifacts/apple/MachoImage.fixture.js"; +import * as regularFile from "../../src/filesystem/RegularFile.js"; +import { createDeferred } from "../fixtures/binarySession.js"; +import { awaitAcknowledgement } from "../fixtures/lifecycleAcknowledgement.js"; +import { NativeFixtureRunner } from "../fixtures/nativeCommands.js"; +import { createTestTempDirectory } from "../fixtures/temporaryDirectory.js"; + +interface OpenControl { + path: string | undefined; + readonly handles: Map Promise>; + onOpen: ((handle: FileHandle) => Promise) | undefined; +} + +const control = vi.hoisted((): OpenControl => ({ + path: undefined, + handles: new Map(), + onOpen: undefined, +})); + +vi.mock("node:fs/promises", async (importOriginal) => { + const original = await importOriginal(); + return { + ...original, + open: async (...args: Parameters) => { + const handle = await original.open(...args); + if (String(args[0]) === control.path) { + control.handles.set(handle, handle.close.bind(handle)); + await control.onOpen?.(handle); + } + return handle; + }, + }; +}); + +afterEach(async () => { + control.onOpen = undefined; + for (const [handle, close] of control.handles) + if (handle.fd >= 0) await close(); + control.handles.clear(); + control.path = undefined; + vi.restoreAllMocks(); +}); + +const machoCarrier = async () => { + const root = await createTestTempDirectory("rea-selected-artifact-source-"); + const selected = join(root, "selected"); + await mkdir(selected); + const path = join(selected, "universal.bin"); + const x86 = Buffer.from(machoImage({ cpu: CPU.x86_64 })); + const arm = Buffer.from(machoImage({ cpu: CPU.arm64 })); + const bytes = Buffer.from( + fatImage([ + { cpu: CPU.x86_64, bytes: x86 }, + { cpu: CPU.arm64, bytes: arm }, + ]), + ); + await writeFile(path, bytes); + control.path = path; + const runner = new NativeFixtureRunner({ + lipo: `architecture x86_64\n cputype 16777223\n cpusubtype 3\n offset 4096\n size ${x86.length}\n align 2^12 (4096)\narchitecture arm64\n cputype 16777228\n cpusubtype 0\n offset 8192\n size ${arm.length}\n align 2^12 (4096)\n`, + }); + return { + path, + root, + selected, + runner, + arm, + sha256: createHash("sha256").update(bytes).digest("hex"), + }; +}; + +const entries = async ( + reader: MachOSliceArtifactReader, +): Promise => { + const values: ArtifactEntry[] = []; + for await (const entry of reader.entries()) values.push(entry); + return values; +}; + +const openedHandle = (): FileHandle => { + expect(control.handles.size).toBe(1); + const handle = control.handles.keys().next().value; + if (handle === undefined) + throw new Error("Missing selected carrier descriptor"); + return handle; +}; + +const replaceSelectedDirectory = async ( + fixture: Awaited>, +) => { + await rename(fixture.selected, join(fixture.root, "original")); + await mkdir(fixture.selected); + await writeFile(fixture.path, "replacement selected carrier"); +}; + +const expectDescriptorUnchanged = async ( + handle: FileHandle, + initial: Stats, +) => { + expect(await handle.stat()).toMatchObject({ + dev: initial.dev, + ino: initial.ino, + size: initial.size, + mode: initial.mode, + mtimeMs: initial.mtimeMs, + ctimeMs: initial.ctimeMs, + }); +}; + +it("joins late Mach-O admission before releasing its descriptor and dispatches no lipo after closure", async () => { + const fixture = await machoCarrier(); + const entered = createDeferred(); + const release = createDeferred(); + control.onOpen = async (handle) => { + vi.spyOn(handle, "close"); + entered.resolve(); + await release.promise; + }; + const run = vi.spyOn(fixture.runner, "run"); + const reader = new MachOSliceArtifactReader(fixture.path, {}, fixture.runner); + const operations: Promise[] = []; + try { + const operation = entries(reader); + const reading = operation.catch((cause: unknown) => cause); + operations.push(reading); + await awaitAcknowledgement(entered.promise, operation, "Mach-O admission"); + const handle = openedHandle(); + let closed = false; + const closings = [reader.close(), reader.close()]; + operations.push(...closings); + const closing = Promise.all(closings).then(() => { + closed = true; + }); + operations.push(closing); + await setImmediate(); + expect(closed).toBe(false); + expect(handle.fd).toBeGreaterThanOrEqual(0); + expect(handle.close).not.toHaveBeenCalled(); + release.resolve(); + expect(await reading).toMatchObject({ reason: "unavailable" }); + await closing; + expect(handle.fd).toBe(-1); + expect(handle.close).toHaveBeenCalledOnce(); + await expect(entries(reader)).rejects.toMatchObject({ + reason: "unavailable", + }); + expect(run).not.toHaveBeenCalled(); + expect(control.handles.size).toBe(1); + await reader.close(); + expect(handle.close).toHaveBeenCalledOnce(); + } finally { + release.resolve(); + await Promise.allSettled(operations); + await reader.close(); + } +}); + +it("retains the same Mach-O descriptor through a denied close and concurrent cleanup retry", async () => { + const fixture = await machoCarrier(); + const failure = new Error("controlled Mach-O close denial"); + let blocked = true; + control.onOpen = async (handle) => { + const close = handle.close.bind(handle); + vi.spyOn(handle, "close").mockImplementation(async () => { + if (blocked) throw failure; + await close(); + }); + }; + const reader = new MachOSliceArtifactReader(fixture.path, {}, fixture.runner); + try { + expect((await entries(reader)).map((entry) => entry.path)).toEqual([ + "slices/x86_64", + "slices/arm64", + ]); + const handle = openedHandle(); + expect(await Promise.allSettled([reader.close(), reader.close()])).toEqual([ + { status: "rejected", reason: failure }, + { status: "rejected", reason: failure }, + ]); + expect(handle.fd).toBeGreaterThanOrEqual(0); + expect(handle.close).toHaveBeenCalledOnce(); + blocked = false; + await Promise.all([reader.close(), reader.close()]); + expect(handle.fd).toBe(-1); + expect(handle.close).toHaveBeenCalledTimes(2); + await reader.close(); + expect(handle.close).toHaveBeenCalledTimes(2); + expect(control.handles.size).toBe(1); + } finally { + blocked = false; + await reader.close(); + } +}); + +it("reads admitted Mach-O slice bytes and leaves the borrowed descriptor open after close", async () => { + const fixture = await machoCarrier(); + const handle = await open(fixture.path, "r"); + const close = vi.spyOn(handle, "close"); + let reader: MachOSliceArtifactReader | undefined; + try { + reader = new MachOSliceArtifactReader(fixture.path, {}, fixture.runner, { + handle, + initial: await handle.stat(), + }); + const entry = (await entries(reader)).find( + (value) => value.path === "slices/arm64", + ); + if (entry === undefined) throw new Error("Missing authored ARM64 slice"); + expect(await buffer(await reader.open(entry))).toEqual(fixture.arm); + await reader.close(); + await reader.close(); + expect(close).not.toHaveBeenCalled(); + expect(handle.fd).toBeGreaterThanOrEqual(0); + expect(control.handles.size).toBe(1); + await handle.close(); + expect(close).toHaveBeenCalledOnce(); + expect(handle.fd).toBe(-1); + } finally { + try { + await reader?.close(); + } finally { + if (handle.fd >= 0) await handle.close(); + } + } +}); + +it("rejects a replaced selected pathname without closing the borrowed Mach-O descriptor", async () => { + const fixture = await machoCarrier(); + const handle = await open(fixture.path, "r"); + const close = vi.spyOn(handle, "close"); + const run = vi.spyOn(fixture.runner, "run"); + let reader: MachOSliceArtifactReader | undefined; + try { + const initial = await handle.stat(); + reader = new MachOSliceArtifactReader(fixture.path, {}, fixture.runner, { + handle, + initial, + }); + const entry = (await entries(reader)).find( + (value) => value.path === "slices/arm64", + ); + if (entry === undefined) throw new Error("Missing authored ARM64 slice"); + await replaceSelectedDirectory(fixture); + await expectDescriptorUnchanged(handle, initial); + await expect(reader.open(entry)).rejects.toMatchObject({ + reason: "integrity", + }); + await expect(entries(reader)).rejects.toMatchObject({ + reason: "integrity", + }); + expect(run).toHaveBeenCalledOnce(); + expect(run).toHaveBeenCalledWith( + "lipo", + ["-detailed_info", fixture.path], + {}, + ); + await reader.close(); + expect(close).not.toHaveBeenCalled(); + expect(handle.fd).toBeGreaterThanOrEqual(0); + expect(control.handles.size).toBe(1); + } finally { + try { + await reader?.close(); + } finally { + if (handle.fd >= 0) await handle.close(); + } + } +}); + +it.each([false, true])( + "rejects selected pathname replacement after real hashing even with a late abort (%s)", + async (abort) => { + const fixture = await machoCarrier(); + const completed = createDeferred(); + const release = createDeferred(); + const controller = new AbortController(); + const originalHash = artifactHash.hashReadable; + vi.spyOn(artifactHash, "hashReadable").mockImplementation( + async (...args) => { + const hashed = await originalHash(...args); + expect(hashed.sha256).toBe(fixture.sha256); + completed.resolve(); + await release.promise; + return hashed; + }, + ); + const source = new ArtifactFileSource(fixture.path, "Selected carrier"); + let binding: Promise | undefined; + try { + const operation = source.bindSelectedPathSha256( + fixture.sha256, + controller.signal, + ); + binding = operation.catch((cause: unknown) => cause); + await awaitAcknowledgement( + completed.promise, + operation, + "selected carrier hash", + ); + const handle = openedHandle(); + const initial = await handle.stat(); + await replaceSelectedDirectory(fixture); + await expectDescriptorUnchanged(handle, initial); + if (abort) + controller.abort({ request: "cancel after completed carrier hash" }); + release.resolve(); + const failure = await binding; + if (!(failure instanceof ArtifactReaderFailure)) + throw new Error("Expected a selected-path identity failure"); + expect(failure.reason).toBe("integrity"); + expect(failure.message).toBe( + `Selected carrier source identity changed: ${fixture.path}`, + ); + expect(failure.cancellation).toBeUndefined(); + await source.close(); + expect(handle.fd).toBe(-1); + } finally { + release.resolve(); + await binding; + await source.close(); + } + }, +); + +it.each([false, true])( + "preserves typed admission identity failure and its exact cleanup owner (closeFails=%s)", + async (closeFails) => { + const fixture = await machoCarrier(); + const changed = new regularFile.RegularFileChangedError(fixture.path); + const cleanupFailure = new Error( + "controlled changed-admission close denial", + ); + let blocked = closeFails; + control.onOpen = async (handle) => { + vi.spyOn(handle, "stat").mockRejectedValueOnce(changed); + const close = handle.close.bind(handle); + vi.spyOn(handle, "close").mockImplementation(async () => { + if (blocked) throw cleanupFailure; + await close(); + }); + }; + const run = vi.spyOn(fixture.runner, "run"); + const reader = new MachOSliceArtifactReader( + fixture.path, + {}, + fixture.runner, + ); + try { + const failure = await entries(reader).catch((cause: unknown) => cause); + if (!(failure instanceof ArtifactReaderFailure)) + throw new Error("Expected a typed admission identity failure"); + expect(failure.reason).toBe("integrity"); + const primary = closeFails ? failure.cause : failure; + if (!(primary instanceof ArtifactReaderFailure)) + throw new Error("Missing the primary admission identity failure"); + expect(primary.cause).toBe(changed); + const handle = openedHandle(); + if (closeFails) { + expect(failure.cleanup).toEqual({ + reason: cleanupFailure.message, + resources: [fixture.path], + }); + expect(handle.fd).toBeGreaterThanOrEqual(0); + await expect(reader.close()).rejects.toBe(cleanupFailure); + expect(handle.close).toHaveBeenCalledTimes(2); + blocked = false; + await reader.close(); + expect(handle.close).toHaveBeenCalledTimes(3); + } else { + expect(failure.cleanup).toBeUndefined(); + await reader.close(); + expect(handle.close).toHaveBeenCalledOnce(); + } + expect(handle.fd).toBe(-1); + expect(run).not.toHaveBeenCalled(); + expect(control.handles.size).toBe(1); + } finally { + blocked = false; + await reader.close(); + } + }, +); + +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(); + } + }, +);