Fix nil-pointer panic when GitHub returns a rate-limit error

pkg/github/client/errorsourcehandling.go

@@ -36,8 +36,20 @@ var (
 //
 // See TestGitHubErrorResponseWithTypedNilErrorsIs for a reproduction of the panic.
 func sanitizeGitHubError(err error) error {
-	var ghErr *googlegithub.ErrorResponse
-	if errors.As(err, &ghErr) {
+	// 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
+		abuseErr  *googlegithub.AbuseRateLimitError
+		acceptErr *googlegithub.AcceptedError
+		redirectErr  *googlegithub.RedirectionError
+	)
+	if errors.As(err, &errResp) ||
+		errors.As(err, &rateErr) ||
+		errors.As(err, &abuseErr) ||
+		errors.As(err, &acceptErr) ||
+		errors.As(err, &redirectErr) {
 		return fmt.Errorf("%s", err.Error())
 	}
 	return err
@@ -74,8 +86,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)

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 {
@@ -242,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")
+}