From 348228cd61eac0ccdfac7efb2e0ede1a193252f9 Mon Sep 17 00:00:00 2001 From: hinotoi-agent Date: Sun, 10 May 2026 13:58:24 +0800 Subject: [PATCH] fix: contain markdown browser workspace paths --- .../requests_markdown_browser.py | 25 ++++- tests/test_markdown_browser_paths.py | 106 ++++++++++++++++++ 2 files changed, 125 insertions(+), 6 deletions(-) create mode 100644 tests/test_markdown_browser_paths.py diff --git a/autoagent/environment/markdown_browser/requests_markdown_browser.py b/autoagent/environment/markdown_browser/requests_markdown_browser.py index b8b17c6..b5b7176 100644 --- a/autoagent/environment/markdown_browser/requests_markdown_browser.py +++ b/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. diff --git a/tests/test_markdown_browser_paths.py b/tests/test_markdown_browser_paths.py new file mode 100644 index 0000000..cccc2c0 --- /dev/null +++ b/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")