mirror of
https://github.com/morluto/rea
synced 2026-10-25 22:08:11 -04:00
refactor(rizin): share validated command requests across admission and execution
Resolve raw input at the application boundary and reuse its facts in both MCP admission paths. Keep cleanup, mutation retirement and execution ordering while retaining structured correction details for rejected commands. Validation: 186 focused tests, source checks, docs conformance and duplication gate passed. The new compiled CLI regression fails on the main baseline. Provider process controls use deterministic fixtures; no real Rizin verification is claimed.
This commit is contained in:
@@ -23,6 +23,7 @@ import {
|
||||
import { AnalysisOutputError } from "../../domain/analysisErrorCore.js";
|
||||
import { ProviderAdapterError } from "../../domain/providerAdapterError.js";
|
||||
import { createEvidence, type Evidence } from "../../domain/evidence.js";
|
||||
import { analysisInputErrorFromIssues } from "../../domain/inputIssueProjection.js";
|
||||
import type { JsonValue } from "../../domain/jsonValue.js";
|
||||
import { err, ok, type Result } from "../../domain/result.js";
|
||||
import {
|
||||
@@ -30,6 +31,7 @@ import {
|
||||
MAX_REVERSE_COMMAND_TIMEOUT_MS,
|
||||
objdumpInputSchema,
|
||||
rizinInputSchema,
|
||||
type RizinInput,
|
||||
} from "../../domain/reverseEngineering.js";
|
||||
import type {
|
||||
ReverseEngineeringCommandPort,
|
||||
@@ -83,6 +85,23 @@ export type ReverseEngineeringServiceRuntimeOptions = Omit<
|
||||
"commands" | "environment"
|
||||
>;
|
||||
|
||||
/** Resolve one command for shared execution and adapter admission decisions. */
|
||||
export const resolveRizinCommandRequest = (
|
||||
input: unknown,
|
||||
): Result<RizinInput, AnalysisInputError> => {
|
||||
const parsed = rizinInputSchema.safeParse(input);
|
||||
return parsed.success
|
||||
? ok(parsed.data)
|
||||
: err(
|
||||
analysisInputErrorFromIssues(
|
||||
"execute_rizin_command",
|
||||
parsed.error.issues,
|
||||
input,
|
||||
{ cause: parsed.error },
|
||||
),
|
||||
);
|
||||
};
|
||||
|
||||
/** Shared application workflow for caller-selected local analysis commands. */
|
||||
export class ReverseEngineeringService {
|
||||
readonly #environment: Readonly<NodeJS.ProcessEnv>;
|
||||
@@ -234,18 +253,24 @@ export class ReverseEngineeringService {
|
||||
rawInput: unknown,
|
||||
options: { readonly signal?: AbortSignal } = {},
|
||||
): Promise<Result<Evidence, AnalysisError>> {
|
||||
const parsed = rizinInputSchema.safeParse(rawInput);
|
||||
if (!parsed.success)
|
||||
return Promise.resolve(
|
||||
err(new AnalysisInputError("execute_rizin_command")),
|
||||
);
|
||||
const { command_timeout_ms: commandTimeoutMs, ...input } = parsed.data;
|
||||
const parsed = resolveRizinCommandRequest(rawInput);
|
||||
return parsed.ok
|
||||
? this.executeRizinCommandValidated(parsed.value, options)
|
||||
: Promise.resolve(parsed);
|
||||
}
|
||||
|
||||
/** Execute owner-validated input after the adapter has admitted its target. */
|
||||
executeRizinCommandValidated(
|
||||
request: RizinInput,
|
||||
options: { readonly signal?: AbortSignal } = {},
|
||||
): Promise<Result<Evidence, AnalysisError>> {
|
||||
const { command_timeout_ms: commandTimeoutMs, ...input } = request;
|
||||
const command = this.#commands.rizin(input);
|
||||
return this.#observe(
|
||||
"execute_rizin_command",
|
||||
command,
|
||||
input.path,
|
||||
commandTimeoutMs === undefined ? input : parsed.data,
|
||||
commandTimeoutMs === undefined ? input : request,
|
||||
options.signal,
|
||||
commandTimeoutMs ?? DEFAULT_REVERSE_COMMAND_TIMEOUT_MS,
|
||||
);
|
||||
|
||||
@@ -90,4 +90,4 @@ export type ObjdumpSelection =
|
||||
| z.input<typeof objdumpArtifactInputSchema>
|
||||
| z.input<typeof objdumpDisassemblyInputSchema>
|
||||
| z.input<typeof objdumpArchitecturesInputSchema>;
|
||||
export type RizinInput = z.input<typeof rizinInputSchema>;
|
||||
export type RizinInput = z.output<typeof rizinInputSchema>;
|
||||
|
||||
@@ -6,7 +6,10 @@ import type {
|
||||
} from "@modelcontextprotocol/server";
|
||||
import type { Logger } from "pino";
|
||||
|
||||
import type { ReverseEngineeringService } from "../application/reverse/ReverseEngineeringService.js";
|
||||
import {
|
||||
resolveRizinCommandRequest,
|
||||
type ReverseEngineeringService,
|
||||
} from "../application/reverse/ReverseEngineeringService.js";
|
||||
import type { EvidenceLookup } from "../application/EvidenceInputResolver.js";
|
||||
import type { EvidenceWriter } from "../application/investigation/InvestigationRecordPort.js";
|
||||
import type { Evidence } from "../domain/evidence.js";
|
||||
@@ -16,7 +19,6 @@ import { logToolExecution } from "./toolLogging.js";
|
||||
import { toolRegistrationOptions } from "./toolRegistrationOptions.js";
|
||||
import type { ToolResultDelivery } from "./toolResult.js";
|
||||
import type { WithAdmittedAnalysis } from "./analysisAdmission.js";
|
||||
import { rizinInputSchema } from "../domain/reverseEngineering.js";
|
||||
import { AnalysisOutputError } from "../domain/analysisErrorCore.js";
|
||||
import type { AnalysisError } from "../domain/analysisErrorBase.js";
|
||||
import type { Result } from "../domain/result.js";
|
||||
@@ -81,18 +83,18 @@ export const registerReverseEngineeringTools = (
|
||||
rizin.name,
|
||||
toolRegistrationOptions(rizin),
|
||||
async (input, context: ServerContext) => {
|
||||
const parsed = rizinInputSchema.safeParse(input);
|
||||
if (parsed.success && withAdmittedExternalMutation !== undefined) {
|
||||
const parsed = resolveRizinCommandRequest(input);
|
||||
if (parsed.ok && withAdmittedExternalMutation !== undefined) {
|
||||
const cleanup = await service.retryCleanup(rizin.name);
|
||||
if (!cleanup.ok)
|
||||
return deliverFailure(cleanup.error, rizin, delivery, recordEvidence);
|
||||
const admitted = await withAdmittedExternalMutation(
|
||||
parsed.data.path,
|
||||
parsed.value.path,
|
||||
context.mcpReq.signal,
|
||||
async () => {
|
||||
invalidateSnapshotForExternalMutation?.();
|
||||
return logToolExecution(logger, rizin.name, () =>
|
||||
service.executeRizinCommand(parsed.data, {
|
||||
service.executeRizinCommandValidated(parsed.value, {
|
||||
signal: context.mcpReq.signal,
|
||||
}),
|
||||
);
|
||||
@@ -143,15 +145,14 @@ export const registerReverseEngineeringTools = (
|
||||
withAdmittedAnalysis,
|
||||
delivery,
|
||||
async () => {
|
||||
if (
|
||||
invalidateSnapshotForExternalMutation !== undefined &&
|
||||
rizinInputSchema.safeParse(input).success
|
||||
)
|
||||
if (invalidateSnapshotForExternalMutation !== undefined && parsed.ok)
|
||||
invalidateSnapshotForExternalMutation();
|
||||
const result = await logToolExecution(logger, rizin.name, () =>
|
||||
service.executeRizinCommand(input, {
|
||||
signal: context.mcpReq.signal,
|
||||
}),
|
||||
parsed.ok
|
||||
? service.executeRizinCommandValidated(parsed.value, {
|
||||
signal: context.mcpReq.signal,
|
||||
})
|
||||
: Promise.resolve(parsed),
|
||||
);
|
||||
if (!result.ok)
|
||||
return deliverFailure(
|
||||
|
||||
@@ -50,3 +50,34 @@ cliTest(
|
||||
}
|
||||
},
|
||||
);
|
||||
|
||||
cliTest(
|
||||
"reports the rejected Rizin command before inspecting the target",
|
||||
async ({ cli }) => {
|
||||
const root = await createTestTempDirectory("rea-rizin-invalid-command-");
|
||||
const result = await cli.run({
|
||||
arguments: [
|
||||
"execute-rizin-command",
|
||||
join(root, "unopened.bin"),
|
||||
"iI\nq",
|
||||
"--format",
|
||||
"json",
|
||||
],
|
||||
cwd: root,
|
||||
});
|
||||
expect(result.exitCode).toBe(1);
|
||||
expect(analysisErrorProjectionSchema.parse(result.json)).toMatchObject({
|
||||
code: "invalid_request",
|
||||
details: {
|
||||
operation: "execute_rizin_command",
|
||||
issues: [
|
||||
{
|
||||
path: ["command"],
|
||||
reason: "invalid_value",
|
||||
message: "A single command line is required",
|
||||
},
|
||||
],
|
||||
},
|
||||
});
|
||||
},
|
||||
);
|
||||
|
||||
@@ -1,4 +1,6 @@
|
||||
import pino from "pino";
|
||||
import type { CallToolResult } from "@modelcontextprotocol/server";
|
||||
import { parseMcpToolError } from "../../fixtures/mcpToolError.js";
|
||||
import { mkdtemp, rm, symlink, writeFile } from "node:fs/promises";
|
||||
import { tmpdir } from "node:os";
|
||||
import { dirname, join } from "node:path";
|
||||
@@ -131,11 +133,9 @@ it("invalidates the snapshot in the legacy Rizin admission path", async () => {
|
||||
it("skips snapshot invalidation for malformed Rizin input", async () => {
|
||||
const { server, handler } = createReverseEngineeringMcpHarness();
|
||||
const invalidateSnapshot = vi.fn();
|
||||
const executeRizinCommand = vi.fn(async () =>
|
||||
err(new AnalysisInputError("execute_rizin_command")),
|
||||
);
|
||||
const executeRizinCommandValidated = vi.fn();
|
||||
const service = {
|
||||
executeRizinCommand,
|
||||
executeRizinCommandValidated,
|
||||
} as unknown as ReverseEngineeringService;
|
||||
registerReverseEngineeringTools(
|
||||
server,
|
||||
@@ -150,10 +150,17 @@ it("skips snapshot invalidation for malformed Rizin input", async () => {
|
||||
|
||||
const response = (await invokeWithLiveSignal(invoke, {
|
||||
path: "package.json",
|
||||
})) as { readonly isError?: boolean };
|
||||
})) as CallToolResult;
|
||||
|
||||
expect(response.isError).toBe(true);
|
||||
expect(executeRizinCommand).toHaveBeenCalledOnce();
|
||||
expect(parseMcpToolError(response).error).toMatchObject({
|
||||
code: "invalid_request",
|
||||
details: {
|
||||
issues: [
|
||||
{ path: ["command"], reason: "missing_argument", expected: "string" },
|
||||
],
|
||||
},
|
||||
});
|
||||
expect(executeRizinCommandValidated).not.toHaveBeenCalled();
|
||||
expect(invalidateSnapshot).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
@@ -193,7 +200,7 @@ it("preserves the Rizin operation error when analysis retirement also fails", as
|
||||
const retirement = new AnalysisInputError("rebind");
|
||||
const service = {
|
||||
retryCleanup: async () => ok(null),
|
||||
executeRizinCommand: async () => err(primary),
|
||||
executeRizinCommandValidated: async () => err(primary),
|
||||
} as unknown as ReverseEngineeringService;
|
||||
const observedOperationResult = vi.fn();
|
||||
registerReverseEngineeringTools(
|
||||
@@ -421,7 +428,7 @@ it("does not treat a malformed partial observation as retained Evidence", async
|
||||
);
|
||||
const service = {
|
||||
inspectWithObjdump: async () => err(invalidError),
|
||||
executeRizinCommand: async () => err(invalidError),
|
||||
executeRizinCommandValidated: async () => err(invalidError),
|
||||
} as unknown as ReverseEngineeringService;
|
||||
const recordEvidence = vi.fn(() => ok("added" as const));
|
||||
registerReverseEngineeringTools(
|
||||
@@ -489,7 +496,7 @@ it("returns admission failures without invoking a reverse-engineering provider",
|
||||
const inspectWithObjdump = vi.fn();
|
||||
const service = {
|
||||
inspectWithObjdump,
|
||||
executeRizinCommand: vi.fn(),
|
||||
executeRizinCommandValidated: vi.fn(),
|
||||
} as unknown as ReverseEngineeringService;
|
||||
const admission: WithAdmittedAnalysis = async () =>
|
||||
err(new AnalysisInputError("inspect_with_objdump"));
|
||||
|
||||
@@ -307,7 +307,13 @@ it("rejects malformed Rizin requests before touching the provider", async () =>
|
||||
|
||||
expect(result).toMatchObject({
|
||||
ok: false,
|
||||
error: { _tag: "AnalysisInputError", operation: "execute_rizin_command" },
|
||||
error: {
|
||||
_tag: "AnalysisInputError",
|
||||
operation: "execute_rizin_command",
|
||||
issues: [
|
||||
{ path: ["command"], reason: "missing_argument", expected: "string" },
|
||||
],
|
||||
},
|
||||
});
|
||||
expect(run).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user