[PR #6230] [MERGED] fix(evm): reject repeated container keys in Solidity metadata #7213

New issue

Closed

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

bsakaguchi commented

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

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/6230
Author: @Sriram-PR
Created: 10/22/2026
Status: ✅ Merged
Merged: 10/23/2026
Merged by: @baron

Base: main ← Head: fix/evm-metadata-repeated-container-keys

📝 Commits (2)

24ee936 fix(evm): reject repeated container keys in Solidity metadata
5832352 test(evm): check the repeat-key diagnostic for deeply nested keys

📊 Changes

2 files changed (+36 additions, -12 deletions)

View changed files

📝 src/evm/SolidityMetadata.test.ts (+18 -0)
📝 src/evm/SolidityMetadata.ts (+18 -12)

📄 Description

Summary

After #4024, a terminal CBOR root map with a repeated byte-string key is not-decoded, but a root map whose repeated key is an array or a map still comes back partial with both entries, and the only diagnostic is the nested-container note. #4024 skipped uninterpreted keys in its comparison because their projection ({ kind: "uninterpreted", container: "map" }) can't tell two different containers apart. coygeek and XinwangTao pointed this case out on #3991.

Refs #3991, follow-up to #4024. No separate issue; this is the case #4024 left out.

Problem and expected behavior

Smallest trigger: the bytecode 608060405200a2800180020005 (the 5-byte map {[]: 1, []: 2}, length 0005), or 608060405200a2a001a0020005 for {{}: 1, {}: 2}. inspect-evm-interface <file> hex --json returned metadata status: "partial", 2 entries, diagnostic "Nested containers are uninterpreted; ...". Expected: not-decoded with a repeat-key diagnostic, like repeated text and byte-string keys.

Change and scope

src/evm/SolidityMetadata.ts: duplicate detection now compares every root key by value through a small keyIdentity helper. Scalars use the same JSON.stringify(scalar(value)) identity as before, so text, integer, bytes, boolean and null repeats give the same diagnostics as on main. Arrays compare element by element; maps compare as an unordered set of entries, since CBOR map order isn't significant. The identity is built by plain string concatenation, so its size is linear in the key's encoded size even at the 64-level nesting limit.
Comparing by decoded value matches how root scalar keys are already compared (integers by value, bytes by hex), rather than by exact encoded bytes.
Output is unchanged for everything else: containers are still projected as uninterpreted, and two different container keys still give partial. The helper is only used for the repeat check.
Not included: repeated keys inside nested maps, which this profile leaves uninterpreted.

Contribution scope: ordinary fix within existing boundaries; no scope decision needed.

Contract and boundary impact

Semantic owner and earliest changed stage: inspectSolidityMetadata in src/evm/SolidityMetadata.ts, when the root map's entries are projected.
Contribution scope fit and linked maintainer decision: not applicable (follow-up bug fix to #4024 / #3991).
CLI and MCP/tool-catalog contract: no schema change. inspect-evm-interface / inspect_evm_interface report not-decoded with a diagnostic for a root map with repeated container keys.
Provider, bridge, target-format, or platform compatibility: none; the EVMole engine and its raw_result are untouched.
Evidence, artifact, provenance, or reconstruction contract: bytecode.metadata only; the original bytecode and recorded range are unchanged.
Target selection, authority, upstream behavior, and cleanup details: not applicable.
Generated metadata, package, or installation impact: 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.

Evidence and regression coverage

Tests added or updated (src/evm/SolidityMetadata.test.ts):
- the malformed-candidate table gains duplicate array key (a280018002), duplicate map key (a2a001a002) and duplicate map key in another order ({{1: 2, 3: 4}: 1, {3: 4, 1: 2}: 2})
- the distinct-keys control adds [[]] vs [{}], which must stay partial
- a 62-level nested array key repeated twice must be not-decoded in under a second
Base reproduction: on the base revision all 4 new cases fail with expected { ... } to match object { status: 'not-decoded', ... } (base returns partial); the other 21 tests in the file pass on both.

User-visible CLI output (inspect-evm-interface <file> hex --json, bytecode.metadata status / entry count / diagnostic, Linux x64):

CBOR map         base                            PR
a280018002       partial, 2, nested-container    not-decoded, 0, CBOR decode error: found repeat map key []
a2a001a002       partial, 2, nested-container    not-decoded, 0, CBOR decode error: found repeat map key {}
a2810101810202   (not run)                       partial, 2, nested-container

Remaining proof gaps: synthetic fixtures only; no real contract artifact with container keys was tried.

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

npx vitest run src/evm/SolidityMetadata.test.ts: 25 passed (4 fail on the base revision, see above)
npx vitest run src/evm tests/boundary/mcp/evmInterfaceMcp.test.ts: 6 files, 74 passed
npx vitest run tests/boundary/evm tests/composition/evmModuleContract.test.ts tests/boundary/mcp/evmLimiterAvailability.test.ts: 6 files, 62 passed, 2 skipped
npm run check:fast: typecheck and lint passed
CLI comparison above, built with npm run build:cached

Compatibility, safety, and release

Breaking changes or migration steps: none. Root maps with repeated container keys move from partial to not-decoded.
Real Hopper/Ghidra, browser, or OS coverage: Linux x64, Node 24.18.0; no provider involved.
Package or release metadata impact: none.
Security, privacy, process, or containment review: none.

Author readiness

Human owner: @Sriram-PR

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/6230 **Author:** [@Sriram-PR](https://github.com/Sriram-PR) **Created:** 10/22/2026 **Status:** ✅ Merged **Merged:** 10/23/2026 **Merged by:** [@baron](https://github.com/baron) **Base:** `main` ← **Head:** `fix/evm-metadata-repeated-container-keys` --- ### 📝 Commits (2) - [`24ee936`](https://github.com/morluto/rea/commit/24ee936c865c9e8ad2144b6ff30a971549e18d8b) fix(evm): reject repeated container keys in Solidity metadata - [`5832352`](https://github.com/morluto/rea/commit/5832352246a7e6916419cef8a72d1b14b4c7fdd4) test(evm): check the repeat-key diagnostic for deeply nested keys ### 📊 Changes **2 files changed** (+36 additions, -12 deletions) <details> <summary>View changed files</summary> 📝 `src/evm/SolidityMetadata.test.ts` (+18 -0) 📝 `src/evm/SolidityMetadata.ts` (+18 -12) </details> ### 📄 Description ## Summary After #4024, a terminal CBOR root map with a repeated byte-string key is `not-decoded`, but a root map whose repeated key is an array or a map still comes back `partial` with both entries, and the only diagnostic is the nested-container note. #4024 skipped `uninterpreted` keys in its comparison because their projection (`{ kind: "uninterpreted", container: "map" }`) can't tell two different containers apart. coygeek and XinwangTao pointed this case out on #3991. Refs #3991, follow-up to #4024. No separate issue; this is the case #4024 left out. ## Problem and expected behavior Smallest trigger: the bytecode `608060405200a2800180020005` (the 5-byte map `{[]: 1, []: 2}`, length `0005`), or `608060405200a2a001a0020005` for `{{}: 1, {}: 2}`. `inspect-evm-interface <file> hex --json` returned metadata `status: "partial"`, 2 entries, diagnostic "Nested containers are uninterpreted; ...". Expected: `not-decoded` with a repeat-key diagnostic, like repeated text and byte-string keys. ## Change and scope - `src/evm/SolidityMetadata.ts`: duplicate detection now compares every root key by value through a small `keyIdentity` helper. Scalars use the same `JSON.stringify(scalar(value))` identity as before, so text, integer, bytes, boolean and null repeats give the same diagnostics as on main. Arrays compare element by element; maps compare as an unordered set of entries, since CBOR map order isn't significant. The identity is built by plain string concatenation, so its size is linear in the key's encoded size even at the 64-level nesting limit. - Comparing by decoded value matches how root scalar keys are already compared (integers by value, bytes by hex), rather than by exact encoded bytes. - Output is unchanged for everything else: containers are still projected as `uninterpreted`, and two different container keys still give `partial`. The helper is only used for the repeat check. - Not included: repeated keys inside nested maps, which this profile leaves uninterpreted. Contribution scope: ordinary fix within existing boundaries; no scope decision needed. ## Contract and boundary impact - Semantic owner and earliest changed stage: `inspectSolidityMetadata` in `src/evm/SolidityMetadata.ts`, when the root map's entries are projected. - Contribution scope fit and linked maintainer decision: not applicable (follow-up bug fix to #4024 / #3991). - CLI and MCP/tool-catalog contract: no schema change. `inspect-evm-interface` / `inspect_evm_interface` report `not-decoded` with a diagnostic for a root map with repeated container keys. - Provider, bridge, target-format, or platform compatibility: none; the EVMole engine and its `raw_result` are untouched. - Evidence, artifact, provenance, or reconstruction contract: `bytecode.metadata` only; the original bytecode and recorded range are unchanged. - Target selection, authority, upstream behavior, and cleanup details: not applicable. - Generated metadata, package, or installation impact: 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. ## Evidence and regression coverage - Tests added or updated (`src/evm/SolidityMetadata.test.ts`): - the malformed-candidate table gains `duplicate array key` (`a280018002`), `duplicate map key` (`a2a001a002`) and `duplicate map key in another order` (`{{1: 2, 3: 4}: 1, {3: 4, 1: 2}: 2}`) - the distinct-keys control adds `[[]]` vs `[{}]`, which must stay `partial` - a 62-level nested array key repeated twice must be `not-decoded` in under a second - Base reproduction: on the base revision all 4 new cases fail with `expected { ... } to match object { status: 'not-decoded', ... }` (base returns `partial`); the other 21 tests in the file pass on both. - User-visible CLI output (`inspect-evm-interface <file> hex --json`, `bytecode.metadata` status / entry count / diagnostic, Linux x64): ``` CBOR map base PR a280018002 partial, 2, nested-container not-decoded, 0, CBOR decode error: found repeat map key [] a2a001a002 partial, 2, nested-container not-decoded, 0, CBOR decode error: found repeat map key {} a2810101810202 (not run) partial, 2, nested-container ``` - Remaining proof gaps: synthetic fixtures only; no real contract artifact with container keys was tried. 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 - `npx vitest run src/evm/SolidityMetadata.test.ts`: 25 passed (4 fail on the base revision, see above) - `npx vitest run src/evm tests/boundary/mcp/evmInterfaceMcp.test.ts`: 6 files, 74 passed - `npx vitest run tests/boundary/evm tests/composition/evmModuleContract.test.ts tests/boundary/mcp/evmLimiterAvailability.test.ts`: 6 files, 62 passed, 2 skipped - `npm run check:fast`: typecheck and lint passed - CLI comparison above, built with `npm run build:cached` ## Compatibility, safety, and release - Breaking changes or migration steps: none. Root maps with repeated container keys move from `partial` to `not-decoded`. - Real Hopper/Ghidra, browser, or OS coverage: Linux x64, Node 24.18.0; no provider involved. - Package or release metadata impact: none. - Security, privacy, process, or containment review: none. ## Author readiness Human owner: @Sriram-PR - [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:11 +00:00