mirror of
https://github.com/bytedance/deer-flow.git
synced 2026-10-26 21:26:39 +00:00
fix(middleware): preserve mounted output permissions on blob restore (#7390)
This commit is contained in:
parent
aa6fb7b67f
commit
6c705f73e7
@ -2750,7 +2750,9 @@ On sandboxes without an attested broker mode, lark-cli
|
||||
commands fail with an explicit unverified-mode error instead of running
|
||||
against an unauthenticated profile.
|
||||
|
||||
Host-externalized tool outputs use the Gateway's normal file-creation umask.
|
||||
Host-externalized tool outputs use the Gateway's normal file-creation permissions.
|
||||
Restoring those outputs from shared blob storage applies the recovering Gateway's
|
||||
creation policy too, including its umask and any inherited default ACL.
|
||||
Host and sandbox outputs use deterministic filenames hashed from the raw tool
|
||||
call ID and output content. Missing, colliding, or oversized IDs cannot overwrite
|
||||
different output or make the filename too long; identical output can reuse its path.
|
||||
|
||||
@ -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; 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. 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
|
||||
|
||||
|
||||
@ -11,14 +11,22 @@ write failure boundary. Invalid UTF-8 text or call IDs return `None` before
|
||||
file publication, allowing the tool wrappers to use the configured inline fallback.
|
||||
|
||||
`_externalize` creates a unique sibling `.tool-output-*.tmp` with exclusive
|
||||
creation (`open(..., "x")`). Its mode is `0o666 & ~umask`, preserving ordinary
|
||||
file-creation permissions without reading or changing the process-wide umask.
|
||||
creation (`open(..., "x")`) requesting mode `0o666`. Effective permissions follow
|
||||
the process umask and any filesystem default ACL; without a default ACL the mode
|
||||
is `0o666 & ~umask`. The writer never reads or changes the process-wide umask.
|
||||
This matters when a mounted sandbox reads the output under a different UID.
|
||||
Restrictive operator umasks remain restrictive. It also passes `newline=""`, so
|
||||
It also passes `newline=""`, so
|
||||
the file holds exactly the bytes of the stamped blob ref on every platform;
|
||||
Windows text mode would write CRLF, and restore discards a file whose size or
|
||||
digest does not match.
|
||||
|
||||
Blob recovery uses the same exclusive sibling creation in binary mode, subject
|
||||
to the recovering Gateway's umask and filesystem default ACL. It flushes, fsyncs and
|
||||
closes the complete file before replacement. A missing or mismatched local
|
||||
copy therefore does not become fixed-mode `0600` solely because it was restored;
|
||||
the same ordinary creation policy applies. Digest-matching local files are reused
|
||||
without changing their permissions.
|
||||
|
||||
The writer closes its file before atomically replacing the deterministic final
|
||||
path. Concurrent publishers of identical output can safely reuse that path;
|
||||
different output retains its own path. Ownership starts only after exclusive
|
||||
|
||||
@ -31,7 +31,6 @@ import os
|
||||
import posixpath
|
||||
import re
|
||||
import shlex
|
||||
import tempfile
|
||||
import uuid
|
||||
from collections.abc import Awaitable, Callable, Mapping
|
||||
from dataclasses import replace as dc_replace
|
||||
@ -930,12 +929,16 @@ def _write_restored_output(filepath: str, data: bytes) -> bool:
|
||||
tmp_path: str | None = None
|
||||
try:
|
||||
os.makedirs(directory, exist_ok=True)
|
||||
fd, tmp_path = tempfile.mkstemp(prefix=f".{os.path.basename(filepath)}.", suffix=".tmp", dir=directory)
|
||||
with os.fdopen(fd, "wb") as handle:
|
||||
candidate_path = os.path.join(directory, f".tool-output-{uuid.uuid4().hex}.tmp")
|
||||
# Match initial publication's creation permissions for mounted sandbox
|
||||
# readers, while retaining the recovering Gateway's restrictive umask.
|
||||
with open(candidate_path, "xb") as handle:
|
||||
tmp_path = candidate_path
|
||||
handle.write(data)
|
||||
handle.flush()
|
||||
os.fsync(handle.fileno())
|
||||
os.replace(tmp_path, filepath)
|
||||
tmp_path = None
|
||||
return True
|
||||
except OSError:
|
||||
logger.warning("Failed to restore externalized tool output on this Gateway", exc_info=True)
|
||||
|
||||
103
backend/tests/test_tool_output_restore_permissions.py
Normal file
103
backend/tests/test_tool_output_restore_permissions.py
Normal file
@ -0,0 +1,103 @@
|
||||
"""Restored tool outputs retain the mounted sandbox's file access contract."""
|
||||
|
||||
import json
|
||||
import os
|
||||
import subprocess
|
||||
import sys
|
||||
from types import SimpleNamespace
|
||||
|
||||
import pytest
|
||||
|
||||
from deerflow.agents.middlewares import tool_output_budget_middleware as mw
|
||||
|
||||
|
||||
@pytest.mark.skipif(os.name == "nt", reason="POSIX permission bits are not available on Windows")
|
||||
@pytest.mark.parametrize("mask", [0o022, 0o002, 0o077], ids=["other-readable", "group-writable", "private"])
|
||||
@pytest.mark.parametrize("hook", ["sync", "async"])
|
||||
def test_blob_restore_respects_gateway_umask(tmp_path, mask, hook):
|
||||
# umask is process-global: isolate changes from middleware worker threads.
|
||||
script = """
|
||||
import asyncio
|
||||
import json
|
||||
import os
|
||||
from pathlib import Path
|
||||
import stat
|
||||
import sys
|
||||
from types import SimpleNamespace
|
||||
|
||||
from langchain.agents.middleware.types import ModelRequest
|
||||
from langchain_core.messages import ToolMessage
|
||||
from deerflow.agents.middlewares import tool_output_budget_middleware as mw
|
||||
from deerflow.config.tool_output_config import ToolOutputConfig
|
||||
from deerflow.storage.backends.local_fs import LocalFsBlobStore
|
||||
|
||||
root = Path(sys.argv[1])
|
||||
os.umask(int(sys.argv[2]))
|
||||
store = LocalFsBlobStore(root / "blobs")
|
||||
mw.get_blob_store_if_enabled = lambda: store
|
||||
content = "complete tool output\\n" * 2000
|
||||
producer_outputs = root / "gateway-a" / "outputs"
|
||||
runtime = SimpleNamespace(state={"thread_data": {"outputs_path": str(producer_outputs)}}, context={"thread_id": "thread-1"}, config={})
|
||||
tool_request = SimpleNamespace(tool_call={"name": "remote_executor", "id": "call-1"}, runtime=runtime)
|
||||
middleware = mw.ToolOutputBudgetMiddleware(config=ToolOutputConfig(max_chars=1000, externalize_min_chars=1000))
|
||||
message = middleware.wrap_tool_call(tool_request, lambda request: ToolMessage(content=content, name="remote_executor", tool_call_id="call-1"))
|
||||
assert mw.TOOL_OUTPUT_BLOB_KEY in message.additional_kwargs
|
||||
payload = message.additional_kwargs[mw.TOOL_OUTPUT_BLOB_KEY]
|
||||
relative = Path(payload["storage_subdir"]) / payload["virtual_path"].rsplit("/", 1)[1]
|
||||
original = producer_outputs / relative
|
||||
assert original.read_bytes() == content.encode()
|
||||
modes = [stat.S_IMODE(original.stat().st_mode)]
|
||||
# Recover only the checkpointed message and a shared real blob store.
|
||||
restored_message = ToolMessage.model_validate_json(message.model_dump_json())
|
||||
for scenario in ("missing", "mismatched"):
|
||||
outputs = root / scenario / "outputs"
|
||||
destination = outputs / relative
|
||||
if scenario == "mismatched":
|
||||
destination.parent.mkdir(parents=True)
|
||||
destination.write_bytes(b"stale output")
|
||||
request = ModelRequest(model=None, messages=[restored_message], tools=[], state={"thread_data": {"outputs_path": str(outputs)}})
|
||||
def handler(prepared):
|
||||
assert destination.read_bytes() == content.encode()
|
||||
modes.append(stat.S_IMODE(destination.stat().st_mode))
|
||||
return []
|
||||
if sys.argv[3] == "async":
|
||||
async def async_handler(prepared):
|
||||
return handler(prepared)
|
||||
asyncio.run(middleware.awrap_model_call(request, async_handler))
|
||||
else:
|
||||
middleware.wrap_model_call(request, handler)
|
||||
assert list(destination.parent.iterdir()) == [destination]
|
||||
print(json.dumps({"modes": modes}))
|
||||
"""
|
||||
completed = subprocess.run([sys.executable, "-c", script, str(tmp_path), str(mask), hook], capture_output=True, text=True, timeout=30, check=True)
|
||||
assert json.loads(completed.stdout)["modes"] == [0o666 & ~mask] * 3
|
||||
|
||||
|
||||
def test_restore_creation_collision_preserves_other_writers_temp(tmp_path, monkeypatch):
|
||||
destination = tmp_path / "output.txt"
|
||||
destination.write_bytes(b"previous complete output")
|
||||
other_temp = tmp_path / ".tool-output-collision.tmp"
|
||||
other_temp.write_bytes(b"another writer's pending output")
|
||||
monkeypatch.setattr(mw, "uuid", SimpleNamespace(uuid4=lambda: SimpleNamespace(hex="collision")))
|
||||
|
||||
assert mw._write_restored_output(str(destination), b"replacement") is False
|
||||
|
||||
assert destination.read_bytes() == b"previous complete output"
|
||||
assert other_temp.read_bytes() == b"another writer's pending output"
|
||||
assert set(tmp_path.iterdir()) == {destination, other_temp}
|
||||
|
||||
|
||||
@pytest.mark.parametrize("operation", ["fsync", "replace"])
|
||||
def test_restore_failure_preserves_published_output_and_cleans_owned_temp(tmp_path, monkeypatch, operation):
|
||||
destination = tmp_path / "output.txt"
|
||||
destination.write_bytes(b"previous complete output")
|
||||
|
||||
def fail(*args):
|
||||
raise OSError("storage unavailable")
|
||||
|
||||
monkeypatch.setattr(mw.os, operation, fail)
|
||||
|
||||
assert mw._write_restored_output(str(destination), b"replacement") is False
|
||||
|
||||
assert destination.read_bytes() == b"previous complete output"
|
||||
assert list(tmp_path.iterdir()) == [destination]
|
||||
Loading…
x
Reference in New Issue
Block a user