mirror of
https://github.com/morluto/rea.git
synced 2026-11-07 05:56:19 -10:00
fix(adb)!: report local filesystem admission failures (#7885)
* fix(adb)!: report local filesystem admission failures * test(adb): assert filesystem error provenance
This commit is contained in:
1 parent
8c83886891
commit
a001231256
4 files changed
+317
-18
No files matched your search
@@ -0,0 +1,47 @@
|
||||
import { stat } from "node:fs/promises";
|
||||
import { AnalysisInputError } from "../domain/analysisErrorCore.js";
|
||||
|
||||
export const adbFilesystemErrorCode = (cause: unknown): string | undefined =>
|
||||
cause instanceof Error && "code" in cause && typeof cause.code === "string"
|
||||
? cause.code
|
||||
: undefined;
|
||||
|
||||
/** Admit the caller's file once, retaining the selection and the host's reason. */
|
||||
export const adbInputFileError = async (
|
||||
operation: string,
|
||||
field: "local_path" | "apk_path",
|
||||
selectedPath: string,
|
||||
absolutePath: string,
|
||||
): Promise<AnalysisInputError | undefined> => {
|
||||
let info;
|
||||
try {
|
||||
info = await stat(absolutePath);
|
||||
} catch (cause) {
|
||||
const code = adbFilesystemErrorCode(cause);
|
||||
const message =
|
||||
code === "ENOENT" || code === "ENOTDIR"
|
||||
? "The selected file does not exist or a parent is not a directory."
|
||||
: "The selected file could not be inspected.";
|
||||
return new AnalysisInputError(operation, { cause }, [
|
||||
{
|
||||
path: [field],
|
||||
reason: "invalid_value",
|
||||
message,
|
||||
observed: {
|
||||
path: selectedPath,
|
||||
...(code === undefined ? {} : { code }),
|
||||
},
|
||||
},
|
||||
]);
|
||||
}
|
||||
if (!info.isFile())
|
||||
return new AnalysisInputError(operation, undefined, [
|
||||
{
|
||||
path: [field],
|
||||
reason: "invalid_value",
|
||||
message: "The selected path is not a regular file.",
|
||||
observed: { path: selectedPath },
|
||||
},
|
||||
]);
|
||||
return undefined;
|
||||
};
|
||||
+28
-18
@@ -4,6 +4,10 @@ import {
|
||||
} from "../filesystem/callerPath.js";
|
||||
import { execFileFailureCapturedOutput } from "../process/ExecFileOutput.js";
|
||||
import { collectNumericIssues } from "./AdbNumericIssues.js";
|
||||
import {
|
||||
adbFilesystemErrorCode,
|
||||
adbInputFileError,
|
||||
} from "./AdbFilesystemAdmission.js";
|
||||
import { analysisClientFailureForOperation } from "../providers/AnalysisClientFailure.js";
|
||||
import { createHash } from "node:crypto";
|
||||
import { constants as fsConstants } from "node:fs";
|
||||
@@ -452,6 +456,18 @@ const prepareOutputDirectory = async (
|
||||
await mkdir(root, { recursive: true });
|
||||
await access(root, fsConstants.W_OK);
|
||||
} catch (cause) {
|
||||
const code = adbFilesystemErrorCode(cause);
|
||||
if (code === "EEXIST" || code === "ENOTDIR")
|
||||
return err(
|
||||
new AnalysisInputError(operation, { cause }, [
|
||||
{
|
||||
path: ["output_directory"],
|
||||
reason: "invalid_value",
|
||||
message: "The selected output path or a parent is not a directory.",
|
||||
observed: { path: directory, code },
|
||||
},
|
||||
]),
|
||||
);
|
||||
return err(
|
||||
new AnalysisOutputError(
|
||||
operation,
|
||||
@@ -2465,16 +2481,13 @@ export class AdbProvider {
|
||||
signal: AbortSignal | undefined,
|
||||
): Promise<Result<AnalysisExecution, AnalysisError>> {
|
||||
const localPath = absoluteCallerPath(input.local_path);
|
||||
const info = await stat(localPath).then(
|
||||
(value) => value,
|
||||
() => null,
|
||||
const invalid = await adbInputFileError(
|
||||
operation,
|
||||
"local_path",
|
||||
input.local_path,
|
||||
localPath,
|
||||
);
|
||||
if (info === null || !info.isFile())
|
||||
return err(
|
||||
new AnalysisInputError(operation, {
|
||||
cause: new Error(`The local file does not exist: ${localPath}`),
|
||||
}),
|
||||
);
|
||||
if (invalid !== undefined) return err(invalid);
|
||||
return this.#withRuntimeRoot(
|
||||
operation,
|
||||
{
|
||||
@@ -2671,16 +2684,13 @@ export class AdbProvider {
|
||||
signal: AbortSignal | undefined,
|
||||
): Promise<Result<AnalysisExecution, AnalysisError>> {
|
||||
const apkPath = absoluteCallerPath(input.apk_path);
|
||||
const info = await stat(apkPath).then(
|
||||
(value) => value,
|
||||
() => null,
|
||||
const invalid = await adbInputFileError(
|
||||
operation,
|
||||
"apk_path",
|
||||
input.apk_path,
|
||||
apkPath,
|
||||
);
|
||||
if (info === null || !info.isFile())
|
||||
return err(
|
||||
new AnalysisInputError(operation, {
|
||||
cause: new Error(`The local APK does not exist: ${apkPath}`),
|
||||
}),
|
||||
);
|
||||
if (invalid !== undefined) return err(invalid);
|
||||
return this.#withRuntimeRoot(
|
||||
operation,
|
||||
{
|
||||
|
||||
@@ -0,0 +1,126 @@
|
||||
import { Client, InMemoryTransport } from "@modelcontextprotocol/client";
|
||||
import { spawnSync } from "node:child_process";
|
||||
import { chmod, mkdir, readFile, symlink, writeFile } from "node:fs/promises";
|
||||
import { join, relative } from "node:path";
|
||||
import { expect, it, onTestFinished } from "vitest";
|
||||
import { AdbProvider } from "../../../src/adb/AdbProvider.js";
|
||||
import { createServer } from "../../../src/server/createServer.js";
|
||||
import { createTestBinarySession } from "../../fixtures/binarySession.js";
|
||||
import { parseMcpToolError } from "../../fixtures/mcpToolError.js";
|
||||
import { createTestTempDirectory } from "../../fixtures/temporaryDirectory.js";
|
||||
|
||||
it.skipIf(process.platform === "win32")(
|
||||
"reports each refused local file before device commands",
|
||||
async () => {
|
||||
const root = await createTestTempDirectory("rea-adb-input-refusal-");
|
||||
const adb = join(root, "adb");
|
||||
const calls = join(root, "calls");
|
||||
await mkdir(join(root, "directory"));
|
||||
await symlink("loop", join(root, "loop"));
|
||||
await writeFile(
|
||||
adb,
|
||||
`#!/bin/sh\nprintf '%s\\n' "$*" >> '${calls}'\nif [ "$1" = version ]; then printf 'Android Debug Bridge version 1.0.41\\nVersion 34.0.5-fixture\\n'; exit 0; fi\nexit 97\n`,
|
||||
);
|
||||
await chmod(adb, 0o755);
|
||||
const server = createServer(
|
||||
{
|
||||
kind: "session",
|
||||
session: createTestBinarySession(() => {
|
||||
throw new Error("unexpected provider");
|
||||
}),
|
||||
},
|
||||
{
|
||||
adbDeviceAnalysis: new AdbProvider({
|
||||
environment: { REA_ADB_PATH: adb },
|
||||
}),
|
||||
},
|
||||
);
|
||||
onTestFinished(() => server.close());
|
||||
const [clientTransport, serverTransport] =
|
||||
InMemoryTransport.createLinkedPair();
|
||||
await server.connect(serverTransport);
|
||||
const client = new Client({ name: "adb-input-refusal", version: "1" });
|
||||
onTestFinished(() => client.close());
|
||||
await client.connect(clientTransport);
|
||||
for (const { selected, hostCode } of [
|
||||
{
|
||||
selected: relative(process.cwd(), join(root, "missing")),
|
||||
hostCode: "ENOENT",
|
||||
},
|
||||
{ selected: join(root, "directory"), hostCode: undefined },
|
||||
{ selected: join(root, "loop"), hostCode: "ELOOP" },
|
||||
]) {
|
||||
for (const [name, field] of [
|
||||
["push_adb_file", "local_path"],
|
||||
["install_adb_package", "apk_path"],
|
||||
] as const) {
|
||||
const result = await client.callTool({
|
||||
name,
|
||||
arguments: {
|
||||
serial: "device",
|
||||
[field]: selected,
|
||||
...(name === "push_adb_file"
|
||||
? { device_path: "/sdcard/file" }
|
||||
: {}),
|
||||
},
|
||||
});
|
||||
const { error } = parseMcpToolError(result);
|
||||
expect(error).toMatchObject({
|
||||
code: "invalid_request",
|
||||
details: {
|
||||
issues: [
|
||||
{
|
||||
path: [field],
|
||||
reason: "invalid_value",
|
||||
observed: {
|
||||
path: selected,
|
||||
...(hostCode === undefined ? {} : { code: hostCode }),
|
||||
},
|
||||
message: expect.any(String),
|
||||
},
|
||||
],
|
||||
},
|
||||
});
|
||||
const cli = spawnSync(
|
||||
process.execPath,
|
||||
[
|
||||
"scripts/rea.mjs",
|
||||
name.replaceAll("_", "-"),
|
||||
"device",
|
||||
selected,
|
||||
...(name === "push_adb_file" ? ["/sdcard/file"] : []),
|
||||
"--json",
|
||||
],
|
||||
{
|
||||
encoding: "utf8",
|
||||
env: { ...process.env, REA_ADB_PATH: adb },
|
||||
timeout: 20_000,
|
||||
},
|
||||
);
|
||||
expect(cli.status).toBe(1);
|
||||
expect(cli.stderr).toBe("");
|
||||
expect(JSON.parse(cli.stdout)).toMatchObject({
|
||||
code: "invalid_request",
|
||||
details: {
|
||||
issues: [
|
||||
{
|
||||
path: [field],
|
||||
reason: "invalid_value",
|
||||
observed: {
|
||||
path: selected,
|
||||
...(hostCode === undefined ? {} : { code: hostCode }),
|
||||
},
|
||||
},
|
||||
],
|
||||
},
|
||||
});
|
||||
}
|
||||
}
|
||||
expect(
|
||||
(await readFile(calls, "utf8"))
|
||||
.trim()
|
||||
.split("\n")
|
||||
.every((call) => call === "version"),
|
||||
).toBe(true);
|
||||
},
|
||||
);
|
||||
@@ -0,0 +1,116 @@
|
||||
import { Client, InMemoryTransport } from "@modelcontextprotocol/client";
|
||||
import { spawnSync } from "node:child_process";
|
||||
import { chmod, readFile, writeFile } from "node:fs/promises";
|
||||
import { join, relative } from "node:path";
|
||||
import { expect, it, onTestFinished } from "vitest";
|
||||
import { AdbProvider } from "../../../src/adb/AdbProvider.js";
|
||||
import { createServer } from "../../../src/server/createServer.js";
|
||||
import { createTestBinarySession } from "../../fixtures/binarySession.js";
|
||||
import { parseMcpToolError } from "../../fixtures/mcpToolError.js";
|
||||
import { createTestTempDirectory } from "../../fixtures/temporaryDirectory.js";
|
||||
|
||||
it.skipIf(process.platform === "win32")(
|
||||
"classifies output paths blocked by a file before device commands",
|
||||
async () => {
|
||||
const root = await createTestTempDirectory("rea-adb-output-refusal-");
|
||||
const adb = join(root, "adb");
|
||||
const calls = join(root, "calls");
|
||||
const file = join(root, "file");
|
||||
await writeFile(file, "untouched");
|
||||
await writeFile(
|
||||
adb,
|
||||
`#!/bin/sh\nprintf '%s\\n' "$*" >> '${calls}'\nif [ "$1" = version ]; then printf 'Android Debug Bridge version 1.0.41\\nVersion 34.0.5-fixture\\n'; exit 0; fi\nexit 97\n`,
|
||||
);
|
||||
await chmod(adb, 0o755);
|
||||
const server = createServer(
|
||||
{
|
||||
kind: "session",
|
||||
session: createTestBinarySession(() => {
|
||||
throw new Error("unexpected provider");
|
||||
}),
|
||||
},
|
||||
{
|
||||
adbDeviceAnalysis: new AdbProvider({
|
||||
environment: { REA_ADB_PATH: adb },
|
||||
}),
|
||||
},
|
||||
);
|
||||
onTestFinished(() => server.close());
|
||||
const [clientTransport, serverTransport] =
|
||||
InMemoryTransport.createLinkedPair();
|
||||
await server.connect(serverTransport);
|
||||
const client = new Client({ name: "adb-output-refusal", version: "1" });
|
||||
onTestFinished(() => client.close());
|
||||
await client.connect(clientTransport);
|
||||
for (const { output, hostCode } of [
|
||||
{ output: relative(process.cwd(), file), hostCode: "EEXIST" },
|
||||
{ output: join(file, "child"), hostCode: "ENOTDIR" },
|
||||
]) {
|
||||
for (const name of [
|
||||
"pull_adb_file",
|
||||
"capture_adb_screen",
|
||||
"collect_adb_bugreport",
|
||||
] as const) {
|
||||
const args = {
|
||||
serial: "device",
|
||||
output_directory: output,
|
||||
...(name === "pull_adb_file" ? { device_path: "/sdcard/file" } : {}),
|
||||
};
|
||||
const { error } = parseMcpToolError(
|
||||
await client.callTool({ name, arguments: args }),
|
||||
);
|
||||
expect(error).toMatchObject({
|
||||
code: "invalid_request",
|
||||
details: {
|
||||
issues: [
|
||||
{
|
||||
path: ["output_directory"],
|
||||
reason: "invalid_value",
|
||||
observed: { path: output, code: hostCode },
|
||||
message: expect.any(String),
|
||||
},
|
||||
],
|
||||
},
|
||||
});
|
||||
const cli = spawnSync(
|
||||
process.execPath,
|
||||
[
|
||||
"scripts/rea.mjs",
|
||||
name.replaceAll("_", "-"),
|
||||
"device",
|
||||
...(name === "pull_adb_file" ? ["/sdcard/file"] : []),
|
||||
"--output-directory",
|
||||
output,
|
||||
"--json",
|
||||
],
|
||||
{
|
||||
encoding: "utf8",
|
||||
env: { ...process.env, REA_ADB_PATH: adb },
|
||||
timeout: 20_000,
|
||||
},
|
||||
);
|
||||
expect(cli.status).toBe(1);
|
||||
expect(cli.stderr).toBe("");
|
||||
expect(JSON.parse(cli.stdout)).toMatchObject({
|
||||
code: "invalid_request",
|
||||
details: {
|
||||
issues: [
|
||||
{
|
||||
path: ["output_directory"],
|
||||
reason: "invalid_value",
|
||||
observed: { path: output, code: hostCode },
|
||||
},
|
||||
],
|
||||
},
|
||||
});
|
||||
}
|
||||
}
|
||||
expect(await readFile(file, "utf8")).toBe("untouched");
|
||||
expect(
|
||||
(await readFile(calls, "utf8"))
|
||||
.trim()
|
||||
.split("\n")
|
||||
.every((call) => call === "version"),
|
||||
).toBe(true);
|
||||
},
|
||||
);
|
||||
Reference in new issue
Block a user