fix(mcp): bound elicitation channel, warn on sensitive fields (#2524, #2523)

Replace the unbounded elicitation mpsc channel in McpManager with a bounded
channel (default capacity 16). Requests arriving when the queue is full are
auto-declined with a warning log, preventing memory exhaustion from misbehaving
or malicious MCP servers. Capacity is configurable via:

    [mcp]
    elicitation_queue_capacity = 16  # default

Add sensitive-field detection to the elicitation handler. Before prompting,
field names are matched case-insensitively against a list of sensitive patterns
(password, token, secret, key, credential, auth, private, passphrase, pin).
If any match, a warning is shown with the server name and field name so the
user can make an informed trust decision. Configurable via:

    [mcp]
    elicitation_warn_sensitive_fields = true  # default

Also fix three pre-existing clippy warnings in the elicitation CLI prompt
builder (match-to-if-let, uninlined format args) and a stale test assertion.

Closes #2524, closes #2523

Author: bug-ops

CHANGELOG.md

@@ -13,6 +13,13 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/).
 
 ### Fixed
 
+- fix(mcp): replace unbounded elicitation mpsc channel with a bounded channel (default capacity 16) to prevent memory exhaustion from misbehaving MCP servers; requests that arrive when the queue is full are auto-declined with a warning log instead of accumulating indefinitely; capacity is configurable via `[mcp] elicitation_queue_capacity` (closes #2524)
+- fix(mcp): pre-existing `clippy::non_exhaustive_omitted_patterns`, `match_single_binding`, and `uninlined_format_args` warnings in elicitation CLI prompt builder and test code (caught while adding bounded-channel support)
+
+### Added
+
+- security(mcp): warn user before prompting for elicitation fields whose names match sensitive patterns (password, token, secret, key, credential, auth, private, passphrase, pin, etc.); warning shows the server name and field name so the user can make an informed decision; configurable via `[mcp] elicitation_warn_sensitive_fields` (default `true`) (closes #2523)
+
 - skills: raise `disambiguation_threshold` default from 0.05 to 0.20 to prevent low-confidence skill injection (#2512)
 - skills: add `min_injection_score` config field (default 0.20) — skills scoring below threshold are no longer injected (#2512)
 - skills: fix `process-management` SKILL.md false positive on user queries containing "memory" by replacing with "RAM" (#2513)

crates/zeph-config/src/channels.rs

@@ -343,6 +343,10 @@ fn default_elicitation_timeout() -> u64 {
     120
 }
 
+fn default_elicitation_queue_capacity() -> usize {
+    16
+}
+
 #[derive(Debug, Clone, Deserialize, Serialize)]
 pub struct McpConfig {
     #[serde(default)]
@@ -374,6 +378,15 @@ pub struct McpConfig {
     /// Timeout for user to respond to an elicitation request (seconds). Default: 120.
     #[serde(default = "default_elicitation_timeout")]
     pub elicitation_timeout: u64,
+    /// Bounded channel capacity for elicitation events. Requests beyond this limit are
+    /// auto-declined with a warning to prevent memory exhaustion from misbehaving servers.
+    /// Default: 16.
+    #[serde(default = "default_elicitation_queue_capacity")]
+    pub elicitation_queue_capacity: usize,
+    /// When true, warn the user before prompting for fields whose names match sensitive
+    /// patterns (password, token, secret, key, credential, etc.). Default: true.
+    #[serde(default = "default_true")]
+    pub elicitation_warn_sensitive_fields: bool,
 }
 
 impl Default for McpConfig {
@@ -389,6 +402,8 @@ impl Default for McpConfig {
             max_instructions_bytes: default_max_instructions_bytes(),
             elicitation_enabled: false,
             elicitation_timeout: default_elicitation_timeout(),
+            elicitation_queue_capacity: default_elicitation_queue_capacity(),
+            elicitation_warn_sensitive_fields: true,
         }
     }
 }

crates/zeph-core/src/agent/builder.rs

@@ -542,6 +542,7 @@ impl<C: Channel> Agent<C> {
             .allowed_commands
             .clone_from(&mcp_config.allowed_commands);
         self.mcp.max_dynamic = mcp_config.max_dynamic_servers;
+        self.mcp.elicitation_warn_sensitive_fields = mcp_config.elicitation_warn_sensitive_fields;
         self
     }
 
@@ -617,7 +618,7 @@ impl<C: Channel> Agent<C> {
     #[must_use]
     pub fn with_mcp_elicitation_rx(
         mut self,
-        rx: tokio::sync::mpsc::UnboundedReceiver<zeph_mcp::ElicitationEvent>,
+        rx: tokio::sync::mpsc::Receiver<zeph_mcp::ElicitationEvent>,
     ) -> Self {
         self.mcp.elicitation_rx = Some(rx);
         self

crates/zeph-core/src/agent/mcp.rs

@@ -542,6 +542,29 @@ impl<C: Channel> Agent<C> {
             }
         };
 
+        if self.mcp.elicitation_warn_sensitive_fields {
+            let sensitive: Vec<&str> = channel_request
+                .fields
+                .iter()
+                .filter(|f| is_sensitive_field(&f.name))
+                .map(|f| f.name.as_str())
+                .collect();
+            if !sensitive.is_empty() {
+                let fields_list = sensitive.join(", ");
+                let warning = format!(
+                    "Warning: [{}] is requesting sensitive information (field: {}). \
+                     Only proceed if you trust this server.",
+                    channel_request.server_name, fields_list,
+                );
+                tracing::warn!(
+                    server_id = event.server_id,
+                    fields = %fields_list,
+                    "elicitation requests sensitive fields"
+                );
+                let _ = self.channel.send(&warning).await;
+            }
+        }
+
         let _ = self
             .channel
             .send_status("MCP server requesting input…")
@@ -682,6 +705,31 @@ fn build_elicitation_fields(
         .collect()
 }
 
+/// Sensitive field name patterns (case-insensitive substring match).
+const SENSITIVE_FIELD_PATTERNS: &[&str] = &[
+    "password",
+    "passwd",
+    "token",
+    "secret",
+    "key",
+    "credential",
+    "apikey",
+    "api_key",
+    "auth",
+    "authorization",
+    "private",
+    "passphrase",
+    "pin",
+];
+
+/// Returns `true` when `field_name` matches any sensitive pattern (case-insensitive).
+fn is_sensitive_field(field_name: &str) -> bool {
+    let lower = field_name.to_lowercase();
+    SENSITIVE_FIELD_PATTERNS
+        .iter()
+        .any(|pattern| lower.contains(pattern))
+}
+
 /// Sanitize an elicitation message: cap length (in chars, not bytes) and strip control chars.
 fn sanitize_elicitation_message(message: &str) -> String {
     const MAX_CHARS: usize = 500;
@@ -942,7 +990,7 @@ mod tests {
         let input: String = "é".repeat(300); // 300 chars = 600 bytes
         let output = sanitize_elicitation_message(&input);
         // Should truncate to exactly 500 chars without panic.
-        assert_eq!(output.chars().count(), 300.min(500));
+        assert_eq!(output.chars().count(), 300);
     }
 
     #[test]
@@ -1018,4 +1066,24 @@ mod tests {
         assert!(req.required);
         assert!(!opt.required);
     }
+
+    #[test]
+    fn is_sensitive_field_detects_common_patterns() {
+        assert!(is_sensitive_field("password"));
+        assert!(is_sensitive_field("PASSWORD"));
+        assert!(is_sensitive_field("user_password"));
+        assert!(is_sensitive_field("api_token"));
+        assert!(is_sensitive_field("SECRET_KEY"));
+        assert!(is_sensitive_field("auth_header"));
+        assert!(is_sensitive_field("private_key"));
+    }
+
+    #[test]
+    fn is_sensitive_field_allows_non_sensitive_names() {
+        assert!(!is_sensitive_field("username"));
+        assert!(!is_sensitive_field("email"));
+        assert!(!is_sensitive_field("message"));
+        assert!(!is_sensitive_field("description"));
+        assert!(!is_sensitive_field("subject"));
+    }
 }

crates/zeph-core/src/agent/mod.rs

@@ -405,6 +405,7 @@ impl<C: Channel> Agent<C> {
                 discovery_strategy: zeph_mcp::ToolDiscoveryStrategy::default(),
                 discovery_params: zeph_mcp::DiscoveryParams::default(),
                 discovery_provider: None,
+                elicitation_warn_sensitive_fields: true,
             },
             index: IndexState {
                 retriever: None,

crates/zeph-core/src/agent/state/mod.rs

@@ -109,8 +109,7 @@ pub(crate) struct McpState {
     /// Receives elicitation requests from MCP server handlers during tool execution.
     /// When `Some`, the agent loop must process these concurrently with tool result awaiting
     /// to avoid deadlock (tool result waits for elicitation, elicitation waits for agent loop).
-    pub(crate) elicitation_rx:
-        Option<tokio::sync::mpsc::UnboundedReceiver<zeph_mcp::ElicitationEvent>>,
+    pub(crate) elicitation_rx: Option<tokio::sync::mpsc::Receiver<zeph_mcp::ElicitationEvent>>,
     /// Shared with `McpToolExecutor` so native `tool_use` sees the current tool list.
     ///
     /// Two methods write to this `RwLock` — ordering matters:
@@ -154,6 +153,9 @@ pub(crate) struct McpState {
     /// Dedicated embedding provider for tool discovery.  `None` = fall back to the
     /// agent's primary embedding provider.
     pub(crate) discovery_provider: Option<zeph_llm::any::AnyProvider>,
+    /// When `true`, show a security warning before prompting for fields whose names
+    /// match sensitive patterns (password, token, secret, key, credential, etc.).
+    pub(crate) elicitation_warn_sensitive_fields: bool,
 }
 
 pub(crate) struct IndexState {

crates/zeph-core/src/bootstrap/mcp.rs

@@ -68,13 +68,17 @@ pub fn create_mcp_manager_with_vault(
         .map(|s| (s.id.clone(), s.policy.clone()))
         .collect();
     let enforcer = zeph_mcp::PolicyEnforcer::new(policy_entries);
-    let mut manager =
-        zeph_mcp::McpManager::new(entries, config.mcp.allowed_commands.clone(), enforcer)
-            .with_suppress_stderr(suppress_stderr)
-            .with_description_limits(
-                config.mcp.max_description_bytes,
-                config.mcp.max_instructions_bytes,
-            );
+    let mut manager = zeph_mcp::McpManager::with_elicitation_capacity(
+        entries,
+        config.mcp.allowed_commands.clone(),
+        enforcer,
+        config.mcp.elicitation_queue_capacity,
+    )
+    .with_suppress_stderr(suppress_stderr)
+    .with_description_limits(
+        config.mcp.max_description_bytes,
+        config.mcp.max_instructions_bytes,
+    );
 
     // Register OAuth credential stores
     for s in &config.mcp.servers {

crates/zeph-mcp/src/client.rs

@@ -19,7 +19,7 @@ use rmcp::transport::streamable_http_client::{
     StreamableHttpClientTransport, StreamableHttpClientTransportConfig,
 };
 use tokio::process::Command;
-use tokio::sync::mpsc::UnboundedSender;
+use tokio::sync::mpsc::{Sender, UnboundedSender};
 use tokio::sync::oneshot;
 use url::Url;
 
@@ -67,7 +67,7 @@ pub struct HandlerConfig {
     pub max_description_bytes: usize,
     /// When `Some`, elicitation requests are forwarded to the agent loop.
     /// When `None`, all requests are auto-declined.
-    pub elicitation_tx: Option<UnboundedSender<ElicitationEvent>>,
+    pub elicitation_tx: Option<Sender<ElicitationEvent>>,
     /// Elicitation response timeout.
     pub elicitation_timeout: Duration,
 }
@@ -91,7 +91,7 @@ pub struct ToolListChangedHandler {
     max_description_bytes: usize,
     /// When `Some`, elicitation requests are forwarded to the agent loop.
     /// When `None`, all elicitation requests are declined.
-    elicitation_tx: Option<UnboundedSender<ElicitationEvent>>,
+    elicitation_tx: Option<Sender<ElicitationEvent>>,
     /// Timeout for the user to respond to an elicitation request.
     elicitation_timeout: Duration,
 }
@@ -103,7 +103,7 @@ impl ToolListChangedHandler {
         last_refresh: Arc<DashMap<String, Instant>>,
         roots: Arc<Vec<rmcp::model::Root>>,
         max_description_bytes: usize,
-        elicitation_tx: Option<UnboundedSender<ElicitationEvent>>,
+        elicitation_tx: Option<Sender<ElicitationEvent>>,
         elicitation_timeout: Duration,
     ) -> Self {
         Self {
@@ -163,12 +163,22 @@ impl rmcp::ClientHandler for ToolListChangedHandler {
                 response_tx,
             };
 
-            if tx.send(event).is_err() {
-                tracing::warn!(
-                    server_id = self.server_id,
-                    "elicitation channel closed — agent loop may have shut down"
-                );
-                return Ok(decline);
+            match tx.try_send(event) {
+                Ok(()) => {}
+                Err(tokio::sync::mpsc::error::TrySendError::Full(_)) => {
+                    tracing::warn!(
+                        server_id = self.server_id,
+                        "elicitation queue full — auto-declining request from misbehaving server"
+                    );
+                    return Ok(decline);
+                }
+                Err(tokio::sync::mpsc::error::TrySendError::Closed(_)) => {
+                    tracing::warn!(
+                        server_id = self.server_id,
+                        "elicitation channel closed — agent loop may have shut down"
+                    );
+                    return Ok(decline);
+                }
             }
 
             match tokio::time::timeout(self.elicitation_timeout, response_rx).await {
@@ -315,7 +325,7 @@ pub struct OAuthPending {
     pub last_refresh: Arc<DashMap<String, Instant>>,
     pub roots: Arc<Vec<rmcp::model::Root>>,
     pub max_description_bytes: usize,
-    pub elicitation_tx: Option<UnboundedSender<ElicitationEvent>>,
+    pub elicitation_tx: Option<Sender<ElicitationEvent>>,
     pub elicitation_timeout: Duration,
 }