diff --git a/github/client.go b/github/client.go index 69baa36..23a945e 100644 --- a/github/client.go +++ b/github/client.go @@ -26,6 +26,10 @@ const ( // APIError represents an HTTP error response from the GitHub API. // It carries the status code so callers can distinguish between // different failure modes (e.g. 404 vs 500). +// +// Note: Error() includes up to 200 bytes of the response body for debugging. +// Callers should avoid logging raw error messages in production if the upstream +// server may return sensitive details in error responses. type APIError struct { StatusCode int Body string @@ -97,6 +101,21 @@ type Client struct { retryBackoff []time.Duration } +// defaultCheckRedirect is the redirect policy used by NewClient and SetHTTPClient(nil). +// It strips the Authorization header on cross-host redirects or protocol downgrades +// (HTTPS→HTTP) to prevent credential leakage, while still following the redirect. +func defaultCheckRedirect(req *http.Request, via []*http.Request) error { + if len(via) >= 10 { + return fmt.Errorf("stopped after 10 redirects") + } + // Strip Authorization on cross-host redirect or protocol downgrade (https→http). + prev := via[len(via)-1] + if req.URL.Host != prev.URL.Host || (prev.URL.Scheme == "https" && req.URL.Scheme == "http") { + req.Header.Del("Authorization") + } + return nil +} + // NewClient creates a new GitHub API client. // If baseURL is empty, it defaults to https://api.github.com. // For GitHub Enterprise, pass the API base URL (e.g. https://github.concur.com/api/v3). @@ -115,18 +134,8 @@ func NewClient(token, baseURL string, opts ...ClientOption) *Client { allowInsecureHTTP: cfg.allowInsecureHTTP, token: token, httpClient: &http.Client{ - Timeout: 30 * time.Second, - CheckRedirect: func(req *http.Request, via []*http.Request) error { - if len(via) >= 10 { - return fmt.Errorf("stopped after 10 redirects") - } - // Strip Authorization on cross-host redirect or protocol downgrade (https→http). - prev := via[len(via)-1] - if req.URL.Host != prev.URL.Host || (prev.URL.Scheme == "https" && req.URL.Scheme == "http") { - req.Header.Del("Authorization") - } - return nil - }, + Timeout: 30 * time.Second, + CheckRedirect: defaultCheckRedirect, }, } } @@ -138,17 +147,8 @@ func NewClient(token, baseURL string, opts ...ClientOption) *Client { func (c *Client) SetHTTPClient(hc *http.Client) { if hc == nil { hc = &http.Client{ - Timeout: 30 * time.Second, - CheckRedirect: func(req *http.Request, via []*http.Request) error { - if len(via) >= 10 { - return fmt.Errorf("stopped after 10 redirects") - } - prev := via[len(via)-1] - if req.URL.Host != prev.URL.Host || (prev.URL.Scheme == "https" && req.URL.Scheme == "http") { - req.Header.Del("Authorization") - } - return nil - }, + Timeout: 30 * time.Second, + CheckRedirect: defaultCheckRedirect, } } c.httpClient = hc @@ -235,7 +235,7 @@ func (c *Client) doRequest(ctx context.Context, method, reqURL string, accept st if err != nil { return nil, fmt.Errorf("read response body: %w", err) } - if int64(len(body)) >= maxResponseBytes { + if len(body) >= maxResponseBytes { return nil, fmt.Errorf("response body exceeded %d bytes (truncated)", maxResponseBytes) } return body, nil diff --git a/github/files.go b/github/files.go index f9a1cf6..aeebe20 100644 --- a/github/files.go +++ b/github/files.go @@ -48,6 +48,9 @@ func (c *Client) ListContents(ctx context.Context, owner, repo, path string) ([] if err2 := json.Unmarshal(body, &single); err2 != nil { return nil, fmt.Errorf("parse contents JSON: %w", err2) } + if single.Name == "" && single.Path == "" && single.Type == "" { + return nil, fmt.Errorf("parse contents JSON: unexpected response format") + } entries = []entry{single} } diff --git a/github/pr_test.go b/github/pr_test.go index 405cc6f..0e05a50 100644 --- a/github/pr_test.go +++ b/github/pr_test.go @@ -528,13 +528,13 @@ func TestGetCommitStatuses_CheckRunConclusions(t *testing.T) { status string want string }{ - {strPtr("success"), "completed", "success"}, - {strPtr("failure"), "completed", "failure"}, - {strPtr("action_required"), "completed", "failure"}, - {strPtr("timed_out"), "completed", "failure"}, - {strPtr("cancelled"), "completed", "success"}, - {strPtr("skipped"), "completed", "success"}, - {strPtr("neutral"), "completed", "success"}, + {stringPtr("success"), "completed", "success"}, + {stringPtr("failure"), "completed", "failure"}, + {stringPtr("action_required"), "completed", "failure"}, + {stringPtr("timed_out"), "completed", "failure"}, + {stringPtr("cancelled"), "completed", "success"}, + {stringPtr("skipped"), "completed", "success"}, + {stringPtr("neutral"), "completed", "success"}, {nil, "in_progress", "pending"}, {nil, "queued", "pending"}, } @@ -632,6 +632,6 @@ func TestGetCommitStatuses_MalformedJSON(t *testing.T) { } } -func strPtr(s string) *string { +func stringPtr(s string) *string { return &s }