morluto/rea

Fork 0

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

[PR #6271] [MERGED] fix(il2cpp): refuse a seed path under a non-directory as caller input #7245

New issue

Closed

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

bsakaguchi commented

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

Owner

📋 Pull Request Information

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

Base: main ← Head: fix-6039-seed-output-notdir

📝 Commits (1)

fb8b766 fix(il2cpp): refuse a seed path under a non-directory as caller input

📊 Changes

2 files changed (+45 additions, -0 deletions)

View changed files

📝 src/il2cpp/Il2cppGhidraSeedWriter.ts (+7 -0)
📝 tests/acceptance/il2cppMetadataCli.test.ts (+38 -0)

📄 Description

Summary

Fixes #6039.

When a parent component of the --ghidra-seeds destination was a regular file, inspect-il2cpp-metadata failed with execution_failure and told the caller to check file permissions, which cannot fix that path. The seed preflight now refuses ENOTDIR as invalid_request / invalid_input with a ghidra_seeds field issue telling the caller to choose an output path inside an existing directory. Create-only behavior and real I/O or permission failures are unchanged.

Problem and expected behavior

Il2cppGhidraSeedWriter.check in src/il2cpp/Il2cppGhidraSeedWriter.ts runs lstat on the destination before inspection. It accepted ENOENT as "absent" and wrapped every other failure as EvidenceFileError("write", "io"), which projects to execution_failure with permission advice. With a regular file regular-file in the output's parent chain:

rea inspect-il2cpp-metadata global-metadata.dat --binary libil2cpp.so --ghidra-seeds regular-file/seeds.tsv --json

Before (exit 1):

{"code":"execution_failure","category":"execution_failure","message":"Evidence file could not be accessed. Check file permissions and try again.","retryable":false,"remediation":{"action":"Evidence file could not be accessed. Check file permissions and try again."},"details":{"operation":"write","reason":"io","path":"<tmp>/regular-file/seeds.tsv"}}

Expected: invalid_request / invalid_input, with guidance to pick a path whose parent is an existing directory.

Change and scope

In the preflight's lstat catch, ENOTDIR now returns the existing invalid("ghidra_seeds", ...) input error, the same helper that already reports a non-absolute path or an existing destination for this field. ENOENT still means "absent, go ahead", and other errors (for example EACCES) still produce EvidenceFileError("write", "io").

I kept this to ENOTDIR, which is what the issue reproduces. I did not change the shared atomic writer or other lstat codes such as ELOOP or ENAMETOOLONG.

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

Contract and boundary impact

Semantic owner and earliest changed stage: Il2cppGhidraSeedWriter.check, the seed-output preflight that Il2cppMetadataService runs before inspection.
Contribution scope fit and linked maintainer decision: ordinary bug fix, not applicable.
CLI and MCP/tool-catalog contract: no schema change. An ENOTDIR seed destination now returns invalid_request / invalid_input with details.issues[0].path: ["ghidra_seeds"] instead of execution_failure / io.
Provider, bridge, target-format, or platform compatibility: none. The new branch only triggers when the OS reports ENOTDIR.
Evidence, artifact, provenance, or reconstruction contract: none. Seed files are still created exclusively and never replaced.
Target selection, authority, upstream behavior, and cleanup details (if applicable): unchanged. Nothing is written for the refused path, and the obstructing file is untouched.
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 lstat of the requested output path, unchanged by this PR.)

Evidence and regression coverage

Tests added or updated: tests/acceptance/il2cppMetadataCli.test.ts gains "refuses a Ghidra seed path below a regular file as caller input". It runs the built CLI with --ghidra-seeds <root>/regular-file/seeds.tsv, asserts invalid_request / invalid_input with a ghidra_seeds issue, and checks the regular file still holds its original bytes. It is skipped on Windows, like the other path-shape test in this file, since the ENOTDIR trigger is POSIX behavior.

Base reproduction (executed): with the test and without the fix (CLI rebuilt by the focused runner):

