diff --git a/kalshi/_base_client.py b/kalshi/_base_client.py index e3be8274..bc5497a8 100644 --- a/kalshi/_base_client.py +++ b/kalshi/_base_client.py @@ -18,6 +18,7 @@ import httpx +from kalshi._constants import AUTH_HEADER_PREFIX from kalshi.auth import KalshiAuth from kalshi.config import KalshiConfig from kalshi.errors import ( @@ -161,6 +162,47 @@ def _would_exceed_budget(start: float, delay: float, total_timeout: float | None return (time.monotonic() - start) + delay > total_timeout + +def _assert_no_auth_headers(h: dict[str, str] | None) -> None: + """Reject caller-supplied ``KALSHI-ACCESS-*`` keys (case-insensitive). + + #298: auth headers are SDK-managed (RSA-PSS signed per attempt) and + must not be set via ``extra_headers``. No-op if no leak is found. + """ + if not h: + return + leaked = [k for k in h if k.lower().startswith(AUTH_HEADER_PREFIX)] + if leaked: + raise ValueError( + f"extra_headers must not include KALSHI-ACCESS-* keys " + f"(got: {sorted(leaked)!r}). These are reserved for " + f"SDK-managed RSA-PSS signing and cannot be overridden." + ) + + +def _ci_merge(*layers: dict[str, str] | None) -> dict[str, str]: + """Case-insensitive, last-layer-wins header merge. + + #298: plain ``dict`` unpacking is case-sensitive, so case-mismatched + keys co-exist on the wire (httpx ships both raw header lines). This + merge drops the earlier-case key when a later layer supplies the same + lowercase form, so the rightmost layer cleanly wins. + """ + out: dict[str, str] = {} + lower_to_canonical: dict[str, str] = {} + for layer in layers: + if not layer: + continue + for k, v in layer.items(): + lk = k.lower() + canonical = lower_to_canonical.get(lk) + if canonical is not None and canonical != k: + out.pop(canonical, None) + lower_to_canonical[lk] = k + out[k] = v + return out + + class SyncTransport: """Synchronous HTTP transport using httpx.Client.""" @@ -209,6 +251,10 @@ def request( client-default one), but the ``KALSHI-ACCESS-*`` auth headers always win — callers cannot forge them via ``extra_headers``. """ + # #298: reject KALSHI-ACCESS-* in caller-supplied extra_headers + # before any merging — auth headers are SDK-signed per attempt and + # the only safe behaviour is a hard error at the boundary. + _assert_no_auth_headers(extra_headers) if json is not None and content is not None: raise TypeError("request() accepts `json=` or `content=`, not both.") # P1.6: canonicalize trailing slash BEFORE both signing and httpx call @@ -227,14 +273,20 @@ def request( # timestamp). Merge the invariant base once outside the loop. # Order matters: config defaults < per-request overrides # < body-helper headers < signed auth. - base_headers: dict[str, str] = { - **(self._config.extra_headers or {}), - **(extra_headers or {}), - **(headers or {}), - } + # #298: case-insensitive merge — plain dict unpacking would leave + # case-mismatched duplicates (e.g. user 'content-type' beside SDK + # 'Content-Type'), and httpx 0.28 ships both raw header lines. + base_headers: dict[str, str] = _ci_merge( + self._config.extra_headers, + extra_headers, + headers, + ) for attempt in range(self._config.max_retries + 1): auth_headers = self._auth.sign_request(method.upper(), sign_path) if self._auth else {} + # auth_headers keys are case-canonical (always uppercase + # KALSHI-ACCESS-*) and base_headers is already case-deduped by + # _ci_merge above, so a plain dict-unpack is safe here (#298). merged_headers = {**base_headers, **auth_headers} logger.debug( @@ -408,6 +460,10 @@ async def request( See :meth:`SyncTransport.request` for ``extra_headers`` semantics. """ + # #298: reject KALSHI-ACCESS-* in caller-supplied extra_headers + # before any merging — auth headers are SDK-signed per attempt and + # the only safe behaviour is a hard error at the boundary. + _assert_no_auth_headers(extra_headers) if json is not None and content is not None: raise TypeError("request() accepts `json=` or `content=`, not both.") @@ -424,17 +480,23 @@ async def request( # out of the retry loop; only auth_headers changes per attempt. # Order matters: config defaults < per-request overrides # < body-helper headers < signed auth. - base_headers: dict[str, str] = { - **(self._config.extra_headers or {}), - **(extra_headers or {}), - **(headers or {}), - } + # #298: case-insensitive merge — plain dict unpacking would leave + # case-mismatched duplicates (e.g. user 'content-type' beside SDK + # 'Content-Type'), and httpx 0.28 ships both raw header lines. + base_headers: dict[str, str] = _ci_merge( + self._config.extra_headers, + extra_headers, + headers, + ) for attempt in range(self._config.max_retries + 1): if self._auth: auth_headers = await self._auth.sign_request_async(method.upper(), sign_path) else: auth_headers = {} + # auth_headers keys are case-canonical (always uppercase + # KALSHI-ACCESS-*) and base_headers is already case-deduped by + # _ci_merge above, so a plain dict-unpack is safe here (#298). merged_headers = {**base_headers, **auth_headers} logger.debug( diff --git a/kalshi/_constants.py b/kalshi/_constants.py new file mode 100644 index 00000000..8ffdc957 --- /dev/null +++ b/kalshi/_constants.py @@ -0,0 +1,5 @@ +"""Internal constants shared across modules to avoid circular imports.""" +from __future__ import annotations + +AUTH_HEADER_PREFIX = "kalshi-access-" +"""All RSA-PSS auth headers start with this prefix (case-insensitive).""" diff --git a/kalshi/config.py b/kalshi/config.py index a590024b..817f0bf7 100644 --- a/kalshi/config.py +++ b/kalshi/config.py @@ -11,6 +11,8 @@ import httpx +from kalshi._constants import AUTH_HEADER_PREFIX + PRODUCTION_BASE_URL = "https://api.elections.kalshi.com/trade-api/v2" DEMO_BASE_URL = "https://demo-api.kalshi.co/trade-api/v2" @@ -133,6 +135,22 @@ def __post_init__(self) -> None: "KalshiConfig.production() / KalshiConfig.demo(), or pass both " "base_url and ws_base_url explicitly." ) + # #298 follow-up: bot review flagged that config.extra_headers + # bypasses the per-request _assert_no_auth_headers check, so a caller + # could still seed KALSHI-ACCESS-* on the httpx.Client default headers + # and forge the auth surface. Validate at construction. The prefix + # lives in kalshi._constants (no imports from either file) to avoid + # the circular hazard with kalshi._base_client. + if self.extra_headers: + leaked = sorted( + k for k in self.extra_headers if k.lower().startswith(AUTH_HEADER_PREFIX) + ) + if leaked: + raise ValueError( + f"KalshiConfig.extra_headers must not include KALSHI-ACCESS-* " + f"keys (got: {leaked!r}). These are reserved for SDK-managed " + f"RSA-PSS signing." + ) if self.http2: import importlib.util diff --git a/kalshi/resources/_base.py b/kalshi/resources/_base.py index 31ff8bb3..b7199474 100644 --- a/kalshi/resources/_base.py +++ b/kalshi/resources/_base.py @@ -179,6 +179,9 @@ def _post( path, params=params, json=json, + # #298: pin Content-Type so a caller-supplied (mismatched-case) + # `content-type` cannot suppress httpx's JSON inference. + headers={"Content-Type": "application/json"}, extra_headers=extra_headers, ) result: dict[str, Any] = self._load_json(response) @@ -222,6 +225,9 @@ def _put( path, params=params, json=json, + # #298: pin Content-Type so a caller-supplied (mismatched-case) + # `content-type` cannot suppress httpx's JSON inference. + headers={"Content-Type": "application/json"}, extra_headers=extra_headers, ) if response.status_code == 204: @@ -258,6 +264,7 @@ def _delete_with_body( "DELETE", path, json=json, + headers={"Content-Type": "application/json"}, extra_headers=extra_headers, ) if response.status_code == 204: @@ -418,6 +425,9 @@ async def _post( path, params=params, json=json, + # #298: pin Content-Type so a caller-supplied (mismatched-case) + # `content-type` cannot suppress httpx's JSON inference. + headers={"Content-Type": "application/json"}, extra_headers=extra_headers, ) result: dict[str, Any] = self._load_json(response) @@ -456,6 +466,9 @@ async def _put( path, params=params, json=json, + # #298: pin Content-Type so a caller-supplied (mismatched-case) + # `content-type` cannot suppress httpx's JSON inference. + headers={"Content-Type": "application/json"}, extra_headers=extra_headers, ) if response.status_code == 204: @@ -492,6 +505,7 @@ async def _delete_with_body( "DELETE", path, json=json, + headers={"Content-Type": "application/json"}, extra_headers=extra_headers, ) if response.status_code == 204: diff --git a/tests/test_client.py b/tests/test_client.py index aa177211..d8a7846a 100644 --- a/tests/test_client.py +++ b/tests/test_client.py @@ -1348,41 +1348,40 @@ def handler(request: httpx.Request) -> httpx.Response: transport.close() @respx.mock - def test_extra_headers_cannot_forge_KALSHI_ACCESS_headers( # noqa: N802 + def test_extra_headers_rejects_KALSHI_ACCESS_headers( # noqa: N802 self, test_auth: KalshiAuth ) -> None: - """Per-call extra_headers MUST NOT override the SDK's signed auth headers.""" + """#298: per-call extra_headers with KALSHI-ACCESS-* keys raise ValueError. + + Previously the SDK silently overrode them; the case-mismatched variant + still leaked on the wire. The fix rejects any kalshi-access-* key at + the public boundary, regardless of case. + """ config = KalshiConfig( base_url="https://test.kalshi.com/trade-api/v2", timeout=5.0, ) transport = SyncTransport(test_auth, config) - captured: list[httpx.Request] = [] - - def handler(request: httpx.Request) -> httpx.Response: - captured.append(request) - return httpx.Response(200, json={}) - - respx.get("https://test.kalshi.com/trade-api/v2/markets").mock(side_effect=handler) - resp = transport.request( - "GET", - "/markets", - extra_headers={ - "KALSHI-ACCESS-KEY": "attacker-key", - "KALSHI-ACCESS-SIGNATURE": "AAAAAA==", - "KALSHI-ACCESS-TIMESTAMP": "1", - "X-Trace": "ok", - }, - ) - assert resp.status_code == 200 - sent = captured[0] - # The signed key (from test_auth fixture) wins, not the forged one. - assert sent.headers["KALSHI-ACCESS-KEY"] == "test-key-id" - # Signature/timestamp were re-signed by the SDK, not "AAAAAA=="/"1". - assert sent.headers["KALSHI-ACCESS-SIGNATURE"] != "AAAAAA==" - assert sent.headers["KALSHI-ACCESS-TIMESTAMP"] != "1" - # Non-auth headers still pass through. - assert sent.headers["X-Trace"] == "ok" + # No respx route: ValueError fires before any request is sent. + # @respx.mock decorator still sandboxes any accidental real call. + with pytest.raises(ValueError, match="KALSHI-ACCESS-"): + transport.request( + "GET", + "/markets", + extra_headers={ + "KALSHI-ACCESS-KEY": "attacker-key", + "KALSHI-ACCESS-SIGNATURE": "AAAAAA==", + "KALSHI-ACCESS-TIMESTAMP": "1", + "X-Trace": "ok", + }, + ) + # Case-mismatched lowercase variant is also rejected. + with pytest.raises(ValueError, match="kalshi-access-"): + transport.request( + "GET", + "/markets", + extra_headers={"kalshi-access-key": "attacker-key"}, + ) transport.close() diff --git a/tests/test_config.py b/tests/test_config.py index f2e40b35..9d9c89e2 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -176,6 +176,30 @@ def test_extra_headers_defaults_to_empty_dict(self) -> None: config_a.extra_headers["X-Trace-Id"] = "a" assert config_b.extra_headers == {} + def test_extra_headers_rejects_kalshi_access_uppercase(self) -> None: + # #298 follow-up: config.extra_headers must not be a back door for the + # SDK-signed KALSHI-ACCESS-* surface. The per-request guard is not + # enough on its own — the config path bypassed it. + with pytest.raises(ValueError, match="KALSHI-ACCESS-"): + KalshiConfig(extra_headers={"KALSHI-ACCESS-KEY": "spoofed"}) + + def test_extra_headers_rejects_kalshi_access_lowercase(self) -> None: + # Case-mismatched key (the forge surface that motivated the per-call + # _ci_merge fix) must also fail at construction. + with pytest.raises(ValueError, match=r"kalshi-access-key"): + KalshiConfig(extra_headers={"kalshi-access-key": "x"}) + + def test_extra_headers_rejects_kalshi_access_signature_and_timestamp(self) -> None: + # Any KALSHI-ACCESS-* prefix is rejected, not just KALSHI-ACCESS-KEY. + for header in ("KALSHI-ACCESS-SIGNATURE", "KALSHI-ACCESS-TIMESTAMP"): + with pytest.raises(ValueError, match="KALSHI-ACCESS-"): + KalshiConfig(extra_headers={header: "v"}) + + def test_extra_headers_with_non_auth_keys_ok(self) -> None: + # Sanity: regular custom headers still work. + cfg = KalshiConfig(extra_headers={"X-Trace-Id": "a", "User-Agent": "sdk"}) + assert cfg.extra_headers == {"X-Trace-Id": "a", "User-Agent": "sdk"} + class TestHttpClientTuning: """http2 and limits are exposed on KalshiConfig and forwarded to httpx.""" diff --git a/tests/test_extra_headers_plumbing.py b/tests/test_extra_headers_plumbing.py index 81792707..1a35da87 100644 --- a/tests/test_extra_headers_plumbing.py +++ b/tests/test_extra_headers_plumbing.py @@ -65,11 +65,16 @@ def test_create_order_threads_extra_headers(self, test_auth: KalshiAuth) -> None assert route.calls[0].request.headers["Idempotency-Key"] == "abc" @respx.mock - def test_create_order_extra_headers_cannot_override_auth(self, test_auth: KalshiAuth) -> None: - route = respx.post(f"{MOCK_BASE}/portfolio/orders").mock( + def test_create_order_extra_headers_rejects_kalshi_access(self, test_auth: KalshiAuth) -> None: + # #298: caller-supplied KALSHI-ACCESS-* keys now raise instead of + # silently leaking onto the wire alongside the SDK-signed pair. + respx.post(f"{MOCK_BASE}/portfolio/orders").mock( return_value=httpx.Response(200, json={"order": _example_order_payload()}) ) - with KalshiClient(auth=test_auth, config=_config()) as client: + with ( + KalshiClient(auth=test_auth, config=_config()) as client, + pytest.raises(ValueError, match="KALSHI-ACCESS-"), + ): client.orders.create( ticker="BTC", side="yes", @@ -83,10 +88,6 @@ def test_create_order_extra_headers_cannot_override_auth(self, test_auth: Kalshi "KALSHI-ACCESS-TIMESTAMP": "0", }, ) - req = route.calls[0].request - assert req.headers["KALSHI-ACCESS-KEY"] == "test-key-id" - assert req.headers["KALSHI-ACCESS-SIGNATURE"] != "spoofed-sig" - assert req.headers["KALSHI-ACCESS-TIMESTAMP"] != "0" @respx.mock def test_cancel_order_threads_extra_headers(self, test_auth: KalshiAuth) -> None: @@ -109,13 +110,15 @@ def test_get_market_threads_extra_headers(self, test_auth: KalshiAuth) -> None: assert route.calls[0].request.headers["X-Test"] == "v" @respx.mock - def test_get_market_extra_headers_cannot_override_auth(self, test_auth: KalshiAuth) -> None: - route = respx.get(f"{MOCK_BASE}/markets/BTC").mock( + def test_get_market_extra_headers_rejects_kalshi_access(self, test_auth: KalshiAuth) -> None: + respx.get(f"{MOCK_BASE}/markets/BTC").mock( return_value=httpx.Response(200, json={"market": _example_market_payload()}) ) - with KalshiClient(auth=test_auth, config=_config()) as client: + with ( + KalshiClient(auth=test_auth, config=_config()) as client, + pytest.raises(ValueError, match="KALSHI-ACCESS-"), + ): client.markets.get("BTC", extra_headers={"KALSHI-ACCESS-KEY": "spoofed"}) - assert route.calls[0].request.headers["KALSHI-ACCESS-KEY"] == "test-key-id" class TestPortfolio: @@ -138,8 +141,8 @@ def test_get_balance_threads_extra_headers(self, test_auth: KalshiAuth) -> None: assert route.calls[0].request.headers["X-Test"] == "v" @respx.mock - def test_balance_extra_headers_cannot_override_auth(self, test_auth: KalshiAuth) -> None: - route = respx.get(f"{MOCK_BASE}/portfolio/balance").mock( + def test_balance_extra_headers_rejects_kalshi_access(self, test_auth: KalshiAuth) -> None: + respx.get(f"{MOCK_BASE}/portfolio/balance").mock( return_value=httpx.Response( 200, json={ @@ -151,9 +154,126 @@ def test_balance_extra_headers_cannot_override_auth(self, test_auth: KalshiAuth) }, ) ) - with KalshiClient(auth=test_auth, config=_config()) as client: + with ( + KalshiClient(auth=test_auth, config=_config()) as client, + pytest.raises(ValueError, match="KALSHI-ACCESS-"), + ): client.portfolio.balance(extra_headers={"KALSHI-ACCESS-KEY": "spoofed"}) - assert route.calls[0].request.headers["KALSHI-ACCESS-KEY"] == "test-key-id" + + +# --------------------------------------------------------------------------- +# #298: KALSHI-ACCESS-* rejection, Content-Type pinning, header round-trip. +# --------------------------------------------------------------------------- + + +class TestHeaderCollision298: + @respx.mock + def test_extra_headers_lowercase_kalshi_access_raises( + self, test_auth: KalshiAuth + ) -> None: + # Case-mismatched key (lowercase) was previously a forge surface; the + # public boundary now raises ValueError naming the leaked key. + respx.post(f"{MOCK_BASE}/portfolio/orders").mock( + return_value=httpx.Response(200, json={"order": _example_order_payload()}) + ) + with ( + KalshiClient(auth=test_auth, config=_config()) as client, + pytest.raises(ValueError, match="kalshi-access-key"), + ): + client.orders.create( + ticker="BTC", + side="yes", + action="buy", + client_order_id="cli-1", + count=1, + yes_price=50, + extra_headers={"kalshi-access-key": "X"}, + ) + + @respx.mock + def test_post_json_body_pins_content_type_against_caller_override( + self, test_auth: KalshiAuth + ) -> None: + # Caller-supplied lowercase ``content-type`` must NOT win over the + # JSON body's content-type, which the SDK pins at the body-helper + # layer in resources/_base.py:_post. + route = respx.post(f"{MOCK_BASE}/portfolio/orders").mock( + return_value=httpx.Response(200, json={"order": _example_order_payload()}) + ) + with KalshiClient(auth=test_auth, config=_config()) as client: + client.orders.create( + ticker="BTC", + side="yes", + action="buy", + client_order_id="cli-1", + count=1, + yes_price=50, + extra_headers={"content-type": "text/plain"}, + ) + wire = route.calls.last.request.headers + assert wire["content-type"].startswith("application/json") + # And only one Content-Type line went on the wire (case-insensitive). + ct_lines = [k for k, _ in wire.raw if k.lower() == b"content-type"] + assert len(ct_lines) == 1, ct_lines + + @respx.mock + def test_extra_headers_non_auth_round_trips_unchanged( + self, test_auth: KalshiAuth + ) -> None: + route = respx.get(f"{MOCK_BASE}/markets/BTC").mock( + return_value=httpx.Response(200, json={"market": _example_market_payload()}) + ) + with KalshiClient(auth=test_auth, config=_config()) as client: + client.markets.get("BTC", extra_headers={"x-request-id": "abc"}) + wire = route.calls.last.request.headers + assert wire["x-request-id"] == "abc" + # And it appeared exactly once. + rid_lines = [k for k, _ in wire.raw if k.lower() == b"x-request-id"] + assert len(rid_lines) == 1, rid_lines + + @respx.mock + def test_delete_with_body_sync_pins_content_type(self, test_auth: KalshiAuth) -> None: + # #298 follow-up: bot review flagged that _delete_with_body (sync) + # left Content-Type unspecified, so a stale config.extra_headers or + # extra_headers={"content-type": "text/plain"} could ride on the wire + # alongside a JSON body. Mirror the _post/_put guarantee. + from kalshi.models.orders import BatchCancelOrdersV2Request + + route = respx.delete( + f"{MOCK_BASE}/portfolio/events/orders/batched" + ).mock(return_value=httpx.Response(200, json={"orders": []})) + with KalshiClient(auth=test_auth, config=_config()) as client: + client.orders.batch_cancel_v2( + request=BatchCancelOrdersV2Request(orders=[]), + extra_headers={"content-type": "text/plain"}, + ) + wire = route.calls.last.request.headers + assert wire["content-type"].startswith("application/json") + ct_lines = [k for k, _ in wire.raw if k.lower() == b"content-type"] + assert len(ct_lines) == 1, ct_lines + + async def test_delete_with_body_async_pins_content_type( + self, test_auth: KalshiAuth + ) -> None: + # Async mirror of the sync wire-assertion above. + import respx as _respx + + from kalshi import AsyncKalshiClient + from kalshi.models.orders import BatchCancelOrdersV2Request + + with _respx.mock() as router: + route = router.delete( + f"{MOCK_BASE}/portfolio/events/orders/batched" + ).mock(return_value=httpx.Response(200, json={"orders": []})) + async with AsyncKalshiClient(auth=test_auth, config=_config()) as client: + await client.orders.batch_cancel_v2( + request=BatchCancelOrdersV2Request(orders=[]), + extra_headers={"content-type": "text/plain"}, + ) + wire = route.calls.last.request.headers + assert wire["content-type"].startswith("application/json") + ct_lines = [k for k, _ in wire.raw if k.lower() == b"content-type"] + assert len(ct_lines) == 1, ct_lines # ---------------------------------------------------------------------------