From 3aa7e66eca0aa26797f07150eb7ac8c0b8d21856 Mon Sep 17 00:00:00 2001 From: YooLCD Date: Thu, 2 Apr 2026 15:52:19 +0900 Subject: [PATCH 1/7] fix: set GetBody in NewRequest and expand sanitizeURL redaction - Set req.GetBody in NewRequest so that rate-limit retries can re-read the request body on POST/PUT/PATCH requests. - Extend sanitizeURL to redact access_token and token query parameters in addition to client_secret. --- github/github.go | 37 ++++++++++++++++++++++++++----------- github/github_test.go | 33 +++++++++++++++++++++++++++++++++ 2 files changed, 59 insertions(+), 11 deletions(-) diff --git a/github/github.go b/github/github.go index bb9e9f3cd72..3e436c30d44 100644 --- a/github/github.go +++ b/github/github.go @@ -559,23 +559,32 @@ func (c *Client) NewRequest(method, urlStr string, body any, opts ...RequestOpti return nil, err } - var buf io.ReadWriter + var rawBody []byte if body != nil { - buf = &bytes.Buffer{} - enc := json.NewEncoder(buf) + var buf bytes.Buffer + enc := json.NewEncoder(&buf) enc.SetEscapeHTML(false) - err := enc.Encode(body) - if err != nil { + if err := enc.Encode(body); err != nil { return nil, err } + rawBody = buf.Bytes() } - req, err := http.NewRequest(method, u.String(), buf) + var bodyReader io.Reader + if rawBody != nil { + bodyReader = bytes.NewReader(rawBody) + } + req, err := http.NewRequest(method, u.String(), bodyReader) if err != nil { return nil, err } + if rawBody != nil { + req.GetBody = func() (io.ReadCloser, error) { + return io.NopCloser(bytes.NewReader(rawBody)), nil + } + } - if body != nil { + if rawBody != nil { req.Header.Set("Content-Type", "application/json") } req.Header.Set("Accept", mediaTypeV3) @@ -1382,15 +1391,21 @@ func (r *RedirectionError) Is(target error) bool { r.Location != nil && v.Location != nil && r.Location.String() == v.Location.String()) // or they are both not nil and marshaled identically } -// sanitizeURL redacts the client_secret parameter from the URL which may be -// exposed to the user. +var sensitiveParams = []string{"client_secret", "access_token", "token"} + func sanitizeURL(uri *url.URL) *url.URL { if uri == nil { return nil } params := uri.Query() - if len(params.Get("client_secret")) > 0 { - params.Set("client_secret", "REDACTED") + redacted := false + for _, p := range sensitiveParams { + if len(params.Get(p)) > 0 { + params.Set(p, "REDACTED") + redacted = true + } + } + if redacted { uri.RawQuery = params.Encode() } return uri diff --git a/github/github_test.go b/github/github_test.go index d5c6826e179..2407d184fba 100644 --- a/github/github_test.go +++ b/github/github_test.go @@ -749,6 +749,36 @@ func TestNewRequest_emptyBody(t *testing.T) { } } +func TestNewRequest_getBody(t *testing.T) { + t.Parallel() + c := NewClient(nil) + + req, err := c.NewRequest("POST", ".", &User{Login: Ptr("l")}) + if err != nil { + t.Fatalf("NewRequest returned unexpected error: %v", err) + } + if req.GetBody == nil { + t.Fatal("NewRequest with body did not set GetBody") + } + original, _ := io.ReadAll(req.Body) + rc, err := req.GetBody() + if err != nil { + t.Fatalf("GetBody returned unexpected error: %v", err) + } + replay, _ := io.ReadAll(rc) + if string(original) != string(replay) { + t.Errorf("GetBody returned %q, want %q", replay, original) + } + + req, err = c.NewRequest("GET", ".", nil) + if err != nil { + t.Fatalf("NewRequest returned unexpected error: %v", err) + } + if req.GetBody != nil { + t.Fatal("NewRequest without body set GetBody unexpectedly") + } +} + func TestNewRequest_errorForNoTrailingSlash(t *testing.T) { t.Parallel() tests := []struct { @@ -2174,6 +2204,9 @@ func TestSanitizeURL(t *testing.T) { {"/?a=b", "/?a=b"}, {"/?a=b&client_secret=secret", "/?a=b&client_secret=REDACTED"}, {"/?a=b&client_id=id&client_secret=secret", "/?a=b&client_id=id&client_secret=REDACTED"}, + {"/?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"}, } for _, tt := range tests { From dca6c570337fb96299784d7b8f2a4f43e3ac490b Mon Sep 17 00:00:00 2001 From: YooLCD Date: Thu, 2 Apr 2026 20:54:42 +0900 Subject: [PATCH 2/7] fix: expand sanitizeURL to redact access_token and token --- github/github.go | 25 +++++++++---------------- github/github_test.go | 30 ------------------------------ 2 files changed, 9 insertions(+), 46 deletions(-) diff --git a/github/github.go b/github/github.go index 3e436c30d44..7edff0e07ba 100644 --- a/github/github.go +++ b/github/github.go @@ -559,32 +559,23 @@ func (c *Client) NewRequest(method, urlStr string, body any, opts ...RequestOpti return nil, err } - var rawBody []byte + var buf io.ReadWriter if body != nil { - var buf bytes.Buffer - enc := json.NewEncoder(&buf) + buf = &bytes.Buffer{} + enc := json.NewEncoder(buf) enc.SetEscapeHTML(false) - if err := enc.Encode(body); err != nil { + err := enc.Encode(body) + if err != nil { return nil, err } - rawBody = buf.Bytes() } - var bodyReader io.Reader - if rawBody != nil { - bodyReader = bytes.NewReader(rawBody) - } - req, err := http.NewRequest(method, u.String(), bodyReader) + req, err := http.NewRequest(method, u.String(), buf) if err != nil { return nil, err } - if rawBody != nil { - req.GetBody = func() (io.ReadCloser, error) { - return io.NopCloser(bytes.NewReader(rawBody)), nil - } - } - if rawBody != nil { + if body != nil { req.Header.Set("Content-Type", "application/json") } req.Header.Set("Accept", mediaTypeV3) @@ -1391,6 +1382,8 @@ func (r *RedirectionError) Is(target error) bool { r.Location != nil && v.Location != nil && r.Location.String() == v.Location.String()) // or they are both not nil and marshaled identically } +// sanitizeURL redacts sensitive parameters from the URL which may be +// exposed to the user. var sensitiveParams = []string{"client_secret", "access_token", "token"} func sanitizeURL(uri *url.URL) *url.URL { diff --git a/github/github_test.go b/github/github_test.go index 2407d184fba..e321da98a09 100644 --- a/github/github_test.go +++ b/github/github_test.go @@ -749,36 +749,6 @@ func TestNewRequest_emptyBody(t *testing.T) { } } -func TestNewRequest_getBody(t *testing.T) { - t.Parallel() - c := NewClient(nil) - - req, err := c.NewRequest("POST", ".", &User{Login: Ptr("l")}) - if err != nil { - t.Fatalf("NewRequest returned unexpected error: %v", err) - } - if req.GetBody == nil { - t.Fatal("NewRequest with body did not set GetBody") - } - original, _ := io.ReadAll(req.Body) - rc, err := req.GetBody() - if err != nil { - t.Fatalf("GetBody returned unexpected error: %v", err) - } - replay, _ := io.ReadAll(rc) - if string(original) != string(replay) { - t.Errorf("GetBody returned %q, want %q", replay, original) - } - - req, err = c.NewRequest("GET", ".", nil) - if err != nil { - t.Fatalf("NewRequest returned unexpected error: %v", err) - } - if req.GetBody != nil { - t.Fatal("NewRequest without body set GetBody unexpectedly") - } -} - func TestNewRequest_errorForNoTrailingSlash(t *testing.T) { t.Parallel() tests := []struct { From f66bd9e693a264e63682c3d78cb0285908fd602e Mon Sep 17 00:00:00 2001 From: YooLCD Date: Thu, 2 Apr 2026 21:03:25 +0900 Subject: [PATCH 3/7] fix: set GetBody in NewRequest --- github/github.go | 23 ++++++++++++++++------- github/github_test.go | 30 ++++++++++++++++++++++++++++++ 2 files changed, 46 insertions(+), 7 deletions(-) diff --git a/github/github.go b/github/github.go index 7edff0e07ba..576f3b59d35 100644 --- a/github/github.go +++ b/github/github.go @@ -559,23 +559,32 @@ func (c *Client) NewRequest(method, urlStr string, body any, opts ...RequestOpti return nil, err } - var buf io.ReadWriter + var rawBody []byte if body != nil { - buf = &bytes.Buffer{} - enc := json.NewEncoder(buf) + var buf bytes.Buffer + enc := json.NewEncoder(&buf) enc.SetEscapeHTML(false) - err := enc.Encode(body) - if err != nil { + if err := enc.Encode(body); err != nil { return nil, err } + rawBody = buf.Bytes() } - req, err := http.NewRequest(method, u.String(), buf) + var bodyReader io.Reader + if rawBody != nil { + bodyReader = bytes.NewReader(rawBody) + } + req, err := http.NewRequest(method, u.String(), bodyReader) if err != nil { return nil, err } + if rawBody != nil { + req.GetBody = func() (io.ReadCloser, error) { + return io.NopCloser(bytes.NewReader(rawBody)), nil + } + } - if body != nil { + if rawBody != nil { req.Header.Set("Content-Type", "application/json") } req.Header.Set("Accept", mediaTypeV3) diff --git a/github/github_test.go b/github/github_test.go index e321da98a09..2407d184fba 100644 --- a/github/github_test.go +++ b/github/github_test.go @@ -749,6 +749,36 @@ func TestNewRequest_emptyBody(t *testing.T) { } } +func TestNewRequest_getBody(t *testing.T) { + t.Parallel() + c := NewClient(nil) + + req, err := c.NewRequest("POST", ".", &User{Login: Ptr("l")}) + if err != nil { + t.Fatalf("NewRequest returned unexpected error: %v", err) + } + if req.GetBody == nil { + t.Fatal("NewRequest with body did not set GetBody") + } + original, _ := io.ReadAll(req.Body) + rc, err := req.GetBody() + if err != nil { + t.Fatalf("GetBody returned unexpected error: %v", err) + } + replay, _ := io.ReadAll(rc) + if string(original) != string(replay) { + t.Errorf("GetBody returned %q, want %q", replay, original) + } + + req, err = c.NewRequest("GET", ".", nil) + if err != nil { + t.Fatalf("NewRequest returned unexpected error: %v", err) + } + if req.GetBody != nil { + t.Fatal("NewRequest without body set GetBody unexpectedly") + } +} + func TestNewRequest_errorForNoTrailingSlash(t *testing.T) { t.Parallel() tests := []struct { From 4779ec164b75d59beb2a7d033e9551fc4e13daa2 Mon Sep 17 00:00:00 2001 From: YooLCD Date: Thu, 2 Apr 2026 21:18:40 +0900 Subject: [PATCH 4/7] fix: drop GetBody change, keep only sanitizeURL fix --- github/github.go | 23 +++++++---------------- github/github_test.go | 30 ------------------------------ 2 files changed, 7 insertions(+), 46 deletions(-) diff --git a/github/github.go b/github/github.go index 576f3b59d35..7edff0e07ba 100644 --- a/github/github.go +++ b/github/github.go @@ -559,32 +559,23 @@ func (c *Client) NewRequest(method, urlStr string, body any, opts ...RequestOpti return nil, err } - var rawBody []byte + var buf io.ReadWriter if body != nil { - var buf bytes.Buffer - enc := json.NewEncoder(&buf) + buf = &bytes.Buffer{} + enc := json.NewEncoder(buf) enc.SetEscapeHTML(false) - if err := enc.Encode(body); err != nil { + err := enc.Encode(body) + if err != nil { return nil, err } - rawBody = buf.Bytes() } - var bodyReader io.Reader - if rawBody != nil { - bodyReader = bytes.NewReader(rawBody) - } - req, err := http.NewRequest(method, u.String(), bodyReader) + req, err := http.NewRequest(method, u.String(), buf) if err != nil { return nil, err } - if rawBody != nil { - req.GetBody = func() (io.ReadCloser, error) { - return io.NopCloser(bytes.NewReader(rawBody)), nil - } - } - if rawBody != nil { + if body != nil { req.Header.Set("Content-Type", "application/json") } req.Header.Set("Accept", mediaTypeV3) diff --git a/github/github_test.go b/github/github_test.go index 2407d184fba..e321da98a09 100644 --- a/github/github_test.go +++ b/github/github_test.go @@ -749,36 +749,6 @@ func TestNewRequest_emptyBody(t *testing.T) { } } -func TestNewRequest_getBody(t *testing.T) { - t.Parallel() - c := NewClient(nil) - - req, err := c.NewRequest("POST", ".", &User{Login: Ptr("l")}) - if err != nil { - t.Fatalf("NewRequest returned unexpected error: %v", err) - } - if req.GetBody == nil { - t.Fatal("NewRequest with body did not set GetBody") - } - original, _ := io.ReadAll(req.Body) - rc, err := req.GetBody() - if err != nil { - t.Fatalf("GetBody returned unexpected error: %v", err) - } - replay, _ := io.ReadAll(rc) - if string(original) != string(replay) { - t.Errorf("GetBody returned %q, want %q", replay, original) - } - - req, err = c.NewRequest("GET", ".", nil) - if err != nil { - t.Fatalf("NewRequest returned unexpected error: %v", err) - } - if req.GetBody != nil { - t.Fatal("NewRequest without body set GetBody unexpectedly") - } -} - func TestNewRequest_errorForNoTrailingSlash(t *testing.T) { t.Parallel() tests := []struct { From 6b21e7c3eb733471eb2e315e8f64003da952aa6b Mon Sep 17 00:00:00 2001 From: YooLCD Date: Fri, 3 Apr 2026 13:15:27 +0900 Subject: [PATCH 5/7] fix: simplify sanitizeURL loop --- github/github.go | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/github/github.go b/github/github.go index 7edff0e07ba..f8f1c66d0c3 100644 --- a/github/github.go +++ b/github/github.go @@ -1391,16 +1391,12 @@ func sanitizeURL(uri *url.URL) *url.URL { return nil } params := uri.Query() - redacted := false for _, p := range sensitiveParams { if len(params.Get(p)) > 0 { params.Set(p, "REDACTED") - redacted = true } } - if redacted { - uri.RawQuery = params.Encode() - } + uri.RawQuery = params.Encode() return uri } From 97b3c05542fda3939ad61425f2f752012330a240 Mon Sep 17 00:00:00 2001 From: YooLCD Date: Fri, 3 Apr 2026 18:32:04 +0900 Subject: [PATCH 6/7] fix: move sensitiveParams before sanitizeURL comment --- github/github.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/github/github.go b/github/github.go index f8f1c66d0c3..bedca202fe1 100644 --- a/github/github.go +++ b/github/github.go @@ -1382,10 +1382,10 @@ func (r *RedirectionError) Is(target error) bool { r.Location != nil && v.Location != nil && r.Location.String() == v.Location.String()) // or they are both not nil and marshaled identically } -// sanitizeURL redacts sensitive parameters from the URL which may be -// exposed to the user. 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 { if uri == nil { return nil From 3b41c361a4919a9d88d049eae30b8deba321b244 Mon Sep 17 00:00:00 2001 From: YooLCD Date: Fri, 3 Apr 2026 23:43:58 +0900 Subject: [PATCH 7/7] fix: use var redacted bool in sanitizeURL --- github/github.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/github/github.go b/github/github.go index bedca202fe1..1e79096856a 100644 --- a/github/github.go +++ b/github/github.go @@ -1391,12 +1391,16 @@ func sanitizeURL(uri *url.URL) *url.URL { return nil } params := uri.Query() + var redacted bool for _, p := range sensitiveParams { if len(params.Get(p)) > 0 { params.Set(p, "REDACTED") + redacted = true } } - uri.RawQuery = params.Encode() + if redacted { + uri.RawQuery = params.Encode() + } return uri }