fix(auth): from_env preserves caller ownership of KalshiAuth + lazy env eval

[TexasCoding]

Summary

KalshiClient.from_env and its async mirror had two overlapping integrity
bugs in how they handled KalshiAuth. First (#311), the method recomputed
_auth_owned from the input kwarg after __init__ had already encoded
the correct ownership, so a caller passing auth=my_auth while env vars were
set would have their auth shut down by client.close() (closing a still-live
shared KalshiAuth), and a caller passing key_id+private_key with no env
vars would silently leak the sign-offload ThreadPoolExecutor because the
SDK forgot it owned the auth it had just built. Second (#316), from_env
unconditionally evaluated KalshiAuth.try_from_env() even when the caller had
already supplied explicit credentials, so a malformed KALSHI_PRIVATE_KEY
lingering in the process env would raise KalshiAuthError before the
caller-supplied auth had a chance to win — turning a CI hygiene paper-cut
into a hard outage.

This PR tightens the ownership invariant in __init__ (_auth_owned is
True only when the constructor actually built a KalshiAuth; False when the
caller supplied one or when there is no auth at all) and rewrites from_env
so it only consults the env when the caller has not provided auth=,
key_id, private_key, or private_key_path. When from_env builds the
auth itself from env vars, it explicitly claims ownership on the resulting
client so close() shuts it down.

Issues closed

• Closes from_env flips _auth_owned from input kwarg, breaking #210 ownership contract #311 — from_env no longer flips _auth_owned from the input kwarg; constructor is the source of truth.
• Closes from_env() eagerly evaluates try_from_env() even when caller passes auth= #316 — from_env evaluates try_from_env() lazily; explicit auth=/key_id/private_key short-circuits env eval.

Behavioral changes

• KalshiClient(...) / AsyncKalshiClient(...) with no auth credentials now
  reports _auth_owned == False (previously True, which was a moot
  inconsistency: there was nothing to own). close() behavior is unchanged
  because it already gates on self._auth is not None.
• from_env(auth=..., ...) and from_env(key_id=..., private_key=..., ...)
  no longer parse KALSHI_PRIVATE_KEY from the environment; a malformed env
  PEM can no longer block a call that passes explicit credentials.

Tests

• tests/test_client.py::TestFromEnvOwnershipAndLazyEval — sync regressions for from_env flips _auth_owned from input kwarg, breaking #210 ownership contract #311 (cases B and C) and from_env() eagerly evaluates try_from_env() even when caller passes auth= #316 (env not eagerly read when auth= or key_id+private_key is supplied).
• tests/test_async_client.py::TestAsyncFromEnvOwnershipAndLazyEval — async mirror of the same four scenarios.

Source

Round-3 independent audit closure plan, wave W1.

[claude]

Code Review — PR #362

Overall: Solid, surgical fix for two real bugs. The logic is correct, the comments explain the invariant clearly, and the tests cover the two primary regression scenarios. A few minor items below.

---

What's correct

__init__ rewrite (both clients): The explicit per-branch assignment of _auth_owned in the if/elif/else chain is a direct improvement. The old one-liner self._auth_owned: bool = auth is None was semantically wrong in two directions (leaked the executor when key_id+private_key was used; set True when there was no auth at all). Each branch now has an unambiguous assignment.

from_env lazy eval (#316): caller_supplied_auth correctly covers all four credential inputs (auth=, key_id, private_key, private_key_path) before calling try_from_env(). The old setdefault("auth", try_from_env()) eagerly evaluated the default expression in Python regardless of whether the key already existed — this was a real footgun.

Post-__init__ ownership correction (#311): client._auth_owned = True after cls(**kwargs) is necessary given how __init__ encodes the invariant (caller-passed auth= → False). The comment explains this well. Not ideal as a design, but the right call short of a larger refactor.

---

Issues

1. Trailing blank lines (minor, ruff-catchable)

Both new test classes end with two added blank lines before the next class definition, giving three total (the existing blank line is still there as context). PEP 8 / ruff E302 wants exactly two.

test_async_client.py — after await client.close() inside TestAsyncFromEnvOwnershipAndLazyEval, there are 2 added blanks + 1 context blank before class TestAsyncTransportNetworkRetry. Same pattern in test_client.py before class TestExtraHeadersPerRequest. One blank line should be removed from each.

2. Missing test: private_key_path lazy-eval path (minor)

caller_supplied_auth gates on private_key_path but there's no test for from_env(key_id="x", private_key_path="/p") with a malformed env PEM. The three other credential sources have coverage; private_key_path is the odd one out. Not a blocker, but consistent with the test philosophy here.

3. Subtle asymmetry in caller_supplied_auth check (low risk)

caller_supplied_auth = (
    "auth" in kwargs          # membership check — catches auth=None
    or kwargs.get("key_id") is not None    # value check — misses key_id=None
    or kwargs.get("private_key") is not None
    or kwargs.get("private_key_path") is not None
)

"auth" in kwargs is the right idiom for auth (a caller might legitimately pass auth=None). But kwargs.get("key_id") is not None would treat from_env(key_id=None) the same as not passing key_id at all, so try_from_env() would still be called. That's probably fine in practice — passing key_id=None explicitly is an unusual thing to do, and __init__ wouldn't build auth from it anyway. But the inconsistency is worth noting if this ever causes a confusing debug session.

---

No concerns with

• Security fix rationale — malformed-PEM-in-env blocking explicit callers is a real availability risk in CI pipelines
• Resource ownership semantics — the _auth_owned invariant is now correct end-to-end
• Sync/async symmetry — both clients received identical treatment
• Test fixtures — monkeypatch.delenv(..., raising=False) for each env var used is thorough setup hygiene

---

Verdict: Approve with the trailing blank lines fixed before merge (ruff will catch them if CI runs linting). The missing private_key_path test is a nice-to-have, not a blocker.