From ca719b36a4e388de687a032fe34afc58a9aa209c Mon Sep 17 00:00:00 2001 From: JP DeVries Date: Wed, 19 Aug 2026 15:33:24 -0700 Subject: [PATCH 1/5] feat(mcp): allow a hosted MCP server on a literal loopback IP Reborn had no transport to an MCP server on the same machine: the admission gate rejected localhost and every IP literal, and the hosted-MCP egress plan hardcoded deny_private_ip_ranges: true, so the dispatcher denied loopback even if admission had passed. Narrow the exemption to a literal loopback IP (127.0.0.0/8 or ::1), which cannot DNS-rebind and never leaves the host. localhost, non- loopback IP literals, and public http stay rejected. The enforcement layer already permits loopback when deny_private_ip_ranges is false, so only the extension-host admission + egress-planning lane changes. Closes #5998 Co-Authored-By: Claude Opus 4.8 --- .../src/hosted_mcp_admission.rs | 64 ++++++++++++++---- .../ironclaw_extension_host/src/mcp.rs | 66 +++++++++++++++++-- 2 files changed, 112 insertions(+), 18 deletions(-) diff --git a/crates/extensions/ironclaw_extension_host/src/hosted_mcp_admission.rs b/crates/extensions/ironclaw_extension_host/src/hosted_mcp_admission.rs index 5fc79e0110..abf7f4139d 100644 --- a/crates/extensions/ironclaw_extension_host/src/hosted_mcp_admission.rs +++ b/crates/extensions/ironclaw_extension_host/src/hosted_mcp_admission.rs @@ -16,23 +16,29 @@ impl CanonicalHostedMcpEndpoint { pub fn parse(input: &HostedMcpEndpoint) -> Result { let url = url::Url::parse(input.as_str()) .map_err(|_| HostedMcpAdmissionError::InvalidEndpoint)?; - if url.scheme() != "https" - || url.host_str().is_none() + let host = url + .host() + .ok_or(HostedMcpAdmissionError::InvalidEndpoint)?; + // A literal loopback IP (127.0.0.0/8 or ::1) is a safe, non-rebindable + // on-device target: it is the single case where `http` is admitted and + // where an IP literal is allowed. Every other endpoint must be a public + // `https` URL, exactly as before. + let loopback_literal = is_loopback_ip_literal(&host); + let scheme_ok = url.scheme() == "https" || (url.scheme() == "http" && loopback_literal); + if !scheme_ok || !url.username().is_empty() || url.password().is_some() || url.fragment().is_some() { return Err(HostedMcpAdmissionError::InvalidEndpoint); } - let host = url + let host_str = url .host_str() .ok_or(HostedMcpAdmissionError::InvalidEndpoint)?; - if host.eq_ignore_ascii_case("localhost") - || matches!( - url.host(), - Some(url::Host::Ipv4(_)) | Some(url::Host::Ipv6(_)) - ) - { + // `localhost` (a DNS name a resolver could rebind) stays rejected; IP + // literals stay rejected unless they are a literal loopback address. + let is_ip_literal = matches!(host, url::Host::Ipv4(_) | url::Host::Ipv6(_)); + if host_str.eq_ignore_ascii_case("localhost") || (is_ip_literal && !loopback_literal) { return Err(HostedMcpAdmissionError::InvalidEndpoint); } // Denylist, not allowlist: query parameters are load-bearing identity @@ -81,6 +87,19 @@ impl CanonicalHostedMcpEndpoint { } } +/// A literal IPv4/IPv6 loopback address (`127.0.0.0/8` or `::1`). Hostnames +/// such as `localhost` are intentionally excluded: only a literal loopback IP +/// is exempted, so no DNS name can later rebind to a non-loopback address. +/// Shared by the admission gate above and the hosted-MCP egress planner in +/// [`crate::mcp`] so the two agree on exactly what "loopback" means. +pub(crate) fn is_loopback_ip_literal(host: &url::Host<&str>) -> bool { + match host { + url::Host::Ipv4(ip) => ip.is_loopback(), + url::Host::Ipv6(ip) => ip.is_loopback(), + url::Host::Domain(_) => false, + } +} + #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub enum HostedMcpAdmissionError { InvalidEndpoint, @@ -108,11 +127,14 @@ mod tests { fn canonical_endpoint_rejects_credential_and_private_literal_forms() { for endpoint in [ "https://user@example.test", - "https://127.0.0.1/mcp", "http://mcp.example.test", "https://mcp.example.test/rpc?Access_Token=must-not-persist", - "https://[::1]/mcp", + // `localhost` is a DNS name, not a literal loopback IP: rejected. + "http://localhost/mcp", + "https://localhost/mcp", + // A non-loopback IP literal stays rejected. "https://[2001:db8::1]/mcp", + "https://192.168.1.10/mcp", "https://mcp.example.test/rpc?client_secret=must-not-persist", "https://mcp.example.test/rpc?Password=must-not-persist", "https://mcp.example.test/rpc?signature=must-not-persist", @@ -126,6 +148,26 @@ mod tests { } } + #[test] + fn canonical_endpoint_admits_literal_loopback_over_http_or_https() { + // A literal loopback IP is a safe on-device target: `http` is admitted + // and the IP literal is allowed, with the scheme/port preserved. + for endpoint in [ + "http://127.0.0.1:5001/mcp", + "https://127.0.0.1/mcp", + "http://[::1]:5001/mcp", + "https://[::1]/mcp", + ] { + let input = HostedMcpEndpoint::new(endpoint).expect("wire endpoint"); + CanonicalHostedMcpEndpoint::parse(&input) + .unwrap_or_else(|_| panic!("loopback endpoint should be admitted: {endpoint}")); + } + + let input = HostedMcpEndpoint::new("http://127.0.0.1:5001/mcp").expect("wire endpoint"); + let endpoint = CanonicalHostedMcpEndpoint::parse(&input).expect("canonical endpoint"); + assert_eq!(endpoint.as_str(), "http://127.0.0.1:5001/mcp"); + } + #[test] fn canonical_endpoint_keeps_query_identity_and_normalizes_path() { let input = HostedMcpEndpoint::new("https://MCP.example.test/a/../rpc?b=2&a=1") diff --git a/crates/extensions/ironclaw_extension_host/src/mcp.rs b/crates/extensions/ironclaw_extension_host/src/mcp.rs index 921e97129d..5a3314a857 100644 --- a/crates/extensions/ironclaw_extension_host/src/mcp.rs +++ b/crates/extensions/ironclaw_extension_host/src/mcp.rs @@ -14,6 +14,8 @@ use ironclaw_mcp::{ McpHostHttpEgressPlanner, McpRuntime, McpRuntimeConfig, McpRuntimeHttpAdapter, }; +use crate::hosted_mcp_admission::is_loopback_ip_literal; + pub const MCP_RESPONSE_BODY_LIMIT: u64 = 2 * 1024 * 1024; const MCP_NETWORK_EGRESS_LIMIT: u64 = 2 * 1024 * 1024; const MCP_TIMEOUT_MS: u32 = 60_000; @@ -106,6 +108,11 @@ impl McpHostHttpEgressPlanner for RegistryMcpEgressPlanner { #[derive(Debug, Clone, PartialEq, Eq)] pub struct HostedMcpEgressEndpoint { + scheme: NetworkScheme, + /// True when `host_pattern` is a literal loopback IP. Such endpoints are + /// exempt from the private-range denial (see + /// [`hosted_mcp_network_policy_for_endpoint`]). + loopback: bool, host_pattern: String, port: Option, path: String, @@ -115,14 +122,24 @@ pub struct HostedMcpEgressEndpoint { impl HostedMcpEgressEndpoint { fn parse(url: &str) -> Option { let parsed = url::Url::parse(url).ok()?; - if parsed.scheme() != "https" - || !parsed.username().is_empty() + let host = parsed.host()?; + let loopback = is_loopback_ip_literal(&host); + // `http` is admitted only for a literal loopback IP; every other target + // must be `https`. Mirrors the admission gate in `hosted_mcp_admission`. + let scheme = match parsed.scheme() { + "https" => NetworkScheme::Https, + "http" if loopback => NetworkScheme::Http, + _ => return None, + }; + if !parsed.username().is_empty() || parsed.password().is_some() || parsed.fragment().is_some() { return None; } Some(Self { + scheme, + loopback, host_pattern: parsed.host_str()?.to_ascii_lowercase(), port: parsed.port(), path: normalize_mcp_path(parsed.path()), @@ -131,7 +148,7 @@ impl HostedMcpEgressEndpoint { } fn allows_target(&self, target: &NetworkTargetPattern) -> bool { - target.scheme == Some(NetworkScheme::Https) + target.scheme == Some(self.scheme) && target.host_pattern.eq_ignore_ascii_case(&self.host_pattern) && target.port == self.port } @@ -187,13 +204,15 @@ pub(crate) fn hosted_mcp_network_policy(package: &ExtensionPackage) -> Option NetworkPolicy { NetworkPolicy { allowed_targets: vec![NetworkTargetPattern { - scheme: Some(NetworkScheme::Https), + scheme: Some(endpoint.scheme), host_pattern: endpoint.host_pattern.clone(), port: endpoint.port, }], - // Matches the bundled manifest's deny_private_ip_ranges default. - // Dispatcher would reject anyway, but the plan must agree. - deny_private_ip_ranges: true, + // A literal loopback endpoint is exempt from the private-range denial: + // it is a safe on-device target and the dispatcher already permits + // loopback when this is false. Every other endpoint keeps the deny in + // force to match the bundled manifest's default. + deny_private_ip_ranges: !endpoint.loopback, max_egress_bytes: Some(MCP_NETWORK_EGRESS_LIMIT), } } @@ -220,6 +239,39 @@ mod tests { const NOTION_MCP_HOST: &str = "mcp.notion.com"; const NOTION_MCP_URL: &str = "https://mcp.notion.com/mcp"; + // ── loopback endpoints ───────────────────────────────────────────────── + + #[test] + fn loopback_endpoint_admits_http_and_waives_private_range_denial() { + let endpoint = HostedMcpEgressEndpoint::parse("http://127.0.0.1:5001/mcp") + .expect("literal loopback http endpoint is admitted"); + assert_eq!(endpoint.scheme, NetworkScheme::Http); + assert!(endpoint.loopback); + + let policy = hosted_mcp_network_policy_for_endpoint(&endpoint); + assert!(!policy.deny_private_ip_ranges); + assert_eq!(policy.allowed_targets.len(), 1); + assert_eq!(policy.allowed_targets[0].scheme, Some(NetworkScheme::Http)); + assert_eq!(policy.allowed_targets[0].host_pattern, "127.0.0.1"); + assert_eq!(policy.allowed_targets[0].port, Some(5001)); + } + + #[test] + fn non_loopback_endpoints_keep_https_and_private_range_denial() { + // Public `http` is not an MCP endpoint at all. + assert!(HostedMcpEgressEndpoint::parse("http://mcp.example.test/mcp").is_none()); + // `localhost` is a DNS name, not a literal loopback IP, so `http` is refused. + assert!(HostedMcpEgressEndpoint::parse("http://localhost:5001/mcp").is_none()); + + // A public `https` endpoint keeps the private-range denial and https scheme. + let public = HostedMcpEgressEndpoint::parse(NOTION_MCP_URL).expect("public https endpoint"); + assert_eq!(public.scheme, NetworkScheme::Https); + assert!(!public.loopback); + let policy = hosted_mcp_network_policy_for_endpoint(&public); + assert!(policy.deny_private_ip_ranges); + assert_eq!(policy.allowed_targets[0].scheme, Some(NetworkScheme::Https)); + } + // ── credential projection ────────────────────────────────────────────── #[test] From f706824ef8f5594b56d08321eff614d3a54377d7 Mon Sep 17 00:00:00 2001 From: JP DeVries Date: Wed, 19 Aug 2026 16:26:28 -0700 Subject: [PATCH 2/5] feat(webui): allow a literal loopback IP in the Add-MCP form The Add-MCP registration modal enforced its own copy of the localhost/IP restriction, so even with the backend admitting a loopback endpoint the WebUI blocked it at step 1 before any connection probe. Mirror the backend exemption: admit a literal loopback IP (127.0.0.0/8 or ::1) over http/https; keep rejecting localhost (DNS-rebindable), non-loopback IP literals, public http, and credential-bearing URLs. Co-Authored-By: Claude Opus 4.8 --- .../custom-mcp-registration-modal.test.ts | 23 +++++++++++++++++++ .../custom-mcp-registration-modal.tsx | 12 ++++++++-- 2 files changed, 33 insertions(+), 2 deletions(-) diff --git a/crates/product/ironclaw_webui/frontend/src/pages/extensions/components/custom-mcp-registration-modal.test.ts b/crates/product/ironclaw_webui/frontend/src/pages/extensions/components/custom-mcp-registration-modal.test.ts index 1179ed5b16..bca0ec7e04 100644 --- a/crates/product/ironclaw_webui/frontend/src/pages/extensions/components/custom-mcp-registration-modal.test.ts +++ b/crates/product/ironclaw_webui/frontend/src/pages/extensions/components/custom-mcp-registration-modal.test.ts @@ -115,6 +115,29 @@ test("keeps the generated ID hidden until Advanced options and validates each co assert.match(document.body.textContent || "", /customMcpIdInvalid/); }); +test("accepts a literal loopback IP over http and advances to review", () => { + renderModal(); + setInput(document.querySelectorAll("input")[0], "Pantry Host MCP"); + setInput(document.querySelectorAll("input")[2], "http://127.0.0.1:5001/mcp"); + clickButton("common.continue"); + + // A literal loopback IP is a safe on-device target: no endpoint error, and + // the wizard advances to the Review step. + assert.doesNotMatch(document.body.textContent || "", /customMcpEndpointHttps/); + assert.match(document.body.textContent || "", /customMcpReviewHint/); +}); + +test("still rejects localhost as a hosted MCP endpoint", () => { + renderModal(); + setInput(document.querySelectorAll("input")[0], "Local MCP"); + // `localhost` is a DNS name that could rebind, so it stays rejected even + // though a literal loopback IP is now allowed. + setInput(document.querySelectorAll("input")[2], "http://localhost:5001/mcp"); + clickButton("common.continue"); + + assert.match(document.body.textContent || "", /customMcpEndpointHttps/); +}); + test("review submits automatic authentication without asking the user to classify the server", () => { let payload: CustomMcpRegistrationPayload | null = null; renderModal({ onRegister: (request) => { payload = request; } }); diff --git a/crates/product/ironclaw_webui/frontend/src/pages/extensions/components/custom-mcp-registration-modal.tsx b/crates/product/ironclaw_webui/frontend/src/pages/extensions/components/custom-mcp-registration-modal.tsx index 699d9ae934..d826c81ad9 100644 --- a/crates/product/ironclaw_webui/frontend/src/pages/extensions/components/custom-mcp-registration-modal.tsx +++ b/crates/product/ironclaw_webui/frontend/src/pages/extensions/components/custom-mcp-registration-modal.tsx @@ -59,12 +59,20 @@ function connectionErrors(name: string, id: string, endpoint: string, t: (key: s const parsed = new URL(trimmedEndpoint); const host = parsed.hostname.toLowerCase(); const isIpLiteral = /^\d{1,3}(?:\.\d{1,3}){3}$/.test(host) || host.includes(":"); + // A literal loopback IP (127.0.0.0/8 or ::1) is a safe, non-rebindable + // on-device target: the one case where `http` and an IP literal are + // allowed. Mirrors the backend admission gate in `hosted_mcp_admission`. + // `localhost` (a DNS name) stays rejected. + const isLoopbackLiteral = + /^127\.\d{1,3}\.\d{1,3}\.\d{1,3}$/.test(host) || host === "[::1]" || host === "::1"; + const schemeOk = + parsed.protocol === "https:" || (parsed.protocol === "http:" && isLoopbackLiteral); const hasCredentialQuery = Array.from(parsed.searchParams.keys()).some((key) => CREDENTIAL_QUERY_KEYS.has(key.toLowerCase()), ); if ( - parsed.protocol !== "https:" || !host || parsed.username || parsed.password || parsed.hash || - host === "localhost" || isIpLiteral || hasCredentialQuery || + !schemeOk || !host || parsed.username || parsed.password || parsed.hash || + host === "localhost" || (isIpLiteral && !isLoopbackLiteral) || hasCredentialQuery || trimmedEndpoint.length > 2048 || CONTROL_CHARACTER_PATTERN.test(trimmedEndpoint) ) throw new Error("invalid hosted MCP endpoint"); } catch { From 1c565d4ed79767a305c4327f8199805f8cd7d3dd Mon Sep 17 00:00:00 2001 From: JP DeVries Date: Wed, 19 Aug 2026 23:18:34 -0700 Subject: [PATCH 3/5] fix(mcp): admit an http loopback endpoint through the registry URL gate MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The loopback exemption stopped short of the call site that decides whether a package is a hosted HTTP MCP provider at all: `valid_hosted_mcp_url` still required `https`, so `hosted_http_mcp_endpoint` returned `None` for a loopback `http` endpoint and the egress planner emitted an empty plan. The admission gate accepted the endpoint and every request was then denied. Admit `http` for a literal loopback IP here too, and derive the capability's network target scheme from the endpoint instead of assuming `https` — an allowlist entry carrying the wrong scheme never matches its own request. Co-Authored-By: Claude Opus 4.8 --- .../src/hosted_mcp_discovery.rs | 33 +++++++++++++++++-- 1 file changed, 30 insertions(+), 3 deletions(-) diff --git a/crates/extensions/ironclaw_extension_registry/src/hosted_mcp_discovery.rs b/crates/extensions/ironclaw_extension_registry/src/hosted_mcp_discovery.rs index 920b63f972..d8f142c0d7 100644 --- a/crates/extensions/ironclaw_extension_registry/src/hosted_mcp_discovery.rs +++ b/crates/extensions/ironclaw_extension_registry/src/hosted_mcp_discovery.rs @@ -101,14 +101,34 @@ fn hosted_http_mcp_url(package: &ExtensionPackage) -> Option<&str> { Some(url.as_str()) } +/// A literal IPv4/IPv6 loopback address (`127.0.0.0/8` or `::1`). Hostnames +/// such as `localhost` are excluded on purpose: only a literal loopback IP is +/// exempted, so no DNS name can later rebind to a non-loopback address. Mirrors +/// `ironclaw_extension_host::hosted_mcp_admission::is_loopback_ip_literal`, +/// duplicated rather than shared because this crate sits below that one. +fn is_loopback_ip_literal(host: &url::Host<&str>) -> bool { + match host { + url::Host::Ipv4(ip) => ip.is_loopback(), + url::Host::Ipv6(ip) => ip.is_loopback(), + url::Host::Domain(_) => false, + } +} + fn valid_hosted_mcp_url(url: &str) -> bool { let Ok(parsed) = url::Url::parse(url) else { return false; }; - parsed.scheme() == "https" + let Some(host) = parsed.host() else { + return false; + }; + // `http` is admitted only for a literal loopback IP — a same-device target + // that cannot rebind and never leaves the host. Every other endpoint must + // still be `https`, exactly as before. + let scheme_ok = + parsed.scheme() == "https" || (parsed.scheme() == "http" && is_loopback_ip_literal(&host)); + scheme_ok && parsed.username().is_empty() && parsed.password().is_none() - && parsed.host_str().is_some() && parsed.fragment().is_none() } @@ -230,7 +250,14 @@ fn discovered_capability_manifest( fn hosted_mcp_network_target(package: &ExtensionPackage) -> Option { let endpoint = url::Url::parse(hosted_http_mcp_url(package)?).ok()?; Some(NetworkTargetPattern { - scheme: Some(NetworkScheme::Https), + // Derived from the endpoint rather than assumed `https`: a loopback + // provider is reachable over `http`, and an allowlist entry carrying + // the wrong scheme would never match its own request. + scheme: Some(if endpoint.scheme() == "http" { + NetworkScheme::Http + } else { + NetworkScheme::Https + }), host_pattern: endpoint.host_str()?.to_ascii_lowercase(), port: endpoint.port(), }) From 70e84365501fe23d7f9942b1b0c0672a15cd52bf Mon Sep 17 00:00:00 2001 From: JP DeVries Date: Wed, 19 Aug 2026 23:18:42 -0700 Subject: [PATCH 4/5] fix(mcp): waive the private-range denial for a loopback-only allowlist Registering a loopback hosted MCP succeeded and discovery published its tools, but every tool call was denied with `policy_denied`. Discovery stages its own endpoint policy, while a dispatched call is governed by the staged grant obligation built from `extension_network_policy`, which set `deny_private_ip_ranges` for any capability with egress targets. The capability's allowlist named the loopback host and the guard then refused it. Waive the denial only when *every* target in the allowlist is a literal loopback IP. A single non-loopback target re-arms the guard, and `localhost` never qualifies, so the SSRF boundary is unchanged for every other shape. The policy stays constrained by its non-empty allowlist, so the `ApplyNetworkPolicy` obligation is still emitted. Co-Authored-By: Claude Opus 4.8 --- .../src/capability_surface.rs | 59 ++++++++++++++++++- .../src/hosted_mcp_admission.rs | 16 ++++- 2 files changed, 71 insertions(+), 4 deletions(-) diff --git a/crates/extensions/ironclaw_extension_host/src/capability_surface.rs b/crates/extensions/ironclaw_extension_host/src/capability_surface.rs index 14c40e7431..388c6b387e 100644 --- a/crates/extensions/ironclaw_extension_host/src/capability_surface.rs +++ b/crates/extensions/ironclaw_extension_host/src/capability_surface.rs @@ -198,9 +198,18 @@ pub fn extension_network_policy(capability: &ActiveExtensionCapability) -> Netwo // declares the `network` effect but no targets is still caught by the // effect-based obligation gate and fails as misconfigured.) let has_egress_targets = !targets.is_empty(); + // A capability whose every target is a literal loopback IP is exempt from + // the private-range denial — the same on-device boundary the hosted-MCP + // egress plan holds. Keeping the deny here would allowlist the loopback + // host and then refuse it anyway. `localhost` and non-loopback literals + // are not loopback patterns, so they keep the guard. + let all_targets_loopback = has_egress_targets + && targets.iter().all(|target| { + crate::hosted_mcp_admission::is_loopback_host_pattern(&target.host_pattern) + }); NetworkPolicy { allowed_targets: targets, - deny_private_ip_ranges: has_egress_targets, + deny_private_ip_ranges: has_egress_targets && !all_targets_loopback, max_egress_bytes: capability.max_egress_bytes.filter(|_| has_egress_targets), } } @@ -426,6 +435,54 @@ mod tests { assert_eq!(policy.max_egress_bytes, None); } + #[test] + fn loopback_only_targets_waive_the_private_range_denial() { + // A hosted MCP on a literal loopback IP: allowlisting the host and then + // denying private ranges would refuse the very target we just allowed. + // Same on-device boundary the hosted-MCP egress plan holds. + let loopback = NetworkTargetPattern { + scheme: Some(NetworkScheme::Https), + host_pattern: "127.0.0.1".to_string(), + port: Some(5443), + }; + let capability = ActiveExtensionCapability { + id: CapabilityId::new("mcp-pantry.search_pantry").unwrap(), + provider: ExtensionId::new("mcp-pantry").unwrap(), + effects: vec![EffectKind::DispatchCapability, EffectKind::Network], + default_permission: PermissionMode::Allow, + runtime_credentials: Vec::new(), + network_targets: vec![loopback.clone()], + max_egress_bytes: None, + owner: ironclaw_extension_registry::InstallationOwner::Tenant, + }; + + let policy = extension_network_policy(&capability); + + assert_eq!(policy.allowed_targets, vec![loopback.clone()]); + assert!( + !policy.deny_private_ip_ranges, + "a loopback-only allowlist waives the private-range guard" + ); + assert!( + !policy.allowed_targets.is_empty(), + "the policy stays constrained by its allowlist, so the obligation is still emitted" + ); + + // One non-loopback target anywhere in the set re-arms the guard, and a + // DNS name that merely resolves to loopback never qualifies. + for other in [https("news.ycombinator.com"), https("localhost")] { + let mixed = ActiveExtensionCapability { + network_targets: vec![loopback.clone(), other.clone()], + ..capability.clone() + }; + assert!( + extension_network_policy(&mixed).deny_private_ip_ranges, + "a non-loopback target ({}) must keep the SSRF guard", + other.host_pattern + ); + } + } + #[test] fn manifest_network_target_deduplicates_matching_credential_audience() { // A host declared in `network_targets` that also appears as a credential diff --git a/crates/extensions/ironclaw_extension_host/src/hosted_mcp_admission.rs b/crates/extensions/ironclaw_extension_host/src/hosted_mcp_admission.rs index abf7f4139d..df2ef3a254 100644 --- a/crates/extensions/ironclaw_extension_host/src/hosted_mcp_admission.rs +++ b/crates/extensions/ironclaw_extension_host/src/hosted_mcp_admission.rs @@ -16,9 +16,7 @@ impl CanonicalHostedMcpEndpoint { pub fn parse(input: &HostedMcpEndpoint) -> Result { let url = url::Url::parse(input.as_str()) .map_err(|_| HostedMcpAdmissionError::InvalidEndpoint)?; - let host = url - .host() - .ok_or(HostedMcpAdmissionError::InvalidEndpoint)?; + let host = url.host().ok_or(HostedMcpAdmissionError::InvalidEndpoint)?; // A literal loopback IP (127.0.0.0/8 or ::1) is a safe, non-rebindable // on-device target: it is the single case where `http` is admitted and // where an IP literal is allowed. Every other endpoint must be a public @@ -100,6 +98,18 @@ pub(crate) fn is_loopback_ip_literal(host: &url::Host<&str>) -> bool { } } +/// [`is_loopback_ip_literal`] for a stored `NetworkTargetPattern` host, which +/// is a bare string rather than a parsed URL host. IPv6 patterns may or may not +/// carry the URL bracket form, so both are accepted. A wildcard or DNS pattern +/// never parses as an IP and is therefore never loopback. +pub(crate) fn is_loopback_host_pattern(host_pattern: &str) -> bool { + host_pattern + .trim_start_matches('[') + .trim_end_matches(']') + .parse::() + .is_ok_and(|ip| ip.is_loopback()) +} + #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub enum HostedMcpAdmissionError { InvalidEndpoint, From 96356e30f69656265c0b0367721be977dabb97af Mon Sep 17 00:00:00 2001 From: JP DeVries Date: Wed, 19 Aug 2026 23:18:49 -0700 Subject: [PATCH 5/5] fix(mcp): reject localhost and non-loopback literals in the egress parser MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review follow-up. `HostedMcpEgressEndpoint::parse` gated only the scheme on loopback, so `https://localhost/mcp` and `https://8.8.8.8/mcp` still produced an egress endpoint even though `CanonicalHostedMcpEndpoint::parse` rejects both. Host-bundled manifests never pass through admission, so this parser is their only host gate — mirror the same rule here. Also covers the loopback path through `RegistryMcpEgressPlanner::plan` rather than only the helpers, which is what surfaced the registry URL gate fixed in the parent commit, plus the IPv6 `[::1]` branch of the Add-MCP form. Co-Authored-By: Claude Opus 4.8 --- .../ironclaw_extension_host/src/mcp.rs | 111 +++++++++++++++++- .../custom-mcp-registration-modal.test.ts | 12 ++ 2 files changed, 122 insertions(+), 1 deletion(-) diff --git a/crates/extensions/ironclaw_extension_host/src/mcp.rs b/crates/extensions/ironclaw_extension_host/src/mcp.rs index 5a3314a857..f3c55039c8 100644 --- a/crates/extensions/ironclaw_extension_host/src/mcp.rs +++ b/crates/extensions/ironclaw_extension_host/src/mcp.rs @@ -137,10 +137,21 @@ impl HostedMcpEgressEndpoint { { return None; } + let host_pattern = parsed.host_str()?.to_ascii_lowercase(); + // The same host rule the admission gate holds, restated here because + // this parser also runs for host-bundled manifests, which never pass + // through admission: `localhost` is a DNS name a resolver could rebind, + // and an IP literal is only ever admitted when it is loopback. Without + // this, a bundled manifest declaring `https://localhost/mcp` or + // `https://8.8.8.8/mcp` would still yield an egress policy. + let is_ip_literal = matches!(host, url::Host::Ipv4(_) | url::Host::Ipv6(_)); + if host_pattern == "localhost" || (is_ip_literal && !loopback) { + return None; + } Some(Self { scheme, loopback, - host_pattern: parsed.host_str()?.to_ascii_lowercase(), + host_pattern, port: parsed.port(), path: normalize_mcp_path(parsed.path()), query: parsed.query().map(str::to_string), @@ -320,6 +331,104 @@ mod tests { assert!(plan.network_policy.allowed_targets.is_empty()); } + /// The loopback exemption through the real call site: `plan()` resolves the + /// package endpoint, matches the request URL, and emits the final policy. + /// The parse/policy unit tests above cover the helpers in isolation; this + /// asserts the production path a dispatched MCP request actually takes. + #[test] + fn planner_emits_http_loopback_plan_without_private_range_denial() { + let registry = Arc::new(SharedExtensionRegistry::new(registry_with_provider( + "local-mcp", + "http://127.0.0.1:5001/mcp", + "local-mcp.search", + "local_token", + ))); + let planner = RegistryMcpEgressPlanner::new(registry); + let provider = ExtensionId::new("local-mcp").unwrap(); + let cap = CapabilityId::new("local-mcp.search").unwrap(); + let scope = sample_scope(); + + let plan = planner.plan(sample_plan_request( + &provider, + &cap, + "http://127.0.0.1:5001/mcp", + &scope, + )); + + assert_eq!( + plan.network_policy.allowed_targets, + vec![NetworkTargetPattern { + scheme: Some(NetworkScheme::Http), + host_pattern: "127.0.0.1".to_string(), + port: Some(5001), + }] + ); + assert!( + !plan.network_policy.deny_private_ip_ranges, + "a literal loopback endpoint waives the private-range denial" + ); + } + + /// A non-loopback provider reached over the planner keeps the guard, so the + /// exemption above cannot be read as "the planner stopped denying". + #[test] + fn planner_keeps_private_range_denial_for_a_public_provider() { + let registry = Arc::new(SharedExtensionRegistry::new(registry_with_provider( + "fixture", + "https://fixture.example.com/mcp", + "fixture.search", + "fixture_token", + ))); + let planner = RegistryMcpEgressPlanner::new(registry); + let provider = ExtensionId::new("fixture").unwrap(); + let cap = CapabilityId::new("fixture.search").unwrap(); + let scope = sample_scope(); + + let plan = planner.plan(sample_plan_request( + &provider, + &cap, + "https://fixture.example.com/mcp", + &scope, + )); + + assert!(plan.network_policy.deny_private_ip_ranges); + } + + /// `localhost` and a non-loopback IP literal never produce an egress plan, + /// even though both are `https`. Host-bundled manifests never pass through + /// `hosted_mcp_admission`, so this parser is their only host gate. + #[test] + fn planner_denies_localhost_and_non_loopback_ip_literal_providers() { + for url in [ + "https://localhost/mcp", + "https://8.8.8.8/mcp", + "https://[2001:db8::1]/mcp", + ] { + assert!( + HostedMcpEgressEndpoint::parse(url).is_none(), + "{url} must not yield an egress endpoint" + ); + + let registry = Arc::new(SharedExtensionRegistry::new(registry_with_provider( + "rebindable", + url, + "rebindable.search", + "rebindable_token", + ))); + let planner = RegistryMcpEgressPlanner::new(registry); + let provider = ExtensionId::new("rebindable").unwrap(); + let cap = CapabilityId::new("rebindable.search").unwrap(); + let scope = sample_scope(); + + let plan = planner.plan(sample_plan_request(&provider, &cap, url, &scope)); + + assert!( + plan.network_policy.allowed_targets.is_empty(), + "{url} must not produce an egress allowlist" + ); + } + } + #[test] fn planner_accepts_any_host_bundled_http_mcp_provider() { let registry = Arc::new(SharedExtensionRegistry::new(registry_with_provider( diff --git a/crates/product/ironclaw_webui/frontend/src/pages/extensions/components/custom-mcp-registration-modal.test.ts b/crates/product/ironclaw_webui/frontend/src/pages/extensions/components/custom-mcp-registration-modal.test.ts index bca0ec7e04..add9b1a25d 100644 --- a/crates/product/ironclaw_webui/frontend/src/pages/extensions/components/custom-mcp-registration-modal.test.ts +++ b/crates/product/ironclaw_webui/frontend/src/pages/extensions/components/custom-mcp-registration-modal.test.ts @@ -127,6 +127,18 @@ test("accepts a literal loopback IP over http and advances to review", () => { assert.match(document.body.textContent || "", /customMcpReviewHint/); }); +test("accepts the IPv6 loopback literal over http and advances to review", () => { + renderModal(); + setInput(document.querySelectorAll("input")[0], "Pantry Host MCP v6"); + // `::1` is the other half of the loopback contract the modal accepts, and it + // exercises the bracketed-authority parse that the IPv4 case cannot. + setInput(document.querySelectorAll("input")[2], "http://[::1]:5001/mcp"); + clickButton("common.continue"); + + assert.doesNotMatch(document.body.textContent || "", /customMcpEndpointHttps/); + assert.match(document.body.textContent || "", /customMcpReviewHint/); +}); + test("still rejects localhost as a hosted MCP endpoint", () => { renderModal(); setInput(document.querySelectorAll("input")[0], "Local MCP");