diff --git a/src/apktool/ApktoolCommandCleanup.ts b/src/apktool/ApktoolCommandCleanup.ts index f720ef2806..62b3188f84 100644 --- a/src/apktool/ApktoolCommandCleanup.ts +++ b/src/apktool/ApktoolCommandCleanup.ts @@ -2,7 +2,10 @@ import { AnalysisCleanupLease, AnalysisCleanupLeaseGroup, } from "../artifacts/AnalysisCleanupLease.js"; -import { ArtifactResourceScope } from "../artifacts/ArtifactResourceScope.js"; +import { + ArtifactResourceRetryFailure, + ArtifactResourceScope, +} from "../artifacts/ArtifactResourceScope.js"; import { ArtifactReaderFailure } from "../artifacts/ArtifactReader.js"; import { AnalysisError } from "../domain/analysisErrorBase.js"; import { analysisErrorWithCleanupFailure } from "../domain/analysisErrorCleanup.js"; @@ -166,8 +169,13 @@ export class ApktoolCommandCleanup { #retryFailure(cause: unknown, operation: string): unknown { if ( - !(cause instanceof ArtifactReaderFailure) || - cause.cleanup === undefined + !(cause instanceof ArtifactResourceRetryFailure) || + cause.cleanup === undefined || + !cause.failedOwners.every( + (failed) => + failed.kind === "lease" && + [...this.#owners].some((owner) => owner.lease === failed.lease), + ) ) return cause; const owners = [...this.#owners].filter((owner) => !owner.lease.released); diff --git a/src/apktool/ApktoolDecodedInventory.ts b/src/apktool/ApktoolDecodedInventory.ts new file mode 100644 index 0000000000..16b6b91e98 --- /dev/null +++ b/src/apktool/ApktoolDecodedInventory.ts @@ -0,0 +1,40 @@ +import { access } from "node:fs/promises"; +import { join } from "node:path"; +import { DirectoryArtifactTraversalLimit } from "../artifacts/DirectoryArtifactReader.js"; +import { assertArtifactReadActive } from "../artifacts/ArtifactFileOperation.js"; +import { androidLocaleQualifierSchema } from "../domain/apktool/apktoolResourceAnalysis.js"; +import type { ApktoolWorkspace } from "./ApktoolWorkspace.js"; + +/** Count files over bounded examined coverage, including omitted special entries. */ +export const countDecodedFiles = async ( + workspace: ApktoolWorkspace, + root: string, + signal: AbortSignal | undefined, + maximumEntries: number, +): Promise<{ count: number; capped: boolean; locales: readonly string[] }> => { + let count = 0; + let capped = false; + const locales: string[] = []; + try { + for await (const entry of workspace.entries(root, signal, maximumEntries)) { + assertArtifactReadActive(signal); + if (entry.kind === "file") count += 1; + if (entry.kind === "directory") { + const match = /^res\/values-(.+)$/u.exec(entry.path); + if ( + match !== null && + androidLocaleQualifierSchema.safeParse(match[1]).success && + (await access(join(root, entry.path, "strings.xml")).then( + () => true, + () => false, + )) + ) + locales.push(match[1]!); + } + } + } catch (cause) { + if (!(cause instanceof DirectoryArtifactTraversalLimit)) throw cause; + capped = true; + } + return { count, capped, locales: locales.sort() }; +}; diff --git a/src/apktool/ApktoolProvider.test.ts b/src/apktool/ApktoolProvider.test.ts index 938e08dd21..3edff9dcc1 100644 --- a/src/apktool/ApktoolProvider.test.ts +++ b/src/apktool/ApktoolProvider.test.ts @@ -1,14 +1,17 @@ +import { createDeferred } from "../../tests/fixtures/binarySession.js"; +import { PrivateRuntimeRoot } from "../process/PrivateRuntimeRoot.js"; import { createHash } from "node:crypto"; import { chmod, mkdtemp, readFile, + stat, rm, truncate, writeFile, } from "node:fs/promises"; import { join } from "node:path"; -import { describe, expect, onTestFinished, test } from "vitest"; +import { describe, expect, onTestFinished, test, vi } from "vitest"; import { ApktoolProvider } from "./ApktoolProvider.js"; import { ApktoolResourceAnalysisService } from "../application/apktool/ApktoolResourceAnalysisService.js"; @@ -622,3 +625,236 @@ it("accepts a BCP47 resource qualifier through the public request schema", async const result = await evidenceResult(providerOver(command), request); expect(result.strings).toEqual([{ name: "greeting", value: "你好" }]); }); + +it("retains completed Evidence and live cleanup until a denied workspace removal recovers", async () => { + const { command, root } = await writeApktoolStub(decodeStub()); + const apk = await apkFixture(root); + let denied = true; + let owned: string | undefined; + const provider = new ApktoolProvider({ + environment: { REA_APKTOOL_COMMAND: command }, + removeWorkspace: async (path) => { + owned = path; + 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 }, + }); + if (result.ok) throw new Error("Expected cleanup failure"); + expect(result.error.cleanupIncomplete).toBe(true); + expect(result.error.partialObservation).toMatchObject({ + operation: "decode_android_resources", + subject: { local_path: apk.path, digest: { sha256: apk.sha256 } }, + normalized_result: { + metadata: { package_name: "com.rea.apktool.probe" }, + strings: [ + { name: "api_endpoint", value: "https://api.example.com/v1" }, + { name: "app_name", value: "ApktoolProbe" }, + ], + }, + }); + const blocked = await provider.execute({ + operation: "inspect_apktool_client", + input: {}, + }); + if (blocked.ok) + throw new Error("Expected retained-workspace admission refusal"); + expect(blocked.error.cleanupIncomplete).toBe(true); + expect(blocked.error.cleanupResources).toContain(owned); + await expect(provider.close()).rejects.toMatchObject({ + cleanupIncomplete: true, + }); + denied = false; + await provider.close(); + expect(result.error.cleanupIncomplete).toBe(false); + expect(blocked.error.cleanupIncomplete).toBe(false); + expect(result.error.partialObservation).toMatchObject({ + normalized_result: { target: { sha256: apk.sha256 } }, + }); + } finally { + denied = false; + await provider.close(); + } +}); + +it.each([false, true])( + "preserves completed Evidence and cancellation during workspace cleanup (denied=%s)", + async (deniedInitially) => { + const { command, root } = await writeApktoolStub(decodeStub()); + const apk = await apkFixture(root); + const controller = new AbortController(); + const reason = { caller: "during final workspace removal" }; + let denied = deniedInitially; + const provider = new ApktoolProvider({ + environment: { REA_APKTOOL_COMMAND: command }, + removeWorkspace: async (path) => { + controller.abort(reason); + if (denied) throw new Error("authored 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 }, + }, + { signal: controller.signal }, + ); + if (result.ok) throw new Error("Expected finalization cancellation"); + const primary = result.error.cleanupPrimary ?? result.error; + expect(primary._tag).toBe("AnalysisCancelledError"); + expect(primary.cause).toBe(reason); + expect(result.error.cleanupIncomplete).toBe(deniedInitially); + expect(result.error.partialObservation).toMatchObject({ + operation: "decode_android_resources", + normalized_result: { target: { sha256: apk.sha256 } }, + }); + denied = false; + await provider.close(); + expect(result.error.cleanupIncomplete).toBe(false); + } finally { + denied = false; + await provider.close(); + } + }, +); + +it.each(["decode", "projection"] as const)( + "keeps a recorded %s failure primary when cancellation arrives during removal", + async (stage) => { + const { command, root } = await writeApktoolStub( + decodeStub({ failDecode: stage === "decode" }), + ); + const apk = await apkFixture(root); + if (stage === "projection") { + const bad = join(root, "bad.xml"); + await writeFile(bad, Buffer.from([0xff])); + await writeFile( + command, + "#!/bin/sh\n" + decodeStub({ stringsFile: bad })(root), + { mode: 0o755 }, + ); + } + const controller = new AbortController(); + const provider = new ApktoolProvider({ + environment: { REA_APKTOOL_COMMAND: command }, + removeWorkspace: async (path) => { + controller.abort({ caller: "after decode refusal" }); + await rm(path, { recursive: true, force: true }); + }, + }); + try { + const result = await provider.execute( + { + operation: "decode_android_resources", + input: { path: apk.path, include_strings: true }, + }, + { signal: controller.signal }, + ); + if (result.ok) throw new Error("Expected decode refusal"); + expect(controller.signal.aborted).toBe(true); + expect(result.error._tag).toBe( + stage === "decode" + ? "AnalysisUnsupportedTargetError" + : "AnalysisProtocolError", + ); + if (stage === "decode") + 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, + }); + } finally { + await provider.close(); + } + }, +); + +it("releases successful workspace peers once while retaining a denied peer for retry", async () => { + const { command, root } = await writeApktoolStub(decodeStub()); + const apk = await apkFixture(root); + let deniedRoot: string | undefined; + let denied = true; + const attempts = new Map(); + const provider = new ApktoolProvider({ + environment: { REA_APKTOOL_COMMAND: command }, + removeWorkspace: async (path) => { + attempts.set(path, (attempts.get(path) ?? 0) + 1); + deniedRoot ??= path; + if (path === deniedRoot && denied) + throw new Error("authored peer denial"); + await rm(path, { recursive: true, force: true }); + }, + }); + try { + const request = { + operation: "decode_android_resources" as const, + input: { path: apk.path, include_strings: true }, + }; + const results = await Promise.all([ + provider.execute(request), + provider.execute(request), + ]); + expect(results.filter((result) => result.ok)).toHaveLength(1); + const retained = results.find((result) => !result.ok); + if (retained === undefined || retained.ok) + throw new Error("Expected retained peer"); + expect(attempts.size).toBe(2); + await expect(provider.close()).rejects.toMatchObject({ + cleanupIncomplete: true, + }); + denied = false; + await provider.close(); + expect(retained.error.cleanupIncomplete).toBe(false); + for (const [path, count] of attempts) + expect(count).toBe(path === deniedRoot ? 3 : 1); + } finally { + denied = false; + await provider.close(); + } +}); + +it("joins a late workspace allocation during shutdown and removes its owned root", async () => { + const { command, root } = await writeApktoolStub(decodeStub()); + const apk = await apkFixture(root); + const created = createDeferred(); + const release = createDeferred(); + const allocate = PrivateRuntimeRoot.create.bind(PrivateRuntimeRoot); + const allocation = vi + .spyOn(PrivateRuntimeRoot, "create") + .mockImplementationOnce(async (options) => { + const runtime = await allocate(options); + created.resolve(runtime.path); + await release.promise; + return runtime; + }); + const provider = providerOver(command); + const pending = provider.execute({ + operation: "decode_android_resources", + input: { path: apk.path, include_strings: true }, + }); + let closing: Promise | undefined; + try { + const owned = await created.promise; + closing = provider.close(); + release.resolve(); + const result = await pending; + if (result.ok) throw new Error("Expected shutdown cancellation"); + expect(result.error._tag).toBe("AnalysisCancelledError"); + await closing; + await expect(stat(owned)).rejects.toMatchObject({ code: "ENOENT" }); + } finally { + release.resolve(); + await pending; + allocation.mockRestore(); + await (closing ?? provider.close()); + } +}); diff --git a/src/apktool/ApktoolProvider.ts b/src/apktool/ApktoolProvider.ts index 8e4c5c288e..d4a748a108 100644 --- a/src/apktool/ApktoolProvider.ts +++ b/src/apktool/ApktoolProvider.ts @@ -1,25 +1,24 @@ +import { countDecodedFiles } from "./ApktoolDecodedInventory.js"; +import { ApktoolWorkspace } from "./ApktoolWorkspace.js"; +import { ArtifactResourceScope } from "../artifacts/ArtifactResourceScope.js"; +import { + AnalysisCleanupLease, + AnalysisCleanupLeaseGroup, +} from "../artifacts/AnalysisCleanupLease.js"; +import { ArtifactReaderFailure } from "../artifacts/ArtifactReader.js"; +import { + ArtifactFileCleanupFailure, + artifactFilePrimaryFailure, + artifactFileCancellationCause, +} from "../artifacts/ArtifactFileOperation.js"; +import { PrivateRuntimeRoot } from "../process/PrivateRuntimeRoot.js"; import { capturedApktoolCommandOutput } from "./ApktoolCommandOutput.js"; import { ApktoolCommandCleanup } from "./ApktoolCommandCleanup.js"; import { AnalysisError } from "../domain/analysisErrorBase.js"; import { analysisClientFailureForOperation } from "../providers/AnalysisClientFailure.js"; -import { regularFileCancellation } from "../filesystem/OwnedRegularFile.js"; -import { createHash, randomUUID } from "node:crypto"; -import { createWriteStream } from "node:fs"; -import { - access, - chmod, - mkdir, - mkdtemp, - opendir, - open, - rm, - stat, -} from "node:fs/promises"; -import { tmpdir } from "node:os"; +import { randomUUID } from "node:crypto"; +import { mkdir } from "node:fs/promises"; import { join } from "node:path"; -import { Transform, type TransformCallback } from "node:stream"; -import { pipeline } from "node:stream/promises"; -import type { FileHandle } from "node:fs/promises"; import type { ExecutionOptions, @@ -28,16 +27,13 @@ import type { import { providerConfigurationRefusal } from "../domain/providerConfigurationRefusal.js"; import { createAnalysisExecution, + createAnalysisExecutionEvidence, type AnalysisExecution, } from "../application/AnalysisProvider.js"; -import { - androidLocaleQualifierSchema, - type ApktoolRequest, -} from "../domain/apktool/apktoolResourceAnalysis.js"; +import type { ApktoolRequest } from "../domain/apktool/apktoolResourceAnalysis.js"; import { AnalysisCapabilityUnavailableError, AnalysisCancelledError, - AnalysisInputError, AnalysisProtocolError, AnalysisResourceConstraintError, AnalysisTimeoutError, @@ -46,10 +42,6 @@ import { import { analysisErrorWithCleanupFailure } from "../domain/analysisErrorCleanup.js"; import { err, ok, type Result } from "../domain/result.js"; import { ProviderCleanupError } from "../domain/providerCleanupError.js"; -import { - openRegularFile, - sameRegularFileState, -} from "../filesystem/RegularFile.js"; import { OwnedCommandFailure, runOwnedCommand, @@ -95,55 +87,6 @@ const throwIfCancelled = ( throw new AnalysisCancelledError(operation, { cause: signal.reason }); }; -/** Read at most the byte budget plus one byte, before decoding UTF-8. */ -const readBoundedText = async ( - path: string, - budget: number, - operation: string, - signal: AbortSignal | undefined, -): Promise<{ text: string | null; truncated: boolean }> => { - const file = await open(path, "r").catch((cause: unknown) => { - if ((cause as NodeJS.ErrnoException).code === "ENOENT") return null; - throw cause; - }); - if (file === null) return { text: null, truncated: false }; - try { - throwIfCancelled(operation, signal); - const bytes = Buffer.alloc(budget + 1); - let length = 0; - while (length < bytes.length) { - throwIfCancelled(operation, signal); - const result = await file.read( - bytes, - length, - bytes.length - length, - null, - ); - if (result.bytesRead === 0) break; - length += result.bytesRead; - } - throwIfCancelled(operation, signal); - const truncated = length > budget; - // A prefix may end within a UTF-8 character; omit those trailing bytes. - try { - const decoder = new TextDecoder("utf-8", { fatal: true }); - return { - text: decoder.decode(bytes.subarray(0, Math.min(length, budget)), { - stream: truncated, - }), - truncated, - }; - } catch (cause: unknown) { - throw new AnalysisProtocolError( - `apktool produced invalid UTF-8 in ${path}`, - { cause }, - ); - } - } finally { - await file.close(); - } -}; - const localeQualifier = (locale: string): string => locale.replace(/^([a-z]{2,3})-([A-Z]{2}|[0-9]{3})$/u, "$1-r$2"); @@ -259,52 +202,6 @@ const apktoolCommandFailure = ( ); }; -/** Count files while bounding streamed directory-entry traversal. */ -const countDecodedFiles = async ( - root: string, - signal: AbortSignal | undefined, - operation: string, -): Promise<{ - count: number; - capped: boolean; - locales: readonly string[]; -}> => { - let count = 0; - let capped = false; - let visited = 0; - const directories = [root]; - const locales: string[] = []; - while (directories.length > 0 && !capped) { - throwIfCancelled(operation, signal); - const directory = directories.pop()!; - for await (const entry of await opendir(directory)) { - throwIfCancelled(operation, signal); - if (visited >= MAX_DECODED_ENTRIES_VISITED) { - capped = true; - break; - } - visited += 1; - const path = join(directory, entry.name); - if (entry.isDirectory()) { - directories.push(path); - if (directory === join(root, "res")) { - const match = /^values-(.+)$/u.exec(entry.name); - if ( - match !== null && - androidLocaleQualifierSchema.safeParse(match[1]).success && - (await access(join(path, "strings.xml")).then( - () => true, - () => false, - )) - ) - locales.push(match[1]!); - } - } else if (entry.isFile()) count += 1; - } - } - return { count, capped, locales: locales.sort() }; -}; - export interface ApktoolProviderOptions { readonly environment?: Readonly>; readonly removeWorkspace?: (path: string) => Promise; @@ -322,8 +219,10 @@ export class ApktoolProvider { | Readonly> | undefined; - private readonly workspaces = new Set(); - private readonly removeWorkspace: (path: string) => Promise; + readonly #artifacts = new ArtifactResourceScope(); + private readonly removeWorkspace: + | ((path: string) => Promise) + | undefined; readonly #commands = new ApktoolCommandCleanup(); readonly #shutdown = new AbortController(); readonly #active = new Set>(); @@ -335,9 +234,7 @@ export class ApktoolProvider { options.environment === undefined ? undefined : snapshotEnvironment(options.environment); - this.removeWorkspace = - options.removeWorkspace ?? - ((path) => rm(path, { recursive: true, force: true })); + this.removeWorkspace = options.removeWorkspace; } private operationEnvironment(): Readonly { @@ -363,10 +260,21 @@ export class ApktoolProvider { ? this.#shutdown.signal : AbortSignal.any([signal, this.#shutdown.signal]); return this.#track( - this.#commands.run( - () => this.#inspectAvailability(operationSignal), - "inspect_apktool_client", - ), + this.#commands + .run( + () => + this.#artifacts.run(() => + this.#inspectAvailability(operationSignal), + ), + "inspect_apktool_client", + ) + .catch((cause: unknown) => { + throw analysisClientFailureForOperation( + cause, + "apktool", + "inspect_apktool_client", + ); + }), ); } @@ -427,7 +335,10 @@ export class ApktoolProvider { : AbortSignal.any([options.signal, this.#shutdown.signal]); return this.#track( this.#commands - .run(() => this.#execute(request, signal), request.operation) + .run( + () => this.#artifacts.run(() => this.#execute(request, signal)), + request.operation, + ) .catch((cause: unknown) => err( analysisClientFailureForOperation( @@ -524,18 +435,30 @@ export class ApktoolProvider { async #drainAndClean(): Promise { await Promise.all(this.#active); - await this.#commands.close(); - const failures: unknown[] = []; - for (const path of this.workspaces) { + const failures: AnalysisError[] = []; + for (const close of [ + () => this.#commands.close(), + () => this.#artifacts.close(), + ]) { try { - await this.removeWorkspace(path); - this.workspaces.delete(path); + await close(); } catch (cause) { - failures.push(cause); + failures.push( + analysisClientFailureForOperation(cause, "apktool", "close"), + ); } } - if (failures.length > 0) - throw new AggregateError(failures, "Apktool workspace cleanup failed"); + const first = failures.shift(); + if (first === undefined) return; + throw failures.reduce( + (primary, cleanup) => + analysisErrorWithCleanupFailure( + primary, + cleanup, + "close", + ).bindCleanupDependencies([primary, cleanup]), + first, + ); } private async decodeResources( @@ -555,101 +478,27 @@ export class ApktoolProvider { }, signal: AbortSignal | undefined, ): Promise> { - const root = await mkdtemp(join(tmpdir(), "rea-apktool-")); - this.workspaces.add(root); + const runtime = await PrivateRuntimeRoot.create({ prefix: "rea-apktool-" }); + const resources = new ApktoolWorkspace( + runtime, + this.#commands, + this.removeWorkspace, + ); + const root = resources.path; const workspace = join(root, "decoded"); const snapshot = join(root, "input.apk"); const framework = join(root, "framework"); - let cleanupFailure: string | null = null; - // The workspace is removed before the result is returned, so an - // unremovable workspace is reported instead of silently abandoned. - const removeWorkspace = async (): Promise => { - if (this.#commands.pendingFor(root)) { - cleanupFailure = `The decode workspace is retained until its apktool process stops: ${root}`; - return; - } - try { - await this.removeWorkspace(root); - this.workspaces.delete(root); - } catch { - cleanupFailure = `The decode workspace could not be removed: ${root}`; - } - }; - const failureWithCleanup = (failure: AnalysisError): AnalysisError => - cleanupFailure === null - ? failure - : analysisErrorWithCleanupFailure( - failure, - new ProviderCleanupError( - APKTOOL_PROVIDER_IDENTITY.id, - [root], - { reason: cleanupFailure }, - { operation }, - ), - operation, - ); - let source: FileHandle | undefined; - let sourceAdmissionFailed = false; + const lease = new AnalysisCleanupLease(() => resources.close(), [root]); + const owner = { kind: "lease" as const, resource: root, lease }; + this.#artifacts.retain(owner); + let result: Result; try { - throwIfCancelled(operation, signal); - sourceAdmissionFailed = true; - source = await openRegularFile(input.path, { - symlinks: "follow", - ...(signal === undefined ? {} : { signal }), - }); - const sourceInfo = await source.stat(); - sourceAdmissionFailed = false; - const sourceSize = sourceInfo.size; - let copiedBytes = 0; - const sourceHash = createHash("sha256"); - const boundedSource = new Transform({ - transform( - chunk: Buffer, - _encoding: BufferEncoding, - callback: TransformCallback, - ) { - copiedBytes += chunk.length; - if (copiedBytes > sourceSize) { - callback( - new AnalysisInputError(operation, { - cause: new Error( - `The selected APK changed while being copied: ${input.path}`, - ), - }), - ); - return; - } - sourceHash.update(chunk); - callback(null, chunk); - }, - }); - await pipeline( - source.createReadStream({ - start: 0, - end: sourceSize, - autoClose: false, - ...(signal === undefined ? {} : { signal }), - }), - boundedSource, - createWriteStream(snapshot, { flags: "wx", mode: 0o600 }), - signal === undefined ? {} : { signal }, + const identity = await resources.snapshotInput( + input.path, + operation, + signal, ); - const finalSourceInfo = await source.stat(); - if ( - copiedBytes !== sourceSize || - !sameRegularFileState(sourceInfo, finalSourceInfo) - ) - throw new AnalysisInputError(operation, { - cause: new Error( - `The selected APK changed while being copied: ${input.path}`, - ), - }); - const sha256 = sourceHash.digest("hex"); - await source.close(); - source = undefined; - await chmod(snapshot, 0o400); - throwIfCancelled(operation, signal); - const snapshotInfo = await stat(snapshot); + const sha256 = identity.sha256; await mkdir(workspace); await mkdir(framework); const decoded = await runApktool( @@ -675,14 +524,12 @@ export class ApktoolProvider { root, ); if (!decoded.ok) { - await removeWorkspace(); - return err(failureWithCleanup(decoded.error)); + throw decoded.error; } throwIfCancelled(operation, signal); - const yml = await readBoundedText( + const yml = await resources.readText( join(workspace, "apktool.yml"), MANIFEST_MAX_BYTES, - operation, signal, ); if (yml.text === null || yml.truncated) { @@ -691,10 +538,9 @@ export class ApktoolProvider { ); } const metadata = parseApktoolYml(yml.text); - const manifestRead = await readBoundedText( + const manifestRead = await resources.readText( join(workspace, "AndroidManifest.xml"), MANIFEST_MAX_BYTES, - operation, signal, ); const manifest = manifestRead.text; @@ -707,10 +553,9 @@ export class ApktoolProvider { ? "values" : `values-${localeQualifier(input.locale)}`; const stringsRead = input.include_strings - ? await readBoundedText( + ? await resources.readText( join(workspace, "res", stringsDirectory, "strings.xml"), STRINGS_XML_MAX_BYTES, - operation, signal, ) : { text: null, truncated: false }; @@ -721,7 +566,12 @@ export class ApktoolProvider { ? { entries: [], unparsedCount: 0 } : parseDecodedStrings(stringsXml); const bounded = parsedStrings.entries.slice(0, MAX_STRINGS); - const files = await countDecodedFiles(workspace, signal, operation); + const files = await countDecodedFiles( + resources, + workspace, + signal, + MAX_DECODED_ENTRIES_VISITED, + ); throwIfCancelled(operation, signal); const coverage = (stringsXml !== null && parsedStrings.entries.length > MAX_STRINGS) || @@ -731,12 +581,11 @@ export class ApktoolProvider { files.capped ? "partial" : "complete"; - await removeWorkspace(); - return ok( + result = ok( createAnalysisExecution( { client, - target: { path: input.path, bytes: snapshotInfo.size, sha256 }, + target: { path: input.path, bytes: identity.bytes, sha256 }, metadata: { version_name: metadata.versionName, version_code: metadata.versionCode, @@ -793,43 +642,95 @@ export class ApktoolProvider { `Decoded workspace enumeration stopped after ${String(MAX_DECODED_ENTRIES_VISITED)} entries; decoded_file_count and locales may be partial`, ] : []), - ...(cleanupFailure === null ? [] : [cleanupFailure]), ], }, ), ); } catch (cause) { - await removeWorkspace(); - const cancellation = regularFileCancellation(cause); - if (cancellation !== undefined) - return err( - failureWithCleanup( - new AnalysisCancelledError(operation, { - cause: cancellation.reason, - }), - ), - ); - return err( - failureWithCleanup( - signal?.aborted === true - ? new AnalysisCancelledError(operation, { cause }) - : sourceAdmissionFailed - ? new AnalysisInputError(operation, { cause }) - : cause instanceof AnalysisInputError - ? cause - : cause instanceof AnalysisCancelledError || - cause instanceof AnalysisProtocolError - ? cause - : new AnalysisProtocolError( - "Apktool resource projection failed", - { - cause, - }, - ), - ), + const physical = artifactFilePrimaryFailure(cause); + const primary = + physical instanceof AnalysisError + ? physical + : physical instanceof ArtifactReaderFailure && + physical.reason === "cancelled" + ? new AnalysisCancelledError(operation, { + cause: artifactFileCancellationCause(physical), + }) + : new AnalysisProtocolError("Apktool resource projection failed", { + cause: physical, + }); + const failure = analysisClientFailureForOperation( + cause, + "apktool", + operation, ); - } finally { - await source?.close().catch(() => undefined); + if ( + cause instanceof ArtifactReaderFailure && + cause.cleanup !== undefined && + !(cause instanceof ArtifactFileCleanupFailure) + ) + new AnalysisCleanupLeaseGroup(failure, [lease]); + const retained = + physical === cause && cause instanceof AnalysisError + ? cause + : failure.cleanupIncomplete + ? analysisErrorWithCleanupFailure( + primary, + failure, + operation, + ).bindCleanupDependencies([primary, failure]) + : primary; + if ( + resources.snapshot !== undefined && + retained.partialObservation === undefined + ) + retained.retainPartialObservation({ + kind: "file-snapshot-hash", + ...resources.snapshot, + }); + result = err(retained); } + const released = await this.#artifacts.release(owner); + const observation = + result.ok && (released.kind === "failed" || signal?.aborted === true) + ? createAnalysisExecutionEvidence( + operation, + input, + result.value, + "observed", + ) + : undefined; + if (result.ok && signal?.aborted) + result = err( + new AnalysisCancelledError(operation, { + cause: signal.reason, + partialObservation: observation, + }), + ); + if (released.kind === "failed") { + const cleanup = new ProviderCleanupError( + "apktool", + [root], + {}, + { + operation, + cause: released.cause, + ...(observation === undefined + ? {} + : { partialObservation: observation }), + }, + ); + new AnalysisCleanupLeaseGroup(cleanup, [lease]); + return err( + result.ok + ? cleanup + : analysisErrorWithCleanupFailure( + result.error, + cleanup, + operation, + ).bindCleanupDependencies([result.error, cleanup]), + ); + } + return result; } } diff --git a/src/apktool/ApktoolWorkspace.lifecycle.test.ts b/src/apktool/ApktoolWorkspace.lifecycle.test.ts new file mode 100644 index 0000000000..ceee8ea00b --- /dev/null +++ b/src/apktool/ApktoolWorkspace.lifecycle.test.ts @@ -0,0 +1,262 @@ +import { countDecodedFiles } from "./ApktoolDecodedInventory.js"; +import * as fs from "node:fs/promises"; +import { join } from "node:path"; +import { execFile } from "node:child_process"; +import { promisify } from "node:util"; +import { afterEach, expect, it, vi } from "vitest"; +import { + artifactFilePrimaryFailure, + ArtifactFileCleanupFailure, + ArtifactFileCancelled, +} from "../artifacts/ArtifactFileOperation.js"; +import { ArtifactReaderFailure } from "../artifacts/ArtifactReader.js"; +import { PrivateRuntimeRoot } from "../process/PrivateRuntimeRoot.js"; +import { ApktoolCommandCleanup } from "./ApktoolCommandCleanup.js"; +import { ApktoolWorkspace } from "./ApktoolWorkspace.js"; + +vi.mock("node:fs/promises", async (original) => { + const actual = await original(); + return { ...actual, open: vi.fn(actual.open) }; +}); +const actual = + await vi.importActual("node:fs/promises"); +afterEach(() => { + vi.mocked(fs.open).mockImplementation(actual.open); +}); + +it.each(["source", "snapshot", "output"] as const)( + "retains the exact %s descriptor after a denied close and fences root removal", + async (kind) => { + const root = await PrivateRuntimeRoot.create({ + prefix: "rea-apktool-owned-test-", + }); + const commands = new ApktoolCommandCleanup(); + const workspace = new ApktoolWorkspace(root, commands); + const input = join(root.path, "source.apk"); + const output = join(root.path, "output.xml"); + await fs.writeFile(input, "small-authored-apk"); + await fs.writeFile(output, ""); + const selected = + kind === "source" + ? input + : kind === "snapshot" + ? join(root.path, "input.apk") + : output; + let denied = true; + const closes = vi.fn(); + vi.mocked(fs.open).mockImplementation(async (path, flags, mode) => { + const handle = await actual.open(path, flags, mode); + if (String(path) === selected) { + const close = handle.close.bind(handle); + handle.close = async () => { + closes(); + if (denied) throw new Error("authored close denial"); + await close(); + }; + } + return handle; + }); + try { + const failure = await ( + kind === "output" + ? workspace.readText(output, 100) + : workspace.snapshotInput(input, "decode_android_resources") + ).catch((cause: unknown) => cause); + expect(failure).toBeInstanceOf(ArtifactFileCleanupFailure); + if (!(failure instanceof ArtifactFileCleanupFailure)) + throw new Error("Expected owned descriptor failure"); + expect(failure.cleanupLease.released).toBe(false); + await expect(workspace.close()).rejects.toBeInstanceOf( + ArtifactReaderFailure, + ); + await expect(fs.stat(root.path)).resolves.toBeDefined(); + denied = false; + await workspace.close(); + expect(failure.cleanupLease.released).toBe(true); + const count = closes.mock.calls.length; + await workspace.close(); + expect(closes).toHaveBeenCalledTimes(count); + await expect(fs.stat(root.path)).rejects.toMatchObject({ + code: "ENOENT", + }); + } finally { + denied = false; + await workspace.close(); + await commands.close(); + } + }, +); + +it("retains a physical decode-output failure when cancellation arrives during descriptor close", async () => { + const root = await PrivateRuntimeRoot.create({ + prefix: "rea-apktool-owned-test-", + }); + const commands = new ApktoolCommandCleanup(); + const workspace = new ApktoolWorkspace(root, commands); + const output = join(root.path, "invalid.xml"); + const controller = new AbortController(); + await fs.writeFile(output, Buffer.from([0xff])); + vi.mocked(fs.open).mockImplementation(async (path, flags, mode) => { + const handle = await actual.open(path, flags, mode); + const close = handle.close.bind(handle); + handle.close = async () => { + controller.abort({ caller: "after malformed bytes" }); + await close(); + }; + return handle; + }); + try { + await expect( + workspace.readText(output, 100, controller.signal), + ).rejects.toMatchObject({ _tag: "AnalysisProtocolError" }); + } finally { + await workspace.close(); + await commands.close(); + } +}); + +it("records cancellation observed during a successful output descriptor close", async () => { + const root = await PrivateRuntimeRoot.create({ + prefix: "rea-apktool-owned-test-", + }); + const commands = new ApktoolCommandCleanup(); + const workspace = new ApktoolWorkspace(root, commands); + const output = join(root.path, "output.xml"); + const controller = new AbortController(); + const reason = { caller: "during descriptor close" }; + await fs.writeFile(output, ""); + vi.mocked(fs.open).mockImplementation(async (path, flags, mode) => { + const handle = await actual.open(path, flags, mode); + const close = handle.close.bind(handle); + handle.close = async () => { + controller.abort(reason); + await close(); + }; + return handle; + }); + try { + const failure = await workspace + .readText(output, 100, controller.signal) + .catch((cause: unknown) => cause); + expect(failure).toBeInstanceOf(ArtifactFileCancelled); + if (!(failure instanceof ArtifactFileCancelled)) + throw new Error("Expected close cancellation"); + expect(failure.fileCancellation.reason).toBe(reason); + expect(failure.fileCancellation.outcome).toEqual({ + kind: "completed", + value: { text: "", truncated: false }, + }); + } finally { + await workspace.close(); + await commands.close(); + } +}); + +it.skipIf(process.platform === "win32")( + "rejects a decoded FIFO without waiting for a writer", + async () => { + const root = await PrivateRuntimeRoot.create({ + prefix: "rea-apktool-owned-test-", + }); + const commands = new ApktoolCommandCleanup(); + const workspace = new ApktoolWorkspace(root, commands); + const fifo = join(root.path, "strings.xml"); + try { + await promisify(execFile)("mkfifo", [fifo]); + await expect(workspace.readText(fifo, 100)).rejects.toMatchObject({ + code: "ENOTFILE", + }); + } finally { + await workspace.close(); + await commands.close(); + } + }, +); + +it.each([false, true])( + "keeps malformed UTF-8 primary when final stat observes cancellation (close denied=%s)", + async (deniedInitially) => { + const root = await PrivateRuntimeRoot.create({ + prefix: "rea-apktool-owned-test-", + }); + const commands = new ApktoolCommandCleanup(); + const workspace = new ApktoolWorkspace(root, commands); + const path = join(root.path, "malformed.xml"); + const controller = new AbortController(); + let denied = deniedInitially; + await fs.writeFile(path, Buffer.from([0xff])); + vi.mocked(fs.open).mockImplementation(async (name, flags, mode) => { + const handle = await actual.open(name, flags, mode); + const stat = handle.stat.bind(handle); + let calls = 0; + handle.stat = ((...args: Parameters) => + stat(...args).then((value) => { + if (++calls === 3) + controller.abort({ caller: "after bytes before interpretation" }); + return value; + })) as typeof handle.stat; + const close = handle.close.bind(handle); + handle.close = async () => { + if (denied) throw new Error("authored denial"); + await close(); + }; + return handle; + }); + try { + const failure = await workspace + .readText(path, 100, controller.signal) + .catch((cause: unknown) => cause); + expect(controller.signal.aborted).toBe(true); + expect(artifactFilePrimaryFailure(failure)).toMatchObject({ + _tag: "AnalysisProtocolError", + }); + if (denied) + await expect(workspace.close()).rejects.toBeInstanceOf( + ArtifactReaderFailure, + ); + denied = false; + await workspace.close(); + } finally { + denied = false; + await workspace.close(); + await commands.close(); + } + }, +); + +it.skipIf(process.platform === "win32")( + "counts omitted special entries toward the decoded inventory budget", + async () => { + const root = await PrivateRuntimeRoot.create({ + prefix: "rea-apktool-owned-test-", + }); + const commands = new ApktoolCommandCleanup(); + const workspace = new ApktoolWorkspace(root, commands); + try { + await fs.writeFile(join(root.path, "one.xml"), "one"); + await fs.writeFile(join(root.path, "two.xml"), "two"); + for (let index = 0; index < 4; index++) + await promisify(execFile)("mkfifo", [ + join(root.path, `special-${index}`), + ]); + const result = await countDecodedFiles( + workspace, + root.path, + undefined, + 3, + ); + expect(result.capped).toBe(true); + expect(result.count).toBeLessThanOrEqual(2); + const complete = await countDecodedFiles( + workspace, + root.path, + undefined, + 6, + ); + expect(complete).toEqual({ count: 2, capped: false, locales: [] }); + } finally { + await workspace.close(); + await commands.close(); + } + }, +); diff --git a/src/apktool/ApktoolWorkspace.ts b/src/apktool/ApktoolWorkspace.ts new file mode 100644 index 0000000000..4d4655f545 --- /dev/null +++ b/src/apktool/ApktoolWorkspace.ts @@ -0,0 +1,227 @@ +import { readArtifactFileChunks } from "../artifacts/readArtifactFileChunks.js"; +import { ArtifactReaderFailure } from "../artifacts/ArtifactReader.js"; +import { createHash } from "node:crypto"; +import { chmod, open } from "node:fs/promises"; +import { join } from "node:path"; +import { ArtifactResourceScope } from "../artifacts/ArtifactResourceScope.js"; +import { DirectoryArtifactReader } from "../artifacts/DirectoryArtifactReader.js"; +import { + assertArtifactReadActive, + mapArtifactFileFailure, + withArtifactFileHandle, + withArtifactRegularFile, +} from "../artifacts/ArtifactFileOperation.js"; +import { + AnalysisInputError, + AnalysisProtocolError, +} from "../domain/analysisErrorCore.js"; +import { sameRegularFileState } from "../filesystem/RegularFile.js"; +import { PrivateRuntimeRoot } from "../process/PrivateRuntimeRoot.js"; +import type { ApktoolCommandCleanup } from "./ApktoolCommandCleanup.js"; + +/** One decode's descriptors and root; removal follows confirmed child cleanup. */ +export class ApktoolWorkspace { + readonly #files = new ArtifactResourceScope(); + #snapshot: + | { readonly path: string; readonly bytes: number; readonly sha256: string } + | undefined; + + constructor( + readonly root: PrivateRuntimeRoot, + readonly commands: ApktoolCommandCleanup, + readonly removeWorkspace?: (path: string) => Promise, + ) {} + + get snapshot() { + return this.#snapshot; + } + + get path(): string { + return this.root.path; + } + + async close(): Promise { + await this.#files.close(); + if (this.commands.pendingFor(this.path)) + throw new Error( + "The decode workspace remains owned until its Apktool command stops", + ); + if (this.removeWorkspace !== undefined) + await this.removeWorkspace(this.path); + await this.root.close(); + } + + /** Stream one admitted input into a separately owned immutable snapshot. */ + async snapshotInput(path: string, operation: string, signal?: AbortSignal) { + let admitted = false; + try { + return await withArtifactRegularFile( + path, + this.#files, + signal, + async (source) => { + admitted = true; + const initial = await source.stat(); + const output = join(this.path, "input.apk"); + const file = await open(output, "wx", 0o600); + await withArtifactFileHandle( + output, + file, + this.#files, + signal, + async (destination) => { + let copied = 0; + const hash = createHash("sha256"); + for await (const value of readArtifactFileChunks(source, { + start: 0, + end: initial.size, + signal, + })) { + assertArtifactReadActive(signal); + if (!Buffer.isBuffer(value)) + throw new AnalysisProtocolError( + "APK snapshot stream returned non-byte data", + ); + copied += value.length; + if (copied > initial.size) + throw new AnalysisInputError(operation, { + cause: new Error( + `The selected APK changed while being copied: ${path}`, + ), + }); + hash.update(value); + let offset = 0; + while (offset < value.length) { + assertArtifactReadActive(signal); + const written = await destination.write( + value, + offset, + value.length - offset, + ); + if (written.bytesWritten === 0) + throw new AnalysisProtocolError( + "APK snapshot write made no progress", + ); + offset += written.bytesWritten; + } + } + const final = await source.stat(); + if ( + copied !== initial.size || + !sameRegularFileState(initial, final) + ) + throw new AnalysisInputError(operation, { + cause: new Error( + `The selected APK changed while being copied: ${path}`, + ), + }); + this.#snapshot = { + path, + bytes: copied, + sha256: hash.digest("hex"), + }; + }, + ); + await chmod(output, 0o400); + return this.#snapshot!; + }, + "follow", + ); + } catch (cause) { + throw mapArtifactFileFailure(cause, (physical) => + admitted || + (physical instanceof ArtifactReaderFailure && + physical.reason === "cancelled") + ? physical + : new AnalysisInputError(operation, { cause: physical }), + ); + } + } + + /** Reject non-regular output before reading; every failed close retains its owner. */ + async readText( + path: string, + budget: number, + signal?: AbortSignal, + ): Promise<{ text: string | null; truncated: boolean }> { + let admitted = false; + try { + return await withArtifactRegularFile( + path, + this.#files, + signal, + async (file) => { + admitted = true; + const initial = await file.stat(); + const bytes = Buffer.alloc(budget + 1); + let length = 0; + while (length < bytes.length) { + assertArtifactReadActive(signal); + const result = await file.read( + bytes, + length, + bytes.length - length, + null, + ); + if (result.bytesRead === 0) break; + length += result.bytesRead; + } + const final = await file.stat(); + if (!sameRegularFileState(initial, final)) + throw new AnalysisProtocolError( + `Apktool output changed while being read: ${path}`, + ); + const truncated = length > budget; + try { + return { + text: new TextDecoder("utf-8", { fatal: true }).decode( + bytes.subarray(0, Math.min(length, budget)), + { stream: truncated }, + ), + truncated, + }; + } catch (cause) { + throw new AnalysisProtocolError( + `apktool produced invalid UTF-8 in ${path}`, + { cause }, + ); + } + }, + ); + } catch (cause) { + if ( + !admitted && + cause instanceof Error && + "code" in cause && + cause.code === "ENOENT" + ) + return { text: null, truncated: false }; + throw cause; + } + } + + async *entries(path: string, signal?: AbortSignal, maximumEntries?: number) { + const reader = new DirectoryArtifactReader(path); + const owner = { kind: "reader" as const, resource: path, reader }; + this.#files.retain(owner); + let failed: { cause: unknown } | undefined; + try { + yield* reader.entries( + signal, + maximumEntries === undefined ? undefined : { maximumEntries }, + ); + } catch (cause) { + failed = { cause }; + throw cause; + } finally { + const released = await this.#files.release(owner); + if (released.kind === "failed") + throw failed === undefined + ? released.cause + : ArtifactReaderFailure.withCleanup( + failed.cause, + ArtifactReaderFailure.cleanupObservation(released.cause, path), + ); + } + } +} diff --git a/src/artifacts/DirectoryArtifactReader.ts b/src/artifacts/DirectoryArtifactReader.ts index dfab54f223..a85c2b1b11 100644 --- a/src/artifacts/DirectoryArtifactReader.ts +++ b/src/artifacts/DirectoryArtifactReader.ts @@ -24,6 +24,20 @@ import { readArtifactFileChunks } from "./readArtifactFileChunks.js"; interface DirectoryTraversalOptions extends ArtifactTraversalOptions { readonly descendInto?: (path: string) => boolean; + /** Bound examined children, including omitted special entries. */ + readonly maximumEntries?: number; +} + +/** Exhausted an examined-entry budget; unrelated I/O and cleanup failures stay distinct. */ +export class DirectoryArtifactTraversalLimit extends ArtifactReaderFailure { + constructor( + readonly examinedEntries: number, + readonly maximumEntries: number, + ) { + super("limit", "Directory traversal reached its examined-entry budget", { + boundary: "capacity", + }); + } } /** Non-symlink-following directory reader. */ @@ -47,6 +61,20 @@ export class DirectoryArtifactReader implements ArtifactReader { const root = await this.#resolveRoot(); const options = typeof selection === "function" ? { descendInto: selection } : selection; + const maximum = options?.maximumEntries; + if ( + maximum !== undefined && + (!Number.isSafeInteger(maximum) || maximum < 0) + ) + throw new RangeError( + "Directory traversal maximumEntries must be a nonnegative safe integer", + ); + let examined = 0; + const examine = () => { + if (maximum !== undefined && examined >= maximum) + throw new DirectoryArtifactTraversalLimit(examined, maximum); + examined += 1; + }; const pending = [root]; while (pending.length > 0) { abortIfNeeded(signal); @@ -60,6 +88,7 @@ export class DirectoryArtifactReader implements ArtifactReader { pending, signal, options, + examine, )) yield entry; await withDirectoryIoContext("inspect directory", directory, () => @@ -88,8 +117,9 @@ export class DirectoryArtifactReader implements ArtifactReader { async *#readDirectory( directory: string, pending: string[], - signal?: AbortSignal, - options?: DirectoryTraversalOptions, + signal: AbortSignal | undefined, + options: DirectoryTraversalOptions | undefined, + examine: () => void, ): AsyncIterable { const root = await this.#resolveRoot(); const handle = new OwnedDirectoryHandle( @@ -111,6 +141,7 @@ export class DirectoryArtifactReader implements ArtifactReader { ); const child = await handle.handle.read(); if (child === null) break; + examine(); abortIfNeeded(signal); const absolute = join(directory, child.name); const metadata = await lstat(absolute, { bigint: true }).catch(