fix(observability): mask ?apikey= in the http.url span attribute

internal/observability/tracing.go

@@ -4,6 +4,7 @@ import (
 	"context"
 	"fmt"
 	"net/http"
+	"net/url"
 
 	"go.opentelemetry.io/otel"
 	"go.opentelemetry.io/otel/attribute"
@@ -15,6 +16,8 @@ import (
 	semconv "go.opentelemetry.io/otel/semconv/v1.4.0"
 	oteltrace "go.opentelemetry.io/otel/trace"
 	"go.uber.org/zap"
+
+	"github.com/smart-mcp-proxy/mcpproxy-go/internal/oauth"
 )
 
 // OTLP transport protocols.
@@ -169,7 +172,7 @@ func (tm *TracingManager) HTTPMiddleware() func(http.Handler) http.Handler {
 			ctx, span := tm.tracer.Start(ctx, spanName,
 				oteltrace.WithAttributes(
 					semconv.HTTPMethodKey.String(r.Method),
-					semconv.HTTPURLKey.String(r.URL.String()),
+					semconv.HTTPURLKey.String(traceSafeURL(r.URL)),
 					semconv.HTTPSchemeKey.String(r.URL.Scheme),
 					semconv.HTTPHostKey.String(r.Host),
 					semconv.HTTPTargetKey.String(r.URL.Path),
@@ -200,6 +203,16 @@ func (tm *TracingManager) HTTPMiddleware() func(http.Handler) http.Handler {
 	}
 }
 
+// traceSafeURL renders the request URL for the http.url span attribute with
+// credential-bearing query parameters masked. ?apikey= is an accepted
+// credential (the Web UI's SSE stream and the tray send the admin key that
+// way), and spans are exported to an external collector.
+func traceSafeURL(u *url.URL) string {
+	safe := *u
+	safe.RawQuery = oauth.LogSafeQueryString(u.RawQuery)
+	return safe.String()
+}
+
 // tracingResponseWriter wraps http.ResponseWriter to capture status code for tracing
 type tracingResponseWriter struct {
 	http.ResponseWriter

internal/observability/tracing_redact_test.go

@@ -0,0 +1,54 @@
+package observability
+
+import (
+	"net/http"
+	"net/http/httptest"
+	"strings"
+	"testing"
+
+	sdktrace "go.opentelemetry.io/otel/sdk/trace"
+	"go.opentelemetry.io/otel/sdk/trace/tracetest"
+	semconv "go.opentelemetry.io/otel/semconv/v1.4.0"
+	"go.uber.org/zap"
+)
+
+// The Web UI's SSE stream and the tray send the admin key as ?apikey=, so the
+// http.url span attribute must not carry it to the OTLP collector.
+func TestHTTPMiddleware_MasksAPIKeyInSpanURL(t *testing.T) {
+	recorder := tracetest.NewSpanRecorder()
+	provider := sdktrace.NewTracerProvider(sdktrace.WithSpanProcessor(recorder))
+	t.Cleanup(func() { _ = provider.Shutdown(t.Context()) })
+
+	tm := &TracingManager{
+		logger:  zap.NewNop().Sugar(),
+		enabled: true,
+		tracer:  provider.Tracer("test"),
+	}
+
+	const key = "3f9a0c1d2e4b5a6978c0d1e2f3a4b5c6d7e8f90a1b2c3d4e5f60718293a4b5c6"
+	handler := tm.HTTPMiddleware()(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
+		w.WriteHeader(http.StatusOK)
+	}))
+	req := httptest.NewRequest(http.MethodGet, "/events?apikey="+key+"&limit=5", nil)
+	handler.ServeHTTP(httptest.NewRecorder(), req)
+
+	spans := recorder.Ended()
+	if len(spans) != 1 {
+		t.Fatalf("got %d ended spans, want 1", len(spans))
+	}
+	var gotURL string
+	for _, kv := range spans[0].Attributes() {
+		if kv.Key == semconv.HTTPURLKey {
+			gotURL = kv.Value.AsString()
+		}
+	}
+	if gotURL == "" {
+		t.Fatal("span has no http.url attribute")
+	}
+	if strings.Contains(gotURL, key) {
+		t.Fatalf("http.url carries the API key: %q", gotURL)
+	}
+	if !strings.HasPrefix(gotURL, "/events?") || !strings.Contains(gotURL, "limit=5") {
+		t.Fatalf("http.url lost its path or non-secret parameters: %q", gotURL)
+	}
+}