fix(artifacts): report malformed ZIP archives as format failures (#2683)
* fix(artifacts): report malformed ZIP archives as format failures zip.js signals archive faults (missing end-of-central-directory record, unrecognized format, overlapping members, corrupt member data, unsupported compression) as plain Errors. ZipArtifactReader passed them through untyped, so the artifact provider's fallback reported every one as `io` with no detail and told the caller to "check file access". Type these faults at the ZIP reader boundary: structural and data corruption become `format` failures and unsupported archive features become `unavailable`, each carrying the zip.js reason as detail. This covers enumeration, the getData rejection, and the WritableStream abort path that delivers corrupt-data errors first. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * test(artifacts): share the ZIP inventory failure assertion Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
Kaoru0822
GitHub
parent
f90382756d
commit
cd2ad1f240
|
|
@ -2,7 +2,29 @@ import type { FileHandle } from "node:fs/promises";
|
|||
import { PassThrough, type Readable } from "node:stream";
|
||||
import { once } from "node:events";
|
||||
|
||||
import { Reader, ZipReader, type Entry, type FileEntry } from "@zip.js/zip.js";
|
||||
import {
|
||||
ERR_AMBIGUOUS_ARCHIVE,
|
||||
ERR_BAD_FORMAT,
|
||||
ERR_CENTRAL_DIRECTORY_NOT_FOUND,
|
||||
ERR_ENCRYPTED_CENTRAL_DIRECTORY,
|
||||
ERR_ENTRY_DATA_OUT_OF_BOUNDS,
|
||||
ERR_EOCDR_LOCATOR_ZIP64_NOT_FOUND,
|
||||
ERR_EOCDR_NOT_FOUND,
|
||||
ERR_EXTRAFIELD_ZIP64_NOT_FOUND,
|
||||
ERR_INVALID_COMPRESSED_DATA,
|
||||
ERR_INVALID_CRC32,
|
||||
ERR_INVALID_UNCOMPRESSED_SIZE,
|
||||
ERR_LOCAL_FILE_HEADER_NOT_FOUND,
|
||||
ERR_OVERLAPPING_ENTRY,
|
||||
ERR_SPLIT_ZIP_FILE,
|
||||
ERR_UNSUPPORTED_COMPRESSION,
|
||||
ERR_UNSUPPORTED_ENCRYPTION,
|
||||
ERR_UNSUPPORTED_UINT64,
|
||||
Reader,
|
||||
ZipReader,
|
||||
type Entry,
|
||||
type FileEntry,
|
||||
} from "@zip.js/zip.js";
|
||||
|
||||
import {
|
||||
ArtifactReaderFailure,
|
||||
|
|
@ -159,9 +181,9 @@ export class ZipArtifactReader implements ArtifactReader {
|
|||
let index = 0;
|
||||
// A traversal owns its accounting, including overlapping or repeated passes.
|
||||
let declaredFileBytes = 0;
|
||||
for await (const entry of this.#reader.getEntriesGenerator({
|
||||
decodeText: decodeZipEntryText,
|
||||
})) {
|
||||
for await (const entry of typedZipFailures(
|
||||
this.#reader.getEntriesGenerator({ decodeText: decodeZipEntryText }),
|
||||
)) {
|
||||
abortIfNeeded(signal);
|
||||
// ZIP member identity is its directory occurrence, not its filename.
|
||||
// Logical path uniqueness belongs to the artifact inventory boundary.
|
||||
|
|
@ -327,7 +349,7 @@ const extractStream = (entry: FileEntry, signal?: AbortSignal): Readable => {
|
|||
output.end();
|
||||
},
|
||||
abort: (reason) => {
|
||||
output.destroy(toError(reason));
|
||||
output.destroy(toError(typedZipFailure(reason)));
|
||||
},
|
||||
});
|
||||
void entry
|
||||
|
|
@ -337,10 +359,61 @@ const extractStream = (entry: FileEntry, signal?: AbortSignal): Readable => {
|
|||
checkOverlappingEntry: true,
|
||||
onprogress: () => abortIfNeeded(signal),
|
||||
})
|
||||
.catch((cause: unknown) => output.destroy(toError(cause)));
|
||||
.catch((cause: unknown) => output.destroy(toError(typedZipFailure(cause))));
|
||||
return output;
|
||||
};
|
||||
|
||||
/** zip.js reports archive faults as plain errors identified by message. */
|
||||
const ZIP_FAILURE_REASONS: ReadonlyMap<string, "format" | "unavailable"> =
|
||||
new Map([
|
||||
...[
|
||||
ERR_AMBIGUOUS_ARCHIVE,
|
||||
ERR_BAD_FORMAT,
|
||||
ERR_CENTRAL_DIRECTORY_NOT_FOUND,
|
||||
ERR_ENTRY_DATA_OUT_OF_BOUNDS,
|
||||
ERR_EOCDR_LOCATOR_ZIP64_NOT_FOUND,
|
||||
ERR_EOCDR_NOT_FOUND,
|
||||
ERR_EXTRAFIELD_ZIP64_NOT_FOUND,
|
||||
ERR_INVALID_COMPRESSED_DATA,
|
||||
ERR_INVALID_CRC32,
|
||||
ERR_INVALID_UNCOMPRESSED_SIZE,
|
||||
ERR_LOCAL_FILE_HEADER_NOT_FOUND,
|
||||
ERR_OVERLAPPING_ENTRY,
|
||||
ERR_UNSUPPORTED_UINT64,
|
||||
].map((message) => [message, "format"] as const),
|
||||
...[
|
||||
ERR_ENCRYPTED_CENTRAL_DIRECTORY,
|
||||
ERR_SPLIT_ZIP_FILE,
|
||||
ERR_UNSUPPORTED_COMPRESSION,
|
||||
ERR_UNSUPPORTED_ENCRYPTION,
|
||||
].map((message) => [message, "unavailable"] as const),
|
||||
]);
|
||||
|
||||
/** Type a zip.js archive fault so consumers do not report it as file I/O. */
|
||||
const typedZipFailure = (cause: unknown): unknown => {
|
||||
if (!(cause instanceof Error) || cause instanceof ArtifactReaderFailure)
|
||||
return cause;
|
||||
const reason = ZIP_FAILURE_REASONS.get(cause.message);
|
||||
if (reason === undefined) return cause;
|
||||
return new ArtifactReaderFailure(
|
||||
reason,
|
||||
reason === "format"
|
||||
? `ZIP archive is malformed: ${cause.message}`
|
||||
: `ZIP archive uses an unsupported feature: ${cause.message}`,
|
||||
{ cause },
|
||||
);
|
||||
};
|
||||
|
||||
async function* typedZipFailures<T>(
|
||||
source: AsyncIterable<T>,
|
||||
): AsyncIterable<T> {
|
||||
try {
|
||||
yield* source;
|
||||
} catch (cause: unknown) {
|
||||
throw typedZipFailure(cause);
|
||||
}
|
||||
}
|
||||
|
||||
const isSymlink = (entry: Entry): boolean =>
|
||||
entry.unixMode !== undefined && (entry.unixMode & 0o170000) === 0o120000;
|
||||
|
||||
|
|
|
|||
|
|
@ -366,16 +366,11 @@ it("rejects overlapping ZIP member data ranges through artifact inventory", asyn
|
|||
bytes.writeUInt32LE(firstLocalOffset, second + 42);
|
||||
await writeFile(path, bytes);
|
||||
|
||||
const target = await parseBinaryTarget(path);
|
||||
if (!target.ok) throw target.error;
|
||||
const result = await withArtifactClient(target.value, (client) =>
|
||||
client.execute("inventory_artifact", {}),
|
||||
await expectInventoryFailure(
|
||||
path,
|
||||
"format",
|
||||
"ZIP archive is malformed: Overlapping entry found",
|
||||
);
|
||||
|
||||
expect(result).toMatchObject({
|
||||
ok: false,
|
||||
error: { _tag: "ArtifactOperationError", reason: "io" },
|
||||
});
|
||||
});
|
||||
|
||||
it("preserves encrypted-member and corrupt-CRC observations", async () => {
|
||||
|
|
@ -438,16 +433,11 @@ it("preserves encrypted-member and corrupt-CRC observations", async () => {
|
|||
bytes[dataOffset] = firstByte ^ 1;
|
||||
await writeFile(corruptPath, bytes);
|
||||
|
||||
const corruptTarget = await parseBinaryTarget(corruptPath);
|
||||
if (!corruptTarget.ok) throw corruptTarget.error;
|
||||
const corruptResult = await withArtifactClient(
|
||||
corruptTarget.value,
|
||||
(client) => client.execute("inventory_artifact", {}),
|
||||
await expectInventoryFailure(
|
||||
corruptPath,
|
||||
"format",
|
||||
"ZIP archive is malformed: Invalid compressed data",
|
||||
);
|
||||
expect(corruptResult).toMatchObject({
|
||||
ok: false,
|
||||
error: { _tag: "ArtifactOperationError", reason: "io" },
|
||||
});
|
||||
});
|
||||
|
||||
it("rejects a truncated ZIP through artifact inventory", async () => {
|
||||
|
|
@ -455,17 +445,51 @@ it("rejects a truncated ZIP through artifact inventory", async () => {
|
|||
const path = join(root, "truncated.zip");
|
||||
await writeFile(path, Buffer.from([0x50, 0x4b, 0x03, 0x04]));
|
||||
|
||||
await expectInventoryFailure(
|
||||
path,
|
||||
"format",
|
||||
"ZIP archive is malformed: File format is not recognized",
|
||||
);
|
||||
});
|
||||
|
||||
it("reports an unsupported ZIP compression method as unavailable", async () => {
|
||||
const root = await createTestTempDirectory("rea-zip-method-");
|
||||
const path = join(root, "lzma.zip");
|
||||
const writer = new ZipWriter(new Uint8ArrayWriter());
|
||||
await writer.add("data.txt", new TextReader("member bytes"), { level: 0 });
|
||||
const bytes = Buffer.from(await writer.close());
|
||||
// Declare LZMA (14) in both headers; REA reads only stored and Deflate data.
|
||||
const localHeader = bytes.indexOf(Buffer.from([0x50, 0x4b, 0x03, 0x04]));
|
||||
const centralHeader = bytes.indexOf(Buffer.from([0x50, 0x4b, 0x01, 0x02]));
|
||||
if (localHeader < 0 || centralHeader < 0)
|
||||
throw new Error("Expected ZIP local and central headers");
|
||||
bytes.writeUInt16LE(14, localHeader + 8);
|
||||
bytes.writeUInt16LE(14, centralHeader + 10);
|
||||
await writeFile(path, bytes);
|
||||
|
||||
await expectInventoryFailure(
|
||||
path,
|
||||
"unavailable",
|
||||
"ZIP archive uses an unsupported feature: Compression method not supported",
|
||||
);
|
||||
});
|
||||
|
||||
/** Inventory one archive through the provider and expect a typed failure. */
|
||||
const expectInventoryFailure = async (
|
||||
path: string,
|
||||
reason: string,
|
||||
detail: string,
|
||||
): Promise<void> => {
|
||||
const target = await parseBinaryTarget(path);
|
||||
if (!target.ok) throw target.error;
|
||||
const result = await withArtifactClient(target.value, (client) =>
|
||||
client.execute("inventory_artifact", {}),
|
||||
);
|
||||
|
||||
expect(result).toMatchObject({
|
||||
ok: false,
|
||||
error: { _tag: "ArtifactOperationError", reason: "io" },
|
||||
error: { _tag: "ArtifactOperationError", reason, detail },
|
||||
});
|
||||
});
|
||||
};
|
||||
|
||||
const writeSingleEntryZip = async (
|
||||
path: string,
|
||||
|
|
|
|||
Loading…
Reference in New Issue