From a77a454ca77957cde701fa87b7609e9fcda9de3b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=93=88=E5=9F=BA=E7=B1=B3?= <140241684+BlueX888@users.noreply.github.com> Date: Sun, 18 Oct 2026 07:46:57 +0800 Subject: [PATCH] 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 --- .../deerflow/sandbox/local/local_sandbox.py | 60 ++++++- .../harness/deerflow/sandbox/tools.py | 7 +- .../test_sandbox_colon_joined_path_lists.py | 151 ++++++++++++++++++ 3 files changed, 215 insertions(+), 3 deletions(-) create mode 100644 backend/tests/test_sandbox_colon_joined_path_lists.py diff --git a/backend/packages/harness/deerflow/sandbox/local/local_sandbox.py b/backend/packages/harness/deerflow/sandbox/local/local_sandbox.py index 4ffe168c20..3a0ba1cb76 100644 --- a/backend/packages/harness/deerflow/sandbox/local/local_sandbox.py +++ b/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. diff --git a/backend/packages/harness/deerflow/sandbox/tools.py b/backend/packages/harness/deerflow/sandbox/tools.py index b5c6a4ed9d..55cddce534 100644 --- a/backend/packages/harness/deerflow/sandbox/tools.py +++ b/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("\\", "/") diff --git a/backend/tests/test_sandbox_colon_joined_path_lists.py b/backend/tests/test_sandbox_colon_joined_path_lists.py new file mode 100644 index 0000000000..3f483d858b --- /dev/null +++ b/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="/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="/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}"