Skip to content

PYTHON-5930 Fix SSL_CERT_FILE/SSL_CERT_DIR trust store handling - #2965

Open
blink1073 wants to merge 14 commits into
mongodb:mainfrom
blink1073:PYTHON-5930
Open

PYTHON-5930 Fix SSL_CERT_FILE/SSL_CERT_DIR trust store handling#2965
blink1073 wants to merge 14 commits into
mongodb:mainfrom
blink1073:PYTHON-5930

Conversation

@blink1073

@blink1073 blink1073 commented Jul 28, 2026

Copy link
Copy Markdown
Member

PYTHON-5930

Replaces #2925, which GitHub auto-closed when master was deleted during the rename to main. Prior review history is there.

Changes in this PR

Fixes SSL_CERT_FILE/SSL_CERT_DIR being merged with the OS/certifi certificate store instead of replacing it, on Windows and on macOS with PyOpenSSL. Other platforms already behave correctly and are unaffected.

Also fixes an Evergreen config bug found while validating this: the pyopenssl-macos variant was never running on macOS, so this fix could not otherwise have been verified in CI.

Test Plan

Added regression tests in a new test/test_ssl_support.py. just typing and just lint pass. Passing patch build with test-win64, pyopenssl-win64, and pyopenssl-macos.

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). (PYTHON-5972 for other tests that should be included in test_ssl_support.py).

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?

blink1073 and others added 13 commits July 9, 2026 16:54
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.
Addresses review feedback: the darwin+PyOpenSSL bypass path had no direct
test coverage, only the win32 bypass and the non-Windows fallback were
exercised.
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.
Co-authored-by: Noah Stapp <noah@noahstapp.com>
The three new SSL_CERT_FILE/SSL_CERT_DIR regression tests are pure unit
tests with no async/sync variants, so mirroring them via synchro adds
duplicated code. Move just those tests to a new non-mirrored
test/test_ssl_support.py, following the test_pyopenssl_context.py
precedent. The rest of TestClientSSL is left in place; refactoring the
existing suite is follow-up work.
Copilot AI review requested due to automatic review settings July 28, 2026 17:18
@blink1073
blink1073 requested a review from a team as a code owner July 28, 2026 17:18
@blink1073
blink1073 requested a review from sleepyStick July 28, 2026 17:18
@blink1073
blink1073 marked this pull request as draft July 28, 2026 17:18

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes TLS trust store selection when SSL_CERT_FILE and/or SSL_CERT_DIR are set, ensuring they replace (rather than get merged with) the OS/certifi trust store on Windows and on macOS when using PyOpenSSL. It also corrects an Evergreen variant configuration so the macOS+PyOpenSSL path is actually exercised in CI.

Changes:

  • Update get_ssl_context() to bypass load_default_certs() on Windows and macOS+PyOpenSSL when SSL_CERT_FILE/SSL_CERT_DIR are present, and instead load only the env-provided trust roots.
  • Add unit regression tests covering the platform/backend-specific behavior.
  • Fix Evergreen variant generation so pyopenssl-macos runs on macos-14, and update the generated variants config accordingly.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.

Show a summary per file
File Description
test/test_ssl_support.py Adds targeted unit tests validating env var trust store replacement vs default cert loading across platforms/backends.
pymongo/ssl_support.py Implements the conditional bypass of load_default_certs() when it would incorrectly merge OS/certifi trust roots.
doc/changelog.rst Documents the bug fix for Windows and macOS+PyOpenSSL trust store handling.
.evergreen/scripts/generate_config.py Ensures the PyOpenSSL macOS variant passes host=host so it schedules on macOS.
.evergreen/generated_configs/variants.yml Updates the generated variant to run on macos-14 (reflecting the generator fix).

@codecov-commenter

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@blink1073

Copy link
Copy Markdown
Member Author

Failures are pre-existing ECS and backoff timing flakes.

@blink1073
blink1073 marked this pull request as ready for review July 29, 2026 13:31
@codeowners-service-app

Copy link
Copy Markdown

Assigned caseyclements for team dbx-python because sleepyStick is out of office.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants