fix(webhook): exempt url_verification handshake from strict signature pre-verify - #14
Open
wz-heng wants to merge 2 commits into
Open
Conversation
… pre-verify The Feishu console's "save request URL" challenge is encrypted but never signed (platform behavior). With an encrypt_key configured, strict mode's encrypted pre-verification rejected the unsigned handshake before the url_verification branch was ever reached, so an integrator using strict mode + encryption could never complete the console's first-time URL-verification step. Peek-decrypt only when signature headers are absent, to check whether the payload is a url_verification handshake; if so, exempt it from the pre-verify signature requirement and let the existing verification_token check downstream (its own proof of identity) and the challenge-echo branch run as normal. Any other unsigned encrypted content, or a payload that fails to decrypt during the peek, is still rejected exactly as before. Fixes larksuite#12 (bug 2 of 2).
test_strict_event_missing_signature_non_handshake_rejects and its card counterpart only asserted a 500 status and the expected audit reason, which a subtly broken exemption (one that records webhook.signature_missing but still lets the request through) could satisfy by coincidence via a later "processor not found" error. Register a processor and assert it is never invoked, so the tests pin down what actually matters: the request never reaches application code. Addresses review feedback from this run's required Snape pass.
|
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes part 2 of #12.
Bug
The Feishu console's "save request URL" challenge (
url_verification)is encrypted (when an encrypt key is configured) but never signed —
that's platform behavior, not something an integrator controls.
With
encrypt_keyconfigured andSecurityConfig(mode="strict"),EventDispatcherHandler._preverify_encrypted_request(and theequivalent branch in
CardActionHandler) rejects any encrypted payloadthat arrives without the three signature headers, before the
url_verificationbranch further downdo()ever gets a chance to run.An integrator using strict mode + encryption can therefore never
complete the console's first-time URL-verification step — the setup
deadlocks.
Fix
When the signature headers are absent, peek-decrypt the payload only to
check whether it's a
url_verificationhandshake. If it is, exempt itfrom the pre-verify signature requirement — its own
verification_token(checked immediately afterward in
do(), same as every other event) isits proof of identity, per Feishu's own design. Any other unsigned
encrypted content — or a payload that fails to decrypt during the peek —
is rejected exactly as before; this does not touch the
allow_unsigned_encrypted_webhookescape hatch or weaken strict mode foranything other than the handshake itself.
Testing
url_verificationhandshake instrict mode now gets a 200 + echoed challenge (both handlers).
requests never reach decrypt") predates this fix and is no longer
literally true — decrypt is now attempted to check for the handshake
exemption. Renamed/re-scoped them to assert what actually matters:
non-handshake unsigned content is still blocked, and a payload that
fails to decrypt during the peek is still blocked (not silently let
through).
Full suite:
python -m pytest -q→ 1102 passed.🤖 Generated with Claude Code