diff --git a/backend/docs/CONFIGURATION.md b/backend/docs/CONFIGURATION.md index 3d8e55b92a..1c0cddd89b 100644 --- a/backend/docs/CONFIGURATION.md +++ b/backend/docs/CONFIGURATION.md @@ -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: diff --git a/backend/packages/harness/deerflow/config/sandbox_config.py b/backend/packages/harness/deerflow/config/sandbox_config.py index df6cb0ff0a..8ba66767b6 100644 --- a/backend/packages/harness/deerflow/config/sandbox_config.py +++ b/backend/packages/harness/deerflow/config/sandbox_config.py @@ -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. diff --git a/backend/packages/harness/deerflow/config/skills_config.py b/backend/packages/harness/deerflow/config/skills_config.py index 38cbcbf0d2..416f852924 100644 --- a/backend/packages/harness/deerflow/config/skills_config.py +++ b/backend/packages/harness/deerflow/config/skills_config.py @@ -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 and the LLM discovers details on demand via the describe_skill tool."), diff --git a/backend/tests/test_sandbox_tools_security.py b/backend/tests/test_sandbox_tools_security.py index 3cebcc1c43..f14d1e41e5 100644 --- a/backend/tests/test_sandbox_tools_security.py +++ b/backend/tests/test_sandbox_tools_security.py @@ -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"):