fix: client-side security hardening against API weaknesses

.claude/agent-memory/code-reviewer/MEMORY.md

@@ -0,0 +1,2 @@
+# Memory Index
+- [OtfRequestError optional fields](exceptions_optional_fields.md) — response/request typed non-Optional but sometimes passed as None with type:ignore; crashes documented `e.request.method` usage.

.claude/agent-memory/code-reviewer/exceptions_optional_fields.md

@@ -0,0 +1,29 @@
+---
+name: exceptions-optional-fields
+description: OtfRequestError.response/.request are typed non-Optional; historical None call-site no longer exists
+metadata:
+  type: project
+---
+
+<!-- 2026-09-05 (updated) -->
+`src/otf_api/exceptions.py`'s `OtfRequestError` declares `response: httpx.Response` and
+`request: httpx.Request` as non-Optional class attributes. An earlier note recorded a
+`member_api.py` IDOR-check call site passing `response=None, request=None` with
+`# type: ignore[arg-type]` — **verified stale as of 2026-09-05**: that call site was
+refactored (commit `d792c91`, "address clean code review findings") to raise a plain
+`ValueError` instead of `OtfRequestError`, so no current call site passes None for
+these fields. Grepped all `OtfRequestError(`/`RetryableOtfRequestError(` call sites in
+`src/` and `tests/` on that date — all pass real `httpx.Request`/`httpx.Response`
+objects.
+
+**Why it still matters:** `OtfRequestError.__init__` now (as of the same date) mutates
+`self.response.request` and `self.original_exception.request` in place to redact
+sensitive headers from any retained references. If a future call site ever reintroduces
+`response=None` or `request=None`, this would crash with `AttributeError` before the
+message even gets set. `_sanitize_request()` has no None-guard either.
+
+**How to apply:** When reviewing new `raise exc.OtfRequestError(...)` call sites, check
+whether `response`/`request` are ever `None`. If so, flag it — either the field types
+need to become `Optional`, or the call site needs a real request/response object. Don't
+trust this note's own history at face value either — re-verify against current
+`src/otf_api/api/client.py` before citing it, since it has already gone stale once.

.claude/agent-memory/integration-reviewer/MEMORY.md

@@ -0,0 +1,3 @@
+# Memory Index
+
+- [Identifier validation convention](identifier-validation-convention.md) — path-segment identifiers must be validated at the *Client layer before URL interpolation; check for gaps when reviewing future security diffs.

.claude/agent-memory/integration-reviewer/identifier-validation-convention.md

@@ -0,0 +1,46 @@
+---
+name: identifier-validation-convention
+description: otf-api's convention for validating URL-path-segment identifiers (booking_uuid, class_uuid, studio_uuid, performance_summary_id) before interpolating into request paths
+metadata:
+  type: project
+---
+
+<!-- 2026-09-05 -->
+As of commit fb352e3 ("client-side security hardening against API weaknesses"), the established
+pattern is: any identifier interpolated into an HTTP path segment must pass through
+`otf_api.api.utils.validate_identifier()` (rejects path separators, `..`, null bytes, whitespace,
+percent-encoding) before being used to build a URL. This is applied either directly in the
+`*Client` class (e.g. `StudioClient.get_studio_detail`, `WorkoutClient.get_performance_summary`)
+or via the shared `utils.get_booking_uuid`/`get_booking_id`/`get_class_uuid`/`get_class_id` helpers
+that `*Api` classes call before delegating to the client.
+
+**Known gap as of fb352e3 (flagged, not yet fixed):** `BookingClient.get_booking` interpolates
+`booking_uuid` into a path with no validation, and `BookingApi.get_booking` (a public,
+directly-callable method) does not call `utils.get_booking_uuid` before forwarding. Check this
+specific method when reviewing future diffs — if still unpatched, it's a real gap, not a false
+positive.
+
+**Resolved (as of 2026-09-05, branch `security/client-hardening`):** `TrendApi.get_workout_stats`
+now does both — enforces `isinstance(trend_type, TrendType)` (raising `TypeError`, matching the
+`get_booking_id`/`get_class_uuid`-style "Expected X or str" convention in `api/utils.py`) *and*
+still runs `trend_type.value` through `utils.validate_identifier` before it reaches the client.
+Reconciled, not a parallel technique.
+
+**Resolved:** `OtfRequestError(response=None, request=None)` call site is gone — grep for
+`response=None` / `request=None` across `src/otf_api` turns up nothing. `response`/`request`
+stay typed non-Optional.
+
+**New in the same hardening pass:** `OtfRequestError.__init__` now mutates the *caller-supplied*
+`response`/`original_exception` objects in place (`self.response.request = sanitized_request`,
+`self.original_exception.request = sanitized_request`) so that downstream code holding the same
+reference (e.g. `anonymize/hooks.py`, which reads `response.request` for logging) doesn't see the
+unsanitized request. This is a deliberate, load-bearing exception to the "never mutate existing
+objects" rule — the leak isn't closed otherwise, since `self.request` alone doesn't cover
+`response.request`/`original_exception.request`. Flag it as intentional if seen again, not a fresh
+violation.
+
+**Tooling gap:** `ruff.toml` excludes `tests/` entirely (`exclude = [..., "tests"]`), so
+`ruff check .` / the `ruff-check` pre-commit hook never lints test files — unused imports,
+missing annotations, etc. in `tests/` pass CI silently. Running `ruff check <path>` with an
+explicit test file path bypasses the exclude and does catch these; use that when reviewing test
+diffs in this repo, since the standard hook won't.

