From 497ef3a2babd9ca5f651ac60a0a7ebe00eebc521 Mon Sep 17 00:00:00 2001
From: Rudy Celekli <rudy@gradiahq.com>
Date: Sun, 25 Oct 2026 15:59:32 -0400
Subject: [PATCH] fix(observability): mask ?apikey= in the http.url span
 attribute

The OTLP tracing middleware recorded r.URL.String() as http.url on every
REST API span. ?apikey= is an accepted credential, and the Web UI's SSE
stream (/events?apikey=...) and the tray send the admin key that way, so
with tracing enabled the admin key was exported to the collector.

Render the query through oauth.LogSafeQueryString, the renderer http.log
already uses for the same requests (SEC-01).
---
 internal/observability/tracing.go             | 15 +++++-
 internal/observability/tracing_redact_test.go | 54 +++++++++++++++++++
 2 files changed, 68 insertions(+), 1 deletion(-)
 create mode 100644 internal/observability/tracing_redact_test.go

diff --git a/internal/observability/tracing.go b/internal/observability/tracing.go
index 299978e2a..25cefb55c 100644
--- a/internal/observability/tracing.go
+++ b/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
diff --git a/internal/observability/tracing_redact_test.go b/internal/observability/tracing_redact_test.go
new file mode 100644
index 000000000..ca2fa50d2
--- /dev/null
+++ b/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)
+	}
+}