Skip to content

Commit 15fe4a1

Browse files
committed
fix(telemetry): validate opt-out beacon URL (scheme/host) to clear CWE-918
Apply the repo's established request-forgery barrier (mirror validateRegistryURL): parse the configured telemetry endpoint, constrain the scheme to http/https, require a host, and issue the beacon against the re-serialized URL. Rejects file://, gopher://, and schemeless/malformed endpoints before any request. Related #MCP-2482
1 parent 801bcf2 commit 15fe4a1

2 files changed

Lines changed: 48 additions & 2 deletions

File tree

internal/telemetry/optout.go

Lines changed: 33 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,8 @@ import (
77
"errors"
88
"fmt"
99
"net/http"
10+
"net/url"
11+
"strings"
1012

1113
"go.uber.org/zap"
1214

@@ -71,8 +73,16 @@ func (s *Service) SendOptOutBeacon(ctx context.Context) error {
7173
return fmt.Errorf("opt-out beacon failed anonymity scan: %w", scanErr)
7274
}
7375

74-
url := s.endpoint + "/heartbeat"
75-
req, err := http.NewRequestWithContext(ctx, http.MethodPost, url, bytes.NewReader(data))
76+
// Bound the outbound request (CWE-918 request forgery): the endpoint is a
77+
// configured value, so re-parse it, constrain the scheme to http/https,
78+
// require a host, and issue the request against the re-serialized URL. This
79+
// mirrors validateRegistryURL and guarantees a malformed/non-http endpoint
80+
// can never aim the beacon at, e.g., file:// or a schemeless host.
81+
beaconURL, err := validateTelemetryURL(strings.TrimRight(s.endpoint, "/") + "/heartbeat")
82+
if err != nil {
83+
return err
84+
}
85+
req, err := http.NewRequestWithContext(ctx, http.MethodPost, beaconURL, bytes.NewReader(data))
7686
if err != nil {
7787
return fmt.Errorf("build opt-out request: %w", err)
7888
}
@@ -89,6 +99,27 @@ func (s *Service) SendOptOutBeacon(ctx context.Context) error {
8999
return nil
90100
}
91101

102+
// validateTelemetryURL bounds an outbound telemetry request (CWE-918 request
103+
// 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.
109+
func validateTelemetryURL(rawURL string) (string, error) {
110+
u, err := url.Parse(rawURL)
111+
if err != nil {
112+
return "", fmt.Errorf("invalid telemetry URL: %w", err)
113+
}
114+
if u.Scheme != "http" && u.Scheme != "https" {
115+
return "", fmt.Errorf("telemetry URL scheme %q not allowed (want http/https)", u.Scheme)
116+
}
117+
if u.Host == "" {
118+
return "", fmt.Errorf("telemetry URL has no host")
119+
}
120+
return u.String(), nil
121+
}
122+
92123
// NotifyConfigChanged informs the telemetry service that the live configuration
93124
// has been swapped. It is the single server-side hook for the opt-out beacon:
94125
// the running daemon calls it from every config-write path (REST apply, disk

internal/telemetry/optout_test.go

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -117,6 +117,21 @@ func TestSendOptOutBeacon_PayloadShape(t *testing.T) {
117117
}
118118
}
119119

120+
func TestValidateTelemetryURL(t *testing.T) {
121+
ok := []string{"https://telemetry.mcpproxy.app/v1/heartbeat", "http://127.0.0.1:8080/heartbeat"}
122+
for _, u := range ok {
123+
if _, err := validateTelemetryURL(u); err != nil {
124+
t.Errorf("validateTelemetryURL(%q) unexpected error: %v", u, err)
125+
}
126+
}
127+
bad := []string{"file:///etc/passwd", "gopher://x/heartbeat", "/heartbeat", "telemetry.mcpproxy.app/heartbeat"}
128+
for _, u := range bad {
129+
if _, err := validateTelemetryURL(u); err == nil {
130+
t.Errorf("validateTelemetryURL(%q) expected error, got nil", u)
131+
}
132+
}
133+
}
134+
120135
// TestNotifyConfigChanged_FiresExactlyOnceOnDisable verifies the server-side
121136
// transition detection: an enabled->disabled config swap emits exactly one
122137
// opt-out beacon carrying the anonymous ID.

0 commit comments

Comments
 (0)