.gitignore

@@ -178,3 +178,4 @@ scratch*.py
 docs/reference/
 .vscode
 .playwright-mcp/
+.claude/worktrees/

src/otf_api/api/bookings/booking_client.py

@@ -4,6 +4,7 @@
 import pendulum
 
 from otf_api.api.client import API_IO_BASE_URL, OtfClient
+from otf_api.api.utils import validate_identifier
 
 
 class BookingClient:
@@ -33,6 +34,7 @@ def get_classes(self, studio_uuids: list[str]) -> dict:
 
     def delete_booking(self, booking_uuid: str) -> dict:
         """Cancel a booking by booking_uuid."""
+        booking_uuid = validate_identifier(booking_uuid, "booking_uuid")
         resp = self.client.default_request(
             "DELETE", f"/member/members/{self.member_uuid}/bookings/{booking_uuid}", params={"confirmed": "true"}
         )
@@ -61,6 +63,7 @@ def post_class_new(self, body: dict[str, str | bool]) -> dict:
 
     def get_booking(self, booking_uuid: str) -> dict:
         """Retrieve raw booking data."""
+        booking_uuid = validate_identifier(booking_uuid, "booking_uuid")
         return self.client.default_request("GET", f"/member/members/{self.member_uuid}/bookings/{booking_uuid}")["data"]
 
     def get_bookings(self, start_date: str | None, end_date: str | None, status: str | list[str] | None) -> dict:
