Harden open_binary_file path normalization boundaries

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

Author: cursoragent

hyperbrowser/client/file_utils.py

@@ -7,52 +7,7 @@
 from hyperbrowser.type_utils import is_plain_string
 
 
-def _validate_error_message_text(message_value: str, *, field_name: str) -> None:
-    if not is_plain_string(message_value):
-        raise HyperbrowserError(f"{field_name} must be a string")
-    try:
-        normalized_message = message_value.strip()
-        if not is_plain_string(normalized_message):
-            raise TypeError(f"normalized {field_name} must be a string")
-        is_empty = len(normalized_message) == 0
-    except HyperbrowserError:
-        raise
-    except Exception as exc:
-        raise HyperbrowserError(
-            f"Failed to normalize {field_name}",
-            original_error=exc,
-        ) from exc
-    if is_empty:
-        raise HyperbrowserError(f"{field_name} must not be empty")
-    try:
-        contains_control_character = any(
-            ord(character) < 32 or ord(character) == 127 for character in message_value
-        )
-    except HyperbrowserError:
-        raise
-    except Exception as exc:
-        raise HyperbrowserError(
-            f"Failed to validate {field_name} characters",
-            original_error=exc,
-        ) from exc
-    if contains_control_character:
-        raise HyperbrowserError(f"{field_name} must not contain control characters")
-
-
-def ensure_existing_file_path(
-    file_path: Union[str, PathLike[str]],
-    *,
-    missing_file_message: str,
-    not_file_message: str,
-) -> str:
-    _validate_error_message_text(
-        missing_file_message,
-        field_name="missing_file_message",
-    )
-    _validate_error_message_text(
-        not_file_message,
-        field_name="not_file_message",
-    )
+def _normalize_file_path_text(file_path: Union[str, PathLike[str]]) -> str:
     try:
         normalized_path = os.fspath(file_path)
     except HyperbrowserError:
@@ -105,6 +60,56 @@ def ensure_existing_file_path(
         raise HyperbrowserError("file_path is invalid", original_error=exc) from exc
     if contains_control_character:
         raise HyperbrowserError("file_path must not contain control characters")
+    return normalized_path
+
+
+def _validate_error_message_text(message_value: str, *, field_name: str) -> None:
+    if not is_plain_string(message_value):
+        raise HyperbrowserError(f"{field_name} must be a string")
+    try:
+        normalized_message = message_value.strip()
+        if not is_plain_string(normalized_message):
+            raise TypeError(f"normalized {field_name} must be a string")
+        is_empty = len(normalized_message) == 0
+    except HyperbrowserError:
+        raise
+    except Exception as exc:
+        raise HyperbrowserError(
+            f"Failed to normalize {field_name}",
+            original_error=exc,
+        ) from exc
+    if is_empty:
+        raise HyperbrowserError(f"{field_name} must not be empty")
+    try:
+        contains_control_character = any(
+            ord(character) < 32 or ord(character) == 127 for character in message_value
+        )
+    except HyperbrowserError:
+        raise
+    except Exception as exc:
+        raise HyperbrowserError(
+            f"Failed to validate {field_name} characters",
+            original_error=exc,
+        ) from exc
+    if contains_control_character:
+        raise HyperbrowserError(f"{field_name} must not contain control characters")
+
+
+def ensure_existing_file_path(
+    file_path: Union[str, PathLike[str]],
+    *,
+    missing_file_message: str,
+    not_file_message: str,
+) -> str:
+    _validate_error_message_text(
+        missing_file_message,
+        field_name="missing_file_message",
+    )
+    _validate_error_message_text(
+        not_file_message,
+        field_name="not_file_message",
+    )
+    normalized_path = _normalize_file_path_text(file_path)
     try:
         path_exists = bool(os.path.exists(normalized_path))
     except HyperbrowserError:
@@ -138,19 +143,7 @@ def open_binary_file(
         open_error_message,
         field_name="open_error_message",
     )
-    try:
-        normalized_path = os.fspath(file_path)
-    except HyperbrowserError:
-        raise
-    except TypeError as exc:
-        raise HyperbrowserError(
-            "file_path must be a string or os.PathLike object",
-            original_error=exc,
-        ) from exc
-    except Exception as exc:
-        raise HyperbrowserError("file_path is invalid", original_error=exc) from exc
-    if not is_plain_string(normalized_path):
-        raise HyperbrowserError("file_path must resolve to a string path")
+    normalized_path = _normalize_file_path_text(file_path)
     try:
         with open(normalized_path, "rb") as file_obj:
             yield file_obj

tests/test_file_utils.py

@@ -528,6 +528,71 @@ def test_open_binary_file_rejects_non_string_fspath_results():
             pass
 
 
+def test_open_binary_file_rejects_string_subclass_fspath_results():
+    class _PathLike:
+        class _PathString(str):
+            pass
+
+        def __fspath__(self):
+            return self._PathString("/tmp/subclass-path")
+
+    with pytest.raises(HyperbrowserError, match="file_path must resolve to a string"):
+        with open_binary_file(
+            _PathLike(),  # type: ignore[arg-type]
+            open_error_message="open failed",
+        ):
+            pass
+
+
+def test_open_binary_file_rejects_empty_string_paths():
+    with pytest.raises(HyperbrowserError, match="file_path must not be empty"):
+        with open_binary_file(
+            "",
+            open_error_message="open failed",
+        ):
+            pass
+    with pytest.raises(HyperbrowserError, match="file_path must not be empty"):
+        with open_binary_file(
+            "   ",
+            open_error_message="open failed",
+        ):
+            pass
+
+
+def test_open_binary_file_rejects_surrounding_whitespace():
+    with pytest.raises(
+        HyperbrowserError,
+        match="file_path must not contain leading or trailing whitespace",
+    ):
+        with open_binary_file(
+            " /tmp/file.txt",
+            open_error_message="open failed",
+        ):
+            pass
+
+
+def test_open_binary_file_rejects_null_byte_paths():
+    with pytest.raises(
+        HyperbrowserError, match="file_path must not contain null bytes"
+    ):
+        with open_binary_file(
+            "bad\x00path.txt",
+            open_error_message="open failed",
+        ):
+            pass
+
+
+def test_open_binary_file_rejects_control_character_paths():
+    with pytest.raises(
+        HyperbrowserError, match="file_path must not contain control characters"
+    ):
+        with open_binary_file(
+            "bad\tpath.txt",
+            open_error_message="open failed",
+        ):
+            pass
+
+
 def test_open_binary_file_wraps_open_errors(tmp_path: Path):
     missing_path = tmp_path / "missing.bin"