fix(sanitize): preserve code in sanitized bodies and cover remaining … · github/github-mcp-server@57d4875 · GitHub
Skip to content

Commit 57d4875

Browse files
IrynaKulakovaCopilot
IrynaKulakova
and
Copilot
committed
fix(sanitize): preserve code in sanitized bodies and cover remaining read paths
Full Sanitize runs bluemonday, which escapes entities and silently truncates a fenced code block at the first '<'. Comment and review bodies are the most code-dense fields the server returns, so applying it there corrupts content delivered to the model. Add sanitize.FilterBody (invisible characters + code fence metadata, no HTML filtering) and use it for issue comment, PR review, PR review comment and sub-issue bodies. Titles keep the full Sanitize treatment. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: b8dd0f23-2c6e-47c1-bad5-c1d0c9361f13
1 parent fe386b2 commit 57d4875

6 files changed

Lines changed: 190 additions & 5 deletions

File tree

‎pkg/github/issues.go‎

Lines changed: 1 addition & 1 deletion

‎pkg/github/issues_test.go‎

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4738,6 +4738,56 @@ func Test_GetSubIssues(t *testing.T) {
47384738
}
47394739
}
47404740

4741+
func Test_GetSubIssues_Sanitization(t *testing.T) {
4742+
serverTool := IssueRead(translations.NullTranslationHelper)
4743+
4744+
hiddenPayload := "Sub-issue\U000E0001\U000E0049\U000E0067\U000E006E\U000E006F\U000E0072\U000E0065"
4745+
bodyWithCode := "Repro:\n```go\nif a<b { fmt.Println(\"x\") }\n```"
4746+
4747+
mockedClient := MockHTTPClientWithHandlers(map[string]http.HandlerFunc{
4748+
GetReposIssuesSubIssuesByOwnerByRepoByIssueNumber: mockResponse(t, http.StatusOK, []*github.Issue{
4749+
{
4750+
Number: github.Ptr(123),
4751+
Title: github.Ptr(hiddenPayload),
4752+
Body: github.Ptr(hiddenPayload),
4753+
State: github.Ptr("open"),
4754+
},
4755+
{
4756+
Number: github.Ptr(124),
4757+
Title: github.Ptr("Sub-issue 2"),
4758+
Body: github.Ptr(bodyWithCode),
4759+
State: github.Ptr("open"),
4760+
},
4761+
}),
4762+
})
4763+
4764+
deps := BaseDeps{
4765+
Client: mustNewGHClient(t, mockedClient),
4766+
GQLClient: githubv4.NewClient(nil),
4767+
RepoAccessCache: stubRepoAccessCache(nil, 15*time.Minute),
4768+
Flags: stubFeatureFlags(map[string]bool{"lockdown-mode": false}),
4769+
}
4770+
handler := serverTool.Handler(deps)
4771+
4772+
request := createMCPRequest(map[string]any{
4773+
"method": "get_sub_issues",
4774+
"owner": "owner",
4775+
"repo": "repo",
4776+
"issue_number": float64(42),
4777+
})
4778+
4779+
result, err := handler(ContextWithDeps(context.Background(), deps), &request)
4780+
require.NoError(t, err)
4781+
4782+
var returnedSubIssues []*github.Issue
4783+
require.NoError(t, json.Unmarshal([]byte(getTextResult(t, result).Text), &returnedSubIssues))
4784+
require.Len(t, returnedSubIssues, 2)
4785+
4786+
assert.Equal(t, "Sub-issue", returnedSubIssues[0].GetTitle(), "hidden characters must be stripped from titles")
4787+
assert.Equal(t, "Sub-issue", returnedSubIssues[0].GetBody(), "hidden characters must be stripped from bodies")
4788+
assert.Equal(t, bodyWithCode, returnedSubIssues[1].GetBody(), "code content must survive sanitization")
4789+
}
4790+
47414791
func TestAddIssueComment(t *testing.T) {
47424792
t.Parallel()
47434793

‎pkg/github/minimal_types.go‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -645,7 +645,7 @@ func convertToMinimalPullRequestReview(review *github.PullRequestReview) Minimal
645645
m := MinimalPullRequestReview{
646646
ID: review.GetID(),
647647
State: review.GetState(),
648-
Body: review.GetBody(),
648+
Body: sanitize.FilterBody(review.GetBody()),
649649
HTMLURL: review.GetHTMLURL(),
650650
User: convertToMinimalUser(review.GetUser()),
651651
CommitID: review.GetCommitID(),
@@ -821,9 +821,9 @@ func convertToMinimalIssuesResponse(fragment IssueQueryFragment) MinimalIssuesRe
821821
func convertToMinimalIssueComment(comment *github.IssueComment) MinimalIssueComment {
822822
m := MinimalIssueComment{
823823
ID: comment.GetID(),
824-
// Bodies carry the same invisible-glyph / HTML injection surface as
824+
// Comment bodies carry the same invisible-glyph injection surface as
825825
// issue and PR bodies, which the read paths already sanitize.
826-
Body: sanitize.Sanitize(comment.GetBody()),
826+
Body: sanitize.FilterBody(comment.GetBody()),
827827
HTMLURL: comment.GetHTMLURL(),
828828
User: convertToMinimalUser(comment.GetUser()),
829829
AuthorAssociation: comment.GetAuthorAssociation(),
@@ -1911,7 +1911,7 @@ func convertToMinimalReviewThread(thread reviewThreadNode) MinimalReviewThread {
19111911

19121912
func convertToMinimalReviewComment(c reviewCommentNode) MinimalReviewComment {
19131913
m := MinimalReviewComment{
1914-
Body: string(c.Body),
1914+
Body: sanitize.FilterBody(string(c.Body)),
19151915
Path: string(c.Path),
19161916
Author: string(c.Author.Login),
19171917
HTMLURL: c.URL.String(),

‎pkg/github/minimal_types_test.go‎

Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,76 @@
1+
package github
2+
3+
import (
4+
"net/url"
5+
"testing"
6+
7+
"github.com/google/go-github/v89/github"
8+
"github.com/shurcooL/githubv4"
9+
"github.com/stretchr/testify/assert"
10+
"github.com/stretchr/testify/require"
11+
)
12+
13+
// bodyWithHiddenPayload embeds Unicode tag characters, which are invisible to a
14+
// human reviewer but legible to a model.
15+
const bodyWithHiddenPayload = "Looks good\U000E0001\U000E0049\U000E0067\U000E006E\U000E006F\U000E0072\U000E0065"
16+
17+
const bodyWithCode = "Compare with:\n```go\nif a<b { fmt.Println(\"x\") }\n```\nand <Foo/> in JSX."
18+
19+
func TestConvertToMinimalIssueCommentSanitizesBody(t *testing.T) {
20+
t.Run("strips hidden characters", func(t *testing.T) {
21+
m := convertToMinimalIssueComment(&github.IssueComment{
22+
ID: github.Ptr(int64(1)),
23+
Body: github.Ptr(bodyWithHiddenPayload),
24+
})
25+
assert.Equal(t, "Looks good", m.Body)
26+
})
27+
28+
t.Run("preserves code content", func(t *testing.T) {
29+
m := convertToMinimalIssueComment(&github.IssueComment{
30+
ID: github.Ptr(int64(1)),
31+
Body: github.Ptr(bodyWithCode),
32+
})
33+
assert.Equal(t, bodyWithCode, m.Body)
34+
})
35+
}
36+
37+
func TestConvertToMinimalPullRequestReviewSanitizesBody(t *testing.T) {
38+
t.Run("strips hidden characters", func(t *testing.T) {
39+
m := convertToMinimalPullRequestReview(&github.PullRequestReview{
40+
ID: github.Ptr(int64(1)),
41+
Body: github.Ptr(bodyWithHiddenPayload),
42+
})
43+
assert.Equal(t, "Looks good", m.Body)
44+
})
45+
46+
t.Run("preserves code content", func(t *testing.T) {
47+
m := convertToMinimalPullRequestReview(&github.PullRequestReview{
48+
ID: github.Ptr(int64(1)),
49+
Body: github.Ptr(bodyWithCode),
50+
})
51+
assert.Equal(t, bodyWithCode, m.Body)
52+
})
53+
}
54+
55+
func TestConvertToMinimalReviewCommentSanitizesBody(t *testing.T) {
56+
commentURL, err := url.Parse("https://github.com/owner/repo/pull/1#discussion_r1")
57+
require.NoError(t, err)
58+
59+
t.Run("strips hidden characters", func(t *testing.T) {
60+
m := convertToMinimalReviewComment(reviewCommentNode{
61+
Body: githubv4.String(bodyWithHiddenPayload),
62+
Path: githubv4.String("main.go"),
63+
URL: githubv4.URI{URL: commentURL},
64+
})
65+
assert.Equal(t, "Looks good", m.Body)
66+
})
67+
68+
t.Run("preserves code content", func(t *testing.T) {
69+
m := convertToMinimalReviewComment(reviewCommentNode{
70+
Body: githubv4.String(bodyWithCode),
71+
Path: githubv4.String("main.go"),
72+
URL: githubv4.URI{URL: commentURL},
73+
})
74+
assert.Equal(t, bodyWithCode, m.Body)
75+
})
76+
}

‎pkg/sanitize/sanitize.go‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,15 @@ func Sanitize(input string) string {
1515
return FilterHTMLTags(FilterCodeFenceMetadata(FilterInvisibleCharacters(input)))
1616
}
1717

18+
// FilterBody strips the injection surface that matters for markdown bodies —
19+
// invisible glyphs and hidden code-fence info strings — without running the
20+
// HTML filter. Bodies routinely contain code (generics, JSX, shell redirects),
21+
// and HTML filtering silently truncates a fenced block at the first '<', which
22+
// would corrupt the content delivered to the model.
23+
func FilterBody(input string) string {
24+
return FilterCodeFenceMetadata(FilterInvisibleCharacters(input))
25+
}
26+
1827
// FilterInvisibleCharacters removes invisible or control characters that should not appear
1928
// in user-facing titles or bodies. This includes:
2029
// - Unicode tag characters: U+E0001, U+E0020–U+E007F

‎pkg/sanitize/sanitize_test.go‎

Lines changed: 50 additions & 0 deletions

0 commit comments

Comments
 (0)