From a8421e97f51c323c02e9901c9fe5274a918aec42 Mon Sep 17 00:00:00 2001 From: Daniel Dolezal Date: Sat, 2 May 2026 15:03:01 +0200 Subject: [PATCH] fix: harden IPC, screenshot, paging, and tab filter error handling - tabs.py: validate screenshot data URL prefix and catch binascii.Error instead of silently writing a zero-byte file or crashing with a raw traceback - serve.py: add 30 s recv timeout on client connections to prevent unbounded thread accumulation; use hmac.compare_digest for constant-time token check - native_host.py: bind Unix socket before _registry_add to eliminate the window where the registry points to an unbound path; cap paging loop at ceil(10000/PAGE_SIZE) iterations to guard against a misbehaving extension; remove dead no-hello fast-path queue that was registered but never consumed - __init__.py: narrow _apply_tab_filter except to (AttributeError, TypeError) so broken filter functions raise instead of silently returning wrong results Co-Authored-By: Claude Sonnet 4.6 --- browser_cli/__init__.py | 4 ++-- browser_cli/commands/serve.py | 5 ++-- browser_cli/commands/tabs.py | 8 ++++++- browser_cli/native_host.py | 45 ++++++++++++++++++++++------------- 4 files changed, 40 insertions(+), 22 deletions(-) diff --git a/browser_cli/__init__.py b/browser_cli/__init__.py index 9128fc5..4216720 100644 --- a/browser_cli/__init__.py +++ b/browser_cli/__init__.py @@ -685,8 +685,8 @@ class BrowserCLI: try: transformed = filter_fn(tabs) - except Exception: - transformed = None + except (AttributeError, TypeError): + return [tab for tab in tabs if filter_fn(tab)] if isinstance(transformed, list): return transformed diff --git a/browser_cli/commands/serve.py b/browser_cli/commands/serve.py index d4d6266..f21c304 100644 --- a/browser_cli/commands/serve.py +++ b/browser_cli/commands/serve.py @@ -1,4 +1,4 @@ -import threading, secrets, socket, struct, click, json, sys +import hmac, threading, secrets, socket, struct, click, json, sys from rich.console import Console from datetime import datetime @@ -51,7 +51,7 @@ def _proxy_request(client_sock:socket.socket, addr:tuple, profile:str|None, serv command = msg.get("command", "?") if server_token is not None: - if msg.get("token") != server_token: + if not hmac.compare_digest(msg.get("token") or "", server_token): _send_error(msg_id, "unauthorized: invalid or missing token") _log(addr, command, None, "DENIED", "bad token") return @@ -110,6 +110,7 @@ def _proxy_request(client_sock:socket.socket, addr:tuple, profile:str|None, serv _log(addr, command, resolved_profile, "ERROR", str(e)) def _handle_client(client_sock:socket.socket, addr:tuple, profile:str|None, server_token:str|None) -> None: + client_sock.settimeout(30) with client_sock: _proxy_request(client_sock, addr, profile, server_token) diff --git a/browser_cli/commands/tabs.py b/browser_cli/commands/tabs.py index b1a4fac..2b7d59a 100644 --- a/browser_cli/commands/tabs.py +++ b/browser_cli/commands/tabs.py @@ -1,4 +1,5 @@ import base64 +import binascii import click from browser_cli.client import BrowserNotConnected, active_browser_targets, remote_browser_targets, send_command from rich.console import Console @@ -288,7 +289,12 @@ def tabs_screenshot(output, tab_id, fmt, quality): data_url = result.get("dataUrl", "") if isinstance(result, dict) else "" if output: header = f"data:image/{fmt};base64," - raw = base64.b64decode(data_url[len(header):]) + if not data_url.startswith(header): + raise click.ClickException("Empty or unexpected screenshot response (incognito/protected tab?)") + try: + raw = base64.b64decode(data_url[len(header):]) + except binascii.Error as e: + raise click.ClickException(f"Failed to decode screenshot data: {e}") with open(output, "wb") as f: f.write(raw) console.print(f"[green]Screenshot saved:[/green] {output}") diff --git a/browser_cli/native_host.py b/browser_cli/native_host.py index a175442..bfaa00c 100644 --- a/browser_cli/native_host.py +++ b/browser_cli/native_host.py @@ -7,6 +7,7 @@ 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 @@ -121,7 +122,7 @@ def stdin_reader(alias: str): # --- Thread B: accept CLI socket connections --- -def socket_server(sock_path: str): +def socket_server(sock_path: str, bound_sock: "socket.socket | None" = None): if is_windows(): while True: try: @@ -132,13 +133,14 @@ def socket_server(sock_path: str): threading.Thread(target=handle_cli_connection, args=(conn, listener), daemon=True).start() return - path = Path(sock_path) - if path.exists(): - path.unlink() - - sock = socket.socket(socket.AF_UNIX, socket.SOCK_STREAM) - sock.bind(sock_path) - sock.listen(16) + sock = bound_sock + if sock is None: + path = Path(sock_path) + if path.exists(): + path.unlink() + sock = socket.socket(socket.AF_UNIX, socket.SOCK_STREAM) + sock.bind(sock_path) + sock.listen(16) while True: try: @@ -212,8 +214,13 @@ def _collect_paged_browser_command(cmd: dict) -> dict: offset = 0 items = [] total = None + max_pages = math.ceil(10_000 / PAGE_SIZE) + pages_fetched = 0 while True: + if pages_fetched >= max_pages: + return {"id": original_id, "success": False, "error": f"paging loop exceeded {max_pages} pages — extension bug?"} + pages_fetched += 1 page_cmd = dict(cmd) page_cmd["id"] = str(uuid.uuid4()) page_args = dict(cmd.get("args") or {}) @@ -284,21 +291,25 @@ def main(): if first_msg and first_msg.get("type") == "hello": alias = _resolve_profile_alias(first_msg) else: - # No hello — use a generated alias and re-queue the first command if needed. + # No hello — use a generated alias; first_msg is dropped (no response path). alias = str(uuid.uuid4()) - if first_msg: - msg_id = first_msg.get("id") - if msg_id: - q: queue.Queue = queue.Queue() - with PENDING_LOCK: - PENDING[msg_id] = q - write_native_message(sys.stdout.buffer, first_msg) runtime_dir().mkdir(mode=0o700, exist_ok=True) sock_path = _socket_path_for(alias) + + if not is_windows(): + path = Path(sock_path) + if path.exists(): + path.unlink() + bound_sock = socket.socket(socket.AF_UNIX, socket.SOCK_STREAM) + bound_sock.bind(sock_path) + bound_sock.listen(16) + else: + bound_sock = None + _registry_add(alias, sock_path) - t = threading.Thread(target=socket_server, args=(sock_path,), daemon=True) + t = threading.Thread(target=socket_server, args=(sock_path,), kwargs={"bound_sock": bound_sock}, daemon=True) t.start() stdin_reader(alias)