diff --git a/src/artifacts/SafeOutputTree.ts b/src/artifacts/SafeOutputTree.ts index a20afb7f9..4bb4cfe1a 100644 --- a/src/artifacts/SafeOutputTree.ts +++ b/src/artifacts/SafeOutputTree.ts @@ -191,7 +191,7 @@ export class SafeOutputTree { "format", `Invalid expected extraction size for ${relativePath}`, ); - const path = normalizeArtifactPath(relativePath); + const path = normalizeDestinationPath(relativePath, this.platform); this.#registry.add(path, "file"); const lineage = await this.#prepareParent(path); const parent = lineage.at(-1); @@ -569,6 +569,19 @@ const writeAll = async ( } }; +const normalizeDestinationPath = ( + relativePath: string, + platform: NodeJS.Platform, +): string => { + const path = normalizeArtifactPath(relativePath); + if (platform === "win32" && path.includes(":")) + throw new ArtifactReaderFailure( + "path", + `Windows destination cannot materialize artifact path as a regular file: ${path}; ':' denotes alternate data stream syntax.`, + ); + return path; +}; + const abortIfNeeded = (signal?: AbortSignal): void => { if (signal?.aborted === true) throw new ArtifactReaderFailure( diff --git a/src/domain/analysisErrorPresentation.ts b/src/domain/analysisErrorPresentation.ts index b7f4629dd..3153cfd84 100644 --- a/src/domain/analysisErrorPresentation.ts +++ b/src/domain/analysisErrorPresentation.ts @@ -345,7 +345,11 @@ const artifactMessage = ({ return "Artifact operation was cancelled. Start it again when ready."; if (reason === "limit") return "Artifact is too large to process safely. Narrow the requested path or use a smaller artifact."; - if (reason === "path") return artifactPathMessage(detail); + if (reason === "path") + return ( + detail ?? + "Artifact contains a conflicting internal path. Inspect the reported path before retrying." + ); if (reason === "unavailable" && details?.unpacked === true) return `ASAR unpacked companion bytes are unavailable for ${details.logicalPath}. Select the archive in place with its .unpacked directory beside it, then retry.`; if (reason === "unavailable") @@ -361,23 +365,6 @@ const artifactMessage = ({ return "Artifact could not be read or written. Check file access and try again."; }; -const artifactPathMessage = (detail: string | undefined): string => { - // Kept in step with DESTINATION_CASE_COLLISION_PREFIX. Domain does not import - // the artifact adapter that produces the detail. - if ( - detail !== undefined && - detail.startsWith("Destination filesystem cannot store both ") - ) - return detail; - if ( - detail !== undefined && - (detail.startsWith("Artifact path is absolute or unsafe:") || - detail.startsWith("Artifact path is not normalized:")) - ) - return "Artifact contains an unsafe internal path. Inspect the reported path and correct the artifact before retrying."; - return "Artifact contains a conflicting internal path. Inspect the reported path before retrying."; -}; - const evidenceFileMessage = ({ operation, reason, diff --git a/tests/boundary/filesystem/safeOutputTree.test.ts b/tests/boundary/filesystem/safeOutputTree.test.ts index 0f9976522..f67704456 100644 --- a/tests/boundary/filesystem/safeOutputTree.test.ts +++ b/tests/boundary/filesystem/safeOutputTree.test.ts @@ -456,3 +456,37 @@ describe("safe artifact output tree", () => { ); }); }); + +describe("safe artifact output Windows stream paths", () => { + it.each([ + "hello.txt:hidden", + "nested/hello.txt::$DATA", + "folder:stream/file.txt", + ])("refuses Windows stream syntax before creating %s", async (path) => { + const parent = await createTestTempDirectory("rea-safe-output-stream-"); + const output = join(parent, "published"); + const tree = await SafeOutputTree.create(output, "win32"); + const bytes = Buffer.from("regular file content"); + const source = Readable.from(bytes); + try { + await expect( + tree.write(path, source, { + sha256: createHash("sha256").update(bytes).digest("hex"), + bytes: bytes.byteLength, + }), + ).rejects.toMatchObject({ + reason: "path", + message: `Windows destination cannot materialize artifact path as a regular file: ${path}; ':' denotes alternate data stream syntax.`, + }); + expect(source.destroyed).toBe(true); + expect(await readdir(output)).toEqual([]); + } finally { + expect(await tree.rollback()).toEqual({ + status: "complete", + residualPaths: [], + }); + } + await expect(access(output)).rejects.toMatchObject({ code: "ENOENT" }); + expect(await readdir(parent)).toEqual([]); + }); +}); diff --git a/tests/boundary/providers/native/artifactProvider.archiveSafety.test.ts b/tests/boundary/providers/native/artifactProvider.archiveSafety.test.ts index 8e8ff883e..e0978e379 100644 --- a/tests/boundary/providers/native/artifactProvider.archiveSafety.test.ts +++ b/tests/boundary/providers/native/artifactProvider.archiveSafety.test.ts @@ -183,6 +183,54 @@ it("extracts case-distinct names only when the destination can preserve them", a expect(await readFile(join(output, "Main.js"), "utf8")).toBe("upper"); expect(await readFile(join(output, "main.js"), "utf8")).toBe("lower"); }); + +it("preserves colon names in inventory and refuses Windows stream materialization", async () => { + const root = await createTestTempDirectory("rea-artifact-stream-"); + const path = join(root, "fixture.zip"); + const writer = new ZipWriter(new Uint8ArrayWriter()); + await writer.add("0-ordinary.txt", new TextReader("ordinary content")); + await writer.add("hello.txt:hidden", new TextReader("secret payload")); + await writeFile(path, await writer.close()); + const parsed = await parseBinaryTarget(path); + if (!parsed.ok) throw parsed.error; + const result = await inventory(parsed.value); + expect(result.occurrences.map(({ logical_path }) => logical_path)).toContain( + "hello.txt:hidden", + ); + + const output = join(root, "output"); + const extracted = await new ArtifactProvider(process.env) + .createClient(parsed.value) + .execute( + "extract_artifact", + artifactExtractionExecutionSchema.parse({ output_root: output }), + ); + if (process.platform === "win32") { + if (extracted.ok) throw new Error("expected a Windows stream path refusal"); + const detail = + "Windows destination cannot materialize artifact path as a regular file: hello.txt:hidden; ':' denotes alternate data stream syntax."; + expect(projectAnalysisError(extracted.error)).toMatchObject({ + message: detail, + details: { operation: "extract_artifact", reason: "path", detail }, + }); + await expect(lstat(output)).rejects.toMatchObject({ code: "ENOENT" }); + await expect( + readFile(join(output, "hello.txt:hidden")), + ).rejects.toMatchObject({ code: "ENOENT" }); + return; + } + if (!extracted.ok) throw extracted.error; + expect(await readFile(join(output, "0-ordinary.txt"), "utf8")).toBe( + "ordinary content", + ); + expect(await readFile(join(output, "hello.txt:hidden"), "utf8")).toBe( + "secret payload", + ); + await expect(lstat(join(output, "hello.txt"))).rejects.toMatchObject({ + code: "ENOENT", + }); +}); + const inventory = async (targetValue: BinaryTarget) => { const result = await new ArtifactProvider(process.env) .createClient(targetValue) diff --git a/tests/boundary/providers/native/artifactProvider.failureDetail.test.ts b/tests/boundary/providers/native/artifactProvider.failureDetail.test.ts index e2b2fd385..e500d2c0c 100644 --- a/tests/boundary/providers/native/artifactProvider.failureDetail.test.ts +++ b/tests/boundary/providers/native/artifactProvider.failureDetail.test.ts @@ -53,8 +53,7 @@ it("reports an unsafe path as a hostile artifact path", async () => { if (result.ok) throw new Error("expected the path constraint to fail"); expect(projectAnalysisError(result.error)).toMatchObject({ code: "artifact_operation_failed", - message: - "Artifact contains an unsafe internal path. Inspect the reported path and correct the artifact before retrying.", + message: 'Artifact path is absolute or unsafe: "a\\\\b.js"', details: { operation: "inventory_artifact", reason: "path", @@ -75,4 +74,11 @@ it("returns a destination case collision as the user-facing path message", () => message: detail, details: { reason: "path", detail }, }); + + expect( + projectAnalysisError(new ArtifactOperationError("extract_artifact", "path")) + .message, + ).toBe( + "Artifact contains a conflicting internal path. Inspect the reported path before retrying.", + ); });