morluto/rea

Fork 0

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

[GH-ISSUE #5475] [Bug] Clearing a Hopper bookmark name reports success but retains the previous name #3329

New issue

Closed

opened 2026-10-31 00:39:13 +00:00 by bsakaguchi · 2 comments

bsakaguchi commented

2026-10-31 00:39:13 +00:00

Owner

Originally created by @coygeek on GitHub (Oct 20, 2026).
Original GitHub issue: https://github.com/morluto/rea/issues/5475

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

On Hopper, replacing an existing named bookmark through MCP set_bookmark with name: "" returns structuredContent.normalized_result: true, but the next list_bookmarks still shows the previous name. The same empty string creates an unnamed bookmark when none exists. Omitting the name on an existing named bookmark also reports success while retaining its name.

This was independently reproduced through the production stdio MCP server in two sequential real-Hopper sessions, without mocks. The report was prepared with AI assistance from that independent verification; the submission check did not repeat the runtime probe.

Reproduction

From the source checkout, compile the existing fixture and start the production MCP server:

work="$(mktemp -d)"
mkdir -p "$work/home" "$work/config" "$work/data" "$work/cache" "$work/state"
clang -O0 -o "$work/bookmark" tests/fixtures/native-macos/native-macho-header/fixture.c
env HOME="$work/home" XDG_CONFIG_HOME="$work/config" XDG_DATA_HOME="$work/data" XDG_CACHE_HOME="$work/cache" XDG_STATE_HOME="$work/state" TMPDIR="$work/cache" HOPPER_LAUNCHER_PATH="/Applications/Hopper Disassembler.app/Contents/MacOS/hopper" REA_ANALYSIS_PROVIDER=hopper node scripts/rea.mjs mcp

Use one MCP connection and only one Hopper session at a time:

Call open_binary with {"path":"<absolute work directory>/bookmark","provider_id":"hopper"}. Call list_procedures and use a returned procedure address as ADDR. In the executed fixture, _owned_value was at 0x100000328; discover it rather than assuming compiler output is fixed. Confirm list_bookmarks is empty.
Call set_bookmark with {"address":ADDR,"name":""}, then list_bookmarks. The write returns true; the name is null.
Call set_bookmark with {"address":ADDR,"name":"stale-name"}. Readback shows stale-name.
Call set_bookmark with {"address":ADDR,"name":""}, then list_bookmarks. The write returns true, but the name remains stale-name. Calling set_bookmark with only {"address":ADDR} also returns true and retains the name.
Controls: nonempty replacement with replacement-name succeeds. unset_bookmark, followed by set_bookmark with only the address, returns true for both writes and produces a null name.
Remove the fixture bookmark and call close_binary. Both executed sessions closed successfully; binary_session reported open: false and cleanup_pending: false.

The sequence reproduced in both sessions: once using the existing fixture above, and once using a separate complete small C executable.

Expected behavior

Source-derived expectation: src/contracts/officialToolContracts.ts:222-224 advertises bookmark creation or replacement, and the live catalog describes name as the exact name used by the operation. The schema accepts an empty string. The same file at lines 102-103 specifies null names for unnamed bookmarks.

Successful explicit empty-name replacement should therefore leave an unnamed bookmark with name: null, consistent with the observed fresh-bookmark control.

Impact and scope

A successful metadata write leaves stale analyst navigation information, so callers cannot rely on success when clearing a bookmark name. Observed scope is the tested Hopper runtime and bookmark metadata; no binary-byte modification or security-boundary failure was established. Other Hopper versions and platforms were not tested.

Verified workaround: remove the bookmark and recreate it without a name. This produces the requested unnamed bookmark but requires two writes and temporarily removes it.

REA version

6.7.0; source commit 074768901f3cdc8205b61773fb1faf6f6c7b3ca8.

Runtime and platform

Node v26.12.0; macOS 27.0.1 arm64; Hopper Disassembler 6.1.0-demo, build 42.6295552.

Target and artifact context

Local Mach-O arm64 executable compiled with clang -O0; isolated home/config/data/cache/state directories. No saved Hopper database or external service was used.

Interface

MCP

Earliest failure stage

Native analysis or reconstruction

Relevant output or evidence

These values were read from production MCP responses at the same mapped address, using structuredContent.normalized_result:

Request	Write result	Name in subsequent `list_bookmarks`
Empty name, no bookmark present	`true`	`null`
Set `stale-name`	`true`	`stale-name`
Replace with empty name	`true`	`stale-name`
Set with name omitted	`true`	`stale-name`
Replace with `replacement-name`	`true`	`replacement-name`
Remove, then create with name omitted	Both `true`	`null`

The fixture compilation and both MCP replay processes exited successfully. Both sessions closed without pending cleanup.

Typed outcome or recovery expected

For explicit empty-name replacement, return success and read back an unnamed bookmark with name: null. Preserve empty-name creation and nonempty replacement behavior.

Analysis or suspected cause

Source-derived reasoning: bridge/hopper_bridge.py:998-1005 validates the mapped address and native text, forwards the optional name to Hopper, and reports whether a bookmark exists. It does not compare the resulting name. This explains how the response can report success despite retaining stale metadata; the native setter's internal cause was not established.

Additional context

Before submission, upstream main at 21e0c580e7456af311ecfb12af8fc2531b429606 had no changes after the verified baseline in the cited contract, bridge, Hopper provider, or MCP registration files. Searches included open and closed issues and PRs for set_bookmark, bookmark empty name, hopper_bridge.py, and bookmark. Related verification trackers #55 and #340 do not report this empty-name replacement failure.

Originally created by @coygeek on GitHub (Oct 20, 2026). Original GitHub issue: https://github.com/morluto/rea/issues/5475 ### 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 On Hopper, replacing an existing named bookmark through MCP `set_bookmark` with `name: ""` returns `structuredContent.normalized_result: true`, but the next `list_bookmarks` still shows the previous name. The same empty string creates an unnamed bookmark when none exists. Omitting the name on an existing named bookmark also reports success while retaining its name. This was independently reproduced through the production stdio MCP server in two sequential real-Hopper sessions, without mocks. The report was prepared with AI assistance from that independent verification; the submission check did not repeat the runtime probe. ### Reproduction From the source checkout, compile the existing fixture and start the production MCP server: ```sh work="$(mktemp -d)" mkdir -p "$work/home" "$work/config" "$work/data" "$work/cache" "$work/state" clang -O0 -o "$work/bookmark" tests/fixtures/native-macos/native-macho-header/fixture.c env HOME="$work/home" XDG_CONFIG_HOME="$work/config" XDG_DATA_HOME="$work/data" XDG_CACHE_HOME="$work/cache" XDG_STATE_HOME="$work/state" TMPDIR="$work/cache" HOPPER_LAUNCHER_PATH="/Applications/Hopper Disassembler.app/Contents/MacOS/hopper" REA_ANALYSIS_PROVIDER=hopper node scripts/rea.mjs mcp ``` Use one MCP connection and only one Hopper session at a time: 1. Call `open_binary` with `{"path":"<absolute work directory>/bookmark","provider_id":"hopper"}`. Call `list_procedures` and use a returned procedure address as `ADDR`. In the executed fixture, `_owned_value` was at `0x100000328`; discover it rather than assuming compiler output is fixed. Confirm `list_bookmarks` is empty. 2. Call `set_bookmark` with `{"address":ADDR,"name":""}`, then `list_bookmarks`. The write returns `true`; the name is `null`. 3. Call `set_bookmark` with `{"address":ADDR,"name":"stale-name"}`. Readback shows `stale-name`. 4. Call `set_bookmark` with `{"address":ADDR,"name":""}`, then `list_bookmarks`. The write returns `true`, but the name remains `stale-name`. Calling `set_bookmark` with only `{"address":ADDR}` also returns `true` and retains the name. 5. Controls: nonempty replacement with `replacement-name` succeeds. `unset_bookmark`, followed by `set_bookmark` with only the address, returns `true` for both writes and produces a `null` name. 6. Remove the fixture bookmark and call `close_binary`. Both executed sessions closed successfully; `binary_session` reported `open: false` and `cleanup_pending: false`. The sequence reproduced in both sessions: once using the existing fixture above, and once using a separate complete small C executable. ### Expected behavior Source-derived expectation: `src/contracts/officialToolContracts.ts:222-224` advertises bookmark creation or replacement, and the live catalog describes `name` as the exact name used by the operation. The schema accepts an empty string. The same file at lines 102-103 specifies `null` names for unnamed bookmarks. Successful explicit empty-name replacement should therefore leave an unnamed bookmark with `name: null`, consistent with the observed fresh-bookmark control. ### Impact and scope A successful metadata write leaves stale analyst navigation information, so callers cannot rely on success when clearing a bookmark name. Observed scope is the tested Hopper runtime and bookmark metadata; no binary-byte modification or security-boundary failure was established. Other Hopper versions and platforms were not tested. Verified workaround: remove the bookmark and recreate it without a name. This produces the requested unnamed bookmark but requires two writes and temporarily removes it. ### REA version 6.7.0; source commit `074768901f3cdc8205b61773fb1faf6f6c7b3ca8`. ### Runtime and platform Node v26.12.0; macOS 27.0.1 arm64; Hopper Disassembler 6.1.0-demo, build 42.6295552. ### Target and artifact context Local Mach-O arm64 executable compiled with `clang -O0`; isolated home/config/data/cache/state directories. No saved Hopper database or external service was used. ### Interface MCP ### Earliest failure stage Native analysis or reconstruction ### Relevant output or evidence These values were read from production MCP responses at the same mapped address, using `structuredContent.normalized_result`: | Request | Write result | Name in subsequent `list_bookmarks` | | --- | --- | --- | | Empty name, no bookmark present | `true` | `null` | | Set `stale-name` | `true` | `stale-name` | | Replace with empty name | `true` | `stale-name` | | Set with name omitted | `true` | `stale-name` | | Replace with `replacement-name` | `true` | `replacement-name` | | Remove, then create with name omitted | Both `true` | `null` | The fixture compilation and both MCP replay processes exited successfully. Both sessions closed without pending cleanup. ### Typed outcome or recovery expected For explicit empty-name replacement, return success and read back an unnamed bookmark with `name: null`. Preserve empty-name creation and nonempty replacement behavior. ### Analysis or suspected cause Source-derived reasoning: `bridge/hopper_bridge.py:998-1005` validates the mapped address and native text, forwards the optional name to Hopper, and reports whether a bookmark exists. It does not compare the resulting name. This explains how the response can report success despite retaining stale metadata; the native setter's internal cause was not established. ### Additional context Before submission, upstream `main` at `21e0c580e7456af311ecfb12af8fc2531b429606` had no changes after the verified baseline in the cited contract, bridge, Hopper provider, or MCP registration files. Searches included open and closed issues and PRs for `set_bookmark`, `bookmark empty name`, `hopper_bridge.py`, and `bookmark`. Related verification trackers #55 and #340 do not report this empty-name replacement failure.

bsakaguchi closed this issue

2026-10-31 00:39:14 +00:00

bsakaguchi commented

2026-10-31 00:39:14 +00:00

Author

Owner

@matbrgz commented on GitHub (Oct 20, 2026):

Cross-referencing with canonical issue #5245 and sibling #5454 regarding Hopper provider lifecycle and state mutations. Cataloged under Cluster AU in consolidation tracker #4330.

@matbrgz commented on GitHub (Oct 20, 2026): Cross-referencing with canonical issue #5245 and sibling #5454 regarding Hopper provider lifecycle and state mutations. Cataloged under Cluster AU in consolidation tracker #4330.

bsakaguchi commented

2026-10-31 00:39:14 +00:00

Author

Owner

@morluto commented on GitHub (Oct 21, 2026):

Closing as fixed on main by github.com/morluto/rea@fd7da77fc0, retained at 67a6477f01.

The Hopper bridge uses renameBookmarkAtAddress for empty-name replacement and verifies the actual name by readback instead of accepting existence alone. Regressions cover explicit empty name, omitted name, ignored setter and missing rename support. No new Hopper 6.1 run was performed; the landed commit records its own Hopper 6.6 verification scope.

This closure follows current-main producer and regression source inspection and the existing issue evidence. Tests and runtime reproductions were not rerun in this audit; no additional platform/provider coverage is implied.

@morluto commented on GitHub (Oct 21, 2026): Closing as fixed on main by https://github.com/morluto/rea/commit/fd7da77fc02dcfaf7d1bc9f2fb240d71cdd27c59, retained at 67a6477f011ae358fc333e5a6e26c86d932654ed. The Hopper bridge uses renameBookmarkAtAddress for empty-name replacement and verifies the actual name by readback instead of accepting existence alone. Regressions cover explicit empty name, omitted name, ignored setter and missing rename support. No new Hopper 6.1 run was performed; the landed commit records its own Hopper 6.6 verification scope. This closure follows current-main producer and regression source inspection and the existing issue evidence. Tests and runtime reproductions were not rerun in this audit; no additional platform/provider coverage is implied.

bsakaguchi referenced this issue

2026-10-31 01:10:46 +00:00

[PR #3904] [MERGED] fix(ghidra): refuse PSP targets with unexamined ABI declarations #5962

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

No description provided.

Rows
Columns