mirror of
https://github.com/morluto/rea.git
synced 2026-10-19 04:19:54 -05:00
fix(apktool): attribute decode refusals to the selected APK
Carry caller-selected target identity separately from launcher selection. Only fully collected ordinary nonzero decode exits represent target refusals; startup, preparation, capture and signal failures retain provider attribution. Reuse one command/preparation output projection for probe and decode errors. Adapt the target-path correction from PR #3462 (08d3d0d3a0) and extend its producer-boundary classification and regression coverage. Validation: 131 focused tests in 12 files, final 50 source checks, build, typecheck/lint and docs. Ten regression controls fail on baseline4844ae812. Twenty-four tiny actual CLI/stdio MCP refusal tasks preserve every projected field except the corrected message/path. Authored launchers only; no real Apktool/JVM or native Windows claim. Heavy stress/profiling remained stopped. Closes #3340 Co-authored-by: Peter <304086198+peter-research@users.noreply.github.com>
This commit is contained in:
1 parent
4844ae812b
commit
cc038da218
6 files changed
+217
-16
No files matched your search
@@ -83,9 +83,11 @@ select Android's `values-pt-rBR`; Android qualifiers such as `pt-rBR` and
|
||||
coverage and an explicit limit, rather than reported as missing.
|
||||
- `decoded_file_count` is capped at 100,000 files; the cap is reported in
|
||||
the limitations when hit.
|
||||
- Apktool's own decode failures (`AndrolibException`, directory traversal
|
||||
refusals) surface as `unsupported_target` with apktool's reason preserved
|
||||
verbatim.
|
||||
- An ordinary nonzero decode exit surfaces as `unsupported_target` naming
|
||||
the caller-selected APK, with Apktool's diagnostic and captured output.
|
||||
Launcher startup, ownership preparation, signal termination and incomplete
|
||||
output collection remain provider failures; they do not establish that the
|
||||
selected APK is unsupported.
|
||||
|
||||
## Real verification
|
||||
|
||||
|
||||
@@ -1,8 +1,10 @@
|
||||
import { OwnedCommandFailure } from "../process/OwnedCommand.js";
|
||||
import { DarwinProcessOwnershipInspectionError } from "../process/DarwinProcessRunTokenReader.js";
|
||||
import type { AnalysisCapturedOutput } from "../domain/analysisErrorBase.js";
|
||||
import type { ProviderProcessSnapshot } from "../process/ProviderProcess.js";
|
||||
|
||||
/** Project the command owner's bounded output without losing examined byte counts. */
|
||||
export const capturedApktoolCommandOutput = (
|
||||
const capturedApktoolCommandOutput = (
|
||||
output: ProviderProcessSnapshot,
|
||||
): AnalysisCapturedOutput => ({
|
||||
stdout: output.stdout.text,
|
||||
@@ -14,3 +16,16 @@ export const capturedApktoolCommandOutput = (
|
||||
exit_code: output.exitCode ?? null,
|
||||
signal: output.signal ?? null,
|
||||
});
|
||||
|
||||
/** The command owner selects command output or preparation output, once. */
|
||||
export const capturedApktoolFailureOutput = (
|
||||
cause: unknown,
|
||||
): AnalysisCapturedOutput | undefined => {
|
||||
if (cause instanceof OwnedCommandFailure)
|
||||
return cause.snapshot === null
|
||||
? cause.preparationFailure?.capturedOutput
|
||||
: capturedApktoolCommandOutput(cause.snapshot);
|
||||
return cause instanceof DarwinProcessOwnershipInspectionError
|
||||
? cause.capturedOutput
|
||||
: undefined;
|
||||
};
|
||||
@@ -1,3 +1,6 @@
|
||||
import { writeFile } from "node:fs/promises";
|
||||
import { join } from "node:path";
|
||||
import { createTestTempDirectory } from "../../tests/fixtures/temporaryDirectory.js";
|
||||
import { afterEach, expect, it, vi } from "vitest";
|
||||
import { createDeferred } from "../../tests/fixtures/binarySession.js";
|
||||
import { AnalysisError } from "../domain/analysisErrorBase.js";
|
||||
@@ -376,3 +379,99 @@ it("preserves an opaque cancellation reason before acquiring a command", async (
|
||||
await client.close();
|
||||
}
|
||||
});
|
||||
|
||||
it.each([
|
||||
"refusal",
|
||||
"spawn",
|
||||
"collection",
|
||||
"signal",
|
||||
"incomplete",
|
||||
"preparation",
|
||||
] as const)(
|
||||
"keeps target identity separate from a %s command outcome",
|
||||
async (kind) => {
|
||||
const root = await createTestTempDirectory("rea-apktool-attribution-");
|
||||
const path = join(root, "caller-selected.apk");
|
||||
await writeFile(path, "authored input");
|
||||
const client = provider();
|
||||
const output = {
|
||||
stdout: { text: "authored stdout", bytes: 15, observedBytes: 15 },
|
||||
stderr: { text: "authored decode refusal", bytes: 23, observedBytes: 23 },
|
||||
exitCode: 1,
|
||||
signal: null,
|
||||
diagnosticTruncated: false,
|
||||
} satisfies Awaited<ReturnType<typeof runOwnedCommand>>;
|
||||
const compiler = {
|
||||
stdout: "compiler output",
|
||||
stderr: "compiler diagnostic",
|
||||
truncated: false,
|
||||
stdout_bytes: 15,
|
||||
stderr_bytes: 19,
|
||||
exit_code: 1,
|
||||
signal: null,
|
||||
};
|
||||
const failure =
|
||||
kind === "preparation"
|
||||
? new DarwinProcessOwnershipInspectionError(
|
||||
"authored preparation failure",
|
||||
{ capturedOutput: compiler },
|
||||
)
|
||||
: new OwnedCommandFailure(
|
||||
"process",
|
||||
"authored command failure",
|
||||
{
|
||||
...output,
|
||||
...(kind === "signal"
|
||||
? { exitCode: null, signal: "SIGTERM" }
|
||||
: {}),
|
||||
...(kind === "incomplete" ? { outputIncomplete: true } : {}),
|
||||
},
|
||||
null,
|
||||
{
|
||||
...(kind === "spawn"
|
||||
? { processError: new Error("authored startup failure") }
|
||||
: {}),
|
||||
...(kind === "collection" ? { collectionFailure: true } : {}),
|
||||
},
|
||||
);
|
||||
vi.mocked(runOwnedCommand)
|
||||
.mockImplementationOnce(async (_spawn, limits) => {
|
||||
limits.observeCompleted?.(version);
|
||||
return version;
|
||||
})
|
||||
.mockRejectedValueOnce(failure);
|
||||
try {
|
||||
const result = await client.execute({
|
||||
operation: "decode_android_resources",
|
||||
input: { path, include_strings: true },
|
||||
});
|
||||
if (result.ok) throw new Error("Expected authored failure");
|
||||
if (kind === "refusal") {
|
||||
expect(result.error).toMatchObject({
|
||||
_tag: "AnalysisUnsupportedTargetError",
|
||||
path,
|
||||
});
|
||||
expect(result.error.message).toContain(path);
|
||||
expect(result.error.message).not.toContain("/authored/apktool");
|
||||
} else
|
||||
expect(result.error).toMatchObject({
|
||||
_tag: "ProviderAdapterError",
|
||||
diagnostics: {
|
||||
target_path: path,
|
||||
selected_command: "/authored/apktool",
|
||||
},
|
||||
});
|
||||
expect(result.error.capturedOutput).toMatchObject(
|
||||
kind === "preparation"
|
||||
? compiler
|
||||
: { stdout: output.stdout.text, stderr: output.stderr.text },
|
||||
);
|
||||
expect(result.error.partialObservation).toMatchObject({
|
||||
kind: "file-snapshot-hash",
|
||||
path,
|
||||
});
|
||||
} finally {
|
||||
await client.close();
|
||||
}
|
||||
},
|
||||
);
|
||||
@@ -1,4 +1,4 @@
|
||||
import { capturedApktoolCommandOutput } from "./ApktoolCommandOutput.js";
|
||||
import { capturedApktoolFailureOutput } from "./ApktoolCommandOutput.js";
|
||||
import { jsonValueSchema } from "../domain/jsonValue.js";
|
||||
import { ok, err, type Result } from "../domain/result.js";
|
||||
import { ApktoolCommandCleanup } from "./ApktoolCommandCleanup.js";
|
||||
@@ -429,10 +429,7 @@ const versionProbeFailure = (
|
||||
: cause instanceof DarwinProcessOwnershipInspectionError
|
||||
? cause
|
||||
: undefined;
|
||||
const output =
|
||||
cause instanceof OwnedCommandFailure && cause.snapshot !== null
|
||||
? capturedApktoolCommandOutput(cause.snapshot)
|
||||
: preparation?.capturedOutput;
|
||||
const output = capturedApktoolFailureOutput(cause);
|
||||
if (cause instanceof OwnedCommandFailure && cause.reason === "cancelled")
|
||||
return new AnalysisCancelledError("inspect_apktool_client", {
|
||||
cause: cause.cancellation?.reason,
|
||||
|
||||
@@ -391,7 +391,12 @@ it("reports apktool's own refusal for undecodable targets", async () => {
|
||||
operation: "decode_android_resources",
|
||||
input: { path: apk.path, include_strings: true },
|
||||
});
|
||||
expect(projected).toMatchObject({ code: "unsupported_target" });
|
||||
expect(projected).toMatchObject({
|
||||
code: "unsupported_target",
|
||||
details: { path: apk.path },
|
||||
});
|
||||
expect(projected.message).toContain(apk.path);
|
||||
expect(projected.message).not.toContain(command);
|
||||
expect(projected.message).toContain("Could not decode");
|
||||
});
|
||||
|
||||
@@ -858,3 +863,55 @@ it("joins a late workspace allocation during shutdown and removes its owned root
|
||||
await (closing ?? provider.close());
|
||||
}
|
||||
});
|
||||
|
||||
it.each(["first-selected.apk", "second-selected.apk"])(
|
||||
"attributes a decode refusal to %s without substituting the launcher",
|
||||
async (name) => {
|
||||
const { command, root } = await writeApktoolStub(
|
||||
decodeStub({ failDecode: true }),
|
||||
);
|
||||
const path = join(root, name);
|
||||
await writeFile(path, "authored target");
|
||||
const provider = providerOver(command);
|
||||
try {
|
||||
const projected = await refusedBy(provider, {
|
||||
operation: "decode_android_resources",
|
||||
input: { path, include_strings: true },
|
||||
});
|
||||
expect(projected).toMatchObject({
|
||||
code: "unsupported_target",
|
||||
details: { path, captured_output: { exit_code: 1 } },
|
||||
});
|
||||
expect(projected.message).toContain(path);
|
||||
expect(projected.message).not.toContain(command);
|
||||
} finally {
|
||||
await provider.close();
|
||||
}
|
||||
},
|
||||
);
|
||||
|
||||
it("reports launcher disappearance after version admission as a provider failure", async () => {
|
||||
const { command, root } = await writeApktoolStub(
|
||||
() =>
|
||||
`if [ "$1" = "--version" ]; then rm -- "$0"; ${VERSION_BLOCK}; exit 0; fi`,
|
||||
);
|
||||
const apk = await apkFixture(root);
|
||||
const provider = providerOver(command);
|
||||
try {
|
||||
const result = await provider.execute({
|
||||
operation: "decode_android_resources",
|
||||
input: { path: apk.path, include_strings: true },
|
||||
});
|
||||
if (result.ok) throw new Error("Expected missing launcher");
|
||||
expect(result.error).toMatchObject({
|
||||
_tag: "ProviderAdapterError",
|
||||
diagnostics: { target_path: apk.path, selected_command: command },
|
||||
});
|
||||
expect(projectAnalysisError(result.error).code).not.toBe(
|
||||
"unsupported_target",
|
||||
);
|
||||
expect(result.error.cause).toBeDefined();
|
||||
} finally {
|
||||
await provider.close();
|
||||
}
|
||||
});
|
||||
@@ -12,7 +12,7 @@ import {
|
||||
artifactFileCancellationCause,
|
||||
} from "../artifacts/ArtifactFileOperation.js";
|
||||
import { PrivateRuntimeRoot } from "../process/PrivateRuntimeRoot.js";
|
||||
import { capturedApktoolCommandOutput } from "./ApktoolCommandOutput.js";
|
||||
import { capturedApktoolFailureOutput } from "./ApktoolCommandOutput.js";
|
||||
import { ApktoolCommandCleanup } from "./ApktoolCommandCleanup.js";
|
||||
import { AnalysisError } from "../domain/analysisErrorBase.js";
|
||||
import { analysisClientFailureForOperation } from "../providers/AnalysisClientFailure.js";
|
||||
@@ -41,6 +41,7 @@ import {
|
||||
} from "../domain/analysisErrorCore.js";
|
||||
import { analysisErrorWithCleanupFailure } from "../domain/analysisErrorCleanup.js";
|
||||
import { err, ok, type Result } from "../domain/result.js";
|
||||
import { ProviderAdapterError } from "../domain/providerAdapterError.js";
|
||||
import { ProviderCleanupError } from "../domain/providerCleanupError.js";
|
||||
import {
|
||||
OwnedCommandFailure,
|
||||
@@ -97,6 +98,7 @@ interface DecodeLimits {
|
||||
|
||||
const runApktool = async (
|
||||
operation: string,
|
||||
targetPath: string,
|
||||
selection: ApktoolCommandSelection,
|
||||
arguments_: readonly string[],
|
||||
limits: DecodeLimits,
|
||||
@@ -147,7 +149,7 @@ const runApktool = async (
|
||||
return err(
|
||||
commands.retainFailure(
|
||||
cause,
|
||||
apktoolCommandFailure(cause, operation, selection, limits),
|
||||
apktoolCommandFailure(cause, operation, targetPath, selection, limits),
|
||||
operation,
|
||||
root,
|
||||
),
|
||||
@@ -158,17 +160,22 @@ const runApktool = async (
|
||||
const apktoolCommandFailure = (
|
||||
cause: unknown,
|
||||
operation: string,
|
||||
targetPath: string,
|
||||
selection: ApktoolCommandSelection,
|
||||
limits: DecodeLimits,
|
||||
): AnalysisError => {
|
||||
const capturedOutput = capturedApktoolFailureOutput(cause);
|
||||
if (!(cause instanceof OwnedCommandFailure))
|
||||
return new AnalysisProtocolError(`apktool failed for ${operation}`, {
|
||||
return new ProviderAdapterError("apktool", operation, {
|
||||
cause,
|
||||
diagnostics: {
|
||||
selected_command: selection.command,
|
||||
target_path: targetPath,
|
||||
},
|
||||
...(capturedOutput === undefined ? {} : { capturedOutput }),
|
||||
});
|
||||
|
||||
const output = cause.snapshot;
|
||||
const capturedOutput =
|
||||
output === null ? undefined : capturedApktoolCommandOutput(output);
|
||||
if (cause.reason === "cancelled")
|
||||
return new AnalysisCancelledError(operation, {
|
||||
cause: cause.cancellation?.reason,
|
||||
@@ -190,9 +197,32 @@ const apktoolCommandFailure = (
|
||||
...(capturedOutput === undefined ? {} : { capturedOutput }),
|
||||
},
|
||||
);
|
||||
// A physical launcher/capture failure does not establish target refusal.
|
||||
// Only a fully collected ordinary nonzero decode exit carries that meaning.
|
||||
if (
|
||||
cause.processError !== undefined ||
|
||||
cause.collectionFailure ||
|
||||
cause.preparationFailure !== undefined ||
|
||||
output === null ||
|
||||
output.exitCode === null ||
|
||||
output.exitCode === undefined ||
|
||||
output.exitCode === 0 ||
|
||||
output.signal !== null ||
|
||||
output.outputIncomplete === true ||
|
||||
output.diagnosticTruncated === true
|
||||
)
|
||||
return new ProviderAdapterError("apktool", operation, {
|
||||
cause,
|
||||
userMessage: `The selected Apktool command failed while decoding ${targetPath}`,
|
||||
diagnostics: {
|
||||
selected_command: selection.command,
|
||||
target_path: targetPath,
|
||||
},
|
||||
...(capturedOutput === undefined ? {} : { capturedOutput }),
|
||||
});
|
||||
return new AnalysisUnsupportedTargetError(
|
||||
operation,
|
||||
selection.command,
|
||||
targetPath,
|
||||
output?.stderr.text.trim().slice(0, STDERR_EXCERPT_BYTES) ||
|
||||
`apktool exited with code ${String(output?.exitCode ?? "unknown")} for ${operation}`,
|
||||
{
|
||||
@@ -503,6 +533,7 @@ export class ApktoolProvider {
|
||||
await mkdir(framework);
|
||||
const decoded = await runApktool(
|
||||
operation,
|
||||
input.path,
|
||||
selection,
|
||||
[
|
||||
"d",
|
||||
|
||||
Reference in new issue
Block a user