diff --git a/docs/apktool-resource-analysis.md b/docs/apktool-resource-analysis.md index 9d1c6b115a..b8969142d8 100644 --- a/docs/apktool-resource-analysis.md +++ b/docs/apktool-resource-analysis.md @@ -93,6 +93,10 @@ still suppresses all string projection, including in this mode. `target.bytes` bind the observation to the exact APK that was decoded. The Evidence subject carries the same snapshot digest and caller-selected path, including when the original file changes during decoding. + A change during snapshot acquisition reports retryable `artifact_changed`; + wait for a stable input before retrying. Host read permission failures report + `access_denied`, while a missing or non-regular selected path reports + `invalid_request` with the rejected `path` field. - **Metadata comes from apktool.yml as apktool printed it.** Version codes keep their own spelling (`'7'` versus `7`); `package_name` is read from the decoded manifest, not inferred. diff --git a/docs/testing.md b/docs/testing.md index de5d285d3f..895817fd17 100644 --- a/docs/testing.md +++ b/docs/testing.md @@ -1699,6 +1699,15 @@ See [Packet capture analysis](pcap-analysis.md) for contracts and budgets. ## Apple native metadata and UI +`tests/boundary/mcp/nativeDispatchBoundTarget.test.ts` uses the production +stdio MCP server on macOS to bind an inert, self-authored Mach-O, then replace +its path with missing, directory and symlink-loop states. Each refusal must +report `artifact_changed`; restoring the same bytes must succeed in the same +session. A separate non-root case checks real read permission denial as +`access_denied`. These tests read the target without executing it. Controlled +provider tests retain the physical acquisition failure through descriptor close +denial and confirm cleanup retry releases the original owner. + `npm run verify:apple-dispatch` compiles the Objective-C fixture (classes, protocols, a property and an `NSString` category) and the Swift conformance/vtable fixture. diff --git a/src/apktool/ApktoolWorkspace.lifecycle.test.ts b/src/apktool/ApktoolWorkspace.lifecycle.test.ts index b2c6dd50f8..69cbffa247 100644 --- a/src/apktool/ApktoolWorkspace.lifecycle.test.ts +++ b/src/apktool/ApktoolWorkspace.lifecycle.test.ts @@ -452,3 +452,48 @@ it("classifies source state changes during snapshot copying as artifact changes" await commands.close(); } }); + +it.each(["EACCES", "EPERM", "EIO", "ENOENT"] as const)( + "preserves the %s acquisition failure classification before snapshot admission", + async (code) => { + const root = await PrivateRuntimeRoot.create({ + prefix: "rea-apktool-admission-test-", + }); + const commands = new ApktoolCommandCleanup(); + const workspace = new ApktoolWorkspace(root, commands); + const input = join(root.path, "source.apk"); + await fs.writeFile(input, "authored-apk"); + const failure = Object.assign(new Error("authored source open failure"), { + code, + }); + vi.mocked(fs.open).mockImplementation(async (path, flags, mode) => { + if (String(path) === input) throw failure; + return await actual.open(path, flags, mode); + }); + try { + const result: unknown = await workspace + .snapshotInput(input, "decode_android_resources") + .catch((cause: unknown) => cause); + if (code === "EIO") expect(result).toBe(failure); + else + expect(result).toMatchObject({ + _tag: + code === "ENOENT" + ? "AnalysisInputError" + : "AnalysisAccessDeniedError", + operation: "decode_android_resources", + cause: failure, + ...(code === "ENOENT" ? {} : { path: input, systemCode: code }), + }); + expect(workspace.snapshot).toBeUndefined(); + await expect(fs.stat(join(root.path, "input.apk"))).rejects.toMatchObject( + { + code: "ENOENT", + }, + ); + } finally { + await workspace.close(); + await commands.close(); + } + }, +); diff --git a/src/apktool/ApktoolWorkspace.ts b/src/apktool/ApktoolWorkspace.ts index 186420d60f..111e8c08e8 100644 --- a/src/apktool/ApktoolWorkspace.ts +++ b/src/apktool/ApktoolWorkspace.ts @@ -10,16 +10,21 @@ import { ArtifactResourceScope } from "../artifacts/ArtifactResourceScope.js"; import { DirectoryArtifactReader } from "../artifacts/DirectoryArtifactReader.js"; import { assertArtifactReadActive, + artifactSelectedPathErrno, mapArtifactFileFailure, withArtifactFileHandle, withArtifactRegularFile, } from "../artifacts/ArtifactFileOperation.js"; import { AnalysisInputError, + AnalysisAccessDeniedError, AnalysisArtifactChangedError, AnalysisProtocolError, } from "../domain/analysisErrorCore.js"; -import { sameRegularFileState } from "../filesystem/RegularFile.js"; +import { + NonRegularFileReadError, + sameRegularFileState, +} from "../filesystem/RegularFile.js"; import { PrivateRuntimeRoot } from "../process/PrivateRuntimeRoot.js"; import type { ApktoolCommandCleanup } from "./ApktoolCommandCleanup.js"; @@ -132,13 +137,29 @@ export class ApktoolWorkspace { "follow", ); } catch (cause) { - throw mapArtifactFileFailure(cause, (physical) => - admitted || - (physical instanceof ArtifactReaderFailure && - physical.reason === "cancelled") - ? physical - : new AnalysisInputError(operation, { cause: physical }), - ); + throw mapArtifactFileFailure(cause, (physical) => { + if (admitted) return physical; + if ( + physical instanceof Error && + "code" in physical && + (physical.code === "EACCES" || physical.code === "EPERM") + ) + return new AnalysisAccessDeniedError(operation, path, physical.code, { + cause: physical, + }); + if ( + physical instanceof NonRegularFileReadError || + artifactSelectedPathErrno(physical) !== undefined + ) + return new AnalysisInputError(operation, { cause: physical }, [ + { + path: ["path"], + reason: "invalid_value", + message: "APK input must resolve to an accessible regular file", + }, + ]); + return physical; + }); } } diff --git a/src/native/AppleDispatchMetadata.ts b/src/native/AppleDispatchMetadata.ts index 6e3dc53c0a..ce249c1ca5 100644 --- a/src/native/AppleDispatchMetadata.ts +++ b/src/native/AppleDispatchMetadata.ts @@ -14,7 +14,6 @@ import { } from "../domain/binaryTargetTypes.js"; import { AnalysisCancelledError, - AnalysisInputError, AnalysisArtifactChangedError, AnalysisAccessDeniedError, AnalysisCapabilityUnavailableError, @@ -473,9 +472,9 @@ const appleDispatchMetadataPrimaryFailure = ( cause, }); if (cause instanceof ArtifactReaderFailure && cause.reason === "path") - return new AnalysisInputError(OPERATION, { cause }, [ - { path: ["path"], reason: "invalid_value", message: cause.message }, - ]); + return new AnalysisArtifactChangedError(OPERATION, path, cause.message, { + cause, + }); } const readCause = origin === "owned-or-classified" && @@ -491,14 +490,15 @@ const appleDispatchMetadataPrimaryFailure = ( return new AnalysisAccessDeniedError(OPERATION, path, readCause.code, { cause, }); + // This path already belongs to the bound target; loss of readability is a + // change to that artifact, rather than a new caller input-selection error. if (isSelectedRegularFileError(readCause)) - return new AnalysisInputError(OPERATION, { cause }, [ - { - path: ["path"], - reason: "invalid_value", - message: `Selected metadata target could not be read (${String(readCause.code)}): ${path}`, - }, - ]); + return new AnalysisArtifactChangedError( + OPERATION, + path, + `Bound metadata target could not be read (${readCause.code}): ${path}`, + { cause }, + ); return new ProviderAdapterError(IDENTITY.id, OPERATION, { cause, diagnostics: { diff --git a/tests/boundary/mcp/nativeDispatchBoundTarget.test.ts b/tests/boundary/mcp/nativeDispatchBoundTarget.test.ts new file mode 100644 index 0000000000..7f9dc78b73 --- /dev/null +++ b/tests/boundary/mcp/nativeDispatchBoundTarget.test.ts @@ -0,0 +1,135 @@ +import { Client } from "@modelcontextprotocol/client"; +import { StdioClientTransport } from "@modelcontextprotocol/client/stdio"; +import { chmod, mkdir, rm, symlink, writeFile } from "node:fs/promises"; +import { join, resolve } from "node:path"; +import { describe, expect, it } from "vitest"; + +import { parseEvidence } from "../../../src/domain/evidence.js"; +import { nativeDispatchMetadataResultSchema } from "../../../src/domain/native/objcSwiftMetadata.js"; +import { parseMcpToolError } from "../../fixtures/mcpToolError.js"; +import { objcRecordBudgetMacho } from "../../fixtures/objcRecordBudgetMacho.js"; +import { createTestTempDirectory } from "../../fixtures/temporaryDirectory.js"; + +type Replacement = "missing" | "directory" | "loop" | "unreadable"; + +// Real native-macos dispatch acquisition, through the compiled stdio server. +// The self-authored Mach-O fixture is only read, never executed. +const inspectReplacements = async (replacements: readonly Replacement[]) => { + const root = await createTestTempDirectory("rea-dispatch-bound-target-"); + const path = join(root, "fixture.macho"); + const bytes = objcRecordBudgetMacho("class"); + await writeFile(path, bytes); + const env: Record = { + PATH: process.env.PATH ?? "", + REA_LOG_LEVEL: "silent", + }; + for (const key of [ + "HOME", + "XDG_CONFIG_HOME", + "XDG_DATA_HOME", + "XDG_CACHE_HOME", + "XDG_STATE_HOME", + "TMPDIR", + ]) { + const directory = join(root, key.toLowerCase()); + env[key] = directory; + await mkdir(directory); + } + const client = new Client({ name: "dispatch-bound-target", version: "1" }); + const transport = new StdioClientTransport({ + command: process.execPath, + args: [resolve("scripts/rea.mjs"), "mcp"], + cwd: process.cwd(), + env, + stderr: "pipe", + }); + let stderr = ""; + transport.stderr?.on("data", (value) => { + stderr += value; + }); + const inspect = () => + client.callTool({ + name: "inspect_native_dispatch_metadata", + arguments: { max_records: 100 }, + }); + const successfulRead = async () => { + const reply = await inspect(); + expect(reply.isError).not.toBe(true); + return nativeDispatchMetadataResultSchema.parse( + parseEvidence(reply.structuredContent).normalized_result, + ); + }; + try { + await client.connect(transport); + const opened = await client.callTool({ + name: "open_binary", + arguments: { path }, + }); + expect(opened.isError).not.toBe(true); + const control = await successfulRead(); + for (const replacement of replacements) { + await rm(path, { recursive: true, force: true }); + if (replacement === "directory") await mkdir(path); + if (replacement === "loop") await symlink("fixture.macho", path); + if (replacement === "unreadable") { + await writeFile(path, bytes); + await chmod(path, 0); + } + try { + const { error } = parseMcpToolError(await inspect()); + const denied = replacement === "unreadable"; + expect(error).toMatchObject({ + code: denied ? "access_denied" : "artifact_changed", + category: denied ? "unavailable" : "integrity_mismatch", + details: { + operation: "inspect_native_dispatch_metadata", + path, + boundary: denied ? "filesystem-read" : "stable-artifact-read", + }, + }); + if (replacement === "unreadable") + expect(error.details?.system_code).toBe("EACCES"); + else { + const code = { + missing: "ENOENT", + directory: "EISDIR", + loop: "ELOOP", + }[replacement]; + expect(error.details?.reason).toContain(code); + } + expect(error.remediation.action).not.toContain("rea doctor"); + } finally { + if (replacement === "unreadable") await chmod(path, 0o600); + } + await rm(path, { recursive: true, force: true }); + await writeFile(path, bytes); + expect(await successfulRead()).toEqual(control); + } + const closed = await client.callTool({ + name: "close_binary", + arguments: {}, + }); + expect(closed.isError).not.toBe(true); + expect(stderr).toBe(""); + } finally { + await client.close(); + await transport.close(); + } +}; + +describe.skipIf(process.platform !== "darwin")( + "bound dispatch target read refusals", + () => { + it("reports missing, directory and symlink-loop replacements as artifact changes and recovers in the same session", async () => { + await inspectReplacements(["missing", "directory", "loop"]); + }, 30_000); + + it.skipIf(process.getuid?.() === 0)( + "keeps permission denial distinct and recovers after restoring read access", + async () => { + await inspectReplacements(["unreadable"]); + }, + 30_000, + ); + }, +); diff --git a/tests/boundary/providers/native/nativeDispatchMetadataFileCleanup.test.ts b/tests/boundary/providers/native/nativeDispatchMetadataFileCleanup.test.ts index 5d973d2181..2be5071fad 100644 --- a/tests/boundary/providers/native/nativeDispatchMetadataFileCleanup.test.ts +++ b/tests/boundary/providers/native/nativeDispatchMetadataFileCleanup.test.ts @@ -11,7 +11,7 @@ import { import { createTestTempDirectory } from "../../../fixtures/temporaryDirectory.js"; /** Open a real target and make only its admitted descriptor's close reject until released. */ -const fixture = async (bindActualDigest: boolean) => { +const fixture = async (bindActualDigest: boolean, admissionFailure?: Error) => { const root = await createTestTempDirectory("rea-native-dispatch-close-"); const path = join(root, "fixture.macho"); await writeFile(path, "bound target bytes"); @@ -41,6 +41,7 @@ const fixture = async (bindActualDigest: boolean) => { if (state.failing) throw new Error("dispatch target close failed"); await release!(); }; + if (admissionFailure !== undefined) throw admissionFailure; return Reflect.apply(original, this, args); }); onTestFinished(async () => { @@ -90,3 +91,24 @@ it("reports a completed read whose close rejects as retained cleanup", async () expect(state.attempts).toBe(2); expect(result.error.cleanupIncomplete).toBe(false); }); + +it.each([ + { code: "ENOENT", tag: "AnalysisArtifactChangedError" }, + { code: "EACCES", tag: "AnalysisAccessDeniedError" }, +])( + "retains the physical $code admission failure through close denial and retry", + async ({ code, tag }) => { + const physical = Object.assign(new Error(`admission ${code}`), { code }); + const { client, path, state } = await fixture(true, physical); + const result = await client.execute("inspect_native_dispatch_metadata", {}); + if (result.ok) throw new Error("Expected admission and cleanup failure"); + expect(result.error.cleanupIncomplete).toBe(true); + expect(result.error.cleanupResources).toEqual([path]); + expect(result.error.cleanupPrimary?._tag).toBe(tag); + expect(result.error.cleanupPrimary?.cause).toBe(physical); + state.failing = false; + expect((await client.close()).ok).toBe(true); + expect(result.error.cleanupIncomplete).toBe(false); + expect(result.error.cleanupPrimary?._tag).toBe(tag); + }, +);