From 18399d47c1b7c81d2d4b57a47fe29b87870f3226 Mon Sep 17 00:00:00 2001 From: Max Freedom Pollard <272618364+MaxFreedomPollard@users.noreply.github.com> Date: Sat, 5 Sep 2026 22:49:28 -0400 Subject: [PATCH] notify: fix panic when truncating multi-byte messages TruncateInBytes indexed the rune slice with a byte count. In notify/util.go it built the starting slice as r[:truncationTarget], where r is []rune(s) and truncationTarget is n-3, a number of bytes. A string holding multi-byte characters has fewer runes than bytes, so whenever the input was longer than n bytes but held fewer than n-3 runes the index ran past the end of the rune slice and panicked with "slice bounds out of range". The Webex notifier is the only caller. It truncates the rendered message to maxMessageSize (7439 bytes) in notify/webex/webex.go:89, so a notification whose markdown was 7440 bytes or more of non-ASCII text crashed Alertmanager: 3720 Cyrillic characters, 2480 CJK characters or 1860 emoji are enough. The notification pipeline runs in a goroutine started by the dispatcher and nothing recovers, so the process exits. Clamp the starting index to the number of runes available. The loop that follows already trims the slice down until it fits the byte budget, so the result is unchanged for every input that did not panic. Add a case to TestTruncate covering a short string whose byte budget exceeds its rune count, TestTruncateInBytesFewerRunesThanBytes covering two-, three- and four-byte runes at the Webex limit, and TestWebexTruncatesMultiByteMessage covering the notifier end to end. Signed-off-by: Max Freedom Pollard <272618364+MaxFreedomPollard@users.noreply.github.com> --- notify/util.go | 7 ++++-- notify/util_test.go | 45 ++++++++++++++++++++++++++++++++++ notify/webex/webex_test.go | 50 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 100 insertions(+), 2 deletions(-) diff --git a/notify/util.go b/notify/util.go index fe4c9ea508..d18258fbda 100644 --- a/notify/util.go +++ b/notify/util.go @@ -135,8 +135,11 @@ func TruncateInBytes(s string, n int) (string, bool) { r := []rune(s) truncationTarget := n - 3 - // Next, let's truncate the runes to the lower possible number. - truncatedRunes := r[:truncationTarget] + // A string holding multi-byte characters has fewer runes than bytes, so + // truncationTarget can exceed the number of runes available. Start from + // the whole rune slice in that case; the loop below still trims it down to + // truncationTarget bytes. + truncatedRunes := r[:min(truncationTarget, len(r))] for len(string(truncatedRunes)) > truncationTarget { truncatedRunes = r[:len(truncatedRunes)-1] } diff --git a/notify/util_test.go b/notify/util_test.go index 2c2d4922e8..ad8b56a4a1 100644 --- a/notify/util_test.go +++ b/notify/util_test.go @@ -23,6 +23,7 @@ import ( "path" "reflect" "runtime" + "strings" "testing" "github.com/prometheus/common/model" @@ -99,6 +100,14 @@ func TestTruncate(t *testing.T) { runes: expect{out: "β€οΈβœ…πŸš€πŸ”₯βŒβ€οΈβœ…πŸš€πŸ”₯βŒβ€οΈβœ…πŸš€πŸ”₯βŒβ€¦", trunc: true}, bytes: expect{out: "β€οΈβœ…πŸš€β€¦", trunc: true}, }, + { + // Two runes, eight bytes: the byte budget left for the text is + // larger than the number of runes to pick it from. + in: "πŸ˜€πŸ˜€", + n: 7, + runes: expect{out: "πŸ˜€πŸ˜€", trunc: false}, + bytes: expect{out: "πŸ˜€β€¦", trunc: true}, + }, } type truncateFunc func(string, int) (string, bool) @@ -130,6 +139,42 @@ func TestTruncate(t *testing.T) { } } +// TestTruncateInBytesFewerRunesThanBytes covers strings that are longer than +// the byte limit while holding fewer runes than it, which is the case for any +// sufficiently long non-ASCII text. TruncateInBytes used to index the rune +// slice with the byte budget and panicked on those inputs. +func TestTruncateInBytesFewerRunesThanBytes(t *testing.T) { + // The message limit of the Webex notifier, the only caller of + // TruncateInBytes. + const n = 7439 + + for _, tc := range []struct { + name string + char string + count int + }{ + {name: "two-byte runes", char: "Π΄", count: 3720}, + {name: "three-byte runes", char: "δΈ–", count: 2600}, + {name: "four-byte runes", char: "πŸ˜€", count: 2000}, + } { + t.Run(tc.name, func(t *testing.T) { + in := strings.Repeat(tc.char, tc.count) + require.Greater(t, len(in), n, "the input must be over the byte limit") + require.Less(t, len([]rune(in)), n, "the input must hold fewer runes than the byte limit") + + out, truncated := TruncateInBytes(in, n) + require.True(t, truncated) + require.LessOrEqual(t, len(out), n) + require.True(t, strings.HasSuffix(out, truncationMarker)) + + kept := strings.TrimSuffix(out, truncationMarker) + require.True(t, strings.HasPrefix(in, kept), "the kept text must be a prefix of the input") + require.Len(t, []rune(kept), (n-len(truncationMarker))/len(tc.char), + "the kept text must hold as many whole runes as the byte budget allows") + }) + } +} + type brokenReader struct{} func (b brokenReader) Read([]byte) (int, error) { diff --git a/notify/webex/webex_test.go b/notify/webex/webex_test.go index eb12cfc2b6..fbb49f6e3a 100644 --- a/notify/webex/webex_test.go +++ b/notify/webex/webex_test.go @@ -15,10 +15,12 @@ package webex import ( "context" + "encoding/json" "io" "net/http" "net/http/httptest" "net/url" + "strings" "testing" "time" @@ -169,6 +171,54 @@ func TestWebexTemplating(t *testing.T) { } } +// TestWebexTruncatesMultiByteMessage sends a message that is over +// maxMessageSize in bytes while holding fewer runes than that, the shape any +// long non-ASCII alert takes. Notify used to panic on it. +func TestWebexTruncatesMultiByteMessage(t *testing.T) { + var out []byte + srv := httptest.NewServer(http.HandlerFunc(func(_ http.ResponseWriter, r *http.Request) { + var err error + out, err = io.ReadAll(r.Body) + require.NoError(t, err) + })) + defer srv.Close() + u, err := url.Parse(srv.URL) + require.NoError(t, err) + + notifier, err := New( + &config.WebexConfig{ + HTTPConfig: &commoncfg.HTTPClientConfig{}, + APIURL: &amcommoncfg.URL{URL: u}, + Message: "{{ .CommonAnnotations.description }}", + }, + test.CreateTmpl(t), + promslog.NewNopLogger(), + ) + require.NoError(t, err) + + // 3720 Cyrillic characters are 7440 bytes, one byte over the limit. + description := strings.Repeat("Π΄", 3720) + require.Greater(t, len(description), maxMessageSize) + require.Less(t, len([]rune(description)), maxMessageSize) + + ctx := notify.WithGroupKey(context.Background(), "1") + retry, err := notifier.Notify(ctx, &types.Alert{ + Alert: model.Alert{ + Labels: model.LabelSet{"lbl1": "val1"}, + Annotations: model.LabelSet{"description": model.LabelValue(description)}, + StartsAt: time.Now(), + EndsAt: time.Now().Add(time.Hour), + }, + }) + require.NoError(t, err) + require.False(t, retry) + + var w webhook + require.NoError(t, json.Unmarshal(out, &w)) + require.LessOrEqual(t, len(w.Markdown), maxMessageSize) + require.True(t, strings.HasPrefix(description, strings.TrimSuffix(w.Markdown, "…"))) +} + func TestWebexFailureReason(t *testing.T) { for _, tc := range []struct { name string