× refuses a Ghidra seed path below a regular file as caller input
-   "category": "invalid_input",
-   "code": "invalid_request",
+   "category": "execution_failure",
+   "code": "execution_failure",
      Tests  1 failed | 11 passed (12)

With the fix: 12 passed.

User-visible CLI/MCP output (if applicable): built CLI, same command as above, temp directory replaced with <tmp>. Before is shown above. After (exit 1):

{"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_il2cpp_metadata; 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_il2cpp_metadata"},"details":{"operation":"inspect_il2cpp_metadata","issues":[{"path":["ghidra_seeds"],"reason":"invalid_value","message":"--ghidra-seeds <tmp>/regular-file/seeds.tsv has a parent path component that is not a directory; choose an output path inside an existing directory."}]}}

Remaining proof gaps: macOS only; Windows and Linux not run. MCP transport not exercised. The issue reason is invalid_value, matching the existing "already exists" refusal for this field.

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/acceptance/il2cppMetadataCli.test.ts: 12 passed (1 failure without the fix, shown above)
npm run test:focused -- src/application/il2cpp/Il2cppMetadataService.test.ts: 11 passed
npm run check tasks: typecheck, lint:built (0 errors), knip, metadata:check and verify:architecture-guards passed. format:check was run as oxfmt --check on the two changed files (clean), because the repo-wide run in my checkout also picked up unrelated untracked local notes.

Compatibility, safety, and release

