fix(sandbox): stop E2B append from overwriting on read failure (#5261)

* fix(sandbox): stop E2B append from overwriting on read failure

E2B has no native append, so write_file(append=True) read-modify-writes.
Treat only FileNotFoundException/FileNotFoundError as an empty file; any
other pre-read error must abort so a timeout cannot replace the original
contents with just the new fragment.

* fix(sandbox): distinguish E2B append pre-read refusal in logs

A non-not-found pre-read error now logs as a refused overwrite instead
of a write failure. Tests pin the successful read-modify-write path,
including a bytes pre-image, so dropping `existing` cannot go green.
This commit is contained in:
wutongyuonce 2026-09-08 09:17:53 +08:00 committed by GitHub
parent cbd6621d52
commit e5d23943ce
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 88 additions and 9 deletions

View File

@ -7,6 +7,7 @@ import shlex
import threading import threading
from typing import TYPE_CHECKING from typing import TYPE_CHECKING
from e2b import FileNotFoundException
from e2b_code_interpreter import Sandbox as E2BClientSandbox from e2b_code_interpreter import Sandbox as E2BClientSandbox
from deerflow.config.paths import VIRTUAL_PATH_PREFIX from deerflow.config.paths import VIRTUAL_PATH_PREFIX
@ -357,16 +358,24 @@ class E2BSandbox(Sandbox):
client = self._client client = self._client
if client is None: if client is None:
raise RuntimeError("sandbox client has been closed") raise RuntimeError("sandbox client has been closed")
try:
if append: if append:
existing = "" # E2B has no append write. Read-modify-write must treat only
# explicit not-found as empty; any other read failure would
# otherwise overwrite the original file with just the tail.
try: try:
existing = client.files.read(resolved) or "" existing = client.files.read(resolved) or ""
except (FileNotFoundException, FileNotFoundError):
existing = ""
except Exception:
logger.error(
"Append pre-read failed for %s; refusing to overwrite",
resolved,
)
raise
if isinstance(existing, bytes): if isinstance(existing, bytes):
existing = existing.decode("utf-8", errors="replace") existing = existing.decode("utf-8", errors="replace")
except Exception: content = existing + content
existing = "" try:
content = (existing or "") + content
client.files.write(resolved, content) client.files.write(resolved, content)
except Exception as e: except Exception as e:
logger.error("Failed to write file %s in e2b sandbox: %s", resolved, e) logger.error("Failed to write file %s in e2b sandbox: %s", resolved, e)

View File

@ -18,6 +18,7 @@ from typing import Any
from unittest.mock import MagicMock from unittest.mock import MagicMock
import pytest import pytest
from e2b import FileNotFoundException, TimeoutException
from pydantic import ValidationError from pydantic import ValidationError
from deerflow.community.e2b_sandbox.capacity import ( from deerflow.community.e2b_sandbox.capacity import (
@ -5213,3 +5214,72 @@ def test_glob_preserves_trailing_space_in_filename():
assert matches == ["/home/user/notes.txt "] assert matches == ["/home/user/notes.txt "]
assert truncated is False assert truncated is False
@pytest.mark.parametrize("missing_exc", [FileNotFoundError, FileNotFoundException])
def test_append_creates_file_when_file_does_not_exist(missing_exc):
# Append has no native write mode, so a missing file must still create one
# containing only the new fragment. Both the e2b SDK exception and the
# stdlib one used by FakeFilesAPI / compatible clients count as not-found.
class MissingFilesAPI(FakeFilesAPI):
def read(self, path: str, *, format: str | None = None):
self.read_calls.append((path, format))
raise missing_exc(path)
files = MissingFilesAPI()
sb = _make_sandbox(FakeClient(files=files))
sb.write_file("/mnt/user-data/outputs/report.txt", "conclusion", append=True)
assert files.write_calls == [("/home/user/outputs/report.txt", "conclusion")]
def test_append_does_not_overwrite_when_read_fails(caplog):
# If the pre-read fails for any reason other than not-found, we cannot
# confirm the existing contents. Continuing would write only the tail and
# destroy the original file. Fail closed: raise, and never call write.
existing = b"important report body"
class TimeoutFilesAPI(FakeFilesAPI):
def read(self, path: str, *, format: str | None = None):
self.read_calls.append((path, format))
raise TimeoutException("read timed out")
files = TimeoutFilesAPI(store={"/home/user/outputs/report.txt": existing})
sb = _make_sandbox(FakeClient(files=files))
with caplog.at_level("ERROR"), pytest.raises(TimeoutException, match="read timed out"):
sb.write_file("/mnt/user-data/outputs/report.txt", "conclusion", append=True)
assert files.write_calls == []
assert files.store["/home/user/outputs/report.txt"] == existing
assert "refusing to overwrite" in caplog.text
assert "Failed to write file" not in caplog.text
def test_append_accumulates_existing_content():
# The rewrite exists to keep read-modify-write. If someone later drops
# `existing` and writes only the tail, the not-found / fail-closed tests
# would still pass.
files = FakeFilesAPI(store={"/home/user/outputs/report.txt": b"hello"})
sb = _make_sandbox(FakeClient(files=files))
sb.write_file("/mnt/user-data/outputs/report.txt", " world", append=True)
assert files.write_calls == [("/home/user/outputs/report.txt", "hello world")]
def test_append_decodes_bytes_preimage():
# FakeFilesAPI.read() returns str for valid utf-8. A bytes pre-image is
# what hits the decode branch before concatenation.
class BytesFilesAPI(FakeFilesAPI):
def read(self, path: str, *, format: str | None = None):
self.read_calls.append((path, format))
return self.store[path]
files = BytesFilesAPI(store={"/home/user/outputs/report.txt": b"hello"})
sb = _make_sandbox(FakeClient(files=files))
sb.write_file("/mnt/user-data/outputs/report.txt", " world", append=True)
assert files.write_calls == [("/home/user/outputs/report.txt", "hello world")]