Filter invisible Unicode after HTML entity normalization

pkg/sanitize/sanitize.go

@@ -4,6 +4,7 @@ import (
 	"strings"
 	"sync"
 	"unicode"
+	"unicode/utf8"
 
 	"github.com/microcosm-cc/bluemonday"
 )
@@ -12,25 +13,54 @@ var policy *bluemonday.Policy
 var policyOnce sync.Once
 
 func Sanitize(input string) string {
-	return FilterHTMLTags(FilterCodeFenceMetadata(FilterInvisibleCharacters(input)))
+	// The invisible-character and code-fence filters both run before and after
+	// HTML processing. The first pass strips raw invisible characters so they
+	// don't interfere with code-fence parsing. HTML sanitization
+	// (FilterHTMLTags) decodes character entities (e.g. "​" or
+	// "​" become U+200B), which can introduce invisible or
+	// bidirectional characters that were not present as literal runes in the
+	// original input. Those decoded characters can both survive on their own
+	// and splice previously inert text into a code fence, so the second pass
+	// re-applies both filters to the fully normalized output.
+	normalized := FilterHTMLTags(FilterCodeFenceMetadata(FilterInvisibleCharacters(input)))
+	return FilterCodeFenceMetadata(FilterInvisibleCharacters(normalized))
 }
 
 // FilterInvisibleCharacters removes invisible or control characters that should not appear
 // in user-facing titles or bodies. This includes:
 // - Unicode tag characters: U+E0001, U+E0020–U+E007F
 // - BiDi control characters: U+202A–U+202E, U+2066–U+2069
