morluto/rea

Fork 0

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

[PR #6730] fix(pyinstaller): leave PE carrier binary members unmarked #7402

New issue

Open

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

bsakaguchi commented

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

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/6730
Author: @Sriram-PR
Created: 10/23/2026
Status: 🔄 Open

Base: main ← Head: fix/pyinstaller-binary-executable

📝 Commits (1)

494c721 fix(pyinstaller): leave PE carrier binary members unmarked

📊 Changes

3 files changed (+38 additions, -4 deletions)

View changed files

📝 docs/python-packaged-applications.md (+6 -3)
📝 src/artifacts/pyinstaller/CArchiveMembers.ts (+2 -1)
📝 src/artifacts/pyinstaller/PyInstallerArtifactReader.test.ts (+30 -0)

📄 Description

Summary

045c7dd6 (for #6702) marks every CArchive type b member executable: true. That matches PyInstaller's POSIX bootloader, but the rule is POSIX only: in bootloader/src/pyi_archive.c the fchmod(... S_IXUSR) for ARCHIVE_ITEM_BINARY sits inside #ifndef WIN32. A Windows PyInstaller executable (PE carrier) extracts b members, usually DLLs and .pyd extensions, with no execute marking, and REA's own Windows host marking only counts .bat/.cmd/.com/.exe. So on main, inventories of Windows PyInstaller apps report every bundled DLL as executable, and comparisons pick up deltas from that.

Follow-up to #6702 (Codex review on the first version of this PR).

Problem and expected behavior

Smallest trigger: a CArchive appended to a PE carrier with helper.dll stored as type b. Main reports executable: true. Expected: false for PE carriers, while ELF and Mach-O carriers keep b executable and x non-executable.

Change and scope

projectStoredMember only applies the b rule when the carrier path dialect is posix. It reuses the carrierPathDialect result from bd4f5e9c, which projectStoredMember already receives, instead of probing the carrier again (earlier versions of this PR read the MZ bytes themselves; that probe and its short-read test are gone).
A carrier whose dialect is unknown also stays unmarked, the same way that function refuses to infer POSIX path semantics without a carrier proof.
docs/python-packaged-applications.md now says the b rule applies to POSIX carriers and that b members of PE or unconfirmed carriers stay non-executable.
Not included: no change to extraction permissions or classification.

Contribution scope: ordinary fix within existing boundaries; no scope decision needed.

Contract and boundary impact

Semantic owner and earliest changed stage: projectStoredMember in src/artifacts/pyinstaller/CArchiveMembers.ts, using the carrier dialect owned by carrierPathDialect in PyInstallerArtifactReader.ts.
Contribution scope fit and linked maintainer decision: not applicable (follow-up fix to #6702).
CLI and MCP/tool-catalog contract: no schema change; executable on type b members of PE-carried (or unconfirmed) archives goes back to false.
Provider, bridge, target-format, or platform compatibility: Windows PyInstaller carriers.
Evidence, artifact, provenance, or reconstruction contract: the occurrence executable flag described above.
Target selection, authority, upstream behavior, and cleanup details: not applicable.
Generated metadata, package, or installation impact: none; docs:check passes.

Expected effects (check known effects; use documentation-only on its own).
Unchecked items do not establish absence; leave uncertainties visible.

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/artifacts/pyinstaller/PyInstallerArtifactReader.test.ts adds leaves binary members of a PE carrier unmarked next to the existing preserves the CArchive binary type as executable metadata (ELF side, from 5bcf71c6). It uses the fixture's existing carrier: "pe" option and checks that helper.dll (type b) is not executable.
Base reproduction: with main's source the new test fails on the PE case:
```
-   "executable": false,
+   "executable": true,
```
User-visible CLI/MCP output: b members of Windows PyInstaller executables report "executable": false again.
Remaining proof gaps: synthetic carriers only (the fixture's minimal MZ/PE header, not a real Windows PyInstaller build); bootloader behavior is from PyInstaller's source.

For evidence-bearing changes:

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:focused -- src/artifacts/pyinstaller/*.test.ts tests/acceptance/pyinstallerArchive.test.ts tests/acceptance/artifactExtractionCoverage.test.ts tests/boundary/providers/native/artifactProvider.capacity.test.ts: 198 passed (the new test fails on main's source)
npm run check:fast: typecheck and lint passed
npm run docs:check: passed

Compatibility, safety, and release

Breaking changes or migration steps: none.
Real Hopper/Ghidra, browser, or OS coverage: Linux x64, Node 24.18.0.
Package or release metadata impact: none.
Security, privacy, process, or containment review: none; no new reads.

Author readiness

Human owner: @Sriram-PR

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.

_{🔄 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/6730 **Author:** [@Sriram-PR](https://github.com/Sriram-PR) **Created:** 10/23/2026 **Status:** 🔄 Open **Base:** `main` ← **Head:** `fix/pyinstaller-binary-executable` --- ### 📝 Commits (1) - [`494c721`](https://github.com/morluto/rea/commit/494c72158ac09c4ef17b99cd91d20142d5fd56d9) fix(pyinstaller): leave PE carrier binary members unmarked ### 📊 Changes **3 files changed** (+38 additions, -4 deletions) <details> <summary>View changed files</summary> 📝 `docs/python-packaged-applications.md` (+6 -3) 📝 `src/artifacts/pyinstaller/CArchiveMembers.ts` (+2 -1) 📝 `src/artifacts/pyinstaller/PyInstallerArtifactReader.test.ts` (+30 -0) </details> ### 📄 Description ## Summary 045c7dd6 (for #6702) marks every CArchive type `b` member `executable: true`. That matches PyInstaller's POSIX bootloader, but the rule is POSIX only: in `bootloader/src/pyi_archive.c` the `fchmod(... S_IXUSR)` for `ARCHIVE_ITEM_BINARY` sits inside `#ifndef WIN32`. A Windows PyInstaller executable (PE carrier) extracts `b` members, usually DLLs and `.pyd` extensions, with no execute marking, and REA's own Windows host marking only counts `.bat/.cmd/.com/.exe`. So on main, inventories of Windows PyInstaller apps report every bundled DLL as executable, and comparisons pick up deltas from that. Follow-up to #6702 (Codex review on the first version of this PR). ## Problem and expected behavior Smallest trigger: a CArchive appended to a PE carrier with `helper.dll` stored as type `b`. Main reports `executable: true`. Expected: false for PE carriers, while ELF and Mach-O carriers keep `b` executable and `x` non-executable. ## Change and scope - `projectStoredMember` only applies the `b` rule when the carrier path dialect is `posix`. It reuses the `carrierPathDialect` result from bd4f5e9c, which `projectStoredMember` already receives, instead of probing the carrier again (earlier versions of this PR read the `MZ` bytes themselves; that probe and its short-read test are gone). - A carrier whose dialect is `unknown` also stays unmarked, the same way that function refuses to infer POSIX path semantics without a carrier proof. - `docs/python-packaged-applications.md` now says the `b` rule applies to POSIX carriers and that `b` members of PE or unconfirmed carriers stay non-executable. - Not included: no change to extraction permissions or classification. Contribution scope: ordinary fix within existing boundaries; no scope decision needed. ## Contract and boundary impact - Semantic owner and earliest changed stage: `projectStoredMember` in `src/artifacts/pyinstaller/CArchiveMembers.ts`, using the carrier dialect owned by `carrierPathDialect` in `PyInstallerArtifactReader.ts`. - Contribution scope fit and linked maintainer decision: not applicable (follow-up fix to #6702). - CLI and MCP/tool-catalog contract: no schema change; `executable` on type `b` members of PE-carried (or unconfirmed) archives goes back to false. - Provider, bridge, target-format, or platform compatibility: Windows PyInstaller carriers. - Evidence, artifact, provenance, or reconstruction contract: the occurrence `executable` flag described above. - Target selection, authority, upstream behavior, and cleanup details: not applicable. - Generated metadata, package, or installation impact: none; `docs:check` passes. Expected effects (check known effects; use documentation-only on its own). Unchecked items do not establish absence; leave uncertainties visible. - [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/artifacts/pyinstaller/PyInstallerArtifactReader.test.ts` adds `leaves binary members of a PE carrier unmarked` next to the existing `preserves the CArchive binary type as executable metadata` (ELF side, from 5bcf71c6). It uses the fixture's existing `carrier: "pe"` option and checks that `helper.dll` (type `b`) is not executable. - Base reproduction: with main's source the new test fails on the PE case: ``` - "executable": false, + "executable": true, ``` - User-visible CLI/MCP output: `b` members of Windows PyInstaller executables report `"executable": false` again. - Remaining proof gaps: synthetic carriers only (the fixture's minimal MZ/PE header, not a real Windows PyInstaller build); bootloader behavior is from PyInstaller's source. For evidence-bearing changes: - [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:focused -- src/artifacts/pyinstaller/*.test.ts tests/acceptance/pyinstallerArchive.test.ts tests/acceptance/artifactExtractionCoverage.test.ts tests/boundary/providers/native/artifactProvider.capacity.test.ts`: 198 passed (the new test fails on main's source) - `npm run check:fast`: typecheck and lint passed - `npm run docs:check`: passed ## Compatibility, safety, and release - Breaking changes or migration steps: none. - Real Hopper/Ghidra, browser, or OS coverage: Linux x64, Node 24.18.0. - Package or release metadata impact: none. - Security, privacy, process, or containment review: none; no new reads. ## Author readiness Human owner: @Sriram-PR - [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. - [x] Owning docs, contracts, and generated metadata are updated where needed. - [x] User-visible CLI/MCP changes include representative output. - [x] I checked the final diff for secrets, unrelated cleanup, and unsupported claims. --- <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:47:36 +00:00

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

No description provided.

Rows
Columns