mirror of
https://github.com/Significant-Gravitas/AutoGPT.git
synced 2026-04-30 03:00:41 -04:00
fix(copilot): rename SDK read_tool_result tool and fix path leak in error message
- Rename `_READ_TOOL_NAME` from `"Read"` to `"read_tool_result"` so the LLM can distinguish it from `read_file` (working-directory tool). The new name plus an updated description make its narrow scope (tool-results/ paths and workspace:// URIs) unambiguous. - Fix path leak in `_read_file_handler`: use `os.path.basename(file_path)` in the "Path not allowed" error, consistent with write/edit handlers. - Update `permissions.py` comment and all `permissions_test.py` assertions to use the new `mcp__copilot__read_tool_result` name.
This commit is contained in:
@@ -422,7 +422,7 @@ def apply_tool_permissions(
|
||||
permitted_sdk: set[str] = set()
|
||||
for s in effective:
|
||||
permitted_sdk.update(to_sdk_names(s))
|
||||
# Always include the internal Read tool (used by SDK for large/truncated outputs)
|
||||
# Always include the internal read_tool_result tool (used by SDK for large/truncated outputs)
|
||||
permitted_sdk.add(f"{MCP_TOOL_PREFIX}{_READ_TOOL_NAME}")
|
||||
|
||||
filtered_allowed = [t for t in base_allowed if t in permitted_sdk]
|
||||
|
||||
@@ -408,12 +408,12 @@ class TestApplyToolPermissions:
|
||||
assert "Task" not in allowed
|
||||
|
||||
def test_read_tool_always_included_even_when_blacklisted(self, mocker):
|
||||
"""mcp__copilot__Read must stay in allowed even if Read is explicitly blacklisted."""
|
||||
"""mcp__copilot__read_tool_result must stay in allowed even if Read is explicitly blacklisted."""
|
||||
mocker.patch(
|
||||
"backend.copilot.sdk.tool_adapter.get_copilot_tool_names",
|
||||
return_value=[
|
||||
"mcp__copilot__run_block",
|
||||
"mcp__copilot__Read",
|
||||
"mcp__copilot__read_tool_result",
|
||||
"Task",
|
||||
],
|
||||
)
|
||||
@@ -432,17 +432,17 @@ class TestApplyToolPermissions:
|
||||
# Explicitly blacklist Read
|
||||
perms = CopilotPermissions(tools=["Read"], tools_exclude=True)
|
||||
allowed, _ = apply_tool_permissions(perms, use_e2b=False)
|
||||
assert "mcp__copilot__Read" in allowed # always preserved for SDK internals
|
||||
assert "mcp__copilot__read_tool_result" in allowed # always preserved for SDK internals
|
||||
assert "mcp__copilot__run_block" in allowed
|
||||
assert "Task" in allowed
|
||||
|
||||
def test_read_tool_always_included_with_narrow_whitelist(self, mocker):
|
||||
"""mcp__copilot__Read must stay in allowed even when not in a whitelist."""
|
||||
"""mcp__copilot__read_tool_result must stay in allowed even when not in a whitelist."""
|
||||
mocker.patch(
|
||||
"backend.copilot.sdk.tool_adapter.get_copilot_tool_names",
|
||||
return_value=[
|
||||
"mcp__copilot__run_block",
|
||||
"mcp__copilot__Read",
|
||||
"mcp__copilot__read_tool_result",
|
||||
"Task",
|
||||
],
|
||||
)
|
||||
@@ -461,7 +461,7 @@ class TestApplyToolPermissions:
|
||||
# Whitelist only run_block — Read not listed
|
||||
perms = CopilotPermissions(tools=["run_block"], tools_exclude=False)
|
||||
allowed, _ = apply_tool_permissions(perms, use_e2b=False)
|
||||
assert "mcp__copilot__Read" in allowed # always preserved for SDK internals
|
||||
assert "mcp__copilot__read_tool_result" in allowed # always preserved for SDK internals
|
||||
assert "mcp__copilot__run_block" in allowed
|
||||
|
||||
def test_e2b_file_tools_included_when_sdk_builtin_whitelisted(self, mocker):
|
||||
@@ -470,7 +470,7 @@ class TestApplyToolPermissions:
|
||||
"backend.copilot.sdk.tool_adapter.get_copilot_tool_names",
|
||||
return_value=[
|
||||
"mcp__copilot__run_block",
|
||||
"mcp__copilot__Read",
|
||||
"mcp__copilot__read_tool_result",
|
||||
"mcp__copilot__read_file",
|
||||
"mcp__copilot__write_file",
|
||||
"Task",
|
||||
@@ -506,7 +506,7 @@ class TestApplyToolPermissions:
|
||||
"backend.copilot.sdk.tool_adapter.get_copilot_tool_names",
|
||||
return_value=[
|
||||
"mcp__copilot__run_block",
|
||||
"mcp__copilot__Read",
|
||||
"mcp__copilot__read_tool_result",
|
||||
"mcp__copilot__read_file",
|
||||
"Task",
|
||||
],
|
||||
@@ -532,8 +532,8 @@ class TestApplyToolPermissions:
|
||||
allowed, _ = apply_tool_permissions(perms, use_e2b=True)
|
||||
assert "mcp__copilot__read_file" not in allowed
|
||||
assert "mcp__copilot__run_block" in allowed
|
||||
# mcp__copilot__Read is always preserved for SDK internals
|
||||
assert "mcp__copilot__Read" in allowed
|
||||
# mcp__copilot__read_tool_result is always preserved for SDK internals
|
||||
assert "mcp__copilot__read_tool_result" in allowed
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
@@ -428,7 +428,7 @@ async def _read_file_handler(args: dict[str, Any]) -> dict[str, Any]:
|
||||
return _mcp_ok(numbered)
|
||||
|
||||
if not is_allowed_local_path(file_path, get_sdk_cwd()):
|
||||
return _mcp_err(f"Path not allowed: {file_path}")
|
||||
return _mcp_err(f"Path not allowed: {os.path.basename(file_path)}")
|
||||
|
||||
resolved = os.path.realpath(os.path.expanduser(file_path))
|
||||
try:
|
||||
@@ -452,9 +452,12 @@ async def _read_file_handler(args: dict[str, Any]) -> dict[str, Any]:
|
||||
return _mcp_err(f"Error reading file: {e}")
|
||||
|
||||
|
||||
_READ_TOOL_NAME = "Read"
|
||||
_READ_TOOL_NAME = "read_tool_result"
|
||||
_READ_TOOL_DESCRIPTION = (
|
||||
"Read a file from the local filesystem. "
|
||||
"Read an SDK-internal tool-result file or a workspace:// URI. "
|
||||
"Use this tool only for paths under ~/.claude/projects/.../tool-results/ "
|
||||
"or tool-outputs/, and for workspace:// URIs returned by other tools. "
|
||||
"For files in the working directory use read_file instead. "
|
||||
"Use offset and limit to read specific line ranges for large files."
|
||||
)
|
||||
_READ_TOOL_SCHEMA = {
|
||||
|
||||
Reference in New Issue
Block a user