From 51d110cacf833e413613ed38f14dc56efb9d72ff Mon Sep 17 00:00:00 2001 From: Kengwang Date: Tue, 11 Aug 2026 21:44:30 +0800 Subject: [PATCH 1/3] Add Windows Named Pipe transport --- AGENTS.md | 10 +- README.md | 16 ++- plugin/bn_agent_bridge/bridge.py | 213 ++++++++++++++++++++++++++----- skills/bn/SKILL.md | 2 +- src/bn/cli.py | 12 +- src/bn/paths.py | 20 +++ src/bn/transport.py | 144 ++++++++++++++++++++- tests/test_bridge.py | 55 ++++++++ tests/test_transport.py | 201 ++++++++++++++++++++++++++++- 9 files changed, 620 insertions(+), 53 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 341ea3a..9d3500f 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -36,7 +36,7 @@ bn skill install # Smoke-check the running bridge(s) bn doctor -bn daemon list # mode + pid + socket + target count per running daemon +bn daemon list # mode + pid + endpoint + target count per running daemon # Start a headless daemon (needs BN headless license on PYTHONPATH) PYTHONPATH=/path/to/binaryninja/python bn daemon start --foreground @@ -49,10 +49,10 @@ Tests do **not** require Binary Ninja. `tests/test_bridge.py` constructs a fake ### Wire path 1. A bridge is running: GUI auto-starts when Binary Ninja loads the plugin; headless is started by `bn daemon start`. -2. `BinaryNinjaBridge.start()` binds an `AF_UNIX` socket at `paths.bridge_socket_path(mode)` and writes a registry JSON at `paths.bridge_registry_path(mode)` (i.e. `cache_home()/daemons/{mode}.json`) carrying `pid`, `socket_path`, `plugin_version`, `plugin_build_id`, and `mode`. +2. `BinaryNinjaBridge.start()` selects its transport with `BN_BRIDGE_TRANSPORT=auto|pipe|tcp|unix`. `auto` means Unix-domain socket on Unix and authenticated Named Pipe on Windows. It writes a mode registry at `paths.bridge_registry_path(mode)` carrying the endpoint, authentication data when required, plugin metadata, and mode. 3. The CLI (`src/bn/transport.py`) calls `list_instances()` which scans `cache_home()/daemons/`. `choose_instance()` picks one via the sticky pointer at `cache_home()/current_daemon`, falling back to "the only one running" when no sticky is set. With both `gui` and `headless` alive and no sticky, the CLI errors and hints the user to run `bn daemon use `. -4. The CLI opens the chosen socket and sends a one-line JSON request: `{"id", "op", "params", "target"}`. It then `shutdown(SHUT_WR)` and reads the whole response until EOF. -5. `BridgeHandler.handle` parses the request and calls `BinaryNinjaBridge.dispatch`. Target-scoped operations are submitted to `JobManager`, which resolves the target to a concrete id, reserves read/write access, and executes `_execute_operation` on an ordinary Python background thread. Multiple reads may run concurrently; read/write and write/write conflicts fail immediately with the blocking job id instead of waiting. +4. The CLI opens the chosen endpoint and sends `{"id", "op", "params", "target"}`. Unix/TCP use a one-line stream request and read until EOF; Windows Pipe uses `multiprocessing.connection` message framing. TCP and Pipe requests include the registry token. +5. The transport handler authenticates and parses the request, then calls `BinaryNinjaBridge.dispatch`. Target-scoped operations are submitted to `JobManager`, which resolves the target to a concrete id, reserves read/write access, and executes `_execute_operation` on an ordinary Python background thread. Multiple reads may run concurrently; read/write and write/write conflicts fail immediately with the blocking job id instead of waiting. 6. The bridge replies with `{"ok", "result", "error"}`; the CLI raises `BridgeError` if `ok` is false. When adding a new operation: add the dispatch branch in `_execute_operation`, add the op name to either `READ_LOCKED_OPS` or `WRITE_LOCKED_OPS` (or deliberately leave it lock-free for state that has its own mutex), and add the matching CLI subparser + handler in `src/bn/cli.py`. @@ -113,7 +113,7 @@ So every mutation lane has four possible result statuses — `verified`, `previe ### `paths.py` is the single source of truth for filesystem layout -`src/bn/paths.py` and the symlinked `plugin/bn_agent_bridge/paths.py` define every path the system uses: per-mode bridge socket (`bridge_socket_path(mode)`), per-mode registry (`bridge_registry_path(mode)` under `bridge_registry_dir()`), sticky daemon mode pointer (`current_daemon_mode_path()`), cache home, spill root, plugin install dir, skill install dirs for both Codex (`$CODEX_HOME/skills/bn`) and Codex (`$CLAUDE_CONFIG_DIR/skills/bn`). The plugin's `paths.py` and `version.py` are **symlinks** into `src/bn/` — keep them as symlinks so the GUI plugin and the CLI agree on path layout without duplication. +`src/bn/paths.py` and the symlinked `plugin/bn_agent_bridge/paths.py` define the transport selection, per-mode Unix socket (`bridge_socket_path(mode)`), per-mode registry (`bridge_registry_path(mode)` under `bridge_registry_dir()`), sticky daemon mode pointer (`current_daemon_mode_path()`), cache home, spill root, plugin install dir, and skill install directories. The plugin's `paths.py` and `version.py` are **symlinks** into `src/bn/` — keep them as symlinks so the GUI plugin and the CLI agree without duplication. ### Async load tracking diff --git a/README.md b/README.md index 210e6ac..486b9dc 100644 --- a/README.md +++ b/README.md @@ -46,17 +46,19 @@ If the plugin code changes, reload Binary Ninja Python plugins or restart Binary - `bn` has two parts: - a normal Python CLI that you can run from your shell or agent tool harness - - a Binary Ninja bridge that exposes the API over a local transport: a Unix-domain socket on Unix, or authenticated loopback TCP on Windows + - a Binary Ninja bridge that exposes the API over a local transport: a Unix-domain socket on Unix, or an authenticated Windows Named Pipe on Windows - The bridge runs in one of two **modes**: - **`gui`** — loaded as a Binary Ninja plugin inside the GUI process. Works with a personal license. Started automatically when Binary Ninja opens with the plugin installed. - **`headless`** — long-running daemon started with `bn daemon start`. Requires a Binary Ninja commercial (headless) license. Built for containers, CI, and AI agent driver loops. -- Each mode has its own endpoint and registry file under the platform cache directory, so both can run simultaneously on the same machine. On Windows, GUI defaults to `127.0.0.1:26765` and headless to `127.0.0.1:26766`; registry entries include a random authentication token and the CLI only accepts loopback endpoints. Override the defaults with `BN_BRIDGE_GUI_PORT` or `BN_BRIDGE_HEADLESS_PORT`. +- Each mode has its own endpoint and registry file under the platform cache directory, so both can run simultaneously on the same machine. Windows uses a fresh local Named Pipe for each bridge start; the registry contains its name and a random authentication token. The CLI rejects remote or nested pipe names. - The CLI auto-discovers all running daemons. When only one is up, it routes to that one. When both are up, it routes to the **sticky** mode chosen via `bn daemon use ` (see [Daemon Mode Selection](#daemon-mode-selection)). -- **No repeated loading**: The daemon keeps loaded binaries resident in memory. Each CLI invocation just opens a socket, sends a JSON request, and reads the response — the binary is never re-imported. +- **No repeated loading**: The daemon keeps loaded binaries resident in memory. Each CLI invocation opens the registered endpoint, sends a JSON request, and reads the response — the binary is never re-imported. -### Changing the GUI bridge port +### Transport selection -On Windows, run `BN Agent Bridge\Set GUI Port...` inside Binary Ninja to save a new user-level port and restart the bridge immediately. You can also edit **GUI Listen Port** under the **BN Agent Bridge** group in Binary Ninja Settings, then run `BN Agent Bridge\Restart Bridge`. If the new port cannot be bound, the plugin restores the previous setting and listener instead of leaving the bridge offline. +`BN_BRIDGE_TRANSPORT=auto|pipe|tcp|unix` controls the bridge transport. `auto` is the default: Windows selects `pipe`, while Unix selects `unix`. Unsupported platform combinations fail explicitly and never fall back silently. + +For temporary Windows compatibility, set `BN_BRIDGE_TRANSPORT=tcp` before starting Binary Ninja or the headless daemon. TCP remains restricted to `127.0.0.1`, requires the registry authentication token, and uses `BN_BRIDGE_GUI_PORT` / `BN_BRIDGE_HEADLESS_PORT` or the GUI **Listen Port** setting. CLI and plugin should be upgraded together because older CLIs do not understand Pipe registry entries. ## Quick Start @@ -255,7 +257,7 @@ The headless daemon imports `binaryninja` and requires a Binary Ninja commercial ```bash bn daemon start --foreground # block in foreground (Docker PID 1 / systemd) -bn daemon status # pid, socket, target count +bn daemon status # pid, endpoint, target count bn daemon stop # authenticated graceful shutdown + registry cleanup ``` @@ -532,6 +534,8 @@ If `bn target list` is empty: - make sure the plugin is installed with `bn plugin install` - reload Binary Ninja plugins or restart Binary Ninja after plugin changes +On Windows, `bn doctor` should report a `pipe://\\.\pipe\...` endpoint. If Pipe creation or access fails, check that the CLI and plugin run as the same Windows user. For explicit compatibility mode, set `BN_BRIDGE_TRANSPORT=tcp` in both environments and restart the bridge; there is no automatic TCP fallback. + On Unix, if `bn doctor` sees a bridge registry but reports `Operation not permitted` under Codex, the Codex sandbox is blocking the Unix socket that connects to the live Binary Ninja GUI process. Let Codex run `bn` outside the sandbox by adding this rule to diff --git a/plugin/bn_agent_bridge/bridge.py b/plugin/bn_agent_bridge/bridge.py index b915a4a..cc03826 100644 --- a/plugin/bn_agent_bridge/bridge.py +++ b/plugin/bn_agent_bridge/bridge.py @@ -8,6 +8,7 @@ import hmac import io import json +import multiprocessing.connection import os import re import secrets @@ -33,6 +34,7 @@ bridge_job_root, bridge_socket_path, bridge_tcp_port, + bridge_transport, ) from .version import VERSION, build_id_for_file @@ -797,6 +799,42 @@ def resolve(self, selector: str | None): raise RuntimeError(f"Unknown target selector: {selector}{hint}") +def _process_bridge_request(bridge, raw: bytes) -> tuple[bytes, str | None, str | None, bool]: + op = None + request_id = None + try: + payload = json.loads(raw.decode("utf-8")) + if not isinstance(payload, dict): + raise ValueError("request must be a JSON object") + except (UnicodeDecodeError, json.JSONDecodeError, ValueError): + response = _json_response(ok=False, error="Invalid JSON request") + else: + op = payload.get("op") + request_id = payload.get("id") + expected_auth = bridge.auth_token + supplied_auth = payload.get("auth") + if expected_auth is not None and ( + not isinstance(supplied_auth, str) + or not hmac.compare_digest(supplied_auth, expected_auth) + ): + response = _json_response(ok=False, error="Unauthorized bridge request") + else: + response = bridge.dispatch(payload) + encoded = json.dumps(response, sort_keys=True, default=str).encode("utf-8") + should_shutdown = op == "shutdown" and bool(response.get("ok")) + return encoded, op, request_id, should_shutdown + + +def _log_disconnected_client(op: str | None, request_id: str | None) -> None: + details = [] + if op: + details.append(f"op={op}") + if request_id: + details.append(f"id={request_id}") + suffix = f" ({', '.join(details)})" if details else "" + bn.log_warn(f"BN Agent Bridge client disconnected before response could be delivered{suffix}") + + class BridgeHandler(socketserver.StreamRequestHandler): def _write_response( self, @@ -812,40 +850,18 @@ def _write_response( except OSError as exc: if exc.errno not in {errno.EPIPE, errno.ECONNRESET}: raise - details = [] - if op: - details.append(f"op={op}") - if request_id: - details.append(f"id={request_id}") - suffix = f" ({', '.join(details)})" if details else "" - bn.log_warn(f"BN Agent Bridge client disconnected before response could be delivered{suffix}") + _log_disconnected_client(op, request_id) return False def handle(self): # pragma: no cover - exercised from CLI raw = self.rfile.readline() if not raw: return - op = None - request_id = None - try: - payload = json.loads(raw.decode("utf-8")) - except json.JSONDecodeError: - response = _json_response(ok=False, error="Invalid JSON request") - else: - op = payload.get("op") - request_id = payload.get("id") - expected_auth = self.server.bridge.auth_token - supplied_auth = payload.get("auth") - if expected_auth is not None and ( - not isinstance(supplied_auth, str) - or not hmac.compare_digest(supplied_auth, expected_auth) - ): - response = _json_response(ok=False, error="Unauthorized bridge request") - else: - response = self.server.bridge.dispatch(payload) - encoded = json.dumps(response, sort_keys=True, default=str).encode("utf-8") + encoded, op, request_id, should_shutdown = _process_bridge_request( + self.server.bridge, raw + ) delivered = self._write_response(encoded, op=op, request_id=request_id) - if delivered and op == "shutdown" and response.get("ok"): + if delivered and should_shutdown: self.server.bridge.request_shutdown() @@ -872,6 +888,91 @@ def __init__(self, address: tuple[str, int], handler, bridge): super().__init__(address, handler) +class ThreadedNamedPipeServer: + def __init__(self, pipe_name: str, bridge): + self.pipe_name = pipe_name + self.bridge = bridge + self._listener = multiprocessing.connection.Listener( + pipe_name, + family="AF_PIPE", + backlog=64, + authkey=None, + ) + self._stopping = threading.Event() + self._stopped = threading.Event() + self._connections: set[Any] = set() + self._connections_lock = threading.Lock() + + def serve_forever(self) -> None: + try: + while not self._stopping.is_set(): + try: + connection = self._listener.accept() + except (EOFError, OSError) as exc: + if not self._stopping.is_set(): + bn.log_error(f"BN Agent Bridge Named Pipe accept failed: {exc}") + return + if self._stopping.is_set(): + connection.close() + return + with self._connections_lock: + self._connections.add(connection) + threading.Thread( + target=self._handle_connection, + args=(connection,), + daemon=True, + ).start() + finally: + self._stopped.set() + + def _handle_connection(self, connection) -> None: + op = None + request_id = None + try: + try: + raw = connection.recv_bytes() + except EOFError: + return + encoded, op, request_id, should_shutdown = _process_bridge_request( + self.bridge, raw + ) + try: + connection.send_bytes(encoded) + except (BrokenPipeError, EOFError, OSError): + _log_disconnected_client(op, request_id) + return + if should_shutdown: + self.bridge.request_shutdown() + finally: + with self._connections_lock: + self._connections.discard(connection) + with contextlib.suppress(OSError): + connection.close() + + def shutdown(self) -> None: + if self._stopping.is_set(): + return + self._stopping.set() + with contextlib.suppress(Exception): + wakeup = multiprocessing.connection.Client( + self.pipe_name, + family="AF_PIPE", + authkey=None, + ) + wakeup.close() + self._stopped.wait(timeout=2.0) + + def server_close(self) -> None: + with contextlib.suppress(Exception): + self._listener.close() + with self._connections_lock: + connections = list(self._connections) + self._connections.clear() + for connection in connections: + with contextlib.suppress(OSError): + connection.close() + + LOAD_ATTEMPTS_LIMIT = 50 COMPLETED_JOBS_LIMIT = 50 @@ -1173,11 +1274,12 @@ def __init__(self, mode: str = "gui"): self.targets = TargetManager(mode=mode) self.socket_path = bridge_socket_path(mode) self.registry_path = bridge_registry_path(mode) - self.transport = "unix" if ThreadedUnixServer is not None and hasattr(socket, "AF_UNIX") else "tcp" + self.transport = bridge_transport() self.host: str | None = None self.port: int | None = None + self.pipe_name: str | None = None self.auth_token: str | None = None - self._server: socketserver.BaseServer | None = None + self._server: Any = None self._thread: threading.Thread | None = None self.shutdown_event = threading.Event() self._target_locks: dict[str, _ReadWriteLock] = {} @@ -1242,12 +1344,31 @@ def _list_loads(self) -> list[dict[str, Any]]: def start(self) -> bool: # pragma: no cover - requires GUI runtime self.shutdown_event.clear() if self.transport == "unix": + if ThreadedUnixServer is None or not hasattr(socket, "AF_UNIX"): + bn.log_error("BN Agent Bridge Unix-domain sockets are unavailable") + return False self.socket_path.parent.mkdir(parents=True, exist_ok=True) if self.socket_path.exists(): self.socket_path.unlink() assert ThreadedUnixServer is not None self._server = ThreadedUnixServer(str(self.socket_path), BridgeHandler, self) - else: + elif self.transport == "pipe": + self.auth_token = secrets.token_urlsafe(32) + self.pipe_name = ( + rf"\\.\pipe\bn-agent-bridge-{self.mode}-{os.getpid()}-{secrets.token_hex(16)}" + ) + try: + self._server = ThreadedNamedPipeServer(self.pipe_name, self) + except (OSError, ValueError) as exc: + self._server = None + self.auth_token = None + bn.log_error( + f"BN Agent Bridge could not create Named Pipe {self.pipe_name}: {exc}. " + "Check the current user's Named Pipe permissions or set " + "BN_BRIDGE_TRANSPORT=tcp for explicit compatibility mode." + ) + return False + elif self.transport == "tcp": self.auth_token = secrets.token_urlsafe(32) requested_port = _configured_tcp_port(self.mode) try: @@ -1272,6 +1393,9 @@ def start(self) -> bool: # pragma: no cover - requires GUI runtime ) return False self.host, self.port = self._server.server_address + else: + bn.log_error(f"BN Agent Bridge does not support transport {self.transport!r}") + return False self._thread = threading.Thread(target=self._server.serve_forever, daemon=True) self._thread.start() self._write_registry() @@ -1285,6 +1409,8 @@ def request_shutdown(self) -> None: def endpoint(self) -> str: if self.transport == "tcp": return f"tcp://{self.host}:{self.port}" + if self.transport == "pipe": + return f"pipe://{self.pipe_name}" return str(self.socket_path) def stop(self): # pragma: no cover - requires GUI runtime @@ -1320,6 +1446,13 @@ def _write_registry(self): "auth_token": self.auth_token, } ) + elif self.transport == "pipe": + payload.update( + { + "pipe_name": self.pipe_name, + "auth_token": self.auth_token, + } + ) self.registry_path.parent.mkdir(parents=True, exist_ok=True) self.registry_path.write_text(json.dumps(payload, indent=2), encoding="utf-8") @@ -1929,6 +2062,7 @@ def _doctor(self): "plugin_build_id": PLUGIN_BUILD_ID, "pid": os.getpid(), "socket_path": self.endpoint, + "endpoint": self.endpoint, "transport": self.transport, "mode": self.mode, "targets": self.targets.refresh(), @@ -6541,7 +6675,12 @@ def _restart_gui_bridge(previous_port: int | None) -> tuple[BinaryNinjaBridge | def _restart_bridge_command(_): # pragma: no cover - GUI runtime - if ThreadedUnixServer is not None and hasattr(socket, "AF_UNIX"): + try: + selected_transport = bridge_transport() + except ValueError as exc: + _show_gui_message(f"Transport configuration is invalid: {exc}") + return + if selected_transport != "tcp": _stop_bridge() restarted = start_bridge(mode="gui") if restarted is not None: @@ -6610,7 +6749,11 @@ def start_bridge(mode: str = "gui"): # pragma: no cover - exercised in real bri return None if _bridge is not None: return _bridge - candidate = BinaryNinjaBridge(mode=mode) + try: + candidate = BinaryNinjaBridge(mode=mode) + except ValueError as exc: + bn.log_error(f"BN Agent Bridge transport configuration is invalid: {exc}") + return None if not candidate.start(): return None _bridge = candidate @@ -6655,7 +6798,11 @@ def _handle_signal(signum, _frame): "Restart the bn CLI bridge", _restart_bridge_command, ) - if ThreadedUnixServer is None or not hasattr(socket, "AF_UNIX"): + try: + _configured_plugin_transport = bridge_transport() + except ValueError: + _configured_plugin_transport = None + if _configured_plugin_transport == "tcp": _register_gui_settings() PluginCommand.register( "BN Agent Bridge\\Set GUI Port...", diff --git a/skills/bn/SKILL.md b/skills/bn/SKILL.md index 38eaabd..a875f13 100644 --- a/skills/bn/SKILL.md +++ b/skills/bn/SKILL.md @@ -36,7 +36,7 @@ bn target list # see loaded BinaryViews on the active daemon If `bn daemon list` shows both `gui` and `headless` running and no sticky mode is set, every command that needs a bridge will error with a hint. Run `bn daemon use ` to pin one, or `bn daemon use --clear` to drop the pin. -On Windows, the GUI bridge defaults to authenticated loopback TCP at `127.0.0.1:26765` and headless defaults to `127.0.0.1:26766`. Change the GUI listener inside Binary Ninja with `BN Agent Bridge\Set GUI Port...`, or edit **GUI Listen Port** in Binary Ninja Settings and run `BN Agent Bridge\Restart Bridge`. Keep using the `bn` CLI so it can read the authentication token from the daemon registry. `bn daemon stop` requests an authenticated graceful shutdown; it does not force-kill the daemon. +On Windows, both modes default to authenticated local Named Pipes with fresh names recorded in their daemon registries. Keep using the `bn` CLI so it can validate the endpoint and read the authentication token. If Pipe access fails, make sure the CLI and Binary Ninja run as the same Windows user. `BN_BRIDGE_TRANSPORT=tcp` enables explicit loopback TCP compatibility mode; it never activates automatically. In TCP mode, GUI/headless default to ports `26765`/`26766`, and the existing GUI port setting plus `BN_BRIDGE_GUI_PORT` / `BN_BRIDGE_HEADLESS_PORT` remain available. Upgrade the CLI and plugin together. `bn daemon stop` requests an authenticated graceful shutdown; it does not force-kill the daemon. 2. Pick a target: - If exactly one BinaryView is registered, target-scoped commands can omit `--target` entirely. diff --git a/src/bn/cli.py b/src/bn/cli.py index 469e3da..ace88b7 100644 --- a/src/bn/cli.py +++ b/src/bn/cli.py @@ -1290,8 +1290,9 @@ def _render_doctor_text(value: Any) -> str: lines.append(" stale: loaded plugin code does not match installed plugin file") if item.get("started_at"): lines.append(f" started: {item['started_at']}") - if item.get("socket_path"): - lines.append(f" socket: {item['socket_path']}") + endpoint = item.get("endpoint") or item.get("socket_path") + if endpoint: + lines.append(f" endpoint: {endpoint}") error = doctor.get("error") if error: lines.append(f" error: {error}") @@ -1478,6 +1479,7 @@ def _doctor(args: argparse.Namespace) -> int: instance_info = { "pid": instance.pid, "socket_path": instance.endpoint, + "endpoint": instance.endpoint, "transport": instance.transport, "plugin_version": instance.plugin_version, "plugin_build_id": loaded_build_id, @@ -1628,7 +1630,8 @@ def _render_daemon_list_text(value: Any) -> str: flag = " [current]" if sticky and item.get("mode") == sticky else "" lines.append( f"- mode={item.get('mode', '')} pid={item.get('pid', '?')} " - f"socket={item.get('socket_path', '')} targets={item.get('target_count', '?')}{flag}" + f"endpoint={item.get('endpoint') or item.get('socket_path', '')} " + f"targets={item.get('target_count', '?')}{flag}" ) return "\n".join(lines) @@ -1641,7 +1644,7 @@ def _render_daemon_status_text(value: Any) -> str: lines = [ f"mode: {value.get('mode')}", f"pid: {value.get('pid')}", - f"socket: {value.get('socket_path')}", + f"endpoint: {value.get('endpoint') or value.get('socket_path')}", f"registry: {value.get('registry_path')}", f"plugin_version: {value.get('plugin_version')}", f"target_count: {value.get('target_count')}", @@ -1665,6 +1668,7 @@ def _instance_summary(instance: BridgeInstance) -> dict[str, Any]: "mode": instance.mode, "pid": instance.pid, "socket_path": instance.endpoint, + "endpoint": instance.endpoint, "transport": instance.transport, "registry_path": str(instance.registry_path), "plugin_version": instance.plugin_version, diff --git a/src/bn/paths.py b/src/bn/paths.py index 540927a..e681b7d 100644 --- a/src/bn/paths.py +++ b/src/bn/paths.py @@ -48,12 +48,32 @@ def cache_home() -> Path: DAEMON_MODES: tuple[str, ...] = ("gui", "headless") +BRIDGE_TRANSPORTS: tuple[str, ...] = ("auto", "pipe", "tcp", "unix") DEFAULT_BRIDGE_TCP_PORTS = { "gui": 26765, "headless": 26766, } +def bridge_transport() -> str: + configured = os.environ.get("BN_BRIDGE_TRANSPORT", "auto").strip().lower() + if configured not in BRIDGE_TRANSPORTS: + expected = ", ".join(BRIDGE_TRANSPORTS) + raise ValueError( + f"BN_BRIDGE_TRANSPORT must be one of {expected}; got {configured!r}" + ) + + system = platform.system() + selected = "pipe" if configured == "auto" and system == "Windows" else configured + if selected == "auto": + selected = "unix" + if selected == "pipe" and system != "Windows": + raise ValueError("BN_BRIDGE_TRANSPORT=pipe is only supported on Windows") + if selected == "unix" and system == "Windows": + raise ValueError("BN_BRIDGE_TRANSPORT=unix is not supported on Windows") + return selected + + def _validate_mode(mode: str) -> str: if mode not in DAEMON_MODES: raise ValueError(f"Unknown daemon mode: {mode!r} (expected one of {DAEMON_MODES})") diff --git a/src/bn/transport.py b/src/bn/transport.py index 2dc71fb..a1e7bec 100644 --- a/src/bn/transport.py +++ b/src/bn/transport.py @@ -1,8 +1,11 @@ from __future__ import annotations import contextlib +import ctypes import errno import json +import multiprocessing.connection +import os import socket import time import uuid @@ -27,6 +30,19 @@ class BridgeError(RuntimeError): errno.ENOENT, } +TRANSIENT_PIPE_WINERRORS = { + 2, # ERROR_FILE_NOT_FOUND + 53, # ERROR_BAD_NETPATH + 121, # ERROR_SEM_TIMEOUT + 231, # ERROR_PIPE_BUSY +} + +DENIED_PIPE_WINERRORS = { + 5, # ERROR_ACCESS_DENIED +} + +LOCAL_PIPE_PREFIX = "\\\\.\\pipe\\" + DENIED_SOCKET_ERRNOS = { errno.EACCES, errno.EPERM, @@ -46,12 +62,15 @@ class BridgeInstance: transport: str = "unix" host: str | None = None port: int | None = None + pipe_name: str | None = None auth_token: str | None = None @property def endpoint(self) -> str: if self.transport == "tcp": return f"tcp://{self.host}:{self.port}" + if self.transport == "pipe": + return f"pipe://{self.pipe_name}" return str(self.socket_path) @@ -70,7 +89,54 @@ def _socket_args(instance: BridgeInstance) -> tuple[int, str | tuple[str, int]]: return socket.AF_UNIX, str(instance.socket_path) +def _validate_pipe_name(pipe_name: str) -> str: + if not pipe_name.startswith(LOCAL_PIPE_PREFIX): + raise OSError(errno.EINVAL, "Invalid local Named Pipe endpoint") + suffix = pipe_name[len(LOCAL_PIPE_PREFIX):] + if not suffix or "\\" in suffix or "/" in suffix: + raise OSError(errno.EINVAL, "Invalid local Named Pipe endpoint") + return pipe_name + + +def _pipe_error(winerror: int, message: str) -> OSError: + error = OSError(winerror, message) + error.winerror = winerror + return error + + +def _wait_named_pipe(pipe_name: str, timeout: float | None) -> None: + _validate_pipe_name(pipe_name) + if os.name != "nt": + raise OSError(errno.EPROTONOSUPPORT, "Windows Named Pipes are unavailable") + timeout_ms = 0xFFFFFFFF if timeout is None else max(0, min(int(timeout * 1000), 0xFFFFFFFE)) + kernel32 = ctypes.WinDLL("kernel32", use_last_error=True) + wait_named_pipe = kernel32.WaitNamedPipeW + wait_named_pipe.argtypes = [ctypes.c_wchar_p, ctypes.c_uint32] + wait_named_pipe.restype = ctypes.c_int + if not wait_named_pipe(pipe_name, timeout_ms): + winerror = ctypes.get_last_error() + raise _pipe_error(winerror, f"WaitNamedPipeW failed for {pipe_name!r}") + + +def _pipe_connect(pipe_name: str, timeout: float | None): + _wait_named_pipe(pipe_name, timeout) + return multiprocessing.connection.Client(pipe_name, family="AF_PIPE") + + +def _pipe_probe_error(instance: BridgeInstance, timeout: float) -> OSError | None: + try: + if instance.pipe_name is None: + raise OSError(errno.EINVAL, "Missing Named Pipe endpoint") + connection = _pipe_connect(instance.pipe_name, timeout) + connection.close() + return None + except OSError as exc: + return exc + + def _socket_probe_error(instance: BridgeInstance, timeout: float = 0.2) -> OSError | None: + if instance.transport == "pipe": + return _pipe_probe_error(instance, timeout) try: family, address = _socket_args(instance) with socket.socket(family, socket.SOCK_STREAM) as sock: @@ -116,6 +182,20 @@ def _load_instance(path: Path) -> BridgeInstance | None: _purge_stale_registry(path) return None socket_path = None + pipe_name = None + elif transport == "pipe": + try: + pipe_name = _validate_pipe_name(str(payload["pipe_name"])) + auth_token = payload["auth_token"] + except (KeyError, TypeError, OSError): + _purge_stale_registry(path) + return None + if not isinstance(auth_token, str) or not auth_token: + _purge_stale_registry(path) + return None + socket_path = None + host = None + port = None elif transport == "unix": try: socket_path = Path(payload["socket_path"]) @@ -125,6 +205,7 @@ def _load_instance(path: Path) -> BridgeInstance | None: host = None port = None auth_token = None + pipe_name = None if not socket_path.exists(): _purge_stale_registry(path) return None @@ -144,11 +225,15 @@ def _load_instance(path: Path) -> BridgeInstance | None: transport=transport, host=host, port=port, + pipe_name=pipe_name, auth_token=auth_token, ) probe_error = _socket_probe_error(instance) - if probe_error is not None and probe_error.errno in DENIED_SOCKET_ERRNOS: + probe_winerror = getattr(probe_error, "winerror", None) if probe_error is not None else None + if probe_error is not None and ( + probe_error.errno in DENIED_SOCKET_ERRNOS or probe_winerror in DENIED_PIPE_WINERRORS + ): payload["socket_probe_error"] = str(probe_error) elif probe_error is not None: _purge_stale_registry(path) @@ -257,6 +342,55 @@ def _send_request_to_instance( encoded = (json.dumps(payload) + "\n").encode("utf-8") + if instance.transport == "pipe": + if instance.pipe_name is None: + raise BridgeError("Named Pipe registry is missing pipe_name") + last_error: OSError | None = None + deadline = None if timeout is None else time.monotonic() + timeout + for attempt in range(connect_retries): + connection = None + try: + remaining = None if deadline is None else max(0.0, deadline - time.monotonic()) + connection = _pipe_connect(instance.pipe_name, remaining) + connection.send_bytes(encoded) + if remaining is not None: + remaining = max(0.0, deadline - time.monotonic()) + if not connection.poll(remaining): + raise TimeoutError("timed out waiting for Named Pipe response") + response_bytes = connection.recv_bytes() + return _decode_response(response_bytes) + except OSError as exc: + last_error = exc + winerror = getattr(exc, "winerror", None) + if ( + winerror not in TRANSIENT_PIPE_WINERRORS + or attempt == connect_retries - 1 + or (deadline is not None and time.monotonic() >= deadline) + ): + break + time.sleep(0.05 * (attempt + 1)) + except EOFError as exc: + last_error = OSError( + errno.ECONNRESET, + "Named Pipe closed before the bridge returned a response", + ) + last_error.__cause__ = exc + break + finally: + if connection is not None: + with contextlib.suppress(OSError): + connection.close() + assert last_error is not None + if isinstance(last_error, TimeoutError) or getattr(last_error, "winerror", None) == 121: + timeout_suffix = f" after {timeout:.1f}s" if timeout is not None else "" + raise BridgeError( + f"Timed out waiting for Binary Ninja bridge pid {instance.pid} at {instance.endpoint}" + f"{timeout_suffix}" + ) from last_error + raise BridgeError( + f"Failed to contact Binary Ninja bridge pid {instance.pid} at {instance.endpoint}: {last_error}" + ) from last_error + chunks: list[bytes] = [] last_error: OSError | None = None for attempt in range(connect_retries): @@ -295,9 +429,13 @@ def _send_request_to_instance( if not chunks: raise BridgeError("Binary Ninja bridge returned an empty response") + return _decode_response(b"".join(chunks)) + + +def _decode_response(encoded: bytes) -> dict[str, Any]: try: - response = json.loads(b"".join(chunks).decode("utf-8")) - except json.JSONDecodeError as exc: + response = json.loads(encoded.decode("utf-8")) + except (UnicodeDecodeError, json.JSONDecodeError) as exc: raise BridgeError("Binary Ninja bridge returned invalid JSON") from exc if not isinstance(response, dict): diff --git a/tests/test_bridge.py b/tests/test_bridge.py index c04e8e8..fb433c2 100644 --- a/tests/test_bridge.py +++ b/tests/test_bridge.py @@ -4,6 +4,8 @@ import importlib.util import io import json +import multiprocessing.connection +import os import socket import sys import threading @@ -2842,6 +2844,59 @@ def _request(payload): instance.stop() +@pytest.mark.skipif(os.name != "nt", reason="Windows Named Pipes are unavailable") +def test_named_pipe_bridge_registry_auth_and_concurrency(monkeypatch, tmp_path): + monkeypatch.setenv("BN_CACHE_DIR", str(tmp_path)) + monkeypatch.setenv("BN_BRIDGE_TRANSPORT", "pipe") + bridge = _load_bridge(monkeypatch) + instance = bridge.BinaryNinjaBridge(mode="headless") + assert instance.start() is True + + def request(request_id): + connection = multiprocessing.connection.Client( + instance.pipe_name, family="AF_PIPE", authkey=None + ) + try: + connection.send_bytes( + json.dumps( + { + "id": request_id, + "op": "doctor", + "params": {}, + "auth": instance.auth_token, + } + ).encode("utf-8") + ) + return json.loads(connection.recv_bytes().decode("utf-8")) + finally: + connection.close() + + try: + registry = json.loads(instance.registry_path.read_text(encoding="utf-8")) + assert registry["transport"] == "pipe" + assert registry["pipe_name"] == instance.pipe_name + assert registry["auth_token"] == instance.auth_token + assert instance.endpoint.startswith("pipe://\\\\.\\pipe\\bn-agent-bridge-headless-") + + results = [] + threads = [ + threading.Thread(target=lambda i=i: results.append(request(str(i)))) + for i in range(4) + ] + for thread in threads: + thread.start() + for thread in threads: + thread.join(timeout=2) + + assert len(results) == 4 + assert all(result["ok"] is True for result in results) + assert all(result["result"]["transport"] == "pipe" for result in results) + finally: + instance.stop() + + assert not instance.registry_path.exists() + + def test_binary_view_reads_do_not_marshal_to_ui_thread(monkeypatch): bridge = _load_bridge(monkeypatch) instance = bridge.BinaryNinjaBridge(mode="headless") diff --git a/tests/test_transport.py b/tests/test_transport.py index 7973e2c..b6f6289 100644 --- a/tests/test_transport.py +++ b/tests/test_transport.py @@ -3,6 +3,7 @@ import json import os import errno +import multiprocessing.connection import socket import socketserver import threading @@ -11,11 +12,18 @@ import pytest -from bn.paths import bridge_registry_path, bridge_tcp_port, current_daemon_mode_path +from bn.paths import ( + bridge_registry_path, + bridge_tcp_port, + bridge_transport, + current_daemon_mode_path, +) from bn.transport import ( BridgeError, BridgeInstance, + _send_request_to_instance, _socket_args, + _validate_pipe_name, choose_instance, list_instances, read_current_daemon_mode, @@ -70,6 +78,197 @@ def test_bridge_tcp_ports_are_fixed_and_overridable(monkeypatch): assert bridge_tcp_port("gui") == 30001 +def test_bridge_transport_defaults_by_platform(monkeypatch): + monkeypatch.delenv("BN_BRIDGE_TRANSPORT", raising=False) + monkeypatch.setattr("bn.paths.platform.system", lambda: "Windows") + assert bridge_transport() == "pipe" + + monkeypatch.setattr("bn.paths.platform.system", lambda: "Linux") + assert bridge_transport() == "unix" + + +def test_bridge_transport_validates_explicit_platform_choices(monkeypatch): + monkeypatch.setattr("bn.paths.platform.system", lambda: "Windows") + monkeypatch.setenv("BN_BRIDGE_TRANSPORT", "tcp") + assert bridge_transport() == "tcp" + + monkeypatch.setenv("BN_BRIDGE_TRANSPORT", "unix") + with pytest.raises(ValueError, match="not supported on Windows"): + bridge_transport() + + monkeypatch.setenv("BN_BRIDGE_TRANSPORT", "bogus") + with pytest.raises(ValueError, match="must be one of"): + bridge_transport() + + +def test_pipe_name_validation_rejects_remote_and_nested_names(): + assert _validate_pipe_name(r"\\.\pipe\bn-agent-bridge-gui-1-token") == ( + r"\\.\pipe\bn-agent-bridge-gui-1-token" + ) + with pytest.raises(OSError, match="Invalid local Named Pipe endpoint"): + _validate_pipe_name(r"\\server\pipe\bn-agent-bridge") + with pytest.raises(OSError, match="Invalid local Named Pipe endpoint"): + _validate_pipe_name(r"\\.\pipe\nested\bn-agent-bridge") + + +def test_list_instances_reads_pipe_registry(monkeypatch, tmp_path): + monkeypatch.setenv("BN_CACHE_DIR", str(tmp_path)) + monkeypatch.setattr("bn.transport._socket_probe_error", lambda _instance: None) + registry_path = bridge_registry_path("gui") + registry_path.parent.mkdir(parents=True, exist_ok=True) + pipe_name = r"\\.\pipe\bn-agent-bridge-gui-123-token" + registry_path.write_text( + json.dumps( + { + "pid": 123, + "transport": "pipe", + "pipe_name": pipe_name, + "auth_token": "secret", + "mode": "gui", + } + ), + encoding="utf-8", + ) + + instances = list_instances() + + assert len(instances) == 1 + assert instances[0].pipe_name == pipe_name + assert instances[0].endpoint == f"pipe://{pipe_name}" + assert instances[0].auth_token == "secret" + + +@pytest.mark.skipif(os.name != "nt", reason="Windows Named Pipes are unavailable") +def test_send_request_over_real_windows_named_pipe(tmp_path): + pipe_name = rf"\\.\pipe\bn-agent-bridge-test-{os.getpid()}-{uuid.uuid4().hex}" + listener = multiprocessing.connection.Listener( + pipe_name, family="AF_PIPE", backlog=4, authkey=None + ) + + def serve(): + connection = listener.accept() + try: + payload = json.loads(connection.recv_bytes().decode("utf-8")) + connection.send_bytes( + json.dumps( + { + "ok": True, + "result": { + "op": payload["op"], + "auth": payload["auth"], + }, + } + ).encode("utf-8") + ) + finally: + connection.close() + listener.close() + + thread = threading.Thread(target=serve, daemon=True) + thread.start() + instance = BridgeInstance( + pid=os.getpid(), + socket_path=None, + registry_path=tmp_path / "gui.json", + plugin_name="bn_agent_bridge", + plugin_version="0.15.0", + started_at=None, + meta={}, + transport="pipe", + pipe_name=pipe_name, + auth_token="test-token", + ) + + response = _send_request_to_instance(instance, "doctor", timeout=2.0) + + assert response["result"] == {"op": "doctor", "auth": "test-token"} + thread.join(timeout=2) + assert not thread.is_alive() + + +def _pipe_instance(tmp_path): + return BridgeInstance( + pid=999, + socket_path=None, + registry_path=tmp_path / "gui.json", + plugin_name="bn_agent_bridge", + plugin_version="0.15.0", + started_at=None, + meta={}, + transport="pipe", + pipe_name=r"\\.\pipe\bn-agent-bridge-gui-999-test", + auth_token="secret", + ) + + +def test_pipe_request_retries_busy_endpoint(monkeypatch, tmp_path): + class _Connection: + def send_bytes(self, payload): + self.payload = json.loads(payload.decode("utf-8")) + + def recv_bytes(self): + return json.dumps({"ok": True, "result": self.payload}).encode("utf-8") + + def close(self): + pass + + attempts = [] + + def connect(_pipe_name, _timeout): + attempts.append(True) + if len(attempts) == 1: + error = OSError(231, "All pipe instances are busy") + error.winerror = 231 + raise error + return _Connection() + + monkeypatch.setattr("bn.transport._pipe_connect", connect) + monkeypatch.setattr("bn.transport.time.sleep", lambda _delay: None) + + response = _send_request_to_instance(_pipe_instance(tmp_path), "doctor") + + assert len(attempts) == 2 + assert response["result"]["auth"] == "secret" + + +def test_pipe_request_reports_response_timeout(monkeypatch, tmp_path): + class _Connection: + def send_bytes(self, _payload): + pass + + def poll(self, _timeout): + return False + + def close(self): + pass + + monkeypatch.setattr( + "bn.transport._pipe_connect", lambda _pipe_name, _timeout: _Connection() + ) + + with pytest.raises(BridgeError, match="Timed out waiting for Binary Ninja bridge pid 999"): + _send_request_to_instance(_pipe_instance(tmp_path), "doctor", timeout=0.01) + + +def test_pipe_request_wraps_early_eof(monkeypatch, tmp_path): + class _Connection: + def send_bytes(self, _payload): + pass + + def recv_bytes(self): + raise EOFError + + def close(self): + pass + + monkeypatch.setattr( + "bn.transport._pipe_connect", lambda _pipe_name, _timeout: _Connection() + ) + + with pytest.raises(BridgeError, match="Failed to contact Binary Ninja bridge pid 999"): + _send_request_to_instance(_pipe_instance(tmp_path), "doctor") + + def test_socket_args_rejects_non_loopback_tcp_endpoint(tmp_path): instance = BridgeInstance( pid=123, From 74df544ad2c0041c3b6054e7cad302af291f8f05 Mon Sep 17 00:00:00 2001 From: Kengwang Date: Wed, 12 Aug 2026 01:55:36 +0800 Subject: [PATCH 2/3] Use fixed Windows Named Pipe names --- README.md | 2 +- plugin/bn_agent_bridge/bridge.py | 5 ++--- skills/bn/SKILL.md | 2 +- src/bn/paths.py | 4 ++++ tests/test_bridge.py | 2 +- tests/test_cli.py | 9 +++++---- tests/test_transport.py | 6 ++++++ 7 files changed, 20 insertions(+), 10 deletions(-) diff --git a/README.md b/README.md index 486b9dc..3f83c5b 100644 --- a/README.md +++ b/README.md @@ -50,7 +50,7 @@ If the plugin code changes, reload Binary Ninja Python plugins or restart Binary - The bridge runs in one of two **modes**: - **`gui`** — loaded as a Binary Ninja plugin inside the GUI process. Works with a personal license. Started automatically when Binary Ninja opens with the plugin installed. - **`headless`** — long-running daemon started with `bn daemon start`. Requires a Binary Ninja commercial (headless) license. Built for containers, CI, and AI agent driver loops. -- Each mode has its own endpoint and registry file under the platform cache directory, so both can run simultaneously on the same machine. Windows uses a fresh local Named Pipe for each bridge start; the registry contains its name and a random authentication token. The CLI rejects remote or nested pipe names. +- Each mode has its own endpoint and registry file under the platform cache directory, so both can run simultaneously on the same machine. Windows uses the fixed local Named Pipes `\\.\pipe\bn-agent-bridge-gui` and `\\.\pipe\bn-agent-bridge-headless`; the registry contains the selected endpoint and a random authentication token. The CLI rejects remote or nested pipe names. - The CLI auto-discovers all running daemons. When only one is up, it routes to that one. When both are up, it routes to the **sticky** mode chosen via `bn daemon use ` (see [Daemon Mode Selection](#daemon-mode-selection)). - **No repeated loading**: The daemon keeps loaded binaries resident in memory. Each CLI invocation opens the registered endpoint, sends a JSON request, and reads the response — the binary is never re-imported. diff --git a/plugin/bn_agent_bridge/bridge.py b/plugin/bn_agent_bridge/bridge.py index cc03826..55a71ba 100644 --- a/plugin/bn_agent_bridge/bridge.py +++ b/plugin/bn_agent_bridge/bridge.py @@ -32,6 +32,7 @@ PLUGIN_NAME, bridge_registry_path, bridge_job_root, + bridge_pipe_name, bridge_socket_path, bridge_tcp_port, bridge_transport, @@ -1354,9 +1355,7 @@ def start(self) -> bool: # pragma: no cover - requires GUI runtime self._server = ThreadedUnixServer(str(self.socket_path), BridgeHandler, self) elif self.transport == "pipe": self.auth_token = secrets.token_urlsafe(32) - self.pipe_name = ( - rf"\\.\pipe\bn-agent-bridge-{self.mode}-{os.getpid()}-{secrets.token_hex(16)}" - ) + self.pipe_name = bridge_pipe_name(self.mode) try: self._server = ThreadedNamedPipeServer(self.pipe_name, self) except (OSError, ValueError) as exc: diff --git a/skills/bn/SKILL.md b/skills/bn/SKILL.md index a875f13..a9800c9 100644 --- a/skills/bn/SKILL.md +++ b/skills/bn/SKILL.md @@ -36,7 +36,7 @@ bn target list # see loaded BinaryViews on the active daemon If `bn daemon list` shows both `gui` and `headless` running and no sticky mode is set, every command that needs a bridge will error with a hint. Run `bn daemon use ` to pin one, or `bn daemon use --clear` to drop the pin. -On Windows, both modes default to authenticated local Named Pipes with fresh names recorded in their daemon registries. Keep using the `bn` CLI so it can validate the endpoint and read the authentication token. If Pipe access fails, make sure the CLI and Binary Ninja run as the same Windows user. `BN_BRIDGE_TRANSPORT=tcp` enables explicit loopback TCP compatibility mode; it never activates automatically. In TCP mode, GUI/headless default to ports `26765`/`26766`, and the existing GUI port setting plus `BN_BRIDGE_GUI_PORT` / `BN_BRIDGE_HEADLESS_PORT` remain available. Upgrade the CLI and plugin together. `bn daemon stop` requests an authenticated graceful shutdown; it does not force-kill the daemon. +On Windows, both modes default to authenticated fixed local Named Pipes: `\\.\pipe\bn-agent-bridge-gui` and `\\.\pipe\bn-agent-bridge-headless`. Keep using the `bn` CLI so it can validate the endpoint and read the authentication token from the daemon registry. If Pipe access fails, make sure the CLI and Binary Ninja run as the same Windows user. `BN_BRIDGE_TRANSPORT=tcp` enables explicit loopback TCP compatibility mode; it never activates automatically. In TCP mode, GUI/headless default to ports `26765`/`26766`, and the existing GUI port setting plus `BN_BRIDGE_GUI_PORT` / `BN_BRIDGE_HEADLESS_PORT` remain available. Upgrade the CLI and plugin together. `bn daemon stop` requests an authenticated graceful shutdown; it does not force-kill the daemon. 2. Pick a target: - If exactly one BinaryView is registered, target-scoped commands can omit `--target` entirely. diff --git a/src/bn/paths.py b/src/bn/paths.py index e681b7d..d0afd24 100644 --- a/src/bn/paths.py +++ b/src/bn/paths.py @@ -92,6 +92,10 @@ def bridge_socket_path(mode: str) -> Path: return cache_home() / f"{PLUGIN_NAME}.{_validate_mode(mode)}.sock" +def bridge_pipe_name(mode: str) -> str: + return rf"\\.\pipe\bn-agent-bridge-{_validate_mode(mode)}" + + def bridge_job_root(mode: str) -> Path: return cache_home() / "jobs" / _validate_mode(mode) diff --git a/tests/test_bridge.py b/tests/test_bridge.py index fb433c2..79a79a7 100644 --- a/tests/test_bridge.py +++ b/tests/test_bridge.py @@ -2876,7 +2876,7 @@ def request(request_id): assert registry["transport"] == "pipe" assert registry["pipe_name"] == instance.pipe_name assert registry["auth_token"] == instance.auth_token - assert instance.endpoint.startswith("pipe://\\\\.\\pipe\\bn-agent-bridge-headless-") + assert instance.endpoint == "pipe://\\\\.\\pipe\\bn-agent-bridge-headless" results = [] threads = [ diff --git a/tests/test_cli.py b/tests/test_cli.py index d237d0e..08d6bf3 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -2375,7 +2375,8 @@ def __init__(self, cmd, **kwargs): assert "did not register" in capsys.readouterr().err -def _fake_headless_instance(tmp_path): +def _fake_headless_instance(tmp_path, monkeypatch): + monkeypatch.setenv("BN_CACHE_DIR", str(tmp_path)) registry = tmp_path / "daemons" / "headless.json" registry.parent.mkdir(parents=True, exist_ok=True) registry.write_text("{}", encoding="utf-8") @@ -2396,7 +2397,7 @@ def _fake_headless_instance(tmp_path): def test_daemon_stop_requests_graceful_shutdown(monkeypatch, tmp_path, capsys): - instance = _fake_headless_instance(tmp_path) + instance = _fake_headless_instance(tmp_path, monkeypatch) captured = {} def send(inst, op, **kwargs): @@ -2422,7 +2423,7 @@ def send(inst, op, **kwargs): def test_daemon_stop_explains_old_bridge_protocol(monkeypatch, tmp_path, capsys): - instance = _fake_headless_instance(tmp_path) + instance = _fake_headless_instance(tmp_path, monkeypatch) monkeypatch.setattr(bn.cli, "_find_instance", lambda _mode: instance) monkeypatch.setattr( bn.cli, @@ -2439,7 +2440,7 @@ def test_daemon_stop_explains_old_bridge_protocol(monkeypatch, tmp_path, capsys) def test_daemon_stop_errors_when_registry_does_not_disappear(monkeypatch, tmp_path, capsys): - instance = _fake_headless_instance(tmp_path) + instance = _fake_headless_instance(tmp_path, monkeypatch) monkeypatch.setattr(bn.cli, "_find_instance", lambda _mode: instance) monkeypatch.setattr( bn.cli, diff --git a/tests/test_transport.py b/tests/test_transport.py index b6f6289..016e183 100644 --- a/tests/test_transport.py +++ b/tests/test_transport.py @@ -13,6 +13,7 @@ import pytest from bn.paths import ( + bridge_pipe_name, bridge_registry_path, bridge_tcp_port, bridge_transport, @@ -87,6 +88,11 @@ def test_bridge_transport_defaults_by_platform(monkeypatch): assert bridge_transport() == "unix" +def test_bridge_pipe_names_are_fixed_per_mode(): + assert bridge_pipe_name("gui") == r"\\.\pipe\bn-agent-bridge-gui" + assert bridge_pipe_name("headless") == r"\\.\pipe\bn-agent-bridge-headless" + + def test_bridge_transport_validates_explicit_platform_choices(monkeypatch): monkeypatch.setattr("bn.paths.platform.system", lambda: "Windows") monkeypatch.setenv("BN_BRIDGE_TRANSPORT", "tcp") From 38f920b3a0f375ab2ad877f3dc400a436d903e7b Mon Sep 17 00:00:00 2001 From: Kengwang Date: Wed, 12 Aug 2026 02:04:45 +0800 Subject: [PATCH 3/3] Add GUI transport settings --- AGENTS.md | 2 +- README.md | 6 +- plugin/bn_agent_bridge/bridge.py | 117 ++++++++++++++++++++++++------- skills/bn/SKILL.md | 2 +- tests/test_bridge.py | 109 ++++++++++++++++++++++++++-- 5 files changed, 199 insertions(+), 37 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 9d3500f..324c4b9 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -49,7 +49,7 @@ Tests do **not** require Binary Ninja. `tests/test_bridge.py` constructs a fake ### Wire path 1. A bridge is running: GUI auto-starts when Binary Ninja loads the plugin; headless is started by `bn daemon start`. -2. `BinaryNinjaBridge.start()` selects its transport with `BN_BRIDGE_TRANSPORT=auto|pipe|tcp|unix`. `auto` means Unix-domain socket on Unix and authenticated Named Pipe on Windows. It writes a mode registry at `paths.bridge_registry_path(mode)` carrying the endpoint, authentication data when required, plugin metadata, and mode. +2. `BinaryNinjaBridge.start()` selects its transport with `BN_BRIDGE_TRANSPORT=auto|pipe|tcp|unix`. `auto` means Unix-domain socket on Unix, the Binary Ninja **GUI Transport** user setting for Windows GUI mode, and authenticated Named Pipe for Windows headless mode. An explicit environment value overrides the GUI setting. It writes a mode registry at `paths.bridge_registry_path(mode)` carrying the endpoint, authentication data when required, plugin metadata, and mode. 3. The CLI (`src/bn/transport.py`) calls `list_instances()` which scans `cache_home()/daemons/`. `choose_instance()` picks one via the sticky pointer at `cache_home()/current_daemon`, falling back to "the only one running" when no sticky is set. With both `gui` and `headless` alive and no sticky, the CLI errors and hints the user to run `bn daemon use `. 4. The CLI opens the chosen endpoint and sends `{"id", "op", "params", "target"}`. Unix/TCP use a one-line stream request and read until EOF; Windows Pipe uses `multiprocessing.connection` message framing. TCP and Pipe requests include the registry token. 5. The transport handler authenticates and parses the request, then calls `BinaryNinjaBridge.dispatch`. Target-scoped operations are submitted to `JobManager`, which resolves the target to a concrete id, reserves read/write access, and executes `_execute_operation` on an ordinary Python background thread. Multiple reads may run concurrently; read/write and write/write conflicts fail immediately with the blocking job id instead of waiting. diff --git a/README.md b/README.md index 3f83c5b..5377795 100644 --- a/README.md +++ b/README.md @@ -56,9 +56,11 @@ If the plugin code changes, reload Binary Ninja Python plugins or restart Binary ### Transport selection -`BN_BRIDGE_TRANSPORT=auto|pipe|tcp|unix` controls the bridge transport. `auto` is the default: Windows selects `pipe`, while Unix selects `unix`. Unsupported platform combinations fail explicitly and never fall back silently. +On Windows, choose **Named Pipe** or **Loopback TCP** with **GUI Transport** under the **BN Agent Bridge** group in Binary Ninja Settings. **GUI Listen Port** configures the TCP port. Run `BN Agent Bridge\Restart Bridge` after changing either setting. If the new transport or port cannot start, the plugin restores the previous working configuration. -For temporary Windows compatibility, set `BN_BRIDGE_TRANSPORT=tcp` before starting Binary Ninja or the headless daemon. TCP remains restricted to `127.0.0.1`, requires the registry authentication token, and uses `BN_BRIDGE_GUI_PORT` / `BN_BRIDGE_HEADLESS_PORT` or the GUI **Listen Port** setting. CLI and plugin should be upgraded together because older CLIs do not understand Pipe registry entries. +`BN_BRIDGE_TRANSPORT=auto|pipe|tcp|unix` also controls the bridge transport. `auto` is the default: the Windows GUI reads **GUI Transport**, Windows headless selects `pipe`, and Unix selects `unix`. An explicit environment value overrides the GUI setting. Unsupported platform combinations fail explicitly and never fall back silently. + +For Windows automation or headless compatibility, set `BN_BRIDGE_TRANSPORT=tcp` before starting Binary Ninja or the headless daemon. TCP remains restricted to `127.0.0.1`, requires the registry authentication token, and uses `BN_BRIDGE_GUI_PORT` / `BN_BRIDGE_HEADLESS_PORT` or the GUI **Listen Port** setting. CLI and plugin should be upgraded together because older CLIs do not understand Pipe registry entries. ## Quick Start diff --git a/plugin/bn_agent_bridge/bridge.py b/plugin/bn_agent_bridge/bridge.py index 55a71ba..402bc98 100644 --- a/plugin/bn_agent_bridge/bridge.py +++ b/plugin/bn_agent_bridge/bridge.py @@ -47,7 +47,9 @@ PLUGIN_BUILD_ID = build_id_for_file(Path(__file__).resolve()) SETTINGS_GROUP = "bnAgentBridge" +GUI_TRANSPORT_SETTING = f"{SETTINGS_GROUP}.guiTransport" GUI_PORT_SETTING = f"{SETTINGS_GROUP}.guiPort" +GUI_TRANSPORTS = ("pipe", "tcp") def _default_tcp_port(mode: str) -> int: @@ -64,6 +66,24 @@ def _register_gui_settings() -> None: return settings = bn.Settings() settings.register_group(SETTINGS_GROUP, "BN Agent Bridge") + settings.register_setting( + GUI_TRANSPORT_SETTING, + json.dumps( + { + "title": "GUI Transport", + "type": "string", + "default": "pipe", + "enum": list(GUI_TRANSPORTS), + "enumDescriptions": ["Named Pipe", "Loopback TCP"], + "description": ( + "Local transport used by the GUI bridge on Windows. Explicit " + "BN_BRIDGE_TRANSPORT values override this setting." + ), + "requiresRestart": True, + "ignore": ["SettingsProjectScope", "SettingsResourceScope"], + } + ), + ) settings.register_setting( GUI_PORT_SETTING, json.dumps( @@ -74,14 +94,38 @@ def _register_gui_settings() -> None: "minValue": 1, "maxValue": 65535, "description": ( - "Authenticated loopback TCP port used by the bn CLI. " + "Authenticated loopback TCP port used when GUI Transport is Loopback TCP. " "Run 'BN Agent Bridge > Restart Bridge' after changing it." ), + "requiresRestart": True, + "ignore": ["SettingsProjectScope", "SettingsResourceScope"], } ), ) +def _configured_transport(mode: str) -> str: + selected = bridge_transport() + env_value = os.environ.get("BN_BRIDGE_TRANSPORT", "auto").strip().lower() + if ( + mode != "gui" + or os.name != "nt" + or env_value != "auto" + or ui is None + or not hasattr(bn, "Settings") + ): + return selected + + configured = str(bn.Settings().get_string(GUI_TRANSPORT_SETTING)).strip().lower() + if configured in GUI_TRANSPORTS: + return configured + bn.log_error( + f"BN Agent Bridge setting {GUI_TRANSPORT_SETTING} is invalid ({configured!r}); " + f"using {selected}" + ) + return selected + + def _configured_tcp_port(mode: str) -> int: if mode == "gui" and ui is not None and hasattr(bn, "Settings"): port = int(bn.Settings().get_integer(GUI_PORT_SETTING)) @@ -1275,7 +1319,7 @@ def __init__(self, mode: str = "gui"): self.targets = TargetManager(mode=mode) self.socket_path = bridge_socket_path(mode) self.registry_path = bridge_registry_path(mode) - self.transport = bridge_transport() + self.transport = _configured_transport(mode) self.host: str | None = None self.port: int | None = None self.pipe_name: str | None = None @@ -6658,48 +6702,67 @@ def _set_gui_port_setting(port: int) -> bool: return bool(settings.set_integer(GUI_PORT_SETTING, port, scope=scope)) -def _restart_gui_bridge(previous_port: int | None) -> tuple[BinaryNinjaBridge | None, bool]: +def _set_gui_transport_setting(transport: str) -> bool: + if transport not in GUI_TRANSPORTS: + raise ValueError(f"Unsupported GUI transport: {transport!r}") + settings = bn.Settings() + scope = getattr(getattr(bn, "SettingsScope", None), "SettingsUserScope", None) + if scope is None: + return bool(settings.set_string(GUI_TRANSPORT_SETTING, transport)) + return bool(settings.set_string(GUI_TRANSPORT_SETTING, transport, scope=scope)) + + +def _restart_gui_bridge( + previous_transport: str | None, + previous_port: int | None, +) -> tuple[BinaryNinjaBridge | None, bool]: + desired_transport = _configured_transport("gui") desired_port = _configured_tcp_port("gui") _stop_bridge() restarted = start_bridge(mode="gui") if restarted is not None: return restarted, False - if previous_port is None or previous_port == desired_port: + restored_setting = False + if previous_transport is not None and previous_transport != desired_transport: + restored_setting = _set_gui_transport_setting(previous_transport) + elif ( + desired_transport == "tcp" + and previous_port is not None + and previous_port != desired_port + ): + restored_setting = _set_gui_port_setting(previous_port) + + if not restored_setting: return None, False - _set_gui_port_setting(previous_port) restored = start_bridge(mode="gui") return restored, restored is not None def _restart_bridge_command(_): # pragma: no cover - GUI runtime try: - selected_transport = bridge_transport() + selected_transport = _configured_transport("gui") except ValueError as exc: _show_gui_message(f"Transport configuration is invalid: {exc}") return - if selected_transport != "tcp": - _stop_bridge() - restarted = start_bridge(mode="gui") - if restarted is not None: - _show_gui_message(f"Bridge is listening on {restarted.endpoint}.") - else: - _show_gui_message("Could not restart the bridge. See the Binary Ninja log for details.") - return - - previous_port = _bridge.port if _bridge is not None and _bridge.transport == "tcp" else None - desired_port = _configured_tcp_port("gui") - restarted, restored = _restart_gui_bridge(previous_port) + previous_transport = _bridge.transport if _bridge is not None else None + previous_port = ( + int(_bridge.port) + if _bridge is not None and _bridge.transport == "tcp" and _bridge.port is not None + else None + ) + restarted, restored = _restart_gui_bridge(previous_transport, previous_port) if restarted is not None and not restored: _show_gui_message(f"Bridge is listening on {restarted.endpoint}.") elif restored: _show_gui_message( - f"Could not listen on port {desired_port}; restored the bridge on {restarted.endpoint}." + f"Could not start {selected_transport}; restored the bridge on {restarted.endpoint}." ) else: _show_gui_message( - f"Could not start the bridge on port {desired_port}. See the Binary Ninja log for details." + f"Could not start the bridge with {selected_transport}. " + "See the Binary Ninja log for details." ) @@ -6727,7 +6790,13 @@ def _set_gui_port_command(_): # pragma: no cover - GUI runtime _show_gui_message("Binary Ninja could not save the GUI port setting.") return - restarted, restored = _restart_gui_bridge(previous_port) + selected_transport = _configured_transport("gui") + if selected_transport != "tcp": + _show_gui_message(f"Port {port} saved for Loopback TCP mode.") + return + + previous_transport = _bridge.transport if _bridge is not None else "tcp" + restarted, restored = _restart_gui_bridge(previous_transport, previous_port) if restarted is not None and not restored: _show_gui_message(f"Bridge is now listening on {restarted.endpoint}.") elif restored: @@ -6797,11 +6866,7 @@ def _handle_signal(signum, _frame): "Restart the bn CLI bridge", _restart_bridge_command, ) - try: - _configured_plugin_transport = bridge_transport() - except ValueError: - _configured_plugin_transport = None - if _configured_plugin_transport == "tcp": + if os.name == "nt": _register_gui_settings() PluginCommand.register( "BN Agent Bridge\\Set GUI Port...", diff --git a/skills/bn/SKILL.md b/skills/bn/SKILL.md index a9800c9..f0e429e 100644 --- a/skills/bn/SKILL.md +++ b/skills/bn/SKILL.md @@ -36,7 +36,7 @@ bn target list # see loaded BinaryViews on the active daemon If `bn daemon list` shows both `gui` and `headless` running and no sticky mode is set, every command that needs a bridge will error with a hint. Run `bn daemon use ` to pin one, or `bn daemon use --clear` to drop the pin. -On Windows, both modes default to authenticated fixed local Named Pipes: `\\.\pipe\bn-agent-bridge-gui` and `\\.\pipe\bn-agent-bridge-headless`. Keep using the `bn` CLI so it can validate the endpoint and read the authentication token from the daemon registry. If Pipe access fails, make sure the CLI and Binary Ninja run as the same Windows user. `BN_BRIDGE_TRANSPORT=tcp` enables explicit loopback TCP compatibility mode; it never activates automatically. In TCP mode, GUI/headless default to ports `26765`/`26766`, and the existing GUI port setting plus `BN_BRIDGE_GUI_PORT` / `BN_BRIDGE_HEADLESS_PORT` remain available. Upgrade the CLI and plugin together. `bn daemon stop` requests an authenticated graceful shutdown; it does not force-kill the daemon. +On Windows, both modes default to authenticated fixed local Named Pipes: `\\.\pipe\bn-agent-bridge-gui` and `\\.\pipe\bn-agent-bridge-headless`. In the GUI, **BN Agent Bridge > GUI Transport** switches between Named Pipe and Loopback TCP, and **GUI Listen Port** configures the port; run `BN Agent Bridge > Restart Bridge` after changing either. Explicit `BN_BRIDGE_TRANSPORT=pipe|tcp` overrides the GUI setting and controls headless mode; TCP defaults to ports `26765`/`26766` and also supports `BN_BRIDGE_GUI_PORT` / `BN_BRIDGE_HEADLESS_PORT`. Keep using the `bn` CLI so it can validate the endpoint and read the authentication token from the daemon registry. Upgrade the CLI and plugin together. `bn daemon stop` requests an authenticated graceful shutdown; it does not force-kill the daemon. 2. Pick a target: - If exactly one BinaryView is registered, target-scoped commands can omit `--target` entirely. diff --git a/tests/test_bridge.py b/tests/test_bridge.py index 79a79a7..8b4e9e8 100644 --- a/tests/test_bridge.py +++ b/tests/test_bridge.py @@ -2625,11 +2625,19 @@ def register_setting(self, key, properties): def get_integer(self, key): return self.values.get(key, self.schemas[key]["default"]) + def get_string(self, key): + return self.values.get(key, self.schemas[key]["default"]) + def set_integer(self, key, value, **kwargs): self.values[key] = int(value) self.set_calls.append((key, int(value), kwargs)) return True + def set_string(self, key, value, **kwargs): + self.values[key] = str(value) + self.set_calls.append((key, str(value), kwargs)) + return True + def test_gui_port_setting_is_registered_and_used(monkeypatch): bridge = _load_bridge(monkeypatch) @@ -2640,15 +2648,48 @@ def test_gui_port_setting_is_registered_and_used(monkeypatch): bridge._register_gui_settings() - schema = settings.schemas[bridge.GUI_PORT_SETTING] + transport_schema = settings.schemas[bridge.GUI_TRANSPORT_SETTING] + port_schema = settings.schemas[bridge.GUI_PORT_SETTING] assert settings.groups[bridge.SETTINGS_GROUP] == "BN Agent Bridge" - assert schema["default"] == 26765 - assert schema["minValue"] == 1 - assert schema["maxValue"] == 65535 + assert transport_schema["default"] == "pipe" + assert transport_schema["enum"] == ["pipe", "tcp"] + assert transport_schema["enumDescriptions"] == ["Named Pipe", "Loopback TCP"] + assert transport_schema["requiresRestart"] is True + assert port_schema["default"] == 26765 + assert port_schema["minValue"] == 1 + assert port_schema["maxValue"] == 65535 + assert port_schema["requiresRestart"] is True settings.values[bridge.GUI_PORT_SETTING] = 28000 assert bridge._configured_tcp_port("gui") == 28000 +def test_gui_transport_setting_is_used_in_auto_mode(monkeypatch): + bridge = _load_bridge(monkeypatch) + settings = _FakePluginSettings() + monkeypatch.delenv("BN_BRIDGE_TRANSPORT", raising=False) + monkeypatch.setattr(bridge, "ui", object()) + monkeypatch.setattr(bridge.bn, "Settings", lambda: settings, raising=False) + monkeypatch.setattr(bridge.os, "name", "nt") + bridge._register_gui_settings() + + settings.values[bridge.GUI_TRANSPORT_SETTING] = "tcp" + + assert bridge._configured_transport("gui") == "tcp" + assert bridge._configured_transport("headless") == bridge.bridge_transport() + + +def test_explicit_transport_environment_overrides_gui_setting(monkeypatch): + bridge = _load_bridge(monkeypatch) + settings = _FakePluginSettings() + monkeypatch.setenv("BN_BRIDGE_TRANSPORT", "pipe") + monkeypatch.setattr(bridge, "ui", object()) + monkeypatch.setattr(bridge.bn, "Settings", lambda: settings, raising=False) + bridge._register_gui_settings() + settings.values[bridge.GUI_TRANSPORT_SETTING] = "tcp" + + assert bridge._configured_transport("gui") == "pipe" + + def test_set_gui_port_command_saves_and_restarts(monkeypatch): bridge = _load_bridge(monkeypatch) settings = _FakePluginSettings() @@ -2664,7 +2705,12 @@ def test_set_gui_port_command_saves_and_restarts(monkeypatch): raising=False, ) monkeypatch.setattr(bridge.bn, "get_int_input", lambda *_args: 28000, raising=False) - monkeypatch.setattr(bridge, "_restart_gui_bridge", lambda previous: (restarted, False)) + monkeypatch.setattr(bridge, "_configured_transport", lambda _mode: "tcp") + monkeypatch.setattr( + bridge, + "_restart_gui_bridge", + lambda previous_transport, previous_port: (restarted, False), + ) monkeypatch.setattr(bridge, "_show_gui_message", messages.append) bridge._set_gui_port_command(None) @@ -2708,11 +2754,16 @@ def test_restart_gui_bridge_restores_previous_port_on_bind_failure(monkeypatch): stopped = [] restored_ports = [] monkeypatch.setattr(bridge, "_configured_tcp_port", lambda _mode: 28000) + monkeypatch.setattr(bridge, "_configured_transport", lambda _mode: "tcp") monkeypatch.setattr(bridge, "_stop_bridge", lambda: stopped.append(True)) monkeypatch.setattr(bridge, "start_bridge", lambda mode: next(starts)) - monkeypatch.setattr(bridge, "_set_gui_port_setting", restored_ports.append) + def restore_port(value): + restored_ports.append(value) + return True + + monkeypatch.setattr(bridge, "_set_gui_port_setting", restore_port) - result, did_restore = bridge._restart_gui_bridge(26765) + result, did_restore = bridge._restart_gui_bridge("tcp", 26765) assert stopped == [True] assert restored_ports == [26765] @@ -2720,6 +2771,50 @@ def test_restart_gui_bridge_restores_previous_port_on_bind_failure(monkeypatch): assert did_restore is True +def test_restart_gui_bridge_restores_previous_transport_on_failure(monkeypatch): + bridge = _load_bridge(monkeypatch) + restored = types.SimpleNamespace(endpoint="pipe://\\\\.\\pipe\\bn-agent-bridge-gui") + starts = iter([None, restored]) + restored_transports = [] + monkeypatch.setattr(bridge, "_configured_transport", lambda _mode: "tcp") + monkeypatch.setattr(bridge, "_stop_bridge", lambda: None) + monkeypatch.setattr(bridge, "start_bridge", lambda mode: next(starts)) + def restore_transport(value): + restored_transports.append(value) + return True + + monkeypatch.setattr(bridge, "_set_gui_transport_setting", restore_transport) + + result, did_restore = bridge._restart_gui_bridge("pipe", None) + + assert restored_transports == ["pipe"] + assert result is restored + assert did_restore is True + + +def test_set_gui_port_command_only_saves_while_pipe_is_selected(monkeypatch): + bridge = _load_bridge(monkeypatch) + settings = _FakePluginSettings() + settings.schemas[bridge.GUI_PORT_SETTING] = {"default": 26765} + messages = [] + monkeypatch.setattr(bridge.bn, "Settings", lambda: settings, raising=False) + monkeypatch.setattr(bridge.bn, "get_int_input", lambda *_args: 28000, raising=False) + monkeypatch.setattr(bridge, "_configured_transport", lambda _mode: "pipe") + monkeypatch.setattr( + bridge, + "_restart_gui_bridge", + lambda *_args: (_ for _ in ()).throw(AssertionError("must not restart")), + ) + monkeypatch.setattr(bridge, "_show_gui_message", messages.append) + bridge._bridge = types.SimpleNamespace(port=None, transport="pipe") + + bridge._set_gui_port_command(None) + + assert settings.values[bridge.GUI_PORT_SETTING] == 28000 + assert messages == ["Port 28000 saved for Loopback TCP mode."] + bridge._bridge = None + + def test_tcp_port_conflict_fails_without_overwriting_registry(monkeypatch, tmp_path): monkeypatch.setenv("BN_CACHE_DIR", str(tmp_path)) bridge = _load_bridge(monkeypatch)