Skip to content

fix(webhook): fail loudly on unverifiable signatures, add timestamp freshness and replay protection (fixes #11) - #15

Open
Xuxchloris wants to merge 2 commits into
larksuite:mainfrom
Xuxchloris:fix/webhook-signature-hardening
Open

fix(webhook): fail loudly on unverifiable signatures, add timestamp freshness and replay protection (fixes #11)#15
Xuxchloris wants to merge 2 commits into
larksuite:mainfrom
Xuxchloris:fix/webhook-signature-hardening

Conversation

@Xuxchloris

@Xuxchloris Xuxchloris commented Aug 14, 2026

Copy link
Copy Markdown

Fixes #11

Problem

The webhook verification path had three gaps reported in #11:

  1. Silent no-op without a secret. _verify_sign returned immediately when no encrypt_key / verification_token was configured, so a request carrying signature headers was accepted without any verification — an integrator who believes "the SDK verifies signatures" could ship an open webhook without realizing it.
  2. No timestamp freshness check. X-Lark-Request-Timestamp was never checked, so a captured request verified forever.
  3. No replay dedup. No (timestamp, nonce) dedup, so a captured legitimate request could be replayed indefinitely.

Changes

  • lark_channel/core/webhook_signature.py (new): shared verify_webhook_signature() implementing the hardened flow for both the event path (SHA-256 + encrypt key) and the card-action path (SHA-1 + verification token), plus a bounded thread-safe ReplayGuard.
    • No secret + signature headers present → fail loudly: blocked in strict mode, audited (webhook.signature_unverifiable) and warned in compat/audit mode while keeping the legacy accepting behaviour.
    • Requests without signature headers are untouched (the existing missing-signature policy applies), so legacy plaintext flows are unaffected.
    • max_timestamp_skew_seconds (opt-in): stale/missing timestamps are blocked in strict mode, audited (webhook.timestamp_stale) otherwise.
    • replay_protection_seconds (opt-in): duplicate (timestamp, nonce) is blocked in strict mode, audited (webhook.replay_detected) otherwise.
  • lark_channel/channel/config.py: SecurityConfig gains the two opt-in fields (positive-int validated; None preserves legacy behaviour).
  • lark_channel/event/dispatcher_handler.py / lark_channel/card/action_handler.py: _verify_sign now delegates to the shared helper (with per-handler replay guards); plain signature mismatches still surface as webhook.signature_invalid / card.signature_invalid as before.
  • lark_channel/channel/tests/test_webhook_signature_hardening.py (new): 9 tests covering unverifiable (compat audit + strict block, event and card), stale timestamp (strict block + compat audit + fresh pass), replay (strict block, event and card), and config validation.

Verification

  • python -m pytest lark_channel — 988 passed; the single failure (test_upload_error_propagation.py::test_gather_buffer_missing_local_file_raises_upload_failed) is a pre-existing Windows-only path-escaping assertion unrelated to this change (CI runs on Linux).
  • All opt-in checks default to disabled, so existing deployments see no behaviour change unless they configure the new fields or run strict mode with signed requests and no secret.

Follow-up

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

webhook: signature verification silently no-ops without encrypt_key; no timestamp-freshness or replay checks

1 participant