diff --git a/browser_cli/auth.py b/browser_cli/auth.py index 97f956e..94dca13 100644 --- a/browser_cli/auth.py +++ b/browser_cli/auth.py @@ -174,7 +174,7 @@ def verify(pub_hex: str, nonce: bytes, msg: dict, sig_hex: str) -> bool: message = nonce + hashlib.sha256(canonical_payload(msg)).digest() pub_key.verify(bytes.fromhex(sig_hex), message) return True - except (InvalidSignature, Exception): + except (InvalidSignature, ValueError): return False diff --git a/browser_cli/client.py b/browser_cli/client.py index 6014cbf..24844fb 100644 --- a/browser_cli/client.py +++ b/browser_cli/client.py @@ -19,6 +19,7 @@ from pathlib import Path from typing import Any from browser_cli.platform import endpoint_for_alias, is_windows, registry_path +from browser_cli.version_manager import MAX_MSG_BYTES as _MAX_MSG_BYTES from browser_cli.registry import load_registry try: @@ -72,7 +73,7 @@ def _load_remotes() -> dict[str, dict[str, str]]: def _is_valid_key_spec(s: str) -> bool: """Return True if s looks like a usable key spec: 'agent', 'agent:', or a file path.""" - return s == "agent" or s.startswith("agent:") or (not s.startswith("<") and "/" in s or Path(s).suffix in {".pem", ".key"}) + return s == "agent" or s.startswith("agent:") or (not s.startswith("<") and ("/" in s or Path(s).suffix in {".pem", ".key"})) def save_remote_key(endpoint: str, key_spec: str) -> None: @@ -327,22 +328,22 @@ def send_command(command: str, args: dict | None = None, profile: str | None = N msg["_route"] = route_profile else: private_key = None - payload = json.dumps(msg).encode("utf-8") - framed = struct.pack(" bytes: raw_len = _recv_exact(sock, 4) msg_len = struct.unpack(" None: if not groups: console.print("[yellow]No groups found[/yellow]") diff --git a/browser_cli/commands/navigate.py b/browser_cli/commands/navigate.py index 70bcbed..ef31b1f 100644 --- a/browser_cli/commands/navigate.py +++ b/browser_cli/commands/navigate.py @@ -1,21 +1,10 @@ import click -from browser_cli.client import send_command, BrowserNotConnected +from browser_cli.commands import _handle from rich.console import Console console = Console() -def _handle(command, args): - try: - return send_command(command, args) - except BrowserNotConnected as e: - console.print(f"[red]Error:[/red] {e}") - raise SystemExit(1) - except RuntimeError as e: - console.print(f"[red]Browser error:[/red] {e}") - raise SystemExit(1) - - @click.group("nav") def nav_group(): """Navigate — open URLs, reload, go back/forward, focus tabs.""" diff --git a/browser_cli/commands/page.py b/browser_cli/commands/page.py index 2579cc4..99cd4de 100644 --- a/browser_cli/commands/page.py +++ b/browser_cli/commands/page.py @@ -1,22 +1,11 @@ import click -from browser_cli.client import send_command, BrowserNotConnected +from browser_cli.commands import _handle from rich.console import Console from rich.table import Table console = Console() -def _handle(command, args=None): - try: - return send_command(command, args or {}) - except BrowserNotConnected as e: - console.print(f"[red]Error:[/red] {e}") - raise SystemExit(1) - except RuntimeError as e: - console.print(f"[red]Browser error:[/red] {e}") - raise SystemExit(1) - - @click.group("page") def page_group(): """Inspect current page metadata.""" diff --git a/browser_cli/commands/search.py b/browser_cli/commands/search.py index 40fb08f..f0aaeaf 100644 --- a/browser_cli/commands/search.py +++ b/browser_cli/commands/search.py @@ -1,6 +1,6 @@ import click from urllib.parse import quote_plus -from browser_cli.client import send_command, BrowserNotConnected +from browser_cli.commands import _handle from rich.console import Console console = Console() @@ -71,14 +71,7 @@ def _build_command(engine_key: str, help_text: str) -> click.Command: def _cmd(query, bg, window, group): terms = " ".join(query) url = ENGINES[engine_key].format(query=quote_plus(terms)) - try: - send_command("navigate.open", {"url": url, "background": bg, "window": window, "group": group}) - except BrowserNotConnected as e: - console.print(f"[red]Error:[/red] {e}") - raise SystemExit(1) - except RuntimeError as e: - console.print(f"[red]Browser error:[/red] {e}") - raise SystemExit(1) + _handle("navigate.open", {"url": url, "background": bg, "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()) console.print(f"[green]Searching[/green] [cyan]{display}[/cyan]: {terms}{suffix}") diff --git a/browser_cli/commands/serve.py b/browser_cli/commands/serve.py index d471c33..8833409 100644 --- a/browser_cli/commands/serve.py +++ b/browser_cli/commands/serve.py @@ -1,25 +1,20 @@ -import re, threading, secrets, socket, struct, click, json, sys, os +import re, threading, secrets, socket, struct, click, json, sys +from datetime import datetime from pathlib import Path -from browser_cli.version_manager import PROTOCOL_MIN_CLIENT, parse_version, get_installed_version -from browser_cli.compat import adapt_request, adapt_response + +from rich.console import Console + +from browser_cli.client import _recv_exact, _recv_all +from browser_cli.compat import adapt_auth, adapt_request, adapt_response +from browser_cli.version_manager import PROTOCOL_MIN_CLIENT, MAX_MSG_BYTES, parse_version, get_installed_version _UA_PATTERN = re.compile(r"^browser-cli/\d") - _CONN_LIMIT = threading.BoundedSemaphore(64) -_MAX_MSG_BYTES = 32 * 1024 * 1024 -from rich.console import Console -from datetime import datetime - console = Console() -def _recv_exact(sock:socket.socket, n:int) -> bytes: - buf = b"" - while len(buf) < n: - chunk = sock.recv(n - len(buf)) - if not chunk: - raise ConnectionError("Connection closed") - buf += chunk - return buf + +def _framed_send(sock: socket.socket, data: bytes) -> None: + sock.sendall(struct.pack(" None: ts = datetime.now().strftime("%H:%M:%S") @@ -37,14 +32,21 @@ def _proxy_request(client_sock:socket.socket, addr:tuple, profile:str|None, auth def _send_error(msg_id, msg:str) -> None: err = json.dumps({"id": msg_id, "success": False, "error": msg}).encode() try: - client_sock.sendall(struct.pack(" None: + out = json.dumps({"id": msg_id, "success": True, "data": payload}).encode() + try: + _framed_send(client_sock, out) except OSError: pass try: header = _recv_exact(client_sock, 4) msg_len = struct.unpack(" _MAX_MSG_BYTES: + if msg_len > MAX_MSG_BYTES: _send_error(None, f"message too large ({msg_len} bytes)") return payload = _recv_exact(client_sock, msg_len) @@ -58,25 +60,26 @@ def _proxy_request(client_sock:socket.socket, addr:tuple, profile:str|None, auth _log(addr, "?", None, "ERROR", "invalid JSON") return - msg_id = msg.get("id") - command = msg.get("command", "?") - # ── user-agent + version check ──────────────────────────────────────────── + msg_id = msg.get("id") ua = msg.get("user_agent") or "" if not _UA_PATTERN.match(ua): _send_error(msg_id, "forbidden: client required") - _log(addr, command, None, "DENIED", f"bad user-agent: {ua!r}") + _log(addr, msg.get("command", "?"), None, "DENIED", f"bad user-agent: {ua!r}") return client_ver = "0" try: client_ver = ua.split("/", 1)[1] if parse_version(client_ver) < parse_version(PROTOCOL_MIN_CLIENT): _send_error(msg_id, f"client version {client_ver} is too old; please upgrade to >= {PROTOCOL_MIN_CLIENT}") - _log(addr, command, None, "DENIED", f"client {client_ver} < min {PROTOCOL_MIN_CLIENT}") + _log(addr, msg.get("command", "?"), None, "DENIED", f"client {client_ver} < min {PROTOCOL_MIN_CLIENT}") return except (IndexError, ValueError): pass + msg = adapt_auth(msg, client_ver) + command = msg.get("command", "?") + # ── auth ────────────────────────────────────────────────────────────────── if auth_keys is not None: pub = msg.get("pubkey") or "" @@ -101,8 +104,7 @@ def _proxy_request(client_sock:socket.socket, addr:tuple, profile:str|None, auth {"profile": target.profile, "displayName": target.display_name} for target in active_browser_targets(include_remotes=False) ] - data = json.dumps({"id": msg_id, "success": True, "data": targets}).encode() - client_sock.sendall(struct.pack(" None: if not tabs: console.print("[yellow]No tabs found[/yellow]") diff --git a/browser_cli/commands/windows.py b/browser_cli/commands/windows.py index a4e8bf9..3d814e3 100644 --- a/browser_cli/commands/windows.py +++ b/browser_cli/commands/windows.py @@ -1,46 +1,11 @@ import click -from browser_cli.client import BrowserNotConnected, active_browser_targets, remote_browser_targets, send_command +from browser_cli.commands import _handle, _handle_multi, _multi_browser_targets from rich.console import Console from rich.table import Table console = Console() -def _handle(command, args=None, profile=None): - try: - return send_command(command, args or {}, profile=profile) - except BrowserNotConnected as e: - console.print(f"[red]Error:[/red] {e}") - raise SystemExit(1) - except RuntimeError as e: - console.print(f"[red]Browser error:[/red] {e}") - raise SystemExit(1) - - -def _handle_multi(command, args=None, profile=None, remote=None): - try: - if remote: - return send_command(command, args or {}, profile=profile, remote=remote) - return send_command(command, args or {}, profile=profile) - except (BrowserNotConnected, RuntimeError): - return None - - -def _multi_browser_targets(): - root = click.get_current_context().find_root() - if root.obj.get("browser_explicit"): - return [] - remote = root.obj.get("remote") - key = root.obj.get("key") - if remote: - targets = remote_browser_targets(remote, key=key) - else: - targets = active_browser_targets(key=key) - if len(targets) <= 1 and not any(target.remote for target in targets): - return [] - return targets - - def _print_windows(windows: list[dict], *, show_browser: bool = False) -> None: if not windows: console.print("[yellow]No windows found[/yellow]") diff --git a/browser_cli/compat.py b/browser_cli/compat.py deleted file mode 100644 index 2851b27..0000000 --- a/browser_cli/compat.py +++ /dev/null @@ -1,49 +0,0 @@ -""" -Stripe-style version compatibility layer for browser-cli serve. - -When a behaviour-breaking change ships in a new server version, add one entry -to _COMPAT below: - - ("X.Y.Z", request_fn, response_fn) - -- ``request_fn(msg: dict) -> dict`` - Upgrade an incoming client message from a client older than X.Y.Z to the - current format before forwarding it to the native host. -- ``response_fn(resp: bytes, command: str) -> bytes`` - Downgrade a native-host response to the format a client older than X.Y.Z - expects before sending it back. - -Either function may be ``None`` when only one direction needs adapting. - -Entries must stay in ascending version order. ``adapt_request`` walks forward -(oldest change first); ``adapt_response`` walks backward (newest change first) -so the transformations compose correctly. - -Current baseline: 0.9.1 — no shims needed yet. -""" - -from __future__ import annotations -from typing import Callable -from browser_cli.version_manager import parse_version - -_COMPAT: list[tuple[str, Callable[[dict], dict] | None, Callable[[bytes, str], bytes] | None]] = [ - # ("1.0.0", _req_1_0_0, _resp_1_0_0), -] - - -def adapt_request(msg: dict, client_version: str) -> dict: - """Upgrade a client message to the current server format.""" - cv = parse_version(client_version) - for version, req_fn, _ in _COMPAT: - if cv < parse_version(version) and req_fn is not None: - msg = req_fn(msg) - return msg - - -def adapt_response(resp: bytes, command: str, client_version: str) -> bytes: - """Downgrade a server response to the format the client expects.""" - cv = parse_version(client_version) - for version, _, resp_fn in reversed(_COMPAT): - if cv < parse_version(version) and resp_fn is not None: - resp = resp_fn(resp, command) - return resp diff --git a/browser_cli/compat/__init__.py b/browser_cli/compat/__init__.py new file mode 100644 index 0000000..a32d711 --- /dev/null +++ b/browser_cli/compat/__init__.py @@ -0,0 +1,4 @@ +from browser_cli.compat.commands import adapt_request, adapt_response +from browser_cli.compat.auth import adapt_auth + +__all__ = ["adapt_auth", "adapt_request", "adapt_response"] diff --git a/browser_cli/compat/auth.py b/browser_cli/compat/auth.py new file mode 100644 index 0000000..c7a6eb0 --- /dev/null +++ b/browser_cli/compat/auth.py @@ -0,0 +1,44 @@ +""" +Auth-field normalizers — applied to the raw incoming message *before* the +auth check runs. Protocol fields (pubkey, sig, …) are still present here. + +Add one entry per breaking auth-field change: + ("X.Y.Z", transformer_fn) + +Entries must stay in ascending version order. +""" +from __future__ import annotations +from typing import Callable +from browser_cli.version_manager import parse_version + + +# ── v0.9.3 ──────────────────────────────────────────────────────────────────── + +def _auth_0_9_3(msg: dict) -> dict: + """pubkey validation tightened to lowercase hex; normalize for older clients.""" + changed: dict = {} + pk = msg.get("pubkey") + if isinstance(pk, str) and pk: + changed["pubkey"] = pk.lower() + if msg.get("command") == "browser-cli.auth.trust": + args = msg.get("args") or {} + trust_pk = args.get("pubkey") + if isinstance(trust_pk, str) and trust_pk: + changed["args"] = {**args, "pubkey": trust_pk.lower()} + return {**msg, **changed} if changed else msg + + +# ── registry ────────────────────────────────────────────────────────────────── + +_AUTH_COMPAT: list[tuple[str, Callable[[dict], dict]]] = [ + ("0.9.3", _auth_0_9_3), +] + + +def adapt_auth(msg: dict, client_version: str) -> dict: + """Apply all auth normalizers needed to bring msg up to the current format.""" + cv = parse_version(client_version) + for version, fn in _AUTH_COMPAT: + if cv < parse_version(version): + msg = fn(msg) + return msg diff --git a/browser_cli/compat/commands.py b/browser_cli/compat/commands.py new file mode 100644 index 0000000..b798c0b --- /dev/null +++ b/browser_cli/compat/commands.py @@ -0,0 +1,43 @@ +""" +Command-format shims — applied to clean_msg (protocol fields already stripped) +before forwarding to the native host, and to responses before sending back. + +Add one entry per breaking command-format change: + ("X.Y.Z", request_fn, response_fn) + +- request_fn(msg: dict) -> dict or None +- response_fn(resp: bytes, command: str) -> bytes or None + +Entries must stay in ascending version order. +adapt_request walks forward (oldest first); adapt_response walks backward. + +Current baseline: 0.9.3 — no command-format shims needed yet. +""" +from __future__ import annotations +from typing import Callable +from browser_cli.version_manager import parse_version + + +# ── registry ────────────────────────────────────────────────────────────────── + +_COMPAT: list[tuple[str, Callable[[dict], dict] | None, Callable[[bytes, str], bytes] | None]] = [ + # ("1.0.0", _req_1_0_0, _resp_1_0_0), +] + + +def adapt_request(msg: dict, client_version: str) -> dict: + """Upgrade a client message to the current browser command format.""" + cv = parse_version(client_version) + for version, req_fn, _ in _COMPAT: + if cv < parse_version(version) and req_fn is not None: + msg = req_fn(msg) + return msg + + +def adapt_response(resp: bytes, command: str, client_version: str) -> bytes: + """Downgrade a native-host response to the format the client expects.""" + cv = parse_version(client_version) + for version, _, resp_fn in reversed(_COMPAT): + if cv < parse_version(version) and resp_fn is not None: + resp = resp_fn(resp, command) + return resp diff --git a/browser_cli/native_host.py b/browser_cli/native_host.py index c10d486..186385d 100644 --- a/browser_cli/native_host.py +++ b/browser_cli/native_host.py @@ -19,6 +19,7 @@ from multiprocessing.connection import Listener from pathlib import Path from browser_cli.platform import DEFAULT_ALIAS, endpoint_for_alias, is_windows, registry_path, runtime_dir +from browser_cli.version_manager import MAX_MSG_BYTES as _MAX_MSG_BYTES from browser_cli.registry import update_registry SOCKET_PATH: str = "" # set after hello handshake @@ -278,6 +279,8 @@ def _recv_all(conn: socket.socket) -> bytes | None: if raw_len is None: return None msg_len = struct.unpack(" _MAX_MSG_BYTES: + return None return _recv_exact(conn, msg_len) diff --git a/browser_cli/version_manager.py b/browser_cli/version_manager.py index e8876ce..be8653a 100644 --- a/browser_cli/version_manager.py +++ b/browser_cli/version_manager.py @@ -1,6 +1,7 @@ from importlib.metadata import version as _pkg_version PROTOCOL_MIN_CLIENT = "0.9.0" +MAX_MSG_BYTES = 32 * 1024 * 1024 def parse_version(v: str) -> tuple[int, ...]: diff --git a/pyproject.toml b/pyproject.toml index 34f6d9c..1fc348c 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,6 +1,6 @@ [project] name = "browser-cli" -version = "0.9.2" +version = "0.9.3" description = "Control your real running browser from the terminal via a browser extension" requires-python = ">=3.10" dependencies = [ diff --git a/tests/test_auth.py b/tests/test_auth.py new file mode 100644 index 0000000..1ffc010 --- /dev/null +++ b/tests/test_auth.py @@ -0,0 +1,160 @@ +import json +import tempfile +from pathlib import Path + +import pytest + +from browser_cli.auth import ( + add_authorized_key, + canonical_payload, + generate_keypair, + load_authorized_keys, + load_authorized_keys_with_names, + load_private_key, + new_nonce, + sign, + verify, +) +from browser_cli.client import _is_valid_key_spec + + +class TestGenerateKeypair: + def test_returns_pem_and_hex(self): + pem, pub_hex = generate_keypair() + assert pem.startswith(b"-----BEGIN PRIVATE KEY-----") + assert len(pub_hex) == 64 + + def test_each_call_unique(self): + _, pub1 = generate_keypair() + _, pub2 = generate_keypair() + assert pub1 != pub2 + + +class TestCanonicalPayload: + def test_strips_pubkey_and_sig(self): + msg = {"command": "tabs.list", "id": "x", "pubkey": "abc", "sig": "def"} + data = json.loads(canonical_payload(msg)) + assert "pubkey" not in data + assert "sig" not in data + + def test_keys_sorted(self): + msg = {"z": 1, "a": 2, "m": 3} + payload = canonical_payload(msg).decode() + assert payload.index('"a"') < payload.index('"m"') < payload.index('"z"') + + def test_deterministic(self): + msg = {"b": 2, "a": 1} + assert canonical_payload(msg) == canonical_payload(msg) + + +@pytest.fixture() +def keypair(tmp_path): + pem, pub_hex = generate_keypair() + key_path = tmp_path / "client.key.pem" + key_path.write_bytes(pem) + priv = load_private_key(key_path) + return priv, pub_hex + + +class TestSignVerify: + def test_valid_signature_verifies(self, keypair): + priv, pub_hex = keypair + nonce = bytes.fromhex(new_nonce()) + msg = {"command": "tabs.list", "id": "uuid-1", "args": {}} + sig = sign(priv, nonce, msg).hex() + assert verify(pub_hex, nonce, msg, sig) is True + + def test_tampered_sig_fails(self, keypair): + priv, pub_hex = keypair + nonce = bytes.fromhex(new_nonce()) + msg = {"command": "tabs.list", "id": "x"} + sign(priv, nonce, msg) + assert verify(pub_hex, nonce, msg, "00" * 64) is False + + def test_wrong_pubkey_fails(self, keypair): + priv, _ = keypair + _, other_pub = generate_keypair() + nonce = bytes.fromhex(new_nonce()) + msg = {"command": "tabs.list"} + sig = sign(priv, nonce, msg).hex() + assert verify(other_pub, nonce, msg, sig) is False + + def test_wrong_nonce_fails(self, keypair): + priv, pub_hex = keypair + nonce = bytes.fromhex(new_nonce()) + msg = {"command": "tabs.list"} + sig = sign(priv, nonce, msg).hex() + other_nonce = bytes.fromhex(new_nonce()) + assert verify(pub_hex, other_nonce, msg, sig) is False + + def test_garbage_pub_hex_returns_false_not_exception(self): + assert verify("not-hex!!!!", b"nonce", {}, "00" * 64) is False + + def test_truncated_sig_hex_returns_false_not_exception(self, keypair): + _, pub_hex = keypair + assert verify(pub_hex, b"nonce", {}, "aabb") is False + + def test_wrong_length_pubkey_returns_false_not_exception(self): + assert verify("aabbcc", b"nonce", {}, "00" * 64) is False + + +class TestAuthorizedKeys: + def test_add_and_load(self, tmp_path): + path = tmp_path / "authorized_keys" + _, pub = generate_keypair() + assert add_authorized_key(path, pub, "alice") is True + assert pub in load_authorized_keys(path) + + def test_add_duplicate_returns_false(self, tmp_path): + path = tmp_path / "authorized_keys" + _, pub = generate_keypair() + add_authorized_key(path, pub) + assert add_authorized_key(path, pub) is False + + def test_load_with_names(self, tmp_path): + path = tmp_path / "authorized_keys" + _, pub1 = generate_keypair() + _, pub2 = generate_keypair() + add_authorized_key(path, pub1, "alice") + add_authorized_key(path, pub2) + entries = load_authorized_keys_with_names(path) + assert (pub1, "alice") in entries + assert (pub2, "") in entries + + def test_ignores_comment_lines(self, tmp_path): + path = tmp_path / "authorized_keys" + path.write_text("# this is a comment\n") + assert load_authorized_keys(path) == [] + + def test_returns_empty_for_missing_file(self, tmp_path): + assert load_authorized_keys(tmp_path / "nofile") == [] + + +class TestIsValidKeySpec: + def test_agent_bare(self): + assert _is_valid_key_spec("agent") is True + + def test_agent_with_selector(self): + assert _is_valid_key_spec("agent:cardno:000012345678") is True + + def test_absolute_pem_path(self): + assert _is_valid_key_spec("/home/user/.config/browser-cli/client.key.pem") is True + + def test_dot_key_extension(self): + assert _is_valid_key_spec("/tmp/mykey.key") is True + + def test_angled_bracket_pem_rejected(self): + # regression: operator precedence bug allowed ".pem" to pass + assert _is_valid_key_spec(".pem") is False + + def test_angled_bracket_key_rejected(self): + assert _is_valid_key_spec(".key") is False + + def test_serialized_object_rejected(self): + assert _is_valid_key_spec(".pem") is False + + def test_empty_string_rejected(self): + assert _is_valid_key_spec("") is False + + def test_bare_filename_no_slash_no_ext_rejected(self): + assert _is_valid_key_spec("mykey") is False diff --git a/tests/test_cli.py b/tests/test_cli.py index ef4cb9c..b4c585f 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -262,12 +262,12 @@ def test_tabs_list_multi_browser_shows_browser_column(): return [{"id": 1 if profile == "default" else 2, "windowId": 1, "active": True, "title": profile, "url": "https://example.com"}] with patch( - "browser_cli.commands.tabs.active_browser_targets", + "browser_cli.commands.active_browser_targets", return_value=[ BrowserTarget("default", "550e8400-e29b-41d4-a716-446655440000", "/tmp/default.sock"), BrowserTarget("work", "work", "/tmp/work.sock"), ], - ), patch("browser_cli.commands.tabs.send_command", side_effect=fake_send_command): + ), patch("browser_cli.commands.send_command", side_effect=fake_send_command): result = CliRunner().invoke(main, ["tabs", "list"]) assert result.exit_code == 0 @@ -278,13 +278,13 @@ def test_tabs_list_multi_browser_shows_browser_column(): def test_tabs_list_with_remote_uses_only_remote_targets(): with patch( - "browser_cli.commands.tabs.active_browser_targets", + "browser_cli.commands.active_browser_targets", side_effect=AssertionError("local targets should not be used for explicit remote"), ), patch( - "browser_cli.commands.tabs.remote_browser_targets", + "browser_cli.commands.remote_browser_targets", return_value=[BrowserTarget("work", "remote-host:work", "", remote="remote-host:8765")], ), patch( - "browser_cli.commands.tabs.send_command", + "browser_cli.commands.send_command", return_value=[{"id": 1, "windowId": 1, "active": True, "title": "Remote", "url": "https://example.com"}], ) as send_command: result = CliRunner().invoke(main, ["--remote", "remote-host:8765", "tabs", "list"]) @@ -297,13 +297,13 @@ def test_tabs_list_with_remote_uses_only_remote_targets(): def test_tabs_list_with_explicit_browser_does_not_show_browser_column(): with patch( - "browser_cli.commands.tabs.active_browser_targets", + "browser_cli.commands.active_browser_targets", return_value=[ BrowserTarget("default", "uuid-1", "/tmp/default.sock"), BrowserTarget("work", "work", "/tmp/work.sock"), ], ), patch( - "browser_cli.commands.tabs.send_command", + "browser_cli.commands.send_command", return_value=[{"id": 1, "windowId": 1, "active": True, "title": "Example", "url": "https://example.com"}], ) as send_command: result = CliRunner().invoke(main, ["--browser", "work", "tabs", "list"]) @@ -322,12 +322,12 @@ def test_tabs_count_multi_browser_shows_total(): return counts[profile] with patch( - "browser_cli.commands.tabs.active_browser_targets", + "browser_cli.commands.active_browser_targets", return_value=[ BrowserTarget("default", "uuid-1", "/tmp/default.sock"), BrowserTarget("work", "work", "/tmp/work.sock"), ], - ), patch("browser_cli.commands.tabs.send_command", side_effect=fake_send_command): + ), patch("browser_cli.commands.send_command", side_effect=fake_send_command): result = CliRunner().invoke(main, ["tabs", "count", "github"]) assert result.exit_code == 0 @@ -344,12 +344,12 @@ def test_group_count_multi_browser_shows_total(): return counts[profile] with patch( - "browser_cli.commands.groups.active_browser_targets", + "browser_cli.commands.active_browser_targets", return_value=[ BrowserTarget("default", "uuid-1", "/tmp/default.sock"), BrowserTarget("work", "work", "/tmp/work.sock"), ], - ), patch("browser_cli.commands.groups.send_command", side_effect=fake_send_command): + ), patch("browser_cli.commands.send_command", side_effect=fake_send_command): result = CliRunner().invoke(main, ["groups", "count"]) assert result.exit_code == 0 @@ -360,7 +360,7 @@ def test_group_count_multi_browser_shows_total(): def test_group_list_leaves_unnamed_group_cell_empty(): with patch( - "browser_cli.commands.groups.send_command", + "browser_cli.commands.send_command", return_value=[{"id": 42, "title": "", "color": "grey", "collapsed": False, "tabCount": 1}], ): result = CliRunner().invoke(main, ["groups", "list"]) @@ -372,7 +372,7 @@ def test_group_list_leaves_unnamed_group_cell_empty(): def test_tabs_move_accepts_right_short_alias(): - with patch("browser_cli.commands.tabs.send_command") as send_command: + with patch("browser_cli.commands.send_command") as send_command: result = CliRunner().invoke(main, ["tabs", "move", "12", "-r"]) assert result.exit_code == 0 @@ -384,7 +384,7 @@ def test_tabs_move_accepts_right_short_alias(): def test_groups_move_accepts_left_short_alias(): - with patch("browser_cli.commands.groups.send_command") as send_command: + with patch("browser_cli.commands.send_command") as send_command: result = CliRunner().invoke(main, ["groups", "move", "research", "-l"]) assert result.exit_code == 0 @@ -399,12 +399,12 @@ def test_windows_list_multi_browser_shows_browser_column(): return [{"id": 1, "alias": profile, "focused": True, "tabCount": 2, "state": "normal"}] with patch( - "browser_cli.commands.windows.active_browser_targets", + "browser_cli.commands.active_browser_targets", return_value=[ BrowserTarget("default", "uuid-1", "/tmp/default.sock"), BrowserTarget("work", "work", "/tmp/work.sock"), ], - ), patch("browser_cli.commands.windows.send_command", side_effect=fake_send_command): + ), patch("browser_cli.commands.send_command", side_effect=fake_send_command): result = CliRunner().invoke(main, ["windows", "list"]) assert result.exit_code == 0 @@ -420,12 +420,12 @@ def test_session_list_multi_browser_shows_browser_column(): return [{"name": f"{profile}-session", "tabs": 2, "savedAt": 1712707200000}] with patch( - "browser_cli.commands.session.active_browser_targets", + "browser_cli.commands.active_browser_targets", return_value=[ BrowserTarget("default", "uuid-1", "/tmp/default.sock"), BrowserTarget("work", "work", "/tmp/work.sock"), ], - ), patch("browser_cli.commands.session.send_command", side_effect=fake_send_command): + ), patch("browser_cli.commands.send_command", side_effect=fake_send_command): result = CliRunner().invoke(main, ["session", "list"]) assert result.exit_code == 0 @@ -438,13 +438,13 @@ def test_session_list_multi_browser_shows_browser_column(): def test_session_list_with_explicit_browser_does_not_show_browser_column(): with patch( - "browser_cli.commands.session.active_browser_targets", + "browser_cli.commands.active_browser_targets", return_value=[ BrowserTarget("default", "uuid-1", "/tmp/default.sock"), BrowserTarget("work", "work", "/tmp/work.sock"), ], ), patch( - "browser_cli.commands.session.send_command", + "browser_cli.commands.send_command", return_value=[{"name": "work-session", "tabs": 2, "savedAt": 1712707200000}], ) as send_command: result = CliRunner().invoke(main, ["--browser", "work", "session", "list"]) @@ -455,7 +455,7 @@ def test_session_list_with_explicit_browser_does_not_show_browser_column(): def test_windows_open_passes_url(): - with patch("browser_cli.commands.windows.send_command", return_value={"id": 7}) as send_command: + with patch("browser_cli.commands.send_command", return_value={"id": 7}) as send_command: result = CliRunner().invoke(main, ["windows", "open", "https://example.com"]) assert result.exit_code == 0 @@ -463,20 +463,20 @@ def test_windows_open_passes_url(): send_command.assert_called_once_with("windows.open", {"url": "https://example.com"}, profile=None) def test_extract_markdown_command(): - with patch("browser_cli.commands.extract.send_command", return_value="# Title") as send_command: + with patch("browser_cli.commands.send_command", return_value="# Title") as send_command: result = CliRunner().invoke(main, ["extract", "markdown"]) assert result.exit_code == 0 assert result.output == "# Title\n" - send_command.assert_called_once_with("extract.markdown", {"selector": None}) + send_command.assert_called_once_with("extract.markdown", {"selector": None}, profile=None) def test_extract_markdown_command_with_selector(): - with patch("browser_cli.commands.extract.send_command", return_value="## Post") as send_command: + with patch("browser_cli.commands.send_command", return_value="## Post") as send_command: result = CliRunner().invoke(main, ["extract", "markdown", "--selector", "article"]) assert result.exit_code == 0 assert result.output == "## Post\n" - send_command.assert_called_once_with("extract.markdown", {"selector": "article"}) + send_command.assert_called_once_with("extract.markdown", {"selector": "article"}, profile=None) def test_clean_markdown_output_removes_escaped_underscores_and_dashes(): @@ -561,7 +561,7 @@ def test_extract_markdown_command_repairs_malformed_tables_and_code_blocks(): "Golden Set │ ▼Promptfoo(Testausführung) │ ▼Plattformen├ Omnifact└ Le Chat\n" "```" ) - with patch("browser_cli.commands.extract.send_command", return_value=raw): + with patch("browser_cli.commands.send_command", return_value=raw): result = CliRunner().invoke(main, ["extract", "markdown"]) assert result.exit_code == 0 @@ -639,8 +639,8 @@ def test_tabs_list_multi_browser_queries_remote_target(): remote=endpoint, ) - with patch("browser_cli.commands.tabs.active_browser_targets", return_value=[remote_target, BrowserTarget("local", "local", "/tmp/local.sock")]), patch( - "browser_cli.commands.tabs.send_command", + with patch("browser_cli.commands.active_browser_targets", return_value=[remote_target, BrowserTarget("local", "local", "/tmp/local.sock")]), patch( + "browser_cli.commands.send_command", return_value=[{"id": 1, "windowId": 1, "active": True, "title": "Remote", "url": "https://example.com"}], ) as send_command: result = CliRunner().invoke(main, ["tabs", "list"]) diff --git a/tests/test_serve.py b/tests/test_serve.py new file mode 100644 index 0000000..2649173 --- /dev/null +++ b/tests/test_serve.py @@ -0,0 +1,249 @@ +"""Unit tests for the TCP serve layer (challenge-response auth, framing, rejection paths).""" +import json +import socket +import struct +import threading + +import pytest + +from browser_cli.auth import generate_keypair, load_private_key, new_nonce, sign +from browser_cli.client import BrowserNotConnected +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 _pair(): + return socket.socketpair() + + +def _mock_no_browser(*_args, **_kwargs): + 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_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) + + +# ── 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 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_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) + + 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_oversized_message_rejected(self): + client, server = _pair() + t = _spawn(server, None) + _recv_framed(client) # consume challenge + client.sendall(struct.pack("