morluto/rea

Fork 0

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

[PR #4416] [MERGED] fix(dotnet): reject unfinished CIL prefix chains #6227

New issue

Closed

opened 2026-10-31 01:12:51 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-31 01:12:51 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/4416
Author: @AbdulSamad94
Created: 10/16/2026
Status: ✅ Merged
Merged: 10/21/2026
Merged by: @morluto

Base: main ← Head: fix/4370-dangling-cil-prefix

📝 Commits (2)

fdf71a1 fix(dotnet): reject unfinished CIL prefix chains
3ab2478 test(dotnet): cover unfinished prefixes in fat method bodies

📊 Changes

3 files changed (+131 additions, -0 deletions)

View changed files

📝 src/dotnet/ManagedCilDecoding.test.ts (+84 -0)
📝 src/dotnet/ManagedMemberInstructionDecoder.ts (+2 -0)
📝 tests/acceptance/managedCliOutput.test.ts (+45 -0)

📄 Description

Summary

Fixes #4370. A CIL body ending in a complete prefix opcode (for example FE 13, volatile.) previously reported present, complete coverage, and a normalized fingerprint despite having no following instruction. It now reports a located malformed-body diagnostic and partial coverage, without losing the raw CIL digest or decoded facts.

Problem and expected behavior

Prefixes modify a following instruction. A terminal prefix chain must not qualify for the documented complete decoded-CIL fingerprint. The original 0A FE 13 tiny-body fixture reproduced through both the production inspector and the built CLI on base d72d856.

Change and scope

Check the decoder's existing pending-prefix state when decoding finishes, preserving an earlier failure if one exists. Existing body-reader and inspector consumers propagate the diagnostic, suppress the normalized hash, and mark member/user-string scan coverage partial. No downstream output repair or new abstraction is needed. General prefix/opcode compatibility and CIL stack/type verification remain outside this fix.

Contract and boundary impact

Semantic owner and earliest changed stage: decodeInstructions in src/dotnet/ManagedMemberInstructionDecoder.ts.
Contribution scope fit and linked maintainer decision: routine static-parser correction within existing boundaries; advance scope decision not applicable.
CLI and MCP/tool-catalog contract: unchanged schemas; existing malformed/partial outcomes now represent unfinished prefix chains correctly. CLI exercised; MCP not separately exercised.
Provider, bridge, target-format, or platform compatibility: managed PE/CIL parser only; no external engine or bridge changes.
Evidence, artifact, provenance, or reconstruction contract: raw CIL hash, decoded instruction counts/opcodes/anchors, and artifact identity remain retained; incomplete bodies have no normalized fingerprint.
Target selection, authority, upstream behavior, and cleanup details: unchanged; static inspection only, no target execution or mutation. CLI fixture directories use the existing awaited test cleanup helper.
Generated metadata, package, or installation impact: none; no schema, catalog, version, or dependency changes.

Expected effects:

Read existing files or captured data.

No additional process, injection, target mutation, network, or protection-recovery capability is introduced.

Evidence and regression coverage

Production-inspector regressions cover all six supported prefixes, a multi-prefix chain, nonzero chain offsets, a later dangling chain after a completed one, valid completed chains, and preservation of existing truncated/unsupported opcode errors.
Built CLI regression checks malformed output, raw-hash retention, absent normalized hash, partial member/user-string coverage, and a valid nop; ret control.
Before the production fix: 8 new source cases failed, with 81 other cases passing; the new CLI case failed, with 7 existing CLI cases passing. Failures showed the erroneous present/complete/hash outputs.
After the fix: all 229 selected tests passed (220 source tests and 9 CLI/comparison-boundary tests).

Executed CLI result for 0A FE 13:

{"status":"malformed","normalized_il_sha256":null,"decoded_instruction_count":1,"opcode_counts":{"volatile.":1},"issue":"CIL prefix chain at IL offset 0 has no following instruction"}

Member and user-string coverage are partial; the raw CIL hash remains present. The valid control remains present with a normalized fingerprint and complete coverage.

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

Executed on Linux, Node 24.18.0 / npm 11.16.0:

npm run test:focused -- src/dotnet/ManagedCilDecoding.test.ts src/dotnet/ManagedMemberInspector.test.ts — 220 passed.
npm run test:focused -- tests/acceptance/managedCliOutput.test.ts tests/boundary/providers/managed/managedMemberComparisonPartialBodies.test.ts — 9 passed; runtime build succeeded.
npm run check — passed typecheck, lint/module boundaries, formatting, knip, metadata checks, and architecture guards. Lint reports repository warnings, with zero errors.
npm run verify:test-discovery — all 1,415 test files discovered exactly once.
node scripts/rea.mjs inspect-managed-members <synthetic-dangling-prefix.dll> --json — exited 0 with the malformed/partial evidence above.
npm run check:fast — passed.
git diff --check — passed.

Compatibility, safety, and release

Breaking changes or migration steps: none; malformed input is classified more accurately through existing result fields.
Real Hopper/Ghidra, browser, or OS coverage: not run; this fix uses the built-in static managed parser. No target was executed. Synthetic PE fixtures only; real-world occurrence and MCP/other-host behavior were not independently tested.
Package or release metadata impact: none.
Security, privacy, process, or containment review: no new authority or external effects. The end-of-decode check uses existing state and does not add an input pass or allocations proportional to input size. No performance benchmark was run.

Author readiness

Human owner: @AbdulSamad94

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 (existing contracts already describe the required behavior).
User-visible CLI/MCP changes include representative output.
I checked the final diff for secrets, unrelated cleanup, and unsupported claims.

Maintainer blocker resolution

The distinct fat-header regression from duplicate #4528 is preserved in commit 3ab24789d2c4fd3e7cddd562036fe4a4f69ac6b3. It checks malformed status, retained opcode/count, absent normalized fingerprint and the dangling-chain diagnostic for a fat method body. On Node 24.18.0/npm 11.16.0, ManagedCilDecoding.test.ts passed all 90 tests and check:fast passed. This is decoder fixture coverage; no CLR/runtime execution was performed.

_{🔄 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/4416 **Author:** [@AbdulSamad94](https://github.com/AbdulSamad94) **Created:** 10/16/2026 **Status:** ✅ Merged **Merged:** 10/21/2026 **Merged by:** [@morluto](https://github.com/morluto) **Base:** `main` ← **Head:** `fix/4370-dangling-cil-prefix` --- ### 📝 Commits (2) - [`fdf71a1`](https://github.com/morluto/rea/commit/fdf71a1149d7d6a82ea18417310d25a7499834ca) fix(dotnet): reject unfinished CIL prefix chains - [`3ab2478`](https://github.com/morluto/rea/commit/3ab24789d2c4fd3e7cddd562036fe4a4f69ac6b3) test(dotnet): cover unfinished prefixes in fat method bodies ### 📊 Changes **3 files changed** (+131 additions, -0 deletions) <details> <summary>View changed files</summary> 📝 `src/dotnet/ManagedCilDecoding.test.ts` (+84 -0) 📝 `src/dotnet/ManagedMemberInstructionDecoder.ts` (+2 -0) 📝 `tests/acceptance/managedCliOutput.test.ts` (+45 -0) </details> ### 📄 Description ## Summary Fixes #4370. A CIL body ending in a complete prefix opcode (for example `FE 13`, `volatile.`) previously reported `present`, complete coverage, and a normalized fingerprint despite having no following instruction. It now reports a located malformed-body diagnostic and partial coverage, without losing the raw CIL digest or decoded facts. ## Problem and expected behavior Prefixes modify a following instruction. A terminal prefix chain must not qualify for the documented complete decoded-CIL fingerprint. The original `0A FE 13` tiny-body fixture reproduced through both the production inspector and the built CLI on base `d72d856`. ## Change and scope Check the decoder's existing pending-prefix state when decoding finishes, preserving an earlier failure if one exists. Existing body-reader and inspector consumers propagate the diagnostic, suppress the normalized hash, and mark member/user-string scan coverage partial. No downstream output repair or new abstraction is needed. General prefix/opcode compatibility and CIL stack/type verification remain outside this fix. ## Contract and boundary impact - Semantic owner and earliest changed stage: `decodeInstructions` in `src/dotnet/ManagedMemberInstructionDecoder.ts`. - Contribution scope fit and linked maintainer decision: routine static-parser correction within existing boundaries; advance scope decision not applicable. - CLI and MCP/tool-catalog contract: unchanged schemas; existing malformed/partial outcomes now represent unfinished prefix chains correctly. CLI exercised; MCP not separately exercised. - Provider, bridge, target-format, or platform compatibility: managed PE/CIL parser only; no external engine or bridge changes. - Evidence, artifact, provenance, or reconstruction contract: raw CIL hash, decoded instruction counts/opcodes/anchors, and artifact identity remain retained; incomplete bodies have no normalized fingerprint. - Target selection, authority, upstream behavior, and cleanup details: unchanged; static inspection only, no target execution or mutation. CLI fixture directories use the existing awaited test cleanup helper. - Generated metadata, package, or installation impact: none; no schema, catalog, version, or dependency changes. Expected effects: - [x] Read existing files or captured data. No additional process, injection, target mutation, network, or protection-recovery capability is introduced. ## Evidence and regression coverage - Production-inspector regressions cover all six supported prefixes, a multi-prefix chain, nonzero chain offsets, a later dangling chain after a completed one, valid completed chains, and preservation of existing truncated/unsupported opcode errors. - Built CLI regression checks malformed output, raw-hash retention, absent normalized hash, partial member/user-string coverage, and a valid `nop; ret` control. - Before the production fix: **8 new source cases failed**, with 81 other cases passing; **the new CLI case failed**, with 7 existing CLI cases passing. Failures showed the erroneous present/complete/hash outputs. - After the fix: all **229 selected tests passed** (220 source tests and 9 CLI/comparison-boundary tests). Executed CLI result for `0A FE 13`: ```json {"status":"malformed","normalized_il_sha256":null,"decoded_instruction_count":1,"opcode_counts":{"volatile.":1},"issue":"CIL prefix chain at IL offset 0 has no following instruction"} ``` Member and user-string coverage are `partial`; the raw CIL hash remains present. The valid control remains `present` with a normalized fingerprint and complete coverage. - [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 Executed on Linux, Node 24.18.0 / npm 11.16.0: - `npm run test:focused -- src/dotnet/ManagedCilDecoding.test.ts src/dotnet/ManagedMemberInspector.test.ts` — 220 passed. - `npm run test:focused -- tests/acceptance/managedCliOutput.test.ts tests/boundary/providers/managed/managedMemberComparisonPartialBodies.test.ts` — 9 passed; runtime build succeeded. - `npm run check` — passed typecheck, lint/module boundaries, formatting, knip, metadata checks, and architecture guards. Lint reports repository warnings, with zero errors. - `npm run verify:test-discovery` — all 1,415 test files discovered exactly once. - `node scripts/rea.mjs inspect-managed-members <synthetic-dangling-prefix.dll> --json` — exited 0 with the malformed/partial evidence above. - `npm run check:fast` — passed. - `git diff --check` — passed. ## Compatibility, safety, and release - Breaking changes or migration steps: none; malformed input is classified more accurately through existing result fields. - Real Hopper/Ghidra, browser, or OS coverage: not run; this fix uses the built-in static managed parser. No target was executed. Synthetic PE fixtures only; real-world occurrence and MCP/other-host behavior were not independently tested. - Package or release metadata impact: none. - Security, privacy, process, or containment review: no new authority or external effects. The end-of-decode check uses existing state and does not add an input pass or allocations proportional to input size. No performance benchmark was run. ## Author readiness Human owner: @AbdulSamad94 - [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 (existing contracts already describe the required behavior). - [x] User-visible CLI/MCP changes include representative output. - [x] I checked the final diff for secrets, unrelated cleanup, and unsupported claims.  ### Maintainer blocker resolution The distinct fat-header regression from duplicate #4528 is preserved in commit `3ab24789d2c4fd3e7cddd562036fe4a4f69ac6b3`. It checks malformed status, retained opcode/count, absent normalized fingerprint and the dangling-chain diagnostic for a fat method body. On Node 24.18.0/npm 11.16.0, ManagedCilDecoding.test.ts passed all 90 tests and check:fast passed. This is decoder fixture coverage; no CLR/runtime execution was performed. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi

2026-10-31 01:12:51 +00:00

closed this issue
added the
pull-request
label

bsakaguchi referenced this issue

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

[PR #6268] [MERGED] fix(doctor): interrupt Hopper display readiness on cancel #7240

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

No description provided.

Rows
Columns