From 5e13e3a781dc6143e8322f50dcfd90093e199117 Mon Sep 17 00:00:00 2001 From: laststylebender14 Date: Tue, 12 May 2026 15:35:16 +0530 Subject: [PATCH 1/7] feat(mcp): add interactive trust gate for project-local configs --- crates/forge_api/src/api.rs | 10 +- crates/forge_api/src/forge_api.rs | 17 +- crates/forge_app/src/services.rs | 41 ++++- crates/forge_domain/src/env.rs | 5 + crates/forge_domain/src/mcp.rs | 188 ++++++++++++++++++++++- crates/forge_main/src/ui.rs | 72 ++++++++- crates/forge_services/src/mcp/manager.rs | 85 +++++++++- crates/forge_services/src/mcp/service.rs | 33 ++-- 8 files changed, 424 insertions(+), 27 deletions(-) diff --git a/crates/forge_api/src/api.rs b/crates/forge_api/src/api.rs index aafb112d49..47ac8c7132 100644 --- a/crates/forge_api/src/api.rs +++ b/crates/forge_api/src/api.rs @@ -1,9 +1,9 @@ -use std::path::PathBuf; +use std::path::{Path, PathBuf}; use anyhow::Result; use forge_app::dto::ToolsOverview; use forge_app::{User, UserUsage}; -use forge_domain::{AgentId, Effort, ModelId, ProviderModels}; +use forge_domain::{AgentId, Effort, McpTrustStatus, ModelId, ProviderModels}; use forge_stream::MpscStream; use futures::stream::BoxStream; use url::Url; @@ -175,6 +175,12 @@ pub trait API: Sync + Send { /// Refresh MCP caches by fetching fresh data async fn reload_mcp(&self) -> Result<()>; + /// Queries the trust status of the given MCP config file. + async fn get_mcp_trust_status(&self, path: &Path) -> Result; + + /// Persists a trust decision for the given MCP config file. + async fn set_mcp_trust(&self, path: &Path, status: McpTrustStatus) -> Result<()>; + /// List of commands defined in .md file(s) async fn get_commands(&self) -> Result>; diff --git a/crates/forge_api/src/forge_api.rs b/crates/forge_api/src/forge_api.rs index b56d485bfd..c69ac482e3 100644 --- a/crates/forge_api/src/forge_api.rs +++ b/crates/forge_api/src/forge_api.rs @@ -1,4 +1,4 @@ -use std::path::PathBuf; +use std::path::{Path, PathBuf}; use std::sync::Arc; use std::time::Duration; @@ -306,6 +306,21 @@ impl< async fn reload_mcp(&self) -> Result<()> { self.services.mcp_service().reload_mcp().await } + + async fn get_mcp_trust_status(&self, path: &Path) -> Result { + self.services + .mcp_config_manager() + .get_mcp_trust_status(path) + .await + } + + async fn set_mcp_trust(&self, path: &Path, status: McpTrustStatus) -> Result<()> { + self.services + .mcp_config_manager() + .set_mcp_trust(path, status) + .await + } + async fn get_commands(&self) -> Result> { self.services.get_commands().await } diff --git a/crates/forge_app/src/services.rs b/crates/forge_app/src/services.rs index 78ab0ca533..f10f5c945b 100644 --- a/crates/forge_app/src/services.rs +++ b/crates/forge_app/src/services.rs @@ -6,9 +6,9 @@ use derive_setters::Setters; use forge_domain::{ AgentId, AnyProvider, Attachment, AuthContextRequest, AuthContextResponse, AuthMethod, ChatCompletionMessage, CommandOutput, Context, Conversation, ConversationId, File, FileInfo, - FileStatus, Image, McpConfig, McpServers, Model, ModelId, Node, Provider, ProviderId, - ResultStream, Scope, SearchParams, SyncProgress, SyntaxError, Template, ToolCallFull, - ToolOutput, WorkspaceAuth, WorkspaceId, WorkspaceInfo, + FileStatus, Image, McpConfig, McpServers, McpTrustStatus, Model, ModelId, Node, Provider, + ProviderId, ResultStream, Scope, SearchParams, SyncProgress, SyntaxError, Template, + ToolCallFull, ToolOutput, WorkspaceAuth, WorkspaceId, WorkspaceInfo, }; use forge_eventsource::EventSource; use reqwest::Response; @@ -214,6 +214,29 @@ pub trait McpConfigManager: Send + Sync { /// Responsible for writing the McpConfig on disk. async fn write_mcp_config(&self, config: &McpConfig, scope: &Scope) -> anyhow::Result<()>; + + /// Queries the trust status of the given MCP config file. + /// + /// Returns the persisted status (`Trusted`, `Rejected`) for the file's + /// current contents, or `Unknown` if no decision has been recorded for + /// this file at its current hash. + /// + /// # Errors + /// Returns an error if the file cannot be read or parsed. + async fn get_mcp_trust_status(&self, path: &Path) -> anyhow::Result; + + /// Persists a trust decision for the given MCP config file. + /// + /// Passing `McpTrustStatus::Unknown` clears any previously recorded + /// decision for the path. + async fn set_mcp_trust(&self, path: &Path, status: McpTrustStatus) -> anyhow::Result<()>; + + /// Drops untrusted servers from `raw`. + /// + /// User-scope servers are always retained. Project-local servers are + /// retained only when the local config has been explicitly trusted at + /// its current content hash. + async fn filter_trusted(&self, raw: McpConfig) -> anyhow::Result; } #[async_trait::async_trait] @@ -680,6 +703,18 @@ impl McpConfigManager for I { .write_mcp_config(config, scope) .await } + + async fn get_mcp_trust_status(&self, path: &Path) -> anyhow::Result { + self.mcp_config_manager().get_mcp_trust_status(path).await + } + + async fn set_mcp_trust(&self, path: &Path, status: McpTrustStatus) -> anyhow::Result<()> { + self.mcp_config_manager().set_mcp_trust(path, status).await + } + + async fn filter_trusted(&self, raw: McpConfig) -> anyhow::Result { + self.mcp_config_manager().filter_trusted(raw).await + } } #[async_trait::async_trait] diff --git a/crates/forge_domain/src/env.rs b/crates/forge_domain/src/env.rs index 7e6ee30601..6f2d0ab076 100644 --- a/crates/forge_domain/src/env.rs +++ b/crates/forge_domain/src/env.rs @@ -82,6 +82,11 @@ impl Environment { self.base_path.join(".mcp.json") } + /// Returns the path where the MCP trust store is persisted across restarts. + pub fn mcp_trust_path(&self) -> PathBuf { + self.base_path.join(".mcp_trust.json") + } + pub fn agent_path(&self) -> PathBuf { self.base_path.join("agents") } diff --git a/crates/forge_domain/src/mcp.rs b/crates/forge_domain/src/mcp.rs index 53afe53725..197081f185 100644 --- a/crates/forge_domain/src/mcp.rs +++ b/crates/forge_domain/src/mcp.rs @@ -3,6 +3,7 @@ use std::collections::BTreeMap; use std::ops::Deref; +use std::path::Path; use derive_more::{Deref, Display, From}; use derive_setters::Setters; @@ -288,23 +289,119 @@ impl From> for McpConfig { } impl McpConfig { - /// Compute a deterministic u64 identifier for this config + /// Compute a deterministic u64 identifier for this config. /// - /// Uses Rust's built-in `Hash` trait (derived) to compute a stable hash - /// and converts it to a hex u64 for use as a cache key. - /// BTreeMap ensures consistent ordering regardless of insertion order. + /// Uses FNV-64 (a non-cryptographic but stable, seed-free hasher) so the + /// same config always produces the same key across process restarts. + /// This is required for persisted trust-store lookups: `DefaultHasher` + /// uses a random seed per-process and would produce a different value on + /// every restart, causing trust decisions to be ignored. + /// `BTreeMap` ensures consistent field ordering regardless of insertion + /// order. pub fn cache_key(&self) -> u64 { - use std::collections::hash_map::DefaultHasher; use std::hash::{Hash, Hasher}; - let mut hasher = DefaultHasher::new(); + let mut hasher = fnv_rs::Fnv64::default(); Hash::hash(self, &mut hasher); hasher.finish() } } +/// The trust status of a single MCP config file (identified by path + content +/// hash). +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum McpTrustStatus { + /// The config has been explicitly accepted by the user. + Trusted, + /// The config has been explicitly rejected by the user. + Rejected, + /// The config has not yet been decided by the user. + Unknown, +} + +/// A persisted trust decision: stores the user's choice together with the +/// content hash it was made against so that any modification to the file +/// invalidates the decision. +#[derive(Debug, Clone, Copy, Serialize, Deserialize)] +struct McpTrustEntry { + hash: u64, + decision: McpTrustDecision, +} + +/// The two terminal (persisted) states for a trust decision. +/// +/// Distinct from `McpTrustStatus` so that the on-disk representation cannot +/// encode the `Unknown` variant (which simply means "no entry"). +#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] +#[serde(rename_all = "lowercase")] +enum McpTrustDecision { + Trusted, + Rejected, +} + +impl From for McpTrustStatus { + fn from(decision: McpTrustDecision) -> Self { + match decision { + McpTrustDecision::Trusted => McpTrustStatus::Trusted, + McpTrustDecision::Rejected => McpTrustStatus::Rejected, + } + } +} + +/// Persists accepted and rejected MCP config hashes across restarts. A path +/// maps to its content hash so that any modification to the file revokes the +/// stored decision and triggers a new prompt. +#[derive(Default, Debug, Clone, Serialize, Deserialize)] +pub struct McpTrustStore { + #[serde(default)] + entries: BTreeMap, +} + +impl McpTrustStore { + /// Derives the JSON-map key for a path. + fn key(path: &Path) -> String { + path.to_string_lossy().into_owned() + } + + /// Returns the trust status for the given path+hash pair. + /// + /// Returns `Unknown` both when no decision has been persisted and when a + /// decision exists but was made against a different content hash (i.e. + /// the file has been modified since). + pub fn get_status(&self, path: &Path, content_hash: u64) -> McpTrustStatus { + match self.entries.get(&Self::key(path)) { + Some(entry) if entry.hash == content_hash => entry.decision.into(), + _ => McpTrustStatus::Unknown, + } + } + + /// Records an accepted trust decision for the given path and content hash. + pub fn trust(&mut self, path: &Path, content_hash: u64) { + self.insert(path, content_hash, McpTrustDecision::Trusted); + } + + /// Records a rejected trust decision for the given path and content hash. + pub fn reject(&mut self, path: &Path, content_hash: u64) { + self.insert(path, content_hash, McpTrustDecision::Rejected); + } + + /// Clears any trust decision (accepted or rejected) for the given path. + pub fn clear(&mut self, path: &Path) { + self.entries.remove(&Self::key(path)); + } + + fn insert(&mut self, path: &Path, hash: u64, decision: McpTrustDecision) { + self.entries + .insert(Self::key(path), McpTrustEntry { hash, decision }); + } +} + #[cfg(test)] mod tests { + use std::path::PathBuf; + + use pretty_assertions::assert_eq; + use super::*; #[test] @@ -593,6 +690,85 @@ mod tests { } } + #[test] + fn test_trust_store_unknown_when_empty() { + let fixture = McpTrustStore::default(); + let actual = fixture.get_status(&PathBuf::from("/tmp/.mcp.json"), 42); + let expected = McpTrustStatus::Unknown; + assert_eq!(actual, expected); + } + + #[test] + fn test_trust_store_remembers_trust_decision() { + let path = PathBuf::from("/tmp/.mcp.json"); + let mut fixture = McpTrustStore::default(); + fixture.trust(&path, 42); + + let actual = fixture.get_status(&path, 42); + let expected = McpTrustStatus::Trusted; + assert_eq!(actual, expected); + } + + #[test] + fn test_trust_store_remembers_reject_decision() { + let path = PathBuf::from("/tmp/.mcp.json"); + let mut fixture = McpTrustStore::default(); + fixture.reject(&path, 42); + + let actual = fixture.get_status(&path, 42); + let expected = McpTrustStatus::Rejected; + assert_eq!(actual, expected); + } + + #[test] + fn test_trust_store_invalidates_on_hash_change() { + let path = PathBuf::from("/tmp/.mcp.json"); + let mut fixture = McpTrustStore::default(); + fixture.trust(&path, 42); + + let actual = fixture.get_status(&path, 43); + let expected = McpTrustStatus::Unknown; + assert_eq!(actual, expected); + } + + #[test] + fn test_trust_store_trust_overwrites_prior_rejection() { + let path = PathBuf::from("/tmp/.mcp.json"); + let mut fixture = McpTrustStore::default(); + fixture.reject(&path, 42); + fixture.trust(&path, 42); + + let actual = fixture.get_status(&path, 42); + let expected = McpTrustStatus::Trusted; + assert_eq!(actual, expected); + } + + #[test] + fn test_trust_store_clear_removes_decision() { + let path = PathBuf::from("/tmp/.mcp.json"); + let mut fixture = McpTrustStore::default(); + fixture.trust(&path, 42); + fixture.clear(&path); + + let actual = fixture.get_status(&path, 42); + let expected = McpTrustStatus::Unknown; + assert_eq!(actual, expected); + } + + #[test] + fn test_trust_store_roundtrips_through_json() { + let path = PathBuf::from("/tmp/.mcp.json"); + let mut fixture = McpTrustStore::default(); + fixture.trust(&path, 42); + + let json = serde_json::to_string(&fixture).unwrap(); + let restored: McpTrustStore = serde_json::from_str(&json).unwrap(); + + let actual = restored.get_status(&path, 42); + let expected = McpTrustStatus::Trusted; + assert_eq!(actual, expected); + } + #[test] fn test_stdio_server_without_timeout() { use pretty_assertions::assert_eq; diff --git a/crates/forge_main/src/ui.rs b/crates/forge_main/src/ui.rs index 9ecd50fc41..495eb7c8ac 100644 --- a/crates/forge_main/src/ui.rs +++ b/crates/forge_main/src/ui.rs @@ -18,7 +18,8 @@ use forge_app::{CommitResult, ToolResolver}; use forge_config::ForgeConfig; use forge_display::MarkdownFormat; use forge_domain::{ - AuthMethod, ChatResponseContent, ConsoleWriter, ContextMessage, Role, TitleFormat, UserCommand, + AuthMethod, ChatResponseContent, ConsoleWriter, ContextMessage, McpTrustStatus, Role, Scope, + TitleFormat, UserCommand, }; use forge_fs::ForgeFS; use forge_select::{ForgeWidget, SelectRow}; @@ -1728,6 +1729,72 @@ impl A + Send + Sync> UI Ok(()) } + /// Applies the interactive trust gate for project-local MCP configs. + /// + /// Checks the persisted trust store first: if the config hash is already + /// recorded as trusted or rejected, the prompt is skipped. Otherwise the + /// declared servers are rendered as non-selectable header rows above the + /// Accept/Reject choices so the user can see exactly what they're being + /// asked to trust. The decision is persisted so it is not asked again as + /// long as the file content is unchanged. A cancelled prompt is treated + /// as a rejection. + /// + /// Servers are NOT connected here — connections remain lazy and happen on + /// first tool use. + async fn init_mcp_trust_gate(&mut self) -> anyhow::Result<()> { + const ACCEPT: &str = "Accept"; + const REJECT: &str = "Reject"; + + let local_path = self.api.environment().mcp_local_config(); + if !local_path.exists() { + return Ok(()); + } + + if self.api.get_mcp_trust_status(&local_path).await? != McpTrustStatus::Unknown { + return Ok(()); + } + + let config = self.api.read_mcp_config(Some(&Scope::Local)).await?; + if config.mcp_servers.is_empty() { + return Ok(()); + } + + // Render the server list as header rows so it stays visible above the + // Accept/Reject options regardless of terminal height. + let mut rows = Vec::with_capacity(config.mcp_servers.len() + 4); + rows.push(SelectRow::header(format!( + "Untrusted MCP config found at {}", + local_path.display(), + ))); + rows.push(SelectRow::header(String::from("Servers:"))); + for (name, server) in &config.mcp_servers { + // Show the endpoint (URL for HTTP, command for stdio) so the user + // can see exactly what will be executed/contacted if they accept. + rows.push(SelectRow::header(format!( + " - {name}: {endpoint}", + endpoint = format_mcp_server(server), + ))); + } + let header_lines = rows.len(); + rows.push(SelectRow::new(ACCEPT, ACCEPT)); + rows.push(SelectRow::new(REJECT, REJECT)); + + // A missing selection (Esc / cancel) is treated as a rejection so the + // user is not silently prompted again on the next run. + let decision = match self.select_raw_row( + "Do you want to trust these servers?", + None, + rows, + header_lines, + None, + )? { + Some(row) if row.raw == ACCEPT => McpTrustStatus::Trusted, + _ => McpTrustStatus::Rejected, + }; + + self.api.set_mcp_trust(&local_path, decision).await + } + async fn on_info( &mut self, porcelain: bool, @@ -3805,6 +3872,9 @@ impl A + Send + Sync> UI return Ok(()); } + // Initialize MCP trust gate for local project configs + self.init_mcp_trust_gate().await?; + let mut operating_model = self.get_agent_model(active_agent.clone()).await; if operating_model.is_none() { // Use the model returned from selection instead of re-fetching diff --git a/crates/forge_services/src/mcp/manager.rs b/crates/forge_services/src/mcp/manager.rs index a49e89c24a..a2153e45f4 100644 --- a/crates/forge_services/src/mcp/manager.rs +++ b/crates/forge_services/src/mcp/manager.rs @@ -3,7 +3,7 @@ use std::sync::Arc; use anyhow::Context; use bytes::Bytes; -use forge_app::domain::{McpConfig, Scope}; +use forge_app::domain::{McpConfig, McpTrustStatus, McpTrustStore, Scope}; use forge_app::{ EnvironmentInfra, FileInfoInfra, FileReaderInfra, FileWriterInfra, KVStore, McpConfigManager, McpServerInfra, @@ -14,14 +14,21 @@ pub struct ForgeMcpManager { infra: Arc, } -impl ForgeMcpManager -where - I: McpServerInfra + FileReaderInfra + FileInfoInfra + EnvironmentInfra + KVStore, -{ +impl ForgeMcpManager { pub fn new(infra: Arc) -> Self { Self { infra } } +} +impl ForgeMcpManager +where + I: McpServerInfra + + FileReaderInfra + + FileInfoInfra + + EnvironmentInfra + + FileWriterInfra + + KVStore, +{ async fn read_config(&self, path: &Path) -> anyhow::Result { let config = self.infra.read_utf8(path).await?; Ok(serde_json::from_str(&config)?) @@ -34,6 +41,40 @@ where Scope::Local => Ok(env.mcp_local_config()), } } + + /// Reads the persisted trust store from disk, returning an empty store if + /// the file is absent or its contents cannot be parsed. + async fn read_trust_store(&self) -> anyhow::Result { + let path = self.infra.get_environment().mcp_trust_path(); + if !self.infra.is_file(&path).await.unwrap_or(false) { + return Ok(McpTrustStore::default()); + } + let content = self.infra.read_utf8(&path).await?; + Ok(serde_json::from_str(&content).unwrap_or_default()) + } + + /// Writes the trust store to disk at the environment's `mcp_trust_path`. + async fn write_trust_store(&self, store: &McpTrustStore) -> anyhow::Result<()> { + let path = self.infra.get_environment().mcp_trust_path(); + let content = serde_json::to_string_pretty(store)?; + self.infra.write(&path, Bytes::from(content)).await + } + + /// Returns `true` if the project-local MCP config is either absent or has + /// been explicitly trusted at its current content hash. + async fn is_local_trusted(&self) -> anyhow::Result { + let local_path = self.infra.get_environment().mcp_local_config(); + if !self.infra.is_file(&local_path).await.unwrap_or(false) { + // No local config => nothing to gate. + return Ok(true); + } + let hash = self.read_config(&local_path).await?.cache_key(); + let store = self.read_trust_store().await?; + Ok(matches!( + store.get_status(&local_path, hash), + McpTrustStatus::Trusted + )) + } } #[async_trait::async_trait] @@ -95,4 +136,38 @@ where Ok(()) } + + async fn get_mcp_trust_status(&self, path: &Path) -> anyhow::Result { + let hash = self.read_config(path).await?.cache_key(); + let store = self.read_trust_store().await?; + Ok(store.get_status(path, hash)) + } + + async fn set_mcp_trust(&self, path: &Path, status: McpTrustStatus) -> anyhow::Result<()> { + let hash = self.read_config(path).await?.cache_key(); + let mut store = self.read_trust_store().await?; + + match status { + McpTrustStatus::Trusted => store.trust(path, hash), + McpTrustStatus::Rejected => store.reject(path, hash), + McpTrustStatus::Unknown => store.clear(path), + } + + self.write_trust_store(&store).await + } + + async fn filter_trusted(&self, raw: McpConfig) -> anyhow::Result { + if self.is_local_trusted().await? { + return Ok(raw); + } + + // Local is untrusted: drop any servers whose names appear only in the + // local config, retaining those defined in the user scope. + let user_config = self.read_mcp_config(Some(&Scope::User)).await?; + let mut filtered = raw; + filtered + .mcp_servers + .retain(|name, _| user_config.mcp_servers.contains_key(name)); + Ok(filtered) + } } diff --git a/crates/forge_services/src/mcp/service.rs b/crates/forge_services/src/mcp/service.rs index a96692de62..e5c6c13901 100644 --- a/crates/forge_services/src/mcp/service.rs +++ b/crates/forge_services/src/mcp/service.rs @@ -233,14 +233,14 @@ where C: From<::Client>, { async fn get_mcp_servers(&self) -> anyhow::Result { - // Read current configs to compute merged hash - let mcp_config = self.manager.read_mcp_config(None).await?; + // Filter the merged config so only trusted servers remain. Using the + // *filtered* config's hash as the cache key ensures that a cache entry + // populated before a rejection cannot leak rejected tools into later + // requests. + let raw_config = self.manager.read_mcp_config(None).await?; + let trusted_config = self.manager.filter_trusted(raw_config).await?; + let config_hash = trusted_config.cache_key(); - // Compute unified hash from merged config - let config_hash = mcp_config.cache_key(); - - // Check if cache is valid (exists and not expired) - // Cache is valid, retrieve it if let Some(cache) = self.infra.cache_get::<_, McpServers>(&config_hash).await? { return Ok(cache.clone()); } @@ -262,12 +262,13 @@ where #[cfg(test)] mod tests { use std::collections::BTreeMap; + use std::path::Path; use std::sync::Arc; use fake::{Fake, Faker}; use forge_app::domain::{ - ConfigOperation, Environment, McpConfig, McpServerConfig, Scope, ServerName, ToolCallFull, - ToolDefinition, ToolName, ToolOutput, + ConfigOperation, Environment, McpConfig, McpServerConfig, McpTrustStatus, Scope, + ServerName, ToolCallFull, ToolDefinition, ToolName, ToolOutput, }; use forge_app::{ EnvironmentInfra, KVStore, McpClientInfra, McpConfigManager, McpServerInfra, McpService, @@ -320,6 +321,20 @@ mod tests { ) -> anyhow::Result<()> { Ok(()) } + + async fn get_mcp_trust_status(&self, _path: &Path) -> anyhow::Result { + // In tests all configs are implicitly trusted + Ok(McpTrustStatus::Trusted) + } + + async fn set_mcp_trust(&self, _path: &Path, _status: McpTrustStatus) -> anyhow::Result<()> { + Ok(()) + } + + async fn filter_trusted(&self, raw: McpConfig) -> anyhow::Result { + // In tests all configs are implicitly trusted + Ok(raw) + } } // ── Mock infrastructure ────────────────────────────────────────────────── From 3396d0e8af3cd0a7e1bb3d27ce2cf9dffd2eaa40 Mon Sep 17 00:00:00 2001 From: laststylebender14 Date: Tue, 12 May 2026 15:43:29 +0530 Subject: [PATCH 2/7] perf(mcp): avoid allocation in McpTrustStore key lookup --- crates/forge_app/src/services.rs | 4 ++++ crates/forge_domain/src/mcp.rs | 15 +++++++++------ crates/forge_main/src/ui.rs | 6 +++--- crates/forge_services/src/mcp/service.rs | 2 -- 4 files changed, 16 insertions(+), 11 deletions(-) diff --git a/crates/forge_app/src/services.rs b/crates/forge_app/src/services.rs index f10f5c945b..a350298742 100644 --- a/crates/forge_app/src/services.rs +++ b/crates/forge_app/src/services.rs @@ -229,6 +229,10 @@ pub trait McpConfigManager: Send + Sync { /// /// Passing `McpTrustStatus::Unknown` clears any previously recorded /// decision for the path. + /// + /// # Errors + /// Returns an error if the file cannot be read or the trust store cannot + /// be persisted. async fn set_mcp_trust(&self, path: &Path, status: McpTrustStatus) -> anyhow::Result<()>; /// Drops untrusted servers from `raw`. diff --git a/crates/forge_domain/src/mcp.rs b/crates/forge_domain/src/mcp.rs index 197081f185..54e8d2b841 100644 --- a/crates/forge_domain/src/mcp.rs +++ b/crates/forge_domain/src/mcp.rs @@ -1,6 +1,7 @@ //! //! Follows the design specifications of Claude's [.mcp.json](https://docs.anthropic.com/en/docs/claude-code/tutorials#set-up-model-context-protocol-mcp) +use std::borrow::Cow; use std::collections::BTreeMap; use std::ops::Deref; use std::path::Path; @@ -358,9 +359,11 @@ pub struct McpTrustStore { } impl McpTrustStore { - /// Derives the JSON-map key for a path. - fn key(path: &Path) -> String { - path.to_string_lossy().into_owned() + /// Derives the JSON-map key for a path. Returns a borrowed `&str` when + /// the path is already valid UTF-8, allocating only for paths containing + /// non-UTF-8 bytes. + fn key(path: &Path) -> Cow<'_, str> { + path.to_string_lossy() } /// Returns the trust status for the given path+hash pair. @@ -369,7 +372,7 @@ impl McpTrustStore { /// decision exists but was made against a different content hash (i.e. /// the file has been modified since). pub fn get_status(&self, path: &Path, content_hash: u64) -> McpTrustStatus { - match self.entries.get(&Self::key(path)) { + match self.entries.get(Self::key(path).as_ref()) { Some(entry) if entry.hash == content_hash => entry.decision.into(), _ => McpTrustStatus::Unknown, } @@ -387,12 +390,12 @@ impl McpTrustStore { /// Clears any trust decision (accepted or rejected) for the given path. pub fn clear(&mut self, path: &Path) { - self.entries.remove(&Self::key(path)); + self.entries.remove(Self::key(path).as_ref()); } fn insert(&mut self, path: &Path, hash: u64, decision: McpTrustDecision) { self.entries - .insert(Self::key(path), McpTrustEntry { hash, decision }); + .insert(Self::key(path).into_owned(), McpTrustEntry { hash, decision }); } } diff --git a/crates/forge_main/src/ui.rs b/crates/forge_main/src/ui.rs index 495eb7c8ac..59ab3d56d4 100644 --- a/crates/forge_main/src/ui.rs +++ b/crates/forge_main/src/ui.rs @@ -1766,13 +1766,13 @@ impl A + Send + Sync> UI "Untrusted MCP config found at {}", local_path.display(), ))); - rows.push(SelectRow::header(String::from("Servers:"))); + rows.push(SelectRow::header("Servers:")); for (name, server) in &config.mcp_servers { // Show the endpoint (URL for HTTP, command for stdio) so the user // can see exactly what will be executed/contacted if they accept. rows.push(SelectRow::header(format!( - " - {name}: {endpoint}", - endpoint = format_mcp_server(server), + " - {name}: {}", + format_mcp_server(server), ))); } let header_lines = rows.len(); diff --git a/crates/forge_services/src/mcp/service.rs b/crates/forge_services/src/mcp/service.rs index e5c6c13901..b42756bb76 100644 --- a/crates/forge_services/src/mcp/service.rs +++ b/crates/forge_services/src/mcp/service.rs @@ -323,7 +323,6 @@ mod tests { } async fn get_mcp_trust_status(&self, _path: &Path) -> anyhow::Result { - // In tests all configs are implicitly trusted Ok(McpTrustStatus::Trusted) } @@ -332,7 +331,6 @@ mod tests { } async fn filter_trusted(&self, raw: McpConfig) -> anyhow::Result { - // In tests all configs are implicitly trusted Ok(raw) } } From 9197f8d41e936e79150f749032749f3d729c6695 Mon Sep 17 00:00:00 2001 From: "autofix-ci[bot]" <114827586+autofix-ci[bot]@users.noreply.github.com> Date: Tue, 12 May 2026 10:16:12 +0000 Subject: [PATCH 3/7] [autofix.ci] apply automated fixes --- crates/forge_domain/src/mcp.rs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/crates/forge_domain/src/mcp.rs b/crates/forge_domain/src/mcp.rs index 54e8d2b841..c167dbb0b9 100644 --- a/crates/forge_domain/src/mcp.rs +++ b/crates/forge_domain/src/mcp.rs @@ -394,8 +394,10 @@ impl McpTrustStore { } fn insert(&mut self, path: &Path, hash: u64, decision: McpTrustDecision) { - self.entries - .insert(Self::key(path).into_owned(), McpTrustEntry { hash, decision }); + self.entries.insert( + Self::key(path).into_owned(), + McpTrustEntry { hash, decision }, + ); } } From 7ac6d43d89b9ee6727f5677a93fdfd857cc71f63 Mon Sep 17 00:00:00 2001 From: laststylebender14 Date: Tue, 12 May 2026 16:07:09 +0530 Subject: [PATCH 4/7] feat(mcp): filter untrusted servers before connecting in init_mcp --- crates/forge_main/src/ui.rs | 2 +- crates/forge_services/src/mcp/service.rs | 9 ++++----- 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/crates/forge_main/src/ui.rs b/crates/forge_main/src/ui.rs index 59ab3d56d4..f0ba5b6ebb 100644 --- a/crates/forge_main/src/ui.rs +++ b/crates/forge_main/src/ui.rs @@ -1771,7 +1771,7 @@ impl A + Send + Sync> UI // Show the endpoint (URL for HTTP, command for stdio) so the user // can see exactly what will be executed/contacted if they accept. rows.push(SelectRow::header(format!( - " - {name}: {}", + " - {name}: {}\n", format_mcp_server(server), ))); } diff --git a/crates/forge_services/src/mcp/service.rs b/crates/forge_services/src/mcp/service.rs index b42756bb76..04c081db99 100644 --- a/crates/forge_services/src/mcp/service.rs +++ b/crates/forge_services/src/mcp/service.rs @@ -101,7 +101,8 @@ where } async fn init_mcp(&self) -> anyhow::Result<()> { - let mcp = self.manager.read_mcp_config(None).await?; + let raw_mcp = self.manager.read_mcp_config(None).await?; + let mcp = self.manager.filter_trusted(raw_mcp).await?; // Fast path: if config is unchanged, skip reinitialization without acquiring // the lock @@ -233,10 +234,8 @@ where C: From<::Client>, { async fn get_mcp_servers(&self) -> anyhow::Result { - // Filter the merged config so only trusted servers remain. Using the - // *filtered* config's hash as the cache key ensures that a cache entry - // populated before a rejection cannot leak rejected tools into later - // requests. + // init_mcp already filters untrusted servers before connecting, so the + // cache key is derived from the trusted config to avoid stale entries. let raw_config = self.manager.read_mcp_config(None).await?; let trusted_config = self.manager.filter_trusted(raw_config).await?; let config_hash = trusted_config.cache_key(); From db757d93e96333fddb292858a6ee943b809bfcff Mon Sep 17 00:00:00 2001 From: laststylebender14 Date: Tue, 12 May 2026 16:15:43 +0530 Subject: [PATCH 5/7] refactor(mcp): consolidate confirmation prompt message --- crates/forge_main/src/ui.rs | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/crates/forge_main/src/ui.rs b/crates/forge_main/src/ui.rs index f0ba5b6ebb..fb503185bb 100644 --- a/crates/forge_main/src/ui.rs +++ b/crates/forge_main/src/ui.rs @@ -1761,11 +1761,7 @@ impl A + Send + Sync> UI // Render the server list as header rows so it stays visible above the // Accept/Reject options regardless of terminal height. - let mut rows = Vec::with_capacity(config.mcp_servers.len() + 4); - rows.push(SelectRow::header(format!( - "Untrusted MCP config found at {}", - local_path.display(), - ))); + let mut rows = Vec::with_capacity(config.mcp_servers.len() + 2); rows.push(SelectRow::header("Servers:")); for (name, server) in &config.mcp_servers { // Show the endpoint (URL for HTTP, command for stdio) so the user @@ -1782,7 +1778,10 @@ impl A + Send + Sync> UI // A missing selection (Esc / cancel) is treated as a rejection so the // user is not silently prompted again on the next run. let decision = match self.select_raw_row( - "Do you want to trust these servers?", + &format!( + "Untrusted MCP config at {} - trust these servers?", + local_path.display() + ), None, rows, header_lines, From 3be9ae99eaaf7f4c020cd3a039e07a6bfe515dbc Mon Sep 17 00:00:00 2001 From: laststylebender14 Date: Tue, 12 May 2026 16:20:10 +0530 Subject: [PATCH 6/7] feat(mcp): improve untrusted MCP config prompt message formatting --- crates/forge_main/src/ui.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/crates/forge_main/src/ui.rs b/crates/forge_main/src/ui.rs index fb503185bb..92f5a9e4bf 100644 --- a/crates/forge_main/src/ui.rs +++ b/crates/forge_main/src/ui.rs @@ -1762,7 +1762,6 @@ impl A + Send + Sync> UI // Render the server list as header rows so it stays visible above the // Accept/Reject options regardless of terminal height. let mut rows = Vec::with_capacity(config.mcp_servers.len() + 2); - rows.push(SelectRow::header("Servers:")); for (name, server) in &config.mcp_servers { // Show the endpoint (URL for HTTP, command for stdio) so the user // can see exactly what will be executed/contacted if they accept. @@ -1779,7 +1778,7 @@ impl A + Send + Sync> UI // user is not silently prompted again on the next run. let decision = match self.select_raw_row( &format!( - "Untrusted MCP config at {} - trust these servers?", + "Untrusted MCP config at '{}', Do you want to trust following servers?", local_path.display() ), None, From cdc5f8b4981adc52e76e90170f92418982af259a Mon Sep 17 00:00:00 2001 From: laststylebender14 Date: Tue, 12 May 2026 16:21:46 +0530 Subject: [PATCH 7/7] feat(mcp): improve untrusted config prompt message --- crates/forge_main/src/ui.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/forge_main/src/ui.rs b/crates/forge_main/src/ui.rs index 92f5a9e4bf..e5f4201e4f 100644 --- a/crates/forge_main/src/ui.rs +++ b/crates/forge_main/src/ui.rs @@ -1778,7 +1778,7 @@ impl A + Send + Sync> UI // user is not silently prompted again on the next run. let decision = match self.select_raw_row( &format!( - "Untrusted MCP config at '{}', Do you want to trust following servers?", + "Untrusted MCP config at '{}' — do you want to trust the following servers?", local_path.display() ), None,