Skip to content

Commit b7b3d3f

Browse files
author
Kyon Caldera
committed
refactor: change allow-write to read-only
refactor: fix tests broken because of changing allowing-write to read-only
1 parent 5be1c50 commit b7b3d3f

5 files changed

Lines changed: 33 additions & 33 deletions

File tree

README.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ Add this to your MCP client configuration, replacing env variables to match the
3535
Optional arguments you can add:
3636
- `--service`: AWS service name for SigV4 signing (inferred from endpoint if not provided)
3737
- `--profile`: AWS profile to use (uses AWS_PROFILE environment variable if not provided)
38-
- `--allow-write`: Allow tools that require write permissions to be enabled (by default, only tools with the `readOnlyHint` annotation are enabled)
38+
- `--read-only`: Disable tools which require write permissions. (tools which DO NOT require write permissions are annotated `readOnlyHint`)
3939

4040
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`.
4141

@@ -57,7 +57,7 @@ Example with all options
5757
"<service-code>",
5858
"--profile",
5959
"default",
60-
"--allow-write"
60+
"--read-only"
6161
]
6262
}
6363
}

aws_mcp_proxy/mcp_proxy_manager.py

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -23,15 +23,15 @@ class McpProxyManager:
2323

2424
logger = logging.getLogger(__name__)
2525

26-
def __init__(self, target_mcp: FastMCP, allow_write: bool = False):
26+
def __init__(self, target_mcp: FastMCP, read_only: bool = False):
2727
"""Initialize the MCP Proxy Manager.
2828
2929
Args:
3030
target_mcp: The target MCP server to add content to
31-
allow_write: Whether to allow tools that require write permissions
31+
read_only: Whether to allow tools that require write permissions
3232
"""
3333
self.target_mcp = target_mcp
34-
self.allow_write = allow_write
34+
self.read_only = read_only
3535

3636
async def add_proxy_content(self, proxy: FastMCP) -> None:
3737
"""Add tools, resources, and prompts from proxy to MCP server.
@@ -68,7 +68,7 @@ async def _add_tools(self, proxy: FastMCP) -> None:
6868
for tool_name, tool in tools.items():
6969
# Check the tool annotations and disable if needed
7070
annotations = tool.annotations
71-
if not self.allow_write:
71+
if self.read_only:
7272
# In readOnly mode, skip the tools with no readOnlyHint=True annotation
7373
if annotations and not annotations.readOnlyHint or not annotations:
7474
self.logger.info(f'Skipping tool {tool_name} needing write permissions')

aws_mcp_proxy/server.py

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,7 @@ async def setup_mcp_mode(mcp: FastMCP, args) -> None:
6363
proxy = FastMCP.as_proxy(transport)
6464

6565
# Use McpProxyManager to add proxy content
66-
proxy_manager = McpProxyManager(mcp, args.allow_write)
66+
proxy_manager = McpProxyManager(mcp, args.read_only)
6767
await proxy_manager.add_proxy_content(proxy)
6868

6969

@@ -81,7 +81,7 @@ def parse_args():
8181
aws-mcp-proxy https://eks-mcp.us-west-2.api.aws --service eks-mcp --profile default
8282
8383
# Run with write permissions enabled
84-
aws-mcp-proxy https://eks-mcp.us-west-2.api.aws --allow-write
84+
aws-mcp-proxy https://eks-mcp.us-west-2.api.aws --read-only
8585
""",
8686
)
8787

@@ -101,9 +101,9 @@ def parse_args():
101101
)
102102

103103
parser.add_argument(
104-
'--allow-write',
104+
'--read-only',
105105
action='store_true',
106-
help='Allow tools that require write permissions to be enabled',
106+
help='Disable tools which require write permissions',
107107
)
108108

109109
parser.add_argument(

tests/test_mcp_proxy_manager.py

Lines changed: 18 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -69,19 +69,19 @@ def mock_prompt(self):
6969
prompt.copy.return_value = prompt
7070
return prompt
7171

72-
def test_init_default_allow_write(self, mock_target_mcp):
73-
"""Test McpProxyManager initialization with default allow_write."""
72+
def test_init_default_read_only(self, mock_target_mcp):
73+
"""Test McpProxyManager initialization with default read_only."""
7474
manager = McpProxyManager(mock_target_mcp)
7575

7676
assert manager.target_mcp == mock_target_mcp
77-
assert manager.allow_write is False
77+
assert manager.read_only is False
7878

79-
def test_init_custom_allow_write(self, mock_target_mcp):
80-
"""Test McpProxyManager initialization with custom allow_write."""
81-
manager = McpProxyManager(mock_target_mcp, allow_write=True)
79+
def test_init_custom_read_only(self, mock_target_mcp):
80+
"""Test McpProxyManager initialization with custom read_only."""
81+
manager = McpProxyManager(mock_target_mcp, read_only=True)
8282

8383
assert manager.target_mcp == mock_target_mcp
84-
assert manager.allow_write is True
84+
assert manager.read_only is True
8585

8686
@pytest.mark.asyncio
8787
async def test_add_proxy_content_success(
@@ -93,7 +93,7 @@ async def test_add_proxy_content_success(
9393
mock_proxy.get_resources.return_value = {'test_resource': mock_resource}
9494
mock_proxy.get_prompts.return_value = {'test_prompt': mock_prompt}
9595

96-
manager = McpProxyManager(mock_target_mcp, allow_write=True)
96+
manager = McpProxyManager(mock_target_mcp, read_only=True)
9797
await manager.add_proxy_content(mock_proxy)
9898

9999
# Verify all methods were called
@@ -120,31 +120,31 @@ async def test_add_tools_success(self, mock_target_mcp, mock_proxy, mock_tool):
120120

121121
@pytest.mark.asyncio
122122
async def test_add_tools_skip_write_tools(self, mock_target_mcp, mock_proxy, mock_tool):
123-
"""Test that tools requiring write permissions are skipped when allow_write=False."""
123+
"""Test that tools requiring write permissions are skipped when read_only=True."""
124124
# Setup tool with write permissions required
125125
annotations = MagicMock()
126126
annotations.readOnlyHint = False
127127
mock_tool.annotations = annotations
128128

129129
mock_proxy.get_tools.return_value = {'write_tool': mock_tool}
130130

131-
manager = McpProxyManager(mock_target_mcp, allow_write=False)
131+
manager = McpProxyManager(mock_target_mcp, read_only=True)
132132
await manager._add_tools(mock_proxy)
133133

134134
# Verify tool was not added (skipped)
135135
mock_target_mcp.add_tool.assert_not_called()
136136

137137
@pytest.mark.asyncio
138-
async def test_add_tools_allow_write_tools(self, mock_target_mcp, mock_proxy, mock_tool):
139-
"""Test that tools requiring write permissions are added when allow_write=True."""
138+
async def test_add_tools_not_read_only_tools(self, mock_target_mcp, mock_proxy, mock_tool):
139+
"""Test that tools requiring write permissions are added when read_only=False."""
140140
# Setup tool with write permissions required
141141
annotations = MagicMock()
142142
annotations.readOnlyHint = False
143143
mock_tool.annotations = annotations
144144

145145
mock_proxy.get_tools.return_value = {'write_tool': mock_tool}
146146

147-
manager = McpProxyManager(mock_target_mcp, allow_write=True)
147+
manager = McpProxyManager(mock_target_mcp, read_only=False)
148148
await manager._add_tools(mock_proxy)
149149

150150
# Verify tool was added
@@ -160,7 +160,7 @@ async def test_add_tools_readonly_tools(self, mock_target_mcp, mock_proxy, mock_
160160

161161
mock_proxy.get_tools.return_value = {'readonly_tool': mock_tool}
162162

163-
manager = McpProxyManager(mock_target_mcp, allow_write=False)
163+
manager = McpProxyManager(mock_target_mcp, read_only=True)
164164
await manager._add_tools(mock_proxy)
165165

166166
# Verify tool was added
@@ -276,27 +276,27 @@ async def test_add_proxy_content_prompts_exception_handled(
276276

277277
@pytest.mark.asyncio
278278
async def test_add_tools_no_annotations(self, mock_target_mcp, mock_proxy, mock_tool):
279-
"""Test that tools with no annotations are skipped when allow_write=False."""
279+
"""Test that tools with no annotations are skipped when read_only=True."""
280280
# Setup tool with no annotations
281281
mock_tool.annotations = None
282282

283283
mock_proxy.get_tools.return_value = {'no_annotations_tool': mock_tool}
284284

285-
manager = McpProxyManager(mock_target_mcp, allow_write=False)
285+
manager = McpProxyManager(mock_target_mcp, read_only=True)
286286
await manager._add_tools(mock_proxy)
287287

288288
# Verify tool was not added (skipped)
289289
mock_target_mcp.add_tool.assert_not_called()
290290

291291
@pytest.mark.asyncio
292292
async def test_add_tools_empty_annotations(self, mock_target_mcp, mock_proxy, mock_tool):
293-
"""Test that tools with empty annotations are skipped when allow_write=False."""
293+
"""Test that tools with empty annotations are skipped when read_only=True."""
294294
# Setup tool with empty annotations
295295
mock_tool.annotations = {}
296296

297297
mock_proxy.get_tools.return_value = {'empty_annotations_tool': mock_tool}
298298

299-
manager = McpProxyManager(mock_target_mcp, allow_write=False)
299+
manager = McpProxyManager(mock_target_mcp, read_only=True)
300300
await manager._add_tools(mock_proxy)
301301

302302
# Verify tool was not added (skipped)

tests/test_server.py

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ async def test_setup_mcp_mode(
3737
mock_args.endpoint = 'https://test.example.com'
3838
mock_args.service = 'test-service'
3939
mock_args.profile = None
40-
mock_args.allow_write = False
40+
mock_args.read_only = True
4141

4242
# Mock the transport and proxy
4343
mock_transport = Mock()
@@ -56,7 +56,7 @@ async def test_setup_mcp_mode(
5656
# Assert
5757
mock_create_transport.assert_called_once()
5858
mock_as_proxy.assert_called_once_with(mock_transport)
59-
mock_proxy_manager_class.assert_called_once_with(mock_mcp, False)
59+
mock_proxy_manager_class.assert_called_once_with(mock_mcp, True)
6060
mock_proxy_manager.add_proxy_content.assert_called_once_with(mock_proxy)
6161

6262
@patch('aws_mcp_proxy.server.McpProxyManager')
@@ -72,7 +72,7 @@ async def test_setup_mcp_mode_with_tools(
7272
mock_args.endpoint = 'https://test.example.com'
7373
mock_args.service = 'test-service'
7474
mock_args.profile = None
75-
mock_args.allow_write = False
75+
mock_args.read_only = True
7676

7777
# Mock the transport and proxy
7878
mock_transport = Mock()
@@ -91,7 +91,7 @@ async def test_setup_mcp_mode_with_tools(
9191
# Assert
9292
mock_create_transport.assert_called_once()
9393
mock_as_proxy.assert_called_once_with(mock_transport)
94-
mock_proxy_manager_class.assert_called_once_with(mock_mcp, False)
94+
mock_proxy_manager_class.assert_called_once_with(mock_mcp, True)
9595
mock_proxy_manager.add_proxy_content.assert_called_once_with(mock_proxy)
9696

9797
@patch('aws_mcp_proxy.server.McpProxyManager')
@@ -107,7 +107,7 @@ async def test_setup_mcp_mode_tool_registration_error(
107107
mock_args.endpoint = 'https://test.example.com'
108108
mock_args.service = 'test-service'
109109
mock_args.profile = None
110-
mock_args.allow_write = False
110+
mock_args.read_only = True
111111

112112
# Mock the transport and proxy
113113
mock_transport = Mock()

0 commit comments

Comments
 (0)