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

## Bug

When OTLP tracing is enabled (`observability.tracing.enabled`),
`TracingManager.HTTPMiddleware` wraps the whole REST API router and
records `r.URL.String()` as the `http.url` attribute of every span. That
string includes the query.

`?apikey=` is an accepted credential (`resolveAuth`). The Web UI's SSE
stream connects to `/events?apikey=<admin key>`
(`frontend/src/services/api.ts`), and the tray sends the admin key the
same way. So the admin API key lands in span attributes and is exported
to the configured collector (Jaeger, Tempo, a shared OTel pipeline).
Anyone who can read traces can then drive the REST API as admin.

`httpLoggingMiddleware` already masks the same query before it writes to
http.log (SEC-01). The tracing middleware sees the same requests and was
missed.

## Trigger

Enable tracing, then open the Web UI. Every SSE (re)connect is a `GET
/events?apikey=…` request, and each sampled one exports the key.

## Fix

Build `http.url` from a copy of the request URL whose `RawQuery` has
been passed through `oauth.LogSafeQueryString`, the same renderer
http.log uses. The path and the non-secret parameters stay as they were.
`http.target` and the span name already use only the path.
`internal/oauth` does not import `internal/observability`, so the new
import adds no cycle.

## Test

`TestHTTPMiddleware_MasksAPIKeyInSpanURL` sends `/events?apikey=<64-hex
key>&limit=5` through the middleware with an in-memory span recorder.
The recorded `http.url` must not contain the key, and must keep
`/events?` and `limit=5`.

## Commands run

- `CGO_ENABLED=0 go test -count=1 -run 'Test'
./internal/observability/`: ok
- On current `main`, and again with the fix reverted, the new test fails
with `http.url carries the API key: "/events?apikey=3f9a…b5c6&limit=5"`.
- `go vet ./internal/observability/`: ok

Author: rudycelekli

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)
+	}
+}