mirror of
https://github.com/bytedance/deer-flow.git
synced 2026-09-09 21:49:37 +00:00
fix(mcp): exclude internal temp files from workspace changes (#4898)
* fix(mcp): exclude internal temp files from workspace changes * fix(mcp): address review — shared tmp subdir constant, any-depth docs, nested test - Export MCP_TMP_SUBDIR from constants.py and import it in both stdio launch paths (tools.py, task_tool_caller.py) so the "/tmp" suffix is composed once. - Document that the .mcp exclusion matches by directory name at any depth (like .git/node_modules) in README.md and mcp/AGENTS.md — subagent work dirs below the workspace root get their own .mcp/tmp. - Pin the any-depth semantic in test_workspace_changes.py with a nested workspace/project/.mcp assertion. * docs(mcp): correct any-depth exclusion rationale; re-home tmp pinning comment The previous commit justified the any-depth `.mcp` exclusion with subagent work dirs sitting below the workspace root — a mechanism that doesn't exist: subagents share the parent's thread_id and both stdio launch paths resolve sandbox_work_dir(thread_id), so `.mcp/tmp` is always pinned at the workspace root. Reword mcp/AGENTS.md and the test comment to the real justification (consistency with the other reserved dir names, robustness against a server creating a relative `.mcp` from another cwd). Also move the orphaned "pinning the process temp dir" rationale from tools.py to constants.py next to MCP_TMP_SUBDIR, where both importers see it.
This commit is contained in:
parent
b47c7838a5
commit
a5acc25de6
@ -1159,7 +1159,7 @@ The built-in `grep` tool searches either one text file or all matching text file
|
||||
|
||||
Image bytes loaded for a vision-model call are transient: DeerFlow removes the hidden base64 message after the model consumes it so later checkpoints do not keep duplicating that payload.
|
||||
|
||||
After each run, DeerFlow records a workspace change summary for the run-owned `workspace` and `outputs` directories. The Web UI shows a compact "files changed" badge on the assistant turn; opening it reveals created, modified, and deleted files with text diffs when safe to display. Uploads are excluded because they are user inputs, not agent-generated changes. Large, binary, or sensitive-looking files are shown as metadata only.
|
||||
After each run, DeerFlow records a workspace change summary for the run-owned `workspace` and `outputs` directories. The Web UI shows a compact "files changed" badge on the assistant turn; opening it reveals created, modified, and deleted files with text diffs when safe to display. Uploads are excluded because they are user inputs, not agent-generated changes, and stdio MCP temporary/debug files under the DeerFlow-owned `.mcp/` namespace are excluded because they are process-internal state (like `.git/` and `node_modules/`, any directory named `.mcp` is excluded at any depth). Large, binary, or sensitive-looking files are shown as metadata only.
|
||||
|
||||
Files presented through `present_files` remain part of the thread's artifact state, and the Web UI restores the artifact panel and selected document after a page refresh. The currently selected formal artifact is refreshed once when the run finishes so edits become visible without a manual reload. Existing UTF-8 text artifacts under `/mnt/user-data/outputs` can also be edited and explicitly saved from the panel on Unix and Windows while the thread is idle; saves use content revisions to prevent overwriting agent changes.
|
||||
|
||||
|
||||
@ -20,6 +20,22 @@ BROWSER_FRAMES_DIRNAME = ".browser-frames"
|
||||
# snapshot capture as an extra excluded dir name.
|
||||
TOOL_RESULTS_DIRNAME = ".tool-results"
|
||||
|
||||
# Hidden directory under a thread workspace owned by stdio MCP runtimes. The
|
||||
# default subprocess temp directory lives at ``.mcp/tmp``; these files are
|
||||
# process-internal state rather than workspace deliverables, so the
|
||||
# workspace-changes scanner excludes the whole reserved namespace. MCP launch
|
||||
# paths and the scanner share this name so writes and filtering cannot drift.
|
||||
MCP_INTERNAL_DIRNAME = ".mcp"
|
||||
|
||||
# Default subprocess temp subdirectory pinned into stdio MCP environments
|
||||
# (``TMPDIR``/``TMP``/``TEMP``). Both stdio launch paths (persistent sessions
|
||||
# and background task calls) import this instead of composing the suffix
|
||||
# themselves. Pinning the process temp dir here (alongside its cwd) makes
|
||||
# tools that write to ``os.tmpdir()`` / ``tempfile.gettempdir()`` land inside
|
||||
# the mounted user-data tree, where their output is resolvable by the
|
||||
# sandbox/artifact API — instead of on an unreachable host temp path.
|
||||
MCP_TMP_SUBDIR = f"{MCP_INTERNAL_DIRNAME}/tmp"
|
||||
|
||||
# Default timeout (seconds) for MCP server bring-up: tool discovery (subprocess
|
||||
# spawn + initialize + tools/list) and persistent-session initialization. A hung
|
||||
# stdio server (e.g. npx blocked on a package download or a server that never
|
||||
|
||||
@ -17,7 +17,7 @@
|
||||
`tools/builtins/tool_search.py::get_mcp_routing_hints_prompt_section`, which
|
||||
references `tool_search` when a hinted MCP tool is currently deferred; do not
|
||||
add a parallel routing middleware for PR1-style preference hints.
|
||||
- **Stdio file outputs**: Persistent stdio sessions are scoped by `user_id:thread_id`. For stdio transports only, DeerFlow pins the subprocess default `cwd` to the thread workspace and `TMPDIR`/`TMP`/`TEMP` to `workspace/.mcp/tmp/`, unless the operator explicitly configured `cwd` or temp env values. SSE/HTTP transports skip this filesystem prep entirely.
|
||||
- **Stdio file outputs**: Persistent stdio sessions are scoped by `user_id:thread_id`. For stdio transports only, DeerFlow pins the subprocess default `cwd` to the thread workspace and `TMPDIR`/`TMP`/`TEMP` to `workspace/.mcp/tmp/`, unless the operator explicitly configured `cwd` or temp env values. `.mcp` is a DeerFlow-owned internal namespace: its temporary/debug files remain addressable when returned by a tool but are excluded from run workspace-change summaries — by directory name at any depth, consistent with the other reserved names in `EXCLUDED_DIR_NAMES` (`.git`, `node_modules`, …) and robust if a server ever creates a relative `.mcp` from a different cwd. Both launch paths pin it at the workspace root today. SSE/HTTP transports skip this filesystem prep entirely.
|
||||
- **Stdio path translation**: MCP-returned local file references are not copied. If a `ResourceLink` or conservative free-text path resolves to an existing file inside the thread's mounted user-data tree, it is translated deterministically to `/mnt/user-data/...`; paths outside that tree remain unchanged.
|
||||
- **Runtime updates**: Gateway API saves to extensions_config.json; the Gateway-embedded runtime detects changes via the resolved-path + content-signature check above, so multi-worker / stale-mtime deployments still pick up an added/removed MCP server without a restart (`PUT /api/mcp/config` keeps whole-payload validation, while `PATCH /api/mcp/config` changes only one server's `enabled` field, normalizes the same `type`/MCP-spec `transport` alias as the runtime config model, and validates the target only when enabling it; either endpoint's reset clears the cache only in its own worker). MCP, skill, and embedded-client writers share `atomic_write_extensions_config()`, which writes and fsyncs a same-directory temporary file before `os.replace()` and preserves an existing file's mode and symlink target; failed serialization or replacement leaves the prior config intact and cleans up the temporary file.
|
||||
- **Stdio launch policy at the HTTP boundary** (`routers/mcp.py::_validate_mcp_update_request`, shared by `PUT` and the enable branch of `PATCH`): a config file may express anything, but the API is untrusted input, so an API-registered stdio server must (a) name a bare executable from the allowlist — `_DEFAULT_MCP_STDIO_COMMAND_ALLOWLIST` = `{npx, uvx}`, extended by `DEER_FLOW_MCP_STDIO_COMMAND_ALLOWLIST`, with path separators, whitespace, and shell metacharacters rejected in `command`; (b) carry no `args` flag in `_ARBITRARY_EXEC_ARGS`; and (c) set no `env` name in `_CODE_INJECTING_ENV_VARS`. Checks (b) and (c) exist because the command check alone names a binary without constraining what that binary runs. The `env` denylist applies to **every** allowlisted command, and both denylists match `--flag=value` as well as `--flag value`. The `args` denylist's **scope depends on the command**, because where a launcher stops parsing its own flags is what decides whether a token is an exec flag at all:
|
||||
|
||||
@ -10,6 +10,7 @@ from typing import Any
|
||||
|
||||
from deerflow.config.extensions_config import ExtensionsConfig
|
||||
from deerflow.config.paths import get_paths
|
||||
from deerflow.constants import MCP_TMP_SUBDIR
|
||||
from deerflow.mcp.client import build_server_params
|
||||
from deerflow.mcp.interceptors import build_mcp_tool_interceptors
|
||||
from deerflow.mcp.oauth import OAuthTokenManager, build_oauth_tool_interceptor
|
||||
@ -17,8 +18,6 @@ from deerflow.mcp.session_pool import get_session_pool
|
||||
|
||||
logger = logging.getLogger(__name__)
|
||||
|
||||
_MCP_TASK_TMP_SUBDIR = ".mcp/tmp"
|
||||
|
||||
|
||||
def mcp_task_session_scope_key(*, user_id: str, thread_id: str) -> str:
|
||||
"""Keep background calls in the same per-user/per-thread session scope."""
|
||||
@ -34,7 +33,7 @@ def _prepare_stdio_connection(
|
||||
paths = get_paths()
|
||||
paths.ensure_thread_dirs(thread_id, user_id=user_id)
|
||||
work_dir = paths.sandbox_work_dir(thread_id, user_id=user_id)
|
||||
tmp_dir = work_dir / _MCP_TASK_TMP_SUBDIR
|
||||
tmp_dir = work_dir / MCP_TMP_SUBDIR
|
||||
tmp_dir.mkdir(parents=True, exist_ok=True)
|
||||
tmp_dir.chmod(0o700)
|
||||
|
||||
|
||||
@ -16,7 +16,7 @@ from langgraph.config import get_config
|
||||
|
||||
from deerflow.config.extensions_config import ExtensionsConfig, McpServerConfig, resolve_effective_mcp_routing
|
||||
from deerflow.config.paths import VIRTUAL_PATH_PREFIX, Paths, get_paths
|
||||
from deerflow.constants import DEFAULT_MCP_SESSION_INIT_TIMEOUT
|
||||
from deerflow.constants import DEFAULT_MCP_SESSION_INIT_TIMEOUT, MCP_TMP_SUBDIR
|
||||
from deerflow.mcp.client import build_servers_config
|
||||
from deerflow.mcp.interceptors import build_mcp_tool_interceptors
|
||||
from deerflow.mcp.oauth import build_oauth_tool_interceptor, get_initial_oauth_headers
|
||||
@ -46,13 +46,6 @@ logger = logging.getLogger(__name__)
|
||||
# the load-time validation skill names get (skills/storage/skill_storage.py).
|
||||
_VALID_MCP_TOOL_NAME = re.compile(r"^[A-Za-z0-9_-]+$")
|
||||
|
||||
# Subdirectory under the thread's workspace used as the temp dir for stdio MCP
|
||||
# subprocesses. Pinning the process temp dir here (alongside its cwd) makes
|
||||
# tools that write to ``os.tmpdir()`` / ``tempfile.gettempdir()`` land inside
|
||||
# the mounted user-data tree, where their output is resolvable by the
|
||||
# sandbox/artifact API — instead of on an unreachable host temp path.
|
||||
_MCP_TMP_SUBDIR = ".mcp/tmp"
|
||||
|
||||
# Matches local-file references embedded in free text returned by an MCP server.
|
||||
# Some servers (notably Playwright's ``browser_take_screenshot``) report saved
|
||||
# files only as text/markdown links rather than ``ResourceLink`` blocks. Those
|
||||
@ -184,7 +177,7 @@ def _prepare_stdio_workspace(paths: Paths, *, thread_id: str, user_id: str) -> t
|
||||
"""
|
||||
paths.ensure_thread_dirs(thread_id, user_id=user_id)
|
||||
source_base_dir = paths.sandbox_work_dir(thread_id, user_id=user_id)
|
||||
tmp_dir = source_base_dir / _MCP_TMP_SUBDIR
|
||||
tmp_dir = source_base_dir / MCP_TMP_SUBDIR
|
||||
try:
|
||||
tmp_dir.mkdir(parents=True, exist_ok=True)
|
||||
tmp_dir.chmod(0o700)
|
||||
|
||||
@ -6,7 +6,7 @@ import os
|
||||
from codecs import BOM_UTF16_BE, BOM_UTF16_LE, getincrementaldecoder
|
||||
from pathlib import Path
|
||||
|
||||
from deerflow.constants import BROWSER_FRAMES_DIRNAME, TOOL_RESULTS_DIRNAME
|
||||
from deerflow.constants import BROWSER_FRAMES_DIRNAME, MCP_INTERNAL_DIRNAME, TOOL_RESULTS_DIRNAME
|
||||
|
||||
from .types import (
|
||||
DiffUnavailableReason,
|
||||
@ -21,6 +21,10 @@ EXCLUDED_DIR_NAMES = {
|
||||
".hg",
|
||||
".svn",
|
||||
".cache",
|
||||
# Stdio MCP subprocess temp/debug files live below this server-owned
|
||||
# namespace. They remain addressable when a tool returns their path, but
|
||||
# they are not user-authored workspace deliverables or workspace changes.
|
||||
MCP_INTERNAL_DIRNAME,
|
||||
".next",
|
||||
".venv",
|
||||
# Transient per-step browser screenshots: live progress feedback surfaced in
|
||||
|
||||
@ -13,6 +13,7 @@ import pytest
|
||||
from mcp.types import CallToolResult, ResourceLink, TextContent
|
||||
|
||||
from deerflow.config.paths import VIRTUAL_PATH_PREFIX, Paths
|
||||
from deerflow.constants import MCP_TMP_SUBDIR
|
||||
from deerflow.mcp import tools as mcp_tools
|
||||
|
||||
|
||||
@ -398,7 +399,7 @@ class TestPrepareStdioWorkspace:
|
||||
source_base_dir, tmp_dir, before = mcp_tools._prepare_stdio_workspace(paths, thread_id="t1", user_id="u1")
|
||||
|
||||
assert source_base_dir == paths.sandbox_work_dir("t1", user_id="u1")
|
||||
assert tmp_dir == source_base_dir / mcp_tools._MCP_TMP_SUBDIR
|
||||
assert tmp_dir == source_base_dir / MCP_TMP_SUBDIR
|
||||
assert tmp_dir.is_dir()
|
||||
assert before == {existing: (existing.stat().st_mtime_ns, existing.stat().st_size)}
|
||||
|
||||
|
||||
@ -302,7 +302,8 @@ async def test_session_pool_tool_pins_cwd_and_temp_env(tmp_path):
|
||||
from pydantic import BaseModel, Field
|
||||
|
||||
from deerflow.config.paths import Paths
|
||||
from deerflow.mcp.tools import _MCP_TMP_SUBDIR, _make_session_pool_tool
|
||||
from deerflow.constants import MCP_TMP_SUBDIR
|
||||
from deerflow.mcp.tools import _make_session_pool_tool
|
||||
|
||||
class Args(BaseModel):
|
||||
url: str = Field(..., description="url")
|
||||
@ -336,7 +337,7 @@ async def test_session_pool_tool_pins_cwd_and_temp_env(tmp_path):
|
||||
|
||||
session_connection = create_session.call_args.args[0]
|
||||
workspace = paths.sandbox_work_dir("thread-42", user_id="user-7")
|
||||
tmp_dir = workspace / _MCP_TMP_SUBDIR
|
||||
tmp_dir = workspace / MCP_TMP_SUBDIR
|
||||
|
||||
assert session_connection["cwd"] == str(workspace)
|
||||
assert session_connection["env"]["KEEP"] == "1"
|
||||
@ -354,7 +355,8 @@ async def test_session_pool_tool_does_not_override_explicit_tmpdir(tmp_path):
|
||||
from pydantic import BaseModel, Field
|
||||
|
||||
from deerflow.config.paths import Paths
|
||||
from deerflow.mcp.tools import _MCP_TMP_SUBDIR, _make_session_pool_tool
|
||||
from deerflow.constants import MCP_TMP_SUBDIR
|
||||
from deerflow.mcp.tools import _make_session_pool_tool
|
||||
|
||||
class Args(BaseModel):
|
||||
url: str = Field(..., description="url")
|
||||
@ -389,7 +391,7 @@ async def test_session_pool_tool_does_not_override_explicit_tmpdir(tmp_path):
|
||||
session_connection = create_session.call_args.args[0]
|
||||
# Operator-provided TMPDIR is preserved; TMP/TEMP still get our default.
|
||||
assert session_connection["env"]["TMPDIR"] == "/operator/tmp"
|
||||
assert session_connection["env"]["TMP"].endswith(_MCP_TMP_SUBDIR)
|
||||
assert session_connection["env"]["TMP"].endswith(MCP_TMP_SUBDIR)
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@ -399,7 +401,8 @@ async def test_session_pool_tool_does_not_override_explicit_cwd(tmp_path):
|
||||
from pydantic import BaseModel, Field
|
||||
|
||||
from deerflow.config.paths import Paths
|
||||
from deerflow.mcp.tools import _MCP_TMP_SUBDIR, _make_session_pool_tool
|
||||
from deerflow.constants import MCP_TMP_SUBDIR
|
||||
from deerflow.mcp.tools import _make_session_pool_tool
|
||||
|
||||
class Args(BaseModel):
|
||||
url: str = Field(..., description="url")
|
||||
@ -434,7 +437,7 @@ async def test_session_pool_tool_does_not_override_explicit_cwd(tmp_path):
|
||||
|
||||
session_connection = create_session.call_args.args[0]
|
||||
workspace = paths.sandbox_work_dir("thread-42", user_id="user-7")
|
||||
tmp_dir = workspace / _MCP_TMP_SUBDIR
|
||||
tmp_dir = workspace / MCP_TMP_SUBDIR
|
||||
|
||||
assert session_connection["cwd"] == operator_cwd
|
||||
assert session_connection["env"]["TMPDIR"] == str(tmp_dir)
|
||||
|
||||
@ -247,6 +247,27 @@ def test_scan_workspace_roots_skips_excluded_directories(tmp_path):
|
||||
assert "/mnt/user-data/workspace/node_modules/ignored.js" not in snapshot.files
|
||||
|
||||
|
||||
def test_scan_workspace_roots_skips_stdio_mcp_temp_files(tmp_path):
|
||||
roots = _roots(tmp_path)
|
||||
workspace = roots[0].host_path
|
||||
(workspace / "report.md").write_text("keep", encoding="utf-8")
|
||||
mcp_tmp = workspace / ".mcp" / "tmp"
|
||||
mcp_tmp.mkdir(parents=True)
|
||||
(mcp_tmp / "debug.json").write_text("internal", encoding="utf-8")
|
||||
# `.mcp` is excluded by directory name at any depth, matching the other
|
||||
# entries in EXCLUDED_DIR_NAMES and staying robust if a server ever
|
||||
# creates a relative `.mcp` from a cwd below the workspace root.
|
||||
nested_mcp = workspace / "project" / ".mcp"
|
||||
nested_mcp.mkdir(parents=True)
|
||||
(nested_mcp / "nested.json").write_text("internal", encoding="utf-8")
|
||||
|
||||
snapshot = scan_workspace_roots(roots)
|
||||
|
||||
assert "/mnt/user-data/workspace/report.md" in snapshot.files
|
||||
assert "/mnt/user-data/workspace/.mcp/tmp/debug.json" not in snapshot.files
|
||||
assert "/mnt/user-data/workspace/project/.mcp/nested.json" not in snapshot.files
|
||||
|
||||
|
||||
def test_scan_workspace_roots_skips_browser_frames(tmp_path):
|
||||
roots = _roots(tmp_path)
|
||||
outputs = roots[1].host_path
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user