From 949966712e68d8857a50a165ecddf04c2d7e6d3b Mon Sep 17 00:00:00 2001 From: morluto <76467478+morluto@users.noreply.github.com> Date: Wed, 21 Oct 2026 06:14:03 +0800 Subject: [PATCH] fix(files): preserve physical read failures across late cancellation Tag physical read outcomes at the owning boundary and preserve opaque caller cancellation reasons through JSON pipelines and reference imports. Share named bounded-read options across consumers and retain failed cleanup ownership for retry. Closes #5686 --- src/application/EvidenceBundleFiles.ts | 37 +- src/application/JsonFiles.ts | 47 ++- src/application/JsonInputFile.ts | 330 ++++++++++++------ src/application/LinuxHopperPublisher.ts | 4 +- .../ReferenceSourceImportPolicy.ts | 58 ++- src/application/RegularFileRead.ts | 85 +++-- src/application/SetupSkill.ts | 9 +- .../analysisView/evidenceFileSource.test.ts | 144 +++++++- .../analysisView/evidenceFileSource.ts | 39 ++- src/application/process/ProcessCli.ts | 21 +- src/cliJsonInput.ts | 46 ++- src/dotnet/ManagedSnapshot.ts | 2 +- src/ghidra/GhidraClient.ts | 16 +- src/ghidra/GhidraDosSnapshot.ts | 10 +- src/native/AppleDispatchMetadata.ts | 10 +- src/process/BoundedFileBytes.ts | 35 +- .../filesystem/boundedFileBytes.test.ts | 15 +- .../cliJsonInputCancellation.test.ts | 62 +++- .../filesystem/evidenceBundleFiles.test.ts | 111 +++++- .../filesystem/processCliFileInput.test.ts | 75 ++++ .../filesystem/referenceSourceImport.test.ts | 264 +++++++++++++- tests/support/failFileReadAfterAbort.ts | 40 +++ 22 files changed, 1203 insertions(+), 257 deletions(-) create mode 100644 tests/support/failFileReadAfterAbort.ts diff --git a/src/application/EvidenceBundleFiles.ts b/src/application/EvidenceBundleFiles.ts index dd1715c96f..df421ecb97 100644 --- a/src/application/EvidenceBundleFiles.ts +++ b/src/application/EvidenceBundleFiles.ts @@ -22,7 +22,6 @@ import { canonicalJsonParts, } from "../domain/jsonSerialization.js"; import { readJsonFile, writeTextParts } from "./JsonFiles.js"; -import { RegularFileCleanupFailure } from "./RegularFileRead.js"; import type { ArtifactResourceScope } from "../artifacts/ArtifactResourceScope.js"; type EvidenceReadFailure = @@ -58,23 +57,27 @@ const readBundle = async ( resources?: ArtifactResourceScope, ): Promise> => { if (signal?.aborted) - return err(new AnalysisCancelledError("import_evidence_bundle")); - const loaded = await readJsonFile(path, signal, resources).catch( - (cause: unknown) => { - // A failed close transfers a live handle owner; preserve that failure. - if ( - signal?.aborted === true && - !(cause instanceof RegularFileCleanupFailure) - ) - return err( - new AnalysisCancelledError("import_evidence_bundle", { cause }), - ); - throw cause; - }, - ); - if (!loaded.ok) return loaded; + return err( + new AnalysisCancelledError("import_evidence_bundle", { + cause: signal.reason, + }), + ); + const loaded = await readJsonFile(path, signal, resources); + if (!loaded.ok) { + if (loaded.error instanceof AnalysisCancelledError) + return err( + new AnalysisCancelledError("import_evidence_bundle", { + cause: loaded.error.cause, + }), + ); + return loaded; + } if (signal?.aborted) - return err(new AnalysisCancelledError("import_evidence_bundle")); + return err( + new AnalysisCancelledError("import_evidence_bundle", { + cause: signal.reason, + }), + ); let bundle: EvidenceBundle; try { bundle = parseBundle(loaded.value); diff --git a/src/application/JsonFiles.ts b/src/application/JsonFiles.ts index 66ae8487f0..2e81dd8fcf 100644 --- a/src/application/JsonFiles.ts +++ b/src/application/JsonFiles.ts @@ -19,6 +19,8 @@ import { } from "../domain/analysisErrorCore.js"; import { err, ok, type Result } from "../domain/result.js"; import { + JsonInputPhysicalReadFailure, + JsonInputCancellationFailure, readJsonInputFile, readJsonObjectFile, type JsonObjectMemberReader, @@ -46,7 +48,10 @@ export const readJsonFile = async ( signal?: AbortSignal, resources?: ArtifactResourceScope, ): Promise< - Result + Result< + unknown, + EvidenceFileError | AnalysisResourceConstraintError | AnalysisCancelledError + > > => readJsonFileInput( path, @@ -93,7 +98,10 @@ const readJsonFileInput = async ( >, signal?: AbortSignal, ): Promise< - Result + Result< + Value, + EvidenceFileError | AnalysisResourceConstraintError | AnalysisCancelledError + > > => { const requestedPath = absoluteCallerPath(path); try { @@ -116,7 +124,25 @@ const readJsonFileInput = async ( } catch (cause: unknown) { if (cause instanceof RegularFileCleanupFailure) throw cause; if (cause instanceof AnalysisResourceConstraintError) return err(cause); - if (signal?.aborted === true) throw cause; + if (cause instanceof JsonInputCancellationFailure) + return err( + new AnalysisCancelledError("read_evidence_file", { + cause: cause.reason, + }), + ); + if (cause instanceof JsonInputPhysicalReadFailure) + return err( + new EvidenceFileError("read", "io", { + cause: cause.physicalCause, + path: requestedPath, + }), + ); + if (isJsonReadCancellation(cause, signal)) + return err( + new AnalysisCancelledError("read_evidence_file", { + cause: signal?.reason, + }), + ); return err( new EvidenceFileError( "read", @@ -132,6 +158,21 @@ const readJsonFileInput = async ( } }; +const isJsonReadCancellation = ( + cause: unknown, + signal?: AbortSignal, +): boolean => { + if (signal?.aborted !== true) return false; + if (cause === signal.reason) return true; + return ( + cause instanceof Error && + cause.name === "AbortError" && + "code" in cause && + cause.code === "ABORT_ERR" && + cause.cause === signal.reason + ); +}; + /** Atomically write text to the caller-supplied path. */ export const writeTextFile = async ( encoded: string, diff --git a/src/application/JsonInputFile.ts b/src/application/JsonInputFile.ts index 2e93d0a679..fe5e0feace 100644 --- a/src/application/JsonInputFile.ts +++ b/src/application/JsonInputFile.ts @@ -1,5 +1,6 @@ import { constants } from "node:buffer"; import { createHash } from "node:crypto"; +import type { Stats } from "node:fs"; import type { FileHandle } from "node:fs/promises"; import { pipeline } from "node:stream/promises"; import { getHeapStatistics } from "node:v8"; @@ -10,10 +11,17 @@ import parser from "stream-json/parser.js"; import streamValues from "stream-json/streamers/stream-values.js"; import streamArray from "stream-json/streamers/stream-array.js"; -import { RegularFileChangedError } from "../filesystem/RegularFile.js"; +import { + assertRegularFileActive, + RegularFileChangedError, +} from "../filesystem/RegularFile.js"; import type { ArtifactResourceScope } from "../artifacts/ArtifactResourceScope.js"; import { sealOwnedJsonInput } from "./OwnedJsonInput.js"; -import { withRegularFile } from "./RegularFileRead.js"; +import { + RegularFileReadFailure, + withRegularFile, + type RegularFileReadOptions, +} from "./RegularFileRead.js"; import { JSON_BYTE_ORDER_MARK_MESSAGE, JSON_INPUT_RESOURCE_REMEDIATION, @@ -34,6 +42,74 @@ const PACKED_TOKEN_PART_CODE_UNITS = 64 * 1024; // This selects an implementation; larger files remain accepted through streaming. const NATIVE_PARSE_READ_BUDGET = 8 * 1024 * 1024; +/** Physical descriptor failure remains primary if cancellation arrives afterward. */ +export class JsonInputPhysicalReadFailure extends Error { + readonly _tag = "JsonInputPhysicalReadFailure"; + readonly physicalCause: unknown; + constructor(physicalCause: unknown) { + super("JSON input descriptor read failed"); + this.physicalCause = physicalCause; + } +} + +/** Explicit cancellation fact; the caller's value is opaque and retained as data. */ +export class JsonInputCancellationFailure extends Error { + readonly _tag = "JsonInputCancellationFailure"; + constructor(readonly reason: unknown) { + super( + reason instanceof Error ? reason.message : "JSON input read cancelled", + ); + } +} + +interface JsonInputPhysicalReadObservation { + failure?: JsonInputPhysicalReadFailure; +} + +const readPhysicalJsonInput = async ( + observation: JsonInputPhysicalReadObservation, + read: () => Promise, +): Promise => { + try { + return await read(); + } catch (physicalCause: unknown) { + const failure = new JsonInputPhysicalReadFailure(physicalCause); + observation.failure = failure; + throw failure; + } +}; + +const withJsonInputFile = ( + path: string, + read: (handle: FileHandle, stats: Stats) => Promise, + options: Pick, +): Promise => + withRegularFile(path, read, { + ...options, + cancellationFailure: (reason) => new JsonInputCancellationFailure(reason), + }).catch((cause: unknown) => { + if (cause instanceof RegularFileReadFailure) throw cause.operationCause; + throw cause; + }); + +const withJsonInputPipeline = async ( + signal: AbortSignal | undefined, + run: (signal?: AbortSignal) => Promise, +): Promise => { + if (signal === undefined) return run(); + const controller = new AbortController(); + const cancel = (): void => { + controller.abort(new JsonInputCancellationFailure(signal.reason)); + }; + if (signal.aborted) cancel(); + else signal.addEventListener("abort", cancel, { once: true }); + try { + return await run(controller.signal); + } finally { + signal.removeEventListener("abort", cancel); + } +}; + /** Heap a whole-file native JSON.parse may need: the assembler's 64x container expansion plus byte and text copies. */ export const nativeJsonParseRequiredBytes = ( inputBytes: number, @@ -66,10 +142,11 @@ export const readJsonObjectFile = ( signal?: AbortSignal, resources?: ArtifactResourceScope, ): Promise> => - withRegularFile( + withJsonInputFile( path, async (handle, stats) => { let previousDigest: string | undefined; + const physicalRead: JsonInputPhysicalReadObservation = {}; try { const native = await readNativeJson( handle, @@ -77,6 +154,8 @@ export const readJsonObjectFile = ( path, operation, signal, + false, + physicalRead, ); if (native !== undefined) { if (!native.ok) return native; @@ -92,7 +171,7 @@ export const readJsonObjectFile = ( for (const members of scans) for (const [key, member] of members) { if (!Object.hasOwn(value, key)) continue; - signal?.throwIfAborted(); + assertRegularFileActive(signal); member.begin(); const selected: unknown = Reflect.get(value, key); if (member.mode === "array" && !Array.isArray(selected)) { @@ -105,7 +184,7 @@ export const readJsonObjectFile = ( Array.isArray(selected) ? selected : [selected]) { - signal?.throwIfAborted(); + assertRegularFileActive(signal); try { await member.accept(item); } catch (cause: unknown) { @@ -123,68 +202,80 @@ export const readJsonObjectFile = ( let consume: | ((token: parser.Token) => typeof none | Promise) | undefined; - await pipeline( - decodedChunks(handle, stats.size, path, signal, (bytes) => { - hash.update(bytes); - }), - asStream( - fun( - parser({ - jsonStreaming: false, - streamValues: false, - packKeys: false, - packStrings: false, - packNumbers: false, - }), - packJsonScalars(operation, stats.size, path), - (token: parser.Token): typeof none | Promise => { - const opens = - token.name === "startObject" || token.name === "startArray"; - const closes = - token.name === "endObject" || token.name === "endArray"; - if (depth === 0) { - rootObject = token.name === "startObject"; - } else if ( - rootObject && - depth === 1 && - token.name === "keyValue" - ) { - key = token.value; - } else if (rootObject && depth === 1 && !closes) { - const member = - key === undefined ? undefined : members.get(key); - consume = undefined; - if (member !== undefined) { - member.begin(); - if ( - member.mode === "array" && - token.name !== "startArray" - ) { - member.reject( - new TypeError(`JSON member ${key} must be an array`), - ); - } else { - consume = memberConsumer( - member, - operation, - stats.size, - path, - ); - } - } - key = undefined; - } - const selected = consume; - if (opens) depth += 1; - if (closes) depth -= 1; - if (rootObject && depth === 1 && token.name !== "keyValue") - consume = undefined; - if (selected === undefined) return none; - return selected(token); + await withJsonInputPipeline(signal, (pipelineSignal) => + pipeline( + decodedChunks( + handle, + stats.size, + path, + signal, + (bytes) => { + hash.update(bytes); }, + physicalRead, ), + asStream( + fun( + parser({ + jsonStreaming: false, + streamValues: false, + packKeys: false, + packStrings: false, + packNumbers: false, + }), + packJsonScalars(operation, stats.size, path), + (token: parser.Token): typeof none | Promise => { + const opens = + token.name === "startObject" || + token.name === "startArray"; + const closes = + token.name === "endObject" || token.name === "endArray"; + if (depth === 0) { + rootObject = token.name === "startObject"; + } else if ( + rootObject && + depth === 1 && + token.name === "keyValue" + ) { + key = token.value; + } else if (rootObject && depth === 1 && !closes) { + const member = + key === undefined ? undefined : members.get(key); + consume = undefined; + if (member !== undefined) { + member.begin(); + if ( + member.mode === "array" && + token.name !== "startArray" + ) { + member.reject( + new TypeError( + `JSON member ${key} must be an array`, + ), + ); + } else { + consume = memberConsumer( + member, + operation, + stats.size, + path, + ); + } + } + key = undefined; + } + const selected = consume; + if (opens) depth += 1; + if (closes) depth -= 1; + if (rootObject && depth === 1 && token.name !== "keyValue") + consume = undefined; + if (selected === undefined) return none; + return selected(token); + }, + ), + ), + pipelineSignal === undefined ? {} : { signal: pipelineSignal }, ), - signal === undefined ? {} : { signal }, ); if (!rootObject) return err( @@ -197,6 +288,7 @@ export const readJsonObjectFile = ( } return ok(undefined); } catch (cause: unknown) { + if (physicalRead.failure !== undefined) throw physicalRead.failure; const failure = jsonReadFailure(cause, operation, stats.size, path); if (failure !== undefined) return err(failure); throw cause; @@ -256,11 +348,12 @@ export const readJsonInputFile = ( resources?: ArtifactResourceScope, snapshot = false, ): Promise> => - withRegularFile( + withJsonInputFile( path, async (handle, stats) => { let found = false; let value: unknown; + const physicalRead: JsonInputPhysicalReadObservation = {}; try { const native = await readNativeJson( handle, @@ -269,44 +362,59 @@ export const readJsonInputFile = ( operation, signal, snapshot, + physicalRead, ); if (native !== undefined) return native.ok && snapshot ? await sealOwnedJsonInput(native.value, operation, signal) : native; - await pipeline( - decodedChunks(handle, stats.size, path, signal), - // Fuse synchronous token stages so only file chunks cross the async - // stream boundary. Admission still precedes scalar/value assembly. - asStream( - fun( - parser({ - jsonStreaming: false, - streamValues: false, - packKeys: false, - packStrings: false, - packNumbers: false, - }), - packJsonScalars(operation, stats.size, path), - admitJsonAssembly(operation, stats.size, path, snapshot ? 2 : 1), - streamValues(), - (row: unknown): typeof none => { - if ( - found || - typeof row !== "object" || - row === null || - !("key" in row) || - row.key !== 0 || - !("value" in row) - ) - throw new Error("Unexpected JSON parser result"); - found = true; - value = row.value; - return none; - }, + await withJsonInputPipeline(signal, (pipelineSignal) => + pipeline( + decodedChunks( + handle, + stats.size, + path, + signal, + undefined, + physicalRead, ), + // Fuse synchronous token stages so only file chunks cross the async + // stream boundary. Admission still precedes scalar/value assembly. + asStream( + fun( + parser({ + jsonStreaming: false, + streamValues: false, + packKeys: false, + packStrings: false, + packNumbers: false, + }), + packJsonScalars(operation, stats.size, path), + admitJsonAssembly( + operation, + stats.size, + path, + snapshot ? 2 : 1, + ), + streamValues(), + (row: unknown): typeof none => { + if ( + found || + typeof row !== "object" || + row === null || + !("key" in row) || + row.key !== 0 || + !("value" in row) + ) + throw new Error("Unexpected JSON parser result"); + found = true; + value = row.value; + return none; + }, + ), + ), + pipelineSignal === undefined ? {} : { signal: pipelineSignal }, ), - signal === undefined ? {} : { signal }, ); return found ? snapshot @@ -314,6 +422,7 @@ export const readJsonInputFile = ( : ok(value) : err(invalidJson(operation, "JSON document is empty")); } catch (cause: unknown) { + if (physicalRead.failure !== undefined) throw physicalRead.failure; const failure = jsonReadFailure(cause, operation, stats.size, path); if (failure !== undefined) return err(failure); throw cause; @@ -526,6 +635,7 @@ async function* decodedChunks( path: string, signal: AbortSignal | undefined, observe?: (bytes: Uint8Array) => void, + physicalRead: JsonInputPhysicalReadObservation = {}, ): AsyncGenerator { const decoder = new TextDecoder("utf-8", { fatal: true, ignoreBOM: true }); let leading = true; @@ -539,14 +649,16 @@ async function* decodedChunks( const bytes = Buffer.allocUnsafe(READ_CHUNK_BYTES); let observedBytes = 0; while (true) { - signal?.throwIfAborted(); - const read = await handle.read( - bytes, - 0, - Math.min(bytes.length, admittedBytes - observedBytes + 1), - observedBytes, + assertRegularFileActive(signal); + const read = await readPhysicalJsonInput(physicalRead, () => + handle.read( + bytes, + 0, + Math.min(bytes.length, admittedBytes - observedBytes + 1), + observedBytes, + ), ); - signal?.throwIfAborted(); + assertRegularFileActive(signal); if (read.bytesRead === 0) break; observedBytes += read.bytesRead; if (observedBytes > admittedBytes) throw new RegularFileChangedError(path); @@ -569,13 +681,16 @@ const readPrefix = async ( handle: FileHandle, size: number, signal: AbortSignal | undefined, + physicalRead: JsonInputPhysicalReadObservation, ): Promise<{ readonly bytes: Buffer; readonly complete: boolean }> => { const bytes = Buffer.allocUnsafe(size); let offset = 0; while (offset < bytes.length) { - signal?.throwIfAborted(); - const read = await handle.read(bytes, offset, bytes.length - offset, null); - signal?.throwIfAborted(); + assertRegularFileActive(signal); + const read = await readPhysicalJsonInput(physicalRead, () => + handle.read(bytes, offset, bytes.length - offset, null), + ); + assertRegularFileActive(signal); if (read.bytesRead === 0) return { bytes: bytes.subarray(0, offset), complete: true }; offset += read.bytesRead; @@ -591,9 +706,10 @@ const readNativeJson = async ( operation: string, signal?: AbortSignal, snapshot = false, + physicalRead: JsonInputPhysicalReadObservation = {}, ): Promise | undefined> => { if (!nativeParseFits(size, snapshot)) return undefined; - const prefix = await readPrefix(handle, size + 1, signal); + const prefix = await readPrefix(handle, size + 1, signal, physicalRead); if (!prefix.complete || prefix.bytes.length !== size) throw new RegularFileChangedError(path); const parsed = parseUtf8Json( diff --git a/src/application/LinuxHopperPublisher.ts b/src/application/LinuxHopperPublisher.ts index eb63d578e1..9d6f1451fb 100644 --- a/src/application/LinuxHopperPublisher.ts +++ b/src/application/LinuxHopperPublisher.ts @@ -926,7 +926,9 @@ export class LinuxHopperPublisher { "integrity", "Retained Hopper evidence exceeds its resource byte budget.", ); - const bytes = await readBoundedFileBytes(handle, stats.size, signal); + const bytes = await readBoundedFileBytes(handle, stats.size, { + signal, + }); if (bytes === undefined) throw new HopperReleaseError( "integrity", diff --git a/src/application/ReferenceSourceImportPolicy.ts b/src/application/ReferenceSourceImportPolicy.ts index ca314bbd80..fabf360700 100644 --- a/src/application/ReferenceSourceImportPolicy.ts +++ b/src/application/ReferenceSourceImportPolicy.ts @@ -5,6 +5,7 @@ import ignore from "ignore"; import { readRegularFileText, RegularFileCleanupFailure, + RegularFileReadFailure, retryRegularFileCleanup, } from "./RegularFileRead.js"; @@ -17,6 +18,7 @@ import { type ReferenceSourceImportOptions, } from "./ReferenceSourceImportTypes.js"; import { rootFilesystemFailure } from "../reference/ReferenceSourceReaderErrors.js"; +import { regularFileCancellation } from "../filesystem/OwnedRegularFile.js"; /** Validated inputs ready for filesystem traversal. */ export interface PreparedReferenceSourceImport { @@ -34,6 +36,25 @@ const failure = ( message, }); +class IgnorePolicyReadCancelled extends Error { + constructor(readonly reason: unknown) { + super("Reference source ignore policy read was cancelled", { + cause: reason, + }); + this.name = "IgnorePolicyReadCancelled"; + } +} + +const isIgnorePolicyCancellation = (cause: unknown): boolean => + cause instanceof IgnorePolicyReadCancelled; + +const ignorePolicyCause = (cause: unknown): unknown => + cause instanceof IgnorePolicyReadCancelled + ? cause.reason + : cause instanceof RegularFileReadFailure + ? cause.operationCause + : cause; + const isAborted = (signal?: AbortSignal): boolean => signal?.aborted === true; const addMarkedPatterns = ( @@ -102,30 +123,46 @@ const buildIgnored = async ( throw cause; present = false; } - signal?.throwIfAborted(); + if (signal?.aborted === true) + throw new IgnorePolicyReadCancelled(signal.reason); if (present) { let text: string; try { - text = await readRegularFileText(policyPath, { signal }); + text = await readRegularFileText(policyPath, { + signal, + cancellationFailure: (reason) => new IgnorePolicyReadCancelled(reason), + }); } catch (cause: unknown) { - if (signal?.aborted === true && cause === signal.reason) throw cause; + if (cause instanceof IgnorePolicyReadCancelled) throw cause; if (!(cause instanceof RegularFileCleanupFailure)) throw cause; const retry = await retryRegularFileCleanup(cause, resources, signal); + const operationCancellation = + cause.outcome.kind === "failed" + ? regularFileCancellation(cause) + : undefined; if (retry.cleanup !== undefined) { const primary = retry.outcome.kind === "failed" ? retry.outcome.cause : cause.cleanupCause; + const primaryCause = ignorePolicyCause(primary); return err({ ...failure( - signal?.aborted === true ? "cancelled" : "io", - `Reference source ignore policy could not be read at ${policyPath}: ${errorMessage(primary)}`, + isIgnorePolicyCancellation(primary) || + operationCancellation !== undefined + ? "cancelled" + : "io", + `Reference source ignore policy could not be read at ${policyPath}: ${errorMessage(primaryCause)}`, ), cleanup: retry.cleanup, - cause: primary, + cause: primaryCause, }); } - if (retry.outcome.kind === "failed") throw retry.outcome.cause; + if (retry.outcome.kind === "failed") { + if (operationCancellation !== undefined) + throw new IgnorePolicyReadCancelled(operationCancellation.reason); + throw retry.outcome.cause; + } text = retry.outcome.value; } addMarkedPatterns(ignored, [text], "project-ignored"); @@ -166,12 +203,13 @@ export const prepareReferenceSourceImport = async ( secrets: ignore().add([...options.policy.secretPatterns]), }); } catch (cause: unknown) { + const causeValue = ignorePolicyCause(cause); return err({ ...failure( - options.signal?.aborted === true ? "cancelled" : "io", - `Reference source ignore policy could not be read at ${join(root.value, ".gitignore")}: ${errorMessage(cause)}`, + isIgnorePolicyCancellation(cause) ? "cancelled" : "io", + `Reference source ignore policy could not be read at ${join(root.value, ".gitignore")}: ${errorMessage(causeValue)}`, ), - cause, + cause: causeValue, }); } }); diff --git a/src/application/RegularFileRead.ts b/src/application/RegularFileRead.ts index cdbadedc46..65ed2af261 100644 --- a/src/application/RegularFileRead.ts +++ b/src/application/RegularFileRead.ts @@ -10,6 +10,7 @@ import { import { RegularFileCleanupFailure, RegularFileCancelled, + regularFileCancellation, withOwnedRegularFile, type RegularFileOperationOutcome, } from "../filesystem/OwnedRegularFile.js"; @@ -24,6 +25,18 @@ export interface RegularFileReadOptions { readonly resources?: ArtifactResourceScope; /** Runs on the opened file's own state before any byte is read; throw to refuse the read. */ readonly admit?: (state: Stats) => void; + /** Tags cancellation at the bounded reader's cooperative checkpoints. */ + readonly cancellationFailure?: (reason: unknown) => Error; +} + +/** A regular-file failure returned alongside an enabled cancellation factory. */ +export class RegularFileReadFailure extends Error { + readonly _tag = "RegularFileReadFailure"; + + constructor(readonly operationCause: unknown) { + super("Regular file operation failed", { cause: operationCause }); + this.name = this._tag; + } } export class InvalidUtf8FileError extends TypeError { @@ -153,21 +166,40 @@ export const withRegularFile = async ( // Caller reasons are opaque, including objects shaped like file failures. // They do not transfer ownership of any descriptor to this operation. if (signal?.aborted === true && cause === signal.reason) throw cause; - if (cause instanceof RegularFileOperationCancelled) throw cause.reason; + if (cause instanceof RegularFileReadFailure) throw cause; if ( cause instanceof RegularFileCancelled && - cause.owner.handle === admitted - ) - throw cause.reason; - if ( - !(cause instanceof RegularFileCleanupFailure) || - resources === undefined + cause.owner.handle !== admitted ) throw cause; - const retry = await retryRegularFileCleanup(cause, resources, signal); - if (retry.cleanup !== undefined) throw cause; - if (retry.outcome.kind === "failed") throw retry.outcome.cause; - return retry.outcome.value; + if (cause instanceof RegularFileCleanupFailure) { + if (resources === undefined) throw cause; + const retry = await retryRegularFileCleanup(cause, resources, signal); + if (retry.cleanup !== undefined) throw cause; + if (retry.outcome.kind === "failed") { + if (signal?.aborted === true && retry.outcome.cause === signal.reason) + throw signal.reason; + const cancellation = + regularFileCancellation(retry.outcome.cause) ?? + regularFileCancellation(cause); + if (cancellation !== undefined) { + if (options.cancellationFailure !== undefined) + throw options.cancellationFailure(cancellation.reason); + throw cancellation.reason; + } + throw retry.outcome.cause; + } + return retry.outcome.value; + } + const cancellation = regularFileCancellation(cause); + if (cancellation !== undefined) { + if (options.cancellationFailure !== undefined) + throw options.cancellationFailure(cancellation.reason); + throw cancellation.reason; + } + if (options.cancellationFailure !== undefined) + throw new RegularFileReadFailure(cause); + throw cause; } }; const value = await (resources === undefined @@ -175,6 +207,8 @@ export const withRegularFile = async ( : resources.run(operation)); // A retained close may have recovered after the descriptor owner's outcome // was captured. Include that retry and the scope's finalization in cancellation. + if (signal?.aborted === true && options.cancellationFailure !== undefined) + throw options.cancellationFailure(signal.reason); signal?.throwIfAborted(); return value; }; @@ -225,15 +259,26 @@ const readRegularFileContent = ( withRegularFile( path, async (handle, stats) => { - options.admit?.(stats); - const bytes = await readBoundedFileBytes( - handle, - stats.size, - options.signal, - ); - if (bytes === undefined || bytes.length !== stats.size) - throw new RegularFileChangedError(path); - return decode(bytes, stats); + try { + options.admit?.(stats); + const bytes = await readBoundedFileBytes(handle, stats.size, { + ...(options.signal === undefined ? {} : { signal: options.signal }), + cancellationFailure: (reason) => + new RegularFileOperationCancelled(reason), + operationFailure: (cause) => new RegularFileReadFailure(cause), + }); + if (bytes === undefined || bytes.length !== stats.size) + throw new RegularFileChangedError(path); + return decode(bytes, stats); + } catch (cause: unknown) { + if ( + options.cancellationFailure !== undefined && + !(cause instanceof RegularFileOperationCancelled) && + !(cause instanceof RegularFileReadFailure) + ) + throw new RegularFileReadFailure(cause); + throw cause; + } }, options, ); diff --git a/src/application/SetupSkill.ts b/src/application/SetupSkill.ts index 87adb71ab5..02e22bfc8a 100644 --- a/src/application/SetupSkill.ts +++ b/src/application/SetupSkill.ts @@ -53,12 +53,9 @@ const readSkillFileState = async ( withRegularFile( path, async (handle, stats) => { - const bytes = await readBoundedFileBytes( - handle, - stats.size, - undefined, - stats.size, - ); + const bytes = await readBoundedFileBytes(handle, stats.size, { + expectedSize: stats.size, + }); if (bytes === undefined || bytes.length !== stats.size) throw new RegularFileChangedError(path); return { bytes, mode: stats.mode & 0o7777 }; diff --git a/src/application/analysisView/evidenceFileSource.test.ts b/src/application/analysisView/evidenceFileSource.test.ts index 8bad7eed31..ac593127e0 100644 --- a/src/application/analysisView/evidenceFileSource.test.ts +++ b/src/application/analysisView/evidenceFileSource.test.ts @@ -1,11 +1,15 @@ import { writeFile } from "node:fs/promises"; import { join } from "node:path"; -import { describe, expect, it } from "vitest"; +import { describe, expect, it, vi } from "vitest"; import { analysisViewCaptureEvidence } from "../../../tests/fixtures/analysisView.js"; import { createTestTempDirectory } from "../../../tests/fixtures/temporaryDirectory.js"; import { ArtifactResourceScope } from "../../artifacts/ArtifactResourceScope.js"; +import { AnalysisCancelledError } from "../../domain/analysisErrorCore.js"; +import { JsonInputPhysicalReadFailure } from "../JsonInputFile.js"; +import { failFileReadAfterAbort } from "../../../tests/support/failFileReadAfterAbort.js"; +import { holdFirstFileRead } from "../../../tests/support/holdFirstFileRead.js"; import { estimateJsonValueBytes, readEvidenceFileSource, @@ -43,12 +47,23 @@ describe("evidence-file source", () => { it("refuses a cancelled request before reading and reads within the caller's resource scope", async () => { const controller = new AbortController(); - controller.abort(); - expect( - await readEvidenceFileSource("/nonexistent/capture.json", { + const reason = new JsonInputPhysicalReadFailure( + new Error("opaque caller reason with physical-failure shape"), + ); + controller.abort(reason); + const cancelled = await readEvidenceFileSource( + "/nonexistent/capture.json", + { signal: controller.signal, - }), - ).toMatchObject({ ok: false, error: { _tag: "AnalysisCancelledError" } }); + }, + ); + expect(cancelled).toMatchObject({ + ok: false, + error: { _tag: "AnalysisCancelledError" }, + }); + if (cancelled.ok || !(cancelled.error instanceof AnalysisCancelledError)) + throw new Error("Expected cancelled analysis-view read"); + expect(cancelled.error.cause).toBe(reason); const path = join( await createTestTempDirectory("rea-evidence-file-source-"), "capture.json", @@ -62,3 +77,120 @@ describe("evidence-file source", () => { expect(read.value).toEqual(JSON.parse(JSON.stringify(parent))); }); }); + +describe("evidence-file read failure provenance", () => { + it("keeps a physical null read cause after the analysis-view request aborts", async () => { + const root = await createTestTempDirectory("rea-view-late-io-"); + const path = join(root, "capture.json"); + await writeFile(path, JSON.stringify(analysisViewCaptureEvidence())); + const controller = new AbortController(); + await failFileReadAfterAbort( + path, + controller, + new Error("later abort"), + null, + ); + try { + const result = await readEvidenceFileSource(path, { + signal: controller.signal, + }); + expect(result).toMatchObject({ + ok: false, + error: { _tag: "EvidenceFileError", reason: "io", path }, + }); + if (result.ok) throw new Error("Expected physical JSON read failure"); + expect(result.error.cause).toBeNull(); + } finally { + vi.restoreAllMocks(); + } + }); + + it("preserves identical physical and abort causes for analysis-view reads", async () => { + const root = await createTestTempDirectory("rea-view-identical-abort-"); + const path = join(root, "capture.json"); + await writeFile(path, JSON.stringify(analysisViewCaptureEvidence())); + const controller = new AbortController(); + const physical = Object.assign(new Error("descriptor EIO"), { + code: "EIO", + }); + await failFileReadAfterAbort(path, controller, physical, physical); + try { + const result = await readEvidenceFileSource(path, { + signal: controller.signal, + }); + expect(result).toMatchObject({ + ok: false, + error: { _tag: "EvidenceFileError", reason: "io" }, + }); + if (result.ok) throw new Error("Expected physical JSON read failure"); + expect(result.error.cause).toBe(physical); + } finally { + vi.restoreAllMocks(); + } + }); + + it("preserves the caller cancellation reason during an analysis-view read", async () => { + const root = await createTestTempDirectory("rea-view-read-cancel-"); + const path = join(root, "capture.json"); + await writeFile(path, JSON.stringify(analysisViewCaptureEvidence())); + const reader = await holdFirstFileRead(path); + const controller = new AbortController(); + const reason = new Error("stop analysis-view read"); + try { + const pending = readEvidenceFileSource(path, { + signal: controller.signal, + }); + await reader.firstRead; + controller.abort(reason); + reader.release(); + const result = await pending; + expect(result).toMatchObject({ + ok: false, + error: { + _tag: "AnalysisCancelledError", + operation: "inspect_analysis_view", + }, + }); + if (result.ok || !(result.error instanceof AnalysisCancelledError)) + throw new Error("Expected cancelled analysis-view read"); + expect(result.error.cause).toBe(reason); + } finally { + controller.abort(reason); + reader.release(); + vi.restoreAllMocks(); + } + }); + + it("does not treat a caller reason with physical-read shape as a physical read", async () => { + const root = await createTestTempDirectory("rea-view-opaque-cancel-"); + const path = join(root, "capture.json"); + await writeFile(path, JSON.stringify(analysisViewCaptureEvidence())); + const reader = await holdFirstFileRead(path); + const controller = new AbortController(); + const reason = new JsonInputPhysicalReadFailure( + new Error("opaque caller reason with physical-failure shape"), + ); + try { + const pending = readEvidenceFileSource(path, { + signal: controller.signal, + }); + await reader.firstRead; + controller.abort(reason); + reader.release(); + const result = await pending; + expect(result).toMatchObject({ + ok: false, + error: { _tag: "AnalysisCancelledError", cause: reason }, + }); + if (result.ok || !(result.error instanceof AnalysisCancelledError)) + throw new Error( + "Expected caller cancellation, not physical I/O failure", + ); + expect(result.error.cause).toBe(reason); + } finally { + controller.abort(reason); + reader.release(); + vi.restoreAllMocks(); + } + }); +}); diff --git a/src/application/analysisView/evidenceFileSource.ts b/src/application/analysisView/evidenceFileSource.ts index 6e644b57ee..a4755234b6 100644 --- a/src/application/analysisView/evidenceFileSource.ts +++ b/src/application/analysisView/evidenceFileSource.ts @@ -16,7 +16,6 @@ import { } from "../../domain/evidenceErrors.js"; import { projectInputIssues } from "../../domain/inputIssueProjection.js"; import { readJsonFile } from "../JsonFiles.js"; -import { RegularFileCleanupFailure } from "../RegularFileRead.js"; /** The request's cancellation and the scope that owns descriptors whose close failed. */ export interface EvidenceFileReadOptions { @@ -79,25 +78,27 @@ export const readEvidenceFileSource = async ( ): Promise> => { const { signal, resources } = options; if (signal?.aborted) - return err(new AnalysisCancelledError(INSPECT_ANALYSIS_VIEW_OPERATION)); - const loaded = await readJsonFile(path, signal, resources).catch( - (cause: unknown) => { - // A failed close transfers a live handle owner; preserve that failure. - if ( - signal?.aborted === true && - !(cause instanceof RegularFileCleanupFailure) - ) - return err( - new AnalysisCancelledError(INSPECT_ANALYSIS_VIEW_OPERATION, { - cause, - }), - ); - throw cause; - }, - ); - if (!loaded.ok) return loaded; + return err( + new AnalysisCancelledError(INSPECT_ANALYSIS_VIEW_OPERATION, { + cause: signal.reason, + }), + ); + const loaded = await readJsonFile(path, signal, resources); + if (!loaded.ok) { + if (loaded.error instanceof AnalysisCancelledError) + return err( + new AnalysisCancelledError(INSPECT_ANALYSIS_VIEW_OPERATION, { + cause: loaded.error.cause, + }), + ); + return loaded; + } if (signal?.aborted) - return err(new AnalysisCancelledError(INSPECT_ANALYSIS_VIEW_OPERATION)); + return err( + new AnalysisCancelledError(INSPECT_ANALYSIS_VIEW_OPERATION, { + cause: signal.reason, + }), + ); const headroom = requireAuthenticationHeadroom(loaded.value, path); if (!headroom.ok) return headroom; try { diff --git a/src/application/process/ProcessCli.ts b/src/application/process/ProcessCli.ts index 6849847a2a..7c3fa62a48 100644 --- a/src/application/process/ProcessCli.ts +++ b/src/application/process/ProcessCli.ts @@ -238,17 +238,16 @@ const readJson = async ( operation: "capture_process_scenario" | "compare_process_captures", signal?: AbortSignal, ): Promise => { - if (signal?.aborted) throw new AnalysisCancelledError(operation); - const loaded = await readJsonFile(path, signal).catch((cause: unknown) => { - if ( - signal?.aborted === true && - !(cause instanceof RegularFileCleanupFailure) - ) - throw new AnalysisCancelledError(operation, { cause }); - throw cause; - }); - if (signal?.aborted) throw new AnalysisCancelledError(operation); - if (loaded.ok) return loaded.value; + if (signal?.aborted) + throw new AnalysisCancelledError(operation, { cause: signal.reason }); + const loaded = await readJsonFile(path, signal); + if (!loaded.ok && loaded.error instanceof AnalysisCancelledError) + throw new AnalysisCancelledError(operation, { cause: loaded.error.cause }); + if (loaded.ok) { + if (signal?.aborted) + throw new AnalysisCancelledError(operation, { cause: signal.reason }); + return loaded.value; + } if (loaded.error._tag !== "EvidenceFileError") throw loaded.error; if (loaded.error.reason === "io") throw loaded.error; const detail = describeValidationFailure(loaded.error.cause); diff --git a/src/cliJsonInput.ts b/src/cliJsonInput.ts index 24c54055e0..c7c0a36d72 100644 --- a/src/cliJsonInput.ts +++ b/src/cliJsonInput.ts @@ -1,7 +1,11 @@ import { absoluteCallerPath } from "./filesystem/callerPath.js"; import { sealOwnedJsonInput } from "./application/OwnedJsonInput.js"; -import { readJsonInputFile } from "./application/JsonInputFile.js"; +import { + JsonInputCancellationFailure, + JsonInputPhysicalReadFailure, + readJsonInputFile, +} from "./application/JsonInputFile.js"; import { NonRegularFileReadError } from "./filesystem/RegularFile.js"; import { AnalysisAccessDeniedError, @@ -22,14 +26,18 @@ export const parseCliJsonInput = async ( | { readonly ok: true; readonly value: unknown } | { readonly ok: false; readonly error: JsonValue } > => { - const cancelled = () => ({ + const cancelled = (cause: unknown) => ({ ok: false as const, error: { error: "Application workflow failed", - ...projectAnalysisError(new AnalysisCancelledError(operation)), + ...projectAnalysisError( + new AnalysisCancelledError(operation, { + cause, + }), + ), }, }); - if (signal?.aborted) return cancelled(); + if (signal?.aborted) return cancelled(signal.reason); try { const inline = safeParseJson(value); if (inline.ok) { @@ -52,7 +60,7 @@ export const parseCliJsonInput = async ( undefined, options?.snapshot ?? false, ); - if (parsed.ok && signal?.aborted) return cancelled(); + if (parsed.ok && signal?.aborted) return cancelled(signal.reason); return parsed.ok ? { ok: true, value: parsed.value } : { @@ -68,12 +76,17 @@ export const parseCliJsonInput = async ( }, }; } catch (cause: unknown) { - if ( - signal?.aborted && - (cause === signal.reason || - (cause instanceof Error && cause.name === "AbortError")) - ) - return cancelled(); + if (cause instanceof JsonInputCancellationFailure) + return cancelled(cause.reason); + if (cause instanceof JsonInputPhysicalReadFailure) { + const physicalCause = cause.physicalCause; + const systemCode = accessDeniedSystemCode(physicalCause); + return systemCode === undefined + ? jsonFileError(value, operation, "read-failed", physicalCause) + : jsonAccessDeniedError(value, operation, systemCode, physicalCause); + } + if (signal?.aborted && isJsonInputCancellation(cause, signal)) + return cancelled(signal.reason); const systemCode = accessDeniedSystemCode(cause); if (systemCode !== undefined) return jsonAccessDeniedError(value, operation, systemCode, cause); @@ -93,6 +106,17 @@ export const parseCliJsonInput = async ( } }; +const isJsonInputCancellation = ( + cause: unknown, + signal: AbortSignal, +): boolean => + cause === signal.reason || + (cause instanceof Error && + cause.name === "AbortError" && + "code" in cause && + cause.code === "ABORT_ERR" && + cause.cause === signal.reason); + const accessDeniedSystemCode = ( cause: unknown, ): "EACCES" | "EPERM" | undefined => { diff --git a/src/dotnet/ManagedSnapshot.ts b/src/dotnet/ManagedSnapshot.ts index 4edfd89f84..7d539171f9 100644 --- a/src/dotnet/ManagedSnapshot.ts +++ b/src/dotnet/ManagedSnapshot.ts @@ -61,7 +61,7 @@ export const readManagedSnapshot = async ( const bytes = await readBoundedFileBytes( handle, MAX_MANAGED_SNAPSHOT_BYTES, - signal, + { signal }, ); if (bytes === undefined) throw managedSnapshotCapacityError(operation); signal?.throwIfAborted(); diff --git a/src/ghidra/GhidraClient.ts b/src/ghidra/GhidraClient.ts index a18f086ab0..b8255f30b2 100644 --- a/src/ghidra/GhidraClient.ts +++ b/src/ghidra/GhidraClient.ts @@ -414,12 +414,16 @@ export class GhidraClient { const bytes = await readBoundedFileBytes( handle, maximumBytes ?? initial.size, - signal, - initial.size, - (reason) => - new ArtifactReaderFailure("cancelled", "Snapshot read cancelled", { - cause: reason, - }), + { + signal, + expectedSize: initial.size, + cancellationFailure: (reason) => + new ArtifactReaderFailure( + "cancelled", + "Snapshot read cancelled", + { cause: reason }, + ), + }, ); if (bytes === undefined || bytes.length !== initial.size) throw new RegularFileChangedError(path); diff --git a/src/ghidra/GhidraDosSnapshot.ts b/src/ghidra/GhidraDosSnapshot.ts index 143d715a70..603cfd6a30 100644 --- a/src/ghidra/GhidraDosSnapshot.ts +++ b/src/ghidra/GhidraDosSnapshot.ts @@ -30,13 +30,11 @@ export const attestGhidraDosSnapshot = async ( signal?: AbortSignal, ) => { const headerBytes = Math.min(fileBytes, 64); - const header = await readBoundedFileBytes( - handle, - headerBytes, + const header = await readBoundedFileBytes(handle, headerBytes, { signal, - headerBytes, - cancelled, - ); + expectedSize: headerBytes, + cancellationFailure: cancelled, + }); if (header === undefined || header.length !== headerBytes) throw new RegularFileChangedError(path); const extent = dosMzImageByteLength(header); diff --git a/src/native/AppleDispatchMetadata.ts b/src/native/AppleDispatchMetadata.ts index a54658f3c0..dfcbb5f924 100644 --- a/src/native/AppleDispatchMetadata.ts +++ b/src/native/AppleDispatchMetadata.ts @@ -271,15 +271,9 @@ export const inspectAppleDispatchMetadata = async ( const before = await handle.stat(); if (before.size > maximumBytes) throw capacityError(); if (signal?.aborted) throw cancelled(); - const read = await readBoundedFileBytes( - handle, - maximumBytes, + const read = await readBoundedFileBytes(handle, maximumBytes, { signal, - ).catch((cause: unknown) => { - // The bounded reader rejects with the caller's exact abort reason. - if (signal?.aborted === true && cause === signal.reason) - throw cancelled(); - throw cause; + cancellationFailure: () => cancelled(), }); if (read === undefined) throw capacityError(); const sha256 = createHash("sha256").update(read).digest("hex"); diff --git a/src/process/BoundedFileBytes.ts b/src/process/BoundedFileBytes.ts index fec1c5d988..a33d17c052 100644 --- a/src/process/BoundedFileBytes.ts +++ b/src/process/BoundedFileBytes.ts @@ -1,13 +1,32 @@ import type { FileHandle } from "node:fs/promises"; +export interface BoundedFileReadOptions { + readonly signal?: AbortSignal | undefined; + readonly expectedSize?: number | undefined; + readonly cancellationFailure?: ((reason: unknown) => Error) | undefined; + readonly operationFailure?: ((cause: unknown) => Error) | undefined; +} + /** Read through EOF, returning undefined on overflow; leave the handle open. */ export const readBoundedFileBytes = async ( handle: FileHandle, maxBytes: number, - signal?: AbortSignal, - expectedSize?: number, - cancellationFailure?: (reason: unknown) => Error, + options: BoundedFileReadOptions = {}, ): Promise => { + const { signal, expectedSize, cancellationFailure, operationFailure } = + options; + const read = async ( + buffer: Buffer, + offset: number, + length: number, + ): Promise<{ readonly bytesRead: number }> => { + try { + return await handle.read(buffer, offset, length, null); + } catch (cause: unknown) { + if (operationFailure !== undefined) throw operationFailure(cause); + throw cause; + } + }; const assertActive = (): void => { if (signal?.aborted && cancellationFailure !== undefined) throw cancellationFailure(signal.reason); @@ -28,11 +47,10 @@ export const readBoundedFileBytes = async ( let total = 0; for (;;) { assertActive(); - const { bytesRead } = await handle.read( + const { bytesRead } = await read( buffer, 0, Math.min(buffer.length, maxBytes - total + 1), - null, ); if (bytesRead === 0) return Buffer.concat(chunks, total); total += bytesRead; @@ -45,12 +63,7 @@ export const readBoundedFileBytes = async ( let total = 0; for (;;) { assertActive(); - const { bytesRead } = await handle.read( - buffer, - total, - expectedSize - total, - null, - ); + const { bytesRead } = await read(buffer, total, expectedSize - total); if (bytesRead === 0) return buffer.subarray(0, total); total += bytesRead; if (total > maxBytes) return undefined; diff --git a/tests/boundary/filesystem/boundedFileBytes.test.ts b/tests/boundary/filesystem/boundedFileBytes.test.ts index 02f34b841f..faa832de19 100644 --- a/tests/boundary/filesystem/boundedFileBytes.test.ts +++ b/tests/boundary/filesystem/boundedFileBytes.test.ts @@ -35,20 +35,21 @@ describe("bounded file byte reads", () => { const controller = new AbortController(); controller.abort(reason); await expect( - readBoundedFileBytes(handle, 5, controller.signal, expectedSize), + readBoundedFileBytes(handle, 5, { + signal: controller.signal, + expectedSize, + }), ).rejects.toBe(reason); const typed = new Error("typed cancellation", { cause: reason }); await expect( - readBoundedFileBytes( - handle, - 5, - controller.signal, + readBoundedFileBytes(handle, 5, { + signal: controller.signal, expectedSize, - (observed) => { + cancellationFailure: (observed) => { expect(observed).toBe(reason); return typed; }, - ), + }), ).rejects.toBe(typed); } } finally { diff --git a/tests/boundary/filesystem/cliJsonInputCancellation.test.ts b/tests/boundary/filesystem/cliJsonInputCancellation.test.ts index 8ecfa23fd7..8812af8b94 100644 --- a/tests/boundary/filesystem/cliJsonInputCancellation.test.ts +++ b/tests/boundary/filesystem/cliJsonInputCancellation.test.ts @@ -1,9 +1,10 @@ import { writeFile } from "node:fs/promises"; import { join } from "node:path"; -import { expect, it } from "vitest"; +import { expect, it, vi } from "vitest"; import { parseCliJsonInput } from "../../../src/cliJsonInput.js"; import { createTestTempDirectory } from "../../fixtures/temporaryDirectory.js"; +import { failFileReadAfterAbort } from "../../support/failFileReadAfterAbort.js"; it("reports an already-cancelled inline JSON read as cancellation", async () => { const controller = new AbortController(); @@ -29,6 +30,65 @@ it("cancels a pending regular-file JSON read through the owning reader", async ( }); }); +it.each([ + { label: "undefined", physical: undefined }, + { label: "null", physical: null }, +])( + "classifies a physical $label read rejection before late cancellation", + async ({ physical }) => { + const root = await createTestTempDirectory("rea-json-input-late-io-"); + const path = join(root, "input.json"); + await writeFile(path, '{"value":1}'); + const controller = new AbortController(); + await failFileReadAfterAbort( + path, + controller, + new Error("later cancellation"), + physical, + ); + try { + expect( + await parseCliJsonInput(path, "test-input", controller.signal), + ).toMatchObject({ + ok: false, + error: { + code: "invalid_request", + input_reason: "read-failed", + details: { operation: "test-input" }, + }, + }); + } finally { + vi.restoreAllMocks(); + } + }, +); + +it("keeps a physical rejection when it is the identical object used to abort", async () => { + const root = await createTestTempDirectory("rea-json-input-identical-abort-"); + const path = join(root, "input.json"); + await writeFile(path, '{"value":1}'); + const controller = new AbortController(); + const physical = Object.assign(new Error("descriptor EIO"), { code: "EIO" }); + await failFileReadAfterAbort(path, controller, physical, physical); + try { + expect( + await parseCliJsonInput(path, "test-input", controller.signal), + ).toMatchObject({ + ok: false, + error: { + code: "invalid_request", + input_reason: "read-failed", + details: { + operation: "test-input", + issues: [{ message: expect.stringContaining("EIO") }], + }, + }, + }); + } finally { + vi.restoreAllMocks(); + } +}); + it("cancels cooperative sealing of an owned inline Evidence input", async () => { const controller = new AbortController(); const input = JSON.stringify( diff --git a/tests/boundary/filesystem/evidenceBundleFiles.test.ts b/tests/boundary/filesystem/evidenceBundleFiles.test.ts index 2d80bcd8e9..d94b949c86 100644 --- a/tests/boundary/filesystem/evidenceBundleFiles.test.ts +++ b/tests/boundary/filesystem/evidenceBundleFiles.test.ts @@ -25,6 +25,9 @@ import { OwnedFileHandle } from "../../../src/filesystem/OwnedFileHandle.js"; import { projectAnalysisError } from "../../../src/domain/analysisErrorProjection.js"; import { createEvidence } from "../../../src/domain/evidence.js"; import { writeTextParts } from "../../../src/application/JsonFiles.js"; +import { failFileReadAfterAbort } from "../../support/failFileReadAfterAbort.js"; +import { holdFirstFileRead } from "../../support/holdFirstFileRead.js"; +import { JsonInputPhysicalReadFailure } from "../../../src/application/JsonInputFile.js"; import { createEvidenceBundle, serializeEvidenceBundle, @@ -47,30 +50,128 @@ it.each([ async (_, read) => { const root = await createTestTempDirectory("rea-bundle-pre-cancel-"); const controller = new AbortController(); - controller.abort(new Error("cancel before bundle read")); - expect( - await read(join(root, "missing.json"), controller.signal), - ).toMatchObject({ + const reason = new Error("cancel before bundle read"); + controller.abort(reason); + const result = await read(join(root, "missing.json"), controller.signal); + expect(result).toMatchObject({ ok: false, error: { _tag: "AnalysisCancelledError", operation: "import_evidence_bundle", }, }); + if (result.ok) throw new Error("Expected cancelled bundle read"); + expect(result.error.cause).toBe(reason); expect(await readdir(root)).toEqual([]); }, ); +it.each([ + ["mutable", readEvidenceBundle], + ["immutable", readImmutableEvidenceBundle], +] as const)( + "preserves the caller cancellation reason during %s read", + async (_, read) => { + const root = await createTestTempDirectory("rea-bundle-read-cancel-"); + const path = join(root, "bundle.json"); + await writeFile(path, JSON.stringify(bundle())); + const reader = await holdFirstFileRead(path); + const controller = new AbortController(); + const reason = new Error("stop bundle import"); + try { + const pending = read(path, controller.signal); + await reader.firstRead; + controller.abort(reason); + reader.release(); + const result = await pending; + expect(result).toMatchObject({ + ok: false, + error: { + _tag: "AnalysisCancelledError", + operation: "import_evidence_bundle", + }, + }); + if (result.ok) throw new Error("Expected cancelled bundle read"); + expect(result.error.cause).toBe(reason); + } finally { + controller.abort(reason); + reader.release(); + vi.restoreAllMocks(); + } + }, +); + +it.each([ + ["mutable", readEvidenceBundle], + ["immutable", readImmutableEvidenceBundle], +] as const)( + "keeps the physical null read cause for %s import after cancellation", + async (_, read) => { + const root = await createTestTempDirectory("rea-bundle-late-io-"); + const path = join(root, "bundle.json"); + await writeFile(path, JSON.stringify(bundle())); + const controller = new AbortController(); + await failFileReadAfterAbort( + path, + controller, + new Error("later abort"), + null, + ); + try { + const result = await read(path, controller.signal); + expect(result).toMatchObject({ + ok: false, + error: { _tag: "EvidenceFileError", reason: "io", path }, + }); + if (result.ok) throw new Error("Expected physical JSON read failure"); + expect(result.error.cause).toBeNull(); + } finally { + vi.restoreAllMocks(); + } + }, +); + +it.each([ + ["mutable", readEvidenceBundle], + ["immutable", readImmutableEvidenceBundle], +] as const)( + "keeps an identical physical and abort reason for %s import", + async (_, read) => { + const root = await createTestTempDirectory("rea-bundle-identical-abort-"); + const path = join(root, "bundle.json"); + await writeFile(path, JSON.stringify(bundle())); + const controller = new AbortController(); + const physical = new JsonInputPhysicalReadFailure( + Object.assign(new Error("physical EIO"), { code: "EIO" }), + ); + await failFileReadAfterAbort(path, controller, physical, physical); + try { + const result = await read(path, controller.signal); + expect(result).toMatchObject({ + ok: false, + error: { _tag: "EvidenceFileError", reason: "io" }, + }); + if (result.ok) throw new Error("Expected physical JSON read failure"); + expect(result.error.cause).toBe(physical); + } finally { + vi.restoreAllMocks(); + } + }, +); + it("preserves the failed-close owner when import is also cancelled", async () => { const root = await createTestTempDirectory("rea-import-cancel-cleanup-"); const path = join(root, "bundle.json"); await writeFile(path, JSON.stringify(bundle())); const controller = new AbortController(); const cleanupCause = new Error("fixture close failure"); + const cancellationReason = new JsonInputPhysicalReadFailure( + new Error("opaque cancellation reason"), + ); const close = vi .spyOn(OwnedFileHandle.prototype, "close") .mockImplementationOnce(async () => { - controller.abort(new Error("cancel during cleanup")); + controller.abort(cancellationReason); throw cleanupCause; }); let failure: unknown; diff --git a/tests/boundary/filesystem/processCliFileInput.test.ts b/tests/boundary/filesystem/processCliFileInput.test.ts index 07295b15c5..578d59d175 100644 --- a/tests/boundary/filesystem/processCliFileInput.test.ts +++ b/tests/boundary/filesystem/processCliFileInput.test.ts @@ -18,6 +18,8 @@ import * as jsonFiles from "../../../src/application/JsonFiles.js"; import { createTestTempDirectory } from "../../fixtures/temporaryDirectory.js"; import { holdFirstFileRead } from "../../support/holdFirstFileRead.js"; +import { failFileReadAfterAbort } from "../../support/failFileReadAfterAbort.js"; +import { JsonInputPhysicalReadFailure } from "../../../src/application/JsonInputFile.js"; describe("process CLI error projection", () => { it.each([ @@ -172,6 +174,79 @@ describe("process CLI file cancellation", () => { ); }); +it.each( + [ + { + label: "EIO", + physical: Object.assign(new Error("physical descriptor failure"), { + code: "EIO", + }), + }, + { label: "null", physical: null }, + { label: "undefined", physical: undefined }, + ].flatMap((failure) => [ + { ...failure, reader: "native" as const }, + { ...failure, reader: "streamed" as const }, + ]), +)( + "preserves a $reader descriptor $label when cancellation arrives after the read fails", + async ({ physical, reader }) => { + const root = await createTestTempDirectory("rea-process-cli-late-io-"); + const path = join(root, "scenario.json"); + const input = JSON.stringify({ + executable: process.execPath, + arguments: [], + }); + await writeFile(path, reader === "streamed" ? stream(input) : input); + const controller = new AbortController(); + await failFileReadAfterAbort( + path, + controller, + new Error("later caller cancellation"), + physical, + ); + try { + expect( + await captureProcessScenarioFile(path, process.env, controller.signal), + ).toMatchObject({ + code: "execution_failure", + category: "execution_failure", + retryable: false, + details: { operation: "read", reason: "io", path }, + }); + } finally { + vi.restoreAllMocks(); + } + }, +); + +it("keeps a physical read primary when its cause is also the abort reason", async () => { + const root = await createTestTempDirectory( + "rea-process-cli-identical-abort-", + ); + const path = join(root, "scenario.json"); + await writeFile( + path, + JSON.stringify({ executable: process.execPath, arguments: [] }), + ); + const controller = new AbortController(); + const physical = new JsonInputPhysicalReadFailure( + Object.assign(new Error("descriptor EIO"), { code: "EIO" }), + ); + await failFileReadAfterAbort(path, controller, physical, physical); + try { + expect( + await captureProcessScenarioFile(path, process.env, controller.signal), + ).toMatchObject({ + code: "execution_failure", + category: "execution_failure", + details: { operation: "read", reason: "io", path }, + }); + } finally { + vi.restoreAllMocks(); + } +}); + describe("process CLI input diagnostics", () => { it("keeps physical file I/O failures in the execution-error projection", async () => { const path = "/selected/scenario.json"; diff --git a/tests/boundary/filesystem/referenceSourceImport.test.ts b/tests/boundary/filesystem/referenceSourceImport.test.ts index 65c6d4b22d..9cb11b5053 100644 --- a/tests/boundary/filesystem/referenceSourceImport.test.ts +++ b/tests/boundary/filesystem/referenceSourceImport.test.ts @@ -1,5 +1,5 @@ import fs from "node:fs"; -import { chmod, mkdir, rm, symlink, writeFile } from "node:fs/promises"; +import { chmod, mkdir, open, rm, symlink, writeFile } from "node:fs/promises"; import { dirname, join } from "node:path"; import { add, commit, init } from "isomorphic-git"; @@ -8,6 +8,11 @@ import { describe, expect, it } from "vitest"; import { createTestTempDirectory } from "../../fixtures/temporaryDirectory.js"; import { normalizeHistoricalSourceParseFailures } from "../../../src/application/ReferenceSourceImport.js"; +import { prepareReferenceSourceImport } from "../../../src/application/ReferenceSourceImportPolicy.js"; +import { + readRegularFileText, + RegularFileReadFailure, +} from "../../../src/application/RegularFileRead.js"; import { importReferenceSource } from "../../support/referenceSourceResourceScope.js"; import { parseReferenceSourceEntries, @@ -15,6 +20,9 @@ import { } from "../../../src/application/ReferenceSourceImportEntries.js"; import type { ReferenceSourceRead } from "../../../src/reference/ReferenceSourceReaderTypes.js"; import { createHistoricalSourceManifest } from "../../../src/domain/referenceSourceGraph.js"; +import { ArtifactResourceScope } from "../../../src/artifacts/ArtifactResourceScope.js"; +import { OwnedFileHandle } from "../../../src/filesystem/OwnedFileHandle.js"; +import { RegularFileCleanupFailure } from "../../../src/filesystem/OwnedRegularFile.js"; const fixture = async (parent: string, name: string): Promise => { const root = join(parent, name); @@ -88,6 +96,260 @@ describe("reference source import projections", () => { }); }); +describe("reference source ignore policy read failures", () => { + it("preserves a late-aborted physical admission failure identical to the reason", async () => { + const root = await createTestTempDirectory("rea-reference-admission-read-"); + const path = join(root, "input.txt"); + await writeFile(path, "source\n"); + const controller = new AbortController(); + const physical = Object.assign(new Error("controlled admission failure"), { + code: "EIO", + }); + const result = readRegularFileText(path, { + signal: controller.signal, + cancellationFailure: (reason) => + new Error("owned cancellation checkpoint", { cause: reason }), + admit: () => { + controller.abort(physical); + throw physical; + }, + }); + + await expect(result).rejects.toMatchObject({ + _tag: new RegularFileReadFailure(physical)._tag, + operationCause: physical, + cause: physical, + }); + }); + + it.each([ + { + name: "distinct physical I/O failure", + physical: "error" as const, + sameAsAbortReason: false, + expected: "io", + }, + { + name: "physical failure identical to the abort reason", + physical: "error" as const, + sameAsAbortReason: true, + expected: "io", + }, + { + name: "non-Error physical failure", + physical: "string" as const, + sameAsAbortReason: false, + expected: "io", + }, + { + name: "cancellation after a successful physical read", + physical: "cancel-after-read" as const, + sameAsAbortReason: false, + expected: "cancelled", + }, + ])( + "preserves $name when the signal is aborted", + async ({ physical, sameAsAbortReason, expected }) => { + const root = await createTestTempDirectory("rea-reference-ignore-read-"); + const path = join(root, ".gitignore"); + await writeFile(path, "ignored/\n"); + const controller = new AbortController(); + const abortReason = new Error("caller cancelled"); + const readFailure: unknown = + physical === "string" + ? "controlled non-Error read failure" + : Object.assign(new Error("controlled read failure"), { + code: "EIO", + }); + const probe = await open(path, "r"); + const identity = await probe.stat(); + const prototype = probe.constructor.prototype as { + read: (...arguments_: never[]) => Promise; + }; + await probe.close(); + const originalRead = prototype.read; + let intercepted = false; + const read = async function ( + this: typeof probe, + ...arguments_: never[] + ): Promise { + if (!intercepted) { + const state = await this.stat(); + if (state.dev === identity.dev && state.ino === identity.ino) { + intercepted = true; + if (physical === "cancel-after-read") { + controller.abort(abortReason); + return originalRead.apply(this, arguments_); + } + controller.abort(sameAsAbortReason ? readFailure : abortReason); + throw readFailure; + } + } + return originalRead.apply(this, arguments_); + }; + prototype.read = read; + try { + const result = await prepareReferenceSourceImport( + { + root, + caller: "reference-import-test", + policy: { secretPatterns: [] }, + signal: controller.signal, + }, + new ArtifactResourceScope(), + ); + expect(intercepted).toBe(true); + expect(result.ok).toBe(false); + if (result.ok) throw new Error("Expected ignore policy read failure"); + expect(result.error.code).toBe(expected); + expect(result.error.cause).toBe( + physical === "cancel-after-read" ? abortReason : readFailure, + ); + } finally { + prototype.read = originalRead; + } + }, + ); +}); + +describe("reference source ignore policy cleanup ownership", () => { + it("keeps a physical read primary when ignore-file cleanup remains pending", async () => { + const root = await createTestTempDirectory("rea-reference-ignore-cleanup-"); + const path = join(root, ".gitignore"); + await writeFile(path, "ignored/\n"); + const controller = new AbortController(); + const physical = Object.assign(new Error("controlled read failure"), { + code: "EIO", + }); + const probe = await open(path, "r"); + const identity = await probe.stat(); + const prototype = probe.constructor.prototype as { + read: (...arguments_: never[]) => Promise; + }; + await probe.close(); + const originalRead = prototype.read; + const ownerPrototype = OwnedFileHandle.prototype; + const originalClose = ownerPrototype.close; + let interceptedRead = false; + let closeFailures = 0; + prototype.read = async function ( + this: typeof probe, + ...arguments_: never[] + ): Promise { + const state = await this.stat(); + if ( + !interceptedRead && + state.dev === identity.dev && + state.ino === identity.ino + ) { + interceptedRead = true; + controller.abort(physical); + throw physical; + } + return originalRead.apply(this, arguments_); + }; + ownerPrototype.close = async function ( + this: OwnedFileHandle, + ): Promise { + const state = await this.handle.stat(); + if ( + state.dev === identity.dev && + state.ino === identity.ino && + closeFailures < 2 + ) { + closeFailures += 1; + throw new Error(`controlled close failure ${closeFailures}`); + } + return originalClose.call(this); + }; + const resources = new ArtifactResourceScope(); + try { + const result = await prepareReferenceSourceImport( + { + root, + caller: "reference-import-test", + policy: { secretPatterns: [] }, + signal: controller.signal, + }, + resources, + ); + expect(interceptedRead).toBe(true); + expect(result.ok).toBe(false); + if (result.ok) throw new Error("Expected ignore policy read failure"); + expect(result.error.code).toBe("io"); + expect(result.error.cause).toBe(physical); + expect(closeFailures).toBe(2); + expect(result.error.cleanup?.resources).toContain(path); + expect(resources.hasPending).toBe(true); + } finally { + await resources.retryCleanup(); + prototype.read = originalRead; + ownerPrototype.close = originalClose; + } + expect(resources.hasPending).toBe(false); + }); + + it("does not transfer cleanup ownership for a physical read shaped like a foreign cleanup failure", async () => { + const root = await createTestTempDirectory("rea-reference-ignore-opaque-"); + const path = join(root, ".gitignore"); + const foreignPath = join(root, "foreign.txt"); + await writeFile(path, "ignored/\n"); + await writeFile(foreignPath, "foreign bytes"); + const foreignOwner = new OwnedFileHandle(await open(foreignPath, "r")); + const reason = new RegularFileCleanupFailure( + foreignPath, + foreignOwner, + { kind: "completed", value: "foreign value" }, + new Error("foreign cleanup failure"), + ); + const controller = new AbortController(); + const probe = await open(path, "r"); + const identity = await probe.stat(); + const prototype = probe.constructor.prototype as { + read: (...arguments_: never[]) => Promise; + }; + await probe.close(); + const originalRead = prototype.read; + let intercepted = false; + prototype.read = async function ( + this: typeof probe, + ...arguments_: never[] + ): Promise { + const state = await this.stat(); + if ( + !intercepted && + state.dev === identity.dev && + state.ino === identity.ino + ) { + intercepted = true; + controller.abort(reason); + throw reason; + } + return originalRead.apply(this, arguments_); + }; + try { + const result = await prepareReferenceSourceImport( + { + root, + caller: "reference-import-test", + policy: { secretPatterns: [] }, + signal: controller.signal, + }, + new ArtifactResourceScope(), + ); + expect(intercepted).toBe(true); + expect(result.ok).toBe(false); + if (result.ok) throw new Error("Expected ignore policy read failure"); + expect(result.error.code).toBe("io"); + expect(result.error.cause).toBe(reason); + expect(foreignOwner.handle.fd).toBeGreaterThanOrEqual(0); + } finally { + prototype.read = originalRead; + await foreignOwner.close(); + } + }); +}); + describe("reference source symlink import", () => { it.each([ { target: "dist", reason: "default-ignored", pattern: "dist/" }, diff --git a/tests/support/failFileReadAfterAbort.ts b/tests/support/failFileReadAfterAbort.ts new file mode 100644 index 0000000000..2890dcaac5 --- /dev/null +++ b/tests/support/failFileReadAfterAbort.ts @@ -0,0 +1,40 @@ +import { open, type FileHandle } from "node:fs/promises"; +import { vi } from "vitest"; + +type PositionalFileRead = ( + this: FileHandle, + ...args: [Buffer, number, number, number | null] +) => Promise<{ readonly bytesRead: number; readonly buffer: Buffer }>; + +/** Fail one selected descriptor read after aborting with an independently chosen reason. */ +export const failFileReadAfterAbort = async ( + path: string, + controller: AbortController, + abortReason: unknown, + physicalCause: unknown, +): Promise => { + const probe = await open(path, "r"); + const identity = await probe.stat(); + const prototype = Object.getPrototypeOf(probe) as { + read: PositionalFileRead; + }; + await probe.close(); + const originalRead = prototype.read; + let failed = false; + vi.spyOn(prototype, "read").mockImplementation(async function ( + this: FileHandle, + ...args: [Buffer, number, number, number | null] + ) { + const current = await this.stat(); + if ( + !failed && + current.dev === identity.dev && + current.ino === identity.ino + ) { + failed = true; + controller.abort(abortReason); + throw physicalCause; + } + return originalRead.call(this, ...args); + }); +};