[PR #2162] [MERGED] feat(managed): decode ldstr user strings in member inspection #5067

New issue

Closed

opened 2026-10-31 01:02:19 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-31 01:02:19 +00:00

Owner

📋 Pull Request Information

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

Base: main ← Head: feat/managed-user-strings

📝 Commits (10+)

4e11582 feat(managed): decode ldstr user strings in member inspection
7280a73 fix(managed): collect user strings only from ldstr
a8e513a fix(managed): admit user-string tokens and failures before collecting them
2777308 docs(managed): describe missing #US reporting and the unvalidated flag byte
6a62d30 test(managed): cover #US items that run past the stream end
57c64b7 fix(managed): report reserved #US length prefixes as string failures
7f05157 fix(managed): append member inspection issues without argument spreading
1c20e2f fix(managed): classify truncated #US and #Blob length prefixes by item
d5b7b07 fix(managed): retain user-string terminal flag conformance issues
67fc5a5 test(managed): write ECMA-335 #US terminal flags and cover flag issues

📊 Changes

19 files changed (+1054 additions, -120 deletions)

View changed files

📝 docs/managed-code-analysis.md (+28 -2)
📝 src/application/managed/ManagedMemberComparisonService.ts (+5 -1)
📝 src/cli/managedCommands.ts (+2 -2)
📝 src/contracts/managed/managedToolContracts.ts (+2 -2)
📝 src/contracts/managed/managedWorkflowExamples.ts (+1 -0)
📝 src/domain/managed/managedArtifact.ts (+21 -0)
📝 src/domain/managed/managedMemberComparisonCoverage.ts (+3 -0)
📝 src/dotnet/ManagedMemberInspector.test.ts (+455 -2)
📝 src/dotnet/ManagedMemberInspector.ts (+29 -8)
📝 src/dotnet/ManagedMemberInspectorCore.ts (+3 -0)
📝 src/dotnet/ManagedMemberRows.ts (+31 -13)
📝 src/dotnet/ManagedMetadataHeaps.test.ts (+27 -1)
📝 src/dotnet/ManagedMetadataHeaps.ts (+157 -35)
📝 src/dotnet/ManagedMetadataLayout.ts (+3 -0)
📝 src/dotnet/ManagedMethodBodyReader.ts (+30 -16)
📝 src/dotnet/ManagedPe.fixture.ts (+102 -38)
📝 src/dotnet/ManagedReaderFailure.ts (+11 -0)
➕ src/dotnet/ManagedUserStrings.ts (+120 -0)
📝 tests/boundary/providers/managed/managedMemberComparisonFromMetadata.test.ts (+24 -0)

📄 Description

Note

🤖 Claude Opus 5.5 responding on behalf of Nick

Summary

Fixes #2158.

inspect_managed_members reported every ldstr operand as a bare 0x70 token, so an analyst could see that a method loads a literal but not which one. This adds a top-level user_strings inventory with its own coverage: { state, literals, issues }. literals holds each distinct ldstr token loaded by an inspected method body, with its #US file offset and decoded value.

Problem and expected behavior

Smallest trigger: any method containing ldstr. Its anchor shows operand_kind: "string" and operand: "0x70...", and no REA output contains the literal.

Expected:

The literal text is available inline.
Anchors and normalized_il_sha256 keep their build-local tokens.
A missing or malformed #US item stays a visible fact in user_strings.issues, with user_strings.state partial, and never makes member coverage partial.

Change and scope

Layout: ManagedMetadataLayout gains an optional userStrings stream (#US). Assemblies without literals may omit it, so it is not required.
Heap reader: readMetadataUserString decodes one #US item. It reads the compressed length, the UTF-16LE code units and the terminal flag byte, and rejects reserved or truncated length prefixes, even lengths, and items that leave the stream, each as invalid-string. The empty entry at offset 0 (ldstr 0x70000000, zero length, no flag byte) reads as "", and any other item at offset 0 is invalid-string. A length prefix wider than ECMA-335 II.23.2 assigns is reported as an invalid-string conformance issue beside the readable literal, like a differing terminal flag. The literal is admitted to the budget before its flag is scanned. The shared compressed-length reader takes the item it reads, so #Blob prefix failures stay invalid-blob. A terminal flag that differs from ECMA-335 II.24.2.4 is reported as invalid-string at the flag byte, and the readable literal is kept (morluto, d5b7b07). This is a conformance observation, not a runtime rejection: Mono-built framework assemblies differ from the spec on some literals. The retained value is charged to the representation budget at six JSON bytes per code unit, the same bound reserveString uses.
Inventory: parseUserStrings (src/dotnet/ManagedUserStrings.ts) collects the distinct operands of ldstr instructions (by opcode, so a malformed ldtoken with a 0x70 token is ignored) from the inspected bodies and decodes each once, in token order. Each token and each failure is charged to the decode budget before it is retained. A malformed item is reported against its token as metadata.#US:<token>, and a missing stream is one metadata.#US issue. An ldstr operand from another table is invalid-heap-index whether or not #US exists, and only 0x70 tokens count toward the missing-stream issue.
Discovery: the inspect_managed_members MCP description, its type_names input, and the CLI command and --type help name the decoded ldstr literals and say a type selection limits them to the selected types' method bodies.
Coverage: user strings own their coverage. #US issues go to user_strings.issues and user_strings.state, never to the member coverage or metadata.status, so the application graph's member facts and member comparisons are unaffected by literal conformance differences. user_strings.state is also partial when an inspected method's CIL was not fully decoded (the body reader reports instruction-scan completeness separately from exception-section validity) or a method that may belong to the inspected types was not inspected: an unreadable row, or a type whose name could not be read under a --type selection. This is judged by the method's owner, so an unreadable row outside the selection does not count, since only ldstr instructions in the inspected types matter.
Comparison: compare_managed_members inspects with user strings skipped, since it never reads literals, so a large literal cannot exhaust a comparison's decode budget. Its limitations state that literals are not compared. Inspection gathers producer issues with appendManagedParseIssues rather than spreading them into push, so a very large issue list returns partial coverage instead of overflowing the stack.
Schema: user_strings is { state, literals, issues }, nullable().default(null) following the exported_types precedent, so older Evidence reads it as not examined.
Selection: --type / type_names restricts user strings along with the methods. The limitation text says so.
Fixture: buildManagedPeFixture gains an optional raw userStringHeap. fixtureUserStringHeap lays out well-formed items and returns their tokens. Without the option, fixture bytes are unchanged.

Not included:

Inline values on anchors. That would repeat a literal at every use and change the anchor shape.
Changes to the normalized CIL digest.
String search across assemblies.
Budget changes.

Contract and boundary impact

Semantic owner and earliest changed stage: src/dotnet metadata layout and heap reader. The member inspection projection adds one field.
CLI and MCP/tool-catalog contract: additive output field user_strings on inspect_managed_members. No input changes.
Provider, bridge, target-format, or platform compatibility: none.
Evidence, artifact, provenance, or reconstruction contract: additive and nullable. Older Evidence parses with user_strings: null. Anchors and normalized_il_sha256 are unchanged, and a test asserts the digest is identical with and without #US.
Process execution, authorization, cleanup, or containment impact: none. Static parsing only.
Generated metadata, package, or installation impact: none committed. CI regenerates the catalog.

Evidence and regression coverage

Tests added or updated, all in src/dotnet/ManagedMemberInspector.test.ts:
- Literals decode once each in token order: ASCII, non-BMP, and an unpaired surrogate. An unreferenced literal is excluded. Anchors keep tokens, and normalized_il_sha256 matches the same IL built without #US.
- A missing #US stream gives partial user-string coverage with one missing-stream issue for all tokens, while member coverage stays complete. The method is kept.
- With 1 KiB of budget left, 100 distinct tokens raise resource-limit before they are collected.
- A malformed even-length item and a reserved length prefix give invalid-string, and an out-of-range index gives invalid-heap-index. Readable literals remain.
- An odd length whose payload crosses the end of #US, and a two-byte length prefix starting on the last #US byte, are both rejected as "User string leaves #US" beside a readable literal.
- A 0x70 token behind ldtoken is not collected.
- Unavailable CLI metadata and legacy Evidence give user_strings: null.
Base reproduction: on main, PEAK's Peak.Quicksave (Unity Mono Assembly-CSharp.dll, 1.6 MB, --type Peak.Quicksave) shows 12 ldstr anchors and no literal text.
User-visible CLI output, same command on this branch, with complete coverage:
```
0x7000e552  "Can't load save data because it doesn't exist."
0x7000e5b0  "Saving game to app data folder as "
0x7000e5f6  "Failed to create save file: {0}"
...
0x7000e875  "quicksave.peak"
```
A whole-assembly run on a 247 KB Unity Assembly-CSharp.dll stays complete with 254 literals. Its JSON grows by about 0.2%.
Remaining proof gaps: no real-provider lane applies. No .NET SDK compiler was used. Fixtures are source-built and the real assemblies above are third-party game binaries that are not committed.

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

All runs used Node 24.18.0 and npm 11.16.0, per .nvmrc and packageManager.

npm run check: passed, including architecture guards.
npm run docs:check: passed.
npm run test:focused -- on all 26 managed test files across src/dotnet, src/domain/managed, src/contracts/managed, tests/boundary/providers/managed, managed MCP, tool-schema validity, managed acceptance, managed application graph, and CLI JSON output: 307 passed.
Built CLI against PEAK Assembly-CSharp.dll with --type Peak.Quicksave: complete coverage, 12 user strings.

Compatibility, safety, and release

Breaking changes or migration steps: none. The field is additive and nullable.
Real Hopper/Ghidra, browser, or OS coverage: not applicable. Linux x64 only.
Package or release metadata impact: none.
Security, privacy, process, or containment review: literal values come from the caller's artifact and are returned like other metadata strings. Nothing is executed. The budget accounts for retained values.

Review checklist

This PR addresses a concrete problem or an agreed enhancement.
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 branch merges main after #2163 landed; user strings and edge-target naming run side by side, and every issue list is appended with appendManagedParseIssues.

🤖 Generated with Claude Code

_{🔄 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/2162 **Author:** [@Nelglor](https://github.com/Nelglor) **Created:** 10/12/2026 **Status:** ✅ Merged **Merged:** 10/13/2026 **Merged by:** [@morluto](https://github.com/morluto) **Base:** `main` ← **Head:** `feat/managed-user-strings` --- ### 📝 Commits (10+) - [`4e11582`](https://github.com/morluto/rea/commit/4e11582923fb43e2d84331ded6b60342671cfdb9) feat(managed): decode ldstr user strings in member inspection - [`7280a73`](https://github.com/morluto/rea/commit/7280a731cc657740a5a97e7d827cf821c40038bf) fix(managed): collect user strings only from ldstr - [`a8e513a`](https://github.com/morluto/rea/commit/a8e513ac4f528e9c021aa600412b3ac0d41cc66b) fix(managed): admit user-string tokens and failures before collecting them - [`2777308`](https://github.com/morluto/rea/commit/2777308037e7d4d9df53420439cad661106c6e74) docs(managed): describe missing #US reporting and the unvalidated flag byte - [`6a62d30`](https://github.com/morluto/rea/commit/6a62d30b2b09459df0a3b75c170f1e6686c91ee2) test(managed): cover #US items that run past the stream end - [`57c64b7`](https://github.com/morluto/rea/commit/57c64b755ecd5ecba5e8cd579cc820bd082b5e1e) fix(managed): report reserved #US length prefixes as string failures - [`7f05157`](https://github.com/morluto/rea/commit/7f0515756253e712ae80a72b402b2a2ccb7155c0) fix(managed): append member inspection issues without argument spreading - [`1c20e2f`](https://github.com/morluto/rea/commit/1c20e2f511d1c4dbbcd4a3774eda50a4d7d85a9b) fix(managed): classify truncated #US and #Blob length prefixes by item - [`d5b7b07`](https://github.com/morluto/rea/commit/d5b7b0704b374e30a410eb3871f68b30773d56bf) fix(managed): retain user-string terminal flag conformance issues - [`67fc5a5`](https://github.com/morluto/rea/commit/67fc5a5143c544665d8699ef3d004273dfac412d) test(managed): write ECMA-335 #US terminal flags and cover flag issues ### 📊 Changes **19 files changed** (+1054 additions, -120 deletions) <details> <summary>View changed files</summary> 📝 `docs/managed-code-analysis.md` (+28 -2) 📝 `src/application/managed/ManagedMemberComparisonService.ts` (+5 -1) 📝 `src/cli/managedCommands.ts` (+2 -2) 📝 `src/contracts/managed/managedToolContracts.ts` (+2 -2) 📝 `src/contracts/managed/managedWorkflowExamples.ts` (+1 -0) 📝 `src/domain/managed/managedArtifact.ts` (+21 -0) 📝 `src/domain/managed/managedMemberComparisonCoverage.ts` (+3 -0) 📝 `src/dotnet/ManagedMemberInspector.test.ts` (+455 -2) 📝 `src/dotnet/ManagedMemberInspector.ts` (+29 -8) 📝 `src/dotnet/ManagedMemberInspectorCore.ts` (+3 -0) 📝 `src/dotnet/ManagedMemberRows.ts` (+31 -13) 📝 `src/dotnet/ManagedMetadataHeaps.test.ts` (+27 -1) 📝 `src/dotnet/ManagedMetadataHeaps.ts` (+157 -35) 📝 `src/dotnet/ManagedMetadataLayout.ts` (+3 -0) 📝 `src/dotnet/ManagedMethodBodyReader.ts` (+30 -16) 📝 `src/dotnet/ManagedPe.fixture.ts` (+102 -38) 📝 `src/dotnet/ManagedReaderFailure.ts` (+11 -0) ➕ `src/dotnet/ManagedUserStrings.ts` (+120 -0) 📝 `tests/boundary/providers/managed/managedMemberComparisonFromMetadata.test.ts` (+24 -0) </details> ### 📄 Description > [!NOTE] > 🤖 **Claude Opus 5.5 responding on behalf of Nick** ## Summary Fixes #2158. `inspect_managed_members` reported every `ldstr` operand as a bare `0x70` token, so an analyst could see that a method loads a literal but not which one. This adds a top-level `user_strings` inventory with its own coverage: `{ state, literals, issues }`. `literals` holds each distinct `ldstr` token loaded by an inspected method body, with its `#US` file offset and decoded value. ## Problem and expected behavior Smallest trigger: any method containing `ldstr`. Its anchor shows `operand_kind: "string"` and `operand: "0x70..."`, and no REA output contains the literal. Expected: - The literal text is available inline. - Anchors and `normalized_il_sha256` keep their build-local tokens. - A missing or malformed `#US` item stays a visible fact in `user_strings.issues`, with `user_strings.state` partial, and never makes member coverage partial. ## Change and scope - **Layout:** `ManagedMetadataLayout` gains an optional `userStrings` stream (`#US`). Assemblies without literals may omit it, so it is not required. - **Heap reader:** `readMetadataUserString` decodes one `#US` item. It reads the compressed length, the UTF-16LE code units and the terminal flag byte, and rejects reserved or truncated length prefixes, even lengths, and items that leave the stream, each as `invalid-string`. The empty entry at offset 0 (`ldstr 0x70000000`, zero length, no flag byte) reads as `""`, and any other item at offset 0 is `invalid-string`. A length prefix wider than ECMA-335 II.23.2 assigns is reported as an `invalid-string` conformance issue beside the readable literal, like a differing terminal flag. The literal is admitted to the budget before its flag is scanned. The shared compressed-length reader takes the item it reads, so #Blob prefix failures stay `invalid-blob`. A terminal flag that differs from ECMA-335 II.24.2.4 is reported as `invalid-string` at the flag byte, and the readable literal is kept (morluto, d5b7b07). This is a conformance observation, not a runtime rejection: Mono-built framework assemblies differ from the spec on some literals. The retained value is charged to the representation budget at six JSON bytes per code unit, the same bound `reserveString` uses. - **Inventory:** `parseUserStrings` (`src/dotnet/ManagedUserStrings.ts`) collects the distinct operands of `ldstr` instructions (by opcode, so a malformed `ldtoken` with a `0x70` token is ignored) from the inspected bodies and decodes each once, in token order. Each token and each failure is charged to the decode budget before it is retained. A malformed item is reported against its token as `metadata.#US:<token>`, and a missing stream is one `metadata.#US` issue. An `ldstr` operand from another table is `invalid-heap-index` whether or not `#US` exists, and only `0x70` tokens count toward the missing-stream issue. - **Discovery:** the `inspect_managed_members` MCP description, its `type_names` input, and the CLI command and `--type` help name the decoded `ldstr` literals and say a type selection limits them to the selected types' method bodies. - **Coverage:** user strings own their coverage. #US issues go to `user_strings.issues` and `user_strings.state`, never to the member `coverage` or `metadata.status`, so the application graph's member facts and member comparisons are unaffected by literal conformance differences. `user_strings.state` is also partial when an inspected method's CIL was not fully decoded (the body reader reports instruction-scan completeness separately from exception-section validity) or a method that may belong to the inspected types was not inspected: an unreadable row, or a type whose name could not be read under a `--type` selection. This is judged by the method's owner, so an unreadable row outside the selection does not count, since only `ldstr` instructions in the inspected types matter. - **Comparison:** `compare_managed_members` inspects with user strings skipped, since it never reads literals, so a large literal cannot exhaust a comparison's decode budget. Its limitations state that literals are not compared. Inspection gathers producer issues with `appendManagedParseIssues` rather than spreading them into `push`, so a very large issue list returns partial coverage instead of overflowing the stack. - **Schema:** `user_strings` is `{ state, literals, issues }`, `nullable().default(null)` following the `exported_types` precedent, so older Evidence reads it as not examined. - **Selection:** `--type` / `type_names` restricts user strings along with the methods. The limitation text says so. - **Fixture:** `buildManagedPeFixture` gains an optional raw `userStringHeap`. `fixtureUserStringHeap` lays out well-formed items and returns their tokens. Without the option, fixture bytes are unchanged. Not included: - Inline values on anchors. That would repeat a literal at every use and change the anchor shape. - Changes to the normalized CIL digest. - String search across assemblies. - Budget changes. ## Contract and boundary impact - Semantic owner and earliest changed stage: `src/dotnet` metadata layout and heap reader. The member inspection projection adds one field. - CLI and MCP/tool-catalog contract: additive output field `user_strings` on `inspect_managed_members`. No input changes. - Provider, bridge, target-format, or platform compatibility: none. - Evidence, artifact, provenance, or reconstruction contract: additive and nullable. Older Evidence parses with `user_strings: null`. Anchors and `normalized_il_sha256` are unchanged, and a test asserts the digest is identical with and without `#US`. - Process execution, authorization, cleanup, or containment impact: none. Static parsing only. - Generated metadata, package, or installation impact: none committed. CI regenerates the catalog. ## Evidence and regression coverage - Tests added or updated, all in `src/dotnet/ManagedMemberInspector.test.ts`: - Literals decode once each in token order: ASCII, non-BMP, and an unpaired surrogate. An unreferenced literal is excluded. Anchors keep tokens, and `normalized_il_sha256` matches the same IL built without `#US`. - A missing `#US` stream gives partial user-string coverage with one `missing-stream` issue for all tokens, while member coverage stays complete. The method is kept. - With 1 KiB of budget left, 100 distinct tokens raise `resource-limit` before they are collected. - A malformed even-length item and a reserved length prefix give `invalid-string`, and an out-of-range index gives `invalid-heap-index`. Readable literals remain. - An odd length whose payload crosses the end of `#US`, and a two-byte length prefix starting on the last `#US` byte, are both rejected as "User string leaves #US" beside a readable literal. - A `0x70` token behind `ldtoken` is not collected. - Unavailable CLI metadata and legacy Evidence give `user_strings: null`. - Base reproduction: on `main`, PEAK's `Peak.Quicksave` (Unity Mono `Assembly-CSharp.dll`, 1.6 MB, `--type Peak.Quicksave`) shows 12 `ldstr` anchors and no literal text. - User-visible CLI output, same command on this branch, with complete coverage: ```text 0x7000e552 "Can't load save data because it doesn't exist." 0x7000e5b0 "Saving game to app data folder as " 0x7000e5f6 "Failed to create save file: {0}" ... 0x7000e875 "quicksave.peak" ``` A whole-assembly run on a 247 KB Unity `Assembly-CSharp.dll` stays `complete` with 254 literals. Its JSON grows by about 0.2%. - Remaining proof gaps: no real-provider lane applies. No `.NET` SDK compiler was used. Fixtures are source-built and the real assemblies above are third-party game binaries that are not committed. 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 All runs used Node 24.18.0 and npm 11.16.0, per `.nvmrc` and `packageManager`. - `npm run check`: passed, including architecture guards. - `npm run docs:check`: passed. - `npm run test:focused --` on all 26 managed test files across `src/dotnet`, `src/domain/managed`, `src/contracts/managed`, `tests/boundary/providers/managed`, managed MCP, tool-schema validity, managed acceptance, managed application graph, and CLI JSON output: 307 passed. - Built CLI against PEAK `Assembly-CSharp.dll` with `--type Peak.Quicksave`: complete coverage, 12 user strings. ## Compatibility, safety, and release - Breaking changes or migration steps: none. The field is additive and nullable. - Real Hopper/Ghidra, browser, or OS coverage: not applicable. Linux x64 only. - Package or release metadata impact: none. - Security, privacy, process, or containment review: literal values come from the caller's artifact and are returned like other metadata strings. Nothing is executed. The budget accounts for retained values. ## Review checklist - [x] This PR addresses a concrete problem or an agreed enhancement. - [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. This branch merges `main` after #2163 landed; user strings and edge-target naming run side by side, and every issue list is appended with `appendManagedParseIssues`. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi

2026-10-31 01:02:19 +00:00