diff --git a/docs/cli.md b/docs/cli.md index 9ac2da7dc7..9cf61a3acc 100644 --- a/docs/cli.md +++ b/docs/cli.md @@ -496,10 +496,12 @@ error and exit `1`, as does a listener that fails after binding. pair refusals, a non-empty or file-occupied directory, a missing package tarball or a name that is not a kebab-case label or is a Windows device name, writing nothing in each case. -A write that fails after the first file (a permission or disk-space failure -on a later path) propagates as the host's failure and leaves the partial -folder in place for inspection; remove it before retrying, since the next -attempt refuses the non-empty directory. +A write that fails after the first file (a permission, disk-space or +pathname-length failure on a later path) propagates as the host's failure, +naming its cause and, when it failed on one bundle file or the package +tarball, that file. A failure while syncing the finished folder names only its +cause. Either way the partial folder stays in place for inspection; remove it +before retrying, since the next attempt refuses the non-empty directory. A `cancelled` result does not by itself determine the exit status; it depends on the command's signal handling. Commands routed through the shared diff --git a/src/application/environment/environmentBundleWrite.test.ts b/src/application/environment/environmentBundleWrite.test.ts index 6bbac53e89..76d73c0398 100644 --- a/src/application/environment/environmentBundleWrite.test.ts +++ b/src/application/environment/environmentBundleWrite.test.ts @@ -9,6 +9,7 @@ import { writeFile, } from "node:fs/promises"; import { join } from "node:path"; +import { Readable } from "node:stream"; import { expect, it, vi } from "vitest"; @@ -18,12 +19,72 @@ import { writeEnvironmentServeFiles, } from "../../../tests/fixtures/environmentServe.js"; import { SafeOutputTree } from "../../artifacts/SafeOutputTree.js"; +import * as fileChunks from "../../filesystem/readFileHandleChunks.js"; import { ArtifactReaderFailure } from "../../artifacts/ArtifactReader.js"; +import { + AnalysisAccessDeniedError, + AnalysisOutputError, +} from "../../domain/analysisErrorCore.js"; import { exportEnvironmentBundle, readEnvironmentExportInputs, } from "./environmentExport.js"; +vi.mock("../../filesystem/readFileHandleChunks.js", { spy: true }); + +it("attributes a pathless tarball read failure during the copy to the tarball", async () => { + const root = await createTestTempDirectory("rea-tarball-read-failure-"); + const files = await writeEnvironmentServeFiles(root); + const inputs = await readEnvironmentExportInputs( + files.manifestPath, + files.capturePath, + ); + if (!inputs.ok) throw inputs.error; + const tarball = join(root, "package.tgz"); + await writeFile(tarball, "admitted package"); + const directory = join(root, "bundle"); + const actual = await vi.importActual( + "../../filesystem/readFileHandleChunks.js", + ); + // Only the tarball descriptor is read twice: once to hash, once to copy. + const reads = new Map(); + const reader = vi + .mocked(fileChunks.readFileHandleChunks) + .mockImplementation((handle, options) => { + const count = (reads.get(handle) ?? 0) + 1; + reads.set(handle, count); + if (count < 2) return actual.readFileHandleChunks(handle, options); + const failure = Object.assign(new Error("EIO: i/o error, read"), { + code: "EIO", + syscall: "read", + }); + return Object.assign( + new Readable({ + read() { + this.destroy(failure); + }, + }), + { readCoverage: () => ({ status: "unknown" as const }) }, + ) as unknown as ReturnType; + }); + try { + const result = await exportEnvironmentBundle({ + ...inputs.value, + source: environmentServeSource(), + directory, + packageTarball: tarball, + }); + if (result.ok || !(result.error instanceof AnalysisOutputError)) + throw new Error("expected an output failure"); + expect(result.error.reason).toContain(`reading ${tarball}`); + expect(result.error.remediationAction).toContain(tarball); + expect(result.error.remediationAction).not.toContain("disk space"); + expect(result.error.diagnostics).toEqual({ output_directory: directory }); + } finally { + reader.mockRestore(); + } +}); + it("retains the admitted tarball descriptor and refuses a pathname replacement during output", async () => { const root = await createTestTempDirectory("rea-tarball-instance-"); const files = await writeEnvironmentServeFiles(root); @@ -186,6 +247,7 @@ it.skipIf(process.platform === "win32" || process.getuid?.() === 0)( error: { _tag: "AnalysisAccessDeniedError", operation: "environment-export", + diagnostics: { output_directory: directory }, }, }); expect(await readdir(directory)).toEqual([]); @@ -195,6 +257,111 @@ it.skipIf(process.platform === "win32" || process.getuid?.() === 0)( }, ); +it.skipIf(process.platform === "win32" || process.getuid?.() === 0)( + "names the nested path when write permission is lost after earlier files", + async () => { + const root = await createTestTempDirectory("rea-bundle-later-denied-"); + const files = await writeEnvironmentServeFiles(root); + const inputs = await readEnvironmentExportInputs( + files.manifestPath, + files.capturePath, + ); + if (!inputs.ok) throw inputs.error; + const directory = join(root, "bundle"); + const original = SafeOutputTree.prototype.write; + let writes = 0; + // The real host refuses the second file once the directory is read-only. + const writer = vi + .spyOn(SafeOutputTree.prototype, "write") + .mockImplementation(async function (this: SafeOutputTree, ...args) { + writes += 1; + if (writes === 2) await chmod(directory, 0o500); + return original.apply(this, args); + }); + try { + const result = await exportEnvironmentBundle({ + ...inputs.value, + source: environmentServeSource(), + directory, + }); + if (result.ok) throw new Error("expected a permission refusal"); + expect(result.error).toMatchObject({ + _tag: "AnalysisAccessDeniedError", + diagnostics: { output_directory: directory }, + }); + if (!(result.error instanceof AnalysisAccessDeniedError)) + throw result.error; + expect(result.error.path.startsWith(`${directory}/`)).toBe(true); + expect(await readdir(directory)).not.toEqual([]); + } finally { + writer.mockRestore(); + await chmod(directory, 0o700); + } + }, +); + +it.each([ + { + failure: "pathless descriptor failure", + // A disk-full `FileHandle.write()` reports a code and syscall, no path. + error: () => + Object.assign(new Error("ENOSPC: no space left on device, write"), { + code: "ENOSPC", + syscall: "write", + }), + }, + { + failure: "integrity refusal", + error: () => + new ArtifactReaderFailure( + "integrity", + "Extracted content disagrees with inventory", + ), + }, +] as const)( + "reports a later $failure with its own recovery", + async ({ failure, error }) => { + const root = await createTestTempDirectory("rea-bundle-later-failure-"); + const files = await writeEnvironmentServeFiles(root); + const inputs = await readEnvironmentExportInputs( + files.manifestPath, + files.capturePath, + ); + if (!inputs.ok) throw inputs.error; + const directory = join(root, "bundle"); + const original = SafeOutputTree.prototype.write; + const targets: string[] = []; + const writer = vi + .spyOn(SafeOutputTree.prototype, "write") + .mockImplementation(async function (this: SafeOutputTree, ...args) { + targets.push(args[0]); + if (targets.length === 2) throw error(); + return original.apply(this, args); + }); + try { + const result = await exportEnvironmentBundle({ + ...inputs.value, + source: environmentServeSource(), + directory, + }); + if (result.ok || !(result.error instanceof AnalysisOutputError)) + throw new Error("expected an output failure"); + expect(result.error.diagnostics).toEqual({ + output_directory: directory, + }); + if (failure === "pathless descriptor failure") { + expect(result.error.reason).toContain( + join(directory, targets[1] ?? ""), + ); + expect(result.error.remediationAction).toContain("free disk space"); + } else expect(result.error.remediationAction).toBeUndefined(); + expect(await readdir(directory)).not.toEqual([]); + } finally { + writer.mockRestore(); + } + }, +); + it .skipIf(process.platform === "win32") .each(["replace-root", "foreign-entry", "symlink-parent"] as const)( diff --git a/src/application/environment/environmentBundleWrite.ts b/src/application/environment/environmentBundleWrite.ts index d3ec3c55dc..47f225e8a0 100644 --- a/src/application/environment/environmentBundleWrite.ts +++ b/src/application/environment/environmentBundleWrite.ts @@ -1,6 +1,6 @@ import { createHash } from "node:crypto"; import { mkdir, type FileHandle } from "node:fs/promises"; -import { dirname } from "node:path"; +import { dirname, join } from "node:path"; import { Readable } from "node:stream"; import { @@ -12,6 +12,7 @@ import { ArtifactFileCleanupFailure } from "../../artifacts/ArtifactFileOperatio import { RegularFileCleanupFailure } from "../../filesystem/OwnedRegularFile.js"; import { readFileHandleChunks } from "../../filesystem/readFileHandleChunks.js"; import { streamChunkToBuffer } from "../../artifacts/StreamBytes.js"; +import { ArtifactReaderFailure } from "../../artifacts/ArtifactReader.js"; import { ArtifactResourceScope } from "../../artifacts/ArtifactResourceScope.js"; import { SafeOutputTree } from "../../artifacts/SafeOutputTree.js"; import { SafeOutputTreeCreationFailure } from "../../artifacts/SafeOutputTreeCreationFailure.js"; @@ -26,15 +27,69 @@ import type { WrittenBundleFile, } from "./environmentExport.js"; +/** + * A host failure on one file the bundle writer was reading or writing. + * Descriptor operations such as `read`, `write` and `fsync` fail without a + * path, so this names the file and the direction of the failed operation. + */ +export class BundleFileHostFailure extends Error { + constructor( + readonly path: string, + readonly operation: "reading" | "writing", + cause: Error, + ) { + super(`${cause.message}, ${operation} ${path}`, { cause }); + } +} + +/** An untyped host system failure, which carries an errno-style code. */ +const isHostSystemFailure = (cause: unknown): cause is Error => + cause instanceof Error && + !(cause instanceof ArtifactReaderFailure) && + typeof Reflect.get(cause, "code") === "string"; + +/** Attribute a pathless host failure to the bundle file being written. */ +const writingBundleFile = async ( + directory: string, + relative: string, + write: () => Promise, +): Promise => { + try { + return await write(); + } catch (cause: unknown) { + if (isHostSystemFailure(cause) && Reflect.get(cause, "path") === undefined) + throw new BundleFileHostFailure( + join(directory, relative), + "writing", + cause, + ); + throw cause; + } +}; + +/** Read the admitted tarball, attributing host failures to the source. */ +async function* tarballChunks(source: BundleTarball): AsyncIterable { + try { + yield* readFileHandleChunks(source.handle); + } catch (cause: unknown) { + if (isHostSystemFailure(cause)) + throw new BundleFileHostFailure(source.path, "reading", cause); + throw cause; + } +} + const writeBytes = async ( tree: SafeOutputTree, + directory: string, relative: string, bytes: Buffer, ): Promise => { - const written = await tree.write(relative, Readable.from([bytes]), { - bytes: bytes.length, - sha256: createHash("sha256").update(bytes).digest("hex"), - }); + const written = await writingBundleFile(directory, relative, () => + tree.write(relative, Readable.from([bytes]), { + bytes: bytes.length, + sha256: createHash("sha256").update(bytes).digest("hex"), + }), + ); return { path: written.relativePath, bytes: written.bytesWritten }; }; @@ -47,29 +102,50 @@ interface BundleTarball { /** Hash and copy only the instance retained before destination admission. */ const writeTarball = async ( tree: SafeOutputTree, + directory: string, relative: string, source: BundleTarball, ): Promise => { const hash = createHash("sha256"); let observed = 0; - for await (const chunk of readFileHandleChunks(source.handle)) { + for await (const chunk of tarballChunks(source)) { const bytes = streamChunkToBuffer(chunk); observed += bytes.length; if (observed > source.size) throw new RegularFileChangedError(source.path); hash.update(bytes); } if (observed !== source.size) throw new RegularFileChangedError(source.path); - const written = await tree.write( - relative, - readFileHandleChunks(source.handle), - { + const sha256 = hash.digest("hex"); + const written = await writingBundleFile(directory, relative, () => + tree.write(relative, Readable.from(tarballChunks(source)), { bytes: source.size, - sha256: hash.digest("hex"), - }, + sha256, + }), ); return { path: written.relativePath, bytes: written.bytesWritten }; }; +/** + * Only destination admission can blame the selected directory. Path failures + * after admission are host write failures on bundle files, not argument refusals. + */ +const directoryRefusal = (cause: unknown): unknown => + cause instanceof ArtifactReaderFailure && + cause.reason === "path" && + cause.cleanup === undefined + ? new AnalysisInputError( + ENVIRONMENT_EXPORT_OPERATION, + { cause, userMessage: cause.message }, + [ + { + path: ["directory"], + reason: "invalid_value", + message: cause.message, + }, + ], + ) + : cause; + /** The output owner anchors every create and verifies the complete tree before publication. */ const writeBundleFiles = async ( input: EnvironmentExportInput, @@ -79,6 +155,7 @@ const writeBundleFiles = async ( const resources = new ArtifactResourceScope(); let tree: SafeOutputTree | undefined; let written: WrittenBundleFile[] | undefined; + let admitted = false; let outcome: | { readonly kind: "completed"; @@ -95,19 +172,37 @@ const writeBundleFiles = async ( if (cause instanceof SafeOutputTreeCreationFailure) tree = cause.tree; throw cause; } + admitted = true; written = []; for (const file of plan.files) written.push( - await writeBytes(tree, file.path, Buffer.from(file.content)), + await writeBytes( + tree, + input.directory, + file.path, + Buffer.from(file.content), + ), ); written.push( - await writeBytes(tree, plan.copies.manifest, input.manifest.bytes), + await writeBytes( + tree, + input.directory, + plan.copies.manifest, + input.manifest.bytes, + ), ); written.push( - await writeBytes(tree, plan.copies.evidence, input.evidence.bytes), + await writeBytes( + tree, + input.directory, + plan.copies.evidence, + input.evidence.bytes, + ), ); if (plan.copies.tarball !== null && tarball !== undefined) - written.push(await writeTarball(tree, plan.copies.tarball, tarball)); + written.push( + await writeTarball(tree, input.directory, plan.copies.tarball, tarball), + ); await tree.commit(); outcome = { kind: "completed", files: written }; } catch (cause: unknown) { @@ -125,7 +220,7 @@ const writeBundleFiles = async ( } if (outcome.kind === "failed") { if (owner?.published === true && written !== undefined) return written; - throw outcome.cause; + throw admitted ? outcome.cause : directoryRefusal(outcome.cause); } return outcome.files; }; diff --git a/src/application/environment/environmentExport.ts b/src/application/environment/environmentExport.ts index 0709b0d5c5..14f5696b62 100644 --- a/src/application/environment/environmentExport.ts +++ b/src/application/environment/environmentExport.ts @@ -1,3 +1,4 @@ +import { isAbsolute } from "node:path"; import { getHeapStatistics } from "node:v8"; import { AnalysisError } from "../../domain/analysisErrorBase.js"; @@ -21,7 +22,10 @@ import { AnalysisCleanupLease } from "../../artifacts/AnalysisCleanupLease.js"; import { ArtifactResourceScope } from "../../artifacts/ArtifactResourceScope.js"; import { RegularFileCleanupFailure } from "../../filesystem/OwnedRegularFile.js"; import { ArtifactReaderFailure } from "../../artifacts/ArtifactReader.js"; -import { writeEnvironmentBundle } from "./environmentBundleWrite.js"; +import { + BundleFileHostFailure, + writeEnvironmentBundle, +} from "./environmentBundleWrite.js"; import { sealOwnedJsonInput } from "../OwnedJsonInput.js"; import { nativeJsonParseRequiredBytes } from "../JsonInputFile.js"; import { readRegularFile } from "../RegularFileRead.js"; @@ -90,9 +94,36 @@ const systemCode = (cause: unknown): string | undefined => ? cause.code : undefined; +/** Recovery for a host write failure that may leave partial output behind. */ +const hostWriteFailure = (directory: string) => ({ + userMessage: `Writing the environment bundle to ${directory} failed on the host; the details name the cause and, when it failed on one bundle file, that file.`, + remediationAction: `Resolve the reported host failure (for example, choose a shorter destination path or free disk space). Inspect ${directory} and remove any partial output this export wrote before retrying, since a retry refuses a non-empty directory.`, + diagnostics: { output_directory: directory }, +}); + +/** Recovery for a host failure reading the package tarball during the copy. */ +const hostReadFailure = (source: string, directory: string) => ({ + userMessage: `Reading ${source} failed on the host while writing the environment bundle to ${directory}; the details name the cause.`, + remediationAction: `Resolve the reported host failure reading ${source}. Inspect ${directory} and remove any partial output this export wrote before retrying, since a retry refuses a non-empty directory.`, + diagnostics: { output_directory: directory }, +}); + +/** The host failure beneath a wrapper, whose message names its code and absolute path. */ +const hostSystemError = (cause: Error): Error | undefined => { + const seen = new Set([cause]); + let current = cause.cause; + while (current instanceof Error && !seen.has(current)) { + if (systemCode(current) !== undefined) return current; + seen.add(current); + current = current.cause; + } + return undefined; +}; + const accessDeniedError = ( cause: unknown, path: string, + diagnostics?: Readonly>, ): AnalysisAccessDeniedError | undefined => { const seen = new Set(); let current = cause; @@ -106,6 +137,7 @@ const accessDeniedError = ( code, { cause, + ...(diagnostics === undefined ? {} : { diagnostics }), ...(cause instanceof ArtifactReaderFailure && cause.cleanup !== undefined ? { cleanup: cause.cleanup } @@ -120,6 +152,21 @@ const accessDeniedError = ( return undefined; }; +/** The absolute path a host failure beneath wrappers names, if it names one. */ +const hostFailurePath = (cause: unknown): string | undefined => { + const seen = new Set(); + let current = cause; + while (current instanceof Error && !seen.has(current)) { + seen.add(current); + if (current instanceof BundleFileHostFailure) return current.path; + const path = Reflect.get(current, "path"); + if (systemCode(current) !== undefined && typeof path === "string") + return isAbsolute(path) ? path : undefined; + current = current.cause; + } + return undefined; +}; + /** Host permission failures retain their typed cause; unexpected failures propagate. */ const rethrowUnlessDenied = (cause: unknown, path: string): never => { throw accessDeniedError(cause, path) ?? cause; @@ -291,7 +338,13 @@ export const exportEnvironmentBundle = async ( files = await writeEnvironmentBundle(input, plan.value); } catch (cause: unknown) { if (cause instanceof AnalysisError) return err(cause); - const denied = accessDeniedError(cause, input.directory); + // A refused bundle write names the path the host refused, which may be a + // nested file after earlier files were written into the directory. + const denied = accessDeniedError( + cause, + hostFailurePath(cause) ?? input.directory, + { output_directory: input.directory }, + ); if (denied !== undefined) return err(denied); if (cause instanceof RegularFileChangedError) return err( @@ -303,32 +356,37 @@ export const exportEnvironmentBundle = async ( ), ); if (cause instanceof ArtifactReaderFailure) { - if (cause.reason === "path" && cause.cleanup === undefined) - return err( - inputError( - { - path: ["directory"], - reason: "invalid_value", - message: cause.message, - }, - cause.message, - ), - ); + const host = hostSystemError(cause); const error = new AnalysisOutputError( ENVIRONMENT_EXPORT_OPERATION, - cause.message, + host === undefined + ? cause.message + : `${cause.message}: ${host.message}`, { cause, - ...(cause.cleanup === undefined ? {} : { cleanup: cause.cleanup }), + // Only a host failure gets host recovery; an integrity or other + // refusal keeps its own message without disk-space advice. + ...(cause.cleanup !== undefined + ? { cleanup: cause.cleanup } + : host === undefined + ? { diagnostics: { output_directory: input.directory } } + : hostWriteFailure(input.directory)), }, ); bindArtifactFileCleanup(cause, error); return err(error); } - if (cause instanceof Error && systemCode(cause) !== undefined) + if ( + cause instanceof BundleFileHostFailure || + (cause instanceof Error && systemCode(cause) !== undefined) + ) return err( new AnalysisOutputError(ENVIRONMENT_EXPORT_OPERATION, cause.message, { cause, + ...(cause instanceof BundleFileHostFailure && + cause.operation === "reading" + ? hostReadFailure(cause.path, input.directory) + : hostWriteFailure(input.directory)), }), ); throw cause; diff --git a/tests/boundary/cli/environmentExportCli.test.ts b/tests/boundary/cli/environmentExportCli.test.ts index 7f6dae529f..9fdb69339f 100644 --- a/tests/boundary/cli/environmentExportCli.test.ts +++ b/tests/boundary/cli/environmentExportCli.test.ts @@ -2,6 +2,7 @@ import { mkdir, readdir, readFile, + realpath, stat, symlink, writeFile, @@ -285,3 +286,57 @@ cliTest.skipIf(process.platform === "win32")( ).rejects.toMatchObject({ code: "ENOENT" }); }, ); + +const HOST_PATH_MAX: Partial> = { + darwin: 1024, + linux: 4096, +}; + +cliTest.skipIf(HOST_PATH_MAX[process.platform] === undefined)( + "reports a later nested-file pathname-limit failure as a host write failure and keeps the partial folder", + async ({ cli }) => { + const limit = HOST_PATH_MAX[process.platform] ?? 0; + const root = await realpath( + await createTestTempDirectory("rea-export-long-path-"), + ); + const files = await writeEnvironmentServeFiles(root); + // Shorter bundle paths fit; the 41-byte nested verifier path reaches the host limit. + const length = limit - 40; + let parent = join(root, "long"); + while (Buffer.byteLength(parent) + 1 + 200 < length - 1) + parent = join(parent, "d".repeat(200)); + await mkdir(parent, { recursive: true }); + const directory = join( + parent, + "l".repeat(length - Buffer.byteLength(parent) - 1), + ); + expect(Buffer.byteLength(directory)).toBe(length); + const failed = join(directory, "artifacts/check_grafana-example/verify.py"); + const result = await cli.run({ + arguments: [ + "environment-export", + files.manifestPath, + "--evidence", + files.capturePath, + directory, + "--json", + ], + timeoutMs: 30_000, + }); + expect(result.exitCode).toBe(1); + expect(result.json).toMatchObject({ + code: "unreadable_output", + remediation: { action: expect.stringContaining(directory) }, + details: { + operation: "environment-export", + diagnostics: { output_directory: directory }, + }, + }); + expect(result.json).not.toHaveProperty("details.issues"); + const reason = (result.json as { details: { reason: string } }).details + .reason; + expect(reason).toContain("ENAMETOOLONG"); + expect(reason).toContain(failed); + expect(await readFile(join(directory, "README.md"))).not.toHaveLength(0); + }, +);