From 8b6425c9818276951ff0afffcf228ffd172b4e6c Mon Sep 17 00:00:00 2001 From: Jeremy Daer Date: Fri, 21 Aug 2026 14:31:45 -0700 Subject: [PATCH 1/6] Strip the confusables a terminal cannot show from sanitized text MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit terminal.Sanitize closed the escape, control and Bidi_Control classes; what was left was text that reads as one thing and is another. A zero width space in a sender's address or a soft hyphen before a filename's extension rendered as the honest spelling while being a different string, and a stack of combining marks could climb out of its cell. The policy, class by class, lives in the package doc. The format characters that draw nothing — ZWSP, word joiner, soft hyphen, BOM, CGJ, the Mongolian vowel separator, the invisible operators and the deprecated format controls — are stripped. The zero width joiner and non-joiner are kept only where they can join: between two non-ASCII, non-space runes, one at a time, so an emoji family, Persian and Devanagari keep theirs and "paypal" does not. Combining marks are capped at four on a base, the deepest stack a writing system produces. Non-breaking and fixed-width spaces are left alone, and homoglyphs are not detected: without confusable tables the check cannot tell a forged name from a multilingual one. Sanitize is still one pass and allocates nothing for text that needs nothing removed; a joiner is held until the rune after it is kept, so every decision is made on kept text and sanitizing twice is the same as once. Closes #249 --- AGENTS.md | 7 +- internal/terminal/sanitize.go | 190 +++++++++++++++++++++++++++-- internal/terminal/sanitize_test.go | 119 ++++++++++++++---- 3 files changed, 282 insertions(+), 34 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index d1f273bcc..97717fc35 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -150,7 +150,12 @@ handed. The model has three layers, and it is worth knowing which one a change t - **Metadata** — a sender, a subject, a filename, a box name, a habit title — goes through `terminal.Sanitize`/`SanitizeLine` (`internal/terminal`), which strips escape - sequences, C0/C1 controls and the `Bidi_Control` set. It is applied once where it + sequences, C0/C1 controls and the `Bidi_Control` set. Its confusable policy (in the + package doc) strips the format characters that draw nothing (zero width space, word + joiner, soft hyphen, BOM, …) and combining marks past four on a base, keeps ZWJ/ZWNJ + where they join (emoji families, Persian, Devanagari), leaves NBSP and friends alone, + and does not detect homoglyphs — a URL-shaped link label is shown beside its + destination by `htmlutil.ToMarkdown` instead. It is applied once where it covers the most: the CLI's `table.addRow` sanitizes its cells, and the TUI's read-only models (`mail.NewPosting`, `mail.NewEntry`, `searchMatchToPosting`) are sanitized as they are built. The diff --git a/internal/terminal/sanitize.go b/internal/terminal/sanitize.go index d0c5a03c3..666a2f078 100644 --- a/internal/terminal/sanitize.go +++ b/internal/terminal/sanitize.go @@ -12,33 +12,172 @@ // "invoice" and "fdp.exe" shows a PDF on screen and an executable on disk, and an // isolate can swap the order of a sender's name and address. Nothing HEY shows in a single line needs // them — an RTL name still reads right-to-left without an explicit override. +// +// # Confusables +// +// Past the controls lies text that reads as one thing and is another. The policy, +// class by class: +// +// Format characters that draw nothing are stripped: the soft hyphen, the zero +// width space, the word joiner, the byte order mark, the combining grapheme joiner, +// the Mongolian vowel separator, the invisible mathematical operators and the +// deprecated format controls next to them. A zero width space in "paypal" or a soft +// hyphen in "invoicepdf.exe" renders as the honest spelling while being a different +// string, which is the same class as a bidi override. They are stripped rather than +// replaced, since a replacement mark is visible debris in a name column. The list is +// enumerated; it is not the Cf category wholesale, which also holds the joiners +// below, the Arabic number and verse signs that are visible, and the tag characters +// a subdivision flag is made of. +// +// The zero width joiner and non-joiner are kept where they can join. They carry +// text: an emoji family is emoji joined by U+200D, and Persian, Urdu and the Indic +// scripts write with both. A joiner survives only between two non-ASCII, +// non-space runes, and a run of them collapses to one; at the start or end of a +// string, or next to an ASCII letter — a joiner inside "paypal" — it joins nothing +// and is dropped. +// +// Combining marks are kept up to four on a base and the rest of the run dropped. +// The deepest stacks a writing system produces reach four — a Hebrew letter with +// its shin dot, dagesh, vowel and cantillation; a Tibetan stack with two subjoined +// letters and a vowel sign — where decomposed Vietnamese, Hindi and the keycap +// emoji use two. Zalgo needs dozens to climb out of its cell. Variation selectors +// are marks and sit under the same cap, so the U+FE0F that makes a heart red stays. +// +// Spaces that are not U+0020 are left alone. A no-break space is ordinary in text +// that came out of HTML, as are the fixed-width spaces; they render as a space and +// read as one, and normalizing them would rewrite names for no safety gain. +// +// Homoglyphs — a Cyrillic "а" in a Latin name — are not detected. Without +// confusable tables and a script heuristic the check cannot tell a forged name from +// a multilingual one, and the false positives land on real people. What a link's +// label says against where it goes is the Markdown serializer's business: a label +// that reads as a URL is written beside its destination rather than collapsed into +// it (see htmlutil.ToMarkdown). package terminal import ( "strings" + "unicode" + "unicode/utf8" "github.com/charmbracelet/x/ansi" ) var lineBreaks = strings.NewReplacer("\n", " ", "\t", " ") -// Sanitize removes escape sequences, control characters and bidirectional controls -// from text on its way to a terminal. Newlines and tabs survive, because text is not -// necessarily one line: a message body, a jq result and a Markdown cell all carry -// them on purpose. +// maxCombiningMarks is how many combining marks may follow one base before the +// rest of the run is dropped. +const maxCombiningMarks = 4 + +const ( + zeroWidthNonJoiner = 0x200c + zeroWidthJoiner = 0x200d +) + +// Sanitize removes escape sequences, control characters, bidirectional controls +// and the confusables described in the package doc from text on its way to a +// terminal. Newlines and tabs survive, because text is not necessarily one line: a +// message body, a jq result and a Markdown cell all carry them on purpose. +// +// One pass, and no allocation for text that needs nothing removed. Every decision +// is made on what has been kept, so sanitizing twice is the same as once. func Sanitize(value string) string { - return strings.Map(func(r rune) rune { + p := pass{in: ansi.Strip(value), kept: -1} + for i, r := range p.in { switch { case r == '\n' || r == '\t': - return r - case r < 0x20 || r == 0x7f || (r >= 0x80 && r <= 0x9f): - return -1 - case isBidiControl(r): - return -1 + p.keep(r) + p.marks = 0 + case isControl(r), isBidiControl(r), isInvisible(r): + p.drop(i) + case r == zeroWidthJoiner || r == zeroWidthNonJoiner: + p.hold(i, r) + case isCombiningMark(r): + if p.marks < maxCombiningMarks { + p.keep(r) + p.marks++ + } else { + p.drop(i) + } default: - return r + p.keep(r) + p.marks = 0 + } + } + return p.String() +} + +// pass is one walk over the input. The output is only built once something is +// dropped; until then the input is its own output. A joiner is held back until the +// rune after it is kept, and written only when that rune is something it can join. +type pass struct { + in string + out *strings.Builder + kept rune // the last rune written, other than a joiner + marks int // combining marks written since the last base + joiner rune // a joiner waiting for its right-hand side, or zero + joinerAt int // its byte offset +} + +func (p *pass) keep(r rune) { + if p.joiner != 0 { + if joinable(r) { + p.write(p.joiner) + } else { + p.diverge(p.joinerAt) } - }, ansi.Strip(value)) + p.joiner = 0 + } + p.write(r) + p.kept = r +} + +func (p *pass) drop(i int) { + if p.joiner != 0 { + i = p.joinerAt + } + p.diverge(i) +} + +// hold keeps a joiner back until keep decides it. A joiner with nothing kept on +// its left, or one held while another is pending, is dropped. +func (p *pass) hold(i int, r rune) { + switch { + case p.joiner != 0 || !joinable(p.kept): + p.drop(i) + default: + p.joiner, p.joinerAt = r, i + } +} + +func (p *pass) write(r rune) { + if p.out != nil { + p.out.WriteRune(r) + } +} + +// diverge starts the output at the point the first dropped rune would have gone, +// which is i or the joiner pending before it. +func (p *pass) diverge(i int) { + if p.out == nil { + p.out = &strings.Builder{} + p.out.Grow(len(p.in)) + p.out.WriteString(p.in[:i]) + } +} + +func (p *pass) String() string { + if p.joiner != 0 { + p.diverge(p.joinerAt) + } + if p.out == nil { + return p.in + } + return p.out.String() +} + +func isControl(r rune) bool { + return r < 0x20 || r == 0x7f || (r >= 0x80 && r <= 0x9f) } // isBidiControl reports the Unicode Bidi_Control property: the Arabic letter mark, @@ -57,6 +196,33 @@ func isBidiControl(r rune) bool { } } +// isInvisible reports the format characters that draw nothing and carry no text: +// the soft hyphen, the combining grapheme joiner, the Mongolian vowel separator, +// the zero width space, the word joiner and the invisible operators after it, the +// deprecated format controls, and the byte order mark. +func isInvisible(r rune) bool { + switch { + case r == 0x00ad, r == 0x034f, r == 0x180e, r == 0x200b, r == 0xfeff: + return true + case r >= 0x2060 && r <= 0x2064: + return true + case r >= 0x206a && r <= 0x206f: + return true + default: + return false + } +} + +// joinable reports whether r is something a joiner next to it can join: non-ASCII +// text that is not a space. +func joinable(r rune) bool { + return r >= utf8.RuneSelf && !unicode.IsSpace(r) +} + +func isCombiningMark(r rune) bool { + return r >= 0x300 && unicode.In(r, unicode.Mn, unicode.Me, unicode.Mc) +} + // SanitizeLine is Sanitize for somewhere only one line fits — a table cell, a // confirmation — where a newline or a tab would move what comes after it rather // than merely reading oddly. Both become a space, so the words stay apart. diff --git a/internal/terminal/sanitize_test.go b/internal/terminal/sanitize_test.go index cfb295bdf..74412561a 100644 --- a/internal/terminal/sanitize_test.go +++ b/internal/terminal/sanitize_test.go @@ -1,28 +1,80 @@ package terminal -import "testing" +import ( + "strings" + "testing" -func TestSanitize(t *testing.T) { - tests := []struct { - name string - value string - want string - }{ - {"plain text is untouched", "Ryan Singer", "Ryan Singer"}, - {"a colour sequence goes with its payload", "\x1b[31mURGENT\x1b[0m", "URGENT"}, - {"a window title sequence leaves nothing behind", "\x1b]0;pwned\x07report.pdf", "report.pdf"}, - {"a stray escape takes the byte it would have escaped", "sub\x1bject", "subect"}, - {"a carriage return cannot rewrite the line", "Invoice\rPAID", "InvoicePAID"}, - {"a C1 control is removed", "caf" + string(rune(0x85)) + "e", "cafe"}, - {"DEL is removed", "note\x7f", "note"}, - {"newlines and tabs survive", "line one\nline\ttwo", "line one\nline\ttwo"}, - {"emoji and accents survive", "Café ☕", "Café ☕"}, - {"a right-to-left override cannot reverse a filename", "invoice\u202efdp.exe", "invoicefdp.exe"}, - {"every bidi control goes", "\u061ca\u200eb\u200fc\u202ad\u202be\u202cf\u202dg\u2066h\u2067i\u2068j\u2069k", "abcdefghijk"}, - {"right-to-left text itself survives", "שלום", "שלום"}, - } + "github.com/charmbracelet/x/ansi" +) - for _, test := range tests { +var sanitizeTests = []struct { + name string + value string + want string +}{ + {"plain text is untouched", "Ryan Singer", "Ryan Singer"}, + {"a colour sequence goes with its payload", "\x1b[31mURGENT\x1b[0m", "URGENT"}, + {"a window title sequence leaves nothing behind", "\x1b]0;pwned\x07report.pdf", "report.pdf"}, + {"a stray escape takes the byte it would have escaped", "sub\x1bject", "subect"}, + {"a carriage return cannot rewrite the line", "Invoice\rPAID", "InvoicePAID"}, + {"a C1 control is removed", "caf" + string(rune(0x85)) + "e", "cafe"}, + {"DEL is removed", "note\x7f", "note"}, + {"newlines and tabs survive", "line one\nline\ttwo", "line one\nline\ttwo"}, + {"emoji and accents survive", "Café ☕", "Café ☕"}, + {"a right-to-left override cannot reverse a filename", "invoice\u202efdp.exe", "invoicefdp.exe"}, + {"every bidi control goes", "\u061ca\u200eb\u200fc\u202ad\u202be\u202cf\u202dg\u2066h\u2067i\u2068j\u2069k", "abcdefghijk"}, + {"right-to-left text itself survives", "שלום", "שלום"}, + + // Invisible format characters draw nothing and are stripped. + {"a zero width space cannot split a domain", "pay\u200bpal@example.com", "paypal@example.com"}, + {"a soft hyphen cannot hide a filename's extension", "invoice\u00adpdf.exe", "invoicepdf.exe"}, + {"a byte order mark goes", "\ufeffReport", "Report"}, + {"a word joiner goes", "Q3\u2060 planning", "Q3 planning"}, + {"every invisible format character goes", "a\u00adb\u034fc\u180ed\u200be\u2060f\u2061g\u2062h\u2063i\u2064j\u206ak\u206fl\ufeffm", "abcdefghijklm"}, + + // Joiners stay where they join and go where they cannot. + {"an emoji family keeps its joiners", "👨\u200d👩\u200d👧", "👨\u200d👩\u200d👧"}, + {"a heart on fire keeps its selector and joiner", "❤\ufe0f\u200d🔥", "❤\ufe0f\u200d🔥"}, + {"a rainbow flag keeps its joiner", "🏳\ufe0f\u200d🌈", "🏳\ufe0f\u200d🌈"}, + {"a skin tone before a joiner keeps it", "👩🏽\u200d💻", "👩🏽\u200d💻"}, + {"Persian keeps its non-joiner", "می\u200cخواهم", "می\u200cخواهم"}, + {"Devanagari keeps a joiner between letters", "क्\u200dष", "क्\u200dष"}, + {"a joiner between ASCII letters goes", "pay\u200dpal", "paypal"}, + {"a non-joiner between ASCII letters goes", "pay\u200cpal", "paypal"}, + {"a joiner at either end goes", "\u200dRyan\u200d", "Ryan"}, + {"a joiner beside a space goes", "é\u200d é", "é é"}, + {"a run of joiners collapses to one", "é\u200d\u200d\u200dé", "é\u200dé"}, + {"a joiner after a stripped invisible is judged by what was kept", "a\u200b\u200dé", "aé"}, + {"a joiner at the end of non-ASCII text goes", "é\u200d", "é"}, + {"a joiner whose right-hand side is stripped goes", "é\u200d\u200b", "é"}, + {"a joiner whose right-hand side is a dropped mark goes", "e" + strings.Repeat("\u0301", 4) + "\u200d\u0301", "e" + strings.Repeat("\u0301", 4)}, + {"a run of joiners before a space goes", "é\u200d\u200d é", "é é"}, + + // Combining marks are capped at four on a base. + {"a precomposed accent survives", "Café", "Café"}, + {"a decomposed accent survives", "Cafe\u0301", "Cafe\u0301"}, + {"decomposed Vietnamese survives", "Vie\u0323\u0302t Nam", "Vie\u0323\u0302t Nam"}, + {"Hindi survives", "हिन्दी", "हिन्दी"}, + {"a nukta and virama survive", "क\u093c\u094dष", "क\u093c\u094dष"}, + {"Hebrew with four marks survives", "ש\u05c1\u05bc\u05b7\u05ab", "ש\u05c1\u05bc\u05b7\u05ab"}, + {"a Tibetan stack survives", "བསྒྲུབས", "བསྒྲུབས"}, + {"a keycap survives", "1\ufe0f\u20e3", "1\ufe0f\u20e3"}, + {"an emoji presentation selector survives", "❤\ufe0f", "❤\ufe0f"}, + {"Zalgo is cut at four marks", "Z" + strings.Repeat("\u0336", 50) + "algo", "Z" + strings.Repeat("\u0336", 4) + "algo"}, + {"the cap resets on each base", "a\u0301\u0302\u0303\u0304\u0305b\u0301\u0302\u0303\u0304\u0305", "a\u0301\u0302\u0303\u0304b\u0301\u0302\u0303\u0304"}, + {"the cap resets on a newline", "a" + strings.Repeat("\u0301", 4) + "\n" + strings.Repeat("\u0301", 5), "a" + strings.Repeat("\u0301", 4) + "\n" + strings.Repeat("\u0301", 4)}, + + // Spaces that are not U+0020 are left alone. + {"a no-break space survives", "Jean\u00a0Dupont", "Jean\u00a0Dupont"}, + {"an ideographic space survives", "山田\u3000太郎", "山田\u3000太郎"}, + {"a narrow no-break space survives", "12\u202f000", "12\u202f000"}, + + // Homoglyphs are not detected. + {"a Cyrillic letter in a Latin word is kept", "p\u0430ypal", "p\u0430ypal"}, +} + +func TestSanitize(t *testing.T) { + for _, test := range sanitizeTests { t.Run(test.name, func(t *testing.T) { if got := Sanitize(test.value); got != test.want { t.Errorf("Sanitize(%q) = %q, want %q", test.value, got, test.want) @@ -31,6 +83,30 @@ func TestSanitize(t *testing.T) { } } +// Sanitizing a second time changes nothing, and sanitizing never lengthens: every +// decision is made on what was kept, so the output is its own fixed point. +func TestSanitizeIsIdempotentAndNeverLonger(t *testing.T) { + for _, test := range sanitizeTests { + once := Sanitize(test.value) + if twice := Sanitize(once); twice != once { + t.Errorf("%s: Sanitize(Sanitize(%q)) = %q, want %q", test.name, test.value, twice, once) + } + if len(once) > len(test.value) { + t.Errorf("%s: Sanitize(%q) = %q is longer than its input", test.name, test.value, once) + } + } +} + +// Text that needs nothing removed is returned as it is: Sanitize allocates nothing +// beyond what ansi.Strip already does. +func TestSanitizeCleanTextDoesNotAllocate(t *testing.T) { + value := "Ryan Singer — Q3 planning ☕ Vie\u0323\u0302t Nam 👨\u200d👩\u200d👧" + stripping := testing.AllocsPerRun(100, func() { ansi.Strip(value) }) + if allocs := testing.AllocsPerRun(100, func() { Sanitize(value) }); allocs > stripping { + t.Errorf("Sanitize(%q) allocates %v times, want %v (ansi.Strip alone)", value, allocs, stripping) + } +} + func TestSanitizeLine(t *testing.T) { tests := []struct { name string @@ -41,6 +117,7 @@ func TestSanitizeLine(t *testing.T) { {"a tab becomes a space", "Ryan\tSinger", "Ryan Singer"}, {"an escape sequence still goes", "\x1b[2JLabel", "Label"}, {"plain text is untouched", "Q3 planning", "Q3 planning"}, + {"an invisible goes and a newline becomes a space", "pay\u200bpal\nRyan", "paypal Ryan"}, } for _, test := range tests { From 0424eff2d9c1ce1b44aacdc2f87102bf7d647903 Mon Sep 17 00:00:00 2001 From: Jeremy Daer Date: Fri, 21 Aug 2026 14:31:53 -0700 Subject: [PATCH 2/6] Pin that a URL-shaped link label is shown beside its destination MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The other half of #249 is a link whose visible host is not the host it points at. The serializer already holds the property: only a label that is its href, character for character, collapses into an autolink, so a homoglyph host in the label — "https://pаypal.com" with a Cyrillic а against https://evil.example — is written [label](destination), and glamour shows the destination beside it and links to it. No confusable table is needed to get there, so none is added; the tests now say so for a homoglyph URL, a www. host, a bare host and path, a same-host different-path label, and a label that is its href, through goldmark in htmlutil and through glamour in markdown. Render's own tests cover the confusable policy on bodies, and both fuzz targets seed the homoglyph link. --- internal/htmlutil/markdown.go | 6 +++++ internal/htmlutil/markdown_safety_test.go | 33 ++++++++++++++++++----- internal/markdown/safety_test.go | 31 +++++++++++++++++++++ 3 files changed, 64 insertions(+), 6 deletions(-) diff --git a/internal/htmlutil/markdown.go b/internal/htmlutil/markdown.go index 1335c9e47..29daa8b0d 100644 --- a/internal/htmlutil/markdown.go +++ b/internal/htmlutil/markdown.go @@ -339,6 +339,12 @@ func (m *markdownizer) code(n *html.Node) { // link writes an anchor. A destination that cannot be linked leaves its text behind as // prose, and a label that is itself a URL never hides the destination it points at: the // destination is written out beside it, where a reader comparing the two can see both. +// Only a label that is its href, character for character, collapses into an autolink, +// so the visible host is the destination's host by construction — a label of +// "https://pаypal.com" with a Cyrillic а pointed at https://evil.example is written +// `[https://pаypal.com](https://evil.example)`, and no confusable table is needed to +// get there. Whether the label's host is itself a homoglyph of something is not +// detected; see the terminal package's policy. func (m *markdownizer) link(n *html.Node) { href := getAttr(n, "href") dest, linkable := destination(href) diff --git a/internal/htmlutil/markdown_safety_test.go b/internal/htmlutil/markdown_safety_test.go index 26fc676a0..b8536cf63 100644 --- a/internal/htmlutil/markdown_safety_test.go +++ b/internal/htmlutil/markdown_safety_test.go @@ -328,13 +328,33 @@ func TestToMarkdownAutolinkOnlyForAbsoluteURLs(t *testing.T) { } } -// A label that reads as one URL while pointing at another never collapses into an -// autolink: the destination is written beside the label, where it can be compared. +// A label that reads as one URL or host while pointing at another never collapses +// into an autolink: the destination is written beside the label, where it can be +// compared. That holds for a homoglyph host as much as an honest one — the Cyrillic +// а in "pаypal" is not detected, it simply is not the href — and a conformant +// renderer links to the destination while showing the label. func TestToMarkdownDeceptiveLabelShowsTheDestination(t *testing.T) { - got := toMarkdown(`

