From 91f059bead8cd303fc96b5cbc5b8170eb5924832 Mon Sep 17 00:00:00 2001 From: kalshi-sdk-bot Date: Thu, 21 May 2026 18:27:06 -0500 Subject: [PATCH 1/3] fix(transport): reject KALSHI-ACCESS-* in extra_headers; case-insensitive merge; explicit Content-Type on JSON body helpers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two wire-level header bugs surfaced in the post-v2.5 audit: * The per-request header merge in SyncTransport.request / AsyncTransport.request used plain dict-unpacking, which is case-sensitive on keys. A caller-supplied 'kalshi-access-key' (lowercase) co-existed with the SDK-signed 'KALSHI-ACCESS-KEY' and httpx 0.28 shipped both raw header lines. RFC 7230 leaves multi-instance handling implementation-defined, so a tolerant intermediary could observe the user-supplied value — a forge surface even though the documented contract promises auth headers are SDK-managed. * _post(json=...) / _put(json=...) (and async mirrors) relied on httpx's implicit Content-Type inference. httpx only infers when no Content-Type is present, so extra_headers={'content-type': 'text/plain'} shipped a JSON body labelled text/plain. _post_json / _delete_with_body_json already pinned Content-Type explicitly; the inferring siblings now match that pattern. Fix: * New module-level helpers in kalshi/_base_client.py: - _AUTH_HEADER_PREFIX constant, _strip_auth_headers() raises ValueError naming the leaked key when extra_headers contains any 'kalshi-access-*' key (case-insensitive). Called at the top of both request methods, before any merging. - _ci_merge(): last-layer-wins, case-insensitive merge that drops the earlier-case key when a later layer supplies the same lowercase form. Replaces the {**a, **b, **c} construction for base_headers in both SyncTransport.request and AsyncTransport.request. * kalshi/resources/_base.py: _post, _put, async _post, async _put now pass headers={'Content-Type': 'application/json'} alongside json=..., matching the existing _post_json / _delete_with_body_json pattern. The per-attempt {**base_headers, **auth_headers} merge stays a plain dict-unpack: auth keys are case-canonical (always uppercase KALSHI-ACCESS-*) and base_headers is already case-deduped, so no further normalization is needed. Behavioral fence (note for v2.6 release notes): extra_headers keys starting with 'kalshi-access-' (any case) now raise ValueError where they previously silently leaked alongside the SDK-signed pair. Three previously-passing tests (test_create_order_extra_headers_cannot_override_auth and siblings) and the matching transport-level test in test_client.py asserted the silent-override behaviour and have been rewritten to assert the new ValueError contract. Tests: * tests/test_extra_headers_plumbing.py — three new wire-assertion cases under TestHeaderCollision298: lowercase 'kalshi-access-key' raises, 'content-type: text/plain' is overridden to 'application/json' on the wire (with exactly one Content-Type line via respx route.calls.last. request.headers.raw), and 'x-request-id' round-trips unchanged. * tests/test_client.py — TestExtraHeadersPerRequest.test_extra_headers_ rejects_KALSHI_ACCESS_headers asserts the boundary raises for both the uppercase and lowercase variants. Verification: uv run mypy kalshi/ (clean), uv run ruff check . (clean), uv run pytest tests/test_extra_headers_plumbing.py tests/test_client.py tests/test_async_client.py -q (437 passed). Closes #298. --- kalshi/_base_client.py | 85 +++++++++++++++-- kalshi/resources/_base.py | 12 +++ tests/test_client.py | 54 +++++------ tests/test_extra_headers_plumbing.py | 134 +++++++++++++++++++++------ 4 files changed, 219 insertions(+), 66 deletions(-) diff --git a/kalshi/_base_client.py b/kalshi/_base_client.py index e3be8274..fe0248e2 100644 --- a/kalshi/_base_client.py +++ b/kalshi/_base_client.py @@ -161,6 +161,51 @@ def _would_exceed_budget(start: float, delay: float, total_timeout: float | None return (time.monotonic() - start) + delay > total_timeout +_AUTH_HEADER_PREFIX = "kalshi-access-" + + +def _strip_auth_headers(h: dict[str, str] | None) -> dict[str, str] | 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``. Returns ``h`` unchanged if no + leak is found. + """ + if not h: + return h + 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." + ) + return h + + +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 +254,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. + _strip_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 +276,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 +463,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. + _strip_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 +483,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/resources/_base.py b/kalshi/resources/_base.py index 31ff8bb3..1faeb20f 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: @@ -418,6 +424,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 +465,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: diff --git a/tests/test_client.py b/tests/test_client.py index aa177211..f34c98cc 100644 --- a/tests/test_client.py +++ b/tests/test_client.py @@ -1348,41 +1348,41 @@ 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", - }, + respx.get("https://test.kalshi.com/trade-api/v2/markets").mock( + return_value=httpx.Response(200, json={}) ) - 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" + 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_extra_headers_plumbing.py b/tests/test_extra_headers_plumbing.py index 81792707..b83b9455 100644 --- a/tests/test_extra_headers_plumbing.py +++ b/tests/test_extra_headers_plumbing.py @@ -65,28 +65,29 @@ 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: - client.orders.create( - ticker="BTC", - side="yes", - action="buy", - client_order_id="cli-1", - count=1, - yes_price=50, - extra_headers={ - "KALSHI-ACCESS-KEY": "spoofed", - "KALSHI-ACCESS-SIGNATURE": "spoofed-sig", - "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" + with ( + KalshiClient(auth=test_auth, config=_config()) as client, + pytest.raises(ValueError, match="KALSHI-ACCESS-"), + ): + client.orders.create( + ticker="BTC", + side="yes", + action="buy", + client_order_id="cli-1", + count=1, + yes_price=50, + extra_headers={ + "KALSHI-ACCESS-KEY": "spoofed", + "KALSHI-ACCESS-SIGNATURE": "spoofed-sig", + "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: - client.markets.get("BTC", extra_headers={"KALSHI-ACCESS-KEY": "spoofed"}) - assert route.calls[0].request.headers["KALSHI-ACCESS-KEY"] == "test-key-id" + 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"}) 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,82 @@ def test_balance_extra_headers_cannot_override_auth(self, test_auth: KalshiAuth) }, ) ) + with ( + KalshiClient(auth=test_auth, config=_config()) as client, + pytest.raises(ValueError, match="KALSHI-ACCESS-"), + ): + client.portfolio.balance(extra_headers={"KALSHI-ACCESS-KEY": "spoofed"}) + + +# --------------------------------------------------------------------------- +# #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.portfolio.balance(extra_headers={"KALSHI-ACCESS-KEY": "spoofed"}) - assert route.calls[0].request.headers["KALSHI-ACCESS-KEY"] == "test-key-id" + 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 # --------------------------------------------------------------------------- From 80e40143d4a442295ba62c35109bbf79aacfe556 Mon Sep 17 00:00:00 2001 From: Jeff West Date: Thu, 21 May 2026 18:50:13 -0500 Subject: [PATCH 2/3] fix-up(transport+config): close config.extra_headers gap; pin Content-Type on _delete_with_body; rename helper; fix test indent Bot review on PR #308 flagged two real bugs plus style nits: - KalshiConfig(extra_headers={'kalshi-access-key': ...}) bypassed the per-request _strip_auth_headers guard, leaving a forge surface on the httpx.Client default headers. __post_init__ now rejects KALSHI-ACCESS-* keys in extra_headers at construction (inlined prefix check to avoid a circular import with kalshi._base_client). New tests in tests/test_config.py pin the uppercase, lowercase, KALSHI-ACCESS-SIGNATURE, and KALSHI-ACCESS-TIMESTAMP cases plus a non-auth sanity check. - Sync and async _delete_with_body in kalshi/resources/_base.py now pass headers={'Content-Type': 'application/json'} into the transport, matching the _post / _put / _delete_with_body_json pattern. tests/test_extra_headers_plumbing.py adds wire-assertion tests via batch_cancel_v2 (sync + async) that pin the Content-Type even when the caller passes extra_headers={'content-type': 'text/plain'}. - Renamed _strip_auth_headers -> _assert_no_auth_headers in kalshi/_base_client.py (3 sites: definition + sync transport + async transport). The helper raises; the new name matches the behaviour. - Fixed over-indented 'with (...)' bodies in tests/test_extra_headers_plumbing.py (four blocks; bodies were at 16 spaces instead of the standard 12). --- kalshi/_base_client.py | 6 +- kalshi/config.py | 16 +++++ kalshi/resources/_base.py | 2 + tests/test_config.py | 24 ++++++++ tests/test_extra_headers_plumbing.py | 92 ++++++++++++++++++++-------- 5 files changed, 113 insertions(+), 27 deletions(-) diff --git a/kalshi/_base_client.py b/kalshi/_base_client.py index fe0248e2..dbac8f56 100644 --- a/kalshi/_base_client.py +++ b/kalshi/_base_client.py @@ -164,7 +164,7 @@ def _would_exceed_budget(start: float, delay: float, total_timeout: float | None _AUTH_HEADER_PREFIX = "kalshi-access-" -def _strip_auth_headers(h: dict[str, str] | None) -> dict[str, str] | None: +def _assert_no_auth_headers(h: dict[str, str] | None) -> dict[str, str] | None: """Reject caller-supplied ``KALSHI-ACCESS-*`` keys (case-insensitive). #298: auth headers are SDK-managed (RSA-PSS signed per attempt) and @@ -257,7 +257,7 @@ def request( # #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. - _strip_auth_headers(extra_headers) + _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 @@ -466,7 +466,7 @@ async def request( # #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. - _strip_auth_headers(extra_headers) + _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.") diff --git a/kalshi/config.py b/kalshi/config.py index a590024b..c9c5dfef 100644 --- a/kalshi/config.py +++ b/kalshi/config.py @@ -133,6 +133,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 + # check is inlined (rather than importing kalshi._base_client) because + # _base_client imports KalshiConfig — avoiding a circular import. + if self.extra_headers: + leaked = sorted( + k for k in self.extra_headers if k.lower().startswith("kalshi-access-") + ) + 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 1faeb20f..b7199474 100644 --- a/kalshi/resources/_base.py +++ b/kalshi/resources/_base.py @@ -264,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: @@ -504,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_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 b83b9455..e75cfe95 100644 --- a/tests/test_extra_headers_plumbing.py +++ b/tests/test_extra_headers_plumbing.py @@ -75,19 +75,19 @@ def test_create_order_extra_headers_rejects_kalshi_access(self, test_auth: Kalsh KalshiClient(auth=test_auth, config=_config()) as client, pytest.raises(ValueError, match="KALSHI-ACCESS-"), ): - client.orders.create( - ticker="BTC", - side="yes", - action="buy", - client_order_id="cli-1", - count=1, - yes_price=50, - extra_headers={ - "KALSHI-ACCESS-KEY": "spoofed", - "KALSHI-ACCESS-SIGNATURE": "spoofed-sig", - "KALSHI-ACCESS-TIMESTAMP": "0", - }, - ) + client.orders.create( + ticker="BTC", + side="yes", + action="buy", + client_order_id="cli-1", + count=1, + yes_price=50, + extra_headers={ + "KALSHI-ACCESS-KEY": "spoofed", + "KALSHI-ACCESS-SIGNATURE": "spoofed-sig", + "KALSHI-ACCESS-TIMESTAMP": "0", + }, + ) @respx.mock def test_cancel_order_threads_extra_headers(self, test_auth: KalshiAuth) -> None: @@ -118,7 +118,7 @@ def test_get_market_extra_headers_rejects_kalshi_access(self, test_auth: KalshiA KalshiClient(auth=test_auth, config=_config()) as client, pytest.raises(ValueError, match="KALSHI-ACCESS-"), ): - client.markets.get("BTC", extra_headers={"KALSHI-ACCESS-KEY": "spoofed"}) + client.markets.get("BTC", extra_headers={"KALSHI-ACCESS-KEY": "spoofed"}) class TestPortfolio: @@ -158,7 +158,7 @@ def test_balance_extra_headers_rejects_kalshi_access(self, test_auth: KalshiAuth KalshiClient(auth=test_auth, config=_config()) as client, pytest.raises(ValueError, match="KALSHI-ACCESS-"), ): - client.portfolio.balance(extra_headers={"KALSHI-ACCESS-KEY": "spoofed"}) + client.portfolio.balance(extra_headers={"KALSHI-ACCESS-KEY": "spoofed"}) # --------------------------------------------------------------------------- @@ -180,15 +180,15 @@ def test_extra_headers_lowercase_kalshi_access_raises( 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"}, - ) + 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( @@ -231,6 +231,50 @@ def test_extra_headers_non_auth_round_trips_unchanged( 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(assert_all_called=False) 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 + # --------------------------------------------------------------------------- # Parametric coverage — every public resource method has the kwarg. From 957efb4b91980a78aebf874860f3ccbc89a9a79d Mon Sep 17 00:00:00 2001 From: Jeff West Date: Thu, 21 May 2026 19:21:10 -0500 Subject: [PATCH 3/3] fix-up(transport+config): _assert_no_auth_headers -> None; extract AUTH_HEADER_PREFIX; clean up tests - Return type on _assert_no_auth_headers is now None (drops misleading dict return). - Extract AUTH_HEADER_PREFIX to kalshi/_constants.py (no imports, no circular hazard); import from both _base_client.py and config.py. - Drop dead respx.get().mock() registration in test_extra_headers_rejects_KALSHI_ACCESS_headers (ValueError fires before any request). - Drop assert_all_called=False on test_delete_with_body_async_pins_content_type to mirror the sync test (route is expected to be called). --- kalshi/_base_client.py | 13 +++++-------- kalshi/_constants.py | 5 +++++ kalshi/config.py | 8 +++++--- tests/test_client.py | 5 ++--- tests/test_extra_headers_plumbing.py | 2 +- 5 files changed, 18 insertions(+), 15 deletions(-) create mode 100644 kalshi/_constants.py diff --git a/kalshi/_base_client.py b/kalshi/_base_client.py index dbac8f56..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,26 +162,22 @@ def _would_exceed_budget(start: float, delay: float, total_timeout: float | None return (time.monotonic() - start) + delay > total_timeout -_AUTH_HEADER_PREFIX = "kalshi-access-" - -def _assert_no_auth_headers(h: dict[str, str] | None) -> dict[str, str] | None: +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``. Returns ``h`` unchanged if no - leak is found. + must not be set via ``extra_headers``. No-op if no leak is found. """ if not h: - return h - leaked = [k for k in h if k.lower().startswith(_AUTH_HEADER_PREFIX)] + 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." ) - return h def _ci_merge(*layers: dict[str, str] | None) -> dict[str, str]: 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 c9c5dfef..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" @@ -137,11 +139,11 @@ def __post_init__(self) -> None: # 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 - # check is inlined (rather than importing kalshi._base_client) because - # _base_client imports KalshiConfig — avoiding a circular import. + # 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("kalshi-access-") + k for k in self.extra_headers if k.lower().startswith(AUTH_HEADER_PREFIX) ) if leaked: raise ValueError( diff --git a/tests/test_client.py b/tests/test_client.py index f34c98cc..d8a7846a 100644 --- a/tests/test_client.py +++ b/tests/test_client.py @@ -1362,9 +1362,8 @@ def test_extra_headers_rejects_KALSHI_ACCESS_headers( # noqa: N802 timeout=5.0, ) transport = SyncTransport(test_auth, config) - respx.get("https://test.kalshi.com/trade-api/v2/markets").mock( - return_value=httpx.Response(200, json={}) - ) + # 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", diff --git a/tests/test_extra_headers_plumbing.py b/tests/test_extra_headers_plumbing.py index e75cfe95..1a35da87 100644 --- a/tests/test_extra_headers_plumbing.py +++ b/tests/test_extra_headers_plumbing.py @@ -261,7 +261,7 @@ async def test_delete_with_body_async_pins_content_type( from kalshi import AsyncKalshiClient from kalshi.models.orders import BatchCancelOrdersV2Request - with _respx.mock(assert_all_called=False) as router: + with _respx.mock() as router: route = router.delete( f"{MOCK_BASE}/portfolio/events/orders/batched" ).mock(return_value=httpx.Response(200, json={"orders": []}))