morluto/rea

Fork 0

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

[PR #1663] [CLOSED] fix(ghidra): disclose that string search covers only defined strings #4734

New issue

Closed

opened 2026-10-31 00:54:29 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-31 00:54:29 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/1663
Author: @Fazmin
Created: 10/11/2026
Status: ❌ Closed

Base: main ← Head: fix/ghidra-search-strings-limitation

📝 Commits (1)

a508b66 fix(ghidra): disclose that string search covers only defined strings

📊 Changes

3 files changed (+26 additions, -2 deletions)

View changed files

📝 docs/ghidra-provider.md (+1 -1)
📝 src/ghidra/GhidraProvider.behavior.test.ts (+19 -0)
📝 src/ghidra/GhidraProviderCapabilities.ts (+6 -1)

📄 Description

Summary

Fixes #1659. Ghidra's search_strings Evidence didn't say that it searches only Ghidra-defined string Data, so an empty result read as "this text is not in the file". list_strings already carries that limitation, and both read the same inventory. search_strings now carries its own limitation naming that scope, and points to read_bytes and xrefs for a known address. The Ghidra provider guide's Search row now says what "complete" covers for strings.

Thanks to @CassieFox23 for the detailed report and reproduction.

Problem and expected behavior

Trigger: search_strings (CLI search --kind strings) on a Ghidra session, for text Ghidra did not define as a string. The issue's example is packed Rust literals without a terminator, in stripped PyO3 modules.
Before: limitationsFor in src/ghidra/GhidraProviderCapabilities.ts put search_strings in the same branch as search_procedures, so its only operation-specific limitation was the Java-regex note. The list_strings branch carried "Only Ghidra-defined string Data is observed…". As the issue notes, the bridge collects strings from getDefinedData(true), so both operations see the same defined-Data inventory.
Expected (from the issue): a caller who gets an empty list should know the list may be short. The search result should carry the same limit as list_strings, and may name the way around it (read_bytes plus xrefs).

Change and scope

GhidraProviderCapabilities.ts: split search_strings from search_procedures. The two still share the regex limitation text through one constant. search_strings adds:

Only Ghidra-defined string Data is searched, so text Ghidra did not define as a string is not matched and an empty result does not show that the bytes are absent. read_bytes and xrefs can inspect a known address.
docs/ghidra-provider.md: the Search row now reads "scan the complete immutable inventory (for strings, Ghidra-defined Data)". This is a single-line change; the table is not re-padded.

Intentionally out of scope:

Shared tool description: the provider-neutral search_strings tool description in officialToolContracts.ts ("complete over the native inventory") is shared with the Hopper and IDA providers. This change keeps the disclosure in Ghidra's per-result Evidence and provider guide; I'm happy to adjust the shared text if you prefer.
Finding undefined text: a byte-pattern search, which the issue suggests as a separate feature.
Hopper and IDA: their string semantics are untouched.

Contract and boundary impact

Semantic owner and earliest changed stage: Ghidra provider limitation metadata attached to search_strings Evidence
CLI and MCP/tool-catalog contract: no schema or tool-description change; Ghidra search_strings Evidence carries one more limitations entry
Provider, bridge, target-format, or platform compatibility: Ghidra only; the bridge is unchanged
Evidence, artifact, provenance, or reconstruction contract: an existing unknown (defined-Data scope) is now disclosed on search results
Process execution, authorization, cleanup, or containment impact: none
Generated metadata, package, or installation impact: none; npm run docs:check passes

Evidence and regression coverage

Tests added or updated: src/ghidra/GhidraProvider.behavior.test.ts, "Ghidra result projection › discloses that string search covers only Ghidra-defined strings". A stubbed Ghidra session returns an empty search_strings result, and the test asserts that the Evidence result is [] and its limitations include the defined-Data note.
Base reproduction: the new test fails on the base revision and passes with this change.
Remaining proof gaps: I did not run a real Ghidra session. The change is limitation metadata attached by the provider client; the bridge and the search itself are unchanged.

Validation performed

All commands ran on macOS (arm64) with Node 24.18.0 and npm 11.16.0:

npm run test:local -- src/ghidra/GhidraProvider.behavior.test.ts: 29 passed. The new case fails on base.
npm run check: passed.
npm run docs:check: passed.

Compatibility, safety, and release

Breaking changes or migration steps: none; one additional limitation string on Ghidra search_strings Evidence
Real Hopper/Ghidra, browser, or OS coverage: no real Ghidra run
Package or release metadata impact: none
Security, privacy, process, or containment review: none

Review checklist

This PR addresses a concrete problem or an agreed enhancement.
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.

🤖 Generated with Claude Code

_{🔄 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/1663 **Author:** [@Fazmin](https://github.com/Fazmin) **Created:** 10/11/2026 **Status:** ❌ Closed **Base:** `main` ← **Head:** `fix/ghidra-search-strings-limitation` --- ### 📝 Commits (1) - [`a508b66`](https://github.com/morluto/rea/commit/a508b6665f106daa73d47319436a6c1868d8937c) fix(ghidra): disclose that string search covers only defined strings ### 📊 Changes **3 files changed** (+26 additions, -2 deletions) <details> <summary>View changed files</summary> 📝 `docs/ghidra-provider.md` (+1 -1) 📝 `src/ghidra/GhidraProvider.behavior.test.ts` (+19 -0) 📝 `src/ghidra/GhidraProviderCapabilities.ts` (+6 -1) </details> ### 📄 Description ## Summary Fixes #1659. Ghidra's `search_strings` Evidence didn't say that it searches only Ghidra-defined string `Data`, so an empty result read as "this text is not in the file". `list_strings` already carries that limitation, and both read the same inventory. `search_strings` now carries its own limitation naming that scope, and points to `read_bytes` and `xrefs` for a known address. The Ghidra provider guide's Search row now says what "complete" covers for strings. Thanks to @CassieFox23 for the detailed report and reproduction. ## Problem and expected behavior - **Trigger:** `search_strings` (CLI `search --kind strings`) on a Ghidra session, for text Ghidra did not define as a string. The issue's example is packed Rust literals without a terminator, in stripped PyO3 modules. - **Before:** `limitationsFor` in `src/ghidra/GhidraProviderCapabilities.ts` put `search_strings` in the same branch as `search_procedures`, so its only operation-specific limitation was the Java-regex note. The `list_strings` branch carried "Only Ghidra-defined string Data is observed…". As the issue notes, the bridge collects strings from `getDefinedData(true)`, so both operations see the same defined-Data inventory. - **Expected (from the issue):** a caller who gets an empty list should know the list may be short. The search result should carry the same limit as `list_strings`, and may name the way around it (`read_bytes` plus `xrefs`). ## Change and scope - **`GhidraProviderCapabilities.ts`:** split `search_strings` from `search_procedures`. The two still share the regex limitation text through one constant. `search_strings` adds: > Only Ghidra-defined string Data is searched, so text Ghidra did not define as a string is not matched and an empty result does not show that the bytes are absent. read_bytes and xrefs can inspect a known address. - **`docs/ghidra-provider.md`:** the Search row now reads "scan the complete immutable inventory (for strings, Ghidra-defined `Data`)". This is a single-line change; the table is not re-padded. Intentionally out of scope: - **Shared tool description:** the provider-neutral `search_strings` tool description in `officialToolContracts.ts` ("complete over the native inventory") is shared with the Hopper and IDA providers. This change keeps the disclosure in Ghidra's per-result Evidence and provider guide; I'm happy to adjust the shared text if you prefer. - **Finding undefined text:** a byte-pattern search, which the issue suggests as a separate feature. - **Hopper and IDA:** their string semantics are untouched. ## Contract and boundary impact - Semantic owner and earliest changed stage: Ghidra provider limitation metadata attached to `search_strings` Evidence - CLI and MCP/tool-catalog contract: no schema or tool-description change; Ghidra `search_strings` Evidence carries one more `limitations` entry - Provider, bridge, target-format, or platform compatibility: Ghidra only; the bridge is unchanged - Evidence, artifact, provenance, or reconstruction contract: an existing unknown (defined-Data scope) is now disclosed on search results - Process execution, authorization, cleanup, or containment impact: none - Generated metadata, package, or installation impact: none; `npm run docs:check` passes ## Evidence and regression coverage - **Tests added or updated:** `src/ghidra/GhidraProvider.behavior.test.ts`, "Ghidra result projection › discloses that string search covers only Ghidra-defined strings". A stubbed Ghidra session returns an empty `search_strings` result, and the test asserts that the Evidence result is `[]` and its limitations include the defined-Data note. - **Base reproduction:** the new test fails on the base revision and passes with this change. - **Remaining proof gaps:** I did not run a real Ghidra session. The change is limitation metadata attached by the provider client; the bridge and the search itself are unchanged. ## Validation performed All commands ran on macOS (arm64) with Node 24.18.0 and npm 11.16.0: - `npm run test:local -- src/ghidra/GhidraProvider.behavior.test.ts`: 29 passed. The new case fails on base. - `npm run check`: passed. - `npm run docs:check`: passed. ## Compatibility, safety, and release - Breaking changes or migration steps: none; one additional limitation string on Ghidra `search_strings` Evidence - Real Hopper/Ghidra, browser, or OS coverage: no real Ghidra run - Package or release metadata impact: none - Security, privacy, process, or containment review: none ## Review checklist - [x] This PR addresses a concrete problem or an agreed enhancement. - [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. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi

2026-10-31 00:54:29 +00:00

closed this issue
added the
pull-request
label

bsakaguchi referenced this issue

2026-11-01 21:43:04 +00:00

[PR #5161] [CLOSED] fix(process): clear cleanup_incomplete after owner-confirmed cleanup retry #6790

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

No description provided.

Rows
Columns