-// - Hidden modifier characters: U+200B, U+200C, U+200E, U+200F, U+00AD, U+FEFF, U+180E, U+2060–U+2064
+// - BiDi/directional marks: U+200E, U+200F, U+061C
+// - Hidden modifier characters: U+200B, U+200C, U+00AD, U+FEFF, U+180E, U+2060–U+2064
+// - Orphaned variation selectors: U+FE00–U+FE0F, U+E0100–U+E01EF
+//
+// Variation selectors are filtered contextually rather than unconditionally.
+// A selector that forms a plausible variation sequence with the character it
+// follows is preserved, so ordinary content such as "✈️", "1️⃣" and CJK
+// ideographic variation sequences survive unchanged. Selectors that cannot
+// belong to such a sequence — those at the start of the input, those following
+// a removed or non-graphic character, and runs of consecutive selectors — are
+// removed, which is the shape used to smuggle hidden payloads.
 func FilterInvisibleCharacters(input string) string {
 	if input == "" {
 		return input
 	}
 
 	// Filter runes
 	out := make([]rune, 0, len(input))
+	var prev rune
+	var prevKept bool
 	for _, r := range input {
-		if !shouldRemoveRune(r) {
+		keep := false
+		if isVariationSelector(r) {
+			keep = prevKept && isValidVariationSequence(prev, r)
+		} else {
+			keep = !shouldRemoveRune(r)
+		}
+		if keep {
 			out = append(out, r)
 		}
+		prev, prevKept = r, keep
 	}
 	return string(out)
 }
@@ -179,6 +209,7 @@ func shouldRemoveRune(r rune) bool {
 		0x200C, // ZERO WIDTH NON-JOINER
 		0x200E, // LEFT-TO-RIGHT MARK
 		0x200F, // RIGHT-TO-LEFT MARK
+		0x061C, // ARABIC LETTER MARK
 		0x00AD, // SOFT HYPHEN
 		0xFEFF, // ZERO WIDTH NO-BREAK SPACE
 		0x180E: // MONGOLIAN VOWEL SEPARATOR
@@ -207,3 +238,40 @@ func shouldRemoveRune(r rune) bool {
 
 	return false
 }
+
+// isVariationSelector reports whether r is a Unicode variation selector, either
+// from the Variation Selectors block (VS1–VS16) or the Variation Selectors
+// Supplement (VS17–VS256).
+func isVariationSelector(r rune) bool {
+	return (r >= 0xFE00 && r <= 0xFE0F) || (r >= 0xE0100 && r <= 0xE01EF)
+}
+
+// isValidVariationSequence reports whether selector can legitimately apply to
+// the base character it immediately follows.
+//
+// A base may carry at most one selector, so a selector following another
+// selector is always rejected; consecutive selectors carry no rendering meaning
+// and are the primary way arbitrary data is hidden in text.
+func isValidVariationSequence(base, selector rune) bool {
+	if isVariationSelector(base) || !unicode.IsGraphic(base) || unicode.IsSpace(base) {
+		return false
+	}
+
+	// The Ideographic Variation Database only registers sequences whose base is
+	// a CJK ideograph, so supplement selectors are meaningless elsewhere.
+	if selector >= 0xE0100 {
+		return unicode.Is(unicode.Han, base)
+	}
+
+	// Standardized variation sequences use non-ASCII bases, except for the
+	// keycap bases '#', '*' and the ASCII digits, which take a presentation
+	// selector (VS15/VS16) only.
+	if base < utf8.RuneSelf {
+		if base != '#' && base != '*' && (base < '0' || base > '9') {
+			return false
+		}
+		return selector == 0xFE0E || selector == 0xFE0F
+	}
+
+	return true
+}

pkg/sanitize/sanitize_test.go

@@ -112,6 +112,56 @@ func TestFilterInvisibleCharacters(t *testing.T) {
 			input:    "This is a\u200B bug report.\n\nSteps to reproduce:\u200C\n1. Do this\u200E\n2. Do that\u200F",
 			expected: "This is a bug report.\n\nSteps to reproduce:\n1. Do this\n2. Do that",
 		},
+		{
+			name:     "text with arabic letter mark",
+			input:    "Hello\u061CWorld",
+			expected: "HelloWorld",
+		},
+		{
+			name:     "orphaned variation selector after ascii letter",
+			input:    "Hello\uFE0FWorld",
+			expected: "HelloWorld",
+		},
+		{
+			name:     "ideographic variation selector after non-ideograph base",
+			input:    "Hello\U000E0100World",
+			expected: "HelloWorld",
+		},
+		{
+			name:     "variation selector at start of input has no base",
+			input:    "\uFE0FHello",
+			expected: "Hello",
+		},
+		{
+			name:     "variation selector orphaned by removed zero width space",
+			input:    "\u2708\u200B\uFE0F",
+			expected: "\u2708",
+		},
+		{
+			name:     "smuggled selector run after emoji keeps only the presentation selector",
+			input:    "\U0001F600\uFE0F\U000E0101\U000E0102Hi",
+			expected: "\U0001F600\uFE0FHi",
+		},
+		{
+			name:     "emoji presentation sequence is preserved",
+			input:    "Book a flight \u2708\uFE0F today",
+			expected: "Book a flight \u2708\uFE0F today",
+		},
+		{
+			name:     "text presentation sequence is preserved",
+			input:    "Book a flight \u2708\uFE0E today",
+			expected: "Book a flight \u2708\uFE0E today",
+		},
+		{
+			name:     "keycap sequence is preserved",
+			input:    "Step 1\uFE0F\u20E3 first",
+			expected: "Step 1\uFE0F\u20E3 first",
+		},
+		{
+			name:     "registered cjk ideographic variation sequence is preserved",
+			input:    "\u845B\U000E0100\u57CE",
+			expected: "\u845B\U000E0100\u57CE",
+		},
 	}
 
 	for _, tt := range tests {
@@ -166,6 +216,17 @@ func TestShouldRemoveRune(t *testing.T) {
 		{name: "before hidden modifier range", rune: 0x205F, expected: false},
 		{name: "after hidden modifier range", rune: 0x2065, expected: false},
 
+		// Additional directional mark
+		{name: "arabic letter mark", rune: 0x061C, expected: true},
+
+		// Variation selectors are filtered contextually by
+		// FilterInvisibleCharacters, so shouldRemoveRune never removes them on
+		// its own. See TestIsValidVariationSequence for that behaviour.
+		{name: "variation selector range start", rune: 0xFE00, expected: false},
+		{name: "variation selector range end (VS16, emoji presentation)", rune: 0xFE0F, expected: false},
+		{name: "variation selector supplement range start", rune: 0xE0100, expected: false},
+		{name: "variation selector supplement range end", rune: 0xE01EF, expected: false},
+
 		// Characters that should NOT be removed
 		{name: "regular ascii letter", rune: 'A', expected: false},
 		{name: "regular ascii digit", rune: '1', expected: false},
@@ -300,3 +361,171 @@ func TestSanitizeRemovesInvisibleCodeFenceMetadata(t *testing.T) {
 	result := Sanitize(input)
 	assert.Equal(t, expected, result)
 }
+
+// TestSanitizeFiltersInvisibleCharactersAfterEntityDecoding covers the core
+// regression from issue #3101: invisible/bidi characters encoded as HTML
+// character entities are decoded by FilterHTMLTags, so the invisible-character
+// policy must also run after HTML processing, not only on the raw input.
+func TestSanitizeFiltersInvisibleCharactersAfterEntityDecoding(t *testing.T) {
+	tests := []struct {
+		name     string
+		input    string
+		expected string
+	}{
+		{
+			name:     "decimal entity for zero width space",
+			input:    "Hello​World",
+			expected: "HelloWorld",
+		},
+		{
+			name:     "hexadecimal entity for zero width space",
+			input:    "Hello​World",
+			expected: "HelloWorld",
+		},
+		{
+			name:     "hexadecimal entity for zero width space (lowercase hex digits)",
+			input:    "Hello​World",
+			expected: "HelloWorld",
+		},
+		{
+			name:     "decimal entity for right-to-left override",
+			input:    "Hello‮World",
+			expected: "HelloWorld",
+		},
+		{
+			name:     "hexadecimal entity for left-to-right override",
+			input:    "Hello‭World",
+			expected: "HelloWorld",
+		},
+		{
+			name:     "decimal entity for orphaned variation selector",
+			input:    "Hello️World",
+			expected: "HelloWorld",
+		},
+		{
+			name:     "hexadecimal entity for orphaned variation selector supplement",
+			input:    "Hello󠄀World",
+			expected: "HelloWorld",
+		},
+		{
+			name:     "entity encoded selector run after emoji is truncated to one selector",
+			input:    "Ship it \U0001F600️󠄁󠄂",
+			expected: "Ship it \U0001F600\uFE0F",
+		},
+		{
+			name:     "direct invisible rune alongside entity encoded one",
+			input:    "Hello\u200B‎World",
+			expected: "HelloWorld",
+		},
+		{
+			name:     "entity for ordinary ascii character is preserved",
+			input:    "HelloAWorld",
+			expected: "HelloAWorld",
+		},
+		{
+			name:     "entity for benign unicode character is preserved",
+			input:    "Hello世World", // 世 is 世
+			expected: "Hello世World",
+		},
+		{
+			name:     "benign unicode text without entities is untouched",
+			input:    "Hello 世界 🌍 αβγ",
+			expected: "Hello 世界 🌍 αβγ",
+		},
+		{
+			name:     "emoji presentation sequence survives the full pipeline",
+			input:    "Book a flight \u2708\uFE0F today",
+			expected: "Book a flight \u2708\uFE0F today",
+		},
+		{
+			name:     "registered cjk ideographic variation sequence survives the full pipeline",
+			input:    "\u845B\U000E0100\u57CE",
+			expected: "\u845B\U000E0100\u57CE",
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			result := Sanitize(tt.input)
+			assert.Equal(t, tt.expected, result)
+		})
+	}
+}
+
+// TestSanitizeRemovesCodeFenceMetadataRevealedByEntityDecoding covers fences
+// that only become fences after HTML entity decoding. A leading "`​“"
+// is not a fence in the raw input, so the first FilterCodeFenceMetadata pass
+// leaves it alone; once the entity is decoded and the zero width space is
+// removed the line is a real fence, so the fence filter has to run again.
+func TestSanitizeRemovesCodeFenceMetadataRevealedByEntityDecoding(t *testing.T) {
+	tests := []struct {
+		name     string
+		input    string
+		expected string
+	}{
+		{
+			name:     "decimal entity hides fence delimiter",
+			input:    "`​``steal secrets\nfmt.Println(42)\n```",
+			expected: "```\nfmt.Println(42)\n```",
+		},
+		{
+			name:     "hexadecimal entity hides fence delimiter",
+			input:    "``​`steal secrets\nfmt.Println(42)\n```",
+			expected: "```\nfmt.Println(42)\n```",
+		},
+		{
+			name:     "entity hides fence delimiter with disallowed info string",
+			input:    "`​``go;rm -rf /\ncode\n```",
+			expected: "```\ncode\n```",
+		},
+		{
+			name:     "entity encoded fence keeps a safe info string",
+			input:    "`​``go\nfmt.Println(42)\n```",
+			expected: "```go\nfmt.Println(42)\n```",
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			result := Sanitize(tt.input)
+			assert.Equal(t, tt.expected, result)
+		})
+	}
+}
+
+func TestIsValidVariationSequence(t *testing.T) {
+	tests := []struct {
+		name     string
+		base     rune
+		selector rune
+		expected bool
+	}{
+		{name: "emoji presentation selector after symbol", base: 0x2708, selector: 0xFE0F, expected: true},
+		{name: "text presentation selector after symbol", base: 0x2708, selector: 0xFE0E, expected: true},
+		{name: "presentation selector after emoji", base: 0x1F600, selector: 0xFE0F, expected: true},
+		{name: "presentation selector after keycap digit", base: '1', selector: 0xFE0F, expected: true},
+		{name: "presentation selector after keycap hash", base: '#', selector: 0xFE0F, expected: true},
+		{name: "presentation selector after keycap asterisk", base: '*', selector: 0xFE0E, expected: true},
+		{name: "non-presentation selector after keycap digit", base: '1', selector: 0xFE00, expected: false},
+		{name: "presentation selector after ascii letter", base: 'a', selector: 0xFE0F, expected: false},
+		{name: "presentation selector after ascii punctuation", base: '.', selector: 0xFE0F, expected: false},
+		{name: "standardized selector after cjk ideograph", base: '葛', selector: 0xFE00, expected: true},
+
+		{name: "ideographic selector after cjk ideograph", base: '葛', selector: 0xE0100, expected: true},
+		{name: "ideographic selector after cjk compatibility ideograph", base: 0xF900, selector: 0xE0101, expected: true},
+		{name: "ideographic selector after emoji", base: 0x1F600, selector: 0xE0100, expected: false},
+		{name: "ideographic selector after ascii letter", base: 'a', selector: 0xE0100, expected: false},
+		{name: "ideographic selector after greek letter", base: 'α', selector: 0xE0100, expected: false},
+
+		{name: "selector after another selector", base: 0xFE0F, selector: 0xFE0F, expected: false},
+		{name: "ideographic selector after another selector", base: 0xE0100, selector: 0xE0101, expected: false},
+		{name: "selector after space", base: ' ', selector: 0xFE0F, expected: false},
+		{name: "selector after newline", base: '\n', selector: 0xFE0F, expected: false},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			assert.Equal(t, tt.expected, isValidVariationSequence(tt.base, tt.selector))
+		})
+	}
+}