fix(mcp): centralize validated partial Evidence delivery

This commit is contained in:
morluto
2026-10-25 01:33:13 +08:00
parent 97194c4b48
commit 900f21b982
12 changed files with 186 additions and 181 deletions
+11 -6
View File
@@ -2,7 +2,8 @@ import type { CallToolResult } from "@modelcontextprotocol/server";
import { STDIO_DEFAULT_MAX_BUFFER_SIZE } from "@modelcontextprotocol/server";
import type { ToolContract } from "../contracts/toolContractTypes.js";
import type { Evidence } from "../domain/evidence.js";
import type { EvidenceWriter } from "../application/investigation/InvestigationRecordPort.js";
import { evidenceSchema, type Evidence } from "../domain/evidence.js";
import {
projectAnalysisError,
type AnalysisErrorProjection,
@@ -146,13 +147,17 @@ export class ToolResultDelivery {
);
}
/** Deliver the original failure with the producer-acknowledged partial Evidence reference. */
toRecordedPartialErrorToolResult(
/** Retain valid partial Evidence and reference it only after recording succeeds. */
toPartialErrorToolResult(
error: AnalysisError,
evidenceId: string,
recorded: Result<unknown, AnalysisError>,
targetContent: boolean,
recordEvidence?: EvidenceWriter["recordEvidence"],
): CallToolResult {
if (recordEvidence === undefined)
return this.toErrorToolResult(error, targetContent);
const partial = evidenceSchema.safeParse(error.partialObservation);
if (!partial.success) return this.toErrorToolResult(error, targetContent);
const recorded = recordEvidence(partial.data);
if (!recorded.ok) return this.toErrorToolResult(error, targetContent);
const projected = projectAnalysisError(error);
return this.projectedErrorResult(
@@ -162,7 +167,7 @@ export class ToolResultDelivery {
...projected.details,
partial_observation: {
kind: "retained-evidence",
evidence_id: evidenceId,
evidence_id: partial.data.evidence_id,
},
},
},
+6 -18
View File
@@ -31,24 +31,12 @@ export const deliverEvidenceResult = (
contract: ToolContract,
recordEvidence?: EvidenceWriter["recordEvidence"],
): CallToolResult => {
if (!result.ok) {
const partial = result.error.partialObservation;
if (
partial !== undefined &&
"evidence_id" in partial &&
typeof partial.evidence_id === "string"
) {
const recorded = recordEvidence?.(partial);
if (recorded !== undefined)
return server.delivery.toRecordedPartialErrorToolResult(
result.error,
partial.evidence_id,
recorded,
contract.returnsTargetContent,
);
}
return server.delivery.toCallToolResult(result, contract);
}
if (!result.ok)
return server.delivery.toPartialErrorToolResult(
result.error,
contract.returnsTargetContent,
recordEvidence,
);
const recorded = recordEvidence?.(result.value);
return server.delivery.toEvidenceToolResult(result.value, contract, recorded);
};
+5 -16
View File
@@ -4,7 +4,6 @@ import type { Logger } from "pino";
import type { CutterBridgeService } from "../cutter/CutterBridgeService.js";
import type { EvidenceWriter } from "../application/investigation/InvestigationRecordPort.js";
import { toolContract } from "../contracts/toolContracts.js";
import { evidenceSchema } from "../domain/evidence.js";
import {
runWithExternalMutationAdmission,
type ExternalMutationAdmission,
@@ -56,22 +55,12 @@ export const registerCutterTools = (
(value) => value,
() => service.retryCleanup(),
);
if (!result.ok) {
const partial = evidenceSchema.safeParse(
result.error.partialObservation,
if (!result.ok)
return delivery.toPartialErrorToolResult(
result.error,
command.returnsTargetContent,
recordEvidence,
);
if (partial.success) {
const recorded = recordEvidence?.(partial.data);
if (recorded !== undefined)
return delivery.toRecordedPartialErrorToolResult(
result.error,
partial.data.evidence_id,
recorded,
command.returnsTargetContent,
);
}
return delivery.toCallToolResult(result, command);
}
return delivery.toEvidenceToolResult(
result.value,
command,
+12 -38
View File
@@ -7,9 +7,6 @@ import { FridaInstrumentationService } from "../application/frida/FridaInstrumen
import type { FridaInstrumentationPort } from "../application/frida/FridaInstrumentationPort.js";
import type { FridaRemoteConnection } from "../application/frida/FridaInstrumentationPort.js";
import { toolContract } from "../contracts/toolContracts.js";
import type { ToolContract } from "../contracts/toolContractTypes.js";
import { evidenceSchema } from "../domain/evidence.js";
import type { AnalysisError } from "../domain/analysisErrorBase.js";
import { AnalysisInputError } from "../domain/analysisErrorCore.js";
import { jsonValueSchema } from "../domain/jsonValue.js";
import { logToolExecution } from "./toolLogging.js";
@@ -96,14 +93,12 @@ export const registerFridaTools = (
error: new AnalysisInputError(start.name),
}),
);
if (!result.ok) {
return deliverPartialEvidenceError(
result,
start,
delivery,
if (!result.ok)
return delivery.toPartialErrorToolResult(
result.error,
start.returnsTargetContent,
recordEvidence,
);
}
return delivery.toCallToolResult(
{
ok: true,
@@ -141,14 +136,12 @@ export const registerFridaTools = (
error: new AnalysisInputError(load.name),
}),
);
if (!result.ok) {
return deliverPartialEvidenceError(
result,
load,
delivery,
if (!result.ok)
return delivery.toPartialErrorToolResult(
result.error,
load.returnsTargetContent,
recordEvidence,
);
}
return delivery.toEvidenceToolResult(
result.value,
load,
@@ -274,14 +267,12 @@ export const registerFridaTools = (
instrument.name,
() => operation,
);
if (!result.ok) {
return deliverPartialEvidenceError(
result,
instrument,
delivery,
if (!result.ok)
return delivery.toPartialErrorToolResult(
result.error,
instrument.returnsTargetContent,
recordEvidence,
);
}
return delivery.toEvidenceToolResult(
result.value.evidence,
instrument,
@@ -291,23 +282,6 @@ export const registerFridaTools = (
);
};
const deliverPartialEvidenceError = (
result: { readonly ok: false; readonly error: AnalysisError },
contract: ToolContract,
delivery: ToolResultDelivery,
recordEvidence: EvidenceWriter["recordEvidence"] | undefined,
): ReturnType<ToolResultDelivery["toCallToolResult"]> => {
const parsed = evidenceSchema.safeParse(result.error.partialObservation);
if (!parsed.success || recordEvidence === undefined)
return delivery.toCallToolResult(result, contract);
return delivery.toRecordedPartialErrorToolResult(
result.error,
parsed.data.evidence_id,
recordEvidence(parsed.data),
contract.returnsTargetContent,
);
};
const remoteConnection = (
input:
| {
+11 -29
View File
@@ -9,7 +9,6 @@ import {
summarizeRetainedAnalysis,
} from "../application/analysisView/AnalysisViewService.js";
import { toolContract } from "../contracts/toolContracts.js";
import { evidenceSchema } from "../domain/evidence.js";
import { err, ok } from "../domain/result.js";
import { AnalysisInputError } from "../domain/analysisErrorCore.js";
import {
@@ -57,19 +56,11 @@ export const registerGdbTools = (
{ ok: true, value: result.value },
start,
);
const partial = result.error.partialObservation;
const parsed = evidenceSchema.safeParse(partial);
if (parsed.success) {
const recorded = recordEvidence?.(parsed.data);
if (recorded !== undefined)
return delivery.toRecordedPartialErrorToolResult(
result.error,
parsed.data.evidence_id,
recorded,
start.returnsTargetContent,
);
}
return delivery.toCallToolResult(result, start);
return delivery.toPartialErrorToolResult(
result.error,
start.returnsTargetContent,
recordEvidence,
);
},
);
server.registerTool(
@@ -105,21 +96,12 @@ export const registerGdbTools = (
: input.detail === "summary"
? summarizeRetainedAnalysis(executed.value.evidence, retention)
: ok(executed.value.evidence);
if (!result.ok) {
const partial = result.error.partialObservation;
const parsed = evidenceSchema.safeParse(partial);
if (parsed.success) {
const recorded = recordEvidence?.(parsed.data);
if (recorded !== undefined)
return delivery.toRecordedPartialErrorToolResult(
result.error,
parsed.data.evidence_id,
recorded,
consoleContract.returnsTargetContent,
);
}
return delivery.toCallToolResult(result, consoleContract);
}
if (!result.ok)
return delivery.toPartialErrorToolResult(
result.error,
consoleContract.returnsTargetContent,
recordEvidence,
);
const recorded = recordEvidence?.(result.value);
return delivery.toEvidenceToolResult(
result.value,
+16 -35
View File
@@ -12,7 +12,6 @@ import {
} from "../application/reverse/ReverseEngineeringService.js";
import type { EvidenceLookup } from "../application/EvidenceInputResolver.js";
import type { EvidenceWriter } from "../application/investigation/InvestigationRecordPort.js";
import type { Evidence } from "../domain/evidence.js";
import { toolContract } from "../contracts/toolContracts.js";
import type { ToolContract } from "../contracts/toolContractTypes.js";
import { logToolExecution } from "./toolLogging.js";
@@ -65,10 +64,9 @@ export const registerReverseEngineeringTools = (
),
);
if (!result.ok)
return deliverFailure(
return delivery.toPartialErrorToolResult(
result.error,
objdump,
delivery,
objdump.returnsTargetContent,
recordEvidence,
);
return delivery.toEvidenceToolResult(
@@ -87,7 +85,11 @@ export const registerReverseEngineeringTools = (
if (parsed.ok && withAdmittedExternalMutation !== undefined) {
const cleanup = await service.retryCleanup(rizin.name);
if (!cleanup.ok)
return deliverFailure(cleanup.error, rizin, delivery, recordEvidence);
return delivery.toPartialErrorToolResult(
cleanup.error,
rizin.returnsTargetContent,
recordEvidence,
);
const admitted = await withAdmittedExternalMutation(
parsed.value.path,
context.mcpReq.signal,
@@ -124,15 +126,18 @@ export const registerReverseEngineeringTools = (
},
);
if (!admitted.ok)
return deliverFailure(
return delivery.toPartialErrorToolResult(
admitted.error,
rizin,
delivery,
rizin.returnsTargetContent,
recordEvidence,
);
const result = admitted.value;
if (!result.ok)
return deliverFailure(result.error, rizin, delivery, recordEvidence);
return delivery.toPartialErrorToolResult(
result.error,
rizin.returnsTargetContent,
recordEvidence,
);
return delivery.toEvidenceToolResult(
result.value,
rizin,
@@ -155,10 +160,9 @@ export const registerReverseEngineeringTools = (
: Promise.resolve(parsed),
);
if (!result.ok)
return deliverFailure(
return delivery.toPartialErrorToolResult(
result.error,
rizin,
delivery,
rizin.returnsTargetContent,
recordEvidence,
);
return delivery.toEvidenceToolResult(
@@ -189,26 +193,3 @@ const runAdmitted = async (
? admitted.value
: delivery.toErrorToolResult(admitted.error, contract.returnsTargetContent);
};
const deliverFailure = (
error: import("../domain/analysisErrorBase.js").AnalysisError,
contract: ToolContract,
delivery: ToolResultDelivery,
recordEvidence?: EvidenceWriter["recordEvidence"],
) => {
const partial = error.partialObservation;
if (isEvidence(partial) && recordEvidence !== undefined)
return delivery.toRecordedPartialErrorToolResult(
error,
partial.evidence_id,
recordEvidence(partial),
contract.returnsTargetContent,
);
return delivery.toErrorToolResult(error, contract.returnsTargetContent);
};
const isEvidence = (value: unknown): value is Evidence =>
typeof value === "object" &&
value !== null &&
"evidence_id" in value &&
typeof value.evidence_id === "string";
+5 -16
View File
@@ -5,7 +5,6 @@ import type { EvidenceWriter } from "../application/investigation/InvestigationR
import { toolContract } from "../contracts/toolContracts.js";
import { AnalysisInputError } from "../domain/analysisErrorCore.js";
import type { RizinDebugSessionManager } from "../rizin/RizinDebugSessionManager.js";
import { evidenceSchema } from "../domain/evidence.js";
import {
runOwnedCleanupWithExternalMutationAdmission,
runWithExternalMutationAdmission,
@@ -82,22 +81,12 @@ export const registerRizinDebugTools = (
(value) => value.evidence,
() => manager.retryCleanupFailures(),
);
if (!result.ok) {
const partial = evidenceSchema.safeParse(
result.error.partialObservation,
if (!result.ok)
return delivery.toPartialErrorToolResult(
result.error,
command.returnsTargetContent,
recordEvidence,
);
if (partial.success) {
const recorded = recordEvidence?.(partial.data);
if (recorded !== undefined)
return delivery.toRecordedPartialErrorToolResult(
result.error,
partial.data.evidence_id,
recorded,
command.returnsTargetContent,
);
}
return delivery.toCallToolResult(result, command);
}
return delivery.toEvidenceToolResult(
result.value.evidence,
command,
@@ -73,12 +73,12 @@ it("retains completed Android Evidence through the MCP cancellation delivery pat
expect(controlReply.isError).not.toBe(true);
const control = parseEvidence(controlReply.structuredContent);
const delivered = createDeferred<CallToolResult>();
const deliver = server.delivery.toRecordedPartialErrorToolResult.bind(
const deliver = server.delivery.toPartialErrorToolResult.bind(
server.delivery,
);
vi.spyOn(
ToolResultDelivery.prototype,
"toRecordedPartialErrorToolResult",
"toPartialErrorToolResult",
).mockImplementation((...args) => {
const result = deliver(...args);
delivered.resolve(result);
@@ -1,4 +1,4 @@
import { describe, expect, it } from "vitest";
import { describe, expect, it, vi } from "vitest";
import type {
CallToolResult,
@@ -10,11 +10,14 @@ import { toolContract } from "../../../src/contracts/toolContracts.js";
import { AnalysisCancelledError } from "../../../src/domain/analysisErrorCore.js";
import { createEvidence } from "../../../src/domain/evidence.js";
import type { Evidence } from "../../../src/domain/evidence.js";
import { projectAnalysisError } from "../../../src/domain/analysisErrorProjection.js";
import type { FridaDeviceList } from "../../../src/domain/fridaDiscovery.js";
import { err, ok } from "../../../src/domain/result.js";
import { silentLogger } from "../../../src/logger.js";
import { createEvidenceToolHandler } from "../../../src/server/contractToolHandler.js";
import { EvidenceMcpServer } from "../../../src/server/EvidenceMcpServer.js";
import { ToolResultDelivery } from "../../../src/server/toolResult.js";
import { parseMcpToolError } from "../../fixtures/mcpToolError.js";
const UNTRUSTED_TARGET_CONTENT = {
provenance: { untrusted_content: true, source: "analysis_target" },
@@ -68,6 +71,48 @@ const deliverPartialFailure = async (): Promise<{
};
describe("shared Evidence handler target-content framing", () => {
it("keeps a non-Evidence partial with a caller-selected evidence_id inline without recording it", async () => {
const partial = {
devices: [],
cleanup_error: null,
evidence_id: "caller-selected",
};
const observation: FridaDeviceList = partial;
const reason = { request: "cancel capture" };
const failure = new AnalysisCancelledError("inspect_packet_capture", {
cause: reason,
partialObservation: observation,
});
const recordEvidence = vi.fn(() => ok("added" as const));
const server = new EvidenceMcpServer(
{ name: "inline-partial-error-test", version: "1" },
{ capabilities: {} },
recordEvidence,
new ToolResultDelivery(STDIO_DEFAULT_MAX_BUFFER_SIZE),
);
const contract = toolContract("inspect_packet_capture");
const handler = createEvidenceToolHandler(
server,
async () => err(failure),
silentLogger,
recordEvidence,
);
const result = await handler(contract)({ path: "recording.pcap" }, context);
const projected = parseMcpToolError(result);
expect(projected.error).toMatchObject({
code: "cancelled",
message: projectAnalysisError(failure).message,
details: { operation: "inspect_packet_capture" },
});
expect(projected.error.details?.partial_observation).toEqual(partial);
expect(result.structuredContent).toEqual(projected);
expect(recordEvidence).not.toHaveBeenCalled();
expect(failure.cause).toBe(reason);
expect(failure.partialObservation).toBe(partial);
expect(result._meta).toEqual(UNTRUSTED_TARGET_CONTENT);
});
it("marks a retained partial-observation error as untrusted target content", async () => {
const { result, evidence } = await deliverPartialFailure();
expect(result.isError).toBe(true);
+5 -8
View File
@@ -384,7 +384,7 @@ it.each([
);
const partial = vi.spyOn(
ToolResultDelivery.prototype,
"toRecordedPartialErrorToolResult",
"toPartialErrorToolResult",
);
onTestFinished(() => {
loadSpy.mockRestore();
@@ -526,7 +526,7 @@ it("preserves the native resume failure after MCP cancellation reaches the serve
fixture.controls.resume = async () => {};
throw new Error("native resume rejected");
};
const delivery = vi.spyOn(ToolResultDelivery.prototype, "toCallToolResult");
const delivery = vi.spyOn(ToolResultDelivery.prototype, "toErrorToolResult");
onTestFinished(() => delivery.mockRestore());
let serverSignal: AbortSignal | undefined;
const resume = fixture.manager.resumeSession.bind(fixture.manager);
@@ -562,12 +562,9 @@ it("preserves the native resume failure after MCP cancellation reaches the serve
await pending;
await vi.waitFor(() => expect(delivery).toHaveBeenCalledOnce());
expect(delivery.mock.calls[0]?.[0]).toMatchObject({
ok: false,
error: {
_tag: "AnalysisCapabilityUnavailableError",
operation: "resume_frida_session",
message: expect.stringContaining("native resume rejected"),
},
_tag: "AnalysisCapabilityUnavailableError",
operation: "resume_frida_session",
message: expect.stringContaining("native resume rejected"),
});
expect(fixture.controls.detachCalls).toBe(1);
expect(fixture.unloaded).toEqual(["resume-failure"]);
@@ -7,7 +7,12 @@ import { dirname, join } from "node:path";
import { expect, it, vi } from "vitest";
import { TestReverseEngineeringService as ReverseEngineeringService } from "../../support/reverseEngineeringService.js";
import { AnalysisInputError } from "../../../src/domain/analysisErrorCore.js";
import {
AnalysisCancelledError,
AnalysisInputError,
} from "../../../src/domain/analysisErrorCore.js";
import { projectAnalysisError } from "../../../src/domain/analysisErrorProjection.js";
import type { FridaDeviceList } from "../../../src/domain/fridaDiscovery.js";
import {
createBinarySessionTargets,
createCacheProvider,
@@ -37,6 +42,50 @@ const createTimedOutRizinService = (): ReverseEngineeringService =>
},
});
it("keeps a non-Evidence partial with a caller-selected evidence_id inline in a reverse tool error", async () => {
const { server, handler } = createReverseEngineeringMcpHarness();
const partial = {
devices: [],
cleanup_error: null,
evidence_id: "caller-selected",
};
const observation: FridaDeviceList = partial;
const reason = { request: "cancel inspection" };
const failure = new AnalysisCancelledError("inspect_with_objdump", {
cause: reason,
partialObservation: observation,
});
const service = new ReverseEngineeringService();
vi.spyOn(service, "inspectWithObjdump").mockResolvedValue(err(failure));
const recordEvidence = vi.fn(() => ok("added" as const));
registerReverseEngineeringTools(
server,
service,
pino({ enabled: false }),
createToolResultDelivery(2 * 1024 * 1024),
recordEvidence,
);
const result = (await invokeWithLiveSignal(handler("inspect_with_objdump"), {
path: "package.json",
operation: "file_headers",
})) as CallToolResult;
const projected = parseMcpToolError(result);
expect(projected.error).toMatchObject({
code: "cancelled",
message: projectAnalysisError(failure).message,
details: { operation: "inspect_with_objdump" },
});
expect(projected.error.details?.partial_observation).toEqual(partial);
expect(result.structuredContent).toEqual(projected);
expect(recordEvidence).not.toHaveBeenCalled();
expect(failure.cause).toBe(reason);
expect(failure.partialObservation).toBe(partial);
expect(result._meta).toEqual({
provenance: { untrusted_content: true, source: "analysis_target" },
});
});
it("records partial Evidence and references it in the original MCP error", async () => {
const { server, handler } = createReverseEngineeringMcpHarness();
const service = createTimedOutRizinService();
+17 -11
View File
@@ -14,6 +14,7 @@ import {
import { toolContract } from "../../../src/contracts/toolContracts.js";
import type { ToolContract } from "../../../src/contracts/toolContractTypes.js";
import { err } from "../../../src/domain/result.js";
import { EvidenceIntegrityError } from "../../../src/domain/evidenceErrors.js";
import { HopperProcessError } from "../../../src/domain/hopperErrors.js";
import { processScenarioSchema } from "../../../src/domain/process/processScenario.js";
import { ToolResultDelivery } from "../../../src/server/toolResult.js";
@@ -482,30 +483,35 @@ describe("untrusted provenance on failed tool results", () => {
});
it("marks a recorded partial-observation error and its retention fallback", () => {
const recorded = delivery.toRecordedPartialErrorToolResult(
failure(),
"evidence-1",
{ ok: true, value: "added" },
true,
const evidence = createEvidence(
undefined,
{ id: "fixture", name: "Partial observation fixture", version: "1" },
{ operation: "fixture", parameters: {}, result: { observed: true } },
);
const original = failure().retainPartialObservation(evidence);
const recorded = delivery.toPartialErrorToolResult(original, true, () => ({
ok: true,
value: "added",
}));
expect(recorded.structuredContent).toMatchObject({
error: {
details: {
partial_observation: {
kind: "retained-evidence",
evidence_id: "evidence-1",
evidence_id: evidence.evidence_id,
},
},
},
});
expect(recorded._meta).toEqual(UNTRUSTED_TARGET_CONTENT);
const unrecorded = delivery.toRecordedPartialErrorToolResult(
failure(),
"evidence-1",
err(failure()),
true,
const unrecorded = delivery.toPartialErrorToolResult(original, true, () =>
err(new EvidenceIntegrityError("fixture rejects partial")),
);
expect(unrecorded.structuredContent).toMatchObject({
error: { details: { partial_observation: evidence } },
});
expect(unrecorded._meta).toEqual(UNTRUSTED_TARGET_CONTENT);
expect(original.partialObservation).toBe(evidence);
});
it("marks the response-budget error that replaces an oversized target-content result", () => {