fix(sandbox): resolve every entry of colon-joined path lists in forward path translation (#7002)

* fix(sandbox): resolve every entry of colon-joined path lists in forward path translation

* fix(sandbox): keep the original text of colon-list entries when reassembling

backend/packages/harness/deerflow/sandbox/local/local_sandbox.py

@@ -157,6 +157,19 @@ def _trim_trailing_partial_utf8(data: bytes) -> bytes:
     return data
 
 
+def _match_ends_before_a_colon(match: re.Match) -> bool:
+    """Whether the subject continues with a ``:`` right after ``match``.
+
+    The forward path-tail patterns stop at ``:`` so substitution resumes at
+    every entry of a colon-joined list, but such a match can also be a
+    truncated component of one POSIX filename (``cache:old.txt``) — the
+    caller must not canonicalize the truncated prefix, because it can be an
+    unrelated symlink (``cache -> data``) and the reassembled path would name
+    a different file (``data:old.txt``).
+    """
+    return match.end() < len(match.string) and match.string[match.end()] == ":"
+
+
 @dataclass(frozen=True)
 class PathMapping:
     """A path mapping from a container path to a local path with optional read-only flag."""
@@ -338,7 +351,12 @@ class LocalSandbox(Sandbox):
             return None
         # The lookahead (?=/|$|...) ensures we only match at a path-segment boundary,
         # preventing /mnt/skills from matching inside /mnt/skills-extra.
-        patterns = [re.escape(m.container_path) + r"(?=/|$|[\s\"';&|<>()])(?:/[^\s\"';&|<>()]*)?" for m in mappings]
+        #
+        # The tail ends at ":" as well (mirroring path_patterns._PATH_TAIL):
+        # substitution resumes after a match, so a tail that ran through a
+        # ":"-joined list ($PATH, $PYTHONPATH) swallowed every later entry and
+        # left those container paths unresolved.
+        patterns = [re.escape(m.container_path) + r"(?=/|$|[\s\"';&|<>()])(?:/[^\s\"';&|<>():]*)?" for m in mappings]
         return re.compile("|".join(f"({p})" for p in patterns))
 
     @cached_property
@@ -347,7 +365,11 @@ class LocalSandbox(Sandbox):
         mappings = sorted(self.path_mappings, key=lambda m: len(m.container_path), reverse=True)
         if not mappings:
             return None
-        patterns = [re.escape(m.container_path) + r"(?=/|$|[^\w./-])(?:/[^\s\"';&|<>()]*)?" for m in mappings]
+        # The tail ends at ":" for the same reason as _command_pattern above:
+        # write_file content regularly stores PATH-style exports, and a tail
+        # that ran through the ":" left the later list entries virtual in the
+        # host file (read_file masked them back, so the corruption was hidden).
+        patterns = [re.escape(m.container_path) + r"(?=/|$|[^\w./-])(?:/[^\s\"';&|<>():]*)?" for m in mappings]
         return re.compile("|".join(f"({p})" for p in patterns))
 
     @cached_property
@@ -437,6 +459,27 @@ class LocalSandbox(Sandbox):
     def _resolve_path(self, path: str) -> str:
         return self._resolve_path_with_mapping(path).path
 
+    def _resolve_path_textually(self, path: str) -> str:
+        """Resolve ``path``'s mapped root while keeping its original tail spelling.
+
+        ``_resolve_path`` canonicalizes the joined path, which is right for a
+        complete path but wrong for one truncated right before a ``:`` that
+        belongs to a single POSIX filename (``cache:old.txt``): following an
+        unrelated symlink at the truncated prefix (``cache -> data``)
+        reassembles a path that names a different file (``data:old.txt``).
+        Substitute only the mapped root and keep the matched tail verbatim.
+        """
+        # The canonical resolution still runs so its containment check keeps
+        # rejecting ``..`` escapes exactly like ``_resolve_path``; only its
+        # spelling is discarded.
+        self._resolve_path(path)
+        mapping_match = self._find_path_mapping(path)
+        if mapping_match is None:
+            return path
+        mapping, relative = mapping_match
+        root = self._resolved_local_paths[mapping]
+        return f"{root}/{relative}" if relative else root
+
     def _is_resolved_path_read_only(self, resolved: ResolvedPath) -> bool:
         return bool(resolved.mapping and resolved.mapping.read_only) or self._is_read_only_path(resolved.path)
 
@@ -534,6 +577,13 @@ class LocalSandbox(Sandbox):
 
         def replace_match(match: re.Match) -> str:
             matched_path = match.group(0)
+            if _match_ends_before_a_colon(match):
+                # The match may be a truncated component of one POSIX
+                # filename (``cache:old.txt``), not a complete list entry:
+                # canonicalizing the truncated prefix would follow an
+                # unrelated symlink and reassemble a path to a different
+                # file, so substitute textually there.
+                return self._resolve_path_textually(matched_path).replace("\\", "/")
             # Normalize to forward slashes so bash doesn't interpret Windows
             # backslash sequences (\\U, \\a, \\d, \\s, \\n, \\t) as escapes.
             return self._resolve_path(matched_path).replace("\\", "/")
@@ -561,6 +611,12 @@ class LocalSandbox(Sandbox):
 
         def replace_match(match: re.Match) -> str:
             matched_path = match.group(0)
+            if _match_ends_before_a_colon(match):
+                # Same truncated-component case as ``_resolve_paths_in_command``:
+                # a ``:`` can sit inside one POSIX filename (``cache:old.txt``),
+                # and canonicalizing the truncated prefix follows an unrelated
+                # symlink, so the stored file would point at a different one.
+                return self._resolve_path_textually(matched_path).replace("\\", "/")
             resolved = self._resolve_path(matched_path)
             # Normalize to forward slashes so that Windows backslash paths
             # don't create invalid escape sequences in source files.

backend/packages/harness/deerflow/sandbox/tools.py

@@ -1541,7 +1541,12 @@ def replace_virtual_paths_in_command(command: str, thread_data: ThreadDataState
         # ``:`` (PATH-style concatenation) or ``,``, which the shell-oriented
         # class rejects — narrowing to it would stop translating paths that
         # translate today. ``$`` covers a command ending exactly at the root.
-        pattern = re.compile(rf"{re.escape(VIRTUAL_PATH_PREFIX)}(?=/|$|[^\w./-])(/[^\s\"';&|<>()]*)?")
+        #
+        # The tail ends at ``:`` (like ``path_patterns._PATH_TAIL``): the
+        # boundary above admits PATH-style concatenation, but a tail that ran
+        # through the ``:`` swallowed every later entry of the list, leaving
+        # those virtual paths unresolved in the command.
+        pattern = re.compile(rf"{re.escape(VIRTUAL_PATH_PREFIX)}(?=/|$|[^\w./-])(/[^\s\"';&|<>():]*)?")
 
         def replace_user_data_match(match: re.Match) -> str:
             return replace_virtual_path(match.group(0), thread_data).replace("\\", "/")

backend/tests/test_sandbox_colon_joined_path_lists.py

@@ -0,0 +1,151 @@
+"""Forward (container→host) resolution must translate every entry of a ``:``-joined list.
+
+The reverse (host→virtual) direction already ends its tail at ``:`` — see
+``path_patterns._PATH_TAIL`` — because a tail that ran through a ``:``-joined
+list ($PATH, $PYTHONPATH, LD_LIBRARY_PATH) swallowed every later entry under
+the same base into one match, and scanning only resumes *after* a match.
+
+The forward direction had the same defect in three copies of the tail:
+``LocalSandbox._content_pattern``, ``LocalSandbox._command_pattern`` and
+``replace_virtual_paths_in_command``. One match starting at the first virtual
+path consumed the ``:`` and the whole rest of the list, so ``pattern.sub``
+never rescanned the later entries and
+``write_file("/mnt/user-data/workspace/env.sh", 'export PATH="/mnt/user-data/workspace/bin:/mnt/user-data/outputs/bin"')``
+persisted ``export PATH="<host_ws>/bin:/mnt/user-data/outputs/bin"`` — a
+dangling virtual entry that anything later consuming the file inherits.
+``read_file`` masks the mixed line back to the original virtual text, so the
+corruption is invisible to the model.
+
+Ending the tail at ``:`` has a mirror-image obligation: a ``:`` inside one
+POSIX filename (``cache:old.txt``) is not a list separator, so the match ends
+at the truncated ``cache`` component and the callback must keep that prefix's
+original spelling. Canonicalizing the truncated prefix follows an unrelated
+symlink (``cache -> data``) and the reassembled path names a different file
+(``data:old.txt``).
+"""
+
+from __future__ import annotations
+
+from pathlib import Path
+
+from support.symlinks import symlink_or_skip
+
+from deerflow.sandbox.local.local_sandbox import LocalSandbox, PathMapping
+from deerflow.sandbox.tools import replace_virtual_paths_in_command
+
+_COLON_LIST_COMMAND = 'export PATH="/mnt/user-data/workspace/bin:/mnt/user-data/outputs/bin"'
+_COLON_FILENAME_COMMAND = "cat '/mnt/user-data/workspace/cache:old.txt'"
+
+
+def _sandbox_with_workspace_and_outputs(tmp_path: Path) -> LocalSandbox:
+    ws = tmp_path / "ws"
+    out = tmp_path / "out"
+    ws.mkdir()
+    out.mkdir()
+    return LocalSandbox(
+        id="local:colon-list",
+        path_mappings=[
+            PathMapping(container_path="/mnt/user-data/workspace", local_path=str(ws)),
+            PathMapping(container_path="/mnt/user-data/outputs", local_path=str(out)),
+        ],
+    )
+
+
+def test_write_file_resolves_every_entry_of_a_colon_joined_list(tmp_path: Path) -> None:
+    """``write_file`` must not persist a dangling virtual entry in a PATH-style list."""
+    sbx = _sandbox_with_workspace_and_outputs(tmp_path)
+
+    sbx.write_file("/mnt/user-data/workspace/env.sh", _COLON_LIST_COMMAND + "\n")
+
+    written = (tmp_path / "ws" / "env.sh").read_text(encoding="utf-8")
+    assert "/mnt/user-data" not in written, f"colon-joined list: write_file left a virtual entry unresolved; host file: {written!r}"
+
+
+def test_command_resolution_translates_every_entry_of_a_colon_joined_list(tmp_path: Path) -> None:
+    """``_resolve_paths_in_command`` must resume at each entry, not swallow the list."""
+    sbx = _sandbox_with_workspace_and_outputs(tmp_path)
+
+    resolved = sbx._resolve_paths_in_command(_COLON_LIST_COMMAND)
+
+    assert "/mnt/user-data" not in resolved, f"colon-joined list: _resolve_paths_in_command left a virtual entry unresolved: {resolved!r}"
+
+
+def test_user_data_command_resolution_translates_every_entry_of_a_colon_joined_list(tmp_path: Path) -> None:
+    """``replace_virtual_paths_in_command`` pre-resolves user-data paths the same way."""
+    ws_local = str(tmp_path / "ws").replace("\\", "/")
+    out_local = str(tmp_path / "out").replace("\\", "/")
+    thread_data = {
+        "workspace_path": str(tmp_path / "ws"),
+        "uploads_path": str(tmp_path / "uploads"),
+        "outputs_path": str(tmp_path / "out"),
+    }
+
+    replaced = replace_virtual_paths_in_command(_COLON_LIST_COMMAND, thread_data)
+
+    assert replaced == f'export PATH="{ws_local}/bin:{out_local}/bin"', f"colon-joined list: replace_virtual_paths_in_command left a virtual entry unresolved: {replaced!r}"
+
+
+def test_single_path_list_entry_keeps_its_dollar_path_suffix(tmp_path: Path) -> None:
+    """The single-entry PATH-style form must keep translating.
+
+    ``export PATH="<venv>/bin:$PATH"`` is the common shape: the ``:$PATH``
+    suffix rides along after the resolved entry and still expands in bash, so
+    ending the tail at ``:`` must not stop the entry itself from resolving.
+    """
+    sbx = _sandbox_with_workspace_and_outputs(tmp_path)
+    ws_local = str((tmp_path / "ws").resolve()).replace("\\", "/")
+
+    resolved = sbx._resolve_paths_in_content('export PATH="/mnt/user-data/workspace/venv/bin:$PATH"\n')
+
+    assert resolved == f'export PATH="{ws_local}/venv/bin:$PATH"\n'
+
+
+def _sandbox_with_symlinked_dir_sharing_a_colon_filenames_prefix(tmp_path: Path) -> LocalSandbox:
+    """Workspace where ``cache`` is a symlink and ``cache:old.txt`` a filename.
+
+    The tail ends at ``:``, so the match for ``cache:old.txt`` stops at the
+    truncated ``cache`` component. ``cache`` is also an unrelated symlink to a
+    sibling ``data`` directory, so canonicalizing the truncated prefix follows
+    it and reassembles a path to ``data:old.txt`` — a different file on POSIX.
+    Colon filenames cannot be created on NTFS, and the divergence under test
+    is the stored spelling, so the files themselves are never created.
+    """
+    sbx = _sandbox_with_workspace_and_outputs(tmp_path)
+    data = tmp_path / "ws" / "data"
+    data.mkdir()
+    symlink_or_skip(tmp_path / "ws" / "cache", data, target_is_directory=True)
+    return sbx
+
+
+def test_write_file_keeps_a_colon_filenames_prefix_through_a_symlinked_directory(tmp_path: Path) -> None:
+    """``write_file`` must keep the original prefix text of a ``:`` filename.
+
+    The match ends at the truncated ``cache`` of ``cache:old.txt``;
+    canonicalizing that prefix follows the ``cache -> data`` symlink, so the
+    stored file pointed at ``data:old.txt`` — the wrong file.
+    """
+    sbx = _sandbox_with_symlinked_dir_sharing_a_colon_filenames_prefix(tmp_path)
+
+    sbx.write_file("/mnt/user-data/workspace/run.sh", _COLON_FILENAME_COMMAND + "\n")
+
+    written = (tmp_path / "ws" / "run.sh").read_text(encoding="utf-8")
+    ws_local = str((tmp_path / "ws").resolve()).replace("\\", "/")
+    expected = _COLON_FILENAME_COMMAND.replace("/mnt/user-data/workspace", ws_local)
+    assert expected in written, f"colon filename: write_file lost the original prefix spelling; host file: {written!r}"
+    assert f"{ws_local}/data:old.txt" not in written, f"colon filename: write_file canonicalized the truncated prefix through the symlink; host file: {written!r}"
+
+
+def test_command_resolution_keeps_a_colon_filenames_prefix_through_a_symlinked_directory(tmp_path: Path) -> None:
+    """``_resolve_paths_in_command`` must not canonicalize the truncated prefix either.
+
+    ``_command_pattern`` uses the same partial-path callback as
+    ``_content_pattern``, so the same wrong-file mechanism applies to
+    commands.
+    """
+    sbx = _sandbox_with_symlinked_dir_sharing_a_colon_filenames_prefix(tmp_path)
+
+    resolved = sbx._resolve_paths_in_command(_COLON_FILENAME_COMMAND)
+
+    ws_local = str((tmp_path / "ws").resolve()).replace("\\", "/")
+    expected = _COLON_FILENAME_COMMAND.replace("/mnt/user-data/workspace", ws_local)
+    assert resolved == expected, f"colon filename: _resolve_paths_in_command canonicalized the truncated prefix through the symlink: {resolved!r}"