https://bank.example/login

`) - want := "[https://bank.example/login](https://evil.example/login)" - if got != want { - t.Errorf("ToMarkdown = %q, want %q", got, want) + const homoglyphHost = "https://p\u0430ypal.com/login" + for _, test := range []struct { + name, label, href, want string + }{ + {"a URL", "https://bank.example/login", "https://evil.example/login", "[https://bank.example/login](https://evil.example/login)"}, + {"a homoglyph URL", homoglyphHost, "https://evil.example/login", "[" + homoglyphHost + "](https://evil.example/login)"}, + {"a www host", "www.bank.example", "https://evil.example", "[www.bank.example](https://evil.example)"}, + {"a bare host and path", "bank.example/login", "https://evil.example/login", "[bank.example/login](https://evil.example/login)"}, + {"the same host, a different path", "https://bank.example/", "https://bank.example/login", "[https://bank.example/](https://bank.example/login)"}, + {"a label that is its href", homoglyphHost, homoglyphHost, "<" + homoglyphHost + ">"}, + } { + got := toMarkdown(`

` + test.label + `

`) + if got != test.want { + t.Errorf("%s: ToMarkdown = %q, want %q", test.name, got, test.want) + } + if links := renderedLinks(t, got); len(links) != 1 || links[0] != test.href { + t.Errorf("%s: rendered links = %q, want the destination %q", test.name, links, test.href) + } + if text := renderedText(t, got); text != test.label { + t.Errorf("%s: rendered = %q, want the label %q", test.name, text, test.label) + } } } @@ -524,6 +544,7 @@ func FuzzToMarkdownTerminalSafety(f *testing.F) { `
&#27;|
`, `
`, "

caf\u009c\u0085e

", + `

