fix(frida): preserve script diagnostics and isolate capture truncation

2026-10-14 09:47:28 +08:00 · 2026-10-14 09:47:28 +08:00 · 5fa7a52d1f
parent b940ad0d65
commit 5fa7a52d1f
3 changed files with 238 additions and 21 deletions
--- 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 = <Handler extends (...args: never[]) => void>() => {
@ -40,13 +41,14 @@ class FakeScript implements Script {
    readonly beforeLoad: () => Promise<void> = async () => {},
    readonly description = "script loaded",
    readonly stack = "",
+    readonly messageValue?: FridaMessage,
  ) {}

  async load(): Promise<void> {
    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";
--- 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<string>;
 }

 export interface FridaInstrumentationManagerOptions {
@ -115,6 +116,7 @@ export class FridaInstrumentationManager implements FridaInstrumentationPort {
  readonly #remoteQueues = new Map<string, Promise<void>>();
  readonly #deviceManagerLoader: () => Promise<DeviceManager>;
  #deviceManager: DeviceManager | undefined;
+  #deviceManagerLoading: Promise<DeviceManager> | undefined;
  #availabilityFailure: OptionalProviderLoadFailure | undefined;
  readonly #operations = new Set<Promise<unknown>>();
  #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<string, JsonValue> = {};
    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<DeviceManager> {
-    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
--- 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;