From 81de82a1a4dda419e85ab55bdf458d4dc1ee0788 Mon Sep 17 00:00:00 2001 From: morluto <76467478+morluto@users.noreply.github.com> Date: Fri, 23 Oct 2026 17:39:26 +0800 Subject: [PATCH] fix(python): classify historical headers and signed flags Fixes #6568, #6487 --- docs/python-pyc.md | 7 +- src/cli/pythonPycCommands.ts | 2 +- .../python/pythonPycToolContracts.ts | 2 +- src/domain/python/pythonPyc.test.ts | 12 ++ src/domain/python/pythonPyc.ts | 23 +-- src/python/PythonPycParser.test.ts | 29 ++++ src/python/PythonPycParser.ts | 41 ++++-- tests/acceptance/pythonPycCli.test.ts | 131 ++++++++++++++++++ tests/boundary/mcp/pythonPycMcp.test.ts | 61 ++++++++ 9 files changed, 281 insertions(+), 27 deletions(-) diff --git a/docs/python-pyc.md b/docs/python-pyc.md index 4998b426be..c2674a8cce 100644 --- a/docs/python-pyc.md +++ b/docs/python-pyc.md @@ -28,7 +28,8 @@ The inline Evidence result contains the artifact path, SHA-256, and byte length. - `magic`, the 16-bit little-endian field; - `python_version` and `code_layout`, mapped from the pinned CPython 3.7–3.13 table (`py37`, `py38`, or `py311`); 3.11 alpha magics before 3494 used - other code layouts and are unsupported; + other code layouts and are unsupported. The 3.7.0a1/a2 magics 3390/3391 + used 12-byte headers and are outside this reader's supported format; - `flags`, `hash_based`, and `check_source` from PEP 552; - `source_hash_hex` for hash-based pycs, or `source_mtime` and `source_size` for timestamp pycs. @@ -71,7 +72,9 @@ The selected file is limited to 16 MiB by default (64 MiB maximum). Marshal object count, nesting depth, and per-string size are bounded; exceeding a budget fails without partial success. Argument counts, positional-only counts, keyword-only counts, local counts, and stack size are signed CPython fields; a -negative encoding is malformed. On the 3.11+ layout, `co_code` must cover a +negative encoding is malformed. Code `flags` preserve signed `co_flags` values; +CPython 3.8+ rejects negative flags, while 3.7 accepts them. On the 3.11+ +layout, `co_code` must cover a whole number of code units. Every layout rejects argument counts that do not fit the local table CPython checks (`co_varnames`, including `*args` / `**kwargs`, or fast locals on 3.11+). Those files are malformed, so the diff --git a/src/cli/pythonPycCommands.ts b/src/cli/pythonPycCommands.ts index cf79aaee4d..f408eee3aa 100644 --- a/src/cli/pythonPycCommands.ts +++ b/src/cli/pythonPycCommands.ts @@ -16,7 +16,7 @@ export const registerPythonPycCommands = ( ): void => { cli.command(CLI_COMMANDS.inspectPythonPyc, { description: - "Inspect a CPython 3.7–3.13 pyc header and marshal code objects without executing bytecode", + "Inspect a supported CPython 3.7–3.13 pyc header and marshal code objects without executing bytecode", args: z.object({ path: z.string().describe("Local CPython .pyc file"), }), diff --git a/src/contracts/python/pythonPycToolContracts.ts b/src/contracts/python/pythonPycToolContracts.ts index 7db11087d8..fe312eee07 100644 --- a/src/contracts/python/pythonPycToolContracts.ts +++ b/src/contracts/python/pythonPycToolContracts.ts @@ -13,7 +13,7 @@ export const PYTHON_PYC_TOOL_CONTRACTS = [ ...toolContractMetadata("inspect_python_pyc"), kind: "artifact-provider", description: - "Inspect one explicit local CPython 3.7–3.13 .pyc file without executing bytecode, invoking CPython or recovering source. Returns the 16-byte header (magic, pinned python_version, PEP 552 flags, source hash or mtime/size), the top-level code object with nested functions, names, const summaries, and bytecode length/SHA-256. Marshal is walked with object, depth and string budgets; unknown magics, truncated streams and exhausted budgets return no partial success. A decoded header is not proof the matching .py is present. Portable on supported Node hosts; no Python installation required.", + "Inspect one explicit local CPython 3.7–3.13 .pyc file with a supported 16-byte header without executing bytecode, invoking CPython or recovering source. Returns the header (magic, pinned python_version, PEP 552 flags, source hash or mtime/size), the top-level code object with nested functions, names, const summaries, and bytecode length/SHA-256. Marshal is walked with object, depth and string budgets; unsupported magics/layouts, truncated streams and exhausted budgets return no partial success. A decoded header is not proof the matching .py is present. Portable on supported Node hosts; no Python installation required.", inputSchema: inspectPythonPycInputSchema, outputSchema: evidenceResultOf(pythonPycSchema), examples: [ diff --git a/src/domain/python/pythonPyc.test.ts b/src/domain/python/pythonPyc.test.ts index ea98b5d25c..1c7bc34c26 100644 --- a/src/domain/python/pythonPyc.test.ts +++ b/src/domain/python/pythonPyc.test.ts @@ -9,18 +9,30 @@ it("maps pinned CPython 3.7–3.13 magics and leaves gaps unknown", () => { expect(pythonPycMagicEntry(3394)).toEqual({ python_version: "3.7", code_layout: "py37", + header_bytes: 16, + }); + expect(pythonPycMagicEntry(3390)).toMatchObject({ + python_version: "3.7", + header_bytes: 12, + }); + expect(pythonPycMagicEntry(3391)).toMatchObject({ + python_version: "3.7", + header_bytes: 12, }); expect(pythonPycMagicEntry(3413)).toEqual({ python_version: "3.8", code_layout: "py38", + header_bytes: 16, }); expect(pythonPycMagicEntry(3531)).toEqual({ python_version: "3.12", code_layout: "py311", + header_bytes: 16, }); expect(pythonPycMagicEntry(3571)).toEqual({ python_version: "3.13", code_layout: "py311", + header_bytes: 16, }); expect(pythonPycMagicEntry(3450)).toBeNull(); expect(pythonPycMagicEntry(62171)).toBeNull(); diff --git a/src/domain/python/pythonPyc.ts b/src/domain/python/pythonPyc.ts index eed477c998..d55167cfe6 100644 --- a/src/domain/python/pythonPyc.ts +++ b/src/domain/python/pythonPyc.ts @@ -9,8 +9,8 @@ export const PYTHON_PYC_CHECK_SOURCE = 0x2; const uint = z.number().int().min(0).max(0xffffffff); const index = z.number().int().nonnegative(); const signedInt32 = z.number().int().min(-0x80000000).max(0x7fffffff); -/** Non-negative CPython `int` counts. `0xffffffff` is -1 and is not evidence. */ -const signedCount = z.number().int().min(0).max(0x7fffffff); +/** Non-negative CPython `int` counts. */ +const nonNegativeInt32 = z.number().int().min(0).max(0x7fffffff); /** Marshal TYPE_CODE field order for CPython 3.7, 3.8–3.10, and 3.11+. */ export const PYTHON_PYC_CODE_LAYOUTS = ["py37", "py38", "py311"] as const; @@ -19,14 +19,16 @@ export type PythonPycCodeLayout = (typeof PYTHON_PYC_CODE_LAYOUTS)[number]; type MagicEntry = { readonly python_version: string; readonly code_layout: PythonPycCodeLayout; + readonly header_bytes: 12 | 16; }; const magics = ( values: readonly number[], python_version: string, code_layout: PythonPycCodeLayout, + header_bytes: 12 | 16 = PYTHON_PYC_HEADER_BYTES, ): ReadonlyArray => - values.map((magic) => [magic, { python_version, code_layout }]); + values.map((magic) => [magic, { python_version, code_layout, header_bytes }]); const consecutive = ( start: number, @@ -46,7 +48,8 @@ const consecutive = ( * bytecode opcode tables. */ export const PYTHON_PYC_MAGIC_TABLE: ReadonlyMap = new Map([ - ...magics([3390, 3391, 3392, 3393, 3394], "3.7", "py37"), + ...magics([3390, 3391], "3.7", "py37", 12), + ...magics([3392, 3393, 3394], "3.7", "py37"), ...magics([3400, 3401], "3.8", "py37"), ...magics([3410, 3411, 3412, 3413], "3.8", "py38"), ...magics([3420, 3421, 3422, 3423, 3424, 3425], "3.9", "py38"), @@ -159,12 +162,12 @@ const pythonCodeFields = { qualname: z.string().nullable(), filename: z.string(), firstlineno: signedInt32, - argcount: signedCount, - posonlyargcount: signedCount.nullable(), - kwonlyargcount: signedCount, - nlocals: signedCount.nullable(), - stacksize: signedCount, - flags: uint, + argcount: nonNegativeInt32, + posonlyargcount: nonNegativeInt32.nullable(), + kwonlyargcount: nonNegativeInt32, + nlocals: nonNegativeInt32.nullable(), + stacksize: nonNegativeInt32, + flags: signedInt32, names: z.array(z.string()), varnames: z.array(z.string()).nullable(), freevars: z.array(z.string()).nullable(), diff --git a/src/python/PythonPycParser.test.ts b/src/python/PythonPycParser.test.ts index d3bc5ad83c..d2cf9d594b 100644 --- a/src/python/PythonPycParser.test.ts +++ b/src/python/PythonPycParser.test.ts @@ -139,6 +139,21 @@ it("fails unknown magics and truncated marshal without partial success", () => { ); }); +it("classifies the 12-byte 3.7 alpha header as unsupported", () => { + for (const magic of [3390, 3391]) { + const header = Buffer.alloc(12); + header.writeUInt16LE(magic); + header[2] = 0x0d; + header[3] = 0x0a; + expect(() => parse(Buffer.concat([header, marshalCode("py37")]))).toThrow( + expect.objectContaining({ + kind: "unsupported", + message: expect.stringContaining("unsupported 12-byte CPython header"), + }), + ); + } +}); + const u32 = (value: number): Buffer => { const bytes = Buffer.alloc(4); bytes.writeUInt32LE(value); @@ -1405,6 +1420,12 @@ it("rejects code CPython treats as corrupt instead of complete coverage", () => expect(() => parse(file("py37", 3394, { argcount: 1 }))).toThrow( /co_varnames is too small/, ); + expect(() => parse(file("py311", 3531, { flags: 0x80000000 }))).toThrow( + /co_flags is negative/, + ); + expect(() => parse(file("py37", 3401, { flags: 0x80000000 }))).toThrow( + /co_flags is negative/, + ); const valid = parse( file("py311", 3531, { @@ -1418,6 +1439,14 @@ it("rejects code CPython treats as corrupt instead of complete coverage", () => ); expect(root(valid).argcount).toBe(1); expect(valid.coverage.status).toBe("complete"); + const unknownPositiveFlags = parse( + file("py311", 3531, { flags: 0x40000000 }), + ); + expect(root(unknownPositiveFlags).flags).toBe(0x40000000); + expect(unknownPositiveFlags.coverage.status).toBe("complete"); + const negativePy37Flags = parse(file("py37", 3394, { flags: 0x80000000 })); + expect(root(negativePy37Flags).flags).toBe(-0x80000000); + expect(pythonPycSchema.safeParse(negativePy37Flags).success).toBe(true); const py37 = parse( file("py37", 3394, { argcount: 1, diff --git a/src/python/PythonPycParser.ts b/src/python/PythonPycParser.ts index 83d53ac8aa..6698323ace 100644 --- a/src/python/PythonPycParser.ts +++ b/src/python/PythonPycParser.ts @@ -188,14 +188,19 @@ export const parsePythonPyc = ( ): PythonPyc => { if (bytes.length !== artifact.bytes) malformed("Selected artifact length disagrees with the retained bytes."); - if (bytes.length < PYTHON_PYC_HEADER_BYTES) - malformed( - "Selected file is shorter than a 16-byte CPython 3.7+ pyc header.", - ); + if (bytes.length < 4) malformed("Selected file is shorter than a pyc magic."); if (bytes[2] !== 0x0d || bytes[3] !== 0x0a) malformed("Pyc magic is not a little-endian integer followed by CR LF."); const magic = bytes.readUInt16LE(0); const mapped = pythonPycMagicEntry(magic); + if (mapped !== null && mapped.header_bytes !== PYTHON_PYC_HEADER_BYTES) + return unsupported( + `Pyc magic ${magic} uses an unsupported ${mapped.header_bytes}-byte CPython header.`, + ); + if (bytes.length < PYTHON_PYC_HEADER_BYTES) + malformed( + "Selected file is shorter than a 16-byte CPython 3.7+ pyc header.", + ); if (mapped === null) return unsupported( `Pyc magic ${magic} is outside the pinned CPython 3.7–3.13 table.`, @@ -214,7 +219,12 @@ export const parsePythonPyc = ( source_size: hashBased ? null : bytes.readUInt32LE(12), }; const marshal = bytes.subarray(PYTHON_PYC_HEADER_BYTES); - const reader = new MarshalReader(marshal, mapped.code_layout, input); + const reader = new MarshalReader( + marshal, + mapped.code_layout, + input, + mapped.python_version !== "3.7", + ); const root = reader.readObject(0); if (reader.offset !== marshal.length) malformed("Marshal stream has trailing bytes after the top-level object."); @@ -297,6 +307,7 @@ class MarshalReader { readonly bytes: Buffer, readonly layout: PythonPycCodeLayout, readonly input: InspectPythonPycInput, + readonly rejectNegativeCodeFlags: boolean, ) {} readObject(depth: number): MarshalValue { @@ -400,16 +411,20 @@ class MarshalReader { } code(depth: number): CodeNode { - const argcount = this.signedCount("co_argcount"); + const argcount = this.nonNegativeInt32("co_argcount"); const posonlyargcount = - this.layout === "py37" ? null : this.signedCount("co_posonlyargcount"); + this.layout === "py37" + ? null + : this.nonNegativeInt32("co_posonlyargcount"); if (posonlyargcount !== null && argcount < posonlyargcount) malformed("co_argcount is smaller than co_posonlyargcount."); - const kwonlyargcount = this.signedCount("co_kwonlyargcount"); + const kwonlyargcount = this.nonNegativeInt32("co_kwonlyargcount"); const nlocals = - this.layout === "py311" ? null : this.signedCount("co_nlocals"); - const stacksize = this.signedCount("co_stacksize"); - const flags = this.u32(); + this.layout === "py311" ? null : this.nonNegativeInt32("co_nlocals"); + const stacksize = this.nonNegativeInt32("co_stacksize"); + const flags = this.i32(); + if (this.rejectNegativeCodeFlags && flags < 0) + malformed("co_flags is negative."); const bytecode = asBytes(this.readObject(depth + 1), "co_code"); // 3.11+ `_PyCode_Validate` rejects a length that is not a whole code unit. // 3.7–3.10 constructors do not, so an odd `co_code` stays readable there. @@ -1406,8 +1421,8 @@ class MarshalReader { return value; } - /** CPython `r_long` count. Negative values are rejected before Evidence. */ - signedCount(field: string): number { + /** CPython `r_long` int fields must not be negative in a code object. */ + nonNegativeInt32(field: string): number { const value = this.i32(); if (value < 0) malformed(`${field} is negative.`); return value; diff --git a/tests/acceptance/pythonPycCli.test.ts b/tests/acceptance/pythonPycCli.test.ts index 98ff658553..352d1a9fb8 100644 --- a/tests/acceptance/pythonPycCli.test.ts +++ b/tests/acceptance/pythonPycCli.test.ts @@ -28,6 +28,137 @@ cliTest( }, ); +cliTest( + "classifies unsupported 3.7 alpha headers without labeling their marshal data malformed", + async ({ cli }) => { + const root = await createTestTempDirectory("rea-python-pyc-alpha-header-"); + const marshal = Buffer.from( + "e3000000000000000000000000010000004000000073040000006400530029014ea900720100000072010000007201000000fa066d6f642e7079da083c6d6f64756c653e010000007300000000", + "hex", + ); + expect(marshal).toHaveLength(77); + for (const magic of [3390, 3391]) { + const path = join(root, `${magic}.pyc`); + const header = Buffer.alloc(12); + header.writeUInt16LE(magic); + header[2] = 0x0d; + header[3] = 0x0a; + await writeFile(path, Buffer.concat([header, marshal])); + const result = await cli.run({ + arguments: ["inspect-python-pyc", path, "--json"], + }); + expect(result.exitCode).toBe(1); + expect(result.json).toMatchObject({ + code: "unsupported_target", + category: "unsupported_target", + }); + expect(JSON.stringify(result.json)).toContain( + "unsupported 12-byte CPython header", + ); + expect(result.json).not.toHaveProperty("normalized_result"); + } + + for (const magic of [3392, 3394]) { + const path = join(root, `${magic}.pyc`); + await writeFile(path, Buffer.concat([pythonPycHeader(magic), marshal])); + const result = await cli.run({ + arguments: ["inspect-python-pyc", path, "--json"], + }); + expect(result.exitCode, result.stderr).toBe(0); + const evidence = toolContract("inspect_python_pyc").outputSchema.parse( + result.json, + ); + expect(evidence.normalized_result.header.magic).toBe(magic); + expect(evidence.normalized_result.code_objects[0]?.bytecode.bytes).toBe( + 4, + ); + } + }, +); + +cliTest( + "rejects constructor-invalid signed code flags and preserves unknown positive flags", + async ({ cli }) => { + const root = await createTestTempDirectory("rea-python-pyc-flags-"); + for (const { name, magic, layout, flags, expected, valid } of [ + { + name: "zero", + magic: 3531, + layout: "py311", + flags: 0, + expected: 0, + valid: true, + }, + { + name: "unknown-positive", + magic: 3531, + layout: "py311", + flags: 0x40000000, + expected: 0x40000000, + valid: true, + }, + { + name: "negative-py311", + magic: 3531, + layout: "py311", + flags: 0x80000000, + expected: -0x80000000, + valid: false, + }, + { + name: "negative-plus-one-py311", + magic: 3531, + layout: "py311", + flags: 0x80000001, + expected: -0x7fffffff, + valid: false, + }, + { + name: "negative-py37", + magic: 3394, + layout: "py37", + flags: 0x80000000, + expected: -0x80000000, + valid: true, + }, + ] as const) { + const path = join(root, `${name}.pyc`); + await writeFile( + path, + Buffer.concat([pythonPycHeader(magic), marshalCode(layout, { flags })]), + ); + const result = await cli.run({ + arguments: ["inspect-python-pyc", path, "--json"], + }); + if (valid) { + expect(result.exitCode, result.stderr).toBe(0); + const evidence = toolContract("inspect_python_pyc").outputSchema.parse( + result.json, + ); + expect(evidence.normalized_result.code_objects[0]?.flags).toBe( + expected, + ); + } else { + expect(result.exitCode).toBe(1); + expect(result.json).toMatchObject({ + code: "invalid_request", + category: "invalid_input", + details: { + operation: "inspect_python_pyc", + issues: [ + { + reason: "invalid_format", + message: expect.stringContaining("co_flags is negative"), + }, + ], + }, + }); + expect(result.json).not.toHaveProperty("normalized_result"); + } + } + }, +); + cliTest( "preserves negative CPython first line numbers through the public CLI", async ({ cli }) => { diff --git a/tests/boundary/mcp/pythonPycMcp.test.ts b/tests/boundary/mcp/pythonPycMcp.test.ts index c8a70cb31c..4f41dae436 100644 --- a/tests/boundary/mcp/pythonPycMcp.test.ts +++ b/tests/boundary/mcp/pythonPycMcp.test.ts @@ -100,6 +100,67 @@ mcpTest("requires an absolute path for inspect_python_pyc", async ({ mcp }) => { } }); +mcpTest( + "advertised pyc output schema accepts signed legacy flags and positive unknown flags", + async ({ mcp }) => { + const { client, session } = await connect(mcp); + try { + const advertised = (await client.listTools()).tools.find( + (tool) => tool.name === "inspect_python_pyc", + ); + if (advertised?.outputSchema === undefined) + throw new Error("Missing inspect_python_pyc output schema"); + const outputSchema: Record = advertised.outputSchema; + const ajv = new Ajv2020({ strict: false, validateFormats: false }); + const root = await createTestTempDirectory("rea-python-pyc-mcp-flags-"); + for (const testCase of [ + { + name: "legacy-negative", + magic: 3394, + layout: "py37" as const, + rawFlags: 0x80000000, + expectedFlags: -0x80000000, + }, + { + name: "unknown-positive", + magic: 3531, + layout: "py311" as const, + rawFlags: 0x40000000, + expectedFlags: 0x40000000, + }, + ]) { + const path = join(root, `${testCase.name}.pyc`); + await writeFile( + path, + Buffer.concat([ + pythonPycHeader(testCase.magic), + marshalCode(testCase.layout, { flags: testCase.rawFlags }), + ]), + ); + const response = await client.callTool({ + name: "inspect_python_pyc", + arguments: { path }, + }); + expect(response.isError, JSON.stringify(response.content)).not.toBe( + true, + ); + const evidence = toolContract("inspect_python_pyc").outputSchema.parse( + response.structuredContent, + ); + expect( + ajv.validate(outputSchema, response.structuredContent), + JSON.stringify(ajv.errors), + ).toBe(true); + expect(evidence.normalized_result.code_objects[0]?.flags).toBe( + testCase.expectedFlags, + ); + } + } finally { + await session.close(); + } + }, +); + mcpTest("reports an unknown pyc magic as unsupported", async ({ mcp }) => { const { client, session } = await connect(mcp); try {