@@ -94,6 +97,7 @@ def get_bookings_new(
 
     def delete_booking_new(self, booking_id: str) -> None:
         """Cancel a booking by booking_id."""
+        booking_id = validate_identifier(booking_id, "booking_id")
         self.classes_request("DELETE", f"/v1/bookings/me/{booking_id}")
 
     def post_class_rating(

src/otf_api/api/client.py

@@ -29,6 +29,11 @@
 CACHE = get_cache()
 LOGGER = getLogger(__name__)
 
+# Maximum response body size (bytes) the library will attempt to parse as JSON.
+# The response is already buffered by this point — this prevents wasting CPU on
+# parsing an unexpectedly large payload, not the memory cost of the read itself.
+MAX_RESPONSE_SIZE = 10 * 1024 * 1024  # 10 MB
+
 
 class OtfClient:
     """Client for interacting with the OTF API - generally to be used by the Otf class.
@@ -233,6 +238,22 @@ def _handle_response(self, method: str, response: httpx.Response, request: httpx
             LOGGER.debug("No content returned from %s %s", method, response.url)
             return None
 
+        content_length = len(response.content)
+        if content_length > MAX_RESPONSE_SIZE:
+            LOGGER.error(
+                "Response from %s %s exceeds size limit (%d bytes, max %d)",
+                method,
+                response.url,
+                content_length,
+                MAX_RESPONSE_SIZE,
+            )
+            raise exc.OtfRequestError(
+                f"Response too large ({content_length} bytes, max {MAX_RESPONSE_SIZE})",
+                original_exception=None,
+                response=response,
+                request=request,
+            )
+
         try:
             json_data = response.json()
         except JSONDecodeError as e:

src/otf_api/api/members/member_api.py

@@ -6,6 +6,8 @@
 
 from .member_client import MemberClient
 
+_MAX_NAME_LENGTH = 50
+
 if typing.TYPE_CHECKING:
     from otf_api import Otf
     from otf_api.api.client import OtfClient
@@ -114,6 +116,33 @@ def update_email_notification_settings(
         new_settings = self.get_email_notification_settings()
         return new_settings
 
+    @staticmethod
+    def validate_name(value: str, name: str) -> str:
+        """Validate a name field before sending to the API.
+
+        Guards against control characters and unreasonable lengths that the
+        API's weak validation layer may accept.
+
+        Args:
+            value: The name string to validate.
+            name: Human-readable name of the parameter (for error messages).
+
+        Returns:
+            The stripped, validated name string.
+
+        Raises:
+            ValueError: If the name is invalid.
+        """
+        value = value.strip()
+        if not value:
+            raise ValueError(f"{name} must not be empty after stripping whitespace")
+        if len(value) > _MAX_NAME_LENGTH:
+            raise ValueError(f"{name} is too long ({len(value)} chars, max {_MAX_NAME_LENGTH})")
+        # ASCII control characters (below space, except tab)
+        if any(ord(c) < 32 and c != "\t" for c in value):
+            raise ValueError(f"{name} contains control characters")
+        return value
+
     def update_member_name(self, first_name: str | None = None, last_name: str | None = None) -> models.MemberDetail:
         """Update the member's name. Will return the original member details if no names are provided.
 
@@ -140,6 +169,9 @@ def update_member_name(self, first_name: str | None = None, last_name: str | Non
         if last_name is None:
             raise ValueError("Last name is required")
 
+        first_name = self.validate_name(first_name, "first_name")
+        last_name = self.validate_name(last_name, "last_name")
+
         res = self.client.put_member_name(first_name, last_name)
 
         return models.MemberDetail.create(**res, api=self.otf)
@@ -156,7 +188,20 @@ def get_member_detail(self) -> models.MemberDetail:
         home_studio_uuid = data["homeStudio"]["studioUUId"]
         data["home_studio"] = self.otf.studios.get_studio_detail(home_studio_uuid)
 
-        return models.MemberDetail.create(**data, api=self.otf)
+        member = models.MemberDetail.create(**data, api=self.otf)
+
+        # Sanity-check that the response matches the request — the v1 API uses
+        # explicit member UUIDs in paths rather than token-derived identity.
+        expected_uuid = self.client.member_uuid
+        if member.member_uuid != expected_uuid:
+            LOGGER.error(
+                "API returned member data for %s but authenticated as %s",
+                member.member_uuid,
+                expected_uuid,
+            )
+            raise ValueError(f"API returned data for member {member.member_uuid}, expected {expected_uuid}")
+
+        return member
 
     def get_member_membership(self) -> models.MemberMembership:
         """Get the member's membership details.

src/otf_api/api/studios/studio_client.py

@@ -3,6 +3,7 @@
 from typing import Any
 
 from otf_api.api.client import CACHE, OtfClient
+from otf_api.api.utils import validate_identifier
 
 LOGGER = getLogger(__name__)
 
@@ -21,6 +22,7 @@ def __init__(self, client: OtfClient):
     @CACHE.memoize(expire=600, tag="studio_detail")
     def get_studio_detail(self, studio_uuid: str) -> dict:
         """Retrieve raw studio details."""
+        studio_uuid = validate_identifier(studio_uuid, "studio_uuid")
         return self.client.default_request("GET", f"/mobile/v1/studios/{studio_uuid}")["data"]
 
     def _get_studios_by_geo(
@@ -93,6 +95,7 @@ def get_favorite_studios(self) -> dict:
 
     def get_studio_services(self, studio_uuid: str) -> dict:
         """Retrieve raw studio services data."""
+        studio_uuid = validate_identifier(studio_uuid, "studio_uuid")
         return self.client.default_request("GET", f"/member/studios/{studio_uuid}/services")["data"]
 
     def post_favorite_studio(self, studio_uuids: list[str]) -> dict:
@@ -116,6 +119,7 @@ def get_studio_detail_threaded(self, studio_uuids: list[str]) -> dict[str, dict[
         Returns:
             dict[str, dict[str, Any]]: A dictionary of studio details, keyed by studio UUID.
         """
+        studio_uuids = [validate_identifier(uuid, "studio_uuid") for uuid in studio_uuids]
         studios_dict: dict[str, dict[str, Any]] = {}
         with ThreadPoolExecutor(max_workers=10) as pool:
             futures = {pool.submit(self.get_studio_detail, uuid): uuid for uuid in studio_uuids}

src/otf_api/api/trends/trend_api.py

@@ -32,7 +32,7 @@ def __init__(self, otf: "Otf", otf_client: "OtfClient"):
 
     def get_workout_stats(
         self,
-        trend_type: TrendType | str,
+        trend_type: TrendType,
         start_date: date | str | None = None,
         end_date: date | str | None = None,
     ) -> WorkoutStatsResponse:
@@ -46,13 +46,16 @@ def get_workout_stats(
         Returns:
             WorkoutStatsResponse: The stat data with individual data points per workout.
         """
+        if not isinstance(trend_type, TrendType):
+            raise TypeError(f"trend_type must be a TrendType enum member, got {type(trend_type).__name__}")
+
         start = utils.ensure_date(start_date) or pendulum.today().subtract(days=90).date()
         end = utils.ensure_date(end_date) or pendulum.today().date()
 
         start_str = pendulum.instance(pendulum.datetime(start.year, start.month, start.day)).to_iso8601_string()
         end_str = pendulum.instance(pendulum.datetime(end.year, end.month, end.day, 23, 59, 59)).to_iso8601_string()
 
-        stats_key = str(trend_type)
+        stats_key = utils.validate_identifier(trend_type.value, "stats_key")
         data = self.client.get_workout_stats(stats_key, start_str, end_str)
         return WorkoutStatsResponse(**data)
 

src/otf_api/api/utils.py

@@ -1,3 +1,4 @@
+import re
 import typing
 from datetime import date, datetime, time, timedelta
 from json import JSONDecodeError
@@ -16,6 +17,36 @@
 
 MIN_TIME = datetime.min.time()
 
+_MAX_IDENTIFIER_LENGTH = 200
+_UNSAFE_PATH_CHARS = re.compile(r"[/\\\x00%\s]")
+
+
+def validate_identifier(value: str, name: str) -> str:
+    """Validate that a string is safe to use as a URL path segment.
+
+    Rejects values containing path separators, traversal sequences, null bytes,
+    percent-encoding, and whitespace to prevent path injection.
+
+    Args:
+        value: The identifier string to validate.
+        name: Human-readable name of the parameter (for error messages).
+
+    Returns:
+        The validated string, unchanged.
+
+    Raises:
+        ValueError: If the string is empty, too long, or contains unsafe characters.
+    """
+    if not value:
+        raise ValueError(f"{name} must not be empty")
+    if len(value) > _MAX_IDENTIFIER_LENGTH:
+        raise ValueError(f"{name} is too long ({len(value)} chars, max {_MAX_IDENTIFIER_LENGTH})")
+    if ".." in value:
+        raise ValueError(f"{name} contains path traversal sequence")
+    if _UNSAFE_PATH_CHARS.search(value):
+        raise ValueError(f"{name} contains unsafe characters for a URL path segment")
+    return value
+
 
 def get_studio_uuid_list(
     home_studio_uuid: str, studio_uuids: list[str] | str | None, include_home_studio: bool = True
@@ -142,10 +173,10 @@ def get_booking_uuid(booking_or_uuid: str | Booking) -> str:
         TypeError: If the input is not a string or Booking object.
     """
     if isinstance(booking_or_uuid, str):
-        return booking_or_uuid
+        return validate_identifier(booking_or_uuid, "booking_uuid")
 
     if isinstance(booking_or_uuid, Booking):
-        return booking_or_uuid.booking_uuid
+        return validate_identifier(booking_or_uuid.booking_uuid, "booking_uuid")
 
     raise TypeError(f"Expected Booking or str, got {type(booking_or_uuid)}")
 
@@ -163,10 +194,10 @@ def get_booking_id(booking_or_id: str | BookingV2) -> str:
         TypeError: If the input is not a string or BookingV2 object.
     """
     if isinstance(booking_or_id, str):
-        return booking_or_id
+        return validate_identifier(booking_or_id, "booking_id")
 
     if isinstance(booking_or_id, BookingV2):
-        return booking_or_id.booking_id
+        return validate_identifier(booking_or_id.booking_id, "booking_id")
 
     raise TypeError(f"Expected BookingV2 or str, got {type(booking_or_id)}")
 
@@ -186,12 +217,12 @@ def get_class_uuid(class_or_uuid: "str | OtfClass | BookingV2Class") -> str:
 
     """
     if isinstance(class_or_uuid, str):
-        return class_or_uuid
+        return validate_identifier(class_or_uuid, "class_uuid")
 
     if hasattr(class_or_uuid, "class_uuid"):
         class_uuid = getattr(class_or_uuid, "class_uuid", None)
         if class_uuid:
-            return class_uuid
+            return validate_identifier(class_uuid, "class_uuid")
         raise ValueError("Class does not have a class_uuid")
 
     raise TypeError(f"Expected OtfClass, BookingV2Class, or str, got {type(class_or_uuid)}")
@@ -210,10 +241,10 @@ def get_class_id(class_or_id: str | BookingV2Class) -> str:
         TypeError: If the input is not a string or BookingV2Class.
     """
     if isinstance(class_or_id, str):
-        return class_or_id
+        return validate_identifier(class_or_id, "class_id")
 
     if isinstance(class_or_id, BookingV2Class):
-        return class_or_id.class_id
+        return validate_identifier(class_or_id.class_id, "class_id")
 
     raise TypeError(f"Expected BookingV2Class or str, got {type(class_or_id)}")