diff --git a/src/apktool/ApktoolProvider.test.ts b/src/apktool/ApktoolProvider.test.ts index ed09cf630..dfafdc107 100644 --- a/src/apktool/ApktoolProvider.test.ts +++ b/src/apktool/ApktoolProvider.test.ts @@ -168,6 +168,74 @@ const apkFixture = async ( }; }; +it.each([ + [false, false], + [true, false], + [false, true], + [true, true], +])( + "retains completed decode facts when a later locale read fails (cleanup denied: %s, late cancellation: %s)", + async (denyCleanup, lateCancellation) => { + const { command, root } = await writeApktoolStub((path) => + decodeStub()(path).replace( + " printf 'I: Using Apktool on stub", + " printf '\\377' > \"$5/res/values-de/strings.xml\"\n printf 'I: Using Apktool on stub", + ), + ); + const apk = await apkFixture(root); + let denied = denyCleanup; + const controller = new AbortController(); + const provider = new ApktoolProvider({ + environment: { REA_APKTOOL_COMMAND: command }, + removeWorkspace: async (path) => { + if (lateCancellation) controller.abort({ stage: "cleanup" }); + if (denied) throw new Error("authored removal denial"); + await rm(path, { recursive: true, force: true }); + }, + }); + try { + const result = await provider.execute( + { + operation: "decode_android_resources", + input: { path: apk.path, include_strings: true, locales: ["de"] }, + }, + { signal: controller.signal }, + ); + if (result.ok) throw new Error("Expected invalid UTF-8 failure"); + expect((result.error.cleanupPrimary ?? result.error).message).toContain( + "UTF-8", + ); + expect(result.error.cleanupIncomplete).toBe(denyCleanup); + const observation = { + operation: "decode_android_resources", + parameters: { path: apk.path, locales: ["de"] }, + subject: { local_path: apk.path, digest: { sha256: apk.sha256 } }, + normalized_result: { + metadata: { package_name: "com.rea.apktool.probe" }, + manifest: `${REAL_MANIFEST}\n`, + strings: [ + { name: "api_endpoint", value: "https://api.example.com/v1" }, + { name: "app_name", value: "ApktoolProbe" }, + ], + string_tables: [{ locale: null, status: "present", total_count: 2 }], + coverage: "partial", + }, + }; + expect(result.error.partialObservation).toMatchObject(observation); + expect(projectAnalysisError(result.error)).toMatchObject({ + details: { partial_observation: observation }, + }); + denied = false; + await provider.close(); + expect(result.error.cleanupIncomplete).toBe(false); + expect(result.error.partialObservation).toMatchObject(observation); + } finally { + denied = false; + await provider.close(); + } + }, +); + it("inspects the apktool launcher", async () => { const { command } = await writeApktoolStub( () => `if [ "$1" = "--version" ]; then ${VERSION_BLOCK}; exit 0; fi`, @@ -1077,11 +1145,23 @@ it.each(["decode", "projection"] as const)( expect(result.error.capturedOutput?.stderr).toContain( "Could not decode the APK", ); - expect(result.error.partialObservation).toMatchObject({ - kind: "file-snapshot-hash", - path: apk.path, - sha256: apk.sha256, - }); + expect(result.error.partialObservation).toMatchObject( + stage === "decode" + ? { + kind: "file-snapshot-hash", + path: apk.path, + sha256: apk.sha256, + } + : { + operation: "decode_android_resources", + subject: { local_path: apk.path, digest: { sha256: apk.sha256 } }, + normalized_result: { + coverage: "partial", + metadata: { package_name: "com.rea.apktool.probe" }, + strings: [], + }, + }, + ); } finally { await provider.close(); } diff --git a/src/apktool/ApktoolProvider.ts b/src/apktool/ApktoolProvider.ts index c45aedc5e..18a28c9d0 100644 --- a/src/apktool/ApktoolProvider.ts +++ b/src/apktool/ApktoolProvider.ts @@ -522,6 +522,7 @@ export class ApktoolProvider { const owner = { kind: "lease" as const, resource: root, lease }; this.#artifacts.retain(owner); let result: Result; + let partialExecution: AnalysisExecution | undefined; try { const identity = await resources.snapshotInput( input.path, @@ -591,12 +592,15 @@ export class ApktoolProvider { ); throwIfCancelled(operation, signal); - const projectedStrings = await projectApktoolStringTables( + const projection = await projectApktoolStringTables( resources, workspace, input, signal, ); + const projectedStrings = projection.ok + ? projection.value + : projection.error.partial; limitations.push(...projectedStrings.limitations); const coverage = projectedStrings.coverage === "partial" || @@ -626,35 +630,38 @@ export class ApktoolProvider { `Decoded workspace enumeration stopped after ${String(MAX_DECODED_ENTRIES_VISITED)} entries; decoded_file_count and locales may be partial`, ); } - result = ok( - createAnalysisExecution( - { - client, - target: { path: input.path, bytes: identity.bytes, sha256 }, - metadata: { - version_name: metadata.versionName, - version_code: metadata.versionCode, - min_sdk_version: metadata.minSdkVersion, - target_sdk_version: metadata.targetSdkVersion, - package_name: packageName, - }, - manifest, - strings: projectedStrings.strings, - locale: input.locale ?? null, - locales: files.locales, - ...(input.locales === undefined - ? {} - : { string_tables: projectedStrings.tables }), - decoded_file_count: files.count, - coverage, + const execution = createAnalysisExecution( + { + client, + target: { path: input.path, bytes: identity.bytes, sha256 }, + metadata: { + version_name: metadata.versionName, + version_code: metadata.versionCode, + min_sdk_version: metadata.minSdkVersion, + target_sdk_version: metadata.targetSdkVersion, + package_name: packageName, }, - provider, - { - subject: { path: input.path, sha256, format: "apk" }, - limitations, - }, - ), + manifest, + strings: projectedStrings.strings, + locale: input.locale ?? null, + locales: files.locales, + ...(input.locales === undefined + ? {} + : { string_tables: projectedStrings.tables }), + decoded_file_count: files.count, + coverage, + }, + provider, + { + subject: { path: input.path, sha256, format: "apk" }, + limitations, + }, ); + if (!projection.ok) { + partialExecution = execution; + throw projection.error.cause; + } + result = ok(execution); } catch (cause) { const physical = artifactFilePrimaryFailure(cause); const primary = @@ -689,6 +696,18 @@ export class ApktoolProvider { operation, ).bindCleanupDependencies([primary, failure]) : primary; + if ( + partialExecution !== undefined && + retained.partialObservation === undefined + ) + retained.retainPartialObservation( + createAnalysisExecutionEvidence( + operation, + input, + partialExecution, + "observed", + ), + ); if ( resources.snapshot !== undefined && retained.partialObservation === undefined diff --git a/src/apktool/ApktoolStringTables.test.ts b/src/apktool/ApktoolStringTables.test.ts index bd8c8cb18..580058593 100644 --- a/src/apktool/ApktoolStringTables.test.ts +++ b/src/apktool/ApktoolStringTables.test.ts @@ -21,7 +21,7 @@ const textRead = (text: string) => ({ describe("physical Apktool string facts", () => { it("reads one physical table for two caller-selected aliases", async () => { const paths: string[] = []; - const result = await projectApktoolStringTables( + const outcome = await projectApktoolStringTables( { async readText(path) { paths.push(path); @@ -31,6 +31,8 @@ describe("physical Apktool string facts", () => { "decoded", { include_strings: true, locale: "pt-BR", locales: ["pt-rBR"] }, ); + if (!outcome.ok) throw outcome.error.cause; + const result = outcome.value; expect(paths).toEqual([ join("decoded", "res", "values-pt-rBR", "strings.xml"), ]); @@ -42,7 +44,7 @@ describe("physical Apktool string facts", () => { }); it("counts complete source entries beyond the retained table prefix", async () => { - const result = await projectApktoolStringTables( + const outcome = await projectApktoolStringTables( { async readText() { return textRead( @@ -56,6 +58,8 @@ describe("physical Apktool string facts", () => { "decoded", { include_strings: true }, ); + if (!outcome.ok) throw outcome.error.cause; + const result = outcome.value; expect(result.tables[0]).toMatchObject({ total_count: MAX_STRINGS + 1, unparsed_count: 1, @@ -69,7 +73,7 @@ describe("physical Apktool string facts", () => { it("does not read unexamined tables after output exhaustion", async () => { const paths: string[] = []; - const result = await projectApktoolStringTables( + const outcome = await projectApktoolStringTables( { async readText(path) { paths.push(path); @@ -81,6 +85,8 @@ describe("physical Apktool string facts", () => { "decoded", { include_strings: true, locales: ["de", "es", "it", "fr"] }, ); + if (!outcome.ok) throw outcome.error.cause; + const result = outcome.value; expect(paths).toHaveLength(4); expect(result.tables.at(-1)).toEqual({ locale: "fr", @@ -98,7 +104,7 @@ describe("physical Apktool string facts", () => { it("keeps a known empty alias present after the output budget is spent", async () => { const paths: string[] = []; - const result = await projectApktoolStringTables( + const outcome = await projectApktoolStringTables( { async readText(path) { paths.push(path); @@ -114,6 +120,8 @@ describe("physical Apktool string facts", () => { locales: ["de", "es", "it", "fr", "en", "pt-rBR"], }, ); + if (!outcome.ok) throw outcome.error.cause; + const result = outcome.value; expect(paths.filter((path) => path.includes("values-pt-rBR"))).toHaveLength( 1, ); @@ -130,8 +138,37 @@ describe("physical Apktool string facts", () => { }); describe("Apktool string projection admission", () => { - it("honors omitted strings without reading any selected table", async () => { + it("retains completed tables when cancellation arrives before the next selection", async () => { + const controller = new AbortController(); + const reason = { selected: "caller" }; + const paths: string[] = []; const result = await projectApktoolStringTables( + { + async readText(path) { + paths.push(path); + controller.abort(reason); + return textRead(xml(1)); + }, + }, + "decoded", + { include_strings: true, locales: ["de", "fr"] }, + controller.signal, + ); + if (result.ok) throw new Error("Expected cancellation"); + expect(result.error.cause).toBeInstanceOf(AnalysisCancelledError); + expect(result.error.cause).toHaveProperty("cause", reason); + expect(paths).toHaveLength(1); + expect(result.error.partial).toMatchObject({ + coverage: "partial", + tables: [{ locale: null, total_count: 1 }], + }); + expect(result.error.partial.limitations).toContain( + 'String projection stopped at locale "de"; this selection and 1 remaining selections were not projected', + ); + }); + + it("honors omitted strings without reading any selected table", async () => { + const outcome = await projectApktoolStringTables( { async readText() { throw new Error("Unexpected read"); @@ -140,27 +177,26 @@ describe("Apktool string projection admission", () => { "decoded", { include_strings: false, locale: "de", locales: ["fr"] }, ); - expect(result).toEqual({ - strings: [], - tables: [], - limitations: [], - coverage: "complete", + expect(outcome).toEqual({ + ok: true, + value: { strings: [], tables: [], limitations: [], coverage: "complete" }, }); }); it("preserves the exact physical read failure", async () => { const cause = new Error("Physical read and close failed"); - await expect( - projectApktoolStringTables( - { - async readText() { - throw cause; - }, + const result = await projectApktoolStringTables( + { + async readText() { + throw cause; }, - "decoded", - { include_strings: true }, - ), - ).rejects.toBe(cause); + }, + "decoded", + { include_strings: true }, + ); + if (result.ok) throw new Error("Expected read failure"); + expect(result.error.cause).toBe(cause); + expect(result.error.partial.coverage).toBe("partial"); }); it("preserves opaque cancellation before any table read", async () => { @@ -175,7 +211,9 @@ describe("Apktool string projection admission", () => { { include_strings: true }, AbortSignal.abort(reason), ); - await expect(pending).rejects.toBeInstanceOf(AnalysisCancelledError); - await expect(pending).rejects.toHaveProperty("cause", reason); + const result = await pending; + if (result.ok) throw new Error("Expected cancellation"); + expect(result.error.cause).toBeInstanceOf(AnalysisCancelledError); + expect(result.error.cause).toHaveProperty("cause", reason); }); }); diff --git a/src/apktool/ApktoolStringTables.ts b/src/apktool/ApktoolStringTables.ts index 95966dfec..d9c8fb0a2 100644 --- a/src/apktool/ApktoolStringTables.ts +++ b/src/apktool/ApktoolStringTables.ts @@ -1,4 +1,5 @@ import { join } from "node:path"; +import { ok, err, type Result } from "../domain/result.js"; import { AGGREGATE_STRINGS_XML_MAX_BYTES, @@ -124,9 +125,19 @@ export const projectApktoolStringTables = async ( workspace: string, input: Selection, signal?: AbortSignal, -): Promise => { +): Promise< + Result< + ApktoolStringProjection, + { readonly cause: unknown; readonly partial: ApktoolStringProjection } + > +> => { if (!input.include_strings) - return { strings: [], tables: [], limitations: [], coverage: "complete" }; + return ok({ + strings: [], + tables: [], + limitations: [], + coverage: "complete", + }); const primary = input.locale ?? null; const locales = [...new Set([primary, ...(input.locales ?? [])])]; const physical = new Map(); @@ -135,118 +146,130 @@ export const projectApktoolStringTables = async ( let xmlBytes = 0; let outputEntries = 0; let aggregateCapped = false; - for (const [index, locale] of locales.entries()) { - if (signal?.aborted) - throw new AnalysisCancelledError("decode_android_resources", { - cause: signal.reason, - }); - const directory = - locale === null - ? "values" - : `values-${androidLocaleDirectorySuffix(locale)}`; - const label = - locale === null - ? "default res/values/strings.xml" - : `res/${directory}/strings.xml`; - let fact = physical.get(directory); - if (fact === undefined) { - const budget = Math.min( - STRINGS_XML_MAX_BYTES, - AGGREGATE_STRINGS_XML_MAX_BYTES - xmlBytes, + const projection = (): ApktoolStringProjection => { + if (aggregateCapped) + limitations.add( + "Aggregate string table projection stopped due to resource limits", ); - if (budget <= 0 || outputEntries >= MAX_AGGREGATE_STRINGS) { + return { + strings: tables[0]?.strings ?? [], + tables, + limitations: [...limitations], + coverage: tables.some((table) => table.coverage === "partial") + ? "partial" + : "complete", + }; + }; + let pendingIndex = 0; + try { + for (const [index, locale] of locales.entries()) { + pendingIndex = index; + if (signal?.aborted) + throw new AnalysisCancelledError("decode_android_resources", { + cause: signal.reason, + }); + const directory = + locale === null + ? "values" + : `values-${androidLocaleDirectorySuffix(locale)}`; + const label = + locale === null + ? "default res/values/strings.xml" + : `res/${directory}/strings.xml`; + let fact = physical.get(directory); + if (fact === undefined) { + const budget = Math.min( + STRINGS_XML_MAX_BYTES, + AGGREGATE_STRINGS_XML_MAX_BYTES - xmlBytes, + ); + if (budget <= 0 || outputEntries >= MAX_AGGREGATE_STRINGS) { + tables.push({ + locale, + status: "skipped", + strings: [], + total_count: null, + unparsed_count: null, + coverage: "partial", + }); + limitations.add( + `Projection for ${label} skipped: aggregate resource limit reached`, + ); + aggregateCapped = true; + continue; + } + const read = await readTable( + resources, + join(workspace, "res", directory, "strings.xml"), + budget, + signal, + ); + fact = read.fact; + xmlBytes += read.bytes; + if (fact.status === "oversized" && budget < STRINGS_XML_MAX_BYTES) + aggregateCapped = true; + physical.set(directory, fact); + } + if (fact.status !== "present") { tables.push({ locale, - status: "skipped", + status: fact.status, strings: [], - total_count: null, - unparsed_count: null, - coverage: "partial", + total_count: fact.totalCount, + unparsed_count: fact.unparsedCount, + coverage: fact.status === "missing" ? "complete" : "partial", }); + limitations.add( + unavailableTableLimitation(fact, directory, locale, input), + ); + continue; + } + // Batch output includes the primary strings twice, in the legacy field and + // in string_tables. Charge that representation before selecting entries. + const copies = index === 0 && input.locales !== undefined ? 2 : 1; + const remaining = Math.max(0, MAX_AGGREGATE_STRINGS - outputEntries); + const selected = fact.entries.slice(0, Math.floor(remaining / copies)); + outputEntries += selected.length * copies; + const skipped = selected.length === 0 && fact.totalCount > 0; + const limited = selected.length < Math.min(fact.totalCount, MAX_STRINGS); + if (limited) aggregateCapped = true; + tables.push({ + locale, + status: skipped ? "skipped" : "present", + strings: selected, + // Output exhaustion cannot erase counts from an already examined table. + total_count: fact.totalCount, + unparsed_count: fact.unparsedCount, + coverage: + selected.length < fact.totalCount || fact.unparsedCount > 0 + ? "partial" + : "complete", + }); + if (skipped) limitations.add( `Projection for ${label} skipped: aggregate resource limit reached`, ); - aggregateCapped = true; - continue; - } - const read = await readTable( - resources, - join(workspace, "res", directory, "strings.xml"), - budget, - signal, - ); - fact = read.fact; - xmlBytes += read.bytes; - if (fact.status === "oversized" && budget < STRINGS_XML_MAX_BYTES) - aggregateCapped = true; - physical.set(directory, fact); + if (fact.unparsedCount > 0) + limitations.add( + input.locales === undefined || locale === null + ? `String entries omitted due to unsupported markup or missing names: ${String(fact.unparsedCount)}` + : `String entries omitted in locale ${locale} due to unsupported markup or missing names: ${String(fact.unparsedCount)}`, + ); + if (limited && !skipped) + limitations.add( + `Locale ${locale ?? "default"} strings reports ${String(selected.length)} entries due to aggregate strings limit`, + ); + else if (fact.totalCount > MAX_STRINGS && !skipped) + limitations.add( + input.locales === undefined || locale === null + ? `strings reports the first ${String(selected.length)} of ${String(fact.totalCount)} entries` + : `Locale ${locale} strings reports the first ${String(selected.length)} of ${String(fact.totalCount)} entries`, + ); } - if (fact.status !== "present") { - tables.push({ - locale, - status: fact.status, - strings: [], - total_count: fact.totalCount, - unparsed_count: fact.unparsedCount, - coverage: fact.status === "missing" ? "complete" : "partial", - }); - limitations.add( - unavailableTableLimitation(fact, directory, locale, input), - ); - continue; - } - // Batch output includes the primary strings twice, in the legacy field and - // in string_tables. Charge that representation before selecting entries. - const copies = index === 0 && input.locales !== undefined ? 2 : 1; - const remaining = Math.max(0, MAX_AGGREGATE_STRINGS - outputEntries); - const selected = fact.entries.slice(0, Math.floor(remaining / copies)); - outputEntries += selected.length * copies; - const skipped = selected.length === 0 && fact.totalCount > 0; - const limited = selected.length < Math.min(fact.totalCount, MAX_STRINGS); - if (limited) aggregateCapped = true; - tables.push({ - locale, - status: skipped ? "skipped" : "present", - strings: selected, - // Output exhaustion cannot erase counts from an already examined table. - total_count: fact.totalCount, - unparsed_count: fact.unparsedCount, - coverage: - selected.length < fact.totalCount || fact.unparsedCount > 0 - ? "partial" - : "complete", - }); - if (skipped) - limitations.add( - `Projection for ${label} skipped: aggregate resource limit reached`, - ); - if (fact.unparsedCount > 0) - limitations.add( - input.locales === undefined || locale === null - ? `String entries omitted due to unsupported markup or missing names: ${String(fact.unparsedCount)}` - : `String entries omitted in locale ${locale} due to unsupported markup or missing names: ${String(fact.unparsedCount)}`, - ); - if (limited && !skipped) - limitations.add( - `Locale ${locale ?? "default"} strings reports ${String(selected.length)} entries due to aggregate strings limit`, - ); - else if (fact.totalCount > MAX_STRINGS && !skipped) - limitations.add( - input.locales === undefined || locale === null - ? `strings reports the first ${String(selected.length)} of ${String(fact.totalCount)} entries` - : `Locale ${locale} strings reports the first ${String(selected.length)} of ${String(fact.totalCount)} entries`, - ); - } - if (aggregateCapped) + } catch (cause) { limitations.add( - "Aggregate string table projection stopped due to resource limits", + `String projection stopped at locale ${JSON.stringify(locales[pendingIndex])}; this selection and ${String(locales.length - pendingIndex - 1)} remaining selections were not projected`, ); - return { - strings: tables[0]?.strings ?? [], - tables, - limitations: [...limitations], - coverage: tables.some((table) => table.coverage === "partial") - ? "partial" - : "complete", - }; + return err({ cause, partial: { ...projection(), coverage: "partial" } }); + } + return ok(projection()); };