From 9cc208fc6052b5d2edab6b85fe50b288fb17225e Mon Sep 17 00:00:00 2001 From: morluto <76467478+morluto@users.noreply.github.com> Date: Mon, 16 Nov 2026 00:39:50 +0800 Subject: [PATCH] fix(verify): report every nested verifier failure reason (#11202) --- scripts/lib/verifier-cleanup.d.mts | 3 +++ scripts/lib/verifier-cleanup.mjs | 12 ++++++++++ scripts/verify-frida.mjs | 8 ++++--- scripts/verify-real-ida.mjs | 11 ++++----- tests/boundary/process/fridaVerifier.test.ts | 9 +++++++- .../boundary/process/verifierCleanup.test.ts | 23 +++++++++++++++++++ 6 files changed, 56 insertions(+), 10 deletions(-) diff --git a/scripts/lib/verifier-cleanup.d.mts b/scripts/lib/verifier-cleanup.d.mts index 9458277128..26156f4bbd 100644 --- a/scripts/lib/verifier-cleanup.d.mts +++ b/scripts/lib/verifier-cleanup.d.mts @@ -8,3 +8,6 @@ export function withVerifierCleanup( operation: () => T | Promise, cleanupTasks: readonly (() => void | Promise)[], ): Promise; + +/** The leaf reasons of a verifier failure, through nested AggregateErrors. */ +export function verifierFailureReasons(cause: unknown): string[]; diff --git a/scripts/lib/verifier-cleanup.mjs b/scripts/lib/verifier-cleanup.mjs index 979106d1d4..fe0013dfdf 100644 --- a/scripts/lib/verifier-cleanup.mjs +++ b/scripts/lib/verifier-cleanup.mjs @@ -36,3 +36,15 @@ export async function withVerifierCleanup(operation, cleanupTasks) { await closeVerifierResources(cleanupTasks); return value; } + +/** + * The leaf reasons of a verifier failure. `withVerifierCleanup` and the + * fixtures it releases nest `AggregateError`s (verification failure, cleanup + * failure, retained root), so a reporter that unwraps one level shows only + * "Verifier cleanup failed" instead of why. + */ +export function verifierFailureReasons(cause) { + if (cause instanceof AggregateError) + return cause.errors.flatMap(verifierFailureReasons); + return [cause instanceof Error ? cause.message : String(cause)]; +} diff --git a/scripts/verify-frida.mjs b/scripts/verify-frida.mjs index 396ddf4f88..7b975c1fe0 100644 --- a/scripts/verify-frida.mjs +++ b/scripts/verify-frida.mjs @@ -3,7 +3,10 @@ import { basename } from "node:path"; import { FridaInstrumentationService } from "../dist/application/frida/FridaInstrumentationService.js"; import { FridaInstrumentationManager } from "../dist/frida/FridaInstrumentationManager.js"; import { isValidFridaRemoteAddress } from "../dist/domain/fridaRemoteAddress.js"; -import { withVerifierCleanup } from "./lib/verifier-cleanup.mjs"; +import { + verifierFailureReasons, + withVerifierCleanup, +} from "./lib/verifier-cleanup.mjs"; const options = parseArguments(process.argv.slice(2)); if (options.help) { @@ -113,9 +116,8 @@ try { process.stdout.write(`${JSON.stringify(report)}\n`); if (!report.ok) process.exitCode = 1; } catch (cause) { - const causes = cause instanceof AggregateError ? cause.errors : [cause]; process.stderr.write( - `Frida real-provider verification failed: ${causes.map((reason) => (reason instanceof Error ? reason.message : String(reason))).join("; ")}\n`, + `Frida real-provider verification failed: ${verifierFailureReasons(cause).join("; ")}\n`, ); process.exitCode = 1; } diff --git a/scripts/verify-real-ida.mjs b/scripts/verify-real-ida.mjs index ef77f5bda5..f2f4083d98 100644 --- a/scripts/verify-real-ida.mjs +++ b/scripts/verify-real-ida.mjs @@ -4,7 +4,10 @@ import { findMappedNonFunction, } from "./lib/ida-feature-trace.mjs"; import { requireMcpToolError } from "./lib/mcp-verifier-results.mjs"; -import { withVerifierCleanup } from "./lib/verifier-cleanup.mjs"; +import { + verifierFailureReasons, + withVerifierCleanup, +} from "./lib/verifier-cleanup.mjs"; import { withVerifierResourceOwner } from "./lib/verifier-file-resources.mjs"; import assert from "node:assert/strict"; @@ -454,11 +457,7 @@ try { phase, message: cause instanceof Error ? cause.message : String(cause), ...(cause instanceof AggregateError - ? { - causes: cause.errors.map((failure) => - failure instanceof Error ? failure.message : String(failure), - ), - } + ? { causes: verifierFailureReasons(cause) } : {}), }; process.exitCode = 1; diff --git a/tests/boundary/process/fridaVerifier.test.ts b/tests/boundary/process/fridaVerifier.test.ts index 28f048e779..2fadeacdba 100644 --- a/tests/boundary/process/fridaVerifier.test.ts +++ b/tests/boundary/process/fridaVerifier.test.ts @@ -32,7 +32,7 @@ export class FridaInstrumentationService { return { ok: true, value: { cleanupError: null, evidence: { normalized_result: { pid, messages: [{ type: 'send', payload: { rea_frida_verification: 'ok', pid: process.env.REA_PROBE_KIND === 'wrong-marker' ? 42 : pid } }] } } } }; } }\`; -const manager = \`export class FridaInstrumentationManager { async closeAll() {} }\`; +const manager = \`export class FridaInstrumentationManager { async closeAll() { if (process.env.REA_PROBE_KIND === 'cleanup') throw new AggregateError([new Error('controlled EACCES removing the Frida fixture')], 'Frida cleanup failed'); } }\`; registerHooks({ resolve(specifier, context, next) { if (context.parentURL?.endsWith('/scripts/verify-frida.mjs')) { if (specifier.endsWith('/FridaInstrumentationService.js')) return { shortCircuit: true, url: 'data:text/javascript,' + encodeURIComponent(service) }; @@ -57,6 +57,13 @@ registerHooks({ resolve(specifier, context, next) { ["missing", false, "Frida returned no valid target PID"], ["wrong", true, "Frida observed a different target PID"], ["wrong-marker", true, "Selected target did not return the Frida marker"], + // The verification failure and the nested cleanup reason both reach the + // operator, not only the outer "cleanup failed" wrapper. + [ + "cleanup", + true, + "Frida observed a different target PID; controlled EACCES removing the Frida fixture", + ], ] as const) { await expect(invoke(kind, remote)).rejects.toMatchObject({ stderr: expect.stringContaining(message), diff --git a/tests/boundary/process/verifierCleanup.test.ts b/tests/boundary/process/verifierCleanup.test.ts index a75383c277..1ee38c43ee 100644 --- a/tests/boundary/process/verifierCleanup.test.ts +++ b/tests/boundary/process/verifierCleanup.test.ts @@ -1,6 +1,7 @@ import { expect, it } from "vitest"; import { closeVerifierResources, + verifierFailureReasons, withVerifierCleanup, } from "../../../scripts/lib/verifier-cleanup.mjs"; @@ -215,3 +216,25 @@ it("preserves nested aggregates and independent failures with the same identity" expect(error.errors[2]).toBe(shared); expect(error.errors[3]).toBe(shared); }); + +it("reports every leaf reason of a nested verifier failure", async () => { + const failure: unknown = await withVerifierCleanup(async () => { + throw new Error("analysis assertion failed"); + }, [ + () => + closeVerifierResources([ + async () => { + throw new AggregateError( + [new Error("EACCES: fixture root not removed"), "raw reason"], + "Retained verifier fixture", + ); + }, + ]), + ]).catch((cause: unknown) => cause); + expect(verifierFailureReasons(failure)).toEqual([ + "analysis assertion failed", + "EACCES: fixture root not removed", + "raw reason", + ]); + expect(verifierFailureReasons(new Error("plain"))).toEqual(["plain"]); +});