mirror of
https://github.com/bytedance/deer-flow.git
synced 2026-09-09 21:49:37 +00:00
fix(sandbox): preserve trailing whitespace in filenames from list_dir and glob in remote providers (#4980)
* fix(sandbox): stop stripping filenames when parsing find output in remote providers The list_dir and glob parsers in the e2b, OpenSandbox, AIO, Tenki, and BoxLite providers called .strip() on every line of find output. A filename that legitimately ends (or begins) in whitespace was corrupted, so the listed path never resolved on any follow-up file API call, and the remote providers diverged from LocalSandbox, which preserves such names via pathlib. splitlines() already removes the line terminators, so filter empty lines only and keep each entry verbatim. Same class of bug as the e2b _sync_outputs_to_host fix (#4861), applied to the search parsers. Adds a trailing-space regression test per provider at the seam each suite already uses. * fix(sandbox): split find output on \n only, and rename the tenki test Review follow-ups from willem-bd: - aio_sandbox.list_dir used str.splitlines(), which also breaks records on \v, \f, \x1c-\x1e and \x85 - all legal inside a Linux filename, and all contrary to this PR's own rule that the newline is the only delimiter. find emits \n and nothing else, so split("\n") is the correct parse. - Renamed test_search_preserves_trailing_space_in_filename to test_list_dir_and_glob_preserve_trailing_space_in_filename, matching the sibling tests in test_opensandbox_provider.py and test_boxlite_provider.py. The body covers list_dir and glob; it never touches grep.
This commit is contained in:
parent
755b328caa
commit
6b4f803354
@ -381,7 +381,13 @@ class AioSandbox(Sandbox):
|
||||
result = self._client.shell.exec_command(command=f"find {shlex.quote(path)} -maxdepth {max_depth} -type f -o -type d 2>/dev/null | head -500", no_change_timeout=self._DEFAULT_NO_CHANGE_TIMEOUT)
|
||||
output = result.data.output if result.data else ""
|
||||
if output:
|
||||
return [line.strip() for line in output.strip().split("\n") if line.strip()]
|
||||
# find delimits records with "\n" and nothing else, so split
|
||||
# on that alone: splitlines() would also break on \v, \f,
|
||||
# \x1c-\x1e and \x85, all of which are legal inside a Linux
|
||||
# filename. Do NOT strip entries either — a filename that
|
||||
# legitimately ends in whitespace would be corrupted and
|
||||
# never resolve again.
|
||||
return [line for line in output.split("\n") if line]
|
||||
return []
|
||||
except Exception as e:
|
||||
logger.error(f"Failed to list directory in sandbox: {e}")
|
||||
|
||||
@ -291,7 +291,9 @@ class BoxliteBox(Sandbox):
|
||||
def list_dir(self, path: str, max_depth: int = 2) -> list[str]:
|
||||
resolved = self._resolve_path(path)
|
||||
r = self._sh(f"find {shlex.quote(resolved)} -maxdepth {int(max_depth)} \\( -type f -o -type d \\) 2>/dev/null | head -500")
|
||||
return [line.strip() for line in (r.stdout or "").splitlines() if line.strip()]
|
||||
# splitlines() already removed the terminators; do NOT strip entries —
|
||||
# a filename that legitimately ends in whitespace would be corrupted.
|
||||
return [line for line in (r.stdout or "").splitlines() if line]
|
||||
|
||||
def glob(
|
||||
self,
|
||||
@ -311,7 +313,7 @@ class BoxliteBox(Sandbox):
|
||||
root = resolved.rstrip("/") or "/"
|
||||
root_prefix = root if root == "/" else f"{root}/"
|
||||
for entry in (r.stdout or "").splitlines():
|
||||
entry = entry.strip()
|
||||
# Do NOT strip: trailing whitespace can be part of the filename.
|
||||
if not entry or (entry != root and not entry.startswith(root_prefix)):
|
||||
continue
|
||||
if should_ignore_path(entry):
|
||||
|
||||
@ -338,7 +338,10 @@ class E2BSandbox(Sandbox):
|
||||
try:
|
||||
result = client.commands.run(f"find {shlex.quote(resolved)} -maxdepth {int(max_depth)} \\( -type f -o -type d \\) 2>/dev/null | head -500")
|
||||
output = getattr(result, "stdout", "") or ""
|
||||
return [line.strip() for line in output.splitlines() if line.strip()]
|
||||
# splitlines() already removed the terminators; do NOT strip
|
||||
# entries — a filename that legitimately ends in whitespace
|
||||
# would be corrupted and never resolve again.
|
||||
return [line for line in output.splitlines() if line]
|
||||
except Exception as e:
|
||||
logger.error("Failed to list_dir %s in e2b sandbox: %s", resolved, e)
|
||||
return []
|
||||
@ -405,7 +408,7 @@ class E2BSandbox(Sandbox):
|
||||
root = resolved.rstrip("/") or "/"
|
||||
root_prefix = root if root == "/" else f"{root}/"
|
||||
for entry in output.splitlines():
|
||||
entry = entry.strip()
|
||||
# Do NOT strip: trailing whitespace can be part of the filename.
|
||||
if not entry:
|
||||
continue
|
||||
if entry != root and not entry.startswith(root_prefix):
|
||||
|
||||
@ -325,7 +325,9 @@ class OpenSandboxSandbox(Sandbox):
|
||||
raise ValueError("max_depth must be non-negative")
|
||||
resolved = self._resolve_path(path)
|
||||
execution = self._run(f"find {shlex.quote(resolved)} -maxdepth {depth} \\( -type f -o -type d \\) 2>/dev/null | head -500")
|
||||
return [line.strip() for line in execution_stdout(execution).splitlines() if line.strip()]
|
||||
# splitlines() already removed the terminators; do NOT strip entries —
|
||||
# a filename that legitimately ends in whitespace would be corrupted.
|
||||
return [line for line in execution_stdout(execution).splitlines() if line]
|
||||
|
||||
def glob(self, path: str, pattern: str, *, include_dirs: bool = False, max_results: int = 200) -> tuple[list[str], bool]:
|
||||
if max_results <= 0:
|
||||
@ -340,7 +342,7 @@ class OpenSandboxSandbox(Sandbox):
|
||||
root = resolved.rstrip("/") or "/"
|
||||
root_prefix = root if root == "/" else f"{root}/"
|
||||
for entry in execution_stdout(execution).splitlines():
|
||||
entry = entry.strip()
|
||||
# Do NOT strip: trailing whitespace can be part of the filename.
|
||||
if not entry or (entry != root and not entry.startswith(root_prefix)) or should_ignore_path(entry):
|
||||
continue
|
||||
relative = entry[len(root) :].lstrip("/")
|
||||
|
||||
@ -369,7 +369,9 @@ class TenkiSandbox(Sandbox):
|
||||
def list_dir(self, path: str, max_depth: int = 2) -> list[str]:
|
||||
resolved = self._resolve_path(path)
|
||||
r = self._sh(f"find {shlex.quote(resolved)} -maxdepth {int(max_depth)} \\( -type f -o -type d \\) 2>/dev/null | head -500")
|
||||
return [self._virtual_path(line.strip()) for line in (r.stdout_text or "").splitlines() if line.strip()]
|
||||
# splitlines() already removed the terminators; do NOT strip entries —
|
||||
# a filename that legitimately ends in whitespace would be corrupted.
|
||||
return [self._virtual_path(line) for line in (r.stdout_text or "").splitlines() if line]
|
||||
|
||||
def glob(
|
||||
self,
|
||||
@ -389,7 +391,7 @@ class TenkiSandbox(Sandbox):
|
||||
root = resolved.rstrip("/") or "/"
|
||||
root_prefix = root if root == "/" else f"{root}/"
|
||||
for entry in (r.stdout_text or "").splitlines():
|
||||
entry = entry.strip()
|
||||
# Do NOT strip: trailing whitespace can be part of the filename.
|
||||
if not entry or (entry != root and not entry.startswith(root_prefix)):
|
||||
continue
|
||||
if should_ignore_path(entry):
|
||||
|
||||
@ -593,3 +593,11 @@ class TestClose:
|
||||
sandbox._client = SimpleNamespace() # no close, no _client_wrapper
|
||||
sandbox.close() # must not raise
|
||||
assert sandbox._client is None
|
||||
|
||||
|
||||
def test_list_dir_preserves_trailing_space_in_filename(sandbox):
|
||||
""" "notes.txt " (trailing space) is a legal Linux filename; find prints it
|
||||
verbatim, one entry per line, so a per-line strip() corrupts the name."""
|
||||
sandbox._client.shell.exec_command = MagicMock(return_value=SimpleNamespace(data=SimpleNamespace(output="/test/notes.txt \n/test/sub\n")))
|
||||
|
||||
assert sandbox.list_dir("/test") == ["/test/notes.txt ", "/test/sub"]
|
||||
|
||||
@ -1302,3 +1302,19 @@ def test_sandbox_id_none_user_quirk_pinned():
|
||||
from deerflow.sandbox.identity import derive_sandbox_scope_token
|
||||
|
||||
assert BoxliteProvider._sandbox_id("t-1", None) == derive_sandbox_scope_token(user_id="None", thread_id="t-1")
|
||||
|
||||
|
||||
def test_list_dir_and_glob_preserve_trailing_space_in_filename() -> None:
|
||||
# "notes.txt " (trailing space) is a legal Linux filename; find prints it
|
||||
# verbatim, one entry per line, so a per-line strip() corrupts the name.
|
||||
class _FindBox:
|
||||
async def exec(self, *argv, env=None, timeout=None):
|
||||
return types.SimpleNamespace(stdout="/mnt/user-data/workspace/notes.txt \n", stderr="", exit_code=0)
|
||||
|
||||
box = BoxliteBox("box-id", box=_FindBox(), run=_fake_run)
|
||||
|
||||
assert box.list_dir("/mnt/user-data/workspace") == ["/mnt/user-data/workspace/notes.txt "]
|
||||
|
||||
found, truncated = box.glob("/mnt/user-data/workspace", "notes*")
|
||||
assert found == ["/mnt/user-data/workspace/notes.txt "]
|
||||
assert truncated is False
|
||||
|
||||
@ -4899,3 +4899,25 @@ def test_stable_seed_matches_shared_identity():
|
||||
).hexdigest()[:16]
|
||||
|
||||
assert provider._stable_seed("t-1", "u-1") == expected
|
||||
|
||||
|
||||
def test_list_dir_preserves_trailing_space_in_filename():
|
||||
# "notes.txt " (trailing space) is a legal Linux filename; find prints it
|
||||
# verbatim, one entry per line, so a per-line strip() corrupts the name and
|
||||
# every follow-up file API call on the listed path misses the real file.
|
||||
listing = SimpleNamespace(stdout="/home/user/notes.txt \n/home/user/sub\n", stderr="", exit_code=0)
|
||||
client = FakeClient(commands=FakeCommandsAPI([listing]))
|
||||
sb = _make_sandbox(client)
|
||||
|
||||
assert sb.list_dir("/home/user") == ["/home/user/notes.txt ", "/home/user/sub"]
|
||||
|
||||
|
||||
def test_glob_preserves_trailing_space_in_filename():
|
||||
listing = SimpleNamespace(stdout="/home/user/notes.txt \n", stderr="", exit_code=0)
|
||||
client = FakeClient(commands=FakeCommandsAPI([listing]))
|
||||
sb = _make_sandbox(client)
|
||||
|
||||
matches, truncated = sb.glob("/home/user", "notes*")
|
||||
|
||||
assert matches == ["/home/user/notes.txt "]
|
||||
assert truncated is False
|
||||
|
||||
@ -757,3 +757,17 @@ def test_sandbox_id_matches_shared_identity():
|
||||
|
||||
assert OpenSandboxProvider._sandbox_id("t-1", "u-1") == derive_sandbox_scope_token(user_id="u-1", thread_id="t-1")
|
||||
assert OpenSandboxProvider._sandbox_id("t-1", "") == derive_sandbox_scope_token(user_id="", thread_id="t-1")
|
||||
|
||||
|
||||
def test_list_dir_and_glob_preserve_trailing_space_in_filename() -> None:
|
||||
# "notes.txt " (trailing space) is a legal Linux filename; find prints it
|
||||
# verbatim, one entry per line, so a per-line strip() corrupts the name.
|
||||
remote = _FakeRemote("remote")
|
||||
box = _box(remote)
|
||||
box.write_file("/mnt/user-data/workspace/notes.txt ", "payload")
|
||||
|
||||
assert "/mnt/user-data/workspace/notes.txt " in box.list_dir("/mnt/user-data/workspace")
|
||||
|
||||
found, truncated = box.glob("/mnt/user-data/workspace", "notes*")
|
||||
assert found == ["/mnt/user-data/workspace/notes.txt "]
|
||||
assert truncated is False
|
||||
|
||||
@ -1015,3 +1015,16 @@ def test_sandbox_id_matches_shared_identity():
|
||||
|
||||
assert TenkiSandboxProvider._sandbox_id("t-1", "u-1") == derive_sandbox_scope_token(user_id="u-1", thread_id="t-1")
|
||||
assert TenkiSandboxProvider._sandbox_id("t-1", "") == derive_sandbox_scope_token(user_id="", thread_id="t-1")
|
||||
|
||||
|
||||
def test_list_dir_and_glob_preserve_trailing_space_in_filename() -> None:
|
||||
# "notes.txt " (trailing space) is a legal Linux filename; find prints it
|
||||
# verbatim, one entry per line, so a per-line strip() corrupts the name.
|
||||
box = TenkiSandbox("sb", _FakeSandbox())
|
||||
box.write_file("/mnt/user-data/workspace/notes.txt ", "payload\n")
|
||||
|
||||
assert box.list_dir("/mnt/user-data/workspace") == ["/mnt/user-data/workspace/notes.txt "]
|
||||
|
||||
found, truncated = box.glob("/mnt/user-data/workspace", "notes*")
|
||||
assert found == ["/mnt/user-data/workspace/notes.txt "]
|
||||
assert truncated is False
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user