mirror of
https://github.com/bytedance/deer-flow.git
synced 2026-10-26 13:16:24 +00:00
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 <willem.jiang@gmail.com>
This commit is contained in:
parent
3d921afa5f
commit
f92118e1e9
@ -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)
|
||||
|
||||
|
||||
@ -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 "<system-reminder>" 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 "<system-reminder>" not in visible.content
|
||||
assert "<system-reminder>" in visible.content
|
||||
assert original.content == content and calls == ["fetch"]
|
||||
|
||||
@ -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()
|
||||
|
||||
@ -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``.
|
||||
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user