Files
optimclaw/src/tools/mcp/process.rs
T
02f85a8ad5 feat(mcp): transport abstraction, stdio/UDS transports, and OAuth fixes (#721)
* feat(mcp): transport abstraction, stdio/UDS transports, and OAuth fixes

Extract McpTransport trait from HTTP-coupled McpClient, enabling pluggable
transport backends. Implements stdio and Unix domain socket transports for
local MCP server integration, fixes OAuth discovery per RFC 9728, and adds
SSRF protection.

Transport abstraction (Step 2):
- McpTransport trait with send(), shutdown(), supports_http_features()
- HttpMcpTransport extracted from McpClient with SSE parsing, session tracking
- Shared JSON-RPC framing helpers (write_jsonrpc_line, spawn_jsonrpc_reader)
- McpClient refactored to hold Arc<dyn McpTransport>

Stdio transport (#652, Step 4):
- StdioMcpTransport spawns child process, communicates via stdin/stdout
- McpProcessManager for lifecycle management with exponential backoff restart
- Background stderr drain task for debug logging

Unix domain socket transport (#134, Step 5):
- UnixMcpTransport connects to existing Unix sockets
- Reuses shared JSON-RPC framing from transport.rs

HTML error body sanitization (#263, Step 1):
- sanitize_error_body() detects HTML, strips control chars, truncates to 500

Custom headers (#639, Step 3):
- headers field on McpServerConfig, merged into every HTTP request
- --header CLI arg for `mcp add`

Config and CLI updates (Step 6):
- McpTransportConfig tagged enum (Http/Stdio/Unix) with serde support
- EffectiveTransport for zero-copy config dispatch
- CLI: --transport, --command, --arg, --env, --socket flags for `mcp add`
- `mcp list` shows transport type

OAuth fixes (#299, Step 8):
- Multi-strategy discovery (401-based, RFC 9728, direct)
- RFC 8707 resource parameter in auth and refresh flows
- SSRF protection with IPv4-mapped IPv6 bypass detection
- Well-known URI construction per RFC 8414

Closes #652, #134, #639, #263, #299

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* fix(mcp): address audit findings from crate review

- Fix SSRF bypass: make validate_url_safe async with DNS resolution to
  block hostnames that resolve to private/link-local IPs
- Fix UTF-8 truncation: use char-based truncation in sanitize_error_body
  to avoid panicking on multi-byte characters
- Fix SSE parser: process only complete lines to handle chunks split
  across boundaries, add 10MB buffer size limit
- Add debug_assert for transport type mismatch in new_with_config
- Propagate custom headers in new_with_transport constructor
- Deduplicate effective_transport() calls in CLI list command
- Gate test-only accessors with #[cfg(test)] to eliminate dead_code warnings
- Document JSON-RPC notification id:0 limitation in protocol.rs
- Document total backoff wait time (31s) in process.rs
- Add regression test for multi-byte UTF-8 truncation

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* fix(mcp): address PR review findings from Copilot, Gemini, and zmanian

Moderate/High fixes:
- Plumb custom headers through new_authenticated constructor
- Restrict HTTP to localhost only in validate_url_safe (prevent
  plaintext credential leaks over non-localhost HTTP)
- Add mcp_process_manager.shutdown_all() to app shutdown path to
  prevent orphaning stdio child processes
- Validate discovered authorization_url before opening browser
  (prevent malicious MCP server redirecting to phishing page)

Medium fixes:
- Upgrade debug_assert to assert in new_with_config (fires in release)
- Remove pending map entry on Ok(Err(_)) in stdio/unix send() to avoid
  stale entries and unnecessary 30s waits
- Shut down old transport in try_restart() before spawning replacement
- Redact env var values in mcp list --verbose (may contain secrets)
- Drain pending requests on shutdown to wake waiters immediately
- Add IPv6 link-local, site-local, unique-local, and documentation
  ranges to is_dangerous_ip SSRF protection

Low fixes:
- Truncate logged JSON parse error lines to 200 chars (prevent
  sensitive data in logs)
- Remove misleading shutdown comment in unix_transport
- Use tempfile::tempdir() instead of hardcoded /tmp/ path in test
- Adopt main's improved sanitize_error_body (HTML tag stripping,
  200-char truncation with char_indices)

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* fix(mcp): gate unix_transport with #[cfg(unix)] for Windows compat

- Add #[cfg(unix)] to unix_transport module declaration
- Add #[cfg(unix)]/#[cfg(not(unix))] branches in app.rs for Unix
  socket MCP server setup
- Remove unused sanitize_error_body import in client.rs tests

[skip-regression-check]

---------

Co-authored-by: Claude Opus 4.6 <[email protected]>
2026-03-09 02:47:42 +00:00

207 lines
6.1 KiB
Rust

//! MCP stdio process manager.
//!
//! Manages the lifecycle of MCP servers running as child processes.
//! Handles spawning, shutdown, and crash recovery with exponential backoff.
use std::collections::HashMap;
use std::sync::Arc;
use std::time::Duration;
use tokio::sync::RwLock;
use crate::tools::mcp::stdio_transport::StdioMcpTransport;
use crate::tools::mcp::transport::McpTransport;
use crate::tools::tool::ToolError;
/// Configuration for spawning a stdio MCP server.
#[derive(Debug, Clone)]
pub struct StdioSpawnConfig {
pub command: String,
pub args: Vec<String>,
pub env: HashMap<String, String>,
}
/// Manages stdio MCP server processes.
///
/// Handles spawning, tracking, and shutdown of child processes.
pub struct McpProcessManager {
transports: RwLock<HashMap<String, Arc<StdioMcpTransport>>>,
configs: RwLock<HashMap<String, StdioSpawnConfig>>,
}
impl McpProcessManager {
pub fn new() -> Self {
Self {
transports: RwLock::new(HashMap::new()),
configs: RwLock::new(HashMap::new()),
}
}
/// Spawn a new stdio MCP server process.
pub async fn spawn_stdio(
&self,
name: impl Into<String>,
command: impl Into<String>,
args: Vec<String>,
env: HashMap<String, String>,
) -> Result<Arc<StdioMcpTransport>, ToolError> {
let name = name.into();
let command = command.into();
// Store config for potential restart
self.configs.write().await.insert(
name.clone(),
StdioSpawnConfig {
command: command.clone(),
args: args.clone(),
env: env.clone(),
},
);
let transport = Arc::new(StdioMcpTransport::spawn(&name, &command, args, env).await?);
self.transports
.write()
.await
.insert(name, Arc::clone(&transport));
Ok(transport)
}
/// Get a transport by server name.
pub async fn get(&self, name: &str) -> Option<Arc<StdioMcpTransport>> {
self.transports.read().await.get(name).cloned()
}
/// Shut down all managed transports.
pub async fn shutdown_all(&self) {
let transports: Vec<(String, Arc<StdioMcpTransport>)> = {
let mut map = self.transports.write().await;
map.drain().collect()
};
for (name, transport) in transports {
if let Err(e) = transport.shutdown().await {
tracing::warn!("Failed to shut down MCP stdio server '{}': {}", name, e);
}
}
}
/// Shut down a specific transport by name.
pub async fn shutdown(&self, name: &str) -> Result<(), ToolError> {
let transport = self.transports.write().await.remove(name);
if let Some(transport) = transport {
transport.shutdown().await?;
}
self.configs.write().await.remove(name);
Ok(())
}
/// Attempt to restart a crashed transport with exponential backoff.
///
/// Tries up to 5 times with delays of 1s, 2s, 4s, 8s, 16s (total: 31s max wait).
pub async fn try_restart(&self, name: &str) -> Result<Arc<StdioMcpTransport>, ToolError> {
let config = self
.configs
.read()
.await
.get(name)
.cloned()
.ok_or_else(|| {
ToolError::ExternalService(format!(
"No spawn config for MCP server '{}', cannot restart",
name
))
})?;
// Shut down and remove old transport to avoid orphaning a wedged process.
if let Some(old_transport) = self.transports.write().await.remove(name) {
let _ = old_transport.shutdown().await;
}
let max_retries = 5;
let mut last_err = None;
for attempt in 0..max_retries {
let delay = Duration::from_secs(1 << attempt);
tokio::time::sleep(delay).await;
match StdioMcpTransport::spawn(
name,
&config.command,
config.args.clone(),
config.env.clone(),
)
.await
{
Ok(transport) => {
let transport = Arc::new(transport);
self.transports
.write()
.await
.insert(name.to_string(), Arc::clone(&transport));
tracing::info!(
"MCP stdio server '{}' restarted after {} attempt(s)",
name,
attempt + 1
);
return Ok(transport);
}
Err(e) => {
tracing::warn!(
"Restart attempt {}/{} for MCP server '{}' failed: {}",
attempt + 1,
max_retries,
name,
e
);
last_err = Some(e);
}
}
}
Err(last_err.unwrap_or_else(|| {
ToolError::ExternalService(format!(
"Failed to restart MCP server '{}' after {} attempts",
name, max_retries
))
}))
}
/// Get names of all managed transports.
pub async fn managed_servers(&self) -> Vec<String> {
self.transports.read().await.keys().cloned().collect()
}
}
impl Default for McpProcessManager {
fn default() -> Self {
Self::new()
}
}
#[cfg(test)]
mod tests {
use super::*;
#[test]
fn test_new_creates_empty_manager() {
let _manager = McpProcessManager::new();
}
#[tokio::test]
async fn test_managed_servers_returns_empty_list_initially() {
let manager = McpProcessManager::new();
let servers = manager.managed_servers().await;
assert!(servers.is_empty());
}
#[tokio::test]
async fn test_shutdown_all_on_empty_manager_does_not_panic() {
let manager = McpProcessManager::new();
manager.shutdown_all().await;
}
}