mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-07-20 15:33:54 +00:00
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.
This commit is contained in:
parent
d6590f8b17
commit
1d98f8dd95
2 changed files with 408 additions and 3 deletions
210
tests/tools/test_mcp_resource_content.py
Normal file
210
tests/tools/test_mcp_resource_content.py
Normal file
|
|
@ -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) == ""
|
||||
|
|
@ -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():
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue