fix: separate /unlock from /unmute and re-trigger captcha on rejoin#53
Open
MattiaFailla wants to merge 1 commit into
Open
fix: separate /unlock from /unmute and re-trigger captcha on rejoin#53MattiaFailla wants to merge 1 commit into
MattiaFailla wants to merge 1 commit into
Conversation
… on rejoin Stale users sat muted forever in public supergroups for two reasons: 1. On rejoin, _handle_new_member re-restricted the user and re-added the pending row, then returned early at the has_been_welcomed() guard before sending the captcha. The user was silently muted with no way to verify, because no LEFT/KICKED handler ever cleared the per-chat welcome state. 2. /unmute only deletes from the mutes table, but stale users were restricted by the captcha flow (pending_verifications), so /unmute never cleared their "exception". Changes: - Add a departure branch to _handle_new_member: on LEFT/BANNED, clear the welcomed + pending state for that (user, chat) so a genuine rejoin re-shows the captcha. Global verification is intentionally preserved. - Add /unlock @handle: an admin command that globally verifies a stuck user, clears pending state, and restores send permissions in their pending chats. Kept distinct from /unmute, which stays a pure moderation-mute reversal. - Add Repository.remove_welcomed (in-memory + postgres) and CaptchaService wrappers (remove_welcomed, remove_pending). Deliberately did NOT flip can_change_info / can_pin_messages to True (as proposed in #52). Per the Telegram Bot API docs those fields are "Ignored in public supergroups", so the flip is a no-op there and a latent privilege escalation in any private supergroup, and it is unrelated to the mute itself (which is can_send_messages=False). Tests: /unlock (admin / non-admin / private chat / restored permissions) and rejoin state-clearing (LEFT, BANNED, global-verification preserved).
ae1506f to
497ac18
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
Investigating #52 (
keobox:fix_stale_users). The reported symptom: users stuck muted forever in group is real, but the proposed fix conflates concerns and includes inert/risky permission edits. This PR fixes the actual root causes and keeps moderation (/mute) and captcha (/unlock) cleanly separated.Root cause (two distinct bugs)
Bug A — silent re-mute on rejoin. In
welcome.py:_handle_new_member, a non-verified rejoiner is re-restricted read-only and re-added topending_verifications, then the function returns early at thehas_been_welcomed()guard before sending the captcha. There was noLEFT/BANNEDhandler to clear the per-chat welcome state, so the user is muted with no way to verify.Bug B —
/unmuteclears the wrong table. Stale users were restricted by the captcha flow (pending_verifications), not by/mute./unmuteonlyDELETEs frommutes, so it never cleared their captcha "exception".Changes
_handle_new_member: onLEFT/BANNED, clearwelcomed+pendingfor(user, chat)so a genuine rejoin re-shows the captcha. Global verification is intentionally preserved (verified users stay verified)./unlock @handle(new admin command): globally verifies a stuck user, clears pending state, and restores send permissions in their pending chats. Distinct from/unmute, which stays a pure moderation-mute reversal.remove_welcomed(in-memory + postgres) +CaptchaServicewrappers (remove_welcomed,remove_pending).Why not flip
can_change_info/can_pin_messagestoTrue(as in #52)Per the official Telegram Bot API and python-telegram-bot docs, both fields are "Ignored in public supergroups" — so in
@PythonMilano(public) the flip is a no-op, and in any private supergroup it would be a latent privilege escalation (every verified/unmuted user gains pin + edit-group-info rights above theFalsegroup default). It is also causally unrelated to the mute, which iscan_send_messages=False; those two fields aren't send permissions and aren't in any permission cascade. This PR keeps themFalse.Tests
tests/test_unlock.py: admin / non-admin rejection / private-chat ignore / permission restoration (assertscan_send_messages=True,can_change_info=False,can_pin_messages=False).tests/test_welcome_rejoin.py:remove_welcomedrepo behavior + departure (LEFT,BANNED) clears welcome/pending + global verification preserved.All 27 tests pass;
ruff check,ruff format, andmypyclean on changed files.Relation to #52
Supersedes #52's approach. Keeps the one load-bearing idea (clearing the captcha exception when an admin frees a user) but moves it to a dedicated
/unlockcommand, drops the inert permission flip, and additionally fixes the root rejoin bug so new stale users stop accumulating.