From 8593916e5a97c8e67ac30e810bf242a11dbbfc98 Mon Sep 17 00:00:00 2001 From: Daniel Dolezal Date: Sat, 2 May 2026 19:50:51 +0200 Subject: [PATCH] fix: propagate key through remote discovery; auto-persist key per remote - remote_browser_targets(), _auto_route_remote(), active_browser_targets() now accept and forward the key parameter so pubkey auth works during the initial browser-cli.targets discovery call - _multi_browser_targets() in tabs/groups/windows/session commands now reads key from ctx.obj and passes it through - send_command() auto-saves the key spec (e.g. "agent") to remotes.json on first explicit use; subsequent calls to the same remote reuse it without requiring --key every time - Added save_remote_key() / key_for_remote() helpers (mirrors token helpers) Co-Authored-By: Claude Sonnet 4.6 --- browser_cli/client.py | 47 +++++++++++++++++++++++++------- browser_cli/commands/groups.py | 5 ++-- browser_cli/commands/session.py | 5 ++-- browser_cli/commands/tabs.py | 5 ++-- browser_cli/commands/windows.py | 5 ++-- tests/test_client.py | 48 +++++++++++++++++++++++++++++++-- 6 files changed, 95 insertions(+), 20 deletions(-) diff --git a/browser_cli/client.py b/browser_cli/client.py index 9d86d3a..38f4b2d 100644 --- a/browser_cli/client.py +++ b/browser_cli/client.py @@ -86,15 +86,37 @@ def token_for_remote(endpoint: str | None) -> str | None: return str(token) if token else None +def save_remote_key(endpoint: str, key_spec: str) -> None: + """Persist the key spec (e.g. 'agent' or a file path) for a remote endpoint.""" + if not endpoint or not key_spec: + return + remotes = _load_remotes() + current = remotes.get(endpoint, {}) + current["key"] = key_spec + remotes[endpoint] = current + REMOTE_REGISTRY_PATH.parent.mkdir(parents=True, exist_ok=True) + fd = os.open(str(REMOTE_REGISTRY_PATH), os.O_WRONLY | os.O_CREAT | os.O_TRUNC, 0o600) + with os.fdopen(fd, "w", encoding="utf-8") as f: + f.write(json.dumps(remotes, indent=2, sort_keys=True)) + + +def key_for_remote(endpoint: str | None) -> str | None: + if not endpoint: + return None + cfg = _load_remotes().get(endpoint) or {} + key = cfg.get("key") + return str(key) if key else None + + def _remote_display_name(endpoint: str, profile_name: str, display_name: str) -> str: host, sep, port = endpoint.rpartition(":") remote_name = host if sep and port == "8765" else endpoint return f"{remote_name}:{display_name or profile_name}" -def remote_browser_targets(endpoint: str, token: str | None = None) -> list[BrowserTarget]: +def remote_browser_targets(endpoint: str, token: str | None = None, key=None) -> list[BrowserTarget]: """Return browser targets advertised by a single remote endpoint.""" - remote_targets = send_command("browser-cli.targets", remote=endpoint, token=token) + remote_targets = send_command("browser-cli.targets", remote=endpoint, token=token, key=key) targets: list[BrowserTarget] = [] for item in remote_targets or []: profile = str(item.get("profile") or "default") @@ -111,12 +133,12 @@ def remote_browser_targets(endpoint: str, token: str | None = None) -> list[Brow return targets -def _remote_browser_targets() -> list[BrowserTarget]: +def _remote_browser_targets(key=None) -> list[BrowserTarget]: targets: list[BrowserTarget] = [] for endpoint, cfg in _load_remotes().items(): token = str(cfg.get("token") or "") or None try: - targets.extend(remote_browser_targets(endpoint, token)) + targets.extend(remote_browser_targets(endpoint, token, key=key)) except (BrowserNotConnected, RuntimeError): continue return targets @@ -144,7 +166,7 @@ def remote_target_for_alias(alias: str | None) -> BrowserTarget | None: return None -def active_browser_targets(*, include_remotes: bool = True) -> list[BrowserTarget]: +def active_browser_targets(*, include_remotes: bool = True, key=None) -> list[BrowserTarget]: targets: list[BrowserTarget] = [] if REGISTRY_PATH.exists(): reg = load_registry(REGISTRY_PATH) @@ -153,7 +175,7 @@ def active_browser_targets(*, include_remotes: bool = True) -> list[BrowserTarge for profile, sock_path in _active_endpoints(reg).items() ) if include_remotes: - targets.extend(_remote_browser_targets()) + targets.extend(_remote_browser_targets(key=key)) return targets @@ -251,8 +273,8 @@ def _send_remote(endpoint: str, msg: dict, private_key=None) -> bytes | None: return _recv_all(sock) -def _auto_route_remote(endpoint: str, token: str | None) -> str | None: - targets = remote_browser_targets(endpoint, token) +def _auto_route_remote(endpoint: str, token: str | None, key=None) -> str | None: + targets = remote_browser_targets(endpoint, token, key=key) if len(targets) == 1: return targets[0].profile if len(targets) > 1: @@ -283,13 +305,18 @@ def send_command(command: str, args: dict | None = None, profile: str | None = N "args": args or {}, } if remote_endpoint: - private_key = _load_private_key(key) + # key priority: explicit flag > saved per-remote config > BROWSER_CLI_KEY env > default file + key_spec = key if key is not None else key_for_remote(remote_endpoint) + private_key = _load_private_key(key_spec) + # persist explicit key spec so future calls don't need --key + if key is not None: + save_remote_key(remote_endpoint, str(key)) # use token auth only when no Ed25519 key is available if private_key is None and resolved_token: msg["token"] = resolved_token route_profile = requested_profile if not route_profile and command != "browser-cli.targets": - route_profile = _auto_route_remote(remote_endpoint, resolved_token) + route_profile = _auto_route_remote(remote_endpoint, resolved_token, key=private_key) if route_profile: msg["_route"] = route_profile else: diff --git a/browser_cli/commands/groups.py b/browser_cli/commands/groups.py index 63d118c..465628f 100644 --- a/browser_cli/commands/groups.py +++ b/browser_cli/commands/groups.py @@ -31,10 +31,11 @@ def _multi_browser_targets(): if root.obj.get("browser_explicit"): return [] remote = root.obj.get("remote") + key = root.obj.get("key") if remote: - targets = remote_browser_targets(remote, root.obj.get("token")) + targets = remote_browser_targets(remote, root.obj.get("token"), key=key) else: - targets = active_browser_targets() + targets = active_browser_targets(key=key) if len(targets) <= 1 and not any(target.remote for target in targets): return [] return targets diff --git a/browser_cli/commands/session.py b/browser_cli/commands/session.py index d8da500..464f9a4 100644 --- a/browser_cli/commands/session.py +++ b/browser_cli/commands/session.py @@ -30,10 +30,11 @@ def _multi_browser_targets(): if root.obj.get("browser_explicit"): return [] remote = root.obj.get("remote") + key = root.obj.get("key") if remote: - targets = remote_browser_targets(remote, root.obj.get("token")) + targets = remote_browser_targets(remote, root.obj.get("token"), key=key) else: - targets = active_browser_targets() + targets = active_browser_targets(key=key) if len(targets) <= 1 and not any(target.remote for target in targets): return [] return targets diff --git a/browser_cli/commands/tabs.py b/browser_cli/commands/tabs.py index 2b7d59a..009cbe3 100644 --- a/browser_cli/commands/tabs.py +++ b/browser_cli/commands/tabs.py @@ -33,10 +33,11 @@ def _multi_browser_targets(): if root.obj.get("browser_explicit"): return [] remote = root.obj.get("remote") + key = root.obj.get("key") if remote: - targets = remote_browser_targets(remote, root.obj.get("token")) + targets = remote_browser_targets(remote, root.obj.get("token"), key=key) else: - targets = active_browser_targets() + targets = active_browser_targets(key=key) if len(targets) <= 1 and not any(target.remote for target in targets): return [] return targets diff --git a/browser_cli/commands/windows.py b/browser_cli/commands/windows.py index 41eb294..fa05900 100644 --- a/browser_cli/commands/windows.py +++ b/browser_cli/commands/windows.py @@ -31,10 +31,11 @@ def _multi_browser_targets(): if root.obj.get("browser_explicit"): return [] remote = root.obj.get("remote") + key = root.obj.get("key") if remote: - targets = remote_browser_targets(remote, root.obj.get("token")) + targets = remote_browser_targets(remote, root.obj.get("token"), key=key) else: - targets = active_browser_targets() + targets = active_browser_targets(key=key) if len(targets) <= 1 and not any(target.remote for target in targets): return [] return targets diff --git a/tests/test_client.py b/tests/test_client.py index 8868380..4b7852a 100644 --- a/tests/test_client.py +++ b/tests/test_client.py @@ -9,6 +9,7 @@ from browser_cli.client import ( _resolve_socket, active_browser_targets, display_browser_name, + key_for_remote, save_remote_token, send_command, remote_target_for_alias, @@ -115,7 +116,7 @@ def test_send_command_auto_routes_single_remote_target(monkeypatch): monkeypatch.setattr( "browser_cli.client.remote_browser_targets", - lambda endpoint, token=None: [BrowserTarget("work", "host:work", "", remote=endpoint, token=token)], + lambda endpoint, token=None, key=None: [BrowserTarget("work", "host:work", "", remote=endpoint, token=token)], ) def fake_send_remote(endpoint, msg, private_key=None): @@ -223,7 +224,7 @@ def test_send_command_requires_browser_for_multiple_remote_targets(monkeypatch): monkeypatch.delenv("BROWSER_CLI_PROFILE", raising=False) monkeypatch.setattr( "browser_cli.client.remote_browser_targets", - lambda endpoint, token=None: [ + lambda endpoint, token=None, key=None: [ BrowserTarget("main", "host:main", "", remote=endpoint, token=token), BrowserTarget("furry", "host:furry", "", remote=endpoint, token=token), ], @@ -255,3 +256,46 @@ def test_active_browser_targets_includes_remote_targets(monkeypatch, tmp_path): assert targets[0].display_name == "browser-host.example:work" assert targets[0].remote == endpoint assert targets[0].token == "secret-token" + + +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 + + remotes_path = tmp_path / "remotes.json" + remotes_path.write_text("{}", encoding="utf-8") + monkeypatch.setattr("browser_cli.client.REMOTE_REGISTRY_PATH", remotes_path) + monkeypatch.setattr("browser_cli.client.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_TOKEN", raising=False) + monkeypatch.delenv("BROWSER_CLI_KEY", raising=False) + + 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 + + monkeypatch.setattr("browser_cli.client._load_private_key", fake_load_private_key) + monkeypatch.setattr( + "browser_cli.client.remote_browser_targets", + lambda endpoint, token=None, 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() + + monkeypatch.setattr("browser_cli.client._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" + + # 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"