From a66ba489ecd0766447c09971f3b4cf2cbb2f119e Mon Sep 17 00:00:00 2001
From: Sam Morrow <info@sam-morrow.com>
Date: Fri, 28 Nov 2025 23:12:14 +0100
Subject: [PATCH] fix panic due to defer reading body of failed request

---
 pkg/github/repository_resource.go      |  5 ++--
 pkg/github/repository_resource_test.go | 41 ++++++++++++++++++++++++++
 2 files changed, 44 insertions(+), 2 deletions(-)

diff --git a/pkg/github/repository_resource.go b/pkg/github/repository_resource.go
index 8fb1a52ed..01b438474 100644
--- a/pkg/github/repository_resource.go
+++ b/pkg/github/repository_resource.go
@@ -138,13 +138,14 @@ func RepositoryResourceContentsHandler(getClient GetClientFn, getRawClient raw.G
 		}
 
 		resp, err := rawClient.GetRawContent(ctx, owner, repo, path, rawOpts)
+		if err != nil {
+			return nil, fmt.Errorf("failed to get raw content: %w", err)
+		}
 		defer func() {
 			_ = resp.Body.Close()
 		}()
 		// If the raw content is not found, we will fall back to the GitHub API (in case it is a directory)
 		switch {
-		case err != nil:
-			return nil, fmt.Errorf("failed to get raw content: %w", err)
 		case resp.StatusCode == http.StatusOK:
 			ext := filepath.Ext(path)
 			mimeType := resp.Header.Get("Content-Type")
diff --git a/pkg/github/repository_resource_test.go b/pkg/github/repository_resource_test.go
index 96bf33b72..312424827 100644
--- a/pkg/github/repository_resource_test.go
+++ b/pkg/github/repository_resource_test.go
@@ -2,6 +2,7 @@ package github
 
 import (
 	"context"
+	"errors"
 	"net/http"
 	"net/url"
 	"testing"
@@ -14,6 +15,15 @@ import (
 	"github.com/stretchr/testify/require"
 )
 
+// errorTransport is a http.RoundTripper that always returns an error.
+type errorTransport struct {
+	err error
+}
+
+func (t *errorTransport) RoundTrip(*http.Request) (*http.Response, error) {
+	return nil, t.err
+}
+
 func Test_repositoryResourceContentsHandler(t *testing.T) {
 	base, _ := url.Parse("https://raw.example.com/")
 	tests := []struct {
@@ -256,6 +266,37 @@ func Test_repositoryResourceContentsHandler(t *testing.T) {
 	}
 }
 
+// Test_repositoryResourceContentsHandler_NetworkError tests that a network error
+// during raw content fetch does not cause a panic (nil response body dereference).
+func Test_repositoryResourceContentsHandler_NetworkError(t *testing.T) {
+	base, _ := url.Parse("https://raw.example.com/")
+	networkErr := errors.New("network error: connection refused")
+
+	httpClient := &http.Client{Transport: &errorTransport{err: networkErr}}
+	client := github.NewClient(httpClient)
+	mockRawClient := raw.NewClient(client, base)
+	handler := RepositoryResourceContentsHandler(stubGetClientFn(client), stubGetRawClientFn(mockRawClient))
+
+	request := mcp.ReadResourceRequest{
+		Params: struct {
+			URI       string         `json:"uri"`
+			Arguments map[string]any `json:"arguments,omitempty"`
+		}{
+			Arguments: map[string]any{
+				"owner": []string{"owner"},
+				"repo":  []string{"repo"},
+				"path":  []string{"README.md"},
+			},
+		},
+	}
+
+	// This should not panic, even though the HTTP client returns an error
+	resp, err := handler(context.TODO(), request)
+	require.Error(t, err)
+	require.Nil(t, resp)
+	require.ErrorContains(t, err, "failed to get raw content")
+}
+
 func Test_GetRepositoryResourceContent(t *testing.T) {
 	mockRawClient := raw.NewClient(github.NewClient(nil), &url.URL{})
 	tmpl, _ := GetRepositoryResourceContent(nil, stubGetRawClientFn(mockRawClient), translations.NullTranslationHelper)