diff --git a/backend/packages/harness/deerflow/sandbox/local/local_sandbox.py b/backend/packages/harness/deerflow/sandbox/local/local_sandbox.py index 397308ae0a..a0b070e96a 100644 --- a/backend/packages/harness/deerflow/sandbox/local/local_sandbox.py +++ b/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 diff --git a/backend/packages/harness/deerflow/sandbox/path_patterns.py b/backend/packages/harness/deerflow/sandbox/path_patterns.py index 885ae0dd32..b34ee7610f 100644 --- a/backend/packages/harness/deerflow/sandbox/path_patterns.py +++ b/backend/packages/harness/deerflow/sandbox/path_patterns.py @@ -70,10 +70,15 @@ PATH_START_BOUNDARY = rf"(?():]*)?" +# +# ``,`` 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: diff --git a/backend/packages/harness/deerflow/sandbox/tools.py b/backend/packages/harness/deerflow/sandbox/tools.py index 93abead000..28bd2e8f4f 100644 --- a/backend/packages/harness/deerflow/sandbox/tools.py +++ b/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("\\", "/") diff --git a/backend/tests/test_sandbox_path_patterns.py b/backend/tests/test_sandbox_path_patterns.py index bf7030ef2f..1cf3c0126e 100644 --- a/backend/tests/test_sandbox_path_patterns.py +++ b/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."""