fix(mcp): redact credentials from URLs before logging
MCP server URLs can carry secrets in userinfo (`https://user:token@host/sse`) or a query string (`?token=...`). A few connect/validate paths logged the raw `cfg.url` / `request.url`, so those secrets could land in log files that are often shared or aggregated. Add a small `_redact_url()` helper that keeps only scheme/host/port/path and use it at the four sites that log a server or request URL. Logging only; no other behavior changes.
This commit is contained in:
committed by
Xubin Ren
parent
1873e948c3
commit
780093d037
@@ -166,12 +166,28 @@ async def _probe_http_url(url: str, timeout: float = 3.0) -> bool:
|
|||||||
return False
|
return False
|
||||||
|
|
||||||
|
|
||||||
|
def _redact_url(url: str) -> str:
|
||||||
|
"""Strip credentials and query/fragment before logging an MCP URL.
|
||||||
|
|
||||||
|
Server URLs may embed secrets (``https://user:token@host/sse`` or a
|
||||||
|
``?token=`` query); only scheme, host, port, and path are safe to log.
|
||||||
|
"""
|
||||||
|
try:
|
||||||
|
parts = urllib.parse.urlsplit(url)
|
||||||
|
netloc = parts.hostname or ""
|
||||||
|
if parts.port:
|
||||||
|
netloc = f"{netloc}:{parts.port}"
|
||||||
|
return urllib.parse.urlunsplit((parts.scheme, netloc, parts.path, "", ""))
|
||||||
|
except Exception:
|
||||||
|
return "<redacted-url>"
|
||||||
|
|
||||||
|
|
||||||
async def _validate_mcp_request_url(request: httpx.Request) -> None:
|
async def _validate_mcp_request_url(request: httpx.Request) -> None:
|
||||||
"""Validate each outgoing MCP HTTP request, including redirect targets."""
|
"""Validate each outgoing MCP HTTP request, including redirect targets."""
|
||||||
ok, error = validate_url_target(str(request.url))
|
ok, error = validate_url_target(str(request.url))
|
||||||
if not ok:
|
if not ok:
|
||||||
raise httpx.RequestError(
|
raise httpx.RequestError(
|
||||||
f"Blocked unsafe MCP URL {request.url} ({error})",
|
f"Blocked unsafe MCP URL {_redact_url(str(request.url))} ({error})",
|
||||||
request=request,
|
request=request,
|
||||||
)
|
)
|
||||||
|
|
||||||
@@ -774,7 +790,7 @@ async def connect_mcp_servers(
|
|||||||
logger.warning(
|
logger.warning(
|
||||||
"MCP server '{}': blocked unsafe URL {} ({})",
|
"MCP server '{}': blocked unsafe URL {} ({})",
|
||||||
name,
|
name,
|
||||||
cfg.url,
|
_redact_url(cfg.url),
|
||||||
error,
|
error,
|
||||||
)
|
)
|
||||||
await server_stack.aclose()
|
await server_stack.aclose()
|
||||||
@@ -795,7 +811,7 @@ async def connect_mcp_servers(
|
|||||||
read, write = await server_stack.enter_async_context(stdio_client(params))
|
read, write = await server_stack.enter_async_context(stdio_client(params))
|
||||||
elif transport_type == "sse":
|
elif transport_type == "sse":
|
||||||
if not await _probe_http_url(cfg.url):
|
if not await _probe_http_url(cfg.url):
|
||||||
logger.warning("MCP server '{}': {} unreachable, skipping", name, cfg.url)
|
logger.warning("MCP server '{}': {} unreachable, skipping", name, _redact_url(cfg.url))
|
||||||
await server_stack.aclose()
|
await server_stack.aclose()
|
||||||
return name, None
|
return name, None
|
||||||
|
|
||||||
@@ -822,7 +838,7 @@ async def connect_mcp_servers(
|
|||||||
)
|
)
|
||||||
elif transport_type == "streamableHttp":
|
elif transport_type == "streamableHttp":
|
||||||
if not await _probe_http_url(cfg.url):
|
if not await _probe_http_url(cfg.url):
|
||||||
logger.warning("MCP server '{}': {} unreachable, skipping", name, cfg.url)
|
logger.warning("MCP server '{}': {} unreachable, skipping", name, _redact_url(cfg.url))
|
||||||
await server_stack.aclose()
|
await server_stack.aclose()
|
||||||
return name, None
|
return name, None
|
||||||
|
|
||||||
|
|||||||
@@ -1240,3 +1240,15 @@ async def test_connect_mcp_servers_enabled_tools_matches_sanitized_name(
|
|||||||
await stack.aclose()
|
await stack.aclose()
|
||||||
|
|
||||||
assert registry.tool_names == ["mcp_test_My_Tool"]
|
assert registry.tool_names == ["mcp_test_My_Tool"]
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.parametrize(
|
||||||
|
"url, expected",
|
||||||
|
[
|
||||||
|
("https://user:secret@host.example/sse", "https://host.example/sse"),
|
||||||
|
("https://host.example:8443/mcp?token=abc#frag", "https://host.example:8443/mcp"),
|
||||||
|
("https://host.example/sse", "https://host.example/sse"),
|
||||||
|
],
|
||||||
|
)
|
||||||
|
def test_redact_url_strips_credentials_and_query(url: str, expected: str) -> None:
|
||||||
|
assert mcp_mod._redact_url(url) == expected
|
||||||
|
|||||||
Reference in New Issue
Block a user