fix(sandbox): end the host-path tail at a comma like the other list separators (#8244)

* fix(sandbox): end the host-path tail at a comma like the other list separators

The reverse-masking tail stops at whitespace and at " ; & | < > ( ) :",
but not at ",". A ,-joined list of host paths in command output therefore
lets the first match's tail swallow every later entry, and those paths
reach the model unmasked -- the same failure mode fixed for :-joined
lists ($PATH, $PYTHONPATH) earlier. The forward patterns have the mirror
defect: a ,-joined list of virtual paths resolves only the first entry.

"," also joins paths in real output (pytest/mypy/sed-style lists), and a
"," inside one path only shortens the match, so no path can be corrupted
by terminating the tail there.

* docs(sandbox): extend the tail-terminator rationale to the comma

Review nits: the _PATH_TAIL rationale comment now explains why `,` ends
the tail alongside `:` (a comma-joined argument listing would otherwise
hide later entries inside the first match's tail), and the snapshot
test's prose no longer calls the `:` stop the only tail change -- the
docstring and the literal's comment both name the `,` delta.

---------

Co-authored-by: laoyyu <laoyyu@users.noreply.github.com>

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

@@ -524,7 +524,7 @@ class LocalSandbox(Sandbox):
         # 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 = [PATH_START_BOUNDARY + re.escape(m.container_path) + r"(?=/|$|[\s\"';&|<>()])(?:/[^\s\"';&|<>():]*)?" for m in mappings]
+        patterns = [PATH_START_BOUNDARY + re.escape(m.container_path) + r"(?=/|$|[\s\"';&|<>()])(?:/[^\s\"';&|<>():,]*)?" for m in mappings]
         return re.compile("|".join(f"({p})" for p in patterns))
 
     @cached_property
@@ -537,7 +537,7 @@ class LocalSandbox(Sandbox):
         # 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 = [PATH_START_BOUNDARY + re.escape(m.container_path) + r"(?=/|$|[^\w./-])(?:/[^\s\"';&|<>():]*)?" for m in mappings]
+        patterns = [PATH_START_BOUNDARY + re.escape(m.container_path) + r"(?=/|$|[^\w./-])(?:/[^\s\"';&|<>():,]*)?" for m in mappings]
         return re.compile("|".join(f"({p})" for p in patterns))
 
     @cached_property

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

@@ -70,10 +70,15 @@ PATH_START_BOUNDARY = rf"(?<!{_PATH_START_BLOCKERS.pattern})"
 # entry under the same base to the model unmasked. ``;``, the Windows list
 # separator, already ended it. A ``:`` inside one path (``grep -n`` output,
 # a file name) only shortens the match; the rest is copied through verbatim.
-_PATH_TAIL = r"(?:[/\\][^\s\"';&|<>():]*)?"
+#
+# ``,`` ends the tail for the same reason: a ``,``-joined list (comma-
+# separated argument listings) would otherwise hide every later entry inside
+# the first match's tail. A ``,`` inside one path only shortens the match;
+# the rest is spliced through verbatim.
+_PATH_TAIL = r"(?:[/\\][^\s\"';&|<>():,]*)?"
 
 _SEGMENT_BOUNDARY_CHAR = re.compile(r"[^\w./-]")
-_PATH_TAIL_TERMINATORS = frozenset("\"';&|<>():")
+_PATH_TAIL_TERMINATORS = frozenset("\"';&|<>():,")
 
 
 def normalize_mask_tail(tail: str) -> str:

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

@@ -2037,7 +2037,7 @@ def replace_virtual_paths_in_command(command: str, thread_data: ThreadDataState
         # 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"{PATH_START_BOUNDARY}{re.escape(VIRTUAL_PATH_PREFIX)}(?=/|$|[^\w./-])(/[^\s\"';&|<>():]*)?")
+        pattern = re.compile(rf"{PATH_START_BOUNDARY}{re.escape(VIRTUAL_PATH_PREFIX)}(?=/|$|[^\w./-])(/[^\s\"';&|<>():,]*)?")
 
         def replace_user_data_match(match: re.Match) -> str:
             resolved = replace_virtual_path(match.group(0), thread_data).replace("\\", "/")

backend/tests/test_sandbox_path_patterns.py

@@ -11,8 +11,8 @@ expressions, run once on the parent commit. That run cannot be committed: after
 this lands there is no old inline expression left to diff against, only the
 frozen copies below. So the committed guard is the weaker snapshot, and its
 red-ness rests on those literals — not on the length of ``_BASES``. The tail has
-changed exactly once since then (it now stops at ``:``), and the base gained a
-left-hand path-start boundary; the snapshot names those deltas instead of
+changed twice since then (it now stops at ``:`` and ``,``), and the base gained
+a left-hand path-start boundary; the snapshot names those deltas instead of
 re-freezing the literals, so any other drift still goes red.
 """
 
@@ -42,11 +42,12 @@ def _legacy_local_pattern(base: str) -> re.Pattern[str]:
     return re.compile(re.escape(base) + r"(?=/|$|[^\w./-])" + r"(?:[/\\][^\s\"';&|<>()]*)?")
 
 
-# The only intentional change to either expression since the extraction: the
-# tail also stops at ``:``, so a ``:``-joined path list ($PATH, $PYTHONPATH)
-# cannot swallow a second host path into the first match's tail.
+# The intentional tail changes since the extraction: it also stops at ``:``,
+# so a ``:``-joined path list ($PATH, $PYTHONPATH) cannot swallow a second
+# host path into the first match's tail, and at ``,``, so a comma-separated
+# argument listing cannot either.
 _LEGACY_TAIL = r"(?:[/\\][^\s\"';&|<>()]*)?"
-_CURRENT_TAIL = r"(?:[/\\][^\s\"';&|<>():]*)?"
+_CURRENT_TAIL = r"(?:[/\\][^\s\"';&|<>():,]*)?"
 
 
 def _with_current_tail(legacy: str) -> str:
@@ -211,6 +212,40 @@ def test_colon_inside_a_single_path_leaves_the_rendered_output_unchanged(mask, t
     assert mask(f"/host/ws{tail}", "/host/ws", "/mnt/ws") == f"/mnt/ws{tail}"
 
 
+@pytest.mark.parametrize("mask", _MASKERS)
+@pytest.mark.parametrize(
+    ("output", "expected"),
+    [
+        ("files=/host/ws/a.py,/host/ws/b.py", "files=/mnt/ws/a.py,/mnt/ws/b.py"),
+        ("copy /host/ws/src,/host/ws/tests /dst", "copy /mnt/ws/src,/mnt/ws/tests /dst"),
+        (
+            "extra=/host/ws/x,other=/host/ws/y",
+            "extra=/mnt/ws/x,other=/mnt/ws/y",
+        ),
+    ],
+)
+def test_tail_stops_at_comma_so_a_list_masks_every_entry(mask, output: str, expected: str) -> None:
+    """A ``,``-joined list must not hide a second host path inside the first tail.
+
+    The same failure mode the ``:`` fix covered for PATH-style lists: once a
+    match consumes the rest of the list, scanning resumes after it, so every
+    later entry under the same base reaches the model as a raw host path.
+    """
+    assert mask(output, "/host/ws", "/mnt/ws") == expected
+
+
+@pytest.mark.parametrize("mask", _MASKERS)
+@pytest.mark.parametrize(
+    "tail",
+    ["/report,final.py", "/notes,a.txt:2024.csv"],
+)
+def test_comma_inside_a_single_path_leaves_the_rendered_output_unchanged(mask, tail: str) -> None:
+    """A ``,`` in a real file name only shortens the match: the rest of the text
+    is copied through verbatim right after the virtual prefix, so the whole
+    path still reads back as the same virtual file."""
+    assert mask(f"/host/ws{tail}", "/host/ws", "/mnt/ws") == f"/mnt/ws{tail}"
+
+
 def test_local_sandbox_reverse_mask_handles_a_colon_joined_path_list(tmp_path: Path) -> None:
     """The callable replacement path: ``_reverse_resolve_path`` receives each
     entry separately instead of the whole list as one fake path."""