shifathosn
/
rea
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

fix(ghidra): preserve Unicode boundaries without breaking sessions 

Validate native annotation text before mutation and escape surrogate code units in Java bridge JSON replies. Reject unsupported text with field and index diagnostics while preserving supported Unicode and line endings.

Probe real CLI/MCP annotation, transport, mapping, rollback and cleanup behavior; replace seven redundant unit tests with E2E and SDK boundary coverage.
This commit is contained in:
morluto 2026-10-09 02:50:33 +08:00
parent 64717ad7da
commit bef234d6ce
8 changed files with 222 additions and 174 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

29
bridge/ghidra/ReaGhidraBridge.java
View File

@ -1007,6 +1007,9 @@ public final class ReaGhidraBridge extends HeadlessScript {
Address entry = function.getEntryPoint();
if (function.isExternal() || !currentProgram.getMemory().contains(entry))
throw new RequestFailure("invalid_request", "Annotations require a local function entry: " + canonicalAddress(entry));
for (String field : List.of("name", "comment", "inline_comment")) {
if (params.has(field)) validateAnnotationText(requireText(params, field), field);
}
int transaction = currentProgram.startTransaction("REA function annotations");
boolean commit = false;
try {
@ -1065,6 +1068,21 @@ public final class ReaGhidraBridge extends HeadlessScript {
return value.getAsString();
}
private static void validateAnnotationText(String text, String field) {
for (int index = 0; index < text.length(); index++) {
char character = text.charAt(index);
if (character == 0)
throw new RequestFailure("invalid_request", "Annotation " + field + " contains NUL at UTF-16 index " + index + "; Ghidra's decompiler cannot marshal NUL text");
if (Character.isHighSurrogate(character) && index + 1 < text.length() &&
Character.isLowSurrogate(text.charAt(index + 1))) {
index++;
continue;
}
if (Character.isSurrogate(character))
throw new RequestFailure("invalid_request", "Annotation " + field + " contains an unpaired Unicode surrogate at UTF-16 index " + index + "; Ghidra cannot preserve it as annotation text");
}
}
private JsonObject analyzeFunction(JsonObject params) throws Exception {
requireKeys(params, Set.of("procedure"));
Function function = resolveProcedure(requireString(params, "procedure"));
@ -3228,7 +3246,16 @@ public final class ReaGhidraBridge extends HeadlessScript {
private static void writeResponse(BufferedWriter writer, JsonObject response) throws IOException {
String encoded = GSON.toJson(response);
writer.write(encoded);
// Gson leaves UTF-16 surrogates literal. Escape them in JSON so even
// diagnostics quoting ill-formed caller text remain lossless UTF-8.
int start = 0;
for (int index = 0; index < encoded.length(); index++) {
if (!Character.isSurrogate(encoded.charAt(index))) continue;
writer.write(encoded, start, index - start);
writer.write("\\u" + Integer.toHexString(encoded.charAt(index)));
start = index + 1;
}
writer.write(encoded, start, encoded.length() - start);
writer.newLine();
writer.flush();
}

5
docs/native-investigation.md
View File

@ -257,7 +257,10 @@ protected runtime DACLs, and handle-based path admission. See [Windows Ghidra P0
[issue #527](https://github.com/morluto/rea/issues/527).
On Linux and macOS, `annotate_native_function` atomically edits a function name
and entry comments in the ephemeral database, returning refreshed analysis
without changing executable bytes. Windows P0 remains read-only. Ghidra has no
without changing executable bytes. Annotation text must contain no NUL or
unpaired Unicode surrogate; a rejection identifies the field and UTF-16 index
and leaves every annotation unchanged. CRLF, supplementary Unicode characters,
and combining characters are preserved. Windows P0 remains read-only. Ghidra has no
GUI authority, and REA never falls back automatically to Hopper.
- `npm run verify:ghidra`: host-native debug/stripped targets, native type layout,

8
docs/testing.md
View File

@ -249,8 +249,12 @@ direct versus targetless calls, byte-read completeness, invalid input diagnostic
CLI/MCP parity, atomic annotation rollback, refreshed inventories, unchanged
executable bytes, and discarded edits after reopen. A deliberately long temporary
path exercises private Unix socket allocation and cleanup, including cancellation
after a real headless process launches. Malformed annotation
readback remains a separate SDK/provider integration case; success from a real
after a real headless process launches. Native annotation probes reject NUL and
unpaired Unicode surrogates without partial edits or a broken bridge, preserve
supported Unicode and control text, and check lossless malformed-text diagnostics.
Memory-to-file mapping is checked against original artifact bytes. Malformed
annotation readback, memory completeness, and inventory data remain separate
SDK/provider integration cases; success from a real
provider cannot establish rejection of a contradictory provider response.
The Linux switch lane checks dense, sparse-with-holes, shared-body, nonzero,

114
scripts/lib/real-ghidra-boundaries.mjs
View File

@ -97,6 +97,16 @@ export async function verifyGhidraBoundaries(
assert.equal(baseline.status, "decoded");
const bytes = await call("read_bytes", { address, length: baseline.length });
assert.equal(bytes.bytes_hex, baseline.bytes);
assert.equal((await call("read_bytes", { address })).requested_bytes, 256);
const mapped = await call("address_to_file_offset", { address });
assert.equal(mapped.address, address);
assert.equal(
(await readFile(target.path))
.subarray(mapped.file_offset, mapped.file_offset + baseline.length)
.toString("hex"),
bytes.bytes_hex,
"Memory-to-file mapping must identify the bytes in the caller's artifact",
);
const largeRead = await call("read_bytes", {
address,
length: Number.MAX_SAFE_INTEGER,
@ -182,6 +192,28 @@ export async function verifyGhidraBoundaries(
}
const strings = await call("list_strings");
assert.ok(strings.length > 0);
assert.deepEqual(
await call("list_strings", { address: strings[0].address }),
[strings[0]],
);
const marker = strings.find(
(item) => item.value === "REA_GHIDRA_INVENTORY_ENTRY",
);
assert.ok(marker);
assert.ok(
(
await call("search_strings", { pattern: marker.value.toLowerCase() })
).some((item) => item.address === marker.address),
);
assert.equal(
(
await call("search_strings", {
pattern: marker.value.toLowerCase(),
case_sensitive: true,
})
).some((item) => item.address === marker.address),
false,
);
assert.equal(
(await call("inspect_native_instruction", { address: strings[0].address }))
.status,
@ -218,6 +250,26 @@ export async function verifyGhidraBoundaries(
{ document: "rea_missing_program" },
/rea_missing_program.*active program/u,
);
// JSON permits escaped lone surrogates. Error replies must preserve them
// without letting the Java UTF-8 writer terminate the provider session.
await invalid(
"procedure_address",
{ procedure: "missing_\ud800" },
/missing_\\ud800/u,
);
await invalid(
"list_procedures",
{ document: "missing_\udfff" },
/missing_\\udfff/u,
);
for (const [operation, args] of [
["read_bytes", { address, length: 0 }],
["list_procedures", { limit: 100 }],
["list_documents", { extra: true }],
["xrefs", {}],
["procedure_info", { procedure: address, extra: true }],
])
await invalid(operation, args);
for (const changes of [
{},
{ name: "" },
@ -296,6 +348,46 @@ export async function verifyGhidraBoundaries(
).some((item) => item.address === address),
);
for (const field of ["name", "comment", "inline_comment"]) {
for (const text of ["bad\0text", "bad\ud800text", "bad\udffftext"]) {
await invalid(
"annotate_native_function",
{
procedure: address,
name: "MUST_ROLL_BACK",
comment: "MUST ROLL BACK",
inline_comment: "MUST ROLL BACK",
[field]: text,
},
new RegExp(
`Annotation ${field}.*(?:NUL|unpaired Unicode surrogate).*index`,
"u",
),
);
assert.deepEqual(
(
await call("annotate_native_function", {
procedure: address,
name: changes.name,
})
).annotations,
updated.annotations,
"Invalid native text must leave every annotation unchanged",
);
}
}
const unicodeText =
"CRLF\r\nastral 🧪 decomposed e\u0301 control \u0001\b\v\f\u001f noncharacter \ufffe\uffff";
const unicode = await call("annotate_native_function", {
procedure: address,
comment: unicodeText,
inline_comment: unicodeText,
});
assert.equal(unicode.annotations.comment, unicodeText);
assert.equal(unicode.annotations.inline_comment, unicodeText);
assert.ok(unicode.dossier.comments.some((item) => item.text === unicodeText));
await call("annotate_native_function", changes);
await invalid(
"annotate_native_function",
{
@ -339,15 +431,17 @@ export async function verifyGhidraBoundaries(
]);
assert.deepEqual(cliUpdated.annotations, updated.annotations);
assert.deepEqual(cliUpdated.effects, updated.effects);
await call("annotate_native_function", {
procedure: address,
name: "0xordinary",
});
assert.equal(
await call("procedure_address", { procedure: "0xordinary" }),
address,
);
assert.equal(await call("address_name", { address }), "0xordinary");
for (const renamed of ["0xordinary", "probe::qualified", "🧪probe"]) {
await call("annotate_native_function", {
procedure: address,
name: renamed,
});
assert.equal(
await call("procedure_address", { procedure: renamed }),
address,
);
assert.equal(await call("address_name", { address }), renamed);
}
await call("close_binary");
await assert.rejects(access(socketRoot), { code: "ENOENT" });
await assert.rejects(access(runtimeRoot), { code: "ENOENT" });
@ -372,6 +466,8 @@ export async function verifyGhidraBoundaries(
rejected_calls: rejectedCalls,
cli_mcp_parity: true,
mutation_rollback: true,
annotation_native_text_validation: true,
lossless_unicode_transport: true,
source_immutable: true,
reopen_discards_edits: true,
long_tmpdir_private_socket_cleanup: true,

46
src/ghidra/GhidraFunctionValues.test.ts
View File

@ -3,7 +3,6 @@ import { describe, expect, it } from "vitest";
import type { JsonValue } from "../domain/jsonValue.js";
import { functionDossierSchema } from "../domain/hopperValues.js";
import {
parseGhidraFunctionInput,
parseGhidraFunctionResult,
type GhidraFunctionOperation,
} from "./GhidraFunctionValues.js";
@ -16,51 +15,6 @@ import {
ghidraReferenceEdge,
} from "../domain/ghidraValues.fixture.js";
describe("Ghidra function-analysis boundary values", () => {
it("defaults inputs and rejects undeclared or implicit addresses", () => {
expect(
parseGhidraFunctionInput("procedure_info", { procedure: "main" }),
).toEqual({
ok: true,
value: { document: null, procedure: "main" },
});
expect(
parseGhidraFunctionInput("procedure_references", { procedure: "main" }),
).toEqual({
ok: true,
value: {
document: null,
procedure: "main",
direction: "outgoing",
},
});
expect(
parseGhidraFunctionInput("analyze_function", { procedure: "main" }),
).toMatchObject({
ok: true,
value: { procedure: "main" },
});
expect(
parseGhidraFunctionInput("read_function_instructions", {
procedure: "main",
}),
).toEqual({
ok: true,
value: { document: null, procedure: "main" },
});
expect(parseGhidraFunctionInput("xrefs", {})).toMatchObject({
ok: false,
error: { _tag: "AnalysisInputError" },
});
expect(
parseGhidraFunctionInput("procedure_info", {
procedure: "main",
extra: true,
}),
).toMatchObject({ ok: false, error: { _tag: "AnalysisInputError" } });
});
});
describe("Ghidra function-analysis result values", () => {
it("parses provider-classified function facts and reference kinds", () => {
expect(

116
src/ghidra/GhidraInventoryValues.test.ts
View File

@ -1,120 +1,6 @@
import { describe, expect, it } from "vitest";
import {
isGhidraInventoryOperation,
parseGhidraInventoryInput,
parseGhidraInventoryResult,
} from "./GhidraInventoryValues.js";
describe("Ghidra inventory boundary values", () => {
it("admits explicit byte reads and source offset mapping without invented caps", () => {
expect(
parseGhidraInventoryInput("read_bytes", { address: "0x10000" }),
).toEqual({
ok: true,
value: { address: "0x10000", document: null, length: 256 },
});
expect(
parseGhidraInventoryInput("read_bytes", {
address: "0x10000",
length: 4097,
}).ok,
).toBe(true);
expect(
parseGhidraInventoryInput("read_bytes", { address: "0x10000", length: 0 })
.ok,
).toBe(false);
expect(
parseGhidraInventoryInput("address_to_file_offset", {
address: "HEADER:0x1",
}).ok,
).toBe(true);
});
it("checks byte length and completeness before accepting memory Evidence", () => {
const result = {
address: "0x10000",
requested_bytes: 4,
returned_bytes: 2,
bytes_hex: "0410",
complete: false,
};
expect(parseGhidraInventoryResult("read_bytes", result)).toEqual({
ok: true,
value: result,
});
for (const change of [
{ complete: true },
{ returned_bytes: 4 },
{ bytes_hex: "04" },
{ requested_bytes: 1 },
])
expect(
parseGhidraInventoryResult("read_bytes", { ...result, ...change }).ok,
).toBe(false);
expect(
parseGhidraInventoryResult("address_to_file_offset", {
address: "0x10000",
file_offset: -1,
}).ok,
).toBe(false);
});
it("accepts complete inventories and searches without page controls", () => {
expect(parseGhidraInventoryInput("list_procedures", {})).toEqual({
ok: true,
value: { document: null },
});
expect(
parseGhidraInventoryInput("search_strings", { pattern: "needle" }),
).toEqual({
ok: true,
value: {
pattern: "needle",
mode: "literal",
case_sensitive: false,
document: null,
},
});
expect(
parseGhidraInventoryInput("list_procedures", { limit: 100 }),
).toMatchObject({ ok: false, error: { _tag: "AnalysisInputError" } });
expect(
parseGhidraInventoryInput("list_documents", { extra: true }),
).toMatchObject({ ok: false, error: { _tag: "AnalysisInputError" } });
});
it("keeps the admitted operation set closed", () => {
expect(isGhidraInventoryOperation("list_names")).toBe(true);
expect(isGhidraInventoryOperation("goto_address")).toBe(false);
expect(isGhidraInventoryOperation("set_comment")).toBe(false);
});
it("returns complete typed inventory arrays", () => {
const procedures = Array.from({ length: 700 }, (_, index) => ({
address: `0x${(0x401000 + index).toString(16)}`,
value: `sub_${index}`,
procedure: { external: false, thunk: false, thunk_target: null },
}));
expect(parseGhidraInventoryResult("list_procedures", procedures)).toEqual({
ok: true,
value: procedures,
});
});
it("rejects malformed inventory items", () => {
expect(
parseGhidraInventoryResult("list_procedures", [
{
address: "00401000",
value: "main",
procedure: { external: false, thunk: false, thunk_target: null },
},
]),
).toMatchObject({ ok: false, error: { _tag: "AnalysisOutputError" } });
expect(
parseGhidraInventoryResult("search_strings", [{ address: "0x1000" }]),
).toMatchObject({ ok: false, error: { _tag: "AnalysisOutputError" } });
});
});
import { parseGhidraInventoryResult } from "./GhidraInventoryValues.js";
// Wire-boundary fixture only; real-provider acceptance belongs to the DOS verifier.
describe("containing-procedure complete body evidence", () => {

1
src/ghidra/GhidraProviderCapabilities.ts
View File

@ -46,6 +46,7 @@ export const limitationsFor = (operation: string): readonly string[] => {
return [
...common,
"Names use Ghidra USER_DEFINED source. Name writes preserve the existing namespace; readback uses the fully qualified name. Regular comments map to PRE and inline comments to EOL at the exact function entry. Changes commit together after readback and refreshed analysis; failure rolls them all back.",
"Annotation text rejects NUL and unpaired Unicode surrogates before mutation, with the field and UTF-16 index in the error. Supported Unicode and line endings are preserved.",
"Metadata edits invalidate immutable analysis snapshots and are discarded on close. CLI returns the updated dossier before session cleanup; this is not a saved Ghidra project.",
];
case "inspect_native_load_image":

77
tests/boundary/mcp/ghidraInventoryMcp.test.ts Normal file
View File

@ -0,0 +1,77 @@
import { expect, it } from "vitest";
import { jsonValueSchema } from "../../../src/domain/jsonValue.js";
import { ok } from "../../../src/domain/result.js";
import { connectGhidraMcp } from "./ghidraMcpHarness.js";
it("rejects contradictory provider memory and inventory data before emitting MCP Evidence", async () => {
const bytes = {
address: "0x401000",
requested_bytes: 4,
returned_bytes: 2,
bytes_hex: "0410",
complete: false,
};
let output = jsonValueSchema.parse(bytes);
const harness = await connectGhidraMcp("ghidra-invalid-inventory", () =>
Promise.resolve(ok(output)),
);
try {
const accepted = await harness.mcp.callTool({
name: "read_bytes",
arguments: { address: bytes.address, length: 4 },
});
expect(accepted.isError).not.toBe(true);
expect(accepted.structuredContent).toMatchObject({ result: bytes });
const cases = [
...[
{ complete: true },
{ returned_bytes: 4 },
{ bytes_hex: "04" },
{ requested_bytes: 1 },
].map((change, index) => ({
name: "read_bytes",
arguments: {
address: `0x${(0x401004 + index * 4).toString(16)}`,
length: 4,
},
output: { ...bytes, ...change },
})),
{
name: "address_to_file_offset",
arguments: { address: bytes.address },
output: { address: bytes.address, file_offset: -1 },
},
{
name: "list_procedures",
arguments: {},
output: [
{
address: "00401000",
value: "main",
procedure: { external: false, thunk: false, thunk_target: null },
},
],
},
{
name: "search_strings",
arguments: { pattern: "needle" },
output: [{ address: "0x401000" }],
},
];
for (const probe of cases) {
output = jsonValueSchema.parse(probe.output);
const rejected = await harness.mcp.callTool({
name: probe.name,
arguments: probe.arguments,
});
expect(rejected.isError, probe.name).toBe(true);
expect(rejected.structuredContent).toMatchObject({
error: { code: "unreadable_output" },
});
expect(rejected.structuredContent).not.toHaveProperty("evidence_id");
expect(rejected.structuredContent).not.toHaveProperty("result");
}
} finally {
await harness.close();
}
});