Skip to content

Commit 704dfb4

Browse files
TexasCodingclaude
andauthored
fix: Order.type rename, BASE_URL validation, Retry-After NaN/negative guard (#112)
* fix(orders, events, series): rename Order.type, normalize bool query params 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) <noreply@anthropic.com> * fix(config): validate KALSHI_API_BASE_URL scheme and host 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://<remote host>: ValueError at config construction. - non-http(s) scheme or missing host: ValueError. Closes #94 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * 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) <noreply@anthropic.com> * 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) <noreply@anthropic.com> * 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) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent ca0d125 commit 704dfb4

11 files changed

Lines changed: 384 additions & 16 deletions

File tree

CHANGELOG.md

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,30 @@
22

33
All notable changes to kalshi-sdk will be documented in this file.
44

5+
## [Unreleased]
6+
7+
### Breaking
8+
9+
- **`Order.type` renamed to `Order.order_type`.** Wire format is unchanged
10+
(`validation_alias=AliasChoices("type", "order_type")` accepts both names on
11+
deserialization), but any user code reading `.type` on an `Order` instance
12+
must migrate to `.order_type`. Rationale: matches the project's existing
13+
builtin-shadow-avoidance convention (`milestone_type`, `target_type`,
14+
`incentive_type`). Spec v3.13.0 still defines `type` as required, so the
15+
field is preserved on the wire — only the Python attribute name changed
16+
(#91).
17+
18+
```python
19+
# Before
20+
order = client.portfolio.orders.get(order_id="...")
21+
print(order.type) # AttributeError after upgrade
22+
23+
# After
24+
print(order.order_type)
25+
```
26+
27+
Version-bump decision (v1.2 vs v2.0) deferred to release cut.
28+
529
## 1.1.0 — 2026-05-16
630

731
Post-1.0 enhancements and polish. 17 issues closed across four parallel waves of

kalshi/_base_client.py

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
from __future__ import annotations
88

99
import logging
10+
import math
1011
import random
1112
import time
1213
from typing import Any
@@ -59,6 +60,9 @@ def _map_error(response: httpx.Response) -> KalshiError:
5960
if retry_after:
6061
try:
6162
retry_after_val = float(retry_after)
63+
# Reject non-finite/negative: NaN crashes time.sleep, negative busy-loops.
64+
if retry_after_val < 0 or not math.isfinite(retry_after_val):
65+
retry_after_val = None
6266
except ValueError:
6367
retry_after_val = None # HTTP-date format, fall back to computed backoff
6468
return KalshiRateLimitError(
@@ -168,8 +172,12 @@ def request(
168172
if not should_retry:
169173
raise error
170174

171-
# Use Retry-After header if available for 429
172-
if isinstance(error, KalshiRateLimitError) and error.retry_after:
175+
# Use Retry-After header if available for 429.
176+
# `is not None` — not truthy — so Retry-After: 0 ("retry immediately") is honored.
177+
if (
178+
isinstance(error, KalshiRateLimitError)
179+
and error.retry_after is not None
180+
):
173181
delay = min(error.retry_after, self._config.retry_max_delay)
174182
else:
175183
delay = _compute_backoff(attempt, self._config)
@@ -282,7 +290,11 @@ async def request(
282290
if not should_retry:
283291
raise error
284292

285-
if isinstance(error, KalshiRateLimitError) and error.retry_after:
293+
# `is not None` so Retry-After: 0 ("retry immediately") is honored.
294+
if (
295+
isinstance(error, KalshiRateLimitError)
296+
and error.retry_after is not None
297+
):
286298
delay = min(error.retry_after, self._config.retry_max_delay)
287299
else:
288300
delay = _compute_backoff(attempt, self._config)

kalshi/config.py

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,9 @@
22

33
from __future__ import annotations
44

5+
import logging
56
from dataclasses import dataclass, field
7+
from urllib.parse import urlparse
68

79
PRODUCTION_BASE_URL = "https://api.elections.kalshi.com/trade-api/v2"
810
DEMO_BASE_URL = "https://demo-api.kalshi.co/trade-api/v2"
@@ -14,6 +16,13 @@
1416
DEFAULT_MAX_RETRIES = 3
1517
DEFAULT_WS_MAX_RETRIES = 10
1618

19+
_KNOWN_HOSTS = frozenset(
20+
{"api.elections.kalshi.com", "demo-api.kalshi.co"}
21+
)
22+
_LOCAL_HOSTS = frozenset({"localhost", "127.0.0.1", "::1"})
23+
24+
logger = logging.getLogger("kalshi")
25+
1726

1827
@dataclass(frozen=True)
1928
class KalshiConfig:
@@ -42,6 +51,41 @@ def __post_init__(self) -> None:
4251
object.__setattr__(self, "base_url", self.base_url.rstrip("/"))
4352
if self.ws_base_url.endswith("/"):
4453
object.__setattr__(self, "ws_base_url", self.ws_base_url.rstrip("/"))
54+
KalshiConfig._validate_url(self.base_url, "base_url", secure="https", plaintext="http")
55+
KalshiConfig._validate_url(self.ws_base_url, "ws_base_url", secure="wss", plaintext="ws")
56+
57+
@staticmethod
58+
def _validate_url(url: str, field_name: str, *, secure: str, plaintext: str) -> None:
59+
"""Reject URLs that would expose credentials (bad scheme or plaintext-to-remote)."""
60+
parsed = urlparse(url)
61+
scheme = parsed.scheme.lower()
62+
host = (parsed.hostname or "").lower()
63+
64+
if scheme not in (secure, plaintext):
65+
raise ValueError(
66+
f"KalshiConfig.{field_name} must use {secure}:// or {plaintext}://, "
67+
f"got scheme={scheme!r} (url={url!r})"
68+
)
69+
if not host:
70+
raise ValueError(
71+
f"KalshiConfig.{field_name} is missing a host: {url!r}"
72+
)
73+
if scheme == plaintext and host not in _LOCAL_HOSTS:
74+
raise ValueError(
75+
f"KalshiConfig.{field_name} must use {secure}:// for non-loopback "
76+
f"hosts; {plaintext}:// is only allowed for {sorted(_LOCAL_HOSTS)} "
77+
f"(url={url!r}). Plaintext to a remote host would "
78+
f"expose the KALSHI-ACCESS-KEY header and request signature."
79+
)
80+
if host not in _KNOWN_HOSTS and host not in _LOCAL_HOSTS:
81+
logger.warning(
82+
"KalshiConfig.%s host %r is not a known Kalshi "
83+
"endpoint (%s). Requests will be signed and sent there "
84+
"with your API key — verify this is intentional.",
85+
field_name,
86+
host,
87+
sorted(_KNOWN_HOSTS),
88+
)
4589

4690
@classmethod
4791
def production(cls, **kwargs: object) -> KalshiConfig:

kalshi/models/orders.py

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,14 @@ class Order(BaseModel):
4444
status: str | None = None
4545
side: str | None = None
4646
is_yes: bool | None = None
47-
type: str | None = None
47+
# Spec field is named ``type`` (enum: limit, market). Renamed to
48+
# ``order_type`` on the SDK side to avoid shadowing the Python builtin —
49+
# same rationale as milestone_type / target_type / incentive_type
50+
# elsewhere. The wire still sends ``type``; validation alias accepts both.
51+
order_type: str | None = Field(
52+
default=None,
53+
validation_alias=AliasChoices("type", "order_type"),
54+
)
4855
yes_price: DollarDecimal | None = Field(
4956
default=None,
5057
validation_alias=AliasChoices("yes_price_dollars", "yes_price"),

kalshi/resources/events.py

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77

88
from kalshi.models.common import Page
99
from kalshi.models.events import Event, EventMetadata, EventStatusLiteral
10-
from kalshi.resources._base import AsyncResource, SyncResource, _params
10+
from kalshi.resources._base import AsyncResource, SyncResource, _bool_param, _params
1111

1212
# Shared param builders (issue #46).
1313

@@ -26,8 +26,8 @@ def _list_events_params(
2626
return _params(
2727
status=status,
2828
series_ticker=series_ticker,
29-
with_nested_markets="true" if with_nested_markets else None,
30-
with_milestones="true" if with_milestones else None,
29+
with_nested_markets=_bool_param(with_nested_markets),
30+
with_milestones=_bool_param(with_milestones),
3131
min_close_ts=min_close_ts,
3232
min_updated_ts=min_updated_ts,
3333
limit=limit,
@@ -46,7 +46,7 @@ def _list_multivariate_events_params(
4646
return _params(
4747
series_ticker=series_ticker,
4848
collection_ticker=collection_ticker,
49-
with_nested_markets="true" if with_nested_markets else None,
49+
with_nested_markets=_bool_param(with_nested_markets),
5050
limit=limit,
5151
cursor=cursor,
5252
)
@@ -136,7 +136,7 @@ def get(
136136
with_nested_markets: bool = False,
137137
) -> Event:
138138
params = _params(
139-
with_nested_markets="true" if with_nested_markets else None,
139+
with_nested_markets=_bool_param(with_nested_markets),
140140
)
141141
data = self._get(f"/events/{event_ticker}", params=params)
142142
return Event.model_validate(data.get("event", data))
@@ -230,7 +230,7 @@ async def get(
230230
with_nested_markets: bool = False,
231231
) -> Event:
232232
params = _params(
233-
with_nested_markets="true" if with_nested_markets else None,
233+
with_nested_markets=_bool_param(with_nested_markets),
234234
)
235235
data = await self._get(f"/events/{event_ticker}", params=params)
236236
return Event.model_validate(data.get("event", data))

kalshi/resources/series.py

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@
1111
Series,
1212
SeriesFeeChange,
1313
)
14-
from kalshi.resources._base import AsyncResource, SyncResource, _params
14+
from kalshi.resources._base import AsyncResource, SyncResource, _bool_param, _params
1515

1616
# Shared param builders (issue #46).
1717

@@ -27,8 +27,8 @@ def _list_series_params(
2727
return _params(
2828
category=category,
2929
tags=tags,
30-
include_product_metadata="true" if include_product_metadata else None,
31-
include_volume="true" if include_volume else None,
30+
include_product_metadata=_bool_param(include_product_metadata),
31+
include_volume=_bool_param(include_volume),
3232
min_updated_ts=min_updated_ts,
3333
)
3434

@@ -40,7 +40,7 @@ def _fee_changes_params(
4040
) -> dict[str, Any]:
4141
return _params(
4242
series_ticker=series_ticker,
43-
show_historical="true" if show_historical else None,
43+
show_historical=_bool_param(show_historical),
4444
)
4545

4646

@@ -98,7 +98,7 @@ def get(
9898
include_volume: bool | None = None,
9999
) -> Series:
100100
params = _params(
101-
include_volume="true" if include_volume else None,
101+
include_volume=_bool_param(include_volume),
102102
)
103103
data = self._get(f"/series/{series_ticker}", params=params)
104104
return Series.model_validate(data.get("series", data))
@@ -188,7 +188,7 @@ async def get(
188188
include_volume: bool | None = None,
189189
) -> Series:
190190
params = _params(
191-
include_volume="true" if include_volume else None,
191+
include_volume=_bool_param(include_volume),
192192
)
193193
data = await self._get(f"/series/{series_ticker}", params=params)
194194
return Series.model_validate(data.get("series", data))

tests/test_async_client.py

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,31 @@ async def test_get_retries_on_429(self, transport: AsyncTransport) -> None:
7171
assert resp.status_code == 200
7272
assert route.call_count == 2
7373

74+
@respx.mock
75+
@pytest.mark.asyncio
76+
async def test_429_retry_after_zero_is_honored_end_to_end(
77+
self, transport: AsyncTransport, monkeypatch: pytest.MonkeyPatch
78+
) -> None:
79+
# Async counterpart of the sync regression: `is not None` keeps Retry-After: 0.
80+
sleeps: list[float] = []
81+
82+
async def fake_sleep(d: float) -> None:
83+
sleeps.append(d)
84+
85+
# _base_client imports asyncio inside the method, so patch the module attr directly.
86+
monkeypatch.setattr("asyncio.sleep", fake_sleep)
87+
respx.get("https://test.kalshi.com/trade-api/v2/markets").mock(
88+
side_effect=[
89+
httpx.Response(429, headers={"Retry-After": "0"}, json={"message": "rl"}),
90+
httpx.Response(200, json={"markets": []}),
91+
]
92+
)
93+
resp = await transport.request("GET", "/markets")
94+
assert resp.status_code == 200
95+
assert sleeps == [0.0], (
96+
f"Expected one sleep of 0.0s honoring Retry-After: 0, got {sleeps!r}"
97+
)
98+
7499
@respx.mock
75100
@pytest.mark.asyncio
76101
async def test_get_retries_on_500(self, transport: AsyncTransport) -> None:

0 commit comments

Comments
 (0)