PYCO-93: Address mTLS PR review feedback

Changes
-------
* Moves Credential._check_replaceable_with to the top of
  update_credential (sync + async) so the type check runs before
  any state changes, on every rotation.
* Drops the duplicate _check_replaceable_with from
  _CredentialHolder.replace; both callers validate first.
* Refactors update_credential to put the holder swap and old-client
  close in the shared path. The cert branch now only handles the
  SSL-context refresh and Client rebuild.
* Logs a warning when the PKCS#12 temporary PEM cleanup fails,
  instead of swallowing the OSError. The cluster request still
  succeeds since the SSL context is already loaded.
* Tightens the cryptography pin from >=41.0 to ~=47.0. Regenerates
  requirements.txt and uv.lock.
* Tidies up two comments (Credential.__slots__ rationale, cert
  chain hook in connection.py).

Author: anirudhlakhotia

acouchbase_analytics/protocol/_core/client_adapter.py

@@ -200,31 +200,29 @@ def reset_client(self) -> None:
             del self._client
 
     async def update_credential(self, new_credential: Credential) -> None:
+        old_client = None
+        self._credential_holder.credential._check_replaceable_with(new_credential)
         if new_credential._kind == 'cert':
             # httpx pins the SSL context to the AsyncClient at construction,
             # and the cert chain is part of that context.  So a cert rotation
             # needs a fresh Client.  Build it before aclosing the old one,
             # otherwise a concurrent send_request can see self._client gone.
-            self._credential_holder.credential._check_replaceable_with(new_credential)
             old_client = getattr(self, '_client', None)
             old_ssl_context = self._conn_details.ssl_context
             old_sni_hostname = self._conn_details.sni_hostname
             try:
                 self._conn_details.validate_security_options(new_credential)
                 # If the cluster hasn't issued a request yet there's no Client
                 # to swap; we still refreshed the SSL context above.
-                new_client = self._build_client() if old_client is not None else None
+                if old_client is not None:
+                    self._client = self._build_client()
             except Exception:
                 self._conn_details.ssl_context = old_ssl_context
                 self._conn_details.sni_hostname = old_sni_hostname
                 raise
-            if new_client is not None:
-                self._client = new_client
-            self._credential_holder.replace(new_credential)
-            if old_client is not None:
-                await old_client.aclose()
-        else:
-            self._credential_holder.replace(new_credential)
+        self._credential_holder.replace(new_credential)
+        if old_client is not None:
+            await old_client.aclose()
         self.log_message('Cluster HTTP credential updated', LogLevel.INFO)
 
 

couchbase_analytics/common/credential.py

@@ -17,6 +17,7 @@
 
 from __future__ import annotations
 
+import logging
 import os
 import ssl
 import tempfile
@@ -29,6 +30,9 @@
     from httpx import Request
 
 
+logger = logging.getLogger(__name__)
+
+
 class Credential:
     """A credential for authenticating with a Couchbase Analytics endpoint.
 
@@ -321,7 +325,12 @@ def _load_pkcs12_into_context(self, ctx: ssl.SSLContext) -> None:
             try:
                 os.unlink(tmp_path)
             except OSError:
-                pass
+                logger.warning(
+                    'Failed to remove temporary PKCS#12 PEM file %s; '
+                    'decrypted private key material may remain on disk.',
+                    tmp_path,
+                    exc_info=True,
+                )
 
     def _check_endpoint_compatible(self, is_secure: bool) -> None:
         if self._kind == 'jwt' and not is_secure:
@@ -365,6 +374,6 @@ def apply_to_request(self, request: Request) -> None:
         self._credential._apply_to_request(request)
 
     def replace(self, new_credential: Credential) -> None:
-        # GIL-atomic store; concurrent rotators are last-writer-wins.
-        self._credential._check_replaceable_with(new_credential)
+        # Caller must have validated replaceability via _check_replaceable_with;
+        # this is a GIL-atomic store, last-writer-wins on concurrent rotators.
         self._credential = new_credential

couchbase_analytics/protocol/_core/client_adapter.py

@@ -199,31 +199,29 @@ def reset_client(self) -> None:
             del self._client
 
     def update_credential(self, new_credential: Credential) -> None:
+        old_client = None
+        self._credential_holder.credential._check_replaceable_with(new_credential)
         if new_credential._kind == 'cert':
             # httpx pins the SSL context to the Client at construction, and
             # the cert chain is part of that context.  So a cert rotation
             # needs a fresh Client.  Build it before closing the old one,
             # otherwise a concurrent send_request can see self._client gone.
-            self._credential_holder.credential._check_replaceable_with(new_credential)
             old_client = getattr(self, '_client', None)
             old_ssl_context = self._conn_details.ssl_context
             old_sni_hostname = self._conn_details.sni_hostname
             try:
                 self._conn_details.validate_security_options(new_credential)
                 # If the cluster hasn't issued a request yet there's no Client
                 # to swap; we still refreshed the SSL context above.
-                new_client = self._build_client() if old_client is not None else None
+                if old_client is not None:
+                    self._client = self._build_client()
             except Exception:
                 self._conn_details.ssl_context = old_ssl_context
                 self._conn_details.sni_hostname = old_sni_hostname
                 raise
-            if new_client is not None:
-                self._client = new_client
-            self._credential_holder.replace(new_credential)
-            if old_client is not None:
-                old_client.close()
-        else:
-            self._credential_holder.replace(new_credential)
+        self._credential_holder.replace(new_credential)
+        if old_client is not None:
+            old_client.close()
         self.log_message('Cluster HTTP credential updated', LogLevel.INFO)
 
 

couchbase_analytics/protocol/connection.py

@@ -255,8 +255,8 @@ def validate_security_options(self, credential: Credential) -> None:  # noqa: C9
             self.ssl_context.verify_mode = ssl.CERT_REQUIRED
 
         # Cert credentials attach their chain here; no-op for password/JWT.
-        # Runs after verify_mode is set so OpenSSL has a fully configured
-        # context when load_cert_chain fires.
+        # Runs after verify_mode is set so the context is finished before
+        # load_cert_chain reads it.
         credential._apply_to_ssl_context(self.ssl_context)
 
     @classmethod

pyproject.toml

@@ -10,7 +10,7 @@ name = "couchbase-analytics"
 version = "1.0.0.dev1"
 dependencies = [
     "anyio~=4.9.0",
-    "cryptography>=41.0",
+    "cryptography~=47.0",
     "httpx~=0.28.1",
     "ijson~=3.4.0",
     "sniffio~=1.3.1",

requirements.in

@@ -2,6 +2,6 @@ anyio~=4.9.0
 sniffio~=1.3.1
 httpx~=0.28.1
 ijson~=3.4.0
-cryptography>=41.0
+cryptography~=47.0
 # Typing support
 typing-extensions~=4.11; python_version<"3.11.0"

requirements.txt

@@ -10,9 +10,7 @@ certifi==2025.6.15
     #   httpx
 cffi==2.0.0 ; platform_python_implementation != 'PyPy'
     # via cryptography
-cryptography==47.0.0 ; python_full_version <= '3.9'
-    # via -r requirements.in
-cryptography==48.0.0 ; python_full_version > '3.9'
+cryptography==47.0.0
     # via -r requirements.in
 exceptiongroup==1.3.0 ; python_full_version < '3.11'
     # via anyio