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..6bdaed2 100644 --- a/lark_channel/channel/tests/test_handle_webhook_request.py +++ b/lark_channel/channel/tests/test_handle_webhook_request.py @@ -284,12 +284,55 @@ 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. + + 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( + { + "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), + ) + .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 + ] + + +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 +353,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 +540,52 @@ 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. + + 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( + { + "type": "card.action.trigger", + "action": {"value": {"key": "value"}}, + }, + "encrypt-key", + ) + 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 + ] + + +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 +606,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,