morluto/rea

Fork 0

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

[PR #6371] [MERGED] fix(native): report plist symlink loops and overlong paths as selection failures #7315

New issue

Closed

opened 2026-11-01 21:46:59 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-11-01 21:46:59 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/6371
Author: @Abelo9996
Created: 10/22/2026
Status: ✅ Merged
Merged: 10/23/2026
Merged by: @baron

Base: main ← Head: fix-5949-plist-eloop-enametoolong

📝 Commits (2)

a579dfb fix(native): report plist symlink loops and overlong paths as selection failures
9f44a11 Merge main to pick up the uniqueBy build fix

📊 Changes

2 files changed (+65 additions, -1 deletions)

View changed files

📝 src/native/NativePlistSource.ts (+8 -0)
📝 tests/boundary/providers/native/nativePlistSelection.test.ts (+57 -1)

📄 Description

Summary

Fixes #5949.

inspect-plist reported execution_failure with rea doctor advice when the selected plist was a symlink loop (ELOOP) or had an overlong name (ENAMETOOLONG), and did the same for a target's default Contents/Info.plist symlink loop. Those are problems with the chosen path, not with the toolchain. They now go through the existing unreadablePlist helper like a missing file: invalid_request with a path issue for an explicit selection, and capability_unavailable with the default-plist explanation for the default. The ELOOP / ENAMETOOLONG code stays in the message.

Problem and expected behavior

nativePlistFailure in src/native/NativePlistSource.ts maps ENOENT/ENOTDIR and EISDIR/ENOTFILE to unreadablePlist, and EACCES/EPERM to BinaryTargetError. Anything else falls through to ProviderAdapterError, which projects as execution_failure. The plist reader follows symlinks, so a loop or an overlong component surfaces as a raw open() error and lands in that fallback:

{"code":"execution_failure","details":{"diagnostics":{"path":"<tmp>/Loop.plist","phase":"plist-file","reason":"ELOOP: too many symbolic links encountered, open '<tmp>/Loop.plist'"},"operation":"inspect_plist","provider_id":"native-macos"}}

Expected, per the selected/default plist contract in docs/native-investigation.md: explicit selection gives invalid_request with an invalid_value issue on path that keeps the filesystem reason; default selection gives capability_unavailable saying the target has no readable default plist.

Change and scope

One new branch in nativePlistFailure: ELOOP and ENAMETOOLONG return unreadablePlist(selection, reason, { cause, capturedOutput }), with a reason naming the code. The wording follows the existing selected-input messages in JavaScriptArtifactReconstruction.ts. The original error is still attached as cause.

Permission errors, scope failures (no selection yet), resource limits, cleanup failures and real provider failures keep their current categories.

Follow the contribution scope.
Scope decision: not applicable (ordinary fix within existing boundaries).

Contract and boundary impact

Semantic owner and earliest changed stage: nativePlistFailure, classifying a failed open of the selected or default plist file.
Contribution scope fit and linked maintainer decision: ordinary bug fix, not applicable.
CLI and MCP/tool-catalog contract: no schema change. These two errno cases now return invalid_request / invalid_input (selected) or capability_unavailable (default) instead of execution_failure.
Provider, bridge, target-format, or platform compatibility: native plist inspection only. The branch only triggers when the OS reports ELOOP or ENAMETOOLONG.
Evidence, artifact, provenance, or reconstruction contract: none.
Target selection, authority, upstream behavior, and cleanup details (if applicable): selection resolution is unchanged; only the classification of the open failure changes.
Generated metadata (docs/public/product-catalog.json), package, or installation impact (CI generates and validates ignored catalogs and skills/; edit authored instructions in .agents/skills/reverse-engineer-anything/ and never commit build outputs): none.

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.

(The existing open of the selected plist, unchanged by this PR.)

Evidence and regression coverage

Tests added or updated: tests/boundary/providers/native/nativePlistSelection.test.ts gains "reports selected and default plist path-resolution failures as selection failures". It selects a self-referential Loop.plist and a 300-character name and expects invalid_request with a path / invalid_value issue naming (ELOOP) / (ENAMETOOLONG), then uses a Mach-O target whose Contents/Info.plist is a symlink loop and expects capability_unavailable with "no readable default plist" and (ELOOP). Skipped on Windows because it creates symlinks.

Base reproduction (executed): with the test and without the fix:

× reports selected and default plist path-resolution failures as selection failures
-   "code": "invalid_request",
+   "code": "execution_failure",
      Tests  1 failed | 6 passed (7)

With the fix: 7 passed.

User-visible CLI/MCP output (if applicable): built CLI after the fix, temp directory replaced with <tmp>. Before is the execution_failure shown above.

rea inspect-plist <tmp>/Good.plist --relative-path <tmp>/Loop.plist --json (exit 1):

{"error":"Analysis failed","code":"invalid_request","category":"invalid_input","message":"Analysis input is invalid. Check the arguments and try again.","retryable":false,"remediation":{"action":"Read the declared input schema or CLI usage for inspect_plist; if using MCP and the catalog may be stale, call tools/list. Correct the listed fields, then retry. Do not repeat the unchanged call."},"recovery":{"kind":"inspect_input_contract","operation":"inspect_plist"},"details":{"operation":"inspect_plist","issues":[{"path":["path"],"reason":"invalid_value","message":"Cannot inspect <tmp>/Loop.plist: symbolic-link traversal looped or exceeded the host limit (ELOOP)."}]}}

The overlong selection gives the same shape with ...: the path exceeds the host path or component length limit (ENAMETOOLONG).

rea inspect-plist <tmp>/program --json, default Contents/Info.plist loop (exit 1):

{"error":"Analysis failed","code":"capability_unavailable","category":"unsupported_provider","message":"The target has no readable default plist at <tmp>/Contents/Info.plist: symbolic-link traversal looped or exceeded the host limit (ELOOP). Select another plist in the target with the path input (CLI: --relative-path).","retryable":false,"remediation":{"action":"The target has no readable default plist at <tmp>/Contents/Info.plist: symbolic-link traversal looped or exceeded the host limit (ELOOP). Select another plist in the target with the path input (CLI: --relative-path)."},"details":{"provider_id":"native-macos","operation":"inspect_plist","reason":"The target has no readable default plist at <tmp>/Contents/Info.plist: symbolic-link traversal looped or exceeded the host limit (ELOOP). Select another plist in the target with the path input (CLI: --relative-path)."}}

A readable Good.plist still succeeds (exit 0).

Remaining proof gaps: macOS only; Linux and Windows not run. MCP transport not exercised separately; it uses the same projection.

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

Run with the pinned Node 24.18.0 on macOS arm64.

npm run test:focused -- tests/boundary/providers/native/nativePlistSelection.test.ts: 7 passed (1 failure without the fix, shown above)
npm run test:focused -- on the other plist tests (nativePlistFileCleanup, nativePlistBinarySource, nativePlistDiagnostics, plistSourceFormat, bundleResourceComposition, nativePlistDiagnosticsMcp, nativePlistSelectionFailureCli) plus the file above: 8 files, 63 tests passed
lint:built (0 errors), knip, metadata:check and verify:architecture-guards passed. oxfmt --check on the two changed files is clean. typecheck reports no errors in the changed files; see the note below about the current base.

Compatibility, safety, and release

Breaking changes or migration steps: none. Callers that matched execution_failure for these two cases now get invalid_request or capability_unavailable.
Real Hopper/Ghidra, browser, or OS coverage: macOS only, system plist utilities via the existing fixture runner and built CLI.
Package or release metadata impact: none.
Security, privacy, process, or containment review: no new filesystem access; the selected path was already echoed back in the existing selection messages.

Author readiness

Human owner: @Abelo9996

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/6371 **Author:** [@Abelo9996](https://github.com/Abelo9996) **Created:** 10/22/2026 **Status:** ✅ Merged **Merged:** 10/23/2026 **Merged by:** [@baron](https://github.com/baron) **Base:** `main` ← **Head:** `fix-5949-plist-eloop-enametoolong` --- ### 📝 Commits (2) - [`a579dfb`](https://github.com/morluto/rea/commit/a579dfbd872682e17bd3fcc88b66149e893298b5) fix(native): report plist symlink loops and overlong paths as selection failures - [`9f44a11`](https://github.com/morluto/rea/commit/9f44a119a4d38901b90257c38d5faad15c40fdd4) Merge main to pick up the uniqueBy build fix ### 📊 Changes **2 files changed** (+65 additions, -1 deletions) <details> <summary>View changed files</summary> 📝 `src/native/NativePlistSource.ts` (+8 -0) 📝 `tests/boundary/providers/native/nativePlistSelection.test.ts` (+57 -1) </details> ### 📄 Description ## Summary Fixes #5949. `inspect-plist` reported `execution_failure` with `rea doctor` advice when the selected plist was a symlink loop (`ELOOP`) or had an overlong name (`ENAMETOOLONG`), and did the same for a target's default `Contents/Info.plist` symlink loop. Those are problems with the chosen path, not with the toolchain. They now go through the existing `unreadablePlist` helper like a missing file: `invalid_request` with a `path` issue for an explicit selection, and `capability_unavailable` with the default-plist explanation for the default. The `ELOOP` / `ENAMETOOLONG` code stays in the message. ## Problem and expected behavior `nativePlistFailure` in `src/native/NativePlistSource.ts` maps `ENOENT`/`ENOTDIR` and `EISDIR`/`ENOTFILE` to `unreadablePlist`, and `EACCES`/`EPERM` to `BinaryTargetError`. Anything else falls through to `ProviderAdapterError`, which projects as `execution_failure`. The plist reader follows symlinks, so a loop or an overlong component surfaces as a raw `open()` error and lands in that fallback: ```json {"code":"execution_failure","details":{"diagnostics":{"path":"<tmp>/Loop.plist","phase":"plist-file","reason":"ELOOP: too many symbolic links encountered, open '<tmp>/Loop.plist'"},"operation":"inspect_plist","provider_id":"native-macos"}} ``` Expected, per the selected/default plist contract in `docs/native-investigation.md`: explicit selection gives `invalid_request` with an `invalid_value` issue on `path` that keeps the filesystem reason; default selection gives `capability_unavailable` saying the target has no readable default plist. ## Change and scope One new branch in `nativePlistFailure`: `ELOOP` and `ENAMETOOLONG` return `unreadablePlist(selection, reason, { cause, capturedOutput })`, with a reason naming the code. The wording follows the existing selected-input messages in `JavaScriptArtifactReconstruction.ts`. The original error is still attached as `cause`. Permission errors, scope failures (no selection yet), resource limits, cleanup failures and real provider failures keep their current categories. Follow the [contribution scope](https://github.com/morluto/rea/blob/main/CONTRIBUTING.md#contribution-scope). Scope decision: not applicable (ordinary fix within existing boundaries). ## Contract and boundary impact - Semantic owner and earliest changed stage: `nativePlistFailure`, classifying a failed open of the selected or default plist file. - Contribution scope fit and linked maintainer decision: ordinary bug fix, not applicable. - CLI and MCP/tool-catalog contract: no schema change. These two errno cases now return `invalid_request` / `invalid_input` (selected) or `capability_unavailable` (default) instead of `execution_failure`. - Provider, bridge, target-format, or platform compatibility: native plist inspection only. The branch only triggers when the OS reports `ELOOP` or `ENAMETOOLONG`. - Evidence, artifact, provenance, or reconstruction contract: none. - Target selection, authority, upstream behavior, and cleanup details (if applicable): selection resolution is unchanged; only the classification of the open failure changes. - Generated metadata (`docs/public/product-catalog.json`), package, or installation impact (CI generates and validates ignored catalogs and `skills/`; edit authored instructions in `.agents/skills/reverse-engineer-anything/` and never commit build outputs): none. 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. (The existing open of the selected plist, unchanged by this PR.) ## Evidence and regression coverage - Tests added or updated: `tests/boundary/providers/native/nativePlistSelection.test.ts` gains "reports selected and default plist path-resolution failures as selection failures". It selects a self-referential `Loop.plist` and a 300-character name and expects `invalid_request` with a `path` / `invalid_value` issue naming `(ELOOP)` / `(ENAMETOOLONG)`, then uses a Mach-O target whose `Contents/Info.plist` is a symlink loop and expects `capability_unavailable` with "no readable default plist" and `(ELOOP)`. Skipped on Windows because it creates symlinks. - Base reproduction (executed): with the test and without the fix: ``` × reports selected and default plist path-resolution failures as selection failures - "code": "invalid_request", + "code": "execution_failure", Tests 1 failed | 6 passed (7) ``` With the fix: 7 passed. - User-visible CLI/MCP output (if applicable): built CLI after the fix, temp directory replaced with `<tmp>`. Before is the `execution_failure` shown above. `rea inspect-plist <tmp>/Good.plist --relative-path <tmp>/Loop.plist --json` (exit 1): ```json {"error":"Analysis failed","code":"invalid_request","category":"invalid_input","message":"Analysis input is invalid. Check the arguments and try again.","retryable":false,"remediation":{"action":"Read the declared input schema or CLI usage for inspect_plist; if using MCP and the catalog may be stale, call tools/list. Correct the listed fields, then retry. Do not repeat the unchanged call."},"recovery":{"kind":"inspect_input_contract","operation":"inspect_plist"},"details":{"operation":"inspect_plist","issues":[{"path":["path"],"reason":"invalid_value","message":"Cannot inspect <tmp>/Loop.plist: symbolic-link traversal looped or exceeded the host limit (ELOOP)."}]}} ``` The overlong selection gives the same shape with `...: the path exceeds the host path or component length limit (ENAMETOOLONG).` `rea inspect-plist <tmp>/program --json`, default `Contents/Info.plist` loop (exit 1): ```json {"error":"Analysis failed","code":"capability_unavailable","category":"unsupported_provider","message":"The target has no readable default plist at <tmp>/Contents/Info.plist: symbolic-link traversal looped or exceeded the host limit (ELOOP). Select another plist in the target with the path input (CLI: --relative-path).","retryable":false,"remediation":{"action":"The target has no readable default plist at <tmp>/Contents/Info.plist: symbolic-link traversal looped or exceeded the host limit (ELOOP). Select another plist in the target with the path input (CLI: --relative-path)."},"details":{"provider_id":"native-macos","operation":"inspect_plist","reason":"The target has no readable default plist at <tmp>/Contents/Info.plist: symbolic-link traversal looped or exceeded the host limit (ELOOP). Select another plist in the target with the path input (CLI: --relative-path)."}} ``` A readable `Good.plist` still succeeds (exit 0). - Remaining proof gaps: macOS only; Linux and Windows not run. MCP transport not exercised separately; it uses the same projection. 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 Run with the pinned Node 24.18.0 on macOS arm64. - `npm run test:focused -- tests/boundary/providers/native/nativePlistSelection.test.ts`: 7 passed (1 failure without the fix, shown above) - `npm run test:focused --` on the other plist tests (`nativePlistFileCleanup`, `nativePlistBinarySource`, `nativePlistDiagnostics`, `plistSourceFormat`, `bundleResourceComposition`, `nativePlistDiagnosticsMcp`, `nativePlistSelectionFailureCli`) plus the file above: 8 files, 63 tests passed - `lint:built` (0 errors), `knip`, `metadata:check` and `verify:architecture-guards` passed. `oxfmt --check` on the two changed files is clean. `typecheck` reports no errors in the changed files; see the note below about the current base. ## Compatibility, safety, and release - Breaking changes or migration steps: none. Callers that matched `execution_failure` for these two cases now get `invalid_request` or `capability_unavailable`. - Real Hopper/Ghidra, browser, or OS coverage: macOS only, system plist utilities via the existing fixture runner and built CLI. - Package or release metadata impact: none. - Security, privacy, process, or containment review: no new filesystem access; the selected path was already echoed back in the existing selection messages. ## Author readiness Human owner: @Abelo9996 - [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

2026-11-01 21:46:59 +00:00