fix(artifacts): bound expanded ASAR listing before allocation (#2724)

Squash merge of https://github.com/morluto/rea/pull/2724.

Co-authored-by: chAngE <172472630+chAng-L19@users.noreply.github.com>
Co-authored-by: morluto <morluto@users.noreply.github.com>

src/artifacts/AsarArtifactReader.test.ts

@@ -1,25 +0,0 @@
-import { describe, expect, it } from "vitest";
-
-import { orderAsarListing } from "./AsarArtifactReader.js";
-
-describe("orderAsarListing", () => {
-  it("orders host-spelled listings by their portable archive path", () => {
-    const posix = orderAsarListing(["/a0", "/a", "/a/b"], "/");
-    const windows = orderAsarListing(["\\a0", "\\a", "\\a\\b"], "\\");
-
-    // "/" (0x2F) precedes "0" (0x30); "\" (0x5C) would follow it.
-    expect(posix.map(({ path }) => path)).toEqual(["a", "a/b", "a0"]);
-    expect(windows.map(({ path }) => path)).toEqual(["a", "a/b", "a0"]);
-    expect(windows.map(({ listed }) => listed)).toEqual([
-      "\\a",
-      "\\a\\b",
-      "\\a0",
-    ]);
-  });
-
-  it("keeps POSIX backslashes as literal filename characters", () => {
-    expect(orderAsarListing(["/a\\b"], "/")).toEqual([
-      { listed: "/a\\b", path: "a\\b" },
-    ]);
-  });
-});

src/artifacts/AsarArtifactReader.ts

@@ -13,7 +13,7 @@ import { isAbsolute, join, relative, sep } from "node:path";
 import { Readable, Transform } from "node:stream";
 import { pipeline } from "node:stream/promises";
 
-import { listPackage, statFile, uncache } from "@electron/asar";
+import { getRawHeader, uncache } from "@electron/asar";
 
 import {
   ArtifactReaderFailure,
@@ -26,7 +26,11 @@ import { closeAsarHandle, readValidatedAsarEntry } from "./AsarEntryStream.js";
 import { admitAsarHeader } from "./AsarHeader.js";
 import { OwnedFileHandle } from "../filesystem/OwnedFileHandle.js";
 import { readFileHandleChunks } from "../filesystem/readFileHandleChunks.js";
-import { compareUnicodeCodePoints } from "../domain/unicodeCodePointOrder.js";
+import {
+  readAsarListing,
+  type AsarListedEntry,
+  type AsarMetadata,
+} from "./AsarListing.js";
 
 /**
  * Official Electron ASAR adapter with range-streamed member reads.
@@ -51,7 +55,7 @@ export class AsarArtifactReader implements ArtifactReader {
   ) {}
 
   async *entries(signal?: AbortSignal): AsyncIterable<ArtifactEntry> {
-    let paths: readonly AsarListedPath[];
+    let paths: readonly AsarListedEntry[];
     try {
       await this.prepareContainer(undefined, signal);
       paths = await this.#withSnapshotOperation(async () => {
@@ -62,8 +66,9 @@ export class AsarArtifactReader implements ArtifactReader {
         this.#archiveSize = admitted.archiveBytes;
         this.#headerSize = admitted.headerBytes;
         abortIfNeeded(signal);
-        const ordered = orderAsarListing(
-          listPackage(snapshotPath, { isPack: false }),
+        const ordered = await readAsarListing(
+          getRawHeader(snapshotPath).header,
+          signal,
         );
         const archiveMetadata = await lstat(snapshotPath);
         if (!archiveMetadata.isFile() || archiveMetadata.isSymbolicLink())
@@ -77,34 +82,25 @@ export class AsarArtifactReader implements ArtifactReader {
     } catch (cause: unknown) {
       throw asarFailure(this.path, "inventory", cause);
     }
-    for (const { listed, path } of paths) {
+    for (const { metadata, path } of paths) {
       abortIfNeeded(signal);
-      if (path.length === 0) continue;
-      const providerPath = listed.startsWith(sep)
-        ? listed.slice(sep.length)
-        : listed;
-      let metadata: ReturnType<typeof statFile>;
-      try {
-        metadata = statFile(this.#snapshotPath(), providerPath, false);
-      } catch (cause: unknown) {
-        throw asarFailure(this.path, `stat ${path}`, cause);
-      }
+      const providerPath = path.split("/").join(sep);
       const kind =
-        "files" in metadata
+        metadata.files !== undefined
           ? "directory"
-          : "link" in metadata
+          : metadata.link !== undefined
             ? "symlink"
             : "file";
       const entry: ArtifactEntry = {
         path,
         kind,
-        declaredSize: "size" in metadata ? metadata.size : null,
+        declaredSize: metadata.size ?? null,
         compressedSize: null,
-        executable: "executable" in metadata && metadata.executable,
+        executable: metadata.executable === true,
         encrypted: false,
         byteOffset: null,
         declaredSha256:
-          "integrity" in metadata &&
+          metadata.integrity !== undefined &&
           metadata.integrity.algorithm === "SHA256" &&
           /^[a-f0-9]{64}$/u.test(metadata.integrity.hash)
             ? metadata.integrity.hash
@@ -603,7 +599,7 @@ export class AsarArtifactReader implements ArtifactReader {
   }
 }
 
-type AsarFileMetadata = Extract<ReturnType<typeof statFile>, { size: number }>;
+type AsarFileMetadata = AsarMetadata & { readonly size: number };
 type AsarEntryState = {
   readonly metadata: AsarFileMetadata;
   readonly entry: ArtifactEntry;
@@ -652,12 +648,11 @@ const asarUnpackedEntryFailure = (
 };
 
 const isAsarFileMetadata = (
-  metadata: ReturnType<typeof statFile>,
+  metadata: AsarMetadata,
 ): metadata is AsarFileMetadata =>
-  "size" in metadata &&
   typeof metadata.size === "number" &&
-  !("files" in metadata) &&
-  !("link" in metadata);
+  metadata.files === undefined &&
+  metadata.link === undefined;
 
 const parseArchiveOffset = (value: string): number | undefined => {
   if (!/^(?:0|[1-9][0-9]*)$/u.test(value)) return undefined;
@@ -699,33 +694,6 @@ const waitForOperation = async (
   });
 };
 
-interface AsarListedPath {
-  readonly listed: string;
-  readonly path: string;
-}
-
-/**
- * Pair each host-spelled listing with its portable archive path, ordered by
- * the portable path so Windows and POSIX hosts agree.
- *
- * @electron/asar returns paths assembled with the host path module. Keep that
- * spelling for its API calls, but expose archive paths with portable
- * separators. On POSIX, backslashes can be literal filename characters and
- * must remain untouched.
- */
-export const orderAsarListing = (
-  listed: readonly string[],
-  separator: string = sep,
-): AsarListedPath[] =>
-  listed
-    .map((entry) => ({ listed: entry, path: toArtifactPath(entry, separator) }))
-    .sort((left, right) => compareUnicodeCodePoints(left.path, right.path));
-
-const toArtifactPath = (listed: string, separator: string): string => {
-  const portable = separator === "\\" ? listed.replaceAll("\\", "/") : listed;
-  return portable.replace(/^\/+|\/+$/gu, "");
-};
-
 const asarFailure = (
   path: string,
   operation: string,

src/artifacts/AsarListing.ts

@@ -0,0 +1,110 @@
+import { setImmediate } from "node:timers/promises";
+import { z } from "zod";
+
+import { ArtifactReaderFailure } from "./ArtifactReader.js";
+import { compareUnicodeCodePoints } from "../domain/unicodeCodePointOrder.js";
+
+/** Capacity for expanded paths and listing records, independent of packed bytes. */
+const MAX_LISTING_BYTES = 64 * 1024 * 1024;
+const nodeSchema = z.looseObject({
+  files: z
+    .custom<Record<string, unknown>>(
+      (value) =>
+        value !== null && typeof value === "object" && !Array.isArray(value),
+    )
+    .optional(),
+  link: z.string().optional(),
+  size: z.number().nonnegative().safe().optional(),
+  offset: z.string().optional(),
+  unpacked: z.boolean().optional(),
+  executable: z.boolean().optional(),
+  integrity: z
+    .looseObject({ algorithm: z.string(), hash: z.string() })
+    .optional(),
+});
+export type AsarMetadata = z.infer<typeof nodeSchema>;
+export interface AsarListedEntry {
+  readonly path: string;
+  readonly metadata: AsarMetadata;
+}
+
+function* ownChildren(files: Record<string, unknown>) {
+  for (const name in files)
+    if (Object.hasOwn(files, name)) yield [name, files[name]] as const;
+}
+
+/** Decode each node once; admit expanded paths before concatenation and retain no recursive lookup. */
+export const readAsarListing = async (
+  header: unknown,
+  signal?: AbortSignal,
+): Promise<AsarListedEntry[]> => {
+  const root = nodeSchema.parse(header);
+  if (root.files === undefined)
+    throw new ArtifactReaderFailure("format", "ASAR root has no files table");
+  const entries: AsarListedEntry[] = [];
+  const pending = [{ parent: "", children: ownChildren(root.files) }];
+  let retainedBytes = 0;
+  let examined = 0;
+  while (pending.length > 0) {
+    if (examined++ % 128 === 0) {
+      await setImmediate();
+      if (signal?.aborted)
+        throw new ArtifactReaderFailure(
+          "cancelled",
+          "ASAR operation cancelled",
+        );
+    }
+    const frame = pending.at(-1);
+    if (frame === undefined) break;
+    const next = frame.children.next();
+    if (next.done) {
+      pending.pop();
+      continue;
+    }
+    const [name, raw] = next.value;
+    // A header member is one component, not a host path to normalize. POSIX
+    // backslashes remain literal; Windows cannot represent them as components.
+    if (
+      name.length === 0 ||
+      name === "." ||
+      name === ".." ||
+      name.includes("/") ||
+      name.includes("\0") ||
+      (process.platform === "win32" && name.includes("\\"))
+    )
+      throw new ArtifactReaderFailure(
+        "path",
+        "ASAR header has an unsafe path component",
+      );
+    const units =
+      frame.parent.length + name.length + (frame.parent.length === 0 ? 0 : 1);
+    // UTF-16 storage, plus records and traversal bookkeeping. Charge every
+    // occurrence even when the engine can share a string prefix.
+    retainedBytes += units * 2 + 256;
+    if (retainedBytes > MAX_LISTING_BYTES)
+      throw new ArtifactReaderFailure(
+        "limit",
+        "ASAR expanded listing exceeds its 64 MiB representation budget",
+      );
+    const path = frame.parent.length === 0 ? name : `${frame.parent}/${name}`;
+    const metadata = nodeSchema.parse(raw);
+    if (
+      metadata.files === undefined &&
+      metadata.link === undefined &&
+      metadata.size === undefined
+    )
+      throw new ArtifactReaderFailure(
+        "format",
+        "ASAR entry has no directory, link or file metadata",
+      );
+    entries.push({ path, metadata });
+    if (metadata.files !== undefined)
+      pending.push({
+        parent: path,
+        children: ownChildren(metadata.files),
+      });
+  }
+  return entries.sort((left, right) =>
+    compareUnicodeCodePoints(left.path, right.path),
+  );
+};

tests/boundary/filesystem/asarListingAdmission.test.ts

@@ -0,0 +1,130 @@
+import { readFile, writeFile } from "node:fs/promises";
+import { join } from "node:path";
+import { buffer } from "node:stream/consumers";
+import { setImmediate } from "node:timers/promises";
+import { expect, it } from "vitest";
+
+import { AsarArtifactReader } from "../../../src/artifacts/AsarArtifactReader.js";
+import { readAsarListing } from "../../../src/artifacts/AsarListing.js";
+import {
+  asarHeaderBytes,
+  nestedAsarHeader,
+} from "../../fixtures/asarHeader.js";
+import { inventoryArtifact } from "../../fixtures/artifactInventory.js";
+import { createTestTempDirectory } from "../../fixtures/temporaryDirectory.js";
+
+const archiveWith = async (bytes: Buffer) => {
+  const root = await createTestTempDirectory("rea-asar-listing-");
+  const path = join(root, "app.asar");
+  await writeFile(path, bytes);
+  return path;
+};
+
+it("interrupts expanded listing admission between traversal batches", async () => {
+  const controller = new AbortController();
+  const header = {
+    files: Object.fromEntries(
+      Array.from({ length: 4000 }, (_, index) => [
+        `f${index}`,
+        { size: 0, offset: "0" },
+      ]),
+    ),
+  };
+  const listing = readAsarListing(header, controller.signal);
+  await setImmediate();
+  controller.abort();
+  await expect(listing).rejects.toMatchObject({ reason: "cancelled" });
+});
+
+it("rejects path expansion before publishing any inventory, preserving the input", async () => {
+  const bytes = nestedAsarHeader(64, "d".repeat(256), 4000);
+  expect(bytes.length).toBeLessThan(150_000);
+  const path = await archiveWith(bytes);
+  await expect(inventoryArtifact(path)).rejects.toMatchObject({
+    reason: "limit",
+    message: "ASAR expanded listing exceeds its 64 MiB representation budget",
+  });
+  expect(await readFile(path)).toEqual(bytes);
+});
+
+it("reads deeply nested packed metadata without recursive listing or member lookup", async () => {
+  const path = await archiveWith(nestedAsarHeader(3500, "d", 1));
+  const reader = new AsarArtifactReader(path);
+  try {
+    let count = 0;
+    for await (const entry of reader.entries()) {
+      count++;
+      if (entry.kind === "file") {
+        expect(entry.path).toBe(`${"d/".repeat(3500)}f0`);
+        expect(await buffer(await reader.open(entry))).toEqual(Buffer.alloc(0));
+      }
+    }
+    expect(count).toBe(3501);
+  } finally {
+    await reader.close();
+  }
+});
+
+it.each(["..", ".", "", "a/../b", "a/b", "a\0b"])(
+  "rejects raw ASAR component %j before host normalization can alias another entry",
+  async (component) => {
+    const json = JSON.stringify({
+      files: { [component]: { size: 0, offset: "0" } },
+    });
+    await expect(
+      inventoryArtifact(await archiveWith(asarHeaderBytes(json))),
+    ).rejects.toMatchObject({ reason: "path" });
+  },
+);
+
+it("orders raw names portably and retains prototype-named entries and links", async () => {
+  const path = await archiveWith(
+    asarHeaderBytes(
+      '{"files":{"a0":{"size":0,"offset":"0"},"__proto__":{"size":0,"offset":"0"},"a":{"files":{"b":{"link":"a0"}}},"😀":{"size":0,"offset":"0"},"\uE000":{"size":0,"offset":"0"}}}',
+    ),
+  );
+  const reader = new AsarArtifactReader(path);
+  try {
+    const entries = [];
+    for await (const entry of reader.entries())
+      entries.push([entry.path, entry.kind]);
+    expect(entries).toEqual([
+      ["__proto__", "file"],
+      ["a", "directory"],
+      ["a/b", "symlink"],
+      ["a0", "file"],
+      ["\uE000", "file"],
+      ["😀", "file"],
+    ]);
+  } finally {
+    await reader.close();
+  }
+});
+
+it("preserves POSIX literal backslashes and rejects Windows component separators", async () => {
+  const path = await archiveWith(
+    asarHeaderBytes(
+      JSON.stringify({ files: { "a\\b": { size: 0, offset: "0" } } }),
+    ),
+  );
+  const reader = new AsarArtifactReader(path);
+  try {
+    const list = async () => {
+      const entries = [];
+      for await (const entry of reader.entries()) entries.push(entry);
+      return entries;
+    };
+    if (process.platform === "win32") {
+      await expect(list()).rejects.toMatchObject({ reason: "path" });
+    } else {
+      const entries = await list();
+      expect(entries).toHaveLength(1);
+      expect(entries[0]?.path).toBe("a\\b");
+      expect(await buffer(await reader.open(entries[0]!))).toEqual(
+        Buffer.alloc(0),
+      );
+    }
+  } finally {
+    await reader.close();
+  }
+});

tests/boundary/mcp/asarListingAdmissionMcp.test.ts

@@ -0,0 +1,60 @@
+import { readFile, writeFile } from "node:fs/promises";
+import { join } from "node:path";
+import { Client, InMemoryTransport } from "@modelcontextprotocol/client";
+import { expect, it } from "vitest";
+
+import { ArtifactProvider } from "../../../src/artifacts/ArtifactProvider.js";
+import { createServer } from "../../../src/server/createServer.js";
+import { nestedAsarHeader } from "../../fixtures/asarHeader.js";
+import { createTestBinarySession } from "../../fixtures/binarySession.js";
+import { parseMcpToolError } from "../../fixtures/mcpToolError.js";
+import { createTestTempDirectory } from "../../fixtures/temporaryDirectory.js";
+
+it("reports expanded ASAR admission failure through MCP and permits a valid retry", async () => {
+  const root = await createTestTempDirectory("rea-asar-listing-mcp-");
+  const path = join(root, "app.asar");
+  const bytes = nestedAsarHeader(64, "d".repeat(256), 4000);
+  await writeFile(path, bytes);
+  const session = createTestBinarySession(new ArtifactProvider(process.env));
+  const server = createServer({ kind: "session", session });
+  const client = new Client({ name: "asar-listing-test", version: "1" });
+  const [clientTransport, serverTransport] =
+    InMemoryTransport.createLinkedPair();
+  try {
+    await server.connect(serverTransport);
+    await client.connect(clientTransport);
+    expect(
+      (await client.callTool({ name: "open_binary", arguments: { path } }))
+        .isError,
+    ).not.toBe(true);
+    const result = await client.callTool({
+      name: "inspect_artifact",
+      arguments: {},
+    });
+    expect(result.isError).toBe(true);
+    expect(parseMcpToolError(result)).toMatchObject({
+      error: {
+        code: "truncated",
+        category: "truncated",
+      },
+    });
+    expect(result.structuredContent).toBeUndefined();
+    expect(await readFile(path)).toEqual(bytes);
+    const valid = join(root, "valid.asar");
+    await writeFile(valid, nestedAsarHeader(1, "lib", 1));
+    expect(
+      (
+        await client.callTool({
+          name: "open_binary",
+          arguments: { path: valid },
+        })
+      ).isError,
+    ).not.toBe(true);
+    expect(
+      (await client.callTool({ name: "inspect_artifact", arguments: {} }))
+        .isError,
+    ).not.toBe(true);
+  } finally {
+    await Promise.allSettled([client.close(), server.close(), session.close()]);
+  }
+});

tests/fixtures/asarHeader.ts

@@ -0,0 +1,27 @@
+/** Serialize a header-only ASAR, including Chromium pickle alignment. */
+export const asarHeaderBytes = (json: string): Buffer => {
+  const text = Buffer.from(json);
+  const headerSize = 8 + Math.ceil(text.length / 4) * 4;
+  const bytes = Buffer.alloc(8 + headerSize);
+  bytes.writeUInt32LE(4, 0);
+  bytes.writeUInt32LE(headerSize, 4);
+  bytes.writeUInt32LE(headerSize - 4, 8);
+  bytes.writeUInt32LE(text.length, 12);
+  text.copy(bytes, 16);
+  return bytes;
+};
+
+/** Nested metadata without a recursive fixture builder or real directories. */
+export const nestedAsarHeader = (
+  depth: number,
+  name: string,
+  leaves: number,
+): Buffer => {
+  let json = `{"files":{${Array.from(
+    { length: leaves },
+    (_, index) => `"f${index}":{"size":0,"offset":"0"}`,
+  ).join(",")}}}`;
+  for (let index = 0; index < depth; index++)
+    json = `{"files":{${JSON.stringify(name)}:${json}}}`;
+  return asarHeaderBytes(json);
+};