From 3058d7a4fac50dde3d11e088f1d862551311d7eb Mon Sep 17 00:00:00 2001 From: acmlau Date: Tue, 30 Sep 2025 13:27:35 +0200 Subject: [PATCH 1/8] Add region to CLI arguments --- aws_mcp_proxy/server.py | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/aws_mcp_proxy/server.py b/aws_mcp_proxy/server.py index fb6fd6b..e086bb6 100644 --- a/aws_mcp_proxy/server.py +++ b/aws_mcp_proxy/server.py @@ -47,8 +47,8 @@ async def setup_mcp_mode(mcp: FastMCP, args) -> None: service = determine_service_name(args.endpoint, args.service) # Get profile and region - profile = args.profile or os.getenv('AWS_PROFILE') - region = os.getenv('AWS_REGION', 'us-west-2') + profile = args.profile + region = args.region # Log server configuration logger.info( @@ -98,6 +98,13 @@ def parse_args(): parser.add_argument( '--profile', help='AWS profile to use (uses AWS_PROFILE environment variable if not provided)', + default=os.getenv('AWS_PROFILE', 'default') + ) + + parser.add_argument( + '--region', + help='AWS region to use (uses AWS_REGION if not provided)', + default=os.getenv('AWS_REGION', 'us-east-1') ) parser.add_argument( From 273a37ec35e77ed763aafc4109819d162bfa6e35 Mon Sep 17 00:00:00 2001 From: acmlau Date: Tue, 30 Sep 2025 15:58:13 +0200 Subject: [PATCH 2/8] Updating default region to us-east-1 --- aws_mcp_proxy/server.py | 4 ++-- aws_mcp_proxy/sigv4_helper.py | 6 +++--- tests/test_sigv4_helper.py | 2 +- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/aws_mcp_proxy/server.py b/aws_mcp_proxy/server.py index e086bb6..a3d6265 100644 --- a/aws_mcp_proxy/server.py +++ b/aws_mcp_proxy/server.py @@ -98,13 +98,13 @@ def parse_args(): parser.add_argument( '--profile', help='AWS profile to use (uses AWS_PROFILE environment variable if not provided)', - default=os.getenv('AWS_PROFILE', 'default') + default=os.getenv('AWS_PROFILE', 'default'), ) parser.add_argument( '--region', help='AWS region to use (uses AWS_REGION if not provided)', - default=os.getenv('AWS_REGION', 'us-east-1') + default=os.getenv('AWS_REGION', 'us-east-1'), ) parser.add_argument( diff --git a/aws_mcp_proxy/sigv4_helper.py b/aws_mcp_proxy/sigv4_helper.py index cd64e40..5fc4178 100644 --- a/aws_mcp_proxy/sigv4_helper.py +++ b/aws_mcp_proxy/sigv4_helper.py @@ -158,7 +158,7 @@ def create_sigv4_auth( Args: service: AWS service name for SigV4 signing profile: AWS profile to use (optional) - region: AWS region (defaults to AWS_REGION env var or us-west-2) + region: AWS region (defaults to AWS_REGION env var or us-east-1) Returns: SigV4HTTPXAuth instance @@ -172,7 +172,7 @@ def create_sigv4_auth( # Get region from parameter, environment variable, or default if not region: - region = os.environ.get('AWS_REGION', 'us-west-2') + region = os.environ.get('AWS_REGION', 'us-east-1') # Create SigV4Auth with explicit credentials sigv4_auth = SigV4HTTPXAuth( @@ -198,7 +198,7 @@ def create_sigv4_client( Args: service: AWS service name for SigV4 signing profile: AWS profile to use (optional) - region: AWS region (optional, defaults to AWS_REGION env var or us-west-2) + region: AWS region (optional, defaults to AWS_REGION env var or us-east-1) headers: Headers to include in requests auth: Auth parameter (ignored as we provide our own) **kwargs: Additional arguments to pass to httpx.AsyncClient diff --git a/tests/test_sigv4_helper.py b/tests/test_sigv4_helper.py index bb7d486..acf7de6 100644 --- a/tests/test_sigv4_helper.py +++ b/tests/test_sigv4_helper.py @@ -243,7 +243,7 @@ def test_create_sigv4_auth_default(self, mock_create_session): # Verify auth was created correctly assert isinstance(result, SigV4HTTPXAuth) assert result.service == 'test-service' - assert result.region == 'us-west-2' # default region + assert result.region == 'us-east-1' # default region assert result.credentials == mock_credentials @patch('aws_mcp_proxy.sigv4_helper.create_aws_session') From 075a9180f2e94eb9b6fdc7cc62eeed0e1db94b2f Mon Sep 17 00:00:00 2001 From: acmlau Date: Wed, 1 Oct 2025 16:06:57 +0200 Subject: [PATCH 3/8] Add region utils --- aws_mcp_proxy/server.py | 10 ++++--- aws_mcp_proxy/sigv4_helper.py | 1 - aws_mcp_proxy/utils.py | 45 ++++++++++++++++++++++++++++++-- tests/test_server.py | 3 +++ tests/test_sigv4_helper.py | 4 +++ tests/test_utils.py | 49 +++++++++++++++++++++++++++++++++-- 6 files changed, 103 insertions(+), 9 deletions(-) diff --git a/aws_mcp_proxy/server.py b/aws_mcp_proxy/server.py index a3d6265..739f027 100644 --- a/aws_mcp_proxy/server.py +++ b/aws_mcp_proxy/server.py @@ -30,6 +30,7 @@ from aws_mcp_proxy.mcp_proxy_manager import McpProxyManager from aws_mcp_proxy.utils import ( create_transport_with_sigv4, + determine_aws_region, determine_service_name, ) from fastmcp.server.server import FastMCP @@ -46,9 +47,11 @@ async def setup_mcp_mode(mcp: FastMCP, args) -> None: # Validate and determine service service = determine_service_name(args.endpoint, args.service) - # Get profile and region + # Validate and determine region + region = determine_aws_region(args.endpoint, args.region) + + # Get profile profile = args.profile - region = args.region # Log server configuration logger.info( @@ -57,7 +60,7 @@ async def setup_mcp_mode(mcp: FastMCP, args) -> None: logger.info('Running in MCP mode') # Create transport with SigV4 authentication - transport = create_transport_with_sigv4(args.endpoint, service, profile) + transport = create_transport_with_sigv4(args.endpoint, service, profile, region) # Create proxy with the transport proxy = FastMCP.as_proxy(transport) @@ -104,7 +107,6 @@ def parse_args(): parser.add_argument( '--region', help='AWS region to use (uses AWS_REGION if not provided)', - default=os.getenv('AWS_REGION', 'us-east-1'), ) parser.add_argument( diff --git a/aws_mcp_proxy/sigv4_helper.py b/aws_mcp_proxy/sigv4_helper.py index 5fc4178..317be19 100644 --- a/aws_mcp_proxy/sigv4_helper.py +++ b/aws_mcp_proxy/sigv4_helper.py @@ -190,7 +190,6 @@ def create_sigv4_client( profile: Optional[str] = None, region: Optional[str] = None, headers: Optional[Dict[str, str]] = None, - auth: Optional[httpx.Auth] = None, **kwargs: Any, ) -> httpx.AsyncClient: """Create an httpx.AsyncClient with SigV4 authentication. diff --git a/aws_mcp_proxy/utils.py b/aws_mcp_proxy/utils.py index 7ab322c..13fe81b 100644 --- a/aws_mcp_proxy/utils.py +++ b/aws_mcp_proxy/utils.py @@ -23,7 +23,10 @@ def create_transport_with_sigv4( - url: str, service: str, profile: Optional[str] = None + url: str, + service: str, + profile: Optional[str] = None, + region: Optional[str] = None, ) -> StreamableHttpTransport: """Create a StreamableHttpTransport with SigV4 authentication. @@ -31,6 +34,7 @@ def create_transport_with_sigv4( url: The endpoint URL service: AWS service name for SigV4 signing profile: AWS profile to use (optional) + region: AWS region to use (Optional) Returns: StreamableHttpTransport instance with SigV4 authentication @@ -42,7 +46,12 @@ def client_factory( auth: Optional[httpx.Auth] = None, ) -> httpx.AsyncClient: return create_sigv4_client( - service=service, profile=profile, headers=headers, timeout=timeout, auth=auth + service=service, + profile=profile, + region=region, + headers=headers, + timeout=timeout, + auth=auth, ) return StreamableHttpTransport( @@ -81,3 +90,35 @@ def determine_service_name(endpoint: str, service: Optional[str] = None) -> str: 'Please provide the service name explicitly using --service argument.' ) return determined_service + + +def determine_aws_region(endpoint: str, region: Optional[str] = None) -> str: + """Validate and determine the AWS region. + + Args: + endpoint: The endpoint URL + region: Optional region name + + Returns: + Validated AWS region + + Raises: + ValueError: If region cannot be determined + """ + if region: + return region + + # Parse AWS region from endpoint URL + parsed = urlparse(endpoint) + hostname = parsed.hostname or '' + + # Extract region name (pattern: service.region.api.aws or service-name.region.api.aws) + region_match = re.search(r'\.([a-z0-9-]+)\.api\.aws', hostname) + determined_region = region_match.group(1) if region_match else None + + if not determined_region: + raise ValueError( + f"Could not determine AWS region from endpoint '{endpoint}'. " + 'Please provide the region explicitly using --region argument.' + ) + return determined_region diff --git a/tests/test_server.py b/tests/test_server.py index 2e310d8..df31257 100644 --- a/tests/test_server.py +++ b/tests/test_server.py @@ -36,6 +36,7 @@ async def test_setup_mcp_mode( mock_args = Mock() mock_args.endpoint = 'https://test.example.com' mock_args.service = 'test-service' + mock_args.region = 'us-east-1' mock_args.profile = None mock_args.allow_write = False @@ -71,6 +72,7 @@ async def test_setup_mcp_mode_with_tools( mock_args = Mock() mock_args.endpoint = 'https://test.example.com' mock_args.service = 'test-service' + mock_args.region = 'us-east-1' mock_args.profile = None mock_args.allow_write = False @@ -106,6 +108,7 @@ async def test_setup_mcp_mode_tool_registration_error( mock_args = Mock() mock_args.endpoint = 'https://test.example.com' mock_args.service = 'test-service' + mock_args.region = 'us-east-1' mock_args.profile = None mock_args.allow_write = False diff --git a/tests/test_sigv4_helper.py b/tests/test_sigv4_helper.py index acf7de6..645b2ac 100644 --- a/tests/test_sigv4_helper.py +++ b/tests/test_sigv4_helper.py @@ -226,6 +226,7 @@ class TestCreateSigv4Auth: """Test cases for the create_sigv4_auth function.""" @patch('aws_mcp_proxy.sigv4_helper.create_aws_session') + @patch.dict(os.environ, {"AWS_REGION": ""}) def test_create_sigv4_auth_default(self, mock_create_session): """Test creating SigV4 auth with default parameters.""" # Mock session and credentials @@ -237,6 +238,9 @@ def test_create_sigv4_auth_default(self, mock_create_session): mock_session.get_credentials.return_value = mock_credentials mock_create_session.return_value = mock_session + # Remove AWS_REGION to check default + del os.environ['AWS_REGION'] + # Test auth creation result = create_sigv4_auth('test-service') diff --git a/tests/test_utils.py b/tests/test_utils.py index 5fd967a..7e22efd 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -17,6 +17,7 @@ import pytest from aws_mcp_proxy.utils import ( create_transport_with_sigv4, + determine_aws_region, determine_service_name, ) from fastmcp.client.transports import StreamableHttpTransport @@ -35,8 +36,9 @@ def test_create_transport_with_sigv4(self, mock_create_sigv4_client): url = 'https://eks-mcp.us-west-2.api.aws/mcp' service = 'eks-mcp' profile = 'test-profile' + region = 'us-east-1' - result = create_transport_with_sigv4(url, service, profile) + result = create_transport_with_sigv4(url, service, profile, region) # Verify result is StreamableHttpTransport assert isinstance(result, StreamableHttpTransport) @@ -54,6 +56,7 @@ def test_create_transport_with_sigv4(self, mock_create_sigv4_client): mock_create_sigv4_client.assert_called_once_with( service=service, profile=profile, + region=region, headers={'test': 'header'}, timeout=Timeout(30.0), auth=None, @@ -77,7 +80,7 @@ def test_create_transport_with_sigv4_no_profile(self, mock_create_sigv4_client): factory(headers=None, timeout=None, auth=None) mock_create_sigv4_client.assert_called_once_with( - service=service, profile=None, headers=None, timeout=None, auth=None + service=service, profile=None, region=None, headers=None, timeout=None, auth=None ) else: # If we can't access the factory directly, just verify the transport was created @@ -144,3 +147,45 @@ def test_validate_service_name_invalid_url_failure(self): assert 'Could not determine AWS service name' in str(exc_info.value) assert endpoint in str(exc_info.value) assert '--service argument' in str(exc_info.value) + + +class TestDetermineRegion: + """Test cases for determine_aws_region function.""" + + def test_determine_region_with_region(self): + """Test determination when region is provided.""" + endpoint = 'https://mcp.us-east-1.api.aws/mcp' + region = 'custom-region' + + result = determine_aws_region(endpoint, region) + + assert result == region + + def test_determine_region_without_region_success(self): + """Test determination when region is not provided but can be parsed.""" + endpoint = 'https://mcp.us-east-1.api.aws/mcp' + expected_region = 'us-east-1' + + result = determine_aws_region(endpoint) + + assert result == expected_region + + def test_determine_region_with_complex_service_name(self): + """Test parsing region from endpoint with complex service name.""" + endpoint = 'https://eks-mcp-beta.us-west-2.api.aws/mcp' + expected_region = 'us-west-2' + + result = determine_aws_region(endpoint) + + assert result == expected_region + + def test_determine_region_without_region_failure(self): + """Test determination when region cannot be determined.""" + endpoint = 'https://service.example.com' + + with pytest.raises(ValueError) as exc_info: + determine_aws_region(endpoint) + + assert 'Could not determine AWS region' in str(exc_info.value) + assert endpoint in str(exc_info.value) + assert '--region argument' in str(exc_info.value) From 89debe0b85f75990b9e69cf279871cd51f34e348 Mon Sep 17 00:00:00 2001 From: acmlau Date: Wed, 1 Oct 2025 16:16:49 +0200 Subject: [PATCH 4/8] Remove auth parameter --- aws_mcp_proxy/sigv4_helper.py | 1 - tests/test_sigv4_helper.py | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/aws_mcp_proxy/sigv4_helper.py b/aws_mcp_proxy/sigv4_helper.py index 317be19..ac8be55 100644 --- a/aws_mcp_proxy/sigv4_helper.py +++ b/aws_mcp_proxy/sigv4_helper.py @@ -199,7 +199,6 @@ def create_sigv4_client( profile: AWS profile to use (optional) region: AWS region (optional, defaults to AWS_REGION env var or us-east-1) headers: Headers to include in requests - auth: Auth parameter (ignored as we provide our own) **kwargs: Additional arguments to pass to httpx.AsyncClient Returns: diff --git a/tests/test_sigv4_helper.py b/tests/test_sigv4_helper.py index 645b2ac..fc20f68 100644 --- a/tests/test_sigv4_helper.py +++ b/tests/test_sigv4_helper.py @@ -226,7 +226,7 @@ class TestCreateSigv4Auth: """Test cases for the create_sigv4_auth function.""" @patch('aws_mcp_proxy.sigv4_helper.create_aws_session') - @patch.dict(os.environ, {"AWS_REGION": ""}) + @patch.dict(os.environ, {'AWS_REGION': ''}) def test_create_sigv4_auth_default(self, mock_create_session): """Test creating SigV4 auth with default parameters.""" # Mock session and credentials From 3e6f243e698fb4bbb9cc6db0722b30257b03c574 Mon Sep 17 00:00:00 2001 From: acmlau Date: Wed, 1 Oct 2025 16:40:58 +0200 Subject: [PATCH 5/8] Undo removal of auth parameter --- aws_mcp_proxy/sigv4_helper.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/aws_mcp_proxy/sigv4_helper.py b/aws_mcp_proxy/sigv4_helper.py index ac8be55..5fc4178 100644 --- a/aws_mcp_proxy/sigv4_helper.py +++ b/aws_mcp_proxy/sigv4_helper.py @@ -190,6 +190,7 @@ def create_sigv4_client( profile: Optional[str] = None, region: Optional[str] = None, headers: Optional[Dict[str, str]] = None, + auth: Optional[httpx.Auth] = None, **kwargs: Any, ) -> httpx.AsyncClient: """Create an httpx.AsyncClient with SigV4 authentication. @@ -199,6 +200,7 @@ def create_sigv4_client( profile: AWS profile to use (optional) region: AWS region (optional, defaults to AWS_REGION env var or us-east-1) headers: Headers to include in requests + auth: Auth parameter (ignored as we provide our own) **kwargs: Additional arguments to pass to httpx.AsyncClient Returns: From bcf393953953f1aca17b15a1ff4e7ee9c971afa2 Mon Sep 17 00:00:00 2001 From: acmlau Date: Thu, 2 Oct 2025 10:23:16 +0200 Subject: [PATCH 6/8] Make region non optional parameter --- aws_mcp_proxy/server.py | 2 +- aws_mcp_proxy/sigv4_helper.py | 6 +++--- aws_mcp_proxy/utils.py | 2 +- tests/test_sigv4_helper.py | 33 ++++----------------------------- tests/test_utils.py | 7 ++++--- 5 files changed, 13 insertions(+), 37 deletions(-) diff --git a/aws_mcp_proxy/server.py b/aws_mcp_proxy/server.py index 739f027..2407fcc 100644 --- a/aws_mcp_proxy/server.py +++ b/aws_mcp_proxy/server.py @@ -60,7 +60,7 @@ async def setup_mcp_mode(mcp: FastMCP, args) -> None: logger.info('Running in MCP mode') # Create transport with SigV4 authentication - transport = create_transport_with_sigv4(args.endpoint, service, profile, region) + transport = create_transport_with_sigv4(args.endpoint, service, region, profile) # Create proxy with the transport proxy = FastMCP.as_proxy(transport) diff --git a/aws_mcp_proxy/sigv4_helper.py b/aws_mcp_proxy/sigv4_helper.py index 5fc4178..72bf9c7 100644 --- a/aws_mcp_proxy/sigv4_helper.py +++ b/aws_mcp_proxy/sigv4_helper.py @@ -151,7 +151,7 @@ def create_aws_session(profile: Optional[str] = None) -> boto3.Session: def create_sigv4_auth( - service: str, profile: Optional[str] = None, region: Optional[str] = None + service: str, region: str, profile: Optional[str] = None ) -> SigV4HTTPXAuth: """Create SigV4 authentication for AWS requests. @@ -187,8 +187,8 @@ def create_sigv4_auth( def create_sigv4_client( service: str = 'eks-mcp', + region: str = '', profile: Optional[str] = None, - region: Optional[str] = None, headers: Optional[Dict[str, str]] = None, auth: Optional[httpx.Auth] = None, **kwargs: Any, @@ -225,7 +225,7 @@ def create_sigv4_client( ) # Create SigV4 auth - sigv4_auth = create_sigv4_auth(service, profile, region) + sigv4_auth = create_sigv4_auth(service, region, profile) # Create the client with SigV4 auth and error handling event hook logger.info("Creating httpx.AsyncClient with SigV4 authentication for service '%s'", service) diff --git a/aws_mcp_proxy/utils.py b/aws_mcp_proxy/utils.py index 13fe81b..907c7ea 100644 --- a/aws_mcp_proxy/utils.py +++ b/aws_mcp_proxy/utils.py @@ -25,8 +25,8 @@ def create_transport_with_sigv4( url: str, service: str, + region: str, profile: Optional[str] = None, - region: Optional[str] = None, ) -> StreamableHttpTransport: """Create a StreamableHttpTransport with SigV4 authentication. diff --git a/tests/test_sigv4_helper.py b/tests/test_sigv4_helper.py index fc20f68..ee39573 100644 --- a/tests/test_sigv4_helper.py +++ b/tests/test_sigv4_helper.py @@ -226,7 +226,6 @@ class TestCreateSigv4Auth: """Test cases for the create_sigv4_auth function.""" @patch('aws_mcp_proxy.sigv4_helper.create_aws_session') - @patch.dict(os.environ, {'AWS_REGION': ''}) def test_create_sigv4_auth_default(self, mock_create_session): """Test creating SigV4 auth with default parameters.""" # Mock session and credentials @@ -238,39 +237,15 @@ def test_create_sigv4_auth_default(self, mock_create_session): mock_session.get_credentials.return_value = mock_credentials mock_create_session.return_value = mock_session - # Remove AWS_REGION to check default - del os.environ['AWS_REGION'] - # Test auth creation - result = create_sigv4_auth('test-service') + result = create_sigv4_auth('test-service', 'test-region') # Verify auth was created correctly assert isinstance(result, SigV4HTTPXAuth) assert result.service == 'test-service' - assert result.region == 'us-east-1' # default region + assert result.region == 'test-region' # default region assert result.credentials == mock_credentials - @patch('aws_mcp_proxy.sigv4_helper.create_aws_session') - @patch.dict(os.environ, {'AWS_REGION': 'eu-west-1'}) - def test_create_sigv4_auth_with_env_region(self, mock_create_session): - """Test creating SigV4 auth with region from environment variable.""" - # Mock session and credentials - mock_session = Mock() - mock_credentials = Mock() - mock_credentials.access_key = 'test_access_key' - mock_credentials.secret_key = 'test_secret_key' - mock_credentials.token = None - mock_session.get_credentials.return_value = mock_credentials - mock_create_session.return_value = mock_session - - # Test auth creation - result = create_sigv4_auth('test-service', profile='test-profile') - - # Verify auth was created with environment region - assert isinstance(result, SigV4HTTPXAuth) - assert result.service == 'test-service' - assert result.region == 'eu-west-1' # from environment - assert result.credentials == mock_credentials @patch('aws_mcp_proxy.sigv4_helper.create_aws_session') def test_create_sigv4_auth_with_explicit_region(self, mock_create_session): @@ -363,7 +338,7 @@ def test_create_sigv4_client_with_custom_service_and_region( ) # Verify auth was created with custom parameters - mock_create_auth.assert_called_once_with('custom-service', 'test-profile', 'us-east-1') + mock_create_auth.assert_called_once_with('custom-service', 'us-east-1', 'test-profile') assert result == mock_client @patch('aws_mcp_proxy.sigv4_helper.create_sigv4_auth') @@ -411,7 +386,7 @@ def test_create_sigv4_client_with_prompt_context(self, mock_client_class, mock_c ) # Verify client was created correctly with prompt context - mock_create_auth.assert_called_once_with('eks-mcp', None, 'us-west-2') + mock_create_auth.assert_called_once_with('eks-mcp', 'us-west-2', None) # Check that AsyncClient was called with correct parameters including prompt headers call_args = mock_client_class.call_args diff --git a/tests/test_utils.py b/tests/test_utils.py index 7e22efd..cf7f472 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -38,7 +38,7 @@ def test_create_transport_with_sigv4(self, mock_create_sigv4_client): profile = 'test-profile' region = 'us-east-1' - result = create_transport_with_sigv4(url, service, profile, region) + result = create_transport_with_sigv4(url, service, region, profile) # Verify result is StreamableHttpTransport assert isinstance(result, StreamableHttpTransport) @@ -70,8 +70,9 @@ def test_create_transport_with_sigv4_no_profile(self, mock_create_sigv4_client): """Test creating transport without profile.""" url = 'https://eks-mcp.us-west-2.api.aws/mcp' service = 'eks-mcp' + region = 'us-west-2' - result = create_transport_with_sigv4(url, service) + result = create_transport_with_sigv4(url, service, region) # Test that the httpx_client_factory calls create_sigv4_client correctly # We need to access the factory through the transport's internal structure @@ -80,7 +81,7 @@ def test_create_transport_with_sigv4_no_profile(self, mock_create_sigv4_client): factory(headers=None, timeout=None, auth=None) mock_create_sigv4_client.assert_called_once_with( - service=service, profile=None, region=None, headers=None, timeout=None, auth=None + service=service, region=region, profile=None, headers=None, timeout=None, auth=None ) else: # If we can't access the factory directly, just verify the transport was created From a916495215bd011e59259d0c5e805fbe512735cb Mon Sep 17 00:00:00 2001 From: acmlau Date: Thu, 2 Oct 2025 11:32:26 +0200 Subject: [PATCH 7/8] Resolving merge conflict --- .github/ISSUE_TEMPLATE/bug_report.yml | 2 +- .github/ISSUE_TEMPLATE/feature_request.yml | 2 +- .github/ISSUE_TEMPLATE/questions.yml | 2 +- .github/workflows/python.yml | 4 +++ CODE_OF_CONDUCT.md | 5 +++ README.md | 6 ++-- aws_mcp_proxy/mcp_proxy_manager.py | 8 ++--- aws_mcp_proxy/server.py | 16 +++++----- aws_mcp_proxy/sigv4_helper.py | 8 ++--- tests/test_mcp_proxy_manager.py | 36 +++++++++++----------- tests/test_server.py | 15 +++++---- tests/test_sigv4_helper.py | 14 +++++---- tests/test_utils.py | 16 +++++----- 13 files changed, 70 insertions(+), 64 deletions(-) create mode 100644 CODE_OF_CONDUCT.md diff --git a/.github/ISSUE_TEMPLATE/bug_report.yml b/.github/ISSUE_TEMPLATE/bug_report.yml index 61d2ccb..91a538a 100644 --- a/.github/ISSUE_TEMPLATE/bug_report.yml +++ b/.github/ISSUE_TEMPLATE/bug_report.yml @@ -97,4 +97,4 @@ body: description: "Are there any related issues? If so, please link them here (e.g., #123, #456)" placeholder: "#123, #456, or type 'None'" validations: - required: true \ No newline at end of file + required: true diff --git a/.github/ISSUE_TEMPLATE/feature_request.yml b/.github/ISSUE_TEMPLATE/feature_request.yml index 59a21ec..218ed34 100644 --- a/.github/ISSUE_TEMPLATE/feature_request.yml +++ b/.github/ISSUE_TEMPLATE/feature_request.yml @@ -44,4 +44,4 @@ body: - label: I may be able to implement this feature request required: false - label: This feature might incur a breaking change - required: false \ No newline at end of file + required: false diff --git a/.github/ISSUE_TEMPLATE/questions.yml b/.github/ISSUE_TEMPLATE/questions.yml index df76aef..691c089 100644 --- a/.github/ISSUE_TEMPLATE/questions.yml +++ b/.github/ISSUE_TEMPLATE/questions.yml @@ -19,4 +19,4 @@ body: description: | Any relevant context about your question. validations: - required: true \ No newline at end of file + required: true diff --git a/.github/workflows/python.yml b/.github/workflows/python.yml index 1995823..0369948 100644 --- a/.github/workflows/python.yml +++ b/.github/workflows/python.yml @@ -48,6 +48,10 @@ jobs: fi echo "✓ Tests directory found" + - name: Run pre-commit + run: | + uv run pre-commit run --all-files + - name: Run tests run: | uv run --frozen pytest --cov --cov-branch --cov-report=term-missing --cov-report=xml:${{ matrix.package }}-coverage.xml diff --git a/CODE_OF_CONDUCT.md b/CODE_OF_CONDUCT.md new file mode 100644 index 0000000..ec98f2b --- /dev/null +++ b/CODE_OF_CONDUCT.md @@ -0,0 +1,5 @@ +## Code of Conduct + +This project has adopted the [Amazon Open Source Code of Conduct](https://aws.github.io/code-of-conduct). +For more information see the [Code of Conduct FAQ](https://aws.github.io/code-of-conduct-faq) or contact +opensource-codeofconduct@amazon.com with any additional questions or comments. diff --git a/README.md b/README.md index 2d9284b..69443cf 100644 --- a/README.md +++ b/README.md @@ -35,9 +35,9 @@ Add this to your MCP client configuration, replacing env variables to match the Optional arguments you can add: - `--service`: AWS service name for SigV4 signing (inferred from endpoint if not provided) - `--profile`: AWS profile to use (uses AWS_PROFILE environment variable if not provided) -- `--allow-write`: Allow tools that require write permissions to be enabled (by default, only tools with the `readOnlyHint` annotation are enabled) +- `--read-only`: Disable tools which require write permissions. (tools which DO NOT require write permissions are annotated with [`readOnlyHint=true`](https://modelcontextprotocol.io/specification/2025-06-18/schema#toolannotations-readonlyhint)) -NOTE: `remote-server-url` should be your remote mcp server's URL (including the `/mcp` part). `service-code` should be the service code for your own mcp service, such as `eks-mcp`. +NOTE: `remote-server-url` should be your remote mcp server's URL (including the `/mcp` part). `service-code` should be the service code for the MCP to be connected. Example with all options ```json @@ -57,7 +57,7 @@ Example with all options "", "--profile", "default", - "--allow-write" + "--read-only" ] } } diff --git a/aws_mcp_proxy/mcp_proxy_manager.py b/aws_mcp_proxy/mcp_proxy_manager.py index 9cd8b22..4dfc10f 100644 --- a/aws_mcp_proxy/mcp_proxy_manager.py +++ b/aws_mcp_proxy/mcp_proxy_manager.py @@ -23,15 +23,15 @@ class McpProxyManager: logger = logging.getLogger(__name__) - def __init__(self, target_mcp: FastMCP, allow_write: bool = False): + def __init__(self, target_mcp: FastMCP, read_only: bool = False): """Initialize the MCP Proxy Manager. Args: target_mcp: The target MCP server to add content to - allow_write: Whether to allow tools that require write permissions + read_only: If true, disable tools that require write permissions OR that do not have `readOnlyHint` set. """ self.target_mcp = target_mcp - self.allow_write = allow_write + self.read_only = read_only async def add_proxy_content(self, proxy: FastMCP) -> None: """Add tools, resources, and prompts from proxy to MCP server. @@ -68,7 +68,7 @@ async def _add_tools(self, proxy: FastMCP) -> None: for tool_name, tool in tools.items(): # Check the tool annotations and disable if needed annotations = tool.annotations - if not self.allow_write: + if self.read_only: # In readOnly mode, skip the tools with no readOnlyHint=True annotation if annotations and not annotations.readOnlyHint or not annotations: self.logger.info(f'Skipping tool {tool_name} needing write permissions') diff --git a/aws_mcp_proxy/server.py b/aws_mcp_proxy/server.py index 2407fcc..8d4b6f7 100644 --- a/aws_mcp_proxy/server.py +++ b/aws_mcp_proxy/server.py @@ -66,7 +66,7 @@ async def setup_mcp_mode(mcp: FastMCP, args) -> None: proxy = FastMCP.as_proxy(transport) # Use McpProxyManager to add proxy content - proxy_manager = McpProxyManager(mcp, args.allow_write) + proxy_manager = McpProxyManager(mcp, args.read_only) await proxy_manager.add_proxy_content(proxy) @@ -77,20 +77,20 @@ def parse_args(): formatter_class=argparse.RawDescriptionHelpFormatter, epilog=""" Examples: - # Run with EKS MCP endpoint - aws-mcp-proxy https://eks-mcp.us-west-2.api.aws + # Run with your endpoint + aws-mcp-proxy # Run with custom service and profile - aws-mcp-proxy https://eks-mcp.us-west-2.api.aws --service eks-mcp --profile default + aws-mcp-proxy --service --profile default # Run with write permissions enabled - aws-mcp-proxy https://eks-mcp.us-west-2.api.aws --allow-write + aws-mcp-proxy --read-only """, ) parser.add_argument( 'endpoint', - help='MCP endpoint URL (e.g., https://eks-mcp.us-west-2.api.aws)', + help='SigV4 MCP endpoint URL', ) parser.add_argument( @@ -110,9 +110,9 @@ def parse_args(): ) parser.add_argument( - '--allow-write', + '--read-only', action='store_true', - help='Allow tools that require write permissions to be enabled', + help='Disable tools which may require write permissions (readOnlyHint True or unknown)', ) parser.add_argument( diff --git a/aws_mcp_proxy/sigv4_helper.py b/aws_mcp_proxy/sigv4_helper.py index 72bf9c7..7fef48b 100644 --- a/aws_mcp_proxy/sigv4_helper.py +++ b/aws_mcp_proxy/sigv4_helper.py @@ -170,10 +170,6 @@ def create_sigv4_auth( session = create_aws_session(profile) credentials = session.get_credentials() - # Get region from parameter, environment variable, or default - if not region: - region = os.environ.get('AWS_REGION', 'us-east-1') - # Create SigV4Auth with explicit credentials sigv4_auth = SigV4HTTPXAuth( credentials=credentials, @@ -186,8 +182,8 @@ def create_sigv4_auth( def create_sigv4_client( - service: str = 'eks-mcp', - region: str = '', + service: str, + region: str, profile: Optional[str] = None, headers: Optional[Dict[str, str]] = None, auth: Optional[httpx.Auth] = None, diff --git a/tests/test_mcp_proxy_manager.py b/tests/test_mcp_proxy_manager.py index 9bf5f43..76cef5f 100644 --- a/tests/test_mcp_proxy_manager.py +++ b/tests/test_mcp_proxy_manager.py @@ -69,19 +69,19 @@ def mock_prompt(self): prompt.copy.return_value = prompt return prompt - def test_init_default_allow_write(self, mock_target_mcp): - """Test McpProxyManager initialization with default allow_write.""" + def test_init_default_read_only(self, mock_target_mcp): + """Test McpProxyManager initialization with default read_only.""" manager = McpProxyManager(mock_target_mcp) assert manager.target_mcp == mock_target_mcp - assert manager.allow_write is False + assert manager.read_only is False - def test_init_custom_allow_write(self, mock_target_mcp): - """Test McpProxyManager initialization with custom allow_write.""" - manager = McpProxyManager(mock_target_mcp, allow_write=True) + def test_init_custom_read_only(self, mock_target_mcp): + """Test McpProxyManager initialization with custom read_only.""" + manager = McpProxyManager(mock_target_mcp, read_only=True) assert manager.target_mcp == mock_target_mcp - assert manager.allow_write is True + assert manager.read_only is True @pytest.mark.asyncio async def test_add_proxy_content_success( @@ -93,7 +93,7 @@ async def test_add_proxy_content_success( mock_proxy.get_resources.return_value = {'test_resource': mock_resource} mock_proxy.get_prompts.return_value = {'test_prompt': mock_prompt} - manager = McpProxyManager(mock_target_mcp, allow_write=True) + manager = McpProxyManager(mock_target_mcp, read_only=True) await manager.add_proxy_content(mock_proxy) # Verify all methods were called @@ -120,7 +120,7 @@ async def test_add_tools_success(self, mock_target_mcp, mock_proxy, mock_tool): @pytest.mark.asyncio async def test_add_tools_skip_write_tools(self, mock_target_mcp, mock_proxy, mock_tool): - """Test that tools requiring write permissions are skipped when allow_write=False.""" + """Test that tools requiring write permissions are skipped when read_only=True.""" # Setup tool with write permissions required annotations = MagicMock() annotations.readOnlyHint = False @@ -128,15 +128,15 @@ async def test_add_tools_skip_write_tools(self, mock_target_mcp, mock_proxy, moc mock_proxy.get_tools.return_value = {'write_tool': mock_tool} - manager = McpProxyManager(mock_target_mcp, allow_write=False) + manager = McpProxyManager(mock_target_mcp, read_only=True) await manager._add_tools(mock_proxy) # Verify tool was not added (skipped) mock_target_mcp.add_tool.assert_not_called() @pytest.mark.asyncio - async def test_add_tools_allow_write_tools(self, mock_target_mcp, mock_proxy, mock_tool): - """Test that tools requiring write permissions are added when allow_write=True.""" + async def test_add_tools_not_read_only_tools(self, mock_target_mcp, mock_proxy, mock_tool): + """Test that tools requiring write permissions are added when read_only=False.""" # Setup tool with write permissions required annotations = MagicMock() annotations.readOnlyHint = False @@ -144,7 +144,7 @@ async def test_add_tools_allow_write_tools(self, mock_target_mcp, mock_proxy, mo mock_proxy.get_tools.return_value = {'write_tool': mock_tool} - manager = McpProxyManager(mock_target_mcp, allow_write=True) + manager = McpProxyManager(mock_target_mcp, read_only=False) await manager._add_tools(mock_proxy) # Verify tool was added @@ -160,7 +160,7 @@ async def test_add_tools_readonly_tools(self, mock_target_mcp, mock_proxy, mock_ mock_proxy.get_tools.return_value = {'readonly_tool': mock_tool} - manager = McpProxyManager(mock_target_mcp, allow_write=False) + manager = McpProxyManager(mock_target_mcp, read_only=True) await manager._add_tools(mock_proxy) # Verify tool was added @@ -276,13 +276,13 @@ async def test_add_proxy_content_prompts_exception_handled( @pytest.mark.asyncio async def test_add_tools_no_annotations(self, mock_target_mcp, mock_proxy, mock_tool): - """Test that tools with no annotations are skipped when allow_write=False.""" + """Test that tools with no annotations are skipped when read_only=True.""" # Setup tool with no annotations mock_tool.annotations = None mock_proxy.get_tools.return_value = {'no_annotations_tool': mock_tool} - manager = McpProxyManager(mock_target_mcp, allow_write=False) + manager = McpProxyManager(mock_target_mcp, read_only=True) await manager._add_tools(mock_proxy) # Verify tool was not added (skipped) @@ -290,13 +290,13 @@ async def test_add_tools_no_annotations(self, mock_target_mcp, mock_proxy, mock_ @pytest.mark.asyncio async def test_add_tools_empty_annotations(self, mock_target_mcp, mock_proxy, mock_tool): - """Test that tools with empty annotations are skipped when allow_write=False.""" + """Test that tools with empty annotations are skipped when read_only=True.""" # Setup tool with empty annotations mock_tool.annotations = {} mock_proxy.get_tools.return_value = {'empty_annotations_tool': mock_tool} - manager = McpProxyManager(mock_target_mcp, allow_write=False) + manager = McpProxyManager(mock_target_mcp, read_only=True) await manager._add_tools(mock_proxy) # Verify tool was not added (skipped) diff --git a/tests/test_server.py b/tests/test_server.py index df31257..62bd1dc 100644 --- a/tests/test_server.py +++ b/tests/test_server.py @@ -38,7 +38,7 @@ async def test_setup_mcp_mode( mock_args.service = 'test-service' mock_args.region = 'us-east-1' mock_args.profile = None - mock_args.allow_write = False + mock_args.read_only = True # Mock the transport and proxy mock_transport = Mock() @@ -57,7 +57,7 @@ async def test_setup_mcp_mode( # Assert mock_create_transport.assert_called_once() mock_as_proxy.assert_called_once_with(mock_transport) - mock_proxy_manager_class.assert_called_once_with(mock_mcp, False) + mock_proxy_manager_class.assert_called_once_with(mock_mcp, True) mock_proxy_manager.add_proxy_content.assert_called_once_with(mock_proxy) @patch('aws_mcp_proxy.server.McpProxyManager') @@ -74,7 +74,7 @@ async def test_setup_mcp_mode_with_tools( mock_args.service = 'test-service' mock_args.region = 'us-east-1' mock_args.profile = None - mock_args.allow_write = False + mock_args.read_only = True # Mock the transport and proxy mock_transport = Mock() @@ -93,7 +93,7 @@ async def test_setup_mcp_mode_with_tools( # Assert mock_create_transport.assert_called_once() mock_as_proxy.assert_called_once_with(mock_transport) - mock_proxy_manager_class.assert_called_once_with(mock_mcp, False) + mock_proxy_manager_class.assert_called_once_with(mock_mcp, True) mock_proxy_manager.add_proxy_content.assert_called_once_with(mock_proxy) @patch('aws_mcp_proxy.server.McpProxyManager') @@ -110,7 +110,7 @@ async def test_setup_mcp_mode_tool_registration_error( mock_args.service = 'test-service' mock_args.region = 'us-east-1' mock_args.profile = None - mock_args.allow_write = False + mock_args.read_only = True # Mock the transport and proxy mock_transport = Mock() @@ -191,8 +191,7 @@ def test_create_sigv4_client(self, mock_sigv4_auth, mock_async_client, mock_sess mock_session.return_value = mock_session_instance # Act - with patch.dict('os.environ', {'AWS_REGION': 'us-west-2'}): - create_sigv4_client(service='test-service', profile='test-profile') + create_sigv4_client(service='test-service', region='us-west-2', profile='test-profile') # Assert mock_session.assert_called_once_with(profile_name='test-profile') @@ -209,7 +208,7 @@ def test_create_sigv4_client_no_credentials(self, mock_session): # Act & Assert with pytest.raises(ValueError) as exc_info: - create_sigv4_client() + create_sigv4_client(service='test-service', region='test-region') assert 'No AWS credentials found' in str(exc_info.value) def test_main_module_execution(self): diff --git a/tests/test_sigv4_helper.py b/tests/test_sigv4_helper.py index ee39573..11957ed 100644 --- a/tests/test_sigv4_helper.py +++ b/tests/test_sigv4_helper.py @@ -283,10 +283,10 @@ def test_create_sigv4_client_default(self, mock_client_class, mock_create_auth): mock_client_class.return_value = mock_client # Test client creation - result = create_sigv4_client() + result = create_sigv4_client(service='test-service', region='test-region') # Verify client was created correctly - mock_create_auth.assert_called_once_with('eks-mcp', None, None) + mock_create_auth.assert_called_once_with('test-service', 'test-region', None) # Check that AsyncClient was called with correct parameters call_args = mock_client_class.call_args @@ -309,7 +309,7 @@ def test_create_sigv4_client_with_custom_headers(self, mock_client_class, mock_c # Test client creation with custom headers custom_headers = {'Custom-Header': 'custom-value'} - result = create_sigv4_client(headers=custom_headers) + result = create_sigv4_client(service='test-service', region='test-region', headers=custom_headers) # Verify client was created with merged headers call_args = mock_client_class.call_args @@ -352,7 +352,9 @@ def test_create_sigv4_client_with_kwargs(self, mock_client_class, mock_create_au mock_client_class.return_value = mock_client # Test client creation with additional kwargs - result = create_sigv4_client(verify=False, proxies={'http': 'http://proxy:8080'}) + result = create_sigv4_client( + service='test-service', region='test-region', verify=False, proxies={'http': 'http://proxy:8080'} + ) # Verify client was created with additional kwargs call_args = mock_client_class.call_args @@ -382,11 +384,11 @@ def test_create_sigv4_client_with_prompt_context(self, mock_client_class, mock_c } result = create_sigv4_client( - service='eks-mcp', headers=prompt_context_headers, region='us-west-2' + service='test-service', headers=prompt_context_headers, region='us-west-2' ) # Verify client was created correctly with prompt context - mock_create_auth.assert_called_once_with('eks-mcp', 'us-west-2', None) + mock_create_auth.assert_called_once_with('test-service', 'us-west-2', None) # Check that AsyncClient was called with correct parameters including prompt headers call_args = mock_client_class.call_args diff --git a/tests/test_utils.py b/tests/test_utils.py index cf7f472..4e2f440 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -33,8 +33,8 @@ def test_create_transport_with_sigv4(self, mock_create_sigv4_client): mock_client = MagicMock() mock_create_sigv4_client.return_value = mock_client - url = 'https://eks-mcp.us-west-2.api.aws/mcp' - service = 'eks-mcp' + url = 'https://test-service.us-west-2.api.aws/mcp' + service = 'test-service' profile = 'test-profile' region = 'us-east-1' @@ -68,9 +68,9 @@ def test_create_transport_with_sigv4(self, mock_create_sigv4_client): @patch('aws_mcp_proxy.utils.create_sigv4_client') def test_create_transport_with_sigv4_no_profile(self, mock_create_sigv4_client): """Test creating transport without profile.""" - url = 'https://eks-mcp.us-west-2.api.aws/mcp' - service = 'eks-mcp' - region = 'us-west-2' + url = 'https://test-service.us-west-2.api.aws/mcp' + service = 'test-service' + region = 'test-region' result = create_transport_with_sigv4(url, service, region) @@ -93,7 +93,7 @@ class TestValidateRequiredArgs: def test_validate_service_name_with_service(self): """Test validation when service is provided.""" - endpoint = 'https://eks-mcp.us-west-2.api.aws' + endpoint = 'https://test-service.us-west-2.api.aws' service = 'custom-service' result = determine_service_name(endpoint, service) @@ -102,8 +102,8 @@ def test_validate_service_name_with_service(self): def test_validate_service_name_without_service_success(self): """Test validation when service is not provided but can be parsed.""" - endpoint = 'https://eks-mcp.us-west-2.api.aws' - expected_service = 'eks-mcp' + endpoint = 'https://test-service.us-west-2.api.aws' + expected_service = 'test-service' result = determine_service_name(endpoint) From 8e60bf179f458b707aa61876050c6f4a3ff67b96 Mon Sep 17 00:00:00 2001 From: acmlau Date: Thu, 2 Oct 2025 13:47:22 +0200 Subject: [PATCH 8/8] Fix formatting --- aws_mcp_proxy/sigv4_helper.py | 5 +---- tests/unit/test_sigv4_helper.py | 10 +++++++--- 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/aws_mcp_proxy/sigv4_helper.py b/aws_mcp_proxy/sigv4_helper.py index 7fef48b..e6bab65 100644 --- a/aws_mcp_proxy/sigv4_helper.py +++ b/aws_mcp_proxy/sigv4_helper.py @@ -17,7 +17,6 @@ import boto3 import httpx import logging -import os from botocore.auth import SigV4Auth from botocore.awsrequest import AWSRequest from botocore.credentials import Credentials @@ -150,9 +149,7 @@ def create_aws_session(profile: Optional[str] = None) -> boto3.Session: return session -def create_sigv4_auth( - service: str, region: str, profile: Optional[str] = None -) -> SigV4HTTPXAuth: +def create_sigv4_auth(service: str, region: str, profile: Optional[str] = None) -> SigV4HTTPXAuth: """Create SigV4 authentication for AWS requests. Args: diff --git a/tests/unit/test_sigv4_helper.py b/tests/unit/test_sigv4_helper.py index 904e845..67abb36 100644 --- a/tests/unit/test_sigv4_helper.py +++ b/tests/unit/test_sigv4_helper.py @@ -245,7 +245,6 @@ def test_create_sigv4_auth_default(self, mock_create_session): assert result.region == 'test-region' # default region assert result.credentials == mock_credentials - @patch('aws_mcp_proxy.sigv4_helper.create_aws_session') def test_create_sigv4_auth_with_explicit_region(self, mock_create_session): """Test creating SigV4 auth with explicit region parameter.""" @@ -308,7 +307,9 @@ def test_create_sigv4_client_with_custom_headers(self, mock_client_class, mock_c # Test client creation with custom headers custom_headers = {'Custom-Header': 'custom-value'} - result = create_sigv4_client(service='test-service', region='test-region', headers=custom_headers) + result = create_sigv4_client( + service='test-service', region='test-region', headers=custom_headers + ) # Verify client was created with merged headers call_args = mock_client_class.call_args @@ -352,7 +353,10 @@ def test_create_sigv4_client_with_kwargs(self, mock_client_class, mock_create_au # Test client creation with additional kwargs result = create_sigv4_client( - service='test-service', region='test-region', verify=False, proxies={'http': 'http://proxy:8080'} + service='test-service', + region='test-region', + verify=False, + proxies={'http': 'http://proxy:8080'}, ) # Verify client was created with additional kwargs