fix(telegram): grant in-chat members regardless of reported can_send
CI / changes (pull_request) Successful in 2s
CI / unit (pull_request) Successful in 11s
CI / integration (pull_request) Successful in 16s
CI / ui (pull_request) Successful in 56s
CI / gate (pull_request) Successful in 0s
CI / deploy (pull_request) Successful in 1m18s
CI / changes (pull_request) Successful in 2s
CI / unit (pull_request) Successful in 11s
CI / integration (pull_request) Successful in 16s
CI / ui (pull_request) Successful in 56s
CI / gate (pull_request) Successful in 0s
CI / deploy (pull_request) Successful in 1m18s
The CanSendMessages loop-guard skipped exactly the stuck case — a restricted member whose chat_member event reports can_send=true yet who cannot actually write. Replace it with a precise loop guard (skip only the bot's own restrict action, i.e. the update whose performer is the bot) and grant any eligible in-chat member (member or restricted) otherwise. Also log the new member's can_send, is_member and the actor id for full visibility.
This commit is contained in:
@@ -54,6 +54,9 @@ type Bot struct {
|
||||
limiter *rate.Limiter
|
||||
// chatID is the moderated discussion chat (0 disables gating).
|
||||
chatID int64
|
||||
// botID is the bot's own Telegram user id (resolved at startup); it skips the
|
||||
// chat_member updates the bot's own restrict actions generate — the grant loop guard.
|
||||
botID int64
|
||||
// eligibility resolves a joining user's chat write eligibility; nil leaves a
|
||||
// joiner muted (fail-closed) until it is wired.
|
||||
eligibility EligibilityResolver
|
||||
@@ -134,6 +137,7 @@ func (t *Bot) logChatAdminStatus(ctx context.Context) {
|
||||
t.log.Warn("chat self-check: getMe failed", zap.Error(err))
|
||||
return
|
||||
}
|
||||
t.botID = me.ID
|
||||
m, err := t.api.GetChatMember(ctx, &tgbot.GetChatMemberParams{ChatID: t.chatID, UserID: me.ID})
|
||||
if err != nil {
|
||||
t.log.Warn("chat gating self-check failed: the bot cannot read the chat — is it added and is TELEGRAM_CHAT_ID the discussion group id?",
|
||||
@@ -260,14 +264,19 @@ func (t *Bot) handleChatMember(ctx context.Context, cm *models.ChatMemberUpdated
|
||||
if user != nil {
|
||||
uid = user.ID
|
||||
}
|
||||
// Log every chat_member update the bot receives: this is the one place to see
|
||||
// whether Telegram is delivering joins at all, for which chat, and the transition.
|
||||
// Log every chat_member update the bot receives: the one place to see whether
|
||||
// Telegram delivers joins, for which chat, the transition, who performed it, and the
|
||||
// new member's send/membership state.
|
||||
canSend, isMember := restrictedSendState(cm.NewChatMember)
|
||||
t.log.Info("chat_member update",
|
||||
zap.Int64("chat_id", cm.Chat.ID),
|
||||
zap.Int64("configured_chat_id", t.chatID),
|
||||
zap.Int64("user_id", uid),
|
||||
zap.Int64("actor_id", cm.From.ID),
|
||||
zap.String("old_status", string(cm.OldChatMember.Type)),
|
||||
zap.String("new_status", string(cm.NewChatMember.Type)))
|
||||
zap.String("new_status", string(cm.NewChatMember.Type)),
|
||||
zap.Bool("new_can_send", canSend),
|
||||
zap.Bool("new_is_member", isMember))
|
||||
|
||||
if t.chatID == 0 || cm.Chat.ID != t.chatID {
|
||||
return
|
||||
@@ -275,22 +284,19 @@ func (t *Bot) handleChatMember(ctx context.Context, cm *models.ChatMemberUpdated
|
||||
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.
|
||||
// Loop guard: the bot's own restrict re-fires a chat_member update whose performer is
|
||||
// the bot; skip those so a grant never re-triggers itself.
|
||||
if t.botID != 0 && cm.From.ID == t.botID {
|
||||
return
|
||||
}
|
||||
// Act only on a user who is in the chat (member or restricted) — a join into the
|
||||
// default-deny chat appears as `restricted`, not `member`. Left/kicked and admins
|
||||
// have nothing to grant here. An eligible user is (re)granted write; revoking a
|
||||
// now-ineligible user is the chat-gate path's job (an admin block or chat_muted).
|
||||
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
|
||||
}
|
||||
case models.ChatMemberTypeMember, models.ChatMemberTypeRestricted:
|
||||
default:
|
||||
return // left / kicked / administrator / owner — nothing to grant here
|
||||
return
|
||||
}
|
||||
if t.eligibility == nil {
|
||||
t.log.Warn("chat access: eligibility resolver not wired", zap.Int64("user_id", uid))
|
||||
@@ -372,6 +378,16 @@ func chatWritePerms() models.ChatPermissions {
|
||||
}
|
||||
}
|
||||
|
||||
// restrictedSendState returns a restricted member's text-send permission and whether
|
||||
// they are currently a member of the chat; (false, false) for any non-restricted
|
||||
// status (the fields exist only on the restricted variant).
|
||||
func restrictedSendState(m models.ChatMember) (canSend, isMember bool) {
|
||||
if m.Type == models.ChatMemberTypeRestricted && m.Restricted != nil {
|
||||
return m.Restricted.CanSendMessages, m.Restricted.IsMember
|
||||
}
|
||||
return false, false
|
||||
}
|
||||
|
||||
// chatMemberUser returns the user a ChatMember refers to across the union variants,
|
||||
// or nil for an unrecognised type.
|
||||
func chatMemberUser(m models.ChatMember) *models.User {
|
||||
|
||||
@@ -3,6 +3,7 @@ package bot
|
||||
import (
|
||||
"context"
|
||||
"encoding/json"
|
||||
"fmt"
|
||||
"io"
|
||||
"net/http"
|
||||
"net/http/httptest"
|
||||
@@ -13,7 +14,10 @@ import (
|
||||
"go.uber.org/zap"
|
||||
)
|
||||
|
||||
const testChatID = 555
|
||||
const (
|
||||
testChatID = 555
|
||||
botSelfID = 111111 // the bot's own id in tests (for the loop guard)
|
||||
)
|
||||
|
||||
// chatAPI is a fake Bot API for the chat-gating tests: it answers getMe, returns a
|
||||
// scripted getChatMember status, and records restrictChatMember calls.
|
||||
@@ -59,6 +63,7 @@ func newChatBot(t *testing.T, api *chatAPI) *Bot {
|
||||
if err != nil {
|
||||
t.Fatalf("new bot: %v", err)
|
||||
}
|
||||
b.botID = botSelfID // normally set at startup; the chat_member updates default actor 0 != this
|
||||
return b
|
||||
}
|
||||
|
||||
@@ -109,14 +114,22 @@ func TestHandleChatMemberGrantsEligibleMemberJoin(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) {
|
||||
// A default-deny group reports a present member as restricted — the real-world case.
|
||||
// A fresh join (left->restricted) or an already-present restricted member
|
||||
// (restricted->restricted), and regardless of the reported can_send (which can be
|
||||
// misleading), an eligible member is granted.
|
||||
for _, tc := range []struct {
|
||||
old models.ChatMemberType
|
||||
canSend bool
|
||||
}{
|
||||
{models.ChatMemberTypeLeft, false},
|
||||
{models.ChatMemberTypeRestricted, false},
|
||||
{models.ChatMemberTypeRestricted, true},
|
||||
} {
|
||||
t.Run(fmt.Sprintf("%s_cansend=%v", tc.old, tc.canSend), func(t *testing.T) {
|
||||
api := &chatAPI{}
|
||||
b := eligibleBot(t, api, true, nil)
|
||||
b.handleChatMember(context.Background(), restrictedUpdate(testChatID, 777, oldType, false))
|
||||
b.handleChatMember(context.Background(), restrictedUpdate(testChatID, 777, tc.old, tc.canSend))
|
||||
if len(api.restricts) != 1 || !api.restricts[0].canSend {
|
||||
t.Fatalf("restricts = %+v, want one grant for a restricted member", api.restricts)
|
||||
}
|
||||
@@ -124,14 +137,16 @@ func TestHandleChatMemberGrantsRestrictedMember(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
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.
|
||||
func TestHandleChatMemberSkipsBotsOwnAction(t *testing.T) {
|
||||
// The bot's own grant re-fires a chat_member update performed by the bot; it must be
|
||||
// skipped so a grant never loops into another grant.
|
||||
api := &chatAPI{}
|
||||
b := eligibleBot(t, api, true, nil)
|
||||
b.handleChatMember(context.Background(), restrictedUpdate(testChatID, 777, models.ChatMemberTypeRestricted, true))
|
||||
upd := restrictedUpdate(testChatID, 777, models.ChatMemberTypeRestricted, false)
|
||||
upd.From = models.User{ID: botSelfID}
|
||||
b.handleChatMember(context.Background(), upd)
|
||||
if len(api.restricts) != 0 {
|
||||
t.Fatalf("restricts = %+v, want none for an already-allowed member (no loop)", api.restricts)
|
||||
t.Fatalf("restricts = %+v, want none for the bot's own action (no loop)", api.restricts)
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user