PYTHON-6147 Add CSFLE HTTP proxy KMS connect helpers by blink1073 · Pull Request #3101 · mongodb/mongo-python-driver

blink1073 · 2026-10-07T11:22:28Z

PYTHON-6147

Changes in this PR

Add convenience kms_connect_callback helpers HTTPProxyKMSConnect and AsyncHTTPProxyKMSConnect.
The helpers tunnel CSFLE/QE KMS connections through a forward proxy with HTTP CONNECT, optionally using TLS.
Support a validated headers parameter (e.g. Proxy-Authorization) for authenticated proxies; names must be RFC 7230 tokens and hosts/headers are checked to prevent request-line injection.
Most of the changes are the tests.

Test Plan

New unit and integration tests for both helpers.

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?

codecov · 2026-10-07T12:03:00Z

Codecov Report

❌ Patch coverage is 89.41799% with 20 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
pymongo/_kms_connect_shared.py	89.41%	18 Missing and 2 partials ⚠️

📢 Thoughts on this report? Let us know!

Add HTTPProxyKMSConnect and AsyncHTTPProxyKMSConnect, ready-made kms_connect_callback implementations that tunnel KMS connections through a forward proxy with HTTP CONNECT, optionally over TLS to the proxy. A headers parameter supports proxies that require authentication (e.g. Proxy-Authorization: Basic); header names and values are validated at construction to prevent request-line injection. The helpers are configured with a proxy_url (e.g. http://proxy.example.com:8080). An https scheme implies TLS to the proxy, defaulting to ssl.create_default_context(). Userinfo in the URL is percent-decoded and sent as a Proxy-Authorization basic auth header. The helpers live in the shared pymongo/_kms_connect_shared.py extracted by PYTHON-6154, re-exported from pymongo.encryption_options.

Copilot

🟡 Changes recommended

Proxy validation currently permits a newline injection case and can expose proxy credentials in errors.

1 open finding

Normalize urlsplit ValueError as proxy configuration errors · New

What changed in this PR

Adds synchronous and asynchronous HTTP CONNECT proxy helpers for CSFLE/QE KMS connections, including TLS proxies and authenticated headers.

Changes:

Implements proxy URL parsing, CONNECT tunneling, TLS bridging, and timeout handling.
Exposes and documents both helper classes.
Adds extensive unit and integration coverage.

File	Description
`pymongo/_kms_connect_shared.py`	Implements proxy helpers and validation.
`pymongo/encryption_options.py`	Exposes the public helpers.
`pymongo/asynchronous/_kms_connect.py`	Improves async callback errors.
`pymongo/asynchronous/encryption.py`	Documents the async helper.
`pymongo/synchronous/_kms_connect.py`	Improves sync callback errors.
`pymongo/synchronous/encryption.py`	Documents the sync helper.
`tools/synchro.py`	Adds helper name synchronization.
`test/test_kms_connect.py`	Adds comprehensive unit tests.
`test/asynchronous/test_kms_connect_prose.py`	Adds async integration tests.
`test/test_kms_connect_prose.py`	Adds generated sync integration tests.
`test/asynchronous/test_encryption.py`	References the relocated async tests.
`test/test_encryption.py`	References the relocated sync tests.
`doc/changelog.rst`	Announces proxy support.

🧠 Review effort: Balanced

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

urllib.parse.urlsplit raises ValueError for malformed URLs, e.g. an unmatched IPv6 bracket, which escaped the helper constructors instead of the ConfigurationError the other invalid proxy URLs raise. Normalize it with the same message the invalid port raises, and cover the malformed bracket forms in the URL validation test.

Copilot

🟡 Changes recommended

Header validation accepts a trailing newline, DNS resolution can exceed the connection deadline, and integration tests assume proxy services that their setup does not start.

2 open findings

1 resolved since last review

Normalize urlsplit ValueError as proxy configuration errors

Previously missed (1)

In code that hasn't changed since last review

Proxy DNS resolution can exceed the connection timeout

pymongo/_kms_connect_shared.py:323

getaddrinfo runs before any call to _remaining(deadline), so a slow or stuck proxy DNS lookup can block the synchronous helper past context.timeout. In the asynchronous helper, the caller can time out while the executor thread remains blocked in DNS, consuming a worker until resolution returns. Use a bounded resolution approach that accounts for the same connection deadline, or otherwise make this timeout limitation explicit and avoid leaving unbounded executor work.

🧠 Review effort: Lite

+KMS_PROXY_HOST = "127.0.0.1"
+KMS_PROXY_PORT = 9004
+KMS_TLS_PROXY_PORT = 9005


Copilot

🟡 Changes recommended

Tunnel reliability and integration-test proxy setup remain unresolved.

2 open findings

1 resolved since last review

Token validation accepts header names ending with a newline

Previously missed (2)

In code that hasn't changed since last review

Avoid shutting down both relay directions on half-close

pymongo/_kms_connect_shared.py:269

A half-close in either direction ends that relay, but SHUT_RDWR on dst also cuts off the opposite relay, and closing src here can close a socket that relay is still using. For example, if the driver finishes sending its request and half-closes, the proxy's KMS reply may be lost. Propagate normal EOF with a write-side shutdown and coordinate socket closure until both directions are finished; reserve full shutdown for teardown or unrecoverable errors.

Include DNS resolution within the connection timeout

pymongo/_kms_connect_shared.py:328

getaddrinfo() runs synchronously before the socket timeout is set. If proxy DNS stalls, the helper can exceed context.timeout: the synchronous callback stays blocked, and the asynchronous helper keeps an executor thread occupied even if a caller with timeoutMS stops waiting. Bound resolution as part of the connection budget, or make the timeout limitation explicit if DNS cannot be interrupted.

🧠 Review effort: Lite

Copilot

🔵 Needs a closer look

The proxy can route an explicit port incorrectly, and connection attempts can exceed their timeout.

2 open findings

Previously missed (3)

In code that hasn't changed since last review

Explicit proxy port zero is incorrectly replaced by a default

pymongo/_kms_connect_shared.py:174

An explicit :0 parses as port 0, but port or ... replaces it with 80 (or 443 for HTTPS). That silently sends the CONNECT request, including any configured proxy credentials, to a different port than requested. Default only when split.port is None; reject zero if it is not a valid proxy port.

Proxy DNS resolution can exceed the connection deadline

pymongo/_kms_connect_shared.py:347

When proxy DNS is slow, this synchronous getaddrinfo runs before any deadline check. The synchronous helper can block past context.timeout, and the asynchronous helper can leave an executor worker occupied even if its caller stops waiting. Bound proxy name resolution or the overall operation to the remaining connect budget; a socket timeout applied after DNS returns cannot enforce it.

Async KMS connection can wait past the deadline

pymongo/_kms_connect_shared.py:397

The captured deadline does not limit this await: when the default executor is backed up or proxy DNS stalls, asyncio.shield(future) can wait past context.timeout. For explicit encryption there is no separate CSOT wait in _connect_kms, so the KMS operation can hang beyond its connect budget. Wait only for the remaining deadline here, and close a socket returned by the executor after either timeout or cancellation.

🧠 Review effort: Lite

Copilot

🟡 Changes recommended

Header validation can be bypassed, credentials can appear in errors, and async timeout cleanup can leave connections open.

3 open findings

1 resolved since last review

Start required HTTP and HTTPS proxy endpoints in test setup

Previously missed (3)

In code that hasn't changed since last review

Redact proxy credentials from URL validation errors

pymongo/_kms_connect_shared.py:178

Validation errors include proxy_url!r, which exposes userinfo such as user:password if a configured URL has an invalid port (and other malformed-URL paths do the same). These exceptions may be logged with the proxy password intact. Redact userinfo in every URL-validation error, or report only the invalid component.

Prevent post-validation CONNECT header mutation

pymongo/_kms_connect_shared.py:188

self.headers is a mutable public dict after validation. A caller can later set callback.headers["X-Trace"] = "ok\r\nInjected: yes" or add a Host field; _tunnel serializes the modified values without rechecking them, bypassing the CONNECT request-injection guard. Store an immutable validated mapping for transmission or validate immediately before constructing the request.

Reject control characters in CONNECT header values

pymongo/_kms_connect_shared.py:201

The value check allows NUL, vertical tab, and other control characters besides CR/LF, and _tunnel sends those bytes in CONNECT headers. Such values are invalid HTTP field values and can be interpreted inconsistently by proxies. Reject disallowed control characters (while permitting legal space and horizontal tab) before storing the headers.

🧠 Review effort: Lite

+            return await asyncio.wait_for(asyncio.shield(future), _remaining(deadline))
+        except asyncio.CancelledError:
+            # The thread runs on regardless, so close the socket it returns.
+            future.add_done_callback(_close_late_socket)


+        except TimeoutError:
+            if future.done():
+                # The executor task timed out itself; report it directly.
+                raise
+            # The budget ran out with the thread still busy. The thread
+            # runs on regardless, so close the socket it returns, and
+            # report the deadline.
+            future.add_done_callback(_close_late_socket)
+            raise socket.timeout("timed out connecting through the proxy") from None


blink1073 force-pushed the PYTHON-6147 branch from 8ba4c81 to 7cc3d01 Compare October 7, 2026 13:25

blink1073 force-pushed the PYTHON-6147 branch from 9781f44 to 29cfcc0 Compare October 9, 2026 22:33

blink1073 requested a balanced review from Copilot October 9, 2026 22:35

Copilot started reviewing on behalf of blink1073 October 9, 2026 22:36 View session

Copilot AI reviewed Oct 9, 2026

View reviewed changes

Comment thread pymongo/_kms_connect_shared.py Outdated

blink1073 requested a lite review from Copilot October 9, 2026 23:01

Copilot started reviewing on behalf of blink1073 October 9, 2026 23:01 View session

Copilot AI reviewed Oct 9, 2026

View reviewed changes

Comment thread pymongo/_kms_connect_shared.py Outdated

Comment thread test/asynchronous/test_kms_connect_prose.py

Comment on lines +29 to +31

KMS_PROXY_HOST = "127.0.0.1"

KMS_PROXY_PORT = 9004

KMS_TLS_PROXY_PORT = 9005

Require a full token match for proxy header names

4c6ab22

blink1073 requested a lite review from Copilot October 9, 2026 23:19

Copilot started reviewing on behalf of blink1073 October 9, 2026 23:20 View session

Copilot AI reviewed Oct 9, 2026

View reviewed changes

Comment thread test/test_kms_connect_prose.py

PYTHON-6147 Keep the tunnel reverse direction alive on a half-close

ef5c2f6

blink1073 requested a lite review from Copilot October 10, 2026 00:52

Copilot started reviewing on behalf of blink1073 October 10, 2026 00:52 View session

Copilot AI reviewed Oct 10, 2026

View reviewed changes

PYTHON-6147 Reject proxy port zero and bound the async proxy wait

1a0e819

blink1073 requested a lite review from Copilot October 10, 2026 01:15

Copilot started reviewing on behalf of blink1073 October 10, 2026 01:15 View session

Copilot AI reviewed Oct 10, 2026

View reviewed changes

Navigation Menu

Repository navigation

PYTHON-6147 Add CSFLE HTTP proxy KMS connect helpers - #3101

PYTHON-6147 Add CSFLE HTTP proxy KMS connect helpers#3101
blink1073 wants to merge 5 commits into
mongodb:mainfrom
blink1073:PYTHON-6147

blink1073 commented Oct 7, 2026 •

edited

Loading

Uh oh!

codecov Bot commented Oct 7, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI left a comment

Uh oh!

This branch has not been deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

blink1073 commented Oct 7, 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 Bot commented Oct 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🔵 Needs a closer look

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

This branch has not been deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

blink1073 commented Oct 7, 2026 •

edited

Loading

codecov Bot commented Oct 7, 2026 •

edited

Loading