diff --git a/src/application/frida/FridaInstrumentationService.test.ts b/src/application/frida/FridaInstrumentationService.test.ts index 482b63c7cc..73d8220c97 100644 --- a/src/application/frida/FridaInstrumentationService.test.ts +++ b/src/application/frida/FridaInstrumentationService.test.ts @@ -368,11 +368,186 @@ it("retains an observed spawn resume after the target detaches before cancellati }); }); +it("returns typed partial Evidence when cancellation arrives during successful final cleanup", async () => { + const controller = new AbortController(); + const abortReason = new Error("cancelled during final detach"); + const script = { + ...createScriptObservation(), + messages: [{ type: "send", payload: "completed-before-detach" }], + }; + let closes = 0; + const service = new FridaInstrumentationService( + createProvider({ + async loadScript() { + return { ok: true, value: script }; + }, + async closeSession() { + closes += 1; + controller.abort(abortReason); + return { ok: true, value: { targetResumeObserved: true } }; + }, + }), + ); + + const result = await service.instrument( + { + mode: "spawn", + deviceId: "local", + program: "/fixture", + source: { + sourceKind: "inline", + source: "send('completed-before-detach')", + }, + durationMs: 0, + }, + controller.signal, + ); + + expect(result.ok).toBe(false); + if (result.ok) return; + expect(result.error).toBeInstanceOf(AnalysisCancelledError); + expect(result.error.cause).toBe(abortReason); + expect(result.error.partialObservation).toMatchObject({ + normalized_result: { + messages: script.messages, + source_sha256: script.sourceSha256, + cleanup_error: null, + target_state_before_cleanup: "paused", + target_resume_observed: true, + instrumentation_cleanup: "released", + target_liveness_after_cleanup: "unknown", + }, + }); + expect(closes).toBe(1); +}); + +it("keeps cancellation primary and records failed final cleanup without retrying it", async () => { + const controller = new AbortController(); + const abortReason = { source: "test", stage: "final-detach" }; + let closes = 0; + const service = new FridaInstrumentationService( + createProvider({ + async loadScript() { + return { ok: true, value: createScriptObservation() }; + }, + async closeSession() { + closes += 1; + controller.abort(abortReason); + return { + ok: false, + error: new AnalysisCapabilityUnavailableError( + "frida", + "close_frida_session", + "detach acknowledgement failed", + { + cleanup: { + reason: "session remains owned", + resources: [createSession().sessionId], + }, + }, + ), + }; + }, + }), + ); + + const result = await service.instrument( + { + mode: "spawn", + deviceId: "local", + program: "/fixture", + source: { sourceKind: "inline", source: "send('ready')" }, + durationMs: 0, + }, + controller.signal, + ); + + expect(result.ok).toBe(false); + if (result.ok) return; + expect(result.error).toBeInstanceOf(AnalysisCancelledError); + expect(result.error.cause).toBe(abortReason); + expect(result.error.cleanup).toEqual({ + reason: + "Provider frida cannot execute close_frida_session: detach acknowledgement failed", + resources: [createSession().sessionId], + }); + expect(result.error.partialObservation).toMatchObject({ + normalized_result: { + cleanup_error: + "Provider frida cannot execute close_frida_session: detach acknowledgement failed", + instrumentation_cleanup: "incomplete_or_unknown", + }, + }); + expect(closes).toBe(1); +}); + +it("retains a spawned resume reported after detach when remote cleanup fails", async () => { + const controller = new AbortController(); + const abortReason = new Error("cancelled during remote cleanup"); + let statusReads = 0; + let closes = 0; + const service = new FridaInstrumentationService( + createProvider({ + status() { + statusReads += 1; + return { + ...createSession(), + state: statusReads === 1 ? "paused" : "detached", + targetResumeObserved: statusReads > 1, + scripts: [], + messages: [], + messagesTruncated: false, + }; + }, + async closeSession() { + closes += 1; + controller.abort(abortReason); + return { + ok: false, + error: new AnalysisCapabilityUnavailableError( + "frida", + "close_frida_session", + "remote removal failed after detach", + ), + }; + }, + }), + ); + + const result = await service.instrument( + { + mode: "spawn", + deviceId: "local", + program: "/fixture", + source: { sourceKind: "inline", source: "send('ready')" }, + durationMs: 0, + }, + controller.signal, + ); + + expect(result.ok).toBe(false); + if (result.ok) return; + expect(result.error).toBeInstanceOf(AnalysisCancelledError); + expect(result.error.cause).toBe(abortReason); + expect(result.error.partialObservation).toMatchObject({ + normalized_result: { + target_state_before_cleanup: "paused", + target_resume_observed: true, + cleanup_error: + "Provider frida cannot execute close_frida_session: remote removal failed after detach", + instrumentation_cleanup: "incomplete_or_unknown", + }, + }); + expect(statusReads).toBe(2); + expect(closes).toBe(1); +}); + it.each([ - { cleanupState: "running" as const, resumeObserved: true }, + { cleanupState: "running" as const, resumeObserved: false }, { cleanupState: "paused" as const, resumeObserved: false }, + { cleanupState: "detached" as const, resumeObserved: true }, ])( - "uses a spawned target state transition after failed cleanup ($cleanupState)", + "uses explicit resume evidence after failed cleanup ($cleanupState)", async ({ cleanupState, resumeObserved }) => { const controller = new AbortController(); let statusCalls = 0; @@ -387,7 +562,7 @@ it.each([ return { ...createSession(), state: statusCalls === 1 ? "paused" : cleanupState, - targetResumeObserved: false, + targetResumeObserved: statusCalls > 1 && resumeObserved, scripts: [], messages: [], messagesTruncated: false, @@ -612,9 +787,14 @@ it("retains a cancelled provider load's source and messages in one-shot Evidence }); }); -it.each([false, true])( - "keeps an ordinary provider load failure primary when cancellation is already signaled before cleanup (cleanupFails=%s)", - async (cleanupFails) => { +it.each([ + ["before cleanup", false], + ["before cleanup", true], + ["during cleanup", false], + ["during cleanup", true], +] as const)( + "keeps an ordinary provider load failure primary when cancellation arrives %s (cleanupFails=%s)", + async (abortTiming, cleanupFails) => { const controller = new AbortController(); const abortReason = new Error("late caller cancellation"); const primary = new AnalysisCapabilityUnavailableError( @@ -627,11 +807,12 @@ it.each([false, true])( const service = new FridaInstrumentationService( createProvider({ async loadScript() { - controller.abort(abortReason); + if (abortTiming === "before cleanup") controller.abort(abortReason); return { ok: false, error: primary }; }, async closeSession() { closes += 1; + if (abortTiming === "during cleanup") controller.abort(abortReason); return cleanupFails ? { ok: false, diff --git a/src/application/frida/FridaInstrumentationService.ts b/src/application/frida/FridaInstrumentationService.ts index b373dc2499..a8dae5fc82 100644 --- a/src/application/frida/FridaInstrumentationService.ts +++ b/src/application/frida/FridaInstrumentationService.ts @@ -32,6 +32,19 @@ type DeviceListOutput = z.output< type ProcessListOutput = z.output< ReturnType>["outputSchema"] >; +type FridaSessionCloseResult = Awaited< + ReturnType +>; +type FridaSessionCancellation = { + readonly operation: string; + readonly input: StartFridaSessionInput; + readonly cause: unknown; + readonly diagnostics?: AnalysisError["diagnostics"]; + readonly observation: FridaSessionStatus; + readonly closed: FridaSessionCloseResult; + readonly statusAfterCleanup: FridaSessionStatus | undefined; + readonly script: FridaScriptObservation | undefined; +}; const unreadableScriptFile = async ( source: FridaScriptSource, @@ -170,41 +183,16 @@ export class FridaInstrumentationService { }; const closed = await this.#provider.closeSession(session.sessionId); const statusAfterCleanup = this.#provider.status(session.sessionId); - const partialObservation = fridaEvidence( - operation, - { - ...sessionStatusOutput(observation), - ...(loaded === undefined ? {} : sourceOutput(loaded)), - cleanup_error: closed.ok ? null : closed.error.message, - target_state_before_cleanup: observation.state, - target_resume_observed: - observation.targetResumeObserved || - (observation.mode === "spawn" && observation.state === "running") || - (closed.ok && closed.value.targetResumeObserved) || - (observation.mode === "spawn" && - observation.state !== "running" && - statusAfterCleanup?.state === "running"), - instrumentation_cleanup: closed.ok - ? "released" - : "incomplete_or_unknown", - target_liveness_after_cleanup: "unknown", - }, - fridaParameters(session.sessionId, input, session.deviceId), - loaded, - ); return err( - new AnalysisCancelledError(operation, { - ...(cause === undefined ? {} : { cause }), + cancelledFridaSession({ + operation, + input, + cause, diagnostics: load?.diagnostics, - partialObservation, - ...(closed.ok - ? {} - : { - cleanup: { - reason: closed.error.message, - resources: [session.sessionId], - }, - }), + observation, + closed, + statusAfterCleanup, + script: loaded, }), ); } @@ -377,6 +365,18 @@ export class FridaInstrumentationService { }; const closed = await this.#provider.closeSession(started.value.sessionId); const cleanupError = closed.ok ? null : closed.error.message; + if (signal?.aborted) + return err( + cancelledFridaSession({ + operation: "instrument_with_frida", + input, + cause: signal.reason, + observation, + closed, + statusAfterCleanup: this.#provider.status(started.value.sessionId), + script: loaded.value, + }), + ); const evidence = fridaEvidence( "instrument_with_frida", { @@ -442,6 +442,50 @@ const fridaEvidence = ( }, ); +const cancelledFridaSession = ({ + operation, + input, + cause, + diagnostics, + observation, + closed, + statusAfterCleanup, + script, +}: FridaSessionCancellation): AnalysisCancelledError => { + const cleanupError = closed.ok ? null : closed.error.message; + return new AnalysisCancelledError(operation, { + ...(cause === undefined ? {} : { cause }), + diagnostics, + partialObservation: fridaEvidence( + operation, + { + ...sessionStatusOutput(observation), + ...(script === undefined ? {} : sourceOutput(script)), + cleanup_error: cleanupError, + target_state_before_cleanup: observation.state, + target_resume_observed: + observation.targetResumeObserved || + (closed.ok && closed.value.targetResumeObserved) || + statusAfterCleanup?.targetResumeObserved === true, + instrumentation_cleanup: closed.ok + ? "released" + : "incomplete_or_unknown", + target_liveness_after_cleanup: "unknown", + }, + fridaParameters(observation.sessionId, input, observation.deviceId), + script, + ), + ...(closed.ok + ? {} + : { + cleanup: { + reason: closed.error.message, + resources: [observation.sessionId], + }, + }), + }); +}; + const fridaParameters = ( sessionId: string, input: StartFridaSessionInput, diff --git a/tests/boundary/mcp/fridaLifecycleMcp.test.ts b/tests/boundary/mcp/fridaLifecycleMcp.test.ts index 2c27ef9897..ec9d03a83a 100644 --- a/tests/boundary/mcp/fridaLifecycleMcp.test.ts +++ b/tests/boundary/mcp/fridaLifecycleMcp.test.ts @@ -12,7 +12,7 @@ import type { Script, Session, } from "frida"; -import { expect, it, onTestFinished } from "vitest"; +import { expect, it, onTestFinished, vi } from "vitest"; import { FridaScriptLoadCancelledError } from "../../../src/application/frida/FridaInstrumentationPort.js"; import type { @@ -35,7 +35,10 @@ import { silentLogger } from "../../../src/logger.js"; import { createServer } from "../../../src/server/createServer.js"; import { EvidenceMcpServer } from "../../../src/server/EvidenceMcpServer.js"; import { registerFridaTools } from "../../../src/server/registerFridaTools.js"; -import { createToolResultDelivery } from "../../../src/server/toolResult.js"; +import { + createToolResultDelivery, + ToolResultDelivery, +} from "../../../src/server/toolResult.js"; import { createTestBinarySession } from "../../fixtures/binarySession.js"; import { parseMcpToolError } from "../../fixtures/mcpToolError.js"; @@ -69,6 +72,7 @@ const nativeFixture = () => { create: async () => {}, load: async () => {}, resume: async () => {}, + detach: async () => {}, afterLoad: () => {}, discover: async () => {}, unloadFailures: 0, @@ -98,6 +102,7 @@ const nativeFixture = () => { isDetached: () => detached, async detach() { controls.detachCalls += 1; + await controls.detach(); if (controls.detachFailures > 0) { controls.detachFailures -= 1; throw new Error("transient detach failure"); @@ -327,6 +332,130 @@ it("retains detached state and resume evidence when remote cleanup needs retry", expect(fixture.controls.remoteRemovals).toBe(2); }); +it.each([ + { cancel: false, detachFails: false }, + { cancel: true, detachFails: false }, + { cancel: false, detachFails: true }, + { cancel: true, detachFails: true }, +])( + "classifies server completion after final detach (cancel=$cancel, detachFails=$detachFails)", + async ({ cancel, detachFails }) => { + const fixture = await connectedFixture(); + const held = gate(); + onTestFinished(held.release); + fixture.controls.detach = held.wait; + fixture.controls.detachFailures = detachFails ? 1 : 0; + let serverSignal: AbortSignal | undefined; + const load = fixture.manager.loadScript.bind(fixture.manager); + const loadSpy = vi + .spyOn(fixture.manager, "loadScript") + .mockImplementation((id, source, signal) => { + serverSignal = signal; + return load(id, source, signal); + }); + const success = vi.spyOn( + ToolResultDelivery.prototype, + "toEvidenceToolResult", + ); + const partial = vi.spyOn( + ToolResultDelivery.prototype, + "toRecordedPartialErrorToolResult", + ); + onTestFinished(() => { + loadSpy.mockRestore(); + success.mockRestore(); + partial.mockRestore(); + }); + const controller = new AbortController(); + const pending = fixture.client + .callTool( + { + name: "instrument_with_frida", + arguments: { + mode: "attach", + pid: 512, + device_id: "local", + source_kind: "inline", + source: "completed-before-detach", + duration_ms: 0, + }, + }, + { signal: controller.signal }, + ) + .then( + (value) => ({ kind: "result" as const, value }), + (cause: unknown) => ({ kind: "rejected" as const, cause }), + ); + await held.entered; + expect(fixture.unloaded).toEqual(["completed-before-detach"]); + if (cancel) { + controller.abort(new Error("cancel during final detach")); + await fixture.client.ping(); + } + expect(serverSignal?.aborted).toBe(cancel); + const serverReason: unknown = serverSignal?.reason; + held.release(); + const result = await pending; + await vi.waitFor(() => + expect(success.mock.calls.length + partial.mock.calls.length).toBe(1), + ); + expect(fixture.controls.detachCalls).toBe(1); + expect(fixture.controls.resumes).toBe(0); + const records = fixture.session + .exportEvidenceBundle() + .records.filter((record) => record.operation === "instrument_with_frida"); + expect(records).toHaveLength(1); + const evidence = records[0]; + expect(evidence).toMatchObject({ + normalized_result: { + messages: [{ type: "send", payload: "completed-before-detach" }], + cleanup_error: detachFails + ? expect.stringContaining("transient detach failure") + : null, + }, + }); + const sessionId = String(evidence?.parameters.session_id); + expect(fixture.manager.status(sessionId) !== undefined).toBe(detachFails); + if (cancel) { + expect(result.kind).toBe("rejected"); + expect(success).not.toHaveBeenCalled(); + const error = partial.mock.calls[0]?.[0]; + expect(error).toBeInstanceOf(AnalysisCancelledError); + expect(error?.cause).toBe(serverReason); + if (detachFails) + expect(error?.cleanup).toMatchObject({ resources: [sessionId] }); + else expect(error?.cleanup).toBeUndefined(); + expect(error?.partialObservation).toEqual(evidence); + expect(evidence).toMatchObject({ + normalized_result: { + instrumentation_cleanup: detachFails + ? "incomplete_or_unknown" + : "released", + target_state_before_cleanup: "running", + target_resume_observed: false, + target_liveness_after_cleanup: "unknown", + }, + }); + } else { + expect(result.kind).toBe("result"); + if (result.kind !== "result") throw result.cause; + expect(result.value.isError).not.toBe(true); + expect(partial).not.toHaveBeenCalled(); + expect(success).toHaveBeenCalledOnce(); + } + if (detachFails) { + const retried = await fixture.client.callTool({ + name: "close_frida_session", + arguments: { session_id: sessionId }, + }); + expect(retried.isError).not.toBe(true); + expect(fixture.controls.detachCalls).toBe(2); + expect(fixture.manager.status(sessionId)).toBeUndefined(); + expect(fixture.unloaded).toEqual(["completed-before-detach"]); + } + }, +); + it.each([ ["a trailing high surrogate", "x".repeat(32_765) + "\ud800"], ["repeated high surrogates", "\ud800".repeat(10_922)],