diff --git a/docs/adb-device-analysis.md b/docs/adb-device-analysis.md index b18e7b4fbe..7e10ac33d6 100644 --- a/docs/adb-device-analysis.md +++ b/docs/adb-device-analysis.md @@ -153,8 +153,10 @@ and during shutdown. - **Mutations are explicit.** `push_adb_file`, `install_adb_package`, and `uninstall_adb_package` declare `mutatesTarget` and never run implicitly: pushes check for existing device files before transfer and refuse them - unless `overwrite` is set. The check is not atomic, so a different process - can create the path before `adb push` writes it. When available, the device's + unless `overwrite` is set. A failed or unexpected existence probe stops + transfer even with `overwrite`; only a clean `test -e` exit of 1 with no + output is interpreted as absence. The check is not atomic, so a different + process can create the path before `adb push` writes it. When available, the device's `sha256sum` verifies the transfer; installs preserve the device's `Failure [...]` reason verbatim. - **Producer identity stays attached.** Every result embeds the producing diff --git a/src/adb/AdbCommand.test.ts b/src/adb/AdbCommand.test.ts new file mode 100644 index 0000000000..63bb5ab0aa --- /dev/null +++ b/src/adb/AdbCommand.test.ts @@ -0,0 +1,112 @@ +import { expect, it, vi } from "vitest"; +import { execFileBytes } from "../process/ExecFileOutput.js"; +import { probeAdbPathExistence } from "./AdbCommand.js"; + +vi.mock("../process/ExecFileOutput.js", async (importOriginal) => ({ + ...(await importOriginal()), + execFileBytes: vi.fn(), +})); + +const probe = (signal?: AbortSignal) => + probeAdbPathExistence( + "push_adb_file", + "device", + "/device/path", + "/host/adb", + { + timeoutMs: 100, + maxOutputBytes: 1024, + }, + signal, + ); + +it("recognizes only the completed empty false result as absence", async () => { + vi.mocked(execFileBytes).mockRejectedValueOnce( + Object.assign(new Error("test false"), { + code: 1, + stdout: "", + stderr: "", + signal: null, + killed: false, + }), + ); + await expect(probe()).resolves.toEqual({ ok: true, value: false }); +}); + +it.each([ + { code: 2 }, + { code: "ENOENT" }, + { signal: "SIGTERM" }, + { collectionFailure: true }, + { stdout: "unexpected" }, + { stderr: " " }, +])("keeps nonstandard false completion unknown: %j", async (extra) => { + vi.mocked(execFileBytes).mockRejectedValueOnce( + Object.assign(new Error("probe failed"), { + code: 1, + stdout: "", + stderr: "", + signal: null, + killed: false, + ...extra, + }), + ); + const result = await probe(); + expect(result).toMatchObject({ + ok: false, + error: { + _tag: "AnalysisProtocolError", + diagnostics: { destination_existence: "unknown" }, + }, + }); +}); + +it.each([ + [{ killed: true }, "AnalysisTimeoutError"], + [{ outputTruncated: true }, "AnalysisResourceConstraintError"], + [{ code: "ABORT_ERR" }, "AnalysisCancelledError"], +] as const)( + "preserves a stopped or incomplete probe: %j", + async (extra, tag) => { + vi.mocked(execFileBytes).mockRejectedValueOnce( + Object.assign(new Error("probe failed"), { + code: 1, + stdout: "", + stderr: "", + signal: null, + killed: false, + ...extra, + }), + ); + await expect(probe()).resolves.toMatchObject({ + ok: false, + error: { _tag: tag }, + }); + }, +); + +it("recognizes an empty successful probe as existence", async () => { + vi.mocked(execFileBytes).mockResolvedValueOnce({ + stdout: Buffer.alloc(0), + stderr: Buffer.alloc(0), + }); + await expect(probe()).resolves.toEqual({ ok: true, value: true }); +}); + +it.each(["stdout", "stderr"] as const)( + "retains unexpected successful %s as unknown", + async (stream) => { + vi.mocked(execFileBytes).mockResolvedValueOnce({ + stdout: Buffer.alloc(0), + stderr: Buffer.alloc(0), + [stream]: Buffer.from("unexpected"), + }); + await expect(probe()).resolves.toMatchObject({ + ok: false, + error: { + _tag: "AnalysisProtocolError", + capturedOutput: { [stream]: "unexpected", exit_code: 0 }, + }, + }); + }, +); diff --git a/src/adb/AdbCommand.ts b/src/adb/AdbCommand.ts new file mode 100644 index 0000000000..5ab13e4b57 --- /dev/null +++ b/src/adb/AdbCommand.ts @@ -0,0 +1,244 @@ +import { + AnalysisCancelledError, + AnalysisProtocolError, + AnalysisResourceConstraintError, + AnalysisTimeoutError, + AnalysisUnsupportedTargetError, +} from "../domain/analysisErrorCore.js"; +import type { AnalysisError } from "../domain/analysisErrorBase.js"; +import { err, ok, type Result } from "../domain/result.js"; +import { + execFileBytes, + execFileOutputFailure, + type ExecFileOutputFailure, +} from "../process/ExecFileOutput.js"; +import { packageChangeFailureReason } from "./AdbShellOutput.js"; + +const STDERR_EXCERPT_BYTES = 512; + +/** Parsed command failure owned by this adapter, separate from its public tag. */ +class AdbProtocolError extends AnalysisProtocolError { + constructor( + message: string, + readonly commandFailure: ExecFileOutputFailure, + cause: unknown, + ) { + super(message, { cause }); + } +} + +/** adb joins shell operands into a remote POSIX command; quote at that boundary. */ +const quoteShellOperand = (value: string): string => + /^[A-Za-z0-9_./:=@+,-]+$/u.test(value) + ? value + : "'" + value.replaceAll("'", "'\\''") + "'"; + +export const adbArguments = ( + serial: string | undefined, + arguments_: readonly string[], +): readonly string[] => { + const operands = + arguments_[0] === "shell" + ? ["shell", ...arguments_.slice(1).map(quoteShellOperand)] + : [...arguments_]; + return serial === undefined ? operands : ["-s", serial, ...operands]; +}; + +export interface AdbRunLimits { + readonly timeoutMs: number; + readonly maxOutputBytes: number; +} + +/** Run one bounded adb invocation and keep its exact failure shape. */ +export const runAdbCapture = async ( + operation: string, + targetPath: string, + binary: string, + arguments_: readonly string[], + limits: AdbRunLimits, + signal: AbortSignal | undefined, +): Promise> => { + if (signal?.aborted === true) + return err(new AnalysisCancelledError(operation, { cause: signal.reason })); + try { + return ok( + await execFileBytes(binary, arguments_, { + timeout: limits.timeoutMs, + maxBuffer: limits.maxOutputBytes, + stopSignal: "SIGTERM", + ...(signal === undefined ? {} : { signal }), + }), + ); + } catch (cause) { + const failure = execFileOutputFailure(cause); + if (signal?.aborted || failure?.code === "ABORT_ERR") + return err(new AnalysisCancelledError(operation, { cause })); + if (failure === undefined) + return err( + new AnalysisProtocolError(`adb failed for ${operation}`, { cause }), + ); + if (failure.outputTruncated) + return err( + new AnalysisResourceConstraintError( + operation, + "memory", + `adb output exceeded the ${String(limits.maxOutputBytes)}-byte capture budget`, + { max_output_bytes: limits.maxOutputBytes }, + { cause }, + ), + ); + const excerpt = failure.stderr.trim().slice(0, STDERR_EXCERPT_BYTES); + if (/unauthorized/iu.test(excerpt)) + return err( + new AnalysisUnsupportedTargetError(operation, targetPath, excerpt, { + cause, + }), + ); + if ( + /device[^\n]*not found|device offline|still authorizing|more than one device/iu.test( + excerpt, + ) + ) + return err( + new AnalysisUnsupportedTargetError(operation, targetPath, excerpt, { + cause, + }), + ); + if ( + /No such file or directory|Permission denied|read: Permission denied/iu.test( + excerpt, + ) + ) + return err( + new AnalysisUnsupportedTargetError(operation, targetPath, excerpt, { + cause, + }), + ); + if (failure.killed) + return err( + new AnalysisTimeoutError(operation, limits.timeoutMs, { cause }), + ); + return err( + new AdbProtocolError( + excerpt === "" + ? `adb exited with code ${String(failure.code)} for ${operation}` + : excerpt, + failure, + cause, + ), + ); + } +}; + +/** Decode textual adb protocols after the shared byte capture and error boundary. */ +export const runAdb = async ( + ...arguments_: Parameters +): Promise< + Result<{ stdout: string; stderr: string; stdoutBytes: number }, AnalysisError> +> => { + const captured = await runAdbCapture(...arguments_); + if (!captured.ok) return captured; + return ok({ + stdout: captured.value.stdout.toString("utf8"), + stderr: captured.value.stderr.toString("utf8"), + // Byte count of the captured output before UTF-8 decoding, so malformed + // sequences replaced by U+FFFD still report their true captured size. + stdoutBytes: captured.value.stdout.byteLength, + }); +}; + +/** + * adb exits nonzero when the device refuses a package change, printing the + * `Failure [...]` verdict to stderr (install) or stdout (uninstall). Read the + * refusal from that failed run so it is not reported as unreadable output. + */ +export const packageChangeRefusal = ( + operation: string, + serial: string, + error: AnalysisError, +): Result | undefined => { + if (!(error instanceof AdbProtocolError)) return undefined; + const failure = error.commandFailure; + const reason = packageChangeFailureReason( + `${failure.stdout}\n${failure.stderr}`, + ); + return reason === null + ? undefined + : err( + new AnalysisUnsupportedTargetError(operation, serial, reason, { + cause: error, + }), + ); +}; + +/** Only the clean false result of test -e establishes absence. */ +export const probeAdbPathExistence = async ( + operation: string, + serial: string, + path: string, + binary: string, + limits: AdbRunLimits, + signal: AbortSignal | undefined, +): Promise> => { + const probed = await runAdbCapture( + operation, + path, + binary, + adbArguments(serial, ["shell", "test", "-e", path]), + limits, + signal, + ); + if (probed.ok) { + if (probed.value.stdout.length === 0 && probed.value.stderr.length === 0) + return ok(true); + return err( + new AnalysisProtocolError( + "The ADB path existence probe returned unexpected output; destination existence is unknown", + { + diagnostics: { destination_existence: "unknown" }, + capturedOutput: { + stdout: probed.value.stdout.toString("utf8"), + stderr: probed.value.stderr.toString("utf8"), + stdout_bytes: probed.value.stdout.length, + stderr_bytes: probed.value.stderr.length, + exit_code: 0, + signal: null, + truncated: false, + }, + }, + ), + ); + } + const error = probed.error; + if (!(error instanceof AdbProtocolError)) return probed; + const failure = error.commandFailure; + if ( + failure.code === 1 && + failure.signal === null && + !failure.killed && + !failure.outputTruncated && + failure.collectionFailure !== true && + failure.stdout === "" && + failure.stderr === "" + ) + return ok(false); + return err( + new AnalysisProtocolError(error.message, { + cause: error.cause, + diagnostics: { destination_existence: "unknown" }, + capturedOutput: { + stdout: failure.stdout, + stderr: failure.stderr, + ...(failure.stdoutBytes === undefined + ? {} + : { stdout_bytes: failure.stdoutBytes }), + ...(failure.stderrBytes === undefined + ? {} + : { stderr_bytes: failure.stderrBytes }), + exit_code: typeof failure.code === "number" ? failure.code : null, + signal: failure.signal, + truncated: failure.outputTruncated, + }, + }), + ); +}; diff --git a/src/adb/AdbProvider.ts b/src/adb/AdbProvider.ts index db696e8994..60d7b3672e 100644 --- a/src/adb/AdbProvider.ts +++ b/src/adb/AdbProvider.ts @@ -70,9 +70,13 @@ import { import { ProviderCleanupError } from "../domain/providerCleanupError.js"; import { PrivateRuntimeRootUnavailableError } from "../process/PrivateRuntimeRootUnavailableError.js"; import { - execFileBytes, - execFileOutputFailure, -} from "../process/ExecFileOutput.js"; + adbArguments, + runAdb, + runAdbCapture, + packageChangeRefusal, + probeAdbPathExistence, + type AdbRunLimits, +} from "./AdbCommand.js"; import { AdbConfigurationFailure, adbConfigurationAvailability, @@ -102,7 +106,6 @@ import { parseBugreportCopiedPath, parseDumpsysPackageOutput, parseFeatureListOutput, - packageChangeFailureReason, parsePackageChangeOutput, parseProcessListOutput, parsePngDimensions, @@ -144,7 +147,6 @@ const SCREEN_TIMEOUT_MS = 60_000; const SCREEN_MAX_OUTPUT_BYTES = 32 * 1024 * 1024; const BUGREPORT_TIMEOUT_MS = 600_000; const BUGREPORT_MAX_OUTPUT_BYTES = 1024 * 1024; -const STDERR_EXCERPT_BYTES = 512; const UNPARSED_LINES_REPORTED = 10; /** Carry the exact digest owner through caller-specific partial-result mapping. */ @@ -364,150 +366,6 @@ const syncStagedAdbFile = async ( if (primary !== undefined) throw primary; }; -/** adb joins shell operands into a remote POSIX command; quote at that boundary. */ -const quoteShellOperand = (value: string): string => - /^[A-Za-z0-9_./:=@+,-]+$/u.test(value) - ? value - : "'" + value.replaceAll("'", "'\\''") + "'"; - -const adbArguments = ( - serial: string | undefined, - arguments_: readonly string[], -): readonly string[] => { - const operands = - arguments_[0] === "shell" - ? ["shell", ...arguments_.slice(1).map(quoteShellOperand)] - : [...arguments_]; - return serial === undefined ? operands : ["-s", serial, ...operands]; -}; - -interface AdbRunLimits { - readonly timeoutMs: number; - readonly maxOutputBytes: number; -} - -/** Run one bounded adb invocation and keep its exact failure shape. */ -const runAdbCapture = async ( - operation: string, - targetPath: string, - binary: string, - arguments_: readonly string[], - limits: AdbRunLimits, - signal: AbortSignal | undefined, -): Promise> => { - if (signal?.aborted === true) - return err(new AnalysisCancelledError(operation, { cause: signal.reason })); - try { - return ok( - await execFileBytes(binary, arguments_, { - timeout: limits.timeoutMs, - maxBuffer: limits.maxOutputBytes, - stopSignal: "SIGTERM", - ...(signal === undefined ? {} : { signal }), - }), - ); - } catch (cause) { - if (signal?.aborted || execFileOutputFailure(cause)?.code === "ABORT_ERR") - return err(new AnalysisCancelledError(operation, { cause })); - const failure = execFileOutputFailure(cause); - if (failure === undefined) - return err( - new AnalysisProtocolError(`adb failed for ${operation}`, { cause }), - ); - if (failure.outputTruncated) - return err( - new AnalysisResourceConstraintError( - operation, - "memory", - `adb output exceeded the ${String(limits.maxOutputBytes)}-byte capture budget`, - { max_output_bytes: limits.maxOutputBytes }, - { cause }, - ), - ); - const excerpt = failure.stderr.trim().slice(0, STDERR_EXCERPT_BYTES); - if (/unauthorized/iu.test(excerpt)) - return err( - new AnalysisUnsupportedTargetError(operation, targetPath, excerpt, { - cause, - }), - ); - if ( - /device[^\n]*not found|device offline|still authorizing|more than one device/iu.test( - excerpt, - ) - ) - return err( - new AnalysisUnsupportedTargetError(operation, targetPath, excerpt, { - cause, - }), - ); - if ( - /No such file or directory|Permission denied|read: Permission denied/iu.test( - excerpt, - ) - ) - return err( - new AnalysisUnsupportedTargetError(operation, targetPath, excerpt, { - cause, - }), - ); - if (failure.killed) - return err( - new AnalysisTimeoutError(operation, limits.timeoutMs, { cause }), - ); - return err( - new AnalysisProtocolError( - excerpt === "" - ? `adb exited with code ${String(failure.code)} for ${operation}` - : excerpt, - { cause }, - ), - ); - } -}; - -/** Decode textual adb protocols after the shared byte capture and error boundary. */ -const runAdb = async ( - ...arguments_: Parameters -): Promise< - Result<{ stdout: string; stderr: string; stdoutBytes: number }, AnalysisError> -> => { - const captured = await runAdbCapture(...arguments_); - if (!captured.ok) return captured; - return ok({ - stdout: captured.value.stdout.toString("utf8"), - stderr: captured.value.stderr.toString("utf8"), - // Byte count of the captured output before UTF-8 decoding, so malformed - // sequences replaced by U+FFFD still report their true captured size. - stdoutBytes: captured.value.stdout.byteLength, - }); -}; - -/** - * adb exits nonzero when the device refuses a package change, printing the - * `Failure [...]` verdict to stderr (install) or stdout (uninstall). Read the - * refusal from that failed run so it is not reported as unreadable output. - */ -const packageChangeRefusal = ( - operation: string, - serial: string, - error: AnalysisError, -): Result | undefined => { - if (!(error instanceof AnalysisProtocolError)) return undefined; - const failure = execFileOutputFailure(error.cause); - if (failure === undefined) return undefined; - const reason = packageChangeFailureReason( - `${failure.stdout}\n${failure.stderr}`, - ); - return reason === null - ? undefined - : err( - new AnalysisUnsupportedTargetError(operation, serial, reason, { - cause: error, - }), - ); -}; - const clientIdentityResult = ( selection: AdbBinarySelection, identity: AdbClientIdentity, @@ -2535,28 +2393,16 @@ export class AdbProvider { operation, signal, ); - // `test -e` exits nonzero with no adb diagnosis when the path is absent. - // Any other probe failure leaves existence unknown, so it is reported as - // such rather than treated as absence or followed by a push. - const existsProbe = await runAdb( + const existsProbe = await probeAdbPathExistence( operation, + input.serial, input.device_path, selection.binary, - adbArguments(input.serial, [ - "shell", - "test", - "-e", - input.device_path, - ]), { timeoutMs: SETTINGS_TIMEOUT_MS, maxOutputBytes: 1024 }, signal, ); - if ( - !existsProbe.ok && - !(existsProbe.error instanceof AnalysisProtocolError) - ) - return existsProbe; - const deviceExists = existsProbe.ok; + if (!existsProbe.ok) return existsProbe; + const deviceExists = existsProbe.value; if (deviceExists && !input.overwrite) return err( new AnalysisOutputError( diff --git a/tests/boundary/cli/adbCli.test.ts b/tests/boundary/cli/adbCli.test.ts index 69925565f7..e41aeb7e84 100644 --- a/tests/boundary/cli/adbCli.test.ts +++ b/tests/boundary/cli/adbCli.test.ts @@ -5,6 +5,10 @@ import { expect, it, onTestFinished } from "vitest"; import { z } from "zod"; import { toolContract } from "../../../src/contracts/toolContracts.js"; +import { + execFileOutput, + execFileOutputFailure, +} from "../../../src/process/ExecFileOutput.js"; import { expectConfigurationRefusal, serveCli as serve, @@ -14,6 +18,71 @@ import { removeTestWorkspace, } from "../../support/workspace/workspaceFixture.js"; +it.skipIf(process.platform === "win32").each([ + { exit: 1, stdout: "", stderr: "adb: error: closed", overwrite: false }, + { exit: 1, stdout: "", stderr: "adb: error: closed", overwrite: true }, + { exit: 1, stdout: "unexpected probe output", stderr: "", overwrite: false }, + { exit: 2, stdout: "", stderr: "", overwrite: false }, +])("refuses an ambiguous destination probe before push: %j", async (probe) => { + const workspace = await createTestWorkspace("rea-adb-probe-cli-"); + onTestFinished(() => removeTestWorkspace(workspace.root)); + const source = await workspace.write("payload.bin", "replacement"); + await workspace.write("device-file", "original"); + const adb = await workspace.write( + "adb", + `#!/bin/sh +root=$(CDPATH= cd -- "$(dirname -- "$0")" && pwd) +printf '%s\\n' "$3" >> "$root/calls" +if [ "$1" = version ]; then + printf 'Android Debug Bridge version 1.0.41\\nVersion 34.0.5-cli\\nInstalled as %s\\n' "$0" + exit 0 +fi +if [ "$3" = shell ] && [ "$4" = test ]; then + printf '%s' '${probe.stdout}' + printf '%s' '${probe.stderr}' >&2 + exit ${String(probe.exit)} +fi +if [ "$3" = push ]; then cp "$4" "$root/device-file"; exit 0; fi +exit 1 +`, + ); + await chmod(adb, 0o755); + const result = await execFileOutput( + process.execPath, + [ + "scripts/rea.mjs", + "push-adb-file", + "device", + source, + "/data/local/tmp/payload.bin", + ...(probe.overwrite ? ["--overwrite"] : []), + "--json", + ], + { + env: { ...process.env, REA_ADB_PATH: adb }, + timeout: 20_000, + stopSignal: "SIGTERM", + maxBuffer: 1024 * 1024, + }, + ).then( + (output) => ({ exitCode: 0, stdout: output.stdout }), + (cause: unknown) => { + const failure = execFileOutputFailure(cause); + if (failure === undefined) throw cause; + return { exitCode: failure.code, stdout: failure.stdout }; + }, + ); + expect(result.exitCode).toBe(1); + expect(JSON.parse(result.stdout)).toMatchObject({ + code: "unreadable_output", + }); + if (probe.stderr !== "") expect(result.stdout).toContain(probe.stderr); + expect(await readFile(workspace.path("device-file"), "utf8")).toBe( + "original", + ); + expect(await readFile(workspace.path("calls"), "utf8")).toBe("\nshell\n"); +}); + it.each([ [ ["inspect-adb-client", "--json"],