fix(errors): safely format GitHub validation failures

Limit structured formatting to HTTP 422 responses, sanitize allowlisted validation fields, and omit request, response, and documentation metadata while preserving other error contracts.
Refs #3080
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Author: SamMorrowDrums

pkg/errors/error.go

@@ -9,6 +9,7 @@ import (
 "strings"
 "time"
 
+"github.com/github/github-mcp-server/pkg/sanitize"
 "github.com/github/github-mcp-server/pkg/utils"
 "github.com/google/go-github/v89/github"
 "github.com/modelcontextprotocol/go-sdk/mcp"
@@ -192,58 +193,74 @@ func NewGitHubAPIErrorResponse(ctx context.Context, message string, resp *github
 "%s: GitHub secondary rate limit exceeded. Wait before retrying.", message))
 	}
 
-returnutils.NewToolResultErrorFromErr(message, formattedGitHubAPIError(err))
+returnutils.NewToolResultErrorFromErr(message, formatGitHubValidationError(resp, err))
 }
 
-// formattedGitHubAPIError unwraps a github.ErrorResponse so tool results include
-// nested validation messages (for example repository ruleset violations) instead
-// of go-github's compact 422 dump.
-funcformattedGitHubAPIError(errerror) error {
+// formatGitHubValidationError exposes the parsed fields of 422 responses without
+// including request or response metadata from the underlying HTTP exchange.
+funcformatGitHubValidationError(resp*github.Response, errerror) error {
 varghErr*github.ErrorResponse
 if!stderrors.As(err, &ghErr) {
 returnerr
 	}
 
-varparts []string
-switch {
-caseghErr.Response!=nil&&ghErr.Response.StatusCode!=0&&ghErr.Message!="":
-parts=append(parts, fmt.Sprintf("HTTP %d %s", ghErr.Response.StatusCode, ghErr.Message))
-caseghErr.Response!=nil&&ghErr.Response.StatusCode!=0:
-parts=append(parts, fmt.Sprintf("HTTP %d", ghErr.Response.StatusCode))
-caseghErr.Message!="":
-parts=append(parts, ghErr.Message)
+statusCode:=0
+ifghErr.Response!=nil {
+statusCode=ghErr.Response.StatusCode
+	}
+ifstatusCode==0&&resp!=nil {
+statusCode=resp.StatusCode
+	}
+ifstatusCode!=http.StatusUnprocessableEntity {
+returnerr
 	}
 
-for_, item:=rangeghErr.Errors {
-detail:=strings.TrimSpace(item.Message)
-ifdetail=="" {
-varbits []string
-ifitem.Resource!="" {
-bits=append(bits, item.Resource)
- }
-ifitem.Field!="" {
-bits=append(bits, item.Field)
- }
-ifitem.Code!="" {
-bits=append(bits, item.Code)
- }
-detail=strings.Join(bits, " ")
- }
-ifdetail!="" {
+parts:=make([]string, 0, len(ghErr.Errors)+1)
+ifsummary:=sanitizeGitHubValidationText(ghErr.Message); summary!="" {
+parts=append(parts, summary)
+	}
+for_, validationErr:=rangeghErr.Errors {
+ifdetail:=formatGitHubValidationDetail(validationErr); detail!="" {
 parts=append(parts, detail)
  }
 	}
 
-ifghErr.DocumentationURL!="" {
-parts=append(parts, "See "+ghErr.DocumentationURL)
-	}
-
 iflen(parts) ==0 {
-returnerr
+returnstderrors.New("GitHub API validation failed")
 	}
 returnstderrors.New(strings.Join(parts, "\n"))
 }
 
+funcformatGitHubValidationDetail(validationErr github.Error) string {
+resource:=sanitizeGitHubValidationText(validationErr.Resource)
+field:=sanitizeGitHubValidationText(validationErr.Field)
+code:=sanitizeGitHubValidationText(validationErr.Code)
+message:=sanitizeGitHubValidationText(validationErr.Message)
+
+location:=strings.Trim(strings.Join([]string{resource, field}, "."), ".")
+switch {
+caselocation!=""&&code!="":
+location+=" ("+code+")"
+caselocation=="":
+location=code
+	}
+
+switch {
+caselocation!=""&&message!="":
+returnlocation+": "+message
+casemessage!="":
+returnmessage
+default:
+returnlocation
+	}
+}
+
+funcsanitizeGitHubValidationText(valuestring) string {
+// Tool errors are plain text; keep quoted branch patterns readable.
+sanitized:=strings.ReplaceAll(sanitize.Sanitize(value), "'", "'")
+returnstrings.Join(strings.Fields(sanitized), " ")
+}
+
 // NewGitHubGraphQLErrorResponse returns an mcp.NewToolResultError and retains the error in the context for access via middleware
 funcNewGitHubGraphQLErrorResponse(ctx context.Context, messagestring, errerror) *mcp.CallToolResult {
 graphQLErr:=newGitHubGraphQLError(message, err)

pkg/errors/error_test.go

@@ -689,48 +689,101 @@ func TestNewGitHubAPIErrorResponse_RateLimits(t *testing.T) {
 }
 
 funcTestNewGitHubAPIErrorResponse_ValidationMessages(t*testing.T) {
-t.Run("ruleset ErrorResponse includes nested validation messages", func(t*testing.T) {
+t.Run("ruleset ErrorResponse includes sanitized structured validation messages", func(t*testing.T) {
 ctx:=ContextWithGitHubErrors(context.Background())
 
+request, err:=http.NewRequest(http.MethodPost, "https://api.github.test/repos/owner/repo/git/refs?private=secret-url-token", nil)
+require.NoError(t, err)
+request.Header.Set("Authorization", "Bearer secret-request-token")
+response:=&http.Response{
+StatusCode: http.StatusUnprocessableEntity,
+Request: request,
+Header: http.Header{"X-Secret": []string{"secret-response-header"}},
+ }
+
 originalErr:=&github.ErrorResponse{
-Response: &http.Response{StatusCode: http.StatusUnprocessableEntity},
-Message: "Validation Failed",
+Response: response,
+Message: "Validation <script>secret-script</script>Failed\u202e",
 Errors: []github.Error{
  {
 Resource: "GitRef",
 Field: "ref",
 Code: "custom",
-Message: "ref name does not match the required pattern 'feature/*'",
+Message: "ref name does not match the required pattern 'feature/*'\u202e",
  },
  },
-DocumentationURL: "https://docs.github.com/rest/git/refs#create-a-reference",
+DocumentationURL: "https://docs.github.test/private?token=secret-doc-token",
  }
 
-result:=NewGitHubAPIErrorResponse(ctx, "failed to create branch", nil, originalErr)
+wrappedErr:=fmt.Errorf("create ref: %w", originalErr)
+result:=NewGitHubAPIErrorResponse(
+ctx,
+"failed to create branch",
+&github.Response{Response: response},
+wrappedErr,
+ )
 
 text:=requireErrorText(t, result)
-assert.Contains(t, text, "failed to create branch")
-assert.Contains(t, text, "HTTP 422 Validation Failed")
-assert.Contains(t, text, "ref name does not match the required pattern 'feature/*'")
-assert.Contains(t, text, "See https://docs.github.com/rest/git/refs#create-a-reference")
-assert.NotContains(t, text, "Resource:")
+assert.Equal(t, "failed to create branch: Validation Failed\nGitRef.ref (custom): ref name does not match the required pattern 'feature/*'", text)
+assert.NotContains(t, text, "create ref")
+assert.NotContains(t, text, "https://")
+assert.NotContains(t, text, "secret-")
+assert.NotContains(t, text, "Authorization")
+assert.NotContains(t, text, "X-Secret")
+assert.NotContains(t, text, "<script>")
+assert.NotContains(t, text, "\u202e")
+assertContextHasError(t, ctx, wrappedErr)
 	})
 
-t.Run("wrapped ErrorResponse is still unwrapped", func(t*testing.T) {
+t.Run("ordinary validation errors retain resource field and code", func(t*testing.T) {
 ctx:=ContextWithGitHubErrors(context.Background())
 
-originalErr:=fmt.Errorf("create ref: %w", &github.ErrorResponse{
+originalErr:=&github.ErrorResponse{
 Response: &http.Response{StatusCode: http.StatusUnprocessableEntity},
 Message: "Validation Failed",
 Errors: []github.Error{
- {Message: "Changes must be made through a pull request."},
+ {
+Resource: "Repository",
+Field: "name",
+Code: "invalid",
+ },
  },
- })
+ }
+
+result:=NewGitHubAPIErrorResponse(ctx, "API call failed", nil, originalErr)
+
+text:=requireErrorText(t, result)
+assert.Equal(t, "API call failed: Validation Failed\nRepository.name (invalid)", text)
+	})
+
+t.Run("top-level validation message is useful without nested errors", func(t*testing.T) {
+ctx:=ContextWithGitHubErrors(context.Background())
+
+originalErr:=&github.ErrorResponse{
+Response: &http.Response{StatusCode: http.StatusUnprocessableEntity},
+Message: "Reference already exists",
+ }
 
 result:=NewGitHubAPIErrorResponse(ctx, "failed to create branch", nil, originalErr)
 
 text:=requireErrorText(t, result)
-assert.Contains(t, text, "Changes must be made through a pull request.")
-assert.NotContains(t, text, "create ref:")
+assert.Equal(t, "failed to create branch: Reference already exists", text)
+	})
+
+t.Run("non-422 ErrorResponse preserves the existing error contract", func(t*testing.T) {
+ctx:=ContextWithGitHubErrors(context.Background())
+
+originalErr:=&github.ErrorResponse{
+Response: &http.Response{StatusCode: http.StatusConflict},
+Message: "Conflict",
+Errors: []github.Error{
+ {Message: "Changes must be made through a pull request."},
+ },
+ }
+
+result:=NewGitHubAPIErrorResponse(ctx, "API call failed", nil, originalErr)
+
+text:=requireErrorText(t, result)
+assert.Equal(t, "API call failed: "+originalErr.Error(), text)
 	})
 }

pkg/github/repositories_test.go

@@ -1008,6 +1008,7 @@ func Test_CreateBranch(t *testing.T) {
 expectErrorbool
 expectedRef*github.Reference
 expectedErrMsgstring
+unexpectedErrs []string
 	}{
  {
 name: "successful branch creation with from_branch",
@@ -1096,7 +1097,8 @@ func Test_CreateBranch(t *testing.T) {
 "from_branch": "main",
  },
 expectError: true,
-expectedErrMsg: "failed to create branch",
+expectedErrMsg: "Reference already exists",
+unexpectedErrs: []string{"422", "http://", "https://"},
  },
  {
 name: "create branch surfaces ruleset validation details",
@@ -1127,6 +1129,7 @@ func Test_CreateBranch(t *testing.T) {
  },
 expectError: true,
 expectedErrMsg: "ref name does not match the required pattern 'feature/*'",
+unexpectedErrs: []string{"422", "https://docs.github.com"},
  },
 	}
 
@@ -1151,6 +1154,9 @@ func Test_CreateBranch(t *testing.T) {
 require.True(t, result.IsError)
 errorContent:=getErrorResult(t, result)
 assert.Contains(t, errorContent.Text, tc.expectedErrMsg)
+for_, unexpectedErr:=rangetc.unexpectedErrs {
+assert.NotContains(t, errorContent.Text, unexpectedErr)
+ }
 return
  }