From 6fa931aa36cc15b0dff371f464ad6f51ea2b6818 Mon Sep 17 00:00:00 2001 From: Daniel Dolezal Date: Thu, 18 Jun 2026 14:24:15 +0200 Subject: [PATCH] feat: harden remote serve and reuse connections - Gate TCP serve commands with safe-by-default policies, per-key allow tokens, per-key rate limiting, and audit labels. - Reuse authenticated encrypted remote sessions and parallelize/caches multi-browser fanout to reduce repeated handshake roundtrips. - Increase paged native-host batch size with extension-side byte budgeting to speed large tab listings safely. - Point install output at public Chrome Web Store / Firefox AMO listings by default, with --dev preserving unpacked workflows. - Share search-engine metadata between CLI and SDK and bump the package/extension version to 0.16.0. - Cover the new security, pooling, paging, install, and fanout behavior with expanded Python and extension tests. --- README.md | 47 +- browser_cli/auth/__init__.py | 4 + browser_cli/auth/keys.py | 50 +- browser_cli/cli.py | 20 +- browser_cli/client/__init__.py | 2 + browser_cli/client/core.py | 211 ++++- browser_cli/client/targets.py | 5 + browser_cli/command_security.py | 17 +- browser_cli/commands/__init__.py | 60 ++ browser_cli/commands/auth.py | 45 +- browser_cli/commands/clients.py | 142 +--- browser_cli/commands/doctor.py | 23 +- browser_cli/commands/install.py | 55 +- browser_cli/commands/raw.py | 22 +- browser_cli/commands/script.py | 12 +- browser_cli/commands/search.py | 57 +- browser_cli/commands/serve.py | 233 ++++-- browser_cli/commands/serve_http.py | 15 +- browser_cli/constants.py | 20 +- browser_cli/models.py | 42 +- browser_cli/native/host.py | 10 +- browser_cli/remote/pool.py | 103 +++ browser_cli/remote/socket.py | 9 +- browser_cli/remote/transport.py | 30 +- browser_cli/sdk/navigation.py | 2 +- browser_cli/sdk/routing.py | 32 +- browser_cli/search/__init__.py | 1 + browser_cli/search/engines.py | 53 ++ browser_cli/serve/control.py | 43 +- browser_cli/serve/logging.py | 14 +- browser_cli/serve/proxy.py | 9 +- browser_cli/serve/runtime.py | 93 ++- browser_cli/serve/security.py | 115 +++ browser_cli/version_manager.py | 34 +- extension/manifest.json | 2 +- extension/src/classes/CommandRegistry.ts | 22 +- extension/test/paging.test.ts | 41 + pyproject.toml | 2 +- tests/conftest.py | 64 +- tests/test_api.py | 26 +- tests/test_cli.py | 56 +- tests/test_client.py | 911 ++++++++++++--------- tests/test_native_host.py | 570 ++++++------- tests/test_new_feature_commands.py | 425 ++++++---- tests/test_remote_pool.py | 52 ++ tests/test_remote_protocol_matrix.py | 354 +++++---- tests/test_serve.py | 966 +++++++++++++---------- tests/test_serve_security.py | 162 ++++ uv.lock | 2 +- 49 files changed, 3407 insertions(+), 1878 deletions(-) create mode 100644 browser_cli/remote/pool.py create mode 100644 browser_cli/search/__init__.py create mode 100644 browser_cli/search/engines.py create mode 100644 browser_cli/serve/security.py create mode 100644 extension/test/paging.test.ts create mode 100644 tests/test_remote_pool.py create mode 100644 tests/test_serve_security.py diff --git a/README.md b/README.md index 4f1b566..3bf521f 100644 --- a/README.md +++ b/README.md @@ -53,8 +53,10 @@ Every response: **Requirements:** Python 3.10+, [uv](https://github.com/astral-sh/uv), Chrome, Chromium, Brave, Edge, Vivaldi, or Firefox +browser-cli has two parts: the **CLI / native host** (a Python package) and the **browser extension** (published on the public stores). + ### Install with uv -Install the CLI from PyPI as a uv tool: +Install the CLI from PyPI as a uv tool, then register the native host: ```sh uv tool install real-browser-cli @@ -76,21 +78,31 @@ To upgrade later: uv tool upgrade real-browser-cli ``` +### Add the browser extension +Install the extension from its public store listing (the `install` command prints the right link for you): + +- Chrome / Chromium / Brave / Edge / Vivaldi — [Chrome Web Store](https://chromewebstore.google.com/detail/browser-cli/hekaebjhbhhdbmakimmaklbblbmccahp) +- Firefox — [Firefox Add-ons](https://addons.mozilla.org/firefox/addon/browser-cli/) + +The native host manifest trusts both the published store ID and the unpacked development ID, so the store extension works out of the box. If you are hacking on the extension yourself, run `browser-cli install --dev` for the unpacked / temporary-add-on load steps instead. + ### Install from source ```sh git clone cd browser-cli uv sync -uv run browser-cli install brave # or: chrome, chromium, edge, vivaldi, firefox +npm ci && npm run build:extension # build the unpacked extension bundles +uv run browser-cli install brave --dev # --dev prints unpacked-load steps; or: chrome, chromium, edge, vivaldi, firefox ``` -The `install` command will: -1. Ask you to load the browser-specific extension package -2. Show the stable extension ID used by that browser family -3. Write the native messaging manifest to your OS so the browser can find the host -4. Copy the native host into an internal `libexec` directory and create a small wrapper outside your `PATH` +Omit `--dev` to be pointed at the public store listing instead of loading the unpacked build. -After install, **fully restart your browser** (Quit and reopen — not just close the window). The extension will connect to the native host automatically on startup. +The `install` command will: +1. Write the native messaging manifest to your OS so the browser can find the host +2. Copy the native host into an internal `libexec` directory and create a small wrapper outside your `PATH` +3. Print the public store link for installing the extension (or, with `--dev`, the unpacked / temporary-add-on load steps) + +After install, add the extension from the store link above and **fully restart your browser** (Quit and reopen — not just close the window). The extension will connect to the native host automatically on startup. Only the `browser-cli` command needs to be on your `PATH`. The browser launches the native host wrapper directly from its absolute path in the native messaging manifest, and that wrapper imports the installed `browser_cli.native.host` entry point. On Windows the install command also registers the host in the current user's Registry for the selected browser. @@ -305,6 +317,13 @@ PUBKEY=$(browser-cli auth show --key ~/.config/browser-cli/client.key | tail -n1 browser-cli auth trust "$PUBKEY" browser-cli serve --host 0.0.0.0 --port 8765 --authorized-keys ~/.config/browser-cli/authorized_keys +# Allow remote browser control (navigation, clicks); safe-only otherwise +browser-cli serve --authorized-keys ~/.config/browser-cli/authorized_keys --allow-control + +# Per-key authorization (inline in authorized_keys) + a tighter rate limit +browser-cli auth trust "$PUBKEY" --name ci-bot --allow-read-page --allow-control +browser-cli serve --authorized-keys ~/.config/browser-cli/authorized_keys --rate-limit 20 + # From another machine browser-cli --remote browser-host.example:8765 --key ~/.config/browser-cli/client.key tabs list browser-cli remote trust browser-host.example:8765 ~/.config/browser-cli/client.key @@ -317,6 +336,16 @@ curl -H "Authorization: Bearer " http://127.0.0.1:8766/tabs Remote auth uses Ed25519 challenge/response. `--remote` domains default to port 443; explicit `host:port` endpoints are also supported. Saved remote endpoints participate in aggregate list/count commands, where output is grouped by endpoint. +#### Security model + +- **`serve` (TCP)** authenticates every connection with an Ed25519 signature over a fresh server nonce and, for modern clients, wraps the transport in an ML-KEM-768 (post-quantum) AEAD channel. Commands are gated by a **safe-only policy by default** — even a trusted key can only run read-only status/listing commands until you open more with `--allow-read-page`, `--allow-control`, `--allow-dangerous`, or `--allow-all` (full control, including `dom.eval`/`storage.*`). `--no-auth` is rejected on non-loopback hosts. + - **Per-key authorization:** a key in `authorized_keys` can carry an optional `allow:` token (` allow:read-page,control`) listing its categories (`all`, `safe`, `read-page`, `control`, `dangerous`). That key uses its own policy, overriding the server-wide `--allow-*` default; keys without a token fall back to the default. Set it with `auth trust --allow-control …` (works locally and over `--remote`); `auth keys` shows each key's policy. + - **Rate limiting:** `--rate-limit N` caps commands/second per client key (token bucket, default `100`, `0` disables) so a compromised key can't hammer the browser. + - **Audit logging:** request logs include the acting key (its name from `authorized_keys` plus a short pubkey), not just the client address. +- **`serve-http`** is a convenience gateway with the inverse trade-off: commands are gated by the same `--allow-*` policy (safe-only by default), but the bearer token travels in **clear text over plain HTTP**. It binds to loopback by default; `--no-auth` is only permitted there. If you must expose it beyond loopback, put it behind a TLS-terminating reverse proxy — never send the token over an untrusted network unencrypted. + +For low latency, an authenticated encrypted remote connection is kept open and reused for further commands in the same process — so SDK scripts and multi-browser fan-out avoid repeating the TCP/TLS/challenge handshake on every command. Aggregate commands also fan out to remote targets concurrently. Both degrade gracefully against older servers that handle one command per connection. + --- ## Python SDK @@ -513,7 +542,7 @@ nix-shell # automatically runs npm ci when node_modules is missing/outdated npm run check:extension ``` -The extension source lives in `extension/src/`. `extension/background.js` and `extension/content-dispatch.js` are generated and ignored by git. Run `npm run build:extension` before using `Load unpacked` with `extension/`. On NixOS, use `nix-shell` first if npm is not installed globally. +The extension source lives in `extension/src/`. `extension/background.js` and `extension/content-dispatch.js` are generated and ignored by git. Run `npm run build:extension` before loading the unpacked `extension/` directory; `browser-cli install --dev` prints the per-browser load steps. On NixOS, use `nix-shell` first if npm is not installed globally. Packaging: diff --git a/browser_cli/auth/__init__.py b/browser_cli/auth/__init__.py index 1c17694..1306d4f 100644 --- a/browser_cli/auth/__init__.py +++ b/browser_cli/auth/__init__.py @@ -17,9 +17,11 @@ from browser_cli.auth.agent import ( ) from browser_cli.auth.keys import ( add_authorized_key, + format_authorized_line, generate_keypair, load_authorized_keys, load_authorized_keys_with_names, + load_authorized_keys_with_policies, load_private_key, public_key_hex, ) @@ -51,9 +53,11 @@ __all__ = [ "agent_list_keys", "agent_sign_raw", "canonical_payload", + "format_authorized_line", "generate_keypair", "load_authorized_keys", "load_authorized_keys_with_names", + "load_authorized_keys_with_policies", "load_private_key", "new_nonce", "pq_decrypt", diff --git a/browser_cli/auth/keys.py b/browser_cli/auth/keys.py index da1993a..416e4d6 100644 --- a/browser_cli/auth/keys.py +++ b/browser_cli/auth/keys.py @@ -29,31 +29,63 @@ def public_key_hex(key: Ed25519PrivateKey | AgentKey) -> str: return key.pubkey_bytes.hex() return key.public_key().public_bytes(Encoding.Raw, PublicFormat.Raw).hex() +def _parse_authorized_line(line: str) -> tuple[str, str, list[str] | None] | None: + """Parse one authorized_keys line into (pubkey, name, categories). + + Line format: `` [name words...] [allow:cat,cat,...]``. The optional + ``allow:`` token may appear anywhere after the pubkey (conventionally last); + the remaining words form the name. ``categories`` is None when no ``allow:`` + token is present (the key falls back to the server-wide policy), or a list of + category strings (possibly empty) otherwise. + """ + line = line.strip() + if not line or line.startswith("#"): + return None + tokens = line.split() + pubkey = tokens[0] + categories: list[str] | None = None + name_tokens: list[str] = [] + for tok in tokens[1:]: + if tok.startswith("allow:"): + categories = [c for c in tok[len("allow:"):].split(",") if c] + else: + name_tokens.append(tok) + return pubkey, " ".join(name_tokens), categories + +def format_authorized_line(pub_hex: str, name: str = "", categories: list[str] | None = None) -> str: + """Render an authorized_keys line. Inverse of :func:`_parse_authorized_line`.""" + parts = [pub_hex] + if name: + parts.append(name) + if categories is not None: + parts.append("allow:" + ",".join(categories)) + return " ".join(parts) + def load_authorized_keys_with_names(path: Path) -> list[tuple[str, str]]: """Return list of (pubkey_hex, name) pairs. Name is empty string if not set.""" + return [(pubkey, name) for pubkey, name, _cats in load_authorized_keys_with_policies(path)] + +def load_authorized_keys_with_policies(path: Path) -> list[tuple[str, str, list[str] | None]]: + """Return list of (pubkey_hex, name, categories) triples. categories is None when unset.""" if not path.exists(): return [] result = [] for line in path.read_text(encoding="utf-8").splitlines(): - line = line.strip() - if not line or line.startswith("#"): - continue - parts = line.split(None, 1) - pubkey = parts[0] - name = parts[1].strip() if len(parts) > 1 else "" - result.append((pubkey, name)) + parsed = _parse_authorized_line(line) + if parsed is not None: + result.append(parsed) return result def load_authorized_keys(path: Path) -> list[str]: return [pubkey for pubkey, _name in load_authorized_keys_with_names(path)] -def add_authorized_key(path: Path, pub_hex: str, name: str = "") -> bool: +def add_authorized_key(path: Path, pub_hex: str, name: str = "", categories: list[str] | None = None) -> bool: """Append pub_hex to authorized_keys. Returns False if already present.""" path.parent.mkdir(parents=True, exist_ok=True) existing = {pubkey for pubkey, _name in load_authorized_keys_with_names(path)} if pub_hex in existing: return False - line = (f"{pub_hex} {name}".rstrip()) + "\n" + line = format_authorized_line(pub_hex, name, categories) + "\n" with open(path, "a", encoding="utf-8") as file: file.write(line) return True diff --git a/browser_cli/cli.py b/browser_cli/cli.py index 9657083..9a4b7af 100755 --- a/browser_cli/cli.py +++ b/browser_cli/cli.py @@ -5,9 +5,6 @@ browser-cli — Control your running browser from the terminal. import click import os import shutil -import re -from importlib.metadata import PackageNotFoundError, version as package_version -from pathlib import Path from rich.console import Console from browser_cli.commands.navigate import nav_group @@ -35,7 +32,7 @@ from browser_cli.commands.serve_http import cmd_serve_http from browser_cli.commands.watch import watch_group from browser_cli.commands.workspace import workspace_group from browser_cli.commands.raw import cmd_command -from browser_cli.constants import PYPI_PACKAGE_NAME +from browser_cli.version_manager import project_version as _project_version console = Console() @@ -53,21 +50,6 @@ def _patched_group_shell_complete(self, ctx, incomplete): click.Group.shell_complete = _patched_group_shell_complete -def _project_version() -> str: - pyproject_path = Path(__file__).resolve().parent.parent / "pyproject.toml" - try: - content = pyproject_path.read_text(encoding="utf-8") - match = re.search(r'^version\s*=\s*"([^"]+)"', content, re.MULTILINE) - if match: - return match.group(1) - except OSError: - pass - - try: - return package_version(PYPI_PACKAGE_NAME) - except PackageNotFoundError: - return "unknown" - def _print_version(ctx, param, value): if not value or ctx.resilient_parsing: return diff --git a/browser_cli/client/__init__.py b/browser_cli/client/__init__.py index a724821..9bd1f1d 100644 --- a/browser_cli/client/__init__.py +++ b/browser_cli/client/__init__.py @@ -7,6 +7,7 @@ from browser_cli.client.core import ( _send_remote, _send_remote_async, active_browser_targets, + collect_browser_clients, remote_browser_targets, remote_browser_targets_async, remote_target_for_alias, @@ -39,6 +40,7 @@ __all__ = [ "_send_remote", "_send_remote_async", "active_browser_targets", + "collect_browser_clients", "display_browser_name", "remote_browser_targets", "remote_browser_targets_async", diff --git a/browser_cli/client/core.py b/browser_cli/client/core.py index ba6e7ef..521aba1 100644 --- a/browser_cli/client/core.py +++ b/browser_cli/client/core.py @@ -15,11 +15,39 @@ from browser_cli import local_transport from browser_cli.client import auth, messages, targets as target_discovery from browser_cli.client.targets import BrowserTarget from browser_cli.remote import registry as remote_registry - from browser_cli.errors import BrowserNotConnected -from browser_cli.endpoints import _remote_display_name +from browser_cli.endpoints import _remote_display_name, display_browser_name +from browser_cli.registry import load_registry from browser_cli.remote.transport import _send_remote, _send_remote_async +def _run_concurrent(factories: list) -> list: + """Run async thunks concurrently, returning results in order. + + Each item in *factories* is a zero-arg callable returning a coroutine. The + return list mirrors the input order; a thunk that raises yields its exception + object in that slot (callers filter as they would in a sequential loop). Falls + back to sequential execution if an event loop is already running on this + thread (e.g. inside the async serve handler), where ``asyncio.run`` is illegal. + """ + if not factories: + return [] + + async def _gather(): + return await asyncio.gather(*(factory() for factory in factories), return_exceptions=True) + + try: + asyncio.get_running_loop() + except RuntimeError: + return asyncio.run(_gather()) + + # An event loop is already running on this thread (e.g. the async serve + # handler), where asyncio.run is illegal. Run the gather on a worker thread + # that has no loop of its own, preserving concurrency and result order. + import concurrent.futures + + with concurrent.futures.ThreadPoolExecutor(max_workers=1) as executor: + return executor.submit(lambda: asyncio.run(_gather())).result() + def _remote_target_items(endpoint: str, items: list[dict] | None) -> list[BrowserTarget]: targets: list[BrowserTarget] = [] for item in items or []: @@ -27,6 +55,8 @@ def _remote_target_items(endpoint: str, items: list[dict] | None) -> list[Browse display = str(item.get("displayName") or profile) display_name = _remote_display_name(endpoint, profile, display) browser_name = item.get("browserName") or item.get("name") + version = item.get("version") + extension_version = item.get("extensionVersion") targets.append( BrowserTarget( profile=profile, @@ -35,6 +65,8 @@ def _remote_target_items(endpoint: str, items: list[dict] | None) -> list[Browse remote=endpoint, browser_name=str(browser_name) if browser_name else None, display_group=display_name.rsplit(":", 1)[0], + version=str(version) if version else None, + extension_version=str(extension_version) if extension_version else None, ) ) return targets @@ -48,12 +80,20 @@ def remote_browser_targets(endpoint: str, key=None, *, suppress_pq_warning: bool ) def _remote_browser_targets(key=None, *, suppress_pq_warning: bool = False) -> list[BrowserTarget]: + endpoints = list(remote_registry.load_remotes()) + if not endpoints: + return [] + results = _run_concurrent([ + (lambda ep=ep: asyncio.to_thread(remote_browser_targets, ep, key=key, suppress_pq_warning=suppress_pq_warning)) + for ep in endpoints + ]) targets: list[BrowserTarget] = [] - for endpoint in remote_registry.load_remotes(): - try: - targets.extend(remote_browser_targets(endpoint, key=key, suppress_pq_warning=suppress_pq_warning)) - except (BrowserNotConnected, RuntimeError): + for result in results: + if isinstance(result, (BrowserNotConnected, RuntimeError)): continue + if isinstance(result, BaseException): + raise result + targets.extend(result) return targets def remote_targets_for_alias(alias: str | None, key=None) -> list[BrowserTarget]: @@ -111,6 +151,160 @@ def active_browser_targets(*, include_remotes: bool = True, key=None, suppress_p targets.extend(_remote_browser_targets(key=key, suppress_pq_warning=suppress_pq_warning)) return targets +def _cached_client_row(target: BrowserTarget) -> dict | None: + """Build a clients row from a target's discovery data, skipping a roundtrip. + + Returns None when the remote didn't advertise its version (older serve), so + callers fall back to an explicit ``clients.list`` query. + """ + if target.version is None and target.extension_version is None: + return None + return { + "profile": target.display_name, + "profileGroup": target.display_group, + "name": target.browser_name or "", + "version": target.version or "", + "extensionVersion": target.extension_version or "", + } + +def _rows_from_result(result, label: str, profile_group: str | None) -> list[dict]: + rows = [] + for item in result or []: + row = dict(item) + row["profile"] = label + if profile_group: + row["profileGroup"] = profile_group + rows.append(row) + return rows + +async def _client_rows_async( + label: str, + *, + profile: str | None = None, + remote: str | None = None, + key=None, + suppress_pq_warning: bool = False, + profile_group: str | None = None, +) -> list[dict]: + """Return display-ready clients.list rows for one browser target.""" + kwargs = {"suppress_pq_warning": True} if suppress_pq_warning else {} + result = await asyncio.to_thread( + send_command, + "clients.list", + profile=profile, + remote=remote, + key=key, + **kwargs, + ) + return _rows_from_result(result, label, profile_group) + +def collect_browser_clients( + *, + browser_alias: str | None = None, + remote: str | None = None, + key=None, + registry_path=None, +) -> list[dict]: + """Return display-ready browser client rows for CLI/SDK consumers. + + Rows preserve the CLI-facing shape: ``profile``, optional ``profileGroup``, + ``name``, ``version``, and ``extensionVersion``. + """ + rows: list[dict] = [] + + if not remote and browser_alias: + resolved = remote_target_for_alias(browser_alias) + if not resolved: + return rows + targets = remote_browser_targets(resolved.remote) + uncached = [] + for target in targets: + cached = _cached_client_row(target) + if cached is not None: + rows.append(cached) + else: + uncached.append(target) + results = _run_concurrent([ + (lambda t=t: _client_rows_async( + t.display_name, + profile=t.profile, + remote=resolved.remote, + key=key, + profile_group=t.display_group, + )) + for t in uncached + ]) + for result in results: + if isinstance(result, (BrowserNotConnected, RuntimeError)): + continue + if isinstance(result, BaseException): + raise result + rows.extend(result) + return rows + + if remote: + result = send_command("clients.list", profile=browser_alias, remote=remote, key=key) + for item in result or []: + row = dict(item) + row["profile"] = row.get("profile") or browser_alias or "remote" + rows.append(row) + return rows + + path = registry_path or target_discovery.REGISTRY_PATH + profiles: dict[str, str] = load_registry(path) if path.exists() else {} + local_items = list(profiles.items()) + + remote_targets = [] + cached_remote_rows = [] # deferred so local profiles still render first + for target in active_browser_targets(suppress_pq_warning=True): + if target.remote is None: + continue + cached = _cached_client_row(target) + if cached is not None: + cached_remote_rows.append(cached) # discovery already carried version/extVersion — no extra roundtrip + else: + remote_targets.append(target) + + factories = [ + (lambda name=name, sock=sock: _client_rows_async( + display_browser_name(name, sock), profile=name, profile_group="local", + )) + for name, sock in local_items + ] + [ + (lambda t=t: _client_rows_async( + t.display_name, + profile=t.profile, + remote=t.remote, + suppress_pq_warning=True, + profile_group=t.display_group, + )) + for t in remote_targets + ] + results = _run_concurrent(factories) + + for (name, sock), result in zip(local_items, results[:len(local_items)]): + if isinstance(result, (BrowserNotConnected, RuntimeError)): + rows.append({ + "profile": display_browser_name(name, sock), + "profileGroup": "local", + "name": "—", + "version": "—", + "extensionVersion": "disconnected", + }) + elif isinstance(result, BaseException): + raise result + else: + rows.extend(result) + + for result in results[len(local_items):]: + if isinstance(result, (BrowserNotConnected, RuntimeError)): + continue + if isinstance(result, BaseException): + raise result + rows.extend(result) + rows.extend(cached_remote_rows) + return rows + def _auto_route_remote(endpoint: str, key=None) -> str | None: targets = remote_browser_targets(endpoint, key=key) if len(targets) == 1: @@ -159,11 +353,12 @@ def send_command( return messages.decode_response(response) -async def remote_browser_targets_async(endpoint: str, key=None) -> list[BrowserTarget]: +async def remote_browser_targets_async(endpoint: str, key=None, *, suppress_pq_warning: bool = False) -> list[BrowserTarget]: """Async variant of :func:`remote_browser_targets`.""" + kwargs = {"suppress_pq_warning": True} if suppress_pq_warning else {} return _remote_target_items( endpoint, - await send_command_async("browser-cli.targets", remote=endpoint, key=key), + await send_command_async("browser-cli.targets", remote=endpoint, key=key, **kwargs), ) async def _auto_route_remote_async(endpoint: str, key=None) -> str | None: diff --git a/browser_cli/client/targets.py b/browser_cli/client/targets.py index adecd00..a4658dc 100644 --- a/browser_cli/client/targets.py +++ b/browser_cli/client/targets.py @@ -19,6 +19,11 @@ class BrowserTarget: remote: str | None = None browser_name: str | None = None display_group: str | None = None + # Populated from a remote ``browser-cli.targets`` response when the remote is + # new enough to advertise them, letting ``clients`` skip a redundant + # ``clients.list`` roundtrip. None means "unknown — fall back to a query". + version: str | None = None + extension_version: str | None = None def is_reachable_unix_endpoint(endpoint: str) -> bool: """Return True when a Unix socket path exists and accepts connections.""" diff --git a/browser_cli/command_security.py b/browser_cli/command_security.py index 22ea8b5..567e165 100644 --- a/browser_cli/command_security.py +++ b/browser_cli/command_security.py @@ -10,6 +10,7 @@ from __future__ import annotations from dataclasses import dataclass SAFE_COMMANDS = { + "browser-cli.targets", "clients.list", "extension.capabilities", "extension.info", @@ -74,15 +75,23 @@ DANGEROUS_PREFIXES = ( "storage.", ) +# Server-side key-management control commands. Gated separately so a key can be +# trusted for browser use without also being able to list or add trusted keys. +KEY_COMMANDS = { + "browser-cli.auth.keys", + "browser-cli.auth.trust", +} + @dataclass(frozen=True) class CommandPolicy: allow_read_page: bool = False allow_control: bool = False allow_dangerous: bool = False + allow_keys: bool = False @classmethod def unrestricted(cls) -> "CommandPolicy": - return cls(allow_read_page=True, allow_control=True, allow_dangerous=True) + return cls(allow_read_page=True, allow_control=True, allow_dangerous=True, allow_keys=True) def _is_control(command: str) -> bool: if command in CONTROL_COMMANDS: @@ -93,6 +102,8 @@ def _is_control(command: str) -> bool: def command_category(command: str) -> str: name = str(command or "") + if name in KEY_COMMANDS: + return "keys" if name in DANGEROUS_COMMANDS or any(name.startswith(prefix) for prefix in DANGEROUS_PREFIXES): return "dangerous" if name in READ_PAGE_COMMANDS: @@ -113,7 +124,9 @@ def assert_command_allowed(command: str, policy: CommandPolicy) -> None: return if category == "dangerous" and policy.allow_dangerous: return + if category == "keys" and policy.allow_keys: + return raise PermissionError( f"Raw command '{command}' is {category} and blocked by default; " - "use --allow-read-page, --allow-control, or --allow-dangerous explicitly" + "use --allow-read-page, --allow-control, --allow-dangerous, or --allow-keys explicitly" ) diff --git a/browser_cli/commands/__init__.py b/browser_cli/commands/__init__.py index a409d34..2c9056e 100644 --- a/browser_cli/commands/__init__.py +++ b/browser_cli/commands/__init__.py @@ -31,6 +31,66 @@ def gentle_mode_option(help_text: str): help=help_text, ) +def command_policy_options(fn): + """Reusable raw-command safety flags for /command-like entry points.""" + fn = click.option( + "--allow-all", + is_flag=True, + help="Allow every command (equivalent to --allow-read-page --allow-control --allow-dangerous --allow-keys)", + )(fn) + fn = click.option( + "--allow-keys", + is_flag=True, + help="Allow key-management commands (list/trust authorized keys over --remote)", + )(fn) + fn = click.option( + "--allow-dangerous", + is_flag=True, + help="Allow high-risk commands such as dom.eval, storage.*, screenshots", + )(fn) + fn = click.option( + "--allow-control", + is_flag=True, + help="Allow browser-control commands such as nav.*, tabs.close, dom.click", + )(fn) + fn = click.option( + "--allow-read-page", + is_flag=True, + help="Allow page-content read commands such as extract.* and dom.text", + )(fn) + return fn + +def command_policy_from_options(*, allow_read_page: bool, allow_control: bool, allow_dangerous: bool, allow_keys: bool = False, allow_all: bool = False): + """Build a CommandPolicy from shared raw-command safety flags.""" + from browser_cli.command_security import CommandPolicy + + if allow_all: + return CommandPolicy.unrestricted() + return CommandPolicy( + allow_read_page=allow_read_page, + allow_control=allow_control, + allow_dangerous=allow_dangerous, + allow_keys=allow_keys, + ) + +def command_categories_from_options(*, allow_read_page: bool, allow_control: bool, allow_dangerous: bool, allow_keys: bool = False, allow_all: bool = False): + """Convert the shared --allow-* flags into a category list, or None if none were set. + + None means "no explicit policy" — the key falls back to the server-wide default. + """ + if allow_all: + return ["all"] + cats = [] + if allow_read_page: + cats.append("read-page") + if allow_control: + cats.append("control") + if allow_dangerous: + cats.append("dangerous") + if allow_keys: + cats.append("keys") + return cats or None + def print_counts(result, noun: str, *, single_suffix: str = "") -> None: """Render a count result. diff --git a/browser_cli/commands/auth.py b/browser_cli/commands/auth.py index 438cb0a..ada3932 100644 --- a/browser_cli/commands/auth.py +++ b/browser_cli/commands/auth.py @@ -8,6 +8,8 @@ from pathlib import Path import click from rich.console import Console +from browser_cli.commands import command_categories_from_options, command_policy_options, handle_errors + console = Console() @click.group("auth") @@ -39,9 +41,15 @@ def cmd_auth_keygen(output, force): @click.argument("pubkey") @click.option("--name", default="", metavar="NAME", help="Human-friendly label for this key.") @click.option("--file", "keys_file", default=None, metavar="PATH", help="Authorized keys file (default: ~/.config/browser-cli/authorized_keys).") +@command_policy_options @click.pass_context -def cmd_auth_trust(ctx, pubkey, name, keys_file): - """Add a public key to the authorized keys file (locally or on a remote serve host).""" +@handle_errors +def cmd_auth_trust(ctx, pubkey, name, keys_file, allow_read_page, allow_control, allow_dangerous, allow_keys, allow_all): + """Add a public key to the authorized keys file (locally or on a remote serve host). + + Pass --allow-read-page/--allow-control/--allow-dangerous/--allow-all to record a + per-key policy (an ``allow:`` token); without any, the key uses the server default. + """ from browser_cli.auth import DEFAULT_AUTHORIZED_KEYS_PATH, add_authorized_key if len(pubkey) != 64: @@ -53,28 +61,37 @@ def cmd_auth_trust(ctx, pubkey, name, keys_file): console.print("[red]Invalid public key:[/red] not valid hex") sys.exit(1) + categories = command_categories_from_options( + allow_read_page=allow_read_page, allow_control=allow_control, + allow_dangerous=allow_dangerous, allow_keys=allow_keys, allow_all=allow_all, + ) + policy_label = f" [dim]allow:{','.join(categories)}[/dim]" if categories else "" + remote = (ctx.obj or {}).get("remote") if remote: from browser_cli.client import send_command + args = {"pubkey": pubkey, "name": name} + if categories is not None: + args["allow"] = categories result = send_command( "browser-cli.auth.trust", - args={"pubkey": pubkey, "name": name}, + args=args, remote=remote, key=(ctx.obj or {}).get("key"), ) added = (result or {}).get("added", False) label = f" ({name})" if name else "" if added: - console.print(f"[green]✓[/green] Trusted on {remote}{label}: [cyan]{pubkey}[/cyan]") + console.print(f"[green]✓[/green] Trusted on {remote}{label}: [cyan]{pubkey}[/cyan]{policy_label}") else: console.print(f"[yellow]Already trusted on {remote}:[/yellow] {pubkey}") return path = Path(keys_file) if keys_file else DEFAULT_AUTHORIZED_KEYS_PATH - added = add_authorized_key(path, pubkey, name) + added = add_authorized_key(path, pubkey, name, categories) label = f" ({name})" if name else "" if added: - console.print(f"[green]✓[/green] Trusted{label}: [cyan]{pubkey}[/cyan]") + console.print(f"[green]✓[/green] Trusted{label}: [cyan]{pubkey}[/cyan]{policy_label}") console.print(f" File: {path}") console.print("\nStart the server with:") console.print(f" [dim]browser-cli serve --authorized-keys {path}[/dim]") @@ -123,6 +140,7 @@ def cmd_auth_show(key_src): @auth_group.command("keys") @click.option("--file", "keys_file", default=None, metavar="PATH", help="Authorized keys file (default: ~/.config/browser-cli/authorized_keys).") @click.pass_context +@handle_errors def cmd_auth_keys(ctx, keys_file): """List trusted public keys (server's authorized_keys). With --remote, queries the remote server.""" from rich.table import Table @@ -138,9 +156,9 @@ def cmd_auth_keys(ctx, keys_file): entries = result or [] source_label = remote else: - from browser_cli.auth import DEFAULT_AUTHORIZED_KEYS_PATH, load_authorized_keys_with_names + from browser_cli.auth import DEFAULT_AUTHORIZED_KEYS_PATH, load_authorized_keys_with_policies path = Path(keys_file) if keys_file else DEFAULT_AUTHORIZED_KEYS_PATH - entries = [{"pubkey": pk, "name": name} for pk, name in load_authorized_keys_with_names(path)] + entries = [{"pubkey": pk, "name": name, "allow": cats} for pk, name, cats in load_authorized_keys_with_policies(path)] source_label = str(path) if not entries: @@ -151,7 +169,16 @@ def cmd_auth_keys(ctx, keys_file): table = Table(show_header=True, header_style="bold cyan") table.add_column("Name") table.add_column("Public Key") + table.add_column("Policy") for entry in entries: name = entry.get("name") or "[dim]—[/dim]" - table.add_row(name, entry.get("pubkey", "")) + table.add_row(name, entry.get("pubkey", ""), _policy_label(entry.get("allow"))) console.print(table) + +def _policy_label(categories) -> str: + """Render an authorized_keys ``allow:`` token for display.""" + if categories is None: + return "[dim]server default[/dim]" + if "all" in categories: + return "[yellow]all[/yellow]" + return ", ".join(categories) if categories else "safe" diff --git a/browser_cli/commands/clients.py b/browser_cli/commands/clients.py index 9c8f36b..429cd0e 100644 --- a/browser_cli/commands/clients.py +++ b/browser_cli/commands/clients.py @@ -11,11 +11,10 @@ from browser_cli.client import ( BrowserNotConnected, REGISTRY_PATH, active_browser_targets, - display_browser_name, - remote_browser_targets, - remote_target_for_alias, + collect_browser_clients, send_command, ) +from browser_cli.commands.rendering import print_browser_grouped_table_rows from browser_cli.registry import load_registry console = Console() @@ -36,23 +35,6 @@ def _ensure_unique_browser_alias(alias: str, target_browser: str | None) -> None if alias in profiles and alias != target_profile: raise click.ClickException(f"Browser alias '{alias}' already exists") -def _append_clients(into, label, *, profile=None, remote=None, key=None, quiet_remote_warning=False, profile_group=None): - """Query clients.list for one target and append each, tagged with *label*.""" - if quiet_remote_warning: - result = send_command( - "clients.list", - profile=profile, - remote=remote, - key=key, - suppress_pq_warning=True, - ) - else: - result = send_command("clients.list", profile=profile, remote=remote, key=key) - for c in (result or []): - c["profile"] = label - if profile_group: - c["profileGroup"] = profile_group - into.append(c) @click.group("clients", invoke_without_command=True) @click.pass_context @@ -61,18 +43,20 @@ def clients_group(ctx): if ctx.invoked_subcommand is not None: return - all_clients = [] - browser_alias = (ctx.obj or {}).get("browser") remote = (ctx.obj or {}).get("remote") or os.environ.get("BROWSER_CLI_REMOTE") key = (ctx.obj or {}).get("key") - if not remote and browser_alias: - _collect_remote_alias_clients(all_clients, browser_alias, key) - elif remote: - _collect_explicit_remote_clients(all_clients, browser_alias, remote, key) - else: - _collect_local_and_saved_remote_clients(all_clients) + try: + all_clients = collect_browser_clients( + browser_alias=browser_alias, + remote=remote, + key=key, + registry_path=REGISTRY_PATH, + ) + except (BrowserNotConnected, RuntimeError) as e: + console.print(f"[red]Error:[/red] {e}") + sys.exit(1) if not all_clients: console.print("[yellow]No browser clients found. Start a browser with the extension enabled first.[/yellow]") @@ -80,98 +64,24 @@ def clients_group(ctx): _print_clients(all_clients) -def _collect_remote_alias_clients(all_clients: list, browser_alias: str, key) -> None: - resolved = remote_target_for_alias(browser_alias) - if not resolved: - return - try: - targets = remote_browser_targets(resolved.remote) - except (BrowserNotConnected, RuntimeError) as e: - console.print(f"[red]Error:[/red] {e}") - sys.exit(1) - for target in targets: - try: - _append_clients( - all_clients, - target.display_name, - profile=target.profile, - remote=resolved.remote, - key=key, - profile_group=target.display_group, - ) - except (BrowserNotConnected, RuntimeError): - continue - -def _collect_explicit_remote_clients(all_clients: list, browser_alias: str | None, remote: str, key) -> None: - try: - result = send_command("clients.list", profile=browser_alias, remote=remote, key=key) - for c in (result or []): - c["profile"] = c.get("profile") or browser_alias or "remote" - all_clients.append(c) - except (BrowserNotConnected, RuntimeError) as e: - console.print(f"[red]Error:[/red] {e}") - sys.exit(1) - -def _collect_local_and_saved_remote_clients(all_clients: list) -> None: - profiles: dict[str, str] = load_registry(REGISTRY_PATH) if REGISTRY_PATH.exists() else {} - - for profile_name, sock_path in profiles.items(): - display_profile = display_browser_name(profile_name, sock_path) - try: - _append_clients(all_clients, display_profile, profile=profile_name, profile_group="local") - except (BrowserNotConnected, RuntimeError): - all_clients.append({ - "profile": display_profile, - "profileGroup": "local", - "name": "—", - "version": "—", - "extensionVersion": "disconnected", - }) - - targets = active_browser_targets(suppress_pq_warning=True) - - for target in targets: - if target.remote is None: - continue - try: - _append_clients( - all_clients, - target.display_name, - profile=target.profile, - remote=target.remote, - quiet_remote_warning=True, - profile_group=target.display_group, - ) - except (BrowserNotConnected, RuntimeError): - continue def _print_clients(all_clients: list) -> None: - from rich.table import Table - table = Table(show_header=True, header_style="bold cyan") - table.add_column("Profile", no_wrap=True) - table.add_column("Browser") - table.add_column("Version") - table.add_column("Extension Version") - rendered_groups: set[str] = set() groups = {c.get("profileGroup") for c in all_clients if c.get("profileGroup")} grouped = bool(groups and groups != {"local"}) - for c in all_clients: - group = c.get("profileGroup") if grouped else None - if group: - if group not in rendered_groups: - table.add_row(f"[bold]{group}[/bold]", "", "", "") - rendered_groups.add(group) - profile = str(c.get("profile", "")).removeprefix(f"{group}:") - profile = f" {profile}" - else: - profile = c.get("profile", "") - table.add_row( - profile, - c.get("name", ""), - c.get("version", ""), - c.get("extensionVersion", ""), - ) - console.print(table) + columns = [ + ("Browser", lambda item: item.get("name", "")), + ("Version", lambda item: item.get("version", "")), + ("Extension Version", lambda item: item.get("extensionVersion", "")), + ] + print_browser_grouped_table_rows( + all_clients, + columns, + console=console, + empty_message="[yellow]No browser clients found. Start a browser with the extension enabled first.[/yellow]", + browser_getter=lambda item: item.get("profile", ""), + group_getter=lambda item: item.get("profileGroup", "") if grouped else "", + browser_header="Profile", + ) @clients_group.command("rename") @click.option( diff --git a/browser_cli/commands/doctor.py b/browser_cli/commands/doctor.py index edd3441..f9e6054 100644 --- a/browser_cli/commands/doctor.py +++ b/browser_cli/commands/doctor.py @@ -1,35 +1,18 @@ from __future__ import annotations -import re import shutil -from importlib.metadata import PackageNotFoundError, version as package_version -from pathlib import Path - import click from rich.console import Console from rich.table import Table from browser_cli.commands import handle_errors, client_from_ctx from browser_cli.client import active_browser_targets -from browser_cli.constants import NATIVE_HOST_DIRS, NATIVE_HOST_NAME, PYPI_PACKAGE_NAME +from browser_cli.constants import NATIVE_HOST_DIRS, NATIVE_HOST_NAME from browser_cli.platform import is_windows +from browser_cli.version_manager import project_version console = Console() -def _project_version() -> str: - pyproject_path = Path(__file__).resolve().parents[2] / "pyproject.toml" - try: - content = pyproject_path.read_text(encoding="utf-8") - match = re.search(r'^version\s*=\s*"([^"]+)"', content, re.MULTILINE) - if match: - return match.group(1) - except OSError: - pass - try: - return package_version(PYPI_PACKAGE_NAME) - except PackageNotFoundError: - return "unknown" - def _status(ok: bool) -> str: return "[green]OK[/green]" if ok else "[red]FAIL[/red]" @@ -39,7 +22,7 @@ def _status(ok: bool) -> str: def cmd_doctor(check_remote): """Diagnose browser-cli installation, extension, and connection health.""" rows: list[tuple[str, bool, str]] = [] - version = _project_version() + version = project_version() rows.append(("Python package", version != "unknown", version)) rows.append(("browser-cli executable", shutil.which("browser-cli") is not None, shutil.which("browser-cli") or "not on PATH")) diff --git a/browser_cli/commands/install.py b/browser_cli/commands/install.py index 1d86779..7c3a471 100644 --- a/browser_cli/commands/install.py +++ b/browser_cli/commands/install.py @@ -10,7 +10,9 @@ from rich.console import Console from browser_cli.constants import ( ALLOWED_EXTENSION_IDS, + CHROME_WEBSTORE_URL, EXTENSION_ID, + FIREFOX_ADDON_URL, FIREFOX_EXTENSION_ID, NATIVE_HOST_DIRS, NATIVE_HOST_NAME, @@ -62,11 +64,44 @@ def _register_windows_native_host(browser: str, manifest_path: Path) -> list[str @click.command("install") @click.argument("browser", type=click.Choice(SUPPORTED_BROWSERS), default="chrome") -def cmd_install(browser): - """Register the native messaging host and print extension load instructions.""" +@click.option("--dev", is_flag=True, help="Print developer instructions for loading an unpacked/temporary build instead of the public store listing.") +def cmd_install(browser, dev): + """Register the native messaging host and print extension install instructions.""" host_exe = native_host_exe() write_native_host_exe(host_exe) + if dev: + _print_dev_instructions(browser) + else: + _print_store_instructions(browser) + + manifest = _native_host_manifest(browser, host_exe) + installed = _install_manifest(browser, host_exe, manifest) + if not installed: + console.print("[red]Failed to install native host manifest[/red]") + sys.exit(1) + + for p in installed: + label = "Registered native host" if is_windows() else "Wrote native host manifest" + console.print(f"[green]✓[/green] {label}: {p}") + console.print(f"[green]✓[/green] Installed native host: {host_exe}") + console.print(f"\n[bold]Step 2:[/bold] Restart {browser.capitalize()} completely (quit app, then reopen)") + console.print("\n[green bold]✓ Installation complete![/green bold]") + console.print(" After restarting the browser, try: [cyan]browser-cli tabs list[/cyan]") + +def _print_store_instructions(browser: str) -> None: + console.print("\n[bold]Step 1:[/bold] Install the extension") + if browser == "firefox": + console.print(" Open Firefox Add-ons and click [bold]Add to Firefox[/bold]:") + console.print(f" [cyan]{FIREFOX_ADDON_URL}[/cyan]") + console.print(" [dim]Firefox support is experimental; tab-group commands require browser tab group APIs.[/dim]\n") + else: + console.print(f" Open the Chrome Web Store and click [bold]Add to {browser.capitalize()}[/bold]:") + console.print(f" [cyan]{CHROME_WEBSTORE_URL}[/cyan]") + console.print(" [dim]Brave, Edge, Vivaldi and Chromium can install from the Chrome Web Store too.[/dim]") + console.print(" [dim]Developing the extension? Run 'browser-cli install --dev' for the unpacked-load steps.[/dim]\n") + +def _print_dev_instructions(browser: str) -> None: ext_url = { "chrome": "chrome://extensions", "chromium": "chrome://extensions", @@ -75,7 +110,7 @@ def cmd_install(browser): "vivaldi": "vivaldi://extensions", "firefox": "about:debugging#/runtime/this-firefox", }[browser] - console.print("\n[bold]Step 1:[/bold] Load the extension in your browser") + console.print("\n[bold]Step 1:[/bold] Load the unpacked extension (developer mode)") console.print(f" 1. Open [cyan]{ext_url}[/cyan]") if browser == "firefox": repo_root = Path(__file__).parent.parent.parent @@ -93,20 +128,6 @@ def cmd_install(browser): console.print(f" 4. Testing extension ID will be [cyan]{EXTENSION_ID}[/cyan] (fixed by built-in key)") console.print(f" Chrome Web Store extension ID is [cyan]{WEBSTORE_EXTENSION_ID}[/cyan]\n") - manifest = _native_host_manifest(browser, host_exe) - installed = _install_manifest(browser, host_exe, manifest) - if not installed: - console.print("[red]Failed to install native host manifest[/red]") - sys.exit(1) - - for p in installed: - label = "Registered native host" if is_windows() else "Wrote native host manifest" - console.print(f"[green]✓[/green] {label}: {p}") - console.print(f"[green]✓[/green] Installed native host: {host_exe}") - console.print(f"\n[bold]Step 2:[/bold] Restart {browser.capitalize()} completely (quit app, then reopen)") - console.print("\n[green bold]✓ Installation complete![/green bold]") - console.print(" After restarting the browser, try: [cyan]browser-cli tabs list[/cyan]") - def _native_host_manifest(browser: str, host_exe: Path) -> dict: base = { "name": NATIVE_HOST_NAME, diff --git a/browser_cli/commands/raw.py b/browser_cli/commands/raw.py index 9573f24..6a87c01 100644 --- a/browser_cli/commands/raw.py +++ b/browser_cli/commands/raw.py @@ -4,20 +4,18 @@ import json import click -from browser_cli.command_security import CommandPolicy, assert_command_allowed -from browser_cli.commands import client_from_ctx, handle_errors +from browser_cli.command_security import assert_command_allowed +from browser_cli.commands import command_policy_from_options, command_policy_options, client_from_ctx, handle_errors @click.command("command") @click.argument("name") @click.argument("args_json", required=False, default="{}") -@click.option("--allow-read-page", is_flag=True, help="Allow page-content read commands such as extract.* and dom.text") -@click.option("--allow-control", is_flag=True, help="Allow browser-control commands such as nav.*, tabs.close, dom.click") -@click.option("--allow-dangerous", is_flag=True, help="Allow high-risk commands such as dom.eval, storage.*, screenshots") +@command_policy_options @handle_errors -def cmd_command(name, args_json, allow_read_page, allow_control, allow_dangerous): - """Send a raw browser-cli wire command and print JSON.""" - policy = CommandPolicy(allow_read_page=allow_read_page, allow_control=allow_control, allow_dangerous=allow_dangerous) - assert_command_allowed(name, policy) - args = json.loads(args_json) if args_json else {} - result = client_from_ctx().command(name, args) - click.echo(json.dumps(result, indent=2, default=str)) +def cmd_command(name, args_json, allow_read_page, allow_control, allow_dangerous, allow_keys, allow_all): + """Send a raw browser-cli wire command and print JSON.""" + policy = command_policy_from_options(allow_read_page=allow_read_page, allow_control=allow_control, allow_dangerous=allow_dangerous, allow_keys=allow_keys, allow_all=allow_all) + assert_command_allowed(name, policy) + args = json.loads(args_json) if args_json else {} + result = client_from_ctx().command(name, args) + click.echo(json.dumps(result, indent=2, default=str)) diff --git a/browser_cli/commands/script.py b/browser_cli/commands/script.py index ae88387..5f0e4bb 100644 --- a/browser_cli/commands/script.py +++ b/browser_cli/commands/script.py @@ -8,8 +8,8 @@ from typing import Any, cast import click from rich.console import Console -from browser_cli.command_security import CommandPolicy, assert_command_allowed -from browser_cli.commands import client_from_ctx, handle_errors +from browser_cli.command_security import assert_command_allowed +from browser_cli.commands import command_policy_from_options, command_policy_options, client_from_ctx, handle_errors console = Console() @@ -38,17 +38,15 @@ def _parse_step(step): @click.argument("file", type=click.Path(exists=True, dir_okay=False, path_type=Path)) @click.option("--json", "json_output", is_flag=True, help="Print all step results as JSON") @click.option("--continue-on-error", is_flag=True, help="Continue after failed steps") -@click.option("--allow-read-page", is_flag=True, help="Allow page-content read commands such as extract.* and dom.text") -@click.option("--allow-control", is_flag=True, help="Allow browser-control commands such as nav.*, tabs.close, dom.click") -@click.option("--allow-dangerous", is_flag=True, help="Allow high-risk commands such as dom.eval, storage.*, screenshots") +@command_policy_options @handle_errors -def cmd_script(file: Path, json_output: bool, continue_on_error: bool, allow_read_page: bool, allow_control: bool, allow_dangerous: bool): +def cmd_script(file: Path, json_output: bool, continue_on_error: bool, allow_read_page: bool, allow_control: bool, allow_dangerous: bool, allow_keys: bool, allow_all: bool): """Run a JSON/YAML batch script of browser-cli wire commands.""" steps = _load_steps(file) if not isinstance(steps, list): raise click.ClickException("Script root must be a list") client = client_from_ctx() - policy = CommandPolicy(allow_read_page=allow_read_page, allow_control=allow_control, allow_dangerous=allow_dangerous) + policy = command_policy_from_options(allow_read_page=allow_read_page, allow_control=allow_control, allow_dangerous=allow_dangerous, allow_keys=allow_keys, allow_all=allow_all) results = [] for index, step in enumerate(steps, start=1): command, args = _parse_step(step) diff --git a/browser_cli/commands/search.py b/browser_cli/commands/search.py index beedb2c..862c384 100644 --- a/browser_cli/commands/search.py +++ b/browser_cli/commands/search.py @@ -1,61 +1,10 @@ import click from browser_cli.commands import client_from_ctx, handle_errors from rich.console import Console +from browser_cli.search.engines import DISPLAY_NAMES, SUBCOMMANDS console = Console() -ENGINES = { - "google": "https://www.google.com/search?q={query}", - "brave": "https://search.brave.com/search?q={query}", - "duckduckgo": "https://duckduckgo.com/?q={query}", - "ddg": "https://duckduckgo.com/?q={query}", - "youtube": "https://www.youtube.com/results?search_query={query}", - "yt": "https://www.youtube.com/results?search_query={query}", - "spotify": "https://open.spotify.com/search/{query}", - "amazon": "https://www.amazon.com/s?k={query}", - "ecosia": "https://www.ecosia.org/search?q={query}", - "furaffinity": "https://www.furaffinity.net/search/?q={query}", - "fa": "https://www.furaffinity.net/search/?q={query}", - "bing": "https://www.bing.com/search?q={query}", - "github": "https://github.com/search?q={query}", - "wikipedia": "https://en.wikipedia.org/wiki/Special:Search?search={query}", - "wiki": "https://en.wikipedia.org/wiki/Special:Search?search={query}", - "reddit": "https://www.reddit.com/search/?q={query}", - "stackoverflow": "https://stackoverflow.com/search?q={query}", - "so": "https://stackoverflow.com/search?q={query}", -} - -_DISPLAY_NAMES = { - "google": "Google", "brave": "Brave Search", "duckduckgo": "DuckDuckGo", - "ddg": "DuckDuckGo", "youtube": "YouTube", "yt": "YouTube", - "spotify": "Spotify", "amazon": "Amazon", "ecosia": "Ecosia", - "furaffinity": "FurAffinity", "fa": "FurAffinity", "bing": "Bing", - "github": "GitHub", "wikipedia": "Wikipedia", "wiki": "Wikipedia", - "reddit": "Reddit", "stackoverflow": "Stack Overflow", "so": "Stack Overflow", -} - -_SUBCOMMANDS = [ - ("google", "Search with Google."), - ("brave", "Search with Brave Search."), - ("duckduckgo", "Search with DuckDuckGo."), - ("ddg", "Search with DuckDuckGo (alias for duckduckgo)."), - ("youtube", "Search YouTube videos."), - ("yt", "Search YouTube (alias for youtube)."), - ("spotify", "Search Spotify."), - ("amazon", "Search Amazon."), - ("ecosia", "Search with Ecosia."), - ("furaffinity", "Search FurAffinity."), - ("fa", "Search FurAffinity (alias for furaffinity)."), - ("bing", "Search with Bing."), - ("github", "Search GitHub."), - ("wikipedia", "Search Wikipedia."), - ("wiki", "Search Wikipedia (alias for wikipedia)."), - ("reddit", "Search Reddit."), - ("stackoverflow", "Search Stack Overflow."), - ("so", "Search Stack Overflow (alias for stackoverflow)."), -] - - @click.group("search") def search_group(): """Search the web — open a query in a search engine.""" @@ -70,10 +19,10 @@ def _build_command(engine_key: str, help_text: str) -> click.Command: terms = " ".join(query) client_from_ctx().nav.search(engine_key, terms, window=window, group=group) suffix = f" in group '{group}'" if group else (f" in window '{window}'" if window else "") - display = _DISPLAY_NAMES.get(engine_key, engine_key.capitalize()) + display = DISPLAY_NAMES.get(engine_key, engine_key.capitalize()) console.print(f"[green]Searching[/green] [cyan]{display}[/cyan]: {terms}{suffix}") return _cmd -for _name, _help in _SUBCOMMANDS: +for _name, _help in SUBCOMMANDS: search_group.add_command(_build_command(_name, _help)) diff --git a/browser_cli/commands/serve.py b/browser_cli/commands/serve.py index 268a1b5..c7fd445 100644 --- a/browser_cli/commands/serve.py +++ b/browser_cli/commands/serve.py @@ -8,110 +8,187 @@ from pathlib import Path import click from browser_cli import transport +from browser_cli.command_security import CommandPolicy +from browser_cli.commands import command_policy_from_options, command_policy_options from browser_cli.serve.runtime import ( - _async_framed_send, - _async_handle_client, - _async_recv_all, - _handle_client, - _serve_async, - console, + _async_framed_send, + _async_handle_client, + _async_recv_all, + _handle_client, + _serve_async, + console, ) +from browser_cli.serve.security import RateLimiter, ServeSecurity, key_policies_from_authorized_keys from browser_cli.version_manager import get_installed_version __all__ = [ - "_async_framed_send", - "_async_handle_client", - "_async_recv_all", - "_handle_client", - "_serve_async", - "cmd_serve", + "_async_framed_send", + "_async_handle_client", + "_async_recv_all", + "_handle_client", + "_serve_async", + "cmd_serve", ] +def _is_loopback(host: str) -> bool: + return host in {"127.0.0.1", "localhost", "::1"} + @click.command("serve") @click.option("--host", default="127.0.0.1", show_default=True, help="Address to bind.") @click.option("--port", default=8765, show_default=True, type=int, help="TCP port to listen on.") @click.option("--no-auth", is_flag=True, default=False, help="Disable authentication (dangerous).") @click.option( - "--authorized-keys", - "auth_keys_file", - default=None, - metavar="FILE", - help="File of trusted Ed25519 public keys (one hex per line). Required unless --no-auth.", + "--authorized-keys", + "auth_keys_file", + default=None, + metavar="FILE", + help="File of trusted Ed25519 public keys (one hex per line). Required unless --no-auth.", ) @click.option( - "--no-compress", - "no_compress", - is_flag=True, - default=False, - help="Disable response compression / msgpack even for clients that support it.", + "--no-compress", + "no_compress", + is_flag=True, + default=False, + help="Disable response compression / msgpack even for clients that support it.", ) +@click.option( + "--rate-limit", + default=100.0, + show_default=True, + type=float, + help="Max commands/sec per client key (0 disables).", +) +@command_policy_options @click.pass_context -def cmd_serve(ctx, host, port, no_auth, auth_keys_file, no_compress): - """Expose this browser over TCP so remote hosts can control it.""" - profile = ctx.obj.get("browser") if ctx.obj else None - compress = not no_compress +def cmd_serve(ctx, host, port, no_auth, auth_keys_file, no_compress, rate_limit, + allow_read_page, allow_control, allow_dangerous, allow_keys, allow_all): + """Expose this browser over TCP so remote hosts can control it. - if host in ("0.0.0.0", "::"): - console.print( - "[yellow]Warning:[/yellow] Binding to all interfaces — " - "anyone who can reach this port controls your browser." - ) + Commands are gated by a safe-only policy by default; remote clients can only + run read-only status/listing commands. Open more with --allow-read-page, + --allow-control, --allow-dangerous, or --allow-all (full control). Per-key + overrides come from an ``allow:`` token in authorized_keys (set via + ``auth trust --allow-*``), and --rate-limit throttles each client key. + """ + profile = ctx.obj.get("browser") if ctx.obj else None + compress = not no_compress - auth_keys_path = _resolve_auth_keys_path(auth_keys_file, no_auth) - if auth_keys_path is False: - sys.exit(1) + if no_auth and not _is_loopback(host): + console.print( + "[red]Error:[/red] --no-auth is only allowed on loopback hosts " + "(127.0.0.1, localhost, ::1). Use --authorized-keys to expose this browser to the network." + ) + sys.exit(1) - _print_startup(host, port, profile, auth_keys_path, compress) + if host in ("0.0.0.0", "::"): + console.print( + "[yellow]Warning:[/yellow] Binding to all interfaces — " + "anyone who can reach this port controls your browser." + ) - try: - asyncio.run(_serve_async(host, port, profile, auth_keys_path, compress)) - except OSError as e: - console.print(f"[red]Cannot bind to {host}:{port}:[/red] {e}") - sys.exit(1) - except KeyboardInterrupt: - console.print("[yellow]Stopped.[/yellow]") + policy = command_policy_from_options( + allow_read_page=allow_read_page, + allow_control=allow_control, + allow_dangerous=allow_dangerous, + allow_keys=allow_keys, + allow_all=allow_all, + ) + + auth_keys_path = _resolve_auth_keys_path(auth_keys_file, no_auth) + if auth_keys_path is False: + sys.exit(1) + + security = _build_security(policy, auth_keys_path, rate_limit) + + _print_startup(host, port, profile, auth_keys_path, compress, security) + + try: + asyncio.run(_serve_async(host, port, profile, auth_keys_path, compress, security)) + except OSError as e: + console.print(f"[red]Cannot bind to {host}:{port}:[/red] {e}") + sys.exit(1) + except KeyboardInterrupt: + console.print("[yellow]Stopped.[/yellow]") + +def _build_security(policy, auth_keys_path, rate_limit) -> ServeSecurity: + """Assemble the serve-time security context from the authorized_keys file.""" + key_policies: dict = {} + key_names: dict = {} + + if auth_keys_path is not None: + from browser_cli.auth import load_authorized_keys_with_names + + key_names = {pk.strip().lower(): name for pk, name in load_authorized_keys_with_names(auth_keys_path)} + key_policies = key_policies_from_authorized_keys(auth_keys_path) + + rate_limiter = RateLimiter(rate_limit) if rate_limit and rate_limit > 0 else None + return ServeSecurity(policy=policy, key_policies=key_policies, key_names=key_names, rate_limiter=rate_limiter) def _resolve_auth_keys_path(auth_keys_file: str | None, no_auth: bool) -> Path | None | bool: - if auth_keys_file: - from browser_cli.auth import load_authorized_keys + if auth_keys_file: + from browser_cli.auth import load_authorized_keys - auth_keys_path = Path(auth_keys_file) - if not load_authorized_keys(auth_keys_path): - console.print(f"[yellow]Warning:[/yellow] No authorized keys found in {auth_keys_path}") - return auth_keys_path - if no_auth: - return None - console.print( - "[red]Error:[/red] --authorized-keys FILE is required. " - "Use --no-auth to explicitly disable auth (dangerous)." - ) - return False + auth_keys_path = Path(auth_keys_file) + if not load_authorized_keys(auth_keys_path): + console.print(f"[yellow]Warning:[/yellow] No authorized keys found in {auth_keys_path}") + return auth_keys_path + if no_auth: + return None + console.print( + "[red]Error:[/red] --authorized-keys FILE is required. " + "Use --no-auth to explicitly disable auth (dangerous)." + ) + return False -def _print_startup(host: str, port: int, profile: str | None, auth_keys_path: Path | None, compress: bool) -> None: - current_ver = get_installed_version() - browser_hint = f" (browser: {profile})" if profile else "" - console.print(f"[green]Serving browser{browser_hint} →[/green] [cyan]{host}:{port}[/cyan] [dim]v{current_ver}[/dim]") +def _print_startup(host: str, port: int, profile: str | None, auth_keys_path: Path | None, compress: bool, security: ServeSecurity | None = None) -> None: + current_ver = get_installed_version() + security = security if security is not None else ServeSecurity() + browser_hint = f" (browser: {profile})" if profile else "" + console.print(f"[green]Serving browser{browser_hint} →[/green] [cyan]{host}:{port}[/cyan] [dim]v{current_ver}[/dim]") - if auth_keys_path is not None: - from browser_cli.auth import load_authorized_keys + if auth_keys_path is not None: + from browser_cli.auth import load_authorized_keys - n = len(load_authorized_keys(auth_keys_path)) - console.print(f" Auth: [bold green]Ed25519 pubkey[/bold green] ({n} trusted key{'s' if n != 1 else ''})") - else: - console.print("[yellow] Auth disabled (--no-auth)[/yellow]") + n = len(load_authorized_keys(auth_keys_path)) + console.print(f" Auth: [bold green]Ed25519 pubkey[/bold green] ({n} trusted key{'s' if n != 1 else ''})") + else: + console.print("[yellow] Auth disabled (--no-auth)[/yellow]") - console.print(f" CLI: [dim]browser-cli --remote {host}:{port} tabs list[/dim]") - console.print(f" Python: [dim]BrowserCLI(remote=\"{host}:{port}\").tabs.list()[/dim]") - _print_encoding_status(compress) - console.print("Ctrl-C to stop.\n") + _print_policy_status(security.policy) + if security.key_policies: + console.print(f" Per-key: [green]{len(security.key_policies)} override(s)[/green] [dim](allow: in authorized_keys)[/dim]") + if security.rate_limiter is not None: + console.print(f" Rate: [green]{security.rate_limiter.rate:g}/s per key[/green] [dim](burst {security.rate_limiter.capacity:g})[/dim]") + else: + console.print(" Rate: [yellow]unlimited[/yellow] [dim](--rate-limit 0)[/dim]") + + console.print(f" CLI: [dim]browser-cli --remote {host}:{port} tabs list[/dim]") + console.print(f" Python: [dim]BrowserCLI(remote=\"{host}:{port}\").tabs.list()[/dim]") + _print_encoding_status(compress) + console.print("Ctrl-C to stop.\n") + +def _print_policy_status(policy: CommandPolicy | None) -> None: + if policy is None or policy == CommandPolicy.unrestricted(): + console.print(" Policy: [yellow]unrestricted (--allow-all)[/yellow] [dim](every command allowed, incl. dom.eval/storage)[/dim]") + return + allowed = ["safe"] + if policy.allow_read_page: + allowed.append("read-page") + if policy.allow_control: + allowed.append("control") + if policy.allow_dangerous: + allowed.append("dangerous") + if policy.allow_keys: + allowed.append("keys") + console.print(f" Policy: [green]restricted[/green] [dim](allowed: {', '.join(allowed)})[/dim]") def _print_encoding_status(compress: bool) -> None: - if not compress: - console.print(" Encode: [yellow]off (--no-compress)[/yellow]") - return - codecs = "+".join(transport.supported_compression()) - sers = "+".join(transport.supported_serialization()) - console.print( - " Encode: [green]on[/green] " - f"[dim](compression: {codecs}; serialization: {sers}; per-client negotiated)[/dim]" - ) + if not compress: + console.print(" Encode: [yellow]off (--no-compress)[/yellow]") + return + codecs = "+".join(transport.supported_compression()) + sers = "+".join(transport.supported_serialization()) + console.print( + " Encode: [green]on[/green] " + f"[dim](compression: {codecs}; serialization: {sers}; per-client negotiated)[/dim]" + ) diff --git a/browser_cli/commands/serve_http.py b/browser_cli/commands/serve_http.py index 09b8e38..62eba2e 100644 --- a/browser_cli/commands/serve_http.py +++ b/browser_cli/commands/serve_http.py @@ -10,6 +10,7 @@ from rich.console import Console from browser_cli import BrowserCLI from browser_cli.command_security import CommandPolicy, assert_command_allowed +from browser_cli.commands import command_policy_from_options, command_policy_options console = Console() @@ -24,9 +25,11 @@ class _Handler(BaseHTTPRequestHandler): def _authorized(self) -> bool: if self.token is None: return True - if self.headers.get("Authorization", "") == f"Bearer {self.token}": + bearer = self.headers.get("Authorization", "") + if bearer.startswith("Bearer ") and secrets.compare_digest(bearer[len("Bearer "):], self.token): return True - return self.headers.get("X-Browser-CLI-Token") == self.token + header = self.headers.get("X-Browser-CLI-Token") + return header is not None and secrets.compare_digest(header, self.token) def _require_auth(self) -> bool: if self._authorized(): @@ -87,10 +90,8 @@ class _Handler(BaseHTTPRequestHandler): @click.option("--key", default=None, help="Remote auth key spec") @click.option("--token", default=None, help="Bearer token required for HTTP access (generated by default)") @click.option("--no-auth", is_flag=True, help="Disable HTTP auth (only allowed on loopback hosts)") -@click.option("--allow-read-page", is_flag=True, help="Allow /command to run page-content read commands") -@click.option("--allow-control", is_flag=True, help="Allow /command to run browser-control commands") -@click.option("--allow-dangerous", is_flag=True, help="Allow /command to run high-risk commands") -def cmd_serve_http(host, port, browser, remote, key, token, no_auth, allow_read_page, allow_control, allow_dangerous): +@command_policy_options +def cmd_serve_http(host, port, browser, remote, key, token, no_auth, allow_read_page, allow_control, allow_dangerous, allow_keys, allow_all): """Expose a tiny local HTTP JSON gateway (/tabs, /clients, /command). Auth is enabled by default. Pass the printed token as either @@ -99,7 +100,7 @@ def cmd_serve_http(host, port, browser, remote, key, token, no_auth, allow_read_ if no_auth and not _is_loopback(host): raise click.ClickException("--no-auth is only allowed on loopback hosts") auth_token = None if no_auth else (token or secrets.token_urlsafe(32)) - policy = CommandPolicy(allow_read_page=allow_read_page, allow_control=allow_control, allow_dangerous=allow_dangerous) + policy = command_policy_from_options(allow_read_page=allow_read_page, allow_control=allow_control, allow_dangerous=allow_dangerous, allow_keys=allow_keys, allow_all=allow_all) handler = type( "BrowserCLIHTTPHandler", (_Handler,), diff --git a/browser_cli/constants.py b/browser_cli/constants.py index c7f7609..a16f7f0 100644 --- a/browser_cli/constants.py +++ b/browser_cli/constants.py @@ -20,11 +20,29 @@ FIREFOX_EXTENSION_ID = "browser-cli@yiprawr.dev" ALLOWED_EXTENSION_IDS = [EXTENSION_ID, WEBSTORE_EXTENSION_ID] SUPPORTED_BROWSERS = ["chrome", "chromium", "brave", "edge", "vivaldi", "firefox"] +# Public store listings — the default install path now that the extension is +# published. Chromium-family browsers (Brave/Edge/Vivaldi/Chromium) can all +# install from the Chrome Web Store. +CHROME_WEBSTORE_URL = f"https://chromewebstore.google.com/detail/browser-cli/{WEBSTORE_EXTENSION_ID}" +FIREFOX_ADDON_URL = "https://addons.mozilla.org/firefox/addon/browser-cli/" + PROTOCOL_MIN_CLIENT = "0.9.0" MAX_MSG_BYTES = 32 * 1024 * 1024 DEFAULT_REMOTE_PORT = 443 -DEFAULT_PAGE_SIZE = 100 +# Count cap requested per page. The extension fills each page up to this many +# items OR a byte budget (whichever comes first), so large items (e.g. data-URI +# favicons) stay under the 1MB native-messaging limit while small items pack +# into far fewer roundtrips. +DEFAULT_PAGE_SIZE = 1000 +# Hard upper bound on total items collected across all pages, and the loop-guard +# page count. Kept independent of page size so byte-budgeted small pages don't +# falsely trip the guard. +MAX_PAGED_ITEMS = 10_000 DEFAULT_TRANSPORT_THRESHOLD = 512 +# How long a remote serve connection stays open waiting for the next command on +# an established encrypted session before closing. Lets the client reuse one +# authenticated connection for multiple commands instead of re-handshaking. +REMOTE_SESSION_IDLE_TIMEOUT = 30 NO_ROUTE_COMMANDS = {"browser-cli.targets", "browser-cli.auth.keys", "browser-cli.auth.trust"} GENTLE_MODES = ["auto", "normal", "gentle", "ultra"] diff --git a/browser_cli/models.py b/browser_cli/models.py index 98d1d8c..295d0e7 100644 --- a/browser_cli/models.py +++ b/browser_cli/models.py @@ -62,27 +62,27 @@ class Tab: def close(self) -> None: """Close this tab.""" - self._command("tabs.close", {"tabId": self.id}) + self._b().tabs.close(self.id) def activate(self) -> None: """Switch browser focus to this tab.""" - self._command("tabs.active", {"tabId": self.id}) + self._b().tabs.activate(self.id) def mute(self) -> None: """Mute this tab.""" - self._command("tabs.mute", {"tabId": self.id}) + self._b().tabs.mute(self.id) def unmute(self) -> None: """Unmute this tab.""" - self._command("tabs.unmute", {"tabId": self.id}) + self._b().tabs.unmute(self.id) def reload(self) -> None: """Reload this tab.""" - self._command("navigate.reload", {"tabId": self.id}) + self._b().nav.reload(self.id) def hard_reload(self) -> None: """Hard-reload this tab (bypass cache).""" - self._command("navigate.hard_reload", {"tabId": self.id}) + self._b().nav.hard_reload(self.id) def move( self, *, @@ -101,18 +101,18 @@ class Tab: window_id: Move to the window with this ID. index: Absolute position index in the target window. """ - self._command("tabs.move", { - "tabId": self.id, - "forward": forward, - "backward": backward, - "groupId": group_id, - "windowId": window_id, - "index": index, - }) + self._b().tabs.move( + self.id, + forward=forward, + backward=backward, + group_id=group_id, + window_id=window_id, + index=index, + ) def html(self) -> str: """Return the full HTML source of this tab.""" - return self._command("tabs.html", {"tabId": self.id}) + return self._b().tabs.html(self.id) def screenshot(self, *, format: str = "png", quality: int | None = None) -> str: """Capture this tab's visible area. Returns a base64 data URL.""" @@ -120,11 +120,11 @@ class Tab: def pin(self) -> None: """Pin this tab.""" - self._command("tabs.pin", {"tabId": self.id}) + self._b().tabs.pin(self.id) def unpin(self) -> None: """Unpin this tab.""" - self._command("tabs.unpin", {"tabId": self.id}) + self._b().tabs.unpin(self.id) def refresh(self) -> Tab: """Return a fresh snapshot of this tab.""" @@ -170,7 +170,7 @@ class Group: def close(self) -> None: """Ungroup (and close) this tab group.""" - self._command("group.close", {"groupId": self.id}) + self._b().groups.close(self.id) def tabs(self) -> list[Tab]: """Return all tabs inside this group.""" @@ -178,11 +178,7 @@ class Group: def move(self, *, forward: bool = False, backward: bool = False) -> None: """Move this group forward or backward among groups.""" - self._command("group.move", { - "group": str(self.id), - "forward": forward, - "backward": backward, - }) + self._b().groups.move(str(self.id), forward=forward, backward=backward) def add_tab(self, url: str | None = None) -> int | None: """Open a new tab inside this group. Returns the new tab ID.""" diff --git a/browser_cli/native/host.py b/browser_cli/native/host.py index 32fb5f1..26333d0 100644 --- a/browser_cli/native/host.py +++ b/browser_cli/native/host.py @@ -7,7 +7,6 @@ It relays messages between extension (stdin/stdout Native Messaging protocol) and CLI (local IPC endpoint: Unix socket on Unix, named pipe on Windows). """ import json -import math import os import queue import socket @@ -17,7 +16,7 @@ import uuid from pathlib import Path from browser_cli.native import local_server, protocol -from browser_cli.constants import DEFAULT_ALIAS, DEFAULT_PAGE_SIZE, PAGEABLE_COMMANDS +from browser_cli.constants import DEFAULT_ALIAS, DEFAULT_PAGE_SIZE, MAX_PAGED_ITEMS, PAGEABLE_COMMANDS from browser_cli.platform import endpoint_for_alias, is_windows, registry_path, runtime_dir from browser_cli.registry import update_registry @@ -126,7 +125,10 @@ def _collect_paged_browser_command(cmd: dict) -> dict: offset = 0 items = [] total = None - max_pages = math.ceil(10_000 / PAGE_SIZE) + # Independent of PAGE_SIZE: the extension may return fewer items per page than + # requested (byte budget), so a page-count guard derived from PAGE_SIZE would + # falsely trip. Bound the page count by the absolute item cap instead. + max_pages = MAX_PAGED_ITEMS pages_fetched = 0 while True: @@ -154,7 +156,7 @@ def _collect_paged_browser_command(cmd: dict) -> dict: items.extend(page_items) total = data.get("total", total) next_offset = data.get("nextOffset") - if next_offset is None: + if next_offset is None or len(items) >= MAX_PAGED_ITEMS: break offset = int(next_offset) diff --git a/browser_cli/remote/pool.py b/browser_cli/remote/pool.py new file mode 100644 index 0000000..e00bbec --- /dev/null +++ b/browser_cli/remote/pool.py @@ -0,0 +1,103 @@ +"""Per-process pool of authenticated remote connections for reuse. + +A ``browser-cli serve`` connection stays open after its first (encrypted) +command, so the client can send further commands over it without re-running the +TCP/TLS/challenge/auth handshake (~hundreds of ms each). Only encrypted (PQ) +sessions are pooled — plaintext/legacy sessions stay one-shot, matching the +server, which only loops for encrypted sessions. + +Connections are checked out exclusively (never shared between threads at once), +returned on success, and dropped on any I/O error or once older than an idle +bound (kept below the server's idle timeout so we don't reuse a connection the +server has already closed). +""" +from __future__ import annotations + +import atexit +import json +import socket +import threading +import time + +from browser_cli.constants import REMOTE_SESSION_IDLE_TIMEOUT +from browser_cli.framing import frame + +# Retire a pooled connection a few seconds before the server would, so we never +# hand back one the server has just timed out and closed. +_MAX_IDLE_SECONDS = max(5, REMOTE_SESSION_IDLE_TIMEOUT - 5) +_MAX_PER_ENDPOINT = 8 + +class PooledConnection: + __slots__ = ("sock", "secret", "last_used") + + def __init__(self, sock: socket.socket, secret: bytes) -> None: + self.sock = sock + self.secret = secret + self.last_used = time.monotonic() + +_POOL: dict[str, list[PooledConnection]] = {} +_LOCK = threading.Lock() + +def _close(sock: socket.socket) -> None: + try: + sock.close() + except OSError: + pass + +def checkout(endpoint: str) -> PooledConnection | None: + """Take an idle authenticated connection for *endpoint*, or None.""" + now = time.monotonic() + with _LOCK: + conns = _POOL.get(endpoint) + while conns: + conn = conns.pop() + if now - conn.last_used <= _MAX_IDLE_SECONDS: + return conn + _close(conn.sock) # too old — assume the server has dropped it + return None + +def checkin(endpoint: str, conn: PooledConnection) -> None: + """Return a still-healthy connection to the pool for reuse.""" + conn.last_used = time.monotonic() + with _LOCK: + bucket = _POOL.setdefault(endpoint, []) + if len(bucket) >= _MAX_PER_ENDPOINT: + _close(conn.sock) + return + bucket.append(conn) + +def discard(conn: PooledConnection) -> None: + """Drop a connection that errored or is no longer usable.""" + _close(conn.sock) + +def close_all() -> None: + """Close every pooled connection (process exit / test isolation).""" + with _LOCK: + for bucket in _POOL.values(): + for conn in bucket: + _close(conn.sock) + _POOL.clear() + +def session_inner_message(msg: dict) -> dict: + """Strip auth/transport fields, leaving the command for an established session.""" + keep = {"id", "command", "args", "user_agent", "accept_encoding", "_route", "_suppress_pq_warning"} + return {k: v for k, v in msg.items() if k in keep} + +def send_over(conn: PooledConnection, msg: dict) -> bytes | None: + """Send one command over an existing encrypted session. Raises on I/O error.""" + from browser_cli.auth import pq_encrypt + from browser_cli.remote.socket import recv_all + from browser_cli.remote.transport import _decode_pq_response + + inner = json.dumps(session_inner_message(msg)).encode("utf-8") + envelope = json.dumps({"encrypted": pq_encrypt(conn.secret, "request", inner)}).encode("utf-8") + conn.sock.sendall(frame(envelope)) + response = recv_all(conn.sock) + if not response: + # EOF — an older server (no session loop) closed after one command. Treat as + # a transport failure so the caller re-handshakes; never as an app error, + # which could double-execute a non-idempotent command on retry. + raise EOFError("remote closed the pooled connection") + return _decode_pq_response(response, conn.secret) + +atexit.register(close_all) diff --git a/browser_cli/remote/socket.py b/browser_cli/remote/socket.py index 0d4c298..cc38933 100644 --- a/browser_cli/remote/socket.py +++ b/browser_cli/remote/socket.py @@ -25,8 +25,8 @@ def split_endpoint(endpoint: str) -> tuple[str, int]: host, _, port_str = connect_ep.rpartition(":") return host, int(port_str) -@contextmanager -def open_socket(endpoint: str): +def connect_socket(endpoint: str) -> socket.socket: + """Open and (on :443) TLS-wrap a socket. Caller owns closing it.""" host, port = split_endpoint(endpoint) raw_sock = socket.socket(socket.AF_INET, socket.SOCK_STREAM) raw_sock.settimeout(30) @@ -40,6 +40,11 @@ def open_socket(endpoint: str): except Exception: raw_sock.close() raise + return sock + +@contextmanager +def open_socket(endpoint: str): + sock = connect_socket(endpoint) with sock: yield sock diff --git a/browser_cli/remote/transport.py b/browser_cli/remote/transport.py index 92ac836..9ed657e 100644 --- a/browser_cli/remote/transport.py +++ b/browser_cli/remote/transport.py @@ -20,24 +20,50 @@ from browser_cli.remote.auth import ( from browser_cli.remote.socket import ( async_recv_all as _async_recv_all, async_recv_exact_bytes as _async_recv_exact, + connect_socket as _connect_socket, open_async_connection as _open_async_connection, open_socket as _open_socket, recv_all as _recv_all, recv_exact_bytes as _recv_exact, split_endpoint as _split_endpoint, ) +from browser_cli.remote import pool as _pool def _send_remote(endpoint: str, msg: dict, private_key=None, *, warn_no_pq: bool | None = None) -> bytes | None: + # Reuse an already-authenticated connection when one is idle for this endpoint. + conn = _pool.checkout(endpoint) + if conn is not None: + try: + response = _pool.send_over(conn, msg) + _pool.checkin(endpoint, conn) + return response + except (OSError, ConnectionError, ValueError, EOFError): + _pool.discard(conn) # stale/closed — fall through to a fresh handshake + + return _send_remote_handshake(endpoint, msg, private_key, warn_no_pq=warn_no_pq) + +def _send_remote_handshake(endpoint: str, msg: dict, private_key=None, *, warn_no_pq: bool | None = None) -> bytes | None: warn = _should_warn_no_pq(msg) if warn_no_pq is None else warn_no_pq def build_auth(sync_msg: dict, challenge: dict | None, nonce_hex: str | None, key): from browser_cli.auth import pq_kex_client_encapsulate return _build_auth_message(sync_msg, challenge, nonce_hex, key, pq_kex_client_encapsulate, warn_no_pq=warn) - with _open_socket(endpoint) as sock: + sock = _connect_socket(endpoint) + try: payload_msg, pq_shared_secret = _with_challenge(_recv_all(sock), msg, private_key, build_auth) sock.sendall(frame(json.dumps(payload_msg).encode("utf-8"))) - return _decode_pq_response(_recv_all(sock), pq_shared_secret) + response = _decode_pq_response(_recv_all(sock), pq_shared_secret) + except BaseException: + _pool._close(sock) + raise + # Only encrypted sessions are reusable — the server keeps those open, and a + # fresh AEAD nonce per frame keeps reuse of the shared secret safe. + if pq_shared_secret is not None: + _pool.checkin(endpoint, _pool.PooledConnection(sock, pq_shared_secret)) + else: + _pool._close(sock) + return response async def _send_remote_async(endpoint: str, msg: dict, private_key=None, *, warn_no_pq: bool | None = None) -> bytes | None: reader, writer = await _open_async_connection(endpoint) diff --git a/browser_cli/sdk/navigation.py b/browser_cli/sdk/navigation.py index 6678cbf..6d13042 100644 --- a/browser_cli/sdk/navigation.py +++ b/browser_cli/sdk/navigation.py @@ -114,7 +114,7 @@ class NavigationNS(Namespace): ) -> None: """Open a search query in the given engine (e.g. 'google', 'youtube', 'ddg').""" from urllib.parse import quote_plus - from browser_cli.commands.search import ENGINES + from browser_cli.search.engines import ENGINES template = ENGINES.get(engine) if template is None: raise ValueError(f"Unknown search engine '{engine}'. Available: {', '.join(ENGINES)}") diff --git a/browser_cli/sdk/routing.py b/browser_cli/sdk/routing.py index 3b863b3..cddffef 100644 --- a/browser_cli/sdk/routing.py +++ b/browser_cli/sdk/routing.py @@ -7,12 +7,14 @@ helpers; single-browser mode falls straight through to ``_cmd``. """ from __future__ import annotations +import asyncio import importlib import sys from collections.abc import Callable, Iterable from typing import TYPE_CHECKING, Protocol, cast from browser_cli.client import BrowserTarget +from browser_cli.client.core import _run_concurrent from browser_cli.errors import BrowserNotConnected from browser_cli.models import BrowserCounts, Tab @@ -81,18 +83,28 @@ class RoutingMixin: return targets def _collect_multi_browser(self, command: str, args: dict | None = None): - results = [] targets = self._multi_browser_targets() - for target in targets: - try: - if target.remote: - data = _browser_cli_package().send_command( - command, args, profile=target.profile, remote=target.remote, key=self._client._key - ) - else: - data = _browser_cli_package().send_command(command, args, profile=target.profile) - except (BrowserNotConnected, RuntimeError): + + def _send(target: BrowserTarget): + package = _browser_cli_package() + if target.remote: + return package.send_command( + command, args, profile=target.profile, remote=target.remote, key=self._client._key + ) + return package.send_command(command, args, profile=target.profile) + + # Run per-target roundtrips concurrently — each is a blocking, network-bound + # send_command, so offloading to threads gives real overlap while still + # invoking the (test-patchable) sync entry point. + raw = _run_concurrent([ + (lambda t=t: asyncio.to_thread(_send, t)) for t in targets + ]) + results = [] + for target, data in zip(targets, raw): + if isinstance(data, (BrowserNotConnected, RuntimeError)): continue + if isinstance(data, BaseException): + raise data results.append((target, data)) if results: return results diff --git a/browser_cli/search/__init__.py b/browser_cli/search/__init__.py new file mode 100644 index 0000000..6eda835 --- /dev/null +++ b/browser_cli/search/__init__.py @@ -0,0 +1 @@ +"""Search metadata and helpers shared by SDK and CLI layers.""" diff --git a/browser_cli/search/engines.py b/browser_cli/search/engines.py new file mode 100644 index 0000000..24cc586 --- /dev/null +++ b/browser_cli/search/engines.py @@ -0,0 +1,53 @@ +"""Shared search-engine metadata for SDK and CLI search commands.""" +from __future__ import annotations + +ENGINES = { + "google": "https://www.google.com/search?q={query}", + "brave": "https://search.brave.com/search?q={query}", + "duckduckgo": "https://duckduckgo.com/?q={query}", + "ddg": "https://duckduckgo.com/?q={query}", + "youtube": "https://www.youtube.com/results?search_query={query}", + "yt": "https://www.youtube.com/results?search_query={query}", + "spotify": "https://open.spotify.com/search/{query}", + "amazon": "https://www.amazon.com/s?k={query}", + "ecosia": "https://www.ecosia.org/search?q={query}", + "furaffinity": "https://www.furaffinity.net/search/?q={query}", + "fa": "https://www.furaffinity.net/search/?q={query}", + "bing": "https://www.bing.com/search?q={query}", + "github": "https://github.com/search?q={query}", + "wikipedia": "https://en.wikipedia.org/wiki/Special:Search?search={query}", + "wiki": "https://en.wikipedia.org/wiki/Special:Search?search={query}", + "reddit": "https://www.reddit.com/search/?q={query}", + "stackoverflow": "https://stackoverflow.com/search?q={query}", + "so": "https://stackoverflow.com/search?q={query}", +} + +DISPLAY_NAMES = { + "google": "Google", "brave": "Brave Search", "duckduckgo": "DuckDuckGo", + "ddg": "DuckDuckGo", "youtube": "YouTube", "yt": "YouTube", + "spotify": "Spotify", "amazon": "Amazon", "ecosia": "Ecosia", + "furaffinity": "FurAffinity", "fa": "FurAffinity", "bing": "Bing", + "github": "GitHub", "wikipedia": "Wikipedia", "wiki": "Wikipedia", + "reddit": "Reddit", "stackoverflow": "Stack Overflow", "so": "Stack Overflow", +} + +SUBCOMMANDS = [ + ("google", "Search with Google."), + ("brave", "Search with Brave Search."), + ("duckduckgo", "Search with DuckDuckGo."), + ("ddg", "Search with DuckDuckGo (alias for duckduckgo)."), + ("youtube", "Search YouTube videos."), + ("yt", "Search YouTube (alias for youtube)."), + ("spotify", "Search Spotify."), + ("amazon", "Search Amazon."), + ("ecosia", "Search with Ecosia."), + ("furaffinity", "Search FurAffinity."), + ("fa", "Search FurAffinity (alias for furaffinity)."), + ("bing", "Search with Bing."), + ("github", "Search GitHub."), + ("wikipedia", "Search Wikipedia."), + ("wiki", "Search Wikipedia (alias for wikipedia)."), + ("reddit", "Search Reddit."), + ("stackoverflow", "Search Stack Overflow."), + ("so", "Search Stack Overflow (alias for stackoverflow)."), +] diff --git a/browser_cli/serve/control.py b/browser_cli/serve/control.py index 2666af5..6b0145f 100644 --- a/browser_cli/serve/control.py +++ b/browser_cli/serve/control.py @@ -10,6 +10,7 @@ class ServeControlMixin: addr: tuple command: str auth_keys_path: Path | None + auth_label: str | None async def send_error(self, msg: str, msg_id=None) -> None: ... async def send_ok(self, payload, command: str | None = None) -> None: ... @@ -23,25 +24,32 @@ class ServeControlMixin: try: clients = send_command("clients.list", profile=target.profile, suppress_pq_warning=True) if clients: - browser_name = clients[0].get("name") - if browser_name: - item["browserName"] = browser_name + # Carry the full client info so a remote `clients` command can render + # from this single roundtrip instead of issuing another clients.list. + info = clients[0] + for src, dst in (("name", "browserName"), ("version", "version"), ("extensionVersion", "extensionVersion")): + value = info.get(src) + if value: + item[dst] = value except Exception: pass targets.append(item) await self.send_ok(targets, self.command) - log_request(self.addr, self.command, None, "OK") + log_request(self.addr, self.command, None, "OK", identity=self.auth_label) return True if self.command == "browser-cli.auth.keys": if self.auth_keys_path is None: await self.send_error("no authorized keys file configured on this server") - log_request(self.addr, self.command, None, "ERROR", "no authorized keys file") + log_request(self.addr, self.command, None, "ERROR", "no authorized keys file", identity=self.auth_label) return True - from browser_cli.auth import load_authorized_keys_with_names - entries = [{"pubkey": pk, "name": name} for pk, name in load_authorized_keys_with_names(self.auth_keys_path)] + from browser_cli.auth import load_authorized_keys_with_policies + entries = [ + {"pubkey": pk, "name": name, "allow": cats} + for pk, name, cats in load_authorized_keys_with_policies(self.auth_keys_path) + ] await self.send_ok(entries, self.command) - log_request(self.addr, self.command, None, "OK") + log_request(self.addr, self.command, None, "OK", identity=self.auth_label) return True if self.command == "browser-cli.auth.trust": @@ -54,14 +62,27 @@ class ServeControlMixin: log_request(self.addr, self.command, None, "ERROR", "no authorized keys file") return True from browser_cli.auth import add_authorized_key + from browser_cli.serve.security import policy_from_categories args = msg.get("args") or {} pubkey = str(args.get("pubkey") or "") name = str(args.get("name") or "") + categories = args.get("allow") if not re.fullmatch(r"[0-9a-f]{64}", pubkey): await self.send_error("invalid pubkey: expected 64 lowercase hex characters") - log_request(self.addr, self.command, None, "ERROR", "invalid pubkey") + log_request(self.addr, self.command, None, "ERROR", "invalid pubkey", identity=self.auth_label) return True - added = add_authorized_key(self.auth_keys_path, pubkey, name) + if categories is not None: + if not isinstance(categories, list): + await self.send_error("invalid allow: expected a list of category strings") + log_request(self.addr, self.command, None, "ERROR", "invalid allow", identity=self.auth_label) + return True + try: + policy_from_categories(categories) # validate before persisting + except ValueError as exc: + await self.send_error(str(exc)) + log_request(self.addr, self.command, None, "ERROR", "invalid allow category", identity=self.auth_label) + return True + added = add_authorized_key(self.auth_keys_path, pubkey, name, categories) await self.send_ok({"added": added}, self.command) - log_request(self.addr, self.command, None, "OK" if added else "ALREADY_TRUSTED") + log_request(self.addr, self.command, None, "OK" if added else "ALREADY_TRUSTED", identity=self.auth_label) return True diff --git a/browser_cli/serve/logging.py b/browser_cli/serve/logging.py index 8405d1f..8a74c3c 100644 --- a/browser_cli/serve/logging.py +++ b/browser_cli/serve/logging.py @@ -6,11 +6,19 @@ from rich.console import Console console = Console() -def log_request(addr: tuple, command: str, profile: str | None, status: str, error: str | None = None) -> None: +def log_request( + addr: tuple, + command: str, + profile: str | None, + status: str, + error: str | None = None, + identity: str | None = None, +) -> None: ts = datetime.now().strftime("%H:%M:%S") addr_str = f"{addr[0]}:{addr[1]}" + identity_str = f"[magenta]{identity}[/magenta] " if identity else "" profile_str = f"[dim]{profile}[/dim] " if profile else "" if error: - console.print(f"[dim]{ts}[/dim] {addr_str} {profile_str}[cyan]{command}[/cyan] [red]{status}[/red] {error}") + console.print(f"[dim]{ts}[/dim] {addr_str} {identity_str}{profile_str}[cyan]{command}[/cyan] [red]{status}[/red] {error}") else: - console.print(f"[dim]{ts}[/dim] {addr_str} {profile_str}[cyan]{command}[/cyan] [green]{status}[/green]") + console.print(f"[dim]{ts}[/dim] {addr_str} {identity_str}{profile_str}[cyan]{command}[/cyan] [green]{status}[/green]") diff --git a/browser_cli/serve/proxy.py b/browser_cli/serve/proxy.py index cc0c7e5..52e3b68 100644 --- a/browser_cli/serve/proxy.py +++ b/browser_cli/serve/proxy.py @@ -18,6 +18,7 @@ class ServeProxyMixin: command: str compress: bool accept_encoding: dict | None + auth_label: str | None async def send_error(self, msg: str, msg_id=None) -> None: ... async def send_payload(self, data: bytes) -> None: ... @@ -35,7 +36,7 @@ class ServeProxyMixin: sock_path = resolve_socket(resolved_profile) except BrowserNotConnected as e: await self.send_error(str(e)) - log_request(self.addr, self.command, resolved_profile, "ERROR", "browser not connected") + log_request(self.addr, self.command, resolved_profile, "ERROR", "browser not connected", identity=self.auth_label) return try: @@ -46,7 +47,7 @@ class ServeProxyMixin: await self.send_browser_response(adapt_response(resp_payload, self.command, self.client_ver), resolved_profile) except (OSError, json.JSONDecodeError, ConnectionError) as e: await self.send_error(str(e)) - log_request(self.addr, self.command, resolved_profile, "ERROR", str(e)) + log_request(self.addr, self.command, resolved_profile, "ERROR", str(e), identity=self.auth_label) async def _windows_roundtrip(self, sock_path: str, payload: bytes) -> bytes: from multiprocessing.connection import Client as PipeClient @@ -74,6 +75,6 @@ class ServeProxyMixin: else: await self.send_payload(resp_payload) if resp_data.get("success", True): - log_request(self.addr, self.command, resolved_profile, "OK") + log_request(self.addr, self.command, resolved_profile, "OK", identity=self.auth_label) else: - log_request(self.addr, self.command, resolved_profile, "ERROR", resp_data.get("error", "")) + log_request(self.addr, self.command, resolved_profile, "ERROR", resp_data.get("error", ""), identity=self.auth_label) diff --git a/browser_cli/serve/runtime.py b/browser_cli/serve/runtime.py index 2331a8d..be86267 100644 --- a/browser_cli/serve/runtime.py +++ b/browser_cli/serve/runtime.py @@ -9,17 +9,20 @@ from __future__ import annotations import asyncio import json import socket -from dataclasses import dataclass +from dataclasses import dataclass, field from pathlib import Path from browser_cli import transport +from browser_cli.command_security import assert_command_allowed from browser_cli.compat import adapt_auth +from browser_cli.constants import REMOTE_SESSION_IDLE_TIMEOUT from browser_cli.framing import async_recv_frame, async_send_frame from browser_cli.serve.auth import ServeAuthMixin from browser_cli.serve.challenge import build_challenge as _build_challenge, load_auth_keys as _load_auth_keys from browser_cli.serve.control import ServeControlMixin from browser_cli.serve.logging import console, log_request from browser_cli.serve.proxy import ServeProxyMixin +from browser_cli.serve.security import ServeSecurity async def _async_framed_send(writer: asyncio.StreamWriter, data: bytes) -> None: await async_send_frame(writer, data) @@ -38,12 +41,15 @@ class ServeRequest(ServeAuthMixin, ServeControlMixin, ServeProxyMixin): nonce: str pq_private_key: object | None = None compress: bool = True + security: ServeSecurity = field(default_factory=ServeSecurity) response_secret: bytes | None = None accept_encoding: dict | None = None client_ver: str = "0" msg_id: object = None command: str = "?" + auth_pubkey: str | None = None + auth_label: str | None = None async def send_payload(self, data: bytes) -> None: if self.response_secret is not None: @@ -89,11 +95,73 @@ class ServeRequest(ServeAuthMixin, ServeControlMixin, ServeProxyMixin): msg = await self.authenticate(msg) if msg is None: return + self._apply_identity(msg) + await self._dispatch(msg) + # Once an encrypted session is established, keep serving further commands on + # the same connection — the client may reuse it without re-authenticating. + # Safe because every frame carries a fresh AEAD nonce (see pq_encrypt). + while self.response_secret is not None: + nxt = await self._read_session_message() + if nxt is None: + return + await self._dispatch(nxt) + + def _apply_identity(self, msg: dict) -> None: + """Record the authenticated pubkey (if any) for per-key policy and audit logs.""" + pub = (msg.get("pubkey") or "").strip().lower() + self.auth_pubkey = pub or None + self.auth_label = self.security.label_for(self.auth_pubkey) + + async def _enforce_rate_limit(self) -> bool: + limiter = self.security.rate_limiter + if limiter is None or limiter.allow(self.auth_pubkey or str(self.addr[0])): + return True + await self.send_error("rate limit exceeded; slow down and retry") + log_request(self.addr, self.command, None, "DENIED", "rate limit exceeded", identity=self.auth_label) + return False + + async def _dispatch(self, msg: dict) -> None: self.accept_encoding = msg.get("accept_encoding") + if not await self._enforce_rate_limit(): + return + # Gate every command — including server control commands like the key-management + # ones — so the policy is enforced before handle_control_command acts on it. + try: + assert_command_allowed(self.command, self.security.effective_policy(self.auth_pubkey)) + except PermissionError as exc: + await self.send_error(str(exc)) + log_request(self.addr, self.command, None, "DENIED", "blocked by command policy", identity=self.auth_label) + return if await self.handle_control_command(msg): return await self.forward_to_browser(msg) + async def _read_session_message(self) -> dict | None: + """Read the next command on an established encrypted session, or None to close.""" + try: + payload = await asyncio.wait_for(_async_recv_all(self.reader), timeout=REMOTE_SESSION_IDLE_TIMEOUT) + except (asyncio.TimeoutError, ConnectionError, OSError): + return None + if not payload: + return None + try: + outer = json.loads(payload) + except (json.JSONDecodeError, ValueError): + return None + if not isinstance(outer, dict) or "encrypted" not in outer: + return None # an authenticated session only accepts encrypted frames + from browser_cli.auth import pq_decrypt + try: + inner = json.loads(pq_decrypt(self.response_secret, "request", outer["encrypted"])) + except Exception: + return None + if not isinstance(inner, dict): + return None + inner = adapt_auth(inner, self.client_ver) + self.msg_id = inner.get("id") + self.command = inner.get("command", "?") + return inner + async def _async_proxy_request( reader: asyncio.StreamReader, writer: asyncio.StreamWriter, @@ -104,8 +172,12 @@ async def _async_proxy_request( nonce: str, pq_private_key=None, compress: bool = True, + security: ServeSecurity | None = None, ) -> None: - await ServeRequest(reader, writer, addr, profile, auth_keys, auth_keys_path, nonce, pq_private_key, compress).run() + await ServeRequest( + reader, writer, addr, profile, auth_keys, auth_keys_path, nonce, pq_private_key, compress, + security if security is not None else ServeSecurity(), + ).run() async def _async_handle_client( reader: asyncio.StreamReader, @@ -115,6 +187,7 @@ async def _async_handle_client( auth_keys_path: Path | None, compress: bool = True, conn_limit: asyncio.Semaphore | None = None, + security: ServeSecurity | None = None, ) -> None: if conn_limit is None: conn_limit = asyncio.Semaphore(64) @@ -130,7 +203,7 @@ async def _async_handle_client( await _async_framed_send(writer, json.dumps(challenge_msg).encode()) except OSError: return - await _async_proxy_request(reader, writer, addr, profile, auth_keys, auth_keys_path, nonce, pq_private_key, compress) + await _async_proxy_request(reader, writer, addr, profile, auth_keys, auth_keys_path, nonce, pq_private_key, compress, security) finally: conn_limit.release() writer.close() @@ -145,12 +218,13 @@ def _handle_client( profile: str | None, auth_keys_path: Path | None, compress: bool = True, + security: ServeSecurity | None = None, ) -> None: """Run one accepted socket through the async serve pipeline.""" async def _run() -> None: reader, writer = await asyncio.open_connection(sock=client_sock) - await _async_handle_client(reader, writer, addr, profile, auth_keys_path, compress) + await _async_handle_client(reader, writer, addr, profile, auth_keys_path, compress, None, security) try: asyncio.run(_run()) @@ -160,12 +234,19 @@ def _handle_client( except OSError: pass -async def _serve_async(host: str, port: int, profile: str | None, auth_keys_path: Path | None, compress: bool) -> None: +async def _serve_async( + host: str, + port: int, + profile: str | None, + auth_keys_path: Path | None, + compress: bool, + security: ServeSecurity | None = None, +) -> None: conn_limit = asyncio.Semaphore(64) async def _client_connected(reader: asyncio.StreamReader, writer: asyncio.StreamWriter) -> None: peer = writer.get_extra_info("peername") or ("?", 0) - await _async_handle_client(reader, writer, peer, profile, auth_keys_path, compress, conn_limit) + await _async_handle_client(reader, writer, peer, profile, auth_keys_path, compress, conn_limit, security) server = await asyncio.start_server(_client_connected, host, port, backlog=16) async with server: diff --git a/browser_cli/serve/security.py b/browser_cli/serve/security.py new file mode 100644 index 0000000..65c181d --- /dev/null +++ b/browser_cli/serve/security.py @@ -0,0 +1,115 @@ +"""Server-side authorization, per-key policy and rate limiting for ``browser-cli serve``. + +This bundles the three serve-time security concerns that travel together through +the connection-handling chain: + +- ``policy`` the server-wide default ``CommandPolicy`` (from ``--allow-*``) +- ``key_policies`` optional per-pubkey overrides parsed from the ``allow:`` token + in the ``authorized_keys`` file +- ``key_names`` pubkey -> friendly name (from authorized_keys), for audit logs +- ``rate_limiter`` optional per-identity token-bucket throttle +""" +from __future__ import annotations + +import threading +import time +from dataclasses import dataclass, field +from pathlib import Path + +from browser_cli.command_security import CommandPolicy + +# ── per-key authorization ─────────────────────────────────────────────────────── + +_CATEGORY_FLAGS = { + "read-page": "allow_read_page", + "control": "allow_control", + "dangerous": "allow_dangerous", + "keys": "allow_keys", +} + +def policy_from_categories(categories) -> CommandPolicy: + """Build a CommandPolicy from category strings (``all``/``safe``/``read-page``/``control``/``dangerous``).""" + cats = [str(c).strip().lower() for c in categories] + if "all" in cats: + return CommandPolicy.unrestricted() + kwargs: dict[str, bool] = {} + for cat in cats: + if cat in ("", "safe"): + continue + flag = _CATEGORY_FLAGS.get(cat) + if flag is None: + raise ValueError( + f"unknown command category {cat!r}; expected one of: all, safe, read-page, control, dangerous" + ) + kwargs[flag] = True + return CommandPolicy(**kwargs) + +def key_policies_from_authorized_keys(path: Path | str | None) -> dict[str, CommandPolicy]: + """Build ``{pubkey: CommandPolicy}`` from the ``allow:`` tokens in authorized_keys. + + Only keys that carry an explicit ``allow:`` token get an entry; keys without + one fall back to the server-wide default policy. Pubkeys are normalised to + lowercase hex. Raises ``ValueError`` on an unknown category so the server fails + loudly at startup rather than silently mis-gating. + """ + if path is None: + return {} + from browser_cli.auth import load_authorized_keys_with_policies + + out: dict[str, CommandPolicy] = {} + for pubkey, _name, categories in load_authorized_keys_with_policies(Path(path)): + if categories is not None: + out[pubkey.strip().lower()] = policy_from_categories(categories) + return out + +# ── per-identity rate limiting ─────────────────────────────────────────────────── + +class RateLimiter: + """Token bucket keyed by identity (pubkey, or client address when unauthenticated). + + ``rate`` is the sustained refill in tokens/second; ``burst`` is the bucket + capacity (defaults to ``rate``). ``rate <= 0`` disables limiting entirely. + Thread-safe so it can be shared across all connections of one serve process. + """ + + def __init__(self, rate: float, burst: float | None = None) -> None: + self.rate = float(rate) + self.capacity = float(burst) if burst is not None else max(float(rate), 1.0) + self._buckets: dict[str, tuple[float, float]] = {} + self._lock = threading.Lock() + + def allow(self, key: str) -> bool: + if self.rate <= 0: + return True + now = time.monotonic() + with self._lock: + tokens, last = self._buckets.get(key, (self.capacity, now)) + tokens = min(self.capacity, tokens + (now - last) * self.rate) + if tokens < 1.0: + self._buckets[key] = (tokens, now) + return False + self._buckets[key] = (tokens - 1.0, now) + return True + +# ── bundled server security context ────────────────────────────────────────────── + +@dataclass(frozen=True) +class ServeSecurity: + policy: CommandPolicy = field(default_factory=CommandPolicy.unrestricted) + key_policies: dict[str, CommandPolicy] = field(default_factory=dict) + key_names: dict[str, str] = field(default_factory=dict) + rate_limiter: RateLimiter | None = None + + def effective_policy(self, pubkey: str | None) -> CommandPolicy: + """Per-key override if one exists for this pubkey, else the server default.""" + if pubkey and pubkey in self.key_policies: + return self.key_policies[pubkey] + return self.policy + + def label_for(self, pubkey: str | None) -> str | None: + """Audit label for log lines: `` …`` or just the short pubkey.""" + if not pubkey: + return None + short = f"{pubkey[:8]}…" + name = self.key_names.get(pubkey, "") + return f"{name} {short}".strip() if name else short diff --git a/browser_cli/version_manager.py b/browser_cli/version_manager.py index 20b7531..fea5069 100644 --- a/browser_cli/version_manager.py +++ b/browser_cli/version_manager.py @@ -1,17 +1,33 @@ -from importlib.metadata import version as _pkg_version +from importlib.metadata import PackageNotFoundError, version as _pkg_version +from pathlib import Path from browser_cli.constants import MAX_MSG_BYTES, PROTOCOL_MIN_CLIENT, PYPI_PACKAGE_NAME def parse_version(v: str) -> tuple[int, ...]: - try: - return tuple(int(x) for x in v.lstrip("v").split(".")) - except ValueError: - return (0,) + try: + return tuple(int(x) for x in v.lstrip("v").split(".")) + except ValueError: + return (0,) def get_installed_version() -> str: - try: - return _pkg_version(PYPI_PACKAGE_NAME) - except Exception: - return "0.0.0" + try: + return _pkg_version(PYPI_PACKAGE_NAME) + except Exception: + return "0.0.0" + +def project_version() -> str: + pyproject_path = Path(__file__).resolve().parent.parent / "pyproject.toml" + try: + content = pyproject_path.read_text(encoding="utf-8") + for line in content.splitlines(): + if line.startswith("version = "): + return line.split('"')[1] + except OSError: + pass + + try: + return _pkg_version(PYPI_PACKAGE_NAME) + except PackageNotFoundError: + return "unknown" USER_AGENT = f"browser-cli/{get_installed_version()}" diff --git a/extension/manifest.json b/extension/manifest.json index f13d26f..4ed41b7 100644 --- a/extension/manifest.json +++ b/extension/manifest.json @@ -1,7 +1,7 @@ { "manifest_version": 3, "name": "browser-cli", - "version": "0.15.6", + "version": "0.16.0", "description": "Control your browser from the terminal or Python SDK", "browser_specific_settings": { "gecko": { diff --git a/extension/src/classes/CommandRegistry.ts b/extension/src/classes/CommandRegistry.ts index d85a62e..a9aabf8 100644 --- a/extension/src/classes/CommandRegistry.ts +++ b/extension/src/classes/CommandRegistry.ts @@ -17,17 +17,33 @@ function isCommandSpec(entry: CommandEntry): entry is CommandSpec { return typeof entry !== "function"; } +// Fill each page up to a byte budget kept safely under the 1MB native-messaging +// limit (extension → host). This makes paging adaptive: many small items pack +// into one page, while a few oversized items (e.g. data-URI favicons) split +// across pages instead of overflowing the limit. +const PAGE_BYTE_BUDGET = 768 * 1024; + export function makePagedData(items: Serializable[], page: PageRequest) { const total = items.length; const offset = Math.max(0, Number(page.offset) || 0); const requestedLimit = Math.max(1, Number(page.limit) || 100); - const limit = Math.min(requestedLimit, 1000); - const end = Math.min(offset + limit, total); + const maxCount = Math.min(requestedLimit, 1000); + + let end = offset; + let bytes = 0; + while (end < total && end - offset < maxCount) { + const itemBytes = JSON.stringify(items[end]).length + 1; // +1 ≈ separator + // Always include at least one item so a single oversized item still advances. + if (end > offset && bytes + itemBytes > PAGE_BYTE_BUDGET) break; + bytes += itemBytes; + end++; + } + return { __browserCliPage: true, items: items.slice(offset, end), offset, - limit, + limit: maxCount, total, nextOffset: end < total ? end : null, }; diff --git a/extension/test/paging.test.ts b/extension/test/paging.test.ts new file mode 100644 index 0000000..3f5cc63 --- /dev/null +++ b/extension/test/paging.test.ts @@ -0,0 +1,41 @@ +// @ts-nocheck +import test from 'node:test'; +import assert from 'node:assert/strict'; +import { makePagedData } from '../src/classes/CommandRegistry'; + +test('makePagedData packs many small items into one page up to the count cap', () => { + const items = Array.from({ length: 1500 }, (_, i) => ({ id: i, url: 'chrome://newtab/' })); + const page = makePagedData(items, { offset: 0, limit: 1000 }); + + assert.equal(page.items.length, 1000); // count cap, well under the byte budget + assert.equal(page.nextOffset, 1000); + assert.equal(page.total, 1500); +}); + +test('makePagedData splits on the byte budget for oversized items', () => { + // Each item ~200KB; only a few fit under the 768KB budget per page. + const big = 'x'.repeat(200 * 1024); + const items = Array.from({ length: 10 }, (_, i) => ({ id: i, favIconUrl: big })); + const page = makePagedData(items, { offset: 0, limit: 1000 }); + + assert.ok(page.items.length >= 1 && page.items.length < 10, `expected partial page, got ${page.items.length}`); + assert.equal(page.nextOffset, page.items.length); +}); + +test('makePagedData always advances by at least one item', () => { + // A single item larger than the whole budget must still be returned alone. + const huge = 'x'.repeat(2 * 1024 * 1024); + const items = [{ id: 0, favIconUrl: huge }, { id: 1 }]; + const page = makePagedData(items, { offset: 0, limit: 1000 }); + + assert.equal(page.items.length, 1); + assert.equal(page.nextOffset, 1); +}); + +test('makePagedData reports null nextOffset on the final page', () => { + const items = [{ id: 0 }, { id: 1 }, { id: 2 }]; + const page = makePagedData(items, { offset: 2, limit: 1000 }); + + assert.equal(page.items.length, 1); + assert.equal(page.nextOffset, null); +}); diff --git a/pyproject.toml b/pyproject.toml index 7d322f4..df0adca 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,6 +1,6 @@ [project] name = "real-browser-cli" -version = "0.15.6" +version = "0.16.0" description = "Control your real running browser from the terminal or Python SDK" readme = "README.md" license = { file = "LICENSE" } diff --git a/tests/conftest.py b/tests/conftest.py index db42767..b3f48b4 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -8,45 +8,51 @@ They are automatically skipped if the native host socket is not reachable. import time import pytest from browser_cli.client import send_command, BrowserNotConnected +from browser_cli.remote import pool as _remote_pool TEST_BROWSER_PROFILE = "testing" +@pytest.fixture(autouse=True) +def _clear_remote_pool(): + """Close any pooled remote connections between tests so a connection opened + against one test's throwaway server can't leak into the next.""" + yield + _remote_pool.close_all() @pytest.fixture(scope="session") def browser(): - """Returns a connected send_command callable for the testing profile, or skips the test.""" - try: - send_command("tabs.list", profile=TEST_BROWSER_PROFILE) - except (BrowserNotConnected, RuntimeError) as e: - pytest.skip( - f"Browser 'testing' not connected — start Brave/Chrome with the extension loaded for that profile ({e})" - ) + """Returns a connected send_command callable for the testing profile, or skips the test.""" + try: + send_command("tabs.list", profile=TEST_BROWSER_PROFILE) + except (BrowserNotConnected, RuntimeError) as e: + pytest.skip( + f"Browser 'testing' not connected — start Brave/Chrome with the extension loaded for that profile ({e})" + ) - def _browser(command, args=None): - return send_command(command, args, profile=TEST_BROWSER_PROFILE) - - return _browser + def _browser(command, args=None): + return send_command(command, args, profile=TEST_BROWSER_PROFILE) + return _browser @pytest.fixture() def http_tab(browser): - """Opens a dedicated http/https tab for the current test and returns its tab info.""" - created = browser("navigate.open", {"url": "https://example.com", "background": True}) - tab_id = created["id"] + """Opens a dedicated http/https tab for the current test and returns its tab info.""" + created = browser("navigate.open", {"url": "https://example.com", "background": True}) + tab_id = created["id"] - tab = None + tab = None + try: + for _ in range(30): + tabs = browser("tabs.list") + tab = next((t for t in tabs if t.get("id") == tab_id and t.get("url", "").startswith("http")), None) + if tab is not None: + break + time.sleep(0.1) + if tab is None: + pytest.skip("Dedicated http/https test tab did not finish loading") + yield tab + finally: try: - for _ in range(30): - tabs = browser("tabs.list") - tab = next((t for t in tabs if t.get("id") == tab_id and t.get("url", "").startswith("http")), None) - if tab is not None: - break - time.sleep(0.1) - if tab is None: - pytest.skip("Dedicated http/https test tab did not finish loading") - yield tab - finally: - try: - browser("tabs.close", {"tabId": tab_id}) - except Exception: - pass + browser("tabs.close", {"tabId": tab_id}) + except Exception: + pass diff --git a/tests/test_api.py b/tests/test_api.py index 9cefafc..74d0ed2 100644 --- a/tests/test_api.py +++ b/tests/test_api.py @@ -32,6 +32,12 @@ GROUP_DATA = { "tabCount": 3, } +def tab_close_args(tab_id: int): + return {"tabId": tab_id, "tabIds": None, "inactive": False, "duplicates": False, "gentleMode": "auto"} + +def group_close_args(group_id: int): + return {"groupId": group_id, "gentleMode": "auto"} + @pytest.fixture() def mock_send(): """Patch send_command for the duration of one test. @@ -454,7 +460,7 @@ class TestTabs: assert mock_send.call_args_list == [ call("tabs.list", {}, profile="default"), call("tabs.list", {}, profile="work"), - call("tabs.close", {"tabId": 11}, profile="work", remote=None, key=None), + call("tabs.close", tab_close_args(11), profile="work", remote=None, key=None), ] def test_tabs_list_remote_uses_only_requested_remote_and_binds_actions(self, mock_send): @@ -473,7 +479,7 @@ class TestTabs: assert [tab.browser for tab in tabs] == ["work"] assert mock_send.call_args_list == [ call("tabs.list", {}, profile="work", remote="host:8765", key=None), - call("tabs.close", {"tabId": 10}, profile="work", remote="host:8765", key=None), + call("tabs.close", tab_close_args(10), profile="work", remote="host:8765", key=None), ] def test_tabs_list_remote_bound_actions_preserve_key(self, mock_send): @@ -488,7 +494,7 @@ class TestTabs: assert mock_send.call_args_list == [ call("tabs.list", {}, profile="work", remote="browser-host.example", key="agent"), - call("tabs.close", {"tabId": 10}, profile="work", remote="browser-host.example", key="agent"), + call("tabs.close", tab_close_args(10), profile="work", remote="browser-host.example", key="agent"), ] def test_tabs_list_browser_host_alias_fans_out_to_remote_targets(self, mock_send): @@ -510,7 +516,7 @@ class TestTabs: assert mock_send.call_args_list == [ call("tabs.list", {}, profile="main", remote="browser-host.example:8765", key="agent"), call("tabs.list", {}, profile="work", remote="browser-host.example:8765", key="agent"), - call("tabs.close", {"tabId": 11}, profile="work", remote="browser-host.example:8765", key="agent"), + call("tabs.close", tab_close_args(11), profile="work", remote="browser-host.example:8765", key="agent"), ] def test_tabs_active_returns_active_tab(self, b, mock_send): @@ -690,7 +696,7 @@ class TestGroups: assert mock_send.call_args_list == [ call("group.list", {}, profile="default"), call("group.list", {}, profile="work"), - call("group.close", {"groupId": 99}, profile="work", remote=None, key=None), + call("group.close", group_close_args(99), profile="work", remote=None, key=None), ] def test_group_list_remote_uses_only_requested_remote_and_binds_actions(self, mock_send): @@ -709,7 +715,7 @@ class TestGroups: assert [group.browser for group in groups] == ["work"] assert mock_send.call_args_list == [ call("group.list", {}, profile="work", remote="host:8765", key=None), - call("group.close", {"groupId": 42}, profile="work", remote="host:8765", key=None), + call("group.close", group_close_args(42), profile="work", remote="host:8765", key=None), ] def test_group_list_remote_bound_actions_preserve_key(self, mock_send): @@ -724,7 +730,7 @@ class TestGroups: assert mock_send.call_args_list == [ call("group.list", {}, profile="work", remote="browser-host.example", key="agent"), - call("group.close", {"groupId": 42}, profile="work", remote="browser-host.example", key="agent"), + call("group.close", group_close_args(42), profile="work", remote="browser-host.example", key="agent"), ] def test_group_count_multi_browser_returns_browser_counts(self, b, mock_send): @@ -954,7 +960,7 @@ class TestTabModel: def test_close(self, tab, mock_send): tab.close() - mock_send.assert_called_once_with("tabs.close", {"tabId": 10}, profile=None, remote=None, key=None) + mock_send.assert_called_once_with("tabs.close", tab_close_args(10), profile=None, remote=None, key=None) def test_activate(self, tab, mock_send): tab.activate() @@ -1043,7 +1049,7 @@ class TestGroupModel: def test_close(self, group, mock_send): group.close() - mock_send.assert_called_once_with("group.close", {"groupId": 42}, profile=None, remote=None, key=None) + mock_send.assert_called_once_with("group.close", group_close_args(42), profile=None, remote=None, key=None) def test_tabs(self, group, mock_send): mock_send.return_value = [TAB_DATA] @@ -1115,7 +1121,7 @@ class TestSDKDecorators: remote=None, key=None, ), - call("tabs.close", {"tabId": 123}, profile=None, remote=None, key=None), + call("tabs.close", tab_close_args(123), profile=None, remote=None, key=None), ] def test_wait_for_selector_runs_before_function_and_can_inject_result(self, b, mock_send): diff --git a/tests/test_cli.py b/tests/test_cli.py index 8cfbe24..24dfc8f 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -28,8 +28,8 @@ def test_long_version_option(): assert result.output.strip() == _expected_version() def test_project_version_falls_back_to_installed_package_metadata(): - with patch("browser_cli.cli.Path.read_text", side_effect=OSError), patch( - "browser_cli.cli.package_version", return_value="9.9.9" + with patch("browser_cli.version_manager.Path.read_text", side_effect=OSError), patch( + "browser_cli.version_manager._pkg_version", return_value="9.9.9" ): assert _project_version() == "9.9.9" @@ -114,8 +114,8 @@ def test_install_writes_testing_and_webstore_allowed_origins(tmp_path): ], } ] - assert "Testing extension ID" in result.output - assert "Chrome Web Store extension ID" in result.output + assert "chromewebstore.google.com" in result.output + assert "Add to Brave" in result.output def test_install_writes_firefox_allowed_extensions(tmp_path): manifests = [] @@ -139,12 +139,34 @@ def test_install_writes_firefox_allowed_extensions(tmp_path): "allowed_extensions": ["browser-cli@yiprawr.dev"], } ] + assert "addons.mozilla.org/firefox/addon/browser-cli" in result.output + assert "Add to Firefox" in result.output + +def test_install_dev_flag_prints_unpacked_instructions(tmp_path): + with patch("browser_cli.commands.install.native_host_exe", return_value=tmp_path / "browser-cli-native-host"), patch( + "browser_cli.commands.install.write_native_host_exe" + ), patch("browser_cli.commands.install._install_manifest", return_value=[tmp_path / "com.browsercli.host.json"]): + result = CliRunner().invoke(main, ["install", "brave", "--dev"]) + + assert result.exit_code == 0 + assert "Load unpacked" in result.output + assert "Developer mode" in result.output + assert "Testing extension ID" in result.output + assert "Chrome Web Store extension ID" in result.output + assert "chromewebstore.google.com" not in result.output # store path is the non-dev default + +def test_install_dev_flag_prints_firefox_unpacked_instructions(tmp_path): + with patch("browser_cli.commands.install.native_host_exe", return_value=tmp_path / "browser-cli-native-host"), patch( + "browser_cli.commands.install.write_native_host_exe" + ), patch("browser_cli.commands.install._install_manifest", return_value=[tmp_path / "com.browsercli.host.json"]): + result = CliRunner().invoke(main, ["install", "firefox", "--dev"]) + + assert result.exit_code == 0 assert "about:debugging#/runtime/this-firefox" in result.output assert "npm run package:extension:firefox" in result.output output_unwrapped = result.output.replace("\n", "") assert "dist/extension-package-firefox/manifest.json" in output_unwrapped assert "Do not select extension/manifest.json" in output_unwrapped - assert "Firefox extension ID" in result.output def test_install_windows_registers_native_host(tmp_path): writes = [] @@ -205,7 +227,7 @@ def test_write_native_host_exe_windows(tmp_path): def test_clients_exits_cleanly_when_registry_is_missing(): with patch("browser_cli.commands.clients.REGISTRY_PATH", Path("/nonexistent/browser-cli-registry.json")), patch( - "browser_cli.commands.clients.active_browser_targets", return_value=[] + "browser_cli.client.core.active_browser_targets", return_value=[] ): result = CliRunner().invoke(main, ["clients"]) @@ -239,8 +261,8 @@ def test_clients_without_remote_shows_saved_remotes_without_pq_warning(tmp_path) return [remote_target] with patch("browser_cli.commands.clients.REGISTRY_PATH", registry_path), patch( - "browser_cli.commands.clients.send_command", side_effect=fake_send_command - ), patch("browser_cli.commands.clients.active_browser_targets", side_effect=fake_active_browser_targets) as active_targets: + "browser_cli.client.core.send_command", side_effect=fake_send_command + ), patch("browser_cli.client.core.active_browser_targets", side_effect=fake_active_browser_targets) as active_targets: result = CliRunner().invoke(main, ["clients"]) assert result.exit_code == 0 @@ -263,8 +285,8 @@ def test_clients_reads_registry_with_trailing_garbage(tmp_path): return [{"profile": "main", "name": "Chrome", "version": "1", "extensionVersion": "0.8.2"}] with patch("browser_cli.commands.clients.REGISTRY_PATH", registry_path), patch( - "browser_cli.commands.clients.send_command", side_effect=fake_send_command - ), patch("browser_cli.commands.clients.active_browser_targets", return_value=[]): + "browser_cli.client.core.send_command", side_effect=fake_send_command + ), patch("browser_cli.client.core.active_browser_targets", return_value=[]): result = CliRunner().invoke(main, ["clients"]) assert result.exit_code == 0 @@ -280,7 +302,7 @@ def test_clients_remote_uses_remote_endpoint_without_local_registry(): with patch.dict(os.environ, {}, clear=True), patch( "browser_cli.commands.clients.REGISTRY_PATH", Path("/nonexistent/browser-cli-registry.json") - ), patch("browser_cli.commands.clients.send_command", side_effect=fake_send_command) as send_command: + ), patch("browser_cli.client.core.send_command", side_effect=fake_send_command) as send_command: result = CliRunner().invoke(main, ["--remote", "127.0.0.1:8765", "clients"]) assert result.exit_code == 0 @@ -290,7 +312,7 @@ def test_clients_remote_uses_remote_endpoint_without_local_registry(): assert "2.3.4" in result.output def test_clients_remote_respects_global_browser_route(): - with patch.dict(os.environ, {}, clear=True), patch("browser_cli.commands.clients.send_command", return_value=[]) as send_command: + with patch.dict(os.environ, {}, clear=True), patch("browser_cli.client.core.send_command", return_value=[]) as send_command: result = CliRunner().invoke(main, ["--remote", "127.0.0.1:8765", "--browser", "work", "clients"]) assert result.exit_code == 1 @@ -316,10 +338,10 @@ def test_clients_browser_alias_resolves_to_remote(): return [{"name": "Chrome", "version": "147.0.0.0", "extensionVersion": "0.8.5"}] with patch.dict(os.environ, {}, clear=True), patch( - "browser_cli.commands.clients.remote_target_for_alias", return_value=resolved_target + "browser_cli.client.core.remote_target_for_alias", return_value=resolved_target ), patch( - "browser_cli.commands.clients.remote_browser_targets", return_value=all_remote_targets - ), patch("browser_cli.commands.clients.send_command", side_effect=fake_send_command) as send_command: + "browser_cli.client.core.remote_browser_targets", return_value=all_remote_targets + ), patch("browser_cli.client.core.send_command", side_effect=fake_send_command) as send_command: result = CliRunner().invoke(main, ["--browser", "browser-host.example", "clients"]) assert result.exit_code == 0 @@ -346,8 +368,8 @@ def test_clients_shows_named_profile_and_uses_socket_uuid_for_default(tmp_path): return responses[profile] with patch("browser_cli.commands.clients.REGISTRY_PATH", registry_path), patch( - "browser_cli.commands.clients.send_command", side_effect=fake_send_command - ), patch("browser_cli.commands.clients.active_browser_targets", return_value=[]): + "browser_cli.client.core.send_command", side_effect=fake_send_command + ), patch("browser_cli.client.core.active_browser_targets", return_value=[]): result = CliRunner().invoke(main, ["clients"]) assert result.exit_code == 0 diff --git a/tests/test_client.py b/tests/test_client.py index 096bb16..b2271b8 100644 --- a/tests/test_client.py +++ b/tests/test_client.py @@ -7,553 +7,678 @@ from pathlib import Path import pytest from browser_cli.client import ( - BrowserNotConnected, - BrowserTarget, - _looks_like_domain, - _normalize_endpoint, - _resolve_connect_endpoint, - active_browser_targets, - display_browser_name, - send_command, - send_command_async, - remote_target_for_alias, + BrowserNotConnected, + BrowserTarget, + _looks_like_domain, + _normalize_endpoint, + _resolve_connect_endpoint, + active_browser_targets, + display_browser_name, + send_command, + send_command_async, + remote_target_for_alias, ) from browser_cli.client.targets import resolve_socket from browser_cli.platform import endpoint_for_alias from browser_cli.remote.registry import key_for_remote def _listening_unix_socket(path: Path) -> socket.socket: - sock = socket.socket(socket.AF_UNIX, socket.SOCK_STREAM) - sock.bind(str(path)) - sock.listen(1) - return sock + sock = socket.socket(socket.AF_UNIX, socket.SOCK_STREAM) + sock.bind(str(path)) + sock.listen(1) + return sock def test_resolve_socket_raises_when_registry_missing(monkeypatch): - monkeypatch.delenv("BROWSER_CLI_PROFILE", raising=False) - monkeypatch.setattr("browser_cli.client.targets.REGISTRY_PATH", Path("/nonexistent/browser-cli-registry.json")) + monkeypatch.delenv("BROWSER_CLI_PROFILE", raising=False) + monkeypatch.setattr("browser_cli.client.targets.REGISTRY_PATH", Path("/nonexistent/browser-cli-registry.json")) - with pytest.raises(BrowserNotConnected, match="Cannot resolve a browser socket automatically"): - resolve_socket() + with pytest.raises(BrowserNotConnected, match="Cannot resolve a browser socket automatically"): + resolve_socket() def test_resolve_socket_uses_only_active_registry_entry(monkeypatch, tmp_path): - monkeypatch.delenv("BROWSER_CLI_PROFILE", raising=False) + monkeypatch.delenv("BROWSER_CLI_PROFILE", raising=False) - socket_path = tmp_path / "browser.sock" - listener = _listening_unix_socket(socket_path) - registry_path = tmp_path / "registry.json" - registry_path.write_text(json.dumps({"abc-uuid": str(socket_path)})) + socket_path = tmp_path / "browser.sock" + listener = _listening_unix_socket(socket_path) + registry_path = tmp_path / "registry.json" + registry_path.write_text(json.dumps({"abc-uuid": str(socket_path)})) - monkeypatch.setattr("browser_cli.client.targets.REGISTRY_PATH", registry_path) + monkeypatch.setattr("browser_cli.client.targets.REGISTRY_PATH", registry_path) - try: - assert resolve_socket() == str(socket_path) - finally: - listener.close() + try: + assert resolve_socket() == str(socket_path) + finally: + listener.close() def test_resolve_socket_raises_when_multiple_active_entries(monkeypatch, tmp_path): - monkeypatch.delenv("BROWSER_CLI_PROFILE", raising=False) + monkeypatch.delenv("BROWSER_CLI_PROFILE", raising=False) - first_socket = tmp_path / "one.sock" - second_socket = tmp_path / "two.sock" - first_listener = _listening_unix_socket(first_socket) - second_listener = _listening_unix_socket(second_socket) - registry_path = tmp_path / "registry.json" - registry_path.write_text(json.dumps({"uuid-1": str(first_socket), "uuid-2": str(second_socket)})) + first_socket = tmp_path / "one.sock" + second_socket = tmp_path / "two.sock" + first_listener = _listening_unix_socket(first_socket) + second_listener = _listening_unix_socket(second_socket) + registry_path = tmp_path / "registry.json" + registry_path.write_text(json.dumps({"uuid-1": str(first_socket), "uuid-2": str(second_socket)})) - monkeypatch.setattr("browser_cli.client.targets.REGISTRY_PATH", registry_path) + monkeypatch.setattr("browser_cli.client.targets.REGISTRY_PATH", registry_path) - try: - with pytest.raises(BrowserNotConnected, match="Multiple browser instances are active: uuid-1, uuid-2"): - resolve_socket() - finally: - first_listener.close() - second_listener.close() + try: + with pytest.raises(BrowserNotConnected, match="Multiple browser instances are active: uuid-1, uuid-2"): + resolve_socket() + finally: + first_listener.close() + second_listener.close() def test_display_browser_name_uses_uuid_stem_for_default(): - assert display_browser_name("default", "/tmp/.browser_cli/550e8400-e29b-41d4-a716-446655440000.sock") == ( - "550e8400-e29b-41d4-a716-446655440000" - ) + assert display_browser_name("default", "/tmp/.browser_cli/550e8400-e29b-41d4-a716-446655440000.sock") == ( + "550e8400-e29b-41d4-a716-446655440000" + ) def test_resolve_socket_uses_platform_endpoint_for_explicit_alias(monkeypatch): - monkeypatch.delenv("BROWSER_CLI_PROFILE", raising=False) - monkeypatch.setattr("browser_cli.client.targets.REGISTRY_PATH", Path("/nonexistent/browser-cli-registry.json")) + monkeypatch.delenv("BROWSER_CLI_PROFILE", raising=False) + monkeypatch.setattr("browser_cli.client.targets.REGISTRY_PATH", Path("/nonexistent/browser-cli-registry.json")) - assert resolve_socket("work") == endpoint_for_alias("work") + assert resolve_socket("work") == endpoint_for_alias("work") def test_active_browser_targets_filters_stale_entries(monkeypatch, tmp_path): - active_socket = tmp_path / "work.sock" - listener = _listening_unix_socket(active_socket) - stale_socket = tmp_path / "stale.sock" - stale_listener = _listening_unix_socket(stale_socket) - stale_listener.close() - registry_path = tmp_path / "registry.json" - registry_path.write_text(json.dumps({"work": str(active_socket), "default": str(stale_socket)})) + active_socket = tmp_path / "work.sock" + listener = _listening_unix_socket(active_socket) + stale_socket = tmp_path / "stale.sock" + stale_listener = _listening_unix_socket(stale_socket) + stale_listener.close() + registry_path = tmp_path / "registry.json" + registry_path.write_text(json.dumps({"work": str(active_socket), "default": str(stale_socket)})) - monkeypatch.setattr("browser_cli.client.targets.REGISTRY_PATH", registry_path) + monkeypatch.setattr("browser_cli.client.targets.REGISTRY_PATH", registry_path) - try: - targets = active_browser_targets(include_remotes=False) - finally: - listener.close() + try: + targets = active_browser_targets(include_remotes=False) + finally: + listener.close() - assert len(targets) == 1 - assert targets[0].profile == "work" - assert targets[0].display_name == "work" + assert len(targets) == 1 + assert targets[0].profile == "work" + assert targets[0].display_name == "work" def test_active_browser_targets_keeps_windows_registry_entries(monkeypatch, tmp_path): - registry_path = tmp_path / "registry.json" - registry_path.write_text(json.dumps({"work": r"\\.\pipe\browser-cli-work"})) + registry_path = tmp_path / "registry.json" + registry_path.write_text(json.dumps({"work": r"\\.\pipe\browser-cli-work"})) - monkeypatch.setattr("browser_cli.client.targets.REGISTRY_PATH", registry_path) - monkeypatch.setattr("browser_cli.client.targets.is_windows", lambda: True) + monkeypatch.setattr("browser_cli.client.targets.REGISTRY_PATH", registry_path) + monkeypatch.setattr("browser_cli.client.targets.is_windows", lambda: True) - targets = active_browser_targets(include_remotes=False) + targets = active_browser_targets(include_remotes=False) - assert len(targets) == 1 - assert targets[0].socket_path == r"\\.\pipe\browser-cli-work" + assert len(targets) == 1 + assert targets[0].socket_path == r"\\.\pipe\browser-cli-work" def test_send_command_auto_routes_single_remote_target(monkeypatch): - monkeypatch.delenv("BROWSER_CLI_PROFILE", raising=False) - monkeypatch.delenv("BROWSER_CLI_REMOTE", raising=False) - sent = {} + monkeypatch.delenv("BROWSER_CLI_PROFILE", raising=False) + monkeypatch.delenv("BROWSER_CLI_REMOTE", raising=False) + sent = {} - monkeypatch.setattr( - "browser_cli.client.core.remote_browser_targets", - lambda endpoint, key=None: [BrowserTarget("work", "host:work", "", remote=endpoint)], - ) + monkeypatch.setattr( + "browser_cli.client.core.remote_browser_targets", + lambda endpoint, key=None: [BrowserTarget("work", "host:work", "", remote=endpoint)], + ) - def fake_send_remote(endpoint, msg, private_key=None): - sent.update(msg) - return json.dumps({"success": True, "data": "ok"}).encode("utf-8") + def fake_send_remote(endpoint, msg, private_key=None): + sent.update(msg) + return json.dumps({"success": True, "data": "ok"}).encode("utf-8") - monkeypatch.setattr("browser_cli.client.core._send_remote", fake_send_remote) + monkeypatch.setattr("browser_cli.client.core._send_remote", fake_send_remote) - assert send_command("tabs.list", remote="host:8765", key=None) == "ok" - assert sent["_route"] == "work" - assert "token" not in sent + assert send_command("tabs.list", remote="host:8765", key=None) == "ok" + assert sent["_route"] == "work" + assert "token" not in sent def test_send_command_uses_env_profile_for_local_transport(monkeypatch): - monkeypatch.delenv("BROWSER_CLI_REMOTE", raising=False) - monkeypatch.setenv("BROWSER_CLI_PROFILE", "work") - seen = {} + monkeypatch.delenv("BROWSER_CLI_REMOTE", raising=False) + monkeypatch.setenv("BROWSER_CLI_PROFILE", "work") + seen = {} - def fake_send_local(profile, payload, resolve_socket): - seen["profile"] = profile - seen["payload"] = json.loads(payload) - return json.dumps({"success": True, "data": "ok"}).encode("utf-8") + def fake_send_local(profile, payload, resolve_socket): + seen["profile"] = profile + seen["payload"] = json.loads(payload) + return json.dumps({"success": True, "data": "ok"}).encode("utf-8") - monkeypatch.setattr("browser_cli.client.targets.is_active_local_profile", lambda profile: True) - monkeypatch.setattr("browser_cli.client.core.local_transport.send_local_sync", fake_send_local) + monkeypatch.setattr("browser_cli.client.targets.is_active_local_profile", lambda profile: True) + monkeypatch.setattr("browser_cli.client.core.local_transport.send_local_sync", fake_send_local) - assert send_command("tabs.list") == "ok" - assert seen["profile"] == "work" - assert seen["payload"]["command"] == "tabs.list" + assert send_command("tabs.list") == "ok" + assert seen["profile"] == "work" + assert seen["payload"]["command"] == "tabs.list" async def _async_local_profile_result(monkeypatch): - seen = {} + seen = {} - async def fake_send_local(profile, payload, resolve_socket): - seen["profile"] = profile - return json.dumps({"success": True, "data": "ok"}).encode("utf-8") + async def fake_send_local(profile, payload, resolve_socket): + seen["profile"] = profile + return json.dumps({"success": True, "data": "ok"}).encode("utf-8") - monkeypatch.setattr("browser_cli.client.targets.is_active_local_profile", lambda profile: True) - monkeypatch.setattr("browser_cli.client.core.local_transport.send_local_async", fake_send_local) - result = await send_command_async("tabs.list") - return result, seen + monkeypatch.setattr("browser_cli.client.targets.is_active_local_profile", lambda profile: True) + monkeypatch.setattr("browser_cli.client.core.local_transport.send_local_async", fake_send_local) + result = await send_command_async("tabs.list") + return result, seen def test_send_command_async_uses_env_profile_for_local_transport(monkeypatch): - monkeypatch.delenv("BROWSER_CLI_REMOTE", raising=False) - monkeypatch.setenv("BROWSER_CLI_PROFILE", "work") + monkeypatch.delenv("BROWSER_CLI_REMOTE", raising=False) + monkeypatch.setenv("BROWSER_CLI_PROFILE", "work") - result, seen = asyncio.run(_async_local_profile_result(monkeypatch)) + result, seen = asyncio.run(_async_local_profile_result(monkeypatch)) - assert result == "ok" - assert seen["profile"] == "work" + assert result == "ok" + assert seen["profile"] == "work" def test_send_command_prefers_active_local_profile_over_saved_remote_alias(monkeypatch, tmp_path): - monkeypatch.delenv("BROWSER_CLI_REMOTE", raising=False) - monkeypatch.delenv("BROWSER_CLI_PROFILE", raising=False) - socket_path = tmp_path / "work.sock" - socket_path.write_text("") - registry_path = tmp_path / "registry.json" - registry_path.write_text(json.dumps({"work": str(socket_path)}), encoding="utf-8") - monkeypatch.setattr("browser_cli.client.targets.REGISTRY_PATH", registry_path) - monkeypatch.setattr( - "browser_cli.client.remote_target_for_alias", - lambda alias: pytest.fail("active local profile must not trigger remote alias discovery"), - ) - monkeypatch.setattr("browser_cli.client.targets.is_active_local_profile", lambda profile: True) + monkeypatch.delenv("BROWSER_CLI_REMOTE", raising=False) + monkeypatch.delenv("BROWSER_CLI_PROFILE", raising=False) + socket_path = tmp_path / "work.sock" + socket_path.write_text("") + registry_path = tmp_path / "registry.json" + registry_path.write_text(json.dumps({"work": str(socket_path)}), encoding="utf-8") + monkeypatch.setattr("browser_cli.client.targets.REGISTRY_PATH", registry_path) + monkeypatch.setattr( + "browser_cli.client.remote_target_for_alias", + lambda alias: pytest.fail("active local profile must not trigger remote alias discovery"), + ) + monkeypatch.setattr("browser_cli.client.targets.is_active_local_profile", lambda profile: True) - payload = json.dumps({"success": True, "data": "local-ok"}).encode("utf-8") - framed = len(payload).to_bytes(4, "little") + payload + payload = json.dumps({"success": True, "data": "local-ok"}).encode("utf-8") + framed = len(payload).to_bytes(4, "little") + payload - class FakeSocket: - def __init__(self, *args, **kwargs): - self.sent = b"" - self._response = bytearray(framed) - self.connected_to = None + class FakeSocket: + def __init__(self, *args, **kwargs): + self.sent = b"" + self._response = bytearray(framed) + self.connected_to = None - def __enter__(self): - return self + def __enter__(self): + return self - def __exit__(self, *exc): - return False + def __exit__(self, *exc): + return False - def connect(self, path): - self.connected_to = path + def connect(self, path): + self.connected_to = path - def sendall(self, data): - self.sent += data + def sendall(self, data): + self.sent += data - def recv(self, n): - chunk = bytes(self._response[:n]) - del self._response[:n] - return chunk + def recv(self, n): + chunk = bytes(self._response[:n]) + del self._response[:n] + return chunk - monkeypatch.setattr("browser_cli.client.targets.socket.socket", lambda *args, **kwargs: FakeSocket()) + monkeypatch.setattr("browser_cli.client.targets.socket.socket", lambda *args, **kwargs: FakeSocket()) - assert send_command("tabs.list", profile="work") == "local-ok" + assert send_command("tabs.list", profile="work") == "local-ok" def test_send_command_resolves_browser_alias_to_remote_target(monkeypatch): - monkeypatch.delenv("BROWSER_CLI_REMOTE", raising=False) - monkeypatch.setenv("BROWSER_CLI_PROFILE", "host:work") - sent = {} + monkeypatch.delenv("BROWSER_CLI_REMOTE", raising=False) + monkeypatch.setenv("BROWSER_CLI_PROFILE", "host:work") + sent = {} - monkeypatch.setattr( - "browser_cli.client.core._remote_browser_targets", - lambda: [BrowserTarget("work", "host:work", "", remote="host:8765")], - ) + monkeypatch.setattr( + "browser_cli.client.core._remote_browser_targets", + lambda: [BrowserTarget("work", "host:work", "", remote="host:8765")], + ) - def fake_send_remote(endpoint, msg, private_key=None): - sent["endpoint"] = endpoint - sent.update(msg) - return json.dumps({"success": True, "data": []}).encode("utf-8") + def fake_send_remote(endpoint, msg, private_key=None): + sent["endpoint"] = endpoint + sent.update(msg) + return json.dumps({"success": True, "data": []}).encode("utf-8") - monkeypatch.setattr("browser_cli.client.core._send_remote", fake_send_remote) + monkeypatch.setattr("browser_cli.client.core._send_remote", fake_send_remote) - assert send_command("tabs.list") == [] - assert sent["endpoint"] == "host:8765" - assert sent["_route"] == "work" - assert "token" not in sent + assert send_command("tabs.list") == [] + assert sent["endpoint"] == "host:8765" + assert sent["_route"] == "work" + assert "token" not in sent def test_remote_target_for_alias_accepts_full_endpoint_profile(monkeypatch): - monkeypatch.setattr( - "browser_cli.client.core._remote_browser_targets", - lambda: [BrowserTarget("work", "host:work", "", remote="host:8765")], - ) + monkeypatch.setattr( + "browser_cli.client.core._remote_browser_targets", + lambda: [BrowserTarget("work", "host:work", "", remote="host:8765")], + ) - target = remote_target_for_alias("host:8765:work") + target = remote_target_for_alias("host:8765:work") - assert target is not None - assert target.profile == "work" - assert target.remote == "host:8765" + assert target is not None + assert target.profile == "work" + assert target.remote == "host:8765" def test_remote_target_for_alias_accepts_host_when_only_one_remote_target(monkeypatch): - remote_host = "browser-host.example" - remote_endpoint = f"{remote_host}:8765" - monkeypatch.setattr( - "browser_cli.client.core._remote_browser_targets", - lambda: [BrowserTarget("work", f"{remote_host}:work", "", remote=remote_endpoint)], - ) + remote_host = "browser-host.example" + remote_endpoint = f"{remote_host}:8765" + monkeypatch.setattr( + "browser_cli.client.core._remote_browser_targets", + lambda: [BrowserTarget("work", f"{remote_host}:work", "", remote=remote_endpoint)], + ) - target = remote_target_for_alias(remote_host) + target = remote_target_for_alias(remote_host) - assert target is not None - assert target.profile == "work" - assert target.remote == remote_endpoint + assert target is not None + assert target.profile == "work" + assert target.remote == remote_endpoint def test_send_command_resolves_host_alias_to_single_remote_target(monkeypatch): - remote_host = "browser-host.example" - remote_endpoint = f"{remote_host}:8765" - monkeypatch.delenv("BROWSER_CLI_REMOTE", raising=False) - monkeypatch.setenv("BROWSER_CLI_PROFILE", remote_host) - sent = {} + remote_host = "browser-host.example" + remote_endpoint = f"{remote_host}:8765" + monkeypatch.delenv("BROWSER_CLI_REMOTE", raising=False) + monkeypatch.setenv("BROWSER_CLI_PROFILE", remote_host) + sent = {} - monkeypatch.setattr( - "browser_cli.client.core._remote_browser_targets", - lambda: [BrowserTarget("work", f"{remote_host}:work", "", remote=remote_endpoint)], - ) + monkeypatch.setattr( + "browser_cli.client.core._remote_browser_targets", + lambda: [BrowserTarget("work", f"{remote_host}:work", "", remote=remote_endpoint)], + ) - def fake_send_remote(endpoint, msg, private_key=None): - sent["endpoint"] = endpoint - sent.update(msg) - return json.dumps({"success": True, "data": []}).encode("utf-8") + def fake_send_remote(endpoint, msg, private_key=None): + sent["endpoint"] = endpoint + sent.update(msg) + return json.dumps({"success": True, "data": []}).encode("utf-8") - monkeypatch.setattr("browser_cli.client.core._send_remote", fake_send_remote) + monkeypatch.setattr("browser_cli.client.core._send_remote", fake_send_remote) - assert send_command("tabs.list") == [] - assert sent["endpoint"] == remote_endpoint - assert sent["_route"] == "work" - assert "token" not in sent + assert send_command("tabs.list") == [] + assert sent["endpoint"] == remote_endpoint + assert sent["_route"] == "work" + assert "token" not in sent def test_remote_target_for_alias_reports_ambiguous_host_for_multiple_targets(monkeypatch): - monkeypatch.setattr( - "browser_cli.client.core._remote_browser_targets", - lambda: [ - BrowserTarget("main", "host:main", "", remote="host:8765"), - BrowserTarget("work", "host:work", "", remote="host:8765"), - ], - ) + monkeypatch.setattr( + "browser_cli.client.core._remote_browser_targets", + lambda: [ + BrowserTarget("main", "host:main", "", remote="host:8765"), + BrowserTarget("work", "host:work", "", remote="host:8765"), + ], + ) - with pytest.raises(BrowserNotConnected) as exc: - remote_target_for_alias("host") + with pytest.raises(BrowserNotConnected) as exc: + remote_target_for_alias("host") - msg = str(exc.value) - assert "Multiple remote browser instances are active on host: main, work" in msg - assert "browser-cli --remote host:8765 --browser main" in msg - assert "browser-cli --browser host:work" in msg + msg = str(exc.value) + assert "Multiple remote browser instances are active on host: main, work" in msg + assert "browser-cli --remote host:8765 --browser main" in msg + assert "browser-cli --browser host:work" in msg def test_send_command_reports_ambiguous_remote_browser_alias(monkeypatch): - monkeypatch.delenv("BROWSER_CLI_REMOTE", raising=False) - monkeypatch.setenv("BROWSER_CLI_PROFILE", "host") - monkeypatch.setattr("browser_cli.client.targets.REGISTRY_PATH", Path("/nonexistent/browser-cli-registry.json")) - monkeypatch.setattr("browser_cli.client.targets.is_windows", lambda: False) - monkeypatch.setattr( - "browser_cli.client.core._remote_browser_targets", - lambda: [ - BrowserTarget("main", "host:main", "", remote="host:8765"), - BrowserTarget("work", "host:work", "", remote="host:8765"), - ], - ) + monkeypatch.delenv("BROWSER_CLI_REMOTE", raising=False) + monkeypatch.setenv("BROWSER_CLI_PROFILE", "host") + monkeypatch.setattr("browser_cli.client.targets.REGISTRY_PATH", Path("/nonexistent/browser-cli-registry.json")) + monkeypatch.setattr("browser_cli.client.targets.is_windows", lambda: False) + monkeypatch.setattr( + "browser_cli.client.core._remote_browser_targets", + lambda: [ + BrowserTarget("main", "host:main", "", remote="host:8765"), + BrowserTarget("work", "host:work", "", remote="host:8765"), + ], + ) - with pytest.raises(BrowserNotConnected, match="Multiple remote browser instances are active on host: main, work"): - send_command("tabs.list") + with pytest.raises(BrowserNotConnected, match="Multiple remote browser instances are active on host: main, work"): + send_command("tabs.list") def test_send_command_requires_browser_for_multiple_remote_targets(monkeypatch): - monkeypatch.delenv("BROWSER_CLI_PROFILE", raising=False) - monkeypatch.setattr( - "browser_cli.client.core.remote_browser_targets", - lambda endpoint, key=None: [ - BrowserTarget("main", "host:main", "", remote=endpoint), - BrowserTarget("furry", "host:furry", "", remote=endpoint), - ], - ) + monkeypatch.delenv("BROWSER_CLI_PROFILE", raising=False) + monkeypatch.setattr( + "browser_cli.client.core.remote_browser_targets", + lambda endpoint, key=None: [ + BrowserTarget("main", "host:main", "", remote=endpoint), + BrowserTarget("furry", "host:furry", "", remote=endpoint), + ], + ) - with pytest.raises(BrowserNotConnected, match="Multiple remote browser instances are active: main, furry"): - send_command("tabs.list", remote="host:8765") + with pytest.raises(BrowserNotConnected, match="Multiple remote browser instances are active: main, furry"): + send_command("tabs.list", remote="host:8765") def test_active_browser_targets_includes_remote_targets(monkeypatch, tmp_path): - remotes_path = tmp_path / "remotes.json" - endpoint = "browser-host.example:8765" - remotes_path.write_text(json.dumps({endpoint: {}}), encoding="utf-8") - monkeypatch.setattr("browser_cli.client.targets.REGISTRY_PATH", tmp_path / "missing-registry.json") - monkeypatch.setattr("browser_cli.remote.registry.REMOTE_REGISTRY_PATH", remotes_path) + remotes_path = tmp_path / "remotes.json" + endpoint = "browser-host.example:8765" + remotes_path.write_text(json.dumps({endpoint: {}}), encoding="utf-8") + monkeypatch.setattr("browser_cli.client.targets.REGISTRY_PATH", tmp_path / "missing-registry.json") + monkeypatch.setattr("browser_cli.remote.registry.REMOTE_REGISTRY_PATH", remotes_path) - def fake_send_command(command, args=None, profile=None, remote=None, key=None): - assert command == "browser-cli.targets" - assert remote == endpoint - return [{"profile": "work", "displayName": "work", "browserName": "Firefox"}] + def fake_send_command(command, args=None, profile=None, remote=None, key=None): + assert command == "browser-cli.targets" + assert remote == endpoint + return [{"profile": "work", "displayName": "work", "browserName": "Firefox"}] - monkeypatch.setattr("browser_cli.client.core.send_command", fake_send_command) + monkeypatch.setattr("browser_cli.client.core.send_command", fake_send_command) - targets = active_browser_targets() + targets = active_browser_targets() - assert len(targets) == 1 - assert targets[0].profile == "work" - assert targets[0].display_name == "browser-host.example:work" - assert targets[0].remote == endpoint - assert targets[0].browser_name == "Firefox" - assert targets[0].display_group == "browser-host.example" + assert len(targets) == 1 + assert targets[0].profile == "work" + assert targets[0].display_name == "browser-host.example:work" + assert targets[0].remote == endpoint + assert targets[0].browser_name == "Firefox" + assert targets[0].display_group == "browser-host.example" def test_looks_like_domain(): - assert _looks_like_domain("browsercli.yiprawr.dev") is True - assert _looks_like_domain("browser-host.example") is True - assert _looks_like_domain("sub.domain.org") is True - assert _looks_like_domain("localhost") is False - assert _looks_like_domain("127.0.0.1") is False - assert _looks_like_domain("203.0.113.100") is False - assert _looks_like_domain("host") is False # no dot + assert _looks_like_domain("browsercli.yiprawr.dev") is True + assert _looks_like_domain("browser-host.example") is True + assert _looks_like_domain("sub.domain.org") is True + assert _looks_like_domain("localhost") is False + assert _looks_like_domain("127.0.0.1") is False + assert _looks_like_domain("203.0.113.100") is False + assert _looks_like_domain("host") is False # no dot def test_normalize_endpoint_strips_443_for_domains(): - assert _normalize_endpoint("browsercli.yiprawr.dev:443") == "browsercli.yiprawr.dev" - assert _normalize_endpoint("browsercli.yiprawr.dev") == "browsercli.yiprawr.dev" - assert _normalize_endpoint("203.0.113.1:443") == "203.0.113.1:443" # IP: keep port - assert _normalize_endpoint("localhost:443") == "localhost:443" # localhost: keep port - assert _normalize_endpoint("host:8765") == "host:8765" # non-443 port: unchanged - assert _normalize_endpoint("browsercli.yiprawr.dev:8765") == "browsercli.yiprawr.dev:8765" + assert _normalize_endpoint("browsercli.yiprawr.dev:443") == "browsercli.yiprawr.dev" + assert _normalize_endpoint("browsercli.yiprawr.dev") == "browsercli.yiprawr.dev" + assert _normalize_endpoint("203.0.113.1:443") == "203.0.113.1:443" # IP: keep port + assert _normalize_endpoint("localhost:443") == "localhost:443" # localhost: keep port + assert _normalize_endpoint("host:8765") == "host:8765" # non-443 port: unchanged + assert _normalize_endpoint("browsercli.yiprawr.dev:8765") == "browsercli.yiprawr.dev:8765" def test_resolve_connect_endpoint_adds_443_for_domain(): - assert _resolve_connect_endpoint("browsercli.yiprawr.dev") == "browsercli.yiprawr.dev:443" - assert _resolve_connect_endpoint("browsercli.yiprawr.dev:443") == "browsercli.yiprawr.dev:443" - assert _resolve_connect_endpoint("browsercli.yiprawr.dev:8765") == "browsercli.yiprawr.dev:8765" - assert _resolve_connect_endpoint("host:8765") == "host:8765" + assert _resolve_connect_endpoint("browsercli.yiprawr.dev") == "browsercli.yiprawr.dev:443" + assert _resolve_connect_endpoint("browsercli.yiprawr.dev:443") == "browsercli.yiprawr.dev:443" + assert _resolve_connect_endpoint("browsercli.yiprawr.dev:8765") == "browsercli.yiprawr.dev:8765" + assert _resolve_connect_endpoint("host:8765") == "host:8765" def test_resolve_connect_endpoint_raises_for_bare_non_domain(): - with pytest.raises(BrowserNotConnected, match="expected host:port"): - _resolve_connect_endpoint("localhost") + with pytest.raises(BrowserNotConnected, match="expected host:port"): + _resolve_connect_endpoint("localhost") def test_send_command_normalizes_domain_port_443(monkeypatch): - """--remote domain:443 is normalized; _send_remote gets the portless domain.""" - monkeypatch.delenv("BROWSER_CLI_PROFILE", raising=False) - monkeypatch.delenv("BROWSER_CLI_REMOTE", raising=False) - sent_to = {} + """--remote domain:443 is normalized; _send_remote gets the portless domain.""" + monkeypatch.delenv("BROWSER_CLI_PROFILE", raising=False) + monkeypatch.delenv("BROWSER_CLI_REMOTE", raising=False) + sent_to = {} - monkeypatch.setattr( - "browser_cli.client.core.remote_browser_targets", - lambda endpoint, key=None: [BrowserTarget("default", f"{endpoint}:default", "", remote=endpoint)], - ) + monkeypatch.setattr( + "browser_cli.client.core.remote_browser_targets", + lambda endpoint, key=None: [BrowserTarget("default", f"{endpoint}:default", "", remote=endpoint)], + ) - def fake_send_remote(endpoint, msg, private_key=None): - sent_to["endpoint"] = endpoint - return json.dumps({"success": True, "data": "ok"}).encode() + def fake_send_remote(endpoint, msg, private_key=None): + sent_to["endpoint"] = endpoint + return json.dumps({"success": True, "data": "ok"}).encode() - monkeypatch.setattr("browser_cli.client.core._send_remote", fake_send_remote) + monkeypatch.setattr("browser_cli.client.core._send_remote", fake_send_remote) - result = send_command("tabs.list", remote="browsercli.yiprawr.dev:443") - assert result == "ok" - assert sent_to["endpoint"] == "browsercli.yiprawr.dev" # stored/routed without port + result = send_command("tabs.list", remote="browsercli.yiprawr.dev:443") + assert result == "ok" + assert sent_to["endpoint"] == "browsercli.yiprawr.dev" # stored/routed without port def test_send_command_domain_without_port_defaults_to_443(monkeypatch): - """--remote domain (no port) is treated as :443.""" - monkeypatch.delenv("BROWSER_CLI_PROFILE", raising=False) - monkeypatch.delenv("BROWSER_CLI_REMOTE", raising=False) - sent_to = {} + """--remote domain (no port) is treated as :443.""" + monkeypatch.delenv("BROWSER_CLI_PROFILE", raising=False) + monkeypatch.delenv("BROWSER_CLI_REMOTE", raising=False) + sent_to = {} - monkeypatch.setattr( - "browser_cli.client.core.remote_browser_targets", - lambda endpoint, key=None: [BrowserTarget("default", f"{endpoint}:default", "", remote=endpoint)], - ) + monkeypatch.setattr( + "browser_cli.client.core.remote_browser_targets", + lambda endpoint, key=None: [BrowserTarget("default", f"{endpoint}:default", "", remote=endpoint)], + ) - def fake_send_remote(endpoint, msg, private_key=None): - sent_to["endpoint"] = endpoint - return json.dumps({"success": True, "data": "ok"}).encode() + def fake_send_remote(endpoint, msg, private_key=None): + sent_to["endpoint"] = endpoint + return json.dumps({"success": True, "data": "ok"}).encode() - monkeypatch.setattr("browser_cli.client.core._send_remote", fake_send_remote) + monkeypatch.setattr("browser_cli.client.core._send_remote", fake_send_remote) - result = send_command("tabs.list", remote="browsercli.yiprawr.dev") - assert result == "ok" - assert sent_to["endpoint"] == "browsercli.yiprawr.dev" + result = send_command("tabs.list", remote="browsercli.yiprawr.dev") + assert result == "ok" + assert sent_to["endpoint"] == "browsercli.yiprawr.dev" def test_domain_display_name_omits_port(monkeypatch, tmp_path): - """Domain endpoints stored without :443 display as 'domain:profile', not 'domain:443:profile'.""" - remotes_path = tmp_path / "remotes.json" - endpoint = "browsercli.yiprawr.dev" - remotes_path.write_text(json.dumps({endpoint: {}}), encoding="utf-8") - monkeypatch.setattr("browser_cli.client.targets.REGISTRY_PATH", tmp_path / "missing-registry.json") - monkeypatch.setattr("browser_cli.remote.registry.REMOTE_REGISTRY_PATH", remotes_path) + """Domain endpoints stored without :443 display as 'domain:profile', not 'domain:443:profile'.""" + remotes_path = tmp_path / "remotes.json" + endpoint = "browsercli.yiprawr.dev" + remotes_path.write_text(json.dumps({endpoint: {}}), encoding="utf-8") + monkeypatch.setattr("browser_cli.client.targets.REGISTRY_PATH", tmp_path / "missing-registry.json") + monkeypatch.setattr("browser_cli.remote.registry.REMOTE_REGISTRY_PATH", remotes_path) - def fake_send_command(command, args=None, profile=None, remote=None, key=None): - return [{"profile": "automatisation", "displayName": "automatisation"}] + def fake_send_command(command, args=None, profile=None, remote=None, key=None): + return [{"profile": "automatisation", "displayName": "automatisation"}] - monkeypatch.setattr("browser_cli.client.core.send_command", fake_send_command) + monkeypatch.setattr("browser_cli.client.core.send_command", fake_send_command) - targets = active_browser_targets() + targets = active_browser_targets() - assert len(targets) == 1 - assert targets[0].display_name == "browsercli.yiprawr.dev:automatisation" - assert targets[0].remote == endpoint + assert len(targets) == 1 + assert targets[0].display_name == "browsercli.yiprawr.dev:automatisation" + assert targets[0].remote == endpoint def test_domain_display_name_backward_compat_with_stored_443(monkeypatch, tmp_path): - """Old remotes.json with :443 still displays cleanly without the port.""" - remotes_path = tmp_path / "remotes.json" - endpoint = "browsercli.yiprawr.dev:443" # old format - remotes_path.write_text(json.dumps({endpoint: {}}), encoding="utf-8") - monkeypatch.setattr("browser_cli.client.targets.REGISTRY_PATH", tmp_path / "missing-registry.json") - monkeypatch.setattr("browser_cli.remote.registry.REMOTE_REGISTRY_PATH", remotes_path) + """Old remotes.json with :443 still displays cleanly without the port.""" + remotes_path = tmp_path / "remotes.json" + endpoint = "browsercli.yiprawr.dev:443" # old format + remotes_path.write_text(json.dumps({endpoint: {}}), encoding="utf-8") + monkeypatch.setattr("browser_cli.client.targets.REGISTRY_PATH", tmp_path / "missing-registry.json") + monkeypatch.setattr("browser_cli.remote.registry.REMOTE_REGISTRY_PATH", remotes_path) - def fake_send_command(command, args=None, profile=None, remote=None, key=None): - return [{"profile": "automatisation", "displayName": "automatisation"}] + def fake_send_command(command, args=None, profile=None, remote=None, key=None): + return [{"profile": "automatisation", "displayName": "automatisation"}] - monkeypatch.setattr("browser_cli.client.core.send_command", fake_send_command) + monkeypatch.setattr("browser_cli.client.core.send_command", fake_send_command) - targets = active_browser_targets() + targets = active_browser_targets() - assert len(targets) == 1 - assert targets[0].display_name == "browsercli.yiprawr.dev:automatisation" + assert len(targets) == 1 + assert targets[0].display_name == "browsercli.yiprawr.dev:automatisation" def test_send_command_auto_saves_and_reuses_key_for_remote(monkeypatch, tmp_path): - """--key agent is saved on first use; omitting --key on subsequent calls reuses it.""" - import json as _json + """--key agent is saved on first use; omitting --key on subsequent calls reuses it.""" + import json as _json - remotes_path = tmp_path / "remotes.json" - remotes_path.write_text("{}", encoding="utf-8") - monkeypatch.setattr("browser_cli.remote.registry.REMOTE_REGISTRY_PATH", remotes_path) - monkeypatch.setattr("browser_cli.client.targets.REGISTRY_PATH", tmp_path / "missing-registry.json") - monkeypatch.delenv("BROWSER_CLI_PROFILE", raising=False) - monkeypatch.delenv("BROWSER_CLI_REMOTE", raising=False) - monkeypatch.delenv("BROWSER_CLI_KEY", raising=False) + remotes_path = tmp_path / "remotes.json" + remotes_path.write_text("{}", encoding="utf-8") + monkeypatch.setattr("browser_cli.remote.registry.REMOTE_REGISTRY_PATH", remotes_path) + monkeypatch.setattr("browser_cli.client.targets.REGISTRY_PATH", tmp_path / "missing-registry.json") + monkeypatch.delenv("BROWSER_CLI_PROFILE", raising=False) + monkeypatch.delenv("BROWSER_CLI_REMOTE", raising=False) + monkeypatch.delenv("BROWSER_CLI_KEY", raising=False) - from pathlib import Path as _Path - used_keys = [] + from pathlib import Path as _Path + used_keys = [] - def fake_load_private_key(key_path=None): - used_keys.append(str(key_path) if key_path is not None else None) - return None # no actual key needed for this test + def fake_load_private_key(key_path=None): + used_keys.append(str(key_path) if key_path is not None else None) + return None # no actual key needed for this test - monkeypatch.setattr("browser_cli.client.auth.load_private_key", fake_load_private_key) - monkeypatch.setattr( - "browser_cli.client.core.remote_browser_targets", - lambda endpoint, key=None: [BrowserTarget("default", "host:default", "", remote=endpoint)], - ) + monkeypatch.setattr("browser_cli.client.auth.load_private_key", fake_load_private_key) + monkeypatch.setattr( + "browser_cli.client.core.remote_browser_targets", + lambda endpoint, key=None: [BrowserTarget("default", "host:default", "", remote=endpoint)], + ) - def fake_send_remote(endpoint, msg, private_key=None): - return _json.dumps({"success": True, "data": "ok"}).encode() + def fake_send_remote(endpoint, msg, private_key=None): + return _json.dumps({"success": True, "data": "ok"}).encode() - monkeypatch.setattr("browser_cli.client.core._send_remote", fake_send_remote) + monkeypatch.setattr("browser_cli.client.core._send_remote", fake_send_remote) - # First call with explicit --key agent - send_command("tabs.list", remote="host:8765", key=_Path("agent")) - assert used_keys[-1] == "agent" + # First call with explicit --key agent + send_command("tabs.list", remote="host:8765", key=_Path("agent")) + assert used_keys[-1] == "agent" - # Key must be persisted now - assert key_for_remote("host:8765") == "agent" + # Key must be persisted now + assert key_for_remote("host:8765") == "agent" - # Second call without --key — should reuse saved "agent" - send_command("tabs.list", remote="host:8765") - assert used_keys[-1] == "agent" + # Second call without --key — should reuse saved "agent" + send_command("tabs.list", remote="host:8765") + assert used_keys[-1] == "agent" # ── async command transport ────────────────────────────────────────────────── def test_send_command_async_uses_async_remote_transport(monkeypatch): - monkeypatch.delenv("BROWSER_CLI_PROFILE", raising=False) - monkeypatch.delenv("BROWSER_CLI_REMOTE", raising=False) - sent = {} + monkeypatch.delenv("BROWSER_CLI_PROFILE", raising=False) + monkeypatch.delenv("BROWSER_CLI_REMOTE", raising=False) + sent = {} - async def fake_send_remote_async(endpoint, msg, private_key=None): - sent["endpoint"] = endpoint - sent.update(msg) - return json.dumps({"success": True, "data": "ok"}).encode() + async def fake_send_remote_async(endpoint, msg, private_key=None): + sent["endpoint"] = endpoint + sent.update(msg) + return json.dumps({"success": True, "data": "ok"}).encode() - monkeypatch.setattr("browser_cli.client.core._send_remote_async", fake_send_remote_async) - monkeypatch.setattr("browser_cli.client.core.remote_browser_targets_async", lambda endpoint, key=None: _async_targets(endpoint)) + monkeypatch.setattr("browser_cli.client.core._send_remote_async", fake_send_remote_async) + monkeypatch.setattr("browser_cli.client.core.remote_browser_targets_async", lambda endpoint, key=None: _async_targets(endpoint)) - async def run(): - return await send_command_async("tabs.list", remote="host:8765", key=None) + async def run(): + return await send_command_async("tabs.list", remote="host:8765", key=None) - async def _async_targets(endpoint): - return [BrowserTarget("work", "host:work", "", remote=endpoint)] + async def _async_targets(endpoint): + return [BrowserTarget("work", "host:work", "", remote=endpoint)] - assert asyncio.run(run()) == "ok" - assert sent["endpoint"] == "host:8765" - assert sent["_route"] == "work" - assert sent["user_agent"].startswith("browser-cli/") + assert asyncio.run(run()) == "ok" + assert sent["endpoint"] == "host:8765" + assert sent["_route"] == "work" + assert sent["user_agent"].startswith("browser-cli/") def test_send_command_async_local_unix_roundtrip(monkeypatch, tmp_path): - monkeypatch.delenv("BROWSER_CLI_PROFILE", raising=False) - monkeypatch.delenv("BROWSER_CLI_REMOTE", raising=False) - sock_path = tmp_path / "browser.sock" - seen = [] + monkeypatch.delenv("BROWSER_CLI_PROFILE", raising=False) + monkeypatch.delenv("BROWSER_CLI_REMOTE", raising=False) + sock_path = tmp_path / "browser.sock" + seen = [] - async def run(): - async def handle(reader, writer): - raw_len = await reader.readexactly(4) - msg_len = struct.unpack("= 2, f"expected concurrent endpoint discovery, peak was {max_active}" + +def test_collect_browser_clients_uses_cached_target_version(monkeypatch, tmp_path): + """A remote target advertising version/extVersion skips the clients.list roundtrip.""" + from browser_cli.client import collect_browser_clients + import browser_cli.client.core as core + + monkeypatch.setattr("browser_cli.client.targets.REGISTRY_PATH", tmp_path / "missing-registry.json") + + cached_target = BrowserTarget( + profile="work", + display_name="host.example:work", + socket_path="", + remote="host.example:8765", + browser_name="Firefox", + display_group="host.example", + version="151.0", + extension_version="0.15.6", + ) + monkeypatch.setattr(core, "active_browser_targets", lambda **kw: [cached_target]) + + calls = [] + monkeypatch.setattr(core, "send_command", lambda *a, **k: calls.append((a, k)) or []) + + rows = collect_browser_clients(registry_path=tmp_path / "missing-registry.json") + + assert calls == [] # no clients.list roundtrip was needed + assert rows == [{ + "profile": "host.example:work", + "profileGroup": "host.example", + "name": "Firefox", + "version": "151.0", + "extensionVersion": "0.15.6", + }] + +def test_collect_browser_clients_falls_back_when_version_unknown(monkeypatch, tmp_path): + """An older remote (no advertised version) still triggers a clients.list query.""" + from browser_cli.client import collect_browser_clients + import browser_cli.client.core as core + + monkeypatch.setattr("browser_cli.client.targets.REGISTRY_PATH", tmp_path / "missing-registry.json") + + legacy_target = BrowserTarget( + profile="work", + display_name="host.example:work", + socket_path="", + remote="host.example:8765", + browser_name="Firefox", + display_group="host.example", + ) + monkeypatch.setattr(core, "active_browser_targets", lambda **kw: [legacy_target]) + + calls = [] + + def fake_send(command, args=None, profile=None, remote=None, key=None, **kw): + calls.append(command) + return [{"name": "Firefox", "version": "151.0", "extensionVersion": "0.15.2"}] + + monkeypatch.setattr(core, "send_command", fake_send) + + rows = collect_browser_clients(registry_path=tmp_path / "missing-registry.json") + + assert calls == ["clients.list"] # fell back to a query + assert rows[0]["version"] == "151.0" diff --git a/tests/test_native_host.py b/tests/test_native_host.py index 14c1079..5b154f2 100644 --- a/tests/test_native_host.py +++ b/tests/test_native_host.py @@ -9,412 +9,432 @@ from browser_cli import framing, local_transport from browser_cli.native import local_server, protocol as native_protocol def _raise_system_exit(code: int): - raise SystemExit(code) + raise SystemExit(code) def test_cleanup_removes_socket_and_registry_entry(monkeypatch, tmp_path): - alias = "work" - socket_path = tmp_path / "work.sock" - socket_path.write_text("") - registry_path = tmp_path / "registry.json" - registry_path.write_text(json.dumps({alias: str(socket_path), "other": str(tmp_path / "other.sock")})) + alias = "work" + socket_path = tmp_path / "work.sock" + socket_path.write_text("") + registry_path = tmp_path / "registry.json" + registry_path.write_text(json.dumps({alias: str(socket_path), "other": str(tmp_path / "other.sock")})) - monkeypatch.setattr(native_host, "REGISTRY_PATH", registry_path) - monkeypatch.setattr(native_host, "_socket_path_for", lambda alias: str(socket_path)) - monkeypatch.setattr(native_host, "is_windows", lambda: False) + monkeypatch.setattr(native_host, "REGISTRY_PATH", registry_path) + monkeypatch.setattr(native_host, "_socket_path_for", lambda alias: str(socket_path)) + monkeypatch.setattr(native_host, "is_windows", lambda: False) - native_host._cleanup(alias) + native_host._cleanup(alias) - assert not socket_path.exists() - assert json.loads(registry_path.read_text()) == {"other": str(tmp_path / "other.sock")} + assert not socket_path.exists() + assert json.loads(registry_path.read_text()) == {"other": str(tmp_path / "other.sock")} def test_stdin_reader_cleans_up_on_eof(monkeypatch): - cleaned = [] + cleaned = [] - monkeypatch.setattr(native_host.protocol, "read_native_message", lambda stream: None) - monkeypatch.setattr(native_host, "_cleanup", cleaned.append) - monkeypatch.setattr(native_host.os, "_exit", _raise_system_exit) - monkeypatch.setattr(native_host.sys, "stdin", SimpleNamespace(buffer=object())) + monkeypatch.setattr(native_host.protocol, "read_native_message", lambda stream: None) + monkeypatch.setattr(native_host, "_cleanup", cleaned.append) + monkeypatch.setattr(native_host.os, "_exit", _raise_system_exit) + monkeypatch.setattr(native_host.sys, "stdin", SimpleNamespace(buffer=object())) - with pytest.raises(SystemExit, match="0"): - native_host.stdin_reader("work") + with pytest.raises(SystemExit, match="0"): + native_host.stdin_reader("work") - assert cleaned == ["work"] + assert cleaned == ["work"] def test_cleanup_windows_skips_socket_unlink(monkeypatch, tmp_path): - registry_path = tmp_path / "registry.json" - registry_path.write_text(json.dumps({"work": r"\\.\pipe\browser-cli-work"})) + registry_path = tmp_path / "registry.json" + registry_path.write_text(json.dumps({"work": r"\\.\pipe\browser-cli-work"})) - monkeypatch.setattr(native_host, "REGISTRY_PATH", registry_path) - monkeypatch.setattr(native_host, "is_windows", lambda: True) + monkeypatch.setattr(native_host, "REGISTRY_PATH", registry_path) + monkeypatch.setattr(native_host, "is_windows", lambda: True) - native_host._cleanup("work") + native_host._cleanup("work") - assert json.loads(registry_path.read_text()) == {} + assert json.loads(registry_path.read_text()) == {} def test_stdin_reader_cleans_up_on_bye(monkeypatch): - cleaned = [] - messages = iter([{"type": "bye"}]) + cleaned = [] + messages = iter([{"type": "bye"}]) - monkeypatch.setattr(native_host.protocol, "read_native_message", lambda stream: next(messages)) - monkeypatch.setattr(native_host, "_cleanup", cleaned.append) - monkeypatch.setattr(native_host.os, "_exit", _raise_system_exit) - monkeypatch.setattr(native_host.sys, "stdin", SimpleNamespace(buffer=object())) + monkeypatch.setattr(native_host.protocol, "read_native_message", lambda stream: next(messages)) + monkeypatch.setattr(native_host, "_cleanup", cleaned.append) + monkeypatch.setattr(native_host.os, "_exit", _raise_system_exit) + monkeypatch.setattr(native_host.sys, "stdin", SimpleNamespace(buffer=object())) - with pytest.raises(SystemExit, match="0"): - native_host.stdin_reader("work") + with pytest.raises(SystemExit, match="0"): + native_host.stdin_reader("work") - assert cleaned == ["work"] + assert cleaned == ["work"] def test_stdin_reader_routes_response_messages(monkeypatch): - response_queue = native_host.queue.Queue() - native_host.PENDING["msg-1"] = response_queue - messages = iter([{"type": "hello"}, {"id": "msg-1", "success": True}, None]) + response_queue = native_host.queue.Queue() + native_host.PENDING["msg-1"] = response_queue + messages = iter([{"type": "hello"}, {"id": "msg-1", "success": True}, None]) - monkeypatch.setattr(native_host.protocol, "read_native_message", lambda stream: next(messages)) - monkeypatch.setattr(native_host, "_cleanup", lambda alias: None) - monkeypatch.setattr(native_host.os, "_exit", _raise_system_exit) - monkeypatch.setattr(native_host.sys, "stdin", SimpleNamespace(buffer=object())) + monkeypatch.setattr(native_host.protocol, "read_native_message", lambda stream: next(messages)) + monkeypatch.setattr(native_host, "_cleanup", lambda alias: None) + monkeypatch.setattr(native_host.os, "_exit", _raise_system_exit) + monkeypatch.setattr(native_host.sys, "stdin", SimpleNamespace(buffer=object())) - with pytest.raises(SystemExit, match="0"): - native_host.stdin_reader("work") + with pytest.raises(SystemExit, match="0"): + native_host.stdin_reader("work") - assert response_queue.get_nowait() == {"id": "msg-1", "success": True} - native_host.PENDING.clear() + assert response_queue.get_nowait() == {"id": "msg-1", "success": True} + native_host.PENDING.clear() def test_collect_paged_browser_command_accumulates_pages(monkeypatch): - calls = [] - pages = iter([ - {"success": True, "data": {"__browserCliPage": True, "items": [1, 2], "total": 3, "nextOffset": 2}}, - {"success": True, "data": {"__browserCliPage": True, "items": [3], "total": 3, "nextOffset": None}}, - ]) + calls = [] + pages = iter([ + {"success": True, "data": {"__browserCliPage": True, "items": [1, 2], "total": 3, "nextOffset": 2}}, + {"success": True, "data": {"__browserCliPage": True, "items": [3], "total": 3, "nextOffset": None}}, + ]) - def fake_send(cmd): - calls.append(cmd) - return next(pages) + def fake_send(cmd): + calls.append(cmd) + return next(pages) - monkeypatch.setattr(native_host, "PAGE_SIZE", 2) - monkeypatch.setattr(native_host, "_send_browser_command", fake_send) + monkeypatch.setattr(native_host, "PAGE_SIZE", 2) + monkeypatch.setattr(native_host, "_send_browser_command", fake_send) - result = native_host._collect_paged_browser_command({"id": "orig", "command": "tabs.list", "args": {"foo": "bar"}}) + result = native_host._collect_paged_browser_command({"id": "orig", "command": "tabs.list", "args": {"foo": "bar"}}) - assert result == {"id": "orig", "success": True, "data": [1, 2, 3], "pageSize": 2, "total": 3} - assert [call["args"]["__page"] for call in calls] == [ - {"offset": 0, "limit": 2}, - {"offset": 2, "limit": 2}, - ] - assert all(call["args"]["foo"] == "bar" for call in calls) - assert all(call["id"] != "orig" for call in calls) + assert result == {"id": "orig", "success": True, "data": [1, 2, 3], "pageSize": 2, "total": 3} + assert [call["args"]["__page"] for call in calls] == [ + {"offset": 0, "limit": 2}, + {"offset": 2, "limit": 2}, + ] + assert all(call["args"]["foo"] == "bar" for call in calls) + assert all(call["id"] != "orig" for call in calls) def test_collect_paged_browser_command_passes_through_non_paged_response(monkeypatch): - monkeypatch.setattr(native_host, "_send_browser_command", lambda cmd: {"id": cmd["id"], "success": True, "data": {"value": 1}}) + monkeypatch.setattr(native_host, "_send_browser_command", lambda cmd: {"id": cmd["id"], "success": True, "data": {"value": 1}}) - result = native_host._collect_paged_browser_command({"id": "orig", "command": "tabs.list", "args": {}}) + result = native_host._collect_paged_browser_command({"id": "orig", "command": "tabs.list", "args": {}}) - assert result == {"id": "orig", "success": True, "data": {"value": 1}} + assert result == {"id": "orig", "success": True, "data": {"value": 1}} def test_handle_browser_command_pages_known_list_commands(monkeypatch): - seen = [] + seen = [] - monkeypatch.setattr(native_host, "_collect_paged_browser_command", lambda cmd: seen.append(cmd) or {"success": True, "data": []}) + monkeypatch.setattr(native_host, "_collect_paged_browser_command", lambda cmd: seen.append(cmd) or {"success": True, "data": []}) - result = native_host._handle_browser_command({"id": "orig", "command": "tabs.list", "args": {}}) + result = native_host._handle_browser_command({"id": "orig", "command": "tabs.list", "args": {}}) - assert result == {"success": True, "data": []} - assert seen[0]["command"] == "tabs.list" + assert result == {"success": True, "data": []} + assert seen[0]["command"] == "tabs.list" def test_handle_browser_command_sends_non_pageable_directly(monkeypatch): - seen = [] - monkeypatch.setattr(native_host, "_send_browser_command", lambda cmd: seen.append(cmd) or {"success": True, "data": "ok"}) + seen = [] + monkeypatch.setattr(native_host, "_send_browser_command", lambda cmd: seen.append(cmd) or {"success": True, "data": "ok"}) - result = native_host._handle_browser_command({"id": "x", "command": "navigate.open", "args": {}}) + result = native_host._handle_browser_command({"id": "x", "command": "navigate.open", "args": {}}) - assert result == {"success": True, "data": "ok"} - assert seen[0]["command"] == "navigate.open" + assert result == {"success": True, "data": "ok"} + assert seen[0]["command"] == "navigate.open" # --------------------------------------------------------------------------- # _read_exact_stream # --------------------------------------------------------------------------- def test_read_exact_stream_full_read(): - """Returns the exact bytes when stream delivers them in one shot.""" - import io - stream = io.BytesIO(b"hello") - assert native_protocol.read_exact_stream(stream, 5) == b"hello" + """Returns the exact bytes when stream delivers them in one shot.""" + import io + stream = io.BytesIO(b"hello") + assert native_protocol.read_exact_stream(stream, 5) == b"hello" def test_read_exact_stream_partial_chunks(): - """Accumulates multiple short chunks until n bytes are read.""" - import io + """Accumulates multiple short chunks until n bytes are read.""" + import io - class _ChunkyStream: - def __init__(self, data, chunk_size): - self._data = data - self._pos = 0 - self._chunk_size = chunk_size + class _ChunkyStream: + def __init__(self, data, chunk_size): + self._data = data + self._pos = 0 + self._chunk_size = chunk_size - def read(self, n): - end = min(self._pos + self._chunk_size, len(self._data)) - chunk = self._data[self._pos:end] - self._pos = end - return chunk + def read(self, n): + end = min(self._pos + self._chunk_size, len(self._data)) + chunk = self._data[self._pos:end] + self._pos = end + return chunk - stream = _ChunkyStream(b"abcdefgh", 3) - assert native_protocol.read_exact_stream(stream, 8) == b"abcdefgh" + stream = _ChunkyStream(b"abcdefgh", 3) + assert native_protocol.read_exact_stream(stream, 8) == b"abcdefgh" def test_read_exact_stream_eof_returns_none(): - """Returns None if stream is exhausted before n bytes are delivered.""" - import io - stream = io.BytesIO(b"ab") # only 2 bytes, asking for 4 - assert native_protocol.read_exact_stream(stream, 4) is None + """Returns None if stream is exhausted before n bytes are delivered.""" + import io + stream = io.BytesIO(b"ab") # only 2 bytes, asking for 4 + assert native_protocol.read_exact_stream(stream, 4) is None def test_read_exact_stream_immediate_eof(): - """Returns None on an empty stream.""" - import io - stream = io.BytesIO(b"") - assert native_protocol.read_exact_stream(stream, 1) is None + """Returns None on an empty stream.""" + import io + stream = io.BytesIO(b"") + assert native_protocol.read_exact_stream(stream, 1) is None # --------------------------------------------------------------------------- # write_native_message / read_native_message round-trip # --------------------------------------------------------------------------- def test_write_and_read_native_message_roundtrip(): - """write_native_message followed by read_native_message recovers the original dict.""" - import io - buf = io.BytesIO() - msg = {"id": "abc", "command": "tabs.list", "args": {}} - native_protocol.write_native_message(buf, msg) - buf.seek(0) - result = native_protocol.read_native_message(buf) - assert result == msg + """write_native_message followed by read_native_message recovers the original dict.""" + import io + buf = io.BytesIO() + msg = {"id": "abc", "command": "tabs.list", "args": {}} + native_protocol.write_native_message(buf, msg) + buf.seek(0) + result = native_protocol.read_native_message(buf) + assert result == msg def test_read_native_message_eof_at_length_prefix(): - """Returns None when the stream is empty (no length prefix).""" - import io - stream = io.BytesIO(b"") - assert native_protocol.read_native_message(stream) is None + """Returns None when the stream is empty (no length prefix).""" + import io + stream = io.BytesIO(b"") + assert native_protocol.read_native_message(stream) is None def test_read_native_message_eof_at_body(): - """Returns None when the body is truncated after reading the length prefix.""" - import io - import struct - # Write a 10-byte length prefix but only 5 bytes of body - buf = struct.pack("= 5 # stopped at/just past the cap, not unbounded def test_collect_paged_browser_command_invalid_items(monkeypatch): - """If items is not a list the command returns an error dict.""" - monkeypatch.setattr( - native_host, "_send_browser_command", - lambda cmd: { - "id": cmd["id"], - "success": True, - "data": {"__browserCliPage": True, "items": "not-a-list", "total": 1, "nextOffset": None}, - }, - ) - result = native_host._collect_paged_browser_command({"id": "bad", "command": "tabs.list", "args": {}}) - assert result["success"] is False - assert "invalid paged response" in result["error"] + """If items is not a list the command returns an error dict.""" + monkeypatch.setattr( + native_host, "_send_browser_command", + lambda cmd: { + "id": cmd["id"], + "success": True, + "data": {"__browserCliPage": True, "items": "not-a-list", "total": 1, "nextOffset": None}, + }, + ) + result = native_host._collect_paged_browser_command({"id": "bad", "command": "tabs.list", "args": {}}) + assert result["success"] is False + assert "invalid paged response" in result["error"] # --------------------------------------------------------------------------- # _resolve_profile_alias # --------------------------------------------------------------------------- def test_resolve_profile_alias_uses_hello_alias(): - alias = native_host._resolve_profile_alias({"type": "hello", "alias": "brave-work"}) - assert alias == "brave-work" + alias = native_host._resolve_profile_alias({"type": "hello", "alias": "brave-work"}) + assert alias == "brave-work" def test_resolve_profile_alias_no_hello_returns_uuid(): - alias = native_host._resolve_profile_alias(None) - import uuid - uuid.UUID(alias) # raises ValueError if not a valid UUID + alias = native_host._resolve_profile_alias(None) + import uuid + uuid.UUID(alias) # raises ValueError if not a valid UUID def test_resolve_profile_alias_default_alias_returns_uuid(): - from browser_cli.platform import DEFAULT_ALIAS - alias = native_host._resolve_profile_alias({"type": "hello", "alias": DEFAULT_ALIAS}) - import uuid - uuid.UUID(alias) + from browser_cli.platform import DEFAULT_ALIAS + alias = native_host._resolve_profile_alias({"type": "hello", "alias": DEFAULT_ALIAS}) + import uuid + uuid.UUID(alias) def test_resolve_profile_alias_non_hello_type_returns_uuid(): - alias = native_host._resolve_profile_alias({"type": "bye", "alias": "some"}) - import uuid - uuid.UUID(alias) + alias = native_host._resolve_profile_alias({"type": "bye", "alias": "some"}) + import uuid + uuid.UUID(alias) # --------------------------------------------------------------------------- # asyncio Unix-socket server path # --------------------------------------------------------------------------- def test_async_recv_all_and_send_all_roundtrip(): - """local_transport async framing mirrors the sync length-prefixed socket framing.""" - import asyncio + """local_transport async framing mirrors the sync length-prefixed socket framing.""" + import asyncio - async def run(): - async def handle(reader, writer): - payload = await local_transport.async_recv_all(reader) - await local_transport.async_send_all(writer, payload + b"-reply") - writer.close() - await writer.wait_closed() + async def run(): + async def handle(reader, writer): + payload = await local_transport.async_recv_all(reader) + await local_transport.async_send_all(writer, payload + b"-reply") + writer.close() + await writer.wait_closed() - server = await asyncio.start_server(handle, "127.0.0.1", 0) - host, port = server.sockets[0].getsockname() - async with server: - reader, writer = await asyncio.open_connection(host, port) - await local_transport.async_send_all(writer, b"hello") - assert await local_transport.async_recv_all(reader) == b"hello-reply" - writer.close() - await writer.wait_closed() + server = await asyncio.start_server(handle, "127.0.0.1", 0) + host, port = server.sockets[0].getsockname() + async with server: + reader, writer = await asyncio.open_connection(host, port) + await local_transport.async_send_all(writer, b"hello") + assert await local_transport.async_recv_all(reader) == b"hello-reply" + writer.close() + await writer.wait_closed() - asyncio.run(run()) + asyncio.run(run()) def test_async_socket_server_handles_cli_request(monkeypatch, tmp_path): - """Unix CLI socket server accepts requests concurrently via asyncio.""" - import asyncio - import struct + """Unix CLI socket server accepts requests concurrently via asyncio.""" + import asyncio + import struct - async def read_frame(reader): - raw_len = await reader.readexactly(4) - msg_len = struct.unpack(" None: - payload = json.dumps(msg).encode("utf-8") - sock.sendall(struct.pack(" dict: - raw_len = b"" - while len(raw_len) < 4: - chunk = sock.recv(4 - len(raw_len)) - if not chunk: - raise ConnectionError("socket closed before response header") - raw_len += chunk - msg_len = struct.unpack(" tuple[dict, bytes]: - if "pq_kex" not in challenge: - pytest.skip("ML-KEM backend not available") + if "pq_kex" not in challenge: + pytest.skip("ML-KEM backend not available") - ciphertext_hex, shared_secret = pq_kex_client_encapsulate(challenge["pq_kex"]["public_key"]) - clean_msg = { - **command_msg, - "pq_kex": {"alg": "ML-KEM-768", "ciphertext": ciphertext_hex}, - } - sig = sign(priv, bytes.fromhex(nonce_hex), clean_msg, shared_secret).hex() - if not encrypted: - return {**clean_msg, "pubkey": pub, "sig": sig}, shared_secret + ciphertext_hex, shared_secret = pq_kex_client_encapsulate(challenge["pq_kex"]["public_key"]) + clean_msg = { + **command_msg, + "pq_kex": {"alg": "ML-KEM-768", "ciphertext": ciphertext_hex}, + } + sig = sign(priv, bytes.fromhex(nonce_hex), clean_msg, shared_secret).hex() + if not encrypted: + return {**clean_msg, "pubkey": pub, "sig": sig}, shared_secret - envelope = { - "id": clean_msg["id"], - "user_agent": clean_msg["user_agent"], - "pubkey": pub, - "sig": sig, - "pq_kex": clean_msg["pq_kex"], - "encrypted": pq_encrypt(shared_secret, "request", json.dumps(clean_msg).encode("utf-8")), - } - return envelope, shared_secret + envelope = { + "id": clean_msg["id"], + "user_agent": clean_msg["user_agent"], + "pubkey": pub, + "sig": sig, + "pq_kex": clean_msg["pq_kex"], + "encrypted": pq_encrypt(shared_secret, "request", json.dumps(clean_msg).encode("utf-8")), + } + return envelope, shared_secret def _assert_browser_not_connected(resp: dict) -> None: - assert resp.get("success") is False - assert "browser" in resp.get("error", "").lower() or "connected" in resp.get("error", "").lower() + assert resp.get("success") is False + assert "browser" in resp.get("error", "").lower() or "connected" in resp.get("error", "").lower() def test_real_mlkem_primitive_roundtrip(): - keypair = pq_kex_server_keypair() - if keypair is None: - pytest.skip("ML-KEM backend not available") - private_key, public_key = keypair + keypair = pq_kex_server_keypair() + if keypair is None: + pytest.skip("ML-KEM backend not available") + private_key, public_key = keypair - ciphertext_hex, client_secret = pq_kex_client_encapsulate(public_key.hex()) - server_secret = pq_kex_server_decapsulate(private_key, ciphertext_hex) + ciphertext_hex, client_secret = pq_kex_client_encapsulate(public_key.hex()) + server_secret = pq_kex_server_decapsulate(private_key, ciphertext_hex) - assert server_secret == client_secret + assert server_secret == client_secret @pytest.mark.parametrize( - ("client_version", "encrypted", "expect_encrypted_response"), - [ - ("0.9.3", False, False), # legacy client stays compatible - ("0.9.5", True, True), # current client must use encrypted transport - ], + ("client_version", "encrypted", "expect_encrypted_response"), + [ + ("0.9.3", False, False), # legacy client stays compatible + ("0.9.5", True, True), # current client must use encrypted transport + ], ) def test_remote_protocol_version_matrix(auth_material, client_version, encrypted, expect_encrypted_response): - selected_version = os.environ.get("BROWSER_CLI_COMPAT_CLIENT_VERSION") - if selected_version and selected_version != client_version: - pytest.skip(f"compat matrix selected {selected_version}") + selected_version = os.environ.get("BROWSER_CLI_COMPAT_CLIENT_VERSION") + if selected_version and selected_version != client_version: + pytest.skip(f"compat matrix selected {selected_version}") - _key_path, auth_path, priv, pub = auth_material - client, thread, challenge = _connect(auth_path) + _key_path, auth_path, priv, pub = auth_material + client, thread, challenge = _connect(auth_path) - msg = { - "id": f"tabs-{client_version}", - "command": "tabs.list", - "args": {}, - "user_agent": f"browser-cli/{client_version}", - } - wire_msg, shared_secret = _pq_auth_message(priv, pub, challenge["nonce"], msg, challenge, encrypted=encrypted) - _send_framed(client, wire_msg) - resp = _recv_framed(client) + msg = { + "id": f"tabs-{client_version}", + "command": "tabs.list", + "args": {}, + "user_agent": f"browser-cli/{client_version}", + } + wire_msg, shared_secret = _pq_auth_message(priv, pub, challenge["nonce"], msg, challenge, encrypted=encrypted) + _send_framed(client, wire_msg) + resp = _recv_framed(client) - if expect_encrypted_response: - assert set(resp) == {"encrypted"} - resp = json.loads(pq_decrypt(shared_secret, "response", resp["encrypted"])) - else: - assert "encrypted" not in resp + if expect_encrypted_response: + assert set(resp) == {"encrypted"} + resp = json.loads(pq_decrypt(shared_secret, "response", resp["encrypted"])) + else: + assert "encrypted" not in resp - _assert_browser_not_connected(resp) - client.close() - thread.join(timeout=2) + _assert_browser_not_connected(resp) + client.close() + thread.join(timeout=2) def test_current_client_plaintext_transport_is_rejected(auth_material): - _key_path, auth_path, priv, pub = auth_material - client, thread, challenge = _connect(auth_path) + _key_path, auth_path, priv, pub = auth_material + client, thread, challenge = _connect(auth_path) - msg = { - "id": "new-plain", - "command": "tabs.list", - "args": {}, - "user_agent": "browser-cli/0.9.5", - } - wire_msg, _shared_secret = _pq_auth_message(priv, pub, challenge["nonce"], msg, challenge, encrypted=False) - _send_framed(client, wire_msg) - resp = _recv_framed(client) + msg = { + "id": "new-plain", + "command": "tabs.list", + "args": {}, + "user_agent": "browser-cli/0.9.5", + } + wire_msg, _shared_secret = _pq_auth_message(priv, pub, challenge["nonce"], msg, challenge, encrypted=False) + _send_framed(client, wire_msg) + resp = _recv_framed(client) - assert resp.get("success") is False - assert "encrypted transport" in resp.get("error", "").lower() - client.close() - thread.join(timeout=2) + assert resp.get("success") is False + assert "encrypted transport" in resp.get("error", "").lower() + client.close() + thread.join(timeout=2) -def test_send_command_uses_encrypted_remote_transport(auth_material): - key_path, auth_path, _priv, _pub = auth_material - server = socket.socket(socket.AF_INET, socket.SOCK_STREAM) - server.bind(("127.0.0.1", 0)) - server.listen(1) - host, port = server.getsockname() +def test_send_command_uses_encrypted_remote_transport(auth_material, monkeypatch, tmp_path): + monkeypatch.setattr( + "browser_cli.remote.registry.REMOTE_REGISTRY_PATH", tmp_path / "remotes.json" + ) + key_path, auth_path, _priv, _pub = auth_material + server = socket.socket(socket.AF_INET, socket.SOCK_STREAM) + server.bind(("127.0.0.1", 0)) + server.listen(1) + host, port = server.getsockname() - def _accept_once(): - conn, addr = server.accept() - _handle_client(conn, addr, None, auth_path) - server.close() + def _accept_once(): + conn, addr = server.accept() + _handle_client(conn, addr, None, auth_path) + server.close() - thread = threading.Thread(target=_accept_once, daemon=True) - thread.start() + thread = threading.Thread(target=_accept_once, daemon=True) + thread.start() + with pytest.raises(RuntimeError, match="browser|connected"): + send_command("tabs.list", remote=f"{host}:{port}", profile="default", key=key_path) + + thread.join(timeout=2) + +def test_no_mlkem_backend_falls_back_and_client_warns(auth_material, monkeypatch, tmp_path): + monkeypatch.setattr( + "browser_cli.remote.registry.REMOTE_REGISTRY_PATH", tmp_path / "remotes.json" + ) + key_path, auth_path, _priv, _pub = auth_material + monkeypatch.setattr("browser_cli.auth.pq_kex_server_keypair", lambda: None) + + server = socket.socket(socket.AF_INET, socket.SOCK_STREAM) + server.bind(("127.0.0.1", 0)) + server.listen(1) + host, port = server.getsockname() + + def _accept_once(): + conn, addr = server.accept() + _handle_client(conn, addr, None, auth_path) + server.close() + + thread = threading.Thread(target=_accept_once, daemon=True) + thread.start() + + stderr = io.StringIO() + with contextlib.redirect_stderr(stderr): with pytest.raises(RuntimeError, match="browser|connected"): - send_command("tabs.list", remote=f"{host}:{port}", profile="default", key=key_path) + send_command("tabs.list", remote=f"{host}:{port}", profile="default", key=key_path) - thread.join(timeout=2) + assert "not using a post-quantum key exchange" in stderr.getvalue() + thread.join(timeout=2) -def test_no_mlkem_backend_falls_back_and_client_warns(auth_material, monkeypatch): - key_path, auth_path, _priv, _pub = auth_material - monkeypatch.setattr("browser_cli.auth.pq_kex_server_keypair", lambda: None) +def _run_pool_server(server, auth_path, connections): + server.settimeout(3) + while True: + try: + conn, addr = server.accept() + except OSError: + return + connections.append(conn) + threading.Thread(target=_handle_client, args=(conn, addr, None, auth_path), daemon=True).start() - server = socket.socket(socket.AF_INET, socket.SOCK_STREAM) - server.bind(("127.0.0.1", 0)) - server.listen(1) - host, port = server.getsockname() +def test_send_command_reuses_pooled_connection(auth_material): + """Two sequential commands to one endpoint share a single authenticated connection.""" + from browser_cli.remote import pool + pool.close_all() - def _accept_once(): - conn, addr = server.accept() - _handle_client(conn, addr, None, auth_path) - server.close() + key_path, auth_path, _priv, _pub = auth_material + server = socket.socket(socket.AF_INET, socket.SOCK_STREAM) + server.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEADDR, 1) + server.bind(("127.0.0.1", 0)) + server.listen(2) + host, port = server.getsockname() + connections = [] + threading.Thread(target=_run_pool_server, args=(server, auth_path, connections), daemon=True).start() - thread = threading.Thread(target=_accept_once, daemon=True) - thread.start() + endpoint = f"{host}:{port}" + try: + for _ in range(2): + with pytest.raises(RuntimeError, match="browser|connected"): + send_command("tabs.list", remote=endpoint, profile="default", key=key_path) + assert len(connections) == 1 # the second command reused the first connection + finally: + pool.close_all() + server.close() - stderr = io.StringIO() - with contextlib.redirect_stderr(stderr): - with pytest.raises(RuntimeError, match="browser|connected"): - send_command("tabs.list", remote=f"{host}:{port}", profile="default", key=key_path) +def test_send_command_opens_new_connection_when_pool_empty(auth_material): + """With no pooled connection to reuse, each command opens its own.""" + from browser_cli.remote import pool - assert "not using a post-quantum key exchange" in stderr.getvalue() - thread.join(timeout=2) + key_path, auth_path, _priv, _pub = auth_material + server = socket.socket(socket.AF_INET, socket.SOCK_STREAM) + server.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEADDR, 1) + server.bind(("127.0.0.1", 0)) + server.listen(2) + host, port = server.getsockname() + connections = [] + threading.Thread(target=_run_pool_server, args=(server, auth_path, connections), daemon=True).start() + + endpoint = f"{host}:{port}" + try: + for _ in range(2): + pool.close_all() # drop the pool before each call → no reuse + with pytest.raises(RuntimeError, match="browser|connected"): + send_command("tabs.list", remote=endpoint, profile="default", key=key_path) + assert len(connections) == 2 # each command handshaked its own connection + finally: + pool.close_all() + server.close() diff --git a/tests/test_serve.py b/tests/test_serve.py index 1fe21a2..825e35a 100644 --- a/tests/test_serve.py +++ b/tests/test_serve.py @@ -13,518 +13,640 @@ from browser_cli.commands.serve import _handle_client FAKE_UA = "browser-cli/0.9.3" - # ── helpers ──────────────────────────────────────────────────────────────────── def _send_framed(sock: socket.socket, data: bytes) -> None: - sock.sendall(struct.pack(" dict: - raw = b"" - while len(raw) < 4: - chunk = sock.recv(4 - len(raw)) - if not chunk: - raise ConnectionError("socket closed before response header") - raw += chunk - n = struct.unpack(" threading.Thread: - t = threading.Thread( - target=_handle_client, - args=(server_sock, ("127.0.0.1", 9999), None, auth_keys_path), - daemon=True, - ) - t.start() - return t +def _spawn(server_sock: socket.socket, auth_keys_path, security=None) -> threading.Thread: + t = threading.Thread( + target=_handle_client, + args=(server_sock, ("127.0.0.1", 9999), None, auth_keys_path, True, security), + daemon=True, + ) + t.start() + return t def _pair(): - return socket.socketpair() + return socket.socketpair() def _mock_no_browser(*_args, **_kwargs): - raise BrowserNotConnected("no browser") + raise BrowserNotConnected("no browser") # ── challenge frame ──────────────────────────────────────────────────────────── class TestChallenge: - def test_challenge_sent_on_connect(self): - client, server = _pair() - t = _spawn(server, None) - challenge = _recv_framed(client) - assert challenge["type"] == "challenge" - assert "nonce" in challenge - client.close() - t.join(timeout=2) + def test_challenge_sent_on_connect(self): + client, server = _pair() + t = _spawn(server, None) + challenge = _recv_framed(client) + assert challenge["type"] == "challenge" + assert "nonce" in challenge + client.close() + t.join(timeout=2) - def test_challenge_includes_version_fields(self): - client, server = _pair() - t = _spawn(server, None) - challenge = _recv_framed(client) - assert "server_version" in challenge - assert "min_client_version" in challenge - client.close() - t.join(timeout=2) + def test_challenge_includes_version_fields(self): + client, server = _pair() + t = _spawn(server, None) + challenge = _recv_framed(client) + assert "server_version" in challenge + assert "min_client_version" in challenge + client.close() + t.join(timeout=2) - def test_challenge_advertises_post_quantum_kex_when_available(self, tmp_path, monkeypatch): - monkeypatch.setattr("browser_cli.auth.pq_kex_server_keypair", lambda: ("fake-private", b"fake-public")) - path = tmp_path / "authorized_keys" - _, pub = generate_keypair() - path.write_text(pub + "\n") + def test_challenge_advertises_post_quantum_kex_when_available(self, tmp_path, monkeypatch): + monkeypatch.setattr("browser_cli.auth.pq_kex_server_keypair", lambda: ("fake-private", b"fake-public")) + path = tmp_path / "authorized_keys" + _, pub = generate_keypair() + path.write_text(pub + "\n") - client, server = _pair() - t = _spawn(server, path) - challenge = _recv_framed(client) + client, server = _pair() + t = _spawn(server, path) + challenge = _recv_framed(client) - assert challenge["pq_kex"] == {"alg": "ML-KEM-768", "public_key": b"fake-public".hex()} - client.close() - t.join(timeout=2) + assert challenge["pq_kex"] == {"alg": "ML-KEM-768", "public_key": b"fake-public".hex()} + client.close() + t.join(timeout=2) # ── rejection paths ──────────────────────────────────────────────────────────── class TestRejection: - def _connect(self, auth_keys_path): - client, server = _pair() - t = _spawn(server, auth_keys_path) - challenge = _recv_framed(client) - return client, t, challenge + def _connect(self, auth_keys_path): + client, server = _pair() + t = _spawn(server, auth_keys_path) + challenge = _recv_framed(client) + return client, t, challenge - def test_bad_user_agent_rejected(self): - client, t, _ = self._connect(None) - msg = {"id": "x", "command": "tabs.list", "args": {}, "user_agent": "curl/7.88"} - _send_framed(client, json.dumps(msg).encode()) - resp = _recv_framed(client) - assert resp["success"] is False - assert "forbidden" in resp["error"].lower() or "client" in resp["error"].lower() - client.close() - t.join(timeout=2) + def test_bad_user_agent_rejected(self): + client, t, _ = self._connect(None) + msg = {"id": "x", "command": "tabs.list", "args": {}, "user_agent": "curl/7.88"} + _send_framed(client, json.dumps(msg).encode()) + resp = _recv_framed(client) + assert resp["success"] is False + assert "forbidden" in resp["error"].lower() or "client" in resp["error"].lower() + client.close() + t.join(timeout=2) - def test_missing_pubkey_sig_rejected(self, tmp_path): - path = tmp_path / "authorized_keys" - _, pub = generate_keypair() - path.write_text(pub + "\n") - client, t, _ = self._connect(path) - msg = {"id": "x", "command": "tabs.list", "args": {}, "user_agent": FAKE_UA} - _send_framed(client, json.dumps(msg).encode()) - resp = _recv_framed(client) - assert resp["success"] is False - assert "unauthorized" in resp["error"].lower() - client.close() - t.join(timeout=2) + def test_missing_pubkey_sig_rejected(self, tmp_path): + path = tmp_path / "authorized_keys" + _, pub = generate_keypair() + path.write_text(pub + "\n") + client, t, _ = self._connect(path) + msg = {"id": "x", "command": "tabs.list", "args": {}, "user_agent": FAKE_UA} + _send_framed(client, json.dumps(msg).encode()) + resp = _recv_framed(client) + assert resp["success"] is False + assert "unauthorized" in resp["error"].lower() + client.close() + t.join(timeout=2) - def test_untrusted_pubkey_rejected(self, tmp_path): - path = tmp_path / "authorized_keys" - _, trusted_pub = generate_keypair() - path.write_text(trusted_pub + "\n") + def test_untrusted_pubkey_rejected(self, tmp_path): + path = tmp_path / "authorized_keys" + _, trusted_pub = generate_keypair() + path.write_text(trusted_pub + "\n") - pem, untrusted_pub = generate_keypair() - key_path = tmp_path / "other.pem" - key_path.write_bytes(pem) - priv = load_private_key(key_path) + pem, untrusted_pub = generate_keypair() + key_path = tmp_path / "other.pem" + key_path.write_bytes(pem) + priv = load_private_key(key_path) - client, t, challenge = self._connect(path) - nonce = bytes.fromhex(challenge["nonce"]) - msg = {"id": "x", "command": "tabs.list", "args": {}, "user_agent": FAKE_UA, "pubkey": untrusted_pub} - msg["sig"] = sign(priv, nonce, msg).hex() - _send_framed(client, json.dumps(msg).encode()) - resp = _recv_framed(client) - assert resp["success"] is False - assert "untrusted" in resp["error"].lower() - client.close() - t.join(timeout=2) + client, t, challenge = self._connect(path) + nonce = bytes.fromhex(challenge["nonce"]) + msg = {"id": "x", "command": "tabs.list", "args": {}, "user_agent": FAKE_UA, "pubkey": untrusted_pub} + msg["sig"] = sign(priv, nonce, msg).hex() + _send_framed(client, json.dumps(msg).encode()) + resp = _recv_framed(client) + assert resp["success"] is False + assert "untrusted" in resp["error"].lower() + client.close() + t.join(timeout=2) - def test_bad_signature_rejected(self, tmp_path): - path = tmp_path / "authorized_keys" - _, pub = generate_keypair() - path.write_text(pub + "\n") - client, t, _ = self._connect(path) - msg = {"id": "x", "command": "tabs.list", "args": {}, "user_agent": FAKE_UA, "pubkey": pub, "sig": "00" * 64} - _send_framed(client, json.dumps(msg).encode()) - resp = _recv_framed(client) - assert resp["success"] is False - assert "signature" in resp["error"].lower() or "invalid" in resp["error"].lower() - client.close() - t.join(timeout=2) + def test_bad_signature_rejected(self, tmp_path): + path = tmp_path / "authorized_keys" + _, pub = generate_keypair() + path.write_text(pub + "\n") + client, t, _ = self._connect(path) + msg = {"id": "x", "command": "tabs.list", "args": {}, "user_agent": FAKE_UA, "pubkey": pub, "sig": "00" * 64} + _send_framed(client, json.dumps(msg).encode()) + resp = _recv_framed(client) + assert resp["success"] is False + assert "signature" in resp["error"].lower() or "invalid" in resp["error"].lower() + client.close() + t.join(timeout=2) - def test_missing_post_quantum_kex_rejected_when_required(self, tmp_path, monkeypatch): - monkeypatch.setattr("browser_cli.auth.pq_kex_server_keypair", lambda: ("fake-private", b"fake-public")) - path = tmp_path / "authorized_keys" - pem, pub = generate_keypair() - path.write_text(pub + "\n") - priv_path = tmp_path / "client.pem" - priv_path.write_bytes(pem) - priv = load_private_key(priv_path) + def test_missing_post_quantum_kex_rejected_when_required(self, tmp_path, monkeypatch): + monkeypatch.setattr("browser_cli.auth.pq_kex_server_keypair", lambda: ("fake-private", b"fake-public")) + path = tmp_path / "authorized_keys" + pem, pub = generate_keypair() + path.write_text(pub + "\n") + priv_path = tmp_path / "client.pem" + priv_path.write_bytes(pem) + priv = load_private_key(priv_path) - client, t, challenge = self._connect(path) - nonce = bytes.fromhex(challenge["nonce"]) - msg = {"id": "x", "command": "tabs.list", "args": {}, "user_agent": "browser-cli/0.9.5", "pubkey": pub} - msg["sig"] = sign(priv, nonce, msg).hex() - _send_framed(client, json.dumps(msg).encode()) - resp = _recv_framed(client) + client, t, challenge = self._connect(path) + nonce = bytes.fromhex(challenge["nonce"]) + msg = {"id": "x", "command": "tabs.list", "args": {}, "user_agent": "browser-cli/0.9.5", "pubkey": pub} + msg["sig"] = sign(priv, nonce, msg).hex() + _send_framed(client, json.dumps(msg).encode()) + resp = _recv_framed(client) - assert resp["success"] is False - assert "post-quantum" in resp["error"].lower() - client.close() - t.join(timeout=2) + assert resp["success"] is False + assert "post-quantum" in resp["error"].lower() + client.close() + t.join(timeout=2) - def test_oversized_message_rejected(self): - client, server = _pair() - t = _spawn(server, None) - _recv_framed(client) # consume challenge - client.sendall(struct.pack(" bytes: - raw = b"" - while len(raw) < 4: - chunk = sock.recv(4 - len(raw)) - if not chunk: - raise ConnectionError("socket closed before response header") - raw += chunk - n = struct.unpack("