Skip to content

Commit 25f11e6

Browse files
fix(sanitize): preserve Markdown body fidelity on read surfaces (#3177)
* fix: preserve Markdown content in GitHub responses Route body-bearing response fields through the fidelity-preserving content path while retaining strict title handling and remove only unconditional invisible characters. Cover direct and converter read-modify-write surfaces for issues, releases, comments, discussions, projects, and commits. Refs #2202, #3165 Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 69c5ab30-9815-4c07-8385-11a206e68f66 * chore: regenerate license files Auto-generated by license-check workflow --------- Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Copilot-Session: 69c5ab30-9815-4c07-8385-11a206e68f66
1 parent 59847fb commit 25f11e6

15 files changed

Lines changed: 129 additions & 262 deletions

pkg/github/discussions.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -362,7 +362,7 @@ func GetDiscussion(t translations.TranslationHelperFunc) inventory.ServerTool {
362362
response := map[string]any{
363363
"number": int(d.Number),
364364
"title": sanitize.PlainText(string(d.Title)),
365-
"body": sanitize.Sanitize(string(d.Body)),
365+
"body": sanitize.Content(string(d.Body)),
366366
"url": string(d.URL),
367367
"closed": bool(d.Closed),
368368
"isAnswered": bool(d.IsAnswered),

pkg/github/discussions_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -571,7 +571,7 @@ func Test_GetDiscussion(t *testing.T) {
571571
expected: map[string]any{
572572
"number": float64(1),
573573
"title": sanitizedText,
574-
"body": sanitizedText,
574+
"body": sanitizedContentText,
575575
"url": "https://github.com/owner/repo/discussions/1",
576576
"closed": false,
577577
"isAnswered": false,

pkg/github/find_duplicate_test.go

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -166,7 +166,6 @@ func Test_FindDuplicate_SanitizesIssueTitle(t *testing.T) {
166166
require.NoError(t, json.Unmarshal([]byte(text.Text), &candidates))
167167
require.Len(t, candidates, 1)
168168
assert.Equal(t, sanitizedText, candidates[0].Issue.Title)
169-
assert.NotContains(t, text.Text, "<script>")
170169
}
171170

172171
func Test_FindDuplicate_OmitsUnsetParams(t *testing.T) {

pkg/github/issues.go

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1118,6 +1118,9 @@ func GetSubIssues(ctx context.Context, client *github.Client, deps ToolDependenc
11181118
subIssues = filteredSubIssues
11191119
}
11201120

1121+
for _, subIssue := range subIssues {
1122+
sanitizeSubIssueTitleAndBody(subIssue)
1123+
}
11211124
r, err := json.Marshal(subIssues)
11221125
if err != nil {
11231126
return nil, fmt.Errorf("failed to marshal response: %w", err)
@@ -1708,6 +1711,7 @@ func AddSubIssue(ctx context.Context, client *github.Client, owner string, repo
17081711
return ghErrors.NewGitHubAPIStatusErrorResponse(ctx, "failed to add sub-issue", resp, body), nil
17091712
}
17101713

1714+
sanitizeSubIssueTitleAndBody(subIssue)
17111715
r, err := json.Marshal(subIssue)
17121716
if err != nil {
17131717
return nil, fmt.Errorf("failed to marshal response: %w", err)
@@ -1739,6 +1743,7 @@ func RemoveSubIssue(ctx context.Context, client *github.Client, owner string, re
17391743
return ghErrors.NewGitHubAPIStatusErrorResponse(ctx, "failed to remove sub-issue", resp, body), nil
17401744
}
17411745

1746+
sanitizeSubIssueTitleAndBody(subIssue)
17421747
r, err := json.Marshal(subIssue)
17431748
if err != nil {
17441749
return nil, fmt.Errorf("failed to marshal response: %w", err)
@@ -1788,6 +1793,7 @@ func ReprioritizeSubIssue(ctx context.Context, client *github.Client, owner stri
17881793
return ghErrors.NewGitHubAPIStatusErrorResponse(ctx, "failed to reprioritize sub-issue", resp, body), nil
17891794
}
17901795

1796+
sanitizeSubIssueTitleAndBody(subIssue)
17911797
r, err := json.Marshal(subIssue)
17921798
if err != nil {
17931799
return nil, fmt.Errorf("failed to marshal response: %w", err)
@@ -1998,7 +2004,19 @@ func sanitizeIssueTitleAndBody(issue *github.Issue) {
19982004
issue.Title = github.Ptr(sanitize.PlainText(*issue.Title))
19992005
}
20002006
if issue.Body != nil {
2001-
issue.Body = github.Ptr(sanitize.Sanitize(*issue.Body))
2007+
issue.Body = github.Ptr(sanitize.Content(*issue.Body))
2008+
}
2009+
}
2010+
2011+
func sanitizeSubIssueTitleAndBody(issue *github.SubIssue) {
2012+
if issue == nil {
2013+
return
2014+
}
2015+
if issue.Title != nil {
2016+
issue.Title = github.Ptr(sanitize.Sanitize(*issue.Title))
2017+
}
2018+
if issue.Body != nil {
2019+
issue.Body = github.Ptr(sanitize.Content(*issue.Body))
20022020
}
20032021
}
20042022

pkg/github/issues_test.go

Lines changed: 21 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -5774,8 +5774,8 @@ func Test_AddSubIssue(t *testing.T) {
57745774
// Setup mock issue for success case (matches GitHub API response format)
57755775
mockIssue := &github.Issue{
57765776
Number: github.Ptr(42),
5777-
Title: github.Ptr("Parent Issue"),
5778-
Body: github.Ptr("This is the parent issue with a sub-issue"),
5777+
Title: github.Ptr("<int>\u200B"),
5778+
Body: github.Ptr("This is **Markdown**\u200B"),
57795779
State: github.Ptr("open"),
57805780
HTMLURL: github.Ptr("https://github.com/owner/repo/issues/42"),
57815781
User: &github.User{
@@ -5970,8 +5970,8 @@ func Test_AddSubIssue(t *testing.T) {
59705970
err = json.Unmarshal([]byte(textContent.Text), &returnedIssue)
59715971
require.NoError(t, err)
59725972
assert.Equal(t, *tc.expectedIssue.Number, *returnedIssue.Number)
5973-
assert.Equal(t, *tc.expectedIssue.Title, *returnedIssue.Title)
5974-
assert.Equal(t, *tc.expectedIssue.Body, *returnedIssue.Body)
5973+
assert.Empty(t, *returnedIssue.Title)
5974+
assert.Equal(t, "This is **Markdown**", *returnedIssue.Body)
59755975
assert.Equal(t, *tc.expectedIssue.State, *returnedIssue.State)
59765976
assert.Equal(t, *tc.expectedIssue.HTMLURL, *returnedIssue.HTMLURL)
59775977
assert.Equal(t, *tc.expectedIssue.User.Login, *returnedIssue.User.Login)
@@ -5999,8 +5999,8 @@ func Test_GetSubIssues(t *testing.T) {
59995999
mockSubIssues := []*github.Issue{
60006000
{
60016001
Number: github.Ptr(123),
6002-
Title: github.Ptr("Sub-issue 1"),
6003-
Body: github.Ptr("This is the first sub-issue"),
6002+
Title: github.Ptr("<int>\u200B"),
6003+
Body: github.Ptr("This is **Markdown**\u200B"),
60046004
State: github.Ptr("open"),
60056005
HTMLURL: github.Ptr("https://github.com/owner/repo/issues/123"),
60066006
User: &github.User{
@@ -6199,12 +6199,17 @@ func Test_GetSubIssues(t *testing.T) {
61996199
for i, subIssue := range returnedSubIssues {
62006200
if i < len(tc.expectedSubIssues) {
62016201
assert.Equal(t, *tc.expectedSubIssues[i].Number, *subIssue.Number)
6202-
assert.Equal(t, *tc.expectedSubIssues[i].Title, *subIssue.Title)
6202+
if i == 0 {
6203+
assert.Empty(t, *subIssue.Title)
6204+
assert.Equal(t, "This is **Markdown**", *subIssue.Body)
6205+
} else {
6206+
assert.Equal(t, *tc.expectedSubIssues[i].Title, *subIssue.Title)
6207+
}
62036208
assert.Equal(t, *tc.expectedSubIssues[i].State, *subIssue.State)
62046209
assert.Equal(t, *tc.expectedSubIssues[i].HTMLURL, *subIssue.HTMLURL)
62056210
assert.Equal(t, *tc.expectedSubIssues[i].User.Login, *subIssue.User.Login)
62066211

6207-
if tc.expectedSubIssues[i].Body != nil {
6212+
if i != 0 && tc.expectedSubIssues[i].Body != nil {
62086213
assert.Equal(t, *tc.expectedSubIssues[i].Body, *subIssue.Body)
62096214
}
62106215
}
@@ -6652,8 +6657,8 @@ func Test_RemoveSubIssue(t *testing.T) {
66526657
// Setup mock issue for success case (matches GitHub API response format - the updated parent issue)
66536658
mockIssue := &github.Issue{
66546659
Number: github.Ptr(42),
6655-
Title: github.Ptr("Parent Issue"),
6656-
Body: github.Ptr("This is the parent issue after sub-issue removal"),
6660+
Title: github.Ptr("<int>\u200B"),
6661+
Body: github.Ptr("This is **Markdown**\u200B"),
66576662
State: github.Ptr("open"),
66586663
HTMLURL: github.Ptr("https://github.com/owner/repo/issues/42"),
66596664
User: &github.User{
@@ -6831,8 +6836,8 @@ func Test_RemoveSubIssue(t *testing.T) {
68316836
err = json.Unmarshal([]byte(textContent.Text), &returnedIssue)
68326837
require.NoError(t, err)
68336838
assert.Equal(t, *tc.expectedIssue.Number, *returnedIssue.Number)
6834-
assert.Equal(t, *tc.expectedIssue.Title, *returnedIssue.Title)
6835-
assert.Equal(t, *tc.expectedIssue.Body, *returnedIssue.Body)
6839+
assert.Empty(t, *returnedIssue.Title)
6840+
assert.Equal(t, "This is **Markdown**", *returnedIssue.Body)
68366841
assert.Equal(t, *tc.expectedIssue.State, *returnedIssue.State)
68376842
assert.Equal(t, *tc.expectedIssue.HTMLURL, *returnedIssue.HTMLURL)
68386843
assert.Equal(t, *tc.expectedIssue.User.Login, *returnedIssue.User.Login)
@@ -6860,8 +6865,8 @@ func Test_ReprioritizeSubIssue(t *testing.T) {
68606865
// Setup mock issue for success case (matches GitHub API response format - the updated parent issue)
68616866
mockIssue := &github.Issue{
68626867
Number: github.Ptr(42),
6863-
Title: github.Ptr("Parent Issue"),
6864-
Body: github.Ptr("This is the parent issue with reprioritized sub-issues"),
6868+
Title: github.Ptr("<int>\u200B"),
6869+
Body: github.Ptr("This is **Markdown**\u200B"),
68656870
State: github.Ptr("open"),
68666871
HTMLURL: github.Ptr("https://github.com/owner/repo/issues/42"),
68676872
User: &github.User{
@@ -7091,8 +7096,8 @@ func Test_ReprioritizeSubIssue(t *testing.T) {
70917096
err = json.Unmarshal([]byte(textContent.Text), &returnedIssue)
70927097
require.NoError(t, err)
70937098
assert.Equal(t, *tc.expectedIssue.Number, *returnedIssue.Number)
7094-
assert.Equal(t, *tc.expectedIssue.Title, *returnedIssue.Title)
7095-
assert.Equal(t, *tc.expectedIssue.Body, *returnedIssue.Body)
7099+
assert.Empty(t, *returnedIssue.Title)
7100+
assert.Equal(t, "This is **Markdown**", *returnedIssue.Body)
70967101
assert.Equal(t, *tc.expectedIssue.State, *returnedIssue.State)
70977102
assert.Equal(t, *tc.expectedIssue.HTMLURL, *returnedIssue.HTMLURL)
70987103
assert.Equal(t, *tc.expectedIssue.User.Login, *returnedIssue.User.Login)

pkg/github/minimal_types.go

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -797,7 +797,7 @@ func convertToMinimalPullRequestReview(review *github.PullRequestReview) Minimal
797797
m := MinimalPullRequestReview{
798798
ID: review.GetID(),
799799
State: review.GetState(),
800-
Body: sanitize.Sanitize(review.GetBody()),
800+
Body: sanitize.Content(review.GetBody()),
801801
HTMLURL: review.GetHTMLURL(),
802802
User: convertToMinimalUser(review.GetUser()),
803803
CommitID: review.GetCommitID(),
@@ -815,7 +815,7 @@ func convertToMinimalIssue(issue *github.Issue) MinimalIssue {
815815
m := MinimalIssue{
816816
Number: issue.GetNumber(),
817817
Title: sanitize.PlainText(issue.GetTitle()),
818-
Body: sanitize.Sanitize(issue.GetBody()),
818+
Body: sanitize.Content(issue.GetBody()),
819819
State: issue.GetState(),
820820
StateReason: issue.GetStateReason(),
821821
Draft: issue.GetDraft(),
@@ -926,7 +926,7 @@ func fragmentWithoutFieldValuesToMinimalIssue(fragment issueFragmentWithoutField
926926
m := MinimalIssue{
927927
Number: int(fragment.Number),
928928
Title: sanitize.PlainText(string(fragment.Title)),
929-
Body: sanitize.Sanitize(string(fragment.Body)),
929+
Body: sanitize.Content(string(fragment.Body)),
930930
State: string(fragment.State),
931931
Comments: int(fragment.Comments.TotalCount),
932932
CreatedAt: fragment.CreatedAt.Format(time.RFC3339),
@@ -1085,7 +1085,7 @@ func convertToMinimalPullRequest(pr *github.PullRequest) MinimalPullRequest {
10851085
m := MinimalPullRequest{
10861086
Number: pr.GetNumber(),
10871087
Title: sanitize.PlainText(pr.GetTitle()),
1088-
Body: sanitize.Sanitize(pr.GetBody()),
1088+
Body: sanitize.Content(pr.GetBody()),
10891089
State: pr.GetState(),
10901090
Draft: pr.GetDraft(),
10911091
Merged: pr.GetMerged(),
@@ -2039,7 +2039,7 @@ func convertToMinimalRelease(release *github.RepositoryRelease) MinimalRelease {
20392039
ID: release.GetID(),
20402040
TagName: release.GetTagName(),
20412041
Name: sanitize.PlainText(release.GetName()),
2042-
Body: sanitize.Sanitize(release.GetBody()),
2042+
Body: sanitize.Content(release.GetBody()),
20432043
HTMLURL: release.GetHTMLURL(),
20442044
Prerelease: release.GetPrerelease(),
20452045
Draft: release.GetDraft(),

pkg/github/projects.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -266,7 +266,7 @@ func convertToMinimalStatusUpdate(node statusUpdateNode) MinimalProjectStatusUpd
266266

267267
return MinimalProjectStatusUpdate{
268268
ID: fmt.Sprintf("%v", node.ID),
269-
Body: sanitize.Sanitize(derefString(node.Body)),
269+
Body: sanitize.Content(derefString(node.Body)),
270270
Status: derefString(node.Status),
271271
CreatedAt: node.CreatedAt.Time.Format(time.RFC3339),
272272
StartDate: derefString(node.StartDate),

pkg/github/repositories.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2981,7 +2981,7 @@ func GetFileBlame(t translations.TranslationHelperFunc) inventory.ServerTool {
29812981
SHA: sha,
29822982
// Sanitized after truncation so the headline is cut at the author's real
29832983
// first line break rather than one introduced by sanitization.
2984-
MessageHeadline: sanitize.PlainText(headline),
2984+
MessageHeadline: sanitize.Content(headline),
29852985
CommittedDate: r.Commit.CommittedDate.Format("2006-01-02T15:04:05Z"),
29862986
Author: BlameAuthor{
29872987
Name: string(r.Commit.Author.Name),

pkg/github/repositories_test.go

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4855,6 +4855,7 @@ func Test_GetLatestRelease(t *testing.T) {
48554855
ID: 1,
48564856
TagName: "v1.0.0",
48574857
Name: github.Ptr("First Release"),
4858+
Body: github.Ptr("<details>Notes</details>\u200B"),
48584859
}
48594860

48604861
tests := []struct {
@@ -4922,6 +4923,8 @@ func Test_GetLatestRelease(t *testing.T) {
49224923
err = json.Unmarshal([]byte(textContent.Text), &returnedRelease)
49234924
require.NoError(t, err)
49244925
assert.Equal(t, tc.expectedResult.TagName, returnedRelease.TagName)
4926+
assert.Equal(t, "First Release", *returnedRelease.Name)
4927+
assert.Equal(t, "<details>Notes</details>", *returnedRelease.Body)
49254928
})
49264929
}
49274930
}
@@ -4945,7 +4948,7 @@ func Test_GetReleaseByTag(t *testing.T) {
49454948
ID: 1,
49464949
TagName: "v1.0.0",
49474950
Name: github.Ptr("Release v1.0.0"),
4948-
Body: github.Ptr("This is the first stable release."),
4951+
Body: github.Ptr("<details>Notes</details>\u200B"),
49494952
Assets: []*github.ReleaseAsset{
49504953
{
49514954
ID: github.Ptr(int64(1)),
@@ -5087,7 +5090,7 @@ func Test_GetReleaseByTag(t *testing.T) {
50875090
assert.Equal(t, tc.expectedResult.TagName, returnedRelease.TagName)
50885091
assert.Equal(t, *tc.expectedResult.Name, *returnedRelease.Name)
50895092
if tc.expectedResult.Body != nil {
5090-
assert.Equal(t, *tc.expectedResult.Body, *returnedRelease.Body)
5093+
assert.Equal(t, "<details>Notes</details>", *returnedRelease.Body)
50915094
}
50925095
if len(tc.expectedResult.Assets) > 0 {
50935096
require.Len(t, returnedRelease.Assets, len(tc.expectedResult.Assets))
@@ -6200,7 +6203,7 @@ func Test_GetFileBlame(t *testing.T) {
62006203
var br BlameResult
62016204
require.NoError(t, json.Unmarshal([]byte(result), &br))
62026205
require.Contains(t, br.Commits, "badc0ffee0000")
6203-
assert.Equal(t, sanitizedText, br.Commits["badc0ffee0000"].MessageHeadline)
6206+
assert.Equal(t, sanitizedContentText, br.Commits["badc0ffee0000"].MessageHeadline)
62046207
assert.NotContains(t, result, "<script>")
62056208
assert.NotContains(t, result, "Long body that should not appear")
62066209
},

0 commit comments

Comments
 (0)