From c6eadc7d85c18a3117ba0a5af3fc35ebe7083cb4 Mon Sep 17 00:00:00 2001 From: Shri Sukhani Date: Fri, 14 Aug 2026 23:18:42 -0700 Subject: [PATCH 1/4] Add ability to use hx creds if api key not provided Fall back to a saved hx OAuth session when no API key is set, refresh tokens against the frontend host, and retry replayable 401s. --- AGENTS.md | 3 +- CLAUDE.md | 3 +- README.md | 4 +- hyperbrowser/client/async_client.py | 2 + hyperbrowser/client/base.py | 25 +- hyperbrowser/client/sync.py | 2 + hyperbrowser/config.py | 21 +- hyperbrowser/control_auth.py | 870 ++++++++++++++++++++++ hyperbrowser/transport/async_transport.py | 136 +++- hyperbrowser/transport/base.py | 45 +- hyperbrowser/transport/sync.py | 136 +++- tests/test_control_auth.py | 333 +++++++++ tests/test_transport_auth.py | 207 +++++ 13 files changed, 1694 insertions(+), 93 deletions(-) create mode 100644 hyperbrowser/control_auth.py create mode 100644 tests/test_control_auth.py create mode 100644 tests/test_transport_auth.py diff --git a/AGENTS.md b/AGENTS.md index 4301aca8..41e88a0e 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -13,7 +13,7 @@ - `poetry run ruff check .` — run lint checks. - `poetry run ruff format .` — format code with ruff. - `poetry run pytest tests --ignore=tests/sandbox/e2e` — run the local non-E2E suite. -- Example run: create a small script using the README snippets and run with `python path/to/script.py` after setting `HYPERBROWSER_API_KEY`. +- Example run: create a small script using the README snippets and run with `python path/to/script.py` after setting `HYPERBROWSER_API_KEY`, or after `hx auth login`. ## Coding Style & Naming Conventions - Python 3.8+ with 4‑space indentation. @@ -33,6 +33,7 @@ ## Configuration & Secrets - Set `HYPERBROWSER_API_KEY` via environment variables or pass `api_key=` in client constructors. +- If no API key is set, the client uses a saved `hx auth login` session from `~/.hx_config/auth/`. Select a profile with `HYPERBROWSER_PROFILE` or `profile=`. - Never commit API keys or session data; use `.env` or local shell exports for development. ## Cursor Cloud specific instructions diff --git a/CLAUDE.md b/CLAUDE.md index 013f13e7..5bba7624 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -75,7 +75,8 @@ Response and legacy models are re-exported from ## Key Patterns -- **API Key**: Via constructor `api_key=` or `HYPERBROWSER_API_KEY` env var +- **API Key**: Via constructor `api_key=` or `HYPERBROWSER_API_KEY` env var. If omitted, the client uses a saved `hx auth login` session (`~/.hx_config/auth/.json`). +- **Profile**: `HYPERBROWSER_PROFILE`, `profile=`, or `ClientConfig(profile=...)` (default `default`) - **Base URL**: Defaults to `https://api.hyperbrowser.ai`, configurable via `base_url=` or `HYPERBROWSER_BASE_URL` - **Job Polling**: Managers provide `start_and_wait()` methods that poll until completion - **Context Managers**: `AsyncHyperbrowser` supports `async with` for automatic cleanup diff --git a/README.md b/README.md index 667155e6..ddc914d8 100644 --- a/README.md +++ b/README.md @@ -28,12 +28,14 @@ Both the sync and async client follow similar configuration params ### API Key The API key can be configured either from the constructor arguments or environment variables using `HYPERBROWSER_API_KEY` -If no API key is provided, the client falls back to a saved OAuth session created by `hx auth login`. By default it reads `~/.hx_config/auth/default.json`, or `~/.hx_config/auth/.json` when `HYPERBROWSER_PROFILE` or `ClientConfig(profile=...)` is set. +If no API key is provided, the client falls back to a saved OAuth session created by `hx auth login`. By default it reads `~/.hx_config/auth/default.json`, or `~/.hx_config/auth/.json` when `HYPERBROWSER_PROFILE`, `Hyperbrowser(profile=...)`, or `ClientConfig(profile=...)` is set. Profile names must match `^[A-Za-z0-9._-]+$`. `base_url` and `HYPERBROWSER_BASE_URL` accept either `https://host` or `https://host/api`. The client normalizes both to the same control-plane base URL. +Token refresh uses `https://app.hyperbrowser.ai` for the default API host, or `HYPERBROWSER_FRONTEND_URL` / `ClientConfig(frontend_url=...)` when set. Custom `base_url` values refresh against that same host. + ## Usage Hyperbrowser 1.0 accepts plain dictionaries for request parameters. Method diff --git a/hyperbrowser/client/async_client.py b/hyperbrowser/client/async_client.py index ce4153a3..fb361158 100644 --- a/hyperbrowser/client/async_client.py +++ b/hyperbrowser/client/async_client.py @@ -27,6 +27,7 @@ def __init__( base_url: Optional[str] = None, timeout: Optional[int] = 30, runtime_proxy_override: Optional[str] = None, + profile: Optional[str] = None, ): super().__init__( AsyncTransport, @@ -34,6 +35,7 @@ def __init__( api_key, base_url, runtime_proxy_override, + profile, ) self.timeout = timeout or 30 self.transport.client.timeout = timeout diff --git a/hyperbrowser/client/base.py b/hyperbrowser/client/base.py index ac6ac227..a771e0a8 100644 --- a/hyperbrowser/client/base.py +++ b/hyperbrowser/client/base.py @@ -1,7 +1,8 @@ +from dataclasses import replace from typing import Optional -from hyperbrowser.exceptions import HyperbrowserError from ..config import ClientConfig +from ..control_auth import DEFAULT_BASE_URL, resolve_control_plane_config from ..transport.base import TransportStrategy import os @@ -16,29 +17,33 @@ def __init__( api_key: Optional[str] = None, base_url: Optional[str] = None, runtime_proxy_override: Optional[str] = None, + profile: Optional[str] = None, ): if config is None: config = ClientConfig( api_key=( api_key if api_key is not None - else os.environ.get("HYPERBROWSER_API_KEY", "") + else os.environ.get("HYPERBROWSER_API_KEY") ), base_url=( base_url if base_url is not None - else os.environ.get( - "HYPERBROWSER_BASE_URL", "https://api.hyperbrowser.ai" - ) + else os.environ.get("HYPERBROWSER_BASE_URL", DEFAULT_BASE_URL) ), + profile=( + profile + if profile is not None + else os.environ.get("HYPERBROWSER_PROFILE") + ), + frontend_url=os.environ.get("HYPERBROWSER_FRONTEND_URL"), runtime_proxy_override=runtime_proxy_override, ) - if not config.api_key: - raise HyperbrowserError("API key must be provided") - - self.config = config - self.transport = transport(config.api_key) + resolved_base_url, auth = resolve_control_plane_config(config) + self.config = replace(config, base_url=resolved_base_url) + self.auth = auth + self.transport = transport(auth) def _build_url(self, path: str) -> str: return f"{self.config.base_url}/api{path}" diff --git a/hyperbrowser/client/sync.py b/hyperbrowser/client/sync.py index 7d6821e7..8196f1dd 100644 --- a/hyperbrowser/client/sync.py +++ b/hyperbrowser/client/sync.py @@ -27,6 +27,7 @@ def __init__( base_url: Optional[str] = None, timeout: Optional[int] = 30, runtime_proxy_override: Optional[str] = None, + profile: Optional[str] = None, ): super().__init__( SyncTransport, @@ -34,6 +35,7 @@ def __init__( api_key, base_url, runtime_proxy_override, + profile, ) self.timeout = timeout or 30 self.transport.client.timeout = timeout diff --git a/hyperbrowser/config.py b/hyperbrowser/config.py index e1ec8f20..cdb6fdda 100644 --- a/hyperbrowser/config.py +++ b/hyperbrowser/config.py @@ -7,17 +7,22 @@ class ClientConfig: """Configuration for the Hyperbrowser client""" - api_key: str + api_key: Optional[str] = None base_url: str = "https://api.hyperbrowser.ai" + profile: Optional[str] = None + frontend_url: Optional[str] = None + auth_lock_timeout_ms: Optional[int] = None + auth_lock_poll_interval_ms: Optional[int] = None + auth_lock_stale_ms: Optional[int] = None runtime_proxy_override: Optional[str] = None @classmethod def from_env(cls) -> "ClientConfig": - api_key = os.environ.get("HYPERBROWSER_API_KEY") - if api_key is None: - raise ValueError("HYPERBROWSER_API_KEY environment variable is required") - - base_url = os.environ.get( - "HYPERBROWSER_BASE_URL", "https://api.hyperbrowser.ai" + return cls( + api_key=os.environ.get("HYPERBROWSER_API_KEY"), + base_url=os.environ.get( + "HYPERBROWSER_BASE_URL", "https://api.hyperbrowser.ai" + ), + profile=os.environ.get("HYPERBROWSER_PROFILE"), + frontend_url=os.environ.get("HYPERBROWSER_FRONTEND_URL"), ) - return cls(api_key=api_key, base_url=base_url) diff --git a/hyperbrowser/control_auth.py b/hyperbrowser/control_auth.py new file mode 100644 index 00000000..30f2e466 --- /dev/null +++ b/hyperbrowser/control_auth.py @@ -0,0 +1,870 @@ +import asyncio +import json +import os +import re +import tempfile +import time +from datetime import datetime, timedelta, timezone +from pathlib import Path +from typing import Any, Dict, Optional, Tuple +from urllib.parse import urlencode + +import httpx + +from .config import ClientConfig +from .exceptions import HyperbrowserError + +DEFAULT_PROFILE = "default" +DEFAULT_BASE_URL = "https://api.hyperbrowser.ai" +DEFAULT_FRONTEND_BASE_URL = "https://app.hyperbrowser.ai" +LEGACY_DEFAULT_BASE_URL = "https://app.hyperbrowser.ai" +DEFAULT_LOCK_TIMEOUT_MS = 30000 +DEFAULT_LOCK_POLL_INTERVAL_MS = 125 +DEFAULT_LOCK_STALE_MS = 120000 +OAUTH_REFRESH_EARLY_EXPIRY_MS = 30000 +PROFILE_NAME_PATTERN = re.compile(r"^[A-Za-z0-9._-]+$") +TERMINAL_OAUTH_REFRESH_ERRORS = { + "invalid_grant", + "invalid_client", + "unauthorized_client", +} + +ENV_PROFILE = "HYPERBROWSER_PROFILE" +ENV_API_KEY = "HYPERBROWSER_API_KEY" +ENV_BASE_URL = "HYPERBROWSER_BASE_URL" +ENV_FRONTEND_URL = "HYPERBROWSER_FRONTEND_URL" +ENV_LOCK_TIMEOUT_MS = "HYPERBROWSER_AUTH_LOCK_TIMEOUT_MS" +ENV_LOCK_POLL_INTERVAL_MS = "HYPERBROWSER_AUTH_LOCK_POLL_INTERVAL_MS" +ENV_LOCK_STALE_MS = "HYPERBROWSER_AUTH_LOCK_STALE_MS" + + +class ControlPlaneAuthManager: + def __init__(self, mode: Dict[str, object]): + self._mode = mode + + @property + def is_oauth(self) -> bool: + return self._mode["kind"] == "oauth" + + def authorize_headers( + self, + *, + force_refresh: bool = False, + rejected_access_token: Optional[str] = None, + ) -> Tuple[Dict[str, str], Optional[str]]: + if self._mode["kind"] == "api_key": + return {"x-api-key": str(self._mode["api_key"])}, None + + access_token = self._resolve_oauth_access_token( + force_refresh=force_refresh, + rejected_access_token=rejected_access_token, + ) + return {"authorization": f"Bearer {access_token}"}, access_token + + async def aauthorize_headers( + self, + *, + force_refresh: bool = False, + rejected_access_token: Optional[str] = None, + ) -> Tuple[Dict[str, str], Optional[str]]: + if self._mode["kind"] == "api_key": + return {"x-api-key": str(self._mode["api_key"])}, None + + access_token = await self._aresolve_oauth_access_token( + force_refresh=force_refresh, + rejected_access_token=rejected_access_token, + ) + return {"authorization": f"Bearer {access_token}"}, access_token + + def _resolve_oauth_access_token( + self, + *, + force_refresh: bool, + rejected_access_token: Optional[str], + ) -> str: + session, session_mtime_ns = self._load_oauth_session_with_mtime() + if _should_use_oauth_session(session, force_refresh, rejected_access_token): + return _normalize_text(session["access_token"]) + + deadline = time.monotonic() + (int(self._mode["lock_timeout_ms"]) / 1000.0) + while True: + lock_fd = self._try_acquire_rotation_lock() + if lock_fd is not None: + try: + session, session_mtime_ns = self._load_oauth_session_with_mtime() + if _should_use_oauth_session( + session, force_refresh, rejected_access_token + ): + return _normalize_text(session["access_token"]) + if _is_refresh_token_expired(session): + self._expire_oauth_session() + raise HyperbrowserError( + "OAuth session refresh token expired", + code="oauth_session_expired", + retryable=False, + service="control", + ) + refreshed = self._refresh_oauth_session(session) + return _normalize_text(refreshed["access_token"]) + finally: + self._release_rotation_lock(lock_fd) + + self._clear_stale_rotation_lock() + if time.monotonic() > deadline: + raise HyperbrowserError( + "Timed out waiting for OAuth rotation lock", + code="auth_rotation_timeout", + retryable=False, + service="control", + ) + + time.sleep(int(self._mode["lock_poll_interval_ms"]) / 1000.0) + updated = self._load_updated_oauth_session(session_mtime_ns) + if updated is None: + continue + session, session_mtime_ns = updated + if _should_use_oauth_session(session, True, rejected_access_token): + return _normalize_text(session["access_token"]) + if _is_refresh_token_expired(session): + self._expire_oauth_session() + raise HyperbrowserError( + "OAuth session refresh token expired", + code="oauth_session_expired", + retryable=False, + service="control", + ) + + async def _aresolve_oauth_access_token( + self, + *, + force_refresh: bool, + rejected_access_token: Optional[str], + ) -> str: + session, session_mtime_ns = self._load_oauth_session_with_mtime() + if _should_use_oauth_session(session, force_refresh, rejected_access_token): + return _normalize_text(session["access_token"]) + + deadline = time.monotonic() + (int(self._mode["lock_timeout_ms"]) / 1000.0) + while True: + lock_fd = self._try_acquire_rotation_lock() + if lock_fd is not None: + try: + session, session_mtime_ns = self._load_oauth_session_with_mtime() + if _should_use_oauth_session( + session, force_refresh, rejected_access_token + ): + return _normalize_text(session["access_token"]) + if _is_refresh_token_expired(session): + self._expire_oauth_session() + raise HyperbrowserError( + "OAuth session refresh token expired", + code="oauth_session_expired", + retryable=False, + service="control", + ) + refreshed = await self._arefresh_oauth_session(session) + return _normalize_text(refreshed["access_token"]) + finally: + self._release_rotation_lock(lock_fd) + + self._clear_stale_rotation_lock() + if time.monotonic() > deadline: + raise HyperbrowserError( + "Timed out waiting for OAuth rotation lock", + code="auth_rotation_timeout", + retryable=False, + service="control", + ) + + await asyncio.sleep(int(self._mode["lock_poll_interval_ms"]) / 1000.0) + updated = self._load_updated_oauth_session(session_mtime_ns) + if updated is None: + continue + session, session_mtime_ns = updated + if _should_use_oauth_session(session, True, rejected_access_token): + return _normalize_text(session["access_token"]) + if _is_refresh_token_expired(session): + self._expire_oauth_session() + raise HyperbrowserError( + "OAuth session refresh token expired", + code="oauth_session_expired", + retryable=False, + service="control", + ) + + def _load_oauth_session(self) -> Dict[str, str]: + session_path = Path(str(self._mode["session_path"])) + try: + raw = session_path.read_text() + except (FileNotFoundError, OSError) as error: + raise HyperbrowserError( + "Failed to read saved OAuth session", + code="oauth_session_read_failed", + retryable=False, + service="control", + cause=error, + original_error=error, + ) + + try: + session = json.loads(raw) + except json.JSONDecodeError as error: + raise HyperbrowserError( + "Saved OAuth session is invalid JSON", + code="oauth_session_invalid", + retryable=False, + service="control", + cause=error, + original_error=error, + ) + + _validate_oauth_session(session, expected_base_url=str(self._mode["base_url"])) + return session + + def _load_oauth_session_with_mtime(self) -> Tuple[Dict[str, str], Optional[int]]: + session = self._load_oauth_session() + return session, self._get_session_mtime_ns() + + def _load_updated_oauth_session( + self, previous_mtime_ns: Optional[int] + ) -> Optional[Tuple[Dict[str, str], Optional[int]]]: + current_mtime_ns = self._get_session_mtime_ns() + if current_mtime_ns == previous_mtime_ns: + return None + return self._load_oauth_session(), current_mtime_ns + + def _get_session_mtime_ns(self) -> Optional[int]: + session_path = Path(str(self._mode["session_path"])) + try: + return session_path.stat().st_mtime_ns + except (FileNotFoundError, OSError) as error: + raise HyperbrowserError( + "Failed to inspect saved OAuth session", + code="oauth_session_read_failed", + retryable=False, + service="control", + cause=error, + original_error=error, + ) + + def _refresh_oauth_session(self, session: Dict[str, str]) -> Dict[str, str]: + try: + with httpx.Client( + timeout=int(self._mode["lock_timeout_ms"]) / 1000.0 + ) as client: + response = client.post( + str(self._mode["token_url"]), + headers={"content-type": "application/x-www-form-urlencoded"}, + content=_build_refresh_form(session), + ) + except Exception as error: + raise HyperbrowserError( + "Failed to refresh OAuth session", + code="oauth_refresh_failed", + retryable=True, + service="control", + cause=error, + original_error=error if isinstance(error, Exception) else None, + ) + + return self._handle_refresh_response(session, response) + + async def _arefresh_oauth_session(self, session: Dict[str, str]) -> Dict[str, str]: + try: + async with httpx.AsyncClient( + timeout=int(self._mode["lock_timeout_ms"]) / 1000.0 + ) as client: + response = await client.post( + str(self._mode["token_url"]), + headers={"content-type": "application/x-www-form-urlencoded"}, + content=_build_refresh_form(session), + ) + except Exception as error: + raise HyperbrowserError( + "Failed to refresh OAuth session", + code="oauth_refresh_failed", + retryable=True, + service="control", + cause=error, + original_error=error if isinstance(error, Exception) else None, + ) + + return self._handle_refresh_response(session, response) + + def _handle_refresh_response( + self, session: Dict[str, str], response: httpx.Response + ) -> Dict[str, str]: + raw_text = response.text + payload: Any = {} + if raw_text: + try: + payload = response.json() + except ValueError: + payload = {} + + if response.status_code >= 400: + error_code = "" + if isinstance(payload, dict): + error_code = _normalize_text( + _string_value(payload.get("error")) + ) or _normalize_text(_string_value(payload.get("code"))) + if error_code in TERMINAL_OAUTH_REFRESH_ERRORS: + self._expire_oauth_session() + message = ( + _normalize_text( + _string_value(payload.get("message")) + if isinstance(payload, dict) + else "" + ) + or _normalize_text( + _string_value(payload.get("error_description")) + if isinstance(payload, dict) + else "" + ) + or error_code + or f"OAuth refresh failed with status {response.status_code}" + ) + raise HyperbrowserError( + message, + status_code=response.status_code, + code=error_code or "oauth_refresh_failed", + retryable=False, + service="control", + details=_redact_refresh_error_details(payload), + response=response, + ) + + if not isinstance(payload, dict): + payload = {} + + refreshed = _build_refreshed_oauth_session(session, payload) + _write_oauth_session_atomic(Path(str(self._mode["session_path"])), refreshed) + return refreshed + + def _try_acquire_rotation_lock(self) -> Optional[int]: + lock_path = Path(str(self._mode["lock_path"])) + lock_path.parent.mkdir(mode=0o700, parents=True, exist_ok=True) + try: + os.chmod(lock_path.parent, 0o700) + except OSError: + pass + + try: + lock_fd = os.open( + str(lock_path), + os.O_CREAT | os.O_EXCL | os.O_WRONLY, + 0o600, + ) + except FileExistsError: + return None + except OSError as error: + raise HyperbrowserError( + "Failed to create OAuth rotation lock", + code="auth_rotation_lock_failed", + retryable=False, + service="control", + cause=error, + original_error=error, + ) + + try: + os.write( + lock_fd, + f"pid={os.getpid()}\ncreated_at={time.strftime('%Y-%m-%dT%H:%M:%SZ', time.gmtime())}\n".encode( + "utf-8" + ), + ) + os.fsync(lock_fd) + return lock_fd + except OSError as error: + try: + os.close(lock_fd) + except OSError: + pass + try: + lock_path.unlink(missing_ok=True) + except OSError: + pass + raise HyperbrowserError( + "Failed to create OAuth rotation lock", + code="auth_rotation_lock_failed", + retryable=False, + service="control", + cause=error, + original_error=error, + ) + + def _clear_stale_rotation_lock(self) -> None: + lock_path = Path(str(self._mode["lock_path"])) + try: + stat = lock_path.stat() + except FileNotFoundError: + return + except OSError as error: + raise HyperbrowserError( + "Failed to inspect OAuth rotation lock", + code="auth_rotation_lock_failed", + retryable=False, + service="control", + cause=error, + original_error=error, + ) + + if (time.time() * 1000) - (stat.st_mtime * 1000) < int( + self._mode["lock_stale_ms"] + ): + return + + try: + lock_path.unlink(missing_ok=True) + except OSError as error: + raise HyperbrowserError( + "Failed to remove stale OAuth rotation lock", + code="auth_rotation_lock_failed", + retryable=False, + service="control", + cause=error, + original_error=error, + ) + + def _release_rotation_lock(self, lock_fd: int) -> None: + lock_path = Path(str(self._mode["lock_path"])) + try: + os.close(lock_fd) + except OSError: + pass + try: + lock_path.unlink(missing_ok=True) + except OSError: + pass + + def _expire_oauth_session(self) -> None: + _delete_oauth_session( + Path(str(self._mode["session_path"])), + Path(str(self._mode["lock_path"])), + ) + + +def resolve_control_plane_config( + config: ClientConfig, +) -> Tuple[str, ControlPlaneAuthManager]: + explicit_api_key = _normalize_text(config.api_key) + env_api_key = _normalize_text(os.environ.get(ENV_API_KEY)) + explicit_base_url = _normalize_control_base_url(config.base_url) + env_base_url = _normalize_control_base_url(os.environ.get(ENV_BASE_URL)) + + if explicit_api_key or env_api_key: + return ( + explicit_base_url or env_base_url or DEFAULT_BASE_URL, + ControlPlaneAuthManager( + {"kind": "api_key", "api_key": explicit_api_key or env_api_key} + ), + ) + + profile = _normalize_profile( + config.profile or os.environ.get(ENV_PROFILE) or DEFAULT_PROFILE + ) + session_path = _resolve_oauth_session_path(profile) + session = _try_load_oauth_session(session_path) + + resolved_base_url = ( + explicit_base_url + or env_base_url + or _normalize_control_base_url((session or {}).get("base_url", "")) + or DEFAULT_BASE_URL + ) + + if session is None: + raise HyperbrowserError( + "API key must be provided or an OAuth session must be saved with hx auth login", + code="missing_auth", + retryable=False, + service="control", + ) + + if not _oauth_base_urls_match(session.get("base_url"), resolved_base_url): + raise HyperbrowserError( + f"Saved OAuth session for profile {profile} targets {_normalize_base_url(session.get('base_url'))}, not {resolved_base_url}", + code="oauth_base_url_mismatch", + retryable=False, + service="control", + ) + + frontend_base_url = resolve_frontend_base_url( + resolved_base_url, + getattr(config, "frontend_url", None), + ) + + return resolved_base_url, ControlPlaneAuthManager( + { + "kind": "oauth", + "profile": profile, + "session_path": str(session_path), + "lock_path": f"{session_path}.refresh.lock", + "base_url": resolved_base_url, + "token_url": f"{frontend_base_url}/oauth/token", + "lock_timeout_ms": _normalize_positive_int( + getattr(config, "auth_lock_timeout_ms", None), + os.environ.get(ENV_LOCK_TIMEOUT_MS), + DEFAULT_LOCK_TIMEOUT_MS, + ), + "lock_poll_interval_ms": _normalize_positive_int( + getattr(config, "auth_lock_poll_interval_ms", None), + os.environ.get(ENV_LOCK_POLL_INTERVAL_MS), + DEFAULT_LOCK_POLL_INTERVAL_MS, + ), + "lock_stale_ms": _normalize_positive_int( + getattr(config, "auth_lock_stale_ms", None), + os.environ.get(ENV_LOCK_STALE_MS), + DEFAULT_LOCK_STALE_MS, + ), + } + ) + + +def resolve_frontend_base_url( + control_base_url: str, + explicit_frontend_url: Optional[str] = None, +) -> str: + explicit = _normalize_base_url(explicit_frontend_url) + if explicit: + return explicit + env_value = _normalize_base_url(os.environ.get(ENV_FRONTEND_URL)) + if env_value: + return env_value + if _is_default_control_base_url(control_base_url): + return DEFAULT_FRONTEND_BASE_URL + return _normalize_base_url(control_base_url) or DEFAULT_FRONTEND_BASE_URL + + +def _resolve_oauth_session_path(profile: str) -> Path: + return Path.home() / ".hx_config" / "auth" / f"{profile}.json" + + +def _try_load_oauth_session(session_path: Path) -> Optional[Dict[str, str]]: + try: + raw = session_path.read_text() + except FileNotFoundError: + return None + except OSError as error: + raise HyperbrowserError( + "Failed to read saved OAuth session", + code="oauth_session_read_failed", + retryable=False, + service="control", + cause=error, + original_error=error, + ) + + try: + session = json.loads(raw) + except json.JSONDecodeError as error: + raise HyperbrowserError( + "Saved OAuth session is invalid JSON", + code="oauth_session_invalid", + retryable=False, + service="control", + cause=error, + original_error=error, + ) + + _validate_oauth_session(session) + return session + + +def _validate_oauth_session( + session: Dict[str, str], expected_base_url: Optional[str] = None +) -> None: + if not isinstance(session, dict): + raise HyperbrowserError( + "Saved OAuth session is invalid", + code="oauth_session_invalid", + retryable=False, + service="control", + ) + + access_token = _normalize_text(session.get("access_token", "")) + refresh_token = _normalize_text(session.get("refresh_token", "")) + base_url = _normalize_base_url(session.get("base_url", "")) + + if access_token == "" or refresh_token == "": + raise HyperbrowserError( + "Saved OAuth session is missing tokens", + code="oauth_session_invalid", + retryable=False, + service="control", + ) + if base_url == "": + raise HyperbrowserError( + "Saved OAuth session is missing a base URL", + code="oauth_session_invalid", + retryable=False, + service="control", + ) + if _parse_timestamp(session.get("expiry")) is None: + raise HyperbrowserError( + "Saved OAuth session has an invalid expiry", + code="oauth_session_invalid", + retryable=False, + service="control", + ) + + refresh_expiry = _normalize_text(session.get("refresh_token_expiry", "")) + if refresh_expiry and _parse_timestamp(refresh_expiry) is None: + raise HyperbrowserError( + "Saved OAuth session has an invalid refresh token expiry", + code="oauth_session_invalid", + retryable=False, + service="control", + ) + + if expected_base_url and not _oauth_base_urls_match(base_url, expected_base_url): + raise HyperbrowserError( + "Saved OAuth session targets a different base URL", + code="oauth_base_url_mismatch", + retryable=False, + service="control", + ) + + +def _build_refresh_form(session: Dict[str, str]) -> str: + return urlencode( + { + "grant_type": "refresh_token", + "client_id": _normalize_text(session.get("client_id", "")) + or "hyperbrowser-cli", + "refresh_token": _normalize_text(session.get("refresh_token", "")), + } + ) + + +def _build_refreshed_oauth_session( + previous: Dict[str, str], payload: Dict[str, object] +) -> Dict[str, str]: + access_token = _normalize_text(_string_value(payload.get("access_token"))) + if access_token == "": + raise HyperbrowserError( + "OAuth refresh response did not include an access token", + code="oauth_refresh_failed", + retryable=False, + service="control", + details=payload, + ) + + refresh_token = _normalize_text( + _string_value(payload.get("refresh_token")) + ) or _normalize_text(previous.get("refresh_token", "")) + token_type = ( + _normalize_text(_string_value(payload.get("token_type"))) + or _normalize_text(previous.get("token_type", "")) + or "Bearer" + ) + expiry = _derive_expiry(payload.get("expires_in")) or _normalize_text( + previous.get("expiry", "") + ) + refresh_expiry = _derive_expiry( + payload.get("refresh_token_expires_in") + ) or _normalize_text(previous.get("refresh_token_expiry", "")) + + return { + "version": previous.get("version", 1), + "base_url": _normalize_base_url(previous.get("base_url", "")), + "client_id": _normalize_text(previous.get("client_id", "")) + or "hyperbrowser-cli", + "token_type": token_type, + "access_token": access_token, + "refresh_token": refresh_token, + "expiry": expiry, + "scope": _normalize_text(_string_value(payload.get("scope"))) + or _normalize_text(previous.get("scope", "")), + "refresh_token_expiry": refresh_expiry, + } + + +def _write_oauth_session_atomic(session_path: Path, session: Dict[str, str]) -> None: + session_path.parent.mkdir(mode=0o700, parents=True, exist_ok=True) + try: + os.chmod(session_path.parent, 0o700) + except OSError: + pass + + payload = f"{json.dumps(session, indent=2)}\n" + fd, temp_path = tempfile.mkstemp( + prefix=f"{session_path.name}.", + suffix=".tmp", + dir=str(session_path.parent), + ) + renamed = False + try: + os.fchmod(fd, 0o600) + with os.fdopen(fd, "w", encoding="utf-8") as handle: + handle.write(payload) + handle.flush() + os.fsync(handle.fileno()) + os.replace(temp_path, session_path) + renamed = True + try: + os.chmod(session_path, 0o600) + except OSError: + pass + finally: + if not renamed: + try: + os.unlink(temp_path) + except OSError: + pass + + +def _should_use_oauth_session( + session: Dict[str, str], + force_refresh: bool, + rejected_access_token: Optional[str], +) -> bool: + if not _is_access_token_usable(session): + return False + if not force_refresh: + return True + return _normalize_text(session.get("access_token", "")) != _normalize_text( + rejected_access_token or "" + ) + + +def _is_access_token_usable(session: Dict[str, str]) -> bool: + expiry = _parse_timestamp(session.get("expiry")) + if expiry is None or _normalize_text(session.get("access_token", "")) == "": + return False + return (expiry * 1000) - (time.time() * 1000) > OAUTH_REFRESH_EARLY_EXPIRY_MS + + +def _is_refresh_token_expired(session: Dict[str, str]) -> bool: + expiry = _parse_timestamp(session.get("refresh_token_expiry")) + if expiry is None: + return False + return (expiry * 1000) <= (time.time() * 1000) + + +def _redact_refresh_error_details(payload: Any) -> Any: + if isinstance(payload, dict): + redacted: Dict[str, Any] = {} + for key, value in payload.items(): + if key in {"access_token", "refresh_token"}: + redacted[key] = "[REDACTED]" + else: + redacted[key] = _redact_refresh_error_details(value) + return redacted + if isinstance(payload, list): + return [_redact_refresh_error_details(value) for value in payload] + return payload + + +def _derive_expiry(value: object) -> Optional[str]: + if isinstance(value, (int, float)) and value > 0: + return ( + datetime.now(timezone.utc) + timedelta(seconds=float(value)) + ).isoformat() + if isinstance(value, str): + try: + parsed = int(value) + except ValueError: + return None + if parsed > 0: + return (datetime.now(timezone.utc) + timedelta(seconds=parsed)).isoformat() + return None + + +def _parse_timestamp(value: Optional[str]) -> Optional[float]: + normalized = _normalize_text(value or "") + if normalized == "": + return None + try: + adjusted = normalized.replace("Z", "+00:00") + parsed = datetime.fromisoformat(adjusted) + except ValueError: + return None + if parsed.tzinfo is None: + parsed = parsed.replace(tzinfo=timezone.utc) + return parsed.timestamp() + + +def _normalize_positive_int( + explicit_value: Optional[int], env_value: Optional[str], fallback: int +) -> int: + if explicit_value is not None and explicit_value > 0: + return explicit_value + if env_value: + try: + parsed = int(env_value) + except ValueError: + parsed = 0 + if parsed > 0: + return parsed + return fallback + + +def _normalize_profile(value: str) -> str: + normalized = _normalize_text(value) or DEFAULT_PROFILE + if not PROFILE_NAME_PATTERN.fullmatch(normalized): + raise HyperbrowserError( + "Invalid Hyperbrowser profile name", + code="invalid_profile", + retryable=False, + service="control", + ) + return normalized + + +def _normalize_base_url(value: Optional[object]) -> str: + normalized = _normalize_text(value) + if normalized == "": + return "" + normalized = normalized.rstrip("/") + if normalized.endswith("/api"): + normalized = normalized[: -len("/api")] + return normalized.rstrip("/") + + +def _normalize_control_base_url(value: Optional[object]) -> str: + normalized = _normalize_base_url(value) + if normalized == LEGACY_DEFAULT_BASE_URL: + return DEFAULT_BASE_URL + return normalized + + +def _is_default_control_base_url(value: Optional[object]) -> bool: + normalized = _normalize_base_url(value) + return normalized in {DEFAULT_BASE_URL, LEGACY_DEFAULT_BASE_URL} + + +def _oauth_base_urls_match(left: Optional[object], right: Optional[object]) -> bool: + normalized_left = _normalize_base_url(left) + normalized_right = _normalize_base_url(right) + if normalized_left == "" or normalized_right == "": + return False + if normalized_left == normalized_right: + return True + return _is_default_control_base_url( + normalized_left + ) and _is_default_control_base_url(normalized_right) + + +def _delete_oauth_session(session_path: Path, lock_path: Optional[Path] = None) -> None: + try: + session_path.unlink(missing_ok=True) + except OSError: + pass + if lock_path is None: + lock_path = Path(f"{session_path}.refresh.lock") + try: + lock_path.unlink(missing_ok=True) + except OSError: + pass + + +def _normalize_text(value: Optional[object]) -> str: + if not isinstance(value, str): + return "" + return value.strip() + + +def _string_value(value: object) -> str: + return value if isinstance(value, str) else "" diff --git a/hyperbrowser/transport/async_transport.py b/hyperbrowser/transport/async_transport.py index 8bf40338..49a7cf51 100644 --- a/hyperbrowser/transport/async_transport.py +++ b/hyperbrowser/transport/async_transport.py @@ -1,16 +1,17 @@ import asyncio import httpx -from typing import Optional +from typing import Any, Dict, Optional from hyperbrowser.exceptions import HyperbrowserError -from .base import TransportStrategy, APIResponse +from .base import TransportStrategy, APIResponse, is_request_replayable, merge_headers class AsyncTransport(TransportStrategy): """Asynchronous transport implementation using httpx""" - def __init__(self, api_key: str): - self.client = httpx.AsyncClient(headers={"x-api-key": api_key}) + def __init__(self, auth): + self.auth = auth + self.client = httpx.AsyncClient() self._closed = False async def close(self) -> None: @@ -73,49 +74,114 @@ async def post( files: Optional[dict] = None, timeout: Optional[float] = None, ) -> APIResponse: - try: - kwargs = {} - if timeout is not None: - kwargs["timeout"] = timeout - if files: - response = await self.client.post(url, data=data, files=files, **kwargs) - else: - response = await self.client.post(url, json=data, **kwargs) - return await self._handle_response(response) - except HyperbrowserError: - raise - except Exception as e: - raise HyperbrowserError("Post request failed", original_error=e) + return await self._request( + "POST", + url, + json_data=None if files else data, + data=data if files else None, + files=files, + timeout=timeout, + replayable=is_request_replayable(files), + ) async def get( self, url: str, params: Optional[dict] = None, follow_redirects: bool = False ) -> APIResponse: if params: params = {k: v for k, v in params.items() if v is not None} - try: - response = await self.client.get( - url, params=params, follow_redirects=follow_redirects - ) - return await self._handle_response(response) - except HyperbrowserError: - raise - except Exception as e: - raise HyperbrowserError("Get request failed", original_error=e) + return await self._request( + "GET", + url, + params=params, + follow_redirects=follow_redirects, + ) async def put(self, url: str, data: Optional[dict] = None) -> APIResponse: - try: - response = await self.client.put(url, json=data) - return await self._handle_response(response) - except HyperbrowserError: - raise - except Exception as e: - raise HyperbrowserError("Put request failed", original_error=e) + return await self._request("PUT", url, json_data=data) async def delete(self, url: str) -> APIResponse: + return await self._request("DELETE", url) + + async def _request( + self, + method: str, + url: str, + *, + params: Optional[dict] = None, + json_data: Optional[Any] = None, + data: Optional[Any] = None, + files: Optional[Any] = None, + timeout: Optional[float] = None, + follow_redirects: bool = False, + replayable: bool = True, + ) -> APIResponse: try: - response = await self.client.delete(url) + auth_headers, access_token = await self.auth.aauthorize_headers() + response = await self._send( + method, + url, + params=params, + json_data=json_data, + data=data, + files=files, + auth_headers=auth_headers, + timeout=timeout, + follow_redirects=follow_redirects, + ) + if ( + response.status_code == 401 + and getattr(self.auth, "is_oauth", False) + and replayable + ): + await response.aclose() + retry_headers, _ = await self.auth.aauthorize_headers( + force_refresh=True, + rejected_access_token=access_token, + ) + response = await self._send( + method, + url, + params=params, + json_data=json_data, + data=data, + files=files, + auth_headers=retry_headers, + timeout=timeout, + follow_redirects=follow_redirects, + ) return await self._handle_response(response) except HyperbrowserError: raise except Exception as e: - raise HyperbrowserError("Delete request failed", original_error=e) + raise HyperbrowserError( + f"{method.title()} request failed", original_error=e + ) + + async def _send( + self, + method: str, + url: str, + *, + params: Optional[dict], + json_data: Optional[Any], + data: Optional[Any], + files: Optional[Any], + auth_headers: Dict[str, str], + timeout: Optional[float], + follow_redirects: bool, + ) -> httpx.Response: + kwargs: Dict[str, Any] = { + "headers": merge_headers(auth_headers), + "follow_redirects": follow_redirects, + } + if params is not None: + kwargs["params"] = params + if timeout is not None: + kwargs["timeout"] = timeout + if json_data is not None: + kwargs["json"] = json_data + if data is not None: + kwargs["data"] = data + if files is not None: + kwargs["files"] = files + return await self.client.request(method, url, **kwargs) diff --git a/hyperbrowser/transport/base.py b/hyperbrowser/transport/base.py index fee99f95..642b7f73 100644 --- a/hyperbrowser/transport/base.py +++ b/hyperbrowser/transport/base.py @@ -1,8 +1,11 @@ from abc import ABC, abstractmethod -from typing import Optional, TypeVar, Generic, Type, Union +from typing import TYPE_CHECKING, Any, Dict, Optional, TypeVar, Generic, Type, Union from hyperbrowser.exceptions import HyperbrowserError +if TYPE_CHECKING: + from hyperbrowser.control_auth import ControlPlaneAuthManager + T = TypeVar("T") @@ -37,7 +40,7 @@ class TransportStrategy(ABC): """Abstract base class for different transport implementations""" @abstractmethod - def __init__(self, api_key: str): + def __init__(self, auth: "ControlPlaneAuthManager"): pass @abstractmethod @@ -65,3 +68,41 @@ def put(self, url: str) -> APIResponse: @abstractmethod def delete(self, url: str) -> APIResponse: pass + + +def is_request_replayable(files: Optional[Any] = None) -> bool: + if files is None: + return True + return _are_files_replayable(files) + + +def _are_files_replayable(files: Any) -> bool: + if isinstance(files, dict): + values = list(files.values()) + elif isinstance(files, list): + values = [value for _, value in files] + elif isinstance(files, tuple) and len(files) == 2: + values = [files[1]] + else: + values = [files] + return all(_is_file_value_replayable(value) for value in values) + + +def _is_file_value_replayable(value: Any) -> bool: + if isinstance(value, tuple) and len(value) >= 2: + return _is_file_value_replayable(value[1]) + return isinstance(value, (str, bytes, bytearray, memoryview)) + + +def merge_headers( + *header_groups: Optional[Dict[str, str]], +) -> Dict[str, str]: + merged: Dict[str, str] = {} + for headers in header_groups: + if not headers: + continue + for key, value in headers.items(): + if value is None: + continue + merged[str(key)] = str(value) + return merged diff --git a/hyperbrowser/transport/sync.py b/hyperbrowser/transport/sync.py index b4af6e7f..1172f62f 100644 --- a/hyperbrowser/transport/sync.py +++ b/hyperbrowser/transport/sync.py @@ -1,15 +1,16 @@ import httpx -from typing import Optional +from typing import Any, Dict, Optional from hyperbrowser.exceptions import HyperbrowserError -from .base import TransportStrategy, APIResponse +from .base import TransportStrategy, APIResponse, is_request_replayable, merge_headers class SyncTransport(TransportStrategy): """Synchronous transport implementation using httpx""" - def __init__(self, api_key: str): - self.client = httpx.Client(headers={"x-api-key": api_key}) + def __init__(self, auth): + self.auth = auth + self.client = httpx.Client() def _handle_response(self, response: httpx.Response) -> APIResponse: try: @@ -52,49 +53,114 @@ def post( files: Optional[dict] = None, timeout: Optional[float] = None, ) -> APIResponse: - try: - kwargs = {} - if timeout is not None: - kwargs["timeout"] = timeout - if files: - response = self.client.post(url, data=data, files=files, **kwargs) - else: - response = self.client.post(url, json=data, **kwargs) - return self._handle_response(response) - except HyperbrowserError: - raise - except Exception as e: - raise HyperbrowserError("Post request failed", original_error=e) + return self._request( + "POST", + url, + json_data=None if files else data, + data=data if files else None, + files=files, + timeout=timeout, + replayable=is_request_replayable(files), + ) def get( self, url: str, params: Optional[dict] = None, follow_redirects: bool = False ) -> APIResponse: if params: params = {k: v for k, v in params.items() if v is not None} - try: - response = self.client.get( - url, params=params, follow_redirects=follow_redirects - ) - return self._handle_response(response) - except HyperbrowserError: - raise - except Exception as e: - raise HyperbrowserError("Get request failed", original_error=e) + return self._request( + "GET", + url, + params=params, + follow_redirects=follow_redirects, + ) def put(self, url: str, data: Optional[dict] = None) -> APIResponse: - try: - response = self.client.put(url, json=data) - return self._handle_response(response) - except HyperbrowserError: - raise - except Exception as e: - raise HyperbrowserError("Put request failed", original_error=e) + return self._request("PUT", url, json_data=data) def delete(self, url: str) -> APIResponse: + return self._request("DELETE", url) + + def _request( + self, + method: str, + url: str, + *, + params: Optional[dict] = None, + json_data: Optional[Any] = None, + data: Optional[Any] = None, + files: Optional[Any] = None, + timeout: Optional[float] = None, + follow_redirects: bool = False, + replayable: bool = True, + ) -> APIResponse: try: - response = self.client.delete(url) + auth_headers, access_token = self.auth.authorize_headers() + response = self._send( + method, + url, + params=params, + json_data=json_data, + data=data, + files=files, + auth_headers=auth_headers, + timeout=timeout, + follow_redirects=follow_redirects, + ) + if ( + response.status_code == 401 + and getattr(self.auth, "is_oauth", False) + and replayable + ): + response.close() + retry_headers, _ = self.auth.authorize_headers( + force_refresh=True, + rejected_access_token=access_token, + ) + response = self._send( + method, + url, + params=params, + json_data=json_data, + data=data, + files=files, + auth_headers=retry_headers, + timeout=timeout, + follow_redirects=follow_redirects, + ) return self._handle_response(response) except HyperbrowserError: raise except Exception as e: - raise HyperbrowserError("Delete request failed", original_error=e) + raise HyperbrowserError( + f"{method.title()} request failed", original_error=e + ) + + def _send( + self, + method: str, + url: str, + *, + params: Optional[dict], + json_data: Optional[Any], + data: Optional[Any], + files: Optional[Any], + auth_headers: Dict[str, str], + timeout: Optional[float], + follow_redirects: bool, + ) -> httpx.Response: + kwargs: Dict[str, Any] = { + "headers": merge_headers(auth_headers), + "follow_redirects": follow_redirects, + } + if params is not None: + kwargs["params"] = params + if timeout is not None: + kwargs["timeout"] = timeout + if json_data is not None: + kwargs["json"] = json_data + if data is not None: + kwargs["data"] = data + if files is not None: + kwargs["files"] = files + return self.client.request(method, url, **kwargs) diff --git a/tests/test_control_auth.py b/tests/test_control_auth.py new file mode 100644 index 00000000..b4814c56 --- /dev/null +++ b/tests/test_control_auth.py @@ -0,0 +1,333 @@ +import asyncio +import json +from datetime import datetime, timedelta, timezone +from pathlib import Path + +import httpx +import pytest + +from hyperbrowser import Hyperbrowser +from hyperbrowser.config import ClientConfig +from hyperbrowser.control_auth import ( + DEFAULT_BASE_URL, + DEFAULT_FRONTEND_BASE_URL, + resolve_control_plane_config, + resolve_frontend_base_url, +) +from hyperbrowser.exceptions import HyperbrowserError + + +AUTH_ENV = ( + "HYPERBROWSER_API_KEY", + "HYPERBROWSER_BASE_URL", + "HYPERBROWSER_PROFILE", + "HYPERBROWSER_FRONTEND_URL", + "HYPERBROWSER_AUTH_LOCK_TIMEOUT_MS", + "HYPERBROWSER_AUTH_LOCK_POLL_INTERVAL_MS", + "HYPERBROWSER_AUTH_LOCK_STALE_MS", +) + + +@pytest.fixture +def auth_home(tmp_path, monkeypatch): + monkeypatch.setenv("HOME", str(tmp_path)) + for key in AUTH_ENV: + monkeypatch.delenv(key, raising=False) + return tmp_path + + +def _expiry(*, hours=0, minutes=0): + return ( + datetime.now(timezone.utc) + timedelta(hours=hours, minutes=minutes) + ).isoformat() + + +def write_session(home, profile="default", **overrides): + path = Path(home) / ".hx_config" / "auth" / f"{profile}.json" + path.parent.mkdir(parents=True, exist_ok=True) + session = { + "version": 1, + "base_url": DEFAULT_BASE_URL, + "client_id": "hyperbrowser-cli", + "token_type": "Bearer", + "access_token": "access-token", + "refresh_token": "refresh-token", + "expiry": _expiry(hours=1), + "scope": "cli", + } + session.update(overrides) + path.write_text(json.dumps(session) + "\n") + return path + + +def _patch_httpx_client(monkeypatch, handler): + real_client = httpx.Client + + def fake_client(*args, **kwargs): + kwargs["transport"] = httpx.MockTransport(handler) + return real_client(*args, **kwargs) + + monkeypatch.setattr(httpx, "Client", fake_client) + monkeypatch.setattr("hyperbrowser.control_auth.httpx.Client", fake_client) + monkeypatch.setattr("hyperbrowser.transport.sync.httpx.Client", fake_client) + + +def test_missing_auth_raises(auth_home): + with pytest.raises(HyperbrowserError, match="hx auth login") as exc: + Hyperbrowser() + assert exc.value.code == "missing_auth" + + +def test_from_env_does_not_require_api_key(auth_home): + config = ClientConfig.from_env() + assert config.api_key is None + assert config.profile is None + + +def test_api_key_is_preferred_over_saved_session(auth_home): + write_session(auth_home) + base_url, auth = resolve_control_plane_config(ClientConfig(api_key="hb_live_key")) + assert base_url == DEFAULT_BASE_URL + assert auth.is_oauth is False + headers, token = auth.authorize_headers() + assert headers == {"x-api-key": "hb_live_key"} + assert token is None + + +def test_env_api_key_is_preferred_over_saved_session(auth_home, monkeypatch): + write_session(auth_home) + monkeypatch.setenv("HYPERBROWSER_API_KEY", "env-key") + _, auth = resolve_control_plane_config(ClientConfig()) + headers, _ = auth.authorize_headers() + assert headers == {"x-api-key": "env-key"} + + +def test_oauth_session_is_used_when_no_api_key(auth_home): + write_session(auth_home, access_token="session-access") + client = Hyperbrowser() + try: + assert client.auth.is_oauth is True + headers, token = client.auth.authorize_headers() + assert headers == {"authorization": "Bearer session-access"} + assert token == "session-access" + finally: + client.close() + + +def test_profile_comes_from_constructor(auth_home): + write_session(auth_home, profile="work", access_token="work-access") + client = Hyperbrowser(profile="work") + try: + headers, _ = client.auth.authorize_headers() + assert headers["authorization"] == "Bearer work-access" + finally: + client.close() + + +def test_profile_comes_from_env(auth_home, monkeypatch): + write_session(auth_home, profile="ci", access_token="ci-access") + monkeypatch.setenv("HYPERBROWSER_PROFILE", "ci") + _, auth = resolve_control_plane_config(ClientConfig()) + headers, _ = auth.authorize_headers() + assert headers["authorization"] == "Bearer ci-access" + + +def test_invalid_profile_name_is_rejected(auth_home): + with pytest.raises(HyperbrowserError) as exc: + Hyperbrowser(profile="bad profile") + assert exc.value.code == "invalid_profile" + + +def test_base_url_strips_api_suffix(auth_home): + write_session(auth_home) + base_url, _ = resolve_control_plane_config( + ClientConfig(base_url="https://api.hyperbrowser.ai/api") + ) + assert base_url == DEFAULT_BASE_URL + + +def test_legacy_app_base_url_maps_to_api(auth_home): + write_session(auth_home, base_url="https://app.hyperbrowser.ai") + base_url, auth = resolve_control_plane_config(ClientConfig()) + assert base_url == DEFAULT_BASE_URL + assert auth.is_oauth is True + + +def test_oauth_base_url_mismatch(auth_home): + write_session(auth_home, base_url="https://staging.hyperbrowser.dev") + with pytest.raises(HyperbrowserError) as exc: + resolve_control_plane_config(ClientConfig()) + assert exc.value.code == "oauth_base_url_mismatch" + + +def test_invalid_session_json(auth_home): + path = Path(auth_home) / ".hx_config" / "auth" / "default.json" + path.parent.mkdir(parents=True) + path.write_text("{not-json") + with pytest.raises(HyperbrowserError) as exc: + resolve_control_plane_config(ClientConfig()) + assert exc.value.code == "oauth_session_invalid" + + +def test_resolve_frontend_base_url_defaults_and_overrides(auth_home, monkeypatch): + assert resolve_frontend_base_url(DEFAULT_BASE_URL) == DEFAULT_FRONTEND_BASE_URL + assert ( + resolve_frontend_base_url("https://app.hyperbrowser.ai") + == DEFAULT_FRONTEND_BASE_URL + ) + assert ( + resolve_frontend_base_url("https://staging.example.com") + == "https://staging.example.com" + ) + monkeypatch.setenv("HYPERBROWSER_FRONTEND_URL", "https://front.example.com/api") + assert resolve_frontend_base_url(DEFAULT_BASE_URL) == "https://front.example.com" + assert ( + resolve_frontend_base_url( + DEFAULT_BASE_URL, explicit_frontend_url="https://explicit.example" + ) + == "https://explicit.example" + ) + + +def test_refresh_uses_frontend_url_and_persists_session(auth_home, monkeypatch): + write_session( + auth_home, + access_token="expired-access", + expiry=_expiry(minutes=-5), + ) + requests = [] + + def handler(request: httpx.Request) -> httpx.Response: + requests.append(request) + assert str(request.url) == f"{DEFAULT_FRONTEND_BASE_URL}/oauth/token" + assert request.content.decode("utf-8").find("grant_type=refresh_token") >= 0 + return httpx.Response( + 200, + json={ + "access_token": "refreshed-access", + "refresh_token": "rotated-refresh", + "expires_in": 3600, + "token_type": "Bearer", + "scope": "cli", + }, + ) + + _patch_httpx_client(monkeypatch, handler) + _, auth = resolve_control_plane_config(ClientConfig()) + headers, token = auth.authorize_headers() + + assert headers == {"authorization": "Bearer refreshed-access"} + assert token == "refreshed-access" + assert len(requests) == 1 + + saved = json.loads( + (Path(auth_home) / ".hx_config" / "auth" / "default.json").read_text() + ) + assert saved["access_token"] == "refreshed-access" + assert saved["refresh_token"] == "rotated-refresh" + assert saved["base_url"] == DEFAULT_BASE_URL + + +def test_async_refresh_uses_frontend_url(auth_home, monkeypatch): + write_session( + auth_home, + access_token="expired-access", + expiry=_expiry(minutes=-5), + ) + + def handler(request: httpx.Request) -> httpx.Response: + assert request.url.host == "app.hyperbrowser.ai" + return httpx.Response( + 200, + json={"access_token": "async-refreshed", "expires_in": 3600}, + ) + + real_client = httpx.AsyncClient + + def fake_client(*args, **kwargs): + kwargs["transport"] = httpx.MockTransport(handler) + return real_client(*args, **kwargs) + + monkeypatch.setattr("hyperbrowser.control_auth.httpx.AsyncClient", fake_client) + _, auth = resolve_control_plane_config(ClientConfig()) + headers, token = asyncio.run(auth.aauthorize_headers()) + assert headers == {"authorization": "Bearer async-refreshed"} + assert token == "async-refreshed" + + +def test_expired_refresh_token_deletes_session(auth_home): + path = write_session( + auth_home, + access_token="expired-access", + expiry=_expiry(minutes=-5), + refresh_token_expiry=_expiry(minutes=-1), + ) + _, auth = resolve_control_plane_config(ClientConfig()) + with pytest.raises(HyperbrowserError) as exc: + auth.authorize_headers() + assert exc.value.code == "oauth_session_expired" + assert path.exists() is False + + +def test_invalid_grant_deletes_session(auth_home, monkeypatch): + path = write_session( + auth_home, + access_token="expired-access", + expiry=_expiry(minutes=-5), + ) + + def handler(request: httpx.Request) -> httpx.Response: + return httpx.Response(400, json={"error": "invalid_grant"}) + + _patch_httpx_client(monkeypatch, handler) + _, auth = resolve_control_plane_config(ClientConfig()) + with pytest.raises(HyperbrowserError) as exc: + auth.authorize_headers() + assert exc.value.code == "invalid_grant" + assert path.exists() is False + + +def test_rotation_lock_timeout(auth_home): + write_session( + auth_home, + access_token="expired-access", + expiry=_expiry(minutes=-5), + ) + session_path = Path(auth_home) / ".hx_config" / "auth" / "default.json" + lock_path = Path(f"{session_path}.refresh.lock") + lock_path.write_text("pid=1\ncreated_at=2020-01-01T00:00:00Z\n") + + _, auth = resolve_control_plane_config( + ClientConfig(auth_lock_timeout_ms=40, auth_lock_poll_interval_ms=10) + ) + with pytest.raises(HyperbrowserError) as exc: + auth.authorize_headers() + assert exc.value.code == "auth_rotation_timeout" + + +def test_stale_rotation_lock_is_cleared(auth_home, monkeypatch): + write_session( + auth_home, + access_token="expired-access", + expiry=_expiry(minutes=-5), + ) + session_path = Path(auth_home) / ".hx_config" / "auth" / "default.json" + lock_path = Path(f"{session_path}.refresh.lock") + lock_path.write_text("pid=1\n") + import os + + os.utime(lock_path, (0, 0)) + + def handler(request: httpx.Request) -> httpx.Response: + return httpx.Response( + 200, + json={"access_token": "after-stale-lock", "expires_in": 3600}, + ) + + _patch_httpx_client(monkeypatch, handler) + _, auth = resolve_control_plane_config( + ClientConfig(auth_lock_stale_ms=1, auth_lock_timeout_ms=200) + ) + headers, _ = auth.authorize_headers() + assert headers["authorization"] == "Bearer after-stale-lock" + assert lock_path.exists() is False diff --git a/tests/test_transport_auth.py b/tests/test_transport_auth.py new file mode 100644 index 00000000..2a9933b3 --- /dev/null +++ b/tests/test_transport_auth.py @@ -0,0 +1,207 @@ +import asyncio +from datetime import datetime, timedelta, timezone + +import httpx +import pytest + +from hyperbrowser import AsyncHyperbrowser, Hyperbrowser +from hyperbrowser.config import ClientConfig +from hyperbrowser.exceptions import HyperbrowserError + +from tests.test_control_auth import ( + AUTH_ENV, + _patch_httpx_client, + write_session, +) + + +@pytest.fixture +def auth_home(tmp_path, monkeypatch): + monkeypatch.setenv("HOME", str(tmp_path)) + for key in AUTH_ENV: + monkeypatch.delenv(key, raising=False) + return tmp_path + + +def test_api_key_requests_send_x_api_key(auth_home, monkeypatch): + seen = [] + + def handler(request: httpx.Request) -> httpx.Response: + seen.append(request) + assert request.headers.get("x-api-key") == "test-api-key" + return httpx.Response(200, json={"jobId": "job_123"}) + + _patch_httpx_client(monkeypatch, handler) + client = Hyperbrowser(api_key="test-api-key", base_url="https://api.example") + try: + started = client.scrape.start({"url": "https://example.com"}) + finally: + client.close() + + assert started.job_id == "job_123" + assert len(seen) == 1 + + +def test_oauth_401_refreshes_and_retries(auth_home, monkeypatch): + write_session(auth_home, access_token="old-access") + calls = [] + + def handler(request: httpx.Request) -> httpx.Response: + calls.append( + (request.method, request.url.path, request.headers.get("authorization")) + ) + if request.url.path == "/oauth/token": + assert request.url.host == "app.hyperbrowser.ai" + return httpx.Response( + 200, + json={"access_token": "new-access", "expires_in": 3600}, + ) + if request.url.path == "/api/scrape": + if request.headers.get("authorization") == "Bearer old-access": + return httpx.Response(401, json={"message": "unauthorized"}) + assert request.headers.get("authorization") == "Bearer new-access" + return httpx.Response(200, json={"jobId": "job_456"}) + return httpx.Response(404, json={"message": "not found"}) + + _patch_httpx_client(monkeypatch, handler) + client = Hyperbrowser() + try: + started = client.scrape.start({"url": "https://example.com"}) + finally: + client.close() + + assert started.job_id == "job_456" + assert calls == [ + ("POST", "/api/scrape", "Bearer old-access"), + ("POST", "/oauth/token", None), + ("POST", "/api/scrape", "Bearer new-access"), + ] + + +def test_oauth_401_without_replayable_body_does_not_retry( + auth_home, monkeypatch, tmp_path +): + write_session(auth_home, access_token="old-access") + + def handler(request: httpx.Request) -> httpx.Response: + if request.url.path == "/oauth/token": + raise AssertionError("refresh should not run for non-replayable uploads") + return httpx.Response(401, json={"message": "unauthorized"}) + + _patch_httpx_client(monkeypatch, handler) + upload = tmp_path / "file.bin" + upload.write_bytes(b"hello") + client = Hyperbrowser() + try: + with pytest.raises(HyperbrowserError) as exc: + client.sessions.upload_file("session_123", str(upload)) + assert exc.value.status_code == 401 + finally: + client.close() + + +def test_post_timeout_is_forwarded(auth_home, monkeypatch): + seen = [] + + def handler(request: httpx.Request) -> httpx.Response: + seen.append(request.extensions.get("timeout")) + return httpx.Response( + 200, + json={ + "success": True, + "iterationsRequested": 1, + "iterationsRun": 1, + "solved": False, + "solvedCaptchas": [], + "pages": [], + }, + ) + + _patch_httpx_client(monkeypatch, handler) + client = Hyperbrowser(api_key="test-api-key", timeout=12) + try: + client.sessions.evaluate_captcha("session_123") + finally: + client.close() + + assert seen + timeout = seen[0] + assert timeout is not None + if isinstance(timeout, dict): + timeout_values = list(timeout.values()) + else: + timeout_values = [ + getattr(timeout, name, None) + for name in ("read", "write", "connect", "pool") + ] + assert any(value is not None and value >= 12 for value in timeout_values) + + +def test_async_api_key_and_oauth_retry(auth_home, monkeypatch): + write_session(auth_home, access_token="old-access") + calls = [] + + def handler(request: httpx.Request) -> httpx.Response: + calls.append( + (request.method, request.url.path, request.headers.get("authorization")) + ) + if request.url.path == "/oauth/token": + return httpx.Response( + 200, + json={"access_token": "new-access", "expires_in": 3600}, + ) + if request.headers.get("authorization") == "Bearer old-access": + return httpx.Response(401, json={"message": "unauthorized"}) + return httpx.Response(200, json={"jobId": "job_async"}) + + real_async = httpx.AsyncClient + + def fake_async(*args, **kwargs): + kwargs["transport"] = httpx.MockTransport(handler) + return real_async(*args, **kwargs) + + real_client = httpx.Client + + def fake_client(*args, **kwargs): + kwargs["transport"] = httpx.MockTransport(handler) + return real_client(*args, **kwargs) + + monkeypatch.setattr( + "hyperbrowser.transport.async_transport.httpx.AsyncClient", fake_async + ) + monkeypatch.setattr("hyperbrowser.control_auth.httpx.AsyncClient", fake_async) + monkeypatch.setattr("hyperbrowser.control_auth.httpx.Client", fake_client) + + async def run(): + client = AsyncHyperbrowser() + try: + return await client.scrape.start({"url": "https://example.com"}) + finally: + await client.close() + + started = asyncio.run(run()) + assert started.job_id == "job_async" + assert ("POST", "/oauth/token", None) in calls + + +def test_client_config_frontend_url_is_used_for_refresh(auth_home, monkeypatch): + write_session( + auth_home, + access_token="expired-access", + expiry=(datetime.now(timezone.utc) + timedelta(minutes=-5)).isoformat(), + ) + + def handler(request: httpx.Request) -> httpx.Response: + assert str(request.url) == "https://front.example/oauth/token" + return httpx.Response( + 200, + json={"access_token": "front-access", "expires_in": 3600}, + ) + + _patch_httpx_client(monkeypatch, handler) + client = Hyperbrowser(config=ClientConfig(frontend_url="https://front.example")) + try: + headers, _ = client.auth.authorize_headers() + finally: + client.close() + assert headers["authorization"] == "Bearer front-access" From a172a16bce90dab425edce88c977263527c0515a Mon Sep 17 00:00:00 2001 From: Shri Sukhani Date: Sat, 15 Aug 2026 01:26:41 -0700 Subject: [PATCH 2/4] Fix OAuth auth review findings Route sandbox control-plane calls through authenticated transport, stop re-reading env after config is resolved, and harden session refresh, lock handling, and ClientConfig compatibility. --- hyperbrowser/client/base.py | 11 +- .../client/managers/async_manager/sandbox.py | 4 +- .../client/managers/sync_manager/sandbox.py | 4 +- hyperbrowser/config.py | 24 ++- hyperbrowser/control_auth.py | 172 +++++++++++------ hyperbrowser/sandbox_common.py | 14 ++ hyperbrowser/transport/async_transport.py | 93 +++++++--- hyperbrowser/transport/sync.py | 93 +++++++--- tests/test_control_auth.py | 173 +++++++++++++++++- tests/test_transport_auth.py | 25 +++ 10 files changed, 490 insertions(+), 123 deletions(-) diff --git a/hyperbrowser/client/base.py b/hyperbrowser/client/base.py index a771e0a8..3ed3b7b4 100644 --- a/hyperbrowser/client/base.py +++ b/hyperbrowser/client/base.py @@ -1,7 +1,7 @@ from dataclasses import replace from typing import Optional -from ..config import ClientConfig +from ..config import ClientConfig, _env_positive_int from ..control_auth import DEFAULT_BASE_URL, resolve_control_plane_config from ..transport.base import TransportStrategy import os @@ -31,13 +31,20 @@ def __init__( if base_url is not None else os.environ.get("HYPERBROWSER_BASE_URL", DEFAULT_BASE_URL) ), + runtime_proxy_override=runtime_proxy_override, profile=( profile if profile is not None else os.environ.get("HYPERBROWSER_PROFILE") ), frontend_url=os.environ.get("HYPERBROWSER_FRONTEND_URL"), - runtime_proxy_override=runtime_proxy_override, + auth_lock_timeout_ms=_env_positive_int( + "HYPERBROWSER_AUTH_LOCK_TIMEOUT_MS" + ), + auth_lock_poll_interval_ms=_env_positive_int( + "HYPERBROWSER_AUTH_LOCK_POLL_INTERVAL_MS" + ), + auth_lock_stale_ms=_env_positive_int("HYPERBROWSER_AUTH_LOCK_STALE_MS"), ) resolved_base_url, auth = resolve_control_plane_config(config) diff --git a/hyperbrowser/client/managers/async_manager/sandbox.py b/hyperbrowser/client/managers/async_manager/sandbox.py index d879664f..950609ef 100644 --- a/hyperbrowser/client/managers/async_manager/sandbox.py +++ b/hyperbrowser/client/managers/async_manager/sandbox.py @@ -53,6 +53,7 @@ ) from ....sandbox_common import ( RuntimeConnection, + asend_control_http_request, ensure_response_ok, normalize_network_error, parse_json_response, @@ -760,7 +761,8 @@ async def _request( data: Optional[Dict[str, object]] = None, ): try: - response = await self._client.transport.client.request( + response = await asend_control_http_request( + self._client.transport, method, self._client._build_url(path), params={k: v for k, v in (params or {}).items() if v is not None}, diff --git a/hyperbrowser/client/managers/sync_manager/sandbox.py b/hyperbrowser/client/managers/sync_manager/sandbox.py index c1b45fd3..d2522ce9 100644 --- a/hyperbrowser/client/managers/sync_manager/sandbox.py +++ b/hyperbrowser/client/managers/sync_manager/sandbox.py @@ -54,6 +54,7 @@ ensure_response_ok, normalize_network_error, parse_json_response, + send_control_http_request, ) from ..sandboxes.shared import ( _build_sandbox_exposed_url, @@ -749,7 +750,8 @@ def _request( data: Optional[Dict[str, object]] = None, ): try: - response = self._client.transport.client.request( + response = send_control_http_request( + self._client.transport, method, self._client._build_url(path), params={k: v for k, v in (params or {}).items() if v is not None}, diff --git a/hyperbrowser/config.py b/hyperbrowser/config.py index cdb6fdda..a220c4f3 100644 --- a/hyperbrowser/config.py +++ b/hyperbrowser/config.py @@ -3,26 +3,46 @@ import os +def _env_positive_int(name: str) -> Optional[int]: + raw = os.environ.get(name) + if not raw: + return None + try: + parsed = int(raw) + except ValueError: + return None + return parsed if parsed > 0 else None + + @dataclass class ClientConfig: """Configuration for the Hyperbrowser client""" api_key: Optional[str] = None base_url: str = "https://api.hyperbrowser.ai" + runtime_proxy_override: Optional[str] = None profile: Optional[str] = None frontend_url: Optional[str] = None auth_lock_timeout_ms: Optional[int] = None auth_lock_poll_interval_ms: Optional[int] = None auth_lock_stale_ms: Optional[int] = None - runtime_proxy_override: Optional[str] = None @classmethod def from_env(cls) -> "ClientConfig": + api_key = os.environ.get("HYPERBROWSER_API_KEY") + if api_key is None: + raise ValueError("HYPERBROWSER_API_KEY environment variable is required") + return cls( - api_key=os.environ.get("HYPERBROWSER_API_KEY"), + api_key=api_key, base_url=os.environ.get( "HYPERBROWSER_BASE_URL", "https://api.hyperbrowser.ai" ), profile=os.environ.get("HYPERBROWSER_PROFILE"), frontend_url=os.environ.get("HYPERBROWSER_FRONTEND_URL"), + auth_lock_timeout_ms=_env_positive_int("HYPERBROWSER_AUTH_LOCK_TIMEOUT_MS"), + auth_lock_poll_interval_ms=_env_positive_int( + "HYPERBROWSER_AUTH_LOCK_POLL_INTERVAL_MS" + ), + auth_lock_stale_ms=_env_positive_int("HYPERBROWSER_AUTH_LOCK_STALE_MS"), ) diff --git a/hyperbrowser/control_auth.py b/hyperbrowser/control_auth.py index 30f2e466..396923a5 100644 --- a/hyperbrowser/control_auth.py +++ b/hyperbrowser/control_auth.py @@ -21,22 +21,21 @@ DEFAULT_LOCK_TIMEOUT_MS = 30000 DEFAULT_LOCK_POLL_INTERVAL_MS = 125 DEFAULT_LOCK_STALE_MS = 120000 +DEFAULT_OAUTH_REFRESH_TIMEOUT_S = 30.0 OAUTH_REFRESH_EARLY_EXPIRY_MS = 30000 PROFILE_NAME_PATTERN = re.compile(r"^[A-Za-z0-9._-]+$") +ISO_TIMESTAMP_PATTERN = re.compile( + r"^(?P\d{4}-\d{2}-\d{2}[T ]\d{2}:\d{2}:\d{2})" + r"(?P\.\d+)?" + r"(?PZ|[+-]\d{2}:?\d{2})?$", + re.IGNORECASE, +) TERMINAL_OAUTH_REFRESH_ERRORS = { "invalid_grant", "invalid_client", "unauthorized_client", } -ENV_PROFILE = "HYPERBROWSER_PROFILE" -ENV_API_KEY = "HYPERBROWSER_API_KEY" -ENV_BASE_URL = "HYPERBROWSER_BASE_URL" -ENV_FRONTEND_URL = "HYPERBROWSER_FRONTEND_URL" -ENV_LOCK_TIMEOUT_MS = "HYPERBROWSER_AUTH_LOCK_TIMEOUT_MS" -ENV_LOCK_POLL_INTERVAL_MS = "HYPERBROWSER_AUTH_LOCK_POLL_INTERVAL_MS" -ENV_LOCK_STALE_MS = "HYPERBROWSER_AUTH_LOCK_STALE_MS" - class ControlPlaneAuthManager: def __init__(self, mode: Dict[str, object]): @@ -249,9 +248,7 @@ def _get_session_mtime_ns(self) -> Optional[int]: def _refresh_oauth_session(self, session: Dict[str, str]) -> Dict[str, str]: try: - with httpx.Client( - timeout=int(self._mode["lock_timeout_ms"]) / 1000.0 - ) as client: + with httpx.Client(timeout=self._refresh_http_timeout()) as client: response = client.post( str(self._mode["token_url"]), headers={"content-type": "application/x-www-form-urlencoded"}, @@ -272,7 +269,7 @@ def _refresh_oauth_session(self, session: Dict[str, str]) -> Dict[str, str]: async def _arefresh_oauth_session(self, session: Dict[str, str]) -> Dict[str, str]: try: async with httpx.AsyncClient( - timeout=int(self._mode["lock_timeout_ms"]) / 1000.0 + timeout=self._refresh_http_timeout() ) as client: response = await client.post( str(self._mode["token_url"]), @@ -398,6 +395,7 @@ def _clear_stale_rotation_lock(self) -> None: lock_path = Path(str(self._mode["lock_path"])) try: stat = lock_path.stat() + contents = lock_path.read_bytes() except FileNotFoundError: return except OSError as error: @@ -410,22 +408,36 @@ def _clear_stale_rotation_lock(self) -> None: original_error=error, ) - if (time.time() * 1000) - (stat.st_mtime * 1000) < int( - self._mode["lock_stale_ms"] - ): + if not _is_rotation_lock_stale(stat, int(self._mode["lock_stale_ms"])): return try: - lock_path.unlink(missing_ok=True) - except OSError as error: - raise HyperbrowserError( - "Failed to remove stale OAuth rotation lock", - code="auth_rotation_lock_failed", - retryable=False, - service="control", - cause=error, - original_error=error, - ) + restat = lock_path.stat() + current_contents = lock_path.read_bytes() + except FileNotFoundError: + return + except OSError: + return + + if not _is_rotation_lock_stale(restat, int(self._mode["lock_stale_ms"])): + return + if not _same_lock_identity(stat, restat) or current_contents != contents: + return + + try: + lock_path.unlink() + except FileNotFoundError: + return + except OSError: + return + + def _refresh_http_timeout(self) -> float: + timeout = self._mode.get("refresh_timeout_s", DEFAULT_OAUTH_REFRESH_TIMEOUT_S) + try: + parsed = float(timeout) + except (TypeError, ValueError): + return DEFAULT_OAUTH_REFRESH_TIMEOUT_S + return parsed if parsed > 0 else DEFAULT_OAUTH_REFRESH_TIMEOUT_S def _release_rotation_lock(self, lock_fd: int) -> None: lock_path = Path(str(self._mode["lock_path"])) @@ -448,28 +460,25 @@ def _expire_oauth_session(self) -> None: def resolve_control_plane_config( config: ClientConfig, ) -> Tuple[str, ControlPlaneAuthManager]: - explicit_api_key = _normalize_text(config.api_key) - env_api_key = _normalize_text(os.environ.get(ENV_API_KEY)) - explicit_base_url = _normalize_control_base_url(config.base_url) - env_base_url = _normalize_control_base_url(os.environ.get(ENV_BASE_URL)) - - if explicit_api_key or env_api_key: + if config.api_key is not None: + api_key = _normalize_text(config.api_key) + if api_key == "": + raise HyperbrowserError( + "API key must be provided", + code="missing_auth", + retryable=False, + service="control", + ) return ( - explicit_base_url or env_base_url or DEFAULT_BASE_URL, - ControlPlaneAuthManager( - {"kind": "api_key", "api_key": explicit_api_key or env_api_key} - ), + _normalize_control_base_url(config.base_url) or DEFAULT_BASE_URL, + ControlPlaneAuthManager({"kind": "api_key", "api_key": api_key}), ) - profile = _normalize_profile( - config.profile or os.environ.get(ENV_PROFILE) or DEFAULT_PROFILE - ) + profile = _normalize_profile(config.profile or DEFAULT_PROFILE) session_path = _resolve_oauth_session_path(profile) session = _try_load_oauth_session(session_path) - resolved_base_url = ( - explicit_base_url - or env_base_url + _normalize_control_base_url(config.base_url) or _normalize_control_base_url((session or {}).get("base_url", "")) or DEFAULT_BASE_URL ) @@ -503,19 +512,20 @@ def resolve_control_plane_config( "lock_path": f"{session_path}.refresh.lock", "base_url": resolved_base_url, "token_url": f"{frontend_base_url}/oauth/token", + "refresh_timeout_s": DEFAULT_OAUTH_REFRESH_TIMEOUT_S, "lock_timeout_ms": _normalize_positive_int( getattr(config, "auth_lock_timeout_ms", None), - os.environ.get(ENV_LOCK_TIMEOUT_MS), + None, DEFAULT_LOCK_TIMEOUT_MS, ), "lock_poll_interval_ms": _normalize_positive_int( getattr(config, "auth_lock_poll_interval_ms", None), - os.environ.get(ENV_LOCK_POLL_INTERVAL_MS), + None, DEFAULT_LOCK_POLL_INTERVAL_MS, ), "lock_stale_ms": _normalize_positive_int( getattr(config, "auth_lock_stale_ms", None), - os.environ.get(ENV_LOCK_STALE_MS), + None, DEFAULT_LOCK_STALE_MS, ), } @@ -529,9 +539,6 @@ def resolve_frontend_base_url( explicit = _normalize_base_url(explicit_frontend_url) if explicit: return explicit - env_value = _normalize_base_url(os.environ.get(ENV_FRONTEND_URL)) - if env_value: - return env_value if _is_default_control_base_url(control_base_url): return DEFAULT_FRONTEND_BASE_URL return _normalize_base_url(control_base_url) or DEFAULT_FRONTEND_BASE_URL @@ -601,7 +608,8 @@ def _validate_oauth_session( retryable=False, service="control", ) - if _parse_timestamp(session.get("expiry")) is None: + expiry_text = _normalize_text(session.get("expiry")) + if expiry_text and _parse_timestamp(expiry_text) is None: raise HyperbrowserError( "Saved OAuth session has an invalid expiry", code="oauth_session_invalid", @@ -659,9 +667,7 @@ def _build_refreshed_oauth_session( or _normalize_text(previous.get("token_type", "")) or "Bearer" ) - expiry = _derive_expiry(payload.get("expires_in")) or _normalize_text( - previous.get("expiry", "") - ) + expiry = _derive_expiry(payload.get("expires_in")) or "" refresh_expiry = _derive_expiry( payload.get("refresh_token_expires_in") ) or _normalize_text(previous.get("refresh_token_expiry", "")) @@ -696,7 +702,7 @@ def _write_oauth_session_atomic(session_path: Path, session: Dict[str, str]) -> ) renamed = False try: - os.fchmod(fd, 0o600) + _try_fchmod(fd, 0o600) with os.fdopen(fd, "w", encoding="utf-8") as handle: handle.write(payload) handle.flush() @@ -730,9 +736,11 @@ def _should_use_oauth_session( def _is_access_token_usable(session: Dict[str, str]) -> bool: - expiry = _parse_timestamp(session.get("expiry")) - if expiry is None or _normalize_text(session.get("access_token", "")) == "": + if _normalize_text(session.get("access_token", "")) == "": return False + expiry = _parse_timestamp(session.get("expiry")) + if expiry is None: + return True return (expiry * 1000) - (time.time() * 1000) > OAUTH_REFRESH_EARLY_EXPIRY_MS @@ -772,20 +780,64 @@ def _derive_expiry(value: object) -> Optional[str]: return None -def _parse_timestamp(value: Optional[str]) -> Optional[float]: - normalized = _normalize_text(value or "") +def _parse_timestamp(value: Optional[object]) -> Optional[float]: + normalized = _normalize_text(value) if normalized == "": return None - try: - adjusted = normalized.replace("Z", "+00:00") - parsed = datetime.fromisoformat(adjusted) - except ValueError: + parsed = _parse_iso_datetime(normalized) + if parsed is None: return None if parsed.tzinfo is None: parsed = parsed.replace(tzinfo=timezone.utc) return parsed.timestamp() +def _parse_iso_datetime(value: str) -> Optional[datetime]: + match = ISO_TIMESTAMP_PATTERN.fullmatch(value) + if match is None: + try: + return datetime.fromisoformat(value.replace("Z", "+00:00")) + except ValueError: + return None + + head = match.group("head").replace(" ", "T") + frac = match.group("frac") or "" + tz = match.group("tz") or "" + if frac: + frac = "." + frac[1:7].ljust(6, "0") + if tz.upper() == "Z": + tz = "+00:00" + elif len(tz) == 5 and tz[0] in "+-": + tz = f"{tz[:3]}:{tz[3:]}" + + try: + return datetime.fromisoformat(f"{head}{frac}{tz}") + except ValueError: + return None + + +def _is_rotation_lock_stale(stat_result, stale_ms: int) -> bool: + return (time.time() * 1000) - (stat_result.st_mtime * 1000) >= stale_ms + + +def _same_lock_identity(left, right) -> bool: + return ( + getattr(left, "st_dev", None) == getattr(right, "st_dev", None) + and getattr(left, "st_ino", None) == getattr(right, "st_ino", None) + and getattr(left, "st_mtime_ns", None) == getattr(right, "st_mtime_ns", None) + ) + + +def _try_fchmod(fd: int, mode: int) -> None: + setter = getattr(os, "fchmod", None) + if setter is None: + return + try: + setter(fd, mode) + except (NotImplementedError, OSError): + pass + + def _normalize_positive_int( explicit_value: Optional[int], env_value: Optional[str], fallback: int ) -> int: diff --git a/hyperbrowser/sandbox_common.py b/hyperbrowser/sandbox_common.py index a05b1e2a..1bb0936a 100644 --- a/hyperbrowser/sandbox_common.py +++ b/hyperbrowser/sandbox_common.py @@ -30,6 +30,20 @@ def get_request_id(response: httpx.Response) -> Optional[str]: return response.headers.get("x-request-id") or response.headers.get("request-id") +def send_control_http_request(transport, method: str, url: str, **kwargs): + send = getattr(transport, "send_authenticated", None) + if send is not None: + return send(method, url, **kwargs) + return transport.client.request(method, url, **kwargs) + + +async def asend_control_http_request(transport, method: str, url: str, **kwargs): + send = getattr(transport, "send_authenticated", None) + if send is not None: + return await send(method, url, **kwargs) + return await transport.client.request(method, url, **kwargs) + + def is_retryable_network_error(error: BaseException) -> bool: return isinstance( error, diff --git a/hyperbrowser/transport/async_transport.py b/hyperbrowser/transport/async_transport.py index 49a7cf51..d344ac2b 100644 --- a/hyperbrowser/transport/async_transport.py +++ b/hyperbrowser/transport/async_transport.py @@ -102,6 +102,25 @@ async def put(self, url: str, data: Optional[dict] = None) -> APIResponse: async def delete(self, url: str) -> APIResponse: return await self._request("DELETE", url) + async def send_authenticated( + self, + method: str, + url: str, + *, + params: Optional[dict] = None, + json: Optional[Any] = None, + timeout: Optional[float] = None, + follow_redirects: bool = False, + ) -> httpx.Response: + return await self._exchange( + method, + url, + params=params, + json_data=json, + timeout=timeout, + follow_redirects=follow_redirects, + ) + async def _request( self, method: str, @@ -116,39 +135,17 @@ async def _request( replayable: bool = True, ) -> APIResponse: try: - auth_headers, access_token = await self.auth.aauthorize_headers() - response = await self._send( + response = await self._exchange( method, url, params=params, json_data=json_data, data=data, files=files, - auth_headers=auth_headers, timeout=timeout, follow_redirects=follow_redirects, + replayable=replayable, ) - if ( - response.status_code == 401 - and getattr(self.auth, "is_oauth", False) - and replayable - ): - await response.aclose() - retry_headers, _ = await self.auth.aauthorize_headers( - force_refresh=True, - rejected_access_token=access_token, - ) - response = await self._send( - method, - url, - params=params, - json_data=json_data, - data=data, - files=files, - auth_headers=retry_headers, - timeout=timeout, - follow_redirects=follow_redirects, - ) return await self._handle_response(response) except HyperbrowserError: raise @@ -157,6 +154,54 @@ async def _request( f"{method.title()} request failed", original_error=e ) + async def _exchange( + self, + method: str, + url: str, + *, + params: Optional[dict] = None, + json_data: Optional[Any] = None, + data: Optional[Any] = None, + files: Optional[Any] = None, + timeout: Optional[float] = None, + follow_redirects: bool = False, + replayable: bool = True, + ) -> httpx.Response: + auth_headers, access_token = await self.auth.aauthorize_headers() + response = await self._send( + method, + url, + params=params, + json_data=json_data, + data=data, + files=files, + auth_headers=auth_headers, + timeout=timeout, + follow_redirects=follow_redirects, + ) + if ( + response.status_code == 401 + and getattr(self.auth, "is_oauth", False) + and replayable + ): + await response.aclose() + retry_headers, _ = await self.auth.aauthorize_headers( + force_refresh=True, + rejected_access_token=access_token, + ) + response = await self._send( + method, + url, + params=params, + json_data=json_data, + data=data, + files=files, + auth_headers=retry_headers, + timeout=timeout, + follow_redirects=follow_redirects, + ) + return response + async def _send( self, method: str, diff --git a/hyperbrowser/transport/sync.py b/hyperbrowser/transport/sync.py index 1172f62f..ebd68e86 100644 --- a/hyperbrowser/transport/sync.py +++ b/hyperbrowser/transport/sync.py @@ -81,6 +81,25 @@ def put(self, url: str, data: Optional[dict] = None) -> APIResponse: def delete(self, url: str) -> APIResponse: return self._request("DELETE", url) + def send_authenticated( + self, + method: str, + url: str, + *, + params: Optional[dict] = None, + json: Optional[Any] = None, + timeout: Optional[float] = None, + follow_redirects: bool = False, + ) -> httpx.Response: + return self._exchange( + method, + url, + params=params, + json_data=json, + timeout=timeout, + follow_redirects=follow_redirects, + ) + def _request( self, method: str, @@ -95,39 +114,17 @@ def _request( replayable: bool = True, ) -> APIResponse: try: - auth_headers, access_token = self.auth.authorize_headers() - response = self._send( + response = self._exchange( method, url, params=params, json_data=json_data, data=data, files=files, - auth_headers=auth_headers, timeout=timeout, follow_redirects=follow_redirects, + replayable=replayable, ) - if ( - response.status_code == 401 - and getattr(self.auth, "is_oauth", False) - and replayable - ): - response.close() - retry_headers, _ = self.auth.authorize_headers( - force_refresh=True, - rejected_access_token=access_token, - ) - response = self._send( - method, - url, - params=params, - json_data=json_data, - data=data, - files=files, - auth_headers=retry_headers, - timeout=timeout, - follow_redirects=follow_redirects, - ) return self._handle_response(response) except HyperbrowserError: raise @@ -136,6 +133,54 @@ def _request( f"{method.title()} request failed", original_error=e ) + def _exchange( + self, + method: str, + url: str, + *, + params: Optional[dict] = None, + json_data: Optional[Any] = None, + data: Optional[Any] = None, + files: Optional[Any] = None, + timeout: Optional[float] = None, + follow_redirects: bool = False, + replayable: bool = True, + ) -> httpx.Response: + auth_headers, access_token = self.auth.authorize_headers() + response = self._send( + method, + url, + params=params, + json_data=json_data, + data=data, + files=files, + auth_headers=auth_headers, + timeout=timeout, + follow_redirects=follow_redirects, + ) + if ( + response.status_code == 401 + and getattr(self.auth, "is_oauth", False) + and replayable + ): + response.close() + retry_headers, _ = self.auth.authorize_headers( + force_refresh=True, + rejected_access_token=access_token, + ) + response = self._send( + method, + url, + params=params, + json_data=json_data, + data=data, + files=files, + auth_headers=retry_headers, + timeout=timeout, + follow_redirects=follow_redirects, + ) + return response + def _send( self, method: str, diff --git a/tests/test_control_auth.py b/tests/test_control_auth.py index b4814c56..06df0425 100644 --- a/tests/test_control_auth.py +++ b/tests/test_control_auth.py @@ -11,6 +11,8 @@ from hyperbrowser.control_auth import ( DEFAULT_BASE_URL, DEFAULT_FRONTEND_BASE_URL, + DEFAULT_OAUTH_REFRESH_TIMEOUT_S, + _parse_timestamp, resolve_control_plane_config, resolve_frontend_base_url, ) @@ -78,9 +80,24 @@ def test_missing_auth_raises(auth_home): assert exc.value.code == "missing_auth" -def test_from_env_does_not_require_api_key(auth_home): +def test_from_env_requires_api_key(auth_home): + with pytest.raises( + ValueError, match="HYPERBROWSER_API_KEY environment variable is required" + ): + ClientConfig.from_env() + + +def test_from_env_reads_api_key(auth_home, monkeypatch): + monkeypatch.setenv("HYPERBROWSER_API_KEY", "env-key") config = ClientConfig.from_env() - assert config.api_key is None + assert config.api_key == "env-key" + + +def test_client_config_positional_runtime_proxy_override_is_preserved(): + config = ClientConfig("key", "https://api.example", "socks5://proxy") + assert config.api_key == "key" + assert config.base_url == "https://api.example" + assert config.runtime_proxy_override == "socks5://proxy" assert config.profile is None @@ -94,14 +111,46 @@ def test_api_key_is_preferred_over_saved_session(auth_home): assert token is None -def test_env_api_key_is_preferred_over_saved_session(auth_home, monkeypatch): +def test_constructor_env_api_key_is_preferred_over_saved_session( + auth_home, monkeypatch +): write_session(auth_home) monkeypatch.setenv("HYPERBROWSER_API_KEY", "env-key") - _, auth = resolve_control_plane_config(ClientConfig()) - headers, _ = auth.authorize_headers() + client = Hyperbrowser() + try: + headers, _ = client.auth.authorize_headers() + finally: + client.close() assert headers == {"x-api-key": "env-key"} +def test_explicit_empty_api_key_raises(auth_home, monkeypatch): + write_session(auth_home) + monkeypatch.setenv("HYPERBROWSER_API_KEY", "env-key") + with pytest.raises(HyperbrowserError, match="API key must be provided") as exc: + Hyperbrowser(api_key="") + assert exc.value.code == "missing_auth" + + +def test_client_config_none_api_key_does_not_read_env_key(auth_home, monkeypatch): + write_session(auth_home, access_token="session-access") + monkeypatch.setenv("HYPERBROWSER_API_KEY", "env-key") + _, auth = resolve_control_plane_config(ClientConfig(api_key=None)) + headers, _ = auth.authorize_headers() + assert headers == {"authorization": "Bearer session-access"} + + +def test_passed_config_ignores_env_base_url(auth_home, monkeypatch): + write_session(auth_home) + monkeypatch.setenv("HYPERBROWSER_BASE_URL", "https://staging.hyperbrowser.dev") + client = Hyperbrowser(config=ClientConfig()) + try: + assert client.config.base_url == DEFAULT_BASE_URL + assert client.auth.is_oauth is True + finally: + client.close() + + def test_oauth_session_is_used_when_no_api_key(auth_home): write_session(auth_home, access_token="session-access") client = Hyperbrowser() @@ -127,8 +176,11 @@ def test_profile_comes_from_constructor(auth_home): def test_profile_comes_from_env(auth_home, monkeypatch): write_session(auth_home, profile="ci", access_token="ci-access") monkeypatch.setenv("HYPERBROWSER_PROFILE", "ci") - _, auth = resolve_control_plane_config(ClientConfig()) - headers, _ = auth.authorize_headers() + client = Hyperbrowser() + try: + headers, _ = client.auth.authorize_headers() + finally: + client.close() assert headers["authorization"] == "Bearer ci-access" @@ -179,14 +231,14 @@ def test_resolve_frontend_base_url_defaults_and_overrides(auth_home, monkeypatch resolve_frontend_base_url("https://staging.example.com") == "https://staging.example.com" ) - monkeypatch.setenv("HYPERBROWSER_FRONTEND_URL", "https://front.example.com/api") - assert resolve_frontend_base_url(DEFAULT_BASE_URL) == "https://front.example.com" assert ( resolve_frontend_base_url( DEFAULT_BASE_URL, explicit_frontend_url="https://explicit.example" ) == "https://explicit.example" ) + monkeypatch.setenv("HYPERBROWSER_FRONTEND_URL", "https://front.example.com/api") + assert resolve_frontend_base_url(DEFAULT_BASE_URL) == DEFAULT_FRONTEND_BASE_URL def test_refresh_uses_frontend_url_and_persists_session(auth_home, monkeypatch): @@ -331,3 +383,106 @@ def handler(request: httpx.Request) -> httpx.Response: headers, _ = auth.authorize_headers() assert headers["authorization"] == "Bearer after-stale-lock" assert lock_path.exists() is False + + +def test_stale_lock_clear_does_not_delete_replaced_lock(auth_home, monkeypatch): + write_session( + auth_home, + access_token="expired-access", + expiry=_expiry(minutes=-5), + ) + session_path = Path(auth_home) / ".hx_config" / "auth" / "default.json" + lock_path = Path(f"{session_path}.refresh.lock") + lock_path.write_text("pid=old\n") + import os + + os.utime(lock_path, (0, 0)) + + original_clear = None + from hyperbrowser.control_auth import ControlPlaneAuthManager + + original_clear = ControlPlaneAuthManager._clear_stale_rotation_lock + + def wrap(self): + original_clear(self) + lock_path.write_text("pid=fresh\n") + + monkeypatch.setattr(ControlPlaneAuthManager, "_clear_stale_rotation_lock", wrap) + + _, auth = resolve_control_plane_config( + ClientConfig(auth_lock_stale_ms=1, auth_lock_timeout_ms=40) + ) + with pytest.raises(HyperbrowserError) as exc: + auth.authorize_headers() + assert exc.value.code == "auth_rotation_timeout" + assert lock_path.read_text() == "pid=fresh\n" + + +def test_refresh_without_expires_in_does_not_keep_old_expiry(auth_home, monkeypatch): + old_expiry = _expiry(minutes=-5) + write_session( + auth_home, + access_token="expired-access", + expiry=old_expiry, + ) + + def handler(request: httpx.Request) -> httpx.Response: + return httpx.Response( + 200, + json={ + "access_token": "no-expiry-access", + "refresh_token": "same-refresh", + }, + ) + + _patch_httpx_client(monkeypatch, handler) + _, auth = resolve_control_plane_config(ClientConfig()) + headers, token = auth.authorize_headers() + assert headers == {"authorization": "Bearer no-expiry-access"} + assert token == "no-expiry-access" + + saved = json.loads( + (Path(auth_home) / ".hx_config" / "auth" / "default.json").read_text() + ) + assert saved["access_token"] == "no-expiry-access" + assert saved["expiry"] == "" + headers_again, _ = auth.authorize_headers() + assert headers_again == {"authorization": "Bearer no-expiry-access"} + + +def test_parse_timestamp_accepts_variable_fractional_seconds(): + assert _parse_timestamp("2026-08-15T12:00:00Z") is not None + assert _parse_timestamp("2026-08-15T12:00:00.1Z") is not None + assert _parse_timestamp("2026-08-15T12:00:00.123456789+00:00") is not None + assert _parse_timestamp("2026-08-15T12:00:00.123456789Z") is not None + nine = _parse_timestamp("2026-08-15T12:00:00.123456789Z") + six = _parse_timestamp("2026-08-15T12:00:00.123456Z") + assert nine is not None and six is not None + assert abs(nine - six) < 0.001 + + +def test_refresh_timeout_is_independent_of_lock_timeout(auth_home, monkeypatch): + write_session( + auth_home, + access_token="expired-access", + expiry=_expiry(minutes=-5), + ) + seen = [] + + real_client = httpx.Client + + def fake_client(*args, **kwargs): + seen.append(kwargs.get("timeout")) + kwargs["transport"] = httpx.MockTransport( + lambda request: httpx.Response( + 200, + json={"access_token": "tok", "expires_in": 3600}, + ) + ) + return real_client(*args, **kwargs) + + monkeypatch.setattr("hyperbrowser.control_auth.httpx.Client", fake_client) + _, auth = resolve_control_plane_config(ClientConfig(auth_lock_timeout_ms=5)) + auth.authorize_headers() + assert seen + assert seen[0] == DEFAULT_OAUTH_REFRESH_TIMEOUT_S diff --git a/tests/test_transport_auth.py b/tests/test_transport_auth.py index 2a9933b3..5378d51a 100644 --- a/tests/test_transport_auth.py +++ b/tests/test_transport_auth.py @@ -13,6 +13,7 @@ _patch_httpx_client, write_session, ) +from tests.test_sandbox_wire_contract import SANDBOX_DETAIL_PAYLOAD @pytest.fixture @@ -205,3 +206,27 @@ def handler(request: httpx.Request) -> httpx.Response: finally: client.close() assert headers["authorization"] == "Bearer front-access" + + +def test_sandbox_control_requests_include_api_key(auth_home, monkeypatch): + seen = [] + + def handler(request: httpx.Request) -> httpx.Response: + seen.append( + { + "path": request.url.path, + "api_key": request.headers.get("x-api-key"), + } + ) + return httpx.Response(200, json=SANDBOX_DETAIL_PAYLOAD) + + _patch_httpx_client(monkeypatch, handler) + client = Hyperbrowser(api_key="sandbox-key", base_url="https://api.example") + try: + client.sandboxes.get_detail("sbx_123") + finally: + client.close() + + assert seen + assert seen[0]["path"] == "/api/sandbox/sbx_123" + assert seen[0]["api_key"] == "sandbox-key" From fa10ca2d8d3b5bda09759f79f2008006d07917b4 Mon Sep 17 00:00:00 2001 From: Shri Sukhani Date: Sat, 15 Aug 2026 13:10:22 -0700 Subject: [PATCH 3/4] Harden OAuth refresh and collapse duplicated auth plumbing Always refresh on 401, cache tokens in memory, run async auth off the event loop, and stop deleting sessions or locks owned by another process. --- hyperbrowser/client/base.py | 33 +- .../client/managers/async_manager/session.py | 29 +- .../client/managers/sync_manager/session.py | 29 +- hyperbrowser/config.py | 50 +- hyperbrowser/control_auth.py | 563 ++++++++---------- hyperbrowser/sandbox_common.py | 20 +- hyperbrowser/transport/async_transport.py | 50 +- hyperbrowser/transport/base.py | 95 ++- hyperbrowser/transport/sync.py | 50 +- tests/test_control_auth.py | 107 +++- tests/test_sandbox_wire_contract.py | 36 +- tests/test_transport_auth.py | 37 +- 12 files changed, 633 insertions(+), 466 deletions(-) diff --git a/hyperbrowser/client/base.py b/hyperbrowser/client/base.py index 3ed3b7b4..69823e75 100644 --- a/hyperbrowser/client/base.py +++ b/hyperbrowser/client/base.py @@ -1,10 +1,9 @@ from dataclasses import replace from typing import Optional -from ..config import ClientConfig, _env_positive_int -from ..control_auth import DEFAULT_BASE_URL, resolve_control_plane_config +from ..config import ClientConfig +from ..control_auth import resolve_control_plane_config from ..transport.base import TransportStrategy -import os class HyperbrowserBase: @@ -20,31 +19,11 @@ def __init__( profile: Optional[str] = None, ): if config is None: - config = ClientConfig( - api_key=( - api_key - if api_key is not None - else os.environ.get("HYPERBROWSER_API_KEY") - ), - base_url=( - base_url - if base_url is not None - else os.environ.get("HYPERBROWSER_BASE_URL", DEFAULT_BASE_URL) - ), + config = ClientConfig.from_constructor( + api_key=api_key, + base_url=base_url, runtime_proxy_override=runtime_proxy_override, - profile=( - profile - if profile is not None - else os.environ.get("HYPERBROWSER_PROFILE") - ), - frontend_url=os.environ.get("HYPERBROWSER_FRONTEND_URL"), - auth_lock_timeout_ms=_env_positive_int( - "HYPERBROWSER_AUTH_LOCK_TIMEOUT_MS" - ), - auth_lock_poll_interval_ms=_env_positive_int( - "HYPERBROWSER_AUTH_LOCK_POLL_INTERVAL_MS" - ), - auth_lock_stale_ms=_env_positive_int("HYPERBROWSER_AUTH_LOCK_STALE_MS"), + profile=profile, ) resolved_base_url, auth = resolve_control_plane_config(config) diff --git a/hyperbrowser/client/managers/async_manager/session.py b/hyperbrowser/client/managers/async_manager/session.py index 81dd5dca..225f698a 100644 --- a/hyperbrowser/client/managers/async_manager/session.py +++ b/hyperbrowser/client/managers/async_manager/session.py @@ -1,5 +1,6 @@ import warnings from collections.abc import Mapping +from pathlib import Path from typing import IO, List, Optional, Union, overload from hyperbrowser.client._request import coerce_request, dump_request @@ -42,6 +43,17 @@ CAPTCHA_EVALUATION_REQUEST_TIMEOUT_SECONDS = 185 +def _replayable_upload_files(file_path: str): + path = Path(file_path) + return { + "file": ( + path.name or "upload.bin", + path.read_bytes(), + "application/octet-stream", + ) + } + + class SessionEventLogsManager: def __init__(self, client): self._client = client @@ -163,21 +175,14 @@ async def get_downloads_url(self, id: str) -> GetSessionDownloadsUrlResponse: async def upload_file( self, id: str, file_input: Union[str, IO] ) -> UploadFileResponse: - response = None if isinstance(file_input, str): - with open(file_input, "rb") as file_obj: - files = {"file": file_obj} - response = await self._client.transport.post( - self._client._build_url(f"/session/{id}/uploads"), - files=files, - ) + files = _replayable_upload_files(file_input) else: files = {"file": file_input} - response = await self._client.transport.post( - self._client._build_url(f"/session/{id}/uploads"), - files=files, - ) - + response = await self._client.transport.post( + self._client._build_url(f"/session/{id}/uploads"), + files=files, + ) return UploadFileResponse(**response.data) async def extend_session(self, id: str, duration_minutes: int) -> BasicResponse: diff --git a/hyperbrowser/client/managers/sync_manager/session.py b/hyperbrowser/client/managers/sync_manager/session.py index ec510bfe..507b6f7a 100644 --- a/hyperbrowser/client/managers/sync_manager/session.py +++ b/hyperbrowser/client/managers/sync_manager/session.py @@ -1,5 +1,6 @@ import warnings from collections.abc import Mapping +from pathlib import Path from typing import IO, List, Optional, Union, overload from hyperbrowser.client._request import coerce_request, dump_request @@ -42,6 +43,17 @@ CAPTCHA_EVALUATION_REQUEST_TIMEOUT_SECONDS = 185 +def _replayable_upload_files(file_path: str): + path = Path(file_path) + return { + "file": ( + path.name or "upload.bin", + path.read_bytes(), + "application/octet-stream", + ) + } + + class SessionEventLogsManager: def __init__(self, client): self._client = client @@ -159,21 +171,14 @@ def get_downloads_url(self, id: str) -> GetSessionDownloadsUrlResponse: return GetSessionDownloadsUrlResponse(**response.data) def upload_file(self, id: str, file_input: Union[str, IO]) -> UploadFileResponse: - response = None if isinstance(file_input, str): - with open(file_input, "rb") as file_obj: - files = {"file": file_obj} - response = self._client.transport.post( - self._client._build_url(f"/session/{id}/uploads"), - files=files, - ) + files = _replayable_upload_files(file_input) else: files = {"file": file_input} - response = self._client.transport.post( - self._client._build_url(f"/session/{id}/uploads"), - files=files, - ) - + response = self._client.transport.post( + self._client._build_url(f"/session/{id}/uploads"), + files=files, + ) return UploadFileResponse(**response.data) def extend_session(self, id: str, duration_minutes: int) -> BasicResponse: diff --git a/hyperbrowser/config.py b/hyperbrowser/config.py index a220c4f3..5a6baa61 100644 --- a/hyperbrowser/config.py +++ b/hyperbrowser/config.py @@ -2,6 +2,17 @@ from typing import Optional import os +DEFAULT_BASE_URL = "https://api.hyperbrowser.ai" +DEFAULT_FRONTEND_BASE_URL = "https://app.hyperbrowser.ai" + + +def _env_text(name: str) -> Optional[str]: + value = os.environ.get(name) + if value is None: + return None + stripped = value.strip() + return stripped or None + def _env_positive_int(name: str) -> Optional[int]: raw = os.environ.get(name) @@ -19,7 +30,7 @@ class ClientConfig: """Configuration for the Hyperbrowser client""" api_key: Optional[str] = None - base_url: str = "https://api.hyperbrowser.ai" + base_url: str = DEFAULT_BASE_URL runtime_proxy_override: Optional[str] = None profile: Optional[str] = None frontend_url: Optional[str] = None @@ -28,21 +39,38 @@ class ClientConfig: auth_lock_stale_ms: Optional[int] = None @classmethod - def from_env(cls) -> "ClientConfig": - api_key = os.environ.get("HYPERBROWSER_API_KEY") - if api_key is None: - raise ValueError("HYPERBROWSER_API_KEY environment variable is required") - + def from_constructor( + cls, + *, + api_key: Optional[str] = None, + base_url: Optional[str] = None, + runtime_proxy_override: Optional[str] = None, + profile: Optional[str] = None, + ) -> "ClientConfig": return cls( - api_key=api_key, - base_url=os.environ.get( - "HYPERBROWSER_BASE_URL", "https://api.hyperbrowser.ai" + api_key=api_key + if api_key is not None + else _env_text("HYPERBROWSER_API_KEY"), + base_url=( + base_url + if base_url is not None + else (_env_text("HYPERBROWSER_BASE_URL") or DEFAULT_BASE_URL) + ), + runtime_proxy_override=runtime_proxy_override, + profile=( + profile if profile is not None else _env_text("HYPERBROWSER_PROFILE") ), - profile=os.environ.get("HYPERBROWSER_PROFILE"), - frontend_url=os.environ.get("HYPERBROWSER_FRONTEND_URL"), + frontend_url=_env_text("HYPERBROWSER_FRONTEND_URL"), auth_lock_timeout_ms=_env_positive_int("HYPERBROWSER_AUTH_LOCK_TIMEOUT_MS"), auth_lock_poll_interval_ms=_env_positive_int( "HYPERBROWSER_AUTH_LOCK_POLL_INTERVAL_MS" ), auth_lock_stale_ms=_env_positive_int("HYPERBROWSER_AUTH_LOCK_STALE_MS"), ) + + @classmethod + def from_env(cls) -> "ClientConfig": + api_key = _env_text("HYPERBROWSER_API_KEY") + if api_key is None: + raise ValueError("HYPERBROWSER_API_KEY environment variable is required") + return cls.from_constructor(api_key=api_key) diff --git a/hyperbrowser/control_auth.py b/hyperbrowser/control_auth.py index 396923a5..b70837b5 100644 --- a/hyperbrowser/control_auth.py +++ b/hyperbrowser/control_auth.py @@ -1,22 +1,27 @@ import asyncio +import functools import json import os import re import tempfile import time +from dataclasses import dataclass, field from datetime import datetime, timedelta, timezone from pathlib import Path -from typing import Any, Dict, Optional, Tuple +from typing import Any, Dict, Optional, Tuple, Union from urllib.parse import urlencode import httpx -from .config import ClientConfig +from .config import ( + DEFAULT_BASE_URL, + DEFAULT_FRONTEND_BASE_URL, + ClientConfig, +) from .exceptions import HyperbrowserError +from .sandbox_common import parse_error_payload DEFAULT_PROFILE = "default" -DEFAULT_BASE_URL = "https://api.hyperbrowser.ai" -DEFAULT_FRONTEND_BASE_URL = "https://app.hyperbrowser.ai" LEGACY_DEFAULT_BASE_URL = "https://app.hyperbrowser.ai" DEFAULT_LOCK_TIMEOUT_MS = 30000 DEFAULT_LOCK_POLL_INTERVAL_MS = 125 @@ -37,13 +42,40 @@ } +@dataclass +class _OAuthSettings: + profile: str + session_path: Path + lock_path: Path + base_url: str + token_url: str + refresh_timeout_s: float = DEFAULT_OAUTH_REFRESH_TIMEOUT_S + lock_timeout_ms: int = DEFAULT_LOCK_TIMEOUT_MS + lock_poll_interval_ms: int = DEFAULT_LOCK_POLL_INTERVAL_MS + lock_stale_ms: int = DEFAULT_LOCK_STALE_MS + cached_session: Optional[Dict[str, Any]] = field(default=None) + cached_mtime_ns: Optional[int] = field(default=None) + + class ControlPlaneAuthManager: - def __init__(self, mode: Dict[str, object]): - self._mode = mode + def __init__( + self, + *, + api_key: Optional[str] = None, + oauth: Optional[_OAuthSettings] = None, + ): + if api_key is None and oauth is None: + raise ValueError("api_key or oauth settings are required") + self._api_key = api_key + self._oauth = oauth + + @classmethod + def for_api_key(cls, api_key: str) -> "ControlPlaneAuthManager": + return cls(api_key=api_key) @property def is_oauth(self) -> bool: - return self._mode["kind"] == "oauth" + return self._oauth is not None def authorize_headers( self, @@ -51,8 +83,8 @@ def authorize_headers( force_refresh: bool = False, rejected_access_token: Optional[str] = None, ) -> Tuple[Dict[str, str], Optional[str]]: - if self._mode["kind"] == "api_key": - return {"x-api-key": str(self._mode["api_key"])}, None + if self._api_key is not None: + return {"x-api-key": self._api_key}, None access_token = self._resolve_oauth_access_token( force_refresh=force_refresh, @@ -66,14 +98,18 @@ async def aauthorize_headers( force_refresh: bool = False, rejected_access_token: Optional[str] = None, ) -> Tuple[Dict[str, str], Optional[str]]: - if self._mode["kind"] == "api_key": - return {"x-api-key": str(self._mode["api_key"])}, None - - access_token = await self._aresolve_oauth_access_token( - force_refresh=force_refresh, - rejected_access_token=rejected_access_token, + if self._api_key is not None: + return self.authorize_headers() + + loop = asyncio.get_event_loop() + return await loop.run_in_executor( + None, + functools.partial( + self.authorize_headers, + force_refresh=force_refresh, + rejected_access_token=rejected_access_token, + ), ) - return {"authorization": f"Bearer {access_token}"}, access_token def _resolve_oauth_access_token( self, @@ -85,26 +121,16 @@ def _resolve_oauth_access_token( if _should_use_oauth_session(session, force_refresh, rejected_access_token): return _normalize_text(session["access_token"]) - deadline = time.monotonic() + (int(self._mode["lock_timeout_ms"]) / 1000.0) + oauth = self._oauth + deadline = time.monotonic() + (oauth.lock_timeout_ms / 1000.0) while True: lock_fd = self._try_acquire_rotation_lock() if lock_fd is not None: try: - session, session_mtime_ns = self._load_oauth_session_with_mtime() - if _should_use_oauth_session( - session, force_refresh, rejected_access_token - ): - return _normalize_text(session["access_token"]) - if _is_refresh_token_expired(session): - self._expire_oauth_session() - raise HyperbrowserError( - "OAuth session refresh token expired", - code="oauth_session_expired", - retryable=False, - service="control", - ) - refreshed = self._refresh_oauth_session(session) - return _normalize_text(refreshed["access_token"]) + return self._refresh_oauth_access_token_locked( + force_refresh=force_refresh, + rejected_access_token=rejected_access_token, + ) finally: self._release_rotation_lock(lock_fd) @@ -117,7 +143,7 @@ def _resolve_oauth_access_token( service="control", ) - time.sleep(int(self._mode["lock_poll_interval_ms"]) / 1000.0) + time.sleep(oauth.lock_poll_interval_ms / 1000.0) updated = self._load_updated_oauth_session(session_mtime_ns) if updated is None: continue @@ -125,77 +151,41 @@ def _resolve_oauth_access_token( if _should_use_oauth_session(session, True, rejected_access_token): return _normalize_text(session["access_token"]) if _is_refresh_token_expired(session): - self._expire_oauth_session() - raise HyperbrowserError( - "OAuth session refresh token expired", - code="oauth_session_expired", - retryable=False, - service="control", - ) + raise _oauth_session_expired_error() - async def _aresolve_oauth_access_token( + def _refresh_oauth_access_token_locked( self, *, force_refresh: bool, rejected_access_token: Optional[str], ) -> str: - session, session_mtime_ns = self._load_oauth_session_with_mtime() + session, _ = self._load_oauth_session_with_mtime() if _should_use_oauth_session(session, force_refresh, rejected_access_token): return _normalize_text(session["access_token"]) - - deadline = time.monotonic() + (int(self._mode["lock_timeout_ms"]) / 1000.0) - while True: - lock_fd = self._try_acquire_rotation_lock() - if lock_fd is not None: - try: - session, session_mtime_ns = self._load_oauth_session_with_mtime() - if _should_use_oauth_session( - session, force_refresh, rejected_access_token - ): - return _normalize_text(session["access_token"]) - if _is_refresh_token_expired(session): - self._expire_oauth_session() - raise HyperbrowserError( - "OAuth session refresh token expired", - code="oauth_session_expired", - retryable=False, - service="control", - ) - refreshed = await self._arefresh_oauth_session(session) - return _normalize_text(refreshed["access_token"]) - finally: - self._release_rotation_lock(lock_fd) - - self._clear_stale_rotation_lock() - if time.monotonic() > deadline: - raise HyperbrowserError( - "Timed out waiting for OAuth rotation lock", - code="auth_rotation_timeout", - retryable=False, - service="control", - ) - - await asyncio.sleep(int(self._mode["lock_poll_interval_ms"]) / 1000.0) - updated = self._load_updated_oauth_session(session_mtime_ns) - if updated is None: - continue - session, session_mtime_ns = updated - if _should_use_oauth_session(session, True, rejected_access_token): - return _normalize_text(session["access_token"]) - if _is_refresh_token_expired(session): - self._expire_oauth_session() - raise HyperbrowserError( - "OAuth session refresh token expired", - code="oauth_session_expired", - retryable=False, - service="control", - ) - - def _load_oauth_session(self) -> Dict[str, str]: - session_path = Path(str(self._mode["session_path"])) + if _is_refresh_token_expired(session): + self._expire_oauth_session(session) + raise _oauth_session_expired_error() + refreshed = self._refresh_oauth_session(session) + return _normalize_text(refreshed["access_token"]) + + def _load_oauth_session_with_mtime(self) -> Tuple[Dict[str, Any], Optional[int]]: + oauth = self._oauth + mtime_ns = self._get_session_mtime_ns() + if oauth.cached_session is not None and oauth.cached_mtime_ns == mtime_ns: + return oauth.cached_session, mtime_ns + + session = self._read_oauth_session() + oauth.cached_session = session + oauth.cached_mtime_ns = mtime_ns + return session, mtime_ns + + def _read_oauth_session(self) -> Dict[str, Any]: + session_path = self._oauth.session_path try: raw = session_path.read_text() - except (FileNotFoundError, OSError) as error: + except FileNotFoundError: + raise _oauth_session_expired_error() + except OSError as error: raise HyperbrowserError( "Failed to read saved OAuth session", code="oauth_session_read_failed", @@ -217,26 +207,25 @@ def _load_oauth_session(self) -> Dict[str, str]: original_error=error, ) - _validate_oauth_session(session, expected_base_url=str(self._mode["base_url"])) + _validate_oauth_session(session, expected_base_url=self._oauth.base_url) return session - def _load_oauth_session_with_mtime(self) -> Tuple[Dict[str, str], Optional[int]]: - session = self._load_oauth_session() - return session, self._get_session_mtime_ns() - def _load_updated_oauth_session( self, previous_mtime_ns: Optional[int] - ) -> Optional[Tuple[Dict[str, str], Optional[int]]]: + ) -> Optional[Tuple[Dict[str, Any], Optional[int]]]: current_mtime_ns = self._get_session_mtime_ns() if current_mtime_ns == previous_mtime_ns: return None - return self._load_oauth_session(), current_mtime_ns + self._oauth.cached_session = None + self._oauth.cached_mtime_ns = None + return self._load_oauth_session_with_mtime() def _get_session_mtime_ns(self) -> Optional[int]: - session_path = Path(str(self._mode["session_path"])) try: - return session_path.stat().st_mtime_ns - except (FileNotFoundError, OSError) as error: + return self._oauth.session_path.stat().st_mtime_ns + except FileNotFoundError: + raise _oauth_session_expired_error() + except OSError as error: raise HyperbrowserError( "Failed to inspect saved OAuth session", code="oauth_session_read_failed", @@ -246,33 +235,11 @@ def _get_session_mtime_ns(self) -> Optional[int]: original_error=error, ) - def _refresh_oauth_session(self, session: Dict[str, str]) -> Dict[str, str]: + def _refresh_oauth_session(self, session: Dict[str, Any]) -> Dict[str, Any]: try: - with httpx.Client(timeout=self._refresh_http_timeout()) as client: + with httpx.Client(timeout=self._oauth.refresh_timeout_s) as client: response = client.post( - str(self._mode["token_url"]), - headers={"content-type": "application/x-www-form-urlencoded"}, - content=_build_refresh_form(session), - ) - except Exception as error: - raise HyperbrowserError( - "Failed to refresh OAuth session", - code="oauth_refresh_failed", - retryable=True, - service="control", - cause=error, - original_error=error if isinstance(error, Exception) else None, - ) - - return self._handle_refresh_response(session, response) - - async def _arefresh_oauth_session(self, session: Dict[str, str]) -> Dict[str, str]: - try: - async with httpx.AsyncClient( - timeout=self._refresh_http_timeout() - ) as client: - response = await client.post( - str(self._mode["token_url"]), + self._oauth.token_url, headers={"content-type": "application/x-www-form-urlencoded"}, content=_build_refresh_form(session), ) @@ -289,57 +256,52 @@ async def _arefresh_oauth_session(self, session: Dict[str, str]) -> Dict[str, st return self._handle_refresh_response(session, response) def _handle_refresh_response( - self, session: Dict[str, str], response: httpx.Response - ) -> Dict[str, str]: - raw_text = response.text - payload: Any = {} - if raw_text: - try: - payload = response.json() - except ValueError: - payload = {} + self, session: Dict[str, Any], response: httpx.Response + ) -> Dict[str, Any]: + fallback = f"OAuth refresh failed with status {response.status_code}" + message, code, details = parse_error_payload(response.text, fallback) if response.status_code >= 400: - error_code = "" - if isinstance(payload, dict): - error_code = _normalize_text( - _string_value(payload.get("error")) - ) or _normalize_text(_string_value(payload.get("code"))) - if error_code in TERMINAL_OAUTH_REFRESH_ERRORS: - self._expire_oauth_session() - message = ( - _normalize_text( - _string_value(payload.get("message")) - if isinstance(payload, dict) - else "" - ) - or _normalize_text( - _string_value(payload.get("error_description")) - if isinstance(payload, dict) - else "" + error_code = _normalize_text(code) + if isinstance(details, dict): + error_code = error_code or _normalize_text(details.get("error")) + message = ( + _normalize_text(details.get("error_description")) + or _normalize_text(details.get("message")) + or message ) - or error_code - or f"OAuth refresh failed with status {response.status_code}" - ) + if error_code in TERMINAL_OAUTH_REFRESH_ERRORS: + self._expire_oauth_session(session) raise HyperbrowserError( message, status_code=response.status_code, code=error_code or "oauth_refresh_failed", retryable=False, service="control", - details=_redact_refresh_error_details(payload), + details=_redact_refresh_error_details(details), response=response, ) - if not isinstance(payload, dict): - payload = {} + payload = details if isinstance(details, dict) else {} + if not payload and response.text: + try: + parsed = response.json() + except ValueError: + parsed = {} + payload = parsed if isinstance(parsed, dict) else {} refreshed = _build_refreshed_oauth_session(session, payload) - _write_oauth_session_atomic(Path(str(self._mode["session_path"])), refreshed) + _write_oauth_session_atomic(self._oauth.session_path, refreshed) + try: + mtime_ns = self._oauth.session_path.stat().st_mtime_ns + except OSError: + mtime_ns = None + self._oauth.cached_session = refreshed + self._oauth.cached_mtime_ns = mtime_ns return refreshed def _try_acquire_rotation_lock(self) -> Optional[int]: - lock_path = Path(str(self._mode["lock_path"])) + lock_path = self._oauth.lock_path lock_path.parent.mkdir(mode=0o700, parents=True, exist_ok=True) try: os.chmod(lock_path.parent, 0o700) @@ -379,7 +341,7 @@ def _try_acquire_rotation_lock(self) -> Optional[int]: except OSError: pass try: - lock_path.unlink(missing_ok=True) + lock_path.unlink() except OSError: pass raise HyperbrowserError( @@ -392,10 +354,9 @@ def _try_acquire_rotation_lock(self) -> Optional[int]: ) def _clear_stale_rotation_lock(self) -> None: - lock_path = Path(str(self._mode["lock_path"])) + lock_path = self._oauth.lock_path try: - stat = lock_path.stat() - contents = lock_path.read_bytes() + fd = os.open(str(lock_path), os.O_RDONLY) except FileNotFoundError: return except OSError as error: @@ -408,58 +369,78 @@ def _clear_stale_rotation_lock(self) -> None: original_error=error, ) - if not _is_rotation_lock_stale(stat, int(self._mode["lock_stale_ms"])): - return - try: - restat = lock_path.stat() - current_contents = lock_path.read_bytes() - except FileNotFoundError: - return - except OSError: - return - - if not _is_rotation_lock_stale(restat, int(self._mode["lock_stale_ms"])): - return - if not _same_lock_identity(stat, restat) or current_contents != contents: - return - - try: - lock_path.unlink() + file_stat = os.fstat(fd) + if not _is_rotation_lock_stale(file_stat, self._oauth.lock_stale_ms): + return + try: + path_stat = os.stat(str(lock_path)) + except FileNotFoundError: + return + if not _same_lock_identity(file_stat, path_stat): + return + os.unlink(str(lock_path)) except FileNotFoundError: return except OSError: return - - def _refresh_http_timeout(self) -> float: - timeout = self._mode.get("refresh_timeout_s", DEFAULT_OAUTH_REFRESH_TIMEOUT_S) - try: - parsed = float(timeout) - except (TypeError, ValueError): - return DEFAULT_OAUTH_REFRESH_TIMEOUT_S - return parsed if parsed > 0 else DEFAULT_OAUTH_REFRESH_TIMEOUT_S + finally: + try: + os.close(fd) + except OSError: + pass def _release_rotation_lock(self, lock_fd: int) -> None: - lock_path = Path(str(self._mode["lock_path"])) try: os.close(lock_fd) except OSError: pass try: - lock_path.unlink(missing_ok=True) + self._oauth.lock_path.unlink() except OSError: pass - def _expire_oauth_session(self) -> None: - _delete_oauth_session( - Path(str(self._mode["session_path"])), - Path(str(self._mode["lock_path"])), - ) + def _expire_oauth_session(self, session: Dict[str, Any]) -> None: + current = _try_load_oauth_session(self._oauth.session_path) + if current is None: + self._oauth.cached_session = None + self._oauth.cached_mtime_ns = None + return + if _normalize_text(current.get("refresh_token")) != _normalize_text( + session.get("refresh_token") + ) or _normalize_text(current.get("access_token")) != _normalize_text( + session.get("access_token") + ): + return + try: + self._oauth.session_path.unlink() + except OSError: + pass + self._oauth.cached_session = None + self._oauth.cached_mtime_ns = None + + +def coerce_transport_auth( + auth: Union[str, ControlPlaneAuthManager], +) -> ControlPlaneAuthManager: + if isinstance(auth, ControlPlaneAuthManager): + return auth + if isinstance(auth, str): + return ControlPlaneAuthManager.for_api_key(auth) + raise TypeError("auth must be a ControlPlaneAuthManager or API key string") def resolve_control_plane_config( config: ClientConfig, ) -> Tuple[str, ControlPlaneAuthManager]: + if not _normalize_text(config.base_url): + raise HyperbrowserError( + "A base URL must be provided", + code="invalid_base_url", + retryable=False, + service="control", + ) + if config.api_key is not None: api_key = _normalize_text(config.api_key) if api_key == "": @@ -469,20 +450,11 @@ def resolve_control_plane_config( retryable=False, service="control", ) - return ( - _normalize_control_base_url(config.base_url) or DEFAULT_BASE_URL, - ControlPlaneAuthManager({"kind": "api_key", "api_key": api_key}), - ) + return config.base_url, ControlPlaneAuthManager.for_api_key(api_key) profile = _normalize_profile(config.profile or DEFAULT_PROFILE) session_path = _resolve_oauth_session_path(profile) session = _try_load_oauth_session(session_path) - resolved_base_url = ( - _normalize_control_base_url(config.base_url) - or _normalize_control_base_url((session or {}).get("base_url", "")) - or DEFAULT_BASE_URL - ) - if session is None: raise HyperbrowserError( "API key must be provided or an OAuth session must be saved with hx auth login", @@ -491,9 +463,14 @@ def resolve_control_plane_config( service="control", ) - if not _oauth_base_urls_match(session.get("base_url"), resolved_base_url): + session_base_url = _normalize_control_base_url(session.get("base_url")) + if _is_default_control_base_url(config.base_url): + resolved_base_url = session_base_url or DEFAULT_BASE_URL + elif _oauth_base_urls_match(config.base_url, session_base_url): + resolved_base_url = _normalize_control_base_url(config.base_url) + else: raise HyperbrowserError( - f"Saved OAuth session for profile {profile} targets {_normalize_base_url(session.get('base_url'))}, not {resolved_base_url}", + f"Saved OAuth session for profile {profile} targets {_normalize_base_url(session.get('base_url'))}, not {config.base_url}", code="oauth_base_url_mismatch", retryable=False, service="control", @@ -501,34 +478,33 @@ def resolve_control_plane_config( frontend_base_url = resolve_frontend_base_url( resolved_base_url, - getattr(config, "frontend_url", None), + config.frontend_url, ) + try: + cached_mtime_ns = session_path.stat().st_mtime_ns + except OSError: + cached_mtime_ns = None return resolved_base_url, ControlPlaneAuthManager( - { - "kind": "oauth", - "profile": profile, - "session_path": str(session_path), - "lock_path": f"{session_path}.refresh.lock", - "base_url": resolved_base_url, - "token_url": f"{frontend_base_url}/oauth/token", - "refresh_timeout_s": DEFAULT_OAUTH_REFRESH_TIMEOUT_S, - "lock_timeout_ms": _normalize_positive_int( - getattr(config, "auth_lock_timeout_ms", None), - None, - DEFAULT_LOCK_TIMEOUT_MS, + oauth=_OAuthSettings( + profile=profile, + session_path=session_path, + lock_path=Path(f"{session_path}.refresh.lock"), + base_url=resolved_base_url, + token_url=f"{frontend_base_url}/oauth/token", + refresh_timeout_s=DEFAULT_OAUTH_REFRESH_TIMEOUT_S, + lock_timeout_ms=_positive_or_default( + config.auth_lock_timeout_ms, DEFAULT_LOCK_TIMEOUT_MS ), - "lock_poll_interval_ms": _normalize_positive_int( - getattr(config, "auth_lock_poll_interval_ms", None), - None, - DEFAULT_LOCK_POLL_INTERVAL_MS, + lock_poll_interval_ms=_positive_or_default( + config.auth_lock_poll_interval_ms, DEFAULT_LOCK_POLL_INTERVAL_MS ), - "lock_stale_ms": _normalize_positive_int( - getattr(config, "auth_lock_stale_ms", None), - None, - DEFAULT_LOCK_STALE_MS, + lock_stale_ms=_positive_or_default( + config.auth_lock_stale_ms, DEFAULT_LOCK_STALE_MS ), - } + cached_session=session, + cached_mtime_ns=cached_mtime_ns, + ) ) @@ -544,11 +520,20 @@ def resolve_frontend_base_url( return _normalize_base_url(control_base_url) or DEFAULT_FRONTEND_BASE_URL +def _oauth_session_expired_error() -> HyperbrowserError: + return HyperbrowserError( + "OAuth session refresh token expired", + code="oauth_session_expired", + retryable=False, + service="control", + ) + + def _resolve_oauth_session_path(profile: str) -> Path: return Path.home() / ".hx_config" / "auth" / f"{profile}.json" -def _try_load_oauth_session(session_path: Path) -> Optional[Dict[str, str]]: +def _try_load_oauth_session(session_path: Path) -> Optional[Dict[str, Any]]: try: raw = session_path.read_text() except FileNotFoundError: @@ -580,7 +565,7 @@ def _try_load_oauth_session(session_path: Path) -> Optional[Dict[str, str]]: def _validate_oauth_session( - session: Dict[str, str], expected_base_url: Optional[str] = None + session: Any, expected_base_url: Optional[str] = None ) -> None: if not isinstance(session, dict): raise HyperbrowserError( @@ -590,9 +575,9 @@ def _validate_oauth_session( service="control", ) - access_token = _normalize_text(session.get("access_token", "")) - refresh_token = _normalize_text(session.get("refresh_token", "")) - base_url = _normalize_base_url(session.get("base_url", "")) + access_token = _normalize_text(session.get("access_token")) + refresh_token = _normalize_text(session.get("refresh_token")) + base_url = _normalize_base_url(session.get("base_url")) if access_token == "" or refresh_token == "": raise HyperbrowserError( @@ -617,7 +602,7 @@ def _validate_oauth_session( service="control", ) - refresh_expiry = _normalize_text(session.get("refresh_token_expiry", "")) + refresh_expiry = _normalize_text(session.get("refresh_token_expiry")) if refresh_expiry and _parse_timestamp(refresh_expiry) is None: raise HyperbrowserError( "Saved OAuth session has an invalid refresh token expiry", @@ -635,21 +620,21 @@ def _validate_oauth_session( ) -def _build_refresh_form(session: Dict[str, str]) -> str: +def _build_refresh_form(session: Dict[str, Any]) -> str: return urlencode( { "grant_type": "refresh_token", - "client_id": _normalize_text(session.get("client_id", "")) + "client_id": _normalize_text(session.get("client_id")) or "hyperbrowser-cli", - "refresh_token": _normalize_text(session.get("refresh_token", "")), + "refresh_token": _normalize_text(session.get("refresh_token")), } ) def _build_refreshed_oauth_session( - previous: Dict[str, str], payload: Dict[str, object] -) -> Dict[str, str]: - access_token = _normalize_text(_string_value(payload.get("access_token"))) + previous: Dict[str, Any], payload: Dict[str, object] +) -> Dict[str, Any]: + access_token = _normalize_text(payload.get("access_token")) if access_token == "": raise HyperbrowserError( "OAuth refresh response did not include an access token", @@ -659,35 +644,25 @@ def _build_refreshed_oauth_session( details=payload, ) - refresh_token = _normalize_text( - _string_value(payload.get("refresh_token")) - ) or _normalize_text(previous.get("refresh_token", "")) - token_type = ( - _normalize_text(_string_value(payload.get("token_type"))) - or _normalize_text(previous.get("token_type", "")) - or "Bearer" - ) - expiry = _derive_expiry(payload.get("expires_in")) or "" - refresh_expiry = _derive_expiry( - payload.get("refresh_token_expires_in") - ) or _normalize_text(previous.get("refresh_token_expiry", "")) - return { "version": previous.get("version", 1), - "base_url": _normalize_base_url(previous.get("base_url", "")), - "client_id": _normalize_text(previous.get("client_id", "")) - or "hyperbrowser-cli", - "token_type": token_type, + "base_url": _normalize_base_url(previous.get("base_url")), + "client_id": _normalize_text(previous.get("client_id")) or "hyperbrowser-cli", + "token_type": _normalize_text(payload.get("token_type")) + or _normalize_text(previous.get("token_type")) + or "Bearer", "access_token": access_token, - "refresh_token": refresh_token, - "expiry": expiry, - "scope": _normalize_text(_string_value(payload.get("scope"))) - or _normalize_text(previous.get("scope", "")), - "refresh_token_expiry": refresh_expiry, + "refresh_token": _normalize_text(payload.get("refresh_token")) + or _normalize_text(previous.get("refresh_token")), + "expiry": _derive_expiry(payload.get("expires_in")) or "", + "scope": _normalize_text(payload.get("scope")) + or _normalize_text(previous.get("scope")), + "refresh_token_expiry": _derive_expiry(payload.get("refresh_token_expires_in")) + or _normalize_text(previous.get("refresh_token_expiry")), } -def _write_oauth_session_atomic(session_path: Path, session: Dict[str, str]) -> None: +def _write_oauth_session_atomic(session_path: Path, session: Dict[str, Any]) -> None: session_path.parent.mkdir(mode=0o700, parents=True, exist_ok=True) try: os.chmod(session_path.parent, 0o700) @@ -722,7 +697,7 @@ def _write_oauth_session_atomic(session_path: Path, session: Dict[str, str]) -> def _should_use_oauth_session( - session: Dict[str, str], + session: Dict[str, Any], force_refresh: bool, rejected_access_token: Optional[str], ) -> bool: @@ -730,13 +705,13 @@ def _should_use_oauth_session( return False if not force_refresh: return True - return _normalize_text(session.get("access_token", "")) != _normalize_text( - rejected_access_token or "" + return _normalize_text(session.get("access_token")) != _normalize_text( + rejected_access_token ) -def _is_access_token_usable(session: Dict[str, str]) -> bool: - if _normalize_text(session.get("access_token", "")) == "": +def _is_access_token_usable(session: Dict[str, Any]) -> bool: + if _normalize_text(session.get("access_token")) == "": return False expiry = _parse_timestamp(session.get("expiry")) if expiry is None: @@ -744,7 +719,7 @@ def _is_access_token_usable(session: Dict[str, str]) -> bool: return (expiry * 1000) - (time.time() * 1000) > OAUTH_REFRESH_EARLY_EXPIRY_MS -def _is_refresh_token_expired(session: Dict[str, str]) -> bool: +def _is_refresh_token_expired(session: Dict[str, Any]) -> bool: expiry = _parse_timestamp(session.get("refresh_token_expiry")) if expiry is None: return False @@ -821,11 +796,7 @@ def _is_rotation_lock_stale(stat_result, stale_ms: int) -> bool: def _same_lock_identity(left, right) -> bool: - return ( - getattr(left, "st_dev", None) == getattr(right, "st_dev", None) - and getattr(left, "st_ino", None) == getattr(right, "st_ino", None) - and getattr(left, "st_mtime_ns", None) == getattr(right, "st_mtime_ns", None) - ) + return (left.st_dev, left.st_ino) == (right.st_dev, right.st_ino) def _try_fchmod(fd: int, mode: int) -> None: @@ -838,19 +809,10 @@ def _try_fchmod(fd: int, mode: int) -> None: pass -def _normalize_positive_int( - explicit_value: Optional[int], env_value: Optional[str], fallback: int -) -> int: - if explicit_value is not None and explicit_value > 0: - return explicit_value - if env_value: - try: - parsed = int(env_value) - except ValueError: - parsed = 0 - if parsed > 0: - return parsed - return fallback +def _positive_or_default(value: Optional[int], default: int) -> int: + if value is not None and value > 0: + return value + return default def _normalize_profile(value: str) -> str: @@ -899,24 +861,7 @@ def _oauth_base_urls_match(left: Optional[object], right: Optional[object]) -> b ) and _is_default_control_base_url(normalized_right) -def _delete_oauth_session(session_path: Path, lock_path: Optional[Path] = None) -> None: - try: - session_path.unlink(missing_ok=True) - except OSError: - pass - if lock_path is None: - lock_path = Path(f"{session_path}.refresh.lock") - try: - lock_path.unlink(missing_ok=True) - except OSError: - pass - - def _normalize_text(value: Optional[object]) -> str: if not isinstance(value, str): return "" return value.strip() - - -def _string_value(value: object) -> str: - return value if isinstance(value, str) else "" diff --git a/hyperbrowser/sandbox_common.py b/hyperbrowser/sandbox_common.py index 1bb0936a..30c55bc5 100644 --- a/hyperbrowser/sandbox_common.py +++ b/hyperbrowser/sandbox_common.py @@ -32,16 +32,24 @@ def get_request_id(response: httpx.Response) -> Optional[str]: def send_control_http_request(transport, method: str, url: str, **kwargs): send = getattr(transport, "send_authenticated", None) - if send is not None: - return send(method, url, **kwargs) - return transport.client.request(method, url, **kwargs) + if send is None: + raise HyperbrowserError( + "Transport cannot send authenticated control-plane requests", + retryable=False, + service="control", + ) + return send(method, url, **kwargs) async def asend_control_http_request(transport, method: str, url: str, **kwargs): send = getattr(transport, "send_authenticated", None) - if send is not None: - return await send(method, url, **kwargs) - return await transport.client.request(method, url, **kwargs) + if send is None: + raise HyperbrowserError( + "Transport cannot send authenticated control-plane requests", + retryable=False, + service="control", + ) + return await send(method, url, **kwargs) def is_retryable_network_error(error: BaseException) -> bool: diff --git a/hyperbrowser/transport/async_transport.py b/hyperbrowser/transport/async_transport.py index d344ac2b..76c849ee 100644 --- a/hyperbrowser/transport/async_transport.py +++ b/hyperbrowser/transport/async_transport.py @@ -2,15 +2,21 @@ import httpx from typing import Any, Dict, Optional +from hyperbrowser.control_auth import coerce_transport_auth from hyperbrowser.exceptions import HyperbrowserError -from .base import TransportStrategy, APIResponse, is_request_replayable, merge_headers +from .base import ( + TransportStrategy, + APIResponse, + aretry_oauth_unauthorized, + is_request_replayable, +) class AsyncTransport(TransportStrategy): """Asynchronous transport implementation using httpx""" def __init__(self, auth): - self.auth = auth + self.auth = coerce_transport_auth(auth) self.client = httpx.AsyncClient() self._closed = False @@ -168,39 +174,29 @@ async def _exchange( replayable: bool = True, ) -> httpx.Response: auth_headers, access_token = await self.auth.aauthorize_headers() - response = await self._send( - method, - url, - params=params, - json_data=json_data, - data=data, - files=files, - auth_headers=auth_headers, - timeout=timeout, - follow_redirects=follow_redirects, - ) - if ( - response.status_code == 401 - and getattr(self.auth, "is_oauth", False) - and replayable - ): - await response.aclose() - retry_headers, _ = await self.auth.aauthorize_headers( - force_refresh=True, - rejected_access_token=access_token, - ) - response = await self._send( + + async def send(headers: Dict[str, str]) -> httpx.Response: + return await self._send( method, url, params=params, json_data=json_data, data=data, files=files, - auth_headers=retry_headers, + auth_headers=headers, timeout=timeout, follow_redirects=follow_redirects, ) - return response + + return await aretry_oauth_unauthorized( + self.auth, + await send(auth_headers), + access_token=access_token, + replayable=replayable, + authorize=self.auth.aauthorize_headers, + send=send, + close_response=lambda response: response.aclose(), + ) async def _send( self, @@ -216,7 +212,7 @@ async def _send( follow_redirects: bool, ) -> httpx.Response: kwargs: Dict[str, Any] = { - "headers": merge_headers(auth_headers), + "headers": auth_headers, "follow_redirects": follow_redirects, } if params is not None: diff --git a/hyperbrowser/transport/base.py b/hyperbrowser/transport/base.py index 642b7f73..0292b1f4 100644 --- a/hyperbrowser/transport/base.py +++ b/hyperbrowser/transport/base.py @@ -1,5 +1,15 @@ from abc import ABC, abstractmethod -from typing import TYPE_CHECKING, Any, Dict, Optional, TypeVar, Generic, Type, Union +from typing import ( + TYPE_CHECKING, + Any, + Callable, + Dict, + Generic, + Optional, + Type, + TypeVar, + Union, +) from hyperbrowser.exceptions import HyperbrowserError @@ -40,7 +50,7 @@ class TransportStrategy(ABC): """Abstract base class for different transport implementations""" @abstractmethod - def __init__(self, auth: "ControlPlaneAuthManager"): + def __init__(self, auth: Union[str, "ControlPlaneAuthManager"]): pass @abstractmethod @@ -76,33 +86,80 @@ def is_request_replayable(files: Optional[Any] = None) -> bool: return _are_files_replayable(files) +def oauth_unauthorized(auth: Any, response: Any) -> bool: + return response.status_code == 401 and bool(getattr(auth, "is_oauth", False)) + + +def retry_oauth_unauthorized( + auth: Any, + response: Any, + *, + access_token: Optional[str], + replayable: bool, + authorize: Callable[..., Any], + send: Callable[[Dict[str, str]], Any], + close_response: Callable[[Any], None], +): + if not oauth_unauthorized(auth, response): + return response + + retry_headers, _ = authorize( + force_refresh=True, + rejected_access_token=access_token, + ) + if not replayable: + return response + + close_response(response) + return send(retry_headers) + + +async def aretry_oauth_unauthorized( + auth: Any, + response: Any, + *, + access_token: Optional[str], + replayable: bool, + authorize: Callable[..., Any], + send: Callable[[Dict[str, str]], Any], + close_response: Callable[[Any], None], +): + if not oauth_unauthorized(auth, response): + return response + + retry_headers, _ = await authorize( + force_refresh=True, + rejected_access_token=access_token, + ) + if not replayable: + return response + + await close_response(response) + return await send(retry_headers) + + def _are_files_replayable(files: Any) -> bool: if isinstance(files, dict): values = list(files.values()) - elif isinstance(files, list): - values = [value for _, value in files] - elif isinstance(files, tuple) and len(files) == 2: + elif _is_file_pair_sequence(files): + values = [item[1] for item in files] + elif isinstance(files, tuple) and len(files) >= 2 and isinstance(files[0], str): values = [files[1]] + elif isinstance(files, (list, tuple)): + values = list(files) else: values = [files] return all(_is_file_value_replayable(value) for value in values) +def _is_file_pair_sequence(files: Any) -> bool: + if not isinstance(files, (list, tuple)) or not files: + return False + first = files[0] + return isinstance(first, (list, tuple)) and len(first) >= 2 + + def _is_file_value_replayable(value: Any) -> bool: if isinstance(value, tuple) and len(value) >= 2: return _is_file_value_replayable(value[1]) return isinstance(value, (str, bytes, bytearray, memoryview)) - - -def merge_headers( - *header_groups: Optional[Dict[str, str]], -) -> Dict[str, str]: - merged: Dict[str, str] = {} - for headers in header_groups: - if not headers: - continue - for key, value in headers.items(): - if value is None: - continue - merged[str(key)] = str(value) - return merged diff --git a/hyperbrowser/transport/sync.py b/hyperbrowser/transport/sync.py index ebd68e86..5f6a9ca6 100644 --- a/hyperbrowser/transport/sync.py +++ b/hyperbrowser/transport/sync.py @@ -1,15 +1,21 @@ import httpx from typing import Any, Dict, Optional +from hyperbrowser.control_auth import coerce_transport_auth from hyperbrowser.exceptions import HyperbrowserError -from .base import TransportStrategy, APIResponse, is_request_replayable, merge_headers +from .base import ( + TransportStrategy, + APIResponse, + is_request_replayable, + retry_oauth_unauthorized, +) class SyncTransport(TransportStrategy): """Synchronous transport implementation using httpx""" def __init__(self, auth): - self.auth = auth + self.auth = coerce_transport_auth(auth) self.client = httpx.Client() def _handle_response(self, response: httpx.Response) -> APIResponse: @@ -147,39 +153,29 @@ def _exchange( replayable: bool = True, ) -> httpx.Response: auth_headers, access_token = self.auth.authorize_headers() - response = self._send( - method, - url, - params=params, - json_data=json_data, - data=data, - files=files, - auth_headers=auth_headers, - timeout=timeout, - follow_redirects=follow_redirects, - ) - if ( - response.status_code == 401 - and getattr(self.auth, "is_oauth", False) - and replayable - ): - response.close() - retry_headers, _ = self.auth.authorize_headers( - force_refresh=True, - rejected_access_token=access_token, - ) - response = self._send( + + def send(headers: Dict[str, str]) -> httpx.Response: + return self._send( method, url, params=params, json_data=json_data, data=data, files=files, - auth_headers=retry_headers, + auth_headers=headers, timeout=timeout, follow_redirects=follow_redirects, ) - return response + + return retry_oauth_unauthorized( + self.auth, + send(auth_headers), + access_token=access_token, + replayable=replayable, + authorize=self.auth.authorize_headers, + send=send, + close_response=lambda response: response.close(), + ) def _send( self, @@ -195,7 +191,7 @@ def _send( follow_redirects: bool, ) -> httpx.Response: kwargs: Dict[str, Any] = { - "headers": merge_headers(auth_headers), + "headers": auth_headers, "follow_redirects": follow_redirects, } if params is not None: diff --git a/tests/test_control_auth.py b/tests/test_control_auth.py index 06df0425..28844696 100644 --- a/tests/test_control_auth.py +++ b/tests/test_control_auth.py @@ -205,13 +205,48 @@ def test_legacy_app_base_url_maps_to_api(auth_home): assert auth.is_oauth is True +def test_default_client_adopts_session_base_url(auth_home): + write_session(auth_home, base_url="https://staging.hyperbrowser.dev") + base_url, auth = resolve_control_plane_config(ClientConfig()) + assert base_url == "https://staging.hyperbrowser.dev" + assert auth.is_oauth is True + + def test_oauth_base_url_mismatch(auth_home): write_session(auth_home, base_url="https://staging.hyperbrowser.dev") with pytest.raises(HyperbrowserError) as exc: - resolve_control_plane_config(ClientConfig()) + resolve_control_plane_config( + ClientConfig(base_url="https://other.hyperbrowser.dev") + ) assert exc.value.code == "oauth_base_url_mismatch" +def test_empty_base_url_raises(auth_home): + write_session(auth_home) + with pytest.raises(HyperbrowserError) as exc: + resolve_control_plane_config(ClientConfig(api_key="key", base_url="")) + assert exc.value.code == "invalid_base_url" + + +def test_empty_env_api_key_falls_back_to_oauth(auth_home, monkeypatch): + write_session(auth_home, access_token="session-access") + monkeypatch.setenv("HYPERBROWSER_API_KEY", " ") + client = Hyperbrowser() + try: + headers, _ = client.auth.authorize_headers() + finally: + client.close() + assert headers == {"authorization": "Bearer session-access"} + + +def test_api_key_mode_does_not_rewrite_explicit_base_url(auth_home): + base_url, auth = resolve_control_plane_config( + ClientConfig(api_key="key", base_url="https://app.hyperbrowser.ai/api") + ) + assert base_url == "https://app.hyperbrowser.ai/api" + assert auth.is_oauth is False + + def test_invalid_session_json(auth_home): path = Path(auth_home) / ".hx_config" / "auth" / "default.json" path.parent.mkdir(parents=True) @@ -294,13 +329,7 @@ def handler(request: httpx.Request) -> httpx.Response: json={"access_token": "async-refreshed", "expires_in": 3600}, ) - real_client = httpx.AsyncClient - - def fake_client(*args, **kwargs): - kwargs["transport"] = httpx.MockTransport(handler) - return real_client(*args, **kwargs) - - monkeypatch.setattr("hyperbrowser.control_auth.httpx.AsyncClient", fake_client) + _patch_httpx_client(monkeypatch, handler) _, auth = resolve_control_plane_config(ClientConfig()) headers, token = asyncio.run(auth.aauthorize_headers()) assert headers == {"authorization": "Bearer async-refreshed"} @@ -461,6 +490,68 @@ def test_parse_timestamp_accepts_variable_fractional_seconds(): assert abs(nine - six) < 0.001 +def test_invalid_grant_does_not_delete_rotated_session(auth_home, monkeypatch): + path = write_session( + auth_home, + access_token="expired-access", + expiry=_expiry(minutes=-5), + refresh_token="old-refresh", + ) + + def handler(request: httpx.Request) -> httpx.Response: + path.write_text( + json.dumps( + { + "version": 1, + "base_url": DEFAULT_BASE_URL, + "client_id": "hyperbrowser-cli", + "token_type": "Bearer", + "access_token": "rotated-access", + "refresh_token": "rotated-refresh", + "expiry": _expiry(hours=1), + "scope": "cli", + } + ) + ) + return httpx.Response(400, json={"error": "invalid_grant"}) + + _patch_httpx_client(monkeypatch, handler) + _, auth = resolve_control_plane_config(ClientConfig()) + with pytest.raises(HyperbrowserError) as exc: + auth.authorize_headers() + assert exc.value.code == "invalid_grant" + saved = json.loads(path.read_text()) + assert saved["refresh_token"] == "rotated-refresh" + assert saved["access_token"] == "rotated-access" + + +def test_missing_session_after_expire_is_oauth_session_expired(auth_home): + path = write_session( + auth_home, + access_token="expired-access", + expiry=_expiry(minutes=-5), + refresh_token_expiry=_expiry(minutes=-1), + ) + _, auth = resolve_control_plane_config(ClientConfig()) + path.unlink() + with pytest.raises(HyperbrowserError) as exc: + auth.authorize_headers() + assert exc.value.code == "oauth_session_expired" + + +def test_sync_transport_accepts_api_key_string(): + from hyperbrowser.transport.sync import SyncTransport + + transport = SyncTransport("legacy-key") + try: + headers, token = transport.auth.authorize_headers() + assert headers == {"x-api-key": "legacy-key"} + assert token is None + assert transport.auth.is_oauth is False + finally: + transport.close() + + def test_refresh_timeout_is_independent_of_lock_timeout(auth_home, monkeypatch): write_session( auth_home, diff --git a/tests/test_sandbox_wire_contract.py b/tests/test_sandbox_wire_contract.py index e56e842e..ea07ba9b 100644 --- a/tests/test_sandbox_wire_contract.py +++ b/tests/test_sandbox_wire_contract.py @@ -624,7 +624,23 @@ async def stream_bytes( class FakeSyncClient: def __init__(self): http_client = RecordingHTTPClient() - self.transport = type("Transport", (), {"client": http_client})() + + def send_authenticated(method, url, **kwargs): + return http_client.request( + method, + url, + params=kwargs.get("params"), + json=kwargs.get("json"), + ) + + self.transport = type( + "Transport", + (), + { + "client": http_client, + "send_authenticated": staticmethod(send_authenticated), + }, + )() self.config = type("Config", (), {"runtime_proxy_override": None})() self.timeout = 30 @@ -700,7 +716,23 @@ async def request(self, method, url, params=None, json=None): class FakeAsyncClient: def __init__(self): http_client = RecordingAsyncHTTPClient() - self.transport = type("Transport", (), {"client": http_client})() + + async def send_authenticated(method, url, **kwargs): + return await http_client.request( + method, + url, + params=kwargs.get("params"), + json=kwargs.get("json"), + ) + + self.transport = type( + "Transport", + (), + { + "client": http_client, + "send_authenticated": staticmethod(send_authenticated), + }, + )() self.config = type("Config", (), {"runtime_proxy_override": None})() self.timeout = 30 diff --git a/tests/test_transport_auth.py b/tests/test_transport_auth.py index 5378d51a..b31fa0d6 100644 --- a/tests/test_transport_auth.py +++ b/tests/test_transport_auth.py @@ -79,27 +79,40 @@ def handler(request: httpx.Request) -> httpx.Response: ] -def test_oauth_401_without_replayable_body_does_not_retry( - auth_home, monkeypatch, tmp_path +def test_oauth_401_refreshes_non_replayable_upload_without_retrying_body( + auth_home, monkeypatch ): + import io + import json + from pathlib import Path + write_session(auth_home, access_token="old-access") + calls = [] def handler(request: httpx.Request) -> httpx.Response: + calls.append(request.url.path) if request.url.path == "/oauth/token": - raise AssertionError("refresh should not run for non-replayable uploads") + return httpx.Response( + 200, + json={"access_token": "new-access", "expires_in": 3600}, + ) return httpx.Response(401, json={"message": "unauthorized"}) _patch_httpx_client(monkeypatch, handler) - upload = tmp_path / "file.bin" - upload.write_bytes(b"hello") client = Hyperbrowser() try: with pytest.raises(HyperbrowserError) as exc: - client.sessions.upload_file("session_123", str(upload)) + client.sessions.upload_file("session_123", io.BytesIO(b"hello")) assert exc.value.status_code == 401 finally: client.close() + assert calls == ["/api/session/session_123/uploads", "/oauth/token"] + saved = json.loads( + (Path(auth_home) / ".hx_config" / "auth" / "default.json").read_text() + ) + assert saved["access_token"] == "new-access" + def test_post_timeout_is_forwarded(auth_home, monkeypatch): seen = [] @@ -230,3 +243,15 @@ def handler(request: httpx.Request) -> httpx.Response: assert seen assert seen[0]["path"] == "/api/sandbox/sbx_123" assert seen[0]["api_key"] == "sandbox-key" + + +def test_file_pair_sequence_replayability(): + from hyperbrowser.transport.base import is_request_replayable + + class FileLike: + def read(self): + return b"x" + + assert is_request_replayable((("file", b"one"), ("extra", b"two"))) + assert is_request_replayable([("file", b"one"), ("extra", b"two")]) + assert not is_request_replayable({"file": FileLike()}) From 0d60c10eebc41d917e8369e8d1431585306dc423 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Sat, 15 Aug 2026 20:45:22 +0000 Subject: [PATCH 4/4] Clear refresh token expiry when the token is rotated A refresh response that issues a new refresh_token without refresh_token_expires_in no longer inherits the previous token's lifetime. That leftover expiry could make a still-valid session look expired and force another hx auth login. Co-authored-by: Shri Sukhani --- hyperbrowser/control_auth.py | 13 +++-- tests/test_control_auth.py | 102 +++++++++++++++++++++++++++++++++++ 2 files changed, 111 insertions(+), 4 deletions(-) diff --git a/hyperbrowser/control_auth.py b/hyperbrowser/control_auth.py index b70837b5..3af5674b 100644 --- a/hyperbrowser/control_auth.py +++ b/hyperbrowser/control_auth.py @@ -644,6 +644,13 @@ def _build_refreshed_oauth_session( details=payload, ) + previous_refresh_token = _normalize_text(previous.get("refresh_token")) + refresh_token = ( + _normalize_text(payload.get("refresh_token")) or previous_refresh_token + ) + refresh_token_expiry = _derive_expiry(payload.get("refresh_token_expires_in")) + if refresh_token_expiry is None and refresh_token == previous_refresh_token: + refresh_token_expiry = _normalize_text(previous.get("refresh_token_expiry")) return { "version": previous.get("version", 1), "base_url": _normalize_base_url(previous.get("base_url")), @@ -652,13 +659,11 @@ def _build_refreshed_oauth_session( or _normalize_text(previous.get("token_type")) or "Bearer", "access_token": access_token, - "refresh_token": _normalize_text(payload.get("refresh_token")) - or _normalize_text(previous.get("refresh_token")), + "refresh_token": refresh_token, "expiry": _derive_expiry(payload.get("expires_in")) or "", "scope": _normalize_text(payload.get("scope")) or _normalize_text(previous.get("scope")), - "refresh_token_expiry": _derive_expiry(payload.get("refresh_token_expires_in")) - or _normalize_text(previous.get("refresh_token_expiry")), + "refresh_token_expiry": refresh_token_expiry or "", } diff --git a/tests/test_control_auth.py b/tests/test_control_auth.py index 28844696..12018273 100644 --- a/tests/test_control_auth.py +++ b/tests/test_control_auth.py @@ -479,6 +479,108 @@ def handler(request: httpx.Request) -> httpx.Response: assert headers_again == {"authorization": "Bearer no-expiry-access"} +def test_rotated_refresh_token_without_expires_in_clears_old_expiry( + auth_home, monkeypatch +): + old_refresh_expiry = _expiry(minutes=1) + write_session( + auth_home, + access_token="expired-access", + expiry=_expiry(minutes=-5), + refresh_token="old-refresh", + refresh_token_expiry=old_refresh_expiry, + ) + + def handler(request: httpx.Request) -> httpx.Response: + return httpx.Response( + 200, + json={ + "access_token": "rotated-access", + "refresh_token": "rotated-refresh", + "expires_in": 3600, + }, + ) + + _patch_httpx_client(monkeypatch, handler) + _, auth = resolve_control_plane_config(ClientConfig()) + headers, token = auth.authorize_headers() + assert headers == {"authorization": "Bearer rotated-access"} + assert token == "rotated-access" + + saved = json.loads( + (Path(auth_home) / ".hx_config" / "auth" / "default.json").read_text() + ) + assert saved["refresh_token"] == "rotated-refresh" + assert saved["refresh_token_expiry"] == "" + assert saved["refresh_token_expiry"] != old_refresh_expiry + + +def test_reused_refresh_token_without_expires_in_keeps_old_expiry( + auth_home, monkeypatch +): + old_refresh_expiry = _expiry(hours=2) + write_session( + auth_home, + access_token="expired-access", + expiry=_expiry(minutes=-5), + refresh_token="same-refresh", + refresh_token_expiry=old_refresh_expiry, + ) + + def handler(request: httpx.Request) -> httpx.Response: + return httpx.Response( + 200, + json={ + "access_token": "refreshed-access", + "expires_in": 3600, + }, + ) + + _patch_httpx_client(monkeypatch, handler) + _, auth = resolve_control_plane_config(ClientConfig()) + auth.authorize_headers() + + saved = json.loads( + (Path(auth_home) / ".hx_config" / "auth" / "default.json").read_text() + ) + assert saved["refresh_token"] == "same-refresh" + assert saved["refresh_token_expiry"] == old_refresh_expiry + + +def test_rotated_refresh_token_uses_new_expires_in(auth_home, monkeypatch): + old_refresh_expiry = _expiry(minutes=1) + write_session( + auth_home, + access_token="expired-access", + expiry=_expiry(minutes=-5), + refresh_token="old-refresh", + refresh_token_expiry=old_refresh_expiry, + ) + + def handler(request: httpx.Request) -> httpx.Response: + return httpx.Response( + 200, + json={ + "access_token": "rotated-access", + "refresh_token": "rotated-refresh", + "expires_in": 3600, + "refresh_token_expires_in": 7200, + }, + ) + + _patch_httpx_client(monkeypatch, handler) + _, auth = resolve_control_plane_config(ClientConfig()) + auth.authorize_headers() + + saved = json.loads( + (Path(auth_home) / ".hx_config" / "auth" / "default.json").read_text() + ) + assert saved["refresh_token"] == "rotated-refresh" + assert saved["refresh_token_expiry"] != old_refresh_expiry + assert saved["refresh_token_expiry"] != "" + assert _parse_timestamp(saved["refresh_token_expiry"]) is not None + + def test_parse_timestamp_accepts_variable_fractional_seconds(): assert _parse_timestamp("2026-08-15T12:00:00Z") is not None assert _parse_timestamp("2026-08-15T12:00:00.1Z") is not None