https://p\u0430ypal.com/login

`, } { f.Add(seed) } diff --git a/internal/markdown/safety_test.go b/internal/markdown/safety_test.go index a01dc52a6..7e5b0e19a 100644 --- a/internal/markdown/safety_test.go +++ b/internal/markdown/safety_test.go @@ -80,6 +80,36 @@ func TestRenderKeepsQueryStringsInHyperlinks(t *testing.T) { } } +// A label that reads as one host while pointing at another shows its destination in +// the rendering, and the hyperlink goes where the destination says. +func TestRenderDeceptiveLabelShowsTheDestination(t *testing.T) { + out := Render("[https://p\u0430ypal.com/login](https://evil.example/login)", 200) + if !strings.Contains(visible(out), "https://evil.example/login") { + t.Errorf("Render = %q, want the destination visible", out) + } + if !strings.Contains(out, ";https://evil.example/login\x07") { + t.Errorf("Render = %q, want a hyperlink to the destination", out) + } + if strings.Contains(out, ";https://p\u0430ypal.com") { + t.Errorf("Render = %q, want no hyperlink to the label's host", out) + } +} + +// A body goes through the same confusable policy as a subject line: invisible +// format characters go, a stack of combining marks is cut, and an emoji family keeps +// its joiners. +func TestRenderStripsConfusables(t *testing.T) { + for md, want := range map[string]string{ + "pay\u200bpal and invoice\u00adpdf.exe": "paypal and invoicepdf.exe", + "Z" + strings.Repeat("\u0336", 50) + "algo": "Z" + strings.Repeat("\u0336", 4) + "algo", + "the 👨\u200d👩\u200d👧 family": "the 👨\u200d👩\u200d👧 family", + } { + if out := visible(Render(md, 200)); !strings.Contains(out, want) { + t.Errorf("Render(%q) shows %q, want %q in it", md, out, want) + } + } +} + func TestRenderStripsRawControls(t *testing.T) { for name, md := range map[string]string{ "escape": "hello \x1b[31mRED", @@ -282,6 +312,7 @@ func FuzzContainment(f *testing.F) { "| a |\n| --- |\n|  |", "![](/rails/blobs/x.png)", "a\u0085b\u009cc", + "pay\u200bpal Z" + strings.Repeat("\u0336", 8) + "algo [https://p\u0430ypal.com](https://evil.example)", } { f.Add(seed) } From 39c9a8d5de41de4f0e9af3ebbff7ea35ecc5b7bd Mon Sep 17 00:00:00 2001 From: Jeremy Daer Date: Fri, 21 Aug 2026 14:50:12 -0700 Subject: [PATCH 3/6] Say why the confusable policy covers link destinations, and pin it The policy runs over the whole Markdown source, destinations included, so a zero-width character in an href is dropped from the OSC 8 target the same way it is dropped from the label. That is the intent: what a link shows and where it goes are the same text, and a destination that differs from the one shown only by a character the reader cannot see is exactly the confusable the policy removes. The JSON keeps the original. Written down next to stripControls, and held by a render test. --- internal/markdown/render_test.go | 13 +++++++++++++ internal/markdown/source.go | 14 ++++++++++---- 2 files changed, 23 insertions(+), 4 deletions(-) diff --git a/internal/markdown/render_test.go b/internal/markdown/render_test.go index f4624f6cc..c477d39e2 100644 --- a/internal/markdown/render_test.go +++ b/internal/markdown/render_test.go @@ -58,6 +58,19 @@ func TestRenderLinksAreClickable(t *testing.T) { } } +// The confusable policy covers a link's destination as well as its text: a zero-width +// character in the href is dropped from the OSC 8 target the same way it is dropped from +// the label, so the link goes where the reader sees it going. +func TestRenderLinksToWhatItShows(t *testing.T) { + got := Render("[https://example.com/a\u200bb](https://example.com/a\u200bb)", 80) + if strings.Contains(got, "\u200b") { + t.Errorf("Render = %q, a zero-width space survived", got) + } + if !strings.Contains(got, ";https://example.com/ab\a") { + t.Errorf("Render = %q, want the OSC 8 target to be the URL as shown", got) + } +} + func TestRenderFallsBackToDefaultWidth(t *testing.T) { if render("Hello", 0) == "" { t.Error("Render with no width returned nothing") diff --git a/internal/markdown/source.go b/internal/markdown/source.go index 8f0caad0b..c7a62abd9 100644 --- a/internal/markdown/source.go +++ b/internal/markdown/source.go @@ -55,10 +55,16 @@ func prepareSource(md string) (safe, forGlamour string, deep bool) { // milliseconds. const maxNestingDepth = 20 -// stripControls removes escape sequences, C0 and C1 controls and the bidirectional -// controls, keeping newlines and tabs. A body is prose, and the Trojan-source class -// — a right-to-left override that shows one thing and means another — is no more -// welcome in it than in a subject line; the JSON keeps the original. +// stripControls removes escape sequences, C0 and C1 controls, the bidirectional +// controls and the confusables terminal.Sanitize describes, keeping newlines and tabs. +// A body is prose, and the Trojan-source class — a right-to-left override that shows +// one thing and means another — is no more welcome in it than in a subject line; the +// JSON keeps the original. +// +// It runs over the whole source, link destinations included, on purpose: what a link +// shows and where it goes are then the same text, and a destination that differs from +// the one shown only by a zero-width character is exactly the confusable the policy +// removes. Sparing destinations would link a URL the reader cannot see to read. func stripControls(s string) string { return terminal.Sanitize(s) } From d515a4866efa0b7d753a91f3b9bf4721d11803a7 Mon Sep 17 00:00:00 2001 From: Jeremy Daer Date: Fri, 21 Aug 2026 15:01:39 -0700 Subject: [PATCH 4/6] Close five gaps in the confusable policy MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A byte that is not UTF-8 went through the fast path raw, so a stray 0x85 reached a byte-reading terminal as a C1 control; it is dropped now, the way everything else here is stripped rather than replaced, so the output is never longer than the input. The combining-mark cap was four, and fully pointed Hebrew reaches five — shin dot, dagesh, vowel, meteg and cantillation on one letter — so an accent was being taken off a sender's name. The cap is eight: room for any writing system, still far under the dozens Zalgo needs. A format character that is kept, such as the tag characters a subdivision flag is made of, no longer counts as a base, so a tag between two runs of marks cannot restart the cap. A joiner is judged against bases rather than the last rune kept: a mark is not a base on its own, so a joiner before a mark goes, and a mark on an ASCII letter does not make that letter joinable. A virama and an emoji presentation selector are marks on a base and keep joining. markdown.Render runs the same pass over a whole body before parsing it, so a zero width space between two runs of backticks in code joined them by the time the delimiters were read, and a delimiter sized on the content as written could be closed from inside. htmlutil sizes code delimiters on the content as the sanitizer leaves it (backtickRun); the content itself is still written as it is, so the JSON keeps it. --- AGENTS.md | 12 +++-- internal/htmlutil/markdown_safety.go | 17 ++++++- internal/htmlutil/markdown_safety_test.go | 17 +++++++ internal/markdown/safety_test.go | 2 +- internal/terminal/sanitize.go | 55 +++++++++++++++-------- internal/terminal/sanitize_test.go | 23 +++++++--- 6 files changed, 96 insertions(+), 30 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 97717fc35..f28d2b8df 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -152,10 +152,14 @@ handed. The model has three layers, and it is worth knowing which one a change t `terminal.Sanitize`/`SanitizeLine` (`internal/terminal`), which strips escape sequences, C0/C1 controls and the `Bidi_Control` set. Its confusable policy (in the package doc) strips the format characters that draw nothing (zero width space, word - joiner, soft hyphen, BOM, …) and combining marks past four on a base, keeps ZWJ/ZWNJ - where they join (emoji families, Persian, Devanagari), leaves NBSP and friends alone, - and does not detect homoglyphs — a URL-shaped link label is shown beside its - destination by `htmlutil.ToMarkdown` instead. It is applied once where it + joiner, soft hyphen, BOM, …) and combining marks past eight on a base, keeps ZWJ/ZWNJ + where they join (emoji families, Persian, Devanagari), drops a byte that is not + UTF-8, leaves NBSP and friends alone, and does not detect homoglyphs — a + URL-shaped link label is shown beside its destination by `htmlutil.ToMarkdown` + instead. `markdown.Render` runs the same pass over a whole body, destinations + included, so `htmlutil` sizes a code delimiter on the content as that pass leaves it + (`backtickRun`), or a zero width space between two backtick runs would join them + into a closing delimiter. It is applied once where it covers the most: the CLI's `table.addRow` sanitizes its cells, and the TUI's read-only models (`mail.NewPosting`, `mail.NewEntry`, `searchMatchToPosting`) are sanitized as they are built. The diff --git a/internal/htmlutil/markdown_safety.go b/internal/htmlutil/markdown_safety.go index b4f7ae125..7fd111f75 100644 --- a/internal/htmlutil/markdown_safety.go +++ b/internal/htmlutil/markdown_safety.go @@ -3,6 +3,8 @@ package htmlutil import ( "regexp" "strings" + + "github.com/basecamp/hey-cli/internal/terminal" ) // Everything ToMarkdown writes came out of somebody else's email, and the Markdown it @@ -97,7 +99,7 @@ func codeSpan(content string) string { if strings.TrimSpace(content) == "" { return "" } - delimiter := strings.Repeat("`", longestRun(content, '`')+1) + delimiter := strings.Repeat("`", backtickRun(content)+1) if strings.HasPrefix(content, "`") || strings.HasSuffix(content, "`") || (strings.HasPrefix(content, " ") && strings.HasSuffix(content, " ")) { content = " " + content + " " @@ -108,7 +110,18 @@ func codeSpan(content string) string { // codeFence is the fence that contains content: at least three backticks and, whatever // run of them the content holds, one more than that. func codeFence(content string) string { - return strings.Repeat("`", max(3, longestRun(content, '`')+1)) + return strings.Repeat("`", max(3, backtickRun(content)+1)) +} + +// backtickRun is the longest run of backticks in code content as a terminal will see +// it. markdown.Render puts the whole document through terminal.Sanitize before +// parsing it, and that drops characters that draw nothing, so two runs with a zero +// width space between them are one run by the time the delimiters are read; a +// delimiter sized on the content as written would then be closed from inside. The +// content itself is written as it is — the JSON keeps it — and only the measure +// looks through the sanitizer. +func backtickRun(content string) int { + return max(longestRun(content, '`'), longestRun(terminal.Sanitize(content), '`')) } // codeFenceInfo is the info string a fence may carry: a language name, nothing else. A diff --git a/internal/htmlutil/markdown_safety_test.go b/internal/htmlutil/markdown_safety_test.go index b8536cf63..88ddb91fb 100644 --- a/internal/htmlutil/markdown_safety_test.go +++ b/internal/htmlutil/markdown_safety_test.go @@ -211,6 +211,23 @@ func TestToMarkdownInlineCodeSizesItsDelimiters(t *testing.T) { } } +// A delimiter is sized on the content as a terminal will see it: the renderer's +// sanitizer drops a zero width space between two runs of backticks, joining them, so +// the delimiter must be longer than the joined run or the code could be closed from +// inside by text that only looks shorter. +func TestToMarkdownCodeDelimitersOutlastTheSanitizer(t *testing.T) { + for _, test := range []struct{ in, want string }{ + {"

``\u200b``

", "````` ``\u200b`` `````"}, + {"
``\u200b``\nafter
", "`````\n``\u200b``\nafter\n`````"}, + {"
``\u200d``
", "`````\n``\u200d``\n`````"}, + } { + got := ToMarkdown(test.in) + if got != test.want { + t.Errorf("ToMarkdown(%q) = %q, want %q", test.in, got, test.want) + } + } +} + func TestToMarkdownFencedCodeCannotBeClosedFromInside(t *testing.T) { got := toMarkdown("
first\n```\nnot outside\n

