diff --git a/src/artifacts/pyinstaller/PyInstallerMetadata.ts b/src/artifacts/pyinstaller/PyInstallerMetadata.ts index 8319721064..a5848641ad 100644 --- a/src/artifacts/pyinstaller/PyInstallerMetadata.ts +++ b/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, ): 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, diff --git a/src/artifacts/pyinstaller/PyzArchive.test.ts b/src/artifacts/pyinstaller/PyzArchive.test.ts index e553589d5b..a70efae0f4 100644 --- a/src/artifacts/pyinstaller/PyzArchive.test.ts +++ b/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" }, + }); +}); diff --git a/src/artifacts/pyinstaller/PyzArchive.ts b/src/artifacts/pyinstaller/PyzArchive.ts index c398cfae0f..7dcbccbec2 100644 --- a/src/artifacts/pyinstaller/PyzArchive.ts +++ b/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 => { - 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.`, }) diff --git a/tests/boundary/mcp/pyinstallerArtifactMcp.test.ts b/tests/boundary/mcp/pyinstallerArtifactMcp.test.ts index 4401226c77..cb210b2235 100644 --- a/tests/boundary/mcp/pyinstallerArtifactMcp.test.ts +++ b/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(); + } + }, +);