[TexasCoding]

Closes #249
Closes #266
Closes #261
Closes #250

Part of #273 (post-v2.4 audit), Wave 2.

[claude]

Code Review — PR #287: feat(auth+config): defense-in-depth

Good focused security hardening wave. Four independent issues, each with clear motivation, tight implementation, and matching tests. Some issues to flag below.

---

Overview

Issue	Change	Assessment
#249	Reject both KALSHI_PRIVATE_KEY + KALSHI_PRIVATE_KEY_PATH set simultaneously	✅ Correct
#266	from_env(**kwargs: Unpack[ClientInitKwargs]) instead of **kwargs: object	✅ Correct
#261	Short-circuit _normalize_percent_encoding when path has no %	✅ Correct
#250	Default-fail on unknown hosts; opt-in via flag or env var	✅ Correct

---

Issues to Fix

1. _auth_owned is incorrectly set when auth= is passed to from_env (semantic bug)

In the refactored from_env:

kwargs.setdefault("auth", KalshiAuth.try_from_env())
client = cls(**kwargs)
client._auth_owned = kwargs["auth"] is not None   # ← problem

If a caller passes from_env(auth=my_auth), setdefault won't override it, so kwargs["auth"] = my_auth, and client._auth_owned = True. The client then closes the caller's auth on client.close() — which the caller did not expect. The old code TypeError'd on duplicate kwargs, so this is a behavior change.

The intent (expressed in the comment "from_env constructs auth locally — caller never owned it") is to set _auth_owned = True only when from_env created the auth object itself. The correct fix:

env_auth = KalshiAuth.try_from_env()
kwargs.setdefault("auth", env_auth)
client = cls(**kwargs)
# Own the auth only when from_env built it (not when the caller passed one in).
client._auth_owned = kwargs.get("auth") is env_auth and env_auth is not None

Same issue exists in async_client.py.

---

2. conftest.py process-wide KALSHI_ALLOW_UNKNOWN_HOST=1 leaks into tests that don't use monkeypatch.delenv

The test-suite escape hatch set at import time in conftest.py:

os.environ["KALSHI_ALLOW_UNKNOWN_HOST"] = "1"

Tests that construct KalshiConfig with a mock host without calling monkeypatch.delenv("KALSHI_ALLOW_UNKNOWN_HOST", ...) will silently have the check disabled. For example, all existing fixture-based tests skip the host validation. This is fine for them, but any new test author who forgets the delenv call will get a false green on security-validation tests.

Consider a brief comment in conftest.py where this is set explaining that new tests which assert the default-fail behaviour must monkeypatch.delenv("KALSHI_ALLOW_UNKNOWN_HOST", raising=False) first. The new tests in test_config.py already do this correctly; it just isn't documented.

---

Minor Issues

3. Pre-existing KALSHI_DEMO docs/code mismatch (not introduced here, but touched)

docs/environment-variables.md says truthy values are "true", "1", "yes", "on" (case-insensitive). The from_env code (both sync and async) checks only == "true":

kwargs.setdefault("demo", os.environ.get("KALSHI_DEMO", "").lower() == "true")

This PR touches both files but doesn't fix the mismatch. Worth a follow-up issue if not already tracked.

4. ClientInitKwargs TypedDict duplicated across client.py and async_client.py

The only difference is transport: httpx.BaseTransport | None vs transport: httpx.AsyncBaseTransport | None. Not worth merging given the project's "simplicity first" guideline, but if this TypedDict ever grows it'll need syncing. Acceptable as-is.

---

What's Well Done

• _normalize_percent_encoding short-circuit: Returning the original object (is identity) not just equality — the test confirms no allocation on the hot path. Clean.
• KALSHI_ALLOW_UNKNOWN_HOST=1 strict equality: Only exact "1" opt-in; a stale =0, =true, or =yes from a prior shell doesn't silently disable the check. test_env_opt_in_only_honors_exact_one verifies this. Good defensive choice.
• Test coverage for TestUnknownHostDefaultFail: Covers known host, unknown host, error message content, flag opt-in, env opt-in, strict "1" semantics, localhost passthrough, and WS URL independently. Thorough.
• test_no_percent_returns_input_identity: Tests is not ==, which is the actual contract. Good discipline.
• conftest.py env stripping at import: Prevents real credentials from polluting the test run. Cleaner than per-test fixtures.

---

Summary

The _auth_owned ownership issue in from_env (item 1) is a real semantic bug introduced by the refactor. The edge case (from_env(auth=...)) is unlikely in practice but the TypedDict makes it a valid, mypy-clean call. Everything else is solid. The _auth_owned fix is a one-liner per client.