[PR #5363] [MERGED] fix(apple): preserve line terminators in dyld path expansion #6895

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

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/5363
Author: @Doribelove
Created: 10/20/2026
Status: ✅ Merged
Merged: 10/22/2026
Merged by: @morluto

Base: main ← Head: fix/dyld-path-line-terminators


📝 Commits (1)

  • 7866bed fix(apple): preserve line terminators in dyld path expansion

📊 Changes

3 files changed (+115 additions, -1 deletions)

View changed files

📝 src/domain/apple/dyldPaths.ts (+2 -1)
📝 src/domain/apple/dylibResolution.test.ts (+51 -0)
📝 tests/boundary/cli/dylibResolutionCli.test.ts (+62 -0)

📄 Description

Summary

Fixes #5362. Expand @loader_path and @executable_path when the dependency name or rpath suffix contains a line terminator, preserving the exact path rather than reporting an existing dependency as undetermined.

Problem and expected behavior

A synthetic app with LC_LOAD_DYLIB set to @loader_path/lib\u2028core.dylib and the matching file on disk returns an undetermined resolution. An LC_RPATH containing U+2029 has the same problem. The load-command reader retains both names correctly; the path expander's .* rejects line terminators.

Change and scope

Enable dot-all at the existing path-token parser. Keep the exact token/slash grammar, downstream containment checks, rpath ownership, and unknown executable context unchanged. Add domain regressions for LF, CR, CRLF, U+2028 and U+2029 plus lookalike-token controls, and a real-file CLI regression for direct dependencies and rpaths.

This is a routine static-analysis correction within the existing contribution scope; no separate scope decision is required. It does not change the otool parser covered by #4979.

Contract and boundary impact

  • Semantic owner / earliest changed stage: expandPrefix in src/domain/apple/dyldPaths.ts, before candidate path lookup.
  • CLI and MCP/tool catalog: existing commands and schemas; corrected resolution values.
  • Provider/platform: shared static artifact resolver; no external analysis engine or platform-specific API added.
  • Evidence/provenance: retain exact install names, rpaths, and resolved image paths.
  • Target authority/cleanup: existing read-only inspection and containment; tests own their temporary files and use the repository cleanup fixture.
  • Generated metadata, package and installation: no changes.

Expected product effects:

  • Read existing files or captured data.

Evidence and regression coverage

Executed on Ubuntu 22.04.5 x86_64, Node.js 24.18.0 and npm 11.16.0:

  • With the new tests and original implementation at 58d6043303e722115f1da18b9505c24abeeab423: 6 failed, 105 passed across the domain and CLI files. Failures are resolution assertions.
  • After the fix and synchronization with upstream 799e8265b254e225dc1dabdd25e56f0ab7f86bd3: 131 passed across the domain, CLI and MCP files.
  • A separate built CLI run on source-owned synthetic Mach-O files changed both resolutions from undetermined / null to the exact existing paths:
[
  {
    "install_name": "@loader_path/lib\u2028core.dylib",
    "resolution": {"status": "resolved", "image": "Contents/MacOS/lib\u2028core.dylib"}
  },
  {
    "install_name": "@rpath/libsearch.dylib",
    "resolution": {"status": "resolved", "image": "Contents/MacOS/Libraries\u2029Nested/libsearch.dylib"}
  }
]
  • Observed, derived, and inferred claims remain distinguishable.
  • Artifact identity and source provenance remain preserved.
  • Unsupported and uncertain outcomes remain visible; lookalike tokens stay undetermined.

Validation performed

  • npm ci — passed with the pinned toolchain.
  • npm run test:focused -- src/domain/apple/dylibResolution.test.ts tests/boundary/cli/dylibResolutionCli.test.ts tests/boundary/mcp/dylibResolutionMcp.test.ts — 131 passed; required runtime/skill build passed.
  • npm run check — passed: typecheck, lint/module boundaries, formatting, knip, metadata and architecture guards. Lint still reports warnings.
  • npm run verify:test-discovery — all 1,714 test files discovered exactly once.
  • git diff --check — passed.

The first static check caught an overlong enclosing test-group callback after adding the cases. Moving the new tests to the existing top-level style resolved it; the final checks above passed.

Compatibility, safety, and release

No API, schema, dependency or release-metadata changes. Synthetic files were read by the built CLI and the production artifact resolver; fixture programs were never executed. This establishes static resolution on Linux. Actual macOS dyld loading, Windows filesystem execution and Hopper/Ghidra were not exercised. The existing MCP boundary suite passed; no live external MCP client was tested.

Author readiness

Human owner: @Doribelove

  • 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.
  • Scope and authority remain within existing static inspection.
  • The PR has one focused outcome and a Conventional Commit title.
  • Related issue is linked.
  • Tests cover changed behavior and token-boundary controls.
  • No public contracts or generated documentation require changes.
  • Representative CLI output is included.
  • The human owner has completed the final review and readiness declaration.

🔄 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/5363 **Author:** [@Doribelove](https://github.com/Doribelove) **Created:** 10/20/2026 **Status:** ✅ Merged **Merged:** 10/22/2026 **Merged by:** [@morluto](https://github.com/morluto) **Base:** `main` ← **Head:** `fix/dyld-path-line-terminators` --- ### 📝 Commits (1) - [`7866bed`](https://github.com/morluto/rea/commit/7866bed983122375eda51d97ad992d0a3d7e5927) fix(apple): preserve line terminators in dyld path expansion ### 📊 Changes **3 files changed** (+115 additions, -1 deletions) <details> <summary>View changed files</summary> 📝 `src/domain/apple/dyldPaths.ts` (+2 -1) 📝 `src/domain/apple/dylibResolution.test.ts` (+51 -0) 📝 `tests/boundary/cli/dylibResolutionCli.test.ts` (+62 -0) </details> ### 📄 Description ## Summary Fixes #5362. Expand `@loader_path` and `@executable_path` when the dependency name or rpath suffix contains a line terminator, preserving the exact path rather than reporting an existing dependency as undetermined. ## Problem and expected behavior A synthetic app with `LC_LOAD_DYLIB` set to `@loader_path/lib\u2028core.dylib` and the matching file on disk returns an undetermined resolution. An `LC_RPATH` containing U+2029 has the same problem. The load-command reader retains both names correctly; the path expander's `.*` rejects line terminators. ## Change and scope Enable dot-all at the existing path-token parser. Keep the exact token/slash grammar, downstream containment checks, rpath ownership, and unknown executable context unchanged. Add domain regressions for LF, CR, CRLF, U+2028 and U+2029 plus lookalike-token controls, and a real-file CLI regression for direct dependencies and rpaths. This is a routine static-analysis correction within the [existing contribution scope](https://github.com/morluto/rea/blob/main/CONTRIBUTING.md#contribution-scope); no separate scope decision is required. It does not change the `otool` parser covered by #4979. ## Contract and boundary impact - Semantic owner / earliest changed stage: `expandPrefix` in `src/domain/apple/dyldPaths.ts`, before candidate path lookup. - CLI and MCP/tool catalog: existing commands and schemas; corrected resolution values. - Provider/platform: shared static artifact resolver; no external analysis engine or platform-specific API added. - Evidence/provenance: retain exact install names, rpaths, and resolved image paths. - Target authority/cleanup: existing read-only inspection and containment; tests own their temporary files and use the repository cleanup fixture. - Generated metadata, package and installation: no changes. Expected product effects: - [x] Read existing files or captured data. ## Evidence and regression coverage Executed on Ubuntu 22.04.5 x86_64, Node.js 24.18.0 and npm 11.16.0: - With the new tests and original implementation at `58d6043303e722115f1da18b9505c24abeeab423`: **6 failed, 105 passed** across the domain and CLI files. Failures are resolution assertions. - After the fix and synchronization with upstream `799e8265b254e225dc1dabdd25e56f0ab7f86bd3`: **131 passed** across the domain, CLI and MCP files. - A separate built CLI run on source-owned synthetic Mach-O files changed both resolutions from `undetermined` / `null` to the exact existing paths: ```json [ { "install_name": "@loader_path/lib\u2028core.dylib", "resolution": {"status": "resolved", "image": "Contents/MacOS/lib\u2028core.dylib"} }, { "install_name": "@rpath/libsearch.dylib", "resolution": {"status": "resolved", "image": "Contents/MacOS/Libraries\u2029Nested/libsearch.dylib"} } ] ``` - [x] Observed, derived, and inferred claims remain distinguishable. - [x] Artifact identity and source provenance remain preserved. - [x] Unsupported and uncertain outcomes remain visible; lookalike tokens stay undetermined. ## Validation performed - `npm ci` — passed with the pinned toolchain. - `npm run test:focused -- src/domain/apple/dylibResolution.test.ts tests/boundary/cli/dylibResolutionCli.test.ts tests/boundary/mcp/dylibResolutionMcp.test.ts` — **131 passed**; required runtime/skill build passed. - `npm run check` — passed: typecheck, lint/module boundaries, formatting, knip, metadata and architecture guards. Lint still reports warnings. - `npm run verify:test-discovery` — all **1,714** test files discovered exactly once. - `git diff --check` — passed. The first static check caught an overlong enclosing test-group callback after adding the cases. Moving the new tests to the existing top-level style resolved it; the final checks above passed. ## Compatibility, safety, and release No API, schema, dependency or release-metadata changes. Synthetic files were read by the built CLI and the production artifact resolver; fixture programs were never executed. This establishes static resolution on Linux. Actual macOS dyld loading, Windows filesystem execution and Hopper/Ghidra were not exercised. The existing MCP boundary suite passed; no live external MCP client was tested. ## Author readiness Human owner: @Doribelove - [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. - [x] Scope and authority remain within existing static inspection. - [x] The PR has one focused outcome and a Conventional Commit title. - [x] Related issue is linked. - [x] Tests cover changed behavior and token-boundary controls. - [x] No public contracts or generated documentation require changes. - [x] Representative CLI output is included. - [x] The human owner has completed the final review and readiness declaration. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>
bsakaguchi 2026-11-01 21:43:54 +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#6895
No description provided.