From 6b4f803354785798d8824ff9e5d0552cba1380ae Mon Sep 17 00:00:00 2001 From: Jeremy Schoemaker Date: Tue, 1 Sep 2026 20:23:04 -0500 Subject: [PATCH] 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. --- .../community/aio_sandbox/aio_sandbox.py | 8 ++++++- .../harness/deerflow/community/boxlite/box.py | 6 +++-- .../community/e2b_sandbox/e2b_sandbox.py | 7 ++++-- .../deerflow/community/opensandbox/sandbox.py | 6 +++-- .../deerflow/community/tenki/sandbox.py | 6 +++-- backend/tests/test_aio_sandbox.py | 8 +++++++ backend/tests/test_boxlite_provider.py | 16 ++++++++++++++ backend/tests/test_e2b_sandbox_provider.py | 22 +++++++++++++++++++ backend/tests/test_opensandbox_provider.py | 14 ++++++++++++ backend/tests/test_tenki_provider.py | 13 +++++++++++ 10 files changed, 97 insertions(+), 9 deletions(-) diff --git a/backend/packages/harness/deerflow/community/aio_sandbox/aio_sandbox.py b/backend/packages/harness/deerflow/community/aio_sandbox/aio_sandbox.py index 4cebdd6d4..dc6dd1da8 100644 --- a/backend/packages/harness/deerflow/community/aio_sandbox/aio_sandbox.py +++ b/backend/packages/harness/deerflow/community/aio_sandbox/aio_sandbox.py @@ -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}") diff --git a/backend/packages/harness/deerflow/community/boxlite/box.py b/backend/packages/harness/deerflow/community/boxlite/box.py index 05214fe59..c3d21bf2f 100644 --- a/backend/packages/harness/deerflow/community/boxlite/box.py +++ b/backend/packages/harness/deerflow/community/boxlite/box.py @@ -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): diff --git a/backend/packages/harness/deerflow/community/e2b_sandbox/e2b_sandbox.py b/backend/packages/harness/deerflow/community/e2b_sandbox/e2b_sandbox.py index 0fc567b22..10d9a0196 100644 --- a/backend/packages/harness/deerflow/community/e2b_sandbox/e2b_sandbox.py +++ b/backend/packages/harness/deerflow/community/e2b_sandbox/e2b_sandbox.py @@ -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): diff --git a/backend/packages/harness/deerflow/community/opensandbox/sandbox.py b/backend/packages/harness/deerflow/community/opensandbox/sandbox.py index f4e50288b..5f7a20c28 100644 --- a/backend/packages/harness/deerflow/community/opensandbox/sandbox.py +++ b/backend/packages/harness/deerflow/community/opensandbox/sandbox.py @@ -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("/") diff --git a/backend/packages/harness/deerflow/community/tenki/sandbox.py b/backend/packages/harness/deerflow/community/tenki/sandbox.py index e603274e0..d120a52f2 100644 --- a/backend/packages/harness/deerflow/community/tenki/sandbox.py +++ b/backend/packages/harness/deerflow/community/tenki/sandbox.py @@ -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): diff --git a/backend/tests/test_aio_sandbox.py b/backend/tests/test_aio_sandbox.py index 444024b9b..610adc916 100644 --- a/backend/tests/test_aio_sandbox.py +++ b/backend/tests/test_aio_sandbox.py @@ -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"] diff --git a/backend/tests/test_boxlite_provider.py b/backend/tests/test_boxlite_provider.py index c48cf0c23..d13949616 100644 --- a/backend/tests/test_boxlite_provider.py +++ b/backend/tests/test_boxlite_provider.py @@ -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 diff --git a/backend/tests/test_e2b_sandbox_provider.py b/backend/tests/test_e2b_sandbox_provider.py index 387949361..e7574d96a 100644 --- a/backend/tests/test_e2b_sandbox_provider.py +++ b/backend/tests/test_e2b_sandbox_provider.py @@ -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 diff --git a/backend/tests/test_opensandbox_provider.py b/backend/tests/test_opensandbox_provider.py index 084bc2304..69cc25763 100644 --- a/backend/tests/test_opensandbox_provider.py +++ b/backend/tests/test_opensandbox_provider.py @@ -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 diff --git a/backend/tests/test_tenki_provider.py b/backend/tests/test_tenki_provider.py index b0a10fa0a..9a3c70340 100644 --- a/backend/tests/test_tenki_provider.py +++ b/backend/tests/test_tenki_provider.py @@ -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