From 2a086dcdf0c4e3e1b2c24e95d85d339d76ef62ab Mon Sep 17 00:00:00 2001 From: sai5880 Date: Thu, 22 Oct 2026 00:56:49 +0530 Subject: [PATCH 1/3] Redact sensitive query parameters in otel transport --- otel/transport.go | 31 +++++++++++++++++++++- otel/transport_test.go | 60 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 90 insertions(+), 1 deletion(-) diff --git a/otel/transport.go b/otel/transport.go index e431dff3ba7..13bc6b8d06e 100644 --- a/otel/transport.go +++ b/otel/transport.go @@ -9,6 +9,7 @@ package otel import ( "fmt" "net/http" + "net/url" "strconv" "github.com/google/go-github/v93/github" @@ -50,6 +51,34 @@ func NewTransport(base http.RoundTripper, opts ...Option) *Transport { return t } +// sensitiveQueryParams lists query parameters whose values must not be +// recorded in telemetry. +var sensitiveQueryParams = []string{"client_secret", "access_token", "token"} + +// redactURL returns a copy of u with userinfo removed and sensitive query +// values redacted. The scheme, host, path, and non-sensitive query parameters +// are preserved. +func redactURL(u *url.URL) string { + if u == nil { + return "" + } + redacted := *u + redacted.User = nil + + params := redacted.Query() + var changed bool + for _, p := range sensitiveQueryParams { + if params.Has(p) { + params.Set(p, "REDACTED") + changed = true + } + } + if changed { + redacted.RawQuery = params.Encode() + } + return redacted.String() +} + // RoundTrip implements http.RoundTripper. func (t *Transport) RoundTrip(req *http.Request) (*http.Response, error) { ctx := req.Context() @@ -61,7 +90,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", redactURL(req.URL)), attribute.String("http.host", req.URL.Host), ) diff --git a/otel/transport_test.go b/otel/transport_test.go index 0164e95f002..0035f53e173 100644 --- a/otel/transport_test.go +++ b/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,64 @@ 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) + if _, err := transport.RoundTrip(req); err != nil { + t.Fatalf("RoundTrip failed: %v", err) + } + + spans := exporter.GetSpans() + if len(spans) != 1 { + t.Fatalf("Expected 1 span, got %d", 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 sensitiveQueryParams { + 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() From 10f8be813e019dddee6d3e6d1019d50935731f06 Mon Sep 17 00:00:00 2001 From: sai5880 Date: Thu, 22 Oct 2026 01:21:10 +0530 Subject: [PATCH 2/3] Export SanitizeURL and use it in otel --- github/github.go | 27 +++++++++++++++------------ github/github_test.go | 9 +++++++-- otel/transport.go | 31 +------------------------------ otel/transport_test.go | 6 +++++- 4 files changed, 28 insertions(+), 45 deletions(-) diff --git a/github/github.go b/github/github.go index a5d90e225da..471fb156b98 100644 --- a/github/github.go +++ b/github/github.go @@ -1479,7 +1479,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 } } @@ -1774,7 +1774,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) } @@ -1846,7 +1846,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))) } @@ -1904,7 +1904,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) } @@ -1945,8 +1945,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. @@ -1963,13 +1963,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 { @@ -1978,9 +1981,9 @@ func sanitizeURL(uri *url.URL) *url.URL { } } if redacted { - uri.RawQuery = params.Encode() + sanitized.RawQuery = params.Encode() } - return uri + return &sanitized } /* diff --git a/github/github_test.go b/github/github_test.go index c927a46f0cd..de467606e24 100644 --- a/github/github_test.go +++ b/github/github_test.go @@ -3956,14 +3956,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) } } } diff --git a/otel/transport.go b/otel/transport.go index 13bc6b8d06e..58ccc00aee1 100644 --- a/otel/transport.go +++ b/otel/transport.go @@ -9,7 +9,6 @@ package otel import ( "fmt" "net/http" - "net/url" "strconv" "github.com/google/go-github/v93/github" @@ -51,34 +50,6 @@ func NewTransport(base http.RoundTripper, opts ...Option) *Transport { return t } -// sensitiveQueryParams lists query parameters whose values must not be -// recorded in telemetry. -var sensitiveQueryParams = []string{"client_secret", "access_token", "token"} - -// redactURL returns a copy of u with userinfo removed and sensitive query -// values redacted. The scheme, host, path, and non-sensitive query parameters -// are preserved. -func redactURL(u *url.URL) string { - if u == nil { - return "" - } - redacted := *u - redacted.User = nil - - params := redacted.Query() - var changed bool - for _, p := range sensitiveQueryParams { - if params.Has(p) { - params.Set(p, "REDACTED") - changed = true - } - } - if changed { - redacted.RawQuery = params.Encode() - } - return redacted.String() -} - // RoundTrip implements http.RoundTripper. func (t *Transport) RoundTrip(req *http.Request) (*http.Response, error) { ctx := req.Context() @@ -90,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", redactURL(req.URL)), + attribute.String("http.url", github.SanitizeURL(req.URL).String()), attribute.String("http.host", req.URL.Host), ) diff --git a/otel/transport_test.go b/otel/transport_test.go index 0035f53e173..70b248d173d 100644 --- a/otel/transport_test.go +++ b/otel/transport_test.go @@ -134,9 +134,13 @@ func TestRoundTrip_RedactsSensitiveURL(t *testing.T) { 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 { @@ -175,7 +179,7 @@ func TestRoundTrip_RedactsSensitiveURL(t *testing.T) { if u.User != nil { t.Errorf("URL userinfo = %v, want nil", u.User) } - for _, p := range sensitiveQueryParams { + 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) } From ceb22452b59cccc720a56dec922255cefc4fbf5f Mon Sep 17 00:00:00 2001 From: sai5880 Date: Mon, 26 Oct 2026 09:34:03 +0530 Subject: [PATCH 3/3] Use %v for span count in otel test --- otel/transport_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/otel/transport_test.go b/otel/transport_test.go index 70b248d173d..36789dc343d 100644 --- a/otel/transport_test.go +++ b/otel/transport_test.go @@ -144,7 +144,7 @@ func TestRoundTrip_RedactsSensitiveURL(t *testing.T) { spans := exporter.GetSpans() if len(spans) != 1 { - t.Fatalf("Expected 1 span, got %d", len(spans)) + t.Fatalf("Expected 1 span, got %v", len(spans)) } var gotURL string