fix(wecom): constant-time callback signature comparison

[zapabob]

Summary

WeCom callback signature verification (gateway/platforms/wecom_crypto.py) compared the expected SHA1 signature with the attacker-supplied msg_signature using a plain != string comparison.

python expected = _sha1_signature(self.token, timestamp, nonce, encrypt) if expected != msg_signature: raise SignatureError("signature mismatch")

Python's str.__ne__ short-circuits on the first differing byte, so the verification time is proportional to the number of leading characters that matched. This is a classic timing side-channel: a network attacker who controls timestamp / nonce / echostr in a callback request can measure response latency and recover a valid msg_signature byte-by-byte, then forge authenticated WeCom callbacks (spoofed inbound messages, URL-verification bypass).

Fix

• Compare with hmac.compare_digest (constant-time), matching the existing pattern in the Feishu (feishu.py), LINE (line.py), and webhook (webhook.py) verifiers.
• Coerce a missing signature to "" so a None value raises a clean SignatureError instead of a TypeError.

No behavior change for legitimate callers — valid signatures still pass, invalid ones still raise SignatureError.

Tests

tests/gateway/test_wecom_callback.py:

• existing roundtrip + mismatch tests still pass
• new: asserts the verification path goes through hmac.compare_digest
• new: None signature raises SignatureError (not TypeError)

.venv/Scripts/python -m pytest tests/gateway/test_wecom_callback.py → 14 passed.

Scope

Upstream-only file, no fork-specific code. Distinct from the open WeCom PRs (media attachments / target-ref parsing), which do not touch the crypto verifier.

Made with Cursor

[liuhao1024]

Verification review — reviewed the constant-time signature comparison fix.

The change is correct and minimal:

1. hmac.compare_digest(expected, msg_signature or "") replaces expected != msg_signature — standard timing-attack mitigation.
2. The or "" guard handles None signatures gracefully (tested explicitly in test_signature_none_does_not_raise_typeerror).
3. The spy test (test_signature_check_uses_constant_time_compare) confirms the comparison goes through hmac.compare_digest via monkeypatch.
4. No other code paths perform signature comparison in this module — the single != was the only leak.

Clean security fix.

[liuhao1024]

Verification review — reviewed the diff for constant-time signature comparison quality.

The fix is clean and correct:

1. Replacing != with hmac.compare_digest() prevents timing side-channel attacks on the callback signature verification. A plain string comparison leaks character-by-character match information via response timing.
2. The or "" guard on msg_signature correctly handles None inputs — surfaces as SignatureError rather than TypeError.
3. Tests verify both: (a) that hmac.compare_digest is actually called (spy pattern), and (b) that None signature raises SignatureError.

This is a minimal, well-scoped security hardening — one line of production code changed, three focused tests added. No behavioral regressions expected.

[tonydwb]

Code Review Summary

Verdict: Approved

Security: WeCom constant-time callback signature comparison

• Timing attack fix: replaces expected != msg_signature with hmac.compare_digest(expected, msg_signature or ""). A plain != leaks how many leading characters matched via response timing, allowing a network attacker to forge a valid signature byte-by-byte.
• None guard: msg_signature or "" prevents TypeError when the signature field is missing/None, converting it to a guaranteed mismatch.
• Tests: spy test confirms hmac.compare_digest is called, plus a test for the None signature case.

---

Reviewed by Hermes Agent

[teknium1]

Thanks for the focused security hardening. The premise remains valid on current main, but the PR must be relocated before it can be salvaged.

Problems

• The changed production file, gateway/platforms/wecom_crypto.py, was renamed by 5600105478ffde29d7566b45421b100eaa29c4ef to plugins/platforms/wecom/wecom_crypto.py. The active verifier still uses expected != msg_signature at plugins/platforms/wecom/wecom_crypto.py:90, so the proposed edit would not reach the live code.
• The added spy test imports gateway.platforms.wecom_crypto; current tests import the plugin module at tests/gateway/test_wecom_callback.py:10.

Suggested changes

• Move the hmac.compare_digest(expected, msg_signature or "") change to plugins/platforms/wecom/wecom_crypto.py:90.
• Update the new test import to plugins.platforms.wecom.wecom_crypto.

Automated hermes-sweeper review.

[teknium1]

This source file was renamed to plugins/platforms/wecom/wecom_crypto.py by 5600105478ffde29d7566b45421b100eaa29c4ef. Please transplant this change to the active plugin file; its live comparison remains at line 90.

[egilewski]

suggesting changes

The timing-safe comparison is the right direction, but it compares Python str values directly. A percent-encoded non-ASCII msg_signature causes hmac.compare_digest to raise TypeError instead of taking the normal invalid-signature rejection path. Compare normalized byte strings, preserve the missing-value handling, and add a regression test for non-ASCII input. This keeps malformed pre-auth requests on the same fail-closed path and avoids avoidable error/log load.

Security evidence:

• trust boundary: WeCom callback query parameters are unauthenticated request input.
• source/sink/invariant: msg_signature reaches WXBizMsgCrypt before decryption and event handling; only an exact signature should pass.
• current-main reproduction: current main successfully decrypts valid ciphertext and rejects missing, bad, and non-ASCII signatures with SignatureError.
• PR-head or patch-replay validation: the replay preserves valid, missing, and bad-ASCII behavior, but a non-ASCII signature raises TypeError inside the new comparison.
• positive/negative cases: valid decryption succeeds; missing and bad signatures reject; Unicode input reaches the generic callback exception path.
• residual bypass search: both URL verification and POST decryption use this helper; no alternate WeCom signature verifier was found, and adjacent verifiers compare encoded bytes.
• reviewer validation: the focused callback tests pass (8 passed); the new tests cover helper use and missing input but not the Unicode regression.

Not checked:

• Full test suite
• CodeRabbit review

Signed: GPT-5.6-sol-xhigh in Codex