Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 29 additions & 1 deletion mcp_proxy_for_aws/sigv4_helper.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,10 @@
import httpx
import json
import logging
import subprocess

Check notice

Code scanning / Bandit

Consider possible security implications associated with the subprocess module. Note

Consider possible security implications associated with the subprocess module.
Comment thread
rshevchuk-git marked this conversation as resolved.
Dismissed
Comment thread
github-advanced-security[bot] marked this conversation as resolved.
Fixed
from botocore.auth import SigV4Auth
from botocore.awsrequest import AWSRequest
from botocore.credentials import Credentials
from botocore.credentials import Credentials, ProcessProvider
from functools import partial
from httpx import __version__ as httpx_version
from mcp_proxy_for_aws import __version__
Expand All @@ -30,6 +31,33 @@

logger = logging.getLogger(__name__)


def _patch_credential_process_stdin():
"""Patch botocore ProcessProvider to pass stdin=subprocess.DEVNULL.

When mcp-proxy-for-aws runs in stdio transport mode, its stdin is the MCP
JSON-RPC pipe. Botocore's ProcessProvider spawns credential_process
subprocesses without specifying stdin, so the child inherits the MCP pipe.
On Windows this causes the subprocess to hang indefinitely because it holds
an open handle to the pipe, blocking Popen.communicate() from completing.
"""
original_init = ProcessProvider.__init__

def _patched_init(self, *args, **kwargs):
original_init(self, *args, **kwargs)
original_popen = self._popen

def _popen_with_devnull_stdin(*popen_args, **popen_kwargs):
popen_kwargs.setdefault('stdin', subprocess.DEVNULL)
return original_popen(*popen_args, **popen_kwargs)

self._popen = _popen_with_devnull_stdin

ProcessProvider.__init__ = _patched_init


_patch_credential_process_stdin()

# Headers that should be redacted when logging to prevent credential exposure
SENSITIVE_HEADERS = frozenset({'authorization', 'x-amz-security-token', 'x-amz-date'})

Expand Down
96 changes: 96 additions & 0 deletions tests/unit/test_credential_process_stdin_inheritance.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,96 @@
# Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.

"""Tests for the credential_process stdin isolation patch in sigv4_helper.

When mcp-proxy-for-aws runs in stdio transport mode, its stdin is the MCP
JSON-RPC pipe. Without the patch, botocore's ProcessProvider would spawn
credential_process subprocesses that inherit the pipe, causing hangs on Windows.
"""

import mcp_proxy_for_aws.sigv4_helper # noqa: F401 - ensure patch is applied
import subprocess
from botocore.credentials import ProcessProvider
from unittest.mock import MagicMock


class TestCredentialProcessStdinPatch:
"""Verify the sigv4_helper patch injects stdin=DEVNULL into ProcessProvider."""

def test_popen_receives_stdin_devnull(self):
"""ProcessProvider._popen passes stdin=subprocess.DEVNULL after the patch."""
popen_kwargs = {}

def mock_popen(*args, **kwargs):
popen_kwargs.update(kwargs)
mock = MagicMock()
mock.returncode = 0
mock.communicate.return_value = (
b'{"Version":1,"AccessKeyId":"A","SecretAccessKey":"B"}',
b'',
)
return mock

provider = ProcessProvider(
profile_name='test',
load_config=lambda: {'profiles': {'test': {'credential_process': 'echo hi'}}},
popen=mock_popen, # type: ignore[arg-type]
)
provider.load()
assert popen_kwargs.get('stdin') == subprocess.DEVNULL

def test_explicit_stdin_is_not_overridden(self):
"""If caller explicitly sets stdin, the patch does not override it."""
popen_kwargs = {}

def mock_popen(*args, **kwargs):
popen_kwargs.update(kwargs)
mock = MagicMock()
mock.returncode = 0
mock.communicate.return_value = (
b'{"Version":1,"AccessKeyId":"A","SecretAccessKey":"B"}',
b'',
)
return mock

provider = ProcessProvider(
profile_name='test',
load_config=lambda: {'profiles': {'test': {'credential_process': 'echo hi'}}},
popen=mock_popen, # type: ignore[arg-type]
)

# Call _popen directly with an explicit stdin to verify setdefault behavior
provider._popen('echo', stdin=subprocess.PIPE)
assert popen_kwargs.get('stdin') == subprocess.PIPE

def test_credential_process_error_propagates(self):
"""The patch does not swallow errors from credential_process."""
from botocore.exceptions import CredentialRetrievalError

def mock_popen(*args, **kwargs):
mock = MagicMock()
mock.returncode = 1
mock.communicate.return_value = (b'', b'access denied')
return mock

provider = ProcessProvider(
profile_name='test',
load_config=lambda: {'profiles': {'test': {'credential_process': 'fail'}}},
popen=mock_popen, # type: ignore[arg-type]
)

import pytest

with pytest.raises(CredentialRetrievalError):
provider.load()
Loading