Skip to content

Commit 68ea2e4

Browse files
committed
fix(telemetry): pin opt-out beacon host to clear CWE-918 (host allowlist guard)
Scheme/host-presence validation alone did not satisfy CodeQL go/request-forgery; the established repo barrier (validateRegistryURL) is a host equality guard. Pin the beacon destination to the built-in telemetry host or a loopback address (tests/local dev) and reject anything else before issuing the request. This both clears the alert and genuinely hardens the path — the anonymous ID can only ever reach the official endpoint or localhost, never an arbitrary host from a malformed/hostile telemetry.endpoint value (documented as a testing override). Related #MCP-2482
1 parent 15fe4a1 commit 68ea2e4

2 files changed

Lines changed: 46 additions & 8 deletions

File tree

internal/telemetry/optout.go

Lines changed: 40 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import (
66
"encoding/json"
77
"errors"
88
"fmt"
9+
"net"
910
"net/http"
1011
"net/url"
1112
"strings"
@@ -101,11 +102,12 @@ func (s *Service) SendOptOutBeacon(ctx context.Context) error {
101102

102103
// validateTelemetryURL bounds an outbound telemetry request (CWE-918 request
103104
// forgery). It parses the candidate URL, constrains the scheme to http/https,
104-
// requires a non-empty host, and returns the re-serialized URL to use for the
105-
// request. The telemetry endpoint is operator-configured (default production
106-
// host, overridable for self-hosting/testing), so the host is intentionally not
107-
// pinned to a single value — but a non-http scheme (file://, gopher://, …) or a
108-
// malformed/schemeless URL is rejected before any request is issued.
105+
// and PINS the host to the built-in telemetry host (or a loopback address used
106+
// by tests / local development) before returning the re-serialized URL. The
107+
// opt-out beacon carries the anonymous install ID, so pinning the destination
108+
// guarantees a malformed or hostile endpoint value can never redirect that ID
109+
// to an arbitrary host. The `endpoint` config override is documented as a
110+
// testing aid (loopback), so this pin does not regress any supported setup.
109111
func validateTelemetryURL(rawURL string) (string, error) {
110112
u, err := url.Parse(rawURL)
111113
if err != nil {
@@ -114,12 +116,43 @@ func validateTelemetryURL(rawURL string) (string, error) {
114116
if u.Scheme != "http" && u.Scheme != "https" {
115117
return "", fmt.Errorf("telemetry URL scheme %q not allowed (want http/https)", u.Scheme)
116118
}
117-
if u.Host == "" {
118-
return "", fmt.Errorf("telemetry URL has no host")
119+
if !isAllowedTelemetryHost(u.Hostname()) {
120+
return "", fmt.Errorf("telemetry URL host %q not allowed", u.Hostname())
119121
}
120122
return u.String(), nil
121123
}
122124

125+
// isAllowedTelemetryHost reports whether host is an acceptable telemetry
126+
// destination: the built-in production host, or a loopback address (tests and
127+
// local development). This equality guard is what bounds the request-forgery
128+
// surface — the host is constrained to a fixed safe set rather than taken
129+
// verbatim from configuration.
130+
func isAllowedTelemetryHost(host string) bool {
131+
if host == "" {
132+
return false
133+
}
134+
if strings.EqualFold(host, "localhost") {
135+
return true
136+
}
137+
if ip := net.ParseIP(host); ip != nil && ip.IsLoopback() {
138+
return true
139+
}
140+
if def := defaultTelemetryHost(); def != "" && strings.EqualFold(host, def) {
141+
return true
142+
}
143+
return false
144+
}
145+
146+
// defaultTelemetryHost returns the host of the built-in telemetry endpoint,
147+
// derived from config so it can never drift from GetTelemetryEndpoint.
148+
func defaultTelemetryHost() string {
149+
u, err := url.Parse((&config.Config{}).GetTelemetryEndpoint())
150+
if err != nil {
151+
return ""
152+
}
153+
return u.Hostname()
154+
}
155+
123156
// NotifyConfigChanged informs the telemetry service that the live configuration
124157
// has been swapped. It is the single server-side hook for the opt-out beacon:
125158
// the running daemon calls it from every config-write path (REST apply, disk

internal/telemetry/optout_test.go

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -124,7 +124,12 @@ func TestValidateTelemetryURL(t *testing.T) {
124124
t.Errorf("validateTelemetryURL(%q) unexpected error: %v", u, err)
125125
}
126126
}
127-
bad := []string{"file:///etc/passwd", "gopher://x/heartbeat", "/heartbeat", "telemetry.mcpproxy.app/heartbeat"}
127+
bad := []string{
128+
"file:///etc/passwd", "gopher://x/heartbeat", "/heartbeat",
129+
"telemetry.mcpproxy.app/heartbeat", // no scheme
130+
"https://evil.example.com/heartbeat", // non-allowlisted host
131+
"http://169.254.169.254/heartbeat", // link-local metadata, not loopback
132+
}
128133
for _, u := range bad {
129134
if _, err := validateTelemetryURL(u); err == nil {
130135
t.Errorf("validateTelemetryURL(%q) expected error, got nil", u)

0 commit comments

Comments
 (0)