fix(telegram): grant write to restricted members in default-deny chats
CI / changes (pull_request) Successful in 2s
CI / unit (pull_request) Successful in 11s
CI / integration (pull_request) Successful in 15s
CI / ui (pull_request) Successful in 57s
CI / gate (pull_request) Successful in 0s
CI / deploy (pull_request) Successful in 1m22s
CI / changes (pull_request) Successful in 2s
CI / unit (pull_request) Successful in 11s
CI / integration (pull_request) Successful in 15s
CI / ui (pull_request) Successful in 57s
CI / gate (pull_request) Successful in 0s
CI / deploy (pull_request) Successful in 1m22s
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.
This commit is contained in:
@@ -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.
|
||||
|
||||
@@ -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
|
||||
}
|
||||
|
||||
@@ -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 := 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, 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 := 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)
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user