fix(gateway): lock client-management to loopback + harden Host advertising

Addresses the PR review:
- S1: the client-management endpoints (GET/PUT/DELETE /oauth/clients[/{id}]) are
  desktop-only but became LAN-reachable on a 0.0.0.0 bind (the router is shared;
  rmcp's host allowlist only gates /mcp). Reject them from a non-loopback peer
  via a ConnectInfo-based middleware (peer IP, not the spoofable Host header).
  /oauth/clients/{id}/features and the OAuth flow stay reachable. Serve now uses
  into_make_service_with_connect_info.
- S2: validate the Host-derived advertised origin (reject embedded userinfo /
  path / garbage; preserve port + IPv6) instead of reflecting it verbatim.
- B1: document in the Settings warning that per-client OAuth can't complete
  approval over the network (approval is desktop-local); the supported LAN
  models are auth-disabled sharing or the public-URL tunnel.

All 144 gateway unit tests + 24 streamable_http integration tests pass.

Signed-off-by: Mohammod Al Amin Ashik <maa.ashik00@gmail.com>

Author: its-mash

apps/desktop/src/features/settings/SettingsPage.tsx

@@ -885,6 +885,13 @@ export function SettingsPage() {
                                 </span>
                                 .
                               </p>
+                              <p className="mt-1 text-amber-700 dark:text-amber-300">
+                                Per-client OAuth approval happens on this machine, so a remote
+                                client that signs in via OAuth (e.g. ChatGPT) can't finish approval
+                                over the network yet — front the gateway with the public URL + a
+                                tunnel for that. For plain LAN sharing, pair this with
+                                authentication disabled.
+                              </p>
                             </>
                           )}
                         </div>

crates/mcpmux-gateway/src/server/handlers.rs

@@ -66,6 +66,27 @@ pub struct OAuthServerMetadata {
     pub client_id_metadata_document_supported: Option<bool>,
 }
 
+/// Build an `http://HOST` (or `http://HOST:PORT`) origin from a request `Host`
+/// header. Returns `None` when the value is not a bare authority — it carries
+/// userinfo, a path, or is otherwise unparseable — so the caller keeps the
+/// configured local base instead of advertising an attacker-shaped value.
+fn host_to_http_origin(host: &str) -> Option<String> {
+    let parsed = url::Url::parse(&format!("http://{host}")).ok()?;
+    if !parsed.username().is_empty()
+        || parsed.password().is_some()
+        || parsed.path() != "/"
+        || parsed.query().is_some()
+        || parsed.fragment().is_some()
+    {
+        return None;
+    }
+    let host_str = parsed.host_str()?;
+    Some(match parsed.port() {
+        Some(port) => format!("http://{host_str}:{port}"),
+        None => format!("http://{host_str}"),
+    })
+}
+
 /// The base URL to advertise in OAuth / MCP metadata.
 ///
 /// Precedence:
@@ -85,8 +106,12 @@ pub(crate) fn effective_base_url(
         return public.trim_end_matches('/').to_string();
     }
     if network_bind {
-        if let Some(host) = host_header.map(str::trim).filter(|s| !s.is_empty()) {
-            return format!("http://{}", host.trim_end_matches('/'));
+        if let Some(origin) = host_header
+            .map(str::trim)
+            .filter(|s| !s.is_empty())
+            .and_then(host_to_http_origin)
+        {
+            return origin;
         }
     }
     local_fallback.trim_end_matches('/').to_string()
@@ -132,6 +157,32 @@ mod base_url_tests {
             LOCAL
         );
     }
+
+    #[test]
+    fn network_bind_rejects_malformed_host_and_falls_back() {
+        // Embedded userinfo, a path, or garbage is not advertised.
+        assert_eq!(
+            effective_base_url(None, true, Some("user@evil.example"), LOCAL),
+            LOCAL
+        );
+        assert_eq!(
+            effective_base_url(None, true, Some("evil.example/oauth/authorize"), LOCAL),
+            LOCAL
+        );
+        assert_eq!(effective_base_url(None, true, Some("   "), LOCAL), LOCAL);
+    }
+
+    #[test]
+    fn network_bind_preserves_port_and_ipv6_host() {
+        assert_eq!(
+            effective_base_url(None, true, Some("[::1]:45818"), LOCAL),
+            "http://[::1]:45818"
+        );
+        assert_eq!(
+            effective_base_url(None, true, Some("host.local"), LOCAL),
+            "http://host.local"
+        );
+    }
 }
 
 /// OAuth metadata endpoint (RFC 8414)

