refactor(artifacts): index xar loads and centralize pre-entry integrity policy

src/artifacts/CpioArtifactReader.ts

@@ -364,19 +364,17 @@ export class CpioArtifactReader implements ArtifactReader {
       // Zero-size headers carry no bytes, so a CRC archive must declare zero.
       // Under record-and-continue this is the occurrence's own forgotten
       // bytes: yield it unavailable so later siblings still inventory.
-      if (header.format === "crc" && header.check !== 0) {
-        if (this.integrity !== "record-and-continue")
-          throw new ArtifactReaderFailure(
-            "integrity",
-            `cpio CRC disagrees with content: ${raw}`,
-          );
+      const verified = await this.#recoverIntegrity(() =>
+        drain(this.#verified(source, header, raw)),
+      );
+      if ("mismatch" in verified) {
         yield entryOf({
           path,
           kind: "file",
           key,
           header,
           size: null,
-          limitations: [`cpio CRC disagrees with content: ${raw}`],
+          limitations: [verified.mismatch],
           contentUnavailable: true,
         });
         return;
@@ -399,6 +397,10 @@ export class CpioArtifactReader implements ArtifactReader {
     if (type !== S_IFREG && type !== S_IFDIR) {
       // FIFOs, device nodes, sockets and other types are not expanded; keep
       // an explicit unavailable occurrence instead of dropping the path.
+      const verified = await this.#recoverIntegrity(() =>
+        drain(this.#verified(source, header, raw)),
+      );
+      this.#current = undefined;
       yield entryOf({
         path,
         kind: "file",
@@ -407,12 +409,10 @@ export class CpioArtifactReader implements ArtifactReader {
         size: null,
         limitations: [
           `Unsupported cpio member type ${type.toString(8)}; content not expanded.`,
+          ...("mismatch" in verified ? [verified.mismatch] : []),
         ],
         contentUnavailable: true,
       });
-      // Consume its data (if any) so the next header aligns.
-      await drain(this.#verified(source, header, raw));
-      this.#current = undefined;
       return;
     }
     yield entryOf({
@@ -441,29 +441,21 @@ export class CpioArtifactReader implements ArtifactReader {
     path: string | undefined,
   ): AsyncGenerator<ArtifactEntry> {
     const { header, raw, key } = member;
-    let target: Buffer | undefined;
-    let crcLimitation: string | undefined;
-    try {
-      if (header.fileSize > MAX_SYMLINK_TARGET_BYTES)
+    const observed = await this.#recoverIntegrity(async () => {
+      if (header.fileSize > MAX_SYMLINK_TARGET_BYTES) {
         await drain(this.#verified(source, header, raw));
-      else target = await this.#collect(source, header, raw);
-    } catch (cause: unknown) {
-      if (
-        this.integrity !== "record-and-continue" ||
-        !(cause instanceof ArtifactReaderFailure) ||
-        cause.reason !== "integrity"
-      )
-        throw cause;
-      crcLimitation = cause.message;
-    }
+        return undefined;
+      }
+      return this.#collect(source, header, raw);
+    });
     if (path === undefined) return;
-    if (crcLimitation !== undefined) {
+    if ("mismatch" in observed) {
       yield entryOf({
         path,
         kind: "symlink",
         key,
         header,
-        limitations: [crcLimitation],
+        limitations: [observed.mismatch],
         contentUnavailable: true,
       });
       return;
@@ -473,10 +465,27 @@ export class CpioArtifactReader implements ArtifactReader {
       kind: "symlink",
       key,
       header,
-      limitations: [symlinkTargetLimitation(header, target)],
+      limitations: [symlinkTargetLimitation(header, observed.value)],
     });
   }
 
+  /** The one integrity-policy boundary for metadata consumed before yielding an entry. */
+  async #recoverIntegrity<T>(
+    read: () => Promise<T>,
+  ): Promise<{ readonly value: T } | { readonly mismatch: string }> {
+    try {
+      return { value: await read() };
+    } catch (cause: unknown) {
+      if (
+        this.integrity !== "record-and-continue" ||
+        !(cause instanceof ArtifactReaderFailure) ||
+        cause.reason !== "integrity"
+      )
+        throw cause;
+      return { mismatch: cause.message };
+    }
+  }
+
   #alias(
     path: string,
     key: string,

src/artifacts/InstallerPackageLimits.test.ts

@@ -172,3 +172,64 @@ it("verifies gzip integrity even when the cpio trailer has already been decoded"
     await reader.close();
   }
 });
+
+it.each(["0755junk", "-1", "", "08"])(
+  "rejects partially parsed or malformed mode %j",
+  async (mode) => {
+    await withReader(
+      xarArchive([{ name: "a", mode, data: Buffer.from("x") }]),
+      async (reader) => {
+        await expect(
+          (async () => {
+            for await (const entry of reader.entries()) void entry;
+          })(),
+        ).rejects.toMatchObject({
+          reason: "format",
+          message: "xar TOC has an invalid mode",
+        });
+      },
+    );
+  },
+);
+
+it("preserves CRC contradictions for special nodes and reaches later siblings", async () => {
+  const bytes = gzipCpio(
+    [
+      { name: "fifo", mode: MODE.fifo, data: "abc", check: 1 },
+      { name: "good", mode: MODE.file, data: "ok" },
+    ],
+    "crc",
+  );
+  const reader = new CpioArtifactReader(
+    async () => Readable.from([bytes]),
+    "record-and-continue",
+  );
+  try {
+    const paths: string[] = [];
+    for await (const entry of reader.entries()) {
+      paths.push(entry.path);
+      if (entry.path === "fifo")
+        expect(entry.limitations).toContain(
+          "cpio CRC disagrees with content: fifo",
+        );
+      if (entry.path === "good")
+        expect((await buffer(await reader.open(entry))).toString()).toBe("ok");
+    }
+    expect(paths).toEqual(["fifo", "good"]);
+  } finally {
+    await reader.close();
+  }
+});
+
+it("does not expose a partial inventory when a previous TOC load failed", async () => {
+  await withReader(
+    xarArchive([{ name: "a", mode: "broken", data: Buffer.from("x") }]),
+    async (reader) => {
+      const scan = async () => {
+        for await (const entry of reader.entries()) void entry;
+      };
+      await expect(scan()).rejects.toMatchObject({ reason: "format" });
+      await expect(scan()).rejects.toMatchObject({ reason: "format" });
+    },
+  );
+});

src/artifacts/XarArtifactReader.ts

@@ -12,6 +12,7 @@ import {
   xarInteger as integer,
   xarHeapPosition,
   XarPathBudget,
+  xarMode,
 } from "./XarValidation.js";
 import {
   ArtifactReaderFailure,
@@ -211,15 +212,13 @@ const collectMembers = (toc: Element): XarMember[] => {
       throw new ArtifactReaderFailure("path", "xar member has an unsafe name");
     const path = paths.join(next.parent, name);
     const type = textOf(next.element, "type")?.trim();
-    const modeText = textOf(next.element, "mode");
-    const mode =
-      modeText === undefined ? Number.NaN : Number.parseInt(modeText.trim(), 8);
+    const mode = xarMode(textOf(next.element, "mode"));
     const { kind, unsupportedType } = classifyMemberType(type);
     members.push({
       path,
       kind,
       ...(unsupportedType === undefined ? {} : { unsupportedType }),
-      mode: Number.isSafeInteger(mode) ? mode : null,
+      mode,
       data:
         kind === "file" && unsupportedType === undefined
           ? parseData(next.element)
@@ -385,6 +384,10 @@ export class XarArtifactReader implements ArtifactReader {
   #size = 0;
   readonly decodedBudget: ArtifactDecodedBudget;
   #members: readonly XarMember[] = [];
+  readonly #byPath = new Map<string, XarMember>();
+  #loading: Promise<void> | undefined;
+  #closed = false;
+  readonly #streams = new Set<Readable>();
   readonly #integrity = new Map<string, ArtifactChecksumObservation[]>();
 
   readonly #verifyChecksums: boolean;
@@ -408,7 +411,14 @@ export class XarArtifactReader implements ArtifactReader {
   }
 
   async #load(signal?: AbortSignal): Promise<void> {
-    if (this.#handle !== undefined) return;
+    if (this.#closed)
+      throw new ArtifactReaderFailure("unavailable", "xar archive is closed");
+    this.#loading ??= this.#readToc(signal);
+    await this.#loading;
+  }
+
+  /** Load/index once; a rejected parse remains rejected, never a partial inventory. */
+  async #readToc(signal?: AbortSignal): Promise<void> {
     cancelled(signal);
     const handle = await open(this.path, "r");
     this.#handle = handle;
@@ -483,6 +493,14 @@ export class XarArtifactReader implements ArtifactReader {
       throw new ArtifactReaderFailure("format", "xar TOC has no <toc> element");
     await this.#verifyToc(tocElement, compressed, headerChecksumAlg);
     this.#members = collectMembers(tocElement);
+    for (const member of this.#members) {
+      if (this.#byPath.has(member.path))
+        throw new ArtifactReaderFailure(
+          "path",
+          `Duplicate xar member path: ${member.path}`,
+        );
+      this.#byPath.set(member.path, member);
+    }
   }
 
   /** The TOC checksum in the heap covers the compressed TOC bytes. */
@@ -691,7 +709,7 @@ export class XarArtifactReader implements ArtifactReader {
     cancelled(signal);
     this.#integrity.delete(entry.adapterKey);
     await this.#load(signal);
-    const member = this.#members.find(({ path }) => path === entry.adapterKey);
+    const member = this.#byPath.get(entry.adapterKey);
     if (member === undefined || member.kind !== "file")
       throw new ArtifactReaderFailure(
         "path",
@@ -754,11 +772,14 @@ export class XarArtifactReader implements ArtifactReader {
     const checksum = data.extractedChecksum;
     if (checksum !== undefined && this.#verifyChecksums)
       stages.push(new ChecksumVerifier(entry.path, checksum));
-    return artifactStreamPipeline(
+    const stream = artifactStreamPipeline(
       Readable.from(this.#chunks(data.offset, data.length, signal)),
       stages,
       { path: entry.path, signal },
     );
+    this.#streams.add(stream);
+    stream.once("close", () => this.#streams.delete(stream));
+    return stream;
   }
 
   async *#chunks(
@@ -786,8 +807,14 @@ export class XarArtifactReader implements ArtifactReader {
   }
 
   async close(): Promise<void> {
+    this.#closed = true;
+    for (const stream of this.#streams) stream.destroy();
+    await this.#loading?.catch(() => undefined);
     const handle = this.#handle;
     this.#handle = undefined;
+    this.#members = [];
+    this.#byPath.clear();
+    this.#integrity.clear();
     await handle?.close();
   }
 }

src/artifacts/XarValidation.ts

@@ -15,6 +15,16 @@ export const xarInteger = (
   return parsed;
 };
 
+/** Optional mode metadata is accepted only as a complete unsigned octal field. */
+export const xarMode = (value: string | undefined): number | null => {
+  if (value === undefined) return null;
+  const text = value.trim();
+  const mode = /^[0-7]+$/u.test(text) ? Number.parseInt(text, 8) : Number.NaN;
+  if (!Number.isSafeInteger(mode))
+    throw new ArtifactReaderFailure("format", "xar TOC has an invalid mode");
+  return mode;
+};
+
 /** Validate the entire absolute heap range before allocating or requesting it. */
 export const xarHeapPosition = (
   heap: number,