From 0677468877465c8dfbe25c72f142a7b5bc8a3212 Mon Sep 17 00:00:00 2001 From: morluto <76467478+morluto@users.noreply.github.com> Date: Mon, 12 Oct 2026 06:29:13 +0800 Subject: [PATCH] fix(android): preserve decoded values and duplicate class inventories Parse Apktool YAML and XML at the producer boundary, preserving scalar spelling, XML text and partial string coverage. Retain repeated JADX inventory rows with explicit duplicate-name counts and lookup ambiguity. Make the Apktool verifier report cleanup failures. Validation: parser, provider and JADX fixture boundary tests; typecheck, lint and documentation checks. Real Apktool and JADX engines were not exercised. Refs #1978 --- docs/android-analysis.md | 6 +- scripts/verify-apktool.mjs | 147 +++++++------ src/android/JadxAnalysis.ts | 16 +- src/apktool/ApktoolDecodeOutput.test.ts | 134 +++++++++++- src/apktool/ApktoolDecodeOutput.ts | 206 +++++++++++++----- src/apktool/ApktoolProvider.test.ts | 20 +- src/apktool/ApktoolProvider.ts | 2 +- src/contracts/android/androidToolContracts.ts | 2 +- src/domain/android/androidAnalysis.ts | 6 + .../boundary/process/jadxIntegration.test.ts | 21 ++ tests/fixtures/android/jadx-mcp.mjs | 1 + 11 files changed, 430 insertions(+), 131 deletions(-) diff --git a/docs/android-analysis.md b/docs/android-analysis.md index 03fd39850..5a4a2f879 100644 --- a/docs/android-analysis.md +++ b/docs/android-analysis.md @@ -107,7 +107,11 @@ the same source byte limit and explicit truncation reporting. upstream engine and should be checked against the retained XML when needed. Signature verification is explicitly `not_performed`. - Class search consumes all pages and checks their counts. An empty `query` - inventories all class names; class names are decompiler representations. + inventories all class-name rows; class names are decompiler representations. + Repeated names remain repeated in `matches` so the inventory row count and + producer evidence stay visible. `duplicate_names` identifies matching names + with multiple rows and their occurrence counts; a name alone does not + distinguish which provider class node a later lookup selects. - Class inventories read parsed metadata without generating source and return full reported display types, fields and inner class names. Synthetic members that code generation hides can appear in the inventory; it stays consistent diff --git a/scripts/verify-apktool.mjs b/scripts/verify-apktool.mjs index 8117038bc..aaec8a7ac 100644 --- a/scripts/verify-apktool.mjs +++ b/scripts/verify-apktool.mjs @@ -1,7 +1,5 @@ import { createHash } from "node:crypto"; -import { mkdtemp, readFile, rm, stat } from "node:fs/promises"; -import { tmpdir } from "node:os"; -import { join } from "node:path"; +import { readFile, stat } from "node:fs/promises"; import { ApktoolResourceAnalysisService } from "../dist/application/apktool/ApktoolResourceAnalysisService.js"; import { createApktoolResourceAnalysisProvider } from "../dist/composition/apktool.js"; @@ -11,8 +9,8 @@ if (options.help) { process.stdout.write( "Usage: npm run verify:apktool [-- --apk PATH]\n" + "Requires a caller-selected apktool launcher (REA_APKTOOL_COMMAND or\n" + - "PATH) and a Java runtime. Without --apk, a small signed probe APK is\n" + - "expected at the path given by --apk; nothing is downloaded or built.\n", + "PATH) and a Java runtime. Without --apk, only client availability is\n" + + "checked. Supply a local APK to verify decoding; nothing is downloaded or built.\n", ); process.exit(0); } @@ -38,70 +36,80 @@ const run = async (operation, input) => { return result.value.normalized_result; }; -const client = await run("inspect_apktool_client", {}); -if (client !== null) { - check( - "apktool launcher reports a version", - typeof client.client.apktool_version === "string" && - client.client.apktool_version !== "", - `command=${client.client.command} version=${client.client.apktool_version}`, - ); -} +try { + const client = await run("inspect_apktool_client", {}); + if (client !== null) { + check( + "apktool launcher reports a version", + typeof client.client.apktool_version === "string" && + client.client.apktool_version !== "", + `command=${client.client.command} version=${client.client.apktool_version}`, + ); + } -if (options.apk !== null) { - const decoded = await run("decode_android_resources", { - path: options.apk, - include_strings: true, - }); - if (decoded !== null) { - const bytes = await stat(options.apk).then((info) => info.size); - const digest = createHash("sha256") - .update(await readFile(options.apk)) - .digest("hex"); - check( - "decoded target identity matches the file on disk", - decoded.target.bytes === bytes && decoded.target.sha256 === digest, - `${bytes} bytes`, - ); - check( - "metadata carries version and SDK facts", - typeof decoded.metadata.version_name === "string" && - decoded.metadata.min_sdk_version !== null && - decoded.metadata.target_sdk_version !== null, - `version=${String(decoded.metadata.version_name)} sdk=${String(decoded.metadata.min_sdk_version)}..${String(decoded.metadata.target_sdk_version)}`, - ); - check( - "decoded manifest names the package", - typeof decoded.metadata.package_name === "string" && - decoded.manifest !== null && - decoded.manifest.includes(decoded.metadata.package_name), - String(decoded.metadata.package_name), - ); - check( - "string resources projected", - decoded.strings.length > 0, - `${decoded.strings.length} string(s)`, - ); - if (decoded.locales.length > 0) { - const locale = await run("decode_android_resources", { - path: options.apk, - include_strings: true, - locale: decoded.locales[0], - }); - if (locale !== null) - check( - `locale table projected for ${String(decoded.locales[0])}`, - Array.isArray(locale.strings), - `${locale.strings.length} string(s)`, - ); + if (options.apk !== null) { + const decoded = await run("decode_android_resources", { + path: options.apk, + include_strings: true, + }); + if (decoded !== null) { + const bytes = await stat(options.apk).then((info) => info.size); + const digest = createHash("sha256") + .update(await readFile(options.apk)) + .digest("hex"); + check( + "decoded target identity matches the file on disk", + decoded.target.bytes === bytes && decoded.target.sha256 === digest, + `${bytes} bytes`, + ); + check( + "metadata carries version and SDK facts", + typeof decoded.metadata.version_name === "string" && + decoded.metadata.min_sdk_version !== null && + decoded.metadata.target_sdk_version !== null, + `version=${String(decoded.metadata.version_name)} sdk=${String(decoded.metadata.min_sdk_version)}..${String(decoded.metadata.target_sdk_version)}`, + ); + check( + "decoded manifest names the package", + typeof decoded.metadata.package_name === "string" && + decoded.manifest !== null && + decoded.manifest.includes(decoded.metadata.package_name), + String(decoded.metadata.package_name), + ); + check( + "string resources projected", + decoded.strings.length > 0, + `${decoded.strings.length} string(s)`, + ); + if (decoded.locales.length > 0) { + const locale = await run("decode_android_resources", { + path: options.apk, + include_strings: true, + locale: decoded.locales[0], + }); + if (locale !== null) + check( + `locale table projected for ${String(decoded.locales[0])}`, + Array.isArray(locale.strings), + `${locale.strings.length} string(s)`, + ); + } } } -} - -try { - await provider.close(); -} catch { - // The provider owns per-call workspaces only; closing is best-effort. +} catch (cause) { + failures += 1; + process.stdout.write( + `[FAIL] verifier execution — ${cause instanceof Error ? cause.message : String(cause)}\n`, + ); +} finally { + try { + await provider.close(); + } catch (cause) { + failures += 1; + process.stdout.write( + `[FAIL] provider cleanup — ${cause instanceof Error ? cause.message : String(cause)}\n`, + ); + } } if (failures > 0) { @@ -116,7 +124,12 @@ function parseArguments(argv) { const argument = argv[index]; if (argument === "--help" || argument === "-h") parsed.help = true; else if (argument === "--apk") { - parsed.apk = argv[index + 1] ?? null; + const path = argv[index + 1]; + if (path === undefined) { + process.stderr.write("--apk requires a local APK path\n"); + process.exit(2); + } + parsed.apk = path; index += 1; } else { process.stderr.write(`unknown argument: ${argument}\n`); diff --git a/src/android/JadxAnalysis.ts b/src/android/JadxAnalysis.ts index 68fa3d039..d8afed20c 100644 --- a/src/android/JadxAnalysis.ts +++ b/src/android/JadxAnalysis.ts @@ -167,16 +167,26 @@ const searchClasses = async ( "JADX class pagination is incomplete", ); } while (classes.length < total); - if (new Set(classes).size !== classes.length || total !== loaded.class_count) + if (total !== loaded.class_count) throw new AnalysisOutputError( request.operation, - "JADX class inventory is duplicated or differs from the loaded class count", + "JADX class inventory count differs from the loaded class count", ); + const occurrences = new Map(); + for (const name of classes) + occurrences.set(name, (occurrences.get(name) ?? 0) + 1); + const matchingNames = classes.filter((name) => + name.includes(request.input.query), + ); + const duplicateNames = [...occurrences] + .filter(([name, count]) => count > 1 && name.includes(request.input.query)) + .map(([name, count]) => ({ name, occurrences: count })); return androidResultSchemas.search_android_classes.parse({ engine, query: request.input.query, total_classes: total, - matches: classes.filter((name) => name.includes(request.input.query)), + matches: matchingNames, + duplicate_names: duplicateNames, coverage: "complete", }); }; diff --git a/src/apktool/ApktoolDecodeOutput.test.ts b/src/apktool/ApktoolDecodeOutput.test.ts index 067f5ea04..fb258b221 100644 --- a/src/apktool/ApktoolDecodeOutput.test.ts +++ b/src/apktool/ApktoolDecodeOutput.test.ts @@ -26,7 +26,6 @@ const REAL_YML = [ " ids:", " - 1", " tag: null", - "version: 2.7.0-dirty", "versionInfo:", " versionCode: '7'", " versionName: 1.2.3", @@ -44,7 +43,11 @@ describe("parseApktoolYml", () => { }); it("accepts quoted version values some builds emit", () => { - expect(parseApktoolYml("versionName: '2.0.0'\nversionCode: 45\n")).toEqual({ + expect( + parseApktoolYml( + '!!brut.androlib.meta.MetaInfo\nversionInfo:\n versionName: "2.0.0"\n versionCode: 45\n', + ), + ).toEqual({ versionName: "2.0.0", versionCode: "45", minSdkVersion: null, @@ -52,6 +55,49 @@ describe("parseApktoolYml", () => { }); }); + it("reads only the producer's nested fields and preserves scalar spelling", () => { + expect( + parseApktoolYml( + [ + "!!brut.androlib.meta.MetaInfo", + "versionName: decoy", + "versionInfo:", + " versionName: 01.2.3", + " versionCode: 045", + " custom:", + " versionName: unrelated", + "sdkInfo:", + " minSdkVersion: 24", + " targetSdkVersion: '35'", + "", + ].join("\n"), + ), + ).toEqual({ + versionName: "01.2.3", + versionCode: "045", + minSdkVersion: "24", + targetSdkVersion: "35", + }); + }); + + it("distinguishes YAML nulls from quoted null strings in flow mappings", () => { + expect( + parseApktoolYml( + [ + "!!brut.androlib.meta.MetaInfo", + 'versionInfo: { versionName: "null", versionCode: 0045, nested: { versionName: decoy } }', + "sdkInfo: { minSdkVersion: null, targetSdkVersion: ~ }", + "", + ].join("\n"), + ), + ).toEqual({ + versionName: "null", + versionCode: "0045", + minSdkVersion: null, + targetSdkVersion: null, + }); + }); + it("reports null facts for unrelated YAML", () => { expect(parseApktoolYml("version: 2.7.0\n")).toEqual({ versionName: null, @@ -66,7 +112,15 @@ describe("parseManifestPackage", () => { it("reads the package attribute from a decoded manifest", () => { const manifest = ''; - expect(parseManifestPackage(manifest)).toBe("com.rea.apktool.probe"); + expect(parseManifestPackage(`${manifest}`)).toBe( + "com.rea.apktool.probe", + ); + }); + + it("reads package with reordered or single-quoted XML attributes", () => { + expect( + parseManifestPackage(""), + ).toBe("org.rea.app"); }); it("reports null when no package attribute exists", () => { @@ -111,4 +165,78 @@ describe("parseDecodedStrings", () => { "a & b <c>", ); }); + + it("keeps empty, self-closing, and reordered string attributes", () => { + expect( + parseDecodedStrings( + '', + ), + ).toEqual({ + entries: [ + { name: "empty", value: "" }, + { name: "paired", value: "" }, + ], + unparsedCount: 0, + }); + }); + + it("ignores comments without mistaking them for nested markup", () => { + expect( + parseDecodedStrings( + 'beforeafter', + ), + ).toEqual({ + entries: [{ name: "real", value: "beforeafter" }], + unparsedCount: 0, + }); + }); + + it("does not include a trailing document comment in the last string", () => { + expect( + parseDecodedStrings( + 'one', + ), + ).toEqual({ + entries: [{ name: "a", value: "one" }], + unparsedCount: 0, + }); + }); + + it("preserves source whitespace and entity spelling", () => { + expect( + parseDecodedStrings( + ' a & b ', + ).entries, + ).toEqual([{ name: "raw", value: " a & b " }]); + }); + + it("keeps source slices aligned across CRLF and CR line endings", () => { + for (const newline of ["\r\n", "\r"]) { + expect( + parseDecodedStrings( + `${newline} x & y${newline}`, + ), + ).toEqual({ + entries: [{ name: "raw", value: "x & y" }], + unparsedCount: 0, + }); + } + }); + + it("counts a string with no name as unsupported instead of emitting an empty name", () => { + expect( + parseDecodedStrings("value"), + ).toEqual({ entries: [], unparsedCount: 1 }); + }); + + it("reports malformed XML and unsupported string markup as partial", () => { + expect( + parseDecodedStrings("no"), + ).toEqual({ entries: [], unparsedCount: 1 }); + expect( + parseDecodedStrings( + 'Hello world', + ), + ).toEqual({ entries: [], unparsedCount: 1 }); + }); }); diff --git a/src/apktool/ApktoolDecodeOutput.ts b/src/apktool/ApktoolDecodeOutput.ts index d00fb7b0e..05bb54acb 100644 --- a/src/apktool/ApktoolDecodeOutput.ts +++ b/src/apktool/ApktoolDecodeOutput.ts @@ -1,11 +1,17 @@ -/** - * Parsers for the decoded workspace `apktool d --no-src` produces. - * - * Apktool's apktool.yml is a small YAML document; the facts REA projects - * are flat enough that line-oriented extraction stays honest without a - * YAML dependency, and unrecognized content is ignored rather than guessed. - */ +import { + DOMParser, + type Document, + type Element, + type Node, +} from "@xmldom/xmldom"; +import { + isMap, + isScalar, + parseDocument, + type Document as YamlDocument, +} from "yaml"; +/** Project only the Apktool metadata fields REA publishes. */ export interface ApktoolMetadata { readonly versionName: string | null; readonly versionCode: string | null; @@ -13,43 +19,48 @@ export interface ApktoolMetadata { readonly targetSdkVersion: string | null; } -/** - * Project version and SDK facts from apktool.yml. Values keep apktool's - * own spelling (version codes can print as `45` or `45.1`). - */ -export const parseApktoolYml = (yml: string): ApktoolMetadata => { - const metadata: { - versionName: string | null; - versionCode: string | null; - minSdkVersion: string | null; - targetSdkVersion: string | null; - } = { - versionName: null, - versionCode: null, - minSdkVersion: null, - targetSdkVersion: null, - }; - for (const line of yml.split(/\r?\n/u)) { - const versionName = /^\s*versionName:\s*(?:'([^']*)'|(.*))$/u.exec(line); - if (versionName !== null && metadata.versionName === null) - metadata.versionName = (versionName[1] ?? versionName[2] ?? "").trim(); - const versionCode = /^\s*versionCode:\s*(?:'([^']*)'|(.*))$/u.exec(line); - if (versionCode !== null && metadata.versionCode === null) - metadata.versionCode = (versionCode[1] ?? versionCode[2] ?? "").trim(); - const minSdk = /minSdkVersion:\s*'?([0-9]+)'?/u.exec(line); - if (minSdk !== null && metadata.minSdkVersion === null) - metadata.minSdkVersion = minSdk[1]!; - const targetSdk = /targetSdkVersion:\s*'?([0-9]+)'?/u.exec(line); - if (targetSdk !== null && metadata.targetSdkVersion === null) - metadata.targetSdkVersion = targetSdk[1]!; - } - return metadata; +const metaInfoTag = { + tag: "tag:yaml.org,2002:brut.androlib.meta.MetaInfo", + collection: "map" as const, + resolve: (map: unknown) => map, }; -/** Read the package attribute from decoded AndroidManifest.xml text. */ +/** + * Read values from Apktool's tagged metadata document. YAML's core schema + * distinguishes null from quoted strings; numeric values use their source + * range so version codes retain the spelling Apktool emitted. + */ +export const parseApktoolYml = (yml: string): ApktoolMetadata => { + const document = parseDocument(yml, { + schema: "core", + customTags: [metaInfoTag], + strict: true, + uniqueKeys: true, + }); + if (document.errors.length > 0) + throw new TypeError( + `apktool.yml is not valid Apktool metadata: ${document.errors[0]?.message ?? "YAML parse error"}`, + ); + if (!isMap(document.contents)) + throw new TypeError("apktool.yml root must be an Apktool metadata mapping"); + return { + versionName: yamlScalar(document, ["versionInfo", "versionName"], yml), + versionCode: yamlScalar(document, ["versionInfo", "versionCode"], yml), + minSdkVersion: yamlScalar(document, ["sdkInfo", "minSdkVersion"], yml), + targetSdkVersion: yamlScalar( + document, + ["sdkInfo", "targetSdkVersion"], + yml, + ), + }; +}; + +/** Read the package attribute from the decoded Android manifest root. */ export const parseManifestPackage = (manifest: string): string | null => { - const match = /]*\spackage="([^"]+)"/u.exec(manifest); - return match === null ? null : match[1]!; + const document = parseXml(manifest, "AndroidManifest.xml"); + const root = document.documentElement; + if (root === null || root.nodeName !== "manifest") return null; + return root.getAttribute("package") || null; }; export interface DecodedStringEntry { @@ -58,10 +69,10 @@ export interface DecodedStringEntry { } /** - * Project `value` entries from one decoded - * strings.xml. Values are preserved exactly as apktool printed them — - * escaped entities stay escaped — and entries containing nested markup - * are counted as unparsed rather than flattened. + * Project direct `` children from decoded resources XML. DOM parsing + * identifies elements and attributes; text values are sliced from the input + * at parser-reported locations so entity spelling and whitespace are kept. + * Markup-bearing string values are omitted and counted as partial evidence. */ export const parseDecodedStrings = ( stringsXml: string, @@ -69,20 +80,107 @@ export const parseDecodedStrings = ( entries: readonly DecodedStringEntry[]; unparsedCount: number; } => { + let document: Document; + try { + document = parseXml(stringsXml, "strings.xml"); + } catch { + return { entries: [], unparsedCount: 1 }; + } + const root = document.documentElement; + if (root === null || root.nodeName !== "resources") + return { entries: [], unparsedCount: 1 }; + const entries: DecodedStringEntry[] = []; let unparsedCount = 0; - const pattern = /]*>([\s\S]*?)<\/string>/gu; - for ( - let match = pattern.exec(stringsXml); - match !== null; - match = pattern.exec(stringsXml) - ) { - const value = match[2]!; - if (value.includes("<")) { + const offsets = sourceLineOffsets(stringsXml); + for (const child of childNodes(root)) { + if (child.nodeType !== 1) continue; + const element = child as Element; + if (element.nodeName !== "string") continue; + const name = element.getAttribute("name"); + if (name === null || name.length === 0) { unparsedCount += 1; continue; } - entries.push({ name: match[1]!, value: value.trim() }); + const content = childNodes(element); + if (content.some((node) => node.nodeType !== 3 && node.nodeType !== 8)) { + unparsedCount += 1; + continue; + } + if (content.every((node) => node.nodeType === 8)) { + entries.push({ name, value: "" }); + continue; + } + const value = content + .filter((node) => node.nodeType === 3) + .map((node) => { + const start = sourceOffset(node, offsets); + if (start === undefined) return null; + // A literal '<' cannot appear in well-formed XML text. Ending here + // also excludes comments after the document element from this value. + const end = stringsXml.indexOf("<", start); + if (end < start) return null; + return stringsXml.slice(start, end); + }); + if (value.some((part) => part === null)) { + unparsedCount += 1; + continue; + } + entries.push({ name, value: value.join("") }); } return { entries, unparsedCount }; }; + +const yamlScalar = ( + document: YamlDocument, + path: readonly string[], + source: string, +): string | null => { + const node = document.getIn(path, true); + if (!isScalar(node) || node.value === null) return null; + if (typeof node.value === "string") return node.value; + const range = node.range; + return range === null || range === undefined + ? String(node.value) + : source.slice(range[0], range[1]); +}; + +const parseXml = (xml: string, label: string) => { + const errors: string[] = []; + const document = new DOMParser({ + normalizeLineEndings: (source) => source, + onError: (_level, message) => errors.push(message), + }).parseFromString(xml, "application/xml"); + if (errors.length > 0 || document.documentElement === null) + throw new TypeError( + `${label} is not well-formed XML${errors[0] === undefined ? "" : `: ${errors[0]}`}`, + ); + return document; +}; + +const childNodes = (node: Node): Node[] => { + const children: Node[] = []; + for (let child = node.firstChild; child !== null; child = child.nextSibling) + children.push(child); + return children; +}; + +const sourceLineOffsets = (source: string): readonly number[] => { + const offsets = [0]; + for (const match of source.matchAll(/\r\n?|\n/gu)) { + const index = match.index; + offsets.push(index + match[0].length); + } + return offsets; +}; + +const sourceOffset = ( + node: Node, + lineOffsets: readonly number[], +): number | undefined => { + const line = node.lineNumber; + const column = node.columnNumber; + if (line === undefined || column === undefined) return undefined; + const lineStart = lineOffsets[line - 1]; + return lineStart === undefined ? undefined : lineStart + column - 1; +}; diff --git a/src/apktool/ApktoolProvider.test.ts b/src/apktool/ApktoolProvider.test.ts index 23e2bd8b4..3cd86b72a 100644 --- a/src/apktool/ApktoolProvider.test.ts +++ b/src/apktool/ApktoolProvider.test.ts @@ -89,10 +89,12 @@ const decodeStub = ( readonly withStrings?: boolean; readonly withGerman?: boolean; readonly failDecode?: boolean; + readonly stringsXml?: string; } = {}, ): ((root: string) => string) => { const withStrings = options.withStrings ?? true; const withGerman = options.withGerman ?? true; + const stringsXml = options.stringsXml ?? REAL_STRINGS; return () => `if [ "$1" = "--version" ]; then ${VERSION_BLOCK}; exit 0; fi if [ "$1" = "d" ]; then ${ @@ -101,7 +103,7 @@ if [ "$1" = "d" ]; then : `printf '%s\\n' '${REAL_YML.replace(/'/gu, "'\\''")}' > "$5/apktool.yml" printf '%s\\n' '${REAL_MANIFEST.replace(/'/gu, "'\\''")}' > "$5/AndroidManifest.xml" mkdir -p "$5/res/values"${withGerman ? ' "$5/res/values-de"' : ""} - ${withStrings ? `printf '%s\\n' '${REAL_STRINGS.replace(/'/gu, "'\\''")}' > "$5/res/values/strings.xml"` : ""} + ${withStrings ? `printf '%s\\n' '${stringsXml.replace(/'/gu, "'\\''")}' > "$5/res/values/strings.xml"` : ""} ${withGerman ? `printf '%s\\n' '${DE_STRINGS.replace(/'/gu, "'\\''")}' > "$5/res/values-de/strings.xml"` : ""} printf 'I: Using Apktool on stub\\n' exit 0` @@ -173,6 +175,22 @@ it("decodes resources with target identity, metadata, and strings", async () => expect(result.coverage).toBe("complete"); }); +it("preserves empty resource strings and reports markup omissions as partial", async () => { + const stringsXml = + ' A & B Hello world'; + const { command, root } = await writeApktoolStub(decodeStub({ stringsXml })); + const apk = await apkFixture(root); + const result = await evidenceResult(providerOver(command), { + operation: "decode_android_resources", + input: { path: apk.path, include_strings: true }, + }); + expect(result.strings).toEqual([ + { name: "empty", value: "" }, + { name: "raw", value: " A & B " }, + ]); + expect(result.coverage).toBe("partial"); +}); + it("projects one locale's strings on request", async () => { const { command, root } = await writeApktoolStub(decodeStub()); const apk = await apkFixture(root); diff --git a/src/apktool/ApktoolProvider.ts b/src/apktool/ApktoolProvider.ts index 8efb17554..699bccb9a 100644 --- a/src/apktool/ApktoolProvider.ts +++ b/src/apktool/ApktoolProvider.ts @@ -717,7 +717,7 @@ export class ApktoolProvider { : []), ...(parsedStrings.unparsedCount > 0 ? [ - `${String(parsedStrings.unparsedCount)} string entries with nested markup were not projected`, + `String entries omitted due to malformed XML or unsupported markup: ${String(parsedStrings.unparsedCount)}`, ] : []), ...(parsedStrings.entries.length > MAX_STRINGS diff --git a/src/contracts/android/androidToolContracts.ts b/src/contracts/android/androidToolContracts.ts index e20b7261b..df2cccf59 100644 --- a/src/contracts/android/androidToolContracts.ts +++ b/src/contracts/android/androidToolContracts.ts @@ -26,7 +26,7 @@ export const ANDROID_TOOL_CONTRACTS = [ name: "search_android_classes", ...toolContractMetadata("search_android_classes"), description: - "Search complete decompiler class names in an explicit local APK by case-sensitive substring. Empty query inventories all classes. Retains artifact and engine identity; no APK execution.", + "Search complete decompiler class-name rows in an explicit local APK by case-sensitive substring. Empty query inventories all rows. Repeated names remain repeated and appear in duplicate_names; the provider accepts only class names for lookup, so these rows do not identify which same-name class node a later lookup selects. Retains artifact and engine identity; no APK execution.", kind: "android-provider", inputSchema: androidInputSchemas.search_android_classes, outputSchema: evidenceResultOf(androidResultSchemas.search_android_classes), diff --git a/src/domain/android/androidAnalysis.ts b/src/domain/android/androidAnalysis.ts index a692001d9..e27be2b20 100644 --- a/src/domain/android/androidAnalysis.ts +++ b/src/domain/android/androidAnalysis.ts @@ -121,6 +121,12 @@ export const androidResultSchemas = { query: z.string(), total_classes: z.number().int().nonnegative(), matches: z.array(z.string()), + duplicate_names: z.array( + z.strictObject({ + name: z.string(), + occurrences: z.number().int().min(2), + }), + ), coverage: z.literal("complete"), }), inspect_android_class: z.strictObject({ diff --git a/tests/boundary/process/jadxIntegration.test.ts b/tests/boundary/process/jadxIntegration.test.ts index c377a7cfb..768438c0c 100644 --- a/tests/boundary/process/jadxIntegration.test.ts +++ b/tests/boundary/process/jadxIntegration.test.ts @@ -277,6 +277,27 @@ it("fetches all class pages rather than silently retaining the first upstream pa await verifyCleanup(launches); }); +it("preserves duplicate class-name rows and reports lookup ambiguity", async () => { + const { service, apk, launches, provider } = await setup( + "duplicate-class-name", + ); + const outcome = await service.execute("search_android_classes", { + path: apk, + query: "fixture.Target", + }); + if (!outcome.ok) throw outcome.error; + const result = androidResultSchemas.search_android_classes.parse( + outcome.value.normalized_result, + ); + expect(result.total_classes).toBe(402); + expect(result.matches).toEqual(["fixture.Target", "fixture.Target"]); + expect(result.duplicate_names).toEqual([ + { name: "fixture.Target", occurrences: 2 }, + ]); + await provider.close(); + await verifyCleanup(launches); +}); + it("exposes overload candidates and executes only an explicitly selected ambiguous method", async () => { const { service, apk, launches, provider } = await setup(); const input = { diff --git a/tests/fixtures/android/jadx-mcp.mjs b/tests/fixtures/android/jadx-mcp.mjs index 4c008a7f4..212cf4f5b 100644 --- a/tests/fixtures/android/jadx-mcp.mjs +++ b/tests/fixtures/android/jadx-mcp.mjs @@ -26,6 +26,7 @@ const classes = [ { length: generatedCount }, (_, index) => `fixture.Class${index}`, ), + ...(mode === "duplicate-class-name" ? ["fixture.Target"] : []), ]; const methods = [ {