mirror of
https://github.com/bytedance/deer-flow.git
synced 2026-07-25 23:48:00 +00:00
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 <host>/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.
This commit is contained in:
parent
0542d3c5f3
commit
c82fba41d9
@ -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("\\", "/")
|
||||
|
||||
@ -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 ----------
|
||||
|
||||
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user