From c31dd219348bac5be398a324c4d716e0f46f080a Mon Sep 17 00:00:00 2001 From: Zezae Oh Date: Sun, 21 Jun 2026 11:37:05 +0900 Subject: [PATCH] feat(mcp): opt-in allowlist for trusted private MCP hosts (#1248) Registering a remote MCP server (sse/streamable-http) whose hostname resolves to a private IP is rejected at config-validation time by the SSRF guard, with no production escape hatch -- the only bypass is a test-only loopback flag. Self-hosted MCP servers on a private network are therefore unregisterable, even though the runtime MCP client connects to them fine (Test Connection succeeds and lists tools; only create/update input validation blocks). Add an opt-in, operator-configured allowlist (GOCLAW_MCP_ALLOWED_HOSTS, empty by default) of trusted hostnames exempt from the private/loopback IP block during MCP server URL validation only: - security.ValidateAllowingHosts(url, allowedHosts): like Validate but skips the private/loopback block for allowlisted hostnames. The cloud-metadata/link-local (169.254/fe80), multicast and unspecified ranges are never exempted, even for allowlisted hosts. - mcp.SetAllowedHosts wires the operator allowlist into ValidateURL / ValidateServerConfig; default empty => no behavior change. - web_fetch / webhook / redirect SSRF paths are unchanged (those stay agent-influenced and fully guarded). Matching is case-insensitive on the pre-resolution hostname. --- cmd/gateway.go | 1 + internal/config/config_channels.go | 1 + internal/config/config_load.go | 13 ++++++ internal/mcp/validation.go | 34 +++++++++++++- internal/mcp/validation_allowlist_test.go | 57 +++++++++++++++++++++++ internal/security/ssrf.go | 56 ++++++++++++++++++++-- internal/security/ssrf_allowlist_test.go | 47 +++++++++++++++++++ internal/security/ssrf_test.go | 2 +- 8 files changed, 203 insertions(+), 8 deletions(-) create mode 100644 internal/mcp/validation_allowlist_test.go create mode 100644 internal/security/ssrf_allowlist_test.go diff --git a/cmd/gateway.go b/cmd/gateway.go index f932f8a1..988beef5 100644 --- a/cmd/gateway.go +++ b/cmd/gateway.go @@ -387,6 +387,7 @@ func runGateway() { mcpToolLister = mcpMgr } httpapi.InitGatewayToken(cfg.Gateway.Token) + mcpbridge.SetAllowedHosts(cfg.Gateway.MCPAllowedHosts) // operator allowlist: trusted MCP hosts exempt from private-IP SSRF block httpapi.InitGatewayNoAuthFallbackAllowed(config.GatewayNoAuthFallbackAllowed(cfg.Gateway)) exportTokenStore := httpapi.InitExportTokenStore() defer exportTokenStore.Stop() diff --git a/internal/config/config_channels.go b/internal/config/config_channels.go index 52585c63..51cf2b62 100644 --- a/internal/config/config_channels.go +++ b/internal/config/config_channels.go @@ -416,6 +416,7 @@ type GatewayConfig struct { Token string `json:"token,omitempty"` // bearer token for WS/HTTP auth OwnerIDs []string `json:"owner_ids,omitempty"` // sender IDs considered "owner" AllowedOrigins []string `json:"allowed_origins,omitempty"` // WebSocket CORS whitelist (empty = allow all) + MCPAllowedHosts []string `json:"mcp_allowed_hosts,omitempty"` // trusted MCP server hostnames exempt from the private-IP SSRF block during config validation (empty = none) MaxMessageChars int `json:"max_message_chars,omitempty"` // max user message characters (default 32000) RateLimitRPM int `json:"rate_limit_rpm,omitempty"` // rate limit: requests per minute per user (default 20, 0 = disabled) InjectionAction string `json:"injection_action,omitempty"` // prompt injection action: "log", "warn" (default), "block", "off" diff --git a/internal/config/config_load.go b/internal/config/config_load.go index e215bad3..cb63775a 100644 --- a/internal/config/config_load.go +++ b/internal/config/config_load.go @@ -311,6 +311,19 @@ func (c *Config) applyEnvOverrides() { c.Gateway.AllowedOrigins = origins } + // Trusted MCP server hosts from env (comma-separated, whitespace-trimmed). + // These hosts are exempt from the private-IP SSRF block when registering MCP + // servers (e.g. self-hosted MCP on a private network). + if v := os.Getenv("GOCLAW_MCP_ALLOWED_HOSTS"); v != "" { + var hosts []string + for h := range strings.SplitSeq(v, ",") { + if trimmed := strings.TrimSpace(h); trimmed != "" { + hosts = append(hosts, trimmed) + } + } + c.Gateway.MCPAllowedHosts = hosts + } + // Tailscale (tsnet) envStr("GOCLAW_TSNET_HOSTNAME", &c.Tailscale.Hostname) envStr("GOCLAW_TSNET_AUTH_KEY", &c.Tailscale.AuthKey) diff --git a/internal/mcp/validation.go b/internal/mcp/validation.go index 6d8194b8..d73bdecc 100644 --- a/internal/mcp/validation.go +++ b/internal/mcp/validation.go @@ -7,6 +7,7 @@ import ( "regexp" "sort" "strings" + "sync/atomic" "github.com/nextlevelbuilder/goclaw/internal/security" ) @@ -380,6 +381,34 @@ func isRemoteCodeReference(arg string) bool { } } +// mcpAllowedHostsStore holds the operator-configured allowlist of MCP server +// hostnames that are exempt from the private-IP SSRF block during MCP config +// validation (see SetAllowedHosts). +var mcpAllowedHostsStore atomic.Pointer[map[string]bool] + +// SetAllowedHosts configures the MCP server hostname allowlist consulted by +// ValidateURL / ValidateServerConfig. Hostnames are matched +// case-insensitively against the pre-resolution URL host. Call once at gateway +// startup; a nil/empty slice disables the allowlist (the default, no behavior +// change). Only owner/admin gateway config should feed this. +func SetAllowedHosts(hosts []string) { + m := make(map[string]bool, len(hosts)) + for _, h := range hosts { + if h = strings.ToLower(strings.TrimSpace(h)); h != "" { + m[h] = true + } + } + mcpAllowedHostsStore.Store(&m) +} + +// allowedHosts returns the current MCP hostname allowlist, or nil if unset. +func allowedHosts() map[string]bool { + if p := mcpAllowedHostsStore.Load(); p != nil { + return *p + } + return nil +} + // ValidateURL checks URL for SSRF vulnerabilities using the existing security package. // This provides DNS rebinding protection via IP pinning. func ValidateURL(rawURL string) error { @@ -387,8 +416,9 @@ func ValidateURL(rawURL string) error { return nil } - // Reuse existing SSRF validation with DNS rebinding protection - _, _, err := security.Validate(rawURL) + // Reuse existing SSRF validation; an operator may allowlist trusted internal + // MCP hosts (private network self-host) via SetAllowedHosts. + _, _, err := security.ValidateAllowingHosts(rawURL, allowedHosts()) if err != nil { return fmt.Errorf("URL validation failed: %w", err) } diff --git a/internal/mcp/validation_allowlist_test.go b/internal/mcp/validation_allowlist_test.go new file mode 100644 index 00000000..e256691c --- /dev/null +++ b/internal/mcp/validation_allowlist_test.go @@ -0,0 +1,57 @@ +package mcp + +import ( + "testing" + + "github.com/nextlevelbuilder/goclaw/internal/security" +) + +// The package init() enables the loopback/private bypass for most validation +// tests; the SSRF allowlist tests below disable it so the private-IP block is +// actually exercised, then restore it. + +func TestSetAllowedHosts_ExemptsPrivateIPForMCP(t *testing.T) { + security.SetAllowLoopbackForTest(false) + defer security.SetAllowLoopbackForTest(true) + t.Cleanup(func() { SetAllowedHosts(nil) }) + + SetAllowedHosts([]string{"10.1.2.3"}) + if err := ValidateURL("http://10.1.2.3/mcp"); err != nil { + t.Fatalf("allowlisted private MCP host should validate, got %v", err) + } + if err := ValidateServerConfig("streamable-http", "", nil, "http://10.1.2.3/mcp"); err != nil { + t.Fatalf("allowlisted private MCP host should pass ValidateServerConfig, got %v", err) + } +} + +func TestSetAllowedHosts_DefaultStillBlocksPrivate(t *testing.T) { + security.SetAllowLoopbackForTest(false) + defer security.SetAllowLoopbackForTest(true) + SetAllowedHosts(nil) + + if err := ValidateURL("http://10.1.2.3/mcp"); err == nil { + t.Fatal("private MCP host must be rejected when allowlist is empty") + } +} + +func TestSetAllowedHosts_NeverExemptsMetadata(t *testing.T) { + security.SetAllowLoopbackForTest(false) + defer security.SetAllowLoopbackForTest(true) + t.Cleanup(func() { SetAllowedHosts(nil) }) + + SetAllowedHosts([]string{"169.254.169.254"}) + if err := ValidateURL("http://169.254.169.254/latest/meta-data"); err == nil { + t.Fatal("metadata endpoint must stay blocked even if allowlisted") + } +} + +func TestSetAllowedHosts_NormalizesEntries(t *testing.T) { + t.Cleanup(func() { SetAllowedHosts(nil) }) + SetAllowedHosts([]string{" MCP.Internal.Example ", ""}) + if !allowedHosts()["mcp.internal.example"] { + t.Fatal("expected entry to be trimmed and lowercased") + } + if _, ok := allowedHosts()[""]; ok { + t.Fatal("empty entry must be dropped") + } +} diff --git a/internal/security/ssrf.go b/internal/security/ssrf.go index 45b71a12..98bd757d 100644 --- a/internal/security/ssrf.go +++ b/internal/security/ssrf.go @@ -11,6 +11,7 @@ import ( "net" "net/http" "net/url" + "strings" "sync/atomic" "time" ) @@ -67,6 +68,37 @@ func init() { } } +// exemptableCIDRs are the private/loopback ranges that an operator-allowlisted +// MCP host may resolve into (see ValidateAllowingHosts). Cloud-metadata and +// link-local (169.254.0.0/16, fe80::/10), multicast, and unspecified ranges are +// deliberately NOT included: an allowlist entry must never open a path to the +// cloud metadata endpoint. +var exemptableCIDRs []*net.IPNet + +func init() { + for _, cidr := range []string{ + "127.0.0.0/8", "::1/128", // loopback + "10.0.0.0/8", "172.16.0.0/12", "192.168.0.0/16", "fc00::/7", // RFC 1918 + ULA + } { + _, ipNet, err := net.ParseCIDR(cidr) + if err != nil { + panic(fmt.Sprintf("security: bad exemptable CIDR %q: %v", cidr, err)) + } + exemptableCIDRs = append(exemptableCIDRs, ipNet) + } +} + +// isExemptable reports whether ip is in a private/loopback range that an +// operator-allowlisted MCP host is permitted to resolve into. +func isExemptable(ip net.IP) bool { + for _, cidr := range exemptableCIDRs { + if cidr.Contains(ip) { + return true + } + } + return false +} + // isBlocked returns true if ip falls within any blocked CIDR. func isBlocked(ip net.IP) bool { for _, cidr := range blockedCIDRs { @@ -107,11 +139,23 @@ func redactURL(rawURL string) string { // In test code, call SetAllowLoopbackForTest(true) before invoking this // function to permit httptest.NewServer addresses (127.0.0.1). func Validate(rawURL string) (*url.URL, net.IP, error) { - return validate(rawURL, allowLoopbackForTest.Load()) + return validate(rawURL, allowLoopbackForTest.Load(), nil) } -// validate is the internal implementation; allowLoopback is set only in tests. -func validate(rawURL string, allowLoopback bool) (*url.URL, net.IP, error) { +// ValidateAllowingHosts behaves like Validate but exempts an operator-configured +// set of trusted hostnames from the private/loopback IP block, so self-hosted MCP +// servers on a private network can be registered. Matching is case-insensitive on +// the pre-resolution hostname; cloud-metadata/link-local, multicast and +// unspecified ranges are still blocked even for allowlisted hosts. allowedHosts +// may be nil (identical to Validate). Intended ONLY for owner/admin MCP server +// config validation, never for agent-influenced fetch paths. +func ValidateAllowingHosts(rawURL string, allowedHosts map[string]bool) (*url.URL, net.IP, error) { + return validate(rawURL, allowLoopbackForTest.Load(), allowedHosts) +} + +// validate is the internal implementation. allowLoopback is set only in tests; +// allowedHosts is the operator MCP allowlist (nil for all other callers). +func validate(rawURL string, allowLoopback bool, allowedHosts map[string]bool) (*url.URL, net.IP, error) { redacted := redactURL(rawURL) u, err := url.Parse(rawURL) @@ -131,9 +175,11 @@ func validate(rawURL string, allowLoopback bool) (*url.URL, net.IP, error) { return nil, nil, errors.New("ssrf: empty host") } + hostAllowlisted := len(allowedHosts) > 0 && allowedHosts[strings.ToLower(host)] + // If the host is already a literal IP, validate it directly. if ip := net.ParseIP(host); ip != nil { - if !allowLoopback && isBlocked(ip) { + if !allowLoopback && isBlocked(ip) && !(hostAllowlisted && isExemptable(ip)) { slog.Warn("security.hook.ssrf_block", "url", redacted, "reason", "blocked_ip", "ip", ip.String()) return nil, nil, fmt.Errorf("ssrf: IP %s is in a blocked range", ip) } @@ -156,7 +202,7 @@ func validate(rawURL string, allowLoopback bool) (*url.URL, net.IP, error) { return nil, nil, fmt.Errorf("ssrf: resolved address %q is not a valid IP", addrs[0]) } - if !allowLoopback && isBlocked(ip) { + if !allowLoopback && isBlocked(ip) && !(hostAllowlisted && isExemptable(ip)) { slog.Warn("security.hook.ssrf_block", "url", redacted, "reason", "blocked_resolved_ip", "host", host, "ip", ip.String()) return nil, nil, fmt.Errorf("ssrf: %q resolved to blocked IP %s", host, ip) } diff --git a/internal/security/ssrf_allowlist_test.go b/internal/security/ssrf_allowlist_test.go new file mode 100644 index 00000000..daa313dc --- /dev/null +++ b/internal/security/ssrf_allowlist_test.go @@ -0,0 +1,47 @@ +package security + +import "testing" + +// ---- ValidateAllowingHosts (operator MCP host allowlist) ---- + +func TestValidateAllowingHosts_AllowsAllowlistedPrivateIP(t *testing.T) { + allowed := map[string]bool{"10.1.2.3": true} + if _, _, err := ValidateAllowingHosts("http://10.1.2.3/mcp", allowed); err != nil { + t.Fatalf("expected allowlisted private IP to pass, got %v", err) + } +} + +func TestValidateAllowingHosts_RejectsNonAllowlistedPrivateIP(t *testing.T) { + allowed := map[string]bool{"10.9.9.9": true} + if _, _, err := ValidateAllowingHosts("http://10.1.2.3/mcp", allowed); err == nil { + t.Fatal("expected non-allowlisted private IP to be rejected") + } +} + +// The metadata / link-local range must never be exempted, even if an operator +// explicitly allowlists it — this is the crown-jewel SSRF target. +func TestValidateAllowingHosts_NeverExemptsMetadataEndpoint(t *testing.T) { + allowed := map[string]bool{"169.254.169.254": true} + if _, _, err := ValidateAllowingHosts("http://169.254.169.254/latest/meta-data", allowed); err == nil { + t.Fatal("metadata/link-local must never be exempted by the allowlist") + } +} + +func TestValidateAllowingHosts_NilAllowlistMatchesValidate(t *testing.T) { + if _, _, err := ValidateAllowingHosts("http://10.1.2.3/", nil); err == nil { + t.Fatal("nil allowlist must reject private IPs like Validate") + } +} + +func TestValidateAllowingHosts_PublicIPUnaffected(t *testing.T) { + if _, _, err := ValidateAllowingHosts("http://8.8.8.8/", map[string]bool{"10.0.0.1": true}); err != nil { + t.Fatalf("public IP should pass regardless of allowlist, got %v", err) + } +} + +func TestValidateAllowingHosts_AllowsAllowlistedLoopback(t *testing.T) { + allowed := map[string]bool{"127.0.0.1": true} + if _, _, err := ValidateAllowingHosts("http://127.0.0.1/mcp", allowed); err != nil { + t.Fatalf("expected allowlisted loopback to pass, got %v", err) + } +} diff --git a/internal/security/ssrf_test.go b/internal/security/ssrf_test.go index 5a058f4d..c9cdf831 100644 --- a/internal/security/ssrf_test.go +++ b/internal/security/ssrf_test.go @@ -116,7 +116,7 @@ func TestNewSafeClient_NoRedirects(t *testing.T) { defer srv.Close() redirectTarget = srv.URL - _, pinnedIP, err := validate(srv.URL+"/", true) + _, pinnedIP, err := validate(srv.URL+"/", true, nil) if err != nil { t.Fatalf("validate: %v", err) }