after

") want := "````\nfirst\n```\nnot outside\n````\n\nafter" diff --git a/internal/markdown/safety_test.go b/internal/markdown/safety_test.go index 7e5b0e19a..52a6273e5 100644 --- a/internal/markdown/safety_test.go +++ b/internal/markdown/safety_test.go @@ -101,7 +101,7 @@ func TestRenderDeceptiveLabelShowsTheDestination(t *testing.T) { func TestRenderStripsConfusables(t *testing.T) { for md, want := range map[string]string{ "pay\u200bpal and invoice\u00adpdf.exe": "paypal and invoicepdf.exe", - "Z" + strings.Repeat("\u0336", 50) + "algo": "Z" + strings.Repeat("\u0336", 4) + "algo", + "Z" + strings.Repeat("\u0336", 50) + "algo": "Z" + strings.Repeat("\u0336", 8) + "algo", "the 👨\u200d👩\u200d👧 family": "the 👨\u200d👩\u200d👧 family", } { if out := visible(Render(md, 200)); !strings.Contains(out, want) { diff --git a/internal/terminal/sanitize.go b/internal/terminal/sanitize.go index 666a2f078..6b162dba0 100644 --- a/internal/terminal/sanitize.go +++ b/internal/terminal/sanitize.go @@ -32,16 +32,25 @@ // The zero width joiner and non-joiner are kept where they can join. They carry // text: an emoji family is emoji joined by U+200D, and Persian, Urdu and the Indic // scripts write with both. A joiner survives only between two non-ASCII, -// non-space runes, and a run of them collapses to one; at the start or end of a -// string, or next to an ASCII letter — a joiner inside "paypal" — it joins nothing -// and is dropped. +// non-space base characters — the marks on the left base, a virama or an emoji +// presentation selector, are part of it, but a mark is not a base on its own — and a +// run of them collapses to one; at the start or end of a string, or next to an +// ASCII letter — a joiner inside "paypal" — it joins nothing and is dropped. // -// Combining marks are kept up to four on a base and the rest of the run dropped. -// The deepest stacks a writing system produces reach four — a Hebrew letter with -// its shin dot, dagesh, vowel and cantillation; a Tibetan stack with two subjoined -// letters and a vowel sign — where decomposed Vietnamese, Hindi and the keycap -// emoji use two. Zalgo needs dozens to climb out of its cell. Variation selectors -// are marks and sit under the same cap, so the U+FE0F that makes a heart red stays. +// Combining marks are kept up to eight on a base and the rest of the run dropped. +// The deepest stacks a writing system produces reach five — a fully pointed Hebrew +// letter with its shin dot, dagesh, vowel, meteg and cantillation; a Tibetan stack +// with two subjoined letters and a vowel sign is four — where decomposed Vietnamese, +// Hindi and the keycap emoji use two, so eight is room for any of them and Zalgo +// needs dozens to climb out of its cell. Variation selectors are marks and sit +// under the same cap, so the U+FE0F that makes a heart red stays. A format +// character that is kept — a tag character, an Arabic number sign — is not a base: +// it draws nothing of its own, so the marks after it still belong to the letter +// before it and count against the same cap. +// +// A byte that is not UTF-8 is dropped. Left as it is, a stray 0x85 is a C1 control +// to a terminal that reads bytes; and it goes the way everything else here goes, +// stripped rather than replaced, so the output is never longer than the input. // // Spaces that are not U+0020 are left alone. A no-break space is ordinary in text // that came out of HTML, as are the fixed-width spaces; they render as a space and @@ -66,8 +75,8 @@ import ( var lineBreaks = strings.NewReplacer("\n", " ", "\t", " ") // maxCombiningMarks is how many combining marks may follow one base before the -// rest of the run is dropped. -const maxCombiningMarks = 4 +// rest of the run is dropped: the package doc says why eight. +const maxCombiningMarks = 8 const ( zeroWidthNonJoiner = 0x200c @@ -82,12 +91,15 @@ const ( // One pass, and no allocation for text that needs nothing removed. Every decision // is made on what has been kept, so sanitizing twice is the same as once. func Sanitize(value string) string { - p := pass{in: ansi.Strip(value), kept: -1} + p := pass{in: ansi.Strip(value), kept: -1, base: -1} for i, r := range p.in { switch { case r == '\n' || r == '\t': p.keep(r) p.marks = 0 + case r == utf8.RuneError && !strings.HasPrefix(p.in[i:], "\ufffd"): + // An invalid byte, not a replacement character the text carried. + p.drop(i) case isControl(r), isBidiControl(r), isInvisible(r): p.drop(i) case r == zeroWidthJoiner || r == zeroWidthNonJoiner: @@ -101,7 +113,9 @@ func Sanitize(value string) string { } default: p.keep(r) - p.marks = 0 + if !unicode.Is(unicode.Cf, r) { + p.marks = 0 + } } } return p.String() @@ -114,6 +128,7 @@ type pass struct { in string out *strings.Builder kept rune // the last rune written, other than a joiner + base rune // the last rune written that is not a mark, other than a joiner marks int // combining marks written since the last base joiner rune // a joiner waiting for its right-hand side, or zero joinerAt int // its byte offset @@ -121,7 +136,7 @@ type pass struct { func (p *pass) keep(r rune) { if p.joiner != 0 { - if joinable(r) { + if joinable(r) && !isCombiningMark(r) { p.write(p.joiner) } else { p.diverge(p.joinerAt) @@ -130,6 +145,9 @@ func (p *pass) keep(r rune) { } p.write(r) p.kept = r + if !isCombiningMark(r) { + p.base = r + } } func (p *pass) drop(i int) { @@ -139,11 +157,11 @@ func (p *pass) drop(i int) { p.diverge(i) } -// hold keeps a joiner back until keep decides it. A joiner with nothing kept on -// its left, or one held while another is pending, is dropped. +// hold keeps a joiner back until keep decides it. A joiner with no base on its +// left, or one held while another is pending, is dropped. func (p *pass) hold(i int, r rune) { switch { - case p.joiner != 0 || !joinable(p.kept): + case p.joiner != 0 || !joinable(p.base): p.drop(i) default: p.joiner, p.joinerAt = r, i @@ -214,7 +232,8 @@ func isInvisible(r rune) bool { } // joinable reports whether r is something a joiner next to it can join: non-ASCII -// text that is not a space. +// text that is not a space. Whether it is a base rather than a mark is the caller's +// question. func joinable(r rune) bool { return r >= utf8.RuneSelf && !unicode.IsSpace(r) } diff --git a/internal/terminal/sanitize_test.go b/internal/terminal/sanitize_test.go index 74412561a..f85f16008 100644 --- a/internal/terminal/sanitize_test.go +++ b/internal/terminal/sanitize_test.go @@ -47,22 +47,35 @@ var sanitizeTests = []struct { {"a joiner after a stripped invisible is judged by what was kept", "a\u200b\u200dé", "aé"}, {"a joiner at the end of non-ASCII text goes", "é\u200d", "é"}, {"a joiner whose right-hand side is stripped goes", "é\u200d\u200b", "é"}, - {"a joiner whose right-hand side is a dropped mark goes", "e" + strings.Repeat("\u0301", 4) + "\u200d\u0301", "e" + strings.Repeat("\u0301", 4)}, + {"a joiner whose right-hand side is a dropped mark goes", "e" + strings.Repeat("\u0301", 8) + "\u200d\u0301", "e" + strings.Repeat("\u0301", 8)}, {"a run of joiners before a space goes", "é\u200d\u200d é", "é é"}, + {"a joiner after a virama joins", "क\u094d\u200dष", "क\u094d\u200dष"}, + {"a joiner after an emoji presentation selector joins", "❤\ufe0f\u200d🔥", "❤\ufe0f\u200d🔥"}, + {"a joiner before a mark goes", "é\u200d\u0301", "é\u0301"}, + {"a mark on an ASCII base does not make it joinable", "e\u0301\u200dé", "e\u0301é"}, - // Combining marks are capped at four on a base. + // Combining marks are capped at eight on a base. {"a precomposed accent survives", "Café", "Café"}, {"a decomposed accent survives", "Cafe\u0301", "Cafe\u0301"}, {"decomposed Vietnamese survives", "Vie\u0323\u0302t Nam", "Vie\u0323\u0302t Nam"}, {"Hindi survives", "हिन्दी", "हिन्दी"}, {"a nukta and virama survive", "क\u093c\u094dष", "क\u093c\u094dष"}, {"Hebrew with four marks survives", "ש\u05c1\u05bc\u05b7\u05ab", "ש\u05c1\u05bc\u05b7\u05ab"}, + {"fully pointed Hebrew with five marks survives", "ש\u05c1\u05bc\u05b8\u05bd\u0591", "ש\u05c1\u05bc\u05b8\u05bd\u0591"}, {"a Tibetan stack survives", "བསྒྲུབས", "བསྒྲུབས"}, {"a keycap survives", "1\ufe0f\u20e3", "1\ufe0f\u20e3"}, {"an emoji presentation selector survives", "❤\ufe0f", "❤\ufe0f"}, - {"Zalgo is cut at four marks", "Z" + strings.Repeat("\u0336", 50) + "algo", "Z" + strings.Repeat("\u0336", 4) + "algo"}, - {"the cap resets on each base", "a\u0301\u0302\u0303\u0304\u0305b\u0301\u0302\u0303\u0304\u0305", "a\u0301\u0302\u0303\u0304b\u0301\u0302\u0303\u0304"}, - {"the cap resets on a newline", "a" + strings.Repeat("\u0301", 4) + "\n" + strings.Repeat("\u0301", 5), "a" + strings.Repeat("\u0301", 4) + "\n" + strings.Repeat("\u0301", 4)}, + {"Zalgo is cut at eight marks", "Z" + strings.Repeat("\u0336", 50) + "algo", "Z" + strings.Repeat("\u0336", 8) + "algo"}, + {"the cap resets on each base", "a" + strings.Repeat("\u0301", 9) + "b" + strings.Repeat("\u0301", 9), "a" + strings.Repeat("\u0301", 8) + "b" + strings.Repeat("\u0301", 8)}, + {"the cap resets on a newline", "a" + strings.Repeat("\u0301", 8) + "\n" + strings.Repeat("\u0301", 9), "a" + strings.Repeat("\u0301", 8) + "\n" + strings.Repeat("\u0301", 8)}, + {"a kept format character is not a base", "Z" + strings.Repeat("\u0336", 8) + "\U000e0020" + strings.Repeat("\u0336", 8), "Z" + strings.Repeat("\u0336", 8) + "\U000e0020"}, + {"a subdivision flag survives", "\U0001f3f4\U000e0067\U000e0062\U000e0073\U000e0063\U000e0074\U000e007f", "\U0001f3f4\U000e0067\U000e0062\U000e0073\U000e0063\U000e0074\U000e007f"}, + + // A byte that is not UTF-8 is dropped rather than reaching the terminal raw. + {"an invalid byte goes", "caf\xe9.txt", "caf.txt"}, + {"a raw C1 byte goes", "a\x85b", "ab"}, + {"a replacement character the text carried survives", "a\ufffdb", "a\ufffdb"}, + {"a joiner whose right-hand side is an invalid byte goes", "é\u200d\xe9", "é"}, // Spaces that are not U+0020 are left alone. {"a no-break space survives", "Jean\u00a0Dupont", "Jean\u00a0Dupont"}, From a9e23e7c0245905c4d9b75eb1f2023392462973f Mon Sep 17 00:00:00 2001 From: Jeremy Daer Date: Fri, 21 Aug 2026 15:14:00 -0700 Subject: [PATCH 5/6] Pad a code span on its sanitized edges, end a base at a format character, and leave an unedited title alone MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three more from the review. A code span whose content begins with a stripped character and then a backtick was sized right but padded on the content as written, so the backtick met the delimiter once the character was gone; the padding is decided on both views of the content. A format character that is kept — a tag — ends a base rather than riding on it the way a mark does, so a joiner on either side of one joins nothing and goes. The category rename input is filled with the title sanitized for the screen, since the input draws its value as it is; saving that back unedited rewrote a title that carried a character the policy strips. An unedited rename now sends nothing. --- internal/htmlutil/markdown_safety.go | 12 ++++++++++-- internal/htmlutil/markdown_safety_test.go | 2 ++ internal/terminal/sanitize.go | 11 +++++++++-- internal/terminal/sanitize_test.go | 2 ++ internal/tui/calendar.go | 5 +++++ internal/tui/time_track_categories_test.go | 4 ++++ 6 files changed, 32 insertions(+), 4 deletions(-) diff --git a/internal/htmlutil/markdown_safety.go b/internal/htmlutil/markdown_safety.go index 7fd111f75..0bea7ab3b 100644 --- a/internal/htmlutil/markdown_safety.go +++ b/internal/htmlutil/markdown_safety.go @@ -100,13 +100,21 @@ func codeSpan(content string) string { return "" } delimiter := strings.Repeat("`", backtickRun(content)+1) - if strings.HasPrefix(content, "`") || strings.HasSuffix(content, "`") || - (strings.HasPrefix(content, " ") && strings.HasSuffix(content, " ")) { + if needsPadding(content) || needsPadding(terminal.Sanitize(content)) { content = " " + content + " " } return delimiter + content + delimiter } +// needsPadding reports code-span content that would run into its delimiters: a backtick +// at either end, or a space at both. It is asked of the content as written and as the +// renderer's sanitizer leaves it, since a stripped character before a leading backtick +// would otherwise bring that backtick up against the delimiter on screen. +func needsPadding(content string) bool { + return strings.HasPrefix(content, "`") || strings.HasSuffix(content, "`") || + (strings.HasPrefix(content, " ") && strings.HasSuffix(content, " ")) +} + // codeFence is the fence that contains content: at least three backticks and, whatever // run of them the content holds, one more than that. func codeFence(content string) string { diff --git a/internal/htmlutil/markdown_safety_test.go b/internal/htmlutil/markdown_safety_test.go index 88ddb91fb..1d80ac5ae 100644 --- a/internal/htmlutil/markdown_safety_test.go +++ b/internal/htmlutil/markdown_safety_test.go @@ -218,6 +218,8 @@ func TestToMarkdownInlineCodeSizesItsDelimiters(t *testing.T) { func TestToMarkdownCodeDelimitersOutlastTheSanitizer(t *testing.T) { for _, test := range []struct{ in, want string }{ {"

