Skip to content

Commit 3afa11b

Browse files
fix(sanitize): preserve visible markdown content
Separate short metadata sanitization from a Markdown-aware content policy. Keep code faithful, expose render-hidden constructs, and prevent sanitized read-modify-write cycles from silently deleting issue and pull request data. Refs #2202 Refs #3165 Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 69c5ab30-9815-4c07-8385-11a206e68f66
1 parent febc329 commit 3afa11b

15 files changed

Lines changed: 1398 additions & 72 deletions

go.mod

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ require (
1919
github.com/spf13/viper v1.21.0
2020
github.com/stretchr/testify v1.12.1
2121
github.com/yosida95/uritemplate/v3 v3.0.2
22+
github.com/yuin/goldmark v1.8.5
2223
golang.org/x/oauth2 v0.36.0
2324
)
2425

go.sum

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,8 @@ github.com/subosito/gotenv v1.6.0/go.mod h1:Dk4QP5c2W3ibzajGcXpNraDfq2IrhjMIvMSW
7575
github.com/yosida95/uritemplate/v3 v3.0.2 h1:Ed3Oyj9yrmi9087+NczuL5BwkIc4wvTb5zIM+UJPGz4=
7676
github.com/yosida95/uritemplate/v3 v3.0.2/go.mod h1:ILOh0sOhIJR3+L/8afwt/kE++YT040gmv5BQTMR2HP4=
7777
github.com/yuin/goldmark v1.4.13/go.mod h1:6yULJ656Px+3vBD8DxQVa3kxgyrAnzto9xy5taEt/CY=
78+
github.com/yuin/goldmark v1.8.5 h1:r6N5afV5qj/5S4UTch8agZHJ8UxNCMwX7WjkkJam2NA=
79+
github.com/yuin/goldmark v1.8.5/go.mod h1:ip/1k0VRfGynBgxOz0yCqHrbZXhcjxyuS66Brc7iBKg=
7880
go.yaml.in/yaml/v3 v3.0.4/go.mod h1:DhzuOOF2ATzADvBadXxruRBLzYTpT36CKvDb3+aBEFg=
7981
go.yaml.in/yaml/v3 v3.0.5 h1:N6y/pJk8buWs9NY5ERU2HSMfm+IuD/OtfdAnq6kESPw=
8082
go.yaml.in/yaml/v3 v3.0.5/go.mod h1:HVTZu1O7/Vkt2N+BFy8Zza+lnLsABggaTM2ZpNIGuKg=

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.Sanitize(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/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.Sanitize(*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: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5922,6 +5922,26 @@ func Test_GetSubIssues(t *testing.T) {
59225922
},
59235923
},
59245924
}
5925+
unsafeSubIssues := []*github.Issue{
5926+
{
5927+
Number: github.Ptr(125),
5928+
Title: github.Ptr(maliciousText),
5929+
Body: github.Ptr(maliciousText),
5930+
State: github.Ptr("open"),
5931+
HTMLURL: github.Ptr("https://github.com/owner/repo/issues/125"),
5932+
User: &github.User{Login: github.Ptr("user3")},
5933+
},
5934+
}
5935+
sanitizedSubIssues := []*github.Issue{
5936+
{
5937+
Number: github.Ptr(125),
5938+
Title: github.Ptr(sanitizedText),
5939+
Body: github.Ptr(sanitizedContentText),
5940+
State: github.Ptr("open"),
5941+
HTMLURL: github.Ptr("https://github.com/owner/repo/issues/125"),
5942+
User: &github.User{Login: github.Ptr("user3")},
5943+
},
5944+
}
59255945

