fix(tools): keep read-only roots out of write paths

Author: yu-xin-c

nanobot/agent/tools/filesystem.py

@@ -23,6 +23,9 @@
 class _FsTool(Tool):
     """Shared base for filesystem tools — common init and path resolution."""
 
+    _allow_extra_allowed_dirs = False
+    _allow_media_dir = False
+
     def __init__(
         self,
         workspace: Path | None = None,
@@ -57,7 +60,7 @@ def create(cls, ctx: Any) -> Tool:
         )
         sandbox_restricts = bool(ctx.config.exec.sandbox)
         allowed_dir = Path(ctx.workspace) if restrict else None
-        extra_read = [BUILTIN_SKILLS_DIR]
+        extra_read = [BUILTIN_SKILLS_DIR] if cls._allow_extra_allowed_dirs else None
         return cls(
             workspace=Path(ctx.workspace),
             allowed_dir=allowed_dir,
@@ -83,7 +86,8 @@ def _resolve(self, path: str) -> Path:
             path,
             access.project_path,
             access.allowed_root,
-            self._extra_allowed_dirs,
+            self._extra_allowed_dirs if self._allow_extra_allowed_dirs else None,
+            include_media_dir=self._allow_media_dir,
         )
 
     def _display_workspace(self) -> Path | None:
@@ -162,6 +166,8 @@ def _parse_page_range(pages: str, total: int) -> tuple[int, int]:
 class ReadFileTool(_FsTool):
     """Read file contents with optional line-based pagination."""
     _scopes = {"core", "subagent", "memory"}
+    _allow_extra_allowed_dirs = True
+    _allow_media_dir = True
 
     _MAX_CHARS = 128_000
     _DEFAULT_LIMIT = 2000
@@ -951,6 +957,8 @@ def _not_found_msg(old_text: str, content: str, path: str) -> str:
 class ListDirTool(_FsTool):
     """List directory contents with optional recursion."""
     _scopes = {"core", "subagent"}
+    _allow_extra_allowed_dirs = True
+    _allow_media_dir = True
 
     _DEFAULT_MAX = 200
     _IGNORE_DIRS = {

nanobot/agent/tools/path_utils.py

@@ -19,9 +19,16 @@ def resolve_workspace_path(
     workspace: Path | None = None,
     allowed_dir: Path | None = None,
     extra_allowed_dirs: list[Path] | None = None,
+    *,
+    include_media_dir: bool = True,
 ) -> Path:
     """Resolve path against workspace and enforce allowed directory containment."""
-    extra_roots = [get_media_dir(), *(extra_allowed_dirs or [])] if allowed_dir else None
+    extra_roots = None
+    if allowed_dir:
+        extra_roots = [
+            *([get_media_dir()] if include_media_dir else []),
+            *(extra_allowed_dirs or []),
+        ]
     return resolve_allowed_path(
         path,
         workspace=workspace,

tests/tools/test_filesystem_tools.py

@@ -342,13 +342,36 @@ async def test_extra_dirs_does_not_widen_write(self, tmp_path):
 
         workspace = tmp_path / "ws"
         workspace.mkdir()
-        outside = tmp_path / "outside"
-        outside.mkdir()
+        extra = tmp_path / "extra"
+        extra.mkdir()
+
+        tool = WriteFileTool(
+            workspace=workspace,
+            allowed_dir=workspace,
+            extra_allowed_dirs=[extra],
+        )
+        target = extra / "hack.txt"
+        result = await tool.execute(path=str(target), content="pwned")
+        assert "Error" in result
+        assert "outside" in result.lower()
+        assert not target.exists()
+
+    @pytest.mark.asyncio
+    async def test_media_dir_does_not_widen_write(self, tmp_path, monkeypatch):
+        from nanobot.agent.tools.filesystem import WriteFileTool
+
+        workspace = tmp_path / "ws"
+        workspace.mkdir()
+        media_dir = tmp_path / "media"
+        media_dir.mkdir()
+        monkeypatch.setattr("nanobot.agent.tools.path_utils.get_media_dir", lambda: media_dir)
 
         tool = WriteFileTool(workspace=workspace, allowed_dir=workspace)
-        result = await tool.execute(path=str(outside / "hack.txt"), content="pwned")
+        target = media_dir / "generated.txt"
+        result = await tool.execute(path=str(target), content="pwned")
         assert "Error" in result
         assert "outside" in result.lower()
+        assert not target.exists()
 
     @pytest.mark.asyncio
     async def test_read_still_blocked_for_unrelated_dir(self, tmp_path):
@@ -398,7 +421,11 @@ async def test_edit_blocked_in_extra_dir(self, tmp_path):
         skill_file.parent.mkdir()
         skill_file.write_text("# Weather\nOriginal content.")
 
-        tool = EditFileTool(workspace=workspace, allowed_dir=workspace)
+        tool = EditFileTool(
+            workspace=workspace,
+            allowed_dir=workspace,
+            extra_allowed_dirs=[skills_dir],
+        )
         result = await tool.execute(
             path=str(skill_file),
             old_text="Original content.",