What
Found during a code-reduction audit of the Telegram Google Health command handlers in telegram/bot.py and health/service.py. Four related error-handling gaps.
_connect_health has narrower exception handling than its three siblings (bot.py:697): it only catches (GoogleHealthOAuthError, ValueError) around service.begin_authorization(...), while _send_health_summary, _refresh_health, and _handle_health_callback all also catch broad Exception. Any other failure (e.g. a storage/DB error inside storage.store_oauth_state) propagates uncaught to the generic top-level handler in _safe_handle_update, which only logs and never messages the user — unlike the other three commands, which always reply with a specific error string.- Undocumented invariant in the disconnect-confirmation callback (bot.py:808):
query.from_user.id != chat.id is used to authorize the callback. This is true for Telegram private chats but is an implicit assumption with no comment explaining why it's safe — a future change to allow group-chat health commands would silently break this check.
- Token-revocation failure is silently swallowed and can misreport state (
health/service.py, disconnect()): a GoogleHealthApiError during remote token revocation is caught and only logged (logger.warning), while local storage deletion still proceeds. A second /disconnect_health afterward reports "already disconnected" (storage-layer idempotency) even though the remote Google token may still be live and never actually revoked.
notify_health_connection doesn't correlate the callback to a specific pending authorization (bot.py:836): it resolves chat_id from telegram_user_id alone with no check that this is the same chat/session that initiated /connect_health\| recently. Low likelihood of exploitation given consume_oauth_state's behavior, but worth confirming the OAuth state` correlation actually prevents a stale/replayed callback from notifying an unrelated chat.
Priority
Medium for item 1 (a real code path that can leave a user with zero feedback on failure) and item 3 (silently-live remote token is a minor security/privacy concern); Low for items 2 and 4 (robustness/documentation gaps, not confirmed exploitable).
Level of Effort
Small (S) for items 1 and 2 (add a matching except Exception clause; add a comment or an explicit chat-type/ID-equality helper). Medium (M) for item 3 (needs a decision on retry/surfacing behavior for revocation failures, plus a state field to track "revoke pending"). Small (S) for item 4 (verify/document the existing consume_oauth_state guarantee, or add an explicit check).
Sources
Passing criteria / definition of done
_connect_health catches the same exception surface as its siblings (broad Exception, with the existing (GoogleHealthOAuthError, ValueError) handling preserved for its specific messaging); a test simulates a generic storage failure and asserts the user receives an error message rather than silence.- The private-chat/user-ID-equality authorization check has an explicit comment (or a named helper function) documenting the invariant it relies on.
disconnect()'s behavior on revocation failure is either surfaced to the caller (e.g. a distinct return status) or the "already disconnected" message is changed to avoid implying full success when revocation failed; a test covers the revocation-failure path and asserts the returned/displayed state accurately reflects it.- A comment or test confirms
consume_oauth_state prevents a replayed/stale OAuth callback from reaching notify_health_connection for an unintended chat.
pytest tests/test_telegram_health.py -q passes with the new/updated assertions.ruff check, ruff format --check, and mypy src/blacki/ all pass with no new warnings.
What
Found during a code-reduction audit of the Telegram Google Health command handlers in
telegram/bot.pyandhealth/service.py. Four related error-handling gaps._connect_healthhas narrower exception handling than its three siblings (bot.py:697): it only catches(GoogleHealthOAuthError, ValueError)aroundservice.begin_authorization(...), while_send_health_summary,_refresh_health, and_handle_health_callbackall also catch broadException. Any other failure (e.g. a storage/DB error insidestorage.store_oauth_state) propagates uncaught to the generic top-level handler in_safe_handle_update, which only logs and never messages the user — unlike the other three commands, which always reply with a specific error string.query.from_user.id != chat.idis used to authorize the callback. This is true for Telegram private chats but is an implicit assumption with no comment explaining why it's safe — a future change to allow group-chat health commands would silently break this check.health/service.py,disconnect()): aGoogleHealthApiErrorduring remote token revocation is caught and only logged (logger.warning), while local storage deletion still proceeds. A second/disconnect_healthafterward reports "already disconnected" (storage-layer idempotency) even though the remote Google token may still be live and never actually revoked.notify_health_connectiondoesn't correlate the callback to a specific pending authorization (bot.py:836): it resolveschat_idfromtelegram_user_idalone with no check that this is the same chat/session that initiated/connect_health\| recently. Low likelihood of exploitation givenconsume_oauth_state's behavior, but worth confirming the OAuthstate` correlation actually prevents a stale/replayed callback from notifying an unrelated chat.Priority
Medium for item 1 (a real code path that can leave a user with zero feedback on failure) and item 3 (silently-live remote token is a minor security/privacy concern); Low for items 2 and 4 (robustness/documentation gaps, not confirmed exploitable).
Level of Effort
Small (S) for items 1 and 2 (add a matching
except Exceptionclause; add a comment or an explicit chat-type/ID-equality helper). Medium (M) for item 3 (needs a decision on retry/surfacing behavior for revocation failures, plus a state field to track "revoke pending"). Small (S) for item 4 (verify/document the existingconsume_oauth_stateguarantee, or add an explicit check).Sources
telegram/bot.py_connect_healthtelegram/bot.py_handle_health_callbacktelegram/bot.pynotify_health_connectionhealth/service.pydisconnecttests/test_telegram_health.py(existing coverage to extend)Passing criteria / definition of done
_connect_healthcatches the same exception surface as its siblings (broadException, with the existing(GoogleHealthOAuthError, ValueError)handling preserved for its specific messaging); a test simulates a generic storage failure and asserts the user receives an error message rather than silence.disconnect()'s behavior on revocation failure is either surfaced to the caller (e.g. a distinct return status) or the "already disconnected" message is changed to avoid implying full success when revocation failed; a test covers the revocation-failure path and asserts the returned/displayed state accurately reflects it.consume_oauth_stateprevents a replayed/stale OAuth callback from reachingnotify_health_connectionfor an unintended chat.pytest tests/test_telegram_health.py -qpasses with the new/updated assertions.ruff check,ruff format --check, andmypy src/blacki/all pass with no new warnings.