fix(artifacts): share decoding budgets and centralize xar validation

src/artifacts/ArtifactDecodedBudget.ts

@@ -0,0 +1,57 @@
+import { Transform, type TransformCallback } from "node:stream";
+import { ArtifactReaderFailure } from "./ArtifactReader.js";
+
+/** One decoded-byte ceiling shared by an archive and all nested readers. */
+export class ArtifactDecodedBudget {
+  #used = 0;
+
+  constructor(private readonly maximum = 512 * 1024 * 1024) {}
+
+  /** Charge decoding work, including repeated reads, before exposing a chunk. */
+  consume(bytes: number, path: string): void {
+    if (
+      !Number.isSafeInteger(bytes) ||
+      bytes < 0 ||
+      bytes > this.maximum - this.#used
+    )
+      throw new ArtifactReaderFailure(
+        "limit",
+        `Decoded archive budget of ${this.maximum} bytes exhausted at ${path}`,
+      );
+    this.#used += bytes;
+  }
+}
+
+/** Decoder-stage budget/cancellation guard, independent of member metadata. */
+export class ArtifactBudgetTransform extends Transform {
+  constructor(
+    private readonly budget: ArtifactDecodedBudget,
+    private readonly path: string,
+    private readonly signal?: AbortSignal,
+  ) {
+    super();
+  }
+
+  override _transform(
+    chunk: Buffer,
+    _encoding: BufferEncoding,
+    done: TransformCallback,
+  ): void {
+    if (this.signal?.aborted) {
+      done(
+        new ArtifactReaderFailure(
+          "cancelled",
+          `Archive decoding cancelled at ${this.path}`,
+        ),
+      );
+      return;
+    }
+    try {
+      this.budget.consume(chunk.length, this.path);
+    } catch (cause: unknown) {
+      done(cause instanceof Error ? cause : new Error(String(cause)));
+      return;
+    }
+    done(null, chunk);
+  }
+}

src/artifacts/ArtifactReader.ts

@@ -1,6 +1,7 @@
 import type { Readable } from "node:stream";
 import type { ArtifactCommand } from "../domain/artifactGraph.js";
 import type { ZipPackageFormat } from "../domain/zipPackageFormat.js";
