Harden HyperbrowserBase env reads and path normalization

Co-authored-by: Shri Sukhani <shrisukhani@users.noreply.github.com>

Author: cursoragent

hyperbrowser/client/base.py

@@ -30,7 +30,7 @@ def __init__(
             resolved_api_key = (
                 api_key
                 if api_key_from_constructor
-                else os.environ.get("HYPERBROWSER_API_KEY")
+                else self._read_env_value("HYPERBROWSER_API_KEY")
             )
             if resolved_api_key is None:
                 raise HyperbrowserError(
@@ -59,12 +59,12 @@ def __init__(
                 headers
                 if headers is not None
                 else ClientConfig.parse_headers_from_env(
-                    os.environ.get("HYPERBROWSER_HEADERS")
+                    self._read_env_value("HYPERBROWSER_HEADERS")
                 )
             )
             if base_url is None:
                 resolved_base_url = ClientConfig.resolve_base_url_from_env(
-                    os.environ.get("HYPERBROWSER_BASE_URL")
+                    self._read_env_value("HYPERBROWSER_BASE_URL")
                 )
             else:
                 resolved_base_url = base_url
@@ -80,6 +80,18 @@ def __init__(
         self.config = config
         self.transport = transport(config.api_key, headers=config.headers)
 
+    @staticmethod
+    def _read_env_value(env_name: str) -> Optional[str]:
+        try:
+            return os.environ.get(env_name)
+        except HyperbrowserError:
+            raise
+        except Exception as exc:
+            raise HyperbrowserError(
+                f"Failed to read {env_name} environment variable",
+                original_error=exc,
+            ) from exc
+
     @staticmethod
     def _parse_url_components(
         url_value: str, *, component_label: str
@@ -127,7 +139,17 @@ def _parse_url_components(
     def _build_url(self, path: str) -> str:
         if not isinstance(path, str):
             raise HyperbrowserError("path must be a string")
-        stripped_path = path.strip()
+        try:
+            stripped_path = path.strip()
+            if not isinstance(stripped_path, str):
+                raise TypeError("normalized path must be a string")
+        except HyperbrowserError:
+            raise
+        except Exception as exc:
+            raise HyperbrowserError(
+                "Failed to normalize path",
+                original_error=exc,
+            ) from exc
         if stripped_path != path:
             raise HyperbrowserError(
                 "path must not contain leading or trailing whitespace"

tests/test_client_api_key.py

@@ -1,5 +1,6 @@
 import pytest
 
+import hyperbrowser.client.base as client_base_module
 from hyperbrowser import AsyncHyperbrowser, Hyperbrowser
 from hyperbrowser.exceptions import HyperbrowserError
 
@@ -110,6 +111,95 @@ def test_async_client_rejects_control_character_env_api_key(monkeypatch):
         AsyncHyperbrowser()
 
 
+@pytest.mark.parametrize("client_class", [Hyperbrowser, AsyncHyperbrowser])
+def test_client_wraps_api_key_env_read_runtime_errors(
+    client_class, monkeypatch: pytest.MonkeyPatch
+):
+    original_get = client_base_module.os.environ.get
+
+    def _broken_get(env_name: str, default=None):
+        if env_name == "HYPERBROWSER_API_KEY":
+            raise RuntimeError("api key env read exploded")
+        return original_get(env_name, default)
+
+    monkeypatch.setattr(client_base_module.os.environ, "get", _broken_get)
+
+    with pytest.raises(
+        HyperbrowserError,
+        match="Failed to read HYPERBROWSER_API_KEY environment variable",
+    ) as exc_info:
+        client_class()
+
+    assert isinstance(exc_info.value.original_error, RuntimeError)
+
+
+@pytest.mark.parametrize("client_class", [Hyperbrowser, AsyncHyperbrowser])
+def test_client_preserves_hyperbrowser_api_key_env_read_errors(
+    client_class, monkeypatch: pytest.MonkeyPatch
+):
+    original_get = client_base_module.os.environ.get
+
+    def _broken_get(env_name: str, default=None):
+        if env_name == "HYPERBROWSER_API_KEY":
+            raise HyperbrowserError("custom api key env read failure")
+        return original_get(env_name, default)
+
+    monkeypatch.setattr(client_base_module.os.environ, "get", _broken_get)
+
+    with pytest.raises(
+        HyperbrowserError, match="custom api key env read failure"
+    ) as exc_info:
+        client_class()
+
+    assert exc_info.value.original_error is None
+
+
+@pytest.mark.parametrize("client_class", [Hyperbrowser, AsyncHyperbrowser])
+def test_client_wraps_base_url_env_read_runtime_errors(
+    client_class, monkeypatch: pytest.MonkeyPatch
+):
+    original_get = client_base_module.os.environ.get
+    monkeypatch.setenv("HYPERBROWSER_API_KEY", "test-key")
+
+    def _broken_get(env_name: str, default=None):
+        if env_name == "HYPERBROWSER_BASE_URL":
+            raise RuntimeError("base url env read exploded")
+        return original_get(env_name, default)
+
+    monkeypatch.setattr(client_base_module.os.environ, "get", _broken_get)
+
+    with pytest.raises(
+        HyperbrowserError,
+        match="Failed to read HYPERBROWSER_BASE_URL environment variable",
+    ) as exc_info:
+        client_class()
+
+    assert isinstance(exc_info.value.original_error, RuntimeError)
+
+
+@pytest.mark.parametrize("client_class", [Hyperbrowser, AsyncHyperbrowser])
+def test_client_wraps_headers_env_read_runtime_errors(
+    client_class, monkeypatch: pytest.MonkeyPatch
+):
+    original_get = client_base_module.os.environ.get
+    monkeypatch.setenv("HYPERBROWSER_API_KEY", "test-key")
+
+    def _broken_get(env_name: str, default=None):
+        if env_name == "HYPERBROWSER_HEADERS":
+            raise RuntimeError("headers env read exploded")
+        return original_get(env_name, default)
+
+    monkeypatch.setattr(client_base_module.os.environ, "get", _broken_get)
+
+    with pytest.raises(
+        HyperbrowserError,
+        match="Failed to read HYPERBROWSER_HEADERS environment variable",
+    ) as exc_info:
+        client_class()
+
+    assert isinstance(exc_info.value.original_error, RuntimeError)
+
+
 @pytest.mark.parametrize("client_class", [Hyperbrowser, AsyncHyperbrowser])
 def test_client_wraps_api_key_strip_runtime_errors(client_class):
     class _BrokenStripApiKey(str):

tests/test_url_building.py

@@ -353,6 +353,54 @@ def test_client_build_url_rejects_empty_or_non_string_paths():
         client.close()
 
 
+def test_client_build_url_wraps_path_strip_runtime_errors():
+    client = Hyperbrowser(config=ClientConfig(api_key="test-key"))
+    try:
+        class _BrokenPath(str):
+            def strip(self, chars=None):  # type: ignore[override]
+                _ = chars
+                raise RuntimeError("path strip exploded")
+
+        with pytest.raises(HyperbrowserError, match="Failed to normalize path") as exc_info:
+            client._build_url(_BrokenPath("/session"))
+
+        assert isinstance(exc_info.value.original_error, RuntimeError)
+    finally:
+        client.close()
+
+
+def test_client_build_url_preserves_hyperbrowser_path_strip_errors():
+    client = Hyperbrowser(config=ClientConfig(api_key="test-key"))
+    try:
+        class _BrokenPath(str):
+            def strip(self, chars=None):  # type: ignore[override]
+                _ = chars
+                raise HyperbrowserError("custom path strip failure")
+
+        with pytest.raises(HyperbrowserError, match="custom path strip failure") as exc_info:
+            client._build_url(_BrokenPath("/session"))
+
+        assert exc_info.value.original_error is None
+    finally:
+        client.close()
+
+
+def test_client_build_url_wraps_non_string_path_strip_results():
+    client = Hyperbrowser(config=ClientConfig(api_key="test-key"))
+    try:
+        class _BrokenPath(str):
+            def strip(self, chars=None):  # type: ignore[override]
+                _ = chars
+                return object()
+
+        with pytest.raises(HyperbrowserError, match="Failed to normalize path") as exc_info:
+            client._build_url(_BrokenPath("/session"))
+
+        assert isinstance(exc_info.value.original_error, TypeError)
+    finally:
+        client.close()
+
+
 def test_client_build_url_allows_query_values_containing_absolute_urls():
     client = Hyperbrowser(config=ClientConfig(api_key="test-key"))
     try: