Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 25 additions & 3 deletions internal/validators/utils.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package validators

import (
"net"
"net/url"
"regexp"
"strings"
Expand Down Expand Up @@ -147,9 +148,10 @@ func IsValidRemoteURL(rawURL string) bool {
return false
}

// Reject localhost URLs for remotes (security/production concerns)
hostname := u.Hostname()
if hostname == "localhost" || hostname == "127.0.0.1" || strings.HasSuffix(hostname, ".localhost") {
// Reject localhost/loopback/unspecified/private/link-local hosts for remotes
// (security/production concerns - a remote must point at a real, publicly
// reachable, non-internal endpoint).
if isDisallowedRemoteHost(u.Hostname()) {
return false
}

Expand All @@ -160,6 +162,26 @@ func IsValidRemoteURL(rawURL string) bool {
return true
}

// isDisallowedRemoteHost reports whether hostname is not allowed as a remote
// URL host because it is loopback, unspecified, private, or link-local.
//
// hostname may be an IP literal - including bracketed IPv6 forms already
// stripped of their brackets by url.URL.Hostname (e.g. "::1"), and
// IPv4-mapped IPv6 forms (e.g. "::ffff:127.0.0.1") - or a DNS name. IP
// literals are checked with net.ParseIP and net.IP's classification methods
// so every notation for loopback/private/link-local addresses is caught, not
// just the literal strings "127.0.0.1" and "::1" the previous check used.
// DNS names fall back to the pre-existing "localhost" / "*.localhost" string
// checks, since resolving them here would require a network round trip
// during validation.
func isDisallowedRemoteHost(hostname string) bool {
if ip := net.ParseIP(hostname); ip != nil {
return ip.IsLoopback() || ip.IsUnspecified() || ip.IsPrivate() || ip.IsLinkLocalUnicast()
}

return hostname == "localhost" || strings.HasSuffix(hostname, ".localhost")
}

// IsValidTemplatedURL validates a URL with template variables against available variables
// For packages: validates that template variables reference package arguments or environment variables
// For remotes: validates that template variables reference the transport's variables map
Expand Down
56 changes: 56 additions & 0 deletions internal/validators/utils_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
package validators_test

import (
"testing"

"github.com/stretchr/testify/assert"

"github.com/modelcontextprotocol/registry/internal/validators"
)

// TestIsValidRemoteURL_LoopbackAndPrivateAddresses exercises IsValidRemoteURL
// directly against every bypass listed in the GitHub issue: the previous
// implementation only rejected the literal strings "localhost", "127.0.0.1",
// and "*.localhost", so IPv6 loopback, the rest of 127.0.0.0/8, unspecified
// addresses, IPv4-mapped loopback, and RFC1918/link-local addresses all
// passed validation despite the "no localhost allowed" contract.
func TestIsValidRemoteURL_LoopbackAndPrivateAddresses(t *testing.T) {
tests := []struct {
name string
url string
valid bool
}{
// Previously-caught cases - must keep working
{name: "literal localhost", url: "https://localhost/", valid: false},
{name: "literal localhost with port", url: "https://localhost:8443/mcp", valid: false},
{name: "localhost subdomain", url: "https://foo.localhost/", valid: false},
{name: "literal 127.0.0.1", url: "https://127.0.0.1/", valid: false},

// Previously-missed bypasses called out in the issue
{name: "IPv6 loopback", url: "https://[::1]/", valid: false},
{name: "rest of 127.0.0.0/8", url: "https://127.0.0.2/", valid: false},
{name: "127.0.0.0/8 upper range", url: "https://127.255.255.254/", valid: false},
{name: "IPv4 unspecified", url: "https://0.0.0.0/", valid: false},
{name: "IPv6 unspecified", url: "https://[::]/", valid: false},
{name: "IPv4-mapped IPv6 loopback", url: "https://[::ffff:127.0.0.1]/", valid: false},
{name: "RFC1918 10.0.0.0/8", url: "https://10.0.0.1/", valid: false},
{name: "RFC1918 172.16.0.0/12", url: "https://172.16.0.1/", valid: false},
{name: "RFC1918 192.168.0.0/16", url: "https://192.168.1.1/", valid: false},
{name: "IPv4 link-local (cloud metadata endpoint)", url: "https://169.254.169.254/", valid: false},
{name: "IPv6 unique local address", url: "https://[fc00::1]/", valid: false},
{name: "IPv6 link-local unicast", url: "https://[fe80::1]/", valid: false},

// Sanity checks - must not overblock legitimate remotes
{name: "public hostname", url: "https://example.com/mcp", valid: true},
{name: "public IPv4 address", url: "https://8.8.8.8/mcp", valid: true},
{name: "public IPv6 address", url: "https://[2001:4860:4860::8888]/mcp", valid: true},
{name: "http scheme still rejected regardless of host", url: "http://example.com/mcp", valid: false},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
got := validators.IsValidRemoteURL(tt.url)
assert.Equal(t, tt.valid, got, "IsValidRemoteURL(%q)", tt.url)
})
}
}
Loading