refactor(pyinstaller): centralize PYZ header and version facts

src/artifacts/pyinstaller/PyInstallerMetadata.ts

@@ -4,9 +4,8 @@ import type {
 } from "../../domain/artifactFormatMetadata.js";
 import type { CArchiveEntry, PyInstallerCookie } from "./CArchiveLayout.js";
 import type { PyzArchive, PyzEntry } from "./PyzArchive.js";
-import { pythonVersionForMagic } from "./PythonBytecode.js";
 
-const pythonVersion = (value: number) =>
+const cookiePythonVersion = (value: number) =>
   value >= 100
     ? { major: Math.floor(value / 100), minor: value % 100 }
     : value >= 20
@@ -18,7 +17,7 @@ export const cookieMetadata = (
   entries: readonly CArchiveEntry[],
   encryptionMarker: boolean,
 ): PyInstallerMetadata => {
-  const version = pythonVersion(cookie.pythonVersion);
+  const version = cookiePythonVersion(cookie.pythonVersion);
   return {
     kind: "pyinstaller",
     layout_version: cookie.layout,
@@ -45,13 +44,13 @@ export const scriptPythonMagic = (
   cookie: PyInstallerCookie,
   archives: Iterable<PyzArchive>,
 ): Buffer | undefined => {
-  const version = pythonVersion(cookie.pythonVersion);
+  const version = cookiePythonVersion(cookie.pythonVersion);
   if (version === undefined) return undefined;
   let selected: Buffer | undefined;
   for (const archive of archives) {
     const magic = archive.pythonMagic;
     if (
-      pythonVersionForMagic(magic) !==
+      archive.pythonVersion !==
         `${String(version.major)}.${String(version.minor)}` ||
       (selected !== undefined && !selected.equals(magic))
     )
@@ -123,10 +122,7 @@ export const cArchiveEntryMetadata = (
   directory_record_size: entry.tocRecordLength,
   pyz_sha256: archive?.sha256 ?? null,
   pyz_python_magic: archive?.pythonMagic.toString("hex") ?? null,
-  pyz_python_version:
-    archive === undefined
-      ? null
-      : (pythonVersionForMagic(archive.pythonMagic) ?? null),
+  pyz_python_version: archive?.pythonVersion ?? null,
   pyz_entry_count: archive?.kind === "entries" ? archive.entries.length : null,
   encryption_source:
     archive !== undefined && encryptionMarker ? "carchive-marker" : null,

src/artifacts/pyinstaller/PyzArchive.test.ts

@@ -1,3 +1,4 @@
+import { createHash } from "node:crypto";
 import { open, writeFile } from "node:fs/promises";
 import { join } from "node:path";
 import { expect, it } from "vitest";
@@ -31,6 +32,7 @@ it("lists modules from a supported PYZ and leaves an unknown magic opaque", asyn
   if (!opaque.ok || opaque.value.kind !== "opaque") return;
   expect(Buffer.compare(opaque.value.pythonMagic, Buffer.alloc(4))).toBe(0);
   expect(opaque.value.limitation).toContain("left opaque");
+  expect(opaque.value.pythonVersion).toBeUndefined();
 });
 
 it("leaves a PYZ opaque when the table uses an unsupported marshal type", async () => {
@@ -44,6 +46,10 @@ it("leaves a PYZ opaque when the table uses an unsupported marshal type", async
   if (!parsed.ok || parsed.value.kind !== "opaque") return;
   expect(parsed.value.limitation).toContain("left opaque");
   expect(parsed.value.pythonMagic).toEqual(PYTHON_39_MAGIC);
+  expect(parsed.value.pythonVersion).toBe("3.9");
+  expect(parsed.value.sha256).toBe(
+    createHash("sha256").update(image).digest("hex"),
+  );
 });
 
 it("reports an entry that extends outside the PYZ as an integrity error", async () => {
@@ -58,26 +64,29 @@ it("reports an entry that extends outside the PYZ as an integrity error", async
   expect(parsed.error.reason).toBe("integrity");
 });
 
-it("rejects an oversized sparse PYZ table before allocating its bytes", async () => {
-  const root = await createTestTempDirectory("rea-pyz-table-budget-");
-  const path = join(root, "image.pyz");
-  const header = Buffer.alloc(12);
-  header.write("PYZ\0");
-  PYTHON_39_MAGIC.copy(header, 4);
-  header.writeUInt32BE(12, 8);
-  await writeFile(path, header);
-  const handle = await open(path, "r+");
-  try {
-    const size = 12 + MAX_PYZ_TABLE_BYTES + 1;
-    await handle.truncate(size);
-    expect(await readPyzArchive(handle, 0, size)).toMatchObject({
-      ok: false,
-      error: { reason: "limit" },
-    });
-  } finally {
-    await handle.close();
-  }
-});
+it.each(["PYZ\0", "BAD\0"])(
+  "rejects an oversized sparse table before deferred signature rejection (%s)",
+  async (signature) => {
+    const root = await createTestTempDirectory("rea-pyz-table-budget-");
+    const path = join(root, "image.pyz");
+    const header = Buffer.alloc(12);
+    header.write(signature);
+    PYTHON_39_MAGIC.copy(header, 4);
+    header.writeUInt32BE(12, 8);
+    await writeFile(path, header);
+    const handle = await open(path, "r+");
+    try {
+      const size = 12 + MAX_PYZ_TABLE_BYTES + 1;
+      await handle.truncate(size);
+      expect(await readPyzArchive(handle, 0, size)).toMatchObject({
+        ok: false,
+        error: { reason: "limit" },
+      });
+    } finally {
+      await handle.close();
+    }
+  },
+);
 
 it("rejects a PYZ table whose back-references repeat one long row", async () => {
   // One namespace row with a 16 KiB name, flagged for reference, then 999
@@ -124,14 +133,52 @@ const int32 = (value: number): Buffer => {
   return bytes;
 };
 
-const readImage = async (bytes: Buffer) => {
+const readImage = async (bytes: Buffer, declaredSize = bytes.length) => {
   const root = await createTestTempDirectory("rea-pyz-file-");
   const path = join(root, "image.pyz");
   await writeFile(path, bytes);
   const handle = await open(path, "r");
   try {
-    return await readPyzArchive(handle, 0, bytes.length);
+    return await readPyzArchive(handle, 0, declaredSize);
   } finally {
     await handle.close();
   }
 };
+
+it.each([0, 0xffffffff])(
+  "retains an unknown Python magic and full-image digest with invalid TOC offset %s",
+  async (offset) => {
+    const image = pyzArchive({ magic: Buffer.from([1, 2, 3, 4]), modules: [] });
+    image.writeUInt32BE(offset, 8);
+    expect(await readImage(image)).toMatchObject({
+      ok: true,
+      value: {
+        kind: "opaque",
+        pythonMagic: Buffer.from([1, 2, 3, 4]),
+        pythonVersion: undefined,
+        sha256: createHash("sha256").update(image).digest("hex"),
+        limitation: expect.stringContaining("not in the supported set"),
+      },
+    });
+  },
+);
+
+it("reports a truncated table before deferred signature rejection", async () => {
+  const header = Buffer.alloc(12);
+  header.write("BAD\0");
+  PYTHON_39_MAGIC.copy(header, 4);
+  header.writeUInt32BE(12, 8);
+  expect(await readImage(header, 16)).toMatchObject({
+    ok: false,
+    error: { reason: "format", message: "PYZ table is truncated" },
+  });
+});
+
+it("reports the signature after completing a readable table and image hash", async () => {
+  const image = pyzArchive({ magic: PYTHON_39_MAGIC, modules: [] });
+  image.write("BAD\0");
+  expect(await readImage(image)).toMatchObject({
+    ok: false,
+    error: { reason: "format", message: "PYZ archive magic does not match" },
+  });
+});

src/artifacts/pyinstaller/PyzArchive.ts

@@ -38,6 +38,7 @@ export type PyzArchive =
   | {
       readonly kind: "opaque";
       readonly pythonMagic: Buffer;
+      readonly pythonVersion: string | undefined;
       readonly sha256: string;
       readonly limitation: string;
     };
@@ -45,6 +46,31 @@ export type PyzArchive =
 /** Bound the transient marshal table allocation independently of image expansion. */
 export const MAX_PYZ_TABLE_BYTES = 16 * 1024 * 1024;
 
+interface PyzHeaderFacts {
+  readonly signatureMatches: boolean;
+  readonly pythonMagic: Buffer;
+  readonly pythonVersion: string | undefined;
+  readonly tocOffset: number;
+  /** Null means the declaration lies outside this image, not an empty table. */
+  readonly tableBytes: number | null;
+}
+
+/** Called only after the exact fixed-header read has succeeded. */
+const readPyzHeaderFacts = (header: Buffer, size: number): PyzHeaderFacts => {
+  const pythonMagic = Buffer.from(header.subarray(4, 8));
+  const tocOffset = header.readUInt32BE(8);
+  return {
+    signatureMatches: header.subarray(0, 4).equals(PYZ_MAGIC),
+    pythonMagic,
+    pythonVersion: pythonVersionForMagic(pythonMagic),
+    tocOffset,
+    tableBytes:
+      tocOffset >= PYZ_HEADER_BYTES && tocOffset <= size
+        ? size - tocOffset
+        : null,
+  };
+};
+
 /** Parse only the header and table in memory; hash the complete file range in chunks. */
 export const readPyzArchive = async (
   handle: FileHandle,
@@ -63,12 +89,9 @@ export const readPyzArchive = async (
     signal,
   });
   if (!headerRead.ok) return headerRead;
-  const header = headerRead.value;
-  const tocOffset =
-    header.length === PYZ_HEADER_BYTES ? header.readUInt32BE(8) : size;
-  const validOffset = tocOffset >= PYZ_HEADER_BYTES && tocOffset <= size;
-  const knownMagic = pythonVersionForMagic(header.subarray(4, 8)) !== undefined;
-  const tableSize = validOffset && knownMagic ? size - tocOffset : 0;
+  const header = readPyzHeaderFacts(headerRead.value, size);
+  const tableSize =
+    header.pythonVersion === undefined ? 0 : (header.tableBytes ?? 0);
   if (tableSize > MAX_PYZ_TABLE_BYTES)
     return err(
       pyInstallerFailure(
@@ -77,7 +100,7 @@ export const readPyzArchive = async (
       ),
     );
   const read = await readPyInstallerBytes(handle, {
-    position: start + tocOffset,
+    position: start + header.tocOffset,
     length: tableSize,
     label: "PYZ table",
     signal,
@@ -96,33 +119,28 @@ export const readPyzArchive = async (
     return err(
       pyInstallerFailure("format", "PYZ image extends outside the file"),
     );
-  return parsePyzParts(header, table, size, digest.sha256);
+  return parsePyzParts(header, table, digest.sha256);
 };
 
 const parsePyzParts = (
-  header: Buffer,
+  header: PyzHeaderFacts,
   table: Buffer,
-  size: number,
   sha256: string,
 ): Result<PyzArchive, PyInstallerFailure> => {
-  if (
-    header.length < PYZ_HEADER_BYTES ||
-    !header.subarray(0, 4).equals(PYZ_MAGIC)
-  )
+  if (!header.signatureMatches)
     return err(
       pyInstallerFailure("format", "PYZ archive magic does not match"),
     );
-  const pythonMagic = Buffer.from(header.subarray(4, 8));
-  const pythonVersion = pythonVersionForMagic(pythonMagic);
+  const { pythonMagic, pythonVersion, tocOffset } = header;
   if (pythonVersion === undefined)
     return ok({
       kind: "opaque",
       pythonMagic,
+      pythonVersion,
       sha256,
       limitation: `PYZ Python magic ${pythonMagic.toString("hex")} is not in the supported set; the member was left opaque.`,
     });
-  const tocOffset = header.readUInt32BE(8);
-  if (tocOffset < PYZ_HEADER_BYTES || tocOffset > size)
+  if (header.tableBytes === null)
     return err(
       pyInstallerFailure(
         "format",
@@ -135,6 +153,7 @@ const parsePyzParts = (
     return ok({
       kind: "opaque",
       pythonMagic,
+      pythonVersion,
       sha256,
       limitation: `PYZ table of contents could not be parsed (${marshalled.error.message}); the member was left opaque.`,
     });
@@ -145,6 +164,7 @@ const parsePyzParts = (
       ? ok({
           kind: "opaque",
           pythonMagic,
+          pythonVersion,
           sha256,
           limitation: `${entries.error.message}; the member was left opaque.`,
         })

tests/boundary/mcp/pyinstallerArtifactMcp.test.ts

@@ -436,3 +436,139 @@ it("preserves surrogatepass PYZ module names through inspect_artifact", async ()
     await session.close();
   }
 });
+
+it.each([
+  {
+    name: "recognized opaque magic agrees with cookie",
+    cookie: 309,
+    magic: PYTHON_39_MAGIC,
+    version: "3.9",
+    conflict: false,
+    derived: true,
+  },
+  {
+    name: "recognized opaque magic disagrees with cookie",
+    cookie: 312,
+    magic: PYTHON_39_MAGIC,
+    version: "3.9",
+    conflict: false,
+    derived: false,
+  },
+  {
+    name: "unknown opaque magic with invalid TOC",
+    cookie: 309,
+    magic: Buffer.alloc(4),
+    version: null,
+    conflict: false,
+    derived: false,
+  },
+  {
+    name: "recognized opaque siblings disagree",
+    cookie: 309,
+    magic: PYTHON_39_MAGIC,
+    version: "3.9",
+    conflict: true,
+    derived: false,
+  },
+])(
+  "retains PYZ header evidence through inspect_artifact: $name",
+  async ({ cookie, magic, version, conflict, derived }) => {
+    const payload = pyzArchive({
+      magic,
+      modules: [],
+      toc: Buffer.from([0x4e]),
+    });
+    if (version === null) payload.writeUInt32BE(0xffffffff, 8);
+    const directory = await createTestTempDirectory("rea-opaque-pyz-metadata-");
+    const path = join(directory, "opaque-header");
+    await writeFile(
+      path,
+      cArchiveExecutable({
+        layout: "2.1",
+        pythonVersion: cookie,
+        members: [
+          {
+            name: "hello",
+            typeCode: "s",
+            payload: Buffer.from("entry script"),
+            compress: false,
+          },
+          { name: "PYZ.pyz", typeCode: "z", payload, compress: true },
+          ...(conflict
+            ? [
+                {
+                  name: "other.pyz",
+                  typeCode: "z",
+                  payload: pyzArchive({
+                    magic: Buffer.from("f30d0d0a", "hex"),
+                    modules: [],
+                    toc: Buffer.from([0x4e]),
+                  }),
+                  compress: false,
+                },
+              ]
+            : []),
+        ],
+      }),
+    );
+    const session = await createArtifactBinarySession(process.env);
+    const server = createServer({ kind: "session", session });
+    const client = new Client({
+      name: "opaque-pyz-metadata-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 reply = await client.callTool({
+        name: "inspect_artifact",
+        arguments: {},
+      });
+      expect(reply.isError).not.toBe(true);
+      const inspection = artifactInspectionResultSchema.parse(
+        parseEvidence(reply.structuredContent).normalized_result,
+      );
+      const inventory = inspection.substeps[0]?.evidence.normalized_result;
+      expect(inventory?.coverage.status).toBe("unknown");
+      expect(
+        inventory?.occurrences.find((row) => row.logical_path === "PYZ.pyz"),
+      ).toMatchObject({
+        entry_kind: "file",
+        hash_status: "verified",
+        format_metadata: {
+          pyz_status: "opaque",
+          pyz_sha256: createHash("sha256").update(payload).digest("hex"),
+          pyz_python_magic: magic.toString("hex"),
+          pyz_python_version: version,
+          pyz_entry_count: null,
+        },
+      });
+      expect(
+        inventory?.occurrences.find((row) => row.logical_path === "hello")
+          ?.format_metadata,
+      ).toMatchObject({
+        bytecode_header: derived ? "derived" : "unknown",
+        bytecode_magic: derived ? magic.toString("hex") : null,
+      });
+      if (conflict)
+        expect(
+          inventory?.occurrences.find((row) => row.logical_path === "other.pyz")
+            ?.format_metadata,
+        ).toMatchObject({
+          pyz_status: "opaque",
+          pyz_python_version: "3.13",
+          pyz_python_magic: "f30d0d0a",
+        });
+    } finally {
+      await client.close();
+      await server.close();
+      await session.close();
+    }
+  },
+);