fix: Expand sanitizeURL secrets redactions

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

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 {