mirror of
https://github.com/bytedance/deer-flow.git
synced 2026-09-09 21:49:37 +00:00
fix(mcp): resolve drive-qualified paths in file reference rewriting (#5242)
* fix(mcp): resolve drive-qualified paths in file reference rewriting
urlparse reads a Windows drive prefix ("C:/...") as the URI scheme, so
_local_path_from_uri() returned None for every drive-qualified path and
MCP file references were never rewritten to /mnt/user-data/... virtual
paths on Windows hosts. file:// URIs were parsed with urlparse().path
alone, which also drops the drive qualifier.
- resolve file URIs through url2pathname so the /C:/... form keeps its
drive, and treat single-letter schemes as bare drive paths;
- match drive-qualified absolute paths in the free-text reference regex;
- build test URIs with Path.as_uri() and anchor absolute-path fixtures
at tmp_path so expectations are host-portable, and cover the
drive-prefix scheme quirk explicitly.
* fix(mcp): decode file URIs once and guard Windows path rejection
Review follow-up on #5242:
- url2pathname already percent-decodes on both platforms, so the extra
unquote() wrapper decoded references twice and broke filenames that
contain a literal '%'. Pass parsed.path straight through.
- On Windows, url2pathname raises OSError for paths containing a raw
'|' (e.g. file:///C:/tmp/a|b.png); catch it so one odd URI cannot
abort the whole best-effort rewrite pass.
- The relative-reference regex alternative now accepts backslash
separators, which is what Windows servers print for relative paths.
- Add Windows-only regressions driving the backslash free-text form
and a file:///C:/ URI end to end, plus the OSError rejection.
* fix(mcp): resolve file://C:/… URIs with a drive-qualified authority
Review follow-up on #5242 (two-slash Windows drive form):
- Some Windows tools emit file://C:/… without the third slash, which
puts the drive in the URI authority. Consult parsed.netloc: rebuild
the /C:/… URL path for a drive-qualified authority, keep the current
handling for empty and localhost authorities, and reject any other
host instead of silently treating its path as local.
- Extend the free-text regex so the two-slash form matches as one token
instead of the previous stray e://… mid-token match.
- Cover the two-slash form at the _local_path_from_uri unit, through
_rewrite_local_paths_in_text, and add a portable case asserting that
a remote-host file URI is ignored.
* fix(mcp): anchor the drive-qualified text alternative with a lookbehind
Review follow-up on #5242:
- [A-Za-z]:[\/] could steal a token at an earlier scan position:
for file:/tmp/… (single-slash form per RFC 8089 / Java File.toURI())
the match became e:/tmp/…, which resolves as a bare drive path and
left the reference unrewritten where /tmp/… was rewritten before.
Anchor the alternative with (?<![\w.-]) so word:/… shapes fall
through to the earlier alternatives.
- Add the missing coverage for the relative alternative's backslash
support (temp\page.yml through _rewrite_local_paths_in_text) and
a portable regression pinning the file:/… tokenization.
This commit is contained in:
parent
e7c059d8d4
commit
cbd6621d52
@ -9,7 +9,8 @@ from collections.abc import Iterable, Mapping
|
||||
from datetime import timedelta
|
||||
from pathlib import Path
|
||||
from typing import Any
|
||||
from urllib.parse import unquote, urlparse
|
||||
from urllib.parse import urlparse
|
||||
from urllib.request import url2pathname
|
||||
|
||||
from langchain_core.tools import BaseTool, StructuredTool
|
||||
from langgraph.config import get_config
|
||||
@ -54,7 +55,15 @@ _VALID_MCP_TOOL_NAME = re.compile(r"^[A-Za-z0-9_-]+$")
|
||||
# server process cwd (e.g. ``temp/page.yml``, ``./shot.png``). Each match is
|
||||
# only rewritten when it resolves to an existing file inside the thread's
|
||||
# user-data tree, so an over-eager match is harmless (left untouched).
|
||||
_LOCAL_PATH_IN_TEXT_RE = re.compile(r"(?:file://)?/[^\s'\"<>|*?]+|(?:\.{0,2}/|[\w.-]+/)[^\s'\"<>|*?]+")
|
||||
_LOCAL_PATH_IN_TEXT_RE = re.compile(
|
||||
r"(?:file://)?/[^\s'\"<>|*?]+" # POSIX absolute path or file:// URI
|
||||
r"|file://[A-Za-z]:[^\s'\"<>|*?]+" # file://C:/… — some Windows tools skip the third slash
|
||||
# Windows drive-qualified absolute path; the lookbehind keeps a word
|
||||
# character before the colon (file:/…, id:/…) on the earlier alternatives
|
||||
r"|(?<![\w.-])[A-Za-z]:[\\/][^\s'\"<>|*?]+"
|
||||
# path relative to the server cwd (Windows servers print "\" separators)
|
||||
r"|(?:\.{0,2}[\\/]|[\w.-]+[\\/])[^\s'\"<>|*?]+"
|
||||
)
|
||||
|
||||
# Trailing characters that are punctuation/markup rather than part of a path.
|
||||
_TEXT_PATH_TRAILING_CHARS = ".,;:!?)]}>\"'`"
|
||||
@ -77,7 +86,27 @@ def _local_path_from_uri(uri: str, *, base_dir: Path | None = None) -> Path | No
|
||||
except ValueError:
|
||||
return None
|
||||
if parsed.scheme == "file":
|
||||
raw = unquote(parsed.path)
|
||||
# url2pathname converts the "/C:/..." form a file URI's path takes on
|
||||
# Windows into a drive-qualified "C:\..." path; on POSIX it is identity.
|
||||
# It already percent-decodes, so no extra unquote here, and it can
|
||||
# reject odd Windows spellings with OSError — leave those untouched.
|
||||
netloc = parsed.netloc
|
||||
if netloc and netloc.lower() != "localhost":
|
||||
# Some Windows tools emit file://C:/… (two slashes): the drive
|
||||
# lands in the URI authority. Any other host is not a local file.
|
||||
if len(netloc) != 2 or not netloc[0].isalpha() or netloc[1] != ":":
|
||||
return None
|
||||
url_path = f"/{netloc}{parsed.path}"
|
||||
else:
|
||||
url_path = parsed.path
|
||||
try:
|
||||
raw = url2pathname(url_path)
|
||||
except OSError:
|
||||
return None
|
||||
elif len(parsed.scheme) == 1 and parsed.scheme.isalpha():
|
||||
# urlparse reads a Windows drive prefix ("C:\...") as the URI scheme;
|
||||
# the original string is a bare local path, not a remote URI.
|
||||
raw = uri
|
||||
elif parsed.scheme == "":
|
||||
raw = uri
|
||||
else:
|
||||
|
||||
@ -6,6 +6,7 @@ tool wrapper pins stdio cwd/temp under the thread's mounted user-data tree and
|
||||
rewrites returned file references to ``/mnt/user-data/...`` virtual paths.
|
||||
"""
|
||||
|
||||
import os
|
||||
from pathlib import Path
|
||||
from unittest.mock import patch
|
||||
|
||||
@ -34,14 +35,17 @@ def _workspace_file(paths: Paths, relative_path: str, *, content: bytes = b"data
|
||||
|
||||
|
||||
class TestLocalPathFromUri:
|
||||
def test_file_uri(self):
|
||||
assert mcp_tools._local_path_from_uri("file:///tmp/shot.png") == Path("/tmp/shot.png")
|
||||
def test_file_uri(self, tmp_path: Path):
|
||||
src = tmp_path / "shot.png"
|
||||
assert mcp_tools._local_path_from_uri(src.as_uri()) == src
|
||||
|
||||
def test_bare_absolute_path(self):
|
||||
assert mcp_tools._local_path_from_uri("/var/data/out.pdf") == Path("/var/data/out.pdf")
|
||||
def test_bare_absolute_path(self, tmp_path: Path):
|
||||
src = tmp_path / "data" / "out.pdf"
|
||||
assert mcp_tools._local_path_from_uri(str(src)) == src
|
||||
|
||||
def test_file_uri_with_url_encoded_spaces(self):
|
||||
assert mcp_tools._local_path_from_uri("file:///tmp/my%20shot.png") == Path("/tmp/my shot.png")
|
||||
def test_file_uri_with_url_encoded_spaces(self, tmp_path: Path):
|
||||
src = tmp_path / "my shot.png"
|
||||
assert mcp_tools._local_path_from_uri(src.as_uri()) == src
|
||||
|
||||
def test_remote_uri_is_ignored(self):
|
||||
assert mcp_tools._local_path_from_uri("https://example.com/a.png") is None
|
||||
@ -63,9 +67,31 @@ class TestLocalPathFromUri:
|
||||
def test_file_uri_with_empty_path_is_ignored(self):
|
||||
assert mcp_tools._local_path_from_uri("file://") is None
|
||||
|
||||
def test_file_uri_with_localhost_host(self):
|
||||
def test_file_uri_with_localhost_host(self, tmp_path: Path):
|
||||
# file://localhost/abs/path is the host form of file:///abs/path.
|
||||
assert mcp_tools._local_path_from_uri("file://localhost/tmp/shot.png") == Path("/tmp/shot.png")
|
||||
src = tmp_path / "shot.png"
|
||||
assert mcp_tools._local_path_from_uri(src.as_uri().replace("file://", "file://localhost", 1)) == src
|
||||
|
||||
def test_windows_drive_letter_path_is_resolved(self):
|
||||
# urlparse reads a Windows drive prefix ("C:/...") as the URI scheme.
|
||||
# On Windows hosts it must still resolve as a bare local path; on
|
||||
# POSIX it is not a local path at all.
|
||||
path = mcp_tools._local_path_from_uri("C:/Users/shot.png")
|
||||
if os.name == "nt":
|
||||
assert path == Path("C:/Users/shot.png")
|
||||
else:
|
||||
assert path is None
|
||||
|
||||
@pytest.mark.skipif(os.name != "nt", reason="a raw '|' in a file URI path rejects with OSError only on Windows")
|
||||
def test_windows_url2pathname_oserror_is_left_untouched(self):
|
||||
assert mcp_tools._local_path_from_uri("file:///C:/tmp/a|b.png") is None
|
||||
|
||||
@pytest.mark.skipif(os.name != "nt", reason="exercises the file://C:/… two-slash Windows drive URI form")
|
||||
def test_windows_two_slash_file_uri_resolves_drive(self):
|
||||
assert mcp_tools._local_path_from_uri("file://C:/Users/shot.png") == Path("C:/Users/shot.png")
|
||||
|
||||
def test_remote_host_file_uri_is_ignored(self):
|
||||
assert mcp_tools._local_path_from_uri("file://example.com/a.png") is None
|
||||
|
||||
def test_empty_is_ignored(self):
|
||||
assert mcp_tools._local_path_from_uri("") is None
|
||||
@ -110,7 +136,17 @@ class TestLocalUriToVirtualPath:
|
||||
src = _workspace_file(paths, "shot.png")
|
||||
|
||||
with _patch_paths(paths):
|
||||
result = mcp_tools._local_uri_to_virtual_path(f"file://{src}", thread_id="t1", user_id="u1")
|
||||
result = mcp_tools._local_uri_to_virtual_path(src.as_uri(), thread_id="t1", user_id="u1")
|
||||
|
||||
assert result == f"{VIRTUAL_PATH_PREFIX}/workspace/shot.png"
|
||||
|
||||
@pytest.mark.skipif(os.name != "nt", reason="exercises the file:///C:/... drive-qualified URI form")
|
||||
def test_windows_file_uri_translates_to_virtual_path(self, paths: Paths):
|
||||
src = _workspace_file(paths, "shot.png")
|
||||
assert src.as_uri().startswith("file:///C:/")
|
||||
|
||||
with _patch_paths(paths):
|
||||
result = mcp_tools._local_uri_to_virtual_path(src.as_uri(), thread_id="t1", user_id="u1")
|
||||
|
||||
assert result == f"{VIRTUAL_PATH_PREFIX}/workspace/shot.png"
|
||||
|
||||
@ -185,6 +221,45 @@ class TestRewriteLocalPathsInText:
|
||||
|
||||
assert result == f"Saved to {VIRTUAL_PATH_PREFIX}/workspace/.mcp/tmp/page.png"
|
||||
|
||||
@pytest.mark.skipif(os.name != "nt", reason="exercises backslash drive-qualified paths in free text")
|
||||
def test_windows_backslash_drive_path_in_text_is_rewritten(self, paths: Paths):
|
||||
src = _workspace_file(paths, "shot.png")
|
||||
text = f"Saved as {src}"
|
||||
assert "\\" in text
|
||||
|
||||
with _patch_paths(paths):
|
||||
result = mcp_tools._rewrite_local_paths_in_text(text, thread_id="t1", user_id="u1")
|
||||
|
||||
assert result == f"Saved as {VIRTUAL_PATH_PREFIX}/workspace/shot.png"
|
||||
|
||||
@pytest.mark.skipif(os.name != "nt", reason="exercises backslash relative paths in free text")
|
||||
def test_windows_backslash_relative_path_in_text_is_rewritten(self, paths: Paths):
|
||||
_workspace_file(paths, "temp/page.yml")
|
||||
workspace = paths.sandbox_work_dir("t1", user_id="u1")
|
||||
|
||||
with _patch_paths(paths):
|
||||
result = mcp_tools._rewrite_local_paths_in_text("Saved as temp\\page.yml", thread_id="t1", user_id="u1", source_base_dir=workspace)
|
||||
|
||||
assert result == f"Saved as {VIRTUAL_PATH_PREFIX}/workspace/temp/page.yml"
|
||||
|
||||
def test_single_slash_file_uri_is_matched_as_posix_absolute(self):
|
||||
# file:/… (single slash, as RFC 8089 and Java's File.toURI() produce)
|
||||
# must not be stolen mid-token by the drive-qualified alternative: the
|
||||
# engine has to fall through to the /… absolute alternative.
|
||||
match = mcp_tools._LOCAL_PATH_IN_TEXT_RE.search("Saved as file:/tmp/workspace/shot.png")
|
||||
assert match.group(0) == "/tmp/workspace/shot.png"
|
||||
|
||||
@pytest.mark.skipif(os.name != "nt", reason="exercises the file://C:/… two-slash URI form in free text")
|
||||
def test_windows_two_slash_file_uri_in_text_is_rewritten(self, paths: Paths):
|
||||
src = _workspace_file(paths, "shot.png")
|
||||
two_slash_uri = src.as_uri().replace("file:///", "file://", 1)
|
||||
assert two_slash_uri.startswith("file://C:")
|
||||
|
||||
with _patch_paths(paths):
|
||||
result = mcp_tools._rewrite_local_paths_in_text(f"Saved as {two_slash_uri}", thread_id="t1", user_id="u1")
|
||||
|
||||
assert result == f"Saved as {VIRTUAL_PATH_PREFIX}/workspace/shot.png"
|
||||
|
||||
def test_old_tmp_path_outside_user_data_is_left_untouched(self, tmp_path: Path, paths: Paths):
|
||||
src = tmp_path / "playwright-mcp-output" / "page.png"
|
||||
src.parent.mkdir()
|
||||
@ -431,7 +506,7 @@ class TestConvertCallToolResultRewrites:
|
||||
def test_resource_link_image_inside_workspace_rewritten(self, paths: Paths):
|
||||
src = _workspace_file(paths, "page.png", content=b"png")
|
||||
result = CallToolResult(
|
||||
content=[ResourceLink(type="resource_link", name="page", uri=f"file://{src}", mimeType="image/png")],
|
||||
content=[ResourceLink(type="resource_link", name="page", uri=src.as_uri(), mimeType="image/png")],
|
||||
isError=False,
|
||||
)
|
||||
|
||||
@ -447,7 +522,7 @@ class TestConvertCallToolResultRewrites:
|
||||
src = outputs / "doc.pdf"
|
||||
src.write_bytes(b"pdf")
|
||||
result = CallToolResult(
|
||||
content=[ResourceLink(type="resource_link", name="doc", uri=f"file://{src}", mimeType="application/pdf")],
|
||||
content=[ResourceLink(type="resource_link", name="doc", uri=src.as_uri(), mimeType="application/pdf")],
|
||||
isError=False,
|
||||
)
|
||||
|
||||
@ -460,7 +535,7 @@ class TestConvertCallToolResultRewrites:
|
||||
def test_resource_link_outside_user_data_untouched(self, tmp_path: Path, paths: Paths):
|
||||
src = tmp_path / "page.png"
|
||||
src.write_bytes(b"png")
|
||||
uri = f"file://{src}"
|
||||
uri = src.as_uri()
|
||||
result = CallToolResult(
|
||||
content=[ResourceLink(type="resource_link", name="page", uri=uri, mimeType="image/png")],
|
||||
isError=False,
|
||||
@ -514,7 +589,7 @@ class TestConvertCallToolResultRewrites:
|
||||
|
||||
def test_no_context_does_not_rewrite(self, paths: Paths):
|
||||
src = _workspace_file(paths, "x.png", content=b"png")
|
||||
uri = f"file://{src}"
|
||||
uri = src.as_uri()
|
||||
result = CallToolResult(
|
||||
content=[ResourceLink(type="resource_link", name="x", uri=uri, mimeType="image/png")],
|
||||
isError=False,
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user