From d89076d9b2b7be926e60d1200f71a82d4b14f87a Mon Sep 17 00:00:00 2001 From: laststylebender14 Date: Tue, 5 May 2026 09:00:46 +0530 Subject: [PATCH 1/9] feat(mcp): add interactive trust gate for project-local MCP configs --- crates/forge_api/src/api.rs | 5 + crates/forge_api/src/forge_api.rs | 4 + crates/forge_app/src/services.rs | 17 +++ crates/forge_domain/src/env.rs | 5 + crates/forge_domain/src/mcp.rs | 53 +++++++- crates/forge_main/src/ui.rs | 3 + crates/forge_services/src/mcp/manager.rs | 157 ++++++++++++++++++++++- crates/forge_services/src/mcp/service.rs | 45 +++++-- 8 files changed, 266 insertions(+), 23 deletions(-) diff --git a/crates/forge_api/src/api.rs b/crates/forge_api/src/api.rs index aafb112d49..5a2a5217fe 100644 --- a/crates/forge_api/src/api.rs +++ b/crates/forge_api/src/api.rs @@ -175,6 +175,11 @@ pub trait API: Sync + Send { /// Refresh MCP caches by fetching fresh data async fn reload_mcp(&self) -> Result<()>; + /// Applies the interactive trust gate for any project-local MCP config. + /// Servers are NOT connected here — connections remain lazy and happen on + /// first tool use. Must be called once at startup. + async fn init_mcp(&self) -> 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..a056705761 100644 --- a/crates/forge_api/src/forge_api.rs +++ b/crates/forge_api/src/forge_api.rs @@ -306,6 +306,10 @@ impl< async fn reload_mcp(&self) -> Result<()> { self.services.mcp_service().reload_mcp().await } + + async fn init_mcp(&self) -> Result<()> { + self.services.mcp_service().init_mcp().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..a64aaa92bb 100644 --- a/crates/forge_app/src/services.rs +++ b/crates/forge_app/src/services.rs @@ -214,6 +214,11 @@ pub trait McpConfigManager: Send + Sync { /// Responsible for writing the McpConfig on disk. async fn write_mcp_config(&self, config: &McpConfig, scope: &Scope) -> anyhow::Result<()>; + + /// Returns the trusted subset of MCP servers, prompting interactively for + /// any project-local config file not yet approved. Must be called once at + /// the startup boundary, never on pure config-read paths. + async fn filter_trusted(&self, raw: McpConfig) -> anyhow::Result; } #[async_trait::async_trait] @@ -222,6 +227,10 @@ pub trait McpService: Send + Sync { async fn execute_mcp(&self, call: ToolCallFull) -> anyhow::Result; /// Refresh the MCP cache by fetching fresh data async fn reload_mcp(&self) -> anyhow::Result<()>; + /// Applies the interactive trust gate for any project-local MCP config. + /// Servers are NOT connected here — connections remain lazy and happen on + /// first tool use. Must be called once at startup. + async fn init_mcp(&self) -> anyhow::Result<()>; } #[async_trait::async_trait] @@ -680,6 +689,10 @@ impl McpConfigManager for I { .write_mcp_config(config, scope) .await } + + async fn filter_trusted(&self, raw: McpConfig) -> anyhow::Result { + self.mcp_config_manager().filter_trusted(raw).await + } } #[async_trait::async_trait] @@ -695,6 +708,10 @@ impl McpService for I { async fn reload_mcp(&self) -> anyhow::Result<()> { self.mcp_service().reload_mcp().await } + + async fn init_mcp(&self) -> anyhow::Result<()> { + self.mcp_service().init_mcp().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..37b8fdeece 100644 --- a/crates/forge_domain/src/mcp.rs +++ b/crates/forge_domain/src/mcp.rs @@ -8,6 +8,7 @@ use derive_more::{Deref, Display, From}; use derive_setters::Setters; use merge::Merge; use serde::{Deserialize, Serialize}; +use strum_macros::{Display as StrumDisplay, EnumIter, EnumString}; #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, Serialize, Deserialize)] pub enum Scope { @@ -288,21 +289,61 @@ 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 and remember" 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 three choices presented to the user when an untrusted project-local +/// `.mcp.json` is detected at startup. +#[derive(Debug, Clone, PartialEq, Eq, StrumDisplay, EnumIter, EnumString)] +pub enum McpTrustResponse { + /// Allow the servers for this session only, without persisting the decision. + #[strum(to_string = "Trust once")] + TrustOnce, + /// Allow the servers and remember this decision across future sessions. + #[strum(to_string = "Trust and remember")] + TrustAndRemember, + /// Reject all servers from this config file. + #[strum(to_string = "Reject")] + Reject, +} + +/// Persists trust decisions for project-local MCP config files across +/// restarts. Keyed by a `(canonical_path_string, content_hash_u64)` pair so +/// that any modification to the file revokes the stored trust. +#[derive(Default, Debug, Clone, Serialize, Deserialize)] +pub struct McpTrustStore { + #[serde(default)] + trusted: std::collections::HashSet<(String, u64)>, +} + +impl McpTrustStore { + /// Returns true if the given path+hash pair has been permanently trusted. + pub fn is_trusted(&self, path: &std::path::Path, content_hash: u64) -> bool { + self.trusted + .contains(&(path.to_string_lossy().into_owned(), content_hash)) + } + + /// Records a permanent trust decision for the given path and content hash. + pub fn remember(&mut self, path: std::path::PathBuf, content_hash: u64) { + self.trusted + .insert((path.to_string_lossy().into_owned(), content_hash)); + } +} + #[cfg(test)] mod tests { use super::*; diff --git a/crates/forge_main/src/ui.rs b/crates/forge_main/src/ui.rs index 00a75d7e67..69ed17277e 100644 --- a/crates/forge_main/src/ui.rs +++ b/crates/forge_main/src/ui.rs @@ -3802,6 +3802,9 @@ impl A + Send + Sync> UI .await?; // only call on_update if this is the first initialization on_update(self.api.clone(), self.config.updates.as_ref()).await; + // Apply the MCP trust gate. Servers are NOT connected here — + // connections remain lazy and happen on first tool use. + self.api.init_mcp().await?; } // Execute independent operations in parallel to improve performance diff --git a/crates/forge_services/src/mcp/manager.rs b/crates/forge_services/src/mcp/manager.rs index a49e89c24a..fef7a5ae69 100644 --- a/crates/forge_services/src/mcp/manager.rs +++ b/crates/forge_services/src/mcp/manager.rs @@ -1,25 +1,28 @@ +use std::collections::HashSet; use std::path::{Path, PathBuf}; -use std::sync::Arc; +use std::sync::{Arc, Mutex}; use anyhow::Context; use bytes::Bytes; -use forge_app::domain::{McpConfig, Scope}; +use forge_app::domain::{McpConfig, McpServerConfig, McpTrustResponse, McpTrustStore, Scope}; use forge_app::{ EnvironmentInfra, FileInfoInfra, FileReaderInfra, FileWriterInfra, KVStore, McpConfigManager, - McpServerInfra, + McpServerInfra, UserInfra, }; use merge::Merge; pub struct ForgeMcpManager { infra: Arc, + session_trusted: Mutex>, } impl ForgeMcpManager where I: McpServerInfra + FileReaderInfra + FileInfoInfra + EnvironmentInfra + KVStore, { + /// Creates a new [`ForgeMcpManager`] wrapping the provided infrastructure. pub fn new(infra: Arc) -> Self { - Self { infra } + Self { infra, session_trusted: Default::default() } } async fn read_config(&self, path: &Path) -> anyhow::Result { @@ -36,6 +39,118 @@ where } } +impl ForgeMcpManager +where + I: McpServerInfra + + FileReaderInfra + + FileInfoInfra + + EnvironmentInfra + + FileWriterInfra + + KVStore + + UserInfra, +{ + /// Reads the persisted trust store from disk, returning a default empty + /// store if the file is absent. + 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 given path+hash pair is trusted for this session. + fn is_session_trusted(&self, path: &Path, hash: u64) -> bool { + self.session_trusted + .lock() + .unwrap() + .contains(&(path.to_string_lossy().into_owned(), hash)) + } + + /// Records a session-scoped trust decision. + fn add_session_trust(&self, path: &Path, hash: u64) { + self.session_trusted + .lock() + .unwrap() + .insert((path.to_string_lossy().into_owned(), hash)); + } + + /// Applies the interactive trust gate for a project-local MCP config. + /// + /// Returns the config as-is when already trusted (session or persistent), + /// otherwise prompts the user and acts on their choice. + async fn apply_trust_gate( + &self, + local: McpConfig, + local_path: &Path, + ) -> anyhow::Result { + if local.mcp_servers.is_empty() { + return Ok(local); + } + + let hash = local.cache_key(); + + // Fast path: already trusted in this session. + if self.is_session_trusted(local_path, hash) { + return Ok(local); + } + + // Check persistent trust store. + let mut store = self.read_trust_store().await?; + if store.is_trusted(local_path, hash) { + self.add_session_trust(local_path, hash); + return Ok(local); + } + + // Build and display the prompt. + let prompt = format_trust_prompt(local_path, &local); + match self + .infra + .select_one_enum::(&prompt) + .await? + { + Some(McpTrustResponse::TrustAndRemember) => { + store.remember(local_path.to_path_buf(), hash); + self.write_trust_store(&store).await?; + self.add_session_trust(local_path, hash); + Ok(local) + } + Some(McpTrustResponse::TrustOnce) => { + self.add_session_trust(local_path, hash); + Ok(local) + } + Some(McpTrustResponse::Reject) | None => Ok(McpConfig::default()), + } + } +} + +/// Builds the interactive prompt string shown to the user when an untrusted +/// project-local `.mcp.json` is found. Lists the file path and every server +/// name together with its command or URL. +fn format_trust_prompt(path: &Path, config: &McpConfig) -> String { + let mut lines = vec![format!( + "A project-local MCP config was found at {}.\nThe following servers would be started:", + path.display() + )]; + for (name, server) in &config.mcp_servers { + let detail = match server { + McpServerConfig::Stdio(s) => format!("command: {}", s.command), + McpServerConfig::Http(s) => format!("url: {}", s.url), + }; + lines.push(format!(" • {name} ({detail})")); + } + lines.push("\nDo you trust these servers?".to_string()); + lines.join("\n") +} + #[async_trait::async_trait] impl McpConfigManager for ForgeMcpManager where @@ -44,7 +159,8 @@ where + FileInfoInfra + EnvironmentInfra + FileWriterInfra - + KVStore, + + KVStore + + UserInfra, { async fn read_mcp_config(&self, scope: Option<&Scope>) -> anyhow::Result { match scope { @@ -95,4 +211,35 @@ where Ok(()) } + + async fn filter_trusted(&self, raw: McpConfig) -> anyhow::Result { + let env = self.infra.get_environment(); + + // User-scope config is always implicitly trusted. + let user_path = env.mcp_user_config(); + let user_config = if self.infra.is_file(&user_path).await.unwrap_or(false) { + self.read_config(&user_path).await? + } else { + McpConfig::default() + }; + + // Local-scope config must pass the trust gate. + let local_path = env.mcp_local_config(); + let local_config = if self.infra.is_file(&local_path).await.unwrap_or(false) { + let local_raw = self.read_config(&local_path).await?; + self.apply_trust_gate(local_raw, &local_path).await? + } else { + McpConfig::default() + }; + + // Merge: user first, then local (local takes precedence as in read_mcp_config). + let mut merged = user_config; + merged.merge(local_config); + + // Retain only servers that exist in the merged trusted set. + let trusted_keys: std::collections::BTreeSet<_> = merged.mcp_servers.keys().cloned().collect(); + let mut result = raw; + result.mcp_servers.retain(|k, _| trusted_keys.contains(k)); + Ok(result) + } } diff --git a/crates/forge_services/src/mcp/service.rs b/crates/forge_services/src/mcp/service.rs index a96692de62..e9f4fc2bde 100644 --- a/crates/forge_services/src/mcp/service.rs +++ b/crates/forge_services/src/mcp/service.rs @@ -100,12 +100,12 @@ where Ok(()) } - async fn init_mcp(&self) -> anyhow::Result<()> { - let mcp = self.manager.read_mcp_config(None).await?; + async fn ensure_mcp_initialized(&self) -> anyhow::Result<()> { + let raw_mcp = self.manager.read_mcp_config(None).await?; // Fast path: if config is unchanged, skip reinitialization without acquiring // the lock - if !self.is_config_modified(&mcp).await { + if !self.is_config_modified(&raw_mcp).await { return Ok(()); } @@ -114,18 +114,24 @@ where let _guard = self.init_lock.lock().await; // Double-check under the lock: a concurrent caller may have already updated - if !self.is_config_modified(&mcp).await { + if !self.is_config_modified(&raw_mcp).await { return Ok(()); } - self.update_mcp(mcp).await + // Apply the trust gate. If init_mcp was called at startup, the manager's + // session trust cache already holds the decision and this returns immediately + // without re-prompting. + let raw_hash = raw_mcp.cache_key(); + let trusted_mcp = self.manager.filter_trusted(raw_mcp).await?; + + self.update_mcp(trusted_mcp, raw_hash).await } - async fn update_mcp(&self, mcp: McpConfig) -> Result<(), anyhow::Error> { - // Compute the hash early before mcp is consumed, but write it only after - // all connections are established so waiters on init_lock see a consistent - // state. - let new_hash = mcp.cache_key(); + async fn update_mcp(&self, mcp: McpConfig, raw_hash: u64) -> Result<(), anyhow::Error> { + // Use the raw config hash (pre-trust-gate) so that is_config_modified always + // compares against the original file hash, preventing infinite re-prompt loops + // when some servers are rejected. + let new_hash = raw_hash; self.clear_tools().await; // Clear failed servers map before attempting new connections @@ -171,7 +177,7 @@ where } async fn list(&self) -> anyhow::Result { - self.init_mcp().await?; + self.ensure_mcp_initialized().await?; let tools = self.tools.read().await; let mut grouped_tools = std::collections::HashMap::new(); @@ -193,7 +199,7 @@ where async fn call(&self, call: ToolCallFull) -> anyhow::Result { // Ensure MCP connections are initialized before calling tools - self.init_mcp().await?; + self.ensure_mcp_initialized().await?; let tools = self.tools.read().await; @@ -257,6 +263,16 @@ where async fn reload_mcp(&self) -> anyhow::Result<()> { self.refresh_cache().await } + + async fn init_mcp(&self) -> anyhow::Result<()> { + // Run the trust gate to populate the manager's session trust cache. + // The result is intentionally discarded — servers are NOT connected here. + // Connections remain lazy and happen on first tool use via + // ensure_mcp_initialized, which fast-paths through the session cache. + let raw_mcp = self.manager.read_mcp_config(None).await?; + let _ = self.manager.filter_trusted(raw_mcp).await?; + Ok(()) + } } #[cfg(test)] @@ -320,6 +336,11 @@ mod tests { ) -> anyhow::Result<()> { Ok(()) } + + async fn filter_trusted(&self, raw: McpConfig) -> anyhow::Result { + // In tests all configs are implicitly trusted. + Ok(raw) + } } // ── Mock infrastructure ────────────────────────────────────────────────── From adb27b12b40046a8b2bb2143a6a2ea37404ce9ad Mon Sep 17 00:00:00 2001 From: laststylebender14 Date: Tue, 5 May 2026 14:06:01 +0530 Subject: [PATCH 2/9] feat(mcp): simplify trust prompt for untrusted MCP config --- crates/forge_services/src/mcp/manager.rs | 21 +++++---------------- 1 file changed, 5 insertions(+), 16 deletions(-) diff --git a/crates/forge_services/src/mcp/manager.rs b/crates/forge_services/src/mcp/manager.rs index fef7a5ae69..8dc6670060 100644 --- a/crates/forge_services/src/mcp/manager.rs +++ b/crates/forge_services/src/mcp/manager.rs @@ -111,7 +111,7 @@ where } // Build and display the prompt. - let prompt = format_trust_prompt(local_path, &local); + let prompt = format_trust_prompt(local_path); match self .infra .select_one_enum::(&prompt) @@ -135,20 +135,8 @@ where /// Builds the interactive prompt string shown to the user when an untrusted /// project-local `.mcp.json` is found. Lists the file path and every server /// name together with its command or URL. -fn format_trust_prompt(path: &Path, config: &McpConfig) -> String { - let mut lines = vec![format!( - "A project-local MCP config was found at {}.\nThe following servers would be started:", - path.display() - )]; - for (name, server) in &config.mcp_servers { - let detail = match server { - McpServerConfig::Stdio(s) => format!("command: {}", s.command), - McpServerConfig::Http(s) => format!("url: {}", s.url), - }; - lines.push(format!(" • {name} ({detail})")); - } - lines.push("\nDo you trust these servers?".to_string()); - lines.join("\n") +fn format_trust_prompt(path: &Path) -> String { + format!("Untrusted MCP config was found at {}", path.display()) } #[async_trait::async_trait] @@ -237,7 +225,8 @@ where merged.merge(local_config); // Retain only servers that exist in the merged trusted set. - let trusted_keys: std::collections::BTreeSet<_> = merged.mcp_servers.keys().cloned().collect(); + let trusted_keys: std::collections::BTreeSet<_> = + merged.mcp_servers.keys().cloned().collect(); let mut result = raw; result.mcp_servers.retain(|k, _| trusted_keys.contains(k)); Ok(result) From db7101ff34d919068cf64b7b84ca4f6b6990079c Mon Sep 17 00:00:00 2001 From: "autofix-ci[bot]" <114827586+autofix-ci[bot]@users.noreply.github.com> Date: Tue, 5 May 2026 09:54:08 +0000 Subject: [PATCH 3/9] [autofix.ci] apply automated fixes --- crates/forge_domain/src/mcp.rs | 6 ++++-- crates/forge_services/src/mcp/manager.rs | 2 +- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/crates/forge_domain/src/mcp.rs b/crates/forge_domain/src/mcp.rs index 37b8fdeece..68d09fc10f 100644 --- a/crates/forge_domain/src/mcp.rs +++ b/crates/forge_domain/src/mcp.rs @@ -296,7 +296,8 @@ impl McpConfig { /// 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 and remember" to be ignored. - /// `BTreeMap` ensures consistent field ordering regardless of insertion order. + /// `BTreeMap` ensures consistent field ordering regardless of insertion + /// order. pub fn cache_key(&self) -> u64 { use std::hash::{Hash, Hasher}; @@ -310,7 +311,8 @@ impl McpConfig { /// `.mcp.json` is detected at startup. #[derive(Debug, Clone, PartialEq, Eq, StrumDisplay, EnumIter, EnumString)] pub enum McpTrustResponse { - /// Allow the servers for this session only, without persisting the decision. + /// Allow the servers for this session only, without persisting the + /// decision. #[strum(to_string = "Trust once")] TrustOnce, /// Allow the servers and remember this decision across future sessions. diff --git a/crates/forge_services/src/mcp/manager.rs b/crates/forge_services/src/mcp/manager.rs index 8dc6670060..26211f20d9 100644 --- a/crates/forge_services/src/mcp/manager.rs +++ b/crates/forge_services/src/mcp/manager.rs @@ -4,7 +4,7 @@ use std::sync::{Arc, Mutex}; use anyhow::Context; use bytes::Bytes; -use forge_app::domain::{McpConfig, McpServerConfig, McpTrustResponse, McpTrustStore, Scope}; +use forge_app::domain::{McpConfig, McpTrustResponse, McpTrustStore, Scope}; use forge_app::{ EnvironmentInfra, FileInfoInfra, FileReaderInfra, FileWriterInfra, KVStore, McpConfigManager, McpServerInfra, UserInfra, From b098d0aadf27330e3335b0ff6aa83ae7fa785640 Mon Sep 17 00:00:00 2001 From: laststylebender14 Date: Tue, 12 May 2026 11:03:15 +0530 Subject: [PATCH 4/9] refactor(mcp): replace trust-once option with single accept+persist choice --- crates/forge_domain/src/mcp.rs | 12 +++--- crates/forge_services/src/mcp/manager.rs | 49 +++++------------------- crates/forge_services/src/mcp/service.rs | 14 +++---- 3 files changed, 22 insertions(+), 53 deletions(-) diff --git a/crates/forge_domain/src/mcp.rs b/crates/forge_domain/src/mcp.rs index 68d09fc10f..ab2e633a55 100644 --- a/crates/forge_domain/src/mcp.rs +++ b/crates/forge_domain/src/mcp.rs @@ -307,17 +307,15 @@ impl McpConfig { } } -/// The three choices presented to the user when an untrusted project-local +/// The two choices presented to the user when an untrusted project-local /// `.mcp.json` is detected at startup. #[derive(Debug, Clone, PartialEq, Eq, StrumDisplay, EnumIter, EnumString)] pub enum McpTrustResponse { - /// Allow the servers for this session only, without persisting the - /// decision. - #[strum(to_string = "Trust once")] - TrustOnce, /// Allow the servers and remember this decision across future sessions. - #[strum(to_string = "Trust and remember")] - TrustAndRemember, + /// The config hash is persisted so the prompt is skipped on next startup + /// as long as the file has not changed. + #[strum(to_string = "Accept")] + Accept, /// Reject all servers from this config file. #[strum(to_string = "Reject")] Reject, diff --git a/crates/forge_services/src/mcp/manager.rs b/crates/forge_services/src/mcp/manager.rs index 26211f20d9..fc5d8343e9 100644 --- a/crates/forge_services/src/mcp/manager.rs +++ b/crates/forge_services/src/mcp/manager.rs @@ -1,6 +1,5 @@ -use std::collections::HashSet; use std::path::{Path, PathBuf}; -use std::sync::{Arc, Mutex}; +use std::sync::Arc; use anyhow::Context; use bytes::Bytes; @@ -13,7 +12,6 @@ use merge::Merge; pub struct ForgeMcpManager { infra: Arc, - session_trusted: Mutex>, } impl ForgeMcpManager @@ -22,7 +20,7 @@ where { /// Creates a new [`ForgeMcpManager`] wrapping the provided infrastructure. pub fn new(infra: Arc) -> Self { - Self { infra, session_trusted: Default::default() } + Self { infra } } async fn read_config(&self, path: &Path) -> anyhow::Result { @@ -49,8 +47,8 @@ where + KVStore + UserInfra, { - /// Reads the persisted trust store from disk, returning a default empty - /// store if the file is absent. + /// Reads the persisted trust store from disk, returning an empty store if + /// the file is absent or unreadable. 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) { @@ -67,26 +65,11 @@ where self.infra.write(&path, Bytes::from(content)).await } - /// Returns true if the given path+hash pair is trusted for this session. - fn is_session_trusted(&self, path: &Path, hash: u64) -> bool { - self.session_trusted - .lock() - .unwrap() - .contains(&(path.to_string_lossy().into_owned(), hash)) - } - - /// Records a session-scoped trust decision. - fn add_session_trust(&self, path: &Path, hash: u64) { - self.session_trusted - .lock() - .unwrap() - .insert((path.to_string_lossy().into_owned(), hash)); - } - /// Applies the interactive trust gate for a project-local MCP config. /// - /// Returns the config as-is when already trusted (session or persistent), - /// otherwise prompts the user and acts on their choice. + /// Checks the persisted trust store first: if the config hash is already + /// recorded the prompt is skipped entirely. Otherwise the user is asked to + /// Accept (persists the hash) or Reject (returns an empty config). async fn apply_trust_gate( &self, local: McpConfig, @@ -97,34 +80,22 @@ where } let hash = local.cache_key(); - - // Fast path: already trusted in this session. - if self.is_session_trusted(local_path, hash) { - return Ok(local); - } - - // Check persistent trust store. let mut store = self.read_trust_store().await?; + + // Skip the prompt if this exact config was previously accepted. if store.is_trusted(local_path, hash) { - self.add_session_trust(local_path, hash); return Ok(local); } - // Build and display the prompt. let prompt = format_trust_prompt(local_path); match self .infra .select_one_enum::(&prompt) .await? { - Some(McpTrustResponse::TrustAndRemember) => { + Some(McpTrustResponse::Accept) => { store.remember(local_path.to_path_buf(), hash); self.write_trust_store(&store).await?; - self.add_session_trust(local_path, hash); - Ok(local) - } - Some(McpTrustResponse::TrustOnce) => { - self.add_session_trust(local_path, hash); Ok(local) } Some(McpTrustResponse::Reject) | None => Ok(McpConfig::default()), diff --git a/crates/forge_services/src/mcp/service.rs b/crates/forge_services/src/mcp/service.rs index e9f4fc2bde..6eec925c32 100644 --- a/crates/forge_services/src/mcp/service.rs +++ b/crates/forge_services/src/mcp/service.rs @@ -118,9 +118,9 @@ where return Ok(()); } - // Apply the trust gate. If init_mcp was called at startup, the manager's - // session trust cache already holds the decision and this returns immediately - // without re-prompting. + // Apply the trust gate. The prompt was already shown at startup via + // init_mcp, so on first tool use the user's decision is re-applied by + // calling filter_trusted again. let raw_hash = raw_mcp.cache_key(); let trusted_mcp = self.manager.filter_trusted(raw_mcp).await?; @@ -265,10 +265,10 @@ where } async fn init_mcp(&self) -> anyhow::Result<()> { - // Run the trust gate to populate the manager's session trust cache. - // The result is intentionally discarded — servers are NOT connected here. - // Connections remain lazy and happen on first tool use via - // ensure_mcp_initialized, which fast-paths through the session cache. + // Run the trust gate prompt at startup so the user's decision is captured + // before any tool use. The result is intentionally discarded — servers are + // NOT connected here. Connections remain lazy and happen on first tool use + // via ensure_mcp_initialized. let raw_mcp = self.manager.read_mcp_config(None).await?; let _ = self.manager.filter_trusted(raw_mcp).await?; Ok(()) From 0c7e75c605010cd5505738a78afde456a656fbd6 Mon Sep 17 00:00:00 2001 From: laststylebender14 Date: Tue, 12 May 2026 11:11:57 +0530 Subject: [PATCH 5/9] refactor(mcp): change trust store from HashSet to HashMap --- crates/forge_domain/src/mcp.rs | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/crates/forge_domain/src/mcp.rs b/crates/forge_domain/src/mcp.rs index ab2e633a55..a6226fd2e0 100644 --- a/crates/forge_domain/src/mcp.rs +++ b/crates/forge_domain/src/mcp.rs @@ -321,26 +321,27 @@ pub enum McpTrustResponse { Reject, } -/// Persists trust decisions for project-local MCP config files across -/// restarts. Keyed by a `(canonical_path_string, content_hash_u64)` pair so -/// that any modification to the file revokes the stored trust. +/// Persists accepted MCP config hashes across restarts. A path maps to its +/// content hash so that any modification to the file revokes the stored trust +/// and triggers a new prompt. #[derive(Default, Debug, Clone, Serialize, Deserialize)] pub struct McpTrustStore { #[serde(default)] - trusted: std::collections::HashSet<(String, u64)>, + trusted: std::collections::HashMap, } impl McpTrustStore { - /// Returns true if the given path+hash pair has been permanently trusted. + /// Returns true if the given path+hash pair has been previously accepted. pub fn is_trusted(&self, path: &std::path::Path, content_hash: u64) -> bool { self.trusted - .contains(&(path.to_string_lossy().into_owned(), content_hash)) + .get(&path.to_string_lossy().into_owned()) + .is_some_and(|&stored| stored == content_hash) } - /// Records a permanent trust decision for the given path and content hash. + /// Records a trust decision for the given path and content hash. pub fn remember(&mut self, path: std::path::PathBuf, content_hash: u64) { self.trusted - .insert((path.to_string_lossy().into_owned(), content_hash)); + .insert(path.to_string_lossy().into_owned(), content_hash); } } From e4eb018b0b88e68896a226ae2427171a91006c4b Mon Sep 17 00:00:00 2001 From: laststylebender14 Date: Tue, 12 May 2026 11:48:28 +0530 Subject: [PATCH 6/9] feat(mcp): persist rejected config decisions --- crates/forge_domain/src/mcp.rs | 31 +++++++++++++++++++----- crates/forge_services/src/mcp/manager.rs | 16 +++++++++--- 2 files changed, 38 insertions(+), 9 deletions(-) diff --git a/crates/forge_domain/src/mcp.rs b/crates/forge_domain/src/mcp.rs index a6226fd2e0..a065579f71 100644 --- a/crates/forge_domain/src/mcp.rs +++ b/crates/forge_domain/src/mcp.rs @@ -321,13 +321,15 @@ pub enum McpTrustResponse { Reject, } -/// Persists accepted MCP config hashes across restarts. A path maps to its -/// content hash so that any modification to the file revokes the stored trust -/// and triggers a new prompt. +/// 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)] trusted: std::collections::HashMap, + #[serde(default)] + rejected: std::collections::HashMap, } impl McpTrustStore { @@ -338,10 +340,27 @@ impl McpTrustStore { .is_some_and(|&stored| stored == content_hash) } - /// Records a trust decision for the given path and content hash. + /// Returns true if the given path+hash pair has been previously rejected. + pub fn is_rejected(&self, path: &std::path::Path, content_hash: u64) -> bool { + self.rejected + .get(&path.to_string_lossy().into_owned()) + .is_some_and(|&stored| stored == content_hash) + } + + /// Records an accepted trust decision for the given path and content hash. + /// Clears any prior rejection for the same path. pub fn remember(&mut self, path: std::path::PathBuf, content_hash: u64) { - self.trusted - .insert(path.to_string_lossy().into_owned(), content_hash); + let key = path.to_string_lossy().into_owned(); + self.rejected.remove(&key); + self.trusted.insert(key, content_hash); + } + + /// Records a rejected trust decision for the given path and content hash. + /// Clears any prior acceptance for the same path. + pub fn reject(&mut self, path: std::path::PathBuf, content_hash: u64) { + let key = path.to_string_lossy().into_owned(); + self.trusted.remove(&key); + self.rejected.insert(key, content_hash); } } diff --git a/crates/forge_services/src/mcp/manager.rs b/crates/forge_services/src/mcp/manager.rs index fc5d8343e9..ad6c92e6be 100644 --- a/crates/forge_services/src/mcp/manager.rs +++ b/crates/forge_services/src/mcp/manager.rs @@ -68,8 +68,9 @@ where /// Applies the interactive trust gate for a project-local MCP config. /// /// Checks the persisted trust store first: if the config hash is already - /// recorded the prompt is skipped entirely. Otherwise the user is asked to - /// Accept (persists the hash) or Reject (returns an empty config). + /// recorded as accepted or rejected, the prompt is skipped entirely. + /// Otherwise the user is asked to Accept (persists the hash as trusted) or + /// Reject (persists the hash as rejected and returns an empty config). async fn apply_trust_gate( &self, local: McpConfig, @@ -87,6 +88,11 @@ where return Ok(local); } + // Skip the prompt if this exact config was previously rejected. + if store.is_rejected(local_path, hash) { + return Ok(McpConfig::default()); + } + let prompt = format_trust_prompt(local_path); match self .infra @@ -98,7 +104,11 @@ where self.write_trust_store(&store).await?; Ok(local) } - Some(McpTrustResponse::Reject) | None => Ok(McpConfig::default()), + Some(McpTrustResponse::Reject) | None => { + store.reject(local_path.to_path_buf(), hash); + self.write_trust_store(&store).await?; + Ok(McpConfig::default()) + } } } } From 8865c15c6e5cccf711396abea603d894883757ca Mon Sep 17 00:00:00 2001 From: laststylebender14 Date: Tue, 12 May 2026 11:54:42 +0530 Subject: [PATCH 7/9] feat(mcp): apply trust filter before cache key computation --- crates/forge_services/src/mcp/service.rs | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/crates/forge_services/src/mcp/service.rs b/crates/forge_services/src/mcp/service.rs index 6eec925c32..76383bc320 100644 --- a/crates/forge_services/src/mcp/service.rs +++ b/crates/forge_services/src/mcp/service.rs @@ -239,14 +239,15 @@ 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?; - - // Compute unified hash from merged config - let config_hash = mcp_config.cache_key(); + // Apply the trust gate before computing the cache key so that rejected + // servers are excluded. Using the raw config hash would allow a stale KV + // cache entry (populated before a rejection) to be returned, bypassing + // filter_trusted entirely and leaking rejected tools into requests. + let raw_mcp = self.manager.read_mcp_config(None).await?; + let trusted_mcp = self.manager.filter_trusted(raw_mcp).await?; + let config_hash = trusted_mcp.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()); } From 4c04657d61193891f0b245b493c7af0297c8dbed Mon Sep 17 00:00:00 2001 From: laststylebender14 Date: Tue, 12 May 2026 12:25:38 +0530 Subject: [PATCH 8/9] refactor(mcp): move cache_key computation into update_mcp --- crates/forge_services/src/mcp/service.rs | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/crates/forge_services/src/mcp/service.rs b/crates/forge_services/src/mcp/service.rs index 76383bc320..036c19fa67 100644 --- a/crates/forge_services/src/mcp/service.rs +++ b/crates/forge_services/src/mcp/service.rs @@ -121,17 +121,16 @@ where // Apply the trust gate. The prompt was already shown at startup via // init_mcp, so on first tool use the user's decision is re-applied by // calling filter_trusted again. - let raw_hash = raw_mcp.cache_key(); let trusted_mcp = self.manager.filter_trusted(raw_mcp).await?; - self.update_mcp(trusted_mcp, raw_hash).await + self.update_mcp(trusted_mcp).await } - async fn update_mcp(&self, mcp: McpConfig, raw_hash: u64) -> Result<(), anyhow::Error> { + async fn update_mcp(&self, mcp: McpConfig) -> Result<(), anyhow::Error> { // Use the raw config hash (pre-trust-gate) so that is_config_modified always // compares against the original file hash, preventing infinite re-prompt loops // when some servers are rejected. - let new_hash = raw_hash; + let raw_hash = mcp.cache_key(); self.clear_tools().await; // Clear failed servers map before attempting new connections @@ -171,7 +170,7 @@ where // Write the hash only after join_all finishes so that any waiter on // init_lock re-checks is_config_modified only once self.tools is fully // populated, preventing "Tool not found" races. - *self.previous_config_hash.lock().await = new_hash; + *self.previous_config_hash.lock().await = raw_hash; Ok(()) } From 058f21e9c2880558f9c3e01ccc8d5e43f338bb72 Mon Sep 17 00:00:00 2001 From: laststylebender14 Date: Tue, 12 May 2026 12:26:41 +0530 Subject: [PATCH 9/9] fix(mcp): rename variable for clarity in cache key computation --- crates/forge_services/src/mcp/service.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/forge_services/src/mcp/service.rs b/crates/forge_services/src/mcp/service.rs index 036c19fa67..4b848e2b01 100644 --- a/crates/forge_services/src/mcp/service.rs +++ b/crates/forge_services/src/mcp/service.rs @@ -130,7 +130,7 @@ where // Use the raw config hash (pre-trust-gate) so that is_config_modified always // compares against the original file hash, preventing infinite re-prompt loops // when some servers are rejected. - let raw_hash = mcp.cache_key(); + let new_hash = mcp.cache_key(); self.clear_tools().await; // Clear failed servers map before attempting new connections @@ -170,7 +170,7 @@ where // Write the hash only after join_all finishes so that any waiter on // init_lock re-checks is_config_modified only once self.tools is fully // populated, preventing "Tool not found" races. - *self.previous_config_hash.lock().await = raw_hash; + *self.previous_config_hash.lock().await = new_hash; Ok(()) }