[security] fix: contain File Surfer workspace paths

autoagent/environment/markdown_browser/requests_markdown_browser.py

@@ -93,14 +93,27 @@ def __init__(  # type: ignore
     def address(self) -> str:
         """Return the address of the current page."""
         return self.history[-1][0]
+    def _contained_path(self, root: str, candidate: str) -> str:
+        root_path = os.path.realpath(os.path.abspath(os.path.expanduser(root)))
+        candidate_path = os.path.realpath(os.path.abspath(os.path.expanduser(candidate)))
+        if os.path.commonpath([root_path, candidate_path]) != root_path:
+            raise ValueError(f"Path escapes the workspace: {candidate}")
+        return candidate_path
+
     def _convert_docker_to_local(self, path: str) -> str:
-        assert self.docker_workplace in path, f"The path must be a absolute path from `{self.docker_workplace}/` directory"
-        local_path = path.replace(self.docker_workplace, self.local_workplace)
-        return local_path
+        docker_root = pathlib.PurePosixPath(self.docker_workplace)
+        docker_path = pathlib.PurePosixPath(path)
+        try:
+            relative_path = docker_path.relative_to(docker_root)
+        except ValueError as exc:
+            raise ValueError(f"The path must be an absolute path under `{self.docker_workplace}/`") from exc
+        local_path = os.path.join(self.local_workplace, *relative_path.parts)
+        return self._contained_path(self.local_workplace, local_path)
+
     def _convert_local_to_docker(self, path: str) -> str:
-        assert self.local_workplace in path, f"The path must be a absolute path from `{self.local_workplace}/` directory"
-        docker_path = path.replace(self.local_workplace, self.docker_workplace)
-        return docker_path
+        local_path = self._contained_path(self.local_workplace, path)
+        relative_path = os.path.relpath(local_path, os.path.realpath(os.path.abspath(self.local_workplace)))
+        return pathlib.PurePosixPath(self.docker_workplace, *pathlib.PurePath(relative_path).parts).as_posix()
 
     def set_address(self, uri_or_path: str) -> None:
         """Sets the address of the current page.

tests/test_markdown_browser_paths.py

@@ -0,0 +1,106 @@
+import importlib.util
+import sys
+import types
+from pathlib import Path
+
+
+def load_browser(monkeypatch):
+    monkeypatch.setitem(sys.modules, "pathvalidate", types.ModuleType("pathvalidate"))
+    for name in ["autoagent", "autoagent.environment", "autoagent.environment.markdown_browser"]:
+        package = types.ModuleType(name)
+        package.__path__ = []
+        monkeypatch.setitem(sys.modules, name, package)
+
+    abstract = types.ModuleType("autoagent.environment.markdown_browser.abstract_markdown_browser")
+    abstract.AbstractMarkdownBrowser = object
+    monkeypatch.setitem(sys.modules, abstract.__name__, abstract)
+
+    search = types.ModuleType("autoagent.environment.markdown_browser.markdown_search")
+    search.AbstractMarkdownSearch = object
+
+    class BingMarkdownSearch:
+        def search(self, query):
+            return ""
+
+    search.BingMarkdownSearch = BingMarkdownSearch
+    monkeypatch.setitem(sys.modules, search.__name__, search)
+
+    mdconvert = types.ModuleType("autoagent.environment.markdown_browser.mdconvert")
+
+    class FileConversionException(Exception):
+        pass
+
+    class UnsupportedFormatException(Exception):
+        pass
+
+    class MarkdownConverter:
+        def convert_local(self, path):
+            return types.SimpleNamespace(title=None, text_content=Path(path).read_text(errors="replace"))
+
+        def convert_response(self, response):
+            return types.SimpleNamespace(title=None, text_content="")
+
+    mdconvert.FileConversionException = FileConversionException
+    mdconvert.UnsupportedFormatException = UnsupportedFormatException
+    mdconvert.MarkdownConverter = MarkdownConverter
+    monkeypatch.setitem(sys.modules, mdconvert.__name__, mdconvert)
+
+    browser_path = (
+        Path(__file__).resolve().parents[1]
+        / "autoagent"
+        / "environment"
+        / "markdown_browser"
+        / "requests_markdown_browser.py"
+    )
+    spec = importlib.util.spec_from_file_location(
+        "autoagent.environment.markdown_browser.requests_markdown_browser",
+        browser_path,
+    )
+    module = importlib.util.module_from_spec(spec)
+    monkeypatch.setitem(sys.modules, spec.name, module)
+    spec.loader.exec_module(module)
+    return module.RequestsMarkdownBrowser
+
+
+def test_docker_path_conversion_rejects_workspace_escape(monkeypatch, tmp_path):
+    RequestsMarkdownBrowser = load_browser(monkeypatch)
+    (tmp_path / "workplace").mkdir()
+    (tmp_path / "secret.txt").write_text("secret")
+    browser = RequestsMarkdownBrowser(local_root=str(tmp_path), workplace_name="workplace", start_page="about:blank")
+
+    try:
+        browser._convert_docker_to_local("/workplace/../secret.txt")
+    except ValueError as exc:
+        assert "escapes the workspace" in str(exc)
+    else:
+        raise AssertionError("workspace escape was accepted")
+
+
+def test_docker_path_conversion_allows_workspace_file(monkeypatch, tmp_path):
+    RequestsMarkdownBrowser = load_browser(monkeypatch)
+    workplace = tmp_path / "workplace"
+    workplace.mkdir()
+    safe_file = workplace / "note.txt"
+    safe_file.write_text("ok")
+    browser = RequestsMarkdownBrowser(local_root=str(tmp_path), workplace_name="workplace", start_page="about:blank")
+
+    converted = browser._convert_docker_to_local("/workplace/note.txt")
+
+    assert Path(converted) == safe_file.resolve()
+    assert browser.open_local_file(converted).strip() == "ok"
+
+
+def test_local_path_conversion_rejects_sibling_prefix(monkeypatch, tmp_path):
+    RequestsMarkdownBrowser = load_browser(monkeypatch)
+    (tmp_path / "workplace").mkdir()
+    sibling = tmp_path / "workplace-other" / "secret.txt"
+    sibling.parent.mkdir()
+    sibling.write_text("secret")
+    browser = RequestsMarkdownBrowser(local_root=str(tmp_path), workplace_name="workplace", start_page="about:blank")
+
+    try:
+        browser._convert_local_to_docker(str(sibling))
+    except ValueError as exc:
+        assert "escapes the workspace" in str(exc)
+    else:
+        raise AssertionError("sibling prefix path was accepted")