[SPT-1668] Skip CA bundle env vars that don't point to a file

If SSL_CERT_FILE or REQUESTS_CA_BUNDLE is set to a path that doesn't exist, log a warning and fall back to the next option instead of failing, so a stale value doesn't break existing setups. An explicit ca_certs argument is still used as given. RESTClient resolves the path once so it warns once for both the main and vx clients.

Author: justinpolygon

README.md

@@ -160,7 +160,7 @@ client = RESTClient(ca_certs="/path/to/corporate-ca-bundle.pem")
 ws_client = WebSocketClient(ca_certs="/path/to/corporate-ca-bundle.pem")
 ```
 
-The client uses the first one it finds: `ca_certs`, then `SSL_CERT_FILE`, then `REQUESTS_CA_BUNDLE`, then the bundled [certifi](https://pypi.org/project/certifi/) certificates.
+The client uses the first one it finds: `ca_certs`, then `SSL_CERT_FILE`, then `REQUESTS_CA_BUNDLE`, then the bundled [certifi](https://pypi.org/project/certifi/) certificates. If an environment variable points to a file that doesn't exist, it's skipped and a warning is logged.
 
 The bundle replaces certifi, so it must also include the public root certificates. If you only have your company's root certificate, add it to certifi's bundle:
 

massive/certs.py

@@ -1,17 +1,27 @@
+import logging
 import os
+from typing import Optional
+
 import certifi
 
 
-def ca_bundle_path() -> str:
+def ca_bundle_path(logger: Optional[logging.Logger] = None) -> str:
     """
     Path of the CA bundle used to verify TLS connections.
 
     Honors the standard ``SSL_CERT_FILE`` and ``REQUESTS_CA_BUNDLE`` environment
     variables (used for example behind corporate TLS inspection), falling back to
-    the certifi bundle.
+    the certifi bundle. A variable that doesn't point to an existing file is
+    skipped with a warning, so a stale value doesn't break connections.
     """
-    return (
-        os.environ.get("SSL_CERT_FILE")
-        or os.environ.get("REQUESTS_CA_BUNDLE")
-        or certifi.where()
-    )
+    for env_var in ("SSL_CERT_FILE", "REQUESTS_CA_BUNDLE"):
+        path = os.environ.get(env_var)
+        if not path:
+            continue
+        if os.path.isfile(path):
+            return path
+        if logger:
+            logger.warning(
+                "%s is set to %r, which isn't a file; ignoring it", env_var, path
+            )
+    return certifi.where()

massive/rest/__init__.py

@@ -20,6 +20,8 @@
     ContractsClient,
 )
 from .vX import VXClient
+from .base import logger
+from ..certs import ca_bundle_path
 from typing import Optional, Any, Dict
 import os
 
@@ -74,6 +76,8 @@ def __init__(
         :param connection_pool_kw: Other urllib3 PoolManager options. They override
             the client's defaults.
         """
+        # resolve once, so the main and vx clients share it and warn only once
+        ca_certs = ca_certs or ca_bundle_path(logger)
         super().__init__(
             api_key=api_key,
             connect_timeout=connect_timeout,

massive/rest/base.py

@@ -83,7 +83,7 @@ def __init__(
             # clients reconnect (and redo the TLS handshake) for most requests
             "maxsize": maxsize,
             "headers": self.headers,  # default headers sent with each request.
-            "ca_certs": ca_certs or ca_bundle_path(),
+            "ca_certs": ca_certs or ca_bundle_path(logger),
             "cert_reqs": "CERT_REQUIRED",
             "retries": retry_strategy,  # use the customized Retry instance
             "timeout": self.timeout,  # set timeout for each request

massive/websocket/__init__.py

@@ -177,7 +177,7 @@ def _ssl_context(self) -> Optional[ssl.SSLContext]:
         if not self.url.startswith("wss://"):
             return None
         context = ssl.SSLContext(ssl.PROTOCOL_TLS_CLIENT)
-        context.load_verify_locations(self.ca_certs or ca_bundle_path())
+        context.load_verify_locations(self.ca_certs or ca_bundle_path(logger))
         return context
 
     def run(

test_rest/test_connection.py

@@ -1,4 +1,5 @@
 import os
+import tempfile
 import unittest
 from unittest import mock
 
@@ -9,6 +10,14 @@
 
 
 class ConnectionOptionsTest(unittest.TestCase):
+    def setUp(self):
+        tmp = tempfile.TemporaryDirectory()
+        self.addCleanup(tmp.cleanup)
+        self.ssl_ca = os.path.join(tmp.name, "ssl-ca.pem")
+        self.requests_ca = os.path.join(tmp.name, "requests-ca.pem")
+        for path in (self.ssl_ca, self.requests_ca):
+            open(path, "w").close()
+
     def client(self, env=None, **kwargs):
         with mock.patch.dict(os.environ, env or {}, clear=True):
             return RESTClient(api_key="key", **kwargs)
@@ -18,15 +27,31 @@ def test_ca_bundle_defaults_to_certifi(self):
         self.assertEqual(c.client.connection_pool_kw["ca_certs"], certifi.where())
 
     def test_ca_bundle_from_env(self):
-        c = self.client({"REQUESTS_CA_BUNDLE": "/requests-ca.pem"})
-        self.assertEqual(c.client.connection_pool_kw["ca_certs"], "/requests-ca.pem")
+        c = self.client({"REQUESTS_CA_BUNDLE": self.requests_ca})
+        self.assertEqual(c.client.connection_pool_kw["ca_certs"], self.requests_ca)
         c = self.client(
-            {"SSL_CERT_FILE": "/ssl-ca.pem", "REQUESTS_CA_BUNDLE": "/requests-ca.pem"}
+            {"SSL_CERT_FILE": self.ssl_ca, "REQUESTS_CA_BUNDLE": self.requests_ca}
         )
-        self.assertEqual(c.client.connection_pool_kw["ca_certs"], "/ssl-ca.pem")
+        self.assertEqual(c.client.connection_pool_kw["ca_certs"], self.ssl_ca)
+
+    def test_missing_env_bundle_is_skipped_with_a_warning(self):
+        with self.assertLogs("RESTClient", level="WARNING") as logs:
+            c = self.client({"SSL_CERT_FILE": "/does-not-exist.pem"})
+        self.assertEqual(c.client.connection_pool_kw["ca_certs"], certifi.where())
+        self.assertEqual(c.vx.client.connection_pool_kw["ca_certs"], certifi.where())
+        self.assertEqual(len(logs.output), 1, logs.output)
+        self.assertIn("SSL_CERT_FILE", logs.output[0])
+        with self.assertLogs("RESTClient", level="WARNING"):
+            c = self.client(
+                {
+                    "SSL_CERT_FILE": "/does-not-exist.pem",
+                    "REQUESTS_CA_BUNDLE": self.requests_ca,
+                }
+            )
+        self.assertEqual(c.client.connection_pool_kw["ca_certs"], self.requests_ca)
 
     def test_ca_certs_argument_wins_over_env(self):
-        c = self.client({"SSL_CERT_FILE": "/ssl-ca.pem"}, ca_certs="/explicit-ca.pem")
+        c = self.client({"SSL_CERT_FILE": self.ssl_ca}, ca_certs="/explicit-ca.pem")
         self.assertEqual(c.client.connection_pool_kw["ca_certs"], "/explicit-ca.pem")
 
     def test_ca_certs_applies_to_vx(self):

test_websocket/test_ssl.py

@@ -1,4 +1,5 @@
 import os
+import tempfile
 import unittest
 from unittest import mock
 
@@ -8,6 +9,14 @@
 
 
 class WebSocketSSLTest(unittest.TestCase):
+    def setUp(self):
+        tmp = tempfile.TemporaryDirectory()
+        self.addCleanup(tmp.cleanup)
+        self.ssl_ca = os.path.join(tmp.name, "ssl-ca.pem")
+        self.requests_ca = os.path.join(tmp.name, "requests-ca.pem")
+        for path in (self.ssl_ca, self.requests_ca):
+            open(path, "w").close()
+
     def ca_path(self, env=None, **kwargs):
         with mock.patch.dict(os.environ, env or {}, clear=True):
             c = WebSocketClient(api_key="key", **kwargs)
@@ -19,14 +28,20 @@ def test_defaults_to_certifi(self):
         self.assertEqual(self.ca_path(), certifi.where())
 
     def test_env(self):
-        self.assertEqual(self.ca_path({"SSL_CERT_FILE": "/ssl-ca.pem"}), "/ssl-ca.pem")
+        self.assertEqual(self.ca_path({"SSL_CERT_FILE": self.ssl_ca}), self.ssl_ca)
         self.assertEqual(
-            self.ca_path({"REQUESTS_CA_BUNDLE": "/requests-ca.pem"}), "/requests-ca.pem"
+            self.ca_path({"REQUESTS_CA_BUNDLE": self.requests_ca}), self.requests_ca
         )
 
+    def test_missing_env_bundle_is_skipped_with_a_warning(self):
+        with self.assertLogs("WebSocketClient", level="WARNING") as logs:
+            path = self.ca_path({"SSL_CERT_FILE": "/does-not-exist.pem"})
+        self.assertEqual(path, certifi.where())
+        self.assertIn("SSL_CERT_FILE", logs.output[0])
+
     def test_ca_certs_argument_wins_over_env(self):
         self.assertEqual(
-            self.ca_path({"SSL_CERT_FILE": "/ssl-ca.pem"}, ca_certs="/explicit-ca.pem"),
+            self.ca_path({"SSL_CERT_FILE": self.ssl_ca}, ca_certs="/explicit-ca.pem"),
             "/explicit-ca.pem",
         )