mirror of
https://github.com/All-Hands-AI/OpenHands.git
synced 2026-04-29 03:00:45 -04:00
fix(security): redact API keys from MCP config logging (#14020)
Co-authored-by: openhands <openhands@all-hands.dev>
This commit is contained in:
@@ -47,7 +47,7 @@ from openhands.server.session.agent_session import AgentSession
|
||||
from openhands.server.session.conversation_init_data import ConversationInitData
|
||||
from openhands.storage.data_models.settings import Settings
|
||||
from openhands.storage.files import FileStore
|
||||
from openhands.utils._redact_compat import sanitize_dict
|
||||
from openhands.utils._redact_compat import sanitize_config
|
||||
|
||||
|
||||
class WebSession:
|
||||
@@ -188,7 +188,7 @@ class WebSession:
|
||||
|
||||
# NOTE: this need to happen AFTER the config is updated with the search_api_key
|
||||
self.logger.debug(
|
||||
f'MCP configuration before setup - self.config.mcp_config: {sanitize_dict(self.config.mcp.model_dump())}'
|
||||
f'MCP configuration before setup - self.config.mcp_config: {sanitize_config(self.config.mcp.model_dump())}'
|
||||
)
|
||||
|
||||
# Merge user's custom MCP servers from settings
|
||||
@@ -196,7 +196,7 @@ class WebSession:
|
||||
if sdk_mcp and sdk_mcp.mcpServers:
|
||||
self.config.mcp = merge_mcp_configs(self.config.mcp, sdk_mcp)
|
||||
self.logger.debug(
|
||||
f'Merged custom MCP Config: {sanitize_dict(sdk_mcp.model_dump())}'
|
||||
f'Merged custom MCP Config: {sanitize_config(sdk_mcp.model_dump())}'
|
||||
)
|
||||
|
||||
# Add OpenHands' default MCP servers
|
||||
@@ -210,7 +210,7 @@ class WebSession:
|
||||
self.logger.debug('Added default MCP servers to config')
|
||||
|
||||
self.logger.debug(
|
||||
f'MCP configuration after setup - self.config.mcp: {sanitize_dict(self.config.mcp.model_dump())}'
|
||||
f'MCP configuration after setup - self.config.mcp: {sanitize_config(self.config.mcp.model_dump())}'
|
||||
)
|
||||
|
||||
# TODO: override other LLM config & agent config groups (#2075)
|
||||
|
||||
@@ -7,6 +7,7 @@ from openhands.utils._redact_compat import (
|
||||
redact_api_key_literals,
|
||||
redact_text_secrets,
|
||||
redact_url_params,
|
||||
sanitize_config,
|
||||
)
|
||||
|
||||
# The redaction placeholder
|
||||
@@ -118,3 +119,59 @@ class TestMCPConfigLoggingIntegration:
|
||||
assert 'sk-oh-realSessionKey456' not in log_output
|
||||
assert REDACTED in log_output
|
||||
assert 'http://localhost:8000/mcp/sse' in log_output # URL should be visible
|
||||
|
||||
|
||||
class TestSanitizeConfig:
|
||||
"""Tests for sanitize_config (dict-based redaction with URL param handling)."""
|
||||
|
||||
def test_redacts_url_query_params_in_mcp_config(self):
|
||||
"""Reproduce the exact Datadog leak: tavilyApiKey in URL query params."""
|
||||
config = {
|
||||
'mcpServers': {
|
||||
'tavily': {
|
||||
'url': 'https://mcp.tavily.com/mcp/?tavilyApiKey=tvly-realkey123',
|
||||
'transport': 'http',
|
||||
}
|
||||
}
|
||||
}
|
||||
sanitized = sanitize_config(config)
|
||||
assert 'tvly-realkey123' not in str(sanitized)
|
||||
assert 'mcp.tavily.com' in str(sanitized)
|
||||
|
||||
def test_redacts_header_api_keys_in_mcp_config(self):
|
||||
"""Reproduce the Datadog leak: X-Session-API-Key in headers."""
|
||||
config = {
|
||||
'mcpServers': {
|
||||
'myserver': {
|
||||
'url': 'https://example.com/mcp',
|
||||
'headers': {
|
||||
'X-Session-API-Key': 'sk-oh-realsessionkey456',
|
||||
},
|
||||
}
|
||||
}
|
||||
}
|
||||
sanitized = sanitize_config(config)
|
||||
assert 'sk-oh-realsessionkey456' not in str(sanitized)
|
||||
|
||||
def test_combined_url_and_header_secrets(self):
|
||||
"""Full scenario matching the production Datadog log pattern."""
|
||||
config = {
|
||||
'mcpServers': {
|
||||
'tavily': {
|
||||
'url': 'https://mcp.tavily.com/mcp/?tavilyApiKey=tvly-realkey123',
|
||||
'transport': 'http',
|
||||
},
|
||||
'internal': {
|
||||
'url': 'https://internal.example.com/mcp',
|
||||
'headers': {
|
||||
'X-Session-API-Key': 'sk-oh-realsessionkey456',
|
||||
},
|
||||
},
|
||||
}
|
||||
}
|
||||
sanitized = sanitize_config(config)
|
||||
output = str(sanitized)
|
||||
assert 'tvly-realkey123' not in output
|
||||
assert 'sk-oh-realsessionkey456' not in output
|
||||
assert 'tavily' in output
|
||||
assert 'internal' in output
|
||||
|
||||
Reference in New Issue
Block a user