From 4cbcead46cb16b18c9467498e5b4a6778e4b7aa0 Mon Sep 17 00:00:00 2001 From: morluto <76467478+morluto@users.noreply.github.com> Date: Mon, 19 Oct 2026 17:35:10 +0800 Subject: [PATCH] fix(frida): share canonical discovery results across adapters Own device and process discovery projection in the application workflow, bound to each named output contract. CLI and MCP now consume that result directly, including retained observations when final CLI cleanup fails. Remove the duplicate adapter projections and preserve provider failures, caller-selected identifiers, process records and cleanup diagnostics. Validated with 103 focused tests, npm run check and docs:check. Real local Frida 17.23.2 discovery on macOS arm64 returned the same three devices through the compiled CLI and production stdio MCP; both now pass the advertised schema. Remote endpoints and native Linux/Windows unverified. Fixes #4637 Co-authored-by: shreyansh1729 Co-authored-by: Kaushalendra-Marcus --- .../frida/FridaInstrumentationService.ts | 38 +++++- src/cli/fridaCommands.ts | 15 +-- src/server/registerFridaTools.ts | 31 +---- tests/boundary/cli/fridaCli.test.ts | 118 +++++++++++++++++- tests/boundary/mcp/fridaMcp.test.ts | 43 ++++++- 5 files changed, 192 insertions(+), 53 deletions(-) diff --git a/src/application/frida/FridaInstrumentationService.ts b/src/application/frida/FridaInstrumentationService.ts index d228989d99..b373dc2499 100644 --- a/src/application/frida/FridaInstrumentationService.ts +++ b/src/application/frida/FridaInstrumentationService.ts @@ -1,7 +1,9 @@ import { constants } from "node:fs"; import { access, stat } from "node:fs/promises"; import { setTimeout as delay } from "node:timers/promises"; +import type { z } from "zod"; +import type { toolContract } from "../../contracts/toolContracts.js"; import { createEvidence, type Evidence } from "../../domain/evidence.js"; import { jsonObjectSchema, jsonValueSchema } from "../../domain/jsonValue.js"; import type { AnalysisError } from "../../domain/analysisErrorBase.js"; @@ -24,6 +26,13 @@ import type { StartFridaSessionInput, } from "./FridaInstrumentationPort.js"; +type DeviceListOutput = z.output< + ReturnType>["outputSchema"] +>; +type ProcessListOutput = z.output< + ReturnType>["outputSchema"] +>; + const unreadableScriptFile = async ( source: FridaScriptSource, ): Promise => { @@ -90,12 +99,33 @@ export class FridaInstrumentationService { this.#provider = provider; } - listDevices(remote?: FridaRemoteConnection) { - return this.#provider.listDevices(remote); + async listDevices( + remote?: FridaRemoteConnection, + ): Promise> { + const result = await this.#provider.listDevices(remote); + return result.ok + ? ok({ + devices: result.value.devices.map(({ deviceId, name, type }) => ({ + device_id: deviceId, + name, + type, + })), + cleanup_error: result.value.cleanupError, + }) + : result; } - listProcesses(selector: FridaDeviceSelector) { - return this.#provider.listProcesses(selector); + async listProcesses( + selector: FridaDeviceSelector, + ): Promise> { + const result = await this.#provider.listProcesses(selector); + return result.ok + ? ok({ + device_id: result.value.deviceId, + processes: [...result.value.processes], + cleanup_error: result.value.cleanupError, + }) + : result; } async startSession(input: StartFridaSessionInput, signal?: AbortSignal) { diff --git a/src/cli/fridaCommands.ts b/src/cli/fridaCommands.ts index 51f8b1b2c1..aa0157fa4d 100644 --- a/src/cli/fridaCommands.ts +++ b/src/cli/fridaCommands.ts @@ -120,12 +120,7 @@ export const registerFridaCommands = ( const result = await withProviderCleanup(() => service.listDevices(remoteFrom(options)), ); - return result.ok - ? { - devices: result.value.devices, - cleanup_error: result.value.cleanupError, - } - : projectAnalysisError(result.error); + return result.ok ? result.value : projectAnalysisError(result.error); }), }); cli.command(CLI_COMMANDS.listFridaProcesses, { @@ -158,13 +153,7 @@ export const registerFridaCommands = ( : { remote }, ), ); - return result.ok - ? { - device_id: result.value.deviceId, - processes: result.value.processes, - cleanup_error: result.value.cleanupError, - } - : projectAnalysisError(result.error); + return result.ok ? result.value : projectAnalysisError(result.error); }), }); cli.command(CLI_COMMANDS.instrumentWithFrida, { diff --git a/src/server/registerFridaTools.ts b/src/server/registerFridaTools.ts index a56ec95785..016f4afb94 100644 --- a/src/server/registerFridaTools.ts +++ b/src/server/registerFridaTools.ts @@ -42,22 +42,7 @@ export const registerFridaTools = ( const result = await logToolExecution(logger, listDevices.name, () => service.listDevices(remoteConnection(input.remote)), ); - return result.ok - ? delivery.toCallToolResult( - { - ok: true, - value: jsonValueSchema.parse({ - devices: result.value.devices.map((device) => ({ - device_id: device.deviceId, - name: device.name, - type: device.type, - })), - cleanup_error: result.value.cleanupError, - }), - }, - listDevices, - ) - : delivery.toCallToolResult(result, listDevices); + return delivery.toCallToolResult(result, listDevices); }, ); server.registerTool( @@ -73,19 +58,7 @@ export const registerFridaTools = ( ...(remote === undefined ? {} : { remote }), }), ); - return result.ok - ? delivery.toCallToolResult( - { - ok: true, - value: jsonValueSchema.parse({ - device_id: result.value.deviceId, - processes: result.value.processes, - cleanup_error: result.value.cleanupError, - }), - }, - listProcesses, - ) - : delivery.toCallToolResult(result, listProcesses); + return delivery.toCallToolResult(result, listProcesses); }, ); server.registerTool( diff --git a/tests/boundary/cli/fridaCli.test.ts b/tests/boundary/cli/fridaCli.test.ts index 52770b81ba..0fd08f5c01 100644 --- a/tests/boundary/cli/fridaCli.test.ts +++ b/tests/boundary/cli/fridaCli.test.ts @@ -6,10 +6,113 @@ import { expect, it } from "vitest"; import type { DeviceManager } from "frida"; import { createCli } from "../../../src/cli.js"; +import { toolContract } from "../../../src/contracts/toolContracts.js"; import type { FridaInstrumentationPort } from "../../../src/application/frida/FridaInstrumentationPort.js"; import { AnalysisCapabilityUnavailableError } from "../../../src/domain/analysisErrorCore.js"; import { FridaInstrumentationManager } from "../../../src/frida/FridaInstrumentationManager.js"; +it.each([ + { devices: [], cleanupError: null }, + { + devices: [ + { deviceId: "local", name: "Local System", type: "local" }, + { deviceId: "socket", name: "Local Socket", type: "remote" }, + ], + cleanupError: null, + }, + { + devices: [{ deviceId: "remote-device", name: "endpoint", type: "remote" }], + cleanupError: "remote removal failed", + }, +])("returns canonical Frida device JSON for %j", async (observation) => { + const provider: FridaInstrumentationPort = new FridaInstrumentationManager(); + provider.listDevices = async () => ({ ok: true, value: observation }); + let closed = false; + provider.closeAll = async () => { + closed = true; + }; + let stdout = ""; + let exitCode = 0; + await createCli({}, undefined, provider).serve( + ["list-frida-devices", "--json"], + { + env: {}, + exit: (code) => { + exitCode = code; + }, + stdout: (text) => { + stdout += text; + }, + }, + ); + expect(exitCode).toBe(0); + expect(closed).toBe(true); + const output = toolContract("list_frida_devices").outputSchema.parse( + JSON.parse(stdout), + ); + expect(output).toEqual({ + devices: observation.devices.map(({ deviceId, name, type }) => ({ + device_id: deviceId, + name, + type, + })), + cleanup_error: observation.cleanupError, + }); +}); + +it.each([false, true])( + "preserves canonical process discovery when CLI cleanup fails: %s", + async (cleanupFails) => { + const provider: FridaInstrumentationPort = + new FridaInstrumentationManager(); + const processes = [ + { pid: 0, name: "idle", identifier: null }, + { pid: 71, name: "target", identifier: "example.target" }, + ]; + provider.listProcesses = async (selector) => { + expect(selector).toEqual({ deviceId: "chosen-device" }); + return { + ok: true, + value: { + deviceId: "chosen-device", + processes, + cleanupError: "earlier endpoint cleanup diagnostic", + }, + }; + }; + provider.closeAll = async () => { + if (cleanupFails) throw new Error("final cleanup failed"); + }; + let stdout = ""; + await createCli({}, undefined, provider).serve( + ["list-frida-processes", "--device-id", "chosen-device", "--json"], + { + env: {}, + exit: () => {}, + stdout: (text) => { + stdout += text; + }, + }, + ); + const expected = { + device_id: "chosen-device", + processes, + cleanup_error: "earlier endpoint cleanup diagnostic", + }; + if (cleanupFails) + expect(JSON.parse(stdout)).toMatchObject({ + code: "cleanup_incomplete", + details: { diagnostics: { partial_observation: expected } }, + }); + else + expect( + toolContract("list_frida_processes").outputSchema.parse( + JSON.parse(stdout), + ), + ).toEqual(expected); + }, +); + it("maps one-shot Frida CLI target, script, duration, and remote options", async () => { const calls: unknown[] = []; const provider: FridaInstrumentationPort = { @@ -365,7 +468,20 @@ it("preserves observed devices when CLI provider cleanup fails", async () => { const output: unknown = JSON.parse(stdout); expect(output).toMatchObject({ code: "cleanup_incomplete", - details: { diagnostics: { partial_observation: { devices } } }, + details: { + diagnostics: { + partial_observation: { + devices: [ + { + device_id: "remote-device", + name: "selected endpoint", + type: "remote", + }, + ], + cleanup_error: null, + }, + }, + }, }); }); diff --git a/tests/boundary/mcp/fridaMcp.test.ts b/tests/boundary/mcp/fridaMcp.test.ts index 3642a554c6..a665385809 100644 --- a/tests/boundary/mcp/fridaMcp.test.ts +++ b/tests/boundary/mcp/fridaMcp.test.ts @@ -27,7 +27,14 @@ const createFridaTestProvider = ( async listProcesses() { return { ok: true, - value: { deviceId: "remote-device", processes: [], cleanupError: null }, + value: { + deviceId: "remote-device", + processes: [ + { pid: 0, name: "idle", identifier: null }, + { pid: 10, name: "target", identifier: "example.target" }, + ], + cleanupError: "earlier endpoint cleanup diagnostic", + }, }; }, async startSession(input) { @@ -112,6 +119,26 @@ const expectFridaToolsAvailable = async (client: Client) => { ); }; +const expectFridaProcessDiscovery = async (client: Client) => { + const processes = await client.callTool({ + name: "list_frida_processes", + arguments: { device_id: "remote-device" }, + }); + expect(processes.isError).not.toBe(true); + expect( + toolContract("list_frida_processes").outputSchema.parse( + processes.structuredContent, + ), + ).toEqual({ + device_id: "remote-device", + processes: [ + { pid: 0, name: "idle", identifier: null }, + { pid: 10, name: "target", identifier: "example.target" }, + ], + cleanup_error: "earlier endpoint cleanup diagnostic", + }); +}; + it("advertises Frida MCP schemas and keeps remote credentials out of results", async () => { const session = createTestBinarySession(() => { throw new Error("Binary provider must not start"); @@ -156,13 +183,17 @@ it("advertises Frida MCP schemas and keeps remote credentials out of results", a "synthetic-secret-token", ); expect(ajv.validate(outputSchema, response.structuredContent)).toBe(true); + expect( + toolContract("list_frida_devices").outputSchema.parse( + response.structuredContent, + ), + ).toEqual({ + devices: [{ device_id: "remote-device", name: "target", type: "remote" }], + cleanup_error: null, + }); expect(toolContract("start_frida_session").effects.mutatesTarget).toBe(true); - const processes = await client.callTool({ - name: "list_frida_processes", - arguments: { device_id: "remote-device" }, - }); - expect(processes.isError).not.toBe(true); + await expectFridaProcessDiscovery(client); const started = await client.callTool({ name: "start_frida_session",