From 5fa7a52d1f4b28c0b842c5737b84fef6ef2a8eb4 Mon Sep 17 00:00:00 2001 From: morluto <76467478+morluto@users.noreply.github.com> Date: Wed, 14 Oct 2026 09:47:28 +0800 Subject: [PATCH] fix(frida): preserve script diagnostics and isolate capture truncation --- .../frida/FridaInstrumentationManager.test.ts | 163 +++++++++++++++++- src/frida/FridaInstrumentationManager.ts | 93 ++++++++-- src/frida/frida-bindings.d.ts | 3 + 3 files changed, 238 insertions(+), 21 deletions(-) diff --git a/src/application/frida/FridaInstrumentationManager.test.ts b/src/application/frida/FridaInstrumentationManager.test.ts index d66280f66..11f9ca81e 100644 --- a/src/application/frida/FridaInstrumentationManager.test.ts +++ b/src/application/frida/FridaInstrumentationManager.test.ts @@ -9,6 +9,7 @@ import type { Session, Signal, } from "frida"; +import { AnalysisInputError } from "../../domain/analysisErrorCore.js"; import { FridaInstrumentationManager } from "../../frida/FridaInstrumentationManager.js"; const signal = void>() => { @@ -40,13 +41,14 @@ class FakeScript implements Script { readonly beforeLoad: () => Promise = async () => {}, readonly description = "script loaded", readonly stack = "", + readonly messageValue?: FridaMessage, ) {} async load(): Promise { await this.beforeLoad(); if (this.loadFailure !== undefined) throw this.loadFailure; this.message.emit( - { + this.messageValue ?? { type: "send", payload: this.payload, description: this.description, @@ -73,6 +75,7 @@ class FakeSession implements Session { nextPayload: unknown = { event: "loaded" }; nextDescription = "script loaded"; nextStack = ""; + nextMessage: FridaMessage | undefined; nextLoadFailure: Error | undefined; nextUnloadFailureCount = 0; detachFailureCount = 0; @@ -94,12 +97,14 @@ class FakeSession implements Session { }, this.nextDescription, this.nextStack, + this.nextMessage, ); script.loadFailure = this.nextLoadFailure; script.unloadFailureCount = this.nextUnloadFailureCount; this.nextPayload = { event: "loaded" }; this.nextDescription = "script loaded"; this.nextStack = ""; + this.nextMessage = undefined; this.nextLoadFailure = undefined; this.nextUnloadFailureCount = 0; this.scripts.push(script); @@ -744,7 +749,93 @@ it("attributes messages to their producing script and returns each load's messag await manager.closeAll(); }); -it("keeps an active script message bucket after its last message is evicted", async () => { +it("retains Frida error source coordinates in script and session evidence", async () => { + const { bindings, manager, started } = await startLocalAttachedSession(); + expect(started.ok).toBe(true); + if (!started.ok) return; + + bindings.device.session.nextMessage = { + type: "error", + description: "instrumentation failed", + stack: "Error: instrumentation failed", + fileName: "agent.js", + lineNumber: 7, + columnNumber: 3, + }; + const loaded = await manager.loadScript(started.value.sessionId, { + sourceKind: "inline", + source: "throw new Error('instrumentation failed');", + }); + expect(loaded.ok).toBe(true); + if (loaded.ok) { + expect(loaded.value.messages[0]).toMatchObject({ + type: "error", + file_name: "agent.js", + line_number: 7, + column_number: 3, + }); + } + expect(manager.status(started.value.sessionId)?.messages[0]).toMatchObject({ + file_name: "agent.js", + line_number: 7, + column_number: 3, + }); + + bindings.device.session.nextMessage = { + type: "error", + description: "location unavailable", + stack: "Error: location unavailable", + }; + const withoutLocation = await manager.loadScript(started.value.sessionId, { + sourceKind: "inline", + source: "throw new Error('location unavailable');", + }); + expect(withoutLocation.ok).toBe(true); + if (withoutLocation.ok) { + expect(withoutLocation.value.messages[0]).not.toHaveProperty("file_name"); + expect(withoutLocation.value.messages[0]).not.toHaveProperty("line_number"); + expect(withoutLocation.value.messages[0]).not.toHaveProperty( + "column_number", + ); + } + await manager.closeAll(); +}); + +it("does not carry earlier script truncation into a complete later load", async () => { + const { bindings, manager, started } = await startLocalAttachedSession(); + expect(started.ok).toBe(true); + if (!started.ok) return; + + const first = await manager.loadScript(started.value.sessionId, { + sourceKind: "inline", + source: "send('A');", + }); + expect(first.ok).toBe(true); + if (!first.ok) return; + const firstScript = bindings.device.session.scripts[0]; + expect(firstScript).toBeDefined(); + if (firstScript === undefined) return; + for (let index = 0; index < 800; index += 1) + firstScript.message.emit( + { type: "send", payload: "x".repeat(32_700) }, + null, + ); + + bindings.device.session.nextPayload = { tag: "B" }; + const second = await manager.loadScript(started.value.sessionId, { + sourceKind: "inline", + source: "send('B');", + }); + expect(second.ok).toBe(true); + if (second.ok) { + expect(second.value.messages).toHaveLength(1); + expect(second.value.messagesTruncated).toBe(false); + } + expect(manager.status(started.value.sessionId)?.messagesTruncated).toBe(true); + await manager.closeAll(); +}); + +it("reports truncation when that script loses its own message", async () => { const { bindings, manager, started } = await startLocalAttachedSession(); const session = bindings.device.session; expect(started.ok).toBe(true); @@ -800,6 +891,7 @@ it("keeps an active script message bucket after its last message is evicted", as ); expect(second.value.messagesTruncated).toBe(true); } + expect(manager.status(started.value.sessionId)?.messagesTruncated).toBe(true); expect(manager.status(started.value.sessionId)?.messages).toContainEqual( expect.objectContaining({ script_id: second.ok ? second.value.scriptId : "", @@ -926,6 +1018,73 @@ it("redacts remote credentials and maps a missing optional binding", async () => expect(unavailable.error.message).toContain("Cannot find package 'frida'"); }); +it("clears current binding failure after recovery", async () => { + const bindings = new FakeDeviceManager(); + let attempts = 0; + const manager = new FridaInstrumentationManager({ + deviceManagerLoader: async () => { + attempts += 1; + if (attempts === 1) throw new Error("transient binding load failure"); + return bindings; + }, + }); + + const failed = await manager.listDevices(); + expect(failed.ok).toBe(false); + if (!failed.ok) + expect(failed.error.message).toContain("transient binding load failure"); + expect(manager.availabilityFailure()?.reason).toBe( + "transient binding load failure", + ); + + const recovered = await manager.listDevices(); + expect(recovered.ok).toBe(true); + expect(manager.availabilityFailure()).toBeUndefined(); +}); + +it("keeps remote option conflicts as actionable invalid input", async () => { + const manager = new FridaInstrumentationManager({ + deviceManager: new FakeDeviceManager(), + }); + const retained = await manager.startSession({ + mode: "attach", + remote: { address: "127.0.0.1:27042", token: "first-token" }, + pid: 512, + }); + expect(retained.ok).toBe(true); + if (!retained.ok) return; + + const conflicting = [ + await manager.listDevices({ + address: "127.0.0.1:27042", + token: "other-token", + }), + await manager.listProcesses({ + remote: { address: "127.0.0.1:27042", token: "other-token" }, + }), + await manager.startSession({ + mode: "attach", + remote: { address: "127.0.0.1:27042", token: "other-token" }, + pid: 512, + }), + ]; + for (const result of conflicting) { + expect(result.ok).toBe(false); + if (result.ok) continue; + if (!(result.error instanceof AnalysisInputError)) throw result.error; + expect(result.error.operation).not.toBe("frida_remote_connection"); + expect(result.error.issues).toContainEqual( + expect.objectContaining({ + path: ["remote"], + reason: "invalid_value", + message: expect.stringContaining("different connection options"), + }), + ); + expect(result.error.message).not.toContain("other-token"); + } + await manager.closeAll(); +}); + it("redacts remote credentials echoed by a target script", async () => { const token = "synthetic-token-from-auth"; const certificate = "synthetic-certificate-from-auth"; diff --git a/src/frida/FridaInstrumentationManager.ts b/src/frida/FridaInstrumentationManager.ts index b93cffae5..ea77f6e81 100644 --- a/src/frida/FridaInstrumentationManager.ts +++ b/src/frida/FridaInstrumentationManager.ts @@ -92,6 +92,7 @@ interface ManagedSession extends ManagedRemoteConnection { resumeAfterDetachPending: boolean; capturedBytes: number; messagesTruncated: boolean; + readonly truncatedScripts: Set; } export interface FridaInstrumentationManagerOptions { @@ -115,6 +116,7 @@ export class FridaInstrumentationManager implements FridaInstrumentationPort { readonly #remoteQueues = new Map>(); readonly #deviceManagerLoader: () => Promise; #deviceManager: DeviceManager | undefined; + #deviceManagerLoading: Promise | undefined; #availabilityFailure: OptionalProviderLoadFailure | undefined; readonly #operations = new Set>(); #closing = false; @@ -307,6 +309,7 @@ export class FridaInstrumentationManager implements FridaInstrumentationPort { resumeAfterDetachPending: false, capturedBytes: 0, messagesTruncated: false, + truncatedScripts: new Set(), }; session.detached.connect(() => { managed.state = "detached"; @@ -432,7 +435,7 @@ export class FridaInstrumentationManager implements FridaInstrumentationPort { sourcePath: source.sourceKind === "file" ? source.path : null, sourceSha256, messages: managed.messagesByScript.get(scriptId)?.toArray() ?? [], - messagesTruncated: managed.messagesTruncated, + messagesTruncated: managed.truncatedScripts.has(scriptId), }; if (signal?.aborted) { const cleanupReason = await this.#releaseScript(managed, entry); @@ -598,6 +601,7 @@ export class FridaInstrumentationManager implements FridaInstrumentationPort { } managed.scripts.delete(entry.id); managed.messagesByScript.delete(entry.id); + managed.truncatedScripts.delete(entry.id); return undefined; } @@ -916,7 +920,7 @@ export class FridaInstrumentationManager implements FridaInstrumentationPort { bytes: Buffer.byteLength('{"payload_unavailable":true}'), }; } - if (payloadResult.truncated) managed.messagesTruncated = true; + if (payloadResult.truncated) markMessageTruncated(managed, scriptId); } let binaryResult: Record = {}; if (data !== null) { @@ -939,7 +943,7 @@ export class FridaInstrumentationManager implements FridaInstrumentationPort { data_truncated: true, data_bytes: data.byteLength, }; - managed.messagesTruncated = true; + markMessageTruncated(managed, scriptId); } else { binaryResult = { data_base64: data.toString("base64") }; } @@ -961,12 +965,21 @@ export class FridaInstrumentationManager implements FridaInstrumentationPort { message.stack === undefined ? undefined : boundedRedactedString(message.stack, MAX_MESSAGE_TEXT_BYTES, secrets); + const fileName = + message.fileName === undefined + ? undefined + : boundedRedactedString( + message.fileName, + MAX_MESSAGE_TEXT_BYTES, + secrets, + ); if ( type.truncated || description?.truncated === true || - stack?.truncated === true + stack?.truncated === true || + fileName?.truncated === true ) - managed.messagesTruncated = true; + markMessageTruncated(managed, scriptId); const observation: JsonValue = { script_id: scriptId, type: type.value, @@ -989,11 +1002,27 @@ export class FridaInstrumentationManager implements FridaInstrumentationPort { stack: stack.value, ...(stack.truncated ? { stack_truncated: true } : {}), }), + ...(fileName === undefined + ? {} + : { + file_name: fileName.value, + ...(fileName.truncated ? { file_name_truncated: true } : {}), + }), + ...(typeof message.lineNumber === "number" && + Number.isSafeInteger(message.lineNumber) && + message.lineNumber >= 0 + ? { line_number: message.lineNumber } + : {}), + ...(typeof message.columnNumber === "number" && + Number.isSafeInteger(message.columnNumber) && + message.columnNumber >= 0 + ? { column_number: message.columnNumber } + : {}), ...binaryResult, }; const charge = Buffer.byteLength(JSON.stringify(observation)); if (charge > MAX_SESSION_CAPTURE_BYTES) { - managed.messagesTruncated = true; + markMessageTruncated(managed, scriptId); return; } while ( @@ -1008,8 +1037,8 @@ export class FridaInstrumentationManager implements FridaInstrumentationPort { const scriptMessages = managed.messagesByScript.get(removed.scriptId); if (scriptMessages?.peek() === removed.observation) scriptMessages.shift(); + markMessageTruncated(managed, removed.scriptId); } - managed.messagesTruncated = true; } managed.messages.push({ observation, scriptId, charge }); managed.messagesByScript.get(scriptId)?.push(observation); @@ -1017,18 +1046,26 @@ export class FridaInstrumentationManager implements FridaInstrumentationPort { } async #getDeviceManager(): Promise { - if (this.#deviceManager === undefined) { - try { - this.#deviceManager = await this.#deviceManagerLoader(); - } catch (cause: unknown) { - this.#availabilityFailure = { - providerId: "frida", - reason: cause instanceof Error ? cause.message : String(cause), - }; - throw cause; - } + if (this.#deviceManager !== undefined) return this.#deviceManager; + if (this.#deviceManagerLoading !== undefined) + return this.#deviceManagerLoading; + const loading = Promise.resolve().then(this.#deviceManagerLoader); + this.#deviceManagerLoading = loading; + try { + this.#deviceManager = await loading; + if (this.#availabilityFailure?.providerId === "frida") + this.#availabilityFailure = undefined; + return this.#deviceManager; + } catch (cause: unknown) { + this.#availabilityFailure = { + providerId: "frida", + reason: boundedRedactedString(errorMessage(cause), 4096, []).value, + }; + throw cause; + } finally { + if (this.#deviceManagerLoading === loading) + this.#deviceManagerLoading = undefined; } - return this.#deviceManager; } async #selectDevice( @@ -1054,7 +1091,14 @@ export class FridaInstrumentationManager implements FridaInstrumentationPort { let reference = this.#remoteDevices.get(remote.address); if (reference !== undefined) { if (reference.optionsDigest !== optionsDigest) - throw new AnalysisInputError("frida_remote_connection"); + throw new AnalysisInputError("frida_remote_connection", undefined, [ + { + path: ["remote"], + reason: "invalid_value", + message: + "This endpoint is already retained with different connection options. Reuse the existing options or close its sessions before changing them.", + }, + ]); reference.references += 1; } else { const device = manager.addRemoteDevice( @@ -1361,6 +1405,8 @@ const providerError = ( remote?: FridaRemoteConnection, cleanupReason?: string, ): AnalysisError => { + if (cause instanceof AnalysisInputError) + return new AnalysisInputError(operation, { cause }, cause.issues); if (cause instanceof AnalysisCancelledError) return new AnalysisCancelledError(operation, { cause: cause.cause, @@ -1403,6 +1449,15 @@ interface CapturedMessage { readonly charge: number; } +const markMessageTruncated = ( + managed: ManagedSession, + scriptId: string, +): void => { + managed.messagesTruncated = true; + if (managed.messagesByScript.has(scriptId)) + managed.truncatedScripts.add(scriptId); +}; + /** * Arrival-ordered queue whose eviction advances a head offset. A full capture * budget evicts on every message, and `Array#shift` would copy every retained diff --git a/src/frida/frida-bindings.d.ts b/src/frida/frida-bindings.d.ts index 9ccdb8953..280e8c270 100644 --- a/src/frida/frida-bindings.d.ts +++ b/src/frida/frida-bindings.d.ts @@ -4,6 +4,9 @@ declare module "frida" { readonly payload?: unknown; readonly description?: string; readonly stack?: string; + readonly fileName?: string; + readonly lineNumber?: number; + readonly columnNumber?: number; } export type SignalHandler = (...args: never[]) => void;