morluto/rea

Fork 0

mirror of https://github.com/morluto/rea.git synced 2026-11-05 06:45:22 +00:00

[PR #6628] [CLOSED] fix(python): classify negative marshal string sizes as malformed #7383

New issue

Closed

opened 2026-11-01 21:47:27 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-11-01 21:47:27 +00:00

Owner

📋 Pull Request Information

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

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

📝 Commits (2)

5f98ed9 fix(python): classify negative marshal string sizes as malformed
da3669a Merge remote-tracking branch 'origin/main' into fix/python-negative-marshal-length

📊 Changes

3 files changed (+103 additions, -3 deletions)

View changed files

📝 src/python/PythonPycParser.test.ts (+11 -0)
📝 src/python/PythonPycParser.ts (+4 -3)
📝 tests/acceptance/pythonPycCli.test.ts (+88 -0)

📄 Description

Summary

Fixes #6605. Signed-negative marshal string lengths now produce malformed-input errors instead of memory-resource refusals.

Problem and expected behavior

A bytes constant declaring length -1 was interpreted as 4294967295 bytes. The public CLI returned resource_constraint and misleading memory recovery advice for a corrupt file. Recognized negative lengths should return invalid_request / invalid_input with an invalid_format issue; valid strings exceeding the caller's budget must retain their resource refusal.

Change and scope

Read four-byte bytes/Unicode/ASCII string lengths as signed fields and reject negative sizes in the shared blob reader before resource checks or slicing. All five corresponding marshal tags use the same owning boundary. Short-string one-byte lengths and positive-budget checks remain intact. This is an ordinary static-format bug fix; a new maintainer scope decision is not applicable.

Contract and boundary impact

Semantic owner: marshal string-length interpretation in PythonPycParser.
CLI/MCP contracts: existing typed error family now reflects malformed input; no schema/catalog change.
Provider/format/platform: existing standalone pyc inspector; no deep provider or target execution.
Evidence/provenance: malformed artifacts remain refused without successful Evidence; positive controls retain complete results.
Authority/cleanup: existing local-file inspection; tests own temporary fixtures through existing cleanup.
Generated/package/installation impact: none.

Expected effects:

Read existing files or captured data.

Evidence and regression coverage

Executed base reproduction: five parameterized parser cases and one public CLI regression failed (6 failed, 51 passed); the CLI reported a 4294967295-byte resource request for -1.

The repaired public CLI test checks negative length at default and maximum string budgets, truncated positive input, a valid small constant, and a fully present 2 MiB constant at default and sufficient budgets. The negative/truncated cases return invalid_request / invalid_input / invalid_format; the 2 MiB case retains resource_constraint at the default budget and completes at 2097152. Parser cases cover -1 and INT32_MIN across bytes, interned Unicode, Unicode, ASCII and interned ASCII tags.

Supplementary CPython 3.13.5 marshal decoding independently rejects all ten negative primitive strings with ValueError and accepts all five three-byte controls. This is primitive decoding evidence, not a matching 3.11 full-code-object runtime test. CPython's format reader also checks signed-negative sizes before reading string bytes: https://github.com/python/cpython/blob/v3.13.9/Python/marshal.c#L1074-L1150

Observed, derived, and inferred claims remain distinguishable.
Source provenance and failed outcomes remain preserved.
Unsupported, incomplete and resource-limited outcomes remain distinct.

Validation performed

npm run test:focused -- tests/acceptance/pythonPycCli.test.ts src/python/PythonPycParser.test.ts src/application/python/PythonPycService.test.ts tests/boundary/mcp/pythonPycMcp.test.ts tests/boundary/filesystem/pythonPyc.test.ts — 68 tests passed across five files, including six CLI calls in the new regression.
npm run check — six tasks passed; architecture guards passed across 1898 source files. Existing non-blocking lint warnings remain.
CPython primitive oracle — ten negative rejections and five valid controls passed.
git diff --check — passed.

Compatibility, safety, and release

No breaking schema, migration or release metadata changes. Verified on macOS arm64 using synthetic 3.11-layout pycs; matching CPython 3.11 was unavailable. CPython 3.13.5 was used only for supplementary primitive decoding. Linux/Windows and the new error through MCP transport were not replayed; existing MCP tests passed. No full repository suite rerun, target execution or deep-analysis provider run.

CI follow-up and overlapping contribution

Synced main at 5f702dd6c, including cc8835c7a (#6570), which fixes the Linux Hopper process fixtures responsible for both failures in CI attempt 2. The failed attempt is retained: shard 2 failed in classifies a reparented alternate launcher without signaling it and rejects a reparented alternate launcher without signaling it; shard 3 passed, six other shards were cancelled, and optional lanes were skipped. No claim is made that the failed run was green or that cancellation is a failure. Initial-attempt failures also remain visible in the run history.

The new PR diff remains three pyc files; no process-test workaround or CI setting is added. Fresh Linux verification is pending the new CI run because this workstation is macOS.

Overlapping PR #6633 fixes the same issue using the existing non-negative field helper. This PR retains broader committed evidence (all five tags, INT32_MIN and public CLI format/resource controls). These are alternative fixes; both should not be merged unchanged. Maintainer selection/coordination remains open; no duplicate PR was closed by this contribution.

Updated verification on Node 24.22.1: five pyc files passed 75 tests; two Linux-only files skipped 16 tests on macOS. npm run check passed all six tasks and architecture guards across 1903 source files. Full repository suite was not rerun locally.

Author readiness

Human owner: @xiajiahang

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

Maintainer review and merge checks pending.

_{🔄 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/6628 **Author:** [@xiajiahang](https://github.com/xiajiahang) **Created:** 10/23/2026 **Status:** ❌ Closed **Base:** `main` ← **Head:** `fix/python-negative-marshal-length` --- ### 📝 Commits (2) - [`5f98ed9`](https://github.com/morluto/rea/commit/5f98ed922211e0814c088379d2ec4f5321a95025) fix(python): classify negative marshal string sizes as malformed - [`da3669a`](https://github.com/morluto/rea/commit/da3669a72eca1c70f9de24324e7dced07891236f) Merge remote-tracking branch 'origin/main' into fix/python-negative-marshal-length ### 📊 Changes **3 files changed** (+103 additions, -3 deletions) <details> <summary>View changed files</summary> 📝 `src/python/PythonPycParser.test.ts` (+11 -0) 📝 `src/python/PythonPycParser.ts` (+4 -3) 📝 `tests/acceptance/pythonPycCli.test.ts` (+88 -0) </details> ### 📄 Description ## Summary Fixes #6605. Signed-negative marshal string lengths now produce malformed-input errors instead of memory-resource refusals. ## Problem and expected behavior A bytes constant declaring length -1 was interpreted as 4294967295 bytes. The public CLI returned resource_constraint and misleading memory recovery advice for a corrupt file. Recognized negative lengths should return invalid_request / invalid_input with an invalid_format issue; valid strings exceeding the caller's budget must retain their resource refusal. ## Change and scope Read four-byte bytes/Unicode/ASCII string lengths as signed fields and reject negative sizes in the shared blob reader before resource checks or slicing. All five corresponding marshal tags use the same owning boundary. Short-string one-byte lengths and positive-budget checks remain intact. This is an ordinary static-format bug fix; a new maintainer scope decision is not applicable. ## Contract and boundary impact - Semantic owner: marshal string-length interpretation in PythonPycParser. - CLI/MCP contracts: existing typed error family now reflects malformed input; no schema/catalog change. - Provider/format/platform: existing standalone pyc inspector; no deep provider or target execution. - Evidence/provenance: malformed artifacts remain refused without successful Evidence; positive controls retain complete results. - Authority/cleanup: existing local-file inspection; tests own temporary fixtures through existing cleanup. - Generated/package/installation impact: none. Expected effects: - [x] Read existing files or captured data. ## Evidence and regression coverage Executed base reproduction: five parameterized parser cases and one public CLI regression failed (6 failed, 51 passed); the CLI reported a 4294967295-byte resource request for -1. The repaired public CLI test checks negative length at default and maximum string budgets, truncated positive input, a valid small constant, and a fully present 2 MiB constant at default and sufficient budgets. The negative/truncated cases return invalid_request / invalid_input / invalid_format; the 2 MiB case retains resource_constraint at the default budget and completes at 2097152. Parser cases cover -1 and INT32_MIN across bytes, interned Unicode, Unicode, ASCII and interned ASCII tags. Supplementary CPython 3.13.5 marshal decoding independently rejects all ten negative primitive strings with ValueError and accepts all five three-byte controls. This is primitive decoding evidence, not a matching 3.11 full-code-object runtime test. CPython's format reader also checks signed-negative sizes before reading string bytes: https://github.com/python/cpython/blob/v3.13.9/Python/marshal.c#L1074-L1150 - [x] Observed, derived, and inferred claims remain distinguishable. - [x] Source provenance and failed outcomes remain preserved. - [x] Unsupported, incomplete and resource-limited outcomes remain distinct. ## Validation performed - `npm run test:focused -- tests/acceptance/pythonPycCli.test.ts src/python/PythonPycParser.test.ts src/application/python/PythonPycService.test.ts tests/boundary/mcp/pythonPycMcp.test.ts tests/boundary/filesystem/pythonPyc.test.ts` — 68 tests passed across five files, including six CLI calls in the new regression. - `npm run check` — six tasks passed; architecture guards passed across 1898 source files. Existing non-blocking lint warnings remain. - CPython primitive oracle — ten negative rejections and five valid controls passed. - `git diff --check` — passed. ## Compatibility, safety, and release No breaking schema, migration or release metadata changes. Verified on macOS arm64 using synthetic 3.11-layout pycs; matching CPython 3.11 was unavailable. CPython 3.13.5 was used only for supplementary primitive decoding. Linux/Windows and the new error through MCP transport were not replayed; existing MCP tests passed. No full repository suite rerun, target execution or deep-analysis provider run. ## CI follow-up and overlapping contribution Synced main at 5f702dd6c, including cc8835c7a (#6570), which fixes the Linux Hopper process fixtures responsible for both failures in CI attempt 2. The failed attempt is retained: shard 2 failed in `classifies a reparented alternate launcher without signaling it` and `rejects a reparented alternate launcher without signaling it`; shard 3 passed, six other shards were cancelled, and optional lanes were skipped. No claim is made that the failed run was green or that cancellation is a failure. Initial-attempt failures also remain visible in the run history. The new PR diff remains three pyc files; no process-test workaround or CI setting is added. Fresh Linux verification is pending the new CI run because this workstation is macOS. Overlapping PR #6633 fixes the same issue using the existing non-negative field helper. This PR retains broader committed evidence (all five tags, INT32_MIN and public CLI format/resource controls). These are alternative fixes; both should not be merged unchanged. Maintainer selection/coordination remains open; no duplicate PR was closed by this contribution. Updated verification on Node 24.22.1: five pyc files passed 75 tests; two Linux-only files skipped 16 tests on macOS. `npm run check` passed all six tasks and architecture guards across 1903 source files. Full repository suite was not rerun locally. ## Author readiness Human owner: @xiajiahang - [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 - [ ] Maintainer review and merge checks pending. --- <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:27 +00:00

closed this issue
added the
pull-request
label

bsakaguchi referenced this issue

2026-11-03 00:29:32 +00:00

[GH-ISSUE #5653] [Tracker] New issue intake by area (October 2026) #3423

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#7383

No description provided.

Rows
Columns