Filter non-code messages in TOTP gate

[dcellison]

Summary

• Add a format check (exactly 6 digits) before the TOTP verification path in handle_message()
• With concurrent_updates=True, messages arriving while a TOTP challenge is pending were unconditionally treated as code attempts
• This caused three problems for non-code messages: (1) message deletion, (2) spurious "Invalid code" responses, (3) unnecessary sudo calls via get_lockout_remaining()/get_failure_count()
• Non-code messages now get a brief "Enter your 6-digit code" reminder and return without touching the verification machinery
• verify_code() in totp.py already has the same format guard (line 171) as defense-in-depth, but the handler needs to distinguish non-codes before reaching the delete/lockout/verify path

What changed

Single if block added between code = ... and the message deletion block in handle_message(). Everything downstream (delete, lockout check, verify_code, failure handling) is untouched.

Test plan

• test_non_code_message_not_deleted - normal text not deleted, verify_code not called, reminder sent
• test_six_digit_code_still_verified - 6-digit strings still reach verify_code and get deleted
• test_non_code_no_sudo_calls - no sudo-backed functions called for non-code text
• test_partial_digit_string_not_treated_as_code - edge cases: 5 digits, 7 digits, mixed alphanumeric, prefixed digits
• Existing test_code_message_deleted_after_verification (uses "123456") still passes
• Existing test_invalid_code_shows_remaining_attempts (uses "000000") still passes
• 1038 tests pass, make check clean

Fixes #122

[dcellison]

Review by Kai

PR Review: Filter non-code messages in TOTP gate

Overall: Clean, well-scoped fix for a real bug. Logic is correct. A few findings below.

---

Warning

str.isdigit() accepts non-ASCII digit characters (bot.py, new block)

if not code.isdigit() or len(code) != 6:

"¹²³⁴⁵⁶".isdigit() returns True in Python (superscripts, Arabic-Indic numerals, etc.). A 6-character string of Unicode digit characters would slip past this guard and reach update.message.delete() + verify_code().

The PR description notes verify_code() has the same guard at line 171 as defense-in-depth, which mitigates this — but the two guards should agree on what constitutes a "digit." Recommend:

import re
if not re.fullmatch(r'[0-9]{6}', code):

This also eliminates the redundant len() check.

---

Warning

Expiry check ordering is unverifiable from the diff

The new block is inserted between code = ... and the delete block, but the diff doesn't show where totp_pending["expires_at"] is validated. If expiry is checked after this block, a non-code message sent against an already-expired TOTP session will still emit "Authentication required. Enter your 6-digit code." — misleading UX and a minor info leak about internal state. Worth confirming the expiry check fires before this return path.

---

Suggestion

send_message() not in try/except (bot.py, new block)

The delete block immediately below is wrapped in try/except. The new send_message() call is not. A transient network error here raises uncaught, which could surface as an unhandled exception in the update handler. Wrap consistently:

try:
    await update.effective_chat.send_message("Authentication required. Enter your 6-digit code.")
except Exception:
    pass  # or logger.warning(...)
return

---

Suggestion

Overly broad test assertion (test_bot_totp.py, line ~363)

assert "6-digit" in sent.lower() or "code" in sent.lower()

"code" would match almost any response. Since the reminder text is known, assert on the specific string instead:

assert "6-digit" in sent

---

Clean items

• The guard correctly uses early return — no auth bypass risk.
• Existing tests test_code_message_deleted_after_verification and test_invalid_code_shows_remaining_attempts still exercising the hot path is good validation.
• test_partial_digit_string_not_treated_as_code covers the right edge cases (5 digits, 7 digits, mixed, prefixed).
• No changes to verify_code() or the lockout/failure machinery — blast radius is appropriately minimal.

[dcellison]

Review by Kai

PR Review: Filter non-code messages in TOTP gate (Round 2)

Overall: Looks good. Both items from the prior review that warranted code changes were addressed correctly. Nothing new to raise.

---

Addressed from prior review

Non-ASCII digit bypass (Warning → resolved)

The author added code.isascii() as a short-circuit before isdigit():

if not (code.isascii() and code.isdigit() and len(code) == 6):

isascii() rejects any code point > 127, which covers superscripts, Arabic-Indic numerals, and all other non-ASCII "digits." The inline comment explains the rationale. Functionally equivalent to the suggested re.fullmatch(r'[0-9]{6}', code) and arguably clearer without an import. Correct.

Overly broad test assertion (Suggestion → resolved)

assert "6-digit" in sent

Tightened from the prior "code" in sent.lower() form. Good.

---

Unaddressed from prior review

Per review policy, not re-raising items the author has already seen (send_message() exception handling, expiry-check ordering). Both were deliberate choices.

---

New findings

None. The logic is correct, the filter is placed before the delete/verify path, the tests cover the meaningful edge cases (5-digit, 7-digit, alphanumeric, prefixed), and the positive path (test_six_digit_code_still_verified) confirms the hot path is unaffected.

The len(code) == 6 condition is not redundant — isascii() and isdigit() doesn't constrain length, so all three checks are necessary.

---

Clean to merge.