59265946
tests := []struct {
59275947
name string
@@ -5966,6 +5986,19 @@ func Test_GetSubIssues(t *testing.T) {
59665986
expectError: false,
59675987
expectedSubIssues: mockSubIssues,
59685988
},
5989+
{
5990+
name: "sanitizes sub-issue titles and bodies",
5991+
mockedClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{
5992+
GetReposIssuesSubIssuesByOwnerByRepoByIssueNumber: mockResponse(t, http.StatusOK, unsafeSubIssues),
5993+
}),
5994+
requestArgs: map[string]any{
5995+
"method": "get_sub_issues",
5996+
"owner": "owner",
5997+
"repo": "repo",
5998+
"issue_number": float64(42),
5999+
},
6000+
expectedSubIssues: sanitizedSubIssues,
6001+
},
59696002
{
59706003
name: "successful sub-issues listing with empty result",
59716004
mockedClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{

pkg/github/minimal_types.go

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -204,7 +204,7 @@ type MinimalDiscussionComment struct {
204204
func newMinimalDiscussionComment(id string, body string, isAnswer bool) MinimalDiscussionComment {
205205
return MinimalDiscussionComment{
206206
ID: id,
207-
Body: sanitize.Sanitize(body),
207+
Body: sanitize.Content(body),
208208
IsAnswer: isAnswer,
209209
}
210210
}
@@ -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.Sanitize(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.Sanitize(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),
@@ -1015,7 +1015,7 @@ func convertToMinimalIssuesResponseWithoutFieldValues(fragment issueQueryFragmen
10151015
func convertToMinimalIssueComment(comment *github.IssueComment) MinimalIssueComment {
10161016
m := MinimalIssueComment{
10171017
ID: comment.GetID(),
1018-
Body: sanitize.Sanitize(comment.GetBody()),
1018+
Body: sanitize.Content(comment.GetBody()),
10191019
HTMLURL: comment.GetHTMLURL(),
10201020
User: convertToMinimalUser(comment.GetUser()),
10211021
AuthorAssociation: comment.GetAuthorAssociation(),
@@ -1064,7 +1064,7 @@ func convertToMinimalFileContentResponse(resp *github.RepositoryContentResponse)
10641064

10651065
m.Commit = &MinimalFileCommit{
10661066
SHA: resp.Commit.GetSHA(),
1067-
Message: sanitize.Sanitize(resp.Commit.GetMessage()),
1067+
Message: sanitize.Content(resp.Commit.GetMessage()),
10681068
HTMLURL: resp.Commit.GetHTMLURL(),
10691069
}
10701070

@@ -1085,7 +1085,7 @@ func convertToMinimalPullRequest(pr *github.PullRequest) MinimalPullRequest {
10851085
m := MinimalPullRequest{
10861086
Number: pr.GetNumber(),
10871087
Title: sanitize.Sanitize(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(),
@@ -1794,7 +1794,7 @@ func newMinimalCommitFromCore(sha, htmlURL string, commit *github.Commit, author
17941794

17951795
if commit != nil {
17961796
minimalCommit.Commit = &MinimalCommitInfo{
1797-
Message: sanitize.Sanitize(commit.GetMessage()),
1797+
Message: sanitize.Content(commit.GetMessage()),
17981798
}
17991799

18001800
if commit.Author != nil {
@@ -1997,7 +1997,7 @@ func convertToMinimalPullRequestCommits(commits []*github.RepositoryCommit) []Mi
19971997
}
19981998

19991999
if commit.Commit != nil {
2000-
minimalCommit.Message = sanitize.Sanitize(commit.Commit.GetMessage())
2000+
minimalCommit.Message = sanitize.Content(commit.Commit.GetMessage())
20012001
minimalCommit.Author = convertToMinimalCommitAuthor(commit.Commit.Author)
20022002
}
20032003

@@ -2036,7 +2036,7 @@ func convertToMinimalRelease(release *github.RepositoryRelease) MinimalRelease {
20362036
ID: release.GetID(),
20372037
TagName: release.GetTagName(),
20382038
Name: sanitize.Sanitize(release.GetName()),
2039-
Body: sanitize.Sanitize(release.GetBody()),
2039+
Body: sanitize.Content(release.GetBody()),
20402040
HTMLURL: release.GetHTMLURL(),
20412041
Prerelease: release.GetPrerelease(),
20422042
Draft: release.GetDraft(),
@@ -2092,7 +2092,7 @@ func convertToMinimalWorkflowRun(workflowRun *github.WorkflowRun) MinimalWorkflo
20922092

20932093
if headCommit := workflowRun.GetHeadCommit(); headCommit != nil && headCommit.GetMessage() != "" {
20942094
minimalRun.HeadCommit = &MinimalWorkflowRunHeadCommit{
2095-
Message: sanitize.Sanitize(headCommit.GetMessage()),
2095+
Message: sanitize.Content(headCommit.GetMessage()),
20962096
}
20972097
}
20982098

@@ -2277,7 +2277,7 @@ func convertToMinimalReviewThread(thread reviewThreadNode) MinimalReviewThread {
22772277

22782278
func convertToMinimalReviewComment(c reviewCommentNode) MinimalReviewComment {
22792279
m := MinimalReviewComment{
2280-
Body: sanitize.Sanitize(string(c.Body)),
2280+
Body: sanitize.Content(string(c.Body)),
22812281
Path: string(c.Path),
22822282
Author: string(c.Author.Login),
22832283
HTMLURL: c.URL.String(),

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),

0 commit comments

Comments
 (0)