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

kalshi/async_client.py

@@ -91,18 +91,25 @@ def __init__(
         # silently signing with the wrong key.
         if private_key_path is not None and private_key is not None:
             raise ValueError("Provide either private_key_path or private_key, not both.")
-        # #210: only shut down auth on close() when WE built it. If the
-        # caller passed in their own KalshiAuth, they keep ownership.
-        self._auth_owned: bool = auth is None
+        # #210/#311: ``_auth_owned`` means "this client built the auth and is
+        # responsible for closing it". True only when we constructed the
+        # ``KalshiAuth`` from ``key_id``+private-key kwargs; never when the
+        # caller passed a ``KalshiAuth`` and never when there is no auth at
+        # all (nothing to own).
         self._auth: KalshiAuth | None
+        self._auth_owned: bool
         if auth is not None:
             self._auth = auth
+            self._auth_owned = False
         elif key_id and private_key_path:
             self._auth = KalshiAuth.from_key_path(key_id, private_key_path)
+            self._auth_owned = True
         elif key_id and private_key:
             self._auth = KalshiAuth.from_pem(key_id, private_key)
+            self._auth_owned = True
         else:
             self._auth = None
+            self._auth_owned = False
 
         # Build config
         if config is not None:
@@ -207,16 +214,33 @@ def from_env(cls, **kwargs: Unpack[ClientInitKwargs]) -> AsyncKalshiClient:
 
         Returns an unauthenticated client if no credentials are configured.
         """
-        # Caller-supplied kwargs win over env-derived values (legacy behaviour
-        # via ``cls(auth=..., demo=..., base_url=..., **kwargs)`` would have
-        # TypeError'd on duplicates; setdefault preserves env semantics while
-        # giving the static signature a clean shape). See #266.
-        kwargs.setdefault("auth", KalshiAuth.try_from_env())
+        # #316: only consult env for credentials when the caller did not supply
+        # any explicit credential source. ``setdefault("auth", try_from_env())``
+        # always evaluates the default, so a malformed ``KALSHI_PRIVATE_KEY`` in
+        # the process env would raise even when the caller passed ``auth=`` or
+        # ``key_id``+``private_key``.
+        caller_supplied_auth = (
+            "auth" in kwargs
+            or kwargs.get("key_id") is not None
+            or kwargs.get("private_key") is not None
+            or kwargs.get("private_key_path") is not None
+        )
+        env_built_auth: KalshiAuth | None = None
+        if not caller_supplied_auth:
+            env_built_auth = KalshiAuth.try_from_env()
+            if env_built_auth is not None:
+                kwargs["auth"] = env_built_auth
         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
+        # #311: ``__init__`` treats any passed-in ``auth`` as caller-owned. When
+        # ``from_env`` itself built the auth from env vars, the client (not the
+        # caller) owns it and must shut it down on ``close()``. The constructor
+        # is the source of truth for every other path — caller passed ``auth=``
+        # (caller keeps ownership), caller passed ``key_id``+``private_key``
+        # (client owns), or no credentials at all (nothing to own).
+        if env_built_auth is not None:
+            client._auth_owned = True
         return client
 
     async def close(self) -> None:

kalshi/client.py

@@ -93,18 +93,25 @@ def __init__(
         # silently signing with the wrong key.
         if private_key_path is not None and private_key is not None:
             raise ValueError("Provide either private_key_path or private_key, not both.")
-        # #210: only shut down auth on close() when WE built it. If the
-        # caller passed in their own KalshiAuth, they keep ownership.
-        self._auth_owned: bool = auth is None
+        # #210/#311: ``_auth_owned`` means "this client built the auth and is
+        # responsible for closing it". True only when we constructed the
+        # ``KalshiAuth`` from ``key_id``+private-key kwargs; never when the
+        # caller passed a ``KalshiAuth`` and never when there is no auth at
+        # all (nothing to own).
         self._auth: KalshiAuth | None
+        self._auth_owned: bool
         if auth is not None:
             self._auth = auth
+            self._auth_owned = False
         elif key_id and private_key_path:
             self._auth = KalshiAuth.from_key_path(key_id, private_key_path)
+            self._auth_owned = True
         elif key_id and private_key:
             self._auth = KalshiAuth.from_pem(key_id, private_key)
+            self._auth_owned = True
         else:
             self._auth = None
+            self._auth_owned = False
 
         # Build config
         if config is not None:
@@ -175,16 +182,33 @@ def from_env(cls, **kwargs: Unpack[ClientInitKwargs]) -> KalshiClient:
 
         Returns an unauthenticated client if no credentials are configured.
         """
-        # Caller-supplied kwargs win over env-derived values (legacy behaviour
-        # via ``cls(auth=..., demo=..., base_url=..., **kwargs)`` would have
-        # TypeError'd on duplicates; setdefault preserves env semantics while
-        # giving the static signature a clean shape). See #266.
-        kwargs.setdefault("auth", KalshiAuth.try_from_env())
+        # #316: only consult env for credentials when the caller did not supply
+        # any explicit credential source. ``setdefault("auth", try_from_env())``
+        # always evaluates the default, so a malformed ``KALSHI_PRIVATE_KEY`` in
+        # the process env would raise even when the caller passed ``auth=`` or
+        # ``key_id``+``private_key``.
+        caller_supplied_auth = (
+            "auth" in kwargs
+            or kwargs.get("key_id") is not None
+            or kwargs.get("private_key") is not None
+            or kwargs.get("private_key_path") is not None
+        )
+        env_built_auth: KalshiAuth | None = None
+        if not caller_supplied_auth:
+            env_built_auth = KalshiAuth.try_from_env()
+            if env_built_auth is not None:
+                kwargs["auth"] = env_built_auth
         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
+        # #311: ``__init__`` treats any passed-in ``auth`` as caller-owned. When
+        # ``from_env`` itself built the auth from env vars, the client (not the
+        # caller) owns it and must shut it down on ``close()``. The constructor
+        # is the source of truth for every other path — caller passed ``auth=``
+        # (caller keeps ownership), caller passed ``key_id``+``private_key``
+        # (client owns), or no credentials at all (nothing to own).
+        if env_built_auth is not None:
+            client._auth_owned = True
         return client
 
     def close(self) -> None:

tests/test_async_client.py

@@ -746,6 +746,79 @@ async def test_from_env_owns_auth(
         await client.close()
 
 
+class TestAsyncFromEnvOwnershipAndLazyEval:
+    """#311 / #316: async mirror — from_env must (a) honor ``__init__``'s
+    ownership invariant instead of recomputing from the input kwarg, and (b)
+    only consult ``KALSHI_PRIVATE_KEY`` lazily when the caller did not supply
+    an explicit credential source.
+    """
+
+    @pytest.mark.asyncio
+    async def test_issue_311_from_env_preserves_caller_owned_auth(
+        self, monkeypatch: pytest.MonkeyPatch, pem_string: str, test_auth: KalshiAuth
+    ) -> None:
+        monkeypatch.setenv("KALSHI_KEY_ID", "env-key")
+        monkeypatch.setenv("KALSHI_PRIVATE_KEY", pem_string)
+        monkeypatch.delenv("KALSHI_PRIVATE_KEY_PATH", raising=False)
+        monkeypatch.delenv("KALSHI_DEMO", raising=False)
+        monkeypatch.delenv("KALSHI_API_BASE_URL", raising=False)
+        client = AsyncKalshiClient.from_env(auth=test_auth)
+        assert client._auth is test_auth
+        assert client._auth_owned is False
+        await client.close()
+        headers = test_auth.sign_request("GET", "/trade-api/v2/markets")
+        assert "KALSHI-ACCESS-KEY" in headers
+
+    @pytest.mark.asyncio
+    async def test_issue_311_from_env_owns_auth_built_from_key_id(
+        self, monkeypatch: pytest.MonkeyPatch, pem_string: str
+    ) -> None:
+        monkeypatch.delenv("KALSHI_KEY_ID", raising=False)
+        monkeypatch.delenv("KALSHI_PRIVATE_KEY", raising=False)
+        monkeypatch.delenv("KALSHI_PRIVATE_KEY_PATH", raising=False)
+        monkeypatch.delenv("KALSHI_DEMO", raising=False)
+        monkeypatch.delenv("KALSHI_API_BASE_URL", raising=False)
+        client = AsyncKalshiClient.from_env(key_id="caller-key", private_key=pem_string)
+        assert client._auth is not None
+        assert client._auth.key_id == "caller-key"
+        assert client._auth_owned is True
+        auth = client._auth
+        await client.close()
+        # SDK-owned auth has been shut down; ``_closed`` flips True so a
+        # subsequent ``sign_request_async`` raises (per #267).
+        assert auth._closed is True
+
+    @pytest.mark.asyncio
+    async def test_issue_316_from_env_does_not_eagerly_evaluate_env_when_auth_provided(
+        self, monkeypatch: pytest.MonkeyPatch, test_auth: KalshiAuth
+    ) -> None:
+        monkeypatch.setenv("KALSHI_KEY_ID", "env-key")
+        monkeypatch.setenv("KALSHI_PRIVATE_KEY", "-----BEGIN GARBAGE-----\nnotapem\n")
+        monkeypatch.delenv("KALSHI_PRIVATE_KEY_PATH", raising=False)
+        monkeypatch.delenv("KALSHI_DEMO", raising=False)
+        monkeypatch.delenv("KALSHI_API_BASE_URL", raising=False)
+        client = AsyncKalshiClient.from_env(auth=test_auth)
+        assert client._auth is test_auth
+        assert client._auth_owned is False
+        await client.close()
+
+    @pytest.mark.asyncio
+    async def test_issue_316_from_env_does_not_eagerly_evaluate_env_when_key_id_provided(
+        self, monkeypatch: pytest.MonkeyPatch, pem_string: str
+    ) -> None:
+        monkeypatch.setenv("KALSHI_KEY_ID", "env-key")
+        monkeypatch.setenv("KALSHI_PRIVATE_KEY", "-----BEGIN GARBAGE-----\nnotapem\n")
+        monkeypatch.delenv("KALSHI_PRIVATE_KEY_PATH", raising=False)
+        monkeypatch.delenv("KALSHI_DEMO", raising=False)
+        monkeypatch.delenv("KALSHI_API_BASE_URL", raising=False)
+        client = AsyncKalshiClient.from_env(key_id="caller-key", private_key=pem_string)
+        assert client._auth is not None
+        assert client._auth.key_id == "caller-key"
+        assert client._auth_owned is True
+        await client.close()
+
+
+
 class TestAsyncTransportNetworkRetry:
     """#240: async httpx.TransportError retry policy. Mirrors sync tests."""
 

tests/test_client.py

@@ -1312,6 +1312,86 @@ def test_from_env_no_credentials_owns_nothing(self, monkeypatch: pytest.MonkeyPa
         client.close()
 
 
+class TestFromEnvOwnershipAndLazyEval:
+    """#311 / #316: from_env must (a) honor ``__init__``'s ownership invariant
+    instead of recomputing from the input kwarg, and (b) only consult
+    ``KALSHI_PRIVATE_KEY`` lazily when the caller did not supply an explicit
+    credential source.
+    """
+
+    def test_issue_311_from_env_preserves_caller_owned_auth(
+        self, monkeypatch: pytest.MonkeyPatch, pem_string: str, test_auth: KalshiAuth
+    ) -> None:
+        # Case C: env vars set + caller passes auth=. The previous bug flipped
+        # ``_auth_owned`` to True off the input kwarg and ``close()`` would
+        # tear down the caller's auth, breaking every subsequent sign request.
+        monkeypatch.setenv("KALSHI_KEY_ID", "env-key")
+        monkeypatch.setenv("KALSHI_PRIVATE_KEY", pem_string)
+        monkeypatch.delenv("KALSHI_PRIVATE_KEY_PATH", raising=False)
+        monkeypatch.delenv("KALSHI_DEMO", raising=False)
+        monkeypatch.delenv("KALSHI_API_BASE_URL", raising=False)
+        client = KalshiClient.from_env(auth=test_auth)
+        assert client._auth is test_auth
+        assert client._auth_owned is False
+        client.close()
+        # Caller-owned auth survives client.close() and can still sign.
+        headers = test_auth.sign_request("GET", "/trade-api/v2/markets")
+        assert "KALSHI-ACCESS-KEY" in headers
+
+    def test_issue_311_from_env_owns_auth_built_from_key_id(
+        self, monkeypatch: pytest.MonkeyPatch, pem_string: str
+    ) -> None:
+        # Case B: no env vars, caller passes key_id+private_key. __init__
+        # builds auth and is the legitimate owner; the previous override
+        # recomputed ``_auth_owned`` as ``(None is not None) == False`` and
+        # leaked the sign ThreadPoolExecutor on close().
+        monkeypatch.delenv("KALSHI_KEY_ID", raising=False)
+        monkeypatch.delenv("KALSHI_PRIVATE_KEY", raising=False)
+        monkeypatch.delenv("KALSHI_PRIVATE_KEY_PATH", raising=False)
+        monkeypatch.delenv("KALSHI_DEMO", raising=False)
+        monkeypatch.delenv("KALSHI_API_BASE_URL", raising=False)
+        client = KalshiClient.from_env(key_id="caller-key", private_key=pem_string)
+        assert client._auth is not None
+        assert client._auth.key_id == "caller-key"
+        assert client._auth_owned is True
+        auth = client._auth
+        client.close()
+        # SDK-owned auth has been shut down; ``_closed`` flips True so a
+        # subsequent ``sign_request_async`` raises (per #267). The sync
+        # ``sign_request`` path intentionally still works on a retired auth.
+        assert auth._closed is True
+
+    def test_issue_316_from_env_does_not_eagerly_evaluate_env_when_auth_provided(
+        self, monkeypatch: pytest.MonkeyPatch, test_auth: KalshiAuth
+    ) -> None:
+        # Malformed PEM lingering in env must not block an explicit ``auth=``.
+        monkeypatch.setenv("KALSHI_KEY_ID", "env-key")
+        monkeypatch.setenv("KALSHI_PRIVATE_KEY", "-----BEGIN GARBAGE-----\nnotapem\n")
+        monkeypatch.delenv("KALSHI_PRIVATE_KEY_PATH", raising=False)
+        monkeypatch.delenv("KALSHI_DEMO", raising=False)
+        monkeypatch.delenv("KALSHI_API_BASE_URL", raising=False)
+        client = KalshiClient.from_env(auth=test_auth)
+        assert client._auth is test_auth
+        assert client._auth_owned is False
+        client.close()
+
+    def test_issue_316_from_env_does_not_eagerly_evaluate_env_when_key_id_provided(
+        self, monkeypatch: pytest.MonkeyPatch, pem_string: str
+    ) -> None:
+        # A malformed env PEM must not block explicit ``key_id``+``private_key``.
+        monkeypatch.setenv("KALSHI_KEY_ID", "env-key")
+        monkeypatch.setenv("KALSHI_PRIVATE_KEY", "-----BEGIN GARBAGE-----\nnotapem\n")
+        monkeypatch.delenv("KALSHI_PRIVATE_KEY_PATH", raising=False)
+        monkeypatch.delenv("KALSHI_DEMO", raising=False)
+        monkeypatch.delenv("KALSHI_API_BASE_URL", raising=False)
+        client = KalshiClient.from_env(key_id="caller-key", private_key=pem_string)
+        assert client._auth is not None
+        assert client._auth.key_id == "caller-key"
+        assert client._auth_owned is True
+        client.close()
+
+
+
 class TestExtraHeadersPerRequest:
     """P1.2: SyncTransport.request accepts per-call ``extra_headers`` that
     layer on top of config-level ``extra_headers`` but never override the