From 0469e3502d93be4613a6715de8119ecd79b8d0c8 Mon Sep 17 00:00:00 2001 From: Junjun Zhang Date: Mon, 19 Oct 2026 18:30:33 +0800 Subject: [PATCH] fix(im): mattermost mention boundary survives usernames ending in '-' or '.' (#4322) hasMention/stripMention used (?i)@\b, but RE2's \b is an ASCII word boundary that can never fire after a trailing '-' or '.' - both legal in Mattermost's ^[a-z0-9.\-_]+$ username charset. Bots named like "dev-" or "ops1." were therefore silently dropped by the requireMention gate in non-DM channels (no log, no observability) and never stripped from LLM context. Replace \b with the post-match continuation-char boundary check adopted for matrix in #2749: a match counts only when the next byte is not in [a-z0-9._-]. stripMention consumes via matrixASCIIFoldPrefix so matching and consumption share one coordinate system (#3572 lesson). The #963 short-name guard ("al" vs "@alex") is preserved by construction. Closes #4322 Co-Authored-By: ggcode --- internal/im/mattermost_adapter.go | 56 +++++++++++--- internal/im/zz_mention_boundary_test.go | 97 +++++++++++++++++++++++++ 2 files changed, 144 insertions(+), 9 deletions(-) create mode 100644 internal/im/zz_mention_boundary_test.go diff --git a/internal/im/mattermost_adapter.go b/internal/im/mattermost_adapter.go index 005c4f369..cec489706 100644 --- a/internal/im/mattermost_adapter.go +++ b/internal/im/mattermost_adapter.go @@ -11,7 +11,6 @@ import ( "net/http" "net/textproto" "os" - "regexp" "strings" "sync" "time" @@ -467,30 +466,69 @@ func (a *mattermostAdapter) handleWSEvent(ctx context.Context, event map[string] } } +// mattermostUsernameContChars is the set of characters that may CONTINUE a +// Mattermost username past its end. The official charset is ^[a-z0-9.\-_]+$ +// (mattermost/server validUsernameChars), so a mention ends only where this +// set does not continue. RE2 \b cannot express that: it is an ASCII word +// boundary and can never fire after a trailing '-' or '.', silently dropping +// every mention of bots named like "dev-" or "ops1." (#4322). +const mattermostUsernameContChars = "abcdefghijklmnopqrstuvwxyz0123456789._-" + +// mattermostMentionEndsAt reports whether a mention needle ending at byte +// index end is properly terminated: at end-of-text or followed by a character +// that cannot continue a username. This is the boundary \b could not express +// for names ending in '-' or '.' (#4322, pattern adopted from matrix #2749). +func mattermostMentionEndsAt(text string, end int) bool { + return end >= len(text) || !strings.ContainsAny(string(text[end]), mattermostUsernameContChars) +} + func (a *mattermostAdapter) hasMention(text string) bool { lower := strings.ToLower(text) - // Word-boundary match: bot name "al" must not fire on "@alex" (#963). + // Bot name "al" must not fire on "@alex" (#963), while usernames that + // legally end in '-' or '.' ("dev-", "ops1.") must still match (#4322): + // the boundary is a post-match continuation-char check, not \b. for _, id := range []string{a.botUsername, a.botUserID} { if id == "" { continue } - re := regexp.MustCompile(`(?i)@` + regexp.QuoteMeta(id) + `\b`) - if re.MatchString(lower) { - return true + needle := "@" + strings.ToLower(id) + for start := 0; ; { + i := strings.Index(lower[start:], needle) + if i < 0 { + break + } + at := start + i + if mattermostMentionEndsAt(lower, at+len(needle)) { + return true + } + start = at + 1 } } return false } func (a *mattermostAdapter) stripMention(text string) string { - // Strip @bot_username and @bot_user_id mentions, with word boundary so a - // short bot name ("al") doesn't carve text out of "@alex" (#963). + // Strip @bot_username and @bot_user_id mentions. A short bot name ("al") + // must not carve text out of "@alex" (#963), while names ending in '-' or + // '.' must still strip cleanly (#4322): boundary is the post-match + // continuation-char check, not \b. for _, pattern := range []string{a.botUsername, a.botUserID} { if pattern == "" { continue } - re := regexp.MustCompile(`(?i)@` + regexp.QuoteMeta(pattern) + `\b`) - text = re.ReplaceAllString(text, "") + needle := "@" + strings.ToLower(pattern) + var b strings.Builder + for i := 0; i < len(text); { + // Needles are ASCII ([a-z0-9._-]); per-byte ASCII fold keeps one + // coordinate system for matching and consuming bytes (#3572 lesson). + if n, ok := matrixASCIIFoldPrefix(text[i:], needle); ok && mattermostMentionEndsAt(text, i+n) { + i += n + continue + } + b.WriteByte(text[i]) + i++ + } + text = b.String() } return strings.TrimSpace(text) } diff --git a/internal/im/zz_mention_boundary_test.go b/internal/im/zz_mention_boundary_test.go new file mode 100644 index 000000000..fcaa3f675 --- /dev/null +++ b/internal/im/zz_mention_boundary_test.go @@ -0,0 +1,97 @@ +package im + +import "testing" + +// #4322 companion test: RE2 \b never fires after a username legally ending in +// '-' or '.', so hasMention/stripMention silently dropped non-DM mentions of +// such bots. The boundary is now a post-match continuation-char check +// (mattermostUsernameContChars), mirroring the matrix #2749 fix pattern. + +func zzMentionAdapter(username, userID string) *mattermostAdapter { + return &mattermostAdapter{botUsername: username, botUserID: userID} +} + +func TestZZMattermostMentionBoundaryTrailingDashDot(t *testing.T) { + cases := []struct { + name string + username string + text string + want bool + }{ + // Regression core: names ending in '-' or '.' must match (#4322). + {"trailing dash then space", "dev-", "@dev- 帮我看下日志", true}, + {"trailing dash at end", "dev-", "ping @dev-", true}, + {"trailing dot then space", "ops1.", "@ops1. check logs", true}, + {"trailing dot at end", "a.", "hey @a.", true}, + {"mixed case mention", "dev-", "@DEV- look", true}, + // Boundary must still hold where it always did (#963). + {"short name not substring", "al", "@alex hello", false}, + {"continuation blocks match", "dev", "@dev- team note", false}, // "@dev-" is a different, longer name + {"dash continuation blocks", "dev", "@dev. status", false}, + {"underscore continuation blocks", "dev", "@dev_ops", false}, + {"plain mention still true", "dev", "@dev run tests", true}, + {"no mention", "dev", "dev unmentioned", false}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + a := zzMentionAdapter(tc.username, "") + if got := a.hasMention(tc.text); got != tc.want { + t.Fatalf("hasMention(%q, bot=%q) = %v, want %v", tc.text, tc.username, got, tc.want) + } + }) + } +} + +func TestZZMattermostMentionBoundaryUserID(t *testing.T) { + a := zzMentionAdapter("bot", "abc123xyz") + if !a.hasMention("@abc123xyz go") { + t.Fatal("user-ID mention must match") + } + if a.hasMention("@abc123xyz9 go") { // ID continued by alnum is a different ID + t.Fatal("continuation char must block user-ID match") + } +} + +func TestZZMattermostStripMentionTrailingDashDot(t *testing.T) { + cases := []struct { + name string + username string + in string + want string + }{ + {"strip trailing dash", "dev-", "@dev- 帮我看下日志", "帮我看下日志"}, + {"strip trailing dot", "ops1.", "@ops1. check logs", "check logs"}, + {"strip mixed case", "dev-", "@DEV- look", "look"}, + {"short name keeps @alex", "al", "@alex hello", "@alex hello"}, + {"strip plain name", "dev", "@dev run tests", "run tests"}, + {"username strip leaves other mention", "bot", "@bot @abc123 go", "@abc123 go"}, // userID empty: only @bot stripped + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + a := zzMentionAdapter(tc.username, "") + if got := a.stripMention(tc.in); got != tc.want { + t.Fatalf("stripMention(%q, bot=%q) = %q, want %q", tc.in, tc.username, got, tc.want) + } + }) + } +} + +func TestZZMattermostStripMentionUserID(t *testing.T) { + a := zzMentionAdapter("", "abc123xyz") + if got := a.stripMention("@abc123xyz go"); got != "go" { + t.Fatalf("user-ID strip = %q, want \"go\"", got) + } +} + +// Multi-byte text right after the mention must count as a boundary byte-wise +// (high bytes are not in the ASCII continuation set), matching matrix #3572 +// single-coordinate-system behavior. +func TestZZMattermostMentionBoundaryMultibyte(t *testing.T) { + a := zzMentionAdapter("dev-", "") + if !a.hasMention("@dev-你好") { + t.Fatal("mention followed by multibyte text must match") + } + if got := a.stripMention("@dev-你好"); got != "你好" { + t.Fatalf("strip with multibyte suffix = %q, want 你好", got) + } +}