From c82fba41d930648e8afe0307d5a35fdbf2bff41e Mon Sep 17 00:00:00 2001 From: Aari Date: Sun, 12 Jul 2026 23:42:58 +0800 Subject: [PATCH] fix(sandbox): guard the command path-translation regex with a segment boundary (#4110) replace_virtual_paths_in_command matches the virtual root with no segment-boundary lookahead: re.compile(rf"{re.escape(VIRTUAL_PATH_PREFIX)}(/[^\s\"';&|<>()]*)?") The trailing group needs a "/" to consume anything, so when the character after /mnt/user-data is "-", ".", "_", a digit or a letter, the group matches empty and the bare root still matches. The substitution then rewrites it to the thread's host user-data directory and the rest of the sibling name rides along, so a command naming a prefix sibling of the mount root is pointed at a real host directory outside the mount contract: cat /mnt/user-data-backup/secret.txt -> cat /user-data-backup/secret.txt which reads the host file. This is the same defect as #4035 (reverse patterns) and #4053 (masking patterns), mirrored into the virtual->host direction; it is the last unguarded member of that family. The boundary class mirrors LocalSandbox._content_pattern's rather than _command_pattern's: a virtual root can legitimately be followed by ":" (PATH-style concatenation) or ",", which the shell-oriented class rejects, so narrowing to it would stop translating paths that translate today. "$" covers a command ending exactly at the root. --- .../harness/deerflow/sandbox/tools.py | 16 ++++++- backend/tests/test_sandbox_tools_security.py | 47 +++++++++++++++++++ 2 files changed, 62 insertions(+), 1 deletion(-) diff --git a/backend/packages/harness/deerflow/sandbox/tools.py b/backend/packages/harness/deerflow/sandbox/tools.py index 5f171095a..9d44d6a43 100644 --- a/backend/packages/harness/deerflow/sandbox/tools.py +++ b/backend/packages/harness/deerflow/sandbox/tools.py @@ -1244,7 +1244,21 @@ def replace_virtual_paths_in_command(command: str, thread_data: ThreadDataState # 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\"';&|<>()]*)?") + # The segment-boundary lookahead is what keeps the virtual root from + # matching inside a sibling that merely shares its prefix + # (``/mnt/user-data`` inside ``/mnt/user-data-backup``). The trailing + # group needs a ``/`` to consume anything, so without the lookahead the + # bare root still matches and the sibling is rewritten into the thread's + # host directory — a real path outside the mount contract. Same defect as + # #4035 (reverse patterns) and #4053 (masking patterns), mirrored into + # this direction. + # + # The class mirrors ``LocalSandbox._content_pattern``'s rather than + # ``_command_pattern``'s: a virtual root can legitimately be followed by + # ``:`` (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\"';&|<>()]*)?") 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_tools_security.py b/backend/tests/test_sandbox_tools_security.py index b30e98d8d..4356d13b9 100644 --- a/backend/tests/test_sandbox_tools_security.py +++ b/backend/tests/test_sandbox_tools_security.py @@ -451,6 +451,53 @@ def test_replace_virtual_paths_in_command_replaces_user_data_only() -> None: assert "/tmp/deer-flow/threads/t1/user-data/workspace/out.txt" in result +@pytest.mark.parametrize( + "sibling", + [ + "/mnt/user-data-backup/secret.txt", + "/mnt/user-data2/report.txt", + "/mnt/user-data.bak", + "/mnt/user-data_old/x", + ], +) +def test_replace_virtual_paths_in_command_does_not_rewrite_prefix_siblings(sibling: str) -> None: + """A path that merely starts with the virtual root is not a virtual path. + + The matcher's trailing group needs a ``/`` to consume anything, so when the + character after ``/mnt/user-data`` is ``-``, ``.``, ``_``, a digit or a + letter, the group matches empty and the bare root still matches. The + substitution then rewrites it to the thread's host directory, and the rest of + the sibling name rides along — handing the command a real host path outside + the mount contract (``.../user-data-backup``), which the agent then reads or + writes. + + Same defect as #4035 (reverse patterns) and #4053 (masking patterns), + mirrored into the virtual→host command direction. + """ + result = replace_virtual_paths_in_command(f"cat {sibling}", _THREAD_DATA) + + assert result == f"cat {sibling}" + assert "/tmp/deer-flow/threads/t1" not in result + + +@pytest.mark.parametrize( + ("command", "expected"), + [ + # The bare root, at end of string and before shell/text punctuation, must + # keep translating — these guard the boundary from being narrowed too far. + ("ls /mnt/user-data", "ls /tmp/deer-flow/threads/t1/user-data"), + ("ls /mnt/user-data && pwd", "ls /tmp/deer-flow/threads/t1/user-data && pwd"), + ("PYTHONPATH=/mnt/user-data:/opt x", "PYTHONPATH=/tmp/deer-flow/threads/t1/user-data:/opt x"), + ("echo '/mnt/user-data, done'", "echo '/tmp/deer-flow/threads/t1/user-data, done'"), + # Real children still translate. + ("cat /mnt/user-data/workspace/a.txt", "cat /tmp/deer-flow/threads/t1/user-data/workspace/a.txt"), + ], +) +def test_replace_virtual_paths_in_command_still_translates_genuine_paths(command: str, expected: str) -> None: + """The narrowing must not stop translating paths that translate today.""" + assert replace_virtual_paths_in_command(command, _THREAD_DATA) == expected + + # ---------- validate_local_bash_command_paths ----------