[PR #2605] [MERGED] fix(native): keep lipo path echoes out of FAT records #5289

Closed
opened 2026-10-31 01:05:28 +00:00 by bsakaguchi · 0 comments
Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/2605
Author: @FengCi0
Created: 10/13/2026
Status: ✅ Merged
Merged: 10/13/2026
Merged by: @morluto

Base: main ← Head: fix/lipo-fat-path-framing


📝 Commits (4)

  • 552edbf fix(native): keep lipo path echoes out of FAT records
  • 013927f fix(native): use parsed lipo framing for container offsets
  • fc955d4 test(native): use explicit FAT fixtures for CLI regressions
  • 781e425 fix(native): resolve upstream lipo merge conflicts

📊 Changes

10 files changed (+347 additions, -106 deletions)

View changed files

📝 src/artifacts/MachOSliceArtifactReader.ts (+2 -2)
📝 src/native/NativeMacOSProvider.ts (+5 -2)
📝 src/native/NativeMachoInspection.ts (+7 -7)
📝 src/native/parsers/lipo.test.ts (+105 -10)
📝 src/native/parsers/lipo.ts (+38 -17)
📝 tests/boundary/cli/nativeMachoEchoedPathCli.test.ts (+68 -23)
📝 tests/boundary/providers/native/machoSliceReader.test.ts (+53 -1)
📝 tests/boundary/providers/native/nativeProvider.test.ts (+13 -10)
📝 tests/boundary/providers/native/nativeSelectedArchitecture.test.ts (+44 -28)
📝 tests/boundary/providers/native/nativeTextRobustness.test.ts (+12 -6)

📄 Description

Summary

I fixed FAT Mach-O filenames being parsed as architecture records. A filename containing a newline followed by architecture forged made REA report a third slice for a two-slice file, or reject the artifact because the lipo count disagreed with its FAT table.

I found this while checking native parsing and reproduced the producer behavior with system lipo and a generated two-slice fixture. No separate issue is linked for this focused correction. I checked related work: #2040 covers slice selection, and #2107 covers codesign path echoes; neither isolates lipo's FAT operand header.

Problem and expected behavior

lipo -detailed_info prints Fat header in: <path> with the filename unchanged. Splitting that header into lines lets filename text become slice metadata. Only the actual tool records should contribute architectures, thin/FAT framing and ranges. A filename containing Non-fat file: also caused the inspection consumer to use a zero container offset and project segment/entrypoint evidence onto the wrong bytes.

Change and scope

The lipo parser receives the exact operand and removes only a complete matching FAT header ending in LF or CRLF. Native inspection, architecture listing and the slice reader pass their own lipo operand. The parser returns the thin/FAT framing together with architectures. Inspection uses that fact for its container offset rather than rescanning the raw capture. Thin and headerless captures retain their existing behavior, including single-slice FAT and unknown offsets. The CLI regressions share their driver rather than duplicate it.

Contract and boundary impact

  • Semantic owner: parseLipoOutput, before record parsing; consumers reuse its framing and architecture facts.
  • Scope decision: not applicable; this corrects existing static metadata parsing.
  • CLI/MCP/catalog: no command or schema changes. Incorrect filename-derived slices disappear.
  • Compatibility: native macOS workflows; existing thin/FAT formats and unknown CPU text are preserved.
  • Evidence/provenance: raw captures, artifact bytes and identities are preserved; only derived records change.
  • Authority/cleanup: no added native commands or target execution. Temporary test resources use existing awaited cleanup.
  • Generated metadata, package and installation: no changes.

Expected effects of the correction:

  • Read existing files or captured data.

Existing native tool subprocesses are unchanged. The target samples are not executed; this adds no instrumentation, target mutation, network access or protection recovery.

Evidence and regression coverage

  • 11 parser cases cover embedded architecture/header tokens, multiline and CRLF paths, literal regex characters, common prefixes and compatibility.
  • A generated FAT fixture passes through the real system lipo and slice reader, retaining exactly the two declared ranges.
  • Real compiled inspect-macho and list-architectures CLI calls return the same facts for ordinary and multiline names, including a forged Non-fat file: line. Each CLI case compiles the existing C fixture for arm64 and x86_64, then checks multiple slices and a positive selected container offset before comparing filenames. This removes reliance on the host's /usr/bin/true being FAT.
  • LF/CRLF provider cases preserve the selected FAT slice offset and container-relative segment/entrypoint evidence; thin, single-slice FAT and unknown-offset controls remain covered.
  • Restoring the base parser makes five architecture regressions fail. Before the framing follow-up, both provider cases and the real CLI case fail with an incorrect zero container offset (the CLI observed 0 instead of 49152).
  • Observations, derived records and unknowns remain distinct.
  • Artifact identity and original provenance are preserved.
  • Existing incomplete and unavailable outcomes remain visible.

Validation performed

With pinned Node 24.18.0 and npm 11.16.0 on macOS arm64:

  • npm run test:focused -- src/native/parsers/lipo.test.ts tests/boundary/providers/native/nativeTextRobustness.test.ts tests/boundary/providers/native/nativeProvider.test.ts tests/boundary/providers/native/machoSliceReader.test.ts tests/boundary/cli/nativeMachoEchoedPathCli.test.ts tests/boundary/providers/native/artifactProvider.machOSlices.test.ts tests/boundary/providers/native/nativeSelectedArchitecture.test.ts — 88 tests passed across seven files before the CLI fixture follow-up.
  • npm run test:focused -- tests/boundary/cli/nativeMachoEchoedPathCli.test.ts — all four CLI cases passed with the generated universal fixture.
  • Negative fixture experiment: compiling only arm64 makes the FAT precondition fail (1 architecture is not greater than 1), confirming thin fixtures cannot silently pass.
  • npm run check — passed, including typecheck, runtime compilation, lint, format, knip, metadata and architecture checks.
  • npm run verify:test-discovery — all 1023 files discovered once.
  • git diff --check — passed.

Compatibility, safety, and release

No breaking changes or migration. Real Apple metadata tools and compiled CLI coverage ran on macOS; Hopper/Ghidra and other host workflows were not run. No package, release metadata or execution-authority changes. Full deterministic coverage remains with CI.

Before merge

  • Concrete problem and one focused outcome.
  • Existing contribution boundaries preserved; no new provider or execution capability.
  • Conventional title and reason for no separate issue stated.
  • Regression and compatibility coverage included.
  • No owning schema or generated metadata change needed.
  • Representative CLI behavior recorded above.
  • Final diff checked for unrelated changes, secrets and unsupported claims.

Summary by CodeRabbit

  • Bug Fixes

    • Improved macOS architecture and slice detection for filenames containing text that resembles Mach-O metadata.
    • Prevented echoed filename text from being interpreted as architecture records, load commands, or symbols.
    • Improved slice offset reporting for thin and universal binaries across supported lipo output formats.
  • Tests

    • Expanded coverage for metadata-like filenames, including newline characters, and for thin and universal binary output formats.

🔄 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/2605 **Author:** [@FengCi0](https://github.com/FengCi0) **Created:** 10/13/2026 **Status:** ✅ Merged **Merged:** 10/13/2026 **Merged by:** [@morluto](https://github.com/morluto) **Base:** `main` ← **Head:** `fix/lipo-fat-path-framing` --- ### 📝 Commits (4) - [`552edbf`](https://github.com/morluto/rea/commit/552edbfea3594aa107f5ea69a90e77d7d37dce19) fix(native): keep lipo path echoes out of FAT records - [`013927f`](https://github.com/morluto/rea/commit/013927ff5d7bbeaf0e67847fb1d6237d75edce57) fix(native): use parsed lipo framing for container offsets - [`fc955d4`](https://github.com/morluto/rea/commit/fc955d42e19450e95f926d8d4bdcbb78c292377e) test(native): use explicit FAT fixtures for CLI regressions - [`781e425`](https://github.com/morluto/rea/commit/781e425081fc821d4ba2a43918f81d23d9b2a659) fix(native): resolve upstream lipo merge conflicts ### 📊 Changes **10 files changed** (+347 additions, -106 deletions) <details> <summary>View changed files</summary> 📝 `src/artifacts/MachOSliceArtifactReader.ts` (+2 -2) 📝 `src/native/NativeMacOSProvider.ts` (+5 -2) 📝 `src/native/NativeMachoInspection.ts` (+7 -7) 📝 `src/native/parsers/lipo.test.ts` (+105 -10) 📝 `src/native/parsers/lipo.ts` (+38 -17) 📝 `tests/boundary/cli/nativeMachoEchoedPathCli.test.ts` (+68 -23) 📝 `tests/boundary/providers/native/machoSliceReader.test.ts` (+53 -1) 📝 `tests/boundary/providers/native/nativeProvider.test.ts` (+13 -10) 📝 `tests/boundary/providers/native/nativeSelectedArchitecture.test.ts` (+44 -28) 📝 `tests/boundary/providers/native/nativeTextRobustness.test.ts` (+12 -6) </details> ### 📄 Description ## Summary I fixed FAT Mach-O filenames being parsed as architecture records. A filename containing a newline followed by `architecture forged` made REA report a third slice for a two-slice file, or reject the artifact because the lipo count disagreed with its FAT table. I found this while checking native parsing and reproduced the producer behavior with system lipo and a generated two-slice fixture. No separate issue is linked for this focused correction. I checked related work: #2040 covers slice selection, and #2107 covers codesign path echoes; neither isolates lipo's FAT operand header. ## Problem and expected behavior `lipo -detailed_info` prints `Fat header in: <path>` with the filename unchanged. Splitting that header into lines lets filename text become slice metadata. Only the actual tool records should contribute architectures, thin/FAT framing and ranges. A filename containing `Non-fat file:` also caused the inspection consumer to use a zero container offset and project segment/entrypoint evidence onto the wrong bytes. ## Change and scope The lipo parser receives the exact operand and removes only a complete matching FAT header ending in LF or CRLF. Native inspection, architecture listing and the slice reader pass their own lipo operand. The parser returns the thin/FAT framing together with architectures. Inspection uses that fact for its container offset rather than rescanning the raw capture. Thin and headerless captures retain their existing behavior, including single-slice FAT and unknown offsets. The CLI regressions share their driver rather than duplicate it. ## Contract and boundary impact - Semantic owner: `parseLipoOutput`, before record parsing; consumers reuse its framing and architecture facts. - Scope decision: not applicable; this corrects existing static metadata parsing. - CLI/MCP/catalog: no command or schema changes. Incorrect filename-derived slices disappear. - Compatibility: native macOS workflows; existing thin/FAT formats and unknown CPU text are preserved. - Evidence/provenance: raw captures, artifact bytes and identities are preserved; only derived records change. - Authority/cleanup: no added native commands or target execution. Temporary test resources use existing awaited cleanup. - Generated metadata, package and installation: no changes. Expected effects of the correction: - [x] Read existing files or captured data. Existing native tool subprocesses are unchanged. The target samples are not executed; this adds no instrumentation, target mutation, network access or protection recovery. ## Evidence and regression coverage - 11 parser cases cover embedded architecture/header tokens, multiline and CRLF paths, literal regex characters, common prefixes and compatibility. - A generated FAT fixture passes through the real system lipo and slice reader, retaining exactly the two declared ranges. - Real compiled `inspect-macho` and `list-architectures` CLI calls return the same facts for ordinary and multiline names, including a forged `Non-fat file:` line. Each CLI case compiles the existing C fixture for arm64 and x86_64, then checks multiple slices and a positive selected container offset before comparing filenames. This removes reliance on the host's `/usr/bin/true` being FAT. - LF/CRLF provider cases preserve the selected FAT slice offset and container-relative segment/entrypoint evidence; thin, single-slice FAT and unknown-offset controls remain covered. - Restoring the base parser makes five architecture regressions fail. Before the framing follow-up, both provider cases and the real CLI case fail with an incorrect zero container offset (the CLI observed 0 instead of 49152). - [x] Observations, derived records and unknowns remain distinct. - [x] Artifact identity and original provenance are preserved. - [x] Existing incomplete and unavailable outcomes remain visible. ## Validation performed With pinned Node 24.18.0 and npm 11.16.0 on macOS arm64: - `npm run test:focused -- src/native/parsers/lipo.test.ts tests/boundary/providers/native/nativeTextRobustness.test.ts tests/boundary/providers/native/nativeProvider.test.ts tests/boundary/providers/native/machoSliceReader.test.ts tests/boundary/cli/nativeMachoEchoedPathCli.test.ts tests/boundary/providers/native/artifactProvider.machOSlices.test.ts tests/boundary/providers/native/nativeSelectedArchitecture.test.ts` — 88 tests passed across seven files before the CLI fixture follow-up. - `npm run test:focused -- tests/boundary/cli/nativeMachoEchoedPathCli.test.ts` — all four CLI cases passed with the generated universal fixture. - Negative fixture experiment: compiling only arm64 makes the FAT precondition fail (`1` architecture is not greater than `1`), confirming thin fixtures cannot silently pass. - `npm run check` — passed, including typecheck, runtime compilation, lint, format, knip, metadata and architecture checks. - `npm run verify:test-discovery` — all 1023 files discovered once. - `git diff --check` — passed. ## Compatibility, safety, and release No breaking changes or migration. Real Apple metadata tools and compiled CLI coverage ran on macOS; Hopper/Ghidra and other host workflows were not run. No package, release metadata or execution-authority changes. Full deterministic coverage remains with CI. ## Before merge - [x] Concrete problem and one focused outcome. - [x] Existing contribution boundaries preserved; no new provider or execution capability. - [x] Conventional title and reason for no separate issue stated. - [x] Regression and compatibility coverage included. - [x] No owning schema or generated metadata change needed. - [x] Representative CLI behavior recorded above. - [x] Final diff checked for unrelated changes, secrets and unsupported claims. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Improved macOS architecture and slice detection for filenames containing text that resembles Mach-O metadata. * Prevented echoed filename text from being interpreted as architecture records, load commands, or symbols. * Improved slice offset reporting for thin and universal binaries across supported `lipo` output formats. * **Tests** * Expanded coverage for metadata-like filenames, including newline characters, and for thin and universal binary output formats. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>
bsakaguchi 2026-10-31 01:05:28 +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#5289
No description provided.