mirror of
https://github.com/bytedance/deer-flow.git
synced 2026-10-26 13:16:24 +00:00
fix(sandbox): normalize mount container_path trailing slashes at config load (#7393)
* fix(sandbox): normalize mount container_path trailing slashes at config load A container_path declared with a trailing slash (e.g. /mnt/reports/) was accepted by VolumeMountConfig and normalized by LocalSandboxProvider when building path mappings, but the sandbox tool path gates compare the raw configured value, so every ls/read/write/glob/grep/bash access to the mount failed with a confusing PermissionError. Normalize trailing slashes in the config model so both layers agree, and reject the container root '/' which the gates cannot express. * fix(sandbox): normalize skills.container_path and pin empty-value rejection Review follow-up on #7393: - Share the container-path normalizer with SkillsConfig, whose prefix feeds the same as-is comparison in _is_skills_path and had the same trailing-slash dead-path failure mode. - Pin the empty-string rejection: on main an empty container_path made the gates match every absolute path, applying one mount's read_only flag to all paths. - Cover both config fields in CONFIGURATION.md. --------- Co-authored-by: Zhangchenhao-111 <285911435+Zhangchenhao-111@users.noreply.github.com>
This commit is contained in:
parent
811b99c491
commit
f7f9a7971b
@ -1620,6 +1620,8 @@ sandbox:
|
||||
|
||||
When you configure `sandbox.mounts`, DeerFlow exposes those `container_path` values in the agent prompt so the agent can discover and operate on mounted directories directly instead of assuming everything must live under `/mnt/user-data`. See [Skills and sandbox paths](#skills-and-sandbox-paths) for Windows, WSL, and Docker path examples.
|
||||
|
||||
Trailing slashes in `container_path` values are normalized when the config is loaded (`/mnt/reports/` behaves like `/mnt/reports`) — this applies to both `sandbox.mounts[].container_path` and `skills.container_path` — and the container root `/` or an empty value is rejected; mount a specific directory instead.
|
||||
|
||||
#### Sandbox network policy
|
||||
|
||||
Local Docker AIO sandboxes can opt into an outbound policy:
|
||||
|
||||
@ -17,6 +17,20 @@ SandboxNetworkApproval = Literal["deny", "prompt"]
|
||||
_REDIS_MAX_SAFE_TTL_MILLISECONDS = (2**63 - 1) // 2
|
||||
|
||||
|
||||
def normalize_container_path(value: str) -> str:
|
||||
"""Strip trailing slashes from a container mount/virtual path.
|
||||
|
||||
LocalSandboxProvider normalizes when building path mappings, but the tool
|
||||
path gates compare the configured value as-is; one spelling at config load
|
||||
keeps both layers in agreement. An empty result (the container root or a
|
||||
blank value) cannot be expressed in the gates and is rejected instead.
|
||||
"""
|
||||
normalized = value.rstrip("/")
|
||||
if not normalized:
|
||||
raise ValueError("container_path must be a non-empty mount point below the container root")
|
||||
return normalized
|
||||
|
||||
|
||||
class SandboxNetworkConfig(BaseModel):
|
||||
"""Outbound network policy for locally managed AIO sandboxes."""
|
||||
|
||||
@ -165,6 +179,13 @@ class VolumeMountConfig(BaseModel):
|
||||
container_path: str = Field(..., description="Path inside the container")
|
||||
read_only: bool = Field(default=False, description="Whether the mount is read-only")
|
||||
|
||||
@field_validator("container_path")
|
||||
@classmethod
|
||||
def _normalize_container_path(cls, value: str) -> str:
|
||||
# Keep a natural "/mnt/reports/" spelling from producing a mount the
|
||||
# tool gates reject with a confusing PermissionError.
|
||||
return normalize_container_path(value)
|
||||
|
||||
|
||||
class SandboxConfig(BaseModel):
|
||||
"""Config section for a sandbox.
|
||||
|
||||
@ -1,10 +1,11 @@
|
||||
import os
|
||||
from pathlib import Path
|
||||
|
||||
from pydantic import BaseModel, Field
|
||||
from pydantic import BaseModel, Field, field_validator
|
||||
|
||||
from deerflow.config.reload_boundary import format_field_description
|
||||
from deerflow.config.runtime_paths import project_root, resolve_path
|
||||
from deerflow.config.sandbox_config import normalize_container_path
|
||||
from deerflow.constants import DEFAULT_SKILLS_CONTAINER_PATH
|
||||
|
||||
|
||||
@ -33,6 +34,15 @@ class SkillsConfig(BaseModel):
|
||||
field_doc="Path where skills are mounted in the sandbox container.",
|
||||
),
|
||||
)
|
||||
|
||||
@field_validator("container_path")
|
||||
@classmethod
|
||||
def _normalize_container_path(cls, value: str) -> str:
|
||||
# _is_skills_path() compares the configured prefix as-is, so a
|
||||
# trailing slash would make every gated skills access fail with a
|
||||
# confusing PermissionError; share the mounts' normalization.
|
||||
return normalize_container_path(value)
|
||||
|
||||
deferred_discovery: bool = Field(
|
||||
default=False,
|
||||
description=("When enabled, skill metadata is not injected into the system prompt. Instead, only skill names appear in <skill_index> and the LLM discovers details on demand via the describe_skill tool."),
|
||||
|
||||
@ -470,6 +470,85 @@ def test_validate_local_tool_path_prioritizes_user_data_before_custom_mounts() -
|
||||
validate_local_tool_path(f"{VIRTUAL_PATH_PREFIX}/workspace/../../etc/passwd", _THREAD_DATA, read_only=True)
|
||||
|
||||
|
||||
def test_volume_mount_config_normalizes_trailing_slashes() -> None:
|
||||
"""The provider strips trailing slashes when building mappings; the config must too.
|
||||
|
||||
Otherwise a natural ``container_path: /mnt/reports/`` spelling works at the
|
||||
sandbox layer while every tool's path gate compares against the raw value
|
||||
and rejects the mount with a confusing PermissionError.
|
||||
"""
|
||||
from deerflow.config.sandbox_config import VolumeMountConfig
|
||||
|
||||
assert VolumeMountConfig(host_path="/tmp/host", container_path="/mnt/reports/").container_path == "/mnt/reports"
|
||||
assert VolumeMountConfig(host_path="/tmp/host", container_path="/mnt/reports//").container_path == "/mnt/reports"
|
||||
assert VolumeMountConfig(host_path="/tmp/host", container_path="/mnt/reports").container_path == "/mnt/reports"
|
||||
|
||||
|
||||
@pytest.mark.parametrize("raw", ["/", "//", "///", ""])
|
||||
def test_volume_mount_config_rejects_the_container_root(raw: str) -> None:
|
||||
"""The container root and a blank value cannot be expressed in the path gates, so reject them at load.
|
||||
|
||||
On ``main`` an empty ``container_path`` made the gates match every absolute
|
||||
path (``path == "" or path.startswith("/")``), letting one mount's
|
||||
``read_only`` flag silently apply to all paths.
|
||||
"""
|
||||
from pydantic import ValidationError
|
||||
|
||||
from deerflow.config.sandbox_config import VolumeMountConfig
|
||||
|
||||
with pytest.raises(ValidationError, match="container_path"):
|
||||
VolumeMountConfig(host_path="/tmp/host", container_path=raw)
|
||||
|
||||
|
||||
def test_skills_config_normalizes_trailing_slashes() -> None:
|
||||
"""skills.container_path feeds the same as-is prefix comparison in _is_skills_path."""
|
||||
from deerflow.config.skills_config import SkillsConfig
|
||||
|
||||
assert SkillsConfig(container_path="/mnt/skills/").container_path == "/mnt/skills"
|
||||
assert SkillsConfig(container_path="/mnt/skills//").container_path == "/mnt/skills"
|
||||
assert SkillsConfig().container_path == "/mnt/skills"
|
||||
|
||||
|
||||
@pytest.mark.parametrize("raw", ["/", "//", ""])
|
||||
def test_skills_config_rejects_the_container_root(raw: str) -> None:
|
||||
from pydantic import ValidationError
|
||||
|
||||
from deerflow.config.skills_config import SkillsConfig
|
||||
|
||||
with pytest.raises(ValidationError, match="container_path"):
|
||||
SkillsConfig(container_path=raw)
|
||||
|
||||
|
||||
def test_is_skills_path_accepts_prefix_declared_with_trailing_slash() -> None:
|
||||
"""A skills prefix configured as ``/mnt/skills/`` must keep gating skills paths."""
|
||||
from deerflow.config.skills_config import SkillsConfig
|
||||
|
||||
prefix = SkillsConfig(container_path="/mnt/skills/").container_path
|
||||
with patch("deerflow.sandbox.tools._get_skills_container_path", return_value=prefix):
|
||||
assert _is_skills_path("/mnt/skills/reporter/SKILL.md")
|
||||
assert not _is_skills_path("/mnt/skillsmalformed/SKILL.md")
|
||||
|
||||
|
||||
def test_validate_local_tool_path_accepts_mount_declared_with_trailing_slash() -> None:
|
||||
"""A mount configured as ``/mnt/reports/`` must keep working for reads and writes."""
|
||||
from deerflow.config.sandbox_config import VolumeMountConfig
|
||||
|
||||
mounts = [VolumeMountConfig(host_path="/tmp/host-reports", container_path="/mnt/reports/", read_only=False)]
|
||||
with patch("deerflow.sandbox.tools._get_custom_mounts", return_value=mounts):
|
||||
validate_local_tool_path("/mnt/reports/notes.txt", _THREAD_DATA, read_only=True)
|
||||
validate_local_tool_path("/mnt/reports/notes.txt", _THREAD_DATA, read_only=False)
|
||||
|
||||
|
||||
def test_validate_local_tool_path_honors_read_only_mount_declared_with_trailing_slash() -> None:
|
||||
from deerflow.config.sandbox_config import VolumeMountConfig
|
||||
|
||||
mounts = [VolumeMountConfig(host_path="/tmp/host-reports", container_path="/mnt/reports/", read_only=True)]
|
||||
with patch("deerflow.sandbox.tools._get_custom_mounts", return_value=mounts):
|
||||
validate_local_tool_path("/mnt/reports/notes.txt", _THREAD_DATA, read_only=True)
|
||||
with pytest.raises(PermissionError, match="read-only mount"):
|
||||
validate_local_tool_path("/mnt/reports/notes.txt", _THREAD_DATA, read_only=False)
|
||||
|
||||
|
||||
def test_validate_local_tool_path_rejects_bare_virtual_root() -> None:
|
||||
"""The bare /mnt/user-data root without trailing slash is not a valid sub-path."""
|
||||
with pytest.raises(PermissionError, match="Only paths under"):
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user