mirror of
https://github.com/bytedance/deer-flow.git
synced 2026-09-19 02:56:17 +00:00
fix(sandbox): reject incomplete remote list_dir results (#5422)
* fix(sandbox): reject incomplete remote list_dir results * style(sandbox): format incomplete traversal error * fix(sandbox): distinguish missing roots from failed listings --------- Co-authored-by: Willem Jiang <willem.jiang@gmail.com>
This commit is contained in:
parent
26800d1245
commit
15a9a87fbd
@ -517,6 +517,10 @@ For Docker development, service startup follows `config.yaml` sandbox mode. In L
|
||||
|
||||
See the [Sandbox Configuration Guide](backend/docs/CONFIGURATION.md#sandbox) to configure your preferred mode.
|
||||
|
||||
Remote directory listings report traversal failures (for example, unreadable
|
||||
directories) as incomplete results, even when no entries were returned. A
|
||||
missing start path is reported separately as “Directory not found.”
|
||||
|
||||
#### MCP Server
|
||||
|
||||
In the chat UI, enable **Token Usage → Debug** to inspect generic/MCP tool calls.
|
||||
|
||||
@ -97,7 +97,7 @@
|
||||
**Sandbox Tools** (in `packages/harness/deerflow/sandbox/tools.py`):
|
||||
- Every sandbox tool keeps a model-visible `description` field for a human-readable progress label, but the field is optional and defaults to an empty string. Tool execution must depend only on its operational arguments; the frontend supplies localized fallback labels when a provider omits `description`.
|
||||
- `bash` - Execute commands with path translation and error handling. For `LocalSandbox` (host bash), output on POSIX and Windows is captured through bounded pipe-drain threads and stdin is `/dev/null`; Windows capture decodes with the platform text encoding and applies universal-newline translation, matching the former `subprocess.run(..., text=True)` behavior for locale-code-page output, Python UTF-8 Mode, CRLF, and bare CR. That translation is Windows-only so the pre-existing POSIX output contract remains byte-decoded without newline rewriting. On POSIX, a backgrounded long-lived process (`server &`) returns immediately instead of blocking the turn on an inherited pipe, while unredirected background output is drained without growing anonymous temp files. Commands that read stdin get immediate EOF. The command runs in its own process group with a wall-clock timeout (`sandbox.bash_command_timeout`, default 600s); on timeout the whole POSIX process group or Windows process tree is killed and the agent gets a notice telling it to background long-lived processes. The shared bash tool description scopes host environment detection to LocalSandbox: start with `uname -s`, follow with `sw_vers` on Darwin, and read Linux host system files only when the active policy permits them. Local path and `file://` rejections provide the same conditional recovery guidance: command-only probes for environment questions, allowed virtual paths otherwise, and no repetition of the rejected path. The description also instructs the model to background long-lived processes (e.g. servers) up front so it doesn't waste the turn waiting on a foreground server. See `LocalSandbox.execute_command`, its platform runners, and `bash_tool`'s docstring.
|
||||
- `ls` - Directory listing (tree format, max 2 levels)
|
||||
- `ls` - Directory listing (tree format, max 2 levels). Remote commands precheck root existence and emit `__DF_FIND_STATUS__:missing`; `find` status 1 is always an incomplete-traversal `OSError`, including when no entries were printed. Do not infer a missing path from status 1 alone.
|
||||
- `glob` - Find files or directories below a root directory with bounded results
|
||||
- `grep` - Search one text file or recursively search a directory, with optional glob filtering and bounded line-level results
|
||||
- `read_file` - Read file contents with optional line range
|
||||
|
||||
@ -5,8 +5,9 @@ does not enable ``pipefail``, so the pipeline's exit code is ``head``'s, not
|
||||
``find``'s. A missing ``find`` binary (127) then looks like an empty listing
|
||||
and becomes ``FileNotFoundError``.
|
||||
|
||||
The command below writes ``find``'s own status after the bounded listing so
|
||||
callers can tell a missing path from a command failure. ``head`` closing the
|
||||
The command checks root existence before running ``find`` and writes its own
|
||||
status after the bounded listing, distinguishing missing paths from traversal
|
||||
failures even when no entries were printed. ``head`` closing the
|
||||
pipe can kill ``find`` with SIGPIPE (141); that is a successful truncation,
|
||||
not an error.
|
||||
"""
|
||||
@ -16,10 +17,11 @@ from __future__ import annotations
|
||||
import shlex
|
||||
|
||||
_STATUS_PREFIX = "__DF_FIND_STATUS__:"
|
||||
_MISSING_ROOT = "missing"
|
||||
_LIST_LIMIT = 500
|
||||
# 0 = ok, 1 = find reported a missing start point / tree error, 141 = SIGPIPE
|
||||
# from head truncating a large listing.
|
||||
_FIND_OK = (0, 1, 141)
|
||||
# 0 = ok, 141 = SIGPIPE from head truncating a large listing.
|
||||
# A missing root has its own marker; status 1 always means traversal failed.
|
||||
_FIND_OK = (0, 141)
|
||||
|
||||
|
||||
def remote_list_dir_command(path: str, max_depth: int, *, limit: int = _LIST_LIMIT) -> str:
|
||||
@ -33,7 +35,8 @@ def remote_list_dir_command(path: str, max_depth: int, *, limit: int = _LIST_LIM
|
||||
# ``exit`` of that status (126 if the file is missing): the last command
|
||||
# would otherwise be ``rm``, whose 0/1 is not find's status.
|
||||
return (
|
||||
f"set +e; _st=/tmp/df_find_$$; "
|
||||
f"set +e; if [ ! -e {quoted} ]; then printf '%s\\n' {_STATUS_PREFIX}{_MISSING_ROOT}; exit 1; fi; "
|
||||
f"_st=/tmp/df_find_$$; "
|
||||
f"{{ find -H {quoted} -maxdepth {depth} \\( -type f -o -type d \\) 2>/dev/null; "
|
||||
f'echo $? > "$_st"; }} | head -n {n}; '
|
||||
f'st=$(cat "$_st" 2>/dev/null); '
|
||||
@ -51,8 +54,8 @@ def parse_remote_list_dir_output(
|
||||
"""Parse listing stdout, preferring the find-status marker over pipeline status.
|
||||
|
||||
Raises:
|
||||
OSError: Command/client failure (missing binary, invocation error, ...).
|
||||
FileNotFoundError: ``find`` ran and produced no entries (missing path).
|
||||
OSError: Command/client failure or an incomplete traversal.
|
||||
FileNotFoundError: The root is missing or no listable entries exist.
|
||||
"""
|
||||
# find delimits records with "\n" only. splitlines() would also split on
|
||||
# \v, \f, \x1c-\x1e and \x85, which are legal in Linux filenames. Do not
|
||||
@ -64,6 +67,8 @@ def parse_remote_list_dir_output(
|
||||
find_status: int | None = None
|
||||
if lines and lines[-1].startswith(_STATUS_PREFIX):
|
||||
raw = lines.pop()[len(_STATUS_PREFIX) :]
|
||||
if raw == _MISSING_ROOT:
|
||||
raise FileNotFoundError(resolved)
|
||||
try:
|
||||
find_status = int(raw)
|
||||
except ValueError:
|
||||
@ -73,15 +78,18 @@ def parse_remote_list_dir_output(
|
||||
|
||||
if find_status is None:
|
||||
# Do not treat a missing marker as success. The process status used to
|
||||
# be ``rm``'s (0/1, both in _FIND_OK), which reclassified a lost 127
|
||||
# be ``rm``'s (0/1), which reclassified a lost 127
|
||||
# as FileNotFoundError.
|
||||
if pipeline_exit_code is not None and pipeline_exit_code not in _FIND_OK:
|
||||
if pipeline_exit_code is not None and pipeline_exit_code not in (*_FIND_OK, 1):
|
||||
raise OSError(f"Failed to list_dir {resolved}: command exited with code {pipeline_exit_code}")
|
||||
raise OSError(f"Failed to list_dir {resolved}: find status marker missing")
|
||||
|
||||
entries = [line for line in lines if line]
|
||||
if find_status == 1:
|
||||
raise OSError(f"Failed to list_dir {resolved}: find exited with code 1, usually because some files or directories could not be read; results would be incomplete, so list a narrower path")
|
||||
if find_status not in _FIND_OK:
|
||||
raise OSError(f"Failed to list_dir {resolved}: command exited with code {find_status}")
|
||||
|
||||
entries = [line for line in lines if line]
|
||||
if not entries:
|
||||
raise FileNotFoundError(resolved)
|
||||
return entries
|
||||
|
||||
@ -620,11 +620,13 @@ class TestListDirSerialization:
|
||||
with pytest.raises(OSError, match="Failed to list directory"):
|
||||
sandbox.list_dir("/test")
|
||||
|
||||
def test_list_dir_raises_when_find_returns_no_entries(self, sandbox):
|
||||
sandbox._client.shell.exec_command = MagicMock(return_value=SimpleNamespace(data=SimpleNamespace(output="\n__DF_FIND_STATUS__:1\n", exit_code=1)))
|
||||
@pytest.mark.parametrize("marker, error", [("missing", FileNotFoundError), ("1", OSError)])
|
||||
def test_list_dir_classifies_empty_failure(self, sandbox, marker, error):
|
||||
sandbox._client.shell.exec_command = MagicMock(return_value=SimpleNamespace(data=SimpleNamespace(output=f"\n__DF_FIND_STATUS__:{marker}\n", exit_code=1)))
|
||||
|
||||
with pytest.raises(FileNotFoundError):
|
||||
with pytest.raises(error) as exc:
|
||||
sandbox.list_dir("/missing")
|
||||
assert type(exc.value) is error
|
||||
|
||||
def test_list_dir_raises_oserror_when_result_data_is_none(self, sandbox):
|
||||
sandbox._client.shell.exec_command = MagicMock(return_value=SimpleNamespace(data=None))
|
||||
|
||||
@ -1327,19 +1327,21 @@ def test_list_dir_and_glob_preserve_trailing_space_in_filename() -> None:
|
||||
assert truncated is False
|
||||
|
||||
|
||||
def test_list_dir_raises_when_find_returns_no_entries() -> None:
|
||||
@pytest.mark.parametrize("marker, error", [("missing", FileNotFoundError), ("1", OSError)])
|
||||
def test_list_dir_classifies_empty_failure(marker, error) -> None:
|
||||
class _EmptyBox:
|
||||
async def exec(self, *argv, env=None, timeout=None):
|
||||
return types.SimpleNamespace(stdout="\n__DF_FIND_STATUS__:1\n", stderr="", exit_code=1)
|
||||
return types.SimpleNamespace(stdout=f"\n__DF_FIND_STATUS__:{marker}\n", stderr="", exit_code=1)
|
||||
|
||||
box = BoxliteBox("box-id", box=_EmptyBox(), run=_fake_run)
|
||||
|
||||
with pytest.raises(FileNotFoundError):
|
||||
with pytest.raises(error) as exc:
|
||||
box.list_dir("/mnt/user-data/workspace")
|
||||
assert type(exc.value) is error
|
||||
|
||||
|
||||
def test_list_dir_raises_oserror_when_find_exit_is_not_missing_path() -> None:
|
||||
# find exit 1 is "start point absent"; 127 (no binary) must not look missing.
|
||||
# 127 (no binary) must not look like a missing path.
|
||||
class _MissingBinaryBox:
|
||||
async def exec(self, *argv, env=None, timeout=None):
|
||||
return types.SimpleNamespace(stdout="", stderr="", exit_code=127)
|
||||
|
||||
@ -5228,15 +5228,15 @@ def test_list_dir_raises_when_client_closed():
|
||||
sb.list_dir("/home/user")
|
||||
|
||||
|
||||
def test_list_dir_raises_when_find_returns_no_entries():
|
||||
# `find ... 2>/dev/null` on a missing path yields empty stdout; that is not
|
||||
# a real empty directory (`find -type d` still prints the directory itself).
|
||||
listing = SimpleNamespace(stdout="\n__DF_FIND_STATUS__:1\n", stderr="", exit_code=1)
|
||||
@pytest.mark.parametrize("marker, error", [("missing", FileNotFoundError), ("1", OSError)])
|
||||
def test_list_dir_classifies_empty_failure(marker, error):
|
||||
listing = SimpleNamespace(stdout=f"\n__DF_FIND_STATUS__:{marker}\n", stderr="", exit_code=1)
|
||||
client = FakeClient(commands=FakeCommandsAPI([listing]))
|
||||
sb = _make_sandbox(client)
|
||||
|
||||
with pytest.raises(FileNotFoundError):
|
||||
with pytest.raises(error) as exc:
|
||||
sb.list_dir("/home/user/missing")
|
||||
assert type(exc.value) is error
|
||||
|
||||
|
||||
def test_list_dir_raises_oserror_when_find_exit_is_not_missing_path():
|
||||
|
||||
@ -157,7 +157,7 @@ class _FakeCommands:
|
||||
matches = sorted(path for path in set(paths) if path == root or path.startswith(f"{root}/"))
|
||||
if "__DF_FIND_STATUS__:" in command:
|
||||
status = 0 if matches else 1
|
||||
marker = f"__DF_FIND_STATUS__:{status}"
|
||||
marker = "__DF_FIND_STATUS__:0" if matches else "__DF_FIND_STATUS__:missing"
|
||||
stdout = (*matches, "", marker) if matches else ("", marker)
|
||||
return _execution(stdout=stdout, exit_code=status)
|
||||
return _execution(stdout=tuple(matches))
|
||||
|
||||
@ -23,12 +23,23 @@ def test_parse_marker_127_is_command_failure_not_missing_path() -> None:
|
||||
parse_remote_list_dir_output(stdout, "/dir", pipeline_exit_code=0)
|
||||
|
||||
|
||||
def test_parse_marker_1_empty_is_missing_path() -> None:
|
||||
stdout = "\n__DF_FIND_STATUS__:1\n"
|
||||
def test_parse_missing_marker_is_missing_path() -> None:
|
||||
stdout = "__DF_FIND_STATUS__:missing\n"
|
||||
with pytest.raises(FileNotFoundError):
|
||||
parse_remote_list_dir_output(stdout, "/dir", pipeline_exit_code=0)
|
||||
|
||||
|
||||
def test_parse_marker_1_empty_is_incomplete_failure() -> None:
|
||||
with pytest.raises(OSError, match="results would be incomplete"):
|
||||
parse_remote_list_dir_output("\n__DF_FIND_STATUS__:1\n", "/dir", pipeline_exit_code=1)
|
||||
|
||||
|
||||
def test_parse_marker_1_with_entries_is_incomplete_failure() -> None:
|
||||
stdout = "/dir/visible.txt\n\n__DF_FIND_STATUS__:1\n"
|
||||
with pytest.raises(OSError, match="results would be incomplete"):
|
||||
parse_remote_list_dir_output(stdout, "/dir", pipeline_exit_code=1)
|
||||
|
||||
|
||||
def test_parse_marker_0_returns_listing_and_keeps_trailing_space() -> None:
|
||||
stdout = "/dir/notes.txt \n/dir/sub\n\n__DF_FIND_STATUS__:0\n"
|
||||
assert parse_remote_list_dir_output(stdout, "/dir", pipeline_exit_code=0) == [
|
||||
@ -57,7 +68,7 @@ def test_parse_falls_back_to_pipeline_exit_without_marker() -> None:
|
||||
|
||||
@_POSIX_SH
|
||||
def test_parse_without_marker_real_subprocess_status_is_not_always_ok() -> None:
|
||||
"""``rm -f`` exits 0/1, both in _FIND_OK. A real process status with no marker must not become FileNotFoundError."""
|
||||
"""``rm -f`` exits 0/1. A process status without a marker must not become FileNotFoundError."""
|
||||
for script in ("exit 0", "exit 1"):
|
||||
proc = subprocess.run(["sh", "-c", script], capture_output=True, text=True, check=False)
|
||||
with pytest.raises(OSError, match="marker missing"):
|
||||
@ -76,7 +87,7 @@ def test_command_records_find_status_after_head() -> None:
|
||||
assert "head -n 500" in command
|
||||
assert "__DF_FIND_STATUS__:" in command
|
||||
assert command.index("find -H ") < command.index("head -n")
|
||||
assert command.index("head -n") < command.index("__DF_FIND_STATUS__:")
|
||||
assert command.index("head -n") < command.rindex("__DF_FIND_STATUS__:")
|
||||
assert 'exit "${st:-126}"' in command
|
||||
|
||||
|
||||
@ -126,7 +137,7 @@ def _write_fake_find(tmp_path, script: str):
|
||||
def test_list_dir_command_surfaces_find_127_not_head_0(tmp_path) -> None:
|
||||
fake_bin = _write_fake_find(tmp_path, "#!/bin/sh\nexit 127\n")
|
||||
proc = _run_list_dir_script(
|
||||
remote_list_dir_command("/dir", 2),
|
||||
remote_list_dir_command(str(tmp_path), 2),
|
||||
env=_env_with_bin(str(fake_bin)),
|
||||
)
|
||||
assert proc.returncode == 127
|
||||
@ -134,11 +145,23 @@ def test_list_dir_command_surfaces_find_127_not_head_0(tmp_path) -> None:
|
||||
parse_remote_list_dir_output(proc.stdout, "/dir", pipeline_exit_code=proc.returncode)
|
||||
|
||||
|
||||
@_POSIX_SH
|
||||
def test_list_dir_command_surfaces_partial_find_failure(tmp_path) -> None:
|
||||
fake_bin = _write_fake_find(tmp_path, '#!/bin/sh\nprintf "/dir/visible.txt\\n"\nexit 1\n')
|
||||
proc = _run_list_dir_script(
|
||||
remote_list_dir_command(str(tmp_path), 2),
|
||||
env=_env_with_bin(str(fake_bin)),
|
||||
)
|
||||
assert proc.returncode == 1
|
||||
with pytest.raises(OSError, match="results would be incomplete"):
|
||||
parse_remote_list_dir_output(proc.stdout, "/dir", pipeline_exit_code=proc.returncode)
|
||||
|
||||
|
||||
@_POSIX_SH
|
||||
def test_list_dir_command_records_find_127_under_set_e(tmp_path) -> None:
|
||||
fake_bin = _write_fake_find(tmp_path, "#!/bin/sh\nexit 127\n")
|
||||
proc = _run_list_dir_script(
|
||||
"set -e; " + remote_list_dir_command("/dir", 2),
|
||||
"set -e; " + remote_list_dir_command(str(tmp_path), 2),
|
||||
env=_env_with_bin(str(fake_bin)),
|
||||
)
|
||||
assert proc.returncode == 127
|
||||
@ -151,6 +174,7 @@ def test_list_dir_command_records_find_127_under_set_e(tmp_path) -> None:
|
||||
def test_list_dir_command_missing_path_is_file_not_found(tmp_path) -> None:
|
||||
missing = tmp_path / "no-such-dir"
|
||||
proc = _run_list_dir_script(remote_list_dir_command(str(missing), 2))
|
||||
assert proc.stdout.strip() == "__DF_FIND_STATUS__:missing"
|
||||
with pytest.raises(FileNotFoundError):
|
||||
parse_remote_list_dir_output(proc.stdout, str(missing), pipeline_exit_code=proc.returncode)
|
||||
|
||||
@ -177,10 +201,21 @@ def test_list_dir_command_head_truncation_is_not_an_error(tmp_path) -> None:
|
||||
fake_find.chmod(fake_find.stat().st_mode | stat.S_IEXEC)
|
||||
|
||||
proc = _run_list_dir_script(
|
||||
remote_list_dir_command("/dir", 2),
|
||||
remote_list_dir_command(str(tmp_path), 2),
|
||||
env=_env_with_bin(str(fake_bin)),
|
||||
)
|
||||
entries = parse_remote_list_dir_output(proc.stdout, "/dir", pipeline_exit_code=proc.returncode)
|
||||
assert len(entries) == 500
|
||||
assert entries[0] == "/dir/f1"
|
||||
assert entries[-1] == "/dir/f500"
|
||||
|
||||
|
||||
@_POSIX_SH
|
||||
def test_list_dir_existing_root_with_no_output_is_incomplete_failure(tmp_path) -> None:
|
||||
root = tmp_path / "unreadable 'directory"
|
||||
root.mkdir()
|
||||
fake_bin = _write_fake_find(tmp_path, "#!/bin/sh\nexit 1\n")
|
||||
proc = _run_list_dir_script(remote_list_dir_command(str(root), 2), env=_env_with_bin(str(fake_bin)))
|
||||
assert proc.returncode == 1
|
||||
with pytest.raises(OSError, match="results would be incomplete"):
|
||||
parse_remote_list_dir_output(proc.stdout, str(root), pipeline_exit_code=proc.returncode)
|
||||
|
||||
@ -173,7 +173,8 @@ class _FakeSandbox:
|
||||
listing = ("\n".join(hits) + "\n") if hits else ""
|
||||
if "__DF_FIND_STATUS__:" in script:
|
||||
status = 0 if hits else 1
|
||||
return _FakeResult(stdout=f"{listing}\n__DF_FIND_STATUS__:{status}\n".encode(), exit_code=status)
|
||||
marker = "0" if hits else "missing"
|
||||
return _FakeResult(stdout=f"{listing}\n__DF_FIND_STATUS__:{marker}\n".encode(), exit_code=status)
|
||||
return _FakeResult(stdout=listing.encode())
|
||||
if script.startswith("grep "):
|
||||
# grep <flags> -e <pattern> <root> 2>/dev/null | head -N
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user