diff --git a/README.md b/README.md index 6d3e8ca..14953a6 100644 --- a/README.md +++ b/README.md @@ -19,6 +19,7 @@ Atlas via long polling and an in-process cron scheduler. | `stats` | `/stats` (top commands), `/stats users`, `/stats user `, `/stats cmd ` | | `sticker` | `/addsticker` — append a replied sticker, image, video or GIF to one shared pack. See [docs/sticker-packs.md](docs/sticker-packs.md) | | `alias` | `/alias ` save a replied message under a name, then send it back with `/insert `, bare `/`, or inline `@botname `; `/aliases` lists, `/unalias` deletes. See [docs/aliases.md](docs/aliases.md) | +| `blacklist` | Per-topic text deny-list with whitelist exceptions: `/blacklist_add`, `/blacklist_del`, `/whitelist_add`, `/whitelist_del`, `/blacklist_rules` lists both, `/blacklist_check` judges a text. Passive — the bot never scans chat. See [docs/blacklist.md](docs/blacklist.md) | | `monkeyd` | `/monkeyd_crawl [font_size]` export a monkeydd.com novel as a PDF, `/monkeyd_tags ` list its tags as hashtags | Disable modules with the `MODULES` environment variable. diff --git a/cmd/server/command_menu_test.go b/cmd/server/command_menu_test.go index 372da6e..c1189e2 100644 --- a/cmd/server/command_menu_test.go +++ b/cmd/server/command_menu_test.go @@ -75,6 +75,12 @@ func TestCommandDiscovery_AllPublicCommandsHaveSafeMetadata(t *testing.T) { "addsticker": "[emoji...]", "alias": "", "aliases": "", + "blacklist_add": "[text...]", + "blacklist_del": "", + "blacklist_rules": "", + "blacklist_check": "", + "whitelist_add": "[text...]", + "whitelist_del": "", "unalias": "", "insert": "", "stats": "[users | user | cmd ]", diff --git a/cmd/server/main.go b/cmd/server/main.go index bf0a9b4..86e37b8 100644 --- a/cmd/server/main.go +++ b/cmd/server/main.go @@ -20,6 +20,7 @@ import ( "github.com/tiennm99/miti99bot/internal/modules" "github.com/tiennm99/miti99bot/internal/modules/alias" "github.com/tiennm99/miti99bot/internal/modules/amlich" + "github.com/tiennm99/miti99bot/internal/modules/blacklist" "github.com/tiennm99/miti99bot/internal/modules/coin" "github.com/tiennm99/miti99bot/internal/modules/gold" "github.com/tiennm99/miti99bot/internal/modules/lol" @@ -95,6 +96,7 @@ func factories() map[string]modules.Factory { "stats": stats.New, sticker.CollectionName: sticker.New, "alias": alias.New, + "blacklist": blacklist.New, } } diff --git a/cmd/server/main_test.go b/cmd/server/main_test.go index 5365f8e..59a7d37 100644 --- a/cmd/server/main_test.go +++ b/cmd/server/main_test.go @@ -128,3 +128,34 @@ func TestFactoriesIncludesExpectedModules(t *testing.T) { } } } + +func TestFactoriesRegistersBlacklistCommands(t *testing.T) { + catalog := factories() + if catalog["blacklist"] == nil { + t.Fatal("factories missing blacklist") + } + reg, err := modules.Build([]string{"blacklist"}, catalog, storage.NewMemoryProvider(), modules.BuildOptions{}) + if err != nil { + t.Fatalf("Build blacklist: %v", err) + } + for _, name := range []string{ + "blacklist_add", "blacklist_del", "blacklist_rules", "blacklist_check", + "whitelist_add", "whitelist_del", + } { + if _, ok := reg.AllCommands[name]; !ok { + t.Fatalf("missing command %s", name) + } + } + if got := len(reg.AllCommands); got != 6 { + t.Fatalf("blacklist registered %d commands, want 6", got) + } +} + +// An empty MODULES loads every module, so a command name that collides with an +// existing module surfaces here as a test failure rather than as a startup +// crash on deploy. +func TestFactoriesBuildWholeCatalog(t *testing.T) { + if _, err := modules.Build(nil, factories(), storage.NewMemoryProvider(), modules.BuildOptions{}); err != nil { + t.Fatalf("Build whole catalog: %v", err) + } +} diff --git a/docs/blacklist.md b/docs/blacklist.md new file mode 100644 index 0000000..ec2c486 --- /dev/null +++ b/docs/blacklist.md @@ -0,0 +1,86 @@ +# Blacklist + +The `blacklist` module lets a chat keep a list of forbidden text and a list of +exceptions, then ask whether a given text is blocked. + +| Command | Parameters | What it does | +|---|---|---| +| `/blacklist_add` | `[text...]` | Adds text to the blacklist, or the message you replied to | +| `/blacklist_del` | `` | Removes text from the blacklist | +| `/whitelist_add` | `[text...]` | Adds an exception, or the message you replied to | +| `/whitelist_del` | `` | Removes an exception | +| `/blacklist_rules` | — | Lists both lists in one message | +| `/blacklist_check` | `` | Judges a text against both lists | + +All are public and single-shot. + +## The bot does not police the chat + +This is the first thing to know, because the module's name promises something it +deliberately does not do. Nothing happens automatically. The bot never reads +ordinary messages, never deletes anything, and never warns or restricts anyone. +The lists sit inert until `/blacklist_check` asks about a specific text. + +Automatic moderation would need three things this bot does not have: a +message-level hook in the dispatcher, privacy mode disabled in BotFather so +Telegram delivers ordinary group messages at all, and admin rights with +permission to delete in every group. All three are out of scope by choice. + +## Lists belong to a topic, not to the bot + +Each thread keeps its own two lists. In a forum supergroup, entries added in one +topic are invisible in the next. A non-forum group has one set of lists; a +private chat with the bot has its own, shared with nobody. + +This is the most common surprise: running `/blacklist_check` in the wrong topic +gives a different answer than the same command one topic over, and it is not a +bug. Every reply says "in this topic" for that reason. + +Within a thread, anyone can add and anyone can remove — including entries +someone else added. The lists belong to the conversation, so the permission to +edit them does too. A per-owner rule would strand entries whose author has left +the group. + +## How the whitelist works + +The whitelist is not a second independent list. It is an exception layer over +the blacklist, and it rescues a match only when it **covers** that match in the +text being checked. + +With `ass` blacklisted and `assassin` whitelisted: + +| Checked text | Verdict | Why | +|---|---|---| +| `assassin` | Allowed | The whitelist entry spans the whole match | +| `dumbass` | Blacklisted | Nothing whitelisted covers this `ass` | +| `I met an assassin, dumbass` | **Blacklisted** | The first match is rescued; the second is not | + +The third row is the one worth remembering. A whitelist entry does not make a +whole message safe — it only rescues the occurrences it actually contains. + +## Matching + +Matching is by substring, after the text is normalized: + +- **Case does not matter.** `Cat`, `CAT` and `cat` are one rule. +- **Spacing does not matter.** `cat dog` and `cat dog` are one rule, and a + newline is just a space. +- **Diacritics do matter.** `ma`, `má` and `mà` are three separate entries. To + catch all three, add all three. +- **The same word matches across devices.** Vietnamese typed on an iPhone and on + an Android phone can differ in how the accents are encoded; normalization + resolves that, so the two forms match each other. +- **Full-width and other presentation variants fold** onto their plain forms. + +`/blacklist_check` accepts text of any length. Entries themselves are capped at +200 bytes — roughly 200 plain letters, or about 65 Vietnamese characters. + +## Listing + +`/blacklist_rules` prints both lists in one message, each with its entry count, +showing entries as they were typed rather than in the normalized form. Each +entry is tappable to copy, ready to paste into a `_del` command. + +Telegram caps a message at 4096 characters. A list longer than that is trimmed +with a count of what was left out; both headings always appear, so a long +blacklist never hides the whitelist entirely. diff --git a/go.mod b/go.mod index fd4e24c..3609849 100644 --- a/go.mod +++ b/go.mod @@ -10,6 +10,7 @@ require ( github.com/tiennm99/monkeyd-crawler v0.0.0 go.mongodb.org/mongo-driver/v2 v2.7.0 golang.org/x/image v0.45.0 + golang.org/x/text v0.41.0 ) require ( @@ -72,7 +73,6 @@ require ( golang.org/x/crypto v0.54.0 // indirect golang.org/x/sync v0.22.0 // indirect golang.org/x/sys v0.47.0 // indirect - golang.org/x/text v0.41.0 // indirect gopkg.in/yaml.v3 v3.0.1 // indirect ) diff --git a/internal/modules/blacklist/blacklist.go b/internal/modules/blacklist/blacklist.go new file mode 100644 index 0000000..d20cad7 --- /dev/null +++ b/internal/modules/blacklist/blacklist.go @@ -0,0 +1,91 @@ +// Package blacklist implements a per-thread dictionary of forbidden text and a +// companion list of exceptions that rescue false positives. +// +// The module is passive on purpose. It never reads ordinary chat messages and +// never deletes, warns or restricts anyone: the lists are inert until +// /blacklist_check asks about a specific text. Enforcement would need a +// message-level hook the dispatcher does not have, privacy mode disabled in +// BotFather, and group-admin delete rights — all deliberately out of scope. +// Check is a pure function, so a future hook could call it unchanged. +// +// Scope is one thread: (Chat.ID, MessageThreadID), the same pair the lol module +// keys subscriptions by. Entries added in one forum topic are invisible in the +// next, and a DM is simply the thread (user's chat ID, 0). Within a thread the +// lists are world-writable, the same trust model the alias module uses: the +// list belongs to the conversation, so the permission to edit it does too. +package blacklist + +import ( + "github.com/tiennm99/miti99bot/internal/modules" + "github.com/tiennm99/miti99bot/internal/storage" +) + +// Entry is one stored rule. +// +// The storage key holds the normalized form, so Text carries what the adder +// actually typed — the only place the original casing and spacing survive, and +// what /blacklist_rules shows. +type Entry struct { + Text string `bson:"text"` // as typed, for echoing back + OwnerID int64 `bson:"ownerId"` // who added it + CreatedAt int64 `bson:"createdAt"` // unix millis +} + +// Store is the module's typed view over its collection. Both lists live in it, +// separated by the key prefixes in scope.go. +type Store = storage.DocStore[Entry] + +// state holds what the handlers share. +type state struct { + store Store +} + +// New is the module Factory. +func New(deps modules.Deps) modules.Module { + s := &state{store: storage.Typed[Entry](deps.Store)} + return modules.Module{ + Commands: []modules.Command{ + { + Name: "blacklist_add", + Visibility: modules.VisibilityPublic, + Description: "Blacklist a text, or a message you reply to", + Parameters: "[text...]", + Handler: s.handleAdd(listBlack), + }, + { + Name: "blacklist_del", + Visibility: modules.VisibilityPublic, + Description: "Remove a text from the blacklist", + Parameters: "", + Handler: s.handleDel(listBlack), + }, + { + Name: "whitelist_add", + Visibility: modules.VisibilityPublic, + Description: "Whitelist a text, or a message you reply to", + Parameters: "[text...]", + Handler: s.handleAdd(listWhite), + }, + { + Name: "whitelist_del", + Visibility: modules.VisibilityPublic, + Description: "Remove a text from the whitelist", + Parameters: "", + Handler: s.handleDel(listWhite), + }, + { + Name: "blacklist_rules", + Visibility: modules.VisibilityPublic, + Description: "List both lists for this topic", + Handler: s.handleRules, + }, + { + Name: "blacklist_check", + Visibility: modules.VisibilityPublic, + Description: "Check if a text is blacklisted here", + Parameters: "", + Handler: s.handleCheck, + }, + }, + } +} diff --git a/internal/modules/blacklist/handlers.go b/internal/modules/blacklist/handlers.go new file mode 100644 index 0000000..14fbfbf --- /dev/null +++ b/internal/modules/blacklist/handlers.go @@ -0,0 +1,386 @@ +package blacklist + +import ( + "context" + "errors" + "fmt" + "html" + "sort" + "strings" + "time" + + "github.com/go-telegram/bot" + "github.com/go-telegram/bot/models" + + "github.com/tiennm99/miti99bot/internal/log" + "github.com/tiennm99/miti99bot/internal/modules" + "github.com/tiennm99/miti99bot/internal/modules/util/chathelper" + "github.com/tiennm99/miti99bot/internal/storage" +) + +const ( + // handlerTimeout bounds every handler. The bot dispatches updates inline on + // a single worker with no deadline of its own, so without this the + // library's 60s per-call HTTP ceiling is the only bound. These handlers + // only touch storage, so the budget is generous. + handlerTimeout = 10 * time.Second + + // maxListBytes keeps /blacklist_rules inside Telegram's 4096-character + // sendMessage limit, with room for the second heading and a trim notice + // after the budget is spent. + // + // The budget counts the markup, not only the entries: Telegram + // measures the message it is sent, and at 13 bytes a pair the tags outweigh + // a short entry. + maxListBytes = 3800 +) + +const genericFailure = "Something went wrong. Try again in a moment." + +// The three ways resolveText can fail, kept apart because saying which one +// happened is most of the value of the reply. +var ( + errNoText = errors.New("blacklist: nothing to read text from") + errEmptyText = errors.New("blacklist: text normalizes to nothing") + errLongText = errors.New("blacklist: text too long to store as a rule") +) + +// threadOf returns the scope of a message: one forum topic, or the whole chat. +// +// MessageThreadID on its own is not enough to identify a topic. Telegram +// associates a thread id with any reply chain in a supergroup, not only with a +// forum topic, so trusting the field alone would give a reply-form +// /blacklist_add its own scope — one that a later standalone /blacklist_rules +// in the same chat could never read back. IsTopicMessage is the flag that marks +// a real forum topic, so everything else — a plain group, a DM, a forum's +// General topic — is thread 0. +func threadOf(msg *models.Message) (int64, int) { + if !msg.IsTopicMessage { + return msg.Chat.ID, 0 + } + return msg.Chat.ID, msg.MessageThreadID +} + +// listName names a list tag for a sentence, listTitle for a heading. +func listName(list string) string { + if list == listWhite { + return "whitelist" + } + return "blacklist" +} + +func listTitle(list string) string { + if list == listWhite { + return "Whitelist" + } + return "Blacklist" +} + +// textOf reads the text of a message, falling back to a caption so replying to +// a captioned photo works the same way as replying to a plain message. +func textOf(msg *models.Message) string { + if msg.Text != "" { + return msg.Text + } + return msg.Caption +} + +// resolveText picks the entry text out of an update: the command argument when +// there is one, otherwise — when allowReply — the replied-to message's text. +// +// It returns the raw text for echoing back and the normalized form for keying. +// The length cap is checked against both: a user reads the raw text they typed, +// while the key is built from the normalized one, and NFKC can expand as easily +// as it can contract. +func resolveText(msg *models.Message, allowReply bool) (raw, normText string, err error) { + raw = chathelper.ArgAfterCommand(msg.Text) + if raw == "" && allowReply && msg.ReplyToMessage != nil { + raw = textOf(msg.ReplyToMessage) + } + raw = strings.TrimSpace(raw) + + switch { + case raw == "": + return "", "", errNoText + case len(raw) > maxEntryBytes: + return "", "", errLongText + } + + normText, ok := Normalize(raw) + switch { + case !ok: + // Defensive: TrimSpace and Normalize agree on what whitespace is, so + // non-empty raw text should always normalize to something. + return "", "", errEmptyText + case len(normText) > maxEntryBytes: + return "", "", errLongText + } + return raw, normText, nil +} + +// usageFor words a resolveText failure for the user. +func usageFor(command string, err error, allowReply bool) string { + switch { + case errors.Is(err, errLongText): + return fmt.Sprintf( + "That text is too long to keep as a rule. Keep it to at most %d bytes — roughly %d plain letters, or a third of that in Vietnamese.", + maxEntryBytes, maxEntryBytes) + case errors.Is(err, errEmptyText): + return "There is nothing in that text to store as a rule." + case allowReply: + return fmt.Sprintf("Usage: /%s [text...] — or reply to a message with /%s.", command, command) + default: + return fmt.Sprintf("Usage: /%s ", command) + } +} + +// get reads an entry. A missing key is not an error — it is the normal state +// for text nobody has listed. +func (s *state) get(ctx context.Context, key string) (Entry, bool, error) { + entry, _, err := s.store.Get(ctx, key) + if errors.Is(err, storage.ErrNotFound) { + return Entry{}, false, nil + } + if err != nil { + return Entry{}, false, err + } + return entry, true, nil +} + +// handleAdd stores text in one of the thread's two lists. +func (s *state) handleAdd(list string) modules.CommandHandler { + command := listName(list) + "_add" + return func(ctx context.Context, b *bot.Bot, update *models.Update) error { + ctx, cancel := context.WithTimeout(ctx, handlerTimeout) + defer cancel() + + msg := update.Message + if msg == nil { + return nil + } + + raw, normText, err := resolveText(msg, true) + if err != nil { + return chathelper.Reply(ctx, b, msg, usageFor(command, err, true)) + } + + chatID, threadID := threadOf(msg) + key := entryKey(chatID, threadID, list, normText) + + // Read before writing purely to word the reply. The write is + // unconditional either way, so a concurrent add costs a wrong verb in + // one sentence, not wrong stored state. + existing, found, err := s.get(ctx, key) + if err != nil { + log.Error("blacklist_add_lookup", "list", list, "err", err) + return chathelper.Reply(ctx, b, msg, genericFailure) + } + if found { + return chathelper.ReplyHTML(ctx, b, msg, fmt.Sprintf( + "%s is already in this topic's %s.", + html.EscapeString(existing.Text), listName(list))) + } + + entry := Entry{Text: raw, CreatedAt: chathelper.NowMillis()} + if msg.From != nil { + entry.OwnerID = msg.From.ID + } + if err := s.store.Put(ctx, key, entry); err != nil { + log.Error("blacklist_add", "list", list, "err", err) + return chathelper.Reply(ctx, b, msg, genericFailure) + } + return chathelper.ReplyHTML(ctx, b, msg, fmt.Sprintf( + "Added %s to this topic's %s.", + html.EscapeString(raw), listName(list))) + } +} + +// handleDel removes text from one of the thread's two lists. +// +// Anyone may remove anyone's entry. The list belongs to the thread, so the +// permission model does too; a per-owner rule would strand entries whose adder +// has left the group. +func (s *state) handleDel(list string) modules.CommandHandler { + command := listName(list) + "_del" + return func(ctx context.Context, b *bot.Bot, update *models.Update) error { + ctx, cancel := context.WithTimeout(ctx, handlerTimeout) + defer cancel() + + msg := update.Message + if msg == nil { + return nil + } + + raw, normText, err := resolveText(msg, false) + if err != nil { + return chathelper.Reply(ctx, b, msg, usageFor(command, err, false)) + } + + chatID, threadID := threadOf(msg) + key := entryKey(chatID, threadID, list, normText) + + // Read first so absent text is reported as such. Delete on a missing + // key is indistinguishable from a successful one in the store + // contract, and "removed" for something that was never there reads as + // a bug. + if _, found, err := s.get(ctx, key); err != nil { + log.Error("blacklist_del_lookup", "list", list, "err", err) + return chathelper.Reply(ctx, b, msg, genericFailure) + } else if !found { + return chathelper.ReplyHTML(ctx, b, msg, fmt.Sprintf( + "%s is not in this topic's %s.", + html.EscapeString(raw), listName(list))) + } + + if err := s.store.Delete(ctx, key); err != nil { + log.Error("blacklist_del", "list", list, "err", err) + return chathelper.Reply(ctx, b, msg, genericFailure) + } + return chathelper.ReplyHTML(ctx, b, msg, fmt.Sprintf( + "Removed %s from this topic's %s.", + html.EscapeString(raw), listName(list))) + } +} + +// entriesFor returns one list's normalized entries, sorted. +// +// The key holds the normalized text, so this needs no per-entry document read — +// the difference between one round trip and one per rule on the /blacklist_check +// path. +func (s *state) entriesFor(ctx context.Context, chatID int64, threadID int, list string) ([]string, error) { + prefix := scopePrefix(chatID, threadID, list) + keys, err := s.store.List(ctx, prefix) + if err != nil { + return nil, err + } + out := make([]string, 0, len(keys)) + for _, k := range keys { + out = append(out, decodeKeyText(strings.TrimPrefix(k, prefix))) + } + sort.Strings(out) + return out, nil +} + +// handleCheck judges a text against this thread's rules. +// +// Unlike the mutation commands this applies no length cap: judging a long +// message is the point, and nothing here becomes a storage key. +func (s *state) handleCheck(ctx context.Context, b *bot.Bot, update *models.Update) error { + ctx, cancel := context.WithTimeout(ctx, handlerTimeout) + defer cancel() + + msg := update.Message + if msg == nil { + return nil + } + + normText, ok := Normalize(chathelper.ArgAfterCommand(msg.Text)) + if !ok { + return chathelper.Reply(ctx, b, msg, "Usage: /blacklist_check ") + } + + chatID, threadID := threadOf(msg) + black, err := s.entriesFor(ctx, chatID, threadID, listBlack) + if err != nil { + log.Error("blacklist_check_list", "list", listBlack, "err", err) + return chathelper.Reply(ctx, b, msg, genericFailure) + } + white, err := s.entriesFor(ctx, chatID, threadID, listWhite) + if err != nil { + log.Error("blacklist_check_list", "list", listWhite, "err", err) + return chathelper.Reply(ctx, b, msg, genericFailure) + } + + return chathelper.ReplyHTML(ctx, b, msg, renderVerdict(Check(normText, black, white))) +} + +// renderVerdict words the three shapes a Verdict comes in. +// +// The rescued case names both entries rather than just saying "allowed": it is +// the only way someone who added an exception can confirm it is doing anything. +func renderVerdict(v Verdict) string { + switch { + case v.Blocked: + return fmt.Sprintf("🚫 Blacklisted in this topic — matches %s.", + html.EscapeString(v.Entry)) + case v.Entry != "": + return fmt.Sprintf( + "✅ Allowed in this topic. It matches %s, but the whitelist entry %s covers it.", + html.EscapeString(v.Entry), html.EscapeString(v.RescuedBy)) + default: + return "✅ Allowed in this topic — nothing in the blacklist matches." + } +} + +// handleRules lists both of the thread's lists in one message. +func (s *state) handleRules(ctx context.Context, b *bot.Bot, update *models.Update) error { + ctx, cancel := context.WithTimeout(ctx, handlerTimeout) + defer cancel() + + msg := update.Message + if msg == nil { + return nil + } + + chatID, threadID := threadOf(msg) + blackPrefix := scopePrefix(chatID, threadID, listBlack) + whitePrefix := scopePrefix(chatID, threadID, listWhite) + + // Scan rather than List: this needs the stored text of every entry, and + // Scan reads a whole list in one round trip where List would cost a Get per + // rule. Handlers run inline on the bot's single update worker, so a read + // that scales with the entry count is how an ordinary store latency becomes + // a request that expires before it is answered. + black, err := s.store.Scan(ctx, blackPrefix) + if err != nil { + log.Error("blacklist_rules_scan", "list", listBlack, "err", err) + return chathelper.Reply(ctx, b, msg, genericFailure) + } + white, err := s.store.Scan(ctx, whitePrefix) + if err != nil { + log.Error("blacklist_rules_scan", "list", listWhite, "err", err) + return chathelper.Reply(ctx, b, msg, genericFailure) + } + + var sb strings.Builder + renderSection(&sb, listBlack, blackPrefix, black) + sb.WriteString("\n") + renderSection(&sb, listWhite, whitePrefix, white) + return chathelper.ReplyHTML(ctx, b, msg, sb.String()) +} + +// renderSection appends one headed list, trimmed to what is left of the shared +// byte budget. +// +// Both sections draw on the one budget, and the heading is written before the +// budget is consulted, so a blacklist long enough to fill the message still +// leaves the whitelist visibly present rather than silently absent. +// +// Scan returns entries ordered by key, which is their normalized form, so the +// listing is stable across calls without a sort here. +func renderSection(sb *strings.Builder, list, prefix string, docs []storage.Doc[Entry]) { + fmt.Fprintf(sb, "\n%s (%d)", listTitle(list), len(docs)) + if len(docs) == 0 { + sb.WriteString("\n— nothing yet") + return + } + + for i, doc := range docs { + // The record carries what the adder typed; the key carries only the + // normalized form, which is the fallback if a record ever lacks text. + text := doc.Val.Text + if text == "" { + text = decodeKeyText(strings.TrimPrefix(doc.ID, prefix)) + } + + // Wrapped in so tapping an entry copies it ready to paste into a + // _del command. Reserve room for the trim notice before committing to a + // line, so the trim can never be what pushes the message over. + line := "\n" + html.EscapeString(text) + "" + if sb.Len()+len(line) > maxListBytes { + fmt.Fprintf(sb, "\n…and %d more.", len(docs)-i) + return + } + sb.WriteString(line) + } +} diff --git a/internal/modules/blacklist/handlers_test.go b/internal/modules/blacklist/handlers_test.go new file mode 100644 index 0000000..75cc858 --- /dev/null +++ b/internal/modules/blacklist/handlers_test.go @@ -0,0 +1,390 @@ +package blacklist_test + +import ( + "context" + "strings" + "testing" + + "github.com/go-telegram/bot/models" + + "github.com/tiennm99/miti99bot/internal/modules" + "github.com/tiennm99/miti99bot/internal/modules/blacklist" + "github.com/tiennm99/miti99bot/internal/storage" + "github.com/tiennm99/miti99bot/internal/testutil" +) + +// installBlacklist builds a registry holding only this module. Every command is +// public, so no auth is needed for them to dispatch. +func installBlacklist(t *testing.T) *testutil.RecordingBot { + t.Helper() + rb := testutil.NewRecordingBot(t) + reg, err := modules.Build([]string{"blacklist"}, + map[string]modules.Factory{"blacklist": blacklist.New}, + storage.NewMemoryProvider(), modules.BuildOptions{}) + if err != nil { + t.Fatalf("Build: %v", err) + } + modules.Install(rb.Bot, reg, modules.Auth{}) + return rb +} + +// inTopic builds a supergroup message inside a forum topic. +func inTopic(chatID int64, threadID int, text string) *models.Update { + upd := testutil.NewSupergroupMessage(chatID, 7, text) + upd.Message.MessageThreadID = threadID + upd.Message.IsTopicMessage = true + return upd +} + +// send dispatches one command and returns the text of the reply it produced. +func send(t *testing.T, rb *testutil.RecordingBot, upd *models.Update) string { + t.Helper() + rb.Reset() + rb.Bot.ProcessUpdate(context.Background(), upd) + sent := rb.Sent() + if len(sent) == 0 { + t.Fatalf("no reply to %q", upd.Message.Text) + } + return sent[len(sent)-1].Text() +} + +func TestAdd_StoresAsTypedAndKeysNormalized(t *testing.T) { + rb := installBlacklist(t) + + if got := send(t, rb, testutil.NewPrivateMessage(7, "/blacklist_add Cat Dog ")); !strings.Contains(got, "Added") { + t.Fatalf("add reply = %q", got) + } + + // Matching is normalized, so a differently cased and spaced text hits it. + if got := send(t, rb, testutil.NewPrivateMessage(7, "/blacklist_check CAT DOG")); !strings.Contains(got, "🚫") { + t.Fatalf("check reply = %q; want blocked", got) + } + + // Listing shows what was typed, not the normalized form. + if got := send(t, rb, testutil.NewPrivateMessage(7, "/blacklist_rules")); !strings.Contains(got, "Cat Dog") { + t.Fatalf("rules reply = %q; want the text as typed", got) + } +} + +func TestThreadsAndChatsAreIsolated(t *testing.T) { + rb := installBlacklist(t) + send(t, rb, inTopic(-100, 11, "/blacklist_add cat")) + + // A sibling topic of the same forum. + if got := send(t, rb, inTopic(-100, 12, "/blacklist_check cat")); strings.Contains(got, "🚫") { + t.Fatalf("topic 12 saw topic 11's entry: %q", got) + } + // A DM, which is its own scope entirely. + if got := send(t, rb, testutil.NewPrivateMessage(7, "/blacklist_check cat")); strings.Contains(got, "🚫") { + t.Fatalf("DM saw a group entry: %q", got) + } + // The original topic still has it. + if got := send(t, rb, inTopic(-100, 11, "/blacklist_check cat")); !strings.Contains(got, "🚫") { + t.Fatalf("topic 11 lost its own entry: %q", got) + } +} + +func TestAdd_FromReply(t *testing.T) { + rb := installBlacklist(t) + upd := testutil.NewPrivateMessage(7, "/blacklist_add") + upd.Message.ReplyToMessage = &models.Message{Text: "xin chào"} + + if got := send(t, rb, upd); !strings.Contains(got, "xin chào") { + t.Fatalf("add reply = %q; want the replied text", got) + } + if got := send(t, rb, testutil.NewPrivateMessage(7, "/blacklist_check Xin Chào")); !strings.Contains(got, "🚫") { + t.Fatalf("check reply = %q; want blocked", got) + } +} + +func TestAdd_FromReplyCaption(t *testing.T) { + rb := installBlacklist(t) + upd := testutil.NewPrivateMessage(7, "/blacklist_add") + upd.Message.ReplyToMessage = &models.Message{Caption: "captioned"} + + if got := send(t, rb, upd); !strings.Contains(got, "captioned") { + t.Fatalf("add reply = %q; want the caption stored", got) + } +} + +func TestAdd_WithoutTextOrReplyIsUsage(t *testing.T) { + rb := installBlacklist(t) + got := send(t, rb, testutil.NewPrivateMessage(7, "/blacklist_add")) + if !strings.Contains(got, "Usage") || !strings.Contains(got, "reply") { + t.Fatalf("reply = %q; want usage mentioning the reply form", got) + } +} + +func TestAdd_RefusesTooLongReply(t *testing.T) { + rb := installBlacklist(t) + upd := testutil.NewPrivateMessage(7, "/blacklist_add") + upd.Message.ReplyToMessage = &models.Message{Text: strings.Repeat("a", 4096)} + + got := send(t, rb, upd) + if !strings.Contains(got, "too long") { + t.Fatalf("reply = %q; want a length refusal", got) + } + if rules := send(t, rb, testutil.NewPrivateMessage(7, "/blacklist_rules")); strings.Contains(rules, "aaa") { + t.Fatal("an over-long entry was stored anyway") + } +} + +func TestAdd_DuplicateIsReportedNotDoubled(t *testing.T) { + rb := installBlacklist(t) + send(t, rb, testutil.NewPrivateMessage(7, "/blacklist_add cat")) + + if got := send(t, rb, testutil.NewPrivateMessage(7, "/blacklist_add CAT")); !strings.Contains(got, "already") { + t.Fatalf("reply = %q; want an already-present notice", got) + } + if got := send(t, rb, testutil.NewPrivateMessage(7, "/blacklist_rules")); !strings.Contains(got, "Blacklist (1)") { + t.Fatalf("rules reply = %q; want exactly one entry", got) + } +} + +func TestDel_RemovesAndReportsAbsence(t *testing.T) { + rb := installBlacklist(t) + send(t, rb, testutil.NewPrivateMessage(7, "/blacklist_add cat")) + + if got := send(t, rb, testutil.NewPrivateMessage(7, "/blacklist_del CAT")); !strings.Contains(got, "Removed") { + t.Fatalf("reply = %q; want a removal", got) + } + if got := send(t, rb, testutil.NewPrivateMessage(7, "/blacklist_del cat")); !strings.Contains(got, "is not in") { + t.Fatalf("reply = %q; want an absence notice, not a removal", got) + } +} + +// _del takes its text as an argument only, so a bare invocation is usage even +// when it replies to something. +func TestDel_DoesNotTakeTextFromAReply(t *testing.T) { + rb := installBlacklist(t) + send(t, rb, testutil.NewPrivateMessage(7, "/blacklist_add cat")) + + upd := testutil.NewPrivateMessage(7, "/blacklist_del") + upd.Message.ReplyToMessage = &models.Message{Text: "cat"} + if got := send(t, rb, upd); !strings.Contains(got, "Usage") { + t.Fatalf("reply = %q; want usage", got) + } + if got := send(t, rb, testutil.NewPrivateMessage(7, "/blacklist_check cat")); !strings.Contains(got, "🚫") { + t.Fatal("the entry was removed via a reply") + } +} + +func TestLists_AreSeparate(t *testing.T) { + rb := installBlacklist(t) + send(t, rb, testutil.NewPrivateMessage(7, "/whitelist_add exception")) + + got := send(t, rb, testutil.NewPrivateMessage(7, "/blacklist_rules")) + if !strings.Contains(got, "Blacklist (0)") { + t.Fatalf("rules reply = %q; want an empty blacklist", got) + } + if !strings.Contains(got, "Whitelist (1)") { + t.Fatalf("rules reply = %q; want the whitelist entry", got) + } +} + +func TestRules_EmptyShowsBothHeadings(t *testing.T) { + rb := installBlacklist(t) + got := send(t, rb, testutil.NewPrivateMessage(7, "/blacklist_rules")) + for _, want := range []string{"Blacklist (0)", "Whitelist (0)", "nothing yet"} { + if !strings.Contains(got, want) { + t.Fatalf("rules reply = %q; missing %q", got, want) + } + } +} + +func TestRules_EscapesUserText(t *testing.T) { + rb := installBlacklist(t) + send(t, rb, testutil.NewPrivateMessage(7, "/blacklist_add bold")) + + got := send(t, rb, testutil.NewPrivateMessage(7, "/blacklist_rules")) + if strings.Contains(got, "bold") { + t.Fatalf("rules reply = %q; user markup was not escaped", got) + } + if !strings.Contains(got, "<b>bold</b>") { + t.Fatalf("rules reply = %q; want the escaped form", got) + } +} + +func TestRules_TrimsToOneMessage(t *testing.T) { + rb := installBlacklist(t) + const entries = 400 + for i := range entries { + send(t, rb, testutil.NewPrivateMessage(7, + "/blacklist_add "+strings.Repeat("x", 30)+string(rune('a'+i%26))+strings.Repeat("y", i%7))) + } + + got := send(t, rb, testutil.NewPrivateMessage(7, "/blacklist_rules")) + if len([]rune(got)) > 4096 { + t.Fatalf("rules reply is %d characters, over Telegram's limit", len([]rune(got))) + } + if !strings.Contains(got, "more.") { + t.Fatalf("rules reply = %q; want a trim notice", got) + } + // Both headings survive a blacklist long enough to fill the message. + if !strings.Contains(got, "Whitelist") { + t.Fatal("the whitelist heading was trimmed away entirely") + } +} + +// The case the containment rule exists for, end to end through the handlers. +func TestCheck_WhitelistRescuesOnlyWhatItSpans(t *testing.T) { + rb := installBlacklist(t) + send(t, rb, testutil.NewPrivateMessage(7, "/blacklist_add ass")) + send(t, rb, testutil.NewPrivateMessage(7, "/whitelist_add assassin")) + + tests := []struct { + text string + blocked bool + }{ + {text: "assassin", blocked: false}, + {text: "dumbass", blocked: true}, + {text: "I met an assassin, dumbass", blocked: true}, + {text: "nothing here", blocked: false}, + } + for _, tc := range tests { + t.Run(tc.text, func(t *testing.T) { + got := send(t, rb, testutil.NewPrivateMessage(7, "/blacklist_check "+tc.text)) + if blocked := strings.Contains(got, "🚫"); blocked != tc.blocked { + t.Fatalf("check %q = %q; want blocked=%v", tc.text, got, tc.blocked) + } + }) + } +} + +func TestCheck_NamesTheRescuingEntry(t *testing.T) { + rb := installBlacklist(t) + send(t, rb, testutil.NewPrivateMessage(7, "/blacklist_add ass")) + send(t, rb, testutil.NewPrivateMessage(7, "/whitelist_add assassin")) + + got := send(t, rb, testutil.NewPrivateMessage(7, "/blacklist_check assassin")) + if !strings.Contains(got, "ass") || !strings.Contains(got, "assassin") { + t.Fatalf("check reply = %q; want both entries named", got) + } +} + +func TestCheck_EmptyListsAllowEverything(t *testing.T) { + rb := installBlacklist(t) + if got := send(t, rb, testutil.NewPrivateMessage(7, "/blacklist_check anything")); !strings.Contains(got, "✅") { + t.Fatalf("check reply = %q; want allowed", got) + } +} + +func TestCheck_WithoutTextIsUsage(t *testing.T) { + rb := installBlacklist(t) + if got := send(t, rb, testutil.NewPrivateMessage(7, "/blacklist_check")); !strings.Contains(got, "Usage") { + t.Fatalf("check reply = %q; want usage", got) + } +} + +// Diacritics are significant by design: "ma" and "má" are separate entries. +func TestCheck_DiacriticsAreSignificant(t *testing.T) { + rb := installBlacklist(t) + send(t, rb, testutil.NewPrivateMessage(7, "/blacklist_add ma")) + + if got := send(t, rb, testutil.NewPrivateMessage(7, "/blacklist_check má")); strings.Contains(got, "🚫") { + t.Fatalf("check reply = %q; \"má\" must not match the entry \"ma\"", got) + } + if got := send(t, rb, testutil.NewPrivateMessage(7, "/blacklist_check MA")); !strings.Contains(got, "🚫") { + t.Fatalf("check reply = %q; case must still fold", got) + } +} + +// An entry containing the characters that cannot appear literally in a storage +// key must survive the round trip through the store. +func TestEntry_WithKeyHazardsRoundTrips(t *testing.T) { + rb := installBlacklist(t) + const hazard = "50%2F/off" + send(t, rb, testutil.NewPrivateMessage(7, "/blacklist_add "+hazard)) + + if got := send(t, rb, testutil.NewPrivateMessage(7, "/blacklist_rules")); !strings.Contains(got, hazard) { + t.Fatalf("rules reply = %q; want %q intact", got, hazard) + } + if got := send(t, rb, testutil.NewPrivateMessage(7, "/blacklist_check "+hazard)); !strings.Contains(got, "🚫") { + t.Fatalf("check reply = %q; want blocked", got) + } + if got := send(t, rb, testutil.NewPrivateMessage(7, "/blacklist_del "+hazard)); !strings.Contains(got, "Removed") { + t.Fatalf("del reply = %q; want a removal", got) + } +} + +// Replies use parse_mode HTML, so every site that echoes user text must escape +// it. These pin the three sites outside /blacklist_rules. +func TestAddAndDel_EscapeUserText(t *testing.T) { + rb := installBlacklist(t) + + add := send(t, rb, testutil.NewPrivateMessage(7, "/blacklist_add bold")) + if strings.Contains(add, "bold") || !strings.Contains(add, "<b>bold</b>") { + t.Fatalf("add reply = %q; want the markup escaped", add) + } + + dup := send(t, rb, testutil.NewPrivateMessage(7, "/blacklist_add bold")) + if strings.Contains(dup, "bold") { + t.Fatalf("duplicate reply = %q; want the markup escaped", dup) + } + + del := send(t, rb, testutil.NewPrivateMessage(7, "/blacklist_del bold")) + if strings.Contains(del, "bold") || !strings.Contains(del, "<b>bold</b>") { + t.Fatalf("del reply = %q; want the markup escaped", del) + } + + absent := send(t, rb, testutil.NewPrivateMessage(7, "/blacklist_del bold")) + if strings.Contains(absent, "bold") { + t.Fatalf("absence reply = %q; want the markup escaped", absent) + } +} + +func TestCheck_EscapesEntryNames(t *testing.T) { + rb := installBlacklist(t) + send(t, rb, testutil.NewPrivateMessage(7, "/blacklist_add ")) + + blocked := send(t, rb, testutil.NewPrivateMessage(7, "/blacklist_check xy")) + if strings.Contains(blocked, "") || !strings.Contains(blocked, "<b>") { + t.Fatalf("blocked verdict = %q; want the entry escaped", blocked) + } + + // The rescued branch names two entries; both must be escaped. + send(t, rb, testutil.NewPrivateMessage(7, "/whitelist_add xy")) + rescued := send(t, rb, testutil.NewPrivateMessage(7, "/blacklist_check xy")) + if !strings.Contains(rescued, "✅") { + t.Fatalf("verdict = %q; want the rescued branch", rescued) + } + if strings.Contains(rescued, "") || strings.Contains(rescued, "xy") { + t.Fatalf("rescued verdict = %q; want both entries escaped", rescued) + } +} + +// NFKC can expand as easily as it can contract, so the byte cap has to be +// applied to the normalized text as well as to what the user typed. These ten +// runes are 30 bytes as sent and 330 once normalized. +func TestAdd_RefusesTextThatExpandsPastTheCap(t *testing.T) { + rb := installBlacklist(t) + raw := strings.Repeat("ﷺ", 10) + + if got := send(t, rb, testutil.NewPrivateMessage(7, "/blacklist_add "+raw)); !strings.Contains(got, "too long") { + t.Fatalf("reply = %q; want a length refusal", got) + } + if got := send(t, rb, testutil.NewPrivateMessage(7, "/blacklist_rules")); !strings.Contains(got, "Blacklist (0)") { + t.Fatalf("rules reply = %q; the over-long entry was stored anyway", got) + } +} + +// Telegram sets a thread id for reply chains in ordinary supergroups too, not +// only for forum topics. Those must fall back to the chat-wide list, or text +// added by replying would land somewhere a plain /blacklist_rules cannot read. +func TestReplyChainThreadIsNotATopic(t *testing.T) { + rb := installBlacklist(t) + + // A reply in a non-forum supergroup: thread id set, IsTopicMessage false. + add := testutil.NewSupergroupMessage(-100, 7, "/blacklist_add") + add.Message.MessageThreadID = 4242 + add.Message.ReplyToMessage = &models.Message{Text: "cat"} + send(t, rb, add) + + // A later command with no thread id at all must still see it. + if got := send(t, rb, testutil.NewSupergroupMessage(-100, 7, "/blacklist_check cat")); !strings.Contains(got, "🚫") { + t.Fatalf("check reply = %q; a reply-chain add was stored out of reach", got) + } + if got := send(t, rb, testutil.NewSupergroupMessage(-100, 7, "/blacklist_rules")); !strings.Contains(got, "cat") { + t.Fatalf("rules reply = %q; want the entry listed", got) + } +} diff --git a/internal/modules/blacklist/match.go b/internal/modules/blacklist/match.go new file mode 100644 index 0000000..69f9f45 --- /dev/null +++ b/internal/modules/blacklist/match.go @@ -0,0 +1,110 @@ +package blacklist + +import ( + "slices" + "strings" +) + +// Verdict is the outcome of checking one text against one thread's rules. +// +// Entry is set whenever some blacklist entry occurred in the text, blocked or +// not: a rescued match is worth naming, because it is the only way a user can +// confirm an exception they added is doing anything. +type Verdict struct { + Blocked bool + Entry string // the blacklist entry that decided the verdict; "" when none matched + RescuedBy string // the whitelist entry covering Entry; set only when !Blocked && Entry != "" +} + +// span is a half-open byte range within the text being checked. +type span struct{ start, end int } + +// allowSpan is a whitelist occurrence, carrying the entry that produced it so a +// rescue can be reported by name. +type allowSpan struct { + span + entry string +} + +// Check judges normText against two lists of normalized entries. +// +// The rule is span containment, not mere presence: a whitelist entry rescues a +// blacklist match only when it occurs in the text at a range that contains that +// match. The tempting shortcut — allow the text if any whitelist entry appears +// anywhere in it — handles "assassin" correctly and then lets "I met an +// assassin, dumbass" through, because the rescuing span never reaches the +// second match. A whitelist entry that merely overlaps a match without +// containing it does not rescue it. +// +// Both slices are copied and sorted before scanning. Callers read them from +// DocStore.List, which guarantees no ordering, so without this the same text +// could be reported against a different entry on each invocation. +func Check(normText string, blacklist, whitelist []string) Verdict { + if normText == "" { + return Verdict{} + } + + allowed := allowSpans(normText, whitelist) + + // The first rescued match, kept in case no unrescued one is ever found. + var rescued Verdict + + for _, entry := range slices.Sorted(slices.Values(blacklist)) { + if entry == "" { + continue + } + for _, s := range occurrences(normText, entry) { + by, ok := coveredBy(s, allowed) + if !ok { + return Verdict{Blocked: true, Entry: entry} + } + if rescued.Entry == "" { + rescued = Verdict{Entry: entry, RescuedBy: by} + } + } + } + return rescued +} + +// allowSpans collects every occurrence of every whitelist entry. +func allowSpans(text string, whitelist []string) []allowSpan { + var out []allowSpan + for _, entry := range slices.Sorted(slices.Values(whitelist)) { + if entry == "" { + continue + } + for _, s := range occurrences(text, entry) { + out = append(out, allowSpan{span: s, entry: entry}) + } + } + return out +} + +// occurrences returns every range at which sub appears in s. +// +// The scan advances one byte past each hit rather than past the whole match, so +// overlapping occurrences are all reported — "aa" occurs twice in "aaa", and a +// containment test that saw only the first would be wrong. +func occurrences(s, sub string) []span { + var out []span + for off := 0; off < len(s); { + i := strings.Index(s[off:], sub) + if i < 0 { + break + } + start := off + i + out = append(out, span{start: start, end: start + len(sub)}) + off = start + 1 + } + return out +} + +// coveredBy reports the whitelist entry whose span contains s, if any. +func coveredBy(s span, allowed []allowSpan) (string, bool) { + for _, a := range allowed { + if a.start <= s.start && s.end <= a.end { + return a.entry, true + } + } + return "", false +} diff --git a/internal/modules/blacklist/match_test.go b/internal/modules/blacklist/match_test.go new file mode 100644 index 0000000..9b2c2fd --- /dev/null +++ b/internal/modules/blacklist/match_test.go @@ -0,0 +1,84 @@ +package blacklist + +import "testing" + +// The case the whole containment rule exists for. A whitelist entry rescues +// only the match it spans, so a text carrying both a rescued and an unrescued +// occurrence is still blocked. +func TestCheck_WhitelistRescuesOnlyWhatItSpans(t *testing.T) { + black := []string{"ass"} + white := []string{"assassin"} + + tests := []struct { + name string + text string + blocked bool + entry string + rescuedBy string + }{ + {name: "rescued", text: "assassin", blocked: false, entry: "ass", rescuedBy: "assassin"}, + {name: "not rescued", text: "dumbass", blocked: true, entry: "ass"}, + {name: "one of each", text: "i met an assassin, dumbass", blocked: true, entry: "ass"}, + {name: "no match at all", text: "hello there"}, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + got := Check(tc.text, black, white) + if got.Blocked != tc.blocked || got.Entry != tc.entry || got.RescuedBy != tc.rescuedBy { + t.Fatalf("Check(%q) = %+v; want blocked=%v entry=%q rescuedBy=%q", + tc.text, got, tc.blocked, tc.entry, tc.rescuedBy) + } + }) + } +} + +func TestCheck_WithoutWhitelistNothingIsRescued(t *testing.T) { + if got := Check("assassin", []string{"ass"}, nil); !got.Blocked { + t.Fatalf("Check = %+v; want blocked", got) + } +} + +func TestCheck_IdenticalWhitelistEntryRescuesWholeText(t *testing.T) { + got := Check("cat", []string{"cat"}, []string{"cat"}) + if got.Blocked || got.Entry != "cat" || got.RescuedBy != "cat" { + t.Fatalf("Check = %+v; want rescued by an identical entry", got) + } +} + +// A whitelist entry that overlaps a match without containing it is not a +// rescue: "bca" covers only the tail of the "ab" at index 0. +func TestCheck_OverlapWithoutContainmentDoesNotRescue(t *testing.T) { + if got := Check("abca", []string{"ab"}, []string{"bca"}); !got.Blocked { + t.Fatalf("Check = %+v; want blocked", got) + } +} + +// Overlapping occurrences of one entry must all be considered — scanning by +// whole-match strides would miss the second "aa" here and wrongly allow it. +func TestCheck_FindsOverlappingOccurrences(t *testing.T) { + // "xaa" is whitelisted, spanning the first "aa" only. + if got := Check("xaaa", []string{"aa"}, []string{"xaa"}); !got.Blocked { + t.Fatalf("Check = %+v; want blocked by the second, unspanned occurrence", got) + } +} + +func TestCheck_EmptyTextMatchesNothing(t *testing.T) { + if got := Check("", []string{"cat"}, nil); got != (Verdict{}) { + t.Fatalf("Check = %+v; want zero verdict", got) + } +} + +// DocStore.List gives no ordering guarantee, so the verdict must not depend on +// the order entries arrive in. +func TestCheck_IsOrderIndependent(t *testing.T) { + text := "the cat and the dog" + forward := Check(text, []string{"cat", "dog"}, nil) + reverse := Check(text, []string{"dog", "cat"}, nil) + if forward != reverse { + t.Fatalf("order changed the verdict: %+v vs %+v", forward, reverse) + } + if forward.Entry != "cat" { + t.Fatalf("Entry = %q; want the first entry in sorted order", forward.Entry) + } +} diff --git a/internal/modules/blacklist/normalize.go b/internal/modules/blacklist/normalize.go new file mode 100644 index 0000000..0e71daa --- /dev/null +++ b/internal/modules/blacklist/normalize.go @@ -0,0 +1,31 @@ +package blacklist + +import ( + "strings" + + "golang.org/x/text/unicode/norm" +) + +// Normalize converts user text to the single form used both as a storage key +// and as matcher input. It returns ok=false when nothing comparable is left. +// +// The three steps, in this order: +// +// 1. NFKC. Canonical composition is what makes Vietnamese portable: some +// clients send "má" as "m" + "a" + combining acute (NFD) while others send +// the precomposed character (NFC), and without this they are different +// strings that would never match each other. The compatibility half also +// folds full-width and other presentation variants onto their plain forms. +// It does not strip combining marks — "ma" and "má" stay distinct entries, +// which is the behaviour this module wants. +// 2. Lowercase, applied after normalization because case folding can otherwise +// interact with composition. +// 3. Whitespace collapse, so "cat dog" and "cat dog" are one rule and a +// newline is just a separator. +func Normalize(s string) (string, bool) { + fields := strings.Fields(strings.ToLower(norm.NFKC.String(s))) + if len(fields) == 0 { + return "", false + } + return strings.Join(fields, " "), true +} diff --git a/internal/modules/blacklist/normalize_test.go b/internal/modules/blacklist/normalize_test.go new file mode 100644 index 0000000..641bef9 --- /dev/null +++ b/internal/modules/blacklist/normalize_test.go @@ -0,0 +1,57 @@ +package blacklist + +import "testing" + +func TestNormalize_ComposesVietnamese(t *testing.T) { + // The same word as a client sending NFC and a client sending NFD. Without + // composition these are different strings and would never match. + nfc := "má" // U+00E1 precomposed + nfd := "má" // "a" + combining acute + gotNFC, ok := Normalize(nfc) + if !ok { + t.Fatal("NFC form rejected") + } + gotNFD, ok := Normalize(nfd) + if !ok { + t.Fatal("NFD form rejected") + } + if gotNFC != gotNFD { + t.Fatalf("NFC %q and NFD %q normalize differently", gotNFC, gotNFD) + } +} + +func TestNormalize_FoldsCaseButKeepsDiacritics(t *testing.T) { + upper, _ := Normalize("MÁ") + lower, _ := Normalize("má") + if upper != lower { + t.Fatalf("case not folded: %q vs %q", upper, lower) + } + + bare, _ := Normalize("ma") + if bare == lower { + t.Fatalf("diacritic was stripped: %q == %q", bare, lower) + } +} + +func TestNormalize_FoldsCompatibilityVariants(t *testing.T) { + full, _ := Normalize("ＡＢ") // full-width AB + plain, _ := Normalize("ab") + if full != plain { + t.Fatalf("full-width %q did not fold to %q", full, plain) + } +} + +func TestNormalize_CollapsesWhitespace(t *testing.T) { + got, ok := Normalize(" cat \n dog ") + if !ok || got != "cat dog" { + t.Fatalf("Normalize = %q, %v; want \"cat dog\", true", got, ok) + } +} + +func TestNormalize_RejectsEmpty(t *testing.T) { + for _, in := range []string{"", " ", "\n\t "} { + if got, ok := Normalize(in); ok { + t.Fatalf("Normalize(%q) = %q, true; want not ok", in, got) + } + } +} diff --git a/internal/modules/blacklist/scope.go b/internal/modules/blacklist/scope.go new file mode 100644 index 0000000..7f412f8 --- /dev/null +++ b/internal/modules/blacklist/scope.go @@ -0,0 +1,63 @@ +package blacklist + +import ( + "strconv" + "strings" +) + +// The two list tags. They are single letters because they sit inside every +// storage key, and because the ':' after them is what ends the key prefix. +const ( + listBlack = "b" + listWhite = "w" +) + +// maxEntryBytes caps one entry, measured on the normalized text before key +// encoding. +// +// Storage keys are limited to 1500 bytes (internal/storage/keys.go). Worst-case +// encoding triples an entry to 600 bytes, which with a prefix well under 30 +// bytes stays far inside that. Two hundred bytes is roughly 65 Vietnamese +// characters — ample for a phrase, and a deliberate refusal to let a pasted +// essay become a key. +const maxEntryBytes = 200 + +// scopePrefix is the key prefix for one list of one thread. +// +// A thread is (Chat.ID, MessageThreadID), the same pair the lol module uses for +// subscriptions. A zero thread id means a non-forum group, a forum's General +// topic, or a DM — a real scope, not a missing value. +// +// Both numbers are decimal and the list tag is a single letter, so the third +// ':' always ends the prefix even though entry text may contain ':' of its own. +func scopePrefix(chatID int64, threadID int, list string) string { + return strconv.FormatInt(chatID, 10) + ":" + strconv.Itoa(threadID) + ":" + list + ":" +} + +// entryKey names one entry. normText must already have been through Normalize. +func entryKey(chatID int64, threadID int, list, normText string) string { + return scopePrefix(chatID, threadID, list) + encodeKeyText(normText) +} + +// encodeKeyText escapes the one character that cannot appear literally in a +// storage key. +// +// '%' is escaped first so the escape marker itself stays unambiguous: reversing +// the order would make an entry containing the literal text "%2F" decode back +// as "/". Because every literal '%' becomes "%25", every '%' in the result +// starts an escape, so decoding can never match one that spans two of them. +// +// The other key rules need no work here: "." , ".." and the __namespace__ +// pattern are all unreachable behind the numeric scope prefix. +func encodeKeyText(s string) string { + s = strings.ReplaceAll(s, "%", "%25") + return strings.ReplaceAll(s, "/", "%2F") +} + +// decodeKeyText reverses encodeKeyText. The order mirrors it: "%2F" can only +// have come from a real '/', so it is consumed before "%25" reconstitutes the +// '%' characters that could otherwise form a spurious escape. +func decodeKeyText(s string) string { + s = strings.ReplaceAll(s, "%2F", "/") + return strings.ReplaceAll(s, "%25", "%") +} diff --git a/internal/modules/blacklist/scope_test.go b/internal/modules/blacklist/scope_test.go new file mode 100644 index 0000000..a1c5a1e --- /dev/null +++ b/internal/modules/blacklist/scope_test.go @@ -0,0 +1,62 @@ +package blacklist + +import ( + "strings" + "testing" +) + +func TestScopePrefix_EndsAfterThreeColons(t *testing.T) { + got := scopePrefix(-1001234567890, 0, listBlack) + if want := "-1001234567890:0:b:"; got != want { + t.Fatalf("scopePrefix = %q, want %q", got, want) + } + if strings.Count(got, ":") != 3 { + t.Fatalf("prefix %q does not carry exactly three colons", got) + } +} + +// Two threads of one chat must produce prefixes where neither is a prefix of +// the other, or a List for one thread would return the other's entries. +func TestScopePrefix_ThreadsDoNotNest(t *testing.T) { + a := scopePrefix(-100, 1, listBlack) + b := scopePrefix(-100, 12, listBlack) + if strings.HasPrefix(b, a) || strings.HasPrefix(a, b) { + t.Fatalf("thread prefixes nest: %q and %q", a, b) + } +} + +func TestScopePrefix_ListsDoNotCollide(t *testing.T) { + if scopePrefix(-100, 0, listBlack) == scopePrefix(-100, 0, listWhite) { + t.Fatal("blacklist and whitelist share a prefix") + } +} + +func TestEncodeKeyText_RoundTrips(t *testing.T) { + for _, in := range []string{ + "plain", + "a/b", + "100%", + "%2F", // the literal text that a naive decoder turns into "/" + "%25", // likewise for "%" + "a:b", // the key delimiter, which needs no escaping + "a\nb", + "%2F/%25%", // every hazard at once + } { + enc := encodeKeyText(in) + if strings.Contains(enc, "/") { + t.Fatalf("encodeKeyText(%q) = %q still contains '/'", in, enc) + } + if got := decodeKeyText(enc); got != in { + t.Fatalf("round trip of %q gave %q (encoded %q)", in, got, enc) + } + } +} + +func TestEntryKey_StaysWithinStorageLimits(t *testing.T) { + // Worst case: an entry at the cap made entirely of characters that encode + // to three bytes each. + key := entryKey(-1001234567890, 999999, listBlack, strings.Repeat("/", maxEntryBytes)) + if len(key) > 1500 { + t.Fatalf("worst-case key is %d bytes, over the 1500-byte storage limit", len(key)) + } +} diff --git a/plans/260915-1108-blacklist-module/phase-01-scope-normalize-match.md b/plans/260915-1108-blacklist-module/phase-01-scope-normalize-match.md new file mode 100644 index 0000000..fb785b8 --- /dev/null +++ b/plans/260915-1108-blacklist-module/phase-01-scope-normalize-match.md @@ -0,0 +1,196 @@ +--- +phase: 1 +title: "Phase 1: Scope keys, normalization, matcher" +status: done +priority: P1 +effort: "3h" +dependencies: [] +--- + +# Phase 1: Scope keys, normalization, matcher + +## Overview + +The whole decision core of the module, with no Telegram and no storage in it. Three pure +files that a test can drive directly: how a thread becomes a key prefix, how arbitrary user +text becomes a comparable and storable form, and how a text is judged against two lists. + +Every correctness risk in the module lives here. Isolating it means the matcher's behaviour +is settled and tested before a single handler exists. + +## Requirements + +- Functional: a `(chatID, threadID, list)` triple maps to a unique, collision-free key + prefix, and an entry's normalized text maps to a key under it that survives + `internal/storage/keys.go` validation. +- Functional: normalization composes Unicode, folds case and collapses whitespace, and + leaves combining diacritics intact. +- Functional: the matcher reports whether a text is blocked, which blacklist entry decided + it, and which whitelist entry rescued it when one did. +- Non-functional: no dependency on `go-telegram/bot` or `internal/storage` in any of the + three files, so the package's core is unit-testable in isolation. +- Non-functional: deterministic output for the same inputs regardless of slice order. + +## Architecture + +### `internal/modules/blacklist/scope.go` + +A thread is `(Chat.ID, MessageThreadID)`, the same pair `lol` already uses for subscriptions +(`internal/modules/lol/subscribers.go:14-24`). `MessageThreadID == 0` means a non-forum +group, a forum's General topic, or a DM — a real scope, not a missing value. + +```go +const ( + listBlack = "b" + listWhite = "w" +) + +// scopePrefix is the key prefix for one list of one thread. Both numbers are +// decimal and the list tag is a single letter, so the third ':' always ends the +// prefix even though entry text may contain ':' of its own. +func scopePrefix(chatID int64, threadID int, list string) string + +// entryKey names one entry. normText must already be normalized. +func entryKey(chatID int64, threadID int, list, normText string) string +``` + +Key shape: `:::`. Group chat IDs are negative, which +costs a leading `-` and nothing else. + +`internal/storage/keys.go:24-39` forbids `/`, the exact strings `.` and `..`, the +`__namespace__` pattern, and keys over 1500 bytes. The numeric prefix makes the middle three +unreachable by construction. `/` and the length cap are handled explicitly: + +```go +// encodeKeyText escapes the two characters that cannot appear literally in a +// key. '%' is escaped first so the escape marker itself stays unambiguous — +// reversing the order would make an entry containing "%2F" decode as '/'. +func encodeKeyText(s string) string // "%" -> "%25", then "/" -> "%2F" +func decodeKeyText(s string) string // "%2F" -> "/", then "%25" -> "%" +``` + +`maxEntryBytes = 200`, measured on the normalized text before encoding. Worst-case encoding +triples it to 600 bytes, which with a prefix under 30 bytes stays far inside 1500. Two +hundred bytes is roughly 65 Vietnamese characters — ample for a phrase, and a deliberate +refusal to let someone paste an essay into a key. + +### `internal/modules/blacklist/normalize.go` + +```go +// Normalize converts user text to the single form used for both storage keys +// and matching. Returns ok=false when nothing comparable is left. +func Normalize(s string) (norm string, ok bool) +``` + +Three steps, in this order: + +1. **NFKC** via `golang.org/x/text/unicode/norm`. Canonical composition is what makes + Vietnamese portable: iOS clients may send `má` as `m` + `a` + combining acute (NFD) while + Android sends the precomposed character (NFC), and without this they are different + strings. The compatibility half additionally folds full-width and other presentation + variants onto their plain forms, which closes an evasion route for free. It does **not** + strip combining marks, so `má` stays `má` — the project owner's choice. +2. **`strings.ToLower`** after normalization, not before, because folding can otherwise + interact with composition. +3. **Whitespace collapse** — `strings.Fields` joined by a single space, which also trims. + `cat dog` and `cat dog` are the same rule; a newline is just whitespace. + +Empty input, or input that is only whitespace, returns `ok=false`. + +`golang.org/x/text` is already at v0.41.0 in `go.sum` as an indirect dependency, so this +promotes an existing line rather than adding a dependency. + +### `internal/modules/blacklist/match.go` + +```go +// Verdict is the outcome of checking one text against one thread's rules. +type Verdict struct { + Blocked bool + Entry string // the blacklist entry that decided the verdict; "" when none matched + RescuedBy string // the whitelist entry covering Entry; set only when !Blocked && Entry != "" +} + +// Check judges normText. Both slices hold normalized entries and are sorted by +// the caller. +func Check(normText string, blacklist, whitelist []string) Verdict +``` + +The algorithm is span containment, not mere presence: + +1. Collect every occurrence of every whitelist entry in `normText` as a `[start, end)` span, + by looping `strings.Index` over the remainder. +2. For each blacklist entry, in sorted order, walk its occurrences. A span is **rescued** + only if some whitelist span `[a, b)` satisfies `a <= start && end <= b`. +3. The first unrescued span wins: return `Blocked: true` naming its entry. +4. If every occurrence was rescued, return `Blocked: false` naming the first rescued entry + and the whitelist entry that covered it. If nothing matched at all, return the zero + `Verdict`. + +The containment rule is the point of the phase. The tempting shortcut — *allow the text if +any whitelist entry appears anywhere in it* — passes the `assassin` case and then lets +`I met an assassin, dumbass` through, because the whitelist span never covers the second +match. A whitelist entry that merely overlaps a blacklist match without containing it does +not rescue it, which is the same rule stated from the other side. + +Sorting matters for more than tidiness: `DocStore.List` gives no ordering guarantee, so +without it the same text could be reported against a different entry on each invocation. + +## Files + +| File | Change | +|---|---| +| `internal/modules/blacklist/scope.go` | new | +| `internal/modules/blacklist/normalize.go` | new | +| `internal/modules/blacklist/match.go` | new | +| `internal/modules/blacklist/scope_test.go` | new | +| `internal/modules/blacklist/normalize_test.go` | new | +| `internal/modules/blacklist/match_test.go` | new | +| `go.mod` | `golang.org/x/text` moves from indirect to direct | + +## Steps + +1. Write `scope.go` with the two key builders and the encode/decode pair. +2. Write `normalize.go`. +3. Write `match.go`. +4. Write the three test files (see Validation). +5. Run `go mod tidy`, confirming the only diff is `golang.org/x/text` changing section. +6. `gofmt` every new file. + +## Validation + +`go test ./internal/modules/blacklist/...` covering at minimum: + +**scope_test.go** +- A negative chat ID and a zero thread ID produce a prefix ending in exactly three `:`. +- Two different threads of the same chat produce prefixes where neither is a prefix of the + other — the property that keeps `List` from leaking across threads. +- `encodeKeyText` then `decodeKeyText` round-trips text containing `/`, `%`, `%2F`, `:` and + a newline. +- A key built from a 200-byte entry passes `storage`'s key rules. + +**normalize_test.go** +- The same Vietnamese word in NFC and NFD normalizes to one string. +- `Má` and `má` normalize alike; `ma` and `má` do **not**. +- A full-width variant folds onto its plain form. +- `" cat dog "` normalizes to `"cat dog"`. +- `""` and `" "` return `ok=false`. + +**match_test.go** — blacklist `ass`, whitelist `assassin` unless stated: +- `assassin` → not blocked, `Entry: "ass"`, `RescuedBy: "assassin"`. +- `dumbass` → blocked. +- `I met an assassin, dumbass` → **blocked**. The regression test for the whole phase. +- `hello` → zero `Verdict`. +- Empty whitelist → `assassin` is blocked. +- An entry equal to the whole text is rescued by an identical whitelist entry. +- Shuffling the input slices does not change which entry a verdict names. + +## Risk + +The containment rule is easy to implement subtly wrong in a way that passes the obvious two +test cases. The mitigation is that the three-clause `assassin`/`dumbass` case is written +first and named for the behaviour it protects. + +## Rollback + +The package has no importers until Phase 4 wires it into `cmd/server`. Delete the directory +and revert the `go.mod` line. diff --git a/plans/260915-1108-blacklist-module/phase-02-store-and-mutations.md b/plans/260915-1108-blacklist-module/phase-02-store-and-mutations.md new file mode 100644 index 0000000..29b68ea --- /dev/null +++ b/plans/260915-1108-blacklist-module/phase-02-store-and-mutations.md @@ -0,0 +1,178 @@ +--- +phase: 2 +title: "Phase 2: Store, factory, mutation commands" +status: done +priority: P1 +effort: "3h" +dependencies: [1] +--- + +# Phase 2: Store, factory, mutation commands + +## Overview + +The module becomes a real `modules.Module`: a persisted record, a factory registering six +commands, and the four that mutate a list. The two read commands land in Phase 3, but all +six are registered here so the command surface is reviewable in one place — the unimplemented +pair point at stub handlers that Phase 3 fills in. + +Blacklist and whitelist share one handler pair parameterised by list tag. They differ only +in which prefix they write to, and duplicating them would be two bugs to fix instead of one. + +## Requirements + +- Functional: `/blacklist_add`, `/whitelist_add` store an entry in the calling thread's list, + taking the text from the argument or, when there is none, from the replied-to message. +- Functional: `/blacklist_del`, `/whitelist_del` remove an entry, distinguishing "removed" + from "was not there". +- Functional: re-adding an existing entry is reported as such rather than silently + overwriting in silence. +- Functional: text that normalizes to nothing, or exceeds `maxEntryBytes`, is refused with a + message saying which. +- Non-functional: every handler is bounded by a timeout; none can stall the single update + worker. +- Non-functional: user text is HTML-escaped at every render site. + +## Architecture + +### `internal/modules/blacklist/blacklist.go` + +Package doc records the two decisions a reader will otherwise re-litigate: the module is +passive by design, and lists are per-thread and world-writable. + +```go +// Entry is one stored rule. The storage key holds the normalized form, so Text +// carries what the adder actually typed — the only place the original casing and +// spacing survive, and what /blacklist_rules shows. +type Entry struct { + Text string `bson:"text"` + OwnerID int64 `bson:"ownerId"` + CreatedAt int64 `bson:"createdAt"` +} + +type Store = storage.DocStore[Entry] + +type state struct{ store Store } + +func New(deps modules.Deps) modules.Module +``` + +No `Registry` in `state`: nothing here needs to introspect commands the way `alias` does. + +Both lists live in the one `Deps.Store` collection, separated by the key prefixes Phase 1 +built. One `storage.Typed[Entry]` view serves both. + +### Thread resolution + +```go +// threadOf returns the scope of the message. A DM is (user's chat ID, 0) and a +// non-forum group is (chat ID, 0); neither needs a special case. +func threadOf(msg *models.Message) (chatID int64, threadID int) +``` + +Replies are sent with `chathelper.Reply`/`ReplyHTML`, which already forward +`MessageThreadID` so a reply in a forum topic stays in that topic rather than being routed +to General. + +### `internal/modules/blacklist/handlers.go` + +```go +const handlerTimeout = 10 * time.Second +const genericFailure = "Something went wrong. Try again in a moment." +``` + +Ten seconds matches `alias`. These handlers only touch storage, so the budget is generous. + +```go +// resolveText picks the entry text out of the update: the command argument when +// there is one, otherwise the replied-to message's text. Returns the raw text +// for echoing and the normalized form for keying. +func resolveText(msg *models.Message) (raw, norm string, err error) +``` + +Errors are distinguishable, because "say something useful" is the whole job here: +no text found at all; normalizes to empty; longer than `maxEntryBytes`. + +The reply path matters more than it looks: the natural gesture is to see a message, reply to +it, and ban it. It also makes the length cap load-bearing, since a replied-to message can +carry 4096 characters, and the refusal must say so rather than failing opaquely. + +```go +func (s *state) handleAdd(list string) modules.CommandHandler +func (s *state) handleDel(list string) modules.CommandHandler +``` + +Closures over the list tag, so the factory registers `s.handleAdd(listBlack)` and +`s.handleAdd(listWhite)`. + +`handleAdd` reads before writing to word the reply — "Added" against "Already in the +blacklist" — accepting that a concurrent add makes the noun wrong without making the stored +state wrong, the same trade `alias` documents at `handlers.go:120-123`. + +`handleDel` reads before deleting for a sharper reason: `DocStore.Delete` does not +distinguish a missing key from a removed one, so without the read "Removed" would be +reported for something that never existed. + +Anyone may remove anyone's entry. The list is shared by the thread, so the permission model +is too; a per-owner rule would strand entries whose adder has left the group. + +### Command registration + +All `VisibilityPublic`. + +| Name | Parameters | Description | +|---|---|---| +| `blacklist_add` | `[text...]` | `Add text to this thread's blacklist, or reply to a message` | +| `blacklist_del` | `` | `Remove text from this thread's blacklist` | +| `whitelist_add` | `[text...]` | `Add a whitelist exception, or reply to a message` | +| `whitelist_del` | `` | `Remove a whitelist exception` | +| `blacklist_rules` | — | `List this thread's blacklist and whitelist entries` | +| `blacklist_check` | `` | `Check whether a text is blacklisted in this thread` | + +`[text...]` and `` follow `docs/command-parameter-conventions.md`: square brackets +for the optional-because-a-reply-works case, the `...` suffix for remaining text. + +## Files + +| File | Change | +|---|---| +| `internal/modules/blacklist/blacklist.go` | new | +| `internal/modules/blacklist/handlers.go` | new | +| `internal/modules/blacklist/handlers_test.go` | new | + +## Steps + +1. Write `blacklist.go`: `Entry`, `Store`, `state`, `New` with all six registrations (the + two read commands pointing at stubs that reply with nothing yet). +2. Write `threadOf` and `resolveText`. +3. Write `handleAdd` and `handleDel` as list-parameterised closures. +4. Write `handlers_test.go` against `storage.NewMemoryProvider()` and the test bot harness + used by `internal/modules/alias/handlers_test.go`. +5. `gofmt`, then run the package tests. + +## Validation + +`go test ./internal/modules/blacklist/...`: + +- Adding then reading back the key confirms the entry landed under the right prefix with + `Text` as typed, not as normalized. +- The same text added in two different threads of one chat produces two entries, and + removing one leaves the other. +- `/blacklist_add` with no argument and no reply returns usage text, not a stored entry. +- `/blacklist_add` replying to a message stores that message's text. +- A 4096-character replied-to message is refused with the length message. +- Text that is only whitespace or only punctuation stripped by normalization is refused. +- Adding an existing entry replies "already", and the stored record is unchanged. +- `/blacklist_del` for an absent entry replies "not there" and is not reported as removed. +- An entry containing `` renders escaped in every reply that echoes it. +- `/whitelist_add` writes under the `w` prefix and is invisible to a `b`-prefix list. + +## Risk + +`resolveText` is where a caller's 4096-character message meets a 200-byte key. Getting the +order wrong — keying before checking the length — turns a chatty refusal into an opaque +storage error. Normalize, then measure, then key. + +## Rollback + +Still no importers outside the package. Revert the two files; Phase 1 stands alone. diff --git a/plans/260915-1108-blacklist-module/phase-03-rules-and-check.md b/plans/260915-1108-blacklist-module/phase-03-rules-and-check.md new file mode 100644 index 0000000..5f013ff --- /dev/null +++ b/plans/260915-1108-blacklist-module/phase-03-rules-and-check.md @@ -0,0 +1,118 @@ +--- +phase: 3 +title: "Phase 3: /blacklist_rules and /blacklist_check" +status: done +priority: P1 +effort: "2h" +dependencies: [2] +--- + +# Phase 3: `/blacklist_rules` and `/blacklist_check` + +## Overview + +The two read commands, replacing the Phase 2 stubs. `/blacklist_check` is where the Phase 1 +matcher finally meets stored data; `/blacklist_rules` is where arbitrary user text meets +Telegram's message limit. + +## Requirements + +- Functional: `/blacklist_check` reports a verdict naming the blacklist entry that decided it + and, when the text was rescued, the whitelist entry responsible. +- Functional: `/blacklist_rules` shows both lists in one message, each under its own heading, + with empty lists visibly empty rather than absent. +- Functional: a listing too long for one Telegram message is trimmed with a count of what was + omitted. +- Non-functional: `/blacklist_check` costs one `List` per list and no per-entry reads. +- Non-functional: both commands are bounded by `handlerTimeout` and escape all user text. + +## Architecture + +### Loading a thread's entries + +```go +// entriesFor returns one list's normalized entries, sorted. The key holds the +// normalized text, so this needs no per-entry document read — the difference +// between one round trip and one per rule. +func (s *state) entriesFor(ctx context.Context, chatID int64, threadID int, list string) ([]string, error) +``` + +`DocStore.List` returns full keys, as `alias` relies on at `handlers.go:260`. Strip +`scopePrefix(...)` with `strings.TrimPrefix`, run `decodeKeyText`, sort. + +### `/blacklist_check` + +Normalize the argument through `Normalize`, load both lists, call `Check`, and render the +`Verdict`'s three shapes: + +- `Blocked` — say so and name the entry. +- Not blocked but `Entry != ""` — say it is allowed, name the entry that matched, and name + the whitelist entry that rescued it. This case is the reason the whitelist is worth having + a display for at all: without it, a user who added an exception has no way to confirm it + is doing anything. +- Zero `Verdict` — nothing matched. + +Each reply also states the scope, so a user who runs the command in the wrong topic can see +why the answer surprised them. + +### `/blacklist_rules` + +```go +const maxListBytes = 3800 +``` + +The same budget `alias` uses (`internal/modules/alias/handlers.go:29-36`) and for the same +reason: Telegram's 4096-character `sendMessage` cap measures the message actually sent, so +the `` tags count too, and at 13 bytes a pair they outweigh a short entry. + +One message, two headed sections, blacklist first. Entries come from the stored `Entry.Text` +rather than the key, so the list shows what people typed — which costs one read per *listed* +entry, bounded by what fits in the message rather than by how many entries exist, exactly as +`alias.renderNames` is. Each entry is wrapped in `` so tapping it copies the text ready +to paste into a `_del` command. + +Both sections share the single byte budget; the trim notice names how many entries were +omitted. An empty list prints its heading followed by a short "none yet" line, because a +missing heading reads as a bug rather than as an empty list. + +## Files + +| File | Change | +|---|---| +| `internal/modules/blacklist/handlers.go` | stubs replaced; `entriesFor`, `renderRules` added | +| `internal/modules/blacklist/handlers_test.go` | extended | + +## Steps + +1. Write `entriesFor`. +2. Replace the `/blacklist_check` stub; render the three `Verdict` shapes. +3. Replace the `/blacklist_rules` stub; write `renderRules` with the shared byte budget. +4. Extend `handlers_test.go`. +5. `gofmt`, run the package tests. + +## Validation + +`go test ./internal/modules/blacklist/...`: + +- The plan's headline case end to end: blacklist `ass`, whitelist `assassin`, then + `/blacklist_check assassin` is allowed and names both entries, `/blacklist_check dumbass` + is blocked, and `/blacklist_check I met an assassin, dumbass` is **blocked**. +- `/blacklist_check` against empty lists reports nothing matched. +- `/blacklist_check` in thread B does not see entries added in thread A. +- `/blacklist_rules` on empty lists prints both headings and no entries. +- `/blacklist_rules` shows `Entry.Text` as typed, not the normalized form. +- Enough entries to exceed `maxListBytes` produce a message under 4096 characters carrying an + accurate omitted count. +- An entry containing `` appears escaped. +- `entriesFor` returns sorted output given keys listed in any order. + +## Risk + +`renderRules` splits one byte budget across two sections; a naive implementation gives the +first section the whole budget and leaves the whitelist permanently invisible on a busy +thread. Reserve the trim notice before committing a line, as `alias.renderNames` does, and +test with a blacklist alone large enough to exhaust the budget. + +## Rollback + +Restore the Phase 2 stubs. The mutation commands stay usable. diff --git a/plans/260915-1108-blacklist-module/phase-04-wiring-and-docs.md b/plans/260915-1108-blacklist-module/phase-04-wiring-and-docs.md new file mode 100644 index 0000000..ee364a2 --- /dev/null +++ b/plans/260915-1108-blacklist-module/phase-04-wiring-and-docs.md @@ -0,0 +1,106 @@ +--- +phase: 4 +title: "Phase 4: Wiring and documentation" +status: done +priority: P2 +effort: "1h" +dependencies: [3] +--- + +# Phase 4: Wiring and documentation + +## Overview + +Register the module in the composition root and document it. This is the phase that makes +the six commands reachable by a real user. + +## Requirements + +- Functional: the module is in `factories()`, so it loads by default and can be selected or + omitted through `MODULES`. +- Functional: `/help` and Telegram's native command menu list all six commands, which follows + automatically from registration and needs only to be asserted. +- Non-functional: README and a feature doc describe the behaviour a user can observe, + including the two things most likely to surprise them. + +## Architecture + +### `cmd/server/main.go` + +One line in `factories()` (`cmd/server/main.go:80-99`): + +```go +"blacklist": blacklist.New, +``` + +Plain string rather than a `CollectionName` constant, matching `alias`, `gold` and `misc`; +only modules whose name is referenced elsewhere export one. + +An unset or empty `MODULES` loads every registered module (`internal/modules/registry.go:178`), +so this line alone enables it on the existing deployment. No `init*Store` call is needed — +the module has no startup migration and no cross-module state. + +### `docs/blacklist.md` + +Following `docs/aliases.md` in shape. It has to answer the three questions a user will +actually arrive with: + +- **The bot does not police chat.** The lists are inert until `/blacklist_check` asks. Worth + stating first and plainly, because the module's name promises enforcement it deliberately + does not perform. +- **Lists are per topic.** Entries added in one forum topic are invisible in the next, and a + DM has its own private list. This is the most likely support question. +- **Diacritics are significant.** `ma`, `má` and `mà` are three entries; catching all three + means adding all three. Case and spacing are not significant, and the same word typed on + an iPhone and on Android match. + +Plus a worked whitelist example, since span containment is not guessable: blacklist `ass`, +whitelist `assassin`, and the three outcomes including `I met an assassin, dumbass`. + +### `README.md` + +One row in the module table: + +```text +| `blacklist` | Per-topic text deny-list with whitelist exceptions: `/blacklist_add`, `/blacklist_del`, `/whitelist_add`, `/whitelist_del`, `/blacklist_rules` lists both, `/blacklist_check` judges a text. Passive — the bot never scans chat. See [docs/blacklist.md](docs/blacklist.md) | +``` + +## Files + +| File | Change | +|---|---| +| `cmd/server/main.go` | import + one `factories()` entry | +| `cmd/server/main_test.go` | extended | +| `docs/blacklist.md` | new | +| `README.md` | one table row | + +## Steps + +1. Add the import and the `factories()` entry. +2. Extend `TestFactoriesIncludesExpectedModules` (`cmd/server/main_test.go:111`) to build a + registry containing `blacklist` and assert all six command names resolve. +3. Write `docs/blacklist.md`. +4. Add the README row. +5. `gofmt`, then the full gate below. + +## Validation + +- `go test ./...` +- `go vet ./...` +- `golangci-lint run` +- `MODULES=blacklist` builds a registry with exactly the six commands and no conflict. +- Unset `MODULES` builds a registry including `blacklist` alongside every existing module — + the check that no command name collides with an existing one. +- Every link in `README.md` and `docs/blacklist.md` resolves. + +## Risk + +A command-name collision surfaces only when the full catalog is built, because +`Registry.addCommands` rejects duplicates across modules at startup. The unset-`MODULES` +test is what turns that from a deploy-time crash into a test failure. `blacklist_*` and +`whitelist_*` are unused by any existing module today, so the expected result is green. + +## Rollback + +Revert the `factories()` entry; the package becomes dead code without affecting a running +deployment. Revert the docs separately. diff --git a/plans/260915-1108-blacklist-module/plan.md b/plans/260915-1108-blacklist-module/plan.md new file mode 100644 index 0000000..ac3a75e --- /dev/null +++ b/plans/260915-1108-blacklist-module/plan.md @@ -0,0 +1,162 @@ +--- +title: "Blacklist module" +description: "internal/modules/blacklist — per-thread deny-list and exception-list of text, curated by anyone, queried on demand. Passive: the bot never acts on chat traffic." +status: done +priority: P2 +effort: "" +tags: ["blacklist", "telegram-bot", "module"] +created: 2026-09-15 +branch: main +blockedBy: [] +blocks: [] +--- + +# Blacklist module + +## Overview + +New module `internal/modules/blacklist`. Each chat thread curates two lists of text: a +**blacklist** of forbidden entries and a **whitelist** of exceptions that rescue false +positives. `/blacklist_check ` scans a text against that thread's rules and reports a +verdict. + +The module is **passive**. It never reads ordinary chat messages and never deletes, warns, +or restricts anyone. It only answers when a command is invoked. + +## Accepted scope + +| Decision | Value | +|---|---| +| Behaviour | Passive registry. No message scanning, no enforcement | +| Scope | Per thread — `(Chat.ID, MessageThreadID)`; a DM is `(userID, 0)` | +| Visibility | All `VisibilityPublic`; anyone in a thread may modify that thread's lists | +| Matching | Substring, in normalized space | +| Normalization | NFKC + lowercase + whitespace collapse. **Diacritics preserved** | +| Whitelist | Exception layer — rescues a blacklist match only by span containment | + +### Why passive + +Enforcement would need a `Module.MessageHook` primitive the dispatcher does not have +(`internal/modules/dispatcher.go` registers only commands, callback prefixes, one fallback +and one inline query), plus privacy mode disabled in BotFather and group-admin delete +rights. All of that is out of scope. Nothing in this plan forecloses adding it later: the +matcher is a pure function a future hook can call unchanged. + +### Why diacritics are preserved + +Chosen by the project owner. `ma`, `má` and `mà` are three distinct entries. The cost is +that dropping a diacritic evades an entry, so each form worth catching must be added +separately — a curation choice, not a defect. NFKC still runs, so full-width and +compatibility variants of the same characters do collapse, and Vietnamese text composed as +NFD by iOS clients matches the same text composed as NFC by Android clients. + +## Command surface + +All `VisibilityPublic`. `Parameters` follows `docs/command-parameter-conventions.md`. + +| Command | Parameters | Behaviour | +|---|---|---| +| `/blacklist_add` | `[text...]` | Add to this thread's blacklist. No argument → the replied-to message's text | +| `/blacklist_del` | `` | Remove from this thread's blacklist | +| `/whitelist_add` | `[text...]` | Add an exception. No argument → the replied-to message's text | +| `/whitelist_del` | `` | Remove an exception | +| `/blacklist_rules` | — | This thread's blacklist and whitelist entries, in one message | +| `/blacklist_check` | `` | Verdict for the text, naming the entry that matched and any exception that rescued it | + +`/blacklist_rules` carries the `blacklist_` prefix so Telegram's native menu sorts it beside +the other `blacklist_*` commands, and its description states that it shows both lists. + +## Goals + +| # | Goal | Priority | +|---|------|----------| +| 1 | Two threads of one forum keep entirely separate lists | P1 | +| 2 | A whitelist entry rescues only the blacklist matches it actually spans | P1 | +| 3 | Vietnamese text matches regardless of the client's Unicode composition | P1 | +| 4 | No command can stall the bot beyond a bounded deadline | P1 | +| 5 | Arbitrary user text is safe as a storage key and as Telegram HTML | P1 | +| 6 | A list too long for one Telegram message is trimmed, not dropped | P2 | + +## Phases + +| # | Phase | Depends on | Effort | +|---|---|---|---| +| 1 | [Scope keys, normalization, matcher](phase-01-scope-normalize-match.md) | — | done | +| 2 | [Store, factory, mutation commands](phase-02-store-and-mutations.md) | 1 | done | +| 3 | [`/blacklist_rules` and `/blacklist_check`](phase-03-rules-and-check.md) | 2 | done | +| 4 | [Wiring and documentation](phase-04-wiring-and-docs.md) | 3 | done | + +Phases 2 and 3 landed in one edit: both write `internal/modules/blacklist/handlers.go`, +so the planned stub-then-replace step was skipped. + +Phase 1 is pure Go with no Telegram and no storage, so it carries the whole matcher +test suite and can be reviewed on its own. + +## Acceptance criteria + +1. Adding, listing, checking and deleting in two threads of the same forum supergroup do not + leak across threads; the same commands work in a DM. +2. Given blacklist `ass` and whitelist `assassin`: `assassin` is ALLOWED, `dumbass` is + BLACKLISTED, and `I met an assassin, dumbass` is BLACKLISTED. +3. `má` does not match an entry `ma`; `Má` does match an entry `má`; the same Vietnamese + word in NFC and NFD form match each other. +4. An entry containing `/`, `%`, `:` or a newline round-trips through storage and renders + escaped in `/blacklist_rules`. +5. `/blacklist_rules` with more entries than fit in 4096 characters returns a trimmed + message with a count of what was omitted. +6. `go test ./...`, `go vet ./...` and `golangci-lint run` all pass. + +## Stats compatibility + +Six new command names, no rename and no deletion, so `AGENTS.md`'s stats migration rules +impose no work. Command names must not change after release without the migration those +rules require. + +## Risks + +| Risk | Mitigation | +|---|---| +| User text used as a storage key hits the `/`-forbidden and 1500-byte rules in `internal/storage/keys.go` | Percent-encode `%` then `/`; cap normalized entries at 200 bytes (Phase 1) | +| `DocStore.List` has no ordering guarantee, so verdicts could name different entries across runs | Sort entries before scanning (Phase 1) | +| Unescaped user text in an HTML reply | Every entry passes through `html.EscapeString` at every render site (Phases 2-3) | +| `golang.org/x/text` promoted from indirect to direct dependency | Already present at v0.41.0 in `go.sum`; no new download, `go mod tidy` only moves the line | + +## Outcome + +All four phases delivered; every acceptance criterion above verified. Full gate green: +`go test ./...`, `go vet ./...`, `golangci-lint run` (0 issues). + +Two files the plan did not anticipate had to change, both required by existing repo +contracts rather than by the feature: + +- `cmd/server/command_menu_test.go` pins every public command's `Parameters` string, and + its reverse loop makes even an empty entry load-bearing. The six new commands are + registered there. +- The six command `Description` strings were shortened after review. `/help` renders as one + un-chunked message pinned at 4096 runes, and the first draft left 9 runes of headroom; + the shorter wording restores it to 76. See the open questions below — that ceiling is a + shared limit this module did not create and cannot fix alone. + +`/blacklist_rules` reads each list with `DocStore.Scan`, which landed on `main` while this +work was in progress. The phase-3 design called for `List` followed by a `Get` per displayed +entry, copying `alias.renderNames`; `Scan` exists precisely to remove that N+1, and this +command paid it twice per invocation. `/blacklist_check` still uses `List`, because it needs +only the entry names and never their stored text. + +Test integrity was checked by mutation: fourteen mutants were injected against the matcher, +key encoding, normalization, trimming and escaping. The four that initially survived — +three HTML-escaping sites and the post-normalization byte cap — are now covered by tests +verified to fail without the guard they pin. + +## Post-review decisions + +1. **Reply-thread scoping.** `threadOf` now gates on `msg.IsTopicMessage` rather than + trusting `msg.MessageThreadID` alone. Telegram associates a thread id with any reply chain + in a supergroup, not only with a forum topic, so the original code would have given a + reply-form `/blacklist_add` in a plain supergroup a scope that a later standalone + `/blacklist_rules` could never read back. A forum topic keeps its own lists; a plain + group, a DM, and a forum's General topic all resolve to thread 0. Pinned by + `TestReplyChainThreadIsNotATopic`, verified to fail without the gate. +2. **Per-thread entry count stays unbounded**, matching `alias`, which is equally + world-writable and equally uncapped. Entries remain capped at 200 bytes each. Revisit only + if a thread's list grows large enough to make `/blacklist_check` slow.