Skip to content

Commit db9b4b7

Browse files
committed
fix: preserve visible characters in sanitized titles
HTML sanitization was encoding apostrophes and other punctuation as entities in title fields. Route title-like metadata through a Title helper that restores that visible text after the strict policy runs.
1 parent bd47e63 commit db9b4b7

8 files changed

Lines changed: 262 additions & 17 deletions

File tree

‎pkg/errors/error.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -257,7 +257,7 @@ func formatGitHubValidationDetail(validationErr github.Error) string {
257257

258258
func sanitizeGitHubValidationText(value string) string {
259259
// Tool errors are plain text; keep quoted branch patterns readable.
260-
sanitized := strings.ReplaceAll(sanitize.Sanitize(value), "'", "'")
260+
sanitized := sanitize.Title(value)
261261
return strings.Join(strings.Fields(sanitized), " ")
262262
}
263263

‎pkg/github/discussions.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -100,7 +100,7 @@ type WithCategoryNoOrder struct {
100100
func fragmentToDiscussion(fragment NodeFragment) *github.Discussion {
101101
return &github.Discussion{
102102
Number: github.Ptr(int(fragment.Number)),
103-
Title: github.Ptr(sanitize.Sanitize(string(fragment.Title))),
103+
Title: github.Ptr(sanitize.Title(string(fragment.Title))),
104104
HTMLURL: github.Ptr(string(fragment.URL)),
105105
CreatedAt: &github.Timestamp{Time: fragment.CreatedAt.Time},
106106
UpdatedAt: &github.Timestamp{Time: fragment.UpdatedAt.Time},
@@ -361,7 +361,7 @@ func GetDiscussion(t translations.TranslationHelperFunc) inventory.ServerTool {
361361
// like ListDiscussions and GetDiscussionComments).
362362
response := map[string]any{
363363
"number": int(d.Number),
364-
"title": sanitize.Sanitize(string(d.Title)),
364+
"title": sanitize.Title(string(d.Title)),
365365
"body": sanitize.Sanitize(string(d.Body)),
366366
"url": string(d.URL),
367367
"closed": bool(d.Closed),

‎pkg/github/issues.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1194,7 +1194,7 @@ func GetIssueParent(ctx context.Context, client *githubv4.Client, deps ToolDepen
11941194
return MarshalledTextResult(map[string]any{
11951195
"parent": map[string]any{
11961196
"number": int(parent.Number),
1197-
"title": sanitize.Sanitize(string(parent.Title)),
1197+
"title": sanitize.Title(string(parent.Title)),
11981198
"state": string(parent.State),
11991199
"url": string(parent.URL),
12001200
"repository": string(parent.Repository.NameWithOwner),
@@ -1995,7 +1995,7 @@ func sanitizeIssueTitleAndBody(issue *github.Issue) {
19951995
return
19961996
}
19971997
if issue.Title != nil {
1998-
issue.Title = github.Ptr(sanitize.Sanitize(*issue.Title))
1998+
issue.Title = github.Ptr(sanitize.Title(*issue.Title))
19991999
}
20002000
if issue.Body != nil {
20012001
issue.Body = github.Ptr(sanitize.Sanitize(*issue.Body))

‎pkg/github/minimal_types.go‎

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -622,7 +622,7 @@ type MinimalPullRequestRef struct {
622622
func newMinimalPullRequestRef(number int, title, state, url, repository string) MinimalPullRequestRef {
623623
return MinimalPullRequestRef{
624624
Number: number,
625-
Title: sanitize.Sanitize(title),
625+
Title: sanitize.Title(title),
626626
State: state,
627627
URL: url,
628628
Repository: repository,
@@ -646,7 +646,7 @@ type MinimalIssueRef struct {
646646
func newMinimalIssueRef(number int, title, state, url, repository string) MinimalIssueRef {
647647
return MinimalIssueRef{
648648
Number: number,
649-
Title: sanitize.Sanitize(title),
649+
Title: sanitize.Title(title),
650650
State: state,
651651
URL: url,
652652
Repository: repository,
@@ -814,7 +814,7 @@ func convertToMinimalPullRequestReview(review *github.PullRequestReview) Minimal
814814
func convertToMinimalIssue(issue *github.Issue) MinimalIssue {
815815
m := MinimalIssue{
816816
Number: issue.GetNumber(),
817-
Title: sanitize.Sanitize(issue.GetTitle()),
817+
Title: sanitize.Title(issue.GetTitle()),
818818
Body: sanitize.Sanitize(issue.GetBody()),
819819
State: issue.GetState(),
820820
StateReason: issue.GetStateReason(),
@@ -925,7 +925,7 @@ func fragmentToMinimalIssue(fragment IssueFragment) MinimalIssue {
925925
func fragmentWithoutFieldValuesToMinimalIssue(fragment issueFragmentWithoutFieldValues) MinimalIssue {
926926
m := MinimalIssue{
927927
Number: int(fragment.Number),
928-
Title: sanitize.Sanitize(string(fragment.Title)),
928+
Title: sanitize.Title(string(fragment.Title)),
929929
Body: sanitize.Sanitize(string(fragment.Body)),
930930
State: string(fragment.State),
931931
Comments: int(fragment.Comments.TotalCount),
@@ -1084,7 +1084,7 @@ func convertToMinimalFileContentResponse(resp *github.RepositoryContentResponse)
10841084
func convertToMinimalPullRequest(pr *github.PullRequest) MinimalPullRequest {
10851085
m := MinimalPullRequest{
10861086
Number: pr.GetNumber(),
1087-
Title: sanitize.Sanitize(pr.GetTitle()),
1087+
Title: sanitize.Title(pr.GetTitle()),
10881088
Body: sanitize.Sanitize(pr.GetBody()),
10891089
State: pr.GetState(),
10901090
Draft: pr.GetDraft(),
@@ -1279,7 +1279,7 @@ func convertIssueToMinimalProjectItemContent(issue *github.Issue) *MinimalProjec
12791279
ID: issue.GetID(),
12801280
NodeID: issue.GetNodeID(),
12811281
Number: issue.GetNumber(),
1282-
Title: sanitize.Sanitize(issue.GetTitle()),
1282+
Title: sanitize.Title(issue.GetTitle()),
12831283
State: issue.GetState(),
12841284
StateReason: issue.GetStateReason(),
12851285
HTMLURL: issue.GetHTMLURL(),
@@ -1316,7 +1316,7 @@ func convertPullRequestToMinimalProjectItemContent(pr *github.PullRequest) *Mini
13161316
ID: pr.GetID(),
13171317
NodeID: pr.GetNodeID(),
13181318
Number: pr.GetNumber(),
1319-
Title: sanitize.Sanitize(pr.GetTitle()),
1319+
Title: sanitize.Title(pr.GetTitle()),
13201320
State: pr.GetState(),
13211321
HTMLURL: pr.GetHTMLURL(),
13221322
Repository: pullRequestRepositoryFullName(pr),
@@ -1353,7 +1353,7 @@ func convertDraftIssueToMinimalProjectItemContent(draftIssue *github.ProjectV2Dr
13531353
m := &MinimalProjectItemContent{
13541354
ID: draftIssue.GetID(),
13551355
NodeID: draftIssue.GetNodeID(),
1356-
Title: sanitize.Sanitize(draftIssue.GetTitle()),
1356+
Title: sanitize.Title(draftIssue.GetTitle()),
13571357
CreatedAt: formatProjectTimestamp(draftIssue.CreatedAt),
13581358
UpdatedAt: formatProjectTimestamp(draftIssue.UpdatedAt),
13591359
}
@@ -1612,7 +1612,7 @@ func minimalProjectPullRequestRefFromPullRequest(pr *github.PullRequest) minimal
16121612
}
16131613
return minimalProjectPullRequestRef{
16141614
Number: pr.GetNumber(),
1615-
Title: sanitize.Sanitize(pr.GetTitle()),
1615+
Title: sanitize.Title(pr.GetTitle()),
16161616
State: pr.GetState(),
16171617
HTMLURL: pr.GetHTMLURL(),
16181618
Repository: pullRequestRepositoryFullName(pr),
@@ -1634,7 +1634,7 @@ func minimalProjectPullRequestRefFromMap(value map[string]any) minimalProjectPul
16341634

16351635
return minimalProjectPullRequestRef{
16361636
Number: intFromAny(value["number"]),
1637-
Title: sanitize.Sanitize(stringFromMap(value, "title")),
1637+
Title: sanitize.Title(stringFromMap(value, "title")),
16381638
State: stringFromMap(value, "state"),
16391639
HTMLURL: htmlURL,
16401640
Repository: repository,
@@ -2038,7 +2038,7 @@ func convertToMinimalRelease(release *github.RepositoryRelease) MinimalRelease {
20382038
m := MinimalRelease{
20392039
ID: release.GetID(),
20402040
TagName: release.GetTagName(),
2041-
Name: sanitize.Sanitize(release.GetName()),
2041+
Name: sanitize.Title(release.GetName()),
20422042
Body: sanitize.Sanitize(release.GetBody()),
20432043
HTMLURL: release.GetHTMLURL(),
20442044
Prerelease: release.GetPrerelease(),

‎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.Sanitize(headline),
2984+
MessageHeadline: sanitize.Title(headline),
29852985
CommittedDate: r.Commit.CommittedDate.Format("2006-01-02T15:04:05Z"),
29862986
Author: BlameAuthor{
29872987
Name: string(r.Commit.Author.Name),

‎pkg/github/sanitize_coverage_test.go‎

Lines changed: 105 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -257,6 +257,111 @@ func Test_SearchIssueResult_SanitizesTitleAndBody(t *testing.T) {
257257
assert.Equal(t, sanitizedText, decoded.Body)
258258
}
259259

260+
func Test_MinimalConverters_TitlePreservesVisibleText(t *testing.T) {
261+
title := "[bug] can't add a connection to toolkits in desktop app"
262+
263+
tests := []struct {
264+
name string
265+
got func() string
266+
}{
267+
{
268+
name: "issue title (REST)",
269+
got: func() string {
270+
return convertToMinimalIssue(&github.Issue{Title: github.Ptr(title)}).Title
271+
},
272+
},
273+
{
274+
name: "issue title (GraphQL)",
275+
got: func() string {
276+
return fragmentWithoutFieldValuesToMinimalIssue(issueFragmentWithoutFieldValues{
277+
Title: githubv4.String(title),
278+
}).Title
279+
},
280+
},
281+
{
282+
name: "pull request title",
283+
got: func() string {
284+
return convertToMinimalPullRequest(&github.PullRequest{Title: github.Ptr(title)}).Title
285+
},
286+
},
287+
{
288+
name: "release name",
289+
got: func() string {
290+
return convertToMinimalRelease(&github.RepositoryRelease{Name: github.Ptr(title)}).Name
291+
},
292+
},
293+
{
294+
name: "project item content title (issue)",
295+
got: func() string {
296+
return convertIssueToMinimalProjectItemContent(&github.Issue{Title: github.Ptr(title)}).Title
297+
},
298+
},
299+
{
300+
name: "project item content title (pull request)",
301+
got: func() string {
302+
return convertPullRequestToMinimalProjectItemContent(&github.PullRequest{Title: github.Ptr(title)}).Title
303+
},
304+
},
305+
{
306+
name: "project item content title (draft issue)",
307+
got: func() string {
308+
return convertDraftIssueToMinimalProjectItemContent(&github.ProjectV2DraftIssue{Title: github.Ptr(title)}).Title
309+
},
310+
},
311+
{
312+
name: "project pull request ref title (from *github.PullRequest)",
313+
got: func() string {
314+
return minimalProjectPullRequestRefFromPullRequest(&github.PullRequest{Title: github.Ptr(title)}).Title
315+
},
316+
},
317+
{
318+
name: "project pull request ref title (from map)",
319+
got: func() string {
320+
return minimalProjectPullRequestRefFromMap(map[string]any{"title": title}).Title
321+
},
322+
},
323+
{
324+
name: "issue ref title (shared constructor)",
325+
got: func() string {
326+
return newMinimalIssueRef(1, title, "OPEN", "https://github.com/o/r/issues/1", "o/r").Title
327+
},
328+
},
329+
{
330+
name: "pull request ref title (shared constructor)",
331+
got: func() string {
332+
return newMinimalPullRequestRef(1, title, "OPEN", "https://github.com/o/r/pull/1", "o/r").Title
333+
},
334+
},
335+
{
336+
name: "issue dependency ref title",
337+
got: func() string {
338+
return issueToDependencyRef(&github.Issue{Title: github.Ptr(title)}).Title
339+
},
340+
},
341+
{
342+
name: "discussion title",
343+
got: func() string {
344+
discussion := fragmentToDiscussion(NodeFragment{Title: githubv4.String(title)})
345+
return discussion.GetTitle()
346+
},
347+
},
348+
{
349+
name: "search issue result title",
350+
got: func() string {
351+
issue := &github.Issue{Title: github.Ptr(title)}
352+
sanitizeIssueTitleAndBody(issue)
353+
return issue.GetTitle()
354+
},
355+
},
356+
}
357+
358+
for _, tt := range tests {
359+
t.Run(tt.name, func(t *testing.T) {
360+
assert.Equal(t, title, tt.got())
361+
})
362+
}
363+
}
364+
260365
// Test_SanitizeIssueTitleAndBody exercises the shared helper directly, including its nil-safety,
261366
// since it backs both search_issues and search_pull_requests.
262367
func Test_SanitizeIssueTitleAndBody(t *testing.T) {

‎pkg/sanitize/sanitize.go‎

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,45 @@ func Sanitize(input string) string {
3636
return FilterCodeFenceMetadata(FilterInvisibleCharacters(normalized))
3737
}
3838

39+
// Title sanitizes short metadata fields such as issue and pull request titles.
40+
// It applies the same HTML and invisible-character policy as Sanitize, then
41+
// restores the punctuation that policy HTML-escapes so visible characters
42+
// remain as themselves (for example, "can't" instead of "can't").
43+
//
44+
// Angle brackets stay escaped. Decoding < / > would reconstitute markup
45+
// from entity-encoded tags, including nested < payloads.
46+
func Title(input string) string {
47+
return restoreVisiblePunctuation(Sanitize(input))
48+
}
49+
50+
// visiblePunctuationUnescaper inverts html.EscapeString for apostrophe, quote,
51+
// and ampersand only. It must not include < or >.
52+
var visiblePunctuationUnescaper = strings.NewReplacer(
53+
"'", "'",
54+
""", `"`,
55+
""", `"`,
56+
"'", "'",
57+
"&", "&",
58+
)
59+
60+
func restoreVisiblePunctuation(input string) string {
61+
if !strings.Contains(input, "&") {
62+
return input
63+
}
64+
out := input
65+
// Peel stacked & prefixes (' → ' → ') without ever
66+
// turning < / > into angle brackets. Each Replace shortens the
67+
// string or is a no-op, so this is bounded by len(input).
68+
for range len(input) {
69+
next := visiblePunctuationUnescaper.Replace(out)
70+
if next == out {
71+
return out
72+
}
73+
out = next
74+
}
75+
return out
76+
}
77+
3978
// FilterInvisibleCharacters removes invisible or control characters that should not appear
4079
// in user-facing titles or bodies. This includes:
4180
// - Unicode tag characters: U+E0001, U+E0020–U+E007F

0 commit comments

Comments
 (0)