PYTHON-5930 Fix SSL_CERT_FILE/SSL_CERT_DIR trust store handling by blink1073 · Pull Request #2925 · mongodb/mongo-python-driver

blink1073 · 2026-07-09T22:08:05Z

Changes in this PR

Fixes a test failure caused by SSL_CERT_FILE/SSL_CERT_DIR being merged with the OS/certifi certificate store instead of replacing it, on Windows and on macOS when using PyOpenSSL. The fix is scoped to just those platform/backend combinations, since elsewhere load_default_certs() already honors these env vars correctly (including falling back to the platform default for whichever of the two is left unset).

Also fixes an unrelated Evergreen config bug found while validating this change: the pyopenssl-macos variant never actually ran on macOS. create_pyopenssl_variants() wasn't passing host=host to create_variant(), so it silently fell back to a RHEL host. The pyopenssl-win64 variant only worked by accident because of a separate win64 name-based special case. Without this fix, the macOS+PyOpenSSL half of this change could never have been verified in CI.

Test Plan

Ran just typing and just lint; both pass. Added unit tests covering both the Windows bypass and the preserved fallback behavior elsewhere. Confirmed with an Evergreen patch build across test-win64, pyopenssl-win64, and pyopenssl-macos (now genuinely running on macos-14): all tasks passed — patch #10613.

Checklist

Checklist for Author

Did you update the changelog (if necessary)?
Is there test coverage?
Is any followup work tracked in a JIRA ticket? If so, add link(s).

Checklist for Reviewer

Does the title of the PR reference a JIRA Ticket?
Do you fully understand the implementation? (Would you be comfortable explaining how this code works to someone else?)
Is all relevant documentation (README or docstring) updated?

Honor SSL_CERT_FILE/SSL_CERT_DIR as the exclusive trust source across platforms instead of merging them with the OS/certifi certificate store.

…+PyOpenSSL Only bypass load_default_certs() where it actually merges an OS/certifi store (win32, and darwin with PyOpenSSL). Everywhere else, load_default_certs() already honors these env vars correctly, including falling back to the platform default for whichever of the two is left unset -- calling load_verify_locations() unconditionally dropped that fallback, silently narrowing the trust store e.g. on Linux when only one var was set. Adds regression tests covering both branches.

…g host create_pyopenssl_variants() never passed host=host to create_variant(), unlike create_encryption_variants(). The win64 case worked by accident via create_variant()'s display-name-based win64 special case, but pyopenssl-macos silently fell back to the rhel87-small default and never actually ran on macOS, so it couldn't validate macOS-specific PyOpenSSL behavior.

codecov-commenter · 2026-07-10T00:15:02Z

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

Copilot

Pull request overview

This PR addresses PYTHON-5930 by correcting how PyMongo constructs its TLS trust store when SSL_CERT_FILE/SSL_CERT_DIR are set, ensuring those env vars replace (not merge with) the OS/certifi store on affected platform/backend combinations. It also fixes an Evergreen variant-generation issue so the PyOpenSSL macOS variant actually runs on a macOS host.

Changes:

Adjusted get_ssl_context() to bypass load_default_certs() only on Windows and on macOS when using PyOpenSSL when SSL_CERT_FILE/SSL_CERT_DIR are set.
Added regression tests for Windows bypass behavior and for preserving default fallback behavior off Windows.
Fixed Evergreen PyOpenSSL variant generation by correctly passing the host through, and updated the generated variants YAML accordingly.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
`pymongo/ssl_support.py`	Implements conditional bypass of `load_default_certs()` so `SSL_CERT_FILE`/`SSL_CERT_DIR` replace the trust store on Windows and macOS+PyOpenSSL.
`test/asynchronous/test_ssl.py`	Adds unit tests around `SSL_CERT_FILE` behavior (Windows bypass + non-Windows fallback).
`test/test_ssl.py`	Sync-mirrored equivalent of the new SSL env-var tests.
`doc/changelog.rst`	Documents the Windows/macOS+PyOpenSSL trust-store merge bugfix.
`.evergreen/scripts/generate_config.py`	Fixes PyOpenSSL variant generation by passing `host=host` to `create_variant()`.
`.evergreen/generated_configs/variants.yml`	Updates the generated config so “PyOpenSSL macOS” runs on `macos-14` instead of a RHEL host.

