OAuth: host-backed credential store, DCR-aware detection, SSRF guard, no echoed secrets

crates/tinymcp-bus/src/errors/mod.rs

@@ -67,6 +67,8 @@ pub const CLIENT_BUILD: &str = "ai.tinyhumans.tinymcp.Error.ClientBuild";
 pub const SERIALIZATION: &str = "ai.tinyhumans.tinymcp.Error.Serialization";
 /// The installed-server store could not do what was asked of it.
 pub const STORE: &str = "ai.tinyhumans.tinymcp.Error.Store";
+/// A host-supplied credential store could not do what was asked of it.
+pub const CREDENTIAL_STORE: &str = "ai.tinyhumans.tinymcp.Error.CredentialStore";
 /// The store's directory or file could not be reached.
 pub const STORE_IO: &str = "ai.tinyhumans.tinymcp.Error.StoreIo";
 /// The module could not do something it needed of the bus, such as serving a
@@ -103,6 +105,7 @@ pub const ALL: &[&str] = &[
     CLIENT_BUILD,
     SERIALIZATION,
     STORE,
+    CREDENTIAL_STORE,
     STORE_IO,
     BUS,
     INVALID_ARGUMENT,

crates/tinymcp/src/error/mod.rs

@@ -254,6 +254,20 @@ pub enum Error {
         source: Box<rusqlite::Error>,
     },
 
+    /// A host-supplied credential store could not do what was asked of it.
+    ///
+    /// Raised by a host's [`OAuthCredentialStore`] implementation, whose own
+    /// error type this crate cannot name; carried as prose, like [`Self::Bus`].
+    ///
+    /// [`OAuthCredentialStore`]: crate::registry::OAuthCredentialStore
+    #[error("mcp credential store failure while {action}: {detail}")]
+    CredentialStore {
+        /// What was being attempted, in the present participle.
+        action: String,
+        /// What the host's store reported. Must not contain a secret.
+        detail: String,
+    },
+
     /// The store's directory or file could not be reached.
     #[error("mcp store is unreachable at `{}`: {source}", path.display())]
     StoreIo {
@@ -392,6 +406,7 @@ impl Error {
             Self::ClientBuild { .. } => errors::CLIENT_BUILD,
             Self::Serialization { .. } => errors::SERIALIZATION,
             Self::Store { .. } => errors::STORE,
+            Self::CredentialStore { .. } => errors::CREDENTIAL_STORE,
             Self::StoreIo { .. } => errors::STORE_IO,
             Self::Bus { .. } => errors::BUS,
             Self::InvalidArgument { .. } => errors::INVALID_ARGUMENT,

crates/tinymcp/src/error/mod_tests.rs

@@ -38,6 +38,10 @@ fn assorted_other_errors() -> Vec<Error> {
         Error::UnsupportedProtocolVersion {
             version: "1999-01-01".into(),
         },
+        Error::CredentialStore {
+            action: "writing credentials".into(),
+            detail: "the vault is sealed".into(),
+        },
         Error::MalformedResponse {
             detail: "no result member".into(),
         },

crates/tinymcp/src/registry/mod.rs

@@ -23,7 +23,7 @@ pub mod supervisor;
 pub use boot::{BootOutcome, connect_installed_servers};
 pub use config_doc::{AppliedServer, ConfigApplyReport};
 pub use connections::{Connections, ProbeOutcome, REMOTE_REQUEST_TIMEOUT};
-pub use oauth::{AuthDetection, AuthKind, OAuthFlow};
+pub use oauth::{AuthDetection, AuthKind, OAuthCredentialStore, OAuthFlow};
 pub use ops::McpRegistry;
 pub use setup::{SecretRef, SecretVault};
 pub use sources::{Registries, RegistrySource};

crates/tinymcp/src/registry/oauth/credentials.rs

@@ -0,0 +1,91 @@
+//! Where the flow keeps what it mints.
+//!
+//! The flow needs three things from storage: the endpoint of the server being
+//! signed in to, and read/write access to that server's credential map — the
+//! `Authorization` value and the refresh bundle under [`OAUTH_BUNDLE_KEY`].
+//! [`OAuthCredentialStore`] is exactly that, so a host can back sign-in and
+//! refresh with its own secret store (a per-tenant vault, say) instead of the
+//! `SQLite` [`Store`]. [`Store`] implements it, so existing callers are
+//! unchanged.
+//!
+//! [`OAUTH_BUNDLE_KEY`]: super::OAUTH_BUNDLE_KEY
+
+use std::collections::BTreeMap;
+use std::future::Future;
+
+use crate::error::Result;
+use crate::registry::Store;
+use tinymcp_bus::Transport;
+
+/// The storage the OAuth flow and [`refresh_if_expired`] read and write.
+///
+/// `server_id` is the host's key for the server. It is opaque to the flow, so a
+/// multi-tenant host can qualify it (`"<tenant>/<server>"`) and route each call
+/// to the right tenant's secrets; [`OAuthFlow::pending_server`] hands it back
+/// from a redirect's `state` before completing.
+///
+/// Credentials are a whole map, replaced on write. The flow always reads,
+/// merges and writes back, so a value the user set beside their sign-in
+/// survives it.
+///
+/// The futures are `Send` so the flow can run on a multi-threaded runtime.
+///
+/// [`refresh_if_expired`]: super::refresh_if_expired
+/// [`OAuthFlow::pending_server`]: super::OAuthFlow::pending_server
+pub trait OAuthCredentialStore: Send + Sync {
+    /// The endpoint of an HTTP-remote server, or `None` when the server is not
+    /// one (a subprocess has no HTTP authorization).
+    ///
+    /// # Errors
+    ///
+    /// Whatever the store reports when the server cannot be looked up.
+    fn remote_url(&self, server_id: &str) -> impl Future<Output = Result<Option<String>>> + Send;
+
+    /// Every credential stored for `server_id`; empty when there are none.
+    ///
+    /// # Errors
+    ///
+    /// Whatever the store reports when it cannot be read.
+    fn load_credentials(
+        &self,
+        server_id: &str,
+    ) -> impl Future<Output = Result<BTreeMap<String, String>>> + Send;
+
+    /// Replaces every credential stored for `server_id` with `credentials`.
+    ///
+    /// # Errors
+    ///
+    /// Whatever the store reports when it cannot be written.
+    fn store_credentials(
+        &self,
+        server_id: &str,
+        credentials: &BTreeMap<String, String>,
+    ) -> impl Future<Output = Result<()>> + Send;
+}
+
+#[allow(clippy::unused_async_trait_impl)]
+impl OAuthCredentialStore for Store {
+    async fn remote_url(&self, server_id: &str) -> Result<Option<String>> {
+        let server = self.get_server(server_id)?;
+        Ok(match server.transport {
+            Transport::HttpRemote { url } if !url.is_empty() => Some(url),
+            _ => None,
+        })
+    }
+
+    async fn load_credentials(&self, server_id: &str) -> Result<BTreeMap<String, String>> {
+        self.load_env_values(server_id)
+    }
+
+    async fn store_credentials(
+        &self,
+        server_id: &str,
+        credentials: &BTreeMap<String, String>,
+    ) -> Result<()> {
+        self.set_env_values(server_id, credentials)?;
+        // The listing of credential *names* lives on the server row and is
+        // what a caller shows the user, so it has to learn about new keys.
+        let names: Vec<String> = credentials.keys().cloned().collect();
+        self.update_env_keys(server_id, &names)
+    }
+}

crates/tinymcp/src/registry/oauth/endpoint_guard.rs

@@ -0,0 +1,128 @@
+//! Refusing internal targets among the endpoints a server advertises.
+//!
+//! The authorization, registration and token endpoints come from the server
+//! being signed in to, and the flow then POSTs to them **from the host**. An
+//! unchecked endpoint is a server-side request-forgery primitive: a hostile
+//! server can aim the flow at an internal service or the cloud metadata
+//! address. A host serving untrusted servers turns this guard on with
+//! [`OAuthFlow::require_public_endpoints`](super::OAuthFlow::require_public_endpoints).
+//! It is off by default because a desktop host legitimately signs in to
+//! loopback development servers.
+
+use std::net::{IpAddr, SocketAddr, ToSocketAddrs};
+use std::time::Duration;
+
+use reqwest::Url;
+
+use crate::error::{Error, Result};
+
+/// Refuses `raw` unless it is `https` and every address its host resolves to
+/// is public. `what` names the endpoint in the error.
+///
+/// A single blocked record is enough to refuse. The returned addresses can be
+/// pinned to the HTTP client so the eventual connection uses the vetted result
+/// instead of resolving the name again. Resolution runs on the blocking pool
+/// so a slow resolver cannot stall the executor.
+///
+/// # Errors
+///
+/// [`Error::MalformedResponse`] naming the endpoint and why it was refused.
+pub(super) async fn guard_endpoint(raw: &str, what: &str) -> Result<Vec<SocketAddr>> {
+    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" {
+        return Err(refuse(format!("must use https, not `{}`", url.scheme())));
+    }
+    let host = url
+        .host_str()
+        .ok_or_else(|| refuse("it has no host".to_string()))?
+        .trim_start_matches('[')
+        .trim_end_matches(']')
+        .to_string();
+    let port = url.port_or_known_default().unwrap_or(443);
+
+    let addresses: Vec<SocketAddr> = if let Ok(ip) = host.parse::<IpAddr>() {
+        vec![SocketAddr::new(ip, port)]
+    } else {
+        tokio::task::spawn_blocking(move || {
+            (host.as_str(), port)
+                .to_socket_addrs()
+                .map(Iterator::collect::<Vec<_>>)
+        })
+        .await
+        .map_err(|error| refuse(format!("resolution did not finish: {error}")))?
+        .map_err(|error| refuse(format!("its host does not resolve: {error}")))?
+    };
+    if addresses.is_empty() {
+        return Err(refuse("its host does not resolve".to_string()));
+    }
+    if addresses.iter().any(|addr| is_blocked_ip(&addr.ip())) {
+        return Err(refuse("it resolves to a disallowed address".to_string()));
+    }
+    Ok(addresses)
+}
+
+/// Builds an HTTP client pinned to the public addresses checked for `raw`.
+/// Redirects are disabled so a validated endpoint cannot replay credentials to
+/// a different, unchecked destination.
+pub(super) async fn guarded_client(raw: &str, what: &str) -> Result<reqwest::Client> {
+    let addresses = guard_endpoint(raw, what).await?;
+    let url = Url::parse(raw)
+        .map_err(|error| Error::malformed(format!("invalid {what} url: {error}")))?;
+    let host = url
+        .host_str()
+        .ok_or_else(|| Error::malformed(format!("{what} endpoint has no host")))?
+        .trim_start_matches('[')
+        .trim_end_matches(']');
+    client_pinned_to(host, &addresses)
+}
+
+fn client_pinned_to(host: &str, addresses: &[SocketAddr]) -> Result<reqwest::Client> {
+    let mut builder = reqwest::Client::builder()
+        .timeout(Duration::from_secs(30))
+        .connect_timeout(Duration::from_secs(10))
+        .redirect(reqwest::redirect::Policy::none());
+    if host.parse::<IpAddr>().is_err() {
+        builder = builder.resolve_to_addrs(host, addresses);
+    }
+    builder.build().map_err(|source| Error::ClientBuild {
+        source: Box::new(source.without_url()),
+    })
+}
+
+/// Whether an address is one the flow must never POST OAuth material to:
+/// loopback, private or unique-local, link-local (the metadata address among
+/// them), unspecified, broadcast, documentation, multicast, or `0.0.0.0/8`.
+pub(super) fn is_blocked_ip(ip: &IpAddr) -> bool {
+    match ip {
+        IpAddr::V4(v4) => {
+            v4.is_loopback()
+                || v4.is_private()
+                || v4.is_link_local()
+                || v4.is_unspecified()
+                || v4.is_broadcast()
+                || v4.is_documentation()
+                || v4.is_multicast()
+                || v4.octets()[0] == 0
+                || (v4.octets()[0] == 100 && (v4.octets()[1] & 0xc0) == 64)
+                || (v4.octets()[0] == 198 && (v4.octets()[1] & 0xfe) == 18)
+                || v4.octets()[0] >= 240
+        }
+        IpAddr::V6(v6) => {
+            if let Some(v4) = v6.to_ipv4() {
+                return is_blocked_ip(&IpAddr::V4(v4));
+            }
+            v6.is_loopback()
+                || v6.is_unspecified()
+                || v6.is_multicast()
+                || (v6.segments()[0] & 0xfe00) == 0xfc00
+                || (v6.segments()[0] & 0xffc0) == 0xfe80
+                || (v6.segments()[0] == 0x0064 && v6.segments()[1] == 0xff9b)
+                || (v6.segments()[0] & 0xffc0) == 0xfec0
+        }
+    }
+}
+
+#[cfg(test)]
+#[path = "endpoint_guard_tests.rs"]
+mod tests;

crates/tinymcp/src/registry/oauth/endpoint_guard_tests.rs

@@ -0,0 +1,111 @@
+//! Unit tests for the discovery-endpoint guard.
+
+#![allow(clippy::unwrap_used, clippy::expect_used, clippy::panic)]
+
+use std::net::IpAddr;
+
+use super::{client_pinned_to, guard_endpoint, guarded_client, is_blocked_ip};
+
+fn ip(raw: &str) -> IpAddr {
+    raw.parse().unwrap()
+}
+
+#[test]
+fn internal_and_special_addresses_are_blocked() {
+    for raw in [
+        "127.0.0.1",
+        "10.1.2.3",
+        "172.16.0.1",
+        "192.168.1.1",
+        "169.254.169.254",
+        "0.0.0.0",
+        "0.1.2.3",
+        "100.64.0.1",
+        "100.127.255.254",
+        "198.18.0.1",
+        "198.19.255.254",
+        "240.0.0.1",
+        "255.255.255.255",
+        "192.0.2.1",
+        "224.0.0.1",
+        "::1",
+        "::",
+        "fd00::1",
+        "fe80::1",
+        "ff02::1",
+        "64:ff9b::7f00:1",
+        "fec0::1",
+        "::127.0.0.1",
+        "::ffff:127.0.0.1",
+        "::ffff:169.254.169.254",
+    ] {
+        assert!(is_blocked_ip(&ip(raw)), "{raw} should be blocked");
+    }
+}
+
+#[test]
+fn public_addresses_are_allowed() {
+    for raw in [
+        "8.8.8.8",
+        "1.1.1.1",
+        "2606:4700:4700::1111",
+        "::ffff:8.8.8.8",
+        "2001:4860:4860::8888",
+    ] {
+        assert!(!is_blocked_ip(&ip(raw)), "{raw} should be allowed");
+    }
+}
+
+#[tokio::test]
+async fn a_public_https_literal_passes() {
+    let addresses = guard_endpoint("https://8.8.8.8/token", "token")
+        .await
+        .expect("public https");
+    assert_eq!(addresses[0].ip(), ip("8.8.8.8"));
+}
+
+#[tokio::test]
+async fn a_public_https_literal_builds_a_pinned_client() {
+    guarded_client("https://8.8.8.8/token", "token")
+        .await
+        .expect("pinned public https client");
+}
+
+#[test]
+fn a_hostname_client_pins_the_addresses_that_were_checked() {
+    let addresses = ["8.8.8.8:443".parse().expect("socket address")];
+    client_pinned_to("public.example", &addresses).expect("pinned hostname client");
+}
+
+#[tokio::test]
+async fn plain_http_is_refused_even_to_a_public_host() {
+    let error = guard_endpoint("http://8.8.8.8/token", "token")
+        .await
+        .unwrap_err()
+        .to_string();
+    assert!(error.contains("https"), "{error}");
+}
+
+#[tokio::test]
+async fn the_cloud_metadata_address_is_refused() {
+    let error = guard_endpoint("https://169.254.169.254/latest", "registration")
+        .await
+        .unwrap_err()
+        .to_string();
+    assert!(error.contains("registration"), "{error}");
+    assert!(error.contains("disallowed"), "{error}");
+}
+
+#[tokio::test]
+async fn a_hostname_resolving_to_loopback_is_refused() {
+    assert!(
+        guard_endpoint("https://localhost/token", "token")
+            .await
+            .is_err()
+    );
+}
+
+#[tokio::test]
+async fn an_unparseable_endpoint_is_refused() {
+    assert!(guard_endpoint("not a url", "token").await.is_err());
+}