fix: Redact sensitive query parameters in otel transport

github/github.go

@@ -1456,7 +1456,7 @@ func (c *Client) bareDo(caller *http.Client, req *http.Request) (*Response, erro
 		// If the error type is *url.Error, sanitize its URL before returning.
 		if e, ok := errors.AsType[*url.Error](err); ok {
 			if url, err := url.Parse(e.URL); err == nil {
-				e.URL = sanitizeURL(url).String()
+				e.URL = SanitizeURL(url).String()
 				return response, e
 			}
 		}
@@ -1787,7 +1787,7 @@ type ErrorBlock struct {
 func (r *ErrorResponse) Error() string {
 	if r.Response != nil && r.Response.Request != nil {
 		return fmt.Sprintf("%v %v: %v %v %+v",
-			r.Response.Request.Method, sanitizeURL(r.Response.Request.URL),
+			r.Response.Request.Method, SanitizeURL(r.Response.Request.URL),
 			r.Response.StatusCode, r.Message, r.Errors)
 	}
 
@@ -1859,7 +1859,7 @@ type RateLimitError struct {
 
 func (r *RateLimitError) Error() string {
 	return fmt.Sprintf("%v %v: %v %v %v",
-		r.Response.Request.Method, sanitizeURL(r.Response.Request.URL),
+		r.Response.Request.Method, SanitizeURL(r.Response.Request.URL),
 		r.Response.StatusCode, r.Message, formatRateReset(time.Until(r.Rate.Reset.Time)))
 }
 
@@ -1917,7 +1917,7 @@ func (r *AbuseRateLimitError) Error() string {
 		retryInfo = fmt.Sprintf(" [retry after %v]", r.RetryAfter.Round(time.Second))
 	}
 	return fmt.Sprintf("%v %v: %v %v%v",
-		r.Response.Request.Method, sanitizeURL(r.Response.Request.URL),
+		r.Response.Request.Method, SanitizeURL(r.Response.Request.URL),
 		r.Response.StatusCode, r.Message, retryInfo)
 }
 
@@ -1958,8 +1958,8 @@ type RedirectionError struct {
 
 func (r *RedirectionError) Error() string {
 	return fmt.Sprintf("%v %v: %v location %v",
-		r.Response.Request.Method, sanitizeURL(r.Response.Request.URL),
-		r.StatusCode, sanitizeURL(r.Location))
+		r.Response.Request.Method, SanitizeURL(r.Response.Request.URL),
+		r.StatusCode, SanitizeURL(r.Location))
 }
 
 // Is returns whether the provided error equals this error.
@@ -1976,13 +1976,16 @@ func (r *RedirectionError) Is(target error) bool {
 
 var sensitiveParams = []string{"client_secret", "access_token", "token"}
 
-// sanitizeURL redacts sensitive parameters from the URL which may be
-// exposed to the user.
-func sanitizeURL(uri *url.URL) *url.URL {
+// SanitizeURL returns a copy of uri with userinfo removed and sensitive query
+// parameters redacted. It is safe to expose the result to users or to record
+// it in telemetry because the input URL is not modified.
+func SanitizeURL(uri *url.URL) *url.URL {
 	if uri == nil {
 		return nil
 	}
-	params := uri.Query()
+	sanitized := *uri
+	sanitized.User = nil
+	params := sanitized.Query()
 	var redacted bool
 	for _, p := range sensitiveParams {
 		if len(params.Get(p)) > 0 {
@@ -1991,9 +1994,9 @@ func sanitizeURL(uri *url.URL) *url.URL {
 		}
 	}
 	if redacted {
-		uri.RawQuery = params.Encode()
+		sanitized.RawQuery = params.Encode()
 	}
-	return uri
+	return &sanitized
 }
 
 /*

github/github_test.go

@@ -3887,14 +3887,19 @@ func TestSanitizeURL(t *testing.T) {
 		{"/?a=b&access_token=secret", "/?a=b&access_token=REDACTED"},
 		{"/?a=b&token=secret", "/?a=b&token=REDACTED"},
 		{"/?client_secret=s&access_token=t&token=u", "/?access_token=REDACTED&client_secret=REDACTED&token=REDACTED"},
+		{"https://user:password@example.com/path?token=secret", "https://example.com/path?token=REDACTED"},
 	}
 
 	for _, tt := range tests {
 		inURL, _ := url.Parse(tt.in)
 		want, _ := url.Parse(tt.want)
+		original := inURL.String()
 
-		if got := sanitizeURL(inURL); !cmp.Equal(got, want) {
-			t.Errorf("sanitizeURL(%v) returned %v, want %v", tt.in, got, want)
+		if got := SanitizeURL(inURL); !cmp.Equal(got, want) {
+			t.Errorf("SanitizeURL(%v) returned %v, want %v", tt.in, got, want)
+		}
+		if got := inURL.String(); got != original {
+			t.Errorf("SanitizeURL modified its input: got %v, want %v", got, original)
 		}
 	}
 }

otel/transport.go

@@ -61,7 +61,7 @@ func (t *Transport) RoundTrip(req *http.Request) (*http.Response, error) {
 	// Inject Attributes
 	span.SetAttributes(
 		attribute.String("http.method", req.Method),
-		attribute.String("http.url", req.URL.String()),
+		attribute.String("http.url", github.SanitizeURL(req.URL).String()),
 		attribute.String("http.host", req.URL.Host),
 	)
 

otel/transport_test.go

@@ -9,6 +9,8 @@ import (
 	"errors"
 	"net/http"
 	"net/http/httptest"
+	"net/url"
+	"strings"
 	"testing"
 
 	"go.opentelemetry.io/otel"
@@ -125,6 +127,68 @@ func TestRoundTrip_Spans(t *testing.T) {
 	}
 }
 
+func TestRoundTrip_RedactsSensitiveURL(t *testing.T) {
+	t.Parallel()
+	exporter := tracetest.NewInMemoryExporter()
+	tp := trace.NewTracerProvider(trace.WithSyncer(exporter))
+	transport := NewTransport(&mockTransport{}, WithTracerProvider(tp))
+
+	req := httptest.NewRequest("GET", "https://alice:s3cr3t-password@api.github.com/repos/google/go-github?access_token=s3cr3t-access&client_secret=s3cr3t-client&token=s3cr3t-token&page=2", nil)
+	originalURL := req.URL.String()
+	if _, err := transport.RoundTrip(req); err != nil {
+		t.Fatalf("RoundTrip failed: %v", err)
+	}
+	if got := req.URL.String(); got != originalURL {
+		t.Errorf("RoundTrip modified the request URL: got %q, want %q", got, originalURL)
+	}
+
+	spans := exporter.GetSpans()
+	if len(spans) != 1 {
+		t.Fatalf("Expected 1 span, got %v", len(spans))
+	}
+
+	var gotURL string
+	for _, a := range spans[0].Attributes {
+		if a.Key == "http.url" {
+			gotURL = a.Value.AsString()
+		}
+	}
+	if gotURL == "" {
+		t.Fatal("http.url attribute not recorded")
+	}
+
+	for _, secret := range []string{"alice", "s3cr3t"} {
+		if strings.Contains(gotURL, secret) {
+			t.Errorf("http.url = %q, must not contain %q", gotURL, secret)
+		}
+	}
+
+	u, err := url.Parse(gotURL)
+	if err != nil {
+		t.Fatalf("Parsing recorded http.url: %v", err)
+	}
+	if got, want := u.Scheme, "https"; got != want {
+		t.Errorf("URL scheme = %q, want %q", got, want)
+	}
+	if got, want := u.Host, "api.github.com"; got != want {
+		t.Errorf("URL host = %q, want %q", got, want)
+	}
+	if got, want := u.Path, "/repos/google/go-github"; got != want {
+		t.Errorf("URL path = %q, want %q", got, want)
+	}
+	if u.User != nil {
+		t.Errorf("URL userinfo = %v, want nil", u.User)
+	}
+	for _, p := range []string{"access_token", "client_secret", "token"} {
+		if got, want := u.Query().Get(p), "REDACTED"; got != want {
+			t.Errorf("Query parameter %q = %q, want %q", p, got, want)
+		}
+	}
+	if got, want := u.Query().Get("page"), "2"; got != want {
+		t.Errorf("Query parameter %q = %q, want %q", "page", got, want)
+	}
+}
+
 func TestRoundTrip_Error(t *testing.T) {
 	t.Parallel()
 	exporter := tracetest.NewInMemoryExporter()