morluto/rea

Fork 0

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

[PR #3454] [CLOSED] fix(rust): isolate ELF section reads from Go build-info admission #5747

New issue

Closed

opened 2026-10-31 01:09:08 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-31 01:09:08 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/3454
Author: @chqcjq
Created: 10/14/2026
Status: ❌ Closed

Base: main ← Head: fix/rust-elf-go-section-isolation

📝 Commits (5)

e02ca5e fix(rust): isolate ELF section reads from Go build-info admission
d97e4b0 test(rust): pin Go section mapping rejection and named control
189fc29 Merge remote-tracking branch 'origin/main' into fix/rust-elf-go-section-isolation
d6f992c fix(ci): cover GitHub action sources in lint cache inputs
41955b1 fix(ci): normalize Windows paths in duplication reports

📊 Changes

7 files changed (+155 additions, -8 deletions)

View changed files

📝 scripts/ci/static-check-report.mjs (+7 -2)
📝 src/go/GoBinaryImage.ts (+6 -3)
📝 src/go/GoElfImage.ts (+9 -2)
📝 src/rust/RustBinaryImage.ts (+1 -1)
➕ tests/acceptance/rustGoSectionCli.test.ts (+112 -0)
📝 tests/fixtures/rust/image.ts (+18 -0)
📝 turbo.json (+2 -0)

📄 Description

Summary

Fixes #3398. Rust dependency inspection now accepts an ELF with an unrelated nonallocated .go.buildinfo section while preserving the original artifact identity.

Problem and expected behavior

The existing shared ELF reader applies Go build-info mapping admission before Rust can decode .dep-v0. On the affected base, a valid synthetic ELF succeeds when the extra section is named .extra, but the same bytes with .go.buildinfo fail with invalid_request / invalid_format: “Virtual address or string range is not file mapped”. Rust should not interpret that unrelated section as Go metadata.

Change and scope

Distinguish the two existing ELF read purposes at the producing boundary. Rust requests section/container facts; the Go path retains its default build-info admission and mapping checks. This keeps one structural parser and avoids catching or suppressing generic container errors. The internal purpose is not a CLI/MCP option.

Semantic owner: GoElfImage; forwarded by GoBinaryImage and selected by RustBinaryImage.
Scope decision: not applicable; ordinary static metadata fix within existing boundaries.
CLI/MCP schema and generated metadata: unchanged.
Evidence: exact path, bytes, and SHA-256 remain those of the selected artifact.
Effects: reads selected local files; no target execution, attachment, instrumentation, or mutation.

Evidence and regression coverage

Executed on Windows with pinned Node 24.18.0 and locked dependencies:

Before the fix, the compiled CLI and stdio MCP regressions both fail on the Go-named file after the name-only control succeeds.
After the fix, both adapters return identical dependency metadata for the original and control while preserving each file's independently calculated SHA-256.
Malformed ELF section tables and corrupted dependency payloads still return invalid_request; Go inspection still rejects the invalid Go build-info mapping.
Eight focused files: 156 tests pass, 2 existing Windows-conditional tests skip. Covers Rust image/CLI/MCP, Go image/mappings/provider/CLI, and the new public regression.

Validation performed

npm run test:focused -- tests/acceptance/rustGoSectionCli.test.ts tests/acceptance/rustBinaryCli.test.ts tests/acceptance/goBinaryCli.test.ts tests/boundary/rust/rustBinaryImage.test.ts tests/boundary/go/goBinaryImage.test.ts tests/boundary/go/goBinaryMappings.test.ts tests/boundary/go/goBinaryProvider.test.ts tests/boundary/mcp/rustBinaryMcp.test.ts — 156 passed, 2 skipped.
npm run check and npm run check:ci — pass; typecheck, lint, format, knip, metadata, duplication and architecture guards succeed.
npm run docs:check — pass.
npm run verify:test-discovery — all 1,274 test files discovered exactly once.

Local source/diff review traced structural parsing, Go-specific admission, Rust dependency decoding, typed failures, and artifact identity. Fixtures are authored synthetic binaries; no compiler-produced Rust/Go binary or Linux/macOS runtime was exercised, and the full repository test suite was not run.

Review follow-up

Commit d97e4b0f9 addresses Greptile's Wrong rejection still passes finding. Both CLI and stdio MCP now parse the complete typed Go error and assert invalid_format with the mapping-specific diagnostic Virtual address or string range is not file mapped. The name-only .extra control also succeeds as Go with build_info: null, preventing unrelated rejections from satisfying the regression.

Executed for this follow-up: the two public adapter tests pass; npm run check:ci and normal pre-push check:fast pass. Only the regression test changed; production behavior remains the original fix. The earlier 156-test run above is historical validation of the original revision.

Latest CI integration validation

Synced this branch with main eab789aed. Both lint task cache inputs now include checker-consumed code under .github, correcting the CI inventory failure caused by the publishing-action files.

Commit 41955b140 addresses the valid review finding that the previous test-side path conversion could hide incorrect saved report paths. The test again asserts the raw report names. The report writer now normalizes both absolute paths to the same Windows namespace before computing relative paths; clone locations and threshold handling are preserved. Restoring the strict assertion reproduced absolute //?/C:/... names before the writer fix, and the unchanged assertion passes after it.

Executed for this correction: 2 files / 10 tests (CLI/MCP Go-error controls and static-check reports) pass; npm run check:ci and normal pre-push checks pass. New-head remote CI is pending. Earlier validation above applies to its stated revisions; native runtime checks were not repeated for this CI/reporting correction.

Author readiness

Human owner: @chqcjq

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.

The human owner's readiness declarations are recorded above.

_{🔄 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/3454 **Author:** [@chqcjq](https://github.com/chqcjq) **Created:** 10/14/2026 **Status:** ❌ Closed **Base:** `main` ← **Head:** `fix/rust-elf-go-section-isolation` --- ### 📝 Commits (5) - [`e02ca5e`](https://github.com/morluto/rea/commit/e02ca5e0b15c96533844d14667f14af8ba1f08e7) fix(rust): isolate ELF section reads from Go build-info admission - [`d97e4b0`](https://github.com/morluto/rea/commit/d97e4b0f9ed0de75cc90f397395a71d0c841c1cd) test(rust): pin Go section mapping rejection and named control - [`189fc29`](https://github.com/morluto/rea/commit/189fc293fc5c4e77d8bf5b0e413e8b954074491d) Merge remote-tracking branch 'origin/main' into fix/rust-elf-go-section-isolation - [`d6f992c`](https://github.com/morluto/rea/commit/d6f992cdc712820174af7c57eb06786017eff696) fix(ci): cover GitHub action sources in lint cache inputs - [`41955b1`](https://github.com/morluto/rea/commit/41955b140a1b26682c4fc7099cedb3cce16c6e9b) fix(ci): normalize Windows paths in duplication reports ### 📊 Changes **7 files changed** (+155 additions, -8 deletions) <details> <summary>View changed files</summary> 📝 `scripts/ci/static-check-report.mjs` (+7 -2) 📝 `src/go/GoBinaryImage.ts` (+6 -3) 📝 `src/go/GoElfImage.ts` (+9 -2) 📝 `src/rust/RustBinaryImage.ts` (+1 -1) ➕ `tests/acceptance/rustGoSectionCli.test.ts` (+112 -0) 📝 `tests/fixtures/rust/image.ts` (+18 -0) 📝 `turbo.json` (+2 -0) </details> ### 📄 Description ## Summary Fixes #3398. Rust dependency inspection now accepts an ELF with an unrelated nonallocated `.go.buildinfo` section while preserving the original artifact identity. ## Problem and expected behavior The existing shared ELF reader applies Go build-info mapping admission before Rust can decode `.dep-v0`. On the affected base, a valid synthetic ELF succeeds when the extra section is named `.extra`, but the same bytes with `.go.buildinfo` fail with `invalid_request` / `invalid_format`: “Virtual address or string range is not file mapped”. Rust should not interpret that unrelated section as Go metadata. ## Change and scope Distinguish the two existing ELF read purposes at the producing boundary. Rust requests section/container facts; the Go path retains its default build-info admission and mapping checks. This keeps one structural parser and avoids catching or suppressing generic container errors. The internal purpose is not a CLI/MCP option. - Semantic owner: `GoElfImage`; forwarded by `GoBinaryImage` and selected by `RustBinaryImage`. - Scope decision: not applicable; ordinary static metadata fix within existing boundaries. - CLI/MCP schema and generated metadata: unchanged. - Evidence: exact path, bytes, and SHA-256 remain those of the selected artifact. - Effects: reads selected local files; no target execution, attachment, instrumentation, or mutation. ## Evidence and regression coverage Executed on Windows with pinned Node 24.18.0 and locked dependencies: - Before the fix, the compiled CLI and stdio MCP regressions both fail on the Go-named file after the name-only control succeeds. - After the fix, both adapters return identical dependency metadata for the original and control while preserving each file's independently calculated SHA-256. - Malformed ELF section tables and corrupted dependency payloads still return `invalid_request`; Go inspection still rejects the invalid Go build-info mapping. - Eight focused files: **156 tests pass, 2 existing Windows-conditional tests skip**. Covers Rust image/CLI/MCP, Go image/mappings/provider/CLI, and the new public regression. ## Validation performed - `npm run test:focused -- tests/acceptance/rustGoSectionCli.test.ts tests/acceptance/rustBinaryCli.test.ts tests/acceptance/goBinaryCli.test.ts tests/boundary/rust/rustBinaryImage.test.ts tests/boundary/go/goBinaryImage.test.ts tests/boundary/go/goBinaryMappings.test.ts tests/boundary/go/goBinaryProvider.test.ts tests/boundary/mcp/rustBinaryMcp.test.ts` — 156 passed, 2 skipped. - `npm run check` and `npm run check:ci` — pass; typecheck, lint, format, knip, metadata, duplication and architecture guards succeed. - `npm run docs:check` — pass. - `npm run verify:test-discovery` — all 1,274 test files discovered exactly once. Local source/diff review traced structural parsing, Go-specific admission, Rust dependency decoding, typed failures, and artifact identity. Fixtures are authored synthetic binaries; no compiler-produced Rust/Go binary or Linux/macOS runtime was exercised, and the full repository test suite was not run. ## Review follow-up Commit `d97e4b0f9` addresses Greptile's `Wrong rejection still passes` finding. Both CLI and stdio MCP now parse the complete typed Go error and assert `invalid_format` with the mapping-specific diagnostic `Virtual address or string range is not file mapped`. The name-only `.extra` control also succeeds as Go with `build_info: null`, preventing unrelated rejections from satisfying the regression. Executed for this follow-up: the two public adapter tests pass; `npm run check:ci` and normal pre-push `check:fast` pass. Only the regression test changed; production behavior remains the original fix. The earlier 156-test run above is historical validation of the original revision. ## Latest CI integration validation Synced this branch with main `eab789aed`. Both lint task cache inputs now include checker-consumed code under `.github`, correcting the CI inventory failure caused by the publishing-action files. Commit `41955b140` addresses the valid review finding that the previous test-side path conversion could hide incorrect saved report paths. The test again asserts the raw report names. The report writer now normalizes both absolute paths to the same Windows namespace before computing relative paths; clone locations and threshold handling are preserved. Restoring the strict assertion reproduced absolute `//?/C:/...` names before the writer fix, and the unchanged assertion passes after it. Executed for this correction: 2 files / 10 tests (CLI/MCP Go-error controls and static-check reports) pass; `npm run check:ci` and normal pre-push checks pass. New-head remote CI is pending. Earlier validation above applies to its stated revisions; native runtime checks were not repeated for this CI/reporting correction. ## Author readiness Human owner: @chqcjq - [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. The human owner's readiness declarations are recorded above. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi

2026-10-31 01:09:08 +00:00

closed this issue
added the
pull-request
label

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

No description provided.

Rows
Columns