mirror of
https://github.com/bytedance/deer-flow.git
synced 2026-05-24 00:45:57 +00:00
fix(runtime): bound write_file execution-failure observations (#3133)
* fix(runtime): bound write_file execution-failure observations * fix(runtime): preserve write_file error prefixes * test(runtime): trim write_file prefix assertions * refactor(runtime): drop redundant exception suffix for permission/directory write errors Address Copilot review on #3133: the PermissionError and IsADirectoryError branches now return self-contained, non-redundant messages (e.g. "Error: Permission denied writing to file: /mnt/...") via direct truncation, instead of going through _format_write_file_error which appended a duplicate ": PermissionError: permission denied" suffix. OSError, SandboxError and the generic Exception branches keep the unified "Failed to write file '{path}': {ExceptionType}: {detail}" format so the model still sees a stable, machine-readable error class. Removes the now-unused message= parameter from _format_write_file_error, keeping a single code path. Truncation contract (<= 2000 chars) and host-path sanitization unchanged. * fix(runtime): handle write_file sandbox init errors Initialize the requested path before sandbox setup so early sandbox failures can still return a bounded write_file error. Add a regression test for sandbox initialization failures. * style(test): format sandbox security tests
This commit is contained in:
@@ -42,6 +42,7 @@ _DEFAULT_GLOB_MAX_RESULTS = 200
|
|||||||
_MAX_GLOB_MAX_RESULTS = 1000
|
_MAX_GLOB_MAX_RESULTS = 1000
|
||||||
_DEFAULT_GREP_MAX_RESULTS = 100
|
_DEFAULT_GREP_MAX_RESULTS = 100
|
||||||
_MAX_GREP_MAX_RESULTS = 500
|
_MAX_GREP_MAX_RESULTS = 500
|
||||||
|
_DEFAULT_WRITE_FILE_ERROR_MAX_CHARS = 2000
|
||||||
_LOCAL_BASH_CWD_COMMANDS = {"cd", "pushd"}
|
_LOCAL_BASH_CWD_COMMANDS = {"cd", "pushd"}
|
||||||
_LOCAL_BASH_COMMAND_WRAPPERS = {"command", "builtin"}
|
_LOCAL_BASH_COMMAND_WRAPPERS = {"command", "builtin"}
|
||||||
_LOCAL_BASH_COMMAND_PREFIX_KEYWORDS = {"!", "{", "case", "do", "elif", "else", "for", "if", "select", "then", "time", "until", "while"}
|
_LOCAL_BASH_COMMAND_PREFIX_KEYWORDS = {"!", "{", "case", "do", "elif", "else", "for", "if", "select", "then", "time", "until", "while"}
|
||||||
@@ -435,6 +436,42 @@ def _sanitize_error(error: Exception, runtime: Runtime | None = None) -> str:
|
|||||||
return msg
|
return msg
|
||||||
|
|
||||||
|
|
||||||
|
def _truncate_write_file_error_detail(detail: str, max_chars: int) -> str:
|
||||||
|
"""Middle-truncate write_file error details, preserving the head and tail."""
|
||||||
|
if max_chars == 0:
|
||||||
|
return detail
|
||||||
|
if len(detail) <= max_chars:
|
||||||
|
return detail
|
||||||
|
total = len(detail)
|
||||||
|
marker_max_len = len(f"\n... [write_file error truncated: {total} chars skipped] ...\n")
|
||||||
|
kept = max(0, max_chars - marker_max_len)
|
||||||
|
if kept == 0:
|
||||||
|
return detail[:max_chars]
|
||||||
|
head_len = kept // 2
|
||||||
|
tail_len = kept - head_len
|
||||||
|
skipped = total - kept
|
||||||
|
marker = f"\n... [write_file error truncated: {skipped} chars skipped] ...\n"
|
||||||
|
return f"{detail[:head_len]}{marker}{detail[-tail_len:] if tail_len > 0 else ''}"
|
||||||
|
|
||||||
|
|
||||||
|
def _format_write_file_error(
|
||||||
|
requested_path: str,
|
||||||
|
error: Exception,
|
||||||
|
runtime: Runtime | None = None,
|
||||||
|
*,
|
||||||
|
max_chars: int = _DEFAULT_WRITE_FILE_ERROR_MAX_CHARS,
|
||||||
|
) -> str:
|
||||||
|
"""Return a bounded, sanitized error string for write_file failures."""
|
||||||
|
header = f"Error: Failed to write file '{requested_path}'"
|
||||||
|
detail = _sanitize_error(error, runtime)
|
||||||
|
if max_chars == 0:
|
||||||
|
return f"{header}: {detail}"
|
||||||
|
detail_budget = max_chars - len(header) - 2
|
||||||
|
if detail_budget <= 0:
|
||||||
|
return _truncate_write_file_error_detail(f"{header}: {detail}", max_chars)
|
||||||
|
return f"{header}: {_truncate_write_file_error_detail(detail, detail_budget)}"
|
||||||
|
|
||||||
|
|
||||||
def replace_virtual_path(path: str, thread_data: ThreadDataState | None) -> str:
|
def replace_virtual_path(path: str, thread_data: ThreadDataState | None) -> str:
|
||||||
"""Replace virtual /mnt/user-data paths with actual thread data paths.
|
"""Replace virtual /mnt/user-data paths with actual thread data paths.
|
||||||
|
|
||||||
@@ -1651,9 +1688,9 @@ def write_file_tool(
|
|||||||
append: Whether to append content to the end of the file instead of overwriting it. Defaults to false.
|
append: Whether to append content to the end of the file instead of overwriting it. Defaults to false.
|
||||||
"""
|
"""
|
||||||
try:
|
try:
|
||||||
|
requested_path = path
|
||||||
sandbox = ensure_sandbox_initialized(runtime)
|
sandbox = ensure_sandbox_initialized(runtime)
|
||||||
ensure_thread_directories_exist(runtime)
|
ensure_thread_directories_exist(runtime)
|
||||||
requested_path = path
|
|
||||||
if is_local_sandbox(runtime):
|
if is_local_sandbox(runtime):
|
||||||
thread_data = get_thread_data(runtime)
|
thread_data = get_thread_data(runtime)
|
||||||
validate_local_tool_path(path, thread_data)
|
validate_local_tool_path(path, thread_data)
|
||||||
@@ -1664,15 +1701,21 @@ def write_file_tool(
|
|||||||
sandbox.write_file(path, content, append)
|
sandbox.write_file(path, content, append)
|
||||||
return "OK"
|
return "OK"
|
||||||
except SandboxError as e:
|
except SandboxError as e:
|
||||||
return f"Error: {e}"
|
return _format_write_file_error(requested_path, e, runtime)
|
||||||
except PermissionError:
|
except PermissionError:
|
||||||
return f"Error: Permission denied writing to file: {requested_path}"
|
return _truncate_write_file_error_detail(
|
||||||
|
f"Error: Permission denied writing to file: {requested_path}",
|
||||||
|
_DEFAULT_WRITE_FILE_ERROR_MAX_CHARS,
|
||||||
|
)
|
||||||
except IsADirectoryError:
|
except IsADirectoryError:
|
||||||
return f"Error: Path is a directory, not a file: {requested_path}"
|
return _truncate_write_file_error_detail(
|
||||||
|
f"Error: Path is a directory, not a file: {requested_path}",
|
||||||
|
_DEFAULT_WRITE_FILE_ERROR_MAX_CHARS,
|
||||||
|
)
|
||||||
except OSError as e:
|
except OSError as e:
|
||||||
return f"Error: Failed to write file '{requested_path}': {_sanitize_error(e, runtime)}"
|
return _format_write_file_error(requested_path, e, runtime)
|
||||||
except Exception as e:
|
except Exception as e:
|
||||||
return f"Error: Unexpected error writing file: {_sanitize_error(e, runtime)}"
|
return _format_write_file_error(requested_path, e, runtime)
|
||||||
|
|
||||||
|
|
||||||
async def _write_file_tool_async(
|
async def _write_file_tool_async(
|
||||||
|
|||||||
@@ -5,6 +5,7 @@ from unittest.mock import patch
|
|||||||
|
|
||||||
import pytest
|
import pytest
|
||||||
|
|
||||||
|
from deerflow.sandbox.exceptions import SandboxError
|
||||||
from deerflow.sandbox.tools import (
|
from deerflow.sandbox.tools import (
|
||||||
VIRTUAL_PATH_PREFIX,
|
VIRTUAL_PATH_PREFIX,
|
||||||
_apply_cwd_prefix,
|
_apply_cwd_prefix,
|
||||||
@@ -1140,6 +1141,170 @@ def test_str_replace_and_append_on_same_path_should_preserve_both_updates(monkey
|
|||||||
assert sandbox.content == "ALPHA\ntail\n"
|
assert sandbox.content == "ALPHA\ntail\n"
|
||||||
|
|
||||||
|
|
||||||
|
def test_write_file_tool_bounds_large_oserror_and_masks_local_paths(monkeypatch) -> None:
|
||||||
|
class FailingSandbox:
|
||||||
|
id = "sandbox-write-large-oserror"
|
||||||
|
|
||||||
|
def write_file(self, path: str, content: str, append: bool = False) -> None:
|
||||||
|
host_path = f"{_THREAD_DATA['workspace_path']}/nested/output.txt"
|
||||||
|
raise OSError(f"write failed at {host_path}\n{'A' * 12000}\nremote tail marker")
|
||||||
|
|
||||||
|
runtime = SimpleNamespace(state={}, context={"thread_id": "thread-1"}, config={})
|
||||||
|
sandbox = FailingSandbox()
|
||||||
|
|
||||||
|
monkeypatch.setattr("deerflow.sandbox.tools.ensure_sandbox_initialized", lambda runtime: sandbox)
|
||||||
|
monkeypatch.setattr("deerflow.sandbox.tools.ensure_thread_directories_exist", lambda runtime: None)
|
||||||
|
monkeypatch.setattr("deerflow.sandbox.tools.is_local_sandbox", lambda runtime: True)
|
||||||
|
monkeypatch.setattr("deerflow.sandbox.tools.get_thread_data", lambda runtime: _THREAD_DATA)
|
||||||
|
monkeypatch.setattr("deerflow.sandbox.tools.validate_local_tool_path", lambda path, thread_data: None)
|
||||||
|
monkeypatch.setattr(
|
||||||
|
"deerflow.sandbox.tools._resolve_and_validate_user_data_path",
|
||||||
|
lambda path, thread_data: f"{_THREAD_DATA['workspace_path']}/output.txt",
|
||||||
|
)
|
||||||
|
|
||||||
|
result = write_file_tool.func(
|
||||||
|
runtime=runtime,
|
||||||
|
description="写入大文件失败",
|
||||||
|
path="/mnt/user-data/workspace/output.txt",
|
||||||
|
content="report body",
|
||||||
|
)
|
||||||
|
|
||||||
|
assert len(result) <= 2000
|
||||||
|
assert "Error: Failed to write file '/mnt/user-data/workspace/output.txt':" in result
|
||||||
|
assert "/tmp/deer-flow/threads/t1/user-data/workspace" not in result
|
||||||
|
assert "/mnt/user-data/workspace/nested/output.txt" in result
|
||||||
|
assert "remote tail marker" in result
|
||||||
|
assert "[write_file error truncated:" in result
|
||||||
|
|
||||||
|
|
||||||
|
def test_write_file_tool_preserves_short_oserror_without_truncation(monkeypatch) -> None:
|
||||||
|
class FailingSandbox:
|
||||||
|
id = "sandbox-write-short-oserror"
|
||||||
|
|
||||||
|
def write_file(self, path: str, content: str, append: bool = False) -> None:
|
||||||
|
raise OSError("disk quota exceeded")
|
||||||
|
|
||||||
|
runtime = SimpleNamespace(state={}, context={"thread_id": "thread-1"}, config={})
|
||||||
|
sandbox = FailingSandbox()
|
||||||
|
|
||||||
|
monkeypatch.setattr("deerflow.sandbox.tools.ensure_sandbox_initialized", lambda runtime: sandbox)
|
||||||
|
monkeypatch.setattr("deerflow.sandbox.tools.ensure_thread_directories_exist", lambda runtime: None)
|
||||||
|
monkeypatch.setattr("deerflow.sandbox.tools.is_local_sandbox", lambda runtime: False)
|
||||||
|
|
||||||
|
result = write_file_tool.func(
|
||||||
|
runtime=runtime,
|
||||||
|
description="写入失败",
|
||||||
|
path="/mnt/user-data/workspace/output.txt",
|
||||||
|
content="tiny payload",
|
||||||
|
)
|
||||||
|
|
||||||
|
assert result == "Error: Failed to write file '/mnt/user-data/workspace/output.txt': OSError: disk quota exceeded"
|
||||||
|
assert "[write_file error truncated:" not in result
|
||||||
|
|
||||||
|
|
||||||
|
def test_write_file_tool_bounds_large_sandbox_error(monkeypatch) -> None:
|
||||||
|
class FailingSandbox:
|
||||||
|
id = "sandbox-write-large-sandbox-error"
|
||||||
|
|
||||||
|
def write_file(self, path: str, content: str, append: bool = False) -> None:
|
||||||
|
raise SandboxError(f"remote write rejected {'B' * 12000} final detail")
|
||||||
|
|
||||||
|
runtime = SimpleNamespace(state={}, context={"thread_id": "thread-1"}, config={})
|
||||||
|
sandbox = FailingSandbox()
|
||||||
|
|
||||||
|
monkeypatch.setattr("deerflow.sandbox.tools.ensure_sandbox_initialized", lambda runtime: sandbox)
|
||||||
|
monkeypatch.setattr("deerflow.sandbox.tools.ensure_thread_directories_exist", lambda runtime: None)
|
||||||
|
monkeypatch.setattr("deerflow.sandbox.tools.is_local_sandbox", lambda runtime: False)
|
||||||
|
|
||||||
|
result = write_file_tool.func(
|
||||||
|
runtime=runtime,
|
||||||
|
description="远端写入失败",
|
||||||
|
path="/mnt/user-data/workspace/output.txt",
|
||||||
|
content="tiny payload",
|
||||||
|
)
|
||||||
|
|
||||||
|
assert len(result) <= 2000
|
||||||
|
assert "Error: Failed to write file '/mnt/user-data/workspace/output.txt':" in result
|
||||||
|
assert "SandboxError: remote write rejected" in result
|
||||||
|
assert "final detail" in result
|
||||||
|
assert "[write_file error truncated:" in result
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.parametrize(
|
||||||
|
("raised_error", "expected_fragment"),
|
||||||
|
[
|
||||||
|
pytest.param(
|
||||||
|
PermissionError("permission denied"),
|
||||||
|
"Error: Permission denied writing to file: /mnt/user-data/workspace/output.txt",
|
||||||
|
id="permission",
|
||||||
|
),
|
||||||
|
pytest.param(
|
||||||
|
IsADirectoryError("target is a directory"),
|
||||||
|
"Error: Path is a directory, not a file: /mnt/user-data/workspace/output.txt",
|
||||||
|
id="directory",
|
||||||
|
),
|
||||||
|
pytest.param(
|
||||||
|
Exception("remote sandbox timeout"),
|
||||||
|
"Exception: remote sandbox timeout",
|
||||||
|
id="generic",
|
||||||
|
),
|
||||||
|
],
|
||||||
|
)
|
||||||
|
def test_write_file_tool_formats_all_other_failure_branches(
|
||||||
|
monkeypatch,
|
||||||
|
raised_error: Exception,
|
||||||
|
expected_fragment: str,
|
||||||
|
) -> None:
|
||||||
|
class FailingSandbox:
|
||||||
|
id = "sandbox-write-other-failure"
|
||||||
|
|
||||||
|
def write_file(self, path: str, content: str, append: bool = False) -> None:
|
||||||
|
raise raised_error
|
||||||
|
|
||||||
|
runtime = SimpleNamespace(state={}, context={"thread_id": "thread-1"}, config={})
|
||||||
|
sandbox = FailingSandbox()
|
||||||
|
|
||||||
|
monkeypatch.setattr("deerflow.sandbox.tools.ensure_sandbox_initialized", lambda runtime: sandbox)
|
||||||
|
monkeypatch.setattr("deerflow.sandbox.tools.ensure_thread_directories_exist", lambda runtime: None)
|
||||||
|
monkeypatch.setattr("deerflow.sandbox.tools.is_local_sandbox", lambda runtime: False)
|
||||||
|
|
||||||
|
result = write_file_tool.func(
|
||||||
|
runtime=runtime,
|
||||||
|
description="验证错误分支格式化",
|
||||||
|
path="/mnt/user-data/workspace/output.txt",
|
||||||
|
content="tiny payload",
|
||||||
|
)
|
||||||
|
|
||||||
|
assert "/mnt/user-data/workspace/output.txt" in result
|
||||||
|
assert expected_fragment in result
|
||||||
|
assert "[write_file error truncated:" not in result
|
||||||
|
|
||||||
|
|
||||||
|
def test_write_file_tool_handles_sandbox_init_failure(monkeypatch) -> None:
|
||||||
|
"""Regression for #3133 review: SandboxError raised during sandbox
|
||||||
|
initialization (before the local `requested_path` assignment) must still
|
||||||
|
surface as a bounded tool error rather than an UnboundLocalError.
|
||||||
|
"""
|
||||||
|
|
||||||
|
def raise_sandbox_error(runtime):
|
||||||
|
raise SandboxError("sandbox missing")
|
||||||
|
|
||||||
|
runtime = SimpleNamespace(state={}, context={"thread_id": "thread-1"}, config={})
|
||||||
|
monkeypatch.setattr("deerflow.sandbox.tools.ensure_sandbox_initialized", raise_sandbox_error)
|
||||||
|
monkeypatch.setattr("deerflow.sandbox.tools.is_local_sandbox", lambda runtime: False)
|
||||||
|
|
||||||
|
result = write_file_tool.func(
|
||||||
|
runtime=runtime,
|
||||||
|
description="sandbox 初始化失败",
|
||||||
|
path="/mnt/user-data/workspace/output.txt",
|
||||||
|
content="tiny payload",
|
||||||
|
)
|
||||||
|
|
||||||
|
assert "Error: Failed to write file '/mnt/user-data/workspace/output.txt':" in result
|
||||||
|
assert "SandboxError: sandbox missing" in result
|
||||||
|
assert "[write_file error truncated:" not in result
|
||||||
|
|
||||||
|
|
||||||
def test_file_operation_lock_memory_cleanup() -> None:
|
def test_file_operation_lock_memory_cleanup() -> None:
|
||||||
"""Verify that released locks are eventually cleaned up by WeakValueDictionary.
|
"""Verify that released locks are eventually cleaned up by WeakValueDictionary.
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user