From 6ccdc1f2c3f2dcbc5b2c971025954d9c6b2d6788 Mon Sep 17 00:00:00 2001 From: wz-heng <68931789+wz-heng@users.noreply.github.com> Date: Fri, 14 Aug 2026 21:57:32 +0800 Subject: [PATCH 1/2] fix(webhook): exempt url_verification handshake from strict signature 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 #12 (bug 2 of 2). --- lark_channel/card/action_handler.py | 14 +++ .../tests/test_handle_webhook_request.py | 119 +++++++++++++++++- lark_channel/event/dispatcher_handler.py | 13 ++ 3 files changed, 142 insertions(+), 4 deletions(-) diff --git a/lark_channel/card/action_handler.py b/lark_channel/card/action_handler.py index 4dd556c..948c5bd 100644 --- a/lark_channel/card/action_handler.py +++ b/lark_channel/card/action_handler.py @@ -148,6 +148,13 @@ def _preverify_encrypted_request(self, request: RawRequest) -> bool: ) return False if not self._has_signature_headers(request): + if self._is_url_verification_handshake(request): + # The Feishu console's "save request URL" challenge is not + # signed (platform behavior); it carries verification_token + # as its own proof, checked below in do() like any other + # card callback, so it does not need to pass signature + # pre-verify. + return True action = "legacy_flow" if self._security.is_strict: action = ( @@ -184,6 +191,13 @@ def _has_signature_headers(self, request: RawRequest) -> bool: and Strings.is_not_empty(request.headers.get(LARK_REQUEST_SIGNATURE)) ) + def _is_url_verification_handshake(self, request: RawRequest) -> bool: + try: + card = JSON.unmarshal(self._decrypt(request.body), Card) + except Exception: + return False + return URL_VERIFICATION == card.type + def _record_security_audit( self, reason: str, diff --git a/lark_channel/channel/tests/test_handle_webhook_request.py b/lark_channel/channel/tests/test_handle_webhook_request.py index e31558b..5ce71e8 100644 --- a/lark_channel/channel/tests/test_handle_webhook_request.py +++ b/lark_channel/channel/tests/test_handle_webhook_request.py @@ -284,12 +284,44 @@ def fail_decrypt(*_args, **_kwargs): ] -def test_strict_event_missing_signature_rejects_before_decrypt(monkeypatch): +def test_strict_event_missing_signature_non_handshake_rejects(): + """A non-handshake body must stay blocked even though the SDK now + peeks at decrypted content to check for a url_verification exemption.""" + recorder = InMemorySecurityAuditRecorder() + body = _encrypted_body( + { + "schema": "2.0", + "header": { + "event_type": "example.event", + "token": "verification-token", + }, + "event": {"value": "ok"}, + }, + "encrypt-key", + ) + handler = EventDispatcherHandler.builder( + "encrypt-key", + "verification-token", + security=SecurityConfig(mode="strict", audit_recorder=recorder), + ).build() + + resp = handler.do(_request_bytes(body, {})) + + assert resp.status_code == 500 + assert json.loads(resp.content) == {"code": 500, "msg": "internal error"} + assert [event.reason for event in recorder.events] == [ + REASON_WEBHOOK_SIGNATURE_MISSING + ] + + +def test_strict_event_missing_signature_undecryptable_body_rejects(monkeypatch): + """If the url_verification-exemption peek itself can't decrypt the + body, the request must still be rejected, not silently let through.""" recorder = InMemorySecurityAuditRecorder() body = _encrypted_body({"type": "url_verification"}, "encrypt-key") def fail_decrypt(*_args, **_kwargs): - raise AssertionError("decrypt should not run") + raise AssertionError("decrypt failed") monkeypatch.setattr( "lark_channel.event.dispatcher_handler.AESCipher.decrypt_str", @@ -310,6 +342,31 @@ def fail_decrypt(*_args, **_kwargs): ] +def test_strict_event_unsigned_encrypted_url_verification_handshake_is_exempted(): + """Regression test for the first-time webhook setup deadlock: the + Feishu console's "save request URL" challenge is encrypted but never + signed, so strict mode must not block it before the url_verification + branch gets a chance to answer it.""" + body = _encrypted_body( + { + "type": "url_verification", + "challenge": "challenge-code", + "token": "verification-token", + }, + "encrypt-key", + ) + handler = EventDispatcherHandler.builder( + "encrypt-key", + "verification-token", + security=SecurityConfig(mode="strict"), + ).build() + + resp = handler.do(_request_bytes(body, {})) + + assert resp.status_code == 200 + assert json.loads(resp.content) == {"challenge": "challenge-code"} + + def test_strict_event_unsigned_encrypted_allow_records_allow_action(): seen = [] recorder = InMemorySecurityAuditRecorder() @@ -472,12 +529,41 @@ def fail_decrypt(*_args, **_kwargs): ] -def test_strict_card_missing_signature_rejects_before_decrypt(monkeypatch): +def test_strict_card_missing_signature_non_handshake_rejects(): + """A non-handshake card callback must stay blocked even though the + SDK now peeks at decrypted content to check for a url_verification + exemption.""" + recorder = InMemorySecurityAuditRecorder() + body = _encrypted_body( + { + "type": "card.action.trigger", + "action": {"value": {"key": "value"}}, + }, + "encrypt-key", + ) + handler = CardActionHandler.builder( + "encrypt-key", + "verification-token", + security=SecurityConfig(mode="strict", audit_recorder=recorder), + ).build() + + resp = handler.do(_request_bytes(body, {})) + + assert resp.status_code == 500 + assert json.loads(resp.content) == {"code": 500, "msg": "internal error"} + assert [event.reason for event in recorder.events] == [ + REASON_CARD_SIGNATURE_MISSING + ] + + +def test_strict_card_missing_signature_undecryptable_body_rejects(monkeypatch): + """If the url_verification-exemption peek itself can't decrypt the + body, the request must still be rejected, not silently let through.""" recorder = InMemorySecurityAuditRecorder() body = _encrypted_body({"type": "card.action.trigger"}, "encrypt-key") def fail_decrypt(*_args, **_kwargs): - raise AssertionError("decrypt should not run") + raise AssertionError("decrypt failed") monkeypatch.setattr( "lark_channel.card.action_handler.AESCipher.decrypt_str", @@ -498,6 +584,31 @@ def fail_decrypt(*_args, **_kwargs): ] +def test_strict_card_unsigned_encrypted_url_verification_handshake_is_exempted(): + """Regression test for the first-time webhook setup deadlock on the + card callback URL: the challenge is encrypted but never signed, so + strict mode must not block it before the url_verification branch + gets a chance to answer it.""" + body = _encrypted_body( + { + "type": "url_verification", + "challenge": "challenge-code", + "token": "verification-token", + }, + "encrypt-key", + ) + handler = CardActionHandler.builder( + "encrypt-key", + "verification-token", + security=SecurityConfig(mode="strict"), + ).build() + + resp = handler.do(_request_bytes(body, {})) + + assert resp.status_code == 200 + assert json.loads(resp.content) == {"challenge": "challenge-code"} + + def test_strict_card_unsigned_encrypted_allow_records_allow_action(): seen = [] recorder = InMemorySecurityAuditRecorder() diff --git a/lark_channel/event/dispatcher_handler.py b/lark_channel/event/dispatcher_handler.py index b06f7d7..c540ff6 100644 --- a/lark_channel/event/dispatcher_handler.py +++ b/lark_channel/event/dispatcher_handler.py @@ -187,6 +187,12 @@ def _preverify_encrypted_request(self, request: RawRequest) -> bool: if Strings.is_empty(self._encrypt_key): return False if not self._has_signature_headers(request): + if self._is_url_verification_handshake(request): + # The Feishu console's "save request URL" challenge is not + # signed (platform behavior); it carries verification_token + # as its own proof, checked below in do() like any other + # event, so it does not need to pass signature pre-verify. + return True action = "legacy_flow" if self._security.is_strict: action = ( @@ -223,6 +229,13 @@ def _has_signature_headers(self, request: RawRequest) -> bool: and Strings.is_not_empty(request.headers.get(LARK_REQUEST_SIGNATURE)) ) + def _is_url_verification_handshake(self, request: RawRequest) -> bool: + try: + context = self._parse_context(self._decrypt(request.body)) + except Exception: + return False + return URL_VERIFICATION == context.type + def _record_security_audit( self, reason: str, From 10aac4d7155242cea3b4776a1e669024d2f2e5c7 Mon Sep 17 00:00:00 2001 From: wz-heng <68931789+wz-heng@users.noreply.github.com> Date: Fri, 14 Aug 2026 22:04:06 +0800 Subject: [PATCH 2/2] test(webhook): assert non-handshake requests are never dispatched 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. --- .../tests/test_handle_webhook_request.py | 46 ++++++++++++++----- 1 file changed, 34 insertions(+), 12 deletions(-) diff --git a/lark_channel/channel/tests/test_handle_webhook_request.py b/lark_channel/channel/tests/test_handle_webhook_request.py index 5ce71e8..6bdaed2 100644 --- a/lark_channel/channel/tests/test_handle_webhook_request.py +++ b/lark_channel/channel/tests/test_handle_webhook_request.py @@ -286,7 +286,13 @@ def fail_decrypt(*_args, **_kwargs): def test_strict_event_missing_signature_non_handshake_rejects(): """A non-handshake body must stay blocked even though the SDK now - peeks at decrypted content to check for a url_verification exemption.""" + peeks at decrypted content to check for a url_verification exemption. + + Registers a processor and asserts it is never invoked, so an + implementation that merely logs the missing-signature reason while + still letting the request through would fail this test, not just + one that returns the wrong status code.""" + seen = [] recorder = InMemorySecurityAuditRecorder() body = _encrypted_body( { @@ -299,16 +305,21 @@ def test_strict_event_missing_signature_non_handshake_rejects(): }, "encrypt-key", ) - handler = EventDispatcherHandler.builder( - "encrypt-key", - "verification-token", - security=SecurityConfig(mode="strict", audit_recorder=recorder), - ).build() + handler = ( + EventDispatcherHandler.builder( + "encrypt-key", + "verification-token", + security=SecurityConfig(mode="strict", audit_recorder=recorder), + ) + .register_p2_customized_event("example.event", lambda event: seen.append(event)) + .build() + ) resp = handler.do(_request_bytes(body, {})) assert resp.status_code == 500 assert json.loads(resp.content) == {"code": 500, "msg": "internal error"} + assert len(seen) == 0 assert [event.reason for event in recorder.events] == [ REASON_WEBHOOK_SIGNATURE_MISSING ] @@ -532,7 +543,13 @@ def fail_decrypt(*_args, **_kwargs): def test_strict_card_missing_signature_non_handshake_rejects(): """A non-handshake card callback must stay blocked even though the SDK now peeks at decrypted content to check for a url_verification - exemption.""" + exemption. + + Registers a processor and asserts it is never invoked, so an + implementation that merely logs the missing-signature reason while + still letting the request through would fail this test, not just + one that returns the wrong status code.""" + seen = [] recorder = InMemorySecurityAuditRecorder() body = _encrypted_body( { @@ -541,16 +558,21 @@ def test_strict_card_missing_signature_non_handshake_rejects(): }, "encrypt-key", ) - handler = CardActionHandler.builder( - "encrypt-key", - "verification-token", - security=SecurityConfig(mode="strict", audit_recorder=recorder), - ).build() + handler = ( + CardActionHandler.builder( + "encrypt-key", + "verification-token", + security=SecurityConfig(mode="strict", audit_recorder=recorder), + ) + .register(lambda card: seen.append(card)) + .build() + ) resp = handler.do(_request_bytes(body, {})) assert resp.status_code == 500 assert json.loads(resp.content) == {"code": 500, "msg": "internal error"} + assert len(seen) == 0 assert [event.reason for event in recorder.events] == [ REASON_CARD_SIGNATURE_MISSING ]