From 1d98f8dd95ab51b667f4dd7538a3cbb9032a8bc7 Mon Sep 17 00:00:00 2001 From: Victor Kyriazakos Date: Mon, 13 Jul 2026 23:18:00 +0000 Subject: [PATCH] fix(mcp): materialize ResourceLink/EmbeddedResource/Audio blocks instead of dropping them MCP tool results with non-image binary resources (PDFs, archives, office docs) were silently dropped: the success path only handled TextContent and ImageContent, so a PDF-returning MCP tool appeared to return metadata only. - EmbeddedResource blob contents are decoded (50MB cap), materialized into the Hermes document cache via cache_document_from_bytes (sanitized filename, traversal-safe), and surfaced as a local-path marker the agent can read with file/terminal tools. - EmbeddedResource text contents are inlined directly. - ResourceLink blocks preserve the URI and point the agent at the server's read_resource tool; no arbitrary network fetch outside the MCP session. - AudioContent blocks are cached via cache_audio_from_bytes as MEDIA: tags. - read_resource blob contents are materialized the same way instead of returning '[binary data, N bytes]'. - Unsupported blocks are logged instead of silently discarded. - Existing ImageContent MEDIA: behavior unchanged. Reported by an enterprise customer; reproduced against an HTTP MCP server returning application/pdf resources. --- tests/tools/test_mcp_resource_content.py | 210 +++++++++++++++++++++++ tools/mcp_tool.py | 201 +++++++++++++++++++++- 2 files changed, 408 insertions(+), 3 deletions(-) create mode 100644 tests/tools/test_mcp_resource_content.py diff --git a/tests/tools/test_mcp_resource_content.py b/tests/tools/test_mcp_resource_content.py new file mode 100644 index 000000000..eeb15ce99 --- /dev/null +++ b/tests/tools/test_mcp_resource_content.py @@ -0,0 +1,210 @@ +"""Tests for MCP ResourceLink / EmbeddedResource / AudioContent handling. + +Regression coverage for a customer report (2026-07): non-image binary +resources returned through MCP resource blocks were silently dropped from +tool results, so a PDF-returning MCP tool appeared to return metadata only. +""" + +import base64 +import json +from types import SimpleNamespace + +import pytest + + +PDF_BYTES = b"%PDF-1.4 fake pdf payload for tests" + + +def _blob_resource(data: bytes, uri="slack://files/F123/report.pdf", mime="application/pdf"): + return SimpleNamespace( + uri=uri, + mimeType=mime, + blob=base64.b64encode(data).decode("ascii"), + text=None, + ) + + +def _embedded(resource): + return SimpleNamespace(type="resource", resource=resource) + + +@pytest.fixture() +def doc_cache(tmp_path, monkeypatch): + """Point the document cache at a temp dir.""" + import gateway.platforms.base as base + + monkeypatch.setattr(base, "DOCUMENT_CACHE_DIR", tmp_path) + monkeypatch.setenv("HERMES_DOCUMENT_CACHE_DIR", str(tmp_path)) + # _resolve_cache_dir consults the module constant; patching the constant + # is sufficient because the import-default comparison detects the change. + return tmp_path + + +class TestRenderResourceBlock: + def test_embedded_pdf_blob_is_materialized(self, doc_cache): + from tools.mcp_tool import _render_mcp_resource_block + + out = _render_mcp_resource_block(_embedded(_blob_resource(PDF_BYTES)), "slack") + assert "saved to" in out + assert "application/pdf" in out + # Extract path and verify bytes round-trip + path = out.split("saved to ", 1)[1].split(" (", 1)[0] + with open(path, "rb") as fh: + assert fh.read() == PDF_BYTES + assert "report.pdf" in path + + def test_embedded_text_resource_is_inlined(self): + from tools.mcp_tool import _render_mcp_resource_block + + res = SimpleNamespace(uri="mem://notes", mimeType="text/plain", text="hello world", blob=None) + assert _render_mcp_resource_block(_embedded(res), "srv") == "hello world" + + def test_resource_link_preserves_uri_and_points_at_reader(self): + from tools.mcp_tool import _render_mcp_resource_block + + link = SimpleNamespace( + type="resource_link", + uri="slack://files/F123", + name="report.pdf", + mimeType="application/pdf", + ) + out = _render_mcp_resource_block(link, "slack") + assert "slack://files/F123" in out + assert "slack_read_resource" in out + assert "report.pdf" in out + + def test_oversized_blob_fails_explicitly_without_writing(self, doc_cache, monkeypatch): + import tools.mcp_tool as m + + monkeypatch.setattr(m, "_MCP_RESOURCE_MAX_BYTES", 8) + out = m._render_mcp_resource_block(_embedded(_blob_resource(PDF_BYTES)), "srv") + assert "too large" in out + assert not list(doc_cache.glob("doc_*")) + + def test_malformed_base64_fails_explicitly(self): + from tools.mcp_tool import _render_mcp_resource_block + + res = SimpleNamespace(uri="x://y", mimeType="application/pdf", blob="!!!not-base64!!!", text=None) + out = _render_mcp_resource_block(_embedded(res), "srv") + assert "could not be decoded" in out + + def test_non_resource_block_returns_empty(self): + from tools.mcp_tool import _render_mcp_resource_block + + assert _render_mcp_resource_block(SimpleNamespace(type="text", text="hi"), "srv") == "" + + def test_path_traversal_uri_is_neutralized(self, doc_cache): + from tools.mcp_tool import _render_mcp_resource_block + + res = _blob_resource(PDF_BYTES, uri="evil://host/../../etc/passwd") + out = _render_mcp_resource_block(_embedded(res), "srv") + assert "saved to" in out + path = out.split("saved to ", 1)[1].split(" (", 1)[0] + assert str(doc_cache) in path + assert "/etc/passwd" not in path + + +class TestResourceFilename: + def test_uri_last_segment_used(self): + from tools.mcp_tool import _mcp_resource_filename + + assert _mcp_resource_filename("slack://f/ABC/quarterly.pdf", "application/pdf") == "quarterly.pdf" + + def test_fallback_to_mime_extension(self): + from tools.mcp_tool import _mcp_resource_filename + + name = _mcp_resource_filename("", "application/pdf") + assert name.endswith(".pdf") + + def test_dotdot_rejected(self): + from tools.mcp_tool import _mcp_resource_filename + + assert _mcp_resource_filename("x://y/..", "application/pdf") != ".." + + def test_control_chars_stripped(self): + from tools.mcp_tool import _mcp_resource_filename + + name = _mcp_resource_filename("x://h/report.pdf%0Ainjected%1b[31m", "application/pdf") + assert "\n" not in name and "\x1b" not in name + + def test_long_filename_capped_preserving_extension(self): + from tools.mcp_tool import _mcp_resource_filename + + name = _mcp_resource_filename("x://h/" + "a" * 500 + ".pdf", "application/pdf") + assert len(name) <= 150 + assert name.endswith(".pdf") + + +class TestPreDecodeSizeCap: + def test_oversized_b64_rejected_before_decode(self, monkeypatch): + import tools.mcp_tool as m + + monkeypatch.setattr(m, "_MCP_RESOURCE_MAX_B64_CHARS", 16) + res = SimpleNamespace( + uri="x://y/big.pdf", mimeType="application/pdf", + blob="A" * 100, text=None, + ) + called = [] + monkeypatch.setattr(base64, "b64decode", lambda *a, **k: called.append(1)) + out = m._render_mcp_resource_block(_embedded(res), "srv") + assert "too large" in out + assert not called + + +class TestAudioBlock: + def test_non_audio_returns_empty(self): + from tools.mcp_tool import _cache_mcp_audio_block + + block = SimpleNamespace(data=base64.b64encode(b"x").decode(), mimeType="application/pdf") + assert _cache_mcp_audio_block(block) == "" + + def test_audio_block_cached_as_media(self, tmp_path, monkeypatch): + import gateway.platforms.base as base + from tools.mcp_tool import _cache_mcp_audio_block + + monkeypatch.setattr(base, "AUDIO_CACHE_DIR", tmp_path) + block = SimpleNamespace( + data=base64.b64encode(b"RIFFfakewav").decode(), + mimeType="audio/wav", + ) + out = _cache_mcp_audio_block(block) + assert out.startswith("MEDIA:") + + +class TestToolResultLoopOrdering: + def test_mixed_blocks_preserve_order(self, doc_cache): + """Simulate the tool-result block loop with text + pdf resource.""" + from tools.mcp_tool import ( + _cache_mcp_image_block, + _cache_mcp_audio_block, + _render_mcp_resource_block, + ) + + blocks = [ + SimpleNamespace(type="text", text="File ID: F123\nMIME Type: application/pdf"), + _embedded(_blob_resource(PDF_BYTES)), + ] + parts = [] + for block in blocks: + if getattr(block, "text", None): + parts.append(block.text) + continue + tag = _cache_mcp_image_block(block) or _cache_mcp_audio_block(block) + if tag: + parts.append(tag) + continue + rendered = _render_mcp_resource_block(block, "slack") + if rendered: + parts.append(rendered) + assert len(parts) == 2 + assert parts[0].startswith("File ID") + assert "saved to" in parts[1] + + def test_existing_image_behavior_unchanged(self): + from tools.mcp_tool import _cache_mcp_image_block + + block = SimpleNamespace( + data=base64.b64encode(b"some bytes").decode("ascii"), + mimeType="application/pdf", + ) + assert _cache_mcp_image_block(block) == "" diff --git a/tools/mcp_tool.py b/tools/mcp_tool.py index cd999f271..942884dc5 100644 --- a/tools/mcp_tool.py +++ b/tools/mcp_tool.py @@ -102,6 +102,7 @@ import shutil import sys import threading import time +from types import SimpleNamespace from typing import Callable from datetime import datetime from typing import Any, Coroutine, Dict, List, Optional @@ -714,6 +715,165 @@ def _cache_mcp_image_block(block) -> str: return f"MEDIA:{image_path}" +# --------------------------------------------------------------------------- +# MCP resource blocks (ResourceLink / EmbeddedResource / AudioContent) +# --------------------------------------------------------------------------- + +# Hard cap on decoded resource bytes materialized from an MCP tool result. +# Prevents a misbehaving server from filling the cache disk via one block. +_MCP_RESOURCE_MAX_BYTES = 50 * 1024 * 1024 + +# Base64 expands raw bytes by ~4/3; reject oversized payloads before decoding +# so a multi-GB blob string is never transiently doubled in memory. +_MCP_RESOURCE_MAX_B64_CHARS = _MCP_RESOURCE_MAX_BYTES * 4 // 3 + 4 + + +def _mcp_resource_filename(uri: str, mime_type: str) -> str: + """Derive a safe display filename for an MCP resource. + + Only the last path segment of the URI is considered, and only as a + *name hint* — `cache_document_from_bytes` re-sanitizes and prefixes it, + so remote path components can't influence the cache location. + """ + import mimetypes + import re as _re + from pathlib import Path + from urllib.parse import urlparse, unquote + + name = "" + if uri: + try: + name = Path(unquote(urlparse(str(uri)).path or "")).name + except (ValueError, TypeError): + name = "" + # Strip control characters (newlines/ANSI escapes from hostile URIs would + # otherwise land in the filename and the transcript marker) and cap the + # length, preserving the extension. + name = _re.sub(r"[\x00-\x1f\x7f]", "", name).strip() + if len(name) > 150: + stem, dot, ext = name.rpartition(".") + if dot and 0 < len(ext) <= 12: + name = stem[: 150 - len(ext) - 1] + "." + ext + else: + name = name[:150] + if not name or name in {".", ".."}: + normalized = (mime_type or "").split(";", 1)[0].strip().lower() + ext = mimetypes.guess_extension(normalized) or ".bin" + name = f"resource{ext}" + return name + + +def _cache_mcp_audio_block(block) -> str: + """Cache an MCP ``AudioContent`` block and return a ``MEDIA:`` tag. + + Returns an empty string when *block* is not audio or on any failure — + same fail-open contract as ``_cache_mcp_image_block``. + """ + import base64 + + data = getattr(block, "data", None) + mime_type = str(getattr(block, "mimeType", None) or "").split(";", 1)[0].strip().lower() + if data is None or not mime_type.startswith("audio/"): + return "" + if len(data) > _MCP_RESOURCE_MAX_B64_CHARS: + return f"[MCP audio resource too large to cache: ~{len(data) * 3 // 4} bytes]" + try: + raw_bytes = base64.b64decode(data) + except (TypeError, ValueError) as exc: + logger.warning("MCP audio block decode failed (%s): %s", mime_type, exc) + return "" + if len(raw_bytes) > _MCP_RESOURCE_MAX_BYTES: + return f"[MCP audio resource too large to cache: {len(raw_bytes)} bytes]" + try: + from gateway.platforms.base import cache_audio_from_bytes + import mimetypes + + ext = ( + {"audio/wav": ".wav", "audio/x-wav": ".wav", "audio/wave": ".wav"}.get(mime_type) + or mimetypes.guess_extension(mime_type) + or ".ogg" + ) + audio_path = cache_audio_from_bytes(raw_bytes, ext=ext) + except ImportError: + logger.debug("MCP audio caching skipped — gateway.platforms.base unavailable") + return "" + except Exception as exc: + logger.warning("MCP audio block cache failed: %s", exc) + return "" + return f"MEDIA:{audio_path}" + + +def _render_mcp_resource_block(block, server_name: str = "") -> str: + """Render an MCP ``ResourceLink`` or ``EmbeddedResource`` block as text. + + - ``EmbeddedResource`` with text contents → the text itself. + - ``EmbeddedResource`` with blob contents → bytes are decoded (size-capped) + and materialized into the Hermes document cache; returns a marker with + the local path so file/terminal tools can consume it. + - ``ResourceLink`` → the URI plus a pointer at the server's read_resource + tool. No network fetch happens here; the link is only readable through + the originating MCP session. + + Returns an empty string for non-resource blocks. Failures are logged and + reported inline rather than silently dropping the block. + """ + block_type = getattr(block, "type", "") + + if block_type == "resource_link" or ( + hasattr(block, "uri") and not hasattr(block, "resource") and block_type != "text" + ): + uri = getattr(block, "uri", None) + if not uri: + return "" + name = getattr(block, "name", "") or "" + mime = getattr(block, "mimeType", "") or "" + details = f"uri={uri}" + if name: + details += f", name={name}" + if mime: + details += f", mimeType={mime}" + reader = f"{server_name}_read_resource" if server_name else "the MCP server's read_resource tool" + return f"[MCP resource link: {details} — fetch it with {reader}]" + + resource = getattr(block, "resource", None) + if resource is None: + return "" + + text = getattr(resource, "text", None) + if text is not None: + return str(text) + + blob = getattr(resource, "blob", None) + if blob is None: + return "" + + import base64 + + uri = str(getattr(resource, "uri", "") or "") + mime = str(getattr(resource, "mimeType", "") or "") + if len(blob) > _MCP_RESOURCE_MAX_B64_CHARS: + return f"[MCP embedded resource too large to cache: ~{len(blob) * 3 // 4} bytes, uri={uri}]" + try: + raw_bytes = base64.b64decode(blob) + except (TypeError, ValueError) as exc: + logger.warning("MCP embedded resource decode failed (%s): %s", mime or uri, exc) + return f"[MCP embedded resource could not be decoded: {mime or uri}]" + if len(raw_bytes) > _MCP_RESOURCE_MAX_BYTES: + return f"[MCP embedded resource too large to cache: {len(raw_bytes)} bytes, uri={uri}]" + try: + from gateway.platforms.base import cache_document_from_bytes + + path = cache_document_from_bytes(raw_bytes, _mcp_resource_filename(uri, mime)) + except ImportError: + logger.debug("MCP resource caching skipped — gateway.platforms.base unavailable") + return f"[MCP embedded resource received ({len(raw_bytes)} bytes, {mime or 'unknown type'}) but document cache unavailable in this process]" + except Exception as exc: + logger.warning("MCP embedded resource cache failed: %s", exc) + return f"[MCP embedded resource could not be cached: {mime or uri}]" + detail = mime or "unknown type" + return f"[MCP resource saved to {path} ({detail}, {len(raw_bytes)} bytes) — read it with read_file or terminal tools]" + + # --------------------------------------------------------------------------- # Remote MCP URL validation # --------------------------------------------------------------------------- @@ -3974,6 +4134,34 @@ def _make_tool_handler(server_name: str, tool_name: str, tool_timeout: float): image_tag = _cache_mcp_image_block(block) if image_tag: parts.append(image_tag) + continue + audio_tag = _cache_mcp_audio_block(block) + if audio_tag: + parts.append(audio_tag) + continue + # ResourceLink / EmbeddedResource blocks (PDFs, archives, + # office docs, ...). Previously these were silently dropped, + # so document-oriented MCP tools appeared to return metadata + # only (enterprise customer report, 2026-07). + resource_text = _render_mcp_resource_block(block, server_name) + if resource_text: + parts.append(resource_text) + continue + # Benign empty renders (empty text blocks, empty text + # resources, audio in a process without the gateway cache) + # aren't data loss — log at debug. Warn only for genuinely + # unrecognized block shapes. + block_type = getattr(block, "type", None) or type(block).__name__ + if block_type in {"text", "resource", "audio", "image"}: + logger.debug( + "MCP %s: content block type %r rendered empty", + server_name, block_type, + ) + else: + logger.warning( + "MCP %s: dropping unsupported content block type %r", + server_name, block_type, + ) text_result = "\n".join(parts) if parts else "" # Combine content + structuredContent when both are present. @@ -4124,10 +4312,17 @@ def _make_read_resource_handler(server_name: str, tool_timeout: float): parts: List[str] = [] contents = result.contents if hasattr(result, "contents") else [] for block in contents: - if hasattr(block, "text"): + if getattr(block, "text", None) is not None: parts.append(block.text) - elif hasattr(block, "blob"): - parts.append(f"[binary data, {len(block.blob)} bytes]") + elif getattr(block, "blob", None) is not None: + # Materialize binary resource contents into the document + # cache instead of discarding them (same contract as + # EmbeddedResource blocks in tool results). + rendered = _render_mcp_resource_block( + SimpleNamespace(type="resource", resource=block), + server_name, + ) + parts.append(rendered or f"[binary data, {len(block.blob)} bytes]") return json.dumps({"result": "\n".join(parts) if parts else ""}, ensure_ascii=False) def _call_once():