diff --git a/src/application/JavaScriptApplicationService.ts b/src/application/JavaScriptApplicationService.ts index b47b6ed5e..a7e3ecdd8 100644 --- a/src/application/JavaScriptApplicationService.ts +++ b/src/application/JavaScriptApplicationService.ts @@ -75,7 +75,12 @@ export const analyzeJavaScriptApplicationValidated = async ( } catch (cause: unknown) { if (cause instanceof ArtifactReaderFailure) return err( - new ArtifactOperationError(OPERATION, cause.reason, cause.details), + new ArtifactOperationError( + OPERATION, + cause.reason, + cause.details, + cause.message, + ), ); if (cause instanceof z.ZodError) return err( diff --git a/src/artifacts/ArtifactPaths.ts b/src/artifacts/ArtifactPaths.ts index e6dd950e9..d5ec35cb4 100644 --- a/src/artifacts/ArtifactPaths.ts +++ b/src/artifacts/ArtifactPaths.ts @@ -14,7 +14,7 @@ export const normalizeArtifactPath = (input: string): string => { ) throw new ArtifactReaderFailure( "path", - "Artifact path is absolute or unsafe", + `Artifact path is absolute or unsafe: ${JSON.stringify(input)}`, ); const normalized = input.normalize("NFC").replace(/\/+$/u, ""); const parts = normalized.split("/"); @@ -23,7 +23,10 @@ export const normalizeArtifactPath = (input: string): string => { parts.some((part) => part === "" || part === "." || part === "..") || posix.normalize(normalized) !== normalized ) - throw new ArtifactReaderFailure("path", "Artifact path is not normalized"); + throw new ArtifactReaderFailure( + "path", + `Artifact path is not normalized: ${JSON.stringify(input)}`, + ); return normalized; }; @@ -45,7 +48,7 @@ export class ArtifactPathRegistry { if (child !== undefined && child.spelling !== part) throw new ArtifactReaderFailure( "path", - `Artifact path collision: ${path}`, + `Artifact path collision: ${path} differs only in case from ${[...parts.slice(0, index), child.spelling].join("/")}`, ); if (child === undefined) { child = { spelling: part, kind: undefined, children: new Map() }; diff --git a/src/artifacts/ArtifactProvider.ts b/src/artifacts/ArtifactProvider.ts index e095d0dfe..370caa1af 100644 --- a/src/artifacts/ArtifactProvider.ts +++ b/src/artifacts/ArtifactProvider.ts @@ -343,7 +343,12 @@ const translateFailure = ( cause: unknown, ): AnalysisError => { if (cause instanceof ArtifactReaderFailure) - return new ArtifactOperationError(operation, cause.reason, cause.details); + return new ArtifactOperationError( + operation, + cause.reason, + cause.details, + cause.message, + ); return new ArtifactOperationError(operation, "io"); }; diff --git a/src/domain/analysisErrorProjection.ts b/src/domain/analysisErrorProjection.ts index 66f432a37..f4e8b6197 100644 --- a/src/domain/analysisErrorProjection.ts +++ b/src/domain/analysisErrorProjection.ts @@ -216,6 +216,7 @@ const artifactStateErrorDetails = ( operation: error.operation, reason: error.reason, ...(error.reason === "limit" ? { truncated: true } : {}), + ...(error.detail === undefined ? {} : { detail: error.detail }), }; if (error instanceof UnknownRegistryError) return { reason: error.reason }; if (error instanceof EvidenceFileError) diff --git a/src/domain/artifactOperationError.ts b/src/domain/artifactOperationError.ts index 238992196..41d482ab8 100644 --- a/src/domain/artifactOperationError.ts +++ b/src/domain/artifactOperationError.ts @@ -28,6 +28,8 @@ export class ArtifactOperationError extends AnalysisError { calculatedSha256: string | null; unpacked: boolean; }>, + /** The specific constraint that failed, such as the colliding path. */ + readonly detail?: string, ) { super( artifactDetails === undefined diff --git a/tests/boundary/providers/native/artifactProvider.archiveSafety.test.ts b/tests/boundary/providers/native/artifactProvider.archiveSafety.test.ts index 3c8965328..223811110 100644 --- a/tests/boundary/providers/native/artifactProvider.archiveSafety.test.ts +++ b/tests/boundary/providers/native/artifactProvider.archiveSafety.test.ts @@ -96,7 +96,7 @@ describe("artifact archive safety", () => { expect(unsafePathError).toBeInstanceOf(ArtifactReaderFailure); expect(unsafePathError).toMatchObject({ reason: "path", - message: "Artifact path is not normalized", + message: 'Artifact path is not normalized: "../escape"', }); const registry = new ArtifactPathRegistry(); registry.add("A.js", "file"); @@ -109,7 +109,7 @@ describe("artifact archive safety", () => { expect(collisionError).toBeInstanceOf(ArtifactReaderFailure); expect(collisionError).toMatchObject({ reason: "path", - message: "Artifact path collision: a.js", + message: "Artifact path collision: a.js differs only in case from A.js", }); for (const [filePath, childPath] of [ diff --git a/tests/boundary/providers/native/artifactProvider.failureDetail.test.ts b/tests/boundary/providers/native/artifactProvider.failureDetail.test.ts new file mode 100644 index 000000000..bf2669583 --- /dev/null +++ b/tests/boundary/providers/native/artifactProvider.failureDetail.test.ts @@ -0,0 +1,35 @@ +import { join } from "node:path"; + +import { expect, it } from "vitest"; + +import { writeOrderedZip } from "../../../fixtures/artifactEntryOrder.js"; +import { createTestTempDirectory } from "../../../fixtures/temporaryDirectory.js"; + +import { parseBinaryTarget } from "../../../../src/application/BinaryTargetResolver.js"; +import { ArtifactProvider } from "../../../../src/artifacts/ArtifactProvider.js"; +import { projectAnalysisError } from "../../../../src/domain/analysisErrorProjection.js"; + +it.each([ + [ + ["Main.js", "main.js"], + "Artifact path collision: main.js differs only in case from Main.js", + ], + [["a\\b.js"], 'Artifact path is absolute or unsafe: "a\\\\b.js"'], +] as const)( + "reports the failed path constraint for %j", + async (entries, detail) => { + const root = await createTestTempDirectory("rea-artifact-detail-"); + const path = join(root, "fixture.zip"); + await writeOrderedZip(path, entries); + const target = await parseBinaryTarget(path); + if (!target.ok) throw new Error("expected a ZIP target"); + const result = await new ArtifactProvider() + .createClient(target.value) + .execute("inventory_artifact", {}); + if (result.ok) throw new Error("expected the path constraint to fail"); + expect(projectAnalysisError(result.error)).toMatchObject({ + code: "artifact_operation_failed", + details: { operation: "inventory_artifact", reason: "path", detail }, + }); + }, +);