diff --git a/crates/tinymcp/src/lib.rs b/crates/tinymcp/src/lib.rs index 0a1c613..5647c5d 100644 --- a/crates/tinymcp/src/lib.rs +++ b/crates/tinymcp/src/lib.rs @@ -97,7 +97,7 @@ pub use error::{Error, Result}; pub use registry::{ AppliedServer, ConfigApplyReport, Connections, McpRegistry, OAuthFlow, ProbeOutcome, REMOTE_REQUEST_TIMEOUT, SecretRef, SecretVault, ServerRef, Store, SupervisedHost, Supervisor, - SupervisorConfig, SupervisorEvent, TickReport, + SupervisorConfig, SupervisorEvent, TickReport, guard_public_endpoint, }; pub use server::{ ClientSession, McpServerHandler, RequestContext, RequestHeaders, ResourceSpec, ServerInfo, diff --git a/crates/tinymcp/src/registry/mod.rs b/crates/tinymcp/src/registry/mod.rs index e13f3ca..9cedf78 100644 --- a/crates/tinymcp/src/registry/mod.rs +++ b/crates/tinymcp/src/registry/mod.rs @@ -25,6 +25,7 @@ pub use config_doc::{AppliedServer, ConfigApplyReport}; pub use connections::{Connections, ProbeOutcome, REMOTE_REQUEST_TIMEOUT}; pub use oauth::{ AuthDetection, AuthKind, DEFAULT_CLIENT_NAME, OAuthBundle, OAuthCredentialStore, OAuthFlow, + guard_public_endpoint, }; pub use ops::McpRegistry; pub use setup::{SecretRef, SecretVault}; diff --git a/crates/tinymcp/src/registry/oauth/endpoint_guard.rs b/crates/tinymcp/src/registry/oauth/endpoint_guard.rs index 58beeed..c540aa4 100644 --- a/crates/tinymcp/src/registry/oauth/endpoint_guard.rs +++ b/crates/tinymcp/src/registry/oauth/endpoint_guard.rs @@ -28,7 +28,7 @@ use crate::error::{Error, Result}; /// /// [`Error::MalformedResponse`] when the URL is unsafe, or /// [`Error::EndpointResolution`] when DNS resolution could not complete. -pub(crate) async fn guard_endpoint(raw: &str, what: &str) -> Result> { +pub async fn guard_endpoint(raw: &str, what: &str) -> Result> { let refuse = |why: String| Error::malformed(format!("{what} endpoint refused: {why}")); let url = Url::parse(raw).map_err(|error| refuse(format!("not a valid url: {error}")))?; if url.scheme() != "https" { diff --git a/crates/tinymcp/src/registry/oauth/flow.rs b/crates/tinymcp/src/registry/oauth/flow.rs index 4747eab..fdb31e9 100644 --- a/crates/tinymcp/src/registry/oauth/flow.rs +++ b/crates/tinymcp/src/registry/oauth/flow.rs @@ -109,8 +109,9 @@ impl OAuthFlow { &self.client_name } - /// Refuses authorization, registration and token endpoints that are not - /// `https` or that resolve to an internal address. + /// Refuses a server URL, and the authorization, registration and token + /// endpoints it advertises, that are not `https` or that resolve to an + /// internal address. /// /// Those endpoints are advertised by the server being signed in to and /// `POST`ed to from the host, so a host connecting to servers it does not @@ -153,8 +154,10 @@ impl OAuthFlow { /// /// # Errors /// - /// Returns [`Error::UnknownServer`] when there is no such install, and - /// [`Error::Store`] when it cannot be read. A store failure is *not* + /// Returns [`Error::UnknownServer`] when there is no such install, + /// [`Error::Store`] when it cannot be read, and, under + /// [`Self::require_public_endpoints`], [`Error::MalformedResponse`] or + /// [`Error::EndpointResolution`] when the server's own URL is refused. A store failure is *not* /// collapsed into "open": reporting a server as needing nothing when the /// lookup failed would show the user a state that was never checked. pub async fn detect(&self, store: &S, server_id: &str) -> Result @@ -166,10 +169,7 @@ impl OAuthFlow { return Ok(AuthDetection::open()); }; - let client = McpHttpClient::builder(url) - .timeout_secs(PROBE_TIMEOUT_SECS) - .proxy(self.proxy.clone()) - .build()?; + let client = self.probe_client(url).await?; match client.discover_authorization().await { Ok(None) => Ok(AuthDetection::open()), @@ -204,7 +204,11 @@ impl OAuthFlow { /// Returns [`Error::AuthDiscovery`] when the server advertises no /// authorization server, or none offers everything the flow needs, [`Error::MalformedResponse`] when /// registration answers with something unusable, plus whatever the - /// transport returns. + /// transport returns. Under [`Self::require_public_endpoints`], also + /// [`Error::MalformedResponse`] when the server's own URL or an endpoint + /// it advertises is refused (not `https`, or internal), and + /// [`Error::EndpointResolution`] when a host cannot be resolved; both + /// before anything is registered. pub async fn begin(&self, store: &S, server_id: &str, redirect_uri: &str) -> Result where S: OAuthCredentialStore + ?Sized, @@ -214,10 +218,7 @@ impl OAuthFlow { .await? .ok_or_else(|| Error::malformed("oauth applies only to http-remote servers"))?; - let client = McpHttpClient::builder(url.clone()) - .timeout_secs(PROBE_TIMEOUT_SECS) - .proxy(self.proxy.clone()) - .build()?; + let client = self.probe_client(url.clone()).await?; let context = client .discover_authorization() @@ -426,6 +427,20 @@ impl OAuthFlow { } } + /// A client for probing the server itself. Under + /// [`Self::require_public_endpoints`] the server's own URL is checked like + /// the endpoints it advertises, and the client pinned to the addresses + /// that passed: the probe is a request from the host too. + async fn probe_client(&self, url: String) -> Result { + let mut builder = McpHttpClient::builder(url.clone()) + .timeout_secs(PROBE_TIMEOUT_SECS) + .proxy(self.proxy.clone()); + if self.public_endpoints_only { + builder = builder.pinned_public_endpoint(guard_endpoint(&url, "MCP").await?); + } + builder.build() + } + pub(super) async fn token_client( &self, token_endpoint: &str, diff --git a/crates/tinymcp/src/registry/oauth/mod.rs b/crates/tinymcp/src/registry/oauth/mod.rs index 49921dd..a5e391d 100644 --- a/crates/tinymcp/src/registry/oauth/mod.rs +++ b/crates/tinymcp/src/registry/oauth/mod.rs @@ -51,6 +51,7 @@ pub(crate) mod tokens; pub(crate) mod types; pub use credentials::OAuthCredentialStore; +pub use endpoint_guard::guard_endpoint as guard_public_endpoint; pub use flow::{DEFAULT_CLIENT_NAME, OAuthFlow}; pub use tokens::{OAUTH_BUNDLE_KEY, refresh_if_expired}; pub use types::{AuthDetection, AuthKind, OAuthBundle}; diff --git a/crates/tinymcp/src/registry/oauth/mod_host_store_tests.rs b/crates/tinymcp/src/registry/oauth/mod_host_store_tests.rs index a93463c..dfe9628 100644 --- a/crates/tinymcp/src/registry/oauth/mod_host_store_tests.rs +++ b/crates/tinymcp/src/registry/oauth/mod_host_store_tests.rs @@ -280,18 +280,53 @@ async fn a_non_json_token_endpoint_failure_is_not_echoed() { /// exactly such a target, so nothing may be registered with it. #[tokio::test] async fn a_guarded_flow_refuses_an_internal_authorization_server() { - let (endpoint, state) = authority_with_challenge().await; - let store = store_with_remote(&endpoint); + // The server URL itself is now refused first when it is internal (see + // `a_guarded_flow_refuses_an_internal_server_url_before_probing_it`), so + // the endpoints a public server could advertise are checked directly: + // each internal one is refused by name, before anything is registered. let flow = flow().require_public_endpoints(); - - let error = flow - .begin(&store, "srv-1", "https://host.test/cb") + let public = "https://8.8.8.8"; + for (authorize, token, register, what) in [ + ( + "http://127.0.0.1/authorize".to_string(), + format!("{public}/token"), + format!("{public}/register"), + "authorization", + ), + ( + format!("{public}/authorize"), + "https://169.254.169.254/token".to_string(), + format!("{public}/register"), + "token", + ), + ( + format!("{public}/authorize"), + format!("{public}/token"), + "https://10.0.0.1/register".to_string(), + "registration", + ), + ] { + let error = flow + .registration_client(&authorize, &token, ®ister) + .await + .expect_err(what); + assert!( + error + .to_string() + .contains(&format!("{what} endpoint refused")), + "{error}" + ); + } + assert!( + flow.registration_client( + &format!("{public}/authorize"), + &format!("{public}/token"), + &format!("{public}/register"), + ) .await - .expect_err("a loopback authorization server"); - - assert!(error.to_string().contains("https"), "{error}"); - assert_eq!(state.registrations.load(Ordering::SeqCst), 0); - assert_eq!(flow.pending_count(), 0); + .expect("public endpoints pass") + .is_some() + ); } #[tokio::test] @@ -357,3 +392,54 @@ async fn desktop_flow_keeps_the_default_clients() { .is_none() ); } + +// --------------------------------------------------------------------------- +// Refusing an internal server URL itself +// --------------------------------------------------------------------------- + +/// The flow probes the server URL from the host before anything else, so a +/// guarded flow checks that URL like the endpoints the server advertises: +/// a user-supplied server address must not reach an internal service. +#[tokio::test] +async fn a_guarded_flow_refuses_an_internal_server_url_before_probing_it() { + let (endpoint, state) = authority_with_challenge().await; + let store = store_with_remote(&endpoint); + let flow = flow().require_public_endpoints(); + + let begun = flow + .begin(&store, "srv-1", "https://host.test/cb") + .await + .expect_err("a loopback server url"); + let detected = flow + .detect(&store, "srv-1") + .await + .expect_err("a loopback server url"); + + for error in [begun, detected] { + assert!( + error.to_string().contains("MCP endpoint refused"), + "{error}" + ); + } + assert_eq!(state.registrations.load(Ordering::SeqCst), 0); +} + +#[tokio::test] +async fn the_public_endpoint_guard_is_usable_by_a_host() { + let plain = crate::registry::guard_public_endpoint("http://example.com/mcp", "MCP") + .await + .expect_err("plain http"); + let internal = crate::registry::guard_public_endpoint("https://169.254.169.254/mcp", "MCP") + .await + .expect_err("the metadata address"); + let pinned = crate::registry::guard_public_endpoint("https://93.184.215.14/mcp", "MCP") + .await + .expect("a public literal address"); + + assert!(plain.to_string().contains("must use https"), "{plain}"); + assert!( + internal.to_string().contains("disallowed address"), + "{internal}" + ); + assert_eq!(pinned.len(), 1); +} diff --git a/crates/tinymcp/tests/public_reexports.rs b/crates/tinymcp/tests/public_reexports.rs index fc0a789..ed8ad16 100644 --- a/crates/tinymcp/tests/public_reexports.rs +++ b/crates/tinymcp/tests/public_reexports.rs @@ -25,3 +25,9 @@ fn the_contract_types_resolve_at_the_crate_root_to_the_bus_definitions() { let freshness: tinymcp_bus::RegistryFreshness = RegistryFreshness::LocalFallback; assert_ne!(freshness, RegistryFreshness::Live); } + +#[tokio::test] +async fn the_public_endpoint_guard_resolves_at_the_crate_root() { + let refused = tinymcp::guard_public_endpoint("http://example.com/mcp", "MCP").await; + assert!(refused.is_err()); +}