Skip to content

Commit 9a4f317

Browse files
committed
feat(server.py): enforce absolute file paths for text file operations to prevent errors related to relative paths
docs(server.py): update descriptions to clarify that file paths must be absolute test(tests/test_server.py): add tests to ensure handlers raise errors for relative file paths and handle absolute paths correctly
1 parent dbd6994 commit 9a4f317

File tree

2 files changed

+114
-20
lines changed

2 files changed

+114
-20
lines changed

src/mcp_text_editor/server.py

Lines changed: 25 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
import json
44
import logging
5+
import os
56
import traceback
67
from collections.abc import Sequence
78
from typing import Any, Dict, List
@@ -23,7 +24,7 @@ class GetTextFileContentsHandler:
2324
"""Handler for getting text file contents."""
2425

2526
name = "get_text_file_contents"
26-
description = "Read text file contents from multiple files and line ranges. Returns file contents with hashes for concurrency control and line numbers for reference. The hashes are used to detect conflicts when editing the files."
27+
description = "Read text file contents from multiple files and line ranges. Returns file contents with hashes for concurrency control and line numbers for reference. The hashes are used to detect conflicts when editing the files. File paths must be absolute."
2728

2829
def __init__(self):
2930
self.editor = TextEditor()
@@ -44,7 +45,7 @@ def get_tool_description(self) -> Tool:
4445
"properties": {
4546
"file_path": {
4647
"type": "string",
47-
"description": "Path to the text file",
48+
"description": "Path to the text file. File path must be absolute.",
4849
},
4950
"ranges": {
5051
"type": "array",
@@ -81,11 +82,15 @@ def get_tool_description(self) -> Tool:
8182
async def run_tool(self, arguments: Dict[str, Any]) -> Sequence[TextContent]:
8283
"""Execute the tool with given arguments."""
8384
try:
84-
# Check for required argument 'files'
8585
if "files" not in arguments:
8686
raise RuntimeError("Missing required argument: 'files'")
8787

88-
# Handle request
88+
for file_info in arguments["files"]:
89+
if not os.path.isabs(file_info["file_path"]):
90+
raise RuntimeError(
91+
f"File path must be absolute: {file_info['file_path']}"
92+
)
93+
8994
encoding = arguments.get("encoding", "utf-8")
9095
result = await self.editor.read_multiple_ranges(
9196
arguments["files"], encoding=encoding
@@ -104,7 +109,7 @@ class EditTextFileContentsHandler:
104109
"""Handler for editing text file contents."""
105110

106111
name = "edit_text_file_contents"
107-
description = "A line editor that supports editing text file contents by specifying line ranges and content. It handles multiple patches in a single operation with hash-based conflict detection. IMPORTANT: (1) Before using this tool, you must first get the file's current hash and range hashes and line numbers using get_text_file_contents. (2) To avoid line number shifts affecting your patches, use get_text_file_contents to read the SAME ranges you plan to edit before making changes. different line numbers have different rangehashes.(3) Patches must be specified from bottom to top to handle line number shifts correctly, as edits to lower lines don't affect the line numbers of higher lines. (4) To append content to a file, first get the total number of lines with get_text_file_contents, then specify a patch with line_start = total_lines + 1 and line_end = total_lines. This indicates an append operation and range_hash is not required. Similarly, range_hash is not required for new file creation."
112+
description = "A line editor that supports editing text file contents by specifying line ranges and content. It handles multiple patches in a single operation with hash-based conflict detection. File paths must be absolute. IMPORTANT: (1) Before using this tool, you must first get the file's current hash and range hashes and line numbers using get_text_file_contents. (2) To avoid line number shifts affecting your patches, use get_text_file_contents to read the SAME ranges you plan to edit before making changes. different line numbers have different rangehashes.(3) Patches must be specified from bottom to top to handle line number shifts correctly, as edits to lower lines don't affect the line numbers of higher lines. (4) To append content to a file, first get the total number of lines with get_text_file_contents, then specify a patch with line_start = total_lines + 1 and line_end = total_lines. This indicates an append operation and range_hash is not required. Similarly, range_hash is not required for new file creation."
108113

109114
def __init__(self):
110115
self.editor = TextEditor()
@@ -122,8 +127,14 @@ def get_tool_description(self) -> Tool:
122127
"items": {
123128
"type": "object",
124129
"properties": {
125-
"path": {"type": "string"},
126-
"file_hash": {"type": "string"},
130+
"path": {
131+
"type": "string",
132+
"description": "Path to the text file. File path must be absolute.",
133+
},
134+
"file_hash": {
135+
"type": "string",
136+
"description": "Hash of the file contents when get_text_file_contents is called.",
137+
},
127138
"patches": {
128139
"type": "array",
129140
"items": {
@@ -175,13 +186,20 @@ async def run_tool(self, arguments: Dict[str, Any]) -> Sequence[TextContent]:
175186
return [TextContent(type="text", text=json.dumps(results, indent=2))]
176187

177188
for file_operation in files:
189+
# First check if required fields exist
178190
if "path" not in file_operation:
179191
raise RuntimeError("Missing required field: path")
180192
if "file_hash" not in file_operation:
181193
raise RuntimeError("Missing required field: file_hash")
182194
if "patches" not in file_operation:
183195
raise RuntimeError("Missing required field: patches")
184196

197+
# Then check if path is absolute
198+
if not os.path.isabs(file_operation["path"]):
199+
raise RuntimeError(
200+
f"File path must be absolute: {file_operation['path']}"
201+
)
202+
185203
try:
186204
file_path = file_operation["path"]
187205
file_hash = file_operation["file_hash"]

tests/test_server.py

Lines changed: 89 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
"""Tests for the MCP Text Editor Server."""
22

33
import json
4+
from pathlib import Path
45
from typing import List
56

67
import pytest
@@ -9,6 +10,8 @@
910
from pytest_mock import MockerFixture
1011

1112
from mcp_text_editor.server import (
13+
EditTextFileContentsHandler,
14+
GetTextFileContentsHandler,
1215
app,
1316
call_tool,
1417
edit_contents_handler,
@@ -44,14 +47,15 @@ async def test_list_tools():
4447
@pytest.mark.asyncio
4548
async def test_edit_contents_empty_patches():
4649
"""Test editing file contents with empty patches list."""
47-
request = {"files": [{"path": "test.txt", "file_hash": "hash123", "patches": []}]}
50+
# Convert relative path to absolute
51+
test_path = str(Path("test.txt").absolute())
52+
request = {"files": [{"path": test_path, "file_hash": "hash123", "patches": []}]}
4853

4954
response = await edit_contents_handler.run_tool(request)
50-
assert isinstance(response, list)
51-
assert len(response) == 1
55+
assert isinstance(response[0], TextContent)
5256
content = json.loads(response[0].text)
53-
assert content["test.txt"]["result"] == "error"
54-
assert content["test.txt"]["reason"] == "Empty patches list"
57+
assert content[test_path]["result"] == "error"
58+
assert content[test_path]["reason"] == "Empty patches list"
5559

5660

5761
@pytest.mark.asyncio
@@ -75,7 +79,9 @@ async def test_get_contents_handler(test_file):
7579
@pytest.mark.asyncio
7680
async def test_get_contents_handler_invalid_file(test_file):
7781
"""Test GetTextFileContents handler with invalid file."""
78-
args = {"files": [{"file_path": "nonexistent.txt", "ranges": [{"start": 1}]}]}
82+
# Convert relative path to absolute
83+
nonexistent_path = str(Path("nonexistent.txt").absolute())
84+
args = {"files": [{"file_path": nonexistent_path, "ranges": [{"start": 1}]}]}
7985
with pytest.raises(RuntimeError) as exc_info:
8086
await get_contents_handler.run_tool(args)
8187
assert "File not found" in str(exc_info.value)
@@ -159,11 +165,12 @@ async def test_call_tool_error_handling():
159165
await call_tool("get_text_file_contents", {"invalid": "args"})
160166
assert "Missing required argument" in str(exc_info.value)
161167

162-
# Test with invalid file path
168+
# Convert relative path to absolute
169+
nonexistent_path = str(Path("nonexistent.txt").absolute())
163170
with pytest.raises(RuntimeError) as exc_info:
164171
await call_tool(
165172
"get_text_file_contents",
166-
{"files": [{"file_path": "nonexistent.txt", "ranges": [{"start": 1}]}]},
173+
{"files": [{"file_path": nonexistent_path, "ranges": [{"start": 1}]}]},
167174
)
168175
assert "File not found" in str(exc_info.value)
169176

@@ -310,13 +317,13 @@ async def test_edit_contents_handler_malformed_input():
310317
@pytest.mark.asyncio
311318
async def test_edit_contents_handler_empty_patches():
312319
"""Test EditTextFileContents handler with empty patches."""
313-
edit_args = {
314-
"files": [{"path": "test.txt", "file_hash": "any_hash", "patches": []}]
315-
}
320+
# Convert relative path to absolute
321+
test_path = str(Path("test.txt").absolute())
322+
edit_args = {"files": [{"path": test_path, "file_hash": "any_hash", "patches": []}]}
316323
result = await edit_contents_handler.run_tool(edit_args)
317324
edit_results = json.loads(result[0].text)
318-
assert edit_results["test.txt"]["result"] == "error"
319-
assert edit_results["test.txt"]["reason"] == "Empty patches list"
325+
assert edit_results[test_path]["result"] == "error"
326+
assert edit_results[test_path]["reason"] == "Empty patches list"
320327

321328

322329
@pytest.mark.asyncio
@@ -462,3 +469,72 @@ async def test_edit_contents_handler_multiple_patches(tmp_path):
462469
content = f.read()
463470
assert "Modified Line 2" in content
464471
assert "Modified Line 4" in content
472+
473+
474+
@pytest.mark.asyncio
475+
async def test_get_contents_relative_path():
476+
handler = GetTextFileContentsHandler()
477+
with pytest.raises(RuntimeError, match="File path must be absolute:.*"):
478+
await handler.run_tool(
479+
{
480+
"files": [
481+
{"file_path": "relative/path/file.txt", "ranges": [{"start": 1}]}
482+
]
483+
}
484+
)
485+
486+
487+
@pytest.mark.asyncio
488+
async def test_get_contents_absolute_path():
489+
handler = GetTextFileContentsHandler()
490+
abs_path = str(Path("/absolute/path/file.txt").absolute())
491+
492+
# モックを非同期関数として定義
493+
async def mock_read_multiple_ranges(*args, **kwargs):
494+
return []
495+
496+
# モックを設定
497+
handler.editor.read_multiple_ranges = mock_read_multiple_ranges
498+
499+
result = await handler.run_tool(
500+
{"files": [{"file_path": abs_path, "ranges": [{"start": 1}]}]}
501+
)
502+
assert isinstance(result[0], TextContent)
503+
504+
505+
@pytest.mark.asyncio
506+
async def test_edit_contents_relative_path():
507+
handler = EditTextFileContentsHandler()
508+
with pytest.raises(RuntimeError, match="File path must be absolute:.*"):
509+
await handler.run_tool(
510+
{
511+
"files": [
512+
{
513+
"path": "relative/path/file.txt",
514+
"file_hash": "hash123",
515+
"patches": [{"contents": "test"}],
516+
}
517+
]
518+
}
519+
)
520+
521+
522+
@pytest.mark.asyncio
523+
async def test_edit_contents_absolute_path():
524+
handler = EditTextFileContentsHandler()
525+
abs_path = str(Path("/absolute/path/file.txt").absolute())
526+
# モックを使用して実際のファイル操作を避ける
527+
handler.editor.edit_file_contents = lambda *args, **kwargs: {"result": "success"}
528+
529+
result = await handler.run_tool(
530+
{
531+
"files": [
532+
{
533+
"path": abs_path,
534+
"file_hash": "hash123",
535+
"patches": [{"contents": "test"}],
536+
}
537+
]
538+
}
539+
)
540+
assert isinstance(result[0], TextContent)

0 commit comments

Comments
 (0)