[PR #759] [CLOSED] fix(ghidra): reject contradictory NativeAOT reports #2222

New issue

Closed

opened 2026-10-16 21:15:53 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-16 21:15:53 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/759
Author: @kaoru0822-kitauji
Created: 10/6/2026
Status: ❌ Closed

Base: main ← Head: fix/ghidra-nativeaot-report-consistency

📝 Commits (1)

919992a fix(ghidra): reject contradictory NativeAOT reports

📊 Changes

2 files changed (+57 additions, -1 deletions)

View changed files

📝 src/ghidra/extensions/GhidraExtensions.test.ts (+39 -0)
📝 src/ghidra/extensions/nativeaot/NativeAotReport.ts (+18 -1)

📄 Description

Summary

Reject NativeAOT extension reports whose non-recovery status contradicts their payload. A report labeled not_applicable, unsupported, or failed must now include an actionable nonblank reason, report zero method tables, and omit recovered type, derived-memory, and coverage payloads.

Problem and expected behavior

The NativeAOT adapter validator previously applied recovery-shape checks only to complete and partial reports. A producer response could therefore claim not_applicable while reporting recovered method tables, or omit the reason for that unavailable outcome, and still pass the authenticated bridge handshake. That lets contradictory producer metadata enter the session evidence boundary.

The validator should reject those reports as malformed while preserving discovery/version fields that a legitimate failed or unsupported analysis may have observed before stopping.

Change and scope

Add a non-recovery invariant to the NativeAOT producer validator.
Require a nonblank reason and the absence of recovered payloads for non-recovery statuses.
Add regression coverage across all three non-recovery statuses for contradictory table counts and null, empty, or whitespace-only reasons, while proving discovery/version context remains valid.
Keep the existing dedicated loader-failure envelope and successful recovery validation unchanged.

Related to #743.

Contract and boundary impact

Semantic owner and earliest changed stage: Ghidra NativeAOT extension result admission, before provider output enters Evidence.
CLI/MCP/tool catalog: no schema or catalog change; malformed producer output now becomes the existing typed provider-adapter failure.
Provider/bridge/target compatibility: no Java bridge, target-format, host, or runtime coverage change.
Evidence/provenance: contradictory non-recovery metadata can no longer be admitted.
Process/authorization/cleanup: unchanged.

Evidence and regression coverage

Base reproduction: the new focused test failed on current main because validateGhidraExtensionResults returned null for a not_applicable report with method_tables: 1.
The same boundary also accepted null, empty, and whitespace-only non-recovery reasons.
After the fix, the extension boundary and surrounding provider behavior suites pass: 46 tests.
No real Ghidra run was required for this pure result-validation change.
Material unverified coverage: no real Ghidra/NativeAOT toolchain run, Windows run, or full deterministic test suite.

Validation performed

vitest run src/ghidra/extensions/GhidraExtensions.test.ts src/ghidra/GhidraProvider.behavior.test.ts --coverage.enabled=false — 46 passed.
npm exec --package=typescript@5.9.3 -- tsc --noEmit — passed.
npm run lint — passed with existing repository warnings and no changed-file warnings.
npm run lint -- --deny-warnings <changed files> — passed.
npm run knip — passed.
npm run metadata:check — passed.
Targeted oxfmt --check and git diff --check — passed.
npm run check could not complete in this container because the repository TypeScript 7 binary requires /proc/self/exe, which is unavailable here. A separate whole-tree formatter scan also encountered the environment's unavailable resident-memory syscall while checking an unrelated fixture; changed-file formatting passed.

Compatibility, safety, and release

Breaking changes or migration steps: none.
Generated metadata, package, installation, or documentation impact: none.
Security/privacy: this strengthens an executable-extension trust boundary without expanding authority or retaining new data.
AI-assisted implementation and tests; I reviewed the final diff and validation output.

Review checklist

The PR has one focused outcome and the title follows type(scope): outcome.
A related issue is linked.
Tests cover changed observable behavior and meaningful failure paths.
No owning docs, public contracts, or generated metadata require updates.
Unsupported and failed outcomes remain explicit.
The final diff contains no unrelated cleanup or unsupported coverage 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/759 **Author:** [@kaoru0822-kitauji](https://github.com/kaoru0822-kitauji) **Created:** 10/6/2026 **Status:** ❌ Closed **Base:** `main` ← **Head:** `fix/ghidra-nativeaot-report-consistency` --- ### 📝 Commits (1) - [`919992a`](https://github.com/morluto/rea/commit/919992a8a05996ef2a0ae7481a7ac3b4f008043a) fix(ghidra): reject contradictory NativeAOT reports ### 📊 Changes **2 files changed** (+57 additions, -1 deletions) <details> <summary>View changed files</summary> 📝 `src/ghidra/extensions/GhidraExtensions.test.ts` (+39 -0) 📝 `src/ghidra/extensions/nativeaot/NativeAotReport.ts` (+18 -1) </details> ### 📄 Description ## Summary Reject NativeAOT extension reports whose non-recovery status contradicts their payload. A report labeled `not_applicable`, `unsupported`, or `failed` must now include an actionable nonblank reason, report zero method tables, and omit recovered type, derived-memory, and coverage payloads. ## Problem and expected behavior The NativeAOT adapter validator previously applied recovery-shape checks only to `complete` and `partial` reports. A producer response could therefore claim `not_applicable` while reporting recovered method tables, or omit the reason for that unavailable outcome, and still pass the authenticated bridge handshake. That lets contradictory producer metadata enter the session evidence boundary. The validator should reject those reports as malformed while preserving discovery/version fields that a legitimate failed or unsupported analysis may have observed before stopping. ## Change and scope - Add a non-recovery invariant to the NativeAOT producer validator. - Require a nonblank reason and the absence of recovered payloads for non-recovery statuses. - Add regression coverage across all three non-recovery statuses for contradictory table counts and null, empty, or whitespace-only reasons, while proving discovery/version context remains valid. - Keep the existing dedicated loader-failure envelope and successful recovery validation unchanged. Related to #743. ## Contract and boundary impact - Semantic owner and earliest changed stage: Ghidra NativeAOT extension result admission, before provider output enters Evidence. - CLI/MCP/tool catalog: no schema or catalog change; malformed producer output now becomes the existing typed provider-adapter failure. - Provider/bridge/target compatibility: no Java bridge, target-format, host, or runtime coverage change. - Evidence/provenance: contradictory non-recovery metadata can no longer be admitted. - Process/authorization/cleanup: unchanged. ## Evidence and regression coverage - Base reproduction: the new focused test failed on current main because `validateGhidraExtensionResults` returned `null` for a `not_applicable` report with `method_tables: 1`. - The same boundary also accepted null, empty, and whitespace-only non-recovery reasons. - After the fix, the extension boundary and surrounding provider behavior suites pass: 46 tests. - No real Ghidra run was required for this pure result-validation change. - Material unverified coverage: no real Ghidra/NativeAOT toolchain run, Windows run, or full deterministic test suite. ## Validation performed - `vitest run src/ghidra/extensions/GhidraExtensions.test.ts src/ghidra/GhidraProvider.behavior.test.ts --coverage.enabled=false` — 46 passed. - `npm exec --package=typescript@5.9.3 -- tsc --noEmit` — passed. - `npm run lint` — passed with existing repository warnings and no changed-file warnings. - `npm run lint -- --deny-warnings <changed files>` — passed. - `npm run knip` — passed. - `npm run metadata:check` — passed. - Targeted `oxfmt --check` and `git diff --check` — passed. - `npm run check` could not complete in this container because the repository TypeScript 7 binary requires `/proc/self/exe`, which is unavailable here. A separate whole-tree formatter scan also encountered the environment's unavailable resident-memory syscall while checking an unrelated fixture; changed-file formatting passed. ## Compatibility, safety, and release - Breaking changes or migration steps: none. - Generated metadata, package, installation, or documentation impact: none. - Security/privacy: this strengthens an executable-extension trust boundary without expanding authority or retaining new data. - AI-assisted implementation and tests; I reviewed the final diff and validation output. ## Review checklist - [x] The PR has one focused outcome and the title follows `type(scope): outcome`. - [x] A related issue is linked. - [x] Tests cover changed observable behavior and meaningful failure paths. - [x] No owning docs, public contracts, or generated metadata require updates. - [x] Unsupported and failed outcomes remain explicit. - [x] The final diff contains no unrelated cleanup or unsupported coverage claims. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi

2026-10-16 21:15:53 +00:00