crates/mcpmux-gateway/src/server/mod.rs

@@ -30,7 +30,9 @@ pub use startup::{AutoConnectResult, StartupOrchestrator, TokenRefreshResult};
 pub use state::{ClientSession, GatewayState};
 
 use axum::{
+    extract::ConnectInfo,
     middleware,
+    response::IntoResponse,
     routing::{delete, get, post, put},
     Router,
 };
@@ -160,6 +162,46 @@ impl GatewayConfig {
     }
 }
 
+/// The desktop-only client-management routes (list / update / delete clients).
+/// `/oauth/clients/{id}/features` is intentionally excluded — it is the public
+/// client-facing endpoint.
+fn is_management_path(path: &str) -> bool {
+    path == "/oauth/clients"
+        || (path.starts_with("/oauth/clients/") && !path.ends_with("/features"))
+}
+
+/// Reject the desktop-only client-management endpoints when the request comes
+/// from a non-loopback peer.
+///
+/// On a loopback bind every peer is local, so this is a no-op. On a `0.0.0.0`
+/// (network) bind the whole router is exposed, but client enumeration / CRUD
+/// must stay off the LAN — the OAuth flow and `/oauth/clients/{id}/features`
+/// remain reachable. The peer socket address (not the spoofable `Host` header)
+/// is the trust signal. Falls open only when no peer address is available
+/// (an embedded/test server without `ConnectInfo`), which never happens on the
+/// real network listener.
+async fn restrict_management_to_loopback(
+    request: axum::extract::Request,
+    next: middleware::Next,
+) -> axum::response::Response {
+    if is_management_path(request.uri().path()) {
+        let peer_is_local = request
+            .extensions()
+            .get::<ConnectInfo<SocketAddr>>()
+            .map(|info| info.0.ip().is_loopback())
+            .unwrap_or(true);
+        if !peer_is_local {
+            warn!("[Gateway] Rejected non-loopback access to a client-management endpoint");
+            return (
+                axum::http::StatusCode::FORBIDDEN,
+                "Client management is only available from this machine",
+            )
+                .into_response();
+        }
+    }
+    next.run(request).await
+}
+
 /// MCP Gateway Server
 ///
 /// Self-contained server that manages its own services and lifecycle.
@@ -465,7 +507,9 @@ impl GatewayServer {
             ))
             // Rate limiting on OAuth endpoints
             .layer(axum::Extension(rate_limiter))
-            .layer(middleware::from_fn(rate_limit::rate_limit_middleware));
+            .layer(middleware::from_fn(rate_limit::rate_limit_middleware))
+            // Keep desktop-only client management off the LAN on a 0.0.0.0 bind.
+            .layer(middleware::from_fn(restrict_management_to_loopback));
 
         // Add CORS if enabled
         if self.config.enable_cors {
@@ -581,12 +625,15 @@ impl GatewayServer {
 
         info!("[Gateway] Ready to accept connections (servers connecting in background)");
 
-        axum::serve(listener, router)
-            .with_graceful_shutdown(async move {
-                shutdown.await;
-                info!("[Gateway] Graceful shutdown signal received — closing listener");
-            })
-            .await?;
+        axum::serve(
+            listener,
+            router.into_make_service_with_connect_info::<SocketAddr>(),
+        )
+        .with_graceful_shutdown(async move {
+            shutdown.await;
+            info!("[Gateway] Graceful shutdown signal received — closing listener");
+        })
+        .await?;
 
         info!("[Gateway] Listener closed, run_with_shutdown returning");
         Ok(())
@@ -730,4 +777,16 @@ mod config_tests {
             .allowed_hosts()
             .contains(&"localhost".to_string()));
     }
+
+    #[test]
+    fn management_path_matching_excludes_features_and_oauth_flow() {
+        assert!(super::is_management_path("/oauth/clients")); // list
+        assert!(super::is_management_path("/oauth/clients/abc123")); // update/delete
+                                                                     // Client-facing + OAuth-flow + other routes are NOT loopback-gated.
+        assert!(!super::is_management_path("/oauth/clients/abc123/features"));
+        assert!(!super::is_management_path("/oauth/authorize"));
+        assert!(!super::is_management_path("/oauth/token"));
+        assert!(!super::is_management_path("/mcp"));
+        assert!(!super::is_management_path("/health"));
+    }
 }