[PR #6633] [CLOSED] fix(python): classify negative marshal string lengths as malformed input #7386

Closed
opened 2026-11-01 21:47:29 +00:00 by bsakaguchi · 0 comments
Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/6633
Author: @ezrabrilliant
Created: 10/23/2026
Status: ❌ Closed

Base: main ← Head: fix/pyc-negative-marshal-length


📝 Commits (1)

  • b666048 fix(python): classify negative marshal string lengths as malformed input

📊 Changes

2 files changed (+24 additions, -4 deletions)

View changed files

📝 src/python/PythonPycParser.test.ts (+17 -0)
📝 src/python/PythonPycParser.ts (+7 -4)

📄 Description

Summary

Fixes #6605. The public inspect-python-pyc command classified a bytes/string
constant whose marshal length is negative as a memory resource_constraint
instead of malformed input. CPython rejects the same body with
ValueError: bad marshal data (bytes object size out of range).

Problem and expected behavior

Smallest trigger: a CPython pyc whose constant is a bytes object with signed
marshal length -1. Observed: the length was read with an unsigned u32(), so
-1 wrapped to 4294967295 and the budget check reported
resource_constraint / memory:
Marshal string of 4294967295 bytes exceeds max_string_bytes 1048576.

Violated invariant: a structurally invalid (negative) marshal length is
malformed input (invalid_format), not a resource refusal. The existing
controls already distinguish these — a truncated positive length is
invalid_format, a fully present 2 MiB constant is resource_constraint — so a
negative length landing in the budget branch is the defect.

Change and scope

TYPE_STRING/bytes, TYPE_UNICODE, and TYPE_ASCII now read their length
through the existing nonNegativeInt32 boundary helper (CPython r_long), which
rejects a negative value as malformed before the max_string_bytes budget check,
mirroring CPython. This fixes the producing boundary (the length read) and reuses
the owning helper rather than repairing the misclassification downstream; the
helper's doc comment is generalized since it now guards lengths as well as code
counts. Oversized-but-positive and truncated controls are unchanged.

Intentionally out of scope: container counts (tuple/list/set/dict) still read with
u32(). They are a separate token class, not covered by this issue, and are left
for a focused change if they prove to share the pattern.

Contract and boundary impact

  • Semantic owner and earliest changed stage: MarshalReader in
    src/python/PythonPycParser.ts, at the marshal length read.

  • Contribution scope fit and linked maintainer decision: ordinary fix within an
    existing boundary; no maintainer scope decision required (not applicable).

  • CLI and MCP/tool-catalog contract: no schema change. A negative marshal length
    now yields invalid_request/invalid_format instead of
    resource_constraint/memory.

  • Provider, bridge, target-format, or platform compatibility: none.

  • Evidence, artifact, provenance, or reconstruction contract: none; restores
    correct failure classification.

  • Target selection, authority, upstream behavior, and cleanup details: not applicable.

  • Generated metadata, package, or installation impact: none.

  • Read existing files or captured data.

  • Launch or attach to a process.

  • Inject instrumentation or run scripts/code.

  • Change process memory, registers, files, or configuration.

  • Contact network services.

  • Recover protected code or work around protections.

  • Documentation or usability only; no target interaction.

  • Not sure yet; investigation or maintainer guidance needed.

Evidence and regression coverage

  • Tests added or updated: src/python/PythonPycParser.test.ts — new case
    "rejects negative marshal string and bytes lengths as malformed input" covering
    TYPE_STRING/bytes, TYPE_UNICODE, and TYPE_ASCII.

  • Base reproduction (executed): on the base revision the new test fails with
    PythonPycResourceFailure: "Marshal string of 4294967295 bytes exceeds max_string_bytes 1048576."; with the fix it raises PythonPycFormatFailure
    (... is negative).

  • User-visible CLI/MCP output: parser now raises PythonPycFormatFailure
    (invalid_format) rather than PythonPycResourceFailure
    (resource_constraint). The public CLI/service mapping of these failures is
    source-derived (PythonPycService); the full CLI was not executed.

  • Remaining proof gaps: no end-to-end CLI JSON captured; container-count paths not
    assessed at runtime.

  • Observed, derived, and inferred claims remain distinguishable.

  • Artifact identity, source provenance, and failed attempts remain preserved.

  • Unsupported, incomplete, unavailable, or uncertain outcomes remain visible.

Validation performed

  • npm run test:local -- src/python/PythonPycParser.test.ts src/application/python/PythonPycService.test.ts src/artifacts/pyinstaller/MarshalSubset.test.ts — 62 passed (incl. new regression).
  • npm run check:fast — typecheck + lint: 2 tasks successful.

Compatibility, safety, and release

  • Breaking changes or migration steps: none. A pyc that previously returned
    resource_constraint for a negative length now returns invalid_format; both
    are refusals of the same malformed input.
  • Real Hopper/Ghidra, browser, or OS coverage: not applicable.
  • Package or release metadata impact: none.
  • Security, privacy, process, or containment review: none; pure parser
    classification fix.

Author readiness

Human owner: @ezrabrilliant

  • I reviewed the final diff and relevant existing code, understand the change and its edge cases, and take responsibility for the complete PR, including AI-assisted work.
  • I checked the evidence above, confirmed it supports the current change, and recorded meaningful validation gaps.
  • I declare this PR ready for initial maintainer review.

Before merge

  • This PR addresses a concrete problem or an agreed enhancement.
  • Material scope and authority questions are resolved, and any required maintainer scope decision covers the final diff.
  • The PR has one focused outcome and the title follows type(scope): outcome.
  • Related issue is linked, or the reason for not linking one is stated above.
  • Tests cover changed observable behavior and meaningful failure paths.
  • Owning docs, contracts, and generated metadata are updated where needed.
  • User-visible CLI/MCP changes include representative output.
  • I checked the final diff for secrets, unrelated cleanup, and unsupported claims.

🤖 Generated with Claude Code

https://claude.ai/code/session_01VkKgC2HYpt7rG38FnoufT1


🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.

## 📋 Pull Request Information **Original PR:** https://github.com/morluto/rea/pull/6633 **Author:** [@ezrabrilliant](https://github.com/ezrabrilliant) **Created:** 10/23/2026 **Status:** ❌ Closed **Base:** `main` ← **Head:** `fix/pyc-negative-marshal-length` --- ### 📝 Commits (1) - [`b666048`](https://github.com/morluto/rea/commit/b66604848d375a7931ae90a7f477aae02351b960) fix(python): classify negative marshal string lengths as malformed input ### 📊 Changes **2 files changed** (+24 additions, -4 deletions) <details> <summary>View changed files</summary> 📝 `src/python/PythonPycParser.test.ts` (+17 -0) 📝 `src/python/PythonPycParser.ts` (+7 -4) </details> ### 📄 Description ## Summary Fixes #6605. The public `inspect-python-pyc` command classified a bytes/string constant whose marshal length is negative as a memory `resource_constraint` instead of malformed input. CPython rejects the same body with `ValueError: bad marshal data (bytes object size out of range)`. ## Problem and expected behavior Smallest trigger: a CPython pyc whose constant is a bytes object with signed marshal length `-1`. Observed: the length was read with an unsigned `u32()`, so `-1` wrapped to `4294967295` and the budget check reported `resource_constraint` / `memory`: `Marshal string of 4294967295 bytes exceeds max_string_bytes 1048576.` Violated invariant: a structurally invalid (negative) marshal length is malformed input (`invalid_format`), not a resource refusal. The existing controls already distinguish these — a truncated positive length is `invalid_format`, a fully present 2 MiB constant is `resource_constraint` — so a negative length landing in the budget branch is the defect. ## Change and scope `TYPE_STRING`/bytes, `TYPE_UNICODE`, and `TYPE_ASCII` now read their length through the existing `nonNegativeInt32` boundary helper (CPython `r_long`), which rejects a negative value as malformed before the `max_string_bytes` budget check, mirroring CPython. This fixes the producing boundary (the length read) and reuses the owning helper rather than repairing the misclassification downstream; the helper's doc comment is generalized since it now guards lengths as well as code counts. Oversized-but-positive and truncated controls are unchanged. Intentionally out of scope: container counts (tuple/list/set/dict) still read with `u32()`. They are a separate token class, not covered by this issue, and are left for a focused change if they prove to share the pattern. ## Contract and boundary impact - Semantic owner and earliest changed stage: `MarshalReader` in `src/python/PythonPycParser.ts`, at the marshal length read. - Contribution scope fit and linked maintainer decision: ordinary fix within an existing boundary; no maintainer scope decision required (not applicable). - CLI and MCP/tool-catalog contract: no schema change. A negative marshal length now yields `invalid_request`/`invalid_format` instead of `resource_constraint`/`memory`. - Provider, bridge, target-format, or platform compatibility: none. - Evidence, artifact, provenance, or reconstruction contract: none; restores correct failure classification. - Target selection, authority, upstream behavior, and cleanup details: not applicable. - Generated metadata, package, or installation impact: none. - [x] Read existing files or captured data. - [ ] Launch or attach to a process. - [ ] Inject instrumentation or run scripts/code. - [ ] Change process memory, registers, files, or configuration. - [ ] Contact network services. - [ ] Recover protected code or work around protections. - [ ] Documentation or usability only; no target interaction. - [ ] Not sure yet; investigation or maintainer guidance needed. ## Evidence and regression coverage - Tests added or updated: `src/python/PythonPycParser.test.ts` — new case "rejects negative marshal string and bytes lengths as malformed input" covering `TYPE_STRING`/bytes, `TYPE_UNICODE`, and `TYPE_ASCII`. - Base reproduction (executed): on the base revision the new test fails with `PythonPycResourceFailure: "Marshal string of 4294967295 bytes exceeds max_string_bytes 1048576."`; with the fix it raises `PythonPycFormatFailure` (`... is negative`). - User-visible CLI/MCP output: parser now raises `PythonPycFormatFailure` (`invalid_format`) rather than `PythonPycResourceFailure` (`resource_constraint`). The public CLI/service mapping of these failures is source-derived (`PythonPycService`); the full CLI was not executed. - Remaining proof gaps: no end-to-end CLI JSON captured; container-count paths not assessed at runtime. - [x] Observed, derived, and inferred claims remain distinguishable. - [x] Artifact identity, source provenance, and failed attempts remain preserved. - [x] Unsupported, incomplete, unavailable, or uncertain outcomes remain visible. ## Validation performed - `npm run test:local -- src/python/PythonPycParser.test.ts src/application/python/PythonPycService.test.ts src/artifacts/pyinstaller/MarshalSubset.test.ts` — 62 passed (incl. new regression). - `npm run check:fast` — typecheck + lint: 2 tasks successful. ## Compatibility, safety, and release - Breaking changes or migration steps: none. A pyc that previously returned `resource_constraint` for a negative length now returns `invalid_format`; both are refusals of the same malformed input. - Real Hopper/Ghidra, browser, or OS coverage: not applicable. - Package or release metadata impact: none. - Security, privacy, process, or containment review: none; pure parser classification fix. ## Author readiness Human owner: @ezrabrilliant - [x] I reviewed the final diff and relevant existing code, understand the change and its edge cases, and take responsibility for the complete PR, including AI-assisted work. - [x] I checked the evidence above, confirmed it supports the current change, and recorded meaningful validation gaps. - [x] I declare this PR ready for initial maintainer review. ## Before merge - [x] This PR addresses a concrete problem or an agreed enhancement. - [x] Material scope and authority questions are resolved, and any required maintainer scope decision covers the final diff. - [x] The PR has one focused outcome and the title follows `type(scope): outcome`. - [x] Related issue is linked, or the reason for not linking one is stated above. - [x] Tests cover changed observable behavior and meaningful failure paths. - [ ] Owning docs, contracts, and generated metadata are updated where needed. - [ ] User-visible CLI/MCP changes include representative output. - [x] I checked the final diff for secrets, unrelated cleanup, and unsupported claims. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01VkKgC2HYpt7rG38FnoufT1 --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>
bsakaguchi 2026-11-01 21:47:29 +00:00
Sign in to join this conversation.
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
morluto/rea#7386
No description provided.