Skip to content

Commit 4ffd565

Browse files
fix: prevent credential_process hang on Windows in stdio transport mode (#293)
* fix: prevent credential_process hang on Windows in stdio transport mode Monkey-patch botocore's ProcessProvider to pass stdin=subprocess.DEVNULL when spawning credential_process subprocesses. Without this, the child inherits the MCP JSON-RPC pipe as stdin, causing Popen.communicate() to hang indefinitely on Windows (IOCP) due to the open pipe handle. Fixes D454977820 * refactor: patch _popen instead of copying _retrieve_credentials_using Wrap ProcessProvider._popen via __init__ to inject stdin=DEVNULL, rather than replacing the entire _retrieve_credentials_using method. This is less fragile as it doesn't duplicate botocore's credential parsing logic. * chore: trigger PR sync * fix: suppress pyright type error for mock popen argument * test: simplify credential_process stdin tests to focus on the patch
1 parent 9da6e41 commit 4ffd565

2 files changed

Lines changed: 125 additions & 1 deletion

File tree

mcp_proxy_for_aws/sigv4_helper.py

Lines changed: 29 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,9 +18,10 @@
1818
import httpx
1919
import json
2020
import logging
21+
import subprocess
2122
from botocore.auth import SigV4Auth
2223
from botocore.awsrequest import AWSRequest
23-
from botocore.credentials import Credentials
24+
from botocore.credentials import Credentials, ProcessProvider
2425
from functools import partial
2526
from httpx import __version__ as httpx_version
2627
from mcp_proxy_for_aws import __version__
@@ -30,6 +31,33 @@
3031

3132
logger = logging.getLogger(__name__)
3233

34+
35+
def _patch_credential_process_stdin():
36+
"""Patch botocore ProcessProvider to pass stdin=subprocess.DEVNULL.
37+
38+
When mcp-proxy-for-aws runs in stdio transport mode, its stdin is the MCP
39+
JSON-RPC pipe. Botocore's ProcessProvider spawns credential_process
40+
subprocesses without specifying stdin, so the child inherits the MCP pipe.
41+
On Windows this causes the subprocess to hang indefinitely because it holds
42+
an open handle to the pipe, blocking Popen.communicate() from completing.
43+
"""
44+
original_init = ProcessProvider.__init__
45+
46+
def _patched_init(self, *args, **kwargs):
47+
original_init(self, *args, **kwargs)
48+
original_popen = self._popen
49+
50+
def _popen_with_devnull_stdin(*popen_args, **popen_kwargs):
51+
popen_kwargs.setdefault('stdin', subprocess.DEVNULL)
52+
return original_popen(*popen_args, **popen_kwargs)
53+
54+
self._popen = _popen_with_devnull_stdin
55+
56+
ProcessProvider.__init__ = _patched_init
57+
58+
59+
_patch_credential_process_stdin()
60+
3361
# Headers that should be redacted when logging to prevent credential exposure
3462
SENSITIVE_HEADERS = frozenset({'authorization', 'x-amz-security-token', 'x-amz-date'})
3563

Lines changed: 96 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,96 @@
1+
# Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
2+
#
3+
# Licensed under the Apache License, Version 2.0 (the "License");
4+
# you may not use this file except in compliance with the License.
5+
# You may obtain a copy of the License at
6+
#
7+
# http://www.apache.org/licenses/LICENSE-2.0
8+
#
9+
# Unless required by applicable law or agreed to in writing, software
10+
# distributed under the License is distributed on an "AS IS" BASIS,
11+
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
# See the License for the specific language governing permissions and
13+
# limitations under the License.
14+
15+
"""Tests for the credential_process stdin isolation patch in sigv4_helper.
16+
17+
When mcp-proxy-for-aws runs in stdio transport mode, its stdin is the MCP
18+
JSON-RPC pipe. Without the patch, botocore's ProcessProvider would spawn
19+
credential_process subprocesses that inherit the pipe, causing hangs on Windows.
20+
"""
21+
22+
import mcp_proxy_for_aws.sigv4_helper # noqa: F401 - ensure patch is applied
23+
import subprocess
24+
from botocore.credentials import ProcessProvider
25+
from unittest.mock import MagicMock
26+
27+
28+
class TestCredentialProcessStdinPatch:
29+
"""Verify the sigv4_helper patch injects stdin=DEVNULL into ProcessProvider."""
30+
31+
def test_popen_receives_stdin_devnull(self):
32+
"""ProcessProvider._popen passes stdin=subprocess.DEVNULL after the patch."""
33+
popen_kwargs = {}
34+
35+
def mock_popen(*args, **kwargs):
36+
popen_kwargs.update(kwargs)
37+
mock = MagicMock()
38+
mock.returncode = 0
39+
mock.communicate.return_value = (
40+
b'{"Version":1,"AccessKeyId":"A","SecretAccessKey":"B"}',
41+
b'',
42+
)
43+
return mock
44+
45+
provider = ProcessProvider(
46+
profile_name='test',
47+
load_config=lambda: {'profiles': {'test': {'credential_process': 'echo hi'}}},
48+
popen=mock_popen, # type: ignore[arg-type]
49+
)
50+
provider.load()
51+
assert popen_kwargs.get('stdin') == subprocess.DEVNULL
52+
53+
def test_explicit_stdin_is_not_overridden(self):
54+
"""If caller explicitly sets stdin, the patch does not override it."""
55+
popen_kwargs = {}
56+
57+
def mock_popen(*args, **kwargs):
58+
popen_kwargs.update(kwargs)
59+
mock = MagicMock()
60+
mock.returncode = 0
61+
mock.communicate.return_value = (
62+
b'{"Version":1,"AccessKeyId":"A","SecretAccessKey":"B"}',
63+
b'',
64+
)
65+
return mock
66+
67+
provider = ProcessProvider(
68+
profile_name='test',
69+
load_config=lambda: {'profiles': {'test': {'credential_process': 'echo hi'}}},
70+
popen=mock_popen, # type: ignore[arg-type]
71+
)
72+
73+
# Call _popen directly with an explicit stdin to verify setdefault behavior
74+
provider._popen('echo', stdin=subprocess.PIPE)
75+
assert popen_kwargs.get('stdin') == subprocess.PIPE
76+
77+
def test_credential_process_error_propagates(self):
78+
"""The patch does not swallow errors from credential_process."""
79+
from botocore.exceptions import CredentialRetrievalError
80+
81+
def mock_popen(*args, **kwargs):
82+
mock = MagicMock()
83+
mock.returncode = 1
84+
mock.communicate.return_value = (b'', b'access denied')
85+
return mock
86+
87+
provider = ProcessProvider(
88+
profile_name='test',
89+
load_config=lambda: {'profiles': {'test': {'credential_process': 'fail'}}},
90+
popen=mock_popen, # type: ignore[arg-type]
91+
)
92+
93+
import pytest
94+
95+
with pytest.raises(CredentialRetrievalError):
96+
provider.load()

0 commit comments

Comments
 (0)