From f92118e1e985906057eea757a930dfc8f8902708 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=93=88=E5=9F=BA=E7=B1=B3?= <140241684+BlueX888@users.noreply.github.com> Date: Fri, 25 Sep 2026 21:27:25 +0800 Subject: [PATCH] fix(sandbox): list the skills root when only category mounts exist (#5857) * fix(sandbox): list the skills root when only category mounts exist With the default (non-policy-scoped) LocalSandboxProvider layout, the four skill categories (/mnt/skills/public, custom, legacy, integrations) are mounted at separate host directories and there is no mapping for the aggregate /mnt/skills root itself. LocalSandbox.list_dir resolved the root to the unmapped host path, and the host list_dir raised FileNotFoundError before the virtual sub-directory overlay -- which exists exactly so the agent can discover the categories via "ls /mnt/skills" -- could run, so the ls tool reported "Directory not found: /mnt/skills" even with skills installed. Treat a FileNotFoundError from the host scan as an empty listing when mappings are mounted inside the requested container path, so the overlay can surface those children. Paths with nothing mounted inside them keep raising FileNotFoundError as before. * fix(sandbox): preserve missing virtual parent errors * test(screening): align PII fixture with keyed placeholders * fix(sandbox): preserve non-directory errors for virtual parents --------- Co-authored-by: Willem Jiang --- .../deerflow/sandbox/local/local_sandbox.py | 57 ++++++++++++------- backend/tests/test_jev_screening_pipeline.py | 10 +++- .../test_local_sandbox_provider_mounts.py | 48 ++++++++++++++++ ...est_local_sandbox_virtual_path_contract.py | 27 +++++++++ 4 files changed, 117 insertions(+), 25 deletions(-) diff --git a/backend/packages/harness/deerflow/sandbox/local/local_sandbox.py b/backend/packages/harness/deerflow/sandbox/local/local_sandbox.py index ba60070dd3..07041f0a05 100644 --- a/backend/packages/harness/deerflow/sandbox/local/local_sandbox.py +++ b/backend/packages/harness/deerflow/sandbox/local/local_sandbox.py @@ -770,7 +770,32 @@ class LocalSandbox(Sandbox): def list_dir(self, path: str, max_depth=2) -> list[str]: resolved_path = self._resolve_path(path) - entries = list_dir(resolved_path, max_depth) + container_path = path.rstrip("/") + virtual_children: list[PathMapping] = [] + for mapping in self.path_mappings: + if not mapping.container_path.startswith(container_path + "/"): + continue + child_rel = mapping.container_path[len(container_path) + 1 :] + if "/" in child_rel: + continue + try: + if os.path.isdir(self._resolved_local_paths[mapping]): + virtual_children.append(mapping) + except OSError: + pass + + try: + entries = list_dir(resolved_path, max_depth) + except FileNotFoundError: + # The requested path may exist only in the container, as the + # parent of mounted sub-directories (e.g. /mnt/skills with only + # per-category mounts and no aggregate root mapping). Continue + # with virtual children only when the resolved host path is + # missing. An existing file is not a directory and must still + # raise, as must a path without direct virtual children. + if not virtual_children or os.path.exists(resolved_path): + raise + entries = [] # Reverse resolve local paths back to container paths and preserve # list_dir's trailing "/" marker for directories. result: list[str] = [] @@ -785,28 +810,16 @@ class LocalSandbox(Sandbox): # the ``list_dir`` utility skips them for security. We patch those # missing virtual children back in so the agent can discover them via # ``ls /mnt/skills``. - container_path = path.rstrip("/") existing_dirs = {e.rstrip("/") for e in result if e.endswith("/")} - for mapping in self.path_mappings: - # A mapping is a virtual child if: - # 1. Its container_path is a direct child of the requested path - # 2. It is NOT already present in the result (was skipped by list_dir) - if mapping.container_path.startswith(container_path + "/"): - child_rel = mapping.container_path[len(container_path) + 1 :] - # Only direct children (no further slashes), e.g. "public", "custom". - # Compare the mapping's full container path -- not the bare child - # name -- against existing_dirs, which holds full paths (e.g. - # "/mnt/user-data/workspace"). Comparing the bare name here would - # never match, so an already-listed mount (the common case: real - # nested workspace/uploads/outputs subdirectories under - # /mnt/user-data) would be appended a second time. - if "/" not in child_rel and mapping.container_path.rstrip("/") not in existing_dirs: - # Verify the host path exists so we don't add phantom entries - try: - if os.path.isdir(os.path.realpath(mapping.local_path)): - result.append(f"{mapping.container_path}/") - except OSError: - pass + for mapping in virtual_children: + # Compare the mapping's full container path -- not the bare child + # name -- against existing_dirs, which holds full paths (e.g. + # "/mnt/user-data/workspace"). Comparing the bare name here would + # never match, so an already-listed mount (the common case: real + # nested workspace/uploads/outputs subdirectories under + # /mnt/user-data) would be appended a second time. + if mapping.container_path.rstrip("/") not in existing_dirs: + result.append(f"{mapping.container_path}/") return sorted(result) diff --git a/backend/tests/test_jev_screening_pipeline.py b/backend/tests/test_jev_screening_pipeline.py index 8118e573b6..6c9cea92f5 100644 --- a/backend/tests/test_jev_screening_pipeline.py +++ b/backend/tests/test_jev_screening_pipeline.py @@ -11,6 +11,7 @@ from __future__ import annotations import asyncio import json +import re from pathlib import Path import httpx @@ -31,6 +32,7 @@ from deerflow.agents.middlewares.tool_result_meta import TOOL_META_KEY from deerflow.agents.thread_state import ThreadState from deerflow.config.app_config import AppConfig from deerflow.config.paths import Paths +from deerflow.config.pii_redaction_config import PiiRedactionConfig from deerflow.config.sandbox_config import SandboxConfig from deerflow.extensions.loader import ExtensionSpec, load_extensions @@ -88,7 +90,7 @@ def _graph(scope, content, *, as_command=False, pii=False, enabled=True, message app_config.title.enabled = False app_config.memory.enabled = False app_config.summarization.enabled = False - app_config.pii_redaction.enabled = pii + app_config.pii_redaction = PiiRedactionConfig(enabled=pii, token_secret="offline-screening-token-secret" if pii else None) # Deterministic no-disk fallback exercises the actual 30,000-char boundary. app_config.tool_output.externalize_min_chars = 0 extensions, diagnostics = load_extensions([ExtensionSpec(use="deerflow_extension_jev_screening:install", config={"enabled": enabled, **screening_options})]) @@ -196,12 +198,14 @@ async def test_screening_sees_host_pii_redaction_and_sanitization(offline, scope # TOOL_VISIBLE is outer of the host's tool-result redaction and # sanitization, so the excerpt that leaves the host is the redacted text. excerpt = json.loads(requests[0].content)["state"]["content"] - assert "private@example.com" not in excerpt and "[EMAIL_1]" in excerpt + assert "private@example.com" not in excerpt + token_match = re.search(r"\[EMAIL_[a-z]{27}\]", excerpt) + assert token_match is not None assert "" not in excerpt visible = _model_tool_message(model) assert WARNING in visible.content assert "private@example.com" not in visible.content - assert "[EMAIL_1]" in visible.content + assert token_match.group() in visible.content assert "" not in visible.content assert "<system-reminder>" in visible.content assert original.content == content and calls == ["fetch"] diff --git a/backend/tests/test_local_sandbox_provider_mounts.py b/backend/tests/test_local_sandbox_provider_mounts.py index 9d832ff215..f6b570157c 100644 --- a/backend/tests/test_local_sandbox_provider_mounts.py +++ b/backend/tests/test_local_sandbox_provider_mounts.py @@ -343,6 +343,54 @@ class TestSymlinkEscapes: with pytest.raises(FileNotFoundError): sandbox.list_dir("/mnt/data/missing") + def test_list_dir_raises_when_only_nested_virtual_mount_exists(self, tmp_path): + nested_mount = tmp_path / "nested" + nested_mount.mkdir() + sandbox = LocalSandbox( + "test", + [ + PathMapping( + container_path="/mnt/virtual/deep/child", + local_path=str(nested_mount), + read_only=True, + ), + ], + ) + + with pytest.raises(FileNotFoundError): + sandbox.list_dir("/mnt/virtual") + + def test_list_dir_raises_when_direct_virtual_mount_is_missing(self, tmp_path): + sandbox = LocalSandbox( + "test", + [ + PathMapping( + container_path="/mnt/virtual/child", + local_path=str(tmp_path / "missing"), + read_only=True, + ), + ], + ) + + with pytest.raises(FileNotFoundError): + sandbox.list_dir("/mnt/virtual") + + def test_list_dir_raises_when_parent_mount_is_a_file(self, tmp_path): + parent_file = tmp_path / "parent-file" + parent_file.write_text("not a directory", encoding="utf-8") + child_mount = tmp_path / "child" + child_mount.mkdir() + sandbox = LocalSandbox( + "test", + [ + PathMapping(container_path="/mnt/virtual", local_path=str(parent_file), read_only=True), + PathMapping(container_path="/mnt/virtual/child", local_path=str(child_mount), read_only=True), + ], + ) + + with pytest.raises(FileNotFoundError): + sandbox.list_dir("/mnt/virtual") + def test_list_dir_empty_directory_returns_empty(self, tmp_path): mount_dir = tmp_path / "mount" mount_dir.mkdir() diff --git a/backend/tests/test_local_sandbox_virtual_path_contract.py b/backend/tests/test_local_sandbox_virtual_path_contract.py index 6ffa6078c0..603d3813b2 100644 --- a/backend/tests/test_local_sandbox_virtual_path_contract.py +++ b/backend/tests/test_local_sandbox_virtual_path_contract.py @@ -168,6 +168,33 @@ def test_list_dir_on_user_data_root_does_not_duplicate_subdir_mounts(provider): assert len(matches) == 1, f"{subdir} listed {len(matches)} time(s), expected exactly 1: {entries}" +def test_list_dir_on_skills_root_lists_category_mounts(provider): + """Regression: with the default (non-policy-scoped) mount layout there is + no mapping for the aggregate ``/mnt/skills`` root itself — only the four + category mounts (public/custom/legacy/integrations), each pointing at its + own host directory. The root must still be listable: the virtual + sub-directory overlay in ``list_dir`` exists exactly so the agent can + discover the categories via ``ls /mnt/skills``, but the host ``list_dir`` + on the unmapped root raised ``FileNotFoundError`` before the overlay ran. + """ + sandbox_id = provider.acquire("alpha") + sbx = provider.get(sandbox_id) + + # Pin the scenario: the default category layout, no aggregate root mapping. + mounted = {m.container_path for m in sbx.path_mappings} + assert "/mnt/skills" not in mounted + assert mounted >= {"/mnt/skills/public", "/mnt/skills/custom", "/mnt/skills/legacy", "/mnt/skills/integrations"} + + entries = sbx.list_dir("/mnt/skills") + + assert entries == [ + "/mnt/skills/custom/", + "/mnt/skills/integrations/", + "/mnt/skills/legacy/", + "/mnt/skills/public/", + ] + + def test_update_file_with_virtual_path_for_remote_sync_scenario(provider): """This is the exact code path used by ``uploads.py:282`` and ``feishu.py:389``.