diff --git a/AGENTS.md b/AGENTS.md index 341ea3a..324c4b9 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, 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 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..5377795 100644 --- a/README.md +++ b/README.md @@ -46,17 +46,21 @@ 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 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 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. +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. + +`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 @@ -255,7 +259,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 +536,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..402bc98 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 @@ -31,8 +32,10 @@ PLUGIN_NAME, bridge_registry_path, bridge_job_root, + bridge_pipe_name, bridge_socket_path, bridge_tcp_port, + bridge_transport, ) from .version import VERSION, build_id_for_file @@ -44,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: @@ -61,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( @@ -71,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)) @@ -797,6 +844,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 +895,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 +933,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 +1319,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 = _configured_transport(mode) 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 +1389,29 @@ 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 = bridge_pipe_name(self.mode) + 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 +1436,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 +1452,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 +1489,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 +2105,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(), @@ -6525,43 +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 - if ThreadedUnixServer is not None and hasattr(socket, "AF_UNIX"): - _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.") + try: + selected_transport = _configured_transport("gui") + except ValueError as exc: + _show_gui_message(f"Transport configuration is invalid: {exc}") 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." ) @@ -6589,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: @@ -6610,7 +6817,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 +6866,7 @@ def _handle_signal(signum, _frame): "Restart the bn CLI bridge", _restart_bridge_command, ) - if ThreadedUnixServer is None or not hasattr(socket, "AF_UNIX"): + 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 38eaabd..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, 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 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/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..d0afd24 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})") @@ -72,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/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..8b4e9e8 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 @@ -2623,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) @@ -2638,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() @@ -2662,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) @@ -2706,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 - result, did_restore = bridge._restart_gui_bridge(26765) + monkeypatch.setattr(bridge, "_set_gui_port_setting", restore_port) + + result, did_restore = bridge._restart_gui_bridge("tcp", 26765) assert stopped == [True] assert restored_ports == [26765] @@ -2718,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) @@ -2842,6 +2939,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 == "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_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 7973e2c..016e183 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,19 @@ import pytest -from bn.paths import bridge_registry_path, bridge_tcp_port, current_daemon_mode_path +from bn.paths import ( + bridge_pipe_name, + 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 +79,202 @@ 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_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") + 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,