Breaking changes or migration steps: none. Callers that matched execution_failure for this case now get invalid_request.
Real Hopper/Ghidra, browser, or OS coverage: macOS only, synthetic IL2CPP metadata and ELF fixtures. No Ghidra engine involved.
Package or release metadata impact: none.
Security, privacy, process, or containment review: the refusal happens before any write; create-only output is unchanged.

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/6271 **Author:** [@Abelo9996](https://github.com/Abelo9996) **Created:** 10/22/2026 **Status:** ✅ Merged **Merged:** 10/22/2026 **Merged by:** [@morluto](https://github.com/morluto) **Base:** `main` ← **Head:** `fix-6039-seed-output-notdir` --- ### 📝 Commits (1) - [`fb8b766`](https://github.com/morluto/rea/commit/fb8b766f8ffa246f100782c32816b4d50da48cb4) fix(il2cpp): refuse a seed path under a non-directory as caller input ### 📊 Changes **2 files changed** (+45 additions, -0 deletions) <details> <summary>View changed files</summary> 📝 `src/il2cpp/Il2cppGhidraSeedWriter.ts` (+7 -0) 📝 `tests/acceptance/il2cppMetadataCli.test.ts` (+38 -0) </details> ### 📄 Description ## Summary Fixes #6039. When a parent component of the `--ghidra-seeds` destination was a regular file, `inspect-il2cpp-metadata` failed with `execution_failure` and told the caller to check file permissions, which cannot fix that path. The seed preflight now refuses `ENOTDIR` as `invalid_request` / `invalid_input` with a `ghidra_seeds` field issue telling the caller to choose an output path inside an existing directory. Create-only behavior and real I/O or permission failures are unchanged. ## Problem and expected behavior `Il2cppGhidraSeedWriter.check` in `src/il2cpp/Il2cppGhidraSeedWriter.ts` runs `lstat` on the destination before inspection. It accepted `ENOENT` as "absent" and wrapped every other failure as `EvidenceFileError("write", "io")`, which projects to `execution_failure` with permission advice. With a regular file `regular-file` in the output's parent chain: ```sh rea inspect-il2cpp-metadata global-metadata.dat --binary libil2cpp.so --ghidra-seeds regular-file/seeds.tsv --json ``` Before (exit 1): ```json {"code":"execution_failure","category":"execution_failure","message":"Evidence file could not be accessed. Check file permissions and try again.","retryable":false,"remediation":{"action":"Evidence file could not be accessed. Check file permissions and try again."},"details":{"operation":"write","reason":"io","path":"<tmp>/regular-file/seeds.tsv"}} ``` Expected: `invalid_request` / `invalid_input`, with guidance to pick a path whose parent is an existing directory. ## Change and scope In the preflight's `lstat` catch, `ENOTDIR` now returns the existing `invalid("ghidra_seeds", ...)` input error, the same helper that already reports a non-absolute path or an existing destination for this field. `ENOENT` still means "absent, go ahead", and other errors (for example `EACCES`) still produce `EvidenceFileError("write", "io")`. I kept this to `ENOTDIR`, which is what the issue reproduces. I did not change the shared atomic writer or other `lstat` codes such as `ELOOP` or `ENAMETOOLONG`. 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: `Il2cppGhidraSeedWriter.check`, the seed-output preflight that `Il2cppMetadataService` runs before inspection. - Contribution scope fit and linked maintainer decision: ordinary bug fix, not applicable. - CLI and MCP/tool-catalog contract: no schema change. An `ENOTDIR` seed destination now returns `invalid_request` / `invalid_input` with `details.issues[0].path: ["ghidra_seeds"]` instead of `execution_failure` / `io`. - Provider, bridge, target-format, or platform compatibility: none. The new branch only triggers when the OS reports `ENOTDIR`. - Evidence, artifact, provenance, or reconstruction contract: none. Seed files are still created exclusively and never replaced. - Target selection, authority, upstream behavior, and cleanup details (if applicable): unchanged. Nothing is written for the refused path, and the obstructing file is untouched. - 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 `lstat` of the requested output path, unchanged by this PR.) ## Evidence and regression coverage - Tests added or updated: `tests/acceptance/il2cppMetadataCli.test.ts` gains "refuses a Ghidra seed path below a regular file as caller input". It runs the built CLI with `--ghidra-seeds <root>/regular-file/seeds.tsv`, asserts `invalid_request` / `invalid_input` with a `ghidra_seeds` issue, and checks the regular file still holds its original bytes. It is skipped on Windows, like the other path-shape test in this file, since the `ENOTDIR` trigger is POSIX behavior. - Base reproduction (executed): with the test and without the fix (CLI rebuilt by the focused runner): ``` × refuses a Ghidra seed path below a regular file as caller input - "category": "invalid_input", - "code": "invalid_request", + "category": "execution_failure", + "code": "execution_failure", Tests 1 failed | 11 passed (12) ``` With the fix: 12 passed. - User-visible CLI/MCP output (if applicable): built CLI, same command as above, temp directory replaced with `<tmp>`. Before is shown above. After (exit 1): ```json {"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_il2cpp_metadata; 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_il2cpp_metadata"},"details":{"operation":"inspect_il2cpp_metadata","issues":[{"path":["ghidra_seeds"],"reason":"invalid_value","message":"--ghidra-seeds <tmp>/regular-file/seeds.tsv has a parent path component that is not a directory; choose an output path inside an existing directory."}]}} ``` - Remaining proof gaps: macOS only; Windows and Linux not run. MCP transport not exercised. The issue `reason` is `invalid_value`, matching the existing "already exists" refusal for this field. 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/acceptance/il2cppMetadataCli.test.ts`: 12 passed (1 failure without the fix, shown above) - `npm run test:focused -- src/application/il2cpp/Il2cppMetadataService.test.ts`: 11 passed - `npm run check` tasks: `typecheck`, `lint:built` (0 errors), `knip`, `metadata:check` and `verify:architecture-guards` passed. `format:check` was run as `oxfmt --check` on the two changed files (clean), because the repo-wide run in my checkout also picked up unrelated untracked local notes. ## Compatibility, safety, and release - Breaking changes or migration steps: none. Callers that matched `execution_failure` for this case now get `invalid_request`. - Real Hopper/Ghidra, browser, or OS coverage: macOS only, synthetic IL2CPP metadata and ELF fixtures. No Ghidra engine involved. - Package or release metadata impact: none. - Security, privacy, process, or containment review: the refusal happens before any write; create-only output is unchanged. ## 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:28 +00:00