From b0c3fbde7fe5c635c662516f1d5f3adb96fe8eef Mon Sep 17 00:00:00 2001 From: Jeff West Date: Sun, 17 May 2026 06:45:15 -0500 Subject: [PATCH 1/5] fix(orders, events, series): rename Order.type, normalize bool query params MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit F-N-03: Order.type was leftover from the pre-v0.8.0 CreateOrderRequest.type field. The spec's Order schema does still include a `type` enum on responses, so rename SDK-side to `order_type` (validation alias accepts the wire `type`) — same builtin-shadow rationale as milestone_type, target_type, and incentive_type elsewhere. F-N-08: events.py and series.py built bool query params inline with `"true" if x else None`, which silently drops explicit False. Replace with the documented `_bool_param(...)` helper so callers can opt out (False -> "false"). Mechanical, matches markets.py / live_data.py. Closes #91 Co-Authored-By: Claude Opus 4.7 (1M context) --- kalshi/models/orders.py | 9 ++++++++- kalshi/resources/events.py | 12 ++++++------ kalshi/resources/series.py | 12 ++++++------ tests/test_events.py | 39 ++++++++++++++++++++++++++++++++++++++ tests/test_models.py | 22 +++++++++++++++++++++ tests/test_series.py | 37 ++++++++++++++++++++++++++++++++++++ 6 files changed, 118 insertions(+), 13 deletions(-) diff --git a/kalshi/models/orders.py b/kalshi/models/orders.py index c71e7aa..aa2ec87 100644 --- a/kalshi/models/orders.py +++ b/kalshi/models/orders.py @@ -44,7 +44,14 @@ class Order(BaseModel): status: str | None = None side: str | None = None is_yes: bool | None = None - type: str | None = None + # Spec field is named ``type`` (enum: limit, market). Renamed to + # ``order_type`` on the SDK side to avoid shadowing the Python builtin — + # same rationale as milestone_type / target_type / incentive_type + # elsewhere. The wire still sends ``type``; validation alias accepts both. + order_type: str | None = Field( + default=None, + validation_alias=AliasChoices("type", "order_type"), + ) yes_price: DollarDecimal | None = Field( default=None, validation_alias=AliasChoices("yes_price_dollars", "yes_price"), diff --git a/kalshi/resources/events.py b/kalshi/resources/events.py index ebd129f..d780272 100644 --- a/kalshi/resources/events.py +++ b/kalshi/resources/events.py @@ -7,7 +7,7 @@ from kalshi.models.common import Page from kalshi.models.events import Event, EventMetadata, EventStatusLiteral -from kalshi.resources._base import AsyncResource, SyncResource, _params +from kalshi.resources._base import AsyncResource, SyncResource, _bool_param, _params # Shared param builders (issue #46). @@ -26,8 +26,8 @@ def _list_events_params( return _params( status=status, series_ticker=series_ticker, - with_nested_markets="true" if with_nested_markets else None, - with_milestones="true" if with_milestones else None, + with_nested_markets=_bool_param(with_nested_markets), + with_milestones=_bool_param(with_milestones), min_close_ts=min_close_ts, min_updated_ts=min_updated_ts, limit=limit, @@ -46,7 +46,7 @@ def _list_multivariate_events_params( return _params( series_ticker=series_ticker, collection_ticker=collection_ticker, - with_nested_markets="true" if with_nested_markets else None, + with_nested_markets=_bool_param(with_nested_markets), limit=limit, cursor=cursor, ) @@ -136,7 +136,7 @@ def get( with_nested_markets: bool = False, ) -> Event: params = _params( - with_nested_markets="true" if with_nested_markets else None, + with_nested_markets=_bool_param(with_nested_markets), ) data = self._get(f"/events/{event_ticker}", params=params) return Event.model_validate(data.get("event", data)) @@ -230,7 +230,7 @@ async def get( with_nested_markets: bool = False, ) -> Event: params = _params( - with_nested_markets="true" if with_nested_markets else None, + with_nested_markets=_bool_param(with_nested_markets), ) data = await self._get(f"/events/{event_ticker}", params=params) return Event.model_validate(data.get("event", data)) diff --git a/kalshi/resources/series.py b/kalshi/resources/series.py index ba992d9..98c0c31 100644 --- a/kalshi/resources/series.py +++ b/kalshi/resources/series.py @@ -11,7 +11,7 @@ Series, SeriesFeeChange, ) -from kalshi.resources._base import AsyncResource, SyncResource, _params +from kalshi.resources._base import AsyncResource, SyncResource, _bool_param, _params # Shared param builders (issue #46). @@ -27,8 +27,8 @@ def _list_series_params( return _params( category=category, tags=tags, - include_product_metadata="true" if include_product_metadata else None, - include_volume="true" if include_volume else None, + include_product_metadata=_bool_param(include_product_metadata), + include_volume=_bool_param(include_volume), min_updated_ts=min_updated_ts, ) @@ -40,7 +40,7 @@ def _fee_changes_params( ) -> dict[str, Any]: return _params( series_ticker=series_ticker, - show_historical="true" if show_historical else None, + show_historical=_bool_param(show_historical), ) @@ -98,7 +98,7 @@ def get( include_volume: bool | None = None, ) -> Series: params = _params( - include_volume="true" if include_volume else None, + include_volume=_bool_param(include_volume), ) data = self._get(f"/series/{series_ticker}", params=params) return Series.model_validate(data.get("series", data)) @@ -188,7 +188,7 @@ async def get( include_volume: bool | None = None, ) -> Series: params = _params( - include_volume="true" if include_volume else None, + include_volume=_bool_param(include_volume), ) data = await self._get(f"/series/{series_ticker}", params=params) return Series.model_validate(data.get("series", data)) diff --git a/tests/test_events.py b/tests/test_events.py index f5f39c2..e811c83 100644 --- a/tests/test_events.py +++ b/tests/test_events.py @@ -381,3 +381,42 @@ async def test_list_all_multivariate(self, async_events: AsyncEventsResource) -> ) tickers = [e.event_ticker async for e in async_events.list_all_multivariate()] assert tickers == ["A", "B"] + + +class TestBoolParamSerialization: + """Regression: issue #91 F-N-08. + + Bool query params were built with inline ``"true" if x else None`` + which silently drops explicit ``False``. ``_bool_param`` is the + documented helper that emits ``"false"`` for explicit False. + """ + + @respx.mock + def test_list_with_nested_markets_false_is_sent(self, events: EventsResource) -> None: + route = respx.get("https://test.kalshi.com/trade-api/v2/events").mock( + return_value=httpx.Response(200, json={"events": [], "cursor": ""}) + ) + events.list(with_nested_markets=False, with_milestones=False) + params = dict(route.calls[0].request.url.params) + assert params["with_nested_markets"] == "false" + assert params["with_milestones"] == "false" + + @respx.mock + def test_list_with_nested_markets_none_is_dropped( + self, events: EventsResource + ) -> None: + route = respx.get("https://test.kalshi.com/trade-api/v2/events").mock( + return_value=httpx.Response(200, json={"events": [], "cursor": ""}) + ) + events.list() + params = dict(route.calls[0].request.url.params) + assert "with_nested_markets" not in params + assert "with_milestones" not in params + + @respx.mock + def test_list_multivariate_false_is_sent(self, events: EventsResource) -> None: + route = respx.get( + "https://test.kalshi.com/trade-api/v2/events/multivariate" + ).mock(return_value=httpx.Response(200, json={"events": [], "cursor": ""})) + events.list_multivariate(with_nested_markets=False) + assert route.calls[0].request.url.params["with_nested_markets"] == "false" diff --git a/tests/test_models.py b/tests/test_models.py index a96fb33..41ebeef 100644 --- a/tests/test_models.py +++ b/tests/test_models.py @@ -124,6 +124,28 @@ def test_order_accepts_dollars_suffix(self) -> None: assert o.taker_fill_cost == Decimal("6.5000") assert o.taker_fees == Decimal("0.0650") + def test_order_type_populated_from_wire_field(self) -> None: + """Regression: issue #91. + + Spec ``Order.type`` is renamed to ``order_type`` on the SDK side to + avoid shadowing the Python builtin. The wire still sends ``type``; + validation must keep populating ``order_type`` from it so callers + actually see the value (previous dead-field state always returned + ``None``). + """ + o = Order.model_validate({"order_id": "x", "type": "limit"}) + assert o.order_type == "limit" + + def test_order_type_accepts_python_name(self) -> None: + """Pydantic ``populate_by_name`` keeps the SDK kwarg name working.""" + o = Order.model_validate({"order_id": "x", "order_type": "market"}) + assert o.order_type == "market" + + def test_order_has_no_type_field(self) -> None: + """The old ``type`` attribute is gone — callers must use ``order_type``.""" + assert "type" not in Order.model_fields + assert "order_type" in Order.model_fields + def test_fill_accepts_dollars_suffix(self) -> None: from kalshi.models.orders import Fill diff --git a/tests/test_series.py b/tests/test_series.py index 969ad81..797a5a6 100644 --- a/tests/test_series.py +++ b/tests/test_series.py @@ -378,3 +378,40 @@ async def test_forecast_auth_guard(self, unauth_async_series: AsyncSeriesResourc await unauth_async_series.forecast_percentile_history( "SER", "EVT", percentiles=[5000], start_ts=0, end_ts=1, period_interval=60, ) + + +class TestSeriesBoolParamSerialization: + """Regression: issue #91 F-N-08 — explicit False on bool query params + must serialize to ``"false"`` (was silently dropped by inline ternary).""" + + @respx.mock + def test_list_include_volume_false_is_sent( + self, series_resource: SeriesResource + ) -> None: + route = respx.get(f"{BASE}/series").mock( + return_value=httpx.Response(200, json={"series": []}) + ) + series_resource.list(include_volume=False, include_product_metadata=False) + params = dict(route.calls[0].request.url.params) + assert params["include_volume"] == "false" + assert params["include_product_metadata"] == "false" + + @respx.mock + def test_get_include_volume_false_is_sent( + self, series_resource: SeriesResource + ) -> None: + route = respx.get(f"{BASE}/series/ECON-GDP").mock( + return_value=httpx.Response(200, json={"series": SERIES_PAYLOAD}) + ) + series_resource.get("ECON-GDP", include_volume=False) + assert route.calls[0].request.url.params["include_volume"] == "false" + + @respx.mock + def test_fee_changes_show_historical_false_is_sent( + self, series_resource: SeriesResource + ) -> None: + route = respx.get(f"{BASE}/series/fee_changes").mock( + return_value=httpx.Response(200, json={"series_fee_change_arr": []}) + ) + series_resource.fee_changes(show_historical=False) + assert route.calls[0].request.url.params["show_historical"] == "false" From 3ce22ae3f1d730b5d0999cf2a9c2473d18bcb427 Mon Sep 17 00:00:00 2001 From: Jeff West Date: Sun, 17 May 2026 06:46:09 -0500 Subject: [PATCH 2/5] fix(config): validate KALSHI_API_BASE_URL scheme and host MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reject base_url with a non-http(s) scheme or http:// pointing at a remote host. The previous config accepted any string, so an attacker who could write to a process's environment (docker run -e, CI variable, shell history) could redirect every authenticated request — and the KALSHI-ACCESS-KEY header and request signature — to an endpoint they control. - https://* with a known Kalshi host (production, demo): silent. - https://* with any other host: warn (keeps legitimate proxy use cases working but surfaces awareness). - http://localhost, 127.0.0.1, ::1: allowed (local mocks, tests). - http://: ValueError at config construction. - non-http(s) scheme or missing host: ValueError. Closes #94 Co-Authored-By: Claude Opus 4.7 (1M context) --- kalshi/config.py | 52 ++++++++++++++++++++++++++++++++++++++++++++ tests/test_client.py | 49 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 101 insertions(+) diff --git a/kalshi/config.py b/kalshi/config.py index b003b4c..cfd277d 100644 --- a/kalshi/config.py +++ b/kalshi/config.py @@ -2,7 +2,9 @@ from __future__ import annotations +import logging from dataclasses import dataclass, field +from urllib.parse import urlparse PRODUCTION_BASE_URL = "https://api.elections.kalshi.com/trade-api/v2" DEMO_BASE_URL = "https://demo-api.kalshi.co/trade-api/v2" @@ -14,6 +16,13 @@ DEFAULT_MAX_RETRIES = 3 DEFAULT_WS_MAX_RETRIES = 10 +_KNOWN_HOSTS = frozenset( + {"api.elections.kalshi.com", "demo-api.kalshi.co"} +) +_LOCAL_HOSTS = frozenset({"localhost", "127.0.0.1", "::1"}) + +logger = logging.getLogger("kalshi") + @dataclass(frozen=True) class KalshiConfig: @@ -42,6 +51,49 @@ def __post_init__(self) -> None: object.__setattr__(self, "base_url", self.base_url.rstrip("/")) if self.ws_base_url.endswith("/"): object.__setattr__(self, "ws_base_url", self.ws_base_url.rstrip("/")) + self._validate_base_url(self.base_url) + + @staticmethod + def _validate_base_url(base_url: str) -> None: + """Reject base_urls that would leak API credentials. + + An attacker who can write to the process environment (``docker run + -e``, CI variable, shell history) can otherwise redirect signed + requests to an arbitrary host. Enforce https-only, and warn when + the host isn't a known Kalshi endpoint so misroutes surface in + logs. + + ``http://`` is permitted only for loopback hosts (localhost, + 127.0.0.1, ::1) so local mock servers and tests still work. + """ + parsed = urlparse(base_url) + scheme = parsed.scheme.lower() + host = (parsed.hostname or "").lower() + + if scheme not in ("http", "https"): + raise ValueError( + f"KalshiConfig.base_url must use http:// or https://, got " + f"scheme={scheme!r} (url={base_url!r})" + ) + if not host: + raise ValueError( + f"KalshiConfig.base_url is missing a host: {base_url!r}" + ) + if scheme == "http" and host not in _LOCAL_HOSTS: + raise ValueError( + f"KalshiConfig.base_url must use https:// for non-loopback " + f"hosts; http:// is only allowed for {sorted(_LOCAL_HOSTS)} " + f"(url={base_url!r}). Plaintext to a remote host would " + f"expose the KALSHI-ACCESS-KEY header and request signature." + ) + if host not in _KNOWN_HOSTS and host not in _LOCAL_HOSTS: + logger.warning( + "KalshiConfig.base_url host %r is not a known Kalshi " + "endpoint (%s). Requests will be signed and sent there " + "with your API key — verify this is intentional.", + host, + sorted(_KNOWN_HOSTS), + ) @classmethod def production(cls, **kwargs: object) -> KalshiConfig: diff --git a/tests/test_client.py b/tests/test_client.py index 22b06d6..e375e17 100644 --- a/tests/test_client.py +++ b/tests/test_client.py @@ -275,6 +275,55 @@ def test_has_resources(self, test_auth: KalshiAuth) -> None: client.close() +class TestBaseUrlValidation: + """Regression: issue #94. + + KALSHI_API_BASE_URL is reachable via env var on any deployment plane; + accepting an arbitrary scheme/host lets an attacker route signed + requests with the KALSHI-ACCESS-KEY header to a host they control. + Config must reject non-http(s) schemes and plaintext-to-remote-host; + warn (but allow) when the host isn't the known Kalshi production or + demo endpoint. + """ + + def test_https_to_known_host_is_silent(self, caplog: pytest.LogCaptureFixture) -> None: + with caplog.at_level("WARNING", logger="kalshi"): + KalshiConfig(base_url=PRODUCTION_BASE_URL) + assert "is not a known Kalshi endpoint" not in caplog.text + + def test_https_to_unknown_host_warns(self, caplog: pytest.LogCaptureFixture) -> None: + with caplog.at_level("WARNING", logger="kalshi"): + KalshiConfig(base_url="https://attacker.example/trade-api/v2") + assert "not a known Kalshi endpoint" in caplog.text + + def test_http_to_loopback_is_allowed(self) -> None: + # Local mock servers (e.g., respx, fake-API harnesses) must keep working. + for host in ("localhost", "127.0.0.1", "[::1]"): + cfg = KalshiConfig(base_url=f"http://{host}:8000/trade-api/v2") + assert cfg.base_url.startswith("http://") + + def test_http_to_remote_host_rejected(self) -> None: + with pytest.raises(ValueError, match="https://"): + KalshiConfig(base_url="http://api.elections.kalshi.com/trade-api/v2") + + def test_http_to_attacker_rejected(self) -> None: + with pytest.raises(ValueError, match="https://"): + KalshiConfig(base_url="http://attacker.example/trade-api/v2") + + def test_non_http_scheme_rejected(self) -> None: + with pytest.raises(ValueError, match="http://"): + KalshiConfig(base_url="ftp://api.elections.kalshi.com/trade-api/v2") + + def test_file_scheme_rejected(self) -> None: + # urlparse on file:///etc/passwd yields scheme=file, no host. + with pytest.raises(ValueError): + KalshiConfig(base_url="file:///etc/passwd") + + def test_missing_host_rejected(self) -> None: + with pytest.raises(ValueError, match="missing a host"): + KalshiConfig(base_url="https:///trade-api/v2") + + class TestSyncTransportUnauthenticated: """Tests for SyncTransport with auth=None (unauthenticated mode).""" From 002933ad3dadbbf4fb5d90b172791d6a1fd2ed9a Mon Sep 17 00:00:00 2001 From: Jeff West Date: Sun, 17 May 2026 06:46:23 -0500 Subject: [PATCH 3/5] fix(retry): reject negative, NaN, and infinite Retry-After values Retry-After was parsed with bare float() and used straight in ``min(retry_after, retry_max_delay)``. Three failure modes the cap didn't cover: - ``Retry-After: -1`` -> ``min(-1, 30) = -1`` -> ``time.sleep(-1)`` is a POSIX no-op; the client busy-loops the server-controlled retry. - ``Retry-After: nan`` -> ``min(nan, 30) = nan`` -> ``time.sleep(nan)`` raises ValueError outside the documented retry pathway. - ``Retry-After: inf`` -> would survive the cap math but had no test. Reject all three at parse time and fall back to computed backoff. HTTP-date format still falls through unchanged. Closes #96 Co-Authored-By: Claude Opus 4.7 (1M context) --- kalshi/_base_client.py | 8 +++++++ tests/test_client.py | 52 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 60 insertions(+) diff --git a/kalshi/_base_client.py b/kalshi/_base_client.py index 056c331..f129b2b 100644 --- a/kalshi/_base_client.py +++ b/kalshi/_base_client.py @@ -7,6 +7,7 @@ from __future__ import annotations import logging +import math import random import time from typing import Any @@ -59,6 +60,13 @@ def _map_error(response: httpx.Response) -> KalshiError: if retry_after: try: retry_after_val = float(retry_after) + # Reject negative, NaN, and infinity. Negative would make + # ``min(retry_after, retry_max_delay)`` go negative and turn + # ``time.sleep`` into a no-op (busy-loop). NaN propagates + # through ``min`` and crashes ``time.sleep`` with ValueError. + # Fall back to computed backoff in either case. + if retry_after_val < 0 or not math.isfinite(retry_after_val): + retry_after_val = None except ValueError: retry_after_val = None # HTTP-date format, fall back to computed backoff return KalshiRateLimitError( diff --git a/tests/test_client.py b/tests/test_client.py index e375e17..effd243 100644 --- a/tests/test_client.py +++ b/tests/test_client.py @@ -102,6 +102,58 @@ def test_validation_error_with_details(self) -> None: assert isinstance(err, KalshiValidationError) assert err.details == {"ticker": "required"} + # --- Regression: issue #96 ------------------------------------------- + # Retry-After must reject negative, NaN, and infinity values so the + # transport never calls time.sleep with a non-positive or non-finite + # delay (busy-loop or ValueError crash respectively). + + def test_429_retry_after_negative_falls_back(self) -> None: + """Negative Retry-After is rejected so backoff isn't bypassed.""" + resp = httpx.Response( + 429, json={"message": "slow"}, headers={"Retry-After": "-1"} + ) + err = _map_error(resp) + assert isinstance(err, KalshiRateLimitError) + assert err.retry_after is None + + def test_429_retry_after_nan_falls_back(self) -> None: + """NaN Retry-After is rejected so time.sleep never raises ValueError.""" + resp = httpx.Response( + 429, json={"message": "slow"}, headers={"Retry-After": "nan"} + ) + err = _map_error(resp) + assert isinstance(err, KalshiRateLimitError) + assert err.retry_after is None + + def test_429_retry_after_infinity_falls_back(self) -> None: + """Infinity Retry-After is rejected (would survive min() cap math).""" + resp = httpx.Response( + 429, json={"message": "slow"}, headers={"Retry-After": "inf"} + ) + err = _map_error(resp) + assert isinstance(err, KalshiRateLimitError) + assert err.retry_after is None + + def test_429_retry_after_zero_is_kept(self) -> None: + """Zero is a valid 'retry immediately' value; only <0 is rejected.""" + resp = httpx.Response( + 429, json={"message": "slow"}, headers={"Retry-After": "0"} + ) + err = _map_error(resp) + assert isinstance(err, KalshiRateLimitError) + assert err.retry_after == 0.0 + + def test_429_retry_after_http_date_falls_back(self) -> None: + """HTTP-date format isn't parsed; should fall through to backoff.""" + resp = httpx.Response( + 429, + json={"message": "slow"}, + headers={"Retry-After": "Wed, 21 Oct 2026 07:28:00 GMT"}, + ) + err = _map_error(resp) + assert isinstance(err, KalshiRateLimitError) + assert err.retry_after is None + class TestSyncTransportRetry: @respx.mock From b42b123fee8745d8aaf292c57bac6682d119cc1b Mon Sep 17 00:00:00 2001 From: Jeff West Date: Sun, 17 May 2026 06:58:16 -0500 Subject: [PATCH 4/5] review: address #112 bot feedback (ws_base_url, CHANGELOG, comment trim) Per code review on PR #112: - Validate ws_base_url with the same scheme/host rules as base_url. WebSocket connect carries KALSHI-ACCESS-KEY too, so plaintext-to-remote is the same credential-leakage surface as #94. Generalized _validate_base_url -> _validate_url(secure=, plaintext=) so http/https and ws/wss share the rule. - CHANGELOG.md: added [Unreleased] Breaking section for Order.type -> Order.order_type. Version bump (v1.2 vs v2.0) still deferred to release cut, but the entry now exists. - _base_client.py: trimmed the 5-line Retry-After comment block to one line per CLAUDE.md style guide. - Added TestWsBaseUrlValidation (+6 tests). Deferred (not addressed): - Optional @property def type(self) deprecation shim. User explicitly picked the clean-break rename earlier. - Pre-existing Order lacks model_config; outside this PR's scope, will open a follow-up issue. - Cosmetic host -> url_host test variable rename; low value. Co-Authored-By: Claude Opus 4.7 (1M context) --- CHANGELOG.md | 24 ++++++++++++++++++++++++ kalshi/_base_client.py | 6 +----- kalshi/config.py | 36 +++++++++++++++++++----------------- tests/test_client.py | 37 +++++++++++++++++++++++++++++++++++++ 4 files changed, 81 insertions(+), 22 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index fbd020d..1aa4be4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,30 @@ All notable changes to kalshi-sdk will be documented in this file. +## [Unreleased] + +### Breaking + +- **`Order.type` renamed to `Order.order_type`.** Wire format is unchanged + (`validation_alias=AliasChoices("type", "order_type")` accepts both names on + deserialization), but any user code reading `.type` on an `Order` instance + must migrate to `.order_type`. Rationale: matches the project's existing + builtin-shadow-avoidance convention (`milestone_type`, `target_type`, + `incentive_type`). Spec v3.13.0 still defines `type` as required, so the + field is preserved on the wire — only the Python attribute name changed + (#91). + + ```python + # Before + order = client.portfolio.orders.get(order_id="...") + print(order.type) # AttributeError after upgrade + + # After + print(order.order_type) + ``` + + Version-bump decision (v1.2 vs v2.0) deferred to release cut. + ## 1.1.0 — 2026-05-16 Post-1.0 enhancements and polish. 17 issues closed across four parallel waves of diff --git a/kalshi/_base_client.py b/kalshi/_base_client.py index f129b2b..0242f87 100644 --- a/kalshi/_base_client.py +++ b/kalshi/_base_client.py @@ -60,11 +60,7 @@ def _map_error(response: httpx.Response) -> KalshiError: if retry_after: try: retry_after_val = float(retry_after) - # Reject negative, NaN, and infinity. Negative would make - # ``min(retry_after, retry_max_delay)`` go negative and turn - # ``time.sleep`` into a no-op (busy-loop). NaN propagates - # through ``min`` and crashes ``time.sleep`` with ValueError. - # Fall back to computed backoff in either case. + # Reject non-finite/negative: NaN crashes time.sleep, negative busy-loops. if retry_after_val < 0 or not math.isfinite(retry_after_val): retry_after_val = None except ValueError: diff --git a/kalshi/config.py b/kalshi/config.py index cfd277d..75b5f6c 100644 --- a/kalshi/config.py +++ b/kalshi/config.py @@ -51,46 +51,48 @@ def __post_init__(self) -> None: object.__setattr__(self, "base_url", self.base_url.rstrip("/")) if self.ws_base_url.endswith("/"): object.__setattr__(self, "ws_base_url", self.ws_base_url.rstrip("/")) - self._validate_base_url(self.base_url) + self._validate_url(self.base_url, "base_url", secure="https", plaintext="http") + self._validate_url(self.ws_base_url, "ws_base_url", secure="wss", plaintext="ws") @staticmethod - def _validate_base_url(base_url: str) -> None: - """Reject base_urls that would leak API credentials. + def _validate_url(url: str, field_name: str, *, secure: str, plaintext: str) -> None: + """Reject URLs that would leak API credentials. An attacker who can write to the process environment (``docker run -e``, CI variable, shell history) can otherwise redirect signed - requests to an arbitrary host. Enforce https-only, and warn when - the host isn't a known Kalshi endpoint so misroutes surface in - logs. + requests to an arbitrary host. Enforce the secure scheme for remote + hosts; warn when the host isn't a known Kalshi endpoint so misroutes + surface in logs. - ``http://`` is permitted only for loopback hosts (localhost, + The plaintext scheme is permitted only for loopback hosts (localhost, 127.0.0.1, ::1) so local mock servers and tests still work. """ - parsed = urlparse(base_url) + parsed = urlparse(url) scheme = parsed.scheme.lower() host = (parsed.hostname or "").lower() - if scheme not in ("http", "https"): + if scheme not in (secure, plaintext): raise ValueError( - f"KalshiConfig.base_url must use http:// or https://, got " - f"scheme={scheme!r} (url={base_url!r})" + f"KalshiConfig.{field_name} must use {secure}:// or {plaintext}://, " + f"got scheme={scheme!r} (url={url!r})" ) if not host: raise ValueError( - f"KalshiConfig.base_url is missing a host: {base_url!r}" + f"KalshiConfig.{field_name} is missing a host: {url!r}" ) - if scheme == "http" and host not in _LOCAL_HOSTS: + if scheme == plaintext and host not in _LOCAL_HOSTS: raise ValueError( - f"KalshiConfig.base_url must use https:// for non-loopback " - f"hosts; http:// is only allowed for {sorted(_LOCAL_HOSTS)} " - f"(url={base_url!r}). Plaintext to a remote host would " + f"KalshiConfig.{field_name} must use {secure}:// for non-loopback " + f"hosts; {plaintext}:// is only allowed for {sorted(_LOCAL_HOSTS)} " + f"(url={url!r}). Plaintext to a remote host would " f"expose the KALSHI-ACCESS-KEY header and request signature." ) if host not in _KNOWN_HOSTS and host not in _LOCAL_HOSTS: logger.warning( - "KalshiConfig.base_url host %r is not a known Kalshi " + "KalshiConfig.%s host %r is not a known Kalshi " "endpoint (%s). Requests will be signed and sent there " "with your API key — verify this is intentional.", + field_name, host, sorted(_KNOWN_HOSTS), ) diff --git a/tests/test_client.py b/tests/test_client.py index effd243..804e630 100644 --- a/tests/test_client.py +++ b/tests/test_client.py @@ -376,6 +376,43 @@ def test_missing_host_rejected(self) -> None: KalshiConfig(base_url="https:///trade-api/v2") +class TestWsBaseUrlValidation: + """Regression: same credential-leakage risk as #94 but for the WS URL. + + ws_base_url also carries the KALSHI-ACCESS-KEY header on connect, so + plaintext-to-remote and arbitrary schemes must be rejected identically. + """ + + def test_wss_to_known_host_is_silent(self, caplog: pytest.LogCaptureFixture) -> None: + with caplog.at_level("WARNING", logger="kalshi"): + KalshiConfig() # default ws_base_url is wss://api.elections.kalshi.com/... + assert "ws_base_url" not in caplog.text + + def test_wss_to_unknown_host_warns(self, caplog: pytest.LogCaptureFixture) -> None: + with caplog.at_level("WARNING", logger="kalshi"): + KalshiConfig(ws_base_url="wss://attacker.example/ws/v2") + assert "ws_base_url" in caplog.text + assert "not a known Kalshi endpoint" in caplog.text + + def test_ws_to_loopback_is_allowed(self) -> None: + for url_host in ("localhost", "127.0.0.1", "[::1]"): + cfg = KalshiConfig(ws_base_url=f"ws://{url_host}:9000/ws/v2") + assert cfg.ws_base_url.startswith("ws://") + + def test_ws_to_remote_host_rejected(self) -> None: + with pytest.raises(ValueError, match="wss://"): + KalshiConfig(ws_base_url="ws://api.elections.kalshi.com/trade-api/ws/v2") + + def test_https_scheme_rejected_on_ws_url(self) -> None: + # https is not a valid WS scheme; must be ws/wss. + with pytest.raises(ValueError, match="wss://"): + KalshiConfig(ws_base_url="https://api.elections.kalshi.com/ws/v2") + + def test_missing_host_rejected_on_ws_url(self) -> None: + with pytest.raises(ValueError, match="missing a host"): + KalshiConfig(ws_base_url="wss:///ws/v2") + + class TestSyncTransportUnauthenticated: """Tests for SyncTransport with auth=None (unauthenticated mode).""" From 95e9fbf8821212cabc3f9c6bdd53618ee252c20a Mon Sep 17 00:00:00 2001 From: Jeff West Date: Sun, 17 May 2026 07:10:10 -0500 Subject: [PATCH 5/5] fix(retry): honor Retry-After: 0 end-to-end through transport Per second-pass bot review on #112: the prior commit kept Retry-After: 0 on the error object, but transport's "if error.retry_after:" check dropped 0 as falsy and fell through to backoff. Switched to "is not None" in both sync and async transport. Added regression tests that monkeypatch time.sleep / asyncio.sleep and assert the delay is exactly 0.0 (not the backoff fallback). Also addressed: - config._validate_url docstring trimmed to one line (CLAUDE.md). - Called via KalshiConfig._validate_url(...) rather than self. (staticmethod-through-self hides that no instance state is used). Co-Authored-By: Claude Opus 4.7 (1M context) --- kalshi/_base_client.py | 14 +++++++++++--- kalshi/config.py | 16 +++------------- tests/test_async_client.py | 25 +++++++++++++++++++++++++ tests/test_client.py | 20 ++++++++++++++++++++ 4 files changed, 59 insertions(+), 16 deletions(-) diff --git a/kalshi/_base_client.py b/kalshi/_base_client.py index 0242f87..0a33c5f 100644 --- a/kalshi/_base_client.py +++ b/kalshi/_base_client.py @@ -172,8 +172,12 @@ def request( if not should_retry: raise error - # Use Retry-After header if available for 429 - if isinstance(error, KalshiRateLimitError) and error.retry_after: + # Use Retry-After header if available for 429. + # `is not None` — not truthy — so Retry-After: 0 ("retry immediately") is honored. + if ( + isinstance(error, KalshiRateLimitError) + and error.retry_after is not None + ): delay = min(error.retry_after, self._config.retry_max_delay) else: delay = _compute_backoff(attempt, self._config) @@ -286,7 +290,11 @@ async def request( if not should_retry: raise error - if isinstance(error, KalshiRateLimitError) and error.retry_after: + # `is not None` so Retry-After: 0 ("retry immediately") is honored. + if ( + isinstance(error, KalshiRateLimitError) + and error.retry_after is not None + ): delay = min(error.retry_after, self._config.retry_max_delay) else: delay = _compute_backoff(attempt, self._config) diff --git a/kalshi/config.py b/kalshi/config.py index 75b5f6c..c90551f 100644 --- a/kalshi/config.py +++ b/kalshi/config.py @@ -51,22 +51,12 @@ def __post_init__(self) -> None: object.__setattr__(self, "base_url", self.base_url.rstrip("/")) if self.ws_base_url.endswith("/"): object.__setattr__(self, "ws_base_url", self.ws_base_url.rstrip("/")) - self._validate_url(self.base_url, "base_url", secure="https", plaintext="http") - self._validate_url(self.ws_base_url, "ws_base_url", secure="wss", plaintext="ws") + KalshiConfig._validate_url(self.base_url, "base_url", secure="https", plaintext="http") + KalshiConfig._validate_url(self.ws_base_url, "ws_base_url", secure="wss", plaintext="ws") @staticmethod def _validate_url(url: str, field_name: str, *, secure: str, plaintext: str) -> None: - """Reject URLs that would leak API credentials. - - An attacker who can write to the process environment (``docker run - -e``, CI variable, shell history) can otherwise redirect signed - requests to an arbitrary host. Enforce the secure scheme for remote - hosts; warn when the host isn't a known Kalshi endpoint so misroutes - surface in logs. - - The plaintext scheme is permitted only for loopback hosts (localhost, - 127.0.0.1, ::1) so local mock servers and tests still work. - """ + """Reject URLs that would expose credentials (bad scheme or plaintext-to-remote).""" parsed = urlparse(url) scheme = parsed.scheme.lower() host = (parsed.hostname or "").lower() diff --git a/tests/test_async_client.py b/tests/test_async_client.py index 12ef30c..65b0c75 100644 --- a/tests/test_async_client.py +++ b/tests/test_async_client.py @@ -71,6 +71,31 @@ async def test_get_retries_on_429(self, transport: AsyncTransport) -> None: assert resp.status_code == 200 assert route.call_count == 2 + @respx.mock + @pytest.mark.asyncio + async def test_429_retry_after_zero_is_honored_end_to_end( + self, transport: AsyncTransport, monkeypatch: pytest.MonkeyPatch + ) -> None: + # Async counterpart of the sync regression: `is not None` keeps Retry-After: 0. + sleeps: list[float] = [] + + async def fake_sleep(d: float) -> None: + sleeps.append(d) + + # _base_client imports asyncio inside the method, so patch the module attr directly. + monkeypatch.setattr("asyncio.sleep", fake_sleep) + respx.get("https://test.kalshi.com/trade-api/v2/markets").mock( + side_effect=[ + httpx.Response(429, headers={"Retry-After": "0"}, json={"message": "rl"}), + httpx.Response(200, json={"markets": []}), + ] + ) + resp = await transport.request("GET", "/markets") + assert resp.status_code == 200 + assert sleeps == [0.0], ( + f"Expected one sleep of 0.0s honoring Retry-After: 0, got {sleeps!r}" + ) + @respx.mock @pytest.mark.asyncio async def test_get_retries_on_500(self, transport: AsyncTransport) -> None: diff --git a/tests/test_client.py b/tests/test_client.py index 804e630..49f3995 100644 --- a/tests/test_client.py +++ b/tests/test_client.py @@ -180,6 +180,26 @@ def test_get_retries_on_429(self, transport: SyncTransport) -> None: assert resp.status_code == 200 assert route.call_count == 2 + @respx.mock + def test_429_retry_after_zero_is_honored_end_to_end( + self, transport: SyncTransport, monkeypatch: pytest.MonkeyPatch + ) -> None: + # Regression: `if error.retry_after:` would drop 0 ("retry immediately") as falsy. + # `is not None` keeps it; assert the transport actually sleeps 0, not the backoff fallback. + sleeps: list[float] = [] + monkeypatch.setattr("kalshi._base_client.time.sleep", lambda d: sleeps.append(d)) + respx.get("https://test.kalshi.com/trade-api/v2/markets").mock( + side_effect=[ + httpx.Response(429, headers={"Retry-After": "0"}, json={"message": "rl"}), + httpx.Response(200, json={"markets": []}), + ] + ) + resp = transport.request("GET", "/markets") + assert resp.status_code == 200 + assert sleeps == [0.0], ( + f"Expected one sleep of 0.0s honoring Retry-After: 0, got {sleeps!r}" + ) + @respx.mock def test_get_retries_on_500(self, transport: SyncTransport) -> None: """500s are transient on GETs; demo routinely returns them mid-paginate."""