[PR #6949] fix(pyinstaller): require NUL-padded CArchive TOC names #7469

New issue

Open

opened 2026-11-01 21:48:03 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-11-01 21:48:03 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/6949
Author: @rudycelekli
Created: 10/24/2026
Status: 🔄 Open

Base: main ← Head: fix/pyinstaller-toc-name-framing

📝 Commits (5)

281ecb9 fix(pyinstaller): require NUL-padded CArchive TOC names
6315905 fix(pyinstaller): integrate TOC padding validation with current main
f90dafc fix(pyinstaller): refresh current main integration
cdf8978 fix(pyinstaller): pair valid TOC padding boundary and refresh main
9a5b20e test(pyinstaller): assert framing error provenance

📊 Changes

14 files changed (+90 additions, -33 deletions)

View changed files

📝 src/android/JadxAnalysis.ts (+1 -1)
📝 src/android/JadxSession.ts (+3 -4)
📝 src/artifacts/pyinstaller/CArchiveLayout.ts (+5 -1)
📝 src/artifacts/pyinstaller/PyInstallerFormatBoundaries.test.ts (+56 -0)
📝 src/browser/PlaywrightScenarioEvents.ts (+3 -5)
📝 src/native/android/AndroidJniLibrary.ts (+1 -1)
📝 src/setupEnvironment.test.ts (+5 -5)
📝 tests/boundary/android/androidJniLibraries.test.ts (+1 -1)
📝 tests/boundary/cli/cliOutput.test.ts (+4 -2)
📝 tests/boundary/filesystem/clientConfigurationDialects.test.ts (+4 -2)
📝 tests/boundary/filesystem/setupLauncherAlias.test.ts (+1 -1)
📝 tests/boundary/mcp/promptMcp.test.ts (+4 -6)
📝 tests/support/analysis-efficiency/android-jni.mjs (+1 -2)
📝 tests/support/analysis-efficiency/packet-entry.mjs (+1 -2)

📄 Description

Summary

Reject PyInstaller CArchive TOC names without a NUL terminator or with nonzero bytes after it. Keep valid zero-padded names accepted. No linked issue; this corrects existing read-only format admission.

Problem and expected behavior

Malformed name fields previously became successful inventory entries. CArchiveLayout validates the complete name field before decoding its prefix; its shared readers consume that result.

Change and scope

The review follow-up adds the explicit accepted boundary beside the rejection cases: a 32-byte TOC record with member, a NUL terminator and zero padding must decode to that name. Rejection assertions check the format kind and table index/byte offset without freezing the explanatory sentence. No second parser or downstream workaround was introduced.

Merged main 64196b08cab9fab15649b6d0058f980cd3f733f6 using normal append-only history, resolving obsolete import conflicts against upstream. Normal pre-commit hooks also normalized imports and equivalent expressions in twelve upstream files; those mechanical changes remain visible in the diff.

Contract and boundary impact

Owner: CArchiveLayout, before names become inventory facts.
CLI/MCP schemas, source identity and Evidence provenance remain unchanged.
No new provider, target execution, injection, network or mutation capability.
No package, dependency, generated metadata or release change authored by this fix.

Evidence and regression coverage

At 9a5b20e2f9a2c9d884cdcc0bd7695b31c608bfc9, Node24.22.1/npm11.16.0 on macOS arm64:

Replaced only the producer with exact current-main code, keeping the tests fixed: 2 failed / 72 passed. Restored the candidate afterward.
Parser format boundaries and artifact-reader suites: 114 passed, including the new paired accepted record.
Actual in-process MCP boundary suite: 10 passed. The new malformed-name cases remain parser-level regressions; this does not claim malformed-name MCP end-to-end coverage.
Explanation-only intervention: all 74 format-boundary tests pass after changing the parser error sentence; candidate restored afterward.
Normal pre-commit hooks, git diff --check, and normal pre-push check:fast passed.
Full npm run check: five tasks passed; Knip failed on unused HexRecordRegionMetadata. A separate execution with every tracked file matching exact main reproduced the same sole error. Existing draft #7718 contains that main repair. Chained architecture/production-knip/typed-lint checks were not reached. This is not a full-check pass.
An earlier full-check attempt was stopped by the disk-reserve guard; its log is retained separately.

Compatibility, safety, and release

Synthetic CArchive fixtures and actual local MCP workflows were exercised. No external PyInstaller build, disassembler, Windows runtime or full source suite was run. No release or merge was performed.

Author readiness

Human owner: @rudycelekli

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

Resolve the upstream full-check gate and confirm final required CI.
Human owner completes the readiness declarations.

Prepared with OpenAI Codex assistance. The PR remains draft while required human readiness and qualification are outstanding.

_{🔄 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/6949 **Author:** [@rudycelekli](https://github.com/rudycelekli) **Created:** 10/24/2026 **Status:** 🔄 Open **Base:** `main` ← **Head:** `fix/pyinstaller-toc-name-framing` --- ### 📝 Commits (5) - [`281ecb9`](https://github.com/morluto/rea/commit/281ecb984e39c9e2c1cafe1ae9c6e037cd7e4b79) fix(pyinstaller): require NUL-padded CArchive TOC names - [`6315905`](https://github.com/morluto/rea/commit/63159055229e053642f11a3d071bd9ce79aff7f3) fix(pyinstaller): integrate TOC padding validation with current main - [`f90dafc`](https://github.com/morluto/rea/commit/f90dafceb47704cb5ae1b2816811e5ca34b68a13) fix(pyinstaller): refresh current main integration - [`cdf8978`](https://github.com/morluto/rea/commit/cdf89786931e97e11b811fbdf3b9749ab2abb45a) fix(pyinstaller): pair valid TOC padding boundary and refresh main - [`9a5b20e`](https://github.com/morluto/rea/commit/9a5b20e2f9a2c9d884cdcc0bd7695b31c608bfc9) test(pyinstaller): assert framing error provenance ### 📊 Changes **14 files changed** (+90 additions, -33 deletions) <details> <summary>View changed files</summary> 📝 `src/android/JadxAnalysis.ts` (+1 -1) 📝 `src/android/JadxSession.ts` (+3 -4) 📝 `src/artifacts/pyinstaller/CArchiveLayout.ts` (+5 -1) 📝 `src/artifacts/pyinstaller/PyInstallerFormatBoundaries.test.ts` (+56 -0) 📝 `src/browser/PlaywrightScenarioEvents.ts` (+3 -5) 📝 `src/native/android/AndroidJniLibrary.ts` (+1 -1) 📝 `src/setupEnvironment.test.ts` (+5 -5) 📝 `tests/boundary/android/androidJniLibraries.test.ts` (+1 -1) 📝 `tests/boundary/cli/cliOutput.test.ts` (+4 -2) 📝 `tests/boundary/filesystem/clientConfigurationDialects.test.ts` (+4 -2) 📝 `tests/boundary/filesystem/setupLauncherAlias.test.ts` (+1 -1) 📝 `tests/boundary/mcp/promptMcp.test.ts` (+4 -6) 📝 `tests/support/analysis-efficiency/android-jni.mjs` (+1 -2) 📝 `tests/support/analysis-efficiency/packet-entry.mjs` (+1 -2) </details> ### 📄 Description ## Summary Reject PyInstaller CArchive TOC names without a NUL terminator or with nonzero bytes after it. Keep valid zero-padded names accepted. No linked issue; this corrects existing read-only format admission. ## Problem and expected behavior Malformed name fields previously became successful inventory entries. CArchiveLayout validates the complete name field before decoding its prefix; its shared readers consume that result. ## Change and scope The review follow-up adds the explicit accepted boundary beside the rejection cases: a 32-byte TOC record with `member`, a NUL terminator and zero padding must decode to that name. Rejection assertions check the format kind and table index/byte offset without freezing the explanatory sentence. No second parser or downstream workaround was introduced. Merged main `64196b08cab9fab15649b6d0058f980cd3f733f6` using normal append-only history, resolving obsolete import conflicts against upstream. Normal pre-commit hooks also normalized imports and equivalent expressions in twelve upstream files; those mechanical changes remain visible in the diff. ## Contract and boundary impact - Owner: CArchiveLayout, before names become inventory facts. - CLI/MCP schemas, source identity and Evidence provenance remain unchanged. - No new provider, target execution, injection, network or mutation capability. - No package, dependency, generated metadata or release change authored by this fix. ## Evidence and regression coverage At `9a5b20e2f9a2c9d884cdcc0bd7695b31c608bfc9`, Node24.22.1/npm11.16.0 on macOS arm64: - Replaced only the producer with exact current-main code, keeping the tests fixed: 2 failed / 72 passed. Restored the candidate afterward. - Parser format boundaries and artifact-reader suites: 114 passed, including the new paired accepted record. - Actual in-process MCP boundary suite: 10 passed. The new malformed-name cases remain parser-level regressions; this does not claim malformed-name MCP end-to-end coverage. - Explanation-only intervention: all 74 format-boundary tests pass after changing the parser error sentence; candidate restored afterward. - Normal pre-commit hooks, `git diff --check`, and normal pre-push `check:fast` passed. - Full `npm run check`: five tasks passed; Knip failed on unused `HexRecordRegionMetadata`. A separate execution with every tracked file matching exact main reproduced the same sole error. Existing draft #7718 contains that main repair. Chained architecture/production-knip/typed-lint checks were not reached. This is not a full-check pass. - An earlier full-check attempt was stopped by the disk-reserve guard; its log is retained separately. ## Compatibility, safety, and release Synthetic CArchive fixtures and actual local MCP workflows were exercised. No external PyInstaller build, disassembler, Windows runtime or full source suite was run. No release or merge was performed. ## Author readiness Human owner: @rudycelekli - [ ] 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 - [ ] Resolve the upstream full-check gate and confirm final required CI. - [ ] Human owner completes the readiness declarations. Prepared with OpenAI Codex assistance. The PR remains draft while required human readiness and qualification are outstanding. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi added the

pull-request

label

2026-11-01 21:48:03 +00:00

bsakaguchi referenced this issue

2026-11-05 01:48:11 +00:00

[PR #7469] [MERGED] fix(mcp): summarize copied browser observations in input declarations #7857