fix(sandbox): reject write_file payloads that are the elision placeholder (#7374)

* fix(sandbox): reject write_file payloads that are the elision placeholder

The tool-output budget middleware replaces a superseded write_file payload
with a deterministic placeholder in the model-bound request only. That
placeholder is text the model sees in its own history, so it can imitate it
and submit it back as real write_file content: #7319 reports a delivered
file that was 194 bytes of placeholder.

write_file now rejects a payload that is the placeholder (whole payload,
allowing only surrounding whitespace) with an actionable error telling the
model to read_file the target path and write the real content; nothing is
written. The match is whole-payload on purpose: a substring guard would
block legitimate edits to this repository, whose constants.py and
persistence docs quote the template verbatim.

The placeholder wording moves to one shared constant,
WRITE_ELISION_PLACEHOLDER_TEMPLATE in deerflow.constants, which the
middleware renders from and the guard matches on, so the two cannot drift.

Regression tests fail on base (7 failed, 6 passed) and pass with the fix;
near-miss payloads that merely quote the placeholder still write.

Refs #7319

* fix(sandbox): match complete write elision placeholders

* docs: clarify write placeholder validation scope

* chore: re-trigger CLA status check

* fix(sandbox): preflight async writes and trim guidance

---------

Co-authored-by: Willem Jiang <willem.jiang@gmail.com>
This commit is contained in:
lester076 2026-10-25 21:15:14 +08:00 committed by GitHub
parent 5b7daf0e1f
commit 9d0509925e
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
7 changed files with 268 additions and 7 deletions

View File

@ -60,7 +60,7 @@ No match: skip inspection, log ID. Tests: `test_read_mark_tool_call_correlation.
1. **InputSanitizationMiddleware** - First, so it is the outermost `wrap_model_call` wrapper; every inner middleware (including LLM retries) sees sanitized messages. `additional_kwargs.original_user_content` is server-owned provenance: Gateway strips caller-supplied values for non-internal run requests, trusted IM calls may carry the string they captured before adding transport/file context, and the middleware replaces any non-string value before wrapping. Uploads and sanitization retain first-writer-wins only for validated strings. Caller markers are marked `untrusted_input`, never stripped; scope is every turn.
**KnowledgeScopeMiddleware** follows input sanitization: it exposes only Gateway-admitted execution scope, removes scope/display data from model messages, and blocks `knowledge_search`, `knowledge_entries_search` and `entity_relations_search` when disabled without reading storage or RAGFlow.
2. **ToolOutputBudgetMiddleware** - Budgets model-bound text/JSON with limits/exemptions; externalizes to thread outputs (`tool_output.storage_subdir`, default `.tool-results`, `TOOL_RESULTS_DIRNAME`) with synopsis/`read_file` refs. Preserve mixed-result media. Restore blob refs across hosts using the recovering Gateway's normal umask; hash/write failures fall back inline. Internal files bypass workspace/delivery scans. Publication/elision/bash exit marker: [contract](TOOL_OUTPUT_PERSISTENCE.md). Elision: `tool_call_args`; retain newest `keep_recent_writes` writes. Controls: `elide_superseded_writes`, `superseded_write_min_chars`.
2. **ToolOutputBudgetMiddleware** - Budgets model-bound text/JSON with limits/exemptions; externalizes to thread outputs (`tool_output.storage_subdir`, default `.tool-results`, `TOOL_RESULTS_DIRNAME`) with synopsis/`read_file` refs. Preserve mixed-result media. Restore blob refs across hosts using the recovering Gateway's normal umask; hash/write failures fall back inline. Internal files bypass workspace/delivery scans. Publication/elision/bash exit marker: [contract](TOOL_OUTPUT_PERSISTENCE.md). Elision: `tool_call_args`; retain newest `keep_recent_writes` writes. The placeholder comes from `WRITE_ELISION_PLACEHOLDER_TEMPLATE` (`constants.py`) and `write_file` rejects a payload that *is* that placeholder (whole payload, up to surrounding whitespace), so an imitated placeholder cannot become a deliverable (#7319). Controls: `elide_superseded_writes`, `superseded_write_min_chars`.
Tests: `tests/test_tool_output_synopsis_budget.py`.
3. **ToolResultSanitizationMiddleware** - Neutralizes framework/injection tags (e.g. `<system-reminder>`) and boundary markers in *remote-content* tool results (`web_fetch`/`web_search`/`image_search`/`web_capture`) so attacker-controlled fetched pages cannot forge trusted framework context. Mirrors `InputSanitizationMiddleware`'s user-input guardrail for the other untrusted-content entry point; sits inner of `ToolOutputBudgetMiddleware`. Local tool output (bash/read_file) is left untouched. Scope is a name-based allowlist for the first-party web tools, plus every MCP-sourced tool via its `deerflow_mcp` metadata tag, so an MCP server naming its fetcher `fetch_url` is still covered

View File

@ -84,3 +84,21 @@ after a later successful same-path `read_file`, `write_file`, or `str_replace`
writes. Use the shared `tool_call_args` helpers (`pair_tool_call_results` and
argument rewriting). Controls: `elide_superseded_writes` and
`superseded_write_min_chars`.
The placeholder is framework-owned text that the model can see in its own
history, so it can be imitated. `write_file` therefore rejects a payload that
*is* the placeholder rendered from `WRITE_ELISION_PLACEHOLDER_TEMPLATE`
(`constants.py`), allowing only surrounding whitespace, instead of writing a
deliverable that is ~200 bytes of placeholder (#7319). The match is
whole-payload on purpose: a substring guard would block legitimate edits to
this repository, whose `constants.py` and docs quote the template verbatim.
Rendering and rejection derive from that one constant so they cannot drift.
The character count must be an ASCII non-negative integer. A document beginning
with the same prefix, including a quoted placeholder followed by more content,
is allowed.
Both sync and async `write_file` calls reject the placeholder before sandbox
initialization, including when the sandbox provider is unavailable.
This rejection applies only to `write_file`; `str_replace` does not validate
`new_str` against the placeholder template.

View File

@ -52,7 +52,7 @@ from deerflow.agents.middlewares.tool_transform_meta import append_tool_transfor
from deerflow.community.ragflow.sources import budget_source_artifact
from deerflow.config.summarization_config import DEFAULT_SKILL_FILE_READ_TOOL_NAMES
from deerflow.config.tool_output_config import ToolOutputConfig
from deerflow.constants import DEFAULT_SKILLS_CONTAINER_PATH
from deerflow.constants import DEFAULT_SKILLS_CONTAINER_PATH, WRITE_ELISION_PLACEHOLDER_TEMPLATE
from deerflow.sandbox.lease import run_sync_lifecycle_operation
from deerflow.sandbox.sandbox_provider import get_sandbox_provider
from deerflow.storage import BlobRef, BlobStoreError, get_blob_store_if_enabled
@ -1057,8 +1057,9 @@ _SUPERSEDING_READ_STATUSES = frozenset({"success", "partial_success"})
# Deterministic for a given payload so repeated model calls keep the same
# request prefix (prompt caching) instead of drifting. Framework-owned static
# text plus a character count; no model-supplied value is interpolated (the
# path stays visible in the call's own ``path`` argument).
_ELIDED_WRITE_CONTENT_TEMPLATE = "[content elided: {chars} chars; this write_file call succeeded and the file was read or modified again afterwards, so the on-disk file is the current version; call read_file on its path to see it]"
# path stays visible in the call's own ``path`` argument). The template is
# shared with ``write_file``, which refuses to write it back out (#7319).
_ELIDED_WRITE_CONTENT_TEMPLATE = WRITE_ELISION_PLACEHOLDER_TEMPLATE
def elide_superseded_write_payloads(messages: list[Any], *, min_chars: int, keep_recent: int) -> list[Any] | None:

View File

@ -2,6 +2,15 @@
DEFAULT_SKILLS_CONTAINER_PATH = "/mnt/skills"
# Framework-owned placeholder the tool-output budget middleware substitutes for
# an elided ``write_file`` payload in the model-bound request (issue #5328), and
# which ``write_file`` refuses to write back out (issue #7319). The model
# occasionally imitates its own elided history and submits the placeholder as
# content, leaving a ~200-byte deliverable; a rejection the model can act on is
# better than a silent empty file. Rendering and rejection both derive from this
# one string so the wording cannot drift out of sync with the guard.
WRITE_ELISION_PLACEHOLDER_TEMPLATE = "[content elided: {chars} chars; this write_file call succeeded and the file was read or modified again afterwards, so the on-disk file is the current version; call read_file on its path to see it]"
# Host-only per-run capability. Keep this dependency-free: the runtime worker
# must not initialize the tool/subagent packages while importing this key.
CONVERSATION_READER_CONTEXT_KEY = "__conversation_reader"

View File

@ -5,7 +5,7 @@ Sandbox restore requires a thread ID, even forks.
On startup failure, close reads and kill/reap the child, preserving the error.
Only `finally` closes write ends.
On Linux/macOS, local non-append text saves use `_publish_file_revision` and the shared atomic byte publisher. Check write access without truncation and keep the source descriptor open through metadata transfer. Resolve symlinks under existing mapping rules; publish only regular files. Failure before replace retains the old file and cleans staging. Append and other platforms keep their in-place behavior. Replacement requires parent-directory write and metadata-preservation permissions; inode identity changes, so hard-link aliases retain the old version. It is not concurrent-writer arbitration or power-loss recovery. Regressions: `tests/test_local_sandbox_atomic_writes.py`.
On Linux/macOS, LocalSandbox non-append text writes use `_publish_file_revision` and the shared atomic byte publisher. Check write access without truncation; retain the source descriptor through metadata transfer. Preserve symlink mapping; publish only regular files. Pre-replace failures retain the old file and clean staging. Append/other platforms write in place. Replacement needs parent-directory write and metadata permissions; hard-link aliases retain the old inode. No concurrent-writer arbitration or power-loss recovery. Tests: `tests/test_local_sandbox_atomic_writes.py`.
PowerShell command mapping escapes host-path whitespace/metacharacters for the caller's quote context in both user-data and provider mappings. Preserve wildcards; Bash/cmd mapping stays unchanged. Keep portable string assertions alongside native Windows smoke tests. The public bash tool's existing POSIX-quoted cwd prefix requires PowerShell 7 (`&&`) and workspace paths without apostrophes; mapping does not repair that prefix.
@ -141,7 +141,7 @@ symlinks, and visible depth/size bounds.
- `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; full reads keep stored line endings (Local `read_file`/`grep` read `newline="\n"`, writes `newline=""`), ranged reads join with `\n`; remote providers slice with `split_file_lines`, not `splitlines()`
- `write_file` - Write/append to files, creates directories; overwrites by default and exposes the `append` argument in the model-facing schema for end-of-file writes; subject to the read-before-write gate when `read_before_write.enabled` (see Middleware Chain)
- `write_file` - Create/overwrite text, or append with `append=True`. Enforces read-before-write when enabled. Reject exact elision placeholders before sync/async sandbox initialization; [matching rules](../agents/middlewares/TOOL_OUTPUT_PERSISTENCE.md#model-bound-write-elision).
- `str_replace` - Substring replacement (single or all occurrences); `_match_line_endings` gives `old_str`/`new_str` a CRLF file's endings; same-path serialization is scoped to `(sandbox.id, path)` so isolated sandboxes do not contend on identical virtual paths inside one process; subject to the read-before-write gate when `read_before_write.enabled` (see Middleware Chain)
**Optional AIO prewarm**: default off, managed async AIO only. Require the server-owned cleanup marker; direct SDK stays lazy. Preserve authorization, execution-owner cleanup, runtime-only handles, permit limits and normal eviction under the [prewarm lifecycle rules](../../../../docs/CONFIGURATION.md#sandbox).

View File

@ -24,7 +24,7 @@ from deerflow.authz.sandbox_authz import (
)
from deerflow.config import get_app_config
from deerflow.config.paths import VIRTUAL_PATH_PREFIX
from deerflow.constants import DEFAULT_SKILLS_CONTAINER_PATH
from deerflow.constants import DEFAULT_SKILLS_CONTAINER_PATH, WRITE_ELISION_PLACEHOLDER_TEMPLATE
from deerflow.runtime.secret_context import read_active_secrets
from deerflow.runtime.user_context import resolve_runtime_user_id
from deerflow.sandbox.env_policy import is_blocked_env_name
@ -3331,6 +3331,21 @@ async def _read_file_tool_async(
read_file_tool.coroutine = _read_file_tool_async
def _reject_elision_placeholder_content(content: str) -> str | None:
"""Reject a complete framework placeholder, allowing surrounding whitespace."""
pattern = re.escape(WRITE_ELISION_PLACEHOLDER_TEMPLATE).replace(re.escape("{chars}"), r"[0-9]+")
if re.fullmatch(pattern, content.strip()) is None:
return None
return (
"Error: write_file content is the tool-output elision placeholder, which is "
"framework-generated history text rather than file content. Nothing was written. "
"The placeholder stands in for a write_file payload that the harness removed from "
"the model-bound request because a later read or write superseded it; the real "
"content is the file on disk. Call read_file on the target path and write the "
"actual content."
)
def _effective_write_file_max_bytes() -> int:
"""Return the active size cap for non-append write_file calls.
@ -3384,12 +3399,23 @@ def write_file_tool(
Operators can override the cap via env var `DEERFLOW_WRITE_FILE_MAX_BYTES`
(0 disables the guard entirely). Raising it risks streaming timeouts.
PLACEHOLDER GUARD (issue #7319):
A call whose content is the tool-output budget middleware's elision
placeholder is rejected. That text is framework-owned history compression,
never file content: the model occasionally copies it out of its own elided
history and submits it as a write, which silently replaces a real
deliverable with a ~200-byte placeholder. Re-read the source and write the
actual content instead.
Args:
path: The **absolute** path to the file to write to.
content: The content to write to the file.
description: Optional short explanation of this write shown in the UI.
append: Whether to append content to the end of the file instead of overwriting it. Defaults to False.
"""
placeholder_error = _reject_elision_placeholder_content(content)
if placeholder_error is not None:
return placeholder_error
if not append:
max_bytes = _effective_write_file_max_bytes()
if max_bytes > 0:
@ -3441,6 +3467,9 @@ async def _write_file_tool_async(
description: str = "",
append: bool = False,
) -> str:
placeholder_error = _reject_elision_placeholder_content(content)
if placeholder_error is not None:
return placeholder_error
return await _run_sync_tool_after_async_sandbox_init(write_file_tool.func, runtime, path, content, description, append)

View File

@ -0,0 +1,204 @@
"""Placeholder-guard tests for write_file_tool (issue #7319).
The tool-output budget middleware elides a superseded ``write_file`` payload in
the model-bound request and replaces it with a deterministic placeholder. That
placeholder is visible in the model's own history, so the model can imitate it
and submit it as real content: #7319 reports a delivered file that was 194 bytes
of placeholder text, written 16 s after the previous step with a character count
that matched no earlier write.
These tests pin the guard that refuses that write. Like the size-guard tests
they run against the tool's internal check with the sandbox stubbed out, so the
rejection path never touches a filesystem.
"""
from __future__ import annotations
from pathlib import Path
from unittest.mock import MagicMock, patch
import pytest
from deerflow.sandbox import tools as tools_module
from deerflow.sandbox.tools import write_file_tool
from deerflow.tools.types import Runtime
# Spelled out rather than imported so the rejection tests fail on base for the
# right reason (no guard) instead of erroring on a missing import. The last test
# below pins this literal to the framework constant that renders the placeholder.
PLACEHOLDER = "[content elided: {chars} chars; this write_file call succeeded and the file was read or modified again afterwards, so the on-disk file is the current version; call read_file on its path to see it]"
def _call_write_file(*, content: str, append: bool = False) -> str:
"""Invoke write_file_tool with the sandbox initialisation chain stubbed out."""
fn = getattr(write_file_tool, "func", write_file_tool)
runtime = MagicMock()
with (
patch.object(tools_module, "ensure_sandbox_initialized") as mock_ensure,
patch.object(tools_module, "ensure_thread_directories_exist"),
patch.object(tools_module, "is_local_sandbox", return_value=False),
patch.object(tools_module, "get_file_operation_lock") as mock_lock,
):
sandbox = MagicMock()
sandbox.write_file = MagicMock()
mock_ensure.return_value = sandbox
mock_lock.return_value.__enter__ = MagicMock(return_value=None)
mock_lock.return_value.__exit__ = MagicMock(return_value=False)
result = fn(
runtime=runtime,
description="test write",
path="/tmp/report.md",
content=content,
append=append,
)
return result
@pytest.mark.parametrize("chars", [0, 194, 10686])
def test_exact_placeholder_is_rejected(chars):
"""The reported failure: the model writes back the placeholder verbatim."""
placeholder = PLACEHOLDER.format(chars=chars)
result = _call_write_file(content=placeholder)
assert result.startswith("Error:")
assert "placeholder" in result
# The message has to tell the model how to recover, not just refuse.
assert "read_file" in result
def test_placeholder_with_surrounding_whitespace_is_rejected():
"""Trailing newlines from a heredoc-style call must not slip past the guard."""
result = _call_write_file(content="\n " + PLACEHOLDER.format(chars=5000) + "\n\n")
assert result.startswith("Error:")
def test_placeholder_inside_a_document_is_left_alone():
"""A document that merely quotes the placeholder is real work, not the failure.
The reported bug is a deliverable that *is* the placeholder. A substring
guard would also block editing this repo's own ``constants.py`` and
persistence docs, which contain the template verbatim.
"""
body = "# Report\n\n" + PLACEHOLDER.format(chars=5000) + "\n\ntail\n"
assert _call_write_file(content=body) == "OK"
@pytest.mark.parametrize(
"content",
[
"[content elided: example for a user guide]\nThis is real file content.\n",
PLACEHOLDER.format(chars=5000) + "\nThis is real file content.\n",
PLACEHOLDER.format(chars=5000) + " additional text",
PLACEHOLDER,
PLACEHOLDER.format(chars=""),
PLACEHOLDER.format(chars="-1"),
PLACEHOLDER.format(chars="1.5"),
PLACEHOLDER.format(chars="example"),
PLACEHOLDER.format(chars="123"),
PLACEHOLDER.format(chars=5000)[:-1],
],
ids=["prefix-only", "leading-quote", "trailing-text", "template", "empty-count", "negative-count", "decimal-count", "text-count", "unicode-count", "missing-suffix"],
)
def test_placeholder_prefix_in_real_content_is_allowed(content: str) -> None:
assert tools_module._reject_elision_placeholder_content(content) is None
def test_rejection_does_not_reach_the_sandbox():
"""The guard runs before any filesystem work, so a rejected write leaves no trace."""
fn = getattr(write_file_tool, "func", write_file_tool)
runtime = MagicMock()
with (
patch.object(tools_module, "ensure_sandbox_initialized") as mock_ensure,
patch.object(tools_module, "ensure_thread_directories_exist"),
patch.object(tools_module, "is_local_sandbox", return_value=False),
patch.object(tools_module, "get_file_operation_lock") as mock_lock,
):
sandbox = MagicMock()
mock_ensure.return_value = sandbox
mock_lock.return_value.__enter__ = MagicMock(return_value=None)
mock_lock.return_value.__exit__ = MagicMock(return_value=False)
result = fn(
runtime=runtime,
description="test write",
path="/tmp/report.md",
content=PLACEHOLDER.format(chars=5000),
append=False,
)
assert result.startswith("Error:")
sandbox.write_file.assert_not_called()
mock_lock.assert_not_called()
def test_append_mode_is_guarded_too():
"""``append=True`` bypasses the size cap, not the placeholder guard."""
result = _call_write_file(content=PLACEHOLDER.format(chars=9000), append=True)
assert result.startswith("Error:")
@pytest.mark.asyncio
@pytest.mark.parametrize("append", [False, True])
@pytest.mark.parametrize(
"content",
[PLACEHOLDER.format(chars=194), "\n " + PLACEHOLDER.format(chars=5000) + "\n", PLACEHOLDER.format(chars=0)],
ids=["exact", "whitespace", "zero-count"],
)
async def test_async_placeholder_rejection_needs_no_initialized_sandbox(tmp_path: Path, content: str, append: bool) -> None:
target = tmp_path / "report.md"
target.write_text("Existing report\n", encoding="utf-8")
runtime = Runtime(
state={"messages": []},
context={},
config={"configurable": {"thread_id": tmp_path.name}},
stream_writer=print,
tools=[write_file_tool],
tool_call_id="write-placeholder",
store=None,
)
result = await write_file_tool.coroutine(runtime=runtime, path=target.as_posix(), content=content, append=append)
assert result.startswith("Error:")
assert "placeholder" in result
assert "read_file" in result
assert target.read_text(encoding="utf-8") == "Existing report\n"
assert runtime.state == {"messages": []}
assert runtime.context == {}
@pytest.mark.parametrize(
"content",
[
"content elided: 5000 chars", # no leading bracket
"[content elided 5000 chars]", # no colon
"[Content Elided: 5000 chars]", # different case
"# Report\n\nNormal prose that mentions elision.\n",
"See constants.py for WRITE_ELISION_PLACEHOLDER_TEMPLATE.",
],
ids=["no-bracket", "no-colon", "different-case", "ordinary-prose", "mentions-the-name"],
)
def test_near_miss_content_still_passes(content):
"""The guard matches the framework's exact sentinel, not a loose ``elided`` keyword."""
assert _call_write_file(content=content) == "OK"
def test_literal_matches_the_constant_the_middleware_renders():
"""The guard is only useful if it matches what the middleware actually writes.
``PLACEHOLDER`` is a local copy so the rejection tests can fail on base for
the right reason; this pins it to the shared constant the middleware uses to
render the placeholder, so the two cannot drift.
"""
from deerflow.agents.middlewares.tool_output_budget_middleware import _ELIDED_WRITE_CONTENT_TEMPLATE
from deerflow.constants import WRITE_ELISION_PLACEHOLDER_TEMPLATE
assert PLACEHOLDER == WRITE_ELISION_PLACEHOLDER_TEMPLATE
assert _ELIDED_WRITE_CONTENT_TEMPLATE == WRITE_ELISION_PLACEHOLDER_TEMPLATE