diff --git a/docs/reverse-engineering-tools.md b/docs/reverse-engineering-tools.md index 50f317848c..a44cdd990b 100644 --- a/docs/reverse-engineering-tools.md +++ b/docs/reverse-engineering-tools.md @@ -136,6 +136,8 @@ rea debug-with-rizin --inferior-tty /dev/pts/7 ./program dr rea execute-rizin-command ./program iI ``` +Rizin exits with status 0 even when its shell refuses a statement, so the exit code does not show whether the command ran. Rizin 0.6 through 0.9.1 log one of four stderr lines for such a refusal: `ERROR: Command '…' does not exist.`, ``ERROR: Wrong number of arguments passed to `…`…``, `ERROR: core: Error while executing command: …` (an invalid statement), or `ERROR: core: Error while parsing command: …`. Rizin then skips the refused statement and every later one in the command. When the completed process's stderr contains one of these lines, REA reports `completion_status: "rejected"` and adds a limitation naming the refusal. stdout then holds only the output of earlier statements, so empty stdout is not an empty result. Other `ERROR:` lines, such as `ERROR: Cannot find function at …`, come from commands that ran; REA keeps them in stderr and reports `complete`. A command that lowers `log.level` or redirects stderr hides refusals from REA. + Captured stdout and stderr use the same `max_output_bytes` (CLI: `--max-output-bytes`) admission and recovery described above, default 16 MiB. One-shot commands independently use `command_timeout_ms`. Raising output admission cannot recover an earlier capture; command effects and cleanup must be checked before another invocation. ### Decoder and map inputs diff --git a/src/application/reverse/ReverseEngineeringCommandPort.ts b/src/application/reverse/ReverseEngineeringCommandPort.ts index 7228526c86..d5680bf58a 100644 --- a/src/application/reverse/ReverseEngineeringCommandPort.ts +++ b/src/application/reverse/ReverseEngineeringCommandPort.ts @@ -11,6 +11,11 @@ export interface ReverseEngineeringProviderCommand { readonly identity: EvidenceProvider; /** What the provider cannot attest about the arguments it built. */ readonly limitations?: readonly string[]; + /** + * Explain a provider refusal of the command that a completed process + * reported only in its diagnostics, or return undefined. + */ + readonly rejection?: (stderr: string) => string | undefined; readonly environmentOverrides?: Readonly; } diff --git a/src/application/reverse/ReverseEngineeringService.ts b/src/application/reverse/ReverseEngineeringService.ts index 8f3c0efdd9..c3aa241a19 100644 --- a/src/application/reverse/ReverseEngineeringService.ts +++ b/src/application/reverse/ReverseEngineeringService.ts @@ -599,7 +599,7 @@ export class ReverseEngineeringService { let stderrInvalidUtf8 = false; let cleanupIncomplete = false; let cleanupOnlyFailure = false; - let completionStatus: "complete" | "unknown" = "complete"; + let completionStatus: "complete" | "rejected" | "unknown" = "complete"; let outputLimitReached = false; let failure: AnalysisError | undefined; let cleanupOwner: ProviderProcessSupervisor | undefined; @@ -1076,6 +1076,15 @@ export class ReverseEngineeringService { : digestAfter !== digest; const outputBudgetSource = parameters.max_output_bytes === undefined ? "default" : "request"; + // Only a completed process's diagnostics can establish a refusal; partial + // or failed captures keep their unknown completion. + const rejection = + completionStatus === "complete" && + (failure === undefined || + (failure instanceof AnalysisOutputError && failure.cleanupOnly)) + ? profile.rejection?.(stderr) + : undefined; + if (rejection !== undefined) completionStatus = "rejected"; const result = { stdout, stderr, @@ -1116,6 +1125,7 @@ export class ReverseEngineeringService { ] : []), ...(profile.limitations ?? []), + ...(rejection === undefined ? [] : [rejection]), ...(stdoutInvalidUtf8 || stderrInvalidUtf8 ? [ `Captured ${[ diff --git a/src/cli/reverseEngineeringCommands.ts b/src/cli/reverseEngineeringCommands.ts index d8457d62cd..936b2e2295 100644 --- a/src/cli/reverseEngineeringCommands.ts +++ b/src/cli/reverseEngineeringCommands.ts @@ -80,7 +80,7 @@ const cleanupOnlyCompleteEvidence = ( return typeof normalized === "object" && normalized !== null && !Array.isArray(normalized) && - normalized.completion_status === "complete" + normalized.completion_status !== "unknown" ? parsed.data : undefined; }; diff --git a/src/composition/reverseEngineering.ts b/src/composition/reverseEngineering.ts index 6394dc1550..301c65f1aa 100644 --- a/src/composition/reverseEngineering.ts +++ b/src/composition/reverseEngineering.ts @@ -6,6 +6,7 @@ import { OBJDUMP_PROVIDER_IDENTITY } from "../objdump/ObjdumpProviderIdentity.js import { rizinCommand, rizinCommandLimitations, + rizinCommandRejection, } from "../rizin/RizinCommand.js"; import { RIZIN_PROVIDER_IDENTITY } from "../rizin/RizinProviderIdentity.js"; import { resolveWindowsProviderExecutable } from "./resolveWindowsProviderExecutable.js"; @@ -32,6 +33,7 @@ export const createReverseEngineeringCommandPort = ( ), arguments: rizinCommand(input), limitations: rizinCommandLimitations(input), + rejection: rizinCommandRejection, identity: RIZIN_PROVIDER_IDENTITY, }), }); diff --git a/src/contracts/reverseEngineeringToolContracts.ts b/src/contracts/reverseEngineeringToolContracts.ts index 8c31e875e3..73394aca40 100644 --- a/src/contracts/reverseEngineeringToolContracts.ts +++ b/src/contracts/reverseEngineeringToolContracts.ts @@ -39,7 +39,7 @@ export const REVERSE_ENGINEERING_TOOL_CONTRACTS = [ ...toolContractMetadata("execute_rizin_command"), kind: "native-provider", description: - "Run one caller-selected Rizin command on a local artifact. Commands/plugins can execute code, mutate files, access network and read outside the artifact; those external inputs are not Evidence-bound. Suppresses user settings/scripts but retains plugins. completion_status reports process completion; Rizin can reject a command and exit with code 0. Inspect stdout/stderr before treating empty output as absence. Raw stdout/stderr is authoritative, JSON is command-specific, and UTF-8 loss/truncation is explicit; output-limit termination leaves completion unknown. For raw images, asm_arch/asm_bits/asm_cpu select decoding and base_address maps offset 0; base_address also passes -n to skip loaders even on ELF, without REA confirming either. An admitted command retires shared binary analysis: close Ghidra explicitly first, then open_binary after effects settle. command_timeout_ms is per-phase and max_output_bytes bounds combined output (16 MiB default); both apply next call without restart and do not raise client receive limits or persistent debugger deadlines.", + "Run one caller-selected Rizin command on a local artifact. Commands/plugins can execute code, mutate files, access network and read outside the artifact; those external inputs are not Evidence-bound. Suppresses user settings/scripts but retains plugins. Raw stdout/stderr is authoritative, JSON is command-specific, and UTF-8 loss/truncation is explicit; output-limit termination leaves completion unknown. Rizin exits 0 even when its shell refuses a statement; completion_status rejected marks that refusal, and later statements did not run. Other command failures can still exit 0, so inspect stderr before treating empty output as absence. For raw images, asm_arch/asm_bits/asm_cpu select decoding and base_address maps offset 0; base_address also passes -n to skip loaders even on ELF, without REA confirming either. An admitted command retires shared binary analysis: close Ghidra explicitly first, then open_binary after effects settle. command_timeout_ms is per-phase and max_output_bytes bounds combined output (16 MiB default); both apply next call without restart and do not raise client receive limits or persistent debugger deadlines.", inputSchema: rizinInputSchema, outputSchema: evidenceResultOf(commandObservationSchema), examples: [ diff --git a/src/domain/commandObservation.ts b/src/domain/commandObservation.ts index ff383e64be..cbe75404b6 100644 --- a/src/domain/commandObservation.ts +++ b/src/domain/commandObservation.ts @@ -9,7 +9,11 @@ export const commandObservationSchema = z.strictObject({ output_budget_bytes: z.int().positive().exactOptional(), output_budget_source: z.enum(["default", "request"]).exactOptional(), output_truncated: z.boolean(), - completion_status: z.enum(["complete", "unknown"]), + completion_status: z + .enum(["complete", "rejected", "unknown"]) + .describe( + "complete: the provider process completed. rejected: it completed, but the provider reported that it refused the command; limitations say what ran. unknown: the capture or process ended early.", + ), artifact_sha256_after: z .string() .regex(/^[a-f0-9]{64}$/u) diff --git a/src/rizin/RizinCommand.ts b/src/rizin/RizinCommand.ts index 9273cdf3b9..9cfec0aee3 100644 --- a/src/rizin/RizinCommand.ts +++ b/src/rizin/RizinCommand.ts @@ -25,7 +25,7 @@ export const rizinCommand = (input: RizinInput): readonly string[] => [ export const rizinCommandLimitations = ( input: RizinInput, ): readonly string[] => [ - "completion_status reports process completion. Command validity remains unverified: Rizin can reject a command while returning exit_code 0. Inspect stdout and stderr before treating an empty response as absence.", + "completion_status covers process completion and Rizin shell refusals only. A statement that ran can still fail while Rizin returns exit_code 0, so inspect stderr before treating an empty response as absence.", ...(input.asm_arch === undefined && input.asm_bits === undefined && input.asm_cpu === undefined @@ -39,3 +39,43 @@ export const rizinCommandLimitations = ( "With base_address, REA passed -n and -m so that Rizin maps file offset 0 at that address as a raw image; REA does not confirm the map, which oml shows. -n skips Rizin's format loaders, so expect no sections, symbols or entry point, and arch, bits and byte order stay at Rizin's defaults unless the call selects them.", ]), ]; + +/** + * Rizin's shell logs exactly these lines when it refuses a statement + * (librz/core/cmd/cmd.c, unchanged from 0.6 through 0.9.1). Rizin then skips + * the remaining statements and still exits 0. Other ERROR lines come from + * commands that ran, so they are diagnostics rather than refusals. + */ +const RIZIN_SHELL_REFUSALS: readonly { + readonly line: RegExp; + readonly refused: (match: RegExpExecArray) => string; +}[] = [ + { + line: /^ERROR: Command '(.*)' does not exist\.$/u, + refused: (match) => `the unknown command ${JSON.stringify(match[1])}`, + }, + { + line: /^ERROR: Wrong number of arguments passed to `(.*)`, see its help with `.*\?`$/u, + refused: (match) => + `a wrong argument count for ${JSON.stringify(match[1])}`, + }, + { + line: /^ERROR: core: Error while executing command: /u, + refused: () => "an invalid statement", + }, + { + line: /^ERROR: core: Error while parsing command: /u, + refused: () => "input that failed to parse", + }, +]; + +/** Explain the first Rizin shell refusal in a completed process's stderr. */ +export const rizinCommandRejection = (stderr: string): string | undefined => { + for (const text of stderr.split(/\r?\n/u)) + for (const { line, refused } of RIZIN_SHELL_REFUSALS) { + const match = line.exec(text); + if (match !== null) + return `Rizin refused ${refused(match)} (ERROR line on stderr). It skips a refused statement and every later one, so stdout holds at most the output of earlier statements; empty stdout is not an empty result. Correct the statement, using ? for Rizin's help, and run it again.`; + } + return undefined; +}; diff --git a/tests/acceptance/rizinCommandOutcomes.test.ts b/tests/acceptance/rizinCommandOutcomes.test.ts index f3c774af55..180e2ab161 100644 --- a/tests/acceptance/rizinCommandOutcomes.test.ts +++ b/tests/acceptance/rizinCommandOutcomes.test.ts @@ -13,12 +13,16 @@ const outcomes = [ command: "axffj", stdout: "", stderr: "ERROR: Command 'axffj' does not exist.\n", + // The refusal line on stderr is classified; process facts stay as observed. + completion: "rejected", }, - { command: "q", stdout: "", stderr: "" }, + { command: "q", stdout: "", stderr: "", completion: "complete" }, { command: "echo diagnostic", stdout: "ERROR: Command 'axffj' does not exist.\n", stderr: "WARNING: fixture warning\n", + // Refusal text on stdout is command output, not a Rizin diagnostic. + completion: "complete", }, ] as const; @@ -71,7 +75,7 @@ cliTest.skipIf(process.platform === "win32").for(outcomes)( stdout: outcome.stdout, stderr: outcome.stderr, exit_code: 0, - completion_status: "complete", + completion_status: outcome.completion, output_truncated: false, artifact_changed: false, }); diff --git a/tests/acceptance/rizinCommandRejection.test.ts b/tests/acceptance/rizinCommandRejection.test.ts new file mode 100644 index 0000000000..cfddb0747c --- /dev/null +++ b/tests/acceptance/rizinCommandRejection.test.ts @@ -0,0 +1,179 @@ +import { execFileSync } from "node:child_process"; +import { chmod, writeFile } from "node:fs/promises"; +import { join } from "node:path"; +import { expect } from "vitest"; +import { REVERSE_ENGINEERING_TOOL_CONTRACTS } from "../../src/contracts/reverseEngineeringToolContracts.js"; +import { connectLocalToolsMcp } from "../fixtures/localToolsMcp.js"; +import { createTestTempDirectory } from "../fixtures/temporaryDirectory.js"; +import { cliTest } from "../support/cli/cliFixture.js"; + +const contract = REVERSE_ENGINEERING_TOOL_CONTRACTS.find( + ({ name }) => name === "execute_rizin_command", +); +if (contract === undefined) + throw new Error("execute_rizin_command contract is missing"); + +interface Outcome { + readonly command: string; + readonly stdout: string; + readonly stderr: string; + /** Expected completion and the refusal the limitation must name. */ + readonly refused?: RegExp; +} + +// Captured from the official Rizin 0.9.1 static Linux build on /usr/bin/true. +const outcomes: readonly Outcome[] = [ + { + command: "axffj", + stdout: "", + stderr: + "ERROR: Command 'axffj' does not exist.\nERROR: Displaying the help of command 'axf'.\n\nERROR: Usage: axf[jq] # List xrefs from current seek\n", + refused: /unknown command "axffj"/u, + }, + { + // Rizin runs the statements before a refusal and skips the rest. + command: "%v 1; zz; %v 2", + stdout: "0x1\n", + stderr: "ERROR: Command 'zz' does not exist.\n", + refused: /unknown command "zz"/u, + }, + { + command: "%v 1; afl 1 2; %v 2", + stdout: "0x1\n", + stderr: + "ERROR: Wrong number of arguments passed to `afl`, see its help with `afl?`\n\nERROR: Usage: afl[?] # List functions\n", + refused: /wrong argument count for "afl"/u, + }, + { + command: "pd 1 @ nosuch", + stdout: "", + stderr: + "ERROR: core: Invalid address (nosuch)\nERROR: core: Error while executing command: pd 1 @ nosuch\n", + refused: /an invalid statement/u, + }, + { + command: "?e", + stdout: "", + stderr: "ERROR: core: Error while parsing command: `?e`\n", + refused: /input that failed to parse/u, + }, + // A command that ran and reported its own failure is not a shell refusal. + { + command: "afxj", + stdout: "", + stderr: "ERROR: No function found in 0x000023d0.\n", + }, + // Refusal text on stdout is command output, not a Rizin diagnostic. + { + command: "echo refusal", + stdout: "ERROR: Command 'axffj' does not exist.\n", + stderr: "", + }, +]; + +const expectClassified = (raw: unknown, outcome: Outcome): void => { + const evidence = contract.outputSchema.parse(raw); + expect(evidence.normalized_result).toMatchObject({ + stdout: outcome.stdout, + stderr: outcome.stderr, + exit_code: 0, + completion_status: outcome.refused === undefined ? "complete" : "rejected", + }); + expect(evidence.raw_result).toEqual(evidence.normalized_result); + const refusals = evidence.limitations.filter((limitation) => + limitation.startsWith("Rizin refused "), + ); + if (outcome.refused === undefined) expect(refusals).toEqual([]); + else { + expect(refusals).toHaveLength(1); + expect(refusals[0]).toMatch(outcome.refused); + expect(refusals[0]).toMatch(/empty stdout is not an empty result/u); + } +}; + +cliTest.skipIf(process.platform === "win32")( + "reports Rizin shell refusals as rejected through CLI and MCP (simulated Rizin)", + async ({ cli }) => { + const root = await createTestTempDirectory("rea-rizin-rejection-"); + const provider = join(root, "rizin-fixture"); + const artifact = join(root, "artifact.bin"); + await writeFile(artifact, "caller-selected inert artifact"); + // A Node script replays recorded Rizin output; it is not Rizin. + await writeFile( + provider, + `#!${process.execPath}\n` + + `const cases=${JSON.stringify(outcomes)};\n` + + "const selected=cases.find(item=>item.command===process.argv[process.argv.indexOf('-c')+1]);\n" + + "if(!selected)process.exit(99);\n" + + "process.stdout.write(selected.stdout);process.stderr.write(selected.stderr);\n", + ); + await chmod(provider, 0o755); + const environment = { REA_RIZIN_COMMAND: provider }; + const { call } = await connectLocalToolsMcp(environment); + // Both adapters share the service's classification; the CLI checks one + // refusal and one command diagnostic. + for (const outcome of [outcomes[0]!, outcomes[5]!]) { + const response = await cli.sharedCliRun!({ + arguments: [ + "execute-rizin-command", + artifact, + outcome.command, + "--json", + ], + environment, + }); + expect(response.exitCode, `${response.stdout}\n${response.stderr}`).toBe( + 0, + ); + expectClassified(response.json, outcome); + } + for (const outcome of outcomes) { + const mcp = await call("execute_rizin_command", { + path: artifact, + command: outcome.command, + }); + expect(mcp.isError, JSON.stringify(mcp.content)).not.toBe(true); + expectClassified(mcp.structuredContent, outcome); + } + }, +); + +// Real-provider gate: runs the Rizin found on PATH and skips without one. +const rizin = (() => { + try { + const path = execFileSync("which", ["rizin"], { encoding: "utf8" }).trim(); + return path === "" ? undefined : path; + } catch { + return undefined; + } +})(); + +cliTest.skipIf(rizin === undefined)( + "classifies real Rizin refusals and command diagnostics", + async () => { + const root = await createTestTempDirectory("rea-real-rizin-rejection-"); + const artifact = join(root, "artifact.bin"); + await writeFile(artifact, Buffer.from([0x90, 0xc3])); + const { call } = await connectLocalToolsMcp({ REA_RIZIN_COMMAND: rizin! }); + const expected = [ + ["axffj", "rejected"], + ["%v 1; zz; %v 2", "rejected"], + ["%v 1; afl 1 2; %v 2", "rejected"], + ["pd 1 @ nosuch", "rejected"], + ["s nosuch", "complete"], + ["%v 1; %v 2", "complete"], + ] as const; + for (const [command, status] of expected) { + const response = await call("execute_rizin_command", { + path: artifact, + command, + }); + expect(response.isError, JSON.stringify(response.content)).not.toBe(true); + const evidence = contract.outputSchema.parse(response.structuredContent); + expect(evidence.normalized_result, command).toMatchObject({ + exit_code: 0, + completion_status: status, + }); + } + }, +);