``\u200b``

", "````` ``\u200b`` `````"}, + {"

\u200b`x

", "`` \u200b`x ``"}, + {"

x`\u200b

", "`` x`\u200b ``"}, {"
``\u200b``\nafter
", "`````\n``\u200b``\nafter\n`````"}, {"
``\u200d``
", "`````\n``\u200d``\n`````"}, } { diff --git a/internal/terminal/sanitize.go b/internal/terminal/sanitize.go index 6b162dba0..32dc9303b 100644 --- a/internal/terminal/sanitize.go +++ b/internal/terminal/sanitize.go @@ -136,7 +136,7 @@ type pass struct { func (p *pass) keep(r rune) { if p.joiner != 0 { - if joinable(r) && !isCombiningMark(r) { + if isBase(r) { p.write(p.joiner) } else { p.diverge(p.joinerAt) @@ -150,6 +150,13 @@ func (p *pass) keep(r rune) { } } +// isBase reports a rune a joiner can join: non-ASCII text that is neither a space, a +// mark nor a format character. The marks on a base ride along with it; a tag or any +// other format character ends it, so a joiner after one has nothing to join. +func isBase(r rune) bool { + return joinable(r) && !isCombiningMark(r) && !unicode.Is(unicode.Cf, r) +} + func (p *pass) drop(i int) { if p.joiner != 0 { i = p.joinerAt @@ -161,7 +168,7 @@ func (p *pass) drop(i int) { // left, or one held while another is pending, is dropped. func (p *pass) hold(i int, r rune) { switch { - case p.joiner != 0 || !joinable(p.base): + case p.joiner != 0 || !isBase(p.base): p.drop(i) default: p.joiner, p.joinerAt = r, i diff --git a/internal/terminal/sanitize_test.go b/internal/terminal/sanitize_test.go index f85f16008..f69461840 100644 --- a/internal/terminal/sanitize_test.go +++ b/internal/terminal/sanitize_test.go @@ -53,6 +53,8 @@ var sanitizeTests = []struct { {"a joiner after an emoji presentation selector joins", "❤\ufe0f\u200d🔥", "❤\ufe0f\u200d🔥"}, {"a joiner before a mark goes", "é\u200d\u0301", "é\u0301"}, {"a mark on an ASCII base does not make it joinable", "e\u0301\u200dé", "e\u0301é"}, + {"a joiner before a tag character goes", "é\u200d\U000e0020", "é\U000e0020"}, + {"a tag character is not a base for a joiner", "é\U000e0020\u200dé", "é\U000e0020é"}, // Combining marks are capped at eight on a base. {"a precomposed accent survives", "Café", "Café"}, diff --git a/internal/tui/calendar.go b/internal/tui/calendar.go index a661399bf..411363d37 100644 --- a/internal/tui/calendar.go +++ b/internal/tui/calendar.go @@ -379,6 +379,11 @@ func (v *calendarView) handleTimeTrackCategoryKey(msg tea.KeyPressMsg) tea.Cmd { return v.createTimeTrackCategory(title) } if selected != nil { + // The input was filled with the title sanitized for the screen, so an + // unedited rename is no rename — saving it would rewrite the title. + if title == selected.Title || title == terminal.SanitizeLine(selected.Title) { + return nil + } return v.renameTimeTrackCategory(selected.Id, title) } manager.status = "Choose a category to rename" diff --git a/internal/tui/time_track_categories_test.go b/internal/tui/time_track_categories_test.go index 256386299..5f5da10d0 100644 --- a/internal/tui/time_track_categories_test.go +++ b/internal/tui/time_track_categories_test.go @@ -135,6 +135,10 @@ func TestCalendarTimeTrackCategoryMutations(t *testing.T) { if view.timeTrackCategories.mode != timeTrackCategoryRename { t.Fatal("enter should open category rename") } + if cmd := view.HandleContentKey(keyPress("enter")); cmd != nil || view.timeTrackCategories.mode != timeTrackCategoryBrowse { + t.Fatal("an unedited rename should send nothing") + } + view.HandleContentKey(keyPress("enter")) view.timeTrackCategories.input.SetValue("Customer work") rename := view.HandleContentKey(keyPress("enter")) if rename == nil { From bc3cc35257b0a89e74b8172ae3071f6e0280071e Mon Sep 17 00:00:00 2001 From: Jeremy Daer Date: Fri, 21 Aug 2026 15:25:33 -0700 Subject: [PATCH 6/6] Use the unexported seams in the tests that came over the rebase ToMarkdown answers the sealed type on main now, and Render takes only that; the tests added on this branch read through toMarkdown and render, as the rest of the package tests do. --- internal/htmlutil/markdown_safety_test.go | 2 +- internal/markdown/render_test.go | 2 +- internal/markdown/safety_test.go | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/internal/htmlutil/markdown_safety_test.go b/internal/htmlutil/markdown_safety_test.go index 1d80ac5ae..b0f32f842 100644 --- a/internal/htmlutil/markdown_safety_test.go +++ b/internal/htmlutil/markdown_safety_test.go @@ -223,7 +223,7 @@ func TestToMarkdownCodeDelimitersOutlastTheSanitizer(t *testing.T) { {"
``\u200b``\nafter
", "`````\n``\u200b``\nafter\n`````"}, {"
``\u200d``
", "`````\n``\u200d``\n`````"}, } { - got := ToMarkdown(test.in) + got := toMarkdown(test.in) if got != test.want { t.Errorf("ToMarkdown(%q) = %q, want %q", test.in, got, test.want) } diff --git a/internal/markdown/render_test.go b/internal/markdown/render_test.go index c477d39e2..4924716a6 100644 --- a/internal/markdown/render_test.go +++ b/internal/markdown/render_test.go @@ -62,7 +62,7 @@ func TestRenderLinksAreClickable(t *testing.T) { // character in the href is dropped from the OSC 8 target the same way it is dropped from // the label, so the link goes where the reader sees it going. func TestRenderLinksToWhatItShows(t *testing.T) { - got := Render("[https://example.com/a\u200bb](https://example.com/a\u200bb)", 80) + got := render("[https://example.com/a\u200bb](https://example.com/a\u200bb)", 80) if strings.Contains(got, "\u200b") { t.Errorf("Render = %q, a zero-width space survived", got) } diff --git a/internal/markdown/safety_test.go b/internal/markdown/safety_test.go index 52a6273e5..b3ca492a5 100644 --- a/internal/markdown/safety_test.go +++ b/internal/markdown/safety_test.go @@ -83,7 +83,7 @@ func TestRenderKeepsQueryStringsInHyperlinks(t *testing.T) { // A label that reads as one host while pointing at another shows its destination in // the rendering, and the hyperlink goes where the destination says. func TestRenderDeceptiveLabelShowsTheDestination(t *testing.T) { - out := Render("[https://p\u0430ypal.com/login](https://evil.example/login)", 200) + out := render("[https://p\u0430ypal.com/login](https://evil.example/login)", 200) if !strings.Contains(visible(out), "https://evil.example/login") { t.Errorf("Render = %q, want the destination visible", out) } @@ -104,7 +104,7 @@ func TestRenderStripsConfusables(t *testing.T) { "Z" + strings.Repeat("\u0336", 50) + "algo": "Z" + strings.Repeat("\u0336", 8) + "algo", "the 👨\u200d👩\u200d👧 family": "the 👨\u200d👩\u200d👧 family", } { - if out := visible(Render(md, 200)); !strings.Contains(out, want) { + if out := visible(render(md, 200)); !strings.Contains(out, want) { t.Errorf("Render(%q) shows %q, want %q in it", md, out, want) } }