From 6e240b4bc3d0c3210e7a3b196128fa9a798f5e81 Mon Sep 17 00:00:00 2001 From: Daoyuan Li <94409450+DaoyuanLi2816@users.noreply.github.com> Date: Fri, 23 Oct 2026 06:41:24 -0700 Subject: [PATCH] fix(sandbox): search the skills root across category mounts (#7338) * fix(sandbox): search skills across category mounts * fix(sandbox): address virtual search mount review Search explicitly configured category mounts regardless of ignored names, while retaining descendant filters and host-parent exclusion. Warn with the virtual mount path before re-raising broken-mount errors, defer ordinary path resolution, and cover lookahead across mount boundaries. --------- Co-authored-by: Willem Jiang --- README.md | 8 + .../harness/deerflow/sandbox/AGENTS.md | 8 + .../deerflow/sandbox/local/local_sandbox.py | 78 +++++- .../harness/deerflow/sandbox/search.py | 9 +- .../tests/test_local_sandbox_skill_search.py | 264 ++++++++++++++++++ 5 files changed, 363 insertions(+), 4 deletions(-) create mode 100644 backend/tests/test_local_sandbox_skill_search.py diff --git a/README.md b/README.md index 1ddda0ebc7..32621c0828 100644 --- a/README.md +++ b/README.md @@ -884,6 +884,14 @@ DeerFlow supports multiple sandbox execution modes: - **Docker Execution** (runs sandbox code in isolated Docker containers) - **Docker Execution with Kubernetes** (runs sandbox code in Kubernetes pods via provisioner service) +In Local Execution, `glob` and `grep` can search the skills root (default +`/mnt/skills`) across its mounted categories. Patterns stay relative to that +root, and one result limit covers all categories. User isolation, disabled-skill +filtering and read-only mounts still apply. Configured category mounts such as +`build` or `logs` remain searchable; ignore rules apply inside each mount. A broken +category mount stops the root search, and the Gateway warning log identifies its +virtual path. + Sandbox references in conversation state are server-owned. External run and thread-state APIs reject caller-supplied `sandbox` values; when restoring a checkpoint, the runtime resolves the reference against the authenticated user diff --git a/backend/packages/harness/deerflow/sandbox/AGENTS.md b/backend/packages/harness/deerflow/sandbox/AGENTS.md index 301ae4f202..57bf40fdfe 100644 --- a/backend/packages/harness/deerflow/sandbox/AGENTS.md +++ b/backend/packages/harness/deerflow/sandbox/AGENTS.md @@ -8,6 +8,14 @@ PowerShell command mapping escapes host-path whitespace/metacharacters for the c Local `grep` skips non-regular entries before reading. File reads and directory scans propagate errors, except `ENOENT`/`ENOTDIR`/`ELOOP`/`EISDIR`/`ESTALE` races (`ESTALE` when available). Error filenames are reverse-resolved at the local sandbox boundary, including custom mounts; permission errors identify the virtual failing path. +Unmapped virtual parents such as the default skills root search only direct +category mounts, never incidental host files. Shared search helpers accept a +relative prefix for root-anchored patterns; combine mounts under one result cap +with one lookahead match. Mapped roots keep their existing walk. Explicit +mounts bypass name ignores; descendants retain shared ignore rules. Warn with +an unavailable or non-directory mount's container path and re-raise; never +fall back to the host parent. Regression: `tests/test_local_sandbox_skill_search.py`. + **Network approval policy**: `SandboxMiddleware` uses `resolve_run_interaction_policy()`. Autonomous, webhook, scheduled, legacy unattended and GitHub-fallback runs auto-deny; explicit interactive overrides diff --git a/backend/packages/harness/deerflow/sandbox/local/local_sandbox.py b/backend/packages/harness/deerflow/sandbox/local/local_sandbox.py index 483a107b6a..74f5d089d3 100644 --- a/backend/packages/harness/deerflow/sandbox/local/local_sandbox.py +++ b/backend/packages/harness/deerflow/sandbox/local/local_sandbox.py @@ -23,7 +23,7 @@ from deerflow.sandbox.env_policy import build_sandbox_env from deerflow.sandbox.local.list_dir import list_dir from deerflow.sandbox.path_patterns import replace_output_path_matches from deerflow.sandbox.sandbox import Sandbox, _validate_extra_env -from deerflow.sandbox.search import GrepMatch, find_glob_matches, find_grep_matches +from deerflow.sandbox.search import GrepMatch, find_glob_matches, find_grep_matches, path_matches from deerflow.utils.atomic_write import publish_bytes_atomically logger = logging.getLogger(__name__) @@ -1234,9 +1234,64 @@ class LocalSandbox(Sandbox): # Re-raise with the original path for clearer error messages, hiding internal resolved paths raise type(e)(e.errno, e.strerror, path) from None + def _virtual_search_mounts(self, path: str) -> list[tuple[str, Path]] | None: + """Search only mounted children of an unmapped virtual parent. + + Default skills have four category mounts, not an aggregate host root. + Never substitute an incidental host directory for that virtual view. + Mapped roots (including policy-scoped skill views) keep their normal walk. + """ + if self._find_path_mapping(path) is not None: + return None + prefix = path.rstrip("/") + "/" + mounts = None + for mapping in self.path_mappings: + if not mapping.container_path.startswith(prefix): + continue + child = mapping.container_path[len(prefix) :] + if not child or "/" in child: + continue + if mounts is None: + mounts = [] + # Explicit mounts stay visible; ignore rules apply within each mount. + root = Path(self._resolved_local_paths[mapping]) + # A broken mount is an error, not permission to walk the host parent. + try: + if not stat.S_ISDIR(root.stat().st_mode): + raise NotADirectoryError(mapping.container_path) + except OSError: + logger.warning("Could not search virtual mount %s", mapping.container_path, exc_info=True) + raise + mounts.append((child, root)) + if mounts is not None: + # As with list_dir, an existing host file is not a virtual directory. + try: + mode = Path(path).stat().st_mode + except FileNotFoundError: + pass + else: + if not stat.S_ISDIR(mode): + raise NotADirectoryError(path) + return mounts + def glob(self, path: str, pattern: str, *, include_dirs: bool = False, max_results: int = 200) -> tuple[list[str], bool]: - resolved_path = Path(self._resolve_path(path)) try: + mounts = self._virtual_search_mounts(path) + if mounts is not None: + results = [] + for child, root in mounts: + virtual_dir = f"{path.rstrip('/')}/{child}" + if include_dirs and path_matches(pattern, child): + results.append(virtual_dir) + if len(results) <= max_results: + # One lookahead match proves whether the combined view + # exceeds the cap, without collecting a cap per mount. + matches, _ = find_glob_matches(root, pattern, include_dirs=include_dirs, max_results=max_results - len(results) + 1, relative_prefix=child) + results.extend(f"{virtual_dir}/{Path(match).relative_to(root).as_posix()}" for match in matches) + if len(results) > max_results: + return results[:max_results], True + return results, False + resolved_path = Path(self._resolve_path(path)) matches, truncated = find_glob_matches(resolved_path, pattern, include_dirs=include_dirs, max_results=max_results) except OSError as e: if e.filename is None: @@ -1257,8 +1312,25 @@ class LocalSandbox(Sandbox): case_sensitive: bool = False, max_results: int = 100, ) -> tuple[list[GrepMatch], bool]: - resolved_path = Path(self._resolve_path(path)) try: + mounts = self._virtual_search_mounts(path) + if mounts is not None: + results = [] + for child, root in mounts: + matches, _ = find_grep_matches( + root, + pattern, + glob_pattern=glob, + literal=literal, + case_sensitive=case_sensitive, + max_results=max_results - len(results) + 1, + relative_prefix=child, + ) + results.extend(GrepMatch(path=f"{path.rstrip('/')}/{child}/{Path(match.path).relative_to(root).as_posix()}", line_number=match.line_number, line=match.line) for match in matches) + if len(results) > max_results: + return results[:max_results], True + return results, False + resolved_path = Path(self._resolve_path(path)) matches, truncated = find_grep_matches( resolved_path, pattern, diff --git a/backend/packages/harness/deerflow/sandbox/search.py b/backend/packages/harness/deerflow/sandbox/search.py index 63df1e915f..634d12aabe 100644 --- a/backend/packages/harness/deerflow/sandbox/search.py +++ b/backend/packages/harness/deerflow/sandbox/search.py @@ -159,7 +159,7 @@ def is_binary_file(path: Path, sample_size: int = 8192) -> bool: return b"\0" in handle.read(sample_size) -def find_glob_matches(root: Path, pattern: str, *, include_dirs: bool = False, max_results: int = 200) -> tuple[list[str], bool]: +def find_glob_matches(root: Path, pattern: str, *, include_dirs: bool = False, max_results: int = 200, relative_prefix: str = "") -> tuple[list[str], bool]: matches: list[str] = [] truncated = False root = root.resolve() @@ -185,6 +185,8 @@ def find_glob_matches(root: Path, pattern: str, *, include_dirs: bool = False, m if include_dirs: for name in dirs: rel_path = (rel_dir / name).as_posix() + if relative_prefix: + rel_path = f"{relative_prefix}/{rel_path}" if path_matches(pattern, rel_path): matches.append(str(Path(current_root) / name)) if len(matches) >= max_results: @@ -195,6 +197,8 @@ def find_glob_matches(root: Path, pattern: str, *, include_dirs: bool = False, m if should_ignore_name(name): continue rel_path = (rel_dir / name).as_posix() + if relative_prefix: + rel_path = f"{relative_prefix}/{rel_path}" if path_matches(pattern, rel_path): matches.append(str(Path(current_root) / name)) if len(matches) >= max_results: @@ -248,6 +252,7 @@ def find_grep_matches( max_results: int = 100, max_file_size: int = DEFAULT_MAX_FILE_SIZE_BYTES, line_summary_length: int = DEFAULT_LINE_SUMMARY_LENGTH, + relative_prefix: str = "", ) -> tuple[list[GrepMatch], bool]: matches: list[GrepMatch] = [] truncated = False @@ -288,6 +293,8 @@ def find_grep_matches( yield Path(current_root) / name, (rel_dir / name).as_posix() for candidate_path, rel_path in candidate_files(): + if relative_prefix: + rel_path = f"{relative_prefix}/{rel_path}" if glob_pattern is not None and not path_matches(glob_pattern, rel_path): continue diff --git a/backend/tests/test_local_sandbox_skill_search.py b/backend/tests/test_local_sandbox_skill_search.py new file mode 100644 index 0000000000..1b9ccf3bba --- /dev/null +++ b/backend/tests/test_local_sandbox_skill_search.py @@ -0,0 +1,264 @@ +"""Search the default provider's category-only skill mounts through public tools.""" + +import errno +import logging +from pathlib import Path +from types import SimpleNamespace + +import pytest + +from deerflow.config import paths as paths_module +from deerflow.config.app_config import AppConfig +from deerflow.config.extensions_config import ExtensionsConfig, SkillStateConfig +from deerflow.sandbox import tools +from deerflow.sandbox.local.local_sandbox import LocalSandbox, PathMapping +from deerflow.sandbox.local.local_sandbox_provider import LocalSandboxProvider + + +def _write_skill(root, name): + target = root / name / "SKILL.md" + target.parent.mkdir(parents=True, exist_ok=True) + target.write_text(f"---\nname: {name}\ndescription: Search regression fixture\n---\nNEEDLE {name}\n", encoding="utf-8") + + +@pytest.fixture +def skill_search(tmp_path, monkeypatch): + monkeypatch.setenv("DEER_FLOW_HOME", str(tmp_path / "home")) + monkeypatch.setattr(paths_module, "_paths", None) + source = tmp_path / "skills" + for name in ("alpha", "beta", "disabled"): + _write_skill(source / "public", name) + config = AppConfig.model_validate({"skills": {"path": str(source)}, "sandbox": {"use": "deerflow.sandbox.local:LocalSandboxProvider"}}) + extensions = ExtensionsConfig(skills={"disabled": SkillStateConfig(enabled=False)}) + monkeypatch.setattr("deerflow.config.get_app_config", lambda: config) + monkeypatch.setattr("deerflow.config.app_config.get_app_config", lambda: config) + monkeypatch.setattr("deerflow.config.extensions_config.get_extensions_config", lambda: extensions) + monkeypatch.setattr(ExtensionsConfig, "from_file", lambda *args, **kwargs: extensions) + paths = paths_module.get_paths() + for user in ("alice", "bob"): + _write_skill(paths.user_custom_skills_dir(user), f"{user}-only") + provider = LocalSandboxProvider() + sandboxes = {user: provider.get(provider.acquire("shared-thread", user_id=user)) for user in ("alice", "bob")} + monkeypatch.setattr(tools, "ensure_sandbox_initialized", lambda runtime: sandboxes[runtime.context["user_id"]]) + + def runtime(user="alice"): + return SimpleNamespace( + context={"thread_id": "shared-thread", "user_id": user}, + config={}, + state={ + "sandbox": {"sandbox_id": sandboxes[user].id}, + "thread_data": { + "workspace_path": str(paths.sandbox_work_dir("shared-thread", user_id=user)), + "uploads_path": str(paths.sandbox_uploads_dir("shared-thread", user_id=user)), + "outputs_path": str(paths.sandbox_outputs_dir("shared-thread", user_id=user)), + }, + }, + ) + + return SimpleNamespace(sandboxes=sandboxes, runtime=runtime, paths=paths) + + +@pytest.mark.parametrize("tool_name", ["glob", "grep"]) +def test_public_search_of_category_only_root_preserves_visibility(skill_search, tool_name): + sandbox = skill_search.sandboxes["alice"] + assert "/mnt/skills" not in {m.container_path for m in sandbox.path_mappings} + assert "/mnt/skills/public/" in tools.ls_tool.func(runtime=skill_search.runtime(), path="/mnt/skills") + tool = tools.glob_tool if tool_name == "glob" else tools.grep_tool + pattern = "**/SKILL.md" if tool_name == "glob" else "NEEDLE" + for user, other in (("alice", "bob"), ("bob", "alice")): + result = tool.func(runtime=skill_search.runtime(user), path="/mnt/skills", pattern=pattern) + for name in ("alpha", "beta", f"{user}-only"): + assert f"/{name}/SKILL.md" in result + assert f"{other}-only" not in result + assert "disabled" not in result + assert str(skill_search.paths.base_dir) not in result + + +@pytest.mark.parametrize("pattern", ["public/**/*.md", "*/alpha/*.md", "**/SKILL.md"]) +def test_glob_pattern_is_relative_to_aggregate_root(skill_search, pattern): + matches, truncated = skill_search.sandboxes["alice"].glob("/mnt/skills", pattern) + assert "/mnt/skills/public/alpha/SKILL.md" in matches + if pattern.startswith("public/"): + assert all(path.startswith("/mnt/skills/public/") for path in matches) + if "alpha" in pattern: + assert matches == ["/mnt/skills/public/alpha/SKILL.md"] + assert not truncated + + +def test_grep_glob_is_relative_to_aggregate_root(skill_search): + matches, truncated = skill_search.sandboxes["alice"].grep("/mnt/skills", "NEEDLE", glob="custom/**/*.md", literal=True) + assert [match.path for match in matches] == ["/mnt/skills/custom/alice-only/SKILL.md"] + assert not truncated + + +def test_glob_can_match_category_directories(skill_search): + sandbox = skill_search.sandboxes["alice"] + assert sandbox.glob("/mnt/skills", "public", include_dirs=True) == (["/mnt/skills/public"], False) + assert sandbox.glob("/mnt/skills", "public") == ([], False) + + +@pytest.mark.parametrize("method", ["glob", "grep"]) +@pytest.mark.parametrize("limit", [1, 2, 3, 10]) +def test_search_has_one_budget_across_category_mounts(skill_search, method, limit): + sandbox = skill_search.sandboxes["alice"] + pattern = "**/SKILL.md" if method == "glob" else "NEEDLE" + matches, truncated = getattr(sandbox, method)("/mnt/skills", pattern, max_results=limit) + assert len(matches) == min(limit, 3) + assert truncated is (limit < 3) + + +@pytest.mark.parametrize("method", ["glob", "grep"]) +def test_category_search_missing_paths_and_no_matches_are_unchanged(skill_search, method): + search = getattr(skill_search.sandboxes["alice"], method) + pattern = "SKILL.md" if method == "glob" else "NEEDLE" + matches, _ = search("/mnt/skills/public", pattern) + assert len(matches) == 2 + assert search("/mnt/skills", "NEVER_PRESENT_9381") == ([], False) + with pytest.raises(FileNotFoundError): + search("/mnt/skills/missing", pattern) + + +@pytest.mark.parametrize("method", ["glob", "grep"]) +def test_public_search_still_refuses_user_data_root(skill_search, method): + tool = tools.glob_tool if method == "glob" else tools.grep_tool + result = tool.func(runtime=skill_search.runtime(), path="/mnt/user-data", pattern="*") + assert result.startswith("Error:") + + +@pytest.mark.parametrize("method", ["glob", "grep"]) +def test_aggregate_mapping_keeps_its_existing_search_path(tmp_path, method): + _write_skill(tmp_path / "public", "allowed") + sandbox = LocalSandbox("local:scoped", path_mappings=[PathMapping("/mnt/skills", str(tmp_path), read_only=True)]) + pattern = "**/SKILL.md" if method == "glob" else "NEEDLE" + matches, _ = getattr(sandbox, method)("/mnt/skills", pattern) + result_paths = matches if method == "glob" else [match.path for match in matches] + assert result_paths == ["/mnt/skills/public/allowed/SKILL.md"] + with pytest.raises(OSError): + sandbox.write_file("/mnt/skills/public/allowed/SKILL.md", "replacement") + + +@pytest.mark.parametrize("method", ["glob", "grep"]) +def test_virtual_parent_search_does_not_walk_unmounted_host_files(tmp_path, method): + parent = tmp_path / "virtual-parent" + parent.mkdir() + (parent / "unmounted.md").write_text("NEEDLE private host file", encoding="utf-8") + mounted = tmp_path / "mounted" + _write_skill(mounted, "allowed") + virtual = parent.as_posix() + sandbox = LocalSandbox("local:overlay", path_mappings=[PathMapping(f"{virtual}/public", str(mounted), read_only=True)]) + pattern = "**/*.md" if method == "glob" else "NEEDLE" + matches, _ = getattr(sandbox, method)(virtual, pattern) + result_paths = matches if method == "glob" else [match.path for match in matches] + assert result_paths == [f"{virtual}/public/allowed/SKILL.md"] + + +@pytest.mark.parametrize("method", ["glob", "grep"]) +def test_search_does_not_write_through_readonly_category_mounts(skill_search, method): + sandbox = skill_search.sandboxes["alice"] + pattern = "**/SKILL.md" if method == "glob" else "NEEDLE" + getattr(sandbox, method)("/mnt/skills", pattern) + with pytest.raises(OSError): + sandbox.write_file("/mnt/skills/public/alpha/SKILL.md", "replacement") + + +@pytest.mark.parametrize("method", ["glob", "grep"]) +def test_configured_virtual_root_and_empty_mounts(tmp_path, method): + mounted = tmp_path / "public" + mounted.mkdir() + sandbox = LocalSandbox("local:configured", path_mappings=[PathMapping("/opt/team-skills/public", str(mounted), read_only=True)]) + search = getattr(sandbox, method) + pattern = "**/SKILL.md" if method == "glob" else "NEEDLE" + assert search("/opt/team-skills", pattern) == ([], False) + _write_skill(mounted, "allowed") + matches, _ = search("/opt/team-skills/", pattern) + result_paths = matches if method == "glob" else [match.path for match in matches] + assert result_paths == ["/opt/team-skills/public/allowed/SKILL.md"] + + +@pytest.mark.parametrize("method", ["glob", "grep"]) +def test_search_errors_keep_virtual_paths(skill_search, monkeypatch, method): + def denied(root, *args, **kwargs): + raise PermissionError(errno.EACCES, "Permission denied", str(root / "blocked")) + + name = "find_glob_matches" if method == "glob" else "find_grep_matches" + monkeypatch.setattr(f"deerflow.sandbox.local.local_sandbox.{name}", denied) + with pytest.raises(PermissionError) as caught: + getattr(skill_search.sandboxes["alice"], method)("/mnt/skills", "*") + assert caught.value.filename.startswith("/mnt/skills/") + assert str(skill_search.paths.base_dir) not in str(caught.value) + + +@pytest.mark.parametrize("method", ["glob", "grep"]) +def test_existing_file_is_not_treated_as_a_virtual_parent(tmp_path, method): + parent = tmp_path / "not-a-directory" + parent.write_text("host file", encoding="utf-8") + mounted = tmp_path / "public" + mounted.mkdir() + virtual = parent.as_posix() + sandbox = LocalSandbox("local:invalid-parent", path_mappings=[PathMapping(f"{virtual}/public", str(mounted), read_only=True)]) + with pytest.raises(NotADirectoryError): + getattr(sandbox, method)(virtual, "*") + + +@pytest.mark.parametrize("method", ["glob", "grep"]) +def test_missing_mount_cannot_fall_back_to_unmounted_host_files(tmp_path, method): + parent = tmp_path / "virtual-parent" + parent.mkdir() + (parent / "unmounted.md").write_text("NEEDLE private host file", encoding="utf-8") + virtual = parent.as_posix() + sandbox = LocalSandbox("local:missing-mount", path_mappings=[PathMapping(f"{virtual}/public", str(tmp_path / "missing"), read_only=True)]) + with pytest.raises(FileNotFoundError): + getattr(sandbox, method)(virtual, "*") + + +@pytest.mark.parametrize("method", ["glob", "grep"]) +@pytest.mark.parametrize("mount_name", ["node_modules", "build", "dist", "out", "logs", "env", "target"]) +def test_explicit_mount_with_ignored_name_is_searchable(tmp_path, method, mount_name): + parent = tmp_path / "virtual-parent" + parent.mkdir() + (parent / "unmounted.md").write_text("NEEDLE private host file", encoding="utf-8") + mounted = tmp_path / "mounted" + _write_skill(mounted, "allowed") + _write_skill(mounted / "node_modules", "hidden") + (mounted / "ignored.log").write_text("NEEDLE ignored file", encoding="utf-8") + virtual = parent.as_posix() + mount_path = f"{virtual}/{mount_name}" + sandbox = LocalSandbox("local:explicit-mount", path_mappings=[PathMapping(mount_path, str(mounted), read_only=True)]) + expected = [f"{mount_path}/allowed/SKILL.md"] + assert f"{mount_path}/" in sandbox.list_dir(virtual) + assert "NEEDLE allowed" in sandbox.read_file(expected[0]) + + pattern = "**/*" if method == "glob" else "NEEDLE" + matches, truncated = getattr(sandbox, method)(virtual, pattern) + result_paths = matches if method == "glob" else [match.path for match in matches] + assert result_paths == expected + assert not truncated + root_pattern = f"{mount_name}/**/SKILL.md" + if method == "glob": + assert sandbox.glob(virtual, root_pattern) == (expected, False) + assert sandbox.glob(virtual, mount_name, include_dirs=True) == ([mount_path], False) + else: + matches, truncated = sandbox.grep(virtual, "NEEDLE", glob=root_pattern) + assert [match.path for match in matches] == expected + assert not truncated + + +@pytest.mark.parametrize("method", ["glob", "grep"]) +@pytest.mark.parametrize("mount_state", ["missing", "file"]) +def test_broken_mount_warns_with_its_virtual_category(skill_search, caplog, method, mount_state): + sandbox = skill_search.sandboxes["alice"] + mapping = next(mapping for mapping in sandbox.path_mappings if mapping.container_path == "/mnt/skills/public") + root = Path(mapping.local_path) + root.rename(root.with_name("public-backup")) + if mount_state == "file": + root.write_text("not a directory", encoding="utf-8") + tool = tools.glob_tool if method == "glob" else tools.grep_tool + pattern = "**/SKILL.md" if method == "glob" else "NEEDLE" + with caplog.at_level(logging.WARNING, logger="deerflow.sandbox.local.local_sandbox"): + result = tool.func(runtime=skill_search.runtime(), path="/mnt/skills", pattern=pattern) + expected_error = "Directory not found" if mount_state == "missing" else "Path is not a directory" + assert result == f"Error: {expected_error}: /mnt/skills" + warnings = [record for record in caplog.records if record.name == "deerflow.sandbox.local.local_sandbox" and record.levelno == logging.WARNING] + assert len(warnings) == 1 + assert mapping.container_path in warnings[0].getMessage() + assert str(root) not in result