fix(bridge): harden bounded Hopper boundaries
This commit is contained in:
parent
151469f093
commit
f8e8ea35ff
|
|
@ -274,9 +274,15 @@ def _search_page(document, kind, params):
|
|||
raise ValueError("Invalid regex pattern") from error
|
||||
matches = lambda value: expression.search(value) is not None
|
||||
|
||||
matching = [item for item in _search_inventory(document, kind) if matches(item[1])]
|
||||
selected = matching[offset:offset + limit]
|
||||
total = len(matching)
|
||||
selected = []
|
||||
total = 0
|
||||
page_end = offset + limit
|
||||
for item in _search_inventory(document, kind):
|
||||
if not matches(item[1]):
|
||||
continue
|
||||
if offset <= total < page_end:
|
||||
selected.append(item)
|
||||
total += 1
|
||||
next_offset = offset + len(selected)
|
||||
has_more = next_offset < total
|
||||
return {
|
||||
|
|
@ -654,7 +660,7 @@ def _serve_connection(connection):
|
|||
should_stop = request["method"] == "shutdown"
|
||||
response = {"id": request_id, "result": _json_safe(result)}
|
||||
except Exception as error:
|
||||
response = {"id": request_id if isinstance(request_id, int) else 0, "error": {"code": -32000, "message": str(error)[:512]}}
|
||||
response = {"id": request_id if isinstance(request_id, int) else 0, "error": {"code": -32000, "message": str(error)[:512], "type": _diagnostic_type(error)}}
|
||||
file.write((json.dumps(response, separators=(",", ":")) + "\n").encode("utf-8"))
|
||||
file.flush()
|
||||
if should_stop:
|
||||
|
|
@ -663,6 +669,14 @@ def _serve_connection(connection):
|
|||
connection.close()
|
||||
|
||||
|
||||
def _diagnostic_type(error):
|
||||
if isinstance(error, PermissionError):
|
||||
return "authorization"
|
||||
if isinstance(error, (ValueError, TypeError, KeyError)):
|
||||
return "invalid_request"
|
||||
return "bridge_exception"
|
||||
|
||||
|
||||
def _run():
|
||||
"""Own a permission-restricted, single-client Unix socket for this bridge."""
|
||||
if os.path.exists(REA_SOCKET):
|
||||
|
|
|
|||
|
|
@ -6,6 +6,7 @@ import type { EnhancedToolName } from "../contracts/enhancedInputs.js";
|
|||
import { enhancedInputSchemas } from "../contracts/enhancedInputs.js";
|
||||
import {
|
||||
AnalysisInputError,
|
||||
AnalysisCancelledError,
|
||||
AnalysisOutputError,
|
||||
type AnalysisError,
|
||||
} from "../domain/errors.js";
|
||||
|
|
@ -472,6 +473,7 @@ export class EnhancedTools {
|
|||
arguments_: Readonly<Record<string, JsonValue>>,
|
||||
signal?: AbortSignal,
|
||||
): Promise<Result<JsonValue, AnalysisError>> {
|
||||
if (signal?.aborted === true) return err(new AnalysisCancelledError(name));
|
||||
const execution = await this.analysis.execute(
|
||||
name,
|
||||
arguments_,
|
||||
|
|
|
|||
|
|
@ -87,12 +87,14 @@ const providerCapability = z.object({
|
|||
}),
|
||||
limitations: z.array(z.string()),
|
||||
});
|
||||
const providerIdentity = z.object({
|
||||
id: z.string(),
|
||||
name: z.string(),
|
||||
version: z.string().nullable(),
|
||||
});
|
||||
const sessionProvider = z.object({
|
||||
provider: z.object({
|
||||
id: z.string(),
|
||||
name: z.string(),
|
||||
version: z.string().nullable(),
|
||||
}),
|
||||
provider: providerIdentity,
|
||||
providers: z.array(providerIdentity),
|
||||
capabilities: z.array(providerCapability),
|
||||
});
|
||||
const nullableText = z.string().nullable();
|
||||
|
|
|
|||
|
|
@ -216,6 +216,12 @@ export class HopperProtocolError extends HopperError {
|
|||
readonly _tag = "HopperProtocolError";
|
||||
}
|
||||
|
||||
export type HopperDiagnosticType =
|
||||
| "remote"
|
||||
| "authorization"
|
||||
| "invalid_request"
|
||||
| "bridge_exception";
|
||||
|
||||
/** Hopper's JSON-RPC endpoint returned an expected remote error response. */
|
||||
export class HopperRemoteError extends HopperError {
|
||||
readonly _tag = "HopperRemoteError";
|
||||
|
|
@ -223,6 +229,7 @@ export class HopperRemoteError extends HopperError {
|
|||
constructor(
|
||||
readonly code: number,
|
||||
readonly safeMessage: string,
|
||||
readonly diagnosticType: HopperDiagnosticType = "remote",
|
||||
) {
|
||||
super(`Hopper request failed (${String(code)}): ${safeMessage}`);
|
||||
}
|
||||
|
|
@ -322,7 +329,11 @@ const safeDetails = (
|
|||
if (error instanceof HopperTimeoutError)
|
||||
return { timeoutMs: error.timeoutMs };
|
||||
if (error instanceof HopperRemoteError)
|
||||
return { code: error.code, safeMessage: error.safeMessage };
|
||||
return {
|
||||
code: error.code,
|
||||
safeMessage: error.safeMessage,
|
||||
diagnosticType: error.diagnosticType,
|
||||
};
|
||||
if (error instanceof HopperProcessError) return { exitCode: error.exitCode };
|
||||
return {};
|
||||
};
|
||||
|
|
|
|||
|
|
@ -37,11 +37,31 @@ const segmentSchema = z.object({
|
|||
writable: z.boolean().nullable().default(null),
|
||||
executable: z.boolean().nullable().default(null),
|
||||
});
|
||||
const addressedPageSchema = z.object({
|
||||
items: z.array(z.object({ address: z.string(), value: z.string() })),
|
||||
next_offset: z.number().int().min(0).nullable(),
|
||||
has_more: z.boolean(),
|
||||
});
|
||||
const addressedPageSchema = z
|
||||
.object({
|
||||
items: z.array(z.object({ address: z.string(), value: z.string() })),
|
||||
offset: z.number().int().min(0),
|
||||
limit: z.number().int().min(1),
|
||||
total: z.number().int().min(0),
|
||||
next_offset: z.number().int().min(0).nullable(),
|
||||
has_more: z.boolean(),
|
||||
})
|
||||
.superRefine((value, context) => {
|
||||
if (value.has_more && value.next_offset === null) {
|
||||
context.addIssue({
|
||||
code: "custom",
|
||||
message: "a page with more results must provide next_offset",
|
||||
path: ["next_offset"],
|
||||
});
|
||||
}
|
||||
if (!value.has_more && value.next_offset !== null) {
|
||||
context.addIssue({
|
||||
code: "custom",
|
||||
message: "a complete page must not provide next_offset",
|
||||
path: ["next_offset"],
|
||||
});
|
||||
}
|
||||
});
|
||||
const unavailableSchema = z
|
||||
.object({ available: z.literal(false), reason: z.string() })
|
||||
.strict();
|
||||
|
|
@ -86,6 +106,17 @@ const boundedSchema = <T extends z.ZodType>(item: T) =>
|
|||
path: ["next_offset"],
|
||||
});
|
||||
}
|
||||
if (
|
||||
!value.truncated &&
|
||||
value.total !== null &&
|
||||
value.total !== value.returned
|
||||
) {
|
||||
context.addIssue({
|
||||
code: "custom",
|
||||
message: "a complete collection total must equal returned",
|
||||
path: ["total"],
|
||||
});
|
||||
}
|
||||
});
|
||||
const referenceEdgeSchema = z
|
||||
.object({
|
||||
|
|
|
|||
|
|
@ -11,7 +11,18 @@ const responseSchema = z.union([
|
|||
}),
|
||||
z.object({
|
||||
id: z.number().int().nonnegative(),
|
||||
error: z.object({ code: z.number().int(), message: z.string() }),
|
||||
error: z.object({
|
||||
code: z.number().int(),
|
||||
message: z.string(),
|
||||
type: z
|
||||
.enum([
|
||||
"remote",
|
||||
"authorization",
|
||||
"invalid_request",
|
||||
"bridge_exception",
|
||||
])
|
||||
.default("remote"),
|
||||
}),
|
||||
}),
|
||||
]);
|
||||
|
||||
|
|
@ -46,5 +57,11 @@ export const responseResult = (
|
|||
response: HopperResponse,
|
||||
): Result<JsonValue, HopperRemoteError> =>
|
||||
"error" in response
|
||||
? err(new HopperRemoteError(response.error.code, response.error.message))
|
||||
? err(
|
||||
new HopperRemoteError(
|
||||
response.error.code,
|
||||
response.error.message,
|
||||
response.error.type,
|
||||
),
|
||||
)
|
||||
: ok(response.result);
|
||||
|
|
|
|||
|
|
@ -73,6 +73,11 @@ describe("Hopper bridge truthfulness", () => {
|
|||
expect(bridgeSource).toContain("MAX_SEARCH_PATTERN_LENGTH = 256");
|
||||
expect(bridgeSource).toContain("MAX_SEARCH_VALUE_LENGTH = 4096");
|
||||
expect(bridgeSource).toContain('"value_truncated"');
|
||||
expect(bridgeSource).toContain("page_end = offset + limit");
|
||||
expect(bridgeSource).toContain("if offset <= total < page_end:");
|
||||
expect(bridgeSource).not.toContain(
|
||||
"matching = [item for item in _search_inventory(document, kind)",
|
||||
);
|
||||
});
|
||||
|
||||
it("caches sorted search inventories and invalidates them after renames", () => {
|
||||
|
|
@ -80,4 +85,10 @@ describe("Hopper bridge truthfulness", () => {
|
|||
expect(bridgeSource).toContain("tuple(sorted(values.items()");
|
||||
expect(bridgeSource).toContain("_invalidate_search_inventory(document)");
|
||||
});
|
||||
|
||||
it("classifies bridge exceptions without forwarding tracebacks", () => {
|
||||
expect(bridgeSource).toContain("def _diagnostic_type(error):");
|
||||
expect(bridgeSource).toContain('"type": _diagnostic_type(error)');
|
||||
expect(bridgeSource).not.toContain("traceback.format_exc");
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -3,6 +3,7 @@ import type { CallToolResult } from "@modelcontextprotocol/server";
|
|||
import { afterEach, describe, expect, it } from "vitest";
|
||||
|
||||
import type { AnalysisOperationPort } from "../src/application/AnalysisProvider.js";
|
||||
import { EnhancedTools } from "../src/application/EnhancedTools.js";
|
||||
import { ENHANCED_TOOL_CONTRACTS } from "../src/contracts/toolContracts.js";
|
||||
import { observed as ok } from "./fixtures/analysisExecution.js";
|
||||
import { jsonValueSchema, type JsonValue } from "../src/domain/jsonValue.js";
|
||||
|
|
@ -304,6 +305,38 @@ describe("enhanced MCP tools", () => {
|
|||
expect(result).toMatchObject({ count: 2 });
|
||||
});
|
||||
|
||||
it("does not start another page after cancellation", async () => {
|
||||
const controller = new AbortController();
|
||||
let calls = 0;
|
||||
const analysis: AnalysisOperationPort = {
|
||||
execute: () => {
|
||||
calls += 1;
|
||||
controller.abort();
|
||||
return Promise.resolve(
|
||||
ok({
|
||||
items: [{ address: "0x1", value: "_TtC5First" }],
|
||||
offset: 0,
|
||||
limit: 500,
|
||||
total: 2,
|
||||
next_offset: 500,
|
||||
has_more: true,
|
||||
}),
|
||||
);
|
||||
},
|
||||
};
|
||||
|
||||
const result = await new EnhancedTools(analysis).execute(
|
||||
"swift_classes",
|
||||
{},
|
||||
controller.signal,
|
||||
);
|
||||
|
||||
expect(calls).toBe(1);
|
||||
expect(result.ok).toBe(false);
|
||||
if (result.ok) throw new Error("expected cancellation");
|
||||
expect(result.error._tag).toBe("AnalysisCancelledError");
|
||||
});
|
||||
|
||||
it("follows name pagination for exhaustive Objective-C discovery", async () => {
|
||||
const offsets: number[] = [];
|
||||
const client = await connect({
|
||||
|
|
@ -534,4 +567,40 @@ describe("enhanced MCP tools", () => {
|
|||
"AnalysisOutputError",
|
||||
);
|
||||
});
|
||||
|
||||
it("rejects complete dossier collections with inflated totals", async () => {
|
||||
const malformedPort = fixturePort();
|
||||
const client = await connect({
|
||||
execute: async (name, arguments_, options) => {
|
||||
const result = await malformedPort.execute(name, arguments_, options);
|
||||
if (!result.ok || name !== "analyze_function") return result;
|
||||
const dossier = result.value.result;
|
||||
if (
|
||||
typeof dossier !== "object" ||
|
||||
dossier === null ||
|
||||
Array.isArray(dossier)
|
||||
)
|
||||
return result;
|
||||
return ok({
|
||||
...dossier,
|
||||
callers: {
|
||||
items: [],
|
||||
total: 1,
|
||||
returned: 0,
|
||||
truncated: false,
|
||||
next_offset: null,
|
||||
},
|
||||
});
|
||||
},
|
||||
});
|
||||
const result = await client.callTool({
|
||||
name: "analyze_function",
|
||||
arguments: { procedure: "0x1" },
|
||||
});
|
||||
expect(result.isError).toBe(true);
|
||||
const text = result.content.find((item) => item.type === "text");
|
||||
expect(text?.type === "text" ? text.text : "").toContain(
|
||||
"AnalysisOutputError",
|
||||
);
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -26,9 +26,69 @@ const enhancedFixtureResult = (method) => {
|
|||
has_more: false,
|
||||
};
|
||||
case "list_strings":
|
||||
return ["fixture"];
|
||||
return {
|
||||
items: [{ address: "0x1000", value: "fixture" }],
|
||||
offset: 0,
|
||||
limit: 500,
|
||||
total: 1,
|
||||
next_offset: null,
|
||||
has_more: false,
|
||||
};
|
||||
case "procedure_pseudo_code":
|
||||
return "return 0;";
|
||||
case "analyze_function": {
|
||||
const empty = () => ({
|
||||
items: [],
|
||||
total: 0,
|
||||
returned: 0,
|
||||
truncated: false,
|
||||
next_offset: null,
|
||||
});
|
||||
return {
|
||||
procedure: {
|
||||
address: "0x1000",
|
||||
name: "fixture",
|
||||
signature: null,
|
||||
locals: [],
|
||||
},
|
||||
pseudocode: {
|
||||
text: "return 0;",
|
||||
total_chars: 9,
|
||||
returned_chars: 9,
|
||||
truncated: false,
|
||||
next_offset: null,
|
||||
},
|
||||
assembly: { ...empty(), items: ["ret"], total: 1, returned: 1 },
|
||||
comments: empty(),
|
||||
callers: empty(),
|
||||
callees: empty(),
|
||||
incoming_references: empty(),
|
||||
outgoing_references: empty(),
|
||||
referenced_strings: empty(),
|
||||
referenced_names: empty(),
|
||||
basic_blocks: {
|
||||
...empty(),
|
||||
items: [{ start: "0x1000", end: "0x1001", successors: [] }],
|
||||
total: 1,
|
||||
returned: 1,
|
||||
},
|
||||
instruction_scan: { scanned: 1, truncated: false },
|
||||
};
|
||||
}
|
||||
case "search_strings":
|
||||
case "search_procedures":
|
||||
return {
|
||||
items: [
|
||||
{ address: "0x1000", value: "fixture", value_truncated: false },
|
||||
],
|
||||
offset: 0,
|
||||
limit: 100,
|
||||
total: 1,
|
||||
next_offset: null,
|
||||
has_more: false,
|
||||
};
|
||||
case "xrefs":
|
||||
return ["0x1000"];
|
||||
default:
|
||||
return undefined;
|
||||
}
|
||||
|
|
@ -79,7 +139,11 @@ const server = createServer((socket) => {
|
|||
} else if (request.method === "remote_error") {
|
||||
send({
|
||||
id: request.id,
|
||||
error: { code: -32001, message: "safe fake failure" },
|
||||
error: {
|
||||
code: -32001,
|
||||
message: "safe fake failure",
|
||||
type: "bridge_exception",
|
||||
},
|
||||
});
|
||||
} else if (request.method === "current_document") {
|
||||
send({ id: request.id, result: "fixture" });
|
||||
|
|
|
|||
|
|
@ -131,6 +131,17 @@ describe("HopperClient", () => {
|
|||
if (!result.ok) expect(result.error._tag).toBe(expectedTag);
|
||||
});
|
||||
|
||||
it("preserves a sanitized bridge exception diagnostic", async () => {
|
||||
const client = await startClient();
|
||||
const result = await client.callTool("remote_error");
|
||||
expect(result.ok).toBe(false);
|
||||
if (!result.ok)
|
||||
expect(result.error).toMatchObject({
|
||||
_tag: "HopperRemoteError",
|
||||
diagnosticType: "bridge_exception",
|
||||
});
|
||||
});
|
||||
|
||||
it("cancels and ignores late responses without corrupting the session", async () => {
|
||||
const client = await startClient();
|
||||
const controller = new AbortController();
|
||||
|
|
|
|||
|
|
@ -2,6 +2,7 @@ import { describe, expect, it } from "vitest";
|
|||
|
||||
import {
|
||||
parseDocuments,
|
||||
parseAddressedPage,
|
||||
parseListCount,
|
||||
parseNames,
|
||||
parseProcedures,
|
||||
|
|
@ -62,6 +63,29 @@ describe("Hopper boundary values", () => {
|
|||
});
|
||||
});
|
||||
|
||||
it.each([
|
||||
{
|
||||
items: [],
|
||||
offset: 0,
|
||||
limit: 100,
|
||||
total: 1,
|
||||
next_offset: null,
|
||||
has_more: true,
|
||||
},
|
||||
{
|
||||
items: [],
|
||||
offset: 0,
|
||||
limit: 100,
|
||||
total: 0,
|
||||
next_offset: 100,
|
||||
has_more: false,
|
||||
},
|
||||
])("rejects contradictory page continuation metadata", (page) => {
|
||||
const result = parseAddressedPage(page);
|
||||
expect(result.ok).toBe(false);
|
||||
if (!result.ok) expect(result.error._tag).toBe("HopperProtocolError");
|
||||
});
|
||||
|
||||
it.each([
|
||||
["procedures", () => parseProcedures(["not-a-map"])],
|
||||
["names", () => parseNames([{ address: 1, name: "bad" }])],
|
||||
|
|
|
|||
Loading…
Reference in New Issue