Centralize sanitization of untrusted GitHub response fields

pkg/github/discussions.go

@@ -8,6 +8,7 @@ import (
 
 	"github.com/github/github-mcp-server/pkg/ifc"
 	"github.com/github/github-mcp-server/pkg/inventory"
+	"github.com/github/github-mcp-server/pkg/sanitize"
 	"github.com/github/github-mcp-server/pkg/scopes"
 	"github.com/github/github-mcp-server/pkg/translations"
 	"github.com/github/github-mcp-server/pkg/utils"
@@ -99,7 +100,7 @@ type WithCategoryNoOrder struct {
 func fragmentToDiscussion(fragment NodeFragment) *github.Discussion {
 	return &github.Discussion{
 		Number:    github.Ptr(int(fragment.Number)),
-		Title:     github.Ptr(string(fragment.Title)),
+		Title:     github.Ptr(sanitize.Sanitize(string(fragment.Title))),
 		HTMLURL:   github.Ptr(string(fragment.URL)),
 		CreatedAt: &github.Timestamp{Time: fragment.CreatedAt.Time},
 		UpdatedAt: &github.Timestamp{Time: fragment.UpdatedAt.Time},
@@ -360,8 +361,8 @@ func GetDiscussion(t translations.TranslationHelperFunc) inventory.ServerTool {
 			// like ListDiscussions and GetDiscussionComments).
 			response := map[string]any{
 				"number":     int(d.Number),
-				"title":      string(d.Title),
-				"body":       string(d.Body),
+				"title":      sanitize.Sanitize(string(d.Title)),
+				"body":       sanitize.Sanitize(string(d.Body)),
 				"url":        string(d.URL),
 				"closed":     bool(d.Closed),
 				"isAnswered": bool(d.IsAnswered),
@@ -520,18 +521,10 @@ func GetDiscussionComments(t translations.TranslationHelperFunc) inventory.Serve
 					return utils.NewToolResultError(err.Error()), nil, nil
 				}
 				for _, c := range q.Repository.Discussion.Comments.Nodes {
-					comment := MinimalDiscussionComment{
-						ID:              fmt.Sprintf("%v", c.ID),
-						Body:            string(c.Body),
-						IsAnswer:        bool(c.IsAnswer),
-						ReplyTotalCount: c.Replies.TotalCount,
-					}
+					comment := newMinimalDiscussionComment(fmt.Sprintf("%v", c.ID), string(c.Body), bool(c.IsAnswer))
+					comment.ReplyTotalCount = c.Replies.TotalCount
 					for _, r := range c.Replies.Nodes {
-						comment.Replies = append(comment.Replies, MinimalDiscussionComment{
-							ID:       fmt.Sprintf("%v", r.ID),
-							Body:     string(r.Body),
-							IsAnswer: bool(r.IsAnswer),
-						})
+						comment.Replies = append(comment.Replies, newMinimalDiscussionComment(fmt.Sprintf("%v", r.ID), string(r.Body), bool(r.IsAnswer)))
 					}
 					comments = append(comments, comment)
 				}
@@ -562,11 +555,7 @@ func GetDiscussionComments(t translations.TranslationHelperFunc) inventory.Serve
 					return utils.NewToolResultError(err.Error()), nil, nil
 				}
 				for _, c := range q.Repository.Discussion.Comments.Nodes {
-					comments = append(comments, MinimalDiscussionComment{
-						ID:       fmt.Sprintf("%v", c.ID),
-						Body:     string(c.Body),
-						IsAnswer: bool(c.IsAnswer),
-					})
+					comments = append(comments, newMinimalDiscussionComment(fmt.Sprintf("%v", c.ID), string(c.Body), bool(c.IsAnswer)))
 				}
 				pageInfo = q.Repository.Discussion.Comments.PageInfo
 				totalCount = q.Repository.Discussion.Comments.TotalCount

pkg/github/discussions_test.go

@@ -553,6 +553,30 @@ func Test_GetDiscussion(t *testing.T) {
 			expectError: true,
 			errContains: "discussion not found",
 		},
+		{
+			name: "sanitizes malicious title and body",
+			response: githubv4mock.DataResponse(map[string]any{
+				"repository": map[string]any{"discussion": map[string]any{
+					"number":     1,
+					"title":      maliciousText,
+					"body":       maliciousText,
+					"url":        "https://github.com/owner/repo/discussions/1",
+					"createdAt":  "2025-04-25T12:00:00Z",
+					"closed":     false,
+					"isAnswered": false,
+					"category":   map[string]any{"name": "General"},
+				}},
+			}),
+			expectError: false,
+			expected: map[string]any{
+				"number":     float64(1),
+				"title":      sanitizedText,
+				"body":       sanitizedText,
+				"url":        "https://github.com/owner/repo/discussions/1",
+				"closed":     false,
+				"isAnswered": false,
+			},
+		},
 	}
 	for _, tc := range tests {
 		t.Run(tc.name, func(t *testing.T) {

pkg/github/find_duplicate.go

@@ -152,12 +152,15 @@ func FindDuplicate(t translations.TranslationHelperFunc) inventory.ServerTool {
 					return utils.NewToolResultError("ranked duplicate detection is unavailable: the semantic-similarity endpoint returned issues without ranking metadata (the server-side duplicate-ranking feature is not enabled for this caller or repository)"), nil, nil
 				}
 				candidates = append(candidates, duplicateCandidate{
-					Issue: MinimalIssueRef{
-						Number: res.Issue.Number,
-						Title:  res.Issue.Title,
-						State:  res.Issue.State,
-						URL:    res.Issue.HTMLURL,
-					},
+					// Candidates are always scoped to the requested repository, so the
+					// ref's repository field is left empty as it was before.
+					Issue: newMinimalIssueRef(
+						res.Issue.Number,
+						res.Issue.Title,
+						res.Issue.State,
+						res.Issue.HTMLURL,
+						"",
+					),
 					Score:           res.Score,
 					Confidence:      res.Confidence,
 					LikelyDuplicate: res.LikelyDuplicate,

pkg/github/find_duplicate_test.go

@@ -120,6 +120,52 @@ func Test_FindDuplicate_RankedResults(t *testing.T) {
 	assert.False(t, candidates[1].LikelyDuplicate)
 }
 
+// Test_FindDuplicate_SanitizesIssueTitle asserts that candidate issue titles, which are
+// user-authored content from an arbitrary repository, are sanitized before being returned.
+// Without this the tool would forward hidden-instruction payloads straight to the model.
+func Test_FindDuplicate_SanitizesIssueTitle(t *testing.T) {
+	serverTool := FindDuplicate(translations.NullTranslationHelper)
+
+	rankedResults := []map[string]any{
+		{
+			"issue": map[string]any{
+				"number":   456,
+				"title":    maliciousText,
+				"state":    "open",
+				"html_url": "https://github.com/owner/repo/issues/456",
+			},
+			"score":            0.95,
+			"confidence":       "high",
+			"likely_duplicate": true,
+		},
+	}
+
+	handler := func(w http.ResponseWriter, _ *http.Request) {
+		w.WriteHeader(http.StatusOK)
+		_, _ = w.Write(MustMarshal(rankedResults))
+	}
+
+	client := mustNewGHClient(t, NewMockedHTTPClient(WithRequestMatchHandler(endpointSemanticallySimilar, http.HandlerFunc(handler))))
+	deps := BaseDeps{Client: client}
+	toolHandler := serverTool.Handler(deps)
+
+	request := createMCPRequest(map[string]any{
+		"owner":        "owner",
+		"repo":         "repo",
+		"issue_number": float64(123),
+	})
+	result, err := toolHandler(ContextWithDeps(context.Background(), deps), &request)
+	require.NoError(t, err)
+	require.False(t, result.IsError, "expected result to not be an error")
+
+	text := getTextResult(t, result)
+	var candidates []duplicateCandidate
+	require.NoError(t, json.Unmarshal([]byte(text.Text), &candidates))
+	require.Len(t, candidates, 1)
+	assert.Equal(t, sanitizedText, candidates[0].Issue.Title)
+	assert.NotContains(t, text.Text, "<script>")
+}
+
 func Test_FindDuplicate_OmitsUnsetParams(t *testing.T) {
 	serverTool := FindDuplicate(translations.NullTranslationHelper)
 

pkg/github/issue_dependencies.go

@@ -171,16 +171,17 @@ func issueToDependencyRef(issue *github.Issue) MinimalIssueRef {
 	if issue == nil {
 		return MinimalIssueRef{}
 	}
-	ref := MinimalIssueRef{
-		Number: issue.GetNumber(),
-		Title:  issue.GetTitle(),
-		State:  strings.ToUpper(issue.GetState()),
-		URL:    issue.GetHTMLURL(),
-	}
+	var repository string
 	if owner, repo, ok := parseRepositoryURL(issue.GetRepositoryURL()); ok {
-		ref.Repository = owner + "/" + repo
+		repository = owner + "/" + repo
 	}
-	return ref
+	return newMinimalIssueRef(
+		issue.GetNumber(),
+		issue.GetTitle(),
+		strings.ToUpper(issue.GetState()),
+		issue.GetHTMLURL(),
+		repository,
+	)
 }
 
 // IssueDependencyWrite creates a tool to add or remove an issue dependency

pkg/github/issues.go

@@ -920,16 +920,6 @@ func GetIssue(ctx context.Context, client *github.Client, deps ToolDependencies,
 		}
 	}
 
-	// Sanitize title/body on response
-	if issue != nil {
-		if issue.Title != nil {
-			issue.Title = github.Ptr(sanitize.Sanitize(*issue.Title))
-		}
-		if issue.Body != nil {
-			issue.Body = github.Ptr(sanitize.Sanitize(*issue.Body))
-		}
-	}
-
 	minimalIssue := convertToMinimalIssue(issue)
 
 	// Always drop the verbose REST IssueFieldValues; enrich with the GraphQL
@@ -2003,9 +1993,31 @@ type SearchIssueResult struct {
 	FieldValues []MinimalFieldValue `json:"field_values,omitempty"`
 }
 
+// sanitizeIssueTitleAndBody mutates issue.Title and issue.Body in place, applying the shared
+// untrusted-content sanitization policy (pkg/sanitize). It exists for the handful of response
+// paths — search_issues and search_pull_requests — that marshal a raw *github.Issue directly
+// instead of routing through one of the convertToMinimal* helpers in minimal_types.go, which
+// sanitize on their own. It is a no-op for a nil issue or unset fields.
+func sanitizeIssueTitleAndBody(issue *github.Issue) {
+	if issue == nil {
+		return
+	}
+	if issue.Title != nil {
+		issue.Title = github.Ptr(sanitize.Sanitize(*issue.Title))
+	}
+	if issue.Body != nil {
+		issue.Body = github.Ptr(sanitize.Sanitize(*issue.Body))
+	}
+}
+
 // MarshalJSON serializes SearchIssueResult, suppressing the raw issue_field_values from the
 // embedded REST response in favour of the normalized field_values populated via GraphQL enrichment.
+// It also sanitizes the embedded issue's Title and Body in place: search_issues is one of the few
+// response paths that marshals a raw *github.Issue directly rather than routing through a
+// convertToMinimal* helper (see minimal_types.go), so sanitization must happen here instead.
 func (r SearchIssueResult) MarshalJSON() ([]byte, error) {
+	sanitizeIssueTitleAndBody(r.Issue)
+
 	issueBytes, err := json.Marshal(r.Issue)
 	if err != nil {
 		return nil, err
@@ -2184,27 +2196,27 @@ func fetchIssueReadEnrichment(ctx context.Context, gqlClient *githubv4.Client, n
 
 		if p := n.Issue.Parent; p != nil {
 			enrichment.Parent = &issueReadParent{
-				Ref: MinimalIssueRef{
-					Number:     int(p.Number),
-					Title:      sanitize.Sanitize(string(p.Title)),
-					State:      string(p.State),
-					URL:        string(p.URL),
-					Repository: string(p.Repository.NameWithOwner),
-				},
+				Ref: newMinimalIssueRef(
+					int(p.Number),
+					string(p.Title),
+					string(p.State),
+					string(p.URL),
+					string(p.Repository.NameWithOwner),
+				),
 				AuthorLogin: string(p.Author.Login),
 			}
 		}
 
 		closing := make([]issueReadClosingPullRequest, 0, len(n.Issue.ClosedByPullRequestsReferences.Nodes))
 		for _, pr := range n.Issue.ClosedByPullRequestsReferences.Nodes {
 			closing = append(closing, issueReadClosingPullRequest{
-				Ref: MinimalPullRequestRef{
-					Number:     int(pr.Number),
-					Title:      sanitize.Sanitize(string(pr.Title)),
-					State:      string(pr.State),
-					URL:        string(pr.URL),
-					Repository: string(pr.Repository.NameWithOwner),
-				},
+				Ref: newMinimalPullRequestRef(
+					int(pr.Number),
+					string(pr.Title),
+					string(pr.State),
+					string(pr.URL),
+					string(pr.Repository.NameWithOwner),
+				),
 				AuthorLogin: string(pr.Author.Login),
 			})
 		}