Skip to content

from_env flips _auth_owned from input kwarg, breaking #210 ownership contract #311

Closed

Description

@TexasCoding

Problem

KalshiClient.from_env (and the async mirror) overrides _auth_owned based on whether kwargs["auth"] is truthy after setdefault, not based on who actually built the auth. This breaks the #210 contract that caller-supplied KalshiAuth is never closed by the SDK, and simultaneously leaks the sign ThreadPoolExecutor when env vars are empty and the caller passes only key_id + private_key.

Two concrete triggers:

  • Case B (executor leak): KalshiClient.from_env(key_id='K', private_key=PEM) with no env vars set. try_from_env() returns None, so kwargs["auth"]=None. __init__ builds auth from key_id/private_key and correctly sets _auth_owned=True. from_env then overwrites it: (None is not None) == False. The SDK now owns auth that close() will never shut down, leaking two kalshi-sign-* threads per client across long-running pytest sessions, FastAPI lifespans, and worker pools.
  • Case C (closes caller-owned auth): env vars set and caller passes from_env(auth=my_auth). setdefault keeps my_auth; __init__ records _auth_owned=False per Auth: KalshiClient.close() shouldn't shut down externally-provided KalshiAuth executor #210. from_env then flips it to True. client.close() calls my_auth.close(), and every subsequent sign_request_async on the caller's still-referenced auth raises RuntimeError("KalshiAuth has been closed") (auth.py:317).

Evidence

  • kalshi/client.py:182-188 — from_env builds kwargs then forcibly recomputes ownership from the input kwarg:
kwargs.setdefault("auth", KalshiAuth.try_from_env())
kwargs.setdefault("demo", os.environ.get("KALSHI_DEMO", "").lower() == "true")
kwargs.setdefault("base_url", os.environ.get("KALSHI_API_BASE_URL"))
client = cls(**kwargs)
# from_env constructs auth locally — caller never owned it.
client._auth_owned = kwargs["auth"] is not None
return client
  • kalshi/client.py:96-107 — __init__ already encodes the correct invariant (_auth_owned = (auth is None)) and may construct auth from key_id/private_key arguments. The override at :187 ignores that branch entirely.
  • kalshi/async_client.py:214-220 — identical bug in AsyncKalshiClient.from_env.
  • kalshi/client.py:193-194, kalshi/async_client.py:225-226 — close() gates auth.close() on _auth_owned, so case B silently skips it and case C silently runs it on caller-owned auth.

Suggested fix

Recompute ownership from what __init__ actually did, instead of from the input kwarg:

client = cls(**kwargs)
client._auth_owned = (kwargs.get("auth") is None) and (client._auth is not None)
return client

Equivalently, have from_env build auth itself and pass it via the explicit auth= kwarg, dropping the override entirely so the __init__ invariant _auth_owned = (auth is None) carries through. Mirror the fix in both kalshi/client.py and kalshi/async_client.py. Add regression tests covering both case B (executor not leaked when constructed from key_id/private_key) and case C (caller-supplied auth= survives client.close()).

Source

Round-3 independent audit (reviewer: codex+security_auth).

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't workingsecuritySecurity-related concern

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions