From d4afefdf15cb048cead9b92f8b39e5722d776522 Mon Sep 17 00:00:00 2001 From: Sam Morrow Date: Wed, 19 Aug 2026 14:31:09 +0200 Subject: [PATCH 1/2] Centralize sanitization of untrusted GitHub response fields Sanitization was previously applied ad hoc at a handful of tool call sites (GetIssue, GetPullRequest, ListPullRequests) rather than in the shared convertToMinimal* converters, so equivalent user-authored text returned by other tools (issue comments, PR reviews, review comments, releases, commit messages, discussions, project item titles) was returned unsanitized. - Apply sanitize.Sanitize inside the convertToMinimal* helpers in minimal_types.go for issue/PR titles and bodies, issue comments, PR reviews, review comments, releases, commit messages, and project item content titles. This is the single, shared conversion point used by nearly every read tool, so fixing it there covers get/list issues, pull requests, comments, reviews, review comments, releases, commits, and project items consistently. - Add a sanitizeIssueTitleAndBody helper and use it for the two response paths that marshal a raw *github.Issue directly instead of a Minimal* type: search_issues (SearchIssueResult.MarshalJSON) and search_pull_requests (searchHandler). - Sanitize discussion titles/bodies/comments (list_discussions, get_discussion, get_discussion_comments), which previously had no sanitization at all, via a new newMinimalDiscussionComment constructor and inline fixes. - Sanitize project status update bodies. - Remove the now-redundant scattered sanitize calls in GetIssue, GetPullRequest, and ListPullRequests now that the shared converters sanitize on their own. Patches, diffs, and raw file contents are intentionally left untouched to preserve fidelity. Adds table-driven regression tests covering every touched converter, the search_issues/search_pull_requests raw-passthrough paths, and a fidelity check that patches/diffs are not altered. Fixes #3106 Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- pkg/github/discussions.go | 27 +-- pkg/github/discussions_test.go | 24 +++ pkg/github/issues.go | 32 ++- pkg/github/minimal_types.go | 47 +++-- pkg/github/projects.go | 3 +- pkg/github/pullrequests.go | 24 --- pkg/github/sanitize_coverage_test.go | 294 +++++++++++++++++++++++++++ pkg/github/search_utils.go | 6 + 8 files changed, 385 insertions(+), 72 deletions(-) create mode 100644 pkg/github/sanitize_coverage_test.go diff --git a/pkg/github/discussions.go b/pkg/github/discussions.go index 8643acc7ef..9d9d02f82a 100644 --- a/pkg/github/discussions.go +++ b/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 diff --git a/pkg/github/discussions_test.go b/pkg/github/discussions_test.go index c2c0426218..a41a903d4e 100644 --- a/pkg/github/discussions_test.go +++ b/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) { diff --git a/pkg/github/issues.go b/pkg/github/issues.go index fbd51dc2b1..dad4b9e5f7 100644 --- a/pkg/github/issues.go +++ b/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 diff --git a/pkg/github/minimal_types.go b/pkg/github/minimal_types.go index 6902170c1c..cfd5fd2743 100644 --- a/pkg/github/minimal_types.go +++ b/pkg/github/minimal_types.go @@ -198,6 +198,17 @@ type MinimalDiscussionComment struct { ReplyTotalCount int `json:"replyTotalCount,omitempty"` } +// newMinimalDiscussionComment is the single constructor for MinimalDiscussionComment, +// ensuring the untrusted, user-authored body is sanitized consistently regardless of +// which discussion query (with or without replies) produced it. +func newMinimalDiscussionComment(id string, body string, isAnswer bool) MinimalDiscussionComment { + return MinimalDiscussionComment{ + ID: id, + Body: sanitize.Sanitize(body), + IsAnswer: isAnswer, + } +} + // MinimalCodeSearchResult is the trimmed output type for code search results. type MinimalCodeSearchResult struct { TotalCount int `json:"total_count"` @@ -759,7 +770,7 @@ func convertToMinimalPullRequestReview(review *github.PullRequestReview) Minimal m := MinimalPullRequestReview{ ID: review.GetID(), State: review.GetState(), - Body: review.GetBody(), + Body: sanitize.Sanitize(review.GetBody()), HTMLURL: review.GetHTMLURL(), User: convertToMinimalUser(review.GetUser()), CommitID: review.GetCommitID(), @@ -776,8 +787,8 @@ func convertToMinimalPullRequestReview(review *github.PullRequestReview) Minimal func convertToMinimalIssue(issue *github.Issue) MinimalIssue { m := MinimalIssue{ Number: issue.GetNumber(), - Title: issue.GetTitle(), - Body: issue.GetBody(), + Title: sanitize.Sanitize(issue.GetTitle()), + Body: sanitize.Sanitize(issue.GetBody()), State: issue.GetState(), StateReason: issue.GetStateReason(), Draft: issue.GetDraft(), @@ -977,7 +988,7 @@ func convertToMinimalIssuesResponseWithoutFieldValues(fragment issueQueryFragmen func convertToMinimalIssueComment(comment *github.IssueComment) MinimalIssueComment { m := MinimalIssueComment{ ID: comment.GetID(), - Body: comment.GetBody(), + Body: sanitize.Sanitize(comment.GetBody()), HTMLURL: comment.GetHTMLURL(), User: convertToMinimalUser(comment.GetUser()), AuthorAssociation: comment.GetAuthorAssociation(), @@ -1026,7 +1037,7 @@ func convertToMinimalFileContentResponse(resp *github.RepositoryContentResponse) m.Commit = &MinimalFileCommit{ SHA: resp.Commit.GetSHA(), - Message: resp.Commit.GetMessage(), + Message: sanitize.Sanitize(resp.Commit.GetMessage()), HTMLURL: resp.Commit.GetHTMLURL(), } @@ -1046,8 +1057,8 @@ func convertToMinimalFileContentResponse(resp *github.RepositoryContentResponse) func convertToMinimalPullRequest(pr *github.PullRequest) MinimalPullRequest { m := MinimalPullRequest{ Number: pr.GetNumber(), - Title: pr.GetTitle(), - Body: pr.GetBody(), + Title: sanitize.Sanitize(pr.GetTitle()), + Body: sanitize.Sanitize(pr.GetBody()), State: pr.GetState(), Draft: pr.GetDraft(), Merged: pr.GetMerged(), @@ -1241,7 +1252,7 @@ func convertIssueToMinimalProjectItemContent(issue *github.Issue) *MinimalProjec ID: issue.GetID(), NodeID: issue.GetNodeID(), Number: issue.GetNumber(), - Title: issue.GetTitle(), + Title: sanitize.Sanitize(issue.GetTitle()), State: issue.GetState(), StateReason: issue.GetStateReason(), HTMLURL: issue.GetHTMLURL(), @@ -1278,7 +1289,7 @@ func convertPullRequestToMinimalProjectItemContent(pr *github.PullRequest) *Mini ID: pr.GetID(), NodeID: pr.GetNodeID(), Number: pr.GetNumber(), - Title: pr.GetTitle(), + Title: sanitize.Sanitize(pr.GetTitle()), State: pr.GetState(), HTMLURL: pr.GetHTMLURL(), Repository: pullRequestRepositoryFullName(pr), @@ -1315,7 +1326,7 @@ func convertDraftIssueToMinimalProjectItemContent(draftIssue *github.ProjectV2Dr m := &MinimalProjectItemContent{ ID: draftIssue.GetID(), NodeID: draftIssue.GetNodeID(), - Title: draftIssue.GetTitle(), + Title: sanitize.Sanitize(draftIssue.GetTitle()), CreatedAt: formatProjectTimestamp(draftIssue.CreatedAt), UpdatedAt: formatProjectTimestamp(draftIssue.UpdatedAt), } @@ -1574,7 +1585,7 @@ func minimalProjectPullRequestRefFromPullRequest(pr *github.PullRequest) minimal } return minimalProjectPullRequestRef{ Number: pr.GetNumber(), - Title: pr.GetTitle(), + Title: sanitize.Sanitize(pr.GetTitle()), State: pr.GetState(), HTMLURL: pr.GetHTMLURL(), Repository: pullRequestRepositoryFullName(pr), @@ -1596,7 +1607,7 @@ func minimalProjectPullRequestRefFromMap(value map[string]any) minimalProjectPul return minimalProjectPullRequestRef{ Number: intFromAny(value["number"]), - Title: stringFromMap(value, "title"), + Title: sanitize.Sanitize(stringFromMap(value, "title")), State: stringFromMap(value, "state"), HTMLURL: htmlURL, Repository: repository, @@ -1756,7 +1767,7 @@ func newMinimalCommitFromCore(sha, htmlURL string, commit *github.Commit, author if commit != nil { minimalCommit.Commit = &MinimalCommitInfo{ - Message: commit.GetMessage(), + Message: sanitize.Sanitize(commit.GetMessage()), } if commit.Author != nil { @@ -1959,7 +1970,7 @@ func convertToMinimalPullRequestCommits(commits []*github.RepositoryCommit) []Mi } if commit.Commit != nil { - minimalCommit.Message = commit.Commit.GetMessage() + minimalCommit.Message = sanitize.Sanitize(commit.Commit.GetMessage()) minimalCommit.Author = convertToMinimalCommitAuthor(commit.Commit.Author) } @@ -1997,8 +2008,8 @@ func convertToMinimalRelease(release *github.RepositoryRelease) MinimalRelease { m := MinimalRelease{ ID: release.GetID(), TagName: release.GetTagName(), - Name: release.GetName(), - Body: release.GetBody(), + Name: sanitize.Sanitize(release.GetName()), + Body: sanitize.Sanitize(release.GetBody()), HTMLURL: release.GetHTMLURL(), Prerelease: release.GetPrerelease(), Draft: release.GetDraft(), @@ -2054,7 +2065,7 @@ func convertToMinimalWorkflowRun(workflowRun *github.WorkflowRun) MinimalWorkflo if headCommit := workflowRun.GetHeadCommit(); headCommit != nil && headCommit.GetMessage() != "" { minimalRun.HeadCommit = &MinimalWorkflowRunHeadCommit{ - Message: headCommit.GetMessage(), + Message: sanitize.Sanitize(headCommit.GetMessage()), } } @@ -2239,7 +2250,7 @@ func convertToMinimalReviewThread(thread reviewThreadNode) MinimalReviewThread { func convertToMinimalReviewComment(c reviewCommentNode) MinimalReviewComment { m := MinimalReviewComment{ - Body: string(c.Body), + Body: sanitize.Sanitize(string(c.Body)), Path: string(c.Path), Author: string(c.Author.Login), HTMLURL: c.URL.String(), diff --git a/pkg/github/projects.go b/pkg/github/projects.go index dece52cd13..57a5dc145b 100644 --- a/pkg/github/projects.go +++ b/pkg/github/projects.go @@ -15,6 +15,7 @@ import ( ghErrors "github.com/github/github-mcp-server/pkg/errors" "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" @@ -265,7 +266,7 @@ func convertToMinimalStatusUpdate(node statusUpdateNode) MinimalProjectStatusUpd return MinimalProjectStatusUpdate{ ID: fmt.Sprintf("%v", node.ID), - Body: derefString(node.Body), + Body: sanitize.Sanitize(derefString(node.Body)), Status: derefString(node.Status), CreatedAt: node.CreatedAt.Time.Format(time.RFC3339), StartDate: derefString(node.StartDate), diff --git a/pkg/github/pullrequests.go b/pkg/github/pullrequests.go index 3907ebc0ec..c2ff773fbb 100644 --- a/pkg/github/pullrequests.go +++ b/pkg/github/pullrequests.go @@ -17,7 +17,6 @@ import ( "github.com/github/github-mcp-server/pkg/ifc" "github.com/github/github-mcp-server/pkg/inventory" "github.com/github/github-mcp-server/pkg/octicons" - "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" @@ -185,16 +184,6 @@ func GetPullRequest(ctx context.Context, client *github.Client, deps ToolDepende return ghErrors.NewGitHubAPIStatusErrorResponse(ctx, "failed to get pull request", resp, body), nil } - // sanitize title/body on response - if pr != nil { - if pr.Title != nil { - pr.Title = github.Ptr(sanitize.Sanitize(*pr.Title)) - } - if pr.Body != nil { - pr.Body = github.Ptr(sanitize.Sanitize(*pr.Body)) - } - } - if ff.LockdownMode { if restricted, err := authorLockdownResult(ctx, cache, owner, repo, pr.GetUser().GetLogin(), lockdownPullRequestRestrictedMessage); restricted != nil || err != nil { return restricted, err @@ -1464,19 +1453,6 @@ func ListPullRequests(t translations.TranslationHelperFunc) inventory.ServerTool return ghErrors.NewGitHubAPIStatusErrorResponse(ctx, "failed to list pull requests", resp, bodyBytes), nil, nil } - // sanitize title/body on each PR - for _, pr := range prs { - if pr == nil { - continue - } - if pr.Title != nil { - pr.Title = github.Ptr(sanitize.Sanitize(*pr.Title)) - } - if pr.Body != nil { - pr.Body = github.Ptr(sanitize.Sanitize(*pr.Body)) - } - } - minimalPRs := make([]MinimalPullRequest, 0, len(prs)) for _, pr := range prs { if pr != nil { diff --git a/pkg/github/sanitize_coverage_test.go b/pkg/github/sanitize_coverage_test.go new file mode 100644 index 0000000000..701a7d4f7c --- /dev/null +++ b/pkg/github/sanitize_coverage_test.go @@ -0,0 +1,294 @@ +package github + +import ( + "encoding/json" + "net/url" + "testing" + + "github.com/google/go-github/v89/github" + "github.com/shurcooL/githubv4" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// maliciousText contains an HTML payload plus invisible/hidden-instruction characters, +// mirroring the classes of untrusted content pkg/sanitize.Sanitize is meant to strip: +// disallowed HTML tags and zero-width/BiDi control characters that can hide instructions +// from a human reviewer while still being interpreted by a model. +const maliciousText = "Hello\u200BWorld" + +// sanitizedText is what maliciousText becomes after sanitize.Sanitize: the \n+\u200B\n" + + t.Run("commit file patch (get_commit)", func(t *testing.T) { + commit := convertToMinimalCommit(&github.RepositoryCommit{ + Files: []*github.CommitFile{{Filename: github.Ptr("a.go"), Patch: github.Ptr(patch)}}, + }, commitDetailFullPatch) + require.Len(t, commit.Files, 1) + assert.Equal(t, patch, commit.Files[0].Patch) + }) + + t.Run("pull request file patch (get_pull_request_files)", func(t *testing.T) { + files := convertToMinimalPRFiles([]*github.CommitFile{ + {Filename: github.Ptr("a.go"), Patch: github.Ptr(patch)}, + }) + require.Len(t, files, 1) + assert.Equal(t, patch, files[0].Patch) + }) +} + +// Test_Discussion_SanitizesUserAuthoredText covers the discussion helpers, which previously +// applied no sanitization at all to titles, bodies, or comments despite being user-authored, +// untrusted content equivalent to issue/PR text. +func Test_Discussion_SanitizesUserAuthoredText(t *testing.T) { + t.Run("discussion title (fragmentToDiscussion, used by list_discussions)", func(t *testing.T) { + discussion := fragmentToDiscussion(NodeFragment{Title: githubv4.String(maliciousText)}) + require.NotNil(t, discussion.Title) + assert.Equal(t, sanitizedText, *discussion.Title) + }) + + t.Run("discussion comment body (newMinimalDiscussionComment, used by get_discussion_comments)", func(t *testing.T) { + comment := newMinimalDiscussionComment("id", maliciousText, false) + assert.Equal(t, sanitizedText, comment.Body) + }) +} diff --git a/pkg/github/search_utils.go b/pkg/github/search_utils.go index dc800171aa..52d735ca0b 100644 --- a/pkg/github/search_utils.go +++ b/pkg/github/search_utils.go @@ -204,6 +204,12 @@ func searchHandler( return ghErrors.NewGitHubAPIStatusErrorResponse(ctx, errorPrefix, resp, body), nil } + // result.Issues are raw *github.Issue objects marshaled directly below rather than through + // a convertToMinimal* helper (see minimal_types.go), so Title/Body must be sanitized here. + for _, iss := range result.Issues { + sanitizeIssueTitleAndBody(iss) + } + filtered := false var payload any = result if len(cfg.fields) > 0 { From af11e05c7314176c1aa19e738f4ac3df4b2c005c Mon Sep 17 00:00:00 2001 From: Sam Morrow Date: Wed, 19 Aug 2026 14:52:06 +0200 Subject: [PATCH 2/2] Sanitize remaining issue-ref and blame headline response paths Route every MinimalIssueRef/MinimalPullRequestRef construction through shared constructors that sanitize the user-authored title, so issue_dependency_read, issue_dependency_write and find_duplicate no longer forward raw issue titles. Also sanitize the get_file_blame commit message headline, after truncation so the headline is still cut at the author's real first line break. Extends the sanitization regression suite with the project status update body, both ref constructors and the dependency ref, and adds tool-level regression tests for find_duplicate and get_file_blame. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- pkg/github/find_duplicate.go | 15 +++++---- pkg/github/find_duplicate_test.go | 46 ++++++++++++++++++++++++++ pkg/github/issue_dependencies.go | 17 +++++----- pkg/github/issues.go | 28 ++++++++-------- pkg/github/minimal_types.go | 27 ++++++++++++++++ pkg/github/repositories.go | 7 ++-- pkg/github/repositories_test.go | 48 ++++++++++++++++++++++++++++ pkg/github/sanitize_coverage_test.go | 30 +++++++++++++++++ 8 files changed, 188 insertions(+), 30 deletions(-) diff --git a/pkg/github/find_duplicate.go b/pkg/github/find_duplicate.go index 4831f15c5d..e32179ce8d 100644 --- a/pkg/github/find_duplicate.go +++ b/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, diff --git a/pkg/github/find_duplicate_test.go b/pkg/github/find_duplicate_test.go index 4384c92198..be5c3dda81 100644 --- a/pkg/github/find_duplicate_test.go +++ b/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, "