From 39684ab34fa50f9bbbd94e3ce7dac8eba6688ada Mon Sep 17 00:00:00 2001 From: alyssabull Date: Mon, 17 Aug 2026 21:12:19 -0600 Subject: [PATCH 1/4] fix a nil pointer panic in datasource error classification --- pkg/github/client/errorsourcehandling.go | 6 ++++-- pkg/github/client/errorsourcehandling_test.go | 9 +++++++++ 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/pkg/github/client/errorsourcehandling.go b/pkg/github/client/errorsourcehandling.go index d48c7144..39673627 100644 --- a/pkg/github/client/errorsourcehandling.go +++ b/pkg/github/client/errorsourcehandling.go @@ -74,8 +74,10 @@ func addErrorSourceToError(err error, resp *googlegithub.Response) error { return backend.PluginError(err) } } - // If we have response we can use the status code from it - if resp != nil { + // If there is a response, use the status code from it. + // resp.StatusCode reads go-github's embedded *http.Response. That pointer can + // be nil even when resp is not nil. Guard it to avoid a nil-pointer panic. + if resp != nil && resp.Response != nil { if resp.StatusCode/100 != 2 { if backend.ErrorSourceFromHTTPStatus(resp.StatusCode) == backend.ErrorSourceDownstream { return backend.DownstreamError(err) diff --git a/pkg/github/client/errorsourcehandling_test.go b/pkg/github/client/errorsourcehandling_test.go index e2f65d0f..de6d12b3 100644 --- a/pkg/github/client/errorsourcehandling_test.go +++ b/pkg/github/client/errorsourcehandling_test.go @@ -95,6 +95,15 @@ func TestAddErrorSourceToError(t *testing.T) { resp: nil, expected: backend.DownstreamError(errors.New("Resource not accessible by integration")), }, + { + // go-github's Response embeds a *http.Response that can be nil, even + // when the Response wrapper is not nil. Then resp.StatusCode panics. + // The function must return the error as-is. It must not crash. + name: "response wrapper with nil inner http.Response does not panic", + err: errors.New("some transport error"), + resp: &googlegithub.Response{}, + expected: errors.New("some transport error"), + }, } for _, tt := range tests { From ad343e2bc7ba93f49901fd10625bd2e00ab8369e Mon Sep 17 00:00:00 2001 From: alyssabull Date: Wed, 19 Aug 2026 16:09:31 -0600 Subject: [PATCH 2/4] fix nil-pointer panic when GitHub returns a rate-limit error --- pkg/github/client/errorsourcehandling.go | 20 ++++++++++++++-- pkg/github/client/errorsourcehandling_test.go | 24 +++++++++++++++++++ 2 files changed, 42 insertions(+), 2 deletions(-) diff --git a/pkg/github/client/errorsourcehandling.go b/pkg/github/client/errorsourcehandling.go index 39673627..5c597d4f 100644 --- a/pkg/github/client/errorsourcehandling.go +++ b/pkg/github/client/errorsourcehandling.go @@ -36,8 +36,24 @@ var ( // // See TestGitHubErrorResponseWithTypedNilErrorsIs for a reproduction of the panic. func sanitizeGitHubError(err error) error { - var ghErr *googlegithub.ErrorResponse - if errors.As(err, &ghErr) { + // go-github's error types implement Is(target) by calling errors.As(target, &v). + // The SDK's guessErrorStatus calls errors.Is(err, (*url.Error)(nil)) with a typed + // nil target, which makes that errors.As call (*url.Error).Unwrap on a nil + // receiver and panic. Flatten any of these to a plain error: that drops the + // unsafe Is method from the chain while preserving the message. This list + // mirrors every go-github error type that defines such an Is method (github.go). + var ( + errResp *googlegithub.ErrorResponse + rateErr *googlegithub.RateLimitError + abuseErr *googlegithub.AbuseRateLimitError + acceptErr *googlegithub.AcceptedError + redirErr *googlegithub.RedirectionError + ) + if errors.As(err, &errResp) || + errors.As(err, &rateErr) || + errors.As(err, &abuseErr) || + errors.As(err, &acceptErr) || + errors.As(err, &redirErr) { return fmt.Errorf("%s", err.Error()) } return err diff --git a/pkg/github/client/errorsourcehandling_test.go b/pkg/github/client/errorsourcehandling_test.go index de6d12b3..ece45673 100644 --- a/pkg/github/client/errorsourcehandling_test.go +++ b/pkg/github/client/errorsourcehandling_test.go @@ -251,3 +251,27 @@ func TestExtractStatusCode(t *testing.T) { }) } } + +// TestSanitizeGitHubError_RateLimitErrorNoPanic checks that a rate-limit error does +// not crash the SDK's error classification. go-github's *RateLimitError has an Is +// method that calls errors.As(target, &v). The SDK classifies errors with +// errors.Is(err, (*url.Error)(nil)), where the target is a typed nil. That makes the +// errors.As call (*url.Error).Unwrap on a nil receiver and panic. sanitizeGitHubError +// flattens the error so this cannot happen. +func TestSanitizeGitHubError_RateLimitErrorNoPanic(t *testing.T) { + req, _ := http.NewRequest("GET", "https://api.github.com/x", nil) + ghResp := &http.Response{StatusCode: 403, Request: req} + rle := &googlegithub.RateLimitError{ + Response: ghResp, + Message: "API rate limit exceeded", + } + + // What the plugin returns up the chain for a rate-limit error. + classified := addErrorSourceToError(rle, &googlegithub.Response{Response: ghResp}) + + // Exactly what the SDK's guessErrorStatus does (status.go:112). + var connErr *url.Error + require.NotPanics(t, func() { + _ = errors.Is(classified, connErr) + }, "errors.Is with a typed-nil *url.Error target must not panic") +} From 57a1a4848e4e441ffd2d01162167cf00e1137a99 Mon Sep 17 00:00:00 2001 From: alyssabull Date: Wed, 19 Aug 2026 16:22:21 -0600 Subject: [PATCH 3/4] Fix spellcheck --- pkg/github/client/errorsourcehandling.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/github/client/errorsourcehandling.go b/pkg/github/client/errorsourcehandling.go index 5c597d4f..82047169 100644 --- a/pkg/github/client/errorsourcehandling.go +++ b/pkg/github/client/errorsourcehandling.go @@ -47,13 +47,13 @@ func sanitizeGitHubError(err error) error { rateErr *googlegithub.RateLimitError abuseErr *googlegithub.AbuseRateLimitError acceptErr *googlegithub.AcceptedError - redirErr *googlegithub.RedirectionError + redirectErr *googlegithub.RedirectionError ) if errors.As(err, &errResp) || errors.As(err, &rateErr) || errors.As(err, &abuseErr) || errors.As(err, &acceptErr) || - errors.As(err, &redirErr) { + errors.As(err, &redirectErr) { return fmt.Errorf("%s", err.Error()) } return err From c9b4e29fda92590f1bf35a04ea31976d0fe9ed05 Mon Sep 17 00:00:00 2001 From: alyssabull Date: Thu, 20 Aug 2026 09:09:35 -0600 Subject: [PATCH 4/4] Shorten comment --- pkg/github/client/errorsourcehandling.go | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/pkg/github/client/errorsourcehandling.go b/pkg/github/client/errorsourcehandling.go index 82047169..a8fa90d7 100644 --- a/pkg/github/client/errorsourcehandling.go +++ b/pkg/github/client/errorsourcehandling.go @@ -36,12 +36,8 @@ var ( // // See TestGitHubErrorResponseWithTypedNilErrorsIs for a reproduction of the panic. func sanitizeGitHubError(err error) error { - // go-github's error types implement Is(target) by calling errors.As(target, &v). - // The SDK's guessErrorStatus calls errors.Is(err, (*url.Error)(nil)) with a typed - // nil target, which makes that errors.As call (*url.Error).Unwrap on a nil - // receiver and panic. Flatten any of these to a plain error: that drops the - // unsafe Is method from the chain while preserving the message. This list - // mirrors every go-github error type that defines such an Is method (github.go). + // Flatten to a plain error to drop the unsafe Is method while keeping the + // message. These are every go-github error type that defines such an Is (github.go). var ( errResp *googlegithub.ErrorResponse rateErr *googlegithub.RateLimitError