Skip to content

fix(weixin): clamp server-suggested long-poll timeout - #76184

Open
SebTardif wants to merge 1 commit into
NousResearch:mainfrom
SebTardif:fix/weixin-clamp-longpoll-timeout
Open

fix(weixin): clamp server-suggested long-poll timeout#76184
SebTardif wants to merge 1 commit into
NousResearch:mainfrom
SebTardif:fix/weixin-clamp-longpoll-timeout

Conversation

@SebTardif

Copy link
Copy Markdown

What does this PR do?

After each Weixin/iLink getUpdates call, the server may return longpolling_timeout_ms. Hermes stores that value and uses it as the next asyncio.wait_for budget for the long-poll.

Previously any positive int was accepted with no upper bound. A huge or malicious suggestion (for example 10**12) pins _poll_loop for an unbounded wall-clock wait and delays disconnect cancellation while the wait is in progress.

This PR keeps trusting legitimate server suggestions (including values above the 35s default boot timeout) but clamps them to MAX_LONG_POLL_TIMEOUT_MS (120s).

Related Issue

No open issue tracked; found during a scoped security/hang audit of gateway/platforms/weixin.py (audit finding F006).

Type of Change

  • Bug fix (non-breaking change that fixes an issue)

Changes Made

  • gateway/platforms/weixin.py: add MAX_LONG_POLL_TIMEOUT_MS and _apply_long_poll_timeout_hint(); use it in _poll_loop instead of unbounded assignment
  • tests/gateway/test_weixin.py: unit tests for clamp, normal keep, and non-positive / non-int keep-current behavior

How to Test

  1. Red/green unit path (no live Weixin needed):
pytest tests/gateway/test_weixin.py -k TestWeixinLongPollTimeoutClamp -q
  1. Expected without the fix: AttributeError / missing clamp (or huge values accepted).
  2. Expected with the fix: 4 tests pass; 10**12 clamps to MAX_LONG_POLL_TIMEOUT_MS.

Checklist

Code

Documentation & Housekeeping

  • Documentation N/A (internal hang guard, no user-facing config)
  • config example N/A
  • CONTRIBUTING/AGENTS N/A
  • Cross-platform N/A (pure timeout math)
  • Tool schemas N/A

Origin

Unbounded assignment was introduced with native Weixin support in 5b63bf7f9a (2026-04-10, "feat(gateway): add native Weixin/WeChat support via iLink Bot API"). Later work (#35117) moved API waits onto asyncio.wait_for but did not cap the server-suggested long-poll budget.

Related

iLink may return longpolling_timeout_ms after getUpdates; the value is
fed into asyncio.wait_for for the next poll. Trust positive ints but
clamp to MAX_LONG_POLL_TIMEOUT_MS so a huge or malicious suggestion
cannot pin the poll task (and delay disconnect) unbounded.

Signed-off-by: Sebastien Tardif <sebtardif@ncf.ca>
@alt-glitch alt-glitch added type/bug Something isn't working comp/gateway Gateway runner, session dispatch, delivery platform/wecom WeCom / WeChat Work adapter P2 Medium — degraded but workaround exists sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages labels Aug 1, 2026
@teknium1

teknium1 commented Aug 1, 2026

Copy link
Copy Markdown
Contributor

Thanks for the focused guard. Current main still assigns any positive server-provided longpolling_timeout_ms directly to the next poll budget at gateway/platforms/weixin.py:1368-1370; that budget reaches asyncio.wait_for through _get_updates (gateway/platforms/weixin.py:438-445) and _api_post (gateway/platforms/weixin.py:402). The proposed clamp therefore addresses a verified current-main failure mode.

The change is narrow, preserves valid positive hints below the ceiling, and the added tests cover the helper's clamp and invalid-hint behavior. Repository-wide search found no sibling consumer of longpolling_timeout_ms.

Automated hermes-sweeper review.

@teknium1 teknium1 added the sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform label Aug 1, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/gateway Gateway runner, session dispatch, delivery P2 Medium — degraded but workaround exists platform/wecom WeCom / WeChat Work adapter sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants