Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,8 @@ Oversized inline media makes `_writer._truncate_span_event` blank the span's ent

Do not dump raw `kwargs` into LLMObs metadata. Prefer shared helpers such as `get_metadata_from_kwargs()` and provider-specific helpers in `ddtrace/llmobs/_integrations/utils.py` when they apply. They filter provider sentinel defaults such as OpenAI `Omit`/`NotGiven`, serialize SDK objects through `load_data_value()`, and keep metadata limited to supported fields. `_annotate_llmobs_span_data()` coerces metadata keys to strings, but integrations should still avoid sending noisy or high-cardinality raw objects.

Never copy tool configurations verbatim either: hosted MCP tools carry credentials in `headers`, `authorization` and the server URL (userinfo, path, query). Record them through `safe_mcp_tool_config()` in `ddtrace/llmobs/_integrations/utils.py`, which keeps only non-secret fields and the URL origin.

### Token Field Mapping

| Library | Input | Output | Cache Fields |
Expand Down
23 changes: 23 additions & 0 deletions ddtrace/internal/utils/http.py
Original file line number Diff line number Diff line change
Expand Up @@ -181,6 +181,29 @@ def strip_query_string(url: str) -> str:
return h + fs + f


_DEFAULT_PORTS = {"http": 80, "https": 443, "ws": 80, "wss": 443}


def url_origin(url: str) -> Optional[str]:
"""Reduce url to scheme://host[:port], dropping userinfo, path, query and fragment, which can hold credentials.

Scheme and host are lowercased and a default port dropped. Returns None without scheme or host.
"""
try:
parts = parse.urlsplit(url)
host = parts.hostname
port = parts.port
except (TypeError, ValueError):
return None
if not parts.scheme or not host:
return None
scheme = parts.scheme.lower()
if ":" in host:
host = f"[{host}]"
netloc = host if port is None or _DEFAULT_PORTS.get(scheme) == port else f"{host}:{port}"
return f"{scheme}://{netloc}"


def _redact_query_string_match(match: re.Match[bytes]) -> bytes:
return (match.group(1) or b"") + b"<redacted>"

Expand Down
3 changes: 2 additions & 1 deletion ddtrace/llmobs/_integrations/openai_agents.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
from ddtrace.llmobs._integrations.utils import LLMObsTraceInfo
from ddtrace.llmobs._integrations.utils import OaiSpanAdapter
from ddtrace.llmobs._integrations.utils import OaiTraceAdapter
from ddtrace.llmobs._integrations.utils import safe_mcp_tool_config
from ddtrace.llmobs._utils import _annotate_llmobs_span_data
from ddtrace.llmobs._utils import _get_nearest_llmobs_ancestor
from ddtrace.llmobs._utils import get_llmobs_parent_id
Expand Down Expand Up @@ -394,7 +395,7 @@ def _extract_tools_from_agent(self, agent):
tool_dict["tool_config"] = tool.tool_config
elif tool_name == "hosted_mcp":
if hasattr(tool, "tool_config"):
tool_dict["tool_config"] = tool.tool_config
tool_dict["tool_config"] = safe_mcp_tool_config(tool.tool_config)
if hasattr(tool, "on_approval_request"):
tool_dict["on_approval_request"] = tool.on_approval_request
elif tool_name == "image_generation":
Expand Down
38 changes: 37 additions & 1 deletion ddtrace/llmobs/_integrations/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
from ddtrace.internal.utils.formats import _get_attr
from ddtrace.internal.utils.formats import format_trace_id
from ddtrace.internal.utils.formats import safe_load_json
from ddtrace.internal.utils.http import url_origin
from ddtrace.llmobs._constants import AUDIO_FALLBACK_MARKER
from ddtrace.llmobs._constants import DISPATCH_ON_LLM_TOOL_CHOICE
from ddtrace.llmobs._constants import DISPATCH_ON_TOOL_CALL_OUTPUT_USED
Expand Down Expand Up @@ -1500,6 +1501,40 @@ def _openai_decision_answer_to_dict(answer: Any) -> dict[str, Any]:
return {}


# Hosted MCP tool fields that carry no credential; headers, authorization and the URL path, query
# and fragment can, so only these and the URL origin are recorded.
_SAFE_MCP_TOOL_FIELDS = (
Comment thread
avara1986 marked this conversation as resolved.
Resolved
"type",
"server_label",
"server_description",
"connector_id",
"require_approval",
"allowed_tools",
)


def safe_mcp_tool_config(tool: Any) -> dict[str, Any]:
"""Allowlisted view of a hosted MCP tool configuration, with server_url reduced to its origin."""
config = load_data_value(tool)
if not isinstance(config, dict):
return {}
safe = {key: config[key] for key in _SAFE_MCP_TOOL_FIELDS if config.get(key) is not None}
server_url = config.get("server_url")
origin = url_origin(server_url) if isinstance(server_url, str) else None
if origin:
safe["server_url"] = origin
return safe


def _safe_response_tools(tools: Any) -> Any:
"""Response tools as recorded in metadata: hosted MCP tools are reduced to their safe view."""
if not isinstance(tools, list):
return tools
return [
safe_mcp_tool_config(tool) if isinstance(tool, dict) and tool.get("type") == "mcp" else tool for tool in tools
]


def _openai_get_tool_definitions(tools: list[Any]) -> list[ToolDefinition]:
tool_definitions = []
for tool in tools:
Expand Down Expand Up @@ -1806,7 +1841,8 @@ def llmobs_metadata(self) -> Optional[dict[str, Any]]:
if hasattr(self.response, field):
value = getattr(self.response, field)
if value is not None:
metadata[field] = load_data_value(value)
value = load_data_value(value)
metadata[field] = _safe_response_tools(value) if field == "tools" else value

if hasattr(self.response, "text") and self.response.text:
metadata["text"] = load_data_value(self.response.text)
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
fixes:
- |
LLM Observability: Fixes an issue where OpenAI Agents spans recorded the credentials of hosted
MCP tools, such as tokens in the server URL, request headers and the ``authorization`` value.
Only the scheme, host and port of the MCP server URL and its non-secret settings are now recorded.
86 changes: 86 additions & 0 deletions tests/contrib/openai_agents/test_openai_agents_llmobs.py
Original file line number Diff line number Diff line change
@@ -1,11 +1,21 @@
import asyncio
import json
from typing import Optional
from unittest import mock

import openai
import pytest


try:
from openai.types.responses.tool import Mcp
except ImportError: # openai versions without hosted MCP tools
Mcp = None

from ddtrace.internal.utils.version import parse_version
from ddtrace.llmobs._integrations.openai_agents import OpenAIAgentsIntegration
from ddtrace.llmobs._integrations.utils import OaiSpanAdapter
from ddtrace.llmobs._integrations.utils import safe_mcp_tool_config
from ddtrace.llmobs._utils import _get_llmobs_data_metastruct
from ddtrace.llmobs._utils import get_llmobs_span_name
from tests.llmobs._utils import _assert_span_link
Expand Down Expand Up @@ -808,3 +818,79 @@ async def mock_func(*args, **kwargs):
args=(simple_agent,),
kwargs={"input": "What is the capital of France?"},
)


# Fake values shaped like real credentials. Prefixes are split so secret scanners do not flag them.
_MCP_PASSWORD = "Tr0ub4dor-3xample!"
_MCP_PATH_TOKEN = "NGQ3ZTk1YjItOGM0MS00ZTFhLWI2ZDMtOWYyYTdjMWU1ZDQ4"
_MCP_QUERY_KEY = "sk" + "-proj-" + "Zt8yQm3vLk2pW9xRn5bC7dF1gHj4sA6eUo0iTqYwErT"
_MCP_HEADER_TOKEN = "xo" + "xb-" + "1234567890123-4567890123456-Hq7Lm2Xw9Rt4Yb8Nc3Vd6Js1"
_MCP_OAUTH_TOKEN = "ya" + "29." + "a0AfB_byC7kq2ZxW9mLpT4vNbY8cHdF3sJ6aQe1uXo0iR"
_MCP_CREDENTIALS = (_MCP_PASSWORD, _MCP_PATH_TOKEN, _MCP_QUERY_KEY, _MCP_HEADER_TOKEN, _MCP_OAUTH_TOKEN, "/api/mcp/s/")
_LEAKY_MCP_TOOL_CONFIG = {
"type": "mcp",
"server_label": "zapier",
"server_description": "Zapier actions",
"server_url": f"https://svc:{_MCP_PASSWORD}@MCP.Zapier.com:443/api/mcp/s/{_MCP_PATH_TOKEN}/mcp?api_key={_MCP_QUERY_KEY}#x",
"headers": {"Authorization": f"Bearer {_MCP_HEADER_TOKEN}", "Cookie": "sessionid=8f2b"},
"authorization": _MCP_OAUTH_TOKEN,
"require_approval": "never",
"allowed_tools": ["send_email"],
}
_SAFE_MCP_TOOL_CONFIG = {
"type": "mcp",
"server_label": "zapier",
"server_description": "Zapier actions",
"server_url": "https://mcp.zapier.com",
"require_approval": "never",
"allowed_tools": ["send_email"],
}


def _assert_no_mcp_credentials(recorded):
serialized = json.dumps(recorded, default=str)
for credential in _MCP_CREDENTIALS + ("sessionid",):
assert credential not in serialized


def test_agent_manifest_keeps_only_the_safe_view_of_hosted_mcp_tools(agents):
"""A HostedMCPTool config is the application's own, so nothing redacts its credentials upstream."""
agent = agents.Agent(
name="mcp agent",
tools=[agents.HostedMCPTool(tool_config=dict(_LEAKY_MCP_TOOL_CONFIG))],
)

(tool,) = OpenAIAgentsIntegration._extract_tools_from_agent(None, agent)

assert tool["name"] == "hosted_mcp"
assert tool["tool_config"] == _SAFE_MCP_TOOL_CONFIG
_assert_no_mcp_credentials(tool)


@pytest.mark.skipif(Mcp is None, reason="openai has no hosted MCP tool type")
def test_response_metadata_keeps_only_the_safe_view_of_hosted_mcp_tools():
"""OpenAI echoes response.tools with header values redacted but server_url verbatim."""
echoed = dict(_LEAKY_MCP_TOOL_CONFIG, headers={"Authorization": "<redacted>"}, authorization=None)
function_tool = {"type": "function", "name": "lookup", "parameters": {"type": "object"}, "strict": True}
response = mock.Mock(spec=["tools"], tools=[Mcp.model_validate(echoed), function_tool])
span = mock.Mock(span_data=mock.Mock(type="response", response=response))

metadata = OaiSpanAdapter(span).llmobs_metadata

assert metadata["tools"] == [_SAFE_MCP_TOOL_CONFIG, function_tool]
_assert_no_mcp_credentials(metadata)


@pytest.mark.parametrize(
"server_url,expected",
[
(None, None),
("not a url", None),
("https://mcp.example.com:notaport/mcp", None),
],
)
def test_safe_mcp_tool_config_omits_unusable_urls(server_url, expected):
safe = safe_mcp_tool_config({"type": "mcp", "server_label": "x", "server_url": server_url})

assert safe.get("server_url") == expected
assert safe["server_label"] == "x"
23 changes: 23 additions & 0 deletions tests/tracer/test_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
from ddtrace.internal.utils.formats import flatten_key_value
from ddtrace.internal.utils.formats import is_sequence
from ddtrace.internal.utils.formats import parse_tags_str
from ddtrace.internal.utils.http import url_origin
from ddtrace.internal.utils.http import w3c_get_dd_list_member
from ddtrace.internal.utils.importlib import func_name
from ddtrace.trace import Context
Expand Down Expand Up @@ -673,3 +674,25 @@ def test_hourglass_turn():
def test_hourglass_sorting():
"""Test that we can sort hourglasses."""
sorted(time.HourGlass(1) for _ in range(100))


@pytest.mark.parametrize(
"url,expected",
[
("https://mcp.example.com/mcp", "https://mcp.example.com"),
("HTTPS://MCP.Example.COM:443/Mcp/", "https://mcp.example.com"),
("http://mcp.example.com:80/sse", "http://mcp.example.com"),
("http://mcp.example.com:8080/sse", "http://mcp.example.com:8080"),
("https://user:secret@mcp.example.com/mcp?token=abc#frag", "https://mcp.example.com"),
("https://mcp.example.com/api/mcp/s/s3cr3t-t0ken/mcp", "https://mcp.example.com"),
("https://mcp.example.com/mcp;params?q=1", "https://mcp.example.com"),
("http://[::1]:9000/mcp", "http://[::1]:9000"),
("https://mcp.example.com", "https://mcp.example.com"),
("https://mcp.example.com:notaport/mcp", None),
("/relative/path", None),
("not a url", None),
("", None),
],
)
def test_url_origin(url, expected):
assert url_origin(url) == expected
Loading