Remove workspace boundary from read-only tools (read, glob, grep)
- SHA
51bdf4d46dcced9543f2b13a42af74da82dbedcf- Parents
-
9a32994 - Tree
4bcac94
51bdf4d
51bdf4d46dcced9543f2b13a42af74da82dbedcf9a32994
4bcac94| Status | File | + | - |
|---|---|---|---|
| M |
src/loader/tools/file_tools.py
|
4 | 6 |
| M |
src/loader/tools/search_tools.py
|
2 | 3 |
| M |
tests/test_tool_safety.py
|
6 | 6 |
| M |
tests/test_turn_iteration.py
|
2 | 2 |
src/loader/tools/file_tools.pymodified@@ -68,14 +68,13 @@ class ReadTool(Tool): | ||
| 68 | 68 | **kwargs: Any, |
| 69 | 69 | ) -> ToolResult: |
| 70 | 70 | try: |
| 71 | + # Reads are safe — don't enforce workspace boundary | |
| 71 | 72 | path = resolve_workspace_path( |
| 72 | 73 | file_path, |
| 73 | - workspace_root=self.workspace_root, | |
| 74 | + workspace_root=None, | |
| 74 | 75 | ) |
| 75 | 76 | except FileNotFoundError: |
| 76 | 77 | return ToolResult(f"File not found: {file_path}", is_error=True) |
| 77 | - except PermissionError as exc: | |
| 78 | - return ToolResult(f"Permission denied: {exc}", is_error=True) | |
| 79 | 78 | except Exception as exc: |
| 80 | 79 | return ToolResult(f"Error resolving file path: {exc}", is_error=True) |
| 81 | 80 | |
@@ -552,14 +551,13 @@ class GlobTool(Tool): | ||
| 552 | 551 | **kwargs: Any, |
| 553 | 552 | ) -> ToolResult: |
| 554 | 553 | try: |
| 554 | + # Glob is read-only — don't enforce workspace boundary | |
| 555 | 555 | base_path = resolve_workspace_path( |
| 556 | 556 | path, |
| 557 | - workspace_root=self.workspace_root, | |
| 557 | + workspace_root=None, | |
| 558 | 558 | ) |
| 559 | 559 | except FileNotFoundError: |
| 560 | 560 | return ToolResult(f"Directory not found: {path}", is_error=True) |
| 561 | - except PermissionError as exc: | |
| 562 | - return ToolResult(f"Permission denied: {exc}", is_error=True) | |
| 563 | 561 | except Exception as exc: |
| 564 | 562 | return ToolResult(f"Error resolving directory: {exc}", is_error=True) |
| 565 | 563 | |
src/loader/tools/search_tools.pymodified@@ -73,14 +73,13 @@ class GrepTool(Tool): | ||
| 73 | 73 | **kwargs: Any, |
| 74 | 74 | ) -> ToolResult: |
| 75 | 75 | try: |
| 76 | + # Grep is read-only — don't enforce workspace boundary | |
| 76 | 77 | base_path = resolve_workspace_path( |
| 77 | 78 | path, |
| 78 | - workspace_root=self.workspace_root, | |
| 79 | + workspace_root=None, | |
| 79 | 80 | ) |
| 80 | 81 | except FileNotFoundError: |
| 81 | 82 | return ToolResult(f"Path not found: {path}", is_error=True) |
| 82 | - except PermissionError as exc: | |
| 83 | - return ToolResult(f"Permission denied: {exc}", is_error=True) | |
| 84 | 83 | except Exception as exc: |
| 85 | 84 | return ToolResult(f"Error resolving search path: {exc}", is_error=True) |
| 86 | 85 | |
tests/test_tool_safety.pymodified@@ -25,17 +25,17 @@ async def test_read_tool_blocks_binary_file(temp_dir: Path) -> None: | ||
| 25 | 25 | |
| 26 | 26 | |
| 27 | 27 | @pytest.mark.asyncio |
| 28 | -async def test_read_tool_blocks_symlink_escape(temp_dir: Path) -> None: | |
| 28 | +async def test_read_tool_allows_outside_workspace(temp_dir: Path) -> None: | |
| 29 | + """Reads are safe and should not enforce workspace boundaries.""" | |
| 29 | 30 | outside = temp_dir.parent / "outside.txt" |
| 30 | 31 | outside.write_text("outside\n") |
| 31 | - inside_link = temp_dir / "escape.txt" | |
| 32 | - inside_link.symlink_to(outside) | |
| 33 | 32 | tool = ReadTool(workspace_root=temp_dir) |
| 34 | 33 | |
| 35 | - result = await tool.execute(file_path=str(inside_link)) | |
| 34 | + result = await tool.execute(file_path=str(outside)) | |
| 36 | 35 | |
| 37 | - assert result.is_error | |
| 38 | - assert "workspace boundary" in result.output.lower() | |
| 36 | + assert not result.is_error | |
| 37 | + assert "outside" in result.output | |
| 38 | + outside.unlink() | |
| 39 | 39 | |
| 40 | 40 | |
| 41 | 41 | @pytest.mark.asyncio |
tests/test_turn_iteration.pymodified@@ -110,7 +110,7 @@ async def test_turn_iteration_executes_native_tool_batch_and_continues( | ||
| 110 | 110 | ToolCall( |
| 111 | 111 | id="read-1", |
| 112 | 112 | name="read", |
| 113 | - arguments={"file_path": "README.md"}, | |
| 113 | + arguments={"file_path": str(readme)}, | |
| 114 | 114 | ) |
| 115 | 115 | ], |
| 116 | 116 | ) |
@@ -129,7 +129,7 @@ async def test_turn_iteration_executes_native_tool_batch_and_continues( | ||
| 129 | 129 | ) |
| 130 | 130 | |
| 131 | 131 | assert decision.action == TurnIterationAction.CONTINUE |
| 132 | - assert decision.new_actions_taken == ["read: {'file_path': 'README.md'}"] | |
| 132 | + assert decision.new_actions_taken == [f"read: {{'file_path': '{readme}'}}"] | |
| 133 | 133 | assert prepared.summary.assistant_messages[-1].tool_calls[0].name == "read" |
| 134 | 134 | assert len(prepared.summary.tool_result_messages) == 1 |
| 135 | 135 | assert "Loader runtime notes" in prepared.summary.tool_result_messages[0].content |