From 4b689724079bd58e7d5ab431b4798a7a260df127 Mon Sep 17 00:00:00 2001 From: x Date: Fri, 22 May 2026 07:33:57 -0500 Subject: [PATCH 1/3] fix(security): freeze KalshiConfig.extra_headers post-construction; single header pipeline #313: KalshiConfig.extra_headers was a plain dict on a frozen dataclass. The #298 auth-header fence only ran at construction, so post-construction mutation (config.extra_headers['kalshi-access-key'] = 'forged') re-opened the duplicate-auth-header forge surface that #298 had closed. Wrap the field in types.MappingProxyType over a defensive copy in __post_init__ so the stored mapping rejects mutation and is decoupled from the caller's original dict. #341: SyncTransport and AsyncTransport were attaching config.extra_headers to httpx.Client AND re-merging them into every request via _ci_merge. The wire output was correct but the documented precedence (config < per-call < signed) routed through two pipelines, and httpx walked the default headers on every request. Drop the httpx.Client attachment so _ci_merge is the single source of truth and the precedence contract is authoritative. Type annotation widened to Mapping[str, str]; _ci_merge accepts Mapping layers. New regression tests at config level and wire level cover both issues; the prior mutate-bleed default-factory test is reframed to assert per-instance isolation under the freeze. Closes #313, #341 --- kalshi/_base_client.py | 12 +++-- kalshi/config.py | 15 ++++++- tests/test_config.py | 33 +++++++++++--- tests/test_extra_headers_plumbing.py | 67 ++++++++++++++++++++++++++++ 4 files changed, 115 insertions(+), 12 deletions(-) diff --git a/kalshi/_base_client.py b/kalshi/_base_client.py index 72980cbd..431a3613 100644 --- a/kalshi/_base_client.py +++ b/kalshi/_base_client.py @@ -12,6 +12,7 @@ import math import random import time +from collections.abc import Mapping from datetime import UTC, datetime from typing import Any from urllib.parse import urlparse @@ -180,7 +181,7 @@ def _assert_no_auth_headers(h: dict[str, str] | None) -> None: ) -def _ci_merge(*layers: dict[str, str] | None) -> dict[str, str]: +def _ci_merge(*layers: Mapping[str, str] | None) -> dict[str, str]: """Case-insensitive, last-layer-wins header merge. #298: plain ``dict`` unpacking is case-sensitive, so case-mismatched @@ -217,10 +218,14 @@ def __init__( self._config = config # Cached once: base_url is immutable on a frozen KalshiConfig. self._base_path = urlparse(config.base_url).path + # #341: ``config.extra_headers`` is merged per-request via + # ``_ci_merge`` below, which keeps SDK-managed precedence (config + # defaults < per-call extras < signed auth) authoritative. Attaching + # them to the httpx client as well would route the same headers + # through httpx's own merge on every request and blur the contract. client_kwargs: dict[str, Any] = { "base_url": config.base_url, "timeout": config.timeout, - "headers": config.extra_headers, "transport": transport, "http2": config.http2, } @@ -440,10 +445,11 @@ def __init__( self._config = config # Cached once: base_url is immutable on a frozen KalshiConfig. self._base_path = urlparse(config.base_url).path + # #341: see SyncTransport — config.extra_headers is merged per-request + # via ``_ci_merge``, not via httpx.Client's default headers. client_kwargs: dict[str, Any] = { "base_url": config.base_url, "timeout": config.timeout, - "headers": config.extra_headers, "transport": transport, "http2": config.http2, } diff --git a/kalshi/config.py b/kalshi/config.py index 817f0bf7..7b19a898 100644 --- a/kalshi/config.py +++ b/kalshi/config.py @@ -4,8 +4,9 @@ import logging import os -from collections.abc import Callable +from collections.abc import Callable, Mapping from dataclasses import dataclass, field +from types import MappingProxyType from typing import Any from urllib.parse import urlparse @@ -68,7 +69,7 @@ class KalshiConfig: retry_base_delay: float = 0.5 retry_max_delay: float = 30.0 total_timeout: float | None = None - extra_headers: dict[str, str] = field(default_factory=dict) + extra_headers: Mapping[str, str] = field(default_factory=dict) ws_base_url: str = PRODUCTION_WS_URL # trailing slash is stripped automatically ws_max_retries: int = DEFAULT_WS_MAX_RETRIES http2: bool = False @@ -141,6 +142,13 @@ def __post_init__(self) -> None: # 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. + # #313: defense-in-depth — the #298 fence above only runs at + # construction. A plain dict stored on a frozen dataclass is still + # mutable, so a caller could do ``config.extra_headers["kalshi-access- + # key"] = "forged"`` afterwards and bypass the guard entirely. Freeze a + # defensive copy into a ``MappingProxyType`` so post-construction + # mutation raises ``TypeError`` and the original mapping the caller + # passed in can be safely discarded. if self.extra_headers: leaked = sorted( k for k in self.extra_headers if k.lower().startswith(AUTH_HEADER_PREFIX) @@ -151,6 +159,9 @@ def __post_init__(self) -> None: f"keys (got: {leaked!r}). These are reserved for SDK-managed " f"RSA-PSS signing." ) + object.__setattr__( + self, "extra_headers", MappingProxyType(dict(self.extra_headers)) + ) if self.http2: import importlib.util diff --git a/tests/test_config.py b/tests/test_config.py index 9d9c89e2..93e56455 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -166,15 +166,34 @@ def test_extra_headers_persist_across_multiple_requests(self, test_auth: KalshiA finally: transport.close() - def test_extra_headers_defaults_to_empty_dict(self) -> None: - # Each instance gets its own dict (default_factory), so mutating one - # config's extras must not bleed into another's. + def test_extra_headers_defaults_to_empty_mapping(self) -> None: + # Each instance gets its own underlying dict (default_factory), so the + # frozen views are isolated and per-instance state cannot leak between + # configs even via the original mapping passed at construction. config_a = KalshiConfig() config_b = KalshiConfig() - assert config_a.extra_headers == {} - assert config_b.extra_headers == {} - config_a.extra_headers["X-Trace-Id"] = "a" - assert config_b.extra_headers == {} + assert dict(config_a.extra_headers) == {} + assert dict(config_b.extra_headers) == {} + + def test_issue_313_extra_headers_immutable_post_construction(self) -> None: + # #313: a frozen dataclass forbids attribute reassignment but the + # underlying dict was still mutable, re-opening the #298 forge surface. + # extra_headers is now a MappingProxyType and rejects mutation. + cfg = KalshiConfig(extra_headers={"X-Trace-Id": "a"}) + with pytest.raises(TypeError): + cfg.extra_headers["X-Trace-Id"] = "b" # type: ignore[index] + with pytest.raises(TypeError): + cfg.extra_headers["kalshi-access-key"] = "forged" # type: ignore[index] + with pytest.raises(TypeError): + del cfg.extra_headers["X-Trace-Id"] # type: ignore[attr-defined] + # Original passed-in dict must not back the stored view either — + # mutating it after construction must not affect the config. + original = {"X-Other": "v"} + cfg2 = KalshiConfig(extra_headers=original) + original["X-Other"] = "tampered" + original["kalshi-access-key"] = "forged" + assert cfg2.extra_headers["X-Other"] == "v" + assert "kalshi-access-key" not in cfg2.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 diff --git a/tests/test_extra_headers_plumbing.py b/tests/test_extra_headers_plumbing.py index 1a35da87..62b0baec 100644 --- a/tests/test_extra_headers_plumbing.py +++ b/tests/test_extra_headers_plumbing.py @@ -327,3 +327,70 @@ def test_every_public_method_accepts_extra_headers(cls: type, name: str, fn: Any inspect.Parameter.KEYWORD_ONLY, inspect.Parameter.POSITIONAL_OR_KEYWORD, ) + +# --------------------------------------------------------------------------- +# #313 + #341 — post-construction immutability and single header pipeline. +# --------------------------------------------------------------------------- + + +class TestExtraHeadersSecurityFreeze: + @respx.mock + def test_issue_313_extra_headers_post_mutation_does_not_send_auth_forge( + self, test_auth: KalshiAuth + ) -> None: + # #313: the underlying mapping is frozen post-construction, so attempting + # to seed a forged KALSHI-ACCESS-* key after construction must either + # raise (TypeError on the proxy) or otherwise have no effect on the wire + # — never both an SDK-signed and a caller-forged pair on the same line. + cfg = _config(extra_headers={"X-Trace-Id": "t1"}) + with pytest.raises(TypeError): + cfg.extra_headers["kalshi-access-key"] = "forged" # type: ignore[index] + + 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=cfg) as client: + client.markets.get("BTC") + wire = route.calls.last.request.headers + # No forged lowercase pair ever reached the wire. + forged = [k for k, _ in wire.raw if k.lower() == b"kalshi-access-key"] + # Exactly one signed KALSHI-ACCESS-KEY line, never two. + assert len(forged) == 1, forged + # And the signed value is not the forged one. + assert wire["KALSHI-ACCESS-KEY"] != "forged" + + @respx.mock + def test_issue_341_extra_headers_single_pipeline( + self, test_auth: KalshiAuth + ) -> None: + # #341: config.extra_headers is no longer attached to httpx.Client's + # default headers. _ci_merge is the single source of truth, so a + # per-call extra_headers value must override config.extra_headers + # case-insensitively with no duplicate raw header line on the wire. + cfg = _config( + extra_headers={"X-Trace-Id": "config-default", "User-Agent": "kalshi-sdk/cfg"} + ) + 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=cfg) as client: + # Per-call layer overrides the config layer (case-insensitive). + client.markets.get( + "BTC", extra_headers={"x-trace-id": "per-call"} + ) + # No per-call override → config default reaches the wire. + client.markets.get("BTC") + + first = route.calls[0].request.headers + assert first["x-trace-id"] == "per-call" + # Exactly one X-Trace-Id raw header line — no httpx-side duplicate + # alongside the SDK-merged one. + first_trace = [k for k, _ in first.raw if k.lower() == b"x-trace-id"] + assert len(first_trace) == 1, first_trace + # User-Agent from config still reaches both wires (single pipeline). + assert first["user-agent"] == "kalshi-sdk/cfg" + + second = route.calls[1].request.headers + assert second["x-trace-id"] == "config-default" + second_trace = [k for k, _ in second.raw if k.lower() == b"x-trace-id"] + assert len(second_trace) == 1, second_trace From 028c1f72b14baa0539d560c584c1825c91ae2212 Mon Sep 17 00:00:00 2001 From: Jeff West Date: Fri, 22 May 2026 07:57:11 -0500 Subject: [PATCH 2/3] polish(security): rename misleading test var; widen _assert_no_auth_headers Mapping accept --- kalshi/_base_client.py | 2 +- tests/test_extra_headers_plumbing.py | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/kalshi/_base_client.py b/kalshi/_base_client.py index 431a3613..96b89204 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 -def _assert_no_auth_headers(h: dict[str, str] | None) -> None: +def _assert_no_auth_headers(h: Mapping[str, str] | None) -> None: """Reject caller-supplied ``KALSHI-ACCESS-*`` keys (case-insensitive). #298: auth headers are SDK-managed (RSA-PSS signed per attempt) and diff --git a/tests/test_extra_headers_plumbing.py b/tests/test_extra_headers_plumbing.py index 62b0baec..b5ad650f 100644 --- a/tests/test_extra_headers_plumbing.py +++ b/tests/test_extra_headers_plumbing.py @@ -352,10 +352,10 @@ def test_issue_313_extra_headers_post_mutation_does_not_send_auth_forge( with KalshiClient(auth=test_auth, config=cfg) as client: client.markets.get("BTC") wire = route.calls.last.request.headers - # No forged lowercase pair ever reached the wire. - forged = [k for k, _ in wire.raw if k.lower() == b"kalshi-access-key"] - # Exactly one signed KALSHI-ACCESS-KEY line, never two. - assert len(forged) == 1, forged + # Exactly one KALSHI-ACCESS-KEY line on the wire — the SDK-signed + # one, not a forged duplicate. + access_key_lines = [k for k, _ in wire.raw if k.lower() == b"kalshi-access-key"] + assert len(access_key_lines) == 1, access_key_lines # And the signed value is not the forged one. assert wire["KALSHI-ACCESS-KEY"] != "forged" From 00f842289ca47a93ddc99c8106355e935d512807 Mon Sep 17 00:00:00 2001 From: Jeff West Date: Fri, 22 May 2026 08:07:16 -0500 Subject: [PATCH 3/3] polish(security): trim multi-line comments; add async wire-level test for single header pipeline --- kalshi/_base_client.py | 9 ++------ kalshi/config.py | 15 ++---------- tests/test_extra_headers_plumbing.py | 34 +++++++++++++++++++++++++++- 3 files changed, 37 insertions(+), 21 deletions(-) diff --git a/kalshi/_base_client.py b/kalshi/_base_client.py index 96b89204..2724b569 100644 --- a/kalshi/_base_client.py +++ b/kalshi/_base_client.py @@ -218,11 +218,7 @@ def __init__( self._config = config # Cached once: base_url is immutable on a frozen KalshiConfig. self._base_path = urlparse(config.base_url).path - # #341: ``config.extra_headers`` is merged per-request via - # ``_ci_merge`` below, which keeps SDK-managed precedence (config - # defaults < per-call extras < signed auth) authoritative. Attaching - # them to the httpx client as well would route the same headers - # through httpx's own merge on every request and blur the contract. + # #341: extra_headers merged per-request via _ci_merge; attaching here would duplicate it. client_kwargs: dict[str, Any] = { "base_url": config.base_url, "timeout": config.timeout, @@ -445,8 +441,7 @@ def __init__( self._config = config # Cached once: base_url is immutable on a frozen KalshiConfig. self._base_path = urlparse(config.base_url).path - # #341: see SyncTransport — config.extra_headers is merged per-request - # via ``_ci_merge``, not via httpx.Client's default headers. + # #341: extra_headers merged per-request via _ci_merge; attaching here would duplicate it. client_kwargs: dict[str, Any] = { "base_url": config.base_url, "timeout": config.timeout, diff --git a/kalshi/config.py b/kalshi/config.py index 7b19a898..896eda67 100644 --- a/kalshi/config.py +++ b/kalshi/config.py @@ -136,19 +136,7 @@ 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. - # #313: defense-in-depth — the #298 fence above only runs at - # construction. A plain dict stored on a frozen dataclass is still - # mutable, so a caller could do ``config.extra_headers["kalshi-access- - # key"] = "forged"`` afterwards and bypass the guard entirely. Freeze a - # defensive copy into a ``MappingProxyType`` so post-construction - # mutation raises ``TypeError`` and the original mapping the caller - # passed in can be safely discarded. + # #298: reject KALSHI-ACCESS-* prefix at construction so callers cannot forge auth. if self.extra_headers: leaked = sorted( k for k in self.extra_headers if k.lower().startswith(AUTH_HEADER_PREFIX) @@ -159,6 +147,7 @@ def __post_init__(self) -> None: f"keys (got: {leaked!r}). These are reserved for SDK-managed " f"RSA-PSS signing." ) + # #313: defensive copy into MappingProxyType so post-construction mutation raises. object.__setattr__( self, "extra_headers", MappingProxyType(dict(self.extra_headers)) ) diff --git a/tests/test_extra_headers_plumbing.py b/tests/test_extra_headers_plumbing.py index b5ad650f..caa03d50 100644 --- a/tests/test_extra_headers_plumbing.py +++ b/tests/test_extra_headers_plumbing.py @@ -19,7 +19,7 @@ import pytest import respx -from kalshi import KalshiClient +from kalshi import AsyncKalshiClient, KalshiClient from kalshi.auth import KalshiAuth from kalshi.config import DEMO_WS_URL, KalshiConfig from kalshi.resources._base import AsyncResource, SyncResource @@ -394,3 +394,35 @@ def test_issue_341_extra_headers_single_pipeline( assert second["x-trace-id"] == "config-default" second_trace = [k for k, _ in second.raw if k.lower() == b"x-trace-id"] assert len(second_trace) == 1, second_trace + + @respx.mock + @pytest.mark.asyncio + async def test_issue_341_extra_headers_single_pipeline_async( + self, test_auth: KalshiAuth + ) -> None: + # #341 async mirror: AsyncTransport must follow the same single-pipeline + # contract — _ci_merge is the sole header source, per-call extras win over + # config defaults case-insensitively, and no httpx-side default attachment + # produces a duplicate raw header line on the wire. + cfg = _config( + extra_headers={"X-Trace-Id": "config-default", "User-Agent": "kalshi-sdk/cfg"} + ) + route = respx.get(f"{MOCK_BASE}/markets/BTC").mock( + return_value=httpx.Response(200, json={"market": _example_market_payload()}) + ) + async with AsyncKalshiClient(auth=test_auth, config=cfg) as client: + await client.markets.get( + "BTC", extra_headers={"x-trace-id": "per-call"} + ) + await client.markets.get("BTC") + + first = route.calls[0].request.headers + assert first["x-trace-id"] == "per-call" + first_trace = [k for k, _ in first.raw if k.lower() == b"x-trace-id"] + assert len(first_trace) == 1, first_trace + assert first["user-agent"] == "kalshi-sdk/cfg" + + second = route.calls[1].request.headers + assert second["x-trace-id"] == "config-default" + second_trace = [k for k, _ in second.raw if k.lower() == b"x-trace-id"] + assert len(second_trace) == 1, second_trace