Addresses review feedback: the darwin+PyOpenSSL bypass path had no direct test coverage, only the win32 bypass and the non-Windows fallback were exercised.

blink1073 · 2026-07-10T11:48:16Z

I'm investigating the ECS failure separately. It is failing on the waterfall as well.

NoahStapp · 2026-07-10T13:51:24Z

    def test_use_pyopenssl_when_available(self):
        self.assertTrue(HAVE_PYSSL)

+    def test_ssl_cert_file_env_var_preserves_default_fallback_off_windows(self):


This test has "windows" in the name but tests correct behavior for Linux? The comment is also unclear, I don't really follow.

Renamed to test_ssl_cert_file_env_var_uses_default_certs_on_linux and simplified the comment.

NoahStapp · 2026-07-10T13:52:17Z

+            mock_verify.assert_not_called()
+
+    def test_ssl_cert_file_env_var_bypasses_default_certs_on_windows(self):
+        # PYTHON-5930: on win32 (and macOS+PyOpenSSL), load_default_certs() merges


I don't think this comment needs to mention macOS + PyOpenSSL since the following test does.

Dropped the macOS+PyOpenSSL mention from this comment, good catch.

NoahStapp · 2026-07-10T13:52:46Z

    def test_use_pyopenssl_when_available(self):
        self.assertTrue(HAVE_PYSSL)

+    def test_ssl_cert_file_env_var_preserves_default_fallback_off_windows(self):


These tests don't need to be async/sync, can we consolidate into a single shared test file?

I think that should be deferred to the code consolidation work

I'd prefer not to add code that we know is already duplicated at creation.

Went further than just the 3 SSL_CERT_FILE tests: moved the entire TestClientSSL class (all pure unit tests, no async dependency) to a new non-mirrored test/test_ssl_support.py, following the existing test_pyopenssl_context.py precedent for standalone test files. Removed it from both test/asynchronous/test_ssl.py and the generated test/test_ssl.py.

Fair enough, I moved all the unit tests to a new module

Co-authored-by: Noah Stapp <noah@noahstapp.com>

Addresses review feedback: rename the Linux fallback test to name the platform it actually covers, and drop the redundant macOS+PyOpenSSL mention from the Windows test's comment since a dedicated test now covers that case.

Addresses review feedback: the tests in TestClientSSL are pure unit tests (mocked or manual asyncio.run()) with no dependency on the async/sync test class machinery, so they were needlessly duplicated by synchro. Moved them to test/test_ssl_support.py, following the existing test_pyopenssl_context.py precedent for standalone test files that aren't generated/mirrored.

NoahStapp · 2026-07-10T17:29:04Z

+CRL_PEM = os.path.join(CERT_PATH, "crl.pem")
+
+
+class TestClientSSL(unittest.TestCase):


Shouldn't every test in this class (except the first) have @unittest.skipUnless(HAVE_SSL) or @unittest.skipUnless(_HAVE_PYOPENSSL)?

NoahStapp · 2026-07-10T17:31:05Z

+CERT_PATH = os.path.join(pathlib.Path(__file__).resolve().parent, "certificates")
+CLIENT_PEM = os.path.join(CERT_PATH, "client.pem")
+CA_PEM = os.path.join(CERT_PATH, "ca.pem")
+CRL_PEM = os.path.join(CERT_PATH, "crl.pem")


CERT_PATH, CLIENT_PATH, and CA_PEM are all already defined in helpers_shared.py.

NoahStapp · 2026-07-10T17:33:22Z

+            mock_verify.assert_called_once_with(cafile=CA_PEM, capath=None)
+            mock_default.assert_not_called()
+
+    def test_ssl_session_cache(self):


This test doesn't appear to test anything beside list behavior.

NoahStapp · 2026-07-10T17:34:02Z

+    @unittest.skipIf(HAVE_SSL, "The ssl module is available, can't test what happens without it.")
+    def test_no_ssl_module(self):
+        # Explicit
+        self.assertRaises(ConfigurationError, MongoClient, ssl=True)


