morluto/rea

Fork 0

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

[GH-ISSUE #2677] fix(dotnet): operand truncation loses the CIL instruction context in body.issue #976

New issue

Closed

opened 2026-10-15 18:30:02 +00:00 by bsakaguchi · 1 comment

bsakaguchi commented

2026-10-15 18:30:02 +00:00

Owner

Originally created by @coygeek on GitHub (Oct 13, 2026).
Original GitHub issue: https://github.com/morluto/rea/issues/2677

Summary

When I inspect a PE/CLI method whose last instruction has an incomplete operand, the method is correctly marked malformed, but body.issue contains a Node Buffer error without the instruction name or IL offset. The decoder already has a format diagnostic for operands extending beyond the declared method body, and that diagnostic appears for a switch with missing targets. This is a diagnostic wording defect; malformed status, partial coverage and previously decoded instructions are preserved.

Steps to reproduce

Use the source checkout and pinned toolchain listed below, with locked dependencies already prepared. Save this test as src/dotnet/ManagedOperandDiagnostic.test.ts, then run npm run test:focused -- src/dotnet/ManagedOperandDiagnostic.test.ts. It constructs an owned PE/CLI fixture with a one-byte CIL stream containing ldc.i4 and no operand bytes. This short reproduction is extracted from the executed focused test; the displayed standalone command was not separately executed.

import { expect, it } from "vitest";
import { buildManagedPeFixture, managedPeFixtureTarget } from "./ManagedPe.fixture.js";
import { inspectManagedMembersBytes } from "./ManagedMemberInspector.js";

it("reports the reason for an incomplete CIL operand", () => {
  const bytes = buildManagedPeFixture({
    ilBody: Buffer.from([0x06, 0x20]),
  });
  const inspection = inspectManagedMembersBytes(bytes, managedPeFixtureTarget(bytes));
  const body = inspection.methods[0]!.body;
  console.log(JSON.stringify({
    status: body.status,
    decoded_instruction_count: body.decoded_instruction_count,
    issue: body.issue,
    coverage: inspection.coverage.state,
  }));
  expect(body.status).toBe("malformed");
});

The first byte is a tiny method header declaring one IL byte. The second is the ldc.i4 opcode, which needs a four-byte operand. The surrounding PE and CLI metadata remain intact. The same result was also observed through the real managed static provider after writing the fixture to an owned file and invoking inspect_managed_members with empty parameters. CLI and MCP transport execution was not run.

Expected behavior

body.issue should describe the incomplete CIL instruction and identify its IL offset, as the decoder's existing operand-extent diagnostic does. For this input, the existing wording would be Instruction ldc.i4 at IL offset 0 leaves method body. Equivalent format wording is sufficient. The method should remain malformed, coverage should remain partial, and the normalized IL hash should remain null.

Actual behavior

The source test and real static-provider execution both returned this diagnostic result:

{"status":"malformed","decoded_instruction_count":0,"issue":"Attempt to access memory outside buffer bounds","coverage":"partial"}

A fat-body fixture with IL [0x00, 0x20] also failed this way while preserving the preceding nop. Reducing it to a tiny body and then removing the nop preserved the failure. Short branch and variable operands returned an offset RangeError instead. A switch with a readable count of two and no target entries returned Instruction switch at IL offset 1 leaves method body; a switch missing its count returned the Buffer error. A complete nop; ldc.i4 1 control returned status present, two decoded instructions, complete coverage and no issue.

Evidence

Expected source: AGENTS.md, Architecture and Cleanup, requires preserving partial facts and meaningful failure reasons. src/dotnet/ManagedMemberInstructionDecoder.ts:410-411 defines the format diagnostic for an operand extending outside the IL body. The expectation concerns retained instruction context rather than an exact public string guarantee.
Failure source: the executed byte inspector and the rea-dotnet-static provider returned the Buffer error above. Locators: operand read at src/dotnet/ManagedMemberInstructionDecoder.ts:409, runtime-message capture at src/dotnet/ManagedMemberInstructionDecoder.ts:431, and body.issue at src/dotnet/ManagedMethodBodyReader.ts:175.
Evidence provenance: observed
Local verification: reproduced
Reproduction completeness: complete

Public entry points: the inspect-managed-members CLI command (src/cli/managedCommands.ts:57) and the inspect_managed_members MCP operation (src/contracts/managed/managedToolContracts.ts:87), both served by src/dotnet/ManagedStaticProvider.ts:208. The provider was executed directly; CLI and MCP transport were not.

Restoration check

For the one-byte IL input, return a format diagnostic naming ldc.i4 and IL offset 0 while retaining malformed status, partial coverage and a null normalized hash. For [0x00, 0x20], preserve the decoded nop and identify the incomplete instruction at offset 1. Keep the readable-count switch diagnostic and the valid nop; ldc.i4 1 control correct. A truncated switch count should also retain format context instead of the raw Buffer wording.

Runtime or environment

Executed against source revision a2d8b94dd50dab8d966239dd63ab84718aa8b4b4 with Node 24.18.0 and npm 11.16.0. The latest published package identified for this review is rea-agents 6.4.0; its runtime was not tested. A source comparison with its release tag found the relevant operand readers, extent guard and generic runtime-message capture unchanged, despite other decoder changes. No cross-version runtime comparison was executed.

A current upstream recheck at 3ae826ceba6f25b08bce50bcb51001209bc16b57 found the managed decoder, method-body reader, fixture builder, static provider and managed CLI/MCP contracts unchanged since the tested revision, so this defect remains present by source comparison. That recheck was source-only; no new source, package or CLI/MCP execution was performed.

Triage assessment

Impact level: P3
Assessment status: supported
Impact basis: Affected malformed methods lose the opcode and IL offset from the reason string, while status, coverage, hashes and the decoded prefix remain correct. No consequential analysis behavior change was observed.
Workaround status: none
Workaround basis: The member-inspection input schema offers path and type selection, with no option to obtain a different operand diagnostic. Callers can still identify malformed status, but that does not restore the missing instruction context.

Originally created by @coygeek on GitHub (Oct 13, 2026). Original GitHub issue: https://github.com/morluto/rea/issues/2677 ## Summary When I inspect a PE/CLI method whose last instruction has an incomplete operand, the method is correctly marked malformed, but `body.issue` contains a Node Buffer error without the instruction name or IL offset. The decoder already has a format diagnostic for operands extending beyond the declared method body, and that diagnostic appears for a `switch` with missing targets. This is a diagnostic wording defect; malformed status, partial coverage and previously decoded instructions are preserved. ## Steps to reproduce Use the source checkout and pinned toolchain listed below, with locked dependencies already prepared. Save this test as `src/dotnet/ManagedOperandDiagnostic.test.ts`, then run `npm run test:focused -- src/dotnet/ManagedOperandDiagnostic.test.ts`. It constructs an owned PE/CLI fixture with a one-byte CIL stream containing `ldc.i4` and no operand bytes. This short reproduction is extracted from the executed focused test; the displayed standalone command was not separately executed. ```ts import { expect, it } from "vitest"; import { buildManagedPeFixture, managedPeFixtureTarget } from "./ManagedPe.fixture.js"; import { inspectManagedMembersBytes } from "./ManagedMemberInspector.js"; it("reports the reason for an incomplete CIL operand", () => { const bytes = buildManagedPeFixture({ ilBody: Buffer.from([0x06, 0x20]), }); const inspection = inspectManagedMembersBytes(bytes, managedPeFixtureTarget(bytes)); const body = inspection.methods[0]!.body; console.log(JSON.stringify({ status: body.status, decoded_instruction_count: body.decoded_instruction_count, issue: body.issue, coverage: inspection.coverage.state, })); expect(body.status).toBe("malformed"); }); ``` The first byte is a tiny method header declaring one IL byte. The second is the `ldc.i4` opcode, which needs a four-byte operand. The surrounding PE and CLI metadata remain intact. The same result was also observed through the real managed static provider after writing the fixture to an owned file and invoking `inspect_managed_members` with empty parameters. CLI and MCP transport execution was not run. ## Expected behavior `body.issue` should describe the incomplete CIL instruction and identify its IL offset, as the decoder's existing operand-extent diagnostic does. For this input, the existing wording would be `Instruction ldc.i4 at IL offset 0 leaves method body`. Equivalent format wording is sufficient. The method should remain malformed, coverage should remain partial, and the normalized IL hash should remain null. ## Actual behavior The source test and real static-provider execution both returned this diagnostic result: ```json {"status":"malformed","decoded_instruction_count":0,"issue":"Attempt to access memory outside buffer bounds","coverage":"partial"} ``` A fat-body fixture with IL `[0x00, 0x20]` also failed this way while preserving the preceding `nop`. Reducing it to a tiny body and then removing the `nop` preserved the failure. Short branch and variable operands returned an offset RangeError instead. A `switch` with a readable count of two and no target entries returned `Instruction switch at IL offset 1 leaves method body`; a `switch` missing its count returned the Buffer error. A complete `nop; ldc.i4 1` control returned status `present`, two decoded instructions, complete coverage and no issue. ## Evidence - Expected source: `AGENTS.md`, Architecture and Cleanup, requires preserving partial facts and meaningful failure reasons. `src/dotnet/ManagedMemberInstructionDecoder.ts:410-411` defines the format diagnostic for an operand extending outside the IL body. The expectation concerns retained instruction context rather than an exact public string guarantee. - Failure source: the executed byte inspector and the `rea-dotnet-static` provider returned the Buffer error above. Locators: operand read at `src/dotnet/ManagedMemberInstructionDecoder.ts:409`, runtime-message capture at `src/dotnet/ManagedMemberInstructionDecoder.ts:431`, and `body.issue` at `src/dotnet/ManagedMethodBodyReader.ts:175`. - Evidence provenance: observed - Local verification: reproduced - Reproduction completeness: complete Public entry points: the `inspect-managed-members` CLI command (`src/cli/managedCommands.ts:57`) and the `inspect_managed_members` MCP operation (`src/contracts/managed/managedToolContracts.ts:87`), both served by `src/dotnet/ManagedStaticProvider.ts:208`. The provider was executed directly; CLI and MCP transport were not. ## Restoration check For the one-byte IL input, return a format diagnostic naming `ldc.i4` and IL offset 0 while retaining malformed status, partial coverage and a null normalized hash. For `[0x00, 0x20]`, preserve the decoded `nop` and identify the incomplete instruction at offset 1. Keep the readable-count `switch` diagnostic and the valid `nop; ldc.i4 1` control correct. A truncated switch count should also retain format context instead of the raw Buffer wording. ## Runtime or environment Executed against source revision `a2d8b94dd50dab8d966239dd63ab84718aa8b4b4` with Node 24.18.0 and npm 11.16.0. The latest published package identified for this review is `rea-agents` 6.4.0; its runtime was not tested. A source comparison with its release tag found the relevant operand readers, extent guard and generic runtime-message capture unchanged, despite other decoder changes. No cross-version runtime comparison was executed. A current upstream recheck at `3ae826ceba6f25b08bce50bcb51001209bc16b57` found the managed decoder, method-body reader, fixture builder, static provider and managed CLI/MCP contracts unchanged since the tested revision, so this defect remains present by source comparison. That recheck was source-only; no new source, package or CLI/MCP execution was performed. ## Triage assessment - Impact level: P3 - Assessment status: supported - Impact basis: Affected malformed methods lose the opcode and IL offset from the reason string, while status, coverage, hashes and the decoded prefix remain correct. No consequential analysis behavior change was observed. - Workaround status: none - Workaround basis: The member-inspection input schema offers path and type selection, with no option to obtain a different operand diagnostic. Callers can still identify malformed status, but that does not restore the missing instruction context.

bsakaguchi

2026-10-15 18:30:02 +00:00

closed this issue
added the
bug
label

bsakaguchi commented

2026-10-15 18:30:03 +00:00

Author

Owner

@YunzhouLi-hub commented on GitHub (Oct 13, 2026):

Thanks for the detailed report — the fixtures and the expected diagnostic made this straightforward to reproduce.

I picked this up in #2704. Root cause: the operand extent was checked after the operand was read, so the Buffer read threw a raw RangeError whose runtime message replaced the intended diagnostic.

On base 9cf49c7a7:

- "issue": "Instruction ldc.i4 at IL offset 0 leaves method body",
+ "issue": "Attempt to access memory outside buffer bounds",

Your reproduction is now a regression test in src/dotnet/ManagedCilDecoding.test.ts, along with the other 12 operand widths and the switch count field. status, coverage, decoded_instruction_count and the null normalized_il_sha256 are all unchanged — only the diagnostic text improves.

One thing I could not confirm and would like your input on: you mentioned the 3 pattern sites swept by #2384 in your #2343 report. Those were in the Go/native magic-byte readers and are already fixed there, so I believe they are unrelated to this one — but if you hit a similar body.issue in the managed path that names the wrong instruction, that would be worth a separate issue.

Validation on my side: 202/202 dotnet tests, check:fast 2/2, verify:test-discovery all 1038 files, oxfmt clean. No real .exe fixture was used — this is validated on the synthetic fixtures in src/dotnet/ManagedPe.fixture.ts, so if you can share the artifact (or its structure) I can add real-provider coverage.

@YunzhouLi-hub commented on GitHub (Oct 13, 2026): Thanks for the detailed report — the fixtures and the expected diagnostic made this straightforward to reproduce. I picked this up in #2704. Root cause: the operand extent was checked *after* the operand was read, so the `Buffer` read threw a raw `RangeError` whose runtime message replaced the intended diagnostic. On base `9cf49c7a7`: ``` - "issue": "Instruction ldc.i4 at IL offset 0 leaves method body", + "issue": "Attempt to access memory outside buffer bounds", ``` Your reproduction is now a regression test in `src/dotnet/ManagedCilDecoding.test.ts`, along with the other 12 operand widths and the `switch` count field. `status`, `coverage`, `decoded_instruction_count` and the null `normalized_il_sha256` are all unchanged — only the diagnostic text improves. One thing I could not confirm and would like your input on: you mentioned the 3 pattern sites swept by #2384 in your #2343 report. Those were in the Go/native magic-byte readers and are already fixed there, so I believe they are unrelated to this one — but if you hit a similar `body.issue` in the managed path that names the wrong instruction, that would be worth a separate issue. Validation on my side: 202/202 dotnet tests, `check:fast` 2/2, `verify:test-discovery` all 1038 files, `oxfmt` clean. No real `.exe` fixture was used — this is validated on the synthetic fixtures in `src/dotnet/ManagedPe.fixture.ts`, so if you can share the artifact (or its structure) I can add real-provider coverage.

bsakaguchi referenced this issue

2026-10-17 21:07:25 +00:00

[PR #976] [MERGED] fix(apple): traverse keyed archive hierarchy collections #2547

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

No description provided.

Rows
Columns