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..a350298742 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,33 @@ 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. + /// + /// # 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`. + /// + /// 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 +707,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..c167dbb0b9 100644 --- a/crates/forge_domain/src/mcp.rs +++ b/crates/forge_domain/src/mcp.rs @@ -1,8 +1,10 @@ //! //! 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; use derive_more::{Deref, Display, From}; use derive_setters::Setters; @@ -288,23 +290,123 @@ 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. 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. + /// + /// 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).as_ref()) { + 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).as_ref()); + } + + fn insert(&mut self, path: &Path, hash: u64, decision: McpTrustDecision) { + self.entries.insert( + Self::key(path).into_owned(), + McpTrustEntry { hash, decision }, + ); + } +} + #[cfg(test)] mod tests { + use std::path::PathBuf; + + use pretty_assertions::assert_eq; + use super::*; #[test] @@ -593,6 +695,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..e5f4201e4f 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,70 @@ 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() + 2); + 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}: {}\n", + 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( + &format!( + "Untrusted MCP config at '{}' — do you want to trust the following servers?", + local_path.display() + ), + 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 +3870,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..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,14 +234,12 @@ 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?; + // 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(); - // 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 +261,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 +320,18 @@ mod tests { ) -> anyhow::Result<()> { Ok(()) } + + async fn get_mcp_trust_status(&self, _path: &Path) -> anyhow::Result { + 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 { + Ok(raw) + } } // ── Mock infrastructure ──────────────────────────────────────────────────