fix(feishu): gate url_verification challenge on verification_token (salvage #29663)#31465
Merged
Conversation
…cation challenge When FEISHU_VERIFICATION_TOKEN is configured, an unauthenticated remote could previously prove endpoint control by sending a url_verification payload with any attacker-controlled challenge string — the handler reflected the challenge BEFORE running the token check. Move the verification_token check ahead of the url_verification echo so the challenge response is gated on a valid token. Add a regression test covering the wrong-token case. Also fix the stale test_connect_webhook_mode_starts_local_server fixture to set FEISHU_VERIFICATION_TOKEN (post #30746 webhook mode requires a secret). Salvaged from PR #29663 by @m0n3r0 — kept the url_verification reorder and its regression test; dropped the host-conditional weakening of the #30746 secret guard (we want webhook secrets required regardless of bind host, not only on 0.0.0.0/::). Docs updated to call out the gating. Co-authored-by: teknium1 <127238744+teknium1@users.noreply.github.com>
Contributor
🔎 Lint report:
|
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
3 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.
Salvage of #29663 by @m0n3r0 onto current main.
Summary
When
FEISHU_VERIFICATION_TOKENis configured, an unauthenticated remote could previously prove endpoint control by sending aurl_verificationpayload with any attacker-controlled challenge string — the handler reflected the challenge before the token check ran. Reorder so the token check gates the challenge response.Changes
gateway/platforms/feishu.py: moveurl_verificationecho to AFTER the verification token checktests/gateway/test_feishu.py: regression test for wrong-token + stale-fixture fix ontest_connect_webhook_mode_starts_local_server(fix(feishu): require webhook auth secret and honor config extras #30746 made FEISHU_VERIFICATION_TOKEN required in webhook mode; the existing fixture didn't set one and broke when that PR landed)website/docs/user-guide/messaging/feishu.md: call out the gatingSalvage scope
The original PR also added
_webhook_requires_secret()that only required webhook secrets when bind-host was0.0.0.0/::— that would have weakened #30746's stricter "webhook mode always requires a secret" guard. Dropped that change and the corresponding test. Docs around env vars in #29663 already merged via #30746.Test plan
Co-authored-by: m0n3r0 34853915+m0n3r0@users.noreply.github.com
Closes #29663