[PR #6930] test(gpu): retain unsupported PE layout delivery regression #7455

Open
opened 2026-11-01 21:47:59 +00:00 by bsakaguchi · 0 comments
Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/6930
Author: @rudycelekli
Created: 10/24/2026
Status: 🔄 Open

Base: main ← Head: fix-pe-section-directory-overlap


📝 Commits (4)

  • f2fa100 fix(native): report overlapping PE section directories as unsupported
  • 7edf402 fix(native): integrate PE layout classification with current section owner
  • 9bdb736 fix(native): refresh current main integration
  • 001634d test(gpu): preserve provider PE layout regression after upstream fix

📊 Changes

5 files changed (+25 additions, -15 deletions)

View changed files

📝 src/browser/PlaywrightScenarioEvents.ts (+3 -5)
📝 src/catalogIdentity.ts (+4 -2)
📝 src/ida/IdaProvider.ts (+4 -2)
📝 tests/boundary/gpu/codeObjects.test.ts (+10 -0)
📝 tests/boundary/mcp/promptMcp.test.ts (+4 -6)

📄 Description

Summary

Add a GPU provider regression for confirmed PE headers overlapping the DOS header. Upstream commit 1dc33c05435bcb1627d6d01680f4e9c3c1af1867 already fixed production parsing and added five parser cases. This PR now preserves only the distinct provider-delivery assertion; it proposes no additional production parser change.

Problem and expected behavior

The actual GpuCodeObjectsProvider should deliver AnalysisUnsupportedTargetError for an overlapping confirmed PE header. Parser-level classification alone does not verify the provider's mapping into the public application error contract.

Change and scope

Add one temporary-file provider test using the existing synthetic PE fixture. Retain upstream's complete parser implementation and five tests unchanged, including its stronger truncated-COFF case. Append-only merge incorporates main through c66729eca902a21dd70f50ce107fec7598be0f31.

Normal integration hooks also deduplicated imports in four files: PlaywrightScenarioEvents.ts, catalogIdentity.ts, IdaProvider.ts, and promptMcp.test.ts. Those changes are mechanical; the complete remaining diff spans five files.

Contract and boundary impact

  • Semantic owner and earliest changed stage: upstream owns PE/MZ facts and native directory parsing; this test verifies the existing GPU provider delivery boundary. No production owner changes remain.
  • Contribution scope fit and linked maintainer decision: ordinary read-only regression coverage; additional scope approval not applicable. No linked issue; coverage follows the upstream fix.
  • CLI and MCP/tool-catalog contract: no schema/catalog changes. Test asserts existing unsupported-target error mapping; no CLI/MCP end-to-end claim.
  • Provider, bridge, target-format, or platform compatibility: actual source GPU provider; no external bridge required.
  • Evidence, artifact, provenance, or reconstruction contract: no changes; existing temporary-file fixture ownership and provider cleanup used.
  • Target selection, authority, upstream behavior, and cleanup details: no changes.
  • Generated metadata, package, or installation impact: none.

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: one actual GpuCodeObjectsProvider case checks AnalysisUnsupportedTargetError and its overlapping-header explanation.
  • Original failure evidence: before the upstream production fix, substituting the exact 3d311d9eca00eecf51f4c4c869c5e7607c332f26 producer caused the new provider assertion to fail because the result was BinaryTargetError. This is historical regression evidence; current main already fixes the defect.
  • Current main duplicate screen: no overlapping-PE provider case found in current GPU boundary suite. Upstream's five parser cases are preserved unchanged and removed from this PR's diff.
  • Boundary/lifecycle coverage: actual temporary source file and provider result, existing cleanup fixtures. No lifecycle changes.
  • Remaining proof gaps: no full source suite, external disassembler, CLI or MCP execution for this case.

Validation performed

Exact head: 001634d7de7e39347715f5cf7d5f8726812dbb30; Node 24.22.1, npm 11.16.0.

  • node node_modules/vitest/vitest.mjs run src/native/metadata/NativeSectionDirectory.test.ts tests/boundary/gpu/codeObjects.test.ts --maxWorkers=2 — 122 passed (five upstream parser cases and 117 GPU boundary cases).
  • npm run check:fast — passed.
  • git diff --check — passed; clean worktree, DCO and normal Husky hooks used.
  • npm run check was rerun on this exact final head: five tasks passed, then Knip stopped at three independently reproduced upstream exports (staticFileUrlPath, AndroidManifestComponentInventory, ObjcClassImportBindingCoverage). Later chained checks were not reached; no full-check pass is claimed.

Compatibility, safety, and release

  • Breaking changes or migration steps: none; regression coverage only.
  • Real Hopper/Ghidra, browser, or OS coverage: synthetic PE through actual provider; external targets not exercised.
  • Package or release metadata impact: none.
  • Security, privacy, process, or containment review: no production changes or new capabilities.

Author readiness

Human owner: @rudycelekli

  • 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

  • Confirm final required CI and scope with maintainers.
  • Human owner completes readiness declarations.

🔄 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/6930 **Author:** [@rudycelekli](https://github.com/rudycelekli) **Created:** 10/24/2026 **Status:** 🔄 Open **Base:** `main` ← **Head:** `fix-pe-section-directory-overlap` --- ### 📝 Commits (4) - [`f2fa100`](https://github.com/morluto/rea/commit/f2fa1006f6ccc7bf6097594066225f5b18dd3455) fix(native): report overlapping PE section directories as unsupported - [`7edf402`](https://github.com/morluto/rea/commit/7edf402ec858cf0e19bfea32c114720af0d33d70) fix(native): integrate PE layout classification with current section owner - [`9bdb736`](https://github.com/morluto/rea/commit/9bdb73688f5fc53dd8dff49fa28fd39a27955e4d) fix(native): refresh current main integration - [`001634d`](https://github.com/morluto/rea/commit/001634d7de7e39347715f5cf7d5f8726812dbb30) test(gpu): preserve provider PE layout regression after upstream fix ### 📊 Changes **5 files changed** (+25 additions, -15 deletions) <details> <summary>View changed files</summary> 📝 `src/browser/PlaywrightScenarioEvents.ts` (+3 -5) 📝 `src/catalogIdentity.ts` (+4 -2) 📝 `src/ida/IdaProvider.ts` (+4 -2) 📝 `tests/boundary/gpu/codeObjects.test.ts` (+10 -0) 📝 `tests/boundary/mcp/promptMcp.test.ts` (+4 -6) </details> ### 📄 Description ## Summary Add a GPU provider regression for confirmed PE headers overlapping the DOS header. Upstream commit `1dc33c05435bcb1627d6d01680f4e9c3c1af1867` already fixed production parsing and added five parser cases. This PR now preserves only the distinct provider-delivery assertion; it proposes no additional production parser change. ## Problem and expected behavior The actual `GpuCodeObjectsProvider` should deliver `AnalysisUnsupportedTargetError` for an overlapping confirmed PE header. Parser-level classification alone does not verify the provider's mapping into the public application error contract. ## Change and scope Add one temporary-file provider test using the existing synthetic PE fixture. Retain upstream's complete parser implementation and five tests unchanged, including its stronger truncated-COFF case. Append-only merge incorporates main through `c66729eca902a21dd70f50ce107fec7598be0f31`. Normal integration hooks also deduplicated imports in four files: `PlaywrightScenarioEvents.ts`, `catalogIdentity.ts`, `IdaProvider.ts`, and `promptMcp.test.ts`. Those changes are mechanical; the complete remaining diff spans five files. ## Contract and boundary impact - Semantic owner and earliest changed stage: upstream owns PE/MZ facts and native directory parsing; this test verifies the existing GPU provider delivery boundary. No production owner changes remain. - Contribution scope fit and linked maintainer decision: ordinary read-only regression coverage; additional scope approval not applicable. No linked issue; coverage follows the upstream fix. - CLI and MCP/tool-catalog contract: no schema/catalog changes. Test asserts existing unsupported-target error mapping; no CLI/MCP end-to-end claim. - Provider, bridge, target-format, or platform compatibility: actual source GPU provider; no external bridge required. - Evidence, artifact, provenance, or reconstruction contract: no changes; existing temporary-file fixture ownership and provider cleanup used. - Target selection, authority, upstream behavior, and cleanup details: no changes. - Generated metadata, package, or installation impact: none. 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: one actual `GpuCodeObjectsProvider` case checks `AnalysisUnsupportedTargetError` and its overlapping-header explanation. - Original failure evidence: before the upstream production fix, substituting the exact `3d311d9eca00eecf51f4c4c869c5e7607c332f26` producer caused the new provider assertion to fail because the result was `BinaryTargetError`. This is historical regression evidence; current main already fixes the defect. - Current main duplicate screen: no overlapping-PE provider case found in current GPU boundary suite. Upstream's five parser cases are preserved unchanged and removed from this PR's diff. - Boundary/lifecycle coverage: actual temporary source file and provider result, existing cleanup fixtures. No lifecycle changes. - Remaining proof gaps: no full source suite, external disassembler, CLI or MCP execution for this case. ## Validation performed Exact head: `001634d7de7e39347715f5cf7d5f8726812dbb30`; Node 24.22.1, npm 11.16.0. - `node node_modules/vitest/vitest.mjs run src/native/metadata/NativeSectionDirectory.test.ts tests/boundary/gpu/codeObjects.test.ts --maxWorkers=2` — 122 passed (five upstream parser cases and 117 GPU boundary cases). - `npm run check:fast` — passed. - `git diff --check` — passed; clean worktree, DCO and normal Husky hooks used. - `npm run check` was rerun on this exact final head: five tasks passed, then Knip stopped at three independently reproduced upstream exports (`staticFileUrlPath`, `AndroidManifestComponentInventory`, `ObjcClassImportBindingCoverage`). Later chained checks were not reached; no full-check pass is claimed. ## Compatibility, safety, and release - Breaking changes or migration steps: none; regression coverage only. - Real Hopper/Ghidra, browser, or OS coverage: synthetic PE through actual provider; external targets not exercised. - Package or release metadata impact: none. - Security, privacy, process, or containment review: no production changes or new capabilities. ## Author readiness Human owner: @rudycelekli - [ ] 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 - [ ] Confirm final required CI and scope with maintainers. - [ ] Human owner completes readiness declarations. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>
Sign in to join this conversation.
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#7455
No description provided.