diff --git a/kalshi/_base_client.py b/kalshi/_base_client.py index 72980cbd..2724b569 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 @@ -163,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 @@ -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,10 @@ def __init__( self._config = config # Cached once: base_url is immutable on a frozen KalshiConfig. self._base_path = urlparse(config.base_url).path + # #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, - "headers": config.extra_headers, "transport": transport, "http2": config.http2, } @@ -440,10 +441,10 @@ def __init__( self._config = config # Cached once: base_url is immutable on a frozen KalshiConfig. self._base_path = urlparse(config.base_url).path + # #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, - "headers": config.extra_headers, "transport": transport, "http2": config.http2, } diff --git a/kalshi/config.py b/kalshi/config.py index 817f0bf7..896eda67 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 @@ -135,12 +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. + # #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) @@ -151,6 +147,10 @@ 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)) + ) 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..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 @@ -327,3 +327,102 @@ 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 + # 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" + + @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 + + @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