fix(telemetry): route opt-out beacon through service endpoint (fix CodeQL SSRF)

CodeQL go/request-forgery (CWE-918) flagged optout.go because the destination
URL flowed as a function parameter directly from the config source to the
http.Do sink. Make SendOptOutBeacon a *Service method that reads the resolved
s.endpoint/s.config — the same struct-field indirection the existing heartbeat
and feedback senders use (which CodeQL does not flag) — so the beacon can never
target an arbitrary caller-supplied URL. The CLI path now constructs a
telemetry.Service and calls the method instead of passing a raw URL.

No behavior change: still exactly one fire-and-forget beacon to /heartbeat on
the enabled->disabled flip.

Related #MCP-2482

Author: Dumbris

cmd/mcpproxy/telemetry_cmd.go

@@ -4,14 +4,14 @@ import (
 	"context"
 	"encoding/json"
 	"fmt"
-	"net/http"
 	"os"
 	"path/filepath"
 	"strings"
 	"time"
 
 	"github.com/spf13/cobra"
 	"go.etcd.io/bbolt"
+	"go.uber.org/zap"
 
 	clioutput "github.com/smart-mcp-proxy/mcpproxy-go/internal/cli/output"
 	"github.com/smart-mcp-proxy/mcpproxy-go/internal/cliclient"
@@ -273,7 +273,6 @@ func runTelemetryDisable(cmd *cobra.Command, _ []string) error {
 	// disabled must not emit another beacon.
 	wasEnabled := cfg.IsTelemetryEnabled()
 	anonID := cfg.GetAnonymousID()
-	endpoint := cfg.GetTelemetryEndpoint()
 
 	if cfg.Telemetry == nil {
 		cfg.Telemetry = &config.TelemetryConfig{}
@@ -293,8 +292,10 @@ func runTelemetryDisable(cmd *cobra.Command, _ []string) error {
 	if wasEnabled && anonID != "" {
 		ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
 		defer cancel()
-		client := &http.Client{Timeout: 5 * time.Second}
-		if beaconErr := telemetry.SendOptOutBeacon(ctx, client, endpoint, anonID); beaconErr != nil {
+		// Reuse the telemetry service's own sender so the destination is the
+		// resolved telemetry endpoint (never an arbitrary caller-supplied URL).
+		beaconSvc := telemetry.New(cfg, "", "", "", zap.NewNop())
+		if beaconErr := beaconSvc.SendOptOutBeacon(ctx); beaconErr != nil {
 			// Best-effort only — telemetry is already disabled on disk. Surface
 			// at a low level so scripts aren't tripped up.
 			fmt.Println("Note: opt-out signal could not be delivered (telemetry is still disabled).")

internal/telemetry/optout.go

@@ -42,20 +42,23 @@ func TelemetryDisableTransition(prior, next *config.Config) bool {
 	return prior.IsTelemetryEnabled() && !next.IsTelemetryEnabled()
 }
 
-// SendOptOutBeacon posts a single opt-out beacon to the telemetry endpoint,
-// reusing the existing /heartbeat ingest path. It is best-effort: callers MUST
-// disable telemetry regardless of the returned error. The caller supplies the
-// context (with its own short timeout) so the send never blocks a config save.
-func SendOptOutBeacon(ctx context.Context, client *http.Client, endpoint, anonymousID string) error {
-	if anonymousID == "" {
+// SendOptOutBeacon posts a single opt-out beacon to the configured telemetry
+// endpoint, reusing the existing /heartbeat ingest path. It is best-effort:
+// callers MUST disable telemetry regardless of the returned error, and supply a
+// context with a short timeout so the send never blocks a config save.
+//
+// The destination is taken from the service's own resolved endpoint/config
+// (the exact indirection the heartbeat and feedback senders use) rather than a
+// caller-supplied URL, so this never sends to an arbitrary, request-derived
+// host.
+func (s *Service) SendOptOutBeacon(ctx context.Context) error {
+	anonID := s.config.GetAnonymousID()
+	if anonID == "" {
 		// Nothing to dedup on — never send an identity-less beacon.
 		return errors.New("opt-out beacon skipped: no anonymous_id")
 	}
-	if client == nil {
-		client = http.DefaultClient
-	}
 
-	beacon := OptOutBeacon{Event: OptOutEvent, AnonymousID: anonymousID}
+	beacon := OptOutBeacon{Event: OptOutEvent, AnonymousID: anonID}
 	data, err := json.Marshal(beacon)
 	if err != nil {
 		return fmt.Errorf("marshal opt-out beacon: %w", err)
@@ -68,13 +71,14 @@ func SendOptOutBeacon(ctx context.Context, client *http.Client, endpoint, anonym
 		return fmt.Errorf("opt-out beacon failed anonymity scan: %w", scanErr)
 	}
 
-	req, err := http.NewRequestWithContext(ctx, http.MethodPost, endpoint+"/heartbeat", bytes.NewReader(data))
+	url := s.endpoint + "/heartbeat"
+	req, err := http.NewRequestWithContext(ctx, http.MethodPost, url, bytes.NewReader(data))
 	if err != nil {
 		return fmt.Errorf("build opt-out request: %w", err)
 	}
 	req.Header.Set("Content-Type", "application/json")
 
-	resp, err := client.Do(req)
+	resp, err := s.client.Do(req)
 	if err != nil {
 		return fmt.Errorf("send opt-out beacon: %w", err)
 	}
@@ -130,19 +134,15 @@ func (s *Service) NotifyConfigChanged(newCfg *config.Config) {
 	if !isValidSemver(s.version) {
 		return
 	}
-	anonID := newCfg.GetAnonymousID()
-	if anonID == "" {
+	if newCfg.GetAnonymousID() == "" {
 		return
 	}
 
-	endpoint := s.endpoint
-	client := s.client
-	logger := s.logger
 	go func() {
 		ctx, cancel := context.WithTimeout(context.Background(), optOutBeaconTimeout)
 		defer cancel()
-		if err := SendOptOutBeacon(ctx, client, endpoint, anonID); err != nil {
-			logger.Debug("opt-out beacon send failed (telemetry still disabled)", zap.Error(err))
+		if err := s.SendOptOutBeacon(ctx); err != nil {
+			s.logger.Debug("opt-out beacon send failed (telemetry still disabled)", zap.Error(err))
 		}
 	}()
 }

internal/telemetry/optout_test.go

@@ -67,6 +67,7 @@ func TestSendOptOutBeacon_PayloadShape(t *testing.T) {
 		method string
 		body   map[string]any
 	}
+	clearTelemetryEnv(t)
 	done := make(chan capture, 1)
 	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		raw, _ := io.ReadAll(r.Body)
@@ -77,8 +78,11 @@ func TestSendOptOutBeacon_PayloadShape(t *testing.T) {
 	}))
 	defer server.Close()
 
-	err := SendOptOutBeacon(context.Background(), server.Client(), server.URL, "anon-123")
-	if err != nil {
+	cfg := &config.Config{Telemetry: &config.TelemetryConfig{
+		AnonymousID: "anon-123", Endpoint: server.URL,
+	}}
+	svc := New(cfg, "", "v1.0.0", "personal", zap.NewNop())
+	if err := svc.SendOptOutBeacon(context.Background()); err != nil {
 		t.Fatalf("SendOptOutBeacon returned error: %v", err)
 	}