fix(sandbox): push read_file ranges into sandbox reads (#3824)

* fix read_file range validation and passthrough

* chore: format test_aio_sandbox.py

* test: cover e2b sandbox read_file line-range behavior

* fix(boxlite): support ranged file reads

* fix(sandbox): reject non-positive start lines

---------

Co-authored-by: Willem Jiang <willem.jiang@gmail.com>
This commit is contained in:
Minh Vu 2026-07-30 01:53:10 +02:00 committed by GitHub
parent 9d915ca8ca
commit 904cee4a72
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
14 changed files with 342 additions and 34 deletions

View File

@ -281,7 +281,12 @@ class AioSandbox(Sandbox):
logger.error(f"Failed to execute command with injected env in sandbox: {e}")
return f"Error: {e}"
def read_file(self, path: str) -> str:
def read_file(
self,
path: str,
start_line: int | None = None,
end_line: int | None = None,
) -> str:
"""Read the content of a file in the sandbox.
Args:
@ -291,7 +296,12 @@ class AioSandbox(Sandbox):
The content of the file.
"""
try:
result = self._client.file.read_file(file=path)
kwargs = {}
if start_line is not None:
kwargs["start_line"] = max(start_line - 1, 0)
if end_line is not None:
kwargs["end_line"] = max(end_line, 0)
result = self._client.file.read_file(file=path, **kwargs)
return result.data.content if result.data else ""
except Exception as e:
logger.error(f"Failed to read file in sandbox: {e}")

View File

@ -205,7 +205,12 @@ class BoxliteBox(Sandbox):
# ── file operations ─────────────────────────────────────────────────
def read_file(self, path: str) -> str:
def read_file(
self,
path: str,
start_line: int | None = None,
end_line: int | None = None,
) -> str:
resolved = self._resolve_path(path)
try:
r = self._exec("cat", "--", resolved)
@ -214,7 +219,13 @@ class BoxliteBox(Sandbox):
return f"Error: {e}"
if r.exit_code not in (0, None):
return f"Error: {(r.stderr or '').strip() or 'cannot read file'}"
return r.stdout or ""
content = r.stdout or ""
if start_line is None and end_line is None:
return content
lines = content.splitlines()
start = start_line or 1
end = end_line if end_line is not None else len(lines)
return "\n".join(lines[start - 1 : end])
def write_file(self, path: str, content: str, append: bool = False) -> None:
self._write_bytes(self._resolve_path(path), content.encode("utf-8"), append=append)

View File

@ -205,13 +205,23 @@ class E2BSandbox(Sandbox):
logger.warning("e2b sandbox ping raised non-fatal error: %s", e)
return True
def read_file(self, path: str) -> str:
def read_file(
self,
path: str,
start_line: int | None = None,
end_line: int | None = None,
) -> str:
resolved = self._resolve_path(path)
try:
content = self._client.files.read(resolved)
if isinstance(content, bytes):
return content.decode("utf-8", errors="replace")
return content if content is not None else ""
if start_line is None and end_line is None:
return content.decode("utf-8", errors="replace") if isinstance(content, bytes) else content or ""
text = content.decode("utf-8", errors="replace") if isinstance(content, bytes) else content or ""
lines = text.splitlines()
start = start_line or 1
end = end_line if end_line is not None else len(lines)
content = "\n".join(lines[start - 1 : end])
return content
except Exception as e:
logger.error("Failed to read file %s in e2b sandbox: %s", resolved, e)
return f"Error: {e}"

View File

@ -678,11 +678,29 @@ class LocalSandbox(Sandbox):
return sorted(result)
def read_file(self, path: str) -> str:
def read_file(
self,
path: str,
start_line: int | None = None,
end_line: int | None = None,
) -> str:
resolved_path = self._resolve_path(path)
should_slice = start_line is not None or end_line is not None
try:
with open(resolved_path, encoding="utf-8") as f:
content = f.read()
if not should_slice:
content = f.read()
start = max(start_line or 1, 1)
if should_slice:
selected: list[str] = []
for line_number, line in enumerate(f, start=1):
if line_number < start:
continue
if end_line is not None and line_number > end_line:
break
selected.append(line.rstrip("\r\n"))
content = "\n".join(selected)
# Only reverse-resolve paths in files that were previously written
# by write_file (agent-authored content). User-uploaded files,
# external tool output, and other non-agent content should not be

View File

@ -91,11 +91,18 @@ class Sandbox(ABC):
pass
@abstractmethod
def read_file(self, path: str) -> str:
def read_file(
self,
path: str,
start_line: int | None = None,
end_line: int | None = None,
) -> str:
"""Read the content of a file.
Args:
path: The absolute path of the file to read.
start_line: Optional starting line number (1-indexed, inclusive).
end_line: Optional ending line number (1-indexed, inclusive).
Returns:
The content of the file.

View File

@ -2138,29 +2138,44 @@ def read_file_tool(
Args:
description: Explain why you are reading this file in short words. ALWAYS PROVIDE THIS PARAMETER FIRST.
path: The **absolute** path to the file to read.
start_line: Optional starting line number (1-indexed, inclusive). Use with end_line to read a specific range.
end_line: Optional ending line number (1-indexed, inclusive). Use with start_line to read a specific range.
start_line: Optional starting line number (1-indexed, inclusive). Omit to start at the first line.
end_line: Optional ending line number (1-indexed, inclusive). Omit to read through the last line.
"""
try:
# Block access to disabled skill files
if _is_disabled_skill_path(path, user_id=resolve_runtime_user_id(runtime)):
skill_name = _extract_skill_name_from_skills_path(path) or "unknown"
return f"Error: Skill '{skill_name}' is disabled. Access to its files is blocked. Enable the skill in settings before using it."
if start_line is not None and start_line < 1:
return "(start_line must be >= 1)"
effective_start = start_line or 1
if end_line is not None and end_line < 1:
return "(end_line must be >= 1)"
if end_line is not None and effective_start > end_line:
return "(start_line > end_line — no lines in range)"
requested_path = path
content = read_current_file_content(runtime, path)
sandbox = ensure_sandbox_initialized(runtime)
ensure_thread_directories_exist(runtime)
use_line_range = start_line is not None or end_line is not None
if use_line_range:
if is_local_sandbox(runtime):
thread_data = get_thread_data(runtime)
validate_local_tool_path(path, thread_data, read_only=True)
if _is_skills_path(path):
path = _resolve_skills_path(path)
elif _is_acp_workspace_path(path):
path = _resolve_acp_workspace_path(path, _extract_thread_id_from_thread_data(thread_data))
elif not _is_custom_mount_path(path):
path = _resolve_and_validate_user_data_path(path, thread_data)
# Custom mount paths are resolved by LocalSandbox._resolve_path()
content = sandbox.read_file(path, start_line=start_line, end_line=end_line)
else:
content = read_current_file_content(runtime, path)
if not content:
return "(empty)"
if start_line is not None or end_line is not None:
lines = content.splitlines()
s = max(start_line, 1) if start_line is not None else 1
e = end_line if end_line is not None else len(lines)
if e < 1:
return "(end_line must be >= 1)"
if s > len(lines):
if start_line is not None and start_line > 1:
return "(start_line exceeds file length)"
if s > e:
return "(start_line > end_line — no lines in range)"
content = "\n".join(lines[s - 1 : e])
return "(empty)"
try:
from deerflow.config.app_config import get_app_config

View File

@ -358,6 +358,20 @@ class TestNoChangeTimeout:
assert calls[0].get("no_change_timeout") == sandbox._DEFAULT_NO_CHANGE_TIMEOUT
class TestReadFile:
def test_read_file_forwards_requested_line_range(self, sandbox):
sandbox._client.file.read_file = MagicMock(return_value=SimpleNamespace(data=SimpleNamespace(content="line 1\nline 2")))
result = sandbox.read_file("/mnt/user-data/workspace/huge.log", start_line=1, end_line=10)
assert result == "line 1\nline 2"
sandbox._client.file.read_file.assert_called_once_with(
file="/mnt/user-data/workspace/huge.log",
start_line=0,
end_line=10,
)
class TestConcurrentFileWrites:
"""Verify file write paths do not lose concurrent updates."""

View File

@ -165,6 +165,28 @@ def test_execute_command_forwards_timeout_to_sdk_and_loop_runner() -> None:
assert run_timeouts == [5]
def test_read_file_supports_optional_line_ranges(monkeypatch: pytest.MonkeyPatch) -> None:
calls: list[tuple[str, ...]] = []
box = BoxliteBox("box-id", box=object(), run=_fake_run)
def _fake_exec(*argv: str):
calls.append(argv)
return types.SimpleNamespace(
exit_code=0,
stdout="line 1\nline 2\nline 3\nline 4\nline 5",
stderr="",
)
monkeypatch.setattr(box, "_exec", _fake_exec)
assert box.read_file("/mnt/user-data/workspace/range.txt") == "line 1\nline 2\nline 3\nline 4\nline 5"
assert box.read_file("/mnt/user-data/workspace/range.txt", start_line=2, end_line=4) == "line 2\nline 3\nline 4"
assert box.read_file("/mnt/user-data/workspace/range.txt", start_line=4) == "line 4\nline 5"
assert box.read_file("/mnt/user-data/workspace/range.txt", end_line=2) == "line 1\nline 2"
assert all(call == ("cat", "--", "/mnt/user-data/workspace/range.txt") for call in calls)
def test_grep_always_prints_filename_for_single_file_paths() -> None:
fake = _FakeBox(name="box-id")
box = BoxliteBox("box-id", box=fake, run=_fake_run)

View File

@ -1739,6 +1739,29 @@ def test_sync_outputs_to_host_is_noop_when_client_closed():
p._sync_outputs_to_host(sb, thread_id="t1", user_id="u1")
def test_read_file_supports_bounded_ranges():
files = FakeFilesAPI(
store={"/home/user/workspace/range.txt": b"line 1\nline 2\nline 3\nline 4\nline 5"},
)
client = FakeClient(files=files)
sb = _make_sandbox(client, sandbox_id="sb-read-range")
assert sb.read_file("/mnt/user-data/workspace/range.txt") == "line 1\nline 2\nline 3\nline 4\nline 5"
assert sb.read_file("/mnt/user-data/workspace/range.txt", start_line=2, end_line=4) == "line 2\nline 3\nline 4"
assert sb.read_file("/mnt/user-data/workspace/range.txt", start_line=4) == "line 4\nline 5"
assert sb.read_file("/mnt/user-data/workspace/range.txt", end_line=2) == "line 1\nline 2"
resolved_path = "/home/user/workspace/range.txt"
assert all(path == resolved_path for path, _fmt in files.read_calls), files.read_calls
def test_read_file_returns_error_for_missing_file():
client = FakeClient(files=FakeFilesAPI())
sb = _make_sandbox(client, sandbox_id="sb-read-missing")
assert sb.read_file("/mnt/user-data/workspace/missing.txt").startswith("Error:")
def _outputs_dir(tmp_path):
return Paths(base_dir=tmp_path).thread_dir("t1", user_id="u1") / "user-data" / "outputs"

View File

@ -797,6 +797,77 @@ class TestLocalSandboxProviderMounts:
# The container path should be preserved through roundtrip
assert "/mnt/data/config.json" in result
def test_read_file_line_range_streams_without_full_read(self, tmp_path):
"""Bounded line reads should stream without slurping the whole file."""
data_dir = tmp_path / "data"
data_dir.mkdir()
big_file = data_dir / "huge.log"
big_file.write_text("\n".join(f"line {i}" for i in range(1, 2000)), encoding="utf-8")
sandbox = LocalSandbox(
"test",
[
PathMapping(container_path="/mnt/data", local_path=str(data_dir)),
],
)
class GuardedFile:
def __init__(self, wrapped):
self._wrapped = wrapped
def __enter__(self):
self._wrapped.__enter__()
return self
def __exit__(self, exc_type, exc, tb):
return self._wrapped.__exit__(exc_type, exc, tb)
def __iter__(self):
return self
def __next__(self):
return next(self._wrapped)
def read(self, *args, **kwargs):
raise AssertionError("full read() should not be used for ranged reads")
def __getattr__(self, name):
return getattr(self._wrapped, name)
import builtins
real_open = builtins.open
def guarded_open(file, *args, **kwargs):
handle = real_open(file, *args, **kwargs)
if Path(file) == big_file:
return GuardedFile(handle)
return handle
with patch("builtins.open", side_effect=guarded_open):
content = sandbox.read_file("/mnt/data/huge.log", start_line=1, end_line=10)
assert content == "\n".join(f"line {i}" for i in range(1, 11))
def test_read_file_single_sided_line_ranges_supported(self, tmp_path):
"""LocalSandbox should support partial reads when only one bound is provided."""
data_dir = tmp_path / "data"
data_dir.mkdir()
(data_dir / "range.txt").write_text(
"\n".join(f"line {i}" for i in range(1, 11)),
encoding="utf-8",
)
sandbox = LocalSandbox(
"test",
[
PathMapping(container_path="/mnt/data", local_path=str(data_dir)),
],
)
assert sandbox.read_file("/mnt/data/range.txt", start_line=8) == "line 8\nline 9\nline 10"
assert sandbox.read_file("/mnt/data/range.txt", end_line=3) == "line 1\nline 2\nline 3"
def test_setup_path_mappings_normalizes_container_path_trailing_slash(self, tmp_path):
skills_dir = tmp_path / "skills"
skills_dir.mkdir()

View File

@ -3,9 +3,9 @@
Previously ``read_file`` only sliced when BOTH ``start_line`` and ``end_line``
were supplied; a lone ``start_line`` (or lone ``end_line``) was silently ignored
and the whole file was returned. These tests pin the one-sided range contract:
tail-from-start, head-to-end, clamping of ``start_line=0``, and clean error
strings for an inverted range or a start beyond EOF (instead of an empty/garbage
slice).
tail-from-start, head-to-end, rejection of non-positive line numbers, and clean
error strings for an inverted range or a start beyond EOF (instead of an
empty/garbage slice).
"""
from pathlib import Path
@ -54,9 +54,16 @@ def test_only_end_line_returns_head_up_to_that_line(tmp_path, monkeypatch) -> No
assert result == "line1\nline2"
def test_start_line_zero_is_clamped_to_first_line(tmp_path, monkeypatch) -> None:
def test_start_line_zero_returns_clean_error(tmp_path, monkeypatch) -> None:
result = _read(tmp_path, monkeypatch, start_line=0)
assert result == _FIVE_LINES
assert "start_line must be >= 1" in result
assert "line1" not in result
def test_start_line_negative_returns_clean_error(tmp_path, monkeypatch) -> None:
result = _read(tmp_path, monkeypatch, start_line=-1)
assert "start_line must be >= 1" in result
assert "line5" not in result
def test_start_line_greater_than_end_line_returns_clean_error(tmp_path, monkeypatch) -> None:

View File

@ -63,3 +63,83 @@ def test_read_file_tool_text_file_unaffected(tmp_path, monkeypatch) -> None:
assert "hello 你好" in result, result
assert "binary" not in result.lower(), result
def test_read_file_tool_passes_line_range_into_sandbox(monkeypatch) -> None:
captured: dict[str, int | str | None] = {}
class RangeAwareSandbox:
def read_file(
self,
path: str,
start_line: int | None = None,
end_line: int | None = None,
) -> str:
captured["path"] = path
captured["start_line"] = start_line
captured["end_line"] = end_line
return "line 1\nline 2"
runtime = SimpleNamespace(state={"sandbox": {"sandbox_id": "aio:test"}}, context={"thread_id": "t1"})
monkeypatch.setattr("deerflow.sandbox.tools.ensure_sandbox_initialized", lambda runtime: RangeAwareSandbox())
monkeypatch.setattr("deerflow.sandbox.tools.ensure_thread_directories_exist", lambda runtime: None)
result = read_file_tool.func(
runtime=runtime,
description="read top of file",
path="/mnt/user-data/workspace/huge.log",
start_line=1,
end_line=10,
)
assert result == "line 1\nline 2"
assert captured == {
"path": "/mnt/user-data/workspace/huge.log",
"start_line": 1,
"end_line": 10,
}
def test_read_file_tool_passes_open_ended_ranges_into_sandbox(monkeypatch) -> None:
calls: list[tuple[int | None, int | None]] = []
class RangeAwareSandbox:
def read_file(self, path: str, start_line: int | None = None, end_line: int | None = None) -> str:
calls.append((start_line, end_line))
return "line 1\nline 2"
runtime = SimpleNamespace(state={"sandbox": {"sandbox_id": "aio:test"}}, context={"thread_id": "t1"})
monkeypatch.setattr("deerflow.sandbox.tools.ensure_sandbox_initialized", lambda runtime: RangeAwareSandbox())
monkeypatch.setattr("deerflow.sandbox.tools.ensure_thread_directories_exist", lambda runtime: None)
start_only = read_file_tool.func(
runtime=runtime,
description="read tail",
path="/mnt/user-data/workspace/huge.log",
start_line=1,
)
end_only = read_file_tool.func(
runtime=runtime,
description="read head",
path="/mnt/user-data/workspace/huge.log",
end_line=10,
)
assert start_only == "line 1\nline 2"
assert end_only == "line 1\nline 2"
assert calls == [(1, None), (None, 10)]
def test_read_file_tool_validates_range_order(monkeypatch) -> None:
runtime = SimpleNamespace(state={"sandbox": {"sandbox_id": "aio:test"}}, context={"thread_id": "t1"})
monkeypatch.setattr("deerflow.sandbox.tools.ensure_sandbox_initialized", lambda runtime: object())
result = read_file_tool.func(
runtime=runtime,
description="bad order",
path="/mnt/user-data/workspace/huge.log",
start_line=20,
end_line=10,
)
assert "start_line > end_line" in result

View File

@ -46,7 +46,12 @@ class _SandboxStub(Sandbox):
del env, timeout
return "OK"
def read_file(self, path: str) -> str:
def read_file(
self,
path: str,
start_line: int | None = None,
end_line: int | None = None,
) -> str:
return "content"
def download_file(self, path: str) -> bytes:

View File

@ -1245,7 +1245,12 @@ def test_str_replace_parallel_updates_should_preserve_both_edits(monkeypatch) ->
self._state_lock = threading.Lock()
self._overlap_detected = threading.Event()
def read_file(self, path: str) -> str:
def read_file(
self,
path: str,
start_line: int | None = None,
end_line: int | None = None,
) -> str:
with self._state_lock:
self._active_reads += 1
snapshot = self.content
@ -1308,7 +1313,12 @@ def test_str_replace_parallel_updates_in_isolated_sandboxes_should_not_share_pat
self.content = "alpha\nbeta\n"
self._shared_state = shared_state
def read_file(self, path: str) -> str:
def read_file(
self,
path: str,
start_line: int | None = None,
end_line: int | None = None,
) -> str:
state_lock = self._shared_state["state_lock"]
with state_lock:
active_reads = self._shared_state["active_reads"]
@ -1390,7 +1400,12 @@ def test_str_replace_and_append_on_same_path_should_preserve_both_updates(monkey
self.str_replace_has_snapshot = threading.Event()
self.append_finished = threading.Event()
def read_file(self, path: str) -> str:
def read_file(
self,
path: str,
start_line: int | None = None,
end_line: int | None = None,
) -> str:
with self.state_lock:
snapshot = self.content
self.str_replace_has_snapshot.set()