Prefer using PyMongoTestCase.simple_client here to avoid possible leaks if an assertion fails.

NoahStapp · 2026-07-10T17:34:06Z

+CRL_PEM = os.path.join(CERT_PATH, "crl.pem")
+
+
+class TestClientSSL(unittest.TestCase):


Suggested change

class TestClientSSL(unittest.TestCase):

class TestClientSSL(PyMongoTestCase):

Co-authored-by: Noah Stapp <noah@noahstapp.com>

blink1073 · 2026-07-10T17:50:53Z

Making a new file was a mistake, I'm going to revert it. This is exactly the scope creep I was trying to avoid.

… file" This reverts commit 9e55d86.

…into PYTHON-5930

NoahStapp · 2026-07-10T18:20:52Z

Making a new file was a mistake, I'm going to revert it. This is exactly the scope creep I was trying to avoid.

Can we move just the new tests there and open a follow-up ticket to refactor the existing test suite? Avoids scope creep but doesn't add known tech debt here.

blink1073 added 2 commits July 9, 2026 16:54

PYTHON-5930 Fix Windows SSL_CERT_FILE/SSL_CERT_DIR trust store handling

28690bb

Honor SSL_CERT_FILE/SSL_CERT_DIR as the exclusive trust source across platforms instead of merging them with the OS/certifi certificate store.

blink1073 changed the title ~~PYTHON-5930 Fix Windows SSL_CERT_FILE/SSL_CERT_DIR trust store handling~~ PYTHON-5930 Fix SSL_CERT_FILE/SSL_CERT_DIR trust store handling Jul 9, 2026

PYTHON-5930 Trim comment in get_ssl_context

b6ffcf6

blink1073 marked this pull request as ready for review July 10, 2026 11:19

blink1073 requested a review from a team as a code owner July 10, 2026 11:19

blink1073 requested review from NoahStapp and Copilot July 10, 2026 11:19

Copilot started reviewing on behalf of blink1073 July 10, 2026 11:19 View session

Copilot AI reviewed Jul 10, 2026

View reviewed changes

Comment thread test/asynchronous/test_ssl.py

PYTHON-5930 Add regression test for macOS+PyOpenSSL bypass branch

f99e779

Addresses review feedback: the darwin+PyOpenSSL bypass path had no direct test coverage, only the win32 bypass and the non-Windows fallback were exercised.

NoahStapp requested changes Jul 10, 2026

View reviewed changes

blink1073 and others added 2 commits July 10, 2026 08:54

Update pymongo/ssl_support.py

e3724fb

Co-authored-by: Noah Stapp <noah@noahstapp.com>

blink1073 requested a review from NoahStapp July 10, 2026 14:09

NoahStapp requested changes Jul 10, 2026

View reviewed changes

Update pymongo/ssl_support.py

2552339

Co-authored-by: Noah Stapp <noah@noahstapp.com>

Revert "PYTHON-5930 Move TestClientSSL to a shared, non-mirrored test…

4b0f06d

… file" This reverts commit 9e55d86.

blink1073 requested a review from NoahStapp July 10, 2026 17:54

Merge branch 'PYTHON-5930' of github.com:blink1073/mongo-python-driver …

ce1282b

…into PYTHON-5930

blink1073 closed this Jul 10, 2026

		CRL_PEM = os.path.join(CERT_PATH, "crl.pem")


		class TestClientSSL(unittest.TestCase):

	class TestClientSSL(unittest.TestCase):
	class TestClientSSL(PyMongoTestCase):

Uh oh!

Conversation

blink1073 commented Jul 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Changes in this PR

Test Plan

Checklist

Checklist for Author

Checklist for Reviewer

Uh oh!

codecov-commenter commented Jul 10, 2026

Codecov Report

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

blink1073 commented Jul 10, 2026

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

blink1073 commented Jul 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

NoahStapp commented Jul 10, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

blink1073 commented Jul 9, 2026 •

edited

Loading

blink1073 commented Jul 10, 2026 •

edited

Loading