From 380f82438ce0cda1fd2fd32a7f1106f276e9afe2 Mon Sep 17 00:00:00 2001 From: Ilia Denisov Date: Sun, 21 Jun 2026 16:12:58 +0200 Subject: [PATCH] fix(telegram): grant write to restricted members in default-deny chats A default-deny discussion group reports a present or freshly joined member as `restricted` (no send right), not `member`. The join filter required `member`, so the real case never matched and a registered user stayed muted. Grant any eligible in-chat member (member or restricted) that still lacks the send right, with a loop guard (skip when send is already allowed) so the bot's own grant does not re-fire. Revoking a now-ineligible user stays the chat-gate path's job, so this never fights a chat_muted/block. --- PRERELEASE.md | 10 +++ platform/telegram/internal/bot/bot.go | 27 ++++-- platform/telegram/internal/bot/chat_test.go | 99 +++++++++++++++------ 3 files changed, 100 insertions(+), 36 deletions(-) diff --git a/PRERELEASE.md b/PRERELEASE.md index db9dcba..e7f3b80 100644 --- a/PRERELEASE.md +++ b/PRERELEASE.md @@ -611,3 +611,13 @@ Then Stage 18. the sweeper window (unit + integration); gateway hub `ResolveChatEligibility` + the chat-gate command; bot `chat_member` grant + `ApplyChatGate` getChatMember-guard; promo `/start` localization + URL button; config parsing. + - **Post-contour-test fixes (same PR):** a live test surfaced gaps. (1) **Join detection** — a + default-deny discussion group reports a present member as `restricted`, not `member`, so the grant + trigger now fires for any eligible in-chat member (`member` or `restricted`) still lacking the send + right, with a loop guard (skip when send is already allowed, so the bot's own grant does not + re-fire); revoking a now-ineligible user stays the chat-gate path's job, so this never fights a + `chat_muted`/block. (2) **Join-before-register** — a user who joins before registering is covered by + no `chat_member` event, so `ProvisionTelegram` now reports first contact and the Telegram auth + handler emits `chat_access_changed` on it. (3) **Observability** — a startup self-check logs whether + the bot is an admin-with-restrict in the chat (it caught a misconfigured `TELEGRAM_CHAT_ID` set to a + channel id instead of the discussion-group id), plus per-event grant-path logging. diff --git a/platform/telegram/internal/bot/bot.go b/platform/telegram/internal/bot/bot.go index 6cb3b45..a9a7b0b 100644 --- a/platform/telegram/internal/bot/bot.go +++ b/platform/telegram/internal/bot/bot.go @@ -272,23 +272,36 @@ func (t *Bot) handleChatMember(ctx context.Context, cm *models.ChatMemberUpdated if t.chatID == 0 || cm.Chat.ID != t.chatID { return } - // Only a transition into plain membership is a join to evaluate. - if cm.NewChatMember.Type != models.ChatMemberTypeMember || cm.OldChatMember.Type == models.ChatMemberTypeMember { - return - } if user == nil || user.IsBot { return } + // (Re)grant write to an eligible user who is in the chat but cannot yet send: a join + // into the default-deny chat (where a joined member appears `restricted`, sometimes + // `member`) or a restricted member still lacking send rights. A user who can already + // send is skipped — which also breaks the feedback loop from the bot's own grant, + // since restricting re-fires a chat_member event. Revoking a now-ineligible user is + // the chat-gate path's job (an admin block or chat_muted change), not this one. + switch cm.NewChatMember.Type { + case models.ChatMemberTypeMember: + // A plain member follows the chat default; in the gated (default-deny) chat that + // denies sending, so (re)grant. + case models.ChatMemberTypeRestricted: + if cm.NewChatMember.Restricted.CanSendMessages { + return // already allowed to send + } + default: + return // left / kicked / administrator / owner — nothing to grant here + } if t.eligibility == nil { - t.log.Warn("chat join: eligibility resolver not wired", zap.Int64("user_id", uid)) + t.log.Warn("chat access: eligibility resolver not wired", zap.Int64("user_id", uid)) return } eligible, err := t.eligibility(ctx, strconv.FormatInt(user.ID, 10)) if err != nil { - t.log.Warn("chat join eligibility failed", zap.Int64("user_id", user.ID), zap.Error(err)) + t.log.Warn("chat access eligibility failed", zap.Int64("user_id", user.ID), zap.Error(err)) return } - t.log.Info("chat join evaluated", zap.Int64("user_id", user.ID), zap.Bool("eligible", eligible)) + t.log.Info("chat access evaluated", zap.Int64("user_id", user.ID), zap.Bool("eligible", eligible)) if !eligible { return // not registered or blocked: leave muted } diff --git a/platform/telegram/internal/bot/chat_test.go b/platform/telegram/internal/bot/chat_test.go index 1fefb5e..acf0330 100644 --- a/platform/telegram/internal/bot/chat_test.go +++ b/platform/telegram/internal/bot/chat_test.go @@ -62,8 +62,8 @@ func newChatBot(t *testing.T, api *chatAPI) *Bot { return b } -// chatMemberUpdate builds a status transition oldType -> member for userID in chatID. -func chatMemberUpdate(chatID, userID int64, oldType models.ChatMemberType) *models.ChatMemberUpdated { +// memberUpdate builds an oldType -> member transition for userID in chatID. +func memberUpdate(chatID, userID int64, oldType models.ChatMemberType) *models.ChatMemberUpdated { return &models.ChatMemberUpdated{ Chat: models.Chat{ID: chatID}, OldChatMember: models.ChatMember{Type: oldType, Left: &models.ChatMemberLeft{User: &models.User{ID: userID}}}, @@ -71,55 +71,96 @@ func chatMemberUpdate(chatID, userID int64, oldType models.ChatMemberType) *mode } } -func TestHandleChatMemberGrantsEligibleJoin(t *testing.T) { - api := &chatAPI{} +// restrictedUpdate builds an oldType -> restricted transition for userID, the new +// member's text-send permission set to canSend. A default-deny group reports a present +// member as restricted with canSend=false. +func restrictedUpdate(chatID, userID int64, oldType models.ChatMemberType, canSend bool) *models.ChatMemberUpdated { + return &models.ChatMemberUpdated{ + Chat: models.Chat{ID: chatID}, + OldChatMember: models.ChatMember{Type: oldType, Left: &models.ChatMemberLeft{User: &models.User{ID: userID}}}, + NewChatMember: models.ChatMember{Type: models.ChatMemberTypeRestricted, Restricted: &models.ChatMemberRestricted{User: &models.User{ID: userID}, CanSendMessages: canSend}}, + } +} + +// leftUpdate builds a transition to left (a leave) for userID. +func leftUpdate(chatID, userID int64) *models.ChatMemberUpdated { + return &models.ChatMemberUpdated{ + Chat: models.Chat{ID: chatID}, + OldChatMember: models.ChatMember{Type: models.ChatMemberTypeRestricted, Restricted: &models.ChatMemberRestricted{User: &models.User{ID: userID}}}, + NewChatMember: models.ChatMember{Type: models.ChatMemberTypeLeft, Left: &models.ChatMemberLeft{User: &models.User{ID: userID}}}, + } +} + +// eligibleBot builds a gating bot whose resolver returns (eligible, err). +func eligibleBot(t *testing.T, api *chatAPI, eligible bool, err error) *Bot { + t.Helper() b := newChatBot(t, api) - b.SetEligibilityResolver(func(context.Context, string) (bool, error) { return true, nil }) - - b.handleChatMember(context.Background(), chatMemberUpdate(testChatID, 777, models.ChatMemberTypeLeft)) + b.SetEligibilityResolver(func(context.Context, string) (bool, error) { return eligible, err }) + return b +} +func TestHandleChatMemberGrantsEligibleMemberJoin(t *testing.T) { + api := &chatAPI{} + b := eligibleBot(t, api, true, nil) + b.handleChatMember(context.Background(), memberUpdate(testChatID, 777, models.ChatMemberTypeLeft)) if len(api.restricts) != 1 || api.restricts[0].userID != "777" || !api.restricts[0].canSend { t.Fatalf("restricts = %+v, want one grant (can_send=true) for 777", api.restricts) } } -func TestHandleChatMemberSkipsIneligibleJoin(t *testing.T) { +func TestHandleChatMemberGrantsRestrictedMember(t *testing.T) { + // A default-deny group reports a present member as restricted with no send right — + // the real-world case. Both a fresh join (left->restricted) and an already-present + // restricted member (restricted->restricted) must be granted when eligible. + for _, oldType := range []models.ChatMemberType{models.ChatMemberTypeLeft, models.ChatMemberTypeRestricted} { + t.Run(string(oldType), func(t *testing.T) { + api := &chatAPI{} + b := eligibleBot(t, api, true, nil) + b.handleChatMember(context.Background(), restrictedUpdate(testChatID, 777, oldType, false)) + if len(api.restricts) != 1 || !api.restricts[0].canSend { + t.Fatalf("restricts = %+v, want one grant for a restricted member", api.restricts) + } + }) + } +} + +func TestHandleChatMemberSkipsAlreadyAllowed(t *testing.T) { + // The bot's own grant re-fires a chat_member event (restricted, can_send=true); it + // must not loop into another grant. api := &chatAPI{} - b := newChatBot(t, api) - b.SetEligibilityResolver(func(context.Context, string) (bool, error) { return false, nil }) - - b.handleChatMember(context.Background(), chatMemberUpdate(testChatID, 777, models.ChatMemberTypeLeft)) - + b := eligibleBot(t, api, true, nil) + b.handleChatMember(context.Background(), restrictedUpdate(testChatID, 777, models.ChatMemberTypeRestricted, true)) if len(api.restricts) != 0 { - t.Fatalf("an ineligible joiner was restricted: %+v (want left muted, no call)", api.restricts) + t.Fatalf("restricts = %+v, want none for an already-allowed member (no loop)", api.restricts) + } +} + +func TestHandleChatMemberSkipsIneligible(t *testing.T) { + api := &chatAPI{} + b := eligibleBot(t, api, false, nil) + b.handleChatMember(context.Background(), restrictedUpdate(testChatID, 777, models.ChatMemberTypeLeft, false)) + if len(api.restricts) != 0 { + t.Fatalf("an ineligible member was granted: %+v (want left muted)", api.restricts) } } func TestHandleChatMemberFailsClosedOnResolveError(t *testing.T) { api := &chatAPI{} - b := newChatBot(t, api) - b.SetEligibilityResolver(func(context.Context, string) (bool, error) { return true, context.DeadlineExceeded }) - - b.handleChatMember(context.Background(), chatMemberUpdate(testChatID, 777, models.ChatMemberTypeLeft)) - + b := eligibleBot(t, api, true, context.DeadlineExceeded) + b.handleChatMember(context.Background(), restrictedUpdate(testChatID, 777, models.ChatMemberTypeLeft, false)) if len(api.restricts) != 0 { t.Fatalf("a resolve error still granted write: %+v (want fail-closed)", api.restricts) } } -func TestHandleChatMemberIgnoresOtherChatAndNonJoins(t *testing.T) { +func TestHandleChatMemberIgnoresOtherChatAndLeaves(t *testing.T) { api := &chatAPI{} - b := newChatBot(t, api) - b.SetEligibilityResolver(func(context.Context, string) (bool, error) { return true, nil }) + b := eligibleBot(t, api, true, nil) ctx := context.Background() - - // A different chat is ignored. - b.handleChatMember(ctx, chatMemberUpdate(999, 777, models.ChatMemberTypeLeft)) - // A non-join transition (already a member) is ignored. - b.handleChatMember(ctx, chatMemberUpdate(testChatID, 777, models.ChatMemberTypeMember)) - + b.handleChatMember(ctx, restrictedUpdate(999, 777, models.ChatMemberTypeLeft, false)) // foreign chat + b.handleChatMember(ctx, leftUpdate(testChatID, 777)) // a leave if len(api.restricts) != 0 { - t.Fatalf("restricts = %+v, want none for a foreign chat / non-join", api.restricts) + t.Fatalf("restricts = %+v, want none for a foreign chat / a leave", api.restricts) } }