+import type { ArtifactDecodedBudget } from "./ArtifactDecodedBudget.js";
 
 /** Archive-neutral entry metadata. Reader adapters never choose output paths. */
 export interface ArtifactEntry {
@@ -46,6 +47,8 @@ export interface ArtifactChecksumObservation {
 /** Read-only adapter over one directory, archive, or virtual container. */
 export interface ArtifactReader {
   readonly format: "directory" | ZipPackageFormat | "asar" | "pkg" | "file";
+  /** Shared safety budget for compressed representations and nested archives. */
+  readonly decodedBudget?: ArtifactDecodedBudget;
   entries(signal?: AbortSignal): AsyncIterable<ArtifactEntry>;
   open(entry: ArtifactEntry, signal?: AbortSignal): Promise<Readable>;
   /** Checksums observed after the complete member stream has been consumed. */

src/artifacts/CpioArtifactReader.ts

@@ -1,4 +1,8 @@
-import { Readable, Transform, type TransformCallback } from "node:stream";
+import { Readable } from "node:stream";
+import {
+  ArtifactBudgetTransform,
+  ArtifactDecodedBudget,
+} from "./ArtifactDecodedBudget.js";
 import { createGunzip } from "node:zlib";
 
 import type { ArtifactCommand } from "../domain/artifactGraph.js";
@@ -15,8 +19,6 @@ const MAX_LINK_BUFFER_BYTES = 16 * 1024 * 1024;
 const MAX_LINK_BUFFER_TOTAL = 64 * 1024 * 1024;
 /** Longest symlink target read; PATH_MAX is 1024 on macOS and 4096 on Linux. */
 const MAX_SYMLINK_TARGET_BYTES = 4096;
-/** Total decompressed cpio bytes accepted from one archive (gzip-bomb budget). */
-const MAX_TOTAL_DECOMPRESSED_BYTES = 512 * 1024 * 1024;
 const TRAILER = "TRAILER!!!";
 const S_IFMT = 0o170000;
 const S_IFDIR = 0o040000;
@@ -88,29 +90,6 @@ class ByteSource {
   }
 }
 
-/** Bound total decompressed bytes so a gzip bomb cannot exhaust the process. */
-class DecodedBudget extends Transform {
-  #seen = 0;
-
-  override _transform(
-    chunk: Buffer,
-    _encoding: BufferEncoding,
-    done: TransformCallback,
-  ): void {
-    this.#seen += chunk.length;
-    if (this.#seen > MAX_TOTAL_DECOMPRESSED_BYTES) {
-      done(
-        new ArtifactReaderFailure(
-          "limit",
-          `cpio archive decodes beyond ${MAX_TOTAL_DECOMPRESSED_BYTES} bytes`,
-        ),
-      );
-      return;
-    }
-    done(null, chunk);
-  }
-}
-
 interface CpioHeader {
   readonly format: "odc" | "newc" | "crc";
   readonly mode: number;
@@ -252,6 +231,7 @@ const symlinkTargetLimitation = (
 export class CpioArtifactReader implements ArtifactReader {
   readonly format = "file" as const;
   #source: ByteSource | undefined;
+  #signal: AbortSignal | undefined;
   #current:
     | {
         readonly key: string;
@@ -276,9 +256,11 @@ export class CpioArtifactReader implements ArtifactReader {
      * siblings are still inventoried, instead of aborting the archive.
      */
     private readonly integrity: "fail" | "record-and-continue" = "fail",
+    readonly decodedBudget: ArtifactDecodedBudget = new ArtifactDecodedBudget(),
   ) {}
 
   async *entries(signal?: AbortSignal): AsyncIterable<ArtifactEntry> {
+    this.#signal = signal;
     const compressed = await this.openCompressed(signal);
     const gunzip = createGunzip();
     // `pipe()` does not forward source errors (cancellation, truncation);
@@ -296,7 +278,11 @@ export class CpioArtifactReader implements ArtifactReader {
       // Prevent unhandled source errors after the decoder fails first.
       compressed.destroy();
     });
-    const budget = new DecodedBudget();
+    const budget = new ArtifactBudgetTransform(
+      this.decodedBudget,
+      "gzip-cpio",
+      signal,
+    );
     gunzip.on("error", (cause: unknown) => {
       budget.destroy(
         cause instanceof ArtifactReaderFailure
@@ -308,6 +294,7 @@ export class CpioArtifactReader implements ArtifactReader {
     });
     budget.on("error", () => {
       compressed.destroy();
+      gunzip.destroy();
     });
     const source = new ByteSource(compressed.pipe(gunzip).pipe(budget));
     this.#source = source;
@@ -595,7 +582,9 @@ export class CpioArtifactReader implements ArtifactReader {
     path: string,
   ): AsyncGenerator<Buffer> {
     let sum = 0;
+    this.#cancelled();
     for await (const chunk of source.take(header.fileSize, "member data")) {
+      this.#cancelled();
       if (header.format === "crc")
         for (const byte of chunk) sum = (sum + byte) >>> 0;
       yield chunk;
@@ -606,6 +595,7 @@ export class CpioArtifactReader implements ArtifactReader {
     await drain(
       source.take(padding(header.format, header.fileSize), "data padding"),
     );
+    this.#cancelled();
     if (header.format === "crc" && sum !== header.check)
       throw new ArtifactReaderFailure(
         "integrity",
@@ -624,6 +614,14 @@ export class CpioArtifactReader implements ArtifactReader {
     return Buffer.concat(chunks);
   }
 
+  #cancelled(): void {
+    if (this.#signal?.aborted)
+      throw new ArtifactReaderFailure(
+        "cancelled",
+        "cpio expansion was cancelled",
+      );
+  }
+
   open(entry: ArtifactEntry): Promise<Readable> {
     const alias = this.#aliases.get(entry.adapterKey);
     if (alias !== undefined) return Promise.resolve(Readable.from([alias]));

src/artifacts/InstallerPackage.fixture.ts

@@ -1,6 +1,6 @@
 /** xar archives and cpio payloads built field by field for installer-package tests. */
 import { createHash } from "node:crypto";
-import { deflateSync, gzipSync } from "node:zlib";
+import { deflateSync, gzipSync, inflateSync } from "node:zlib";
 
 export interface XarFixtureMember {
   readonly name: string;
@@ -147,3 +147,21 @@ export const MODE = {
   symlink: 0o120755,
   fifo: 0o010644,
 } as const;
+
+/** Mutate a fixture's XML while keeping its compressed TOC checksum/lengths consistent. */
+export const rewriteXarToc = (
+  archive: Uint8Array,
+  rewrite: (xml: string) => string,
+): Uint8Array => {
+  const input = Buffer.from(archive);
+  const oldLength = Number(input.readBigUInt64BE(8));
+  const decoded = inflateSync(input.subarray(28, 28 + oldLength));
+  const xml = Buffer.from(rewrite(decoded.toString("utf8")));
+  const compressed = deflateSync(xml);
+  const header = Buffer.from(input.subarray(0, 28));
+  header.writeBigUInt64BE(BigInt(compressed.length), 8);
+  header.writeBigUInt64BE(BigInt(xml.length), 16);
+  const heap = Buffer.from(input.subarray(28 + oldLength));
+  createHash("sha1").update(compressed).digest().copy(heap, 0);
+  return Buffer.concat([header, compressed, heap]);
+};

src/artifacts/InstallerPackageLimits.test.ts

@@ -0,0 +1,157 @@
+import { mkdtemp, rm, writeFile } from "node:fs/promises";
+import { tmpdir } from "node:os";
+import { join } from "node:path";
+import { buffer } from "node:stream/consumers";
+import { Readable } from "node:stream";
+import { expect, it } from "vitest";
+import { ArtifactDecodedBudget } from "./ArtifactDecodedBudget.js";
+import { CpioArtifactReader } from "./CpioArtifactReader.js";
+import {
+  MODE,
+  gzipCpio,
+  xarArchive,
+  rewriteXarToc,
+  type XarFixtureMember,
+} from "./InstallerPackage.fixture.js";
+import { XarArtifactReader } from "./XarArtifactReader.js";
+
+const withReader = async (
+  bytes: Uint8Array,
+  run: (reader: XarArtifactReader) => Promise<void>,
+  decodedBudget?: ArtifactDecodedBudget,
+) => {
+  const root = await mkdtemp(join(tmpdir(), "rea-pkg-limits-"));
+  const path = join(root, "fixture.pkg");
+  await writeFile(path, bytes);
+  const reader = new XarArtifactReader(
+    path,
+    decodedBudget === undefined ? {} : { decodedBudget },
+  );
+  try {
+    await run(reader);
+  } finally {
+    await reader.close();
+    await rm(root, { recursive: true, force: true });
+  }
+};
+
+it("shares an inflation budget across separate zlib members", async () => {
+  const bytes = xarArchive([
+    { name: "a", data: Buffer.alloc(40, 65), encoding: "zlib" },
+    { name: "b", data: Buffer.alloc(40, 66), encoding: "zlib" },
+  ]);
+  await withReader(
+    bytes,
+    async (reader) => {
+      let read = 0;
+      await expect(
+        (async () => {
+          for await (const entry of reader.entries()) {
+            await buffer(await reader.open(entry));
+            read++;
+          }
+        })(),
+      ).rejects.toMatchObject({
+        reason: "limit",
+        message: expect.stringContaining("Decoded archive budget"),
+      });
+      expect(read).toBe(1);
+    },
+    new ArtifactDecodedBudget(64),
+  );
+});
+
+it.each(["", " ", "1e2", "0x10"])(
+  "rejects nondecimal/empty numeric TOC data: %j",
+  async (value) => {
+    const bytes = rewriteXarToc(
+      xarArchive([{ name: "a", data: Buffer.from("bytes") }]),
+      (xml) =>
+        xml.replace(/<length>\d+<\/length>/u, `<length>${value}</length>`),
+    );
+    await withReader(bytes, async (reader) => {
+      await expect(
+        (async () => {
+          for await (const entry of reader.entries()) void entry;
+        })(),
+      ).rejects.toMatchObject({
+        reason: "format",
+        message: "xar TOC has an invalid data length",
+      });
+    });
+  },
+);
+
+it("rejects unsafe absolute heap ranges with a tagged format failure", async () => {
+  const bytes = rewriteXarToc(
+    xarArchive([{ name: "Payload", data: Buffer.from("gzip") }]),
+    (xml) =>
+      xml.replace(
+        /<offset>20<\/offset>/u,
+        `<offset>${Number.MAX_SAFE_INTEGER}</offset>`,
+      ),
+  );
+  await withReader(bytes, async (reader) => {
+    await expect(
+      (async () => {
+        for await (const entry of reader.entries()) void entry;
+      })(),
+    ).rejects.toMatchObject({
+      reason: "format",
+      message: expect.stringContaining("heap extent"),
+    });
+  });
+});
+
+it("bounds retained full paths from deeply nested TOCs", async () => {
+  let member: XarFixtureMember = { name: "leaf", data: Buffer.from("x") };
+  for (let depth = 0; depth < 300; depth++)
+    member = {
+      name: "directory-segment",
+      type: "directory",
+      children: [member],
+    };
+  await withReader(xarArchive([member]), async (reader) => {
+    await expect(
+      (async () => {
+        for await (const entry of reader.entries()) void entry;
+      })(),
+    ).rejects.toMatchObject({
+      reason: "limit",
+      message: expect.stringContaining("path text"),
+    });
+  });
+});
+
+it("observes cancellation during internal symlink drains", async () => {
+  const controller = new AbortController();
+  const archive = gzipCpio([
+    {
+      name: "huge-link",
+      mode: MODE.symlink,
+      data: "x".repeat(4 * 1024 * 1024),
+    },
+  ]);
+  let decoded = 0;
+  const budget = new (class extends ArtifactDecodedBudget {
+    override consume(bytes: number, path: string): void {
+      super.consume(bytes, path);
+      decoded += bytes;
+      if (decoded >= 64 * 1024) controller.abort();
+    }
+  })();
+  const reader = new CpioArtifactReader(
+    async () => Readable.from([archive]),
+    "fail",
+    budget,
+  );
+  try {
+    await expect(
+      (async () => {
+        for await (const entry of reader.entries(controller.signal)) void entry;
+      })(),
+    ).rejects.toMatchObject({ reason: "cancelled" });
+  } finally {
+    await reader.close();
+  }
+});

src/artifacts/XarArtifactReader.ts

@@ -11,6 +11,12 @@ import { createInflate, inflateSync } from "node:zlib";
 import { DOMParser, type Element, type Node } from "@xmldom/xmldom";
 
 import type { ArtifactCommand } from "../domain/artifactGraph.js";
+import { ArtifactDecodedBudget } from "./ArtifactDecodedBudget.js";
+import {
+  xarInteger as integer,
+  xarHeapPosition,
+  XarPathBudget,
+} from "./XarValidation.js";
 import {
   ArtifactReaderFailure,
   type ArtifactEntry,
@@ -85,16 +91,6 @@ const childElement = (element: Element, name: string): Element | undefined =>
 const textOf = (element: Element, name: string): string | undefined =>
   childElement(element, name)?.textContent ?? undefined;
 
-const integer = (value: string | undefined, label: string): number => {
-  const parsed = value === undefined ? Number.NaN : Number(value.trim());
-  if (!Number.isSafeInteger(parsed) || parsed < 0)
-    throw new ArtifactReaderFailure(
-      "format",
-      `xar TOC has an invalid ${label}`,
-    );
-  return parsed;
-};
-
 const parseChecksum = (
   parent: Element,
   tag: string,
@@ -200,6 +196,7 @@ const classifyMemberType = (
 /** Walk nested `<file>` elements in document order. */
 const collectMembers = (toc: Element): XarMember[] => {
   const members: XarMember[] = [];
+  const paths = new XarPathBudget();
   const pending: Array<{ readonly element: Element; readonly parent: string }> =
     childElements(toc, "file")
       .map((element) => ({ element, parent: "" }))
@@ -216,7 +213,7 @@ const collectMembers = (toc: Element): XarMember[] => {
       name === ".."
     )
       throw new ArtifactReaderFailure("path", "xar member has an unsafe name");
-    const path = next.parent === "" ? name : `${next.parent}/${name}`;
+    const path = paths.join(next.parent, name);
     const type = textOf(next.element, "type")?.trim();
     const modeText = textOf(next.element, "mode");
     const mode =
@@ -272,6 +269,8 @@ class SizeBound extends Transform {
   constructor(
     private readonly path: string,
     private readonly expected: number,
+    private readonly budget: ArtifactDecodedBudget,
+    private readonly signal?: AbortSignal,
   ) {
     super();
     this.#cap = Math.min(expected, MAX_DECODED_MEMBER_BYTES);
@@ -282,6 +281,21 @@ class SizeBound extends Transform {
     _encoding: BufferEncoding,
     done: TransformCallback,
   ): void {
+    if (this.signal?.aborted) {
+      done(
+        new ArtifactReaderFailure(
+          "cancelled",
+          `xar member decoding cancelled: ${this.path}`,
+        ),
+      );
+      return;
+    }
+    try {
+      this.budget.consume(chunk.length, this.path);
+    } catch (cause: unknown) {
+      done(cause instanceof Error ? cause : new Error(String(cause)));
+      return;
+    }
     this.#seen += chunk.length;
     if (this.#seen > this.#cap) {
       done(
@@ -372,6 +386,8 @@ export class XarArtifactReader implements ArtifactReader {
   readonly format = "pkg" as const;
   #handle: FileHandle | undefined;
   #heap = 0;
+  #size = 0;
+  readonly decodedBudget: ArtifactDecodedBudget;
   #members: readonly XarMember[] = [];
   readonly #integrity = new Map<string, ArtifactChecksumObservation[]>();
 
@@ -386,9 +402,13 @@ export class XarArtifactReader implements ArtifactReader {
    */
   constructor(
     private readonly path: string,
-    options: { readonly verifyChecksums?: boolean } = {},
+    options: {
+      readonly verifyChecksums?: boolean;
+      readonly decodedBudget?: ArtifactDecodedBudget;
+    } = {},
   ) {
     this.#verifyChecksums = options.verifyChecksums ?? true;
+    this.decodedBudget = options.decodedBudget ?? new ArtifactDecodedBudget();
   }
 
   async #load(signal?: AbortSignal): Promise<void> {
@@ -396,6 +416,7 @@ export class XarArtifactReader implements ArtifactReader {
     cancelled(signal);
     const handle = await open(this.path, "r");
     this.#handle = handle;
+    this.#size = (await handle.stat()).size;
     const header = Buffer.alloc(28);
     const { bytesRead } = await handle.read(header, 0, 28, 0);
     if (bytesRead < 28 || header.readUInt32BE(0) !== XAR_MAGIC)
@@ -536,18 +557,23 @@ export class XarArtifactReader implements ArtifactReader {
     const handle = this.#handle;
     if (handle === undefined)
       throw new ArtifactReaderFailure("unavailable", "xar archive is closed");
+    const position = xarHeapPosition(this.#heap, offset, length, this.#size);
     const bytes = Buffer.alloc(length);
-    const { bytesRead } = await handle.read(
-      bytes,
-      0,
-      length,
-      this.#heap + offset,
-    );
-    if (bytesRead < length)
-      throw new ArtifactReaderFailure(
-        "format",
-        "xar member extends beyond the archive",
+    let done = 0;
+    while (done < length) {
+      const { bytesRead } = await handle.read(
+        bytes,
+        done,
+        length - done,
+        position + done,
       );
+      if (bytesRead === 0)
+        throw new ArtifactReaderFailure(
+          "format",
+          "xar member extends beyond the archive",
+        );
+      done += bytesRead;
+    }
     return bytes;
   }
 
@@ -744,7 +770,12 @@ export class XarArtifactReader implements ArtifactReader {
         inflate.destroy(memberFailure(entry.path, cause)),
       );
       const inflated = archivedChecked.pipe(inflate);
-      const bounded = new SizeBound(entry.path, data.size);
+      const bounded = new SizeBound(
+        entry.path,
+        data.size,
+        this.decodedBudget,
+        signal,
+      );
       inflated.on("error", (cause: unknown) =>
         bounded.destroy(memberFailure(entry.path, cause)),
       );

src/artifacts/XarValidation.ts

@@ -0,0 +1,63 @@
+import { ArtifactReaderFailure } from "./ArtifactReader.js";
+
+/** Required XAR integer syntax: nonempty unsigned decimal, exactly representable. */
+export const xarInteger = (
+  value: string | undefined,
+  label: string,
+): number => {
+  const text = value?.trim() ?? "";
+  const parsed = /^\d+$/u.test(text) ? Number(text) : Number.NaN;
+  if (!Number.isSafeInteger(parsed))
+    throw new ArtifactReaderFailure(
+      "format",
+      `xar TOC has an invalid ${label}`,
+    );
+  return parsed;
+};
+
+/** Validate the entire absolute heap range before allocating or requesting it. */
+export const xarHeapPosition = (
+  heap: number,
+  offset: number,
+  length: number,
+  size: number,
+): number => {
+  const position = heap + offset;
+  const end = position + length;
+  if (
+    ![position, end, offset, length].every(Number.isSafeInteger) ||
+    offset < 0 ||
+    length < 0 ||
+    end > size
+  )
+    throw new ArtifactReaderFailure(
+      "format",
+      "xar member has an invalid or out-of-file heap extent",
+    );
+  return position;
+};
+
+/** Bound retained path text before constructing paths from deeply nested TOCs. */
+export class XarPathBudget {
+  #used = 0;
+  private readonly maximumPathBytes = 4096;
+  private readonly maximumTotalBytes = 16 * 1024 * 1024;
+
+  /** Form the next path only when individual and cumulative budgets permit it. */
+  join(parent: string, name: string): string {
+    const bytes =
+      Buffer.byteLength(parent) +
+      Buffer.byteLength(name) +
+      (parent.length > 0 ? 1 : 0);
+    if (
+      bytes > this.maximumPathBytes ||
+      bytes > this.maximumTotalBytes - this.#used
+    )
+      throw new ArtifactReaderFailure(
+        "limit",
+        `xar TOC path text exceeds its ${this.maximumPathBytes}-byte path or ${this.maximumTotalBytes}-byte cumulative budget`,
+      );
+    this.#used += bytes;
+    return parent === "" ? name : `${parent}/${name}`;
+  }
+}

src/artifacts/extraction/ArtifactExtraction.ts

@@ -172,8 +172,10 @@ const materializeSelection = async ({
       registry.add(path, expandable ? "directory" : entry.kind);
       if (expandable) {
         const parent = frame.reader;
-        const nested = new CpioArtifactReader((nestedSignal) =>
-          parent.open(entry, nestedSignal),
+        const nested = new CpioArtifactReader(
+          (nestedSignal) => parent.open(entry, nestedSignal),
+          "fail",
+          parent.decodedBudget,
         );
         stack.push({
           reader: nested,

src/artifacts/inventory/scanReader.ts

@@ -182,6 +182,7 @@ const visitArtifactEntries = async (
             ? new CpioArtifactReader(
                 (signal) => parent.open(entry, signal),
                 context.integrity.mode,
+                parent.decodedBudget,
               )
             : new AsarArtifactReader(entry.adapterKey);
         // Only traversed containers can own members, not opaque archive-named files.