feat(sandbox): add read-only support for local sandbox path mappings (#1808)

2026-05-21 23:46:50 +00:00 · 2026-04-03 19:46:22 +08:00
parent c6cdf200ce
commit 1694c616ef
6 changed files with 768 additions and 33 deletions
@@ -1,7 +1,9 @@
+import errno
 import ntpath
 import os
 import shutil
 import subprocess
+from dataclasses import dataclass
 from pathlib import Path

 from deerflow.sandbox.local.list_dir import list_dir
@@ -9,6 +11,15 @@ from deerflow.sandbox.sandbox import Sandbox
 from deerflow.sandbox.search import GrepMatch, find_glob_matches, find_grep_matches


+@dataclass(frozen=True)
+class PathMapping:
+    """A path mapping from a container path to a local path with optional read-only flag."""
+
+    container_path: str
+    local_path: str
+    read_only: bool = False
+
+
 class LocalSandbox(Sandbox):
    @staticmethod
    def _shell_name(shell: str) -> str:
@@ -40,17 +51,42 @@ class LocalSandbox(Sandbox):

        return None

-    def __init__(self, id: str, path_mappings: dict[str, str] | None = None):
+    def __init__(self, id: str, path_mappings: list[PathMapping] | None = None):
        """
        Initialize local sandbox with optional path mappings.

        Args:
            id: Sandbox identifier
-            path_mappings: Dictionary mapping container paths to local paths
-                          Example: {"/mnt/skills": "/absolute/path/to/skills"}
+            path_mappings: List of path mappings with optional read-only flag.
+                          Skills directory is read-only by default.
        """
        super().__init__(id)
-        self.path_mappings = path_mappings or {}
+        self.path_mappings = path_mappings or []
+
+    def _is_read_only_path(self, resolved_path: str) -> bool:
+        """Check if a resolved path is under a read-only mount.
+
+        When multiple mappings match (nested mounts), prefer the most specific
+        mapping (i.e. the one whose local_path is the longest prefix of the
+        resolved path), similar to how ``_resolve_path`` handles container paths.
+        """
+        resolved = str(Path(resolved_path).resolve())
+
+        best_mapping: PathMapping | None = None
+        best_prefix_len = -1
+
+        for mapping in self.path_mappings:
+            local_resolved = str(Path(mapping.local_path).resolve())
+            if resolved == local_resolved or resolved.startswith(local_resolved + os.sep):
+                prefix_len = len(local_resolved)
+                if prefix_len > best_prefix_len:
+                    best_prefix_len = prefix_len
+                    best_mapping = mapping
+
+        if best_mapping is None:
+            return False
+
+        return best_mapping.read_only

    def _resolve_path(self, path: str) -> str:
        """
@@ -65,7 +101,9 @@ class LocalSandbox(Sandbox):
        path_str = str(path)

        # Try each mapping (longest prefix first for more specific matches)
-        for container_path, local_path in sorted(self.path_mappings.items(), key=lambda x: len(x[0]), reverse=True):
+        for mapping in sorted(self.path_mappings, key=lambda m: len(m.container_path), reverse=True):
+            container_path = mapping.container_path
+            local_path = mapping.local_path
            if path_str == container_path or path_str.startswith(container_path + "/"):
                # Replace the container path prefix with local path
                relative = path_str[len(container_path) :].lstrip("/")
@@ -85,15 +123,16 @@ class LocalSandbox(Sandbox):
        Returns:
            Container path if mapping exists, otherwise original path
        """
-        path_str = str(Path(path).resolve())
+        normalized_path = path.replace("\\", "/")
+        path_str = str(Path(normalized_path).resolve())

        # Try each mapping (longest local path first for more specific matches)
-        for container_path, local_path in sorted(self.path_mappings.items(), key=lambda x: len(x[1]), reverse=True):
-            local_path_resolved = str(Path(local_path).resolve())
-            if path_str.startswith(local_path_resolved):
+        for mapping in sorted(self.path_mappings, key=lambda m: len(m.local_path), reverse=True):
+            local_path_resolved = str(Path(mapping.local_path).resolve())
+            if path_str == local_path_resolved or path_str.startswith(local_path_resolved + "/"):
                # Replace the local path prefix with container path
                relative = path_str[len(local_path_resolved) :].lstrip("/")
-                resolved = f"{container_path}/{relative}" if relative else container_path
+                resolved = f"{mapping.container_path}/{relative}" if relative else mapping.container_path
                return resolved

        # No mapping found, return original path
@@ -112,7 +151,7 @@ class LocalSandbox(Sandbox):
        import re

        # Sort mappings by local path length (longest first) for correct prefix matching
-        sorted_mappings = sorted(self.path_mappings.items(), key=lambda x: len(x[1]), reverse=True)
+        sorted_mappings = sorted(self.path_mappings, key=lambda m: len(m.local_path), reverse=True)

        if not sorted_mappings:
            return output
@@ -120,12 +159,11 @@ class LocalSandbox(Sandbox):
        # Create pattern that matches absolute paths
        # Match paths like /Users/... or other absolute paths
        result = output
-        for container_path, local_path in sorted_mappings:
-            local_path_resolved = str(Path(local_path).resolve())
+        for mapping in sorted_mappings:
            # Escape the local path for use in regex
-            escaped_local = re.escape(local_path_resolved)
-            # Match the local path followed by optional path components
-            pattern = re.compile(escaped_local + r"(?:/[^\s\"';&|<>()]*)?")
+            escaped_local = re.escape(str(Path(mapping.local_path).resolve()))
+            # Match the local path followed by optional path components with either separator
+            pattern = re.compile(escaped_local + r"(?:[/\\][^\s\"';&|<>()]*)?")

            def replace_match(match: re.Match) -> str:
                matched_path = match.group(0)
@@ -148,7 +186,7 @@ class LocalSandbox(Sandbox):
        import re

        # Sort mappings by length (longest first) for correct prefix matching
-        sorted_mappings = sorted(self.path_mappings.items(), key=lambda x: len(x[0]), reverse=True)
+        sorted_mappings = sorted(self.path_mappings, key=lambda m: len(m.container_path), reverse=True)

        # Build regex pattern to match all container paths
        # Match container path followed by optional path components
@@ -158,7 +196,7 @@ class LocalSandbox(Sandbox):
        # Create pattern that matches any of the container paths.
        # The lookahead (?=/|$|...) ensures we only match at a path-segment boundary,
        # preventing /mnt/skills from matching inside /mnt/skills-extra.
-        patterns = [re.escape(container_path) + r"(?=/|$|[\s\"';&|<>()])(?:/[^\s\"';&|<>()]*)?" for container_path, _ in sorted_mappings]
+        patterns = [re.escape(m.container_path) + r"(?=/|$|[\s\"';&|<>()])(?:/[^\s\"';&|<>()]*)?" for m in sorted_mappings]
        pattern = re.compile("|".join(f"({p})" for p in patterns))

        def replace_match(match: re.Match) -> str:
@@ -249,6 +287,8 @@ class LocalSandbox(Sandbox):

    def write_file(self, path: str, content: str, append: bool = False) -> None:
        resolved_path = self._resolve_path(path)
+        if self._is_read_only_path(resolved_path):
+            raise OSError(errno.EROFS, "Read-only file system", path)
        try:
            dir_path = os.path.dirname(resolved_path)
            if dir_path:
@@ -295,6 +335,8 @@ class LocalSandbox(Sandbox):

    def update_file(self, path: str, content: bytes) -> None:
        resolved_path = self._resolve_path(path)
+        if self._is_read_only_path(resolved_path):
+            raise OSError(errno.EROFS, "Read-only file system", path)
        try:
            dir_path = os.path.dirname(resolved_path)
            if dir_path:
@@ -1,6 +1,7 @@
 import logging
+from pathlib import Path

-from deerflow.sandbox.local.local_sandbox import LocalSandbox
+from deerflow.sandbox.local.local_sandbox import LocalSandbox, PathMapping
 from deerflow.sandbox.sandbox import Sandbox
 from deerflow.sandbox.sandbox_provider import SandboxProvider

@@ -14,16 +15,17 @@ class LocalSandboxProvider(SandboxProvider):
        """Initialize the local sandbox provider with path mappings."""
        self._path_mappings = self._setup_path_mappings()

-    def _setup_path_mappings(self) -> dict[str, str]:
+    def _setup_path_mappings(self) -> list[PathMapping]:
        """
        Setup path mappings for local sandbox.

-        Maps container paths to actual local paths, including skills directory.
+        Maps container paths to actual local paths, including skills directory
+        and any custom mounts configured in config.yaml.

        Returns:
-            Dictionary of path mappings
+            List of path mappings
        """
-        mappings = {}
+        mappings: list[PathMapping] = []

        # Map skills container path to local skills directory
        try:
@@ -35,10 +37,63 @@ class LocalSandboxProvider(SandboxProvider):

            # Only add mapping if skills directory exists
            if skills_path.exists():
-                mappings[container_path] = str(skills_path)
+                mappings.append(
+                    PathMapping(
+                        container_path=container_path,
+                        local_path=str(skills_path),
+                        read_only=True,  # Skills directory is always read-only
+                    )
+                )
+
+            # Map custom mounts from sandbox config
+            _RESERVED_CONTAINER_PREFIXES = [container_path, "/mnt/acp-workspace", "/mnt/user-data"]
+            sandbox_config = config.sandbox
+            if sandbox_config and sandbox_config.mounts:
+                for mount in sandbox_config.mounts:
+                    host_path = Path(mount.host_path)
+                    container_path = mount.container_path.rstrip("/") or "/"
+
+                    if not host_path.is_absolute():
+                        logger.warning(
+                            "Mount host_path must be absolute, skipping: %s -> %s",
+                            mount.host_path,
+                            mount.container_path,
+                        )
+                        continue
+
+                    if not container_path.startswith("/"):
+                        logger.warning(
+                            "Mount container_path must be absolute, skipping: %s -> %s",
+                            mount.host_path,
+                            mount.container_path,
+                        )
+                        continue
+
+                    # Reject mounts that conflict with reserved container paths
+                    if any(container_path == p or container_path.startswith(p + "/") for p in _RESERVED_CONTAINER_PREFIXES):
+                        logger.warning(
+                            "Mount container_path conflicts with reserved prefix, skipping: %s",
+                            mount.container_path,
+                        )
+                        continue
+                    # Ensure the host path exists before adding mapping
+                    if host_path.exists():
+                        mappings.append(
+                            PathMapping(
+                                container_path=container_path,
+                                local_path=str(host_path.resolve()),
+                                read_only=mount.read_only,
+                            )
+                        )
+                    else:
+                        logger.warning(
+                            "Mount host_path does not exist, skipping: %s -> %s",
+                            mount.host_path,
+                            mount.container_path,
+                        )
        except Exception as e:
            # Log but don't fail if config loading fails
-            logger.warning("Could not setup skills path mapping: %s", e, exc_info=True)
+            logger.warning("Could not setup path mappings: %s", e, exc_info=True)

        return mappings

@@ -119,6 +119,54 @@ def _is_acp_workspace_path(path: str) -> bool:
    return path == _ACP_WORKSPACE_VIRTUAL_PATH or path.startswith(f"{_ACP_WORKSPACE_VIRTUAL_PATH}/")


+def _get_custom_mounts():
+    """Get custom volume mounts from sandbox config.
+
+    Result is cached after the first successful config load.  If config loading
+    fails an empty list is returned *without* caching so that a later call can
+    pick up the real value once the config is available.
+    """
+    cached = getattr(_get_custom_mounts, "_cached", None)
+    if cached is not None:
+        return cached
+    try:
+        from pathlib import Path
+
+        from deerflow.config import get_app_config
+
+        config = get_app_config()
+        mounts = []
+        if config.sandbox and config.sandbox.mounts:
+            # Only include mounts whose host_path exists, consistent with
+            # LocalSandboxProvider._setup_path_mappings() which also filters
+            # by host_path.exists().
+            mounts = [m for m in config.sandbox.mounts if Path(m.host_path).exists()]
+        _get_custom_mounts._cached = mounts  # type: ignore[attr-defined]
+        return mounts
+    except Exception:
+        # If config loading fails, return an empty list without caching so that
+        # a later call can retry once the config is available.
+        return []
+
+
+def _is_custom_mount_path(path: str) -> bool:
+    """Check if path is under a custom mount container_path."""
+    for mount in _get_custom_mounts():
+        if path == mount.container_path or path.startswith(f"{mount.container_path}/"):
+            return True
+    return False
+
+
+def _get_custom_mount_for_path(path: str):
+    """Get the mount config matching this path (longest prefix first)."""
+    best = None
+    for mount in _get_custom_mounts():
+        if path == mount.container_path or path.startswith(f"{mount.container_path}/"):
+            if best is None or len(mount.container_path) > len(best.container_path):
+                best = mount
+    return best
+
+
 def _extract_thread_id_from_thread_data(thread_data: "ThreadDataState | None") -> str | None:
    """Extract thread_id from thread_data by inspecting workspace_path.

@@ -448,6 +496,8 @@ def mask_local_paths_in_output(output: str, thread_data: ThreadDataState | None)

            result = pattern.sub(replace_acp, result)

+    # Custom mount host paths are masked by LocalSandbox._reverse_resolve_paths_in_output()
+
    # Mask user-data host paths
    if thread_data is None:
        return result
@@ -496,6 +546,7 @@ def validate_local_tool_path(path: str, thread_data: ThreadDataState | None, *,
      - ``/mnt/user-data/*``  — always allowed (read + write)
      - ``/mnt/skills/*``     — allowed only when *read_only* is True
      - ``/mnt/acp-workspace/*`` — allowed only when *read_only* is True
+      - Custom mount paths (from config.yaml) — respects per-mount ``read_only`` flag

    Args:
        path: The virtual path to validate.
@@ -527,7 +578,14 @@ def validate_local_tool_path(path: str, thread_data: ThreadDataState | None, *,
    if path.startswith(f"{VIRTUAL_PATH_PREFIX}/"):
        return

-    raise PermissionError(f"Only paths under {VIRTUAL_PATH_PREFIX}/, {_get_skills_container_path()}/, or {_ACP_WORKSPACE_VIRTUAL_PATH}/ are allowed")
+    # Custom mount paths — respect read_only config
+    if _is_custom_mount_path(path):
+        mount = _get_custom_mount_for_path(path)
+        if mount and mount.read_only and not read_only:
+            raise PermissionError(f"Write access to read-only mount is not allowed: {path}")
+        return
+
+    raise PermissionError(f"Only paths under {VIRTUAL_PATH_PREFIX}/, {_get_skills_container_path()}/, {_ACP_WORKSPACE_VIRTUAL_PATH}/, or configured mount paths are allowed")


 def _validate_resolved_user_data_path(resolved: Path, thread_data: ThreadDataState) -> None:
@@ -577,9 +635,10 @@ def validate_local_bash_command_paths(command: str, thread_data: ThreadDataState
    boundary and must not be treated as isolation from the host filesystem.

    In local mode, commands must use virtual paths under /mnt/user-data for
-    user data access. Skills paths under /mnt/skills and ACP workspace paths
-    under /mnt/acp-workspace are allowed (path-traversal checks only; write
-    prevention for bash commands is not enforced here).
+    user data access. Skills paths under /mnt/skills, ACP workspace paths
+    under /mnt/acp-workspace, and custom mount container paths (configured in
+    config.yaml) are allowed (path-traversal checks only; write prevention
+    for bash commands is not enforced here).
    A small allowlist of common system path prefixes is kept for executable
    and device references (e.g. /bin/sh, /dev/null).
    """
@@ -614,6 +673,11 @@ def validate_local_bash_command_paths(command: str, thread_data: ThreadDataState
            _reject_path_traversal(absolute_path)
            continue

+        # Allow custom mount container paths
+        if _is_custom_mount_path(absolute_path):
+            _reject_path_traversal(absolute_path)
+            continue
+
        if any(absolute_path == prefix.rstrip("/") or absolute_path.startswith(prefix) for prefix in _LOCAL_BASH_SYSTEM_PATH_PREFIXES):
            continue

@@ -658,6 +722,8 @@ def replace_virtual_paths_in_command(command: str, thread_data: ThreadDataState

        result = acp_pattern.sub(replace_acp_match, result)

+    # Custom mount paths are resolved by LocalSandbox._resolve_paths_in_command()
+
    # Replace user-data paths
    if VIRTUAL_PATH_PREFIX in result and thread_data is not None:
        pattern = re.compile(rf"{re.escape(VIRTUAL_PATH_PREFIX)}(/[^\s\"';&|<>()]*)?")
@@ -954,8 +1020,9 @@ def ls_tool(runtime: ToolRuntime[ContextT, ThreadState], description: str, path:
                path = _resolve_skills_path(path)
            elif _is_acp_workspace_path(path):
                path = _resolve_acp_workspace_path(path, _extract_thread_id_from_thread_data(thread_data))
-            else:
+            elif not _is_custom_mount_path(path):
                path = _resolve_and_validate_user_data_path(path, thread_data)
+            # Custom mount paths are resolved by LocalSandbox._resolve_path()
        children = sandbox.list_dir(path)
        if not children:
            return "(empty)"
@@ -1117,8 +1184,9 @@ def read_file_tool(
                path = _resolve_skills_path(path)
            elif _is_acp_workspace_path(path):
                path = _resolve_acp_workspace_path(path, _extract_thread_id_from_thread_data(thread_data))
-            else:
+            elif not _is_custom_mount_path(path):
                path = _resolve_and_validate_user_data_path(path, thread_data)
+            # Custom mount paths are resolved by LocalSandbox._resolve_path()
        content = sandbox.read_file(path)
        if not content:
            return "(empty)"
@@ -1166,7 +1234,9 @@ def write_file_tool(
        if is_local_sandbox(runtime):
            thread_data = get_thread_data(runtime)
            validate_local_tool_path(path, thread_data)
-            path = _resolve_and_validate_user_data_path(path, thread_data)
+            if not _is_custom_mount_path(path):
+                path = _resolve_and_validate_user_data_path(path, thread_data)
+            # Custom mount paths are resolved by LocalSandbox._resolve_path()
        with get_file_operation_lock(sandbox, path):
            sandbox.write_file(path, content, append)
        return "OK"
@@ -1208,7 +1278,9 @@ def str_replace_tool(
        if is_local_sandbox(runtime):
            thread_data = get_thread_data(runtime)
            validate_local_tool_path(path, thread_data)
-            path = _resolve_and_validate_user_data_path(path, thread_data)
+            if not _is_custom_mount_path(path):
+                path = _resolve_and_validate_user_data_path(path, thread_data)
+            # Custom mount paths are resolved by LocalSandbox._resolve_path()
        with get_file_operation_lock(sandbox, path):
            content = sandbox.read_file(path)
            if not content: