From fe677f6cf9853fb12490dfb8ef23ccae3a551049 Mon Sep 17 00:00:00 2001 From: Kaoru0822 Date: Thu, 5 Nov 2026 02:38:12 -0800 Subject: [PATCH] fix(rizin): keep debug targets off the debugger command stream (#8647) * fix(rizin): keep debug targets off the debugger command stream A Rizin debug target inherited Rizin's stdin, which is REA's command stream. Once resumed, a target that reads stdin consumed REA's commands, so close returned cleanup_incomplete and the one-shot CLI hung. Route the target's stdin to the caller-selected TTY alongside stdout and stderr. Before detaching, refuse a target whose stdin is still the command stream, so a Rizin or backend that ignores the key gets a specific, retained failure instead of a resumed target and a timeout. Fixes #8642 Co-Authored-By: Claude Opus 5.5 * test(rizin): keep the stdin test launcher's descriptors under dash The launcher gated its target with `cat "$2" >/dev/null`. dash applies a simple command's redirection to the shell itself until the command returns, so on Ubuntu the waiting shell's fd 1 was /dev/null, and that shell is the target REA reads in /proc. All 8 Linux-gated tests then failed the existing output-descriptor check. macOS /bin/sh is bash, which redirects only in the forked child, so this did not show locally. Gate without any redirection. Production code is unchanged. Co-Authored-By: Claude Opus 5.5 * test(rizin): reject unmodeled commands and own cleanup at acquisition Address review of the stdin regression tests. The launcher accepted any unknown line, so the marker that followed could complete it, and assertions only checked that commands were present. An added `dc` or a repeated `olj` still passed. It now records an unknown line and exits, and the tests pin the exact command sequence. Cleanup hung off module state assigned after setup finished, so a failed setup left its directory and terminal child behind. Each resource now registers its own finish hook when acquired, using createTestTempDirectory. Success checks throw the typed error instead of asserting `.ok`, and the refusal probe no longer sits inside a loop and conditional. Production code is unchanged. Co-Authored-By: Claude Opus 5.5 --------- Co-authored-by: Claude Opus 5.5 --- docs/reverse-engineering-tools.md | 2 +- src/cli/reverseEngineeringCommands.ts | 4 +- src/contracts/rizinDebugToolContracts.ts | 4 +- src/rizin/RizinDebugSessionManager.ts | 37 +- .../RizinDebugSessionInferiorStdin.test.ts | 528 ++++++++++++++++++ .../RizinDebugSessionManagers.test.ts | 2 + .../providers/RizinTestInferiorTty.ts | 10 +- 7 files changed, 576 insertions(+), 11 deletions(-) create mode 100644 tests/boundary/providers/RizinDebugSessionInferiorStdin.test.ts diff --git a/docs/reverse-engineering-tools.md b/docs/reverse-engineering-tools.md index 44c12d4437..6136a5f9f3 100644 --- a/docs/reverse-engineering-tools.md +++ b/docs/reverse-engineering-tools.md @@ -79,7 +79,7 @@ close_gdb_session {"session_id":"…"} ## Rizin -`start_rizin_debug_session` and `rea debug-with-rizin` require an explicit `inferior_tty` on Linux. Rizin launches the target with stdout and stderr routed to that caller-selected TTY and `dbg.exitkills=false`; the target can therefore continue running and writing to the TTY after REA closes the debugger. This is a visible target output effect, and the caller is responsible for selecting a suitable terminal that remains available. Before closing, REA serializes after the active command, parses the structured `olj` descriptor list, detaches only a unique `ptrace://` descriptor, confirms its absence, reads back `dbg.exitkills=false`, and verifies `/proc//fd/1` and `/proc//fd/2` still identify the selected TTY by device and inode. If a previously identified inferior exits after Rizin confirms detach, REA verifies that the original process has ended instead of requiring output descriptors from an exited process. Unresolved identity or ambiguous readback keeps the Rizin session owned for retry. After output limits or a failed command interval, cleanup first waits up to 1000 ms for a fresh private NUL-delimited marker, discarding earlier frames before obtaining that new inferior readback. Synchronization never reopens user command admission. A missing marker, unavailable transport, or unknown inferior identity retains the cleanup owner. Caller cancellation keeps its exact reason and typed cancellation outcome even when cleanup remains incomplete; a successful retry resolves that retained cleanup state. Cancellation during final cleanup retains the captured observation and cleanup owner. An already observed startup or protocol failure remains primary and retains a later cancellation as a secondary failure; recovering the transport does not erase that original diagnosis. Persistent Rizin debugger sessions are unavailable outside Linux because this verification depends on `/proc`. +`start_rizin_debug_session` and `rea debug-with-rizin` require an explicit `inferior_tty` on Linux. REA asks Rizin to launch the target with stdin, stdout and stderr routed to that caller-selected TTY and `dbg.exitkills=false`; the target can therefore continue running, reading input from the TTY and writing to it after REA closes the debugger. This is a visible target input and output effect, and the caller is responsible for selecting a suitable terminal that remains available. REA sends Rizin its commands on Rizin's own stdin, so a target left on that stream would read them. Before closing, REA serializes after the active command, parses the structured `olj` descriptor list, detaches only a unique `ptrace://` descriptor, confirms its absence, reads back `dbg.exitkills=false`, verifies `/proc//fd/1` and `/proc//fd/2` still identify the selected TTY by device and inode, and refuses to detach while `/proc//fd/0` is Rizin's command stream. A target may close or replace its own stdin. If a previously identified inferior exits after Rizin confirms detach, REA verifies that the original process has ended instead of requiring output descriptors from an exited process. Unresolved identity or ambiguous readback keeps the Rizin session owned for retry. After output limits or a failed command interval, cleanup first waits up to 1000 ms for a fresh private NUL-delimited marker, discarding earlier frames before obtaining that new inferior readback. Synchronization never reopens user command admission. A missing marker, unavailable transport, or unknown inferior identity retains the cleanup owner. Caller cancellation keeps its exact reason and typed cancellation outcome even when cleanup remains incomplete; a successful retry resolves that retained cleanup state. Cancellation during final cleanup retains the captured observation and cleanup owner. An already observed startup or protocol failure remains primary and retains a later cancellation as a secondary failure; recovering the transport does not erase that original diagnosis. Persistent Rizin debugger sessions are unavailable outside Linux because this verification depends on `/proc`. Persistent command Evidence retains the caller-selected and resolved startup paths in `parameters.launch_provenance`. These paths describe launch provenance, not the current target after unrestricted commands. `rizin_debug_session_status.output_invalid_utf8` reports whether lossy frame decoding has been observed during the session. Command Evidence sets `output_invalid_utf8` only when loss was observed in frames collected for that command, including retained partial output; earlier startup or command frames do not mark later valid output as lossy. diff --git a/src/cli/reverseEngineeringCommands.ts b/src/cli/reverseEngineeringCommands.ts index 59e715fb6d..43a2fe81b6 100644 --- a/src/cli/reverseEngineeringCommands.ts +++ b/src/cli/reverseEngineeringCommands.ts @@ -422,7 +422,9 @@ export const registerReverseEngineeringCommands = ( inferiorTty: z .string() .min(1) - .describe("Caller-selected Linux TTY for inferior stdout and stderr."), + .describe( + "Caller-selected Linux TTY for inferior stdin, stdout and stderr.", + ), backend: z .string() .min(1) diff --git a/src/contracts/rizinDebugToolContracts.ts b/src/contracts/rizinDebugToolContracts.ts index 839248aad3..92fa362833 100644 --- a/src/contracts/rizinDebugToolContracts.ts +++ b/src/contracts/rizinDebugToolContracts.ts @@ -23,7 +23,7 @@ export const RIZIN_DEBUG_TOOL_CONTRACTS = [ ...toolContractMetadata("start_rizin_debug_session"), kind: "native-provider", description: - "Start a persistent Rizin debugger session using `-N` to suppress user settings and scripts while retaining plugins, `-0` for NUL framing, and `-d` with an optional caller-selected IO debugger backend. Requires a caller-selected Linux TTY for inferior stdout and stderr; the target may write directly to that terminal while running and after detach. REA verifies both destinations and `dbg.exitkills=false` before detaching. Backend and target capabilities depend on the installed build. The session lives in this REA MCP process. Persistent sessions require Linux `/proc` descriptor readback. In a shared MCP server, starting this session retires any active binary-analysis session; after target effects have settled, call open_binary before further analysis. Close an active Ghidra session explicitly first.", + "Start a persistent Rizin debugger session using `-N` to suppress user settings and scripts while retaining plugins, `-0` for NUL framing, and `-d` with an optional caller-selected IO debugger backend. Requires a caller-selected Linux TTY for inferior stdin, stdout and stderr; the target may read from and write directly to that terminal while running and after detach. Before detaching, REA verifies both output destinations, that the target does not hold the debugger command stream, and `dbg.exitkills=false`. Backend and target capabilities depend on the installed build. The session lives in this REA MCP process. Persistent sessions require Linux `/proc` descriptor readback. In a shared MCP server, starting this session retires any active binary-analysis session; after target effects have settled, call open_binary before further analysis. Close an active Ghidra session explicitly first.", inputSchema: z.strictObject({ path: localPathStringSchema, inferior_tty: localPathStringSchema, @@ -95,7 +95,7 @@ export const RIZIN_DEBUG_TOOL_CONTRACTS = [ ...toolContractMetadata("close_rizin_debug_session"), kind: "native-provider", description: - "Close the REA-owned Rizin debugger only after verifying no inferior remains or detaching one with a verified caller-selected TTY output sink. A detached target may continue running and write to that TTY; target state is otherwise reported as unknown. REA never signals an inferior based on PID or path. In a shared MCP server, this admitted close retires any active binary-analysis session; after target effects have settled, call open_binary before further analysis. Close an active Ghidra session explicitly first.", + "Close the REA-owned Rizin debugger only after verifying no inferior remains or detaching one with a verified caller-selected TTY output sink. A detached target may continue running, read from and write to that TTY; target state is otherwise reported as unknown. REA never signals an inferior based on PID or path. In a shared MCP server, this admitted close retires any active binary-analysis session; after target effects have settled, call open_binary before further analysis. Close an active Ghidra session explicitly first.", inputSchema: z.strictObject({ session_id: sessionId }), outputSchema: z.strictObject({ target_state: z.literal("unknown") }), examples: [ diff --git a/src/rizin/RizinDebugSessionManager.ts b/src/rizin/RizinDebugSessionManager.ts index 95766ac089..075a43bcf3 100644 --- a/src/rizin/RizinDebugSessionManager.ts +++ b/src/rizin/RizinDebugSessionManager.ts @@ -3,7 +3,7 @@ import { persistentDebuggerHostSupported } from "../process/PersistentDebuggerHo import { randomUUID } from "node:crypto"; import { isUtf8 } from "node:buffer"; import { open, readlink, realpath, stat } from "node:fs/promises"; -import { constants as fsConstants } from "node:fs"; +import { constants as fsConstants, type Stats } from "node:fs"; import { isatty } from "node:tty"; import { @@ -332,7 +332,7 @@ export class RizinDebugSessionManager { path: ["inferior_tty"], reason: "missing_argument", message: - "Select a persistent readable and writable Linux terminal for inferior output.", + "Select a persistent readable and writable Linux terminal for inferior input and output.", }, ]), ); @@ -445,6 +445,10 @@ export class RizinDebugSessionManager { "-N", "-0", "-d", + // Rizin reads REA's commands from its own stdin. Without this key the + // target inherits that stream and, once resumed, reads those commands. + "-R", + `stdin=${ttyPath}`, "-R", `stdout=${ttyPath}`, "-R", @@ -1284,6 +1288,7 @@ export class RizinDebugSessionManager { ); await this.#verifyDebuggerExitPolicy(session); await this.#verifyInferiorOutputDescriptors(session, pid); + await this.#verifyInferiorStdinIsolated(session, pid); await this.#runInternalCommand(session, `o- ${fd}`); const after = parseRizinPtraceRows( await this.#runInternalCommand(session, "olj"), @@ -1343,6 +1348,34 @@ export class RizinDebugSessionManager { } } + /** + * Detach resumes the inferior, which would then read the commands that must + * confirm the detach. Only sharing the command stream is refused: a target may + * close or replace its own stdin, and neither can consume those commands. + */ + async #verifyInferiorStdinIsolated( + session: RizinDebugSession, + pid: string, + ): Promise { + let inferiorStdin: Stats; + try { + inferiorStdin = await stat(`/proc/${pid}/fd/0`); + } catch (cause) { + if ((cause as NodeJS.ErrnoException).code === "ENOENT") return; + throw cause; + } + const commandStream = await stat( + `/proc/${String(session.launched.process.pid)}/fd/0`, + ); + if ( + inferiorStdin.dev === commandStream.dev && + inferiorStdin.ino === commandStream.ino + ) + throw new Error( + "Inferior stdin is the Rizin command stream rather than the selected TTY; detaching would let the target consume debugger commands.", + ); + } + async #synchronizeCleanup(session: RizinDebugSession): Promise { const stdin = session.launched.process.stdin; if ( diff --git a/tests/boundary/providers/RizinDebugSessionInferiorStdin.test.ts b/tests/boundary/providers/RizinDebugSessionInferiorStdin.test.ts new file mode 100644 index 0000000000..cd2a37a76c --- /dev/null +++ b/tests/boundary/providers/RizinDebugSessionInferiorStdin.test.ts @@ -0,0 +1,528 @@ +import { execFileSync, type ChildProcess } from "node:child_process"; +import { once } from "node:events"; +import { chmod, readFile, stat, writeFile } from "node:fs/promises"; +import { join } from "node:path"; + +import { spawn } from "@lydell/node-pty"; +import { Client, InMemoryTransport } from "@modelcontextprotocol/client"; +import { McpServer } from "@modelcontextprotocol/server"; +import pino from "pino"; +import { expect, it, onTestFinished } from "vitest"; + +import { registerReverseEngineeringCommands } from "../../../src/cli/reverseEngineeringCommands.js"; +import type { CliInstance } from "../../../src/cli/types.js"; +import { CLI_COMMANDS } from "../../../src/cliCommandNames.js"; +import { toolContract } from "../../../src/contracts/toolContracts.js"; +import type { AnalysisError } from "../../../src/domain/analysisErrorBase.js"; +import type { Result } from "../../../src/domain/result.js"; +import { silentLogger } from "../../../src/logger.js"; +import { spawnOwnedProviderProcess } from "../../../src/process/ProviderProcess.js"; +import { RizinDebugSessionManager } from "../../../src/rizin/RizinDebugSessionManager.js"; +import { registerRizinDebugTools } from "../../../src/server/registerRizinDebugTools.js"; +import { createToolResultDelivery } from "../../../src/server/toolResult.js"; +import { createTestTempDirectory } from "../../fixtures/temporaryDirectory.js"; +import { rizinTestTtyPathPattern } from "./RizinTestInferiorTty.js"; + +// Real Rizin is not exercised here. These launchers apply `-R` keys the way the +// #8642 report observed on Rizin 0.9.1, and the target is a real process, so the +// tests prove what REA asks for and what a real child then inherits and reads. + +interface LauncherOptions { + /** Shell commands the target runs before it waits to be resumed. */ + readonly prelude?: string; + /** The real command the target becomes once resumed. */ + readonly target: string; + /** Report a ptrace row, which sends close through the Linux detach path. */ + readonly reportsInferior: boolean; + /** Keep the target on the debugger's own stdin despite `-R stdin=`. */ + readonly ignoresStdinProfile?: boolean; +} + +interface Terminal { + readonly path: string; + readonly output: () => string; + readonly write: (text: string) => void; +} + +interface Fixture { + readonly launcher: string; + readonly terminal: Terminal; + readonly targetPid: () => Promise; + readonly launcherPid: () => Promise; + readonly commands: () => Promise; +} + +const openTerminal = async (): Promise => { + const pty = spawn("sh", ["-c", "tty; exec sleep 600"], { + name: "xterm", + cols: 80, + rows: 24, + encoding: "utf8", + }); + let output = ""; + pty.onData((chunk) => { + output += chunk; + }); + let exited = false; + const ended = new Promise((resolve) => { + pty.onExit(() => { + exited = true; + resolve(); + }); + }); + onTestFinished(async () => { + if (!exited) pty.kill(); + await ended; + }); + await expect + .poll(() => rizinTestTtyPathPattern.test(output), { timeout: 2_000 }) + .toBe(true); + return { + path: rizinTestTtyPathPattern.exec(output)![0], + output: () => output, + write: (text) => pty.write(text), + }; +}; + +const launcherSource = ( + config: LauncherOptions & Record, +): string => `#!/usr/bin/env node +const fs = require("node:fs"); +const { spawn } = require("node:child_process"); +const config = ${JSON.stringify(config)}; +const argv = process.argv.slice(2); +const profile = new Map(); +argv.forEach((value, index) => { + if (argv[index - 1] !== "-R") return; + const separator = value.indexOf("="); + profile.set(value.slice(0, separator), value.slice(separator + 1)); +}); +// A stream without a profile key keeps the debugger's own descriptor. +const stream = (key, flags, inherited) => { + const path = profile.get(key); + return path === undefined || (key === "stdin" && config.ignoresStdinProfile) + ? inherited + : fs.openSync(path, flags | fs.constants.O_NOCTTY); +}; +// Like a ptrace stop, the gate holds the target before it touches stdin. These +// commands carry no redirection: dash applies one to the shell itself until the +// command returns, and this shell's descriptors are the target's in /proc. +const script = (config.prelude ?? "") + ' touch "$1"; cat "$2"; exec ' + config.target; +const target = spawn("/bin/sh", ["-c", script, "sh", config.readyPath, config.gatePath], { + detached: true, + stdio: [ + stream("stdin", fs.constants.O_RDONLY, 0), + stream("stdout", fs.constants.O_WRONLY, "ignore"), + stream("stderr", fs.constants.O_WRONLY, "ignore"), + ], +}); +fs.writeFileSync(config.targetPidPath, String(target.pid)); +fs.writeFileSync(config.launcherPidPath, String(process.pid)); +const deadline = Date.now() + 5000; +while (!fs.existsSync(config.readyPath) && Date.now() < deadline) + Atomics.wait(new Int32Array(new SharedArrayBuffer(4)), 0, 0, 5); +let attached = config.reportsInferior; +const resume = () => fs.closeSync(fs.openSync(config.gatePath, "w")); +const answer = (line) => { + if (line === "olj") + process.stdout.write(JSON.stringify(attached ? [{ raised: true, fd: 3, uri: "ptrace://" + target.pid }] : []) + "\\0"); + else if (line === "e dbg.exitkills") process.stdout.write("false\\0"); + else if (line === "dr") process.stdout.write("rax=0x1\\0"); + else if (line.startsWith("!echo ")) process.stdout.write(line.slice(6) + "\\n\\0"); + else if (line === "resume-target" || line === "o- 3") { + attached = false; + resume(); + process.stdout.write("\\0"); + return true; + } else { + // The marker that follows must not complete a command this model cannot describe. + fs.writeFileSync(config.unmodeledPath, line); + process.exit(70); + } + return false; +}; +// A plain read keeps fd 0 blocking for the real child that may share it, and a +// worker-thread read leaves the event loop free to reap that child. A debugger +// busy after resuming a target leaves it the only reader of the shared stream. +const buffer = Buffer.alloc(64 * 1024); +let input = ""; +const readCommands = () => + fs.read(0, buffer, 0, buffer.length, null, (error, bytes) => { + if (error !== null || bytes === 0) process.exit(0); + input += buffer.toString("utf8", 0, bytes); + let resumed = false; + for (let end = input.indexOf("\\n"); end >= 0; end = input.indexOf("\\n")) { + const line = input.slice(0, end); + input = input.slice(end + 1); + fs.appendFileSync(config.commandLogPath, line + "\\n"); + resumed = answer(line) || resumed; + } + setTimeout(readCommands, resumed ? 100 : 0); + }); +process.stdout.write("\\0"); +readCommands(); +`; + +/** Close reads target descriptors from Linux /proc. */ +const linuxIt = it.skipIf(process.platform !== "linux"); + +/** Close inventories the target, detaches it, then reads the result back. */ +const DETACH_COMMANDS = [ + "olj", + "e dbg.exitkills", + "o- 3", + "olj", + "e dbg.exitkills", +]; +/** A refused close stops after the inventory and exit policy, before `o-`. */ +const REFUSED_CLOSE_COMMANDS = ["olj", "e dbg.exitkills"]; + +/** A negative PID also covers the gate reader a detached target may be waiting on. */ +const killIfRunning = (pid: number): void => { + try { + process.kill(pid, "SIGKILL"); + } catch { + // The process may already have ended. + } +}; + +const successValue = (result: Result): Value => { + if (!result.ok) throw result.error; + return result.value; +}; + +/** Finish hooks run in reverse order: manager, processes, terminal, directory. */ +const createFixture = async (options: LauncherOptions): Promise => { + const directory = await createTestTempDirectory("rea-rizin-stdin-"); + const paths = { + readyPath: join(directory, "target.ready"), + gatePath: join(directory, "target.gate"), + targetPidPath: join(directory, "target.pid"), + launcherPidPath: join(directory, "launcher.pid"), + commandLogPath: join(directory, "commands.log"), + unmodeledPath: join(directory, "unmodeled.txt"), + }; + const terminal = await openTerminal(); + const readPid = async (path: string): Promise => + Number(await readFile(path, "utf8")); + onTestFinished(async () => { + const targetPid = await readPid(paths.targetPidPath).catch(() => 0); + const launcherPid = await readPid(paths.launcherPidPath).catch(() => 0); + if (targetPid > 0) killIfRunning(-targetPid); + if (launcherPid > 0) killIfRunning(launcherPid); + expect(await readFile(paths.unmodeledPath, "utf8").catch(() => "")).toBe( + "", + ); + }); + execFileSync("mkfifo", [paths.gatePath]); + const launcher = join(directory, "fake-rizin.cjs"); + await writeFile(launcher, launcherSource({ ...options, ...paths })); + await chmod(launcher, 0o700); + return { + launcher, + terminal, + targetPid: () => readPid(paths.targetPidPath), + launcherPid: () => readPid(paths.launcherPidPath), + commands: async () => + (await readFile(paths.commandLogPath, "utf8").catch(() => "")) + .split("\n") + .filter((line) => line.length > 0 && !line.startsWith("!echo ")), + }; +}; + +const createManager = ( + { launcher }: Fixture, + options: NonNullable< + ConstructorParameters[1] + > = {}, +): RizinDebugSessionManager => { + const manager = new RizinDebugSessionManager( + { ...process.env, REA_RIZIN_COMMAND: launcher }, + options, + ); + onTestFinished(() => manager.closeAll().catch(() => undefined)); + return manager; +}; + +const startSession = async ( + manager: RizinDebugSessionManager, + { terminal }: Fixture, +): Promise => { + const started = await manager.start({ + path: "/usr/bin/cat", + inferior_tty: terminal.path, + }); + return successValue(started).session_id; +}; + +const expectRefusedClose = async ( + manager: RizinDebugSessionManager, + sessionId: string, +): Promise => { + const closed = await manager.close(sessionId); + if (closed.ok) + throw new Error("Close must refuse a target holding the command stream."); + expect(closed.error.message).toContain("Rizin command stream"); + expect(closed.error.cleanupIncomplete).toBe(true); + expect(closed.error.cleanupResources).toEqual([sessionId]); + expect(manager.status(sessionId)?.state).toBe("closed"); +}; + +const expectRunningOnTerminal = async ( + pid: number, + terminal: Terminal, +): Promise => { + expect(() => process.kill(pid, 0)).not.toThrow(); + const [stdin, selected] = await Promise.all([ + stat(`/proc/${String(pid)}/fd/0`), + stat(terminal.path), + ]); + expect([stdin.dev, stdin.ino]).toEqual([selected.dev, selected.ino]); +}; + +const registerCliCommand = (launcher: string) => { + const commands = new Map< + string, + { + run(input: { + args: { path: string; command: string }; + options: { inferiorTty: string }; + }): Promise; + } + >(); + const cli = { + command: (name: string, definition: never) => { + commands.set(name, definition); + return cli; + }, + } as unknown as CliInstance; + registerReverseEngineeringCommands(cli, pino({ enabled: false }), { + ...process.env, + REA_RIZIN_COMMAND: launcher, + }); + return commands.get(CLI_COMMANDS.debugWithRizin)!; +}; + +it.skipIf(process.platform === "win32")( + "keeps debugger commands away from a running target that reads stdin", + async () => { + const current = await createFixture({ + target: "cat", + reportsInferior: false, + }); + const manager = createManager(current, { + // The launcher reports no inferior, so close needs no Linux /proc readback. + platform: "linux", + frameTimeoutMs: 3_000, + }); + const sessionId = await startSession(manager, current); + + successValue(await manager.execute(sessionId, "resume-target")); + const answered = successValue(await manager.execute(sessionId, "dr")); + + expect(answered.result).toMatchObject({ + output: "rax=0x1", + completion_status: "complete", + }); + expect(current.terminal.output()).not.toContain("__REA_"); + + // Line-discipline echo shows the input once. A second copy needs `cat` to + // read it from the selected TTY, so the caller's input channel really works. + current.terminal.write("rea-input-probe\n"); + await expect + .poll( + () => current.terminal.output().split("rea-input-probe").length - 1, + { timeout: 2_000 }, + ) + .toBe(2); + successValue(await manager.close(sessionId)); + expect(await current.commands()).toEqual(["resume-target", "dr", "olj"]); + }, +); + +linuxIt.each([ + { name: "reads stdin", target: "cat", staysRunning: true }, + { name: "ignores stdin", target: "sleep 30", staysRunning: true }, + { name: "exits on resume", target: "true", staysRunning: false }, +])( + "detaches a target that $name through MCP", + async ({ target, staysRunning }) => { + const current = await createFixture({ target, reportsInferior: true }); + const manager = createManager(current); + const server = new McpServer({ name: "rizin-stdin", version: "1" }); + registerRizinDebugTools( + server, + manager, + silentLogger, + createToolResultDelivery(undefined), + ); + const client = new Client({ name: "rizin-stdin", version: "1" }); + const [clientTransport, serverTransport] = + InMemoryTransport.createLinkedPair(); + await server.connect(serverTransport); + await client.connect(clientTransport); + try { + const started = await client.callTool({ + name: "start_rizin_debug_session", + arguments: { + path: "/usr/bin/cat", + inferior_tty: current.terminal.path, + }, + }); + expect(started.isError, JSON.stringify(started)).not.toBe(true); + const { session_id: sessionId } = toolContract( + "start_rizin_debug_session", + ).outputSchema.parse(started.structuredContent); + + const closed = await client.callTool({ + name: "close_rizin_debug_session", + arguments: { session_id: sessionId }, + }); + + expect(closed.isError, JSON.stringify(closed)).not.toBe(true); + expect(closed.structuredContent).toEqual({ target_state: "unknown" }); + expect(manager.status(sessionId)).toBeUndefined(); + expect(await current.commands()).toEqual(DETACH_COMMANDS); + expect(current.terminal.output()).not.toContain("__REA_"); + if (staysRunning) + await expectRunningOnTerminal( + await current.targetPid(), + current.terminal, + ); + } finally { + await client.close(); + await server.close(); + } + }, +); + +linuxIt.each([ + { name: "closed", prelude: "exec 0<&-;" }, + { name: "redirected", prelude: "exec { + const current = await createFixture({ + prelude, + target: "sleep 30", + reportsInferior: true, + }); + const manager = createManager(current); + const sessionId = await startSession(manager, current); + + successValue(await manager.close(sessionId)); + + expect(await current.commands()).toEqual(DETACH_COMMANDS); + }, +); + +linuxIt("completes the one-shot CLI on a target that reads stdin", async () => { + const current = await createFixture({ + target: "cat", + reportsInferior: true, + }); + const previousExitCode = process.exitCode; + process.exitCode = 0; + try { + const result = await registerCliCommand(current.launcher).run({ + args: { path: "/usr/bin/cat", command: "dr" }, + options: { inferiorTty: current.terminal.path }, + }); + + expect(result).toMatchObject({ + operation: "rizin_debug_command", + normalized_result: { + output: "rax=0x1", + completion_status: "complete", + }, + }); + expect(process.exitCode).toBe(0); + expect(await current.commands()).toEqual(["dr", ...DETACH_COMMANDS]); + expect(current.terminal.output()).not.toContain("__REA_"); + await expectRunningOnTerminal(await current.targetPid(), current.terminal); + } finally { + process.exitCode = previousExitCode; + } +}); + +linuxIt( + "refuses to resume a target holding the command stream and retains ownership", + async () => { + const current = await createFixture({ + target: "cat", + reportsInferior: true, + ignoresStdinProfile: true, + }); + let debuggerExited: Promise | undefined; + const manager = createManager(current, { + spawnProcess: async (options) => { + const launched = await spawnOwnedProviderProcess(options); + debuggerExited = once(launched.process as ChildProcess, "exit"); + return launched; + }, + }); + const sessionId = await startSession(manager, current); + + await expectRefusedClose(manager, sessionId); + await expectRefusedClose(manager, sessionId); + expect(await current.commands()).toEqual([ + ...REFUSED_CLOSE_COMMANDS, + ...REFUSED_CLOSE_COMMANDS, + ]); + expect(current.terminal.output()).not.toContain("__REA_"); + await expect(manager.closeAll()).rejects.toThrow("did not stop cleanly"); + expect(manager.status(sessionId)).toBeDefined(); + + // The retained owner settles once the identified target and debugger end. + const targetPid = await current.targetPid(); + killIfRunning(-targetPid); + await expect + .poll(() => + readFile(`/proc/${String(targetPid)}/stat`, "utf8").then( + () => "running", + () => "ended", + ), + ) + .toBe("ended"); + killIfRunning(await current.launcherPid()); + await debuggerExited; + await manager.closeAll(); + expect(manager.status(sessionId)).toBeUndefined(); + }, +); + +linuxIt( + "reports the refusal as cleanup_incomplete from the one-shot CLI", + async () => { + const current = await createFixture({ + target: "cat", + reportsInferior: true, + ignoresStdinProfile: true, + }); + const previousExitCode = process.exitCode; + process.exitCode = 0; + try { + const result = await registerCliCommand(current.launcher).run({ + args: { path: "/usr/bin/cat", command: "dr" }, + options: { inferiorTty: current.terminal.path }, + }); + + expect(result).toMatchObject({ + code: "cleanup_incomplete", + details: { + cleanup: "incomplete", + partial_observation: { operation: "rizin_debug_command" }, + }, + }); + expect(JSON.stringify(result)).toContain("Rizin command stream"); + expect(process.exitCode).toBe(1); + expect(await current.commands()).toEqual([ + "dr", + ...REFUSED_CLOSE_COMMANDS, + ...REFUSED_CLOSE_COMMANDS, + ]); + } finally { + process.exitCode = previousExitCode; + } + }, +); diff --git a/tests/boundary/providers/RizinDebugSessionManagers.test.ts b/tests/boundary/providers/RizinDebugSessionManagers.test.ts index ea1d90a9d4..b485b2df6f 100644 --- a/tests/boundary/providers/RizinDebugSessionManagers.test.ts +++ b/tests/boundary/providers/RizinDebugSessionManagers.test.ts @@ -77,6 +77,8 @@ process.stdin.on("data", (chunk) => { "-0", "-d", "-R", + `stdin=${await rizinTestInferiorTty()}`, + "-R", `stdout=${await rizinTestInferiorTty()}`, "-R", `stderr=${await rizinTestInferiorTty()}`, diff --git a/tests/boundary/providers/RizinTestInferiorTty.ts b/tests/boundary/providers/RizinTestInferiorTty.ts index 294f6e1926..82d33260e9 100644 --- a/tests/boundary/providers/RizinTestInferiorTty.ts +++ b/tests/boundary/providers/RizinTestInferiorTty.ts @@ -5,6 +5,10 @@ import { stopOwnedDebuggerProcess } from "../../../src/process/ownedDebuggerSess let ttyPathPromise: Promise | undefined; +/** Matches the terminal device a test shell prints from `tty` on this host. */ +export const rizinTestTtyPathPattern = + process.platform === "darwin" ? /\/dev\/ttys[0-9a-z]+/u : /\/dev\/pts\/\d+/u; + export const rizinTestInferiorTty = (): Promise => { if (process.platform === "win32") return Promise.reject( @@ -39,11 +43,7 @@ export const rizinTestInferiorTty = (): Promise => { }); terminal.onData((chunk) => { output += chunk; - const pattern = - process.platform === "darwin" - ? /\/dev\/ttys[0-9a-z]+/u - : /\/dev\/pts\/\d+/u; - const match = pattern.exec(output); + const match = rizinTestTtyPathPattern.exec(output); if (match !== null) { clearTimeout(timer); resolve(match[0]);