[PR #2912] [MERGED] fix(managed): retain raw CIL changes in member comparison #5490

New issue

Closed

opened 2026-10-31 01:07:03 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-31 01:07:03 +00:00

Owner

📋 Pull Request Information

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

Base: main ← Head: Vaishnavi220506/fix-managed-cil-identity

📝 Commits (1)

68ecd28 fix(managed): retain raw CIL changes in member comparison

📊 Changes

4 files changed (+78 additions, -9 deletions)

View changed files

📝 docs/managed-code-analysis.md (+12 -7)
📝 src/domain/managed/managedMemberComparisonItems.ts (+5 -1)
📝 src/domain/managed/managedMemberComparisonMatch.ts (+4 -1)
📝 tests/boundary/providers/managed/managedMemberComparisonFromMetadata.test.ts (+57 -0)

📄 Description

Summary

Report raw CIL changes in managed member comparisons, even when the decoded fingerprints are equal. Reordering a switch table currently produces an exact, unchanged method match despite changing which value each case returns.

Problem and expected behavior

A static int Main(int) with ldarg.0; switch; ldc.i4.0; ret; ldc.i4.1; ret has switch offsets [0, 2] in the baseline and [2, 0] in the candidate. Both targets are valid instruction starts. The decoder intentionally fingerprints only the switch case count, so both normalized hashes are equal while the raw CIL hashes differ.

Expected: pair the methods and report a cil change. Previously: status: "unchanged", basis: "exact-il-signature". This was found from the comparison code and reproduced with authored PE fixtures; there is no linked issue.

Change and scope

Include the existing raw CIL digest in the exact matching key and in complete-body change detection. Preserve the decoded fingerprint format and the declared-signature/structural fallback tiers. Add regressions for both same-name and renamed methods, with identical-body controls, and document the distinction between raw identity and decoded fingerprints.

Follow the contribution scope. A maintainer scope decision is not applicable: this fixes existing managed static comparison behavior.

Contract and boundary impact

Semantic owner and earliest changed stage: managed domain member matching and comparison dimensions; existing parsed digests are reused.
Contribution scope fit and linked maintainer decision: ordinary correctness fix; not applicable.
CLI and MCP/tool-catalog contract: no schema, arguments, or catalog changes; changed CIL now produces changed and the cil dimension.
Provider, bridge, target-format, or platform compatibility: existing static PE/CLI observations; no provider or bridge changes.
Evidence, artifact, provenance, or reconstruction contract: evidence links and the v1 normalized fingerprint remain unchanged; raw CIL differences no longer disappear.
Target selection, authority, upstream behavior, and cleanup details: existing read-only file analysis; no changes.
Generated metadata, package, or installation impact: none; no build outputs included.

Expected effects:

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.

Evidence and regression coverage

Tests added: real fixture bytes pass through inspectManagedMembersBytes and the domain comparator. Same-name and renamed switch changes are detected; identical bodies still match exactly.
Base reproduction: at 928b4d5fe4523e3226aeb865f105e3f711bc8ed3, both new tests fail with exact_il_signature = 1; the other 16 tests in that file pass. Equal normalized hashes and differing raw hashes are asserted before the comparison.
User-visible CLI output: executed node scripts/rea.mjs compare-managed-members ../_rea-flagship/left.dll ../_rea-flagship/right.dll --json --full-output. Relevant output:

{"status":"changed","dimensions":["cil"],"match":{"status":"matched","basis":"exact-signature","confidence":"exact"}}

Remaining proof gaps: fixtures were inspected statically; target assemblies were not executed. Full repository test suite was not run.
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

Node 24.18.0 / npm 11.16.0 on Windows; HEAD a002ac1699518111d2f4696ea5f2f73ff9bfd992.

npm run test:focused -- tests/boundary/providers/managed/managedMemberComparison.test.ts tests/boundary/providers/managed/managedMemberComparisonFromMetadata.test.ts tests/boundary/providers/managed/managedMemberComparisonPartialBodies.test.ts tests/boundary/providers/managed/managedDirectAnalysis.test.ts tests/boundary/mcp/managedMcp.test.ts tests/acceptance/managedCliOutput.test.ts — 43 tests pass across six files, including real static-provider CLI and MCP paths. Earlier current-HEAD runs hit two existing 30-second timeouts; the final rerun passes without changing code or timeouts.
npm run check — passes, including type checking, lint, formatting, unused-code and metadata checks, module boundaries, and architecture guards. Latest main fixed the earlier baseline Ghidra lint error.
npm run docs:check — passes, including the documentation build and five README consistency tests.
git diff --check and staged formatting/lint hooks — pass.

Compatibility, safety, and release

Breaking changes or migration steps: none to the schema or fingerprint format. Raw byte differences are intentionally visible as cil changes, including differences lost by decoded operand rendering.
Real Hopper/Ghidra, browser, or OS coverage: static managed provider tested on Windows; native reverse-engineering providers are outside this change.
Package or release metadata impact: none.
Security, privacy, process, or containment review: read-only managed analysis; no target execution or added network access.

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/2912 **Author:** [@Vaishnavi220506](https://github.com/Vaishnavi220506) **Created:** 10/13/2026 **Status:** ✅ Merged **Merged:** 10/13/2026 **Merged by:** [@morluto](https://github.com/morluto) **Base:** `main` ← **Head:** `Vaishnavi220506/fix-managed-cil-identity` --- ### 📝 Commits (1) - [`68ecd28`](https://github.com/morluto/rea/commit/68ecd28bf49c19b31cc1e00e2017bffe3a1d2a3f) fix(managed): retain raw CIL changes in member comparison ### 📊 Changes **4 files changed** (+78 additions, -9 deletions) <details> <summary>View changed files</summary> 📝 `docs/managed-code-analysis.md` (+12 -7) 📝 `src/domain/managed/managedMemberComparisonItems.ts` (+5 -1) 📝 `src/domain/managed/managedMemberComparisonMatch.ts` (+4 -1) 📝 `tests/boundary/providers/managed/managedMemberComparisonFromMetadata.test.ts` (+57 -0) </details> ### 📄 Description ## Summary Report raw CIL changes in managed member comparisons, even when the decoded fingerprints are equal. Reordering a `switch` table currently produces an exact, unchanged method match despite changing which value each case returns. ## Problem and expected behavior A static `int Main(int)` with `ldarg.0; switch; ldc.i4.0; ret; ldc.i4.1; ret` has switch offsets `[0, 2]` in the baseline and `[2, 0]` in the candidate. Both targets are valid instruction starts. The decoder intentionally fingerprints only the switch case count, so both normalized hashes are equal while the raw CIL hashes differ. Expected: pair the methods and report a `cil` change. Previously: `status: "unchanged"`, `basis: "exact-il-signature"`. This was found from the comparison code and reproduced with authored PE fixtures; there is no linked issue. ## Change and scope Include the existing raw CIL digest in the exact matching key and in complete-body change detection. Preserve the decoded fingerprint format and the declared-signature/structural fallback tiers. Add regressions for both same-name and renamed methods, with identical-body controls, and document the distinction between raw identity and decoded fingerprints. Follow the [contribution scope](https://github.com/morluto/rea/blob/main/CONTRIBUTING.md#contribution-scope). A maintainer scope decision is not applicable: this fixes existing managed static comparison behavior. ## Contract and boundary impact - Semantic owner and earliest changed stage: managed domain member matching and comparison dimensions; existing parsed digests are reused. - Contribution scope fit and linked maintainer decision: ordinary correctness fix; not applicable. - CLI and MCP/tool-catalog contract: no schema, arguments, or catalog changes; changed CIL now produces `changed` and the `cil` dimension. - Provider, bridge, target-format, or platform compatibility: existing static PE/CLI observations; no provider or bridge changes. - Evidence, artifact, provenance, or reconstruction contract: evidence links and the v1 normalized fingerprint remain unchanged; raw CIL differences no longer disappear. - Target selection, authority, upstream behavior, and cleanup details: existing read-only file analysis; no changes. - Generated metadata, package, or installation impact: none; no build outputs included. Expected effects: - [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. ## Evidence and regression coverage - Tests added: real fixture bytes pass through `inspectManagedMembersBytes` and the domain comparator. Same-name and renamed switch changes are detected; identical bodies still match exactly. - Base reproduction: at `928b4d5fe4523e3226aeb865f105e3f711bc8ed3`, both new tests fail with `exact_il_signature = 1`; the other 16 tests in that file pass. Equal normalized hashes and differing raw hashes are asserted before the comparison. - User-visible CLI output: executed `node scripts/rea.mjs compare-managed-members ../_rea-flagship/left.dll ../_rea-flagship/right.dll --json --full-output`. Relevant output: ```json {"status":"changed","dimensions":["cil"],"match":{"status":"matched","basis":"exact-signature","confidence":"exact"}} ``` - Remaining proof gaps: fixtures were inspected statically; target assemblies were not executed. Full repository test suite was not run. - [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 Node 24.18.0 / npm 11.16.0 on Windows; HEAD `a002ac1699518111d2f4696ea5f2f73ff9bfd992`. - `npm run test:focused -- tests/boundary/providers/managed/managedMemberComparison.test.ts tests/boundary/providers/managed/managedMemberComparisonFromMetadata.test.ts tests/boundary/providers/managed/managedMemberComparisonPartialBodies.test.ts tests/boundary/providers/managed/managedDirectAnalysis.test.ts tests/boundary/mcp/managedMcp.test.ts tests/acceptance/managedCliOutput.test.ts` — 43 tests pass across six files, including real static-provider CLI and MCP paths. Earlier current-HEAD runs hit two existing 30-second timeouts; the final rerun passes without changing code or timeouts. - `npm run check` — passes, including type checking, lint, formatting, unused-code and metadata checks, module boundaries, and architecture guards. Latest main fixed the earlier baseline Ghidra lint error. - `npm run docs:check` — passes, including the documentation build and five README consistency tests. - `git diff --check` and staged formatting/lint hooks — pass. ## Compatibility, safety, and release - Breaking changes or migration steps: none to the schema or fingerprint format. Raw byte differences are intentionally visible as `cil` changes, including differences lost by decoded operand rendering. - Real Hopper/Ghidra, browser, or OS coverage: static managed provider tested on Windows; native reverse-engineering providers are outside this change. - Package or release metadata impact: none. - Security, privacy, process, or containment review: read-only managed analysis; no target execution or added network access. ## 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. - [ ] 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-10-31 01:07:03 +00:00