morluto/rea

Fork 0

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

[GH-ISSUE #8612] [Bug] Ghidra sessions accept handshake reports the profile did not commit unless the first query is health #7493

New issue

Closed

opened 2026-11-03 00:30:11 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-11-03 00:30:11 +00:00

Owner

Originally created by @Entrpi on GitHub (Nov 2, 2026).
Original GitHub issue: https://github.com/morluto/rea/issues/8612

Originally assigned to: @kaoru0822-kitauji on GitHub.

Before submitting

I searched the open and closed issues for a related report.
I removed credentials, tokens, private paths, and sensitive target data.

Area

Native analysis or decompilation

Observed behavior

The Ghidra bridge handshake can report analysis seeds, disabled-analyzer outcomes and extension results. checkSessionObservations rejects each report that the analysis profile did not commit; for seeds the reason is "The Ghidra session reported analysis seeds that the profile did not commit." But that check runs only inside startSession. A session calls startSession only for health, or when the eager-start condition holds: an existing project, PE, configured extensions, seeds, disabled analyzers, soft-float MIPS, or a RISC-V calling-convention policy. If the profile commits none of these, a first query such as list_procedures starts the bridge lazily inside client.callTool. The report is then never checked, and the query returns its result.

Reproduction

A unit-level probe with a fake GhidraProviderClientFactory, on main d3ab0faf5. The target is a plain x86_64 ELF, and the profile commits no seeds, disabled analyzers or extensions. The fake start returns a valid handshake plus one report, and the fake callTool returns ok([]).

Report in the handshake	First query `health`	First query `list_procedures`
`analysis_seeds`	`ProviderAdapterError`; start 1, call 0, close 1	`ok: true`; start 0, call 1, close 0
`disabled_analyzers`	`ProviderAdapterError`; start 1, call 0, close 1	`ok: true`; start 0, call 1, close 0
`analysis_extensions`	`ProviderAdapterError`; start 1, call 0, close 1	`ok: true`; start 0, call 1, close 0

The existing test "fails closed on an analyzer report the profile did not commit" (src/ghidra/GhidraProvider.behavior.test.ts) uses health. With list_procedures it fails with expected true to be false. Its forward-direction neighbour already uses list_procedures.

Expected behavior

Each report binds in both directions. A session that receives a report its profile did not commit fails closed before it returns any analysis result, whatever the first operation is.

Impact and scope

The check guards against a bridge or launcher fault: a script that the profile did not commit runs, or the bridge reports a stale property. I did not reproduce such a fault against real Ghidra, so how often it happens is NOT_ENOUGH_INFO. If it happens, the returned Evidence can claim a profile that does not describe the analysis that produced it. This applies to every target that starts lazily: not PE, and not an existing project.

REA version

Source main d3ab0faf5 (rea-agents 6.8.0).

Runtime and platform

macOS arm64, Node.js 24.22.1. Unit tests only; real Ghidra not run.

Target and artifact context

Plain x86_64 ELF in a unit-level probe; a profile without seeds, disabled analyzers or extensions.

Interface

Other

Earliest failure stage

Setup or provider admission

Relevant output or evidence

Source at d3ab0faf5:

startSession runs checkSessionObservations (line 654).
The eager-start condition: when no term holds, the query goes straight to client.callTool.

The table above comes from the probe; it was run, not derived. The probe is not committed. Both the CLI and MCP reach this code through the provider adapter.

Typed outcome or recovery expected

ProviderAdapterError from the handshake check, with the session closed before the first query runs.

Analysis or suspected cause

The eager start began with #748 (fb679b63c) as "if extensions are configured, start and check". Each later binding added its own term (#5425 added the one for disabled analyzers), and a report that the profile did not commit is outside every term.
A narrow fix for the MIPS soft-float and RISC-V calling-convention reports is on the fork branch fix-ghidra-checked-start-mips-riscv: every MIPS and every RISC-V session starts through the checked handshake. Those terms cannot cover seeds, disabled analyzers and extensions, which apply to every architecture.
Proposed fix: run startSession before every query and remove the condition. The real client shares one startup attempt, so there is still one launch.
Consequences of that fix, measured in a local trial that is not on any branch:
1. If the provider exits during the first query after a completed handshake, the error gets the "call open_binary" restart guidance, as eager-start lanes already do. Two tests pin the old guidance: tests/boundary/providers/ghidra/ghidraOomRetention.test.ts "reports a kill during a cgroup OOM kill as a memory constraint" (its remediation must end with "Then retry.") and the behaviour test "keeps installation guidance when Ghidra dies before it ever answered".
2. tests/boundary/mcp/ghidraStartupCancellationMcp.test.ts marks a caller as admitted when callTool is reached. With the checked start, callers wait in start, so the test deadlocks until it times out; it would need to observe start. Both designs cancel through the same shared startup attempt in GhidraClient.start.
3. "commits exact provider, isolation, and resource semantics" pins startCount 0 (from #474, before any binding existed), and four shared fakes stub start as "startup is not exercised".
This changes the lifecycle and the recovery guidance, so it needs a maintainer scope decision before anyone implements it.

Additional context

Found while checking that the MIPS soft-float report binds in both directions. #8464 adds the same riscv term for its own elf_load_preparation report.

Originally created by @Entrpi on GitHub (Nov 2, 2026). Original GitHub issue: https://github.com/morluto/rea/issues/8612 Originally assigned to: @kaoru0822-kitauji on GitHub. ### Before submitting - [x] I searched the open and closed issues for a related report. - [x] I removed credentials, tokens, private paths, and sensitive target data. ### Area Native analysis or decompilation ### Observed behavior The Ghidra bridge handshake can report analysis seeds, disabled-analyzer outcomes and extension results. `checkSessionObservations` rejects each report that the analysis profile did not commit; for seeds the reason is "The Ghidra session reported analysis seeds that the profile did not commit." But that check runs only inside `startSession`. A session calls `startSession` only for `health`, or when the eager-start condition holds: an existing project, PE, configured extensions, seeds, disabled analyzers, soft-float MIPS, or a RISC-V calling-convention policy. If the profile commits none of these, a first query such as `list_procedures` starts the bridge lazily inside `client.callTool`. The report is then never checked, and the query returns its result. ### Reproduction A unit-level probe with a fake `GhidraProviderClientFactory`, on `main` `d3ab0faf5`. The target is a plain x86_64 ELF, and the profile commits no seeds, disabled analyzers or extensions. The fake `start` returns a valid handshake plus one report, and the fake `callTool` returns `ok([])`. | Report in the handshake | First query `health` | First query `list_procedures` | | --- | --- | --- | | `analysis_seeds` | `ProviderAdapterError`; start 1, call 0, close 1 | `ok: true`; start 0, call 1, close 0 | | `disabled_analyzers` | `ProviderAdapterError`; start 1, call 0, close 1 | `ok: true`; start 0, call 1, close 0 | | `analysis_extensions` | `ProviderAdapterError`; start 1, call 0, close 1 | `ok: true`; start 0, call 1, close 0 | The existing test "fails closed on an analyzer report the profile did not commit" (`src/ghidra/GhidraProvider.behavior.test.ts`) uses `health`. With `list_procedures` it fails with `expected true to be false`. Its forward-direction neighbour already uses `list_procedures`. ### Expected behavior Each report binds in both directions. A session that receives a report its profile did not commit fails closed before it returns any analysis result, whatever the first operation is. ### Impact and scope The check guards against a bridge or launcher fault: a script that the profile did not commit runs, or the bridge reports a stale property. I did not reproduce such a fault against real Ghidra, so how often it happens is NOT_ENOUGH_INFO. If it happens, the returned Evidence can claim a profile that does not describe the analysis that produced it. This applies to every target that starts lazily: not PE, and not an existing project. ### REA version Source `main` `d3ab0faf5` (`rea-agents` 6.8.0). ### Runtime and platform macOS arm64, Node.js 24.22.1. Unit tests only; real Ghidra not run. ### Target and artifact context Plain x86_64 ELF in a unit-level probe; a profile without seeds, disabled analyzers or extensions. ### Interface Other ### Earliest failure stage Setup or provider admission ### Relevant output or evidence Source at `d3ab0faf5`: - [`startSession`](https://github.com/morluto/rea/blob/d3ab0faf52a02ff5a618f3ed7beb608b78752abd/src/ghidra/GhidraProviderClient.ts#L639-L656) runs `checkSessionObservations` (line 654). - [The eager-start condition](https://github.com/morluto/rea/blob/d3ab0faf52a02ff5a618f3ed7beb608b78752abd/src/ghidra/GhidraProviderClient.ts#L806-L818): when no term holds, the query goes straight to `client.callTool`. The table above comes from the probe; it was run, not derived. The probe is not committed. Both the CLI and MCP reach this code through the provider adapter. ### Typed outcome or recovery expected `ProviderAdapterError` from the handshake check, with the session closed before the first query runs. ### Analysis or suspected cause - The eager start began with #748 (`fb679b63c`) as "if extensions are configured, start and check". Each later binding added its own term (#5425 added the one for disabled analyzers), and a report that the profile did not commit is outside every term. - A narrow fix for the MIPS soft-float and RISC-V calling-convention reports is on the fork branch [`fix-ghidra-checked-start-mips-riscv`](https://github.com/Entrpi/rea/tree/fix-ghidra-checked-start-mips-riscv): every MIPS and every RISC-V session starts through the checked handshake. Those terms cannot cover seeds, disabled analyzers and extensions, which apply to every architecture. - Proposed fix: run `startSession` before every query and remove the condition. The real client shares one startup attempt, so there is still one launch. - Consequences of that fix, measured in a local trial that is not on any branch: 1. If the provider exits during the first query after a completed handshake, the error gets the "call open_binary" restart guidance, as eager-start lanes already do. Two tests pin the old guidance: `tests/boundary/providers/ghidra/ghidraOomRetention.test.ts` "reports a kill during a cgroup OOM kill as a memory constraint" (its remediation must end with "Then retry.") and the behaviour test "keeps installation guidance when Ghidra dies before it ever answered". 2. `tests/boundary/mcp/ghidraStartupCancellationMcp.test.ts` marks a caller as admitted when `callTool` is reached. With the checked start, callers wait in `start`, so the test deadlocks until it times out; it would need to observe `start`. Both designs cancel through the same shared startup attempt in `GhidraClient.start`. 3. "commits exact provider, isolation, and resource semantics" pins `startCount` 0 (from #474, before any binding existed), and four shared fakes stub `start` as "startup is not exercised". - This changes the lifecycle and the recovery guidance, so it needs a maintainer scope decision before anyone implements it. ### Additional context Found while checking that the MIPS soft-float report binds in both directions. #8464 adds the same `riscv` term for its own `elf_load_preparation` report.

bsakaguchi closed this issue

2026-11-03 00:30:11 +00:00

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

No description provided.

Rows
Columns