diff --git a/README.md b/README.md index c37be81e4a..86fb6e97c2 100644 --- a/README.md +++ b/README.md @@ -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. diff --git a/backend/packages/harness/deerflow/agents/middlewares/AGENTS.md b/backend/packages/harness/deerflow/agents/middlewares/AGENTS.md index 4cc9557d98..2797a02adc 100644 --- a/backend/packages/harness/deerflow/agents/middlewares/AGENTS.md +++ b/backend/packages/harness/deerflow/agents/middlewares/AGENTS.md @@ -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. ``) 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 diff --git a/backend/packages/harness/deerflow/agents/middlewares/TOOL_OUTPUT_PERSISTENCE.md b/backend/packages/harness/deerflow/agents/middlewares/TOOL_OUTPUT_PERSISTENCE.md index 9701476f1e..3f3ae13912 100644 --- a/backend/packages/harness/deerflow/agents/middlewares/TOOL_OUTPUT_PERSISTENCE.md +++ b/backend/packages/harness/deerflow/agents/middlewares/TOOL_OUTPUT_PERSISTENCE.md @@ -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 diff --git a/backend/packages/harness/deerflow/agents/middlewares/tool_output_budget_middleware.py b/backend/packages/harness/deerflow/agents/middlewares/tool_output_budget_middleware.py index 3b9082e770..f024596f9d 100644 --- a/backend/packages/harness/deerflow/agents/middlewares/tool_output_budget_middleware.py +++ b/backend/packages/harness/deerflow/agents/middlewares/tool_output_budget_middleware.py @@ -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) diff --git a/backend/tests/test_tool_output_restore_permissions.py b/backend/tests/test_tool_output_restore_permissions.py new file mode 100644 index 0000000000..6dd4f4cafb --- /dev/null +++ b/backend/tests/test_tool_output_restore_permissions.py @@ -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]