From a783a357e428bd6fe57a91a6eb2c60c9471f3aec Mon Sep 17 00:00:00 2001 From: Sam Morrow Date: Thu, 3 Sep 2026 15:51:54 +0200 Subject: [PATCH] fix: restore sanitizer content boundaries Preserve Markdown and HTML in body and commit-message fields while stripping invisible controls. Apply plain-text sanitization consistently to raw release and sub-issue titles and restore blame headline handling. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- pkg/github/issues.go | 2 +- pkg/github/issues_test.go | 32 ++++++++++++++++---------------- pkg/github/minimal_types.go | 14 +++++++------- pkg/github/repositories.go | 16 +++++++++++++++- pkg/github/repositories_test.go | 18 +++++++++--------- 5 files changed, 48 insertions(+), 34 deletions(-) diff --git a/pkg/github/issues.go b/pkg/github/issues.go index 39682c0db7..cc8bc599a1 100644 --- a/pkg/github/issues.go +++ b/pkg/github/issues.go @@ -2013,7 +2013,7 @@ func sanitizeSubIssueTitleAndBody(issue *github.SubIssue) { return } if issue.Title != nil { - issue.Title = github.Ptr(sanitize.Sanitize(*issue.Title)) + issue.Title = github.Ptr(sanitize.PlainText(*issue.Title)) } if issue.Body != nil { issue.Body = github.Ptr(sanitize.Content(*issue.Body)) diff --git a/pkg/github/issues_test.go b/pkg/github/issues_test.go index 321fa09ffc..83bedc5b54 100644 --- a/pkg/github/issues_test.go +++ b/pkg/github/issues_test.go @@ -5774,8 +5774,8 @@ func Test_AddSubIssue(t *testing.T) { // Setup mock issue for success case (matches GitHub API response format) mockIssue := &github.Issue{ Number: github.Ptr(42), - Title: github.Ptr("\u200B"), - Body: github.Ptr("This is **Markdown**\u200B"), + Title: github.Ptr("can't \"quote\" AT&T\u200B"), + Body: github.Ptr("This is **Markdown**\u200B"), State: github.Ptr("open"), HTMLURL: github.Ptr("https://github.com/owner/repo/issues/42"), User: &github.User{ @@ -5970,8 +5970,8 @@ func Test_AddSubIssue(t *testing.T) { err = json.Unmarshal([]byte(textContent.Text), &returnedIssue) require.NoError(t, err) assert.Equal(t, *tc.expectedIssue.Number, *returnedIssue.Number) - assert.Empty(t, *returnedIssue.Title) - assert.Equal(t, "This is **Markdown**", *returnedIssue.Body) + assert.Equal(t, "can't \"quote\" AT&T", *returnedIssue.Title) + assert.Equal(t, "This is **Markdown**", *returnedIssue.Body) assert.Equal(t, *tc.expectedIssue.State, *returnedIssue.State) assert.Equal(t, *tc.expectedIssue.HTMLURL, *returnedIssue.HTMLURL) assert.Equal(t, *tc.expectedIssue.User.Login, *returnedIssue.User.Login) @@ -5999,8 +5999,8 @@ func Test_GetSubIssues(t *testing.T) { mockSubIssues := []*github.Issue{ { Number: github.Ptr(123), - Title: github.Ptr("\u200B"), - Body: github.Ptr("This is **Markdown**\u200B"), + Title: github.Ptr("can't \"quote\" AT&T\u200B"), + Body: github.Ptr("This is **Markdown**\u200B"), State: github.Ptr("open"), HTMLURL: github.Ptr("https://github.com/owner/repo/issues/123"), User: &github.User{ @@ -6200,8 +6200,8 @@ func Test_GetSubIssues(t *testing.T) { if i < len(tc.expectedSubIssues) { assert.Equal(t, *tc.expectedSubIssues[i].Number, *subIssue.Number) if i == 0 { - assert.Empty(t, *subIssue.Title) - assert.Equal(t, "This is **Markdown**", *subIssue.Body) + assert.Equal(t, "can't \"quote\" AT&T", *subIssue.Title) + assert.Equal(t, "This is **Markdown**", *subIssue.Body) } else { assert.Equal(t, *tc.expectedSubIssues[i].Title, *subIssue.Title) } @@ -6657,8 +6657,8 @@ func Test_RemoveSubIssue(t *testing.T) { // Setup mock issue for success case (matches GitHub API response format - the updated parent issue) mockIssue := &github.Issue{ Number: github.Ptr(42), - Title: github.Ptr("\u200B"), - Body: github.Ptr("This is **Markdown**\u200B"), + Title: github.Ptr("can't \"quote\" AT&T\u200B"), + Body: github.Ptr("This is **Markdown**\u200B"), State: github.Ptr("open"), HTMLURL: github.Ptr("https://github.com/owner/repo/issues/42"), User: &github.User{ @@ -6836,8 +6836,8 @@ func Test_RemoveSubIssue(t *testing.T) { err = json.Unmarshal([]byte(textContent.Text), &returnedIssue) require.NoError(t, err) assert.Equal(t, *tc.expectedIssue.Number, *returnedIssue.Number) - assert.Empty(t, *returnedIssue.Title) - assert.Equal(t, "This is **Markdown**", *returnedIssue.Body) + assert.Equal(t, "can't \"quote\" AT&T", *returnedIssue.Title) + assert.Equal(t, "This is **Markdown**", *returnedIssue.Body) assert.Equal(t, *tc.expectedIssue.State, *returnedIssue.State) assert.Equal(t, *tc.expectedIssue.HTMLURL, *returnedIssue.HTMLURL) assert.Equal(t, *tc.expectedIssue.User.Login, *returnedIssue.User.Login) @@ -6865,8 +6865,8 @@ func Test_ReprioritizeSubIssue(t *testing.T) { // Setup mock issue for success case (matches GitHub API response format - the updated parent issue) mockIssue := &github.Issue{ Number: github.Ptr(42), - Title: github.Ptr("\u200B"), - Body: github.Ptr("This is **Markdown**\u200B"), + Title: github.Ptr("can't \"quote\" AT&T\u200B"), + Body: github.Ptr("This is **Markdown**\u200B"), State: github.Ptr("open"), HTMLURL: github.Ptr("https://github.com/owner/repo/issues/42"), User: &github.User{ @@ -7096,8 +7096,8 @@ func Test_ReprioritizeSubIssue(t *testing.T) { err = json.Unmarshal([]byte(textContent.Text), &returnedIssue) require.NoError(t, err) assert.Equal(t, *tc.expectedIssue.Number, *returnedIssue.Number) - assert.Empty(t, *returnedIssue.Title) - assert.Equal(t, "This is **Markdown**", *returnedIssue.Body) + assert.Equal(t, "can't \"quote\" AT&T", *returnedIssue.Title) + assert.Equal(t, "This is **Markdown**", *returnedIssue.Body) assert.Equal(t, *tc.expectedIssue.State, *returnedIssue.State) assert.Equal(t, *tc.expectedIssue.HTMLURL, *returnedIssue.HTMLURL) assert.Equal(t, *tc.expectedIssue.User.Login, *returnedIssue.User.Login) diff --git a/pkg/github/minimal_types.go b/pkg/github/minimal_types.go index 3e72e3631f..2eba9a1628 100644 --- a/pkg/github/minimal_types.go +++ b/pkg/github/minimal_types.go @@ -204,7 +204,7 @@ type MinimalDiscussionComment struct { func newMinimalDiscussionComment(id string, body string, isAnswer bool) MinimalDiscussionComment { return MinimalDiscussionComment{ ID: id, - Body: sanitize.Sanitize(body), + Body: sanitize.Content(body), IsAnswer: isAnswer, } } @@ -1015,7 +1015,7 @@ func convertToMinimalIssuesResponseWithoutFieldValues(fragment issueQueryFragmen func convertToMinimalIssueComment(comment *github.IssueComment) MinimalIssueComment { m := MinimalIssueComment{ ID: comment.GetID(), - Body: sanitize.Sanitize(comment.GetBody()), + Body: sanitize.Content(comment.GetBody()), HTMLURL: comment.GetHTMLURL(), User: convertToMinimalUser(comment.GetUser()), AuthorAssociation: comment.GetAuthorAssociation(), @@ -1064,7 +1064,7 @@ func convertToMinimalFileContentResponse(resp *github.RepositoryContentResponse) m.Commit = &MinimalFileCommit{ SHA: resp.Commit.GetSHA(), - Message: sanitize.Sanitize(resp.Commit.GetMessage()), + Message: sanitize.Content(resp.Commit.GetMessage()), HTMLURL: resp.Commit.GetHTMLURL(), } @@ -1794,7 +1794,7 @@ func newMinimalCommitFromCore(sha, htmlURL string, commit *github.Commit, author if commit != nil { minimalCommit.Commit = &MinimalCommitInfo{ - Message: sanitize.Sanitize(commit.GetMessage()), + Message: sanitize.Content(commit.GetMessage()), } if commit.Author != nil { @@ -2000,7 +2000,7 @@ func convertToMinimalPullRequestCommits(commits []*github.RepositoryCommit) []Mi } if commit.Commit != nil { - minimalCommit.Message = sanitize.Sanitize(commit.Commit.GetMessage()) + minimalCommit.Message = sanitize.Content(commit.Commit.GetMessage()) minimalCommit.Author = convertToMinimalCommitAuthor(commit.Commit.Author) } @@ -2095,7 +2095,7 @@ func convertToMinimalWorkflowRun(workflowRun *github.WorkflowRun) MinimalWorkflo if headCommit := workflowRun.GetHeadCommit(); headCommit != nil && headCommit.GetMessage() != "" { minimalRun.HeadCommit = &MinimalWorkflowRunHeadCommit{ - Message: sanitize.Sanitize(headCommit.GetMessage()), + Message: sanitize.Content(headCommit.GetMessage()), } } @@ -2280,7 +2280,7 @@ func convertToMinimalReviewThread(thread reviewThreadNode) MinimalReviewThread { func convertToMinimalReviewComment(c reviewCommentNode) MinimalReviewComment { m := MinimalReviewComment{ - Body: sanitize.Sanitize(string(c.Body)), + Body: sanitize.Content(string(c.Body)), Path: string(c.Path), Author: string(c.Author.Login), HTMLURL: c.URL.String(), diff --git a/pkg/github/repositories.go b/pkg/github/repositories.go index f98050c8a4..8e2dd25172 100644 --- a/pkg/github/repositories.go +++ b/pkg/github/repositories.go @@ -2233,6 +2233,7 @@ func GetLatestRelease(t translations.TranslationHelperFunc) inventory.ServerTool return ghErrors.NewGitHubAPIStatusErrorResponse(ctx, "failed to get latest release", resp, body), nil, nil } + sanitizeReleaseNameAndBody(release) r, err := json.Marshal(release) if err != nil { return nil, nil, fmt.Errorf("failed to marshal response: %w", err) @@ -2319,6 +2320,7 @@ func GetReleaseByTag(t translations.TranslationHelperFunc) inventory.ServerTool return ghErrors.NewGitHubAPIStatusErrorResponse(ctx, "failed to get release by tag", resp, body), nil, nil } + sanitizeReleaseNameAndBody(release) r, err := json.Marshal(release) if err != nil { return nil, nil, fmt.Errorf("failed to marshal response: %w", err) @@ -2338,6 +2340,18 @@ func GetReleaseByTag(t translations.TranslationHelperFunc) inventory.ServerTool ) } +func sanitizeReleaseNameAndBody(release *github.RepositoryRelease) { + if release == nil { + return + } + if release.Name != nil { + release.Name = github.Ptr(sanitize.PlainText(*release.Name)) + } + if release.Body != nil { + release.Body = github.Ptr(sanitize.Content(*release.Body)) + } +} + // ListStarredRepositories creates a tool to list starred repositories for the authenticated user or a specified user. func ListStarredRepositories(t translations.TranslationHelperFunc) inventory.ServerTool { return NewTool( @@ -2981,7 +2995,7 @@ func GetFileBlame(t translations.TranslationHelperFunc) inventory.ServerTool { SHA: sha, // Sanitized after truncation so the headline is cut at the author's real // first line break rather than one introduced by sanitization. - MessageHeadline: sanitize.Content(headline), + MessageHeadline: sanitize.PlainText(headline), CommittedDate: r.Commit.CommittedDate.Format("2006-01-02T15:04:05Z"), Author: BlameAuthor{ Name: string(r.Commit.Author.Name), diff --git a/pkg/github/repositories_test.go b/pkg/github/repositories_test.go index 601702306a..8194895afa 100644 --- a/pkg/github/repositories_test.go +++ b/pkg/github/repositories_test.go @@ -4854,8 +4854,8 @@ func Test_GetLatestRelease(t *testing.T) { mockRelease := &github.RepositoryRelease{ ID: 1, TagName: "v1.0.0", - Name: github.Ptr("First Release"), - Body: github.Ptr("
Notes
\u200B"), + Name: github.Ptr("can't \"quote\" AT&T\u200B"), + Body: github.Ptr("
Notes
\u200B"), } tests := []struct { @@ -4923,8 +4923,8 @@ func Test_GetLatestRelease(t *testing.T) { err = json.Unmarshal([]byte(textContent.Text), &returnedRelease) require.NoError(t, err) assert.Equal(t, tc.expectedResult.TagName, returnedRelease.TagName) - assert.Equal(t, "First Release", *returnedRelease.Name) - assert.Equal(t, "
Notes
", *returnedRelease.Body) + assert.Equal(t, "can't \"quote\" AT&T", *returnedRelease.Name) + assert.Equal(t, "
Notes
", *returnedRelease.Body) }) } } @@ -4947,8 +4947,8 @@ func Test_GetReleaseByTag(t *testing.T) { mockRelease := &github.RepositoryRelease{ ID: 1, TagName: "v1.0.0", - Name: github.Ptr("Release v1.0.0"), - Body: github.Ptr("
Notes
\u200B"), + Name: github.Ptr("can't \"quote\" AT&T\u200B"), + Body: github.Ptr("
Notes
\u200B"), Assets: []*github.ReleaseAsset{ { ID: github.Ptr(int64(1)), @@ -5088,9 +5088,9 @@ func Test_GetReleaseByTag(t *testing.T) { assert.Equal(t, tc.expectedResult.ID, returnedRelease.ID) assert.Equal(t, tc.expectedResult.TagName, returnedRelease.TagName) - assert.Equal(t, *tc.expectedResult.Name, *returnedRelease.Name) + assert.Equal(t, "can't \"quote\" AT&T", *returnedRelease.Name) if tc.expectedResult.Body != nil { - assert.Equal(t, "
Notes
", *returnedRelease.Body) + assert.Equal(t, "
Notes
", *returnedRelease.Body) } if len(tc.expectedResult.Assets) > 0 { require.Len(t, returnedRelease.Assets, len(tc.expectedResult.Assets)) @@ -6203,7 +6203,7 @@ func Test_GetFileBlame(t *testing.T) { var br BlameResult require.NoError(t, json.Unmarshal([]byte(result), &br)) require.Contains(t, br.Commits, "badc0ffee0000") - assert.Equal(t, sanitizedContentText, br.Commits["badc0ffee0000"].MessageHeadline) + assert.Equal(t, sanitizedText, br.Commits["badc0ffee0000"].MessageHeadline) assert.NotContains(t, result, "