From 40c235c6428f0be5c97d60f1a1d8aebc92e88768 Mon Sep 17 00:00:00 2001 From: morluto <76467478+morluto@users.noreply.github.com> Date: Fri, 23 Oct 2026 23:36:21 +0800 Subject: [PATCH] fix(comparison): preserve decoding uncertainty and known string facts Preserve format and Evidence semantics at their owning boundaries, with focused regression coverage and documentation where applicable. Fixes #6774 --- docs/mcp-contracts.md | 6 + src/domain/functionComparison.test.ts | 2 +- src/domain/functionComparisonDimensions.ts | 298 +++++++++++++++++- src/domain/functionComparisonNormalization.ts | 34 +- ...unctionComparisonStringDecodingMcp.test.ts | 268 ++++++++++++++++ 5 files changed, 581 insertions(+), 27 deletions(-) create mode 100644 tests/boundary/mcp/functionComparisonStringDecodingMcp.test.ts diff --git a/docs/mcp-contracts.md b/docs/mcp-contracts.md index 933a7ca21a..99cc594ff5 100644 --- a/docs/mcp-contracts.md +++ b/docs/mcp-contracts.md @@ -86,6 +86,12 @@ can observe backend recovery; healthy dossiers keep their existing cache policy. `procedure_pseudo_code` still fails for that function. Hopper's unavailable reference-kind metadata and reported unresolved calls still prevent equality conclusions for those dimensions. Analysis profiles version these declarations so older snapshots are not reused as current results. +For `strings_names`, a string marked with `decoding.available: false` keeps its +provider display text in the source Evidence, but that text is not compared as +decoded content. If no independently known string, name, or literal fact differs, +the dimension remains `unknown` with the decoding reason. Known content and typed +string-object differences can establish a change; missing typed metadata remains +unknown, and `encoding_status` describes provenance rather than content. IDA declares the facets its selected adapter cannot provide; an empty unsupported array does not prove observed absence. Evidence and bundle imports reject unsupported fields in their envelopes, diff --git a/src/domain/functionComparison.test.ts b/src/domain/functionComparison.test.ts index bf999f5ef4..11357c6500 100644 --- a/src/domain/functionComparison.test.ts +++ b/src/domain/functionComparison.test.ts @@ -1086,7 +1086,7 @@ describe("function comparison Swift string literals", () => { ).toMatchObject({ status: "unknown", limitations: [ - "Swift String literal coverage differs or is partial; referenced strings and names match, and complete coverage does not establish a differing literal.", + "Swift String literal coverage differs or is partial; complete coverage does not establish a differing literal.", ], }); }); diff --git a/src/domain/functionComparisonDimensions.ts b/src/domain/functionComparisonDimensions.ts index be6ea7eff6..bd4912b6c9 100644 --- a/src/domain/functionComparisonDimensions.ts +++ b/src/domain/functionComparisonDimensions.ts @@ -7,9 +7,10 @@ import { commentProjection, identityProjection, isAutoName, + nameAndSwiftProjection, normalizeCfg, + referencedStringProjection, sorted, - stringAndNameProjection, swiftLiteralSitesConflict, } from "./functionComparisonNormalization.js"; import type { @@ -205,10 +206,34 @@ const compareStringsAndNames = ( const complete = swiftLiteralCoverage(left) === swiftLiteralCoverage(right) && swiftLiteralCoverage(left) !== "partial"; - // Facts both dossiers record decide a change before Swift literal coverage can. - const leftValues = stringAndNameProjection(left, complete); - const rightValues = stringAndNameProjection(right, complete); - if (leftValues === null || rightValues === null) + const leftStrings = referencedStringProjection(left); + const rightStrings = referencedStringProjection(right); + const stringFacts = compareReferencedStrings(leftStrings, rightStrings); + const leftNamesAndLiterals = nameAndSwiftProjection(left, complete); + const rightNamesAndLiterals = nameAndSwiftProjection(right, complete); + if (stringFacts.changed) + return stringsAndNamesResult({ + leftStrings, + rightStrings, + leftNamesAndLiterals, + rightNamesAndLiterals, + context, + status: "changed", + limitations: [ + ...stringFacts.limitations, + ...(leftNamesAndLiterals === null || rightNamesAndLiterals === null + ? [ + "Reference source locations could not be normalized to function offsets.", + ] + : []), + ...(complete + ? [] + : [ + "Swift String literal coverage differs or is partial; complete coverage does not establish a differing literal.", + ]), + ], + }); + if (leftNamesAndLiterals === null || rightNamesAndLiterals === null) return unresolvedDimension({ dimension: "strings_names", links: context.links, @@ -220,29 +245,272 @@ const compareStringsAndNames = ( }); const compared = compareValues( "strings_names", - leftValues, - rightValues, + leftNamesAndLiterals, + rightNamesAndLiterals, context, ); - if (complete || compared.status === "changed") return compared; + if (compared.status === "changed") + return stringsAndNamesResult({ + leftStrings, + rightStrings, + leftNamesAndLiterals, + rightNamesAndLiterals, + context, + status: "changed", + limitations: stringFacts.limitations, + }); // A retained literal differs only when the opposite dossier establishes its absence. if (swiftLiteralSitesConflict(left, right)) { - const leftAll = stringAndNameProjection(left); - const rightAll = stringAndNameProjection(right); - if (leftAll !== null && rightAll !== null) - return compareValues("strings_names", leftAll, rightAll, context); + const leftAll = nameAndSwiftProjection(left); + const rightAll = nameAndSwiftProjection(right); + if (leftAll !== null && rightAll !== null) { + const literalComparison = compareValues( + "strings_names", + leftAll, + rightAll, + context, + ); + if (literalComparison.status === "changed") + return stringsAndNamesResult({ + leftStrings, + rightStrings, + leftNamesAndLiterals: leftAll, + rightNamesAndLiterals: rightAll, + context, + status: "changed", + limitations: stringFacts.limitations, + }); + } } + const limitations = [ + ...stringFacts.limitations, + ...(complete + ? [] + : [ + "Swift String literal coverage differs or is partial; complete coverage does not establish a differing literal.", + ]), + ]; + if (limitations.length === 0) + return stringsAndNamesResult({ + leftStrings, + rightStrings, + leftNamesAndLiterals, + rightNamesAndLiterals, + context, + status: "unchanged", + }); return unresolvedDimension({ dimension: "strings_names", links: context.links, leftCount: null, rightCount: null, - limitations: [ - "Swift String literal coverage differs or is partial; referenced strings and names match, and complete coverage does not establish a differing literal.", - ], + limitations, }); }; +const stringsAndNamesResult = ({ + leftStrings, + rightStrings, + leftNamesAndLiterals, + rightNamesAndLiterals, + context, + status, + limitations = [], +}: { + readonly leftStrings: readonly ReferencedStringFact[]; + readonly rightStrings: readonly ReferencedStringFact[]; + readonly leftNamesAndLiterals: readonly unknown[] | null; + readonly rightNamesAndLiterals: readonly unknown[] | null; + readonly context: ComparisonContext; + readonly status: "changed" | "unchanged"; + readonly limitations?: readonly string[]; +}): FunctionDimension => + dimensionResult({ + dimension: "strings_names", + status, + left: { + referenced_strings: leftStrings, + names_and_literals: leftNamesAndLiterals, + }, + right: { + referenced_strings: rightStrings, + names_and_literals: rightNamesAndLiterals, + }, + ...context, + leftCount: leftStrings.length + (leftNamesAndLiterals?.length ?? 0), + rightCount: rightStrings.length + (rightNamesAndLiterals?.length ?? 0), + textDelta: null, + limitations, + }); + +type ReferencedStringFact = ReturnType< + typeof referencedStringProjection +>[number]; + +const compareReferencedStrings = ( + left: readonly ReferencedStringFact[], + right: readonly ReferencedStringFact[], +): { readonly changed: boolean; readonly limitations: readonly string[] } => { + const leftGroups = groupStringsByOffset(left); + const rightGroups = groupStringsByOffset(right); + const leftUnlocated = leftGroups.get(null) !== undefined; + const rightUnlocated = rightGroups.get(null) !== undefined; + const limitations = new Set( + [...left, ...right].flatMap(({ decoding_reason }) => + decoding_reason === null + ? [] + : [`Referenced string decoding unavailable: ${decoding_reason}`], + ), + ); + let changed = false; + for (const offset of new Set([...leftGroups.keys(), ...rightGroups.keys()])) { + const leftRows = leftGroups.get(offset); + const rightRows = rightGroups.get(offset); + if (offset === null) { + limitations.add( + "Referenced string source location could not be normalized.", + ); + continue; + } + if (leftRows === undefined || rightRows === undefined) { + if (leftUnlocated || rightUnlocated) + limitations.add( + `Referenced string membership at source offset ${offset} is ambiguous because another source location is unavailable.`, + ); + else changed = true; + continue; + } + if (leftRows.length !== 1 || rightRows.length !== 1) { + const repeated = compareRepeatedStringRows(leftRows, rightRows); + if (repeated !== null) { + changed ||= repeated.changed; + for (const limitation of repeated.limitations) + limitations.add(limitation); + continue; + } + limitations.add( + `Multiple referenced strings at source offset ${offset} cannot be paired without ambiguity.`, + ); + continue; + } + const leftRow = leftRows[0]; + const rightRow = rightRows[0]; + if (leftRow === undefined || rightRow === undefined) continue; + if ( + leftRow.content !== null && + rightRow.content !== null && + leftRow.content !== rightRow.content + ) + changed = true; + if ( + leftRow.metadata !== null && + rightRow.metadata !== null && + (leftRow.metadata.encoding !== rightRow.metadata.encoding || + leftRow.metadata.termination !== rightRow.metadata.termination || + leftRow.metadata.byte_length !== rightRow.metadata.byte_length) + ) + changed = true; + if ((leftRow.metadata === null) !== (rightRow.metadata === null)) + limitations.add( + `Typed string metadata is unavailable at source offset ${offset}.`, + ); + if (leftRow.content === null || rightRow.content === null) { + if (leftRow.content !== null || rightRow.content !== null) + limitations.add( + `Referenced string content is unavailable on one side at source offset ${offset}.`, + ); + } + } + return { changed, limitations: [...limitations] }; +}; + +const compareRepeatedStringRows = ( + left: readonly ReferencedStringFact[], + right: readonly ReferencedStringFact[], +): { + readonly changed: boolean; + readonly limitations: readonly string[]; +} | null => { + if ( + left.some(({ content }) => content === null) || + right.some(({ content }) => content === null) + ) + return null; + const leftByContent = groupStringsByContent(left); + const rightByContent = groupStringsByContent(right); + let changed = !sameMultiset( + left.flatMap(({ content }) => (content === null ? [] : [content])), + right.flatMap(({ content }) => (content === null ? [] : [content])), + ); + const limitations = new Set(); + for (const content of new Set([ + ...leftByContent.keys(), + ...rightByContent.keys(), + ])) { + const leftRows = leftByContent.get(content) ?? []; + const rightRows = rightByContent.get(content) ?? []; + if (leftRows.length !== rightRows.length) continue; + const leftMetadata = leftRows.map(({ metadata }) => metadata); + const rightMetadata = rightRows.map(({ metadata }) => metadata); + if ( + leftMetadata.every((metadata) => metadata === null) && + rightMetadata.every((metadata) => metadata === null) + ) + continue; + if ( + leftMetadata.some((metadata) => metadata === null) || + rightMetadata.some((metadata) => metadata === null) + ) { + limitations.add( + "Typed string metadata is unavailable for repeated content at a source offset.", + ); + continue; + } + changed ||= !sameMultiset( + leftMetadata.map((metadata) => JSON.stringify(metadata)), + rightMetadata.map((metadata) => JSON.stringify(metadata)), + ); + } + return { changed, limitations: [...limitations] }; +}; + +const groupStringsByContent = ( + strings: readonly ReferencedStringFact[], +): Map => { + const groups = new Map(); + for (const string of strings) { + if (string.content === null) continue; + const rows = groups.get(string.content); + if (rows === undefined) groups.set(string.content, [string]); + else rows.push(string); + } + return groups; +}; + +const sameMultiset = ( + left: Iterable, + right: Iterable, +): boolean => { + const leftValues = [...left].sort(); + const rightValues = [...right].sort(); + return ( + leftValues.length === rightValues.length && + leftValues.every((value, index) => value === rightValues[index]) + ); +}; + +const groupStringsByOffset = ( + strings: readonly ReferencedStringFact[], +): Map => { + const groups = new Map(); + for (const string of strings) { + const rows = groups.get(string.source_offset); + if (rows === undefined) groups.set(string.source_offset, [string]); + else rows.push(string); + } + return groups; +}; + const swiftLiteralCoverage = ( snapshot: FunctionSnapshot, ): "absent" | "complete" | "partial" => diff --git a/src/domain/functionComparisonNormalization.ts b/src/domain/functionComparisonNormalization.ts index f0e1cd9c60..f5cf4c87ad 100644 --- a/src/domain/functionComparisonNormalization.ts +++ b/src/domain/functionComparisonNormalization.ts @@ -93,21 +93,33 @@ export const commentProjection = ( return values.some(({ offset }) => offset === null) ? null : sorted(values); }; -export const stringAndNameProjection = ( +export const referencedStringProjection = (snapshot: FunctionSnapshot) => { + const values = snapshot.dossier.referenced_strings.map( + ({ source_address: sourceAddress, value, decoding, string }) => ({ + source_offset: relativeAddress( + sourceAddress, + snapshot.dossier.procedure.address, + ), + content: decoding === undefined ? value : null, + decoding_reason: decoding?.reason ?? null, + metadata: + string === undefined + ? null + : { + encoding: string.encoding, + termination: string.termination, + byte_length: string.byte_length, + }, + }), + ); + return sorted(values); +}; + +export const nameAndSwiftProjection = ( snapshot: FunctionSnapshot, includeSwiftLiterals = true, ): readonly unknown[] | null => { const values = [ - ...snapshot.dossier.referenced_strings.map( - ({ source_address: sourceAddress, value }) => ({ - kind: "string", - source_offset: relativeAddress( - sourceAddress, - snapshot.dossier.procedure.address, - ), - value, - }), - ), ...snapshot.dossier.referenced_names.map( ({ source_address: sourceAddress, value }) => ({ kind: "name", diff --git a/tests/boundary/mcp/functionComparisonStringDecodingMcp.test.ts b/tests/boundary/mcp/functionComparisonStringDecodingMcp.test.ts new file mode 100644 index 0000000000..6957f417c4 --- /dev/null +++ b/tests/boundary/mcp/functionComparisonStringDecodingMcp.test.ts @@ -0,0 +1,268 @@ +import { Client, InMemoryTransport } from "@modelcontextprotocol/client"; +import { afterAll, beforeAll, describe, expect, it } from "vitest"; + +import { FUNCTION_COMPARISON_EXAMPLE } from "../../../src/contracts/functionComparisonExample.js"; +import { createEvidence, parseEvidence } from "../../../src/domain/evidence.js"; +import { functionDossierSchema } from "../../../src/domain/hopperValues.js"; +import { + functionComparisonResultSchema, + type FunctionComparisonResult as Comparison, +} from "../../../src/domain/functionComparisonSchemas.js"; +import { jsonValueSchema } from "../../../src/domain/jsonValue.js"; +import { createServer } from "../../../src/server/createServer.js"; +import { createTestBinarySession } from "../../fixtures/binarySession.js"; + +const template = FUNCTION_COMPARISON_EXAMPLE.left; +const base = functionDossierSchema.parse(template.normalized_result); + +const evidence = ( + id: string, + strings: readonly (typeof base.referenced_strings)[number][], + literalValue?: string, +) => + createEvidence( + { + path: `/fixture/function-string-decoding/${id}`, + sha256: "1".repeat(64), + format: "mach-o", + }, + template.provider, + { + operation: "analyze_function", + parameters: { ...template.parameters, observation: id }, + result: jsonValueSchema.parse({ + ...base, + referenced_strings: [...strings], + swift_string_literals: + literalValue === undefined + ? [] + : [ + { + source_address: "0x1002", + value: literalValue, + registers: ["x0", "x1"], + words: ["0x1", "0x2"], + representation: "small", + address: null, + }, + ], + swift_string_literal_omissions: [], + }), + confidence: template.confidence, + authority: template.authority, + relationshipCoverage: { status: "complete" }, + }, + ); + +const reference = ( + value: string, + options: { + readonly decoding?: { readonly available: false; readonly reason: string }; + readonly termination?: "missing" | "present_or_not_required"; + readonly byteLength?: number; + readonly sourceAddress?: string; + readonly metadata?: boolean; + readonly encodingStatus?: "observed" | "inferred"; + } = {}, +) => ({ + address: "0x2000", + source_address: options.sourceAddress ?? "0x1000", + value, + ...(options.decoding === undefined ? {} : { decoding: options.decoding }), + ...(options.metadata === false + ? {} + : { + string: { + encoding: "utf-8", + ...(options.encodingStatus === undefined + ? {} + : { encoding_status: options.encodingStatus }), + termination: options.termination ?? "present_or_not_required", + byte_length: options.byteLength ?? 6, + }, + }), +}); + +type Compare = ( + left: ReturnType, + right: ReturnType, +) => Promise; + +const session = createTestBinarySession(() => { + throw new Error("Function comparison must not launch a provider"); +}); +const server = createServer({ kind: "session", session }); +const client = new Client({ + name: "function-string-decoding-test", + version: "1", +}); +const [clientTransport, serverTransport] = InMemoryTransport.createLinkedPair(); +let compare: Compare; + +beforeAll(async () => { + await server.connect(serverTransport); + await client.connect(clientTransport); + compare = async (left, right) => { + const response = await client.callTool({ + name: "compare_functions", + arguments: { left, right }, + }); + expect( + response.isError, + JSON.stringify(response.structuredContent), + ).not.toBe(true); + return functionComparisonResultSchema.parse( + parseEvidence(response.structuredContent).normalized_result, + ); + }; +}); + +afterAll(async () => { + await Promise.allSettled([client.close(), server.close(), session.close()]); +}); + +const stringsNames = (result: Comparison) => + result.dimensions.find(({ dimension }) => dimension === "strings_names"); + +describe("compare_functions referenced string decoding", () => { + const reason = "The native string bytes are unavailable"; + const undecodable = (id: string, display: string) => + evidence(id, [ + reference(display, { decoding: { available: false, reason } }), + ]); + + it("keeps unavailable previews unknown even when their display differs", async () => { + const samePreview = await compare( + undecodable("1", "display preview"), + undecodable("2", "display preview"), + ); + expect(samePreview.status).toBe("unknown"); + expect(stringsNames(samePreview)).toMatchObject({ + status: "unknown", + limitations: [`Referenced string decoding unavailable: ${reason}`], + }); + + const differentPreview = await compare( + undecodable("3", "first display preview"), + undecodable("4", "second display preview"), + ); + expect(differentPreview.status).toBe("unknown"); + }); + + it("compares known typed string facts alongside unavailable content", async () => { + const metadataChange = await compare( + evidence("9", [ + reference("display preview", { + decoding: { available: false, reason }, + byteLength: 6, + }), + ]), + evidence("a", [ + reference("same display preview", { + decoding: { available: false, reason }, + byteLength: 7, + }), + ]), + ); + expect(metadataChange.status).toBe("changed"); + + const mixedKnownChange = await compare( + evidence("b", [ + reference("uncertain display", { + decoding: { available: false, reason }, + sourceAddress: "0x1000", + }), + reference("before", { sourceAddress: "0x1001" }), + ]), + evidence("c", [ + reference("uncertain display", { + decoding: { available: false, reason }, + sourceAddress: "0x1000", + }), + reference("after", { sourceAddress: "0x1001" }), + ]), + ); + expect(mixedKnownChange.status).toBe("changed"); + }); + + it("retains decoder reasons when strings share a source site", async () => { + const reason = "Typed string bytes could not be read"; + const rows = [ + reference("display preview", { + decoding: { available: false, reason }, + }), + reference("known content"), + ]; + const result = await compare( + evidence("repeated-left", rows), + evidence("repeated-right", rows), + ); + expect(stringsNames(result)).toMatchObject({ + status: "unknown", + limitations: expect.arrayContaining([ + `Referenced string decoding unavailable: ${reason}`, + ]), + }); + }); + + it("compares decoded content and termination facts", async () => { + const decoded = ( + id: string, + termination: + | "missing" + | "present_or_not_required" = "present_or_not_required", + byteLength = 6, + ) => evidence(id, [reference("hello", { termination, byteLength })]); + const equal = await compare(decoded("5"), decoded("6")); + expect(equal.status).toBe("unchanged"); + expect(stringsNames(equal)).toMatchObject({ + status: "unchanged", + left_count: 1, + right_count: 1, + left_digest: expect.stringMatching(/^[a-f0-9]{64}$/u), + }); + expect(stringsNames(equal)?.right_digest).toBe( + stringsNames(equal)?.left_digest, + ); + + const different = await compare(decoded("7"), decoded("8", "missing", 5)); + expect(different.status).toBe("changed"); + expect(stringsNames(different)).toMatchObject({ status: "changed" }); + }); + + it("preserves paired metadata absence and ignores encoding provenance", async () => { + const missingMetadata = await compare( + evidence("d", [reference("same", { metadata: false })]), + evidence("e", [reference("same")]), + ); + expect(missingMetadata.status).toBe("unknown"); + + const absentMetadata = await compare( + evidence("g", [reference("same", { metadata: false })]), + evidence("h", [reference("same", { metadata: false })]), + ); + expect(absentMetadata.status).toBe("unchanged"); + + const provenanceOnly = await compare( + evidence("f", [reference("same", { encodingStatus: "observed" })]), + evidence("0", [reference("same", { encodingStatus: "inferred" })]), + ); + expect(provenanceOnly.status).toBe("unchanged"); + }); + + it("keeps literal changes in the combined string dimension", async () => { + const changed = await compare( + evidence("literal-before", [reference("hello")], "literal one"), + evidence("literal-after", [reference("hello")], "literal two"), + ); + expect(changed.status).toBe("changed"); + expect(stringsNames(changed)).toMatchObject({ + status: "changed", + left_count: 2, + right_count: 2, + }); + expect(stringsNames(changed)?.left_digest).not.toBe( + stringsNames(changed)?.right_digest, + ); + }); +});