diff --git a/package.json b/package.json index 5bf45a9ac01cd..ee438272692d0 100644 --- a/package.json +++ b/package.json @@ -114,6 +114,7 @@ "@types/convert-source-map": "^2.0.3", "@types/css-tree": "^2.3.11", "@types/eslint-config-prettier": "^6.11.3", + "@types/estree": "^1.0.9", "@types/gtag.js": "^0.0.20", "@types/is-url": "^1.2.32", "@types/jquery": "^4.0.0", diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 6db1dfeca5512..bdf017bcc8e29 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -355,6 +355,9 @@ importers: '@types/eslint-config-prettier': specifier: ^6.11.3 version: 6.11.3 + '@types/estree': + specifier: ^1.0.9 + version: 1.0.9 '@types/gtag.js': specifier: ^0.0.20 version: 0.0.20 diff --git a/templates/zerver/development/integrations_dev_panel.html b/templates/zerver/development/integrations_dev_panel.html index b948daf30bbaf..2f9e5f6e076d8 100644 --- a/templates/zerver/development/integrations_dev_panel.html +++ b/templates/zerver/development/integrations_dev_panel.html @@ -69,6 +69,10 @@ +
+ + +

diff --git a/web/src/portico/integrations_dev_panel.ts b/web/src/portico/integrations_dev_panel.ts index 240c7a5e4cea1..56ec628158bb3 100644 --- a/web/src/portico/integrations_dev_panel.ts +++ b/web/src/portico/integrations_dev_panel.ts @@ -49,6 +49,8 @@ const integrations_api_response_schema = z.object({ type ServerResponse = z.infer; +let last_computed_header_key: string | null = null; // Tracks the current signature header for auto-clearing when switching integrations + const loaded_fixtures = new Map(); const url_base = "/api/v1/external/"; @@ -231,11 +233,83 @@ function update_url(): void { params.set("topic", topic_name); } } + const webhook_secret = $("input#webhook_secret").val()!; const url = `${url_base}${integration_name}?${params.toString()}`; url_field!.value = url; + + sync_signature_headers(integration_name, webhook_secret); } +} - return; +function sync_signature_headers(integration_name: string, webhook_secret: string): void { + const $custom_headers_field = $("textarea#custom_http_headers"); + const current_headers_raw = $custom_headers_field.val()?.toString().trim() ?? ""; + + let headers_object: Record = {}; + if (current_headers_raw !== "") { + try { + headers_object = z + .record(z.string(), z.string()) + .parse(JSON.parse(current_headers_raw)); + } catch { + headers_object = {}; + } + } + + if (last_computed_header_key && Object.hasOwn(headers_object, last_computed_header_key)) { + Reflect.deleteProperty(headers_object, last_computed_header_key); + } + + if (webhook_secret.trim() === "") { + last_computed_header_key = null; + if (Object.keys(headers_object).length === 0) { + $custom_headers_field.val("{}"); + } else { + $custom_headers_field.val(JSON.stringify(headers_object, null, 4)); + } + return; + } + + const raw_payload = $("textarea#fixture_body").val() ?? ""; + let cleaned_payload: string; + + try { + cleaned_payload = JSON.stringify(JSON.parse(raw_payload)); + } catch { + cleaned_payload = raw_payload.trim(); + } + + channel.post({ + url: "/devtools/integrations/recalculate_signature", + data: JSON.stringify({ + secret: webhook_secret, + payload: cleaned_payload, + integration_name, + }), + success(raw_data: unknown) { + const data = z + .object({ + supported: z.optional(z.boolean()), + clear_signature: z.optional(z.boolean()), + header_key: z.string(), + signature: z.string(), + }) + .parse(raw_data); + + if (!data.supported || data.clear_signature) { + last_computed_header_key = null; + if (Object.keys(headers_object).length === 0) { + $custom_headers_field.val("{}"); + } else { + $custom_headers_field.val(JSON.stringify(headers_object, null, 4)); + } + } else { + headers_object[data.header_key] = data.signature; + last_computed_header_key = data.header_key; + $custom_headers_field.val(JSON.stringify(headers_object, null, 4)); + } + }, + }); } // API callers: These methods handle communicating with the Python backend API. @@ -440,4 +514,6 @@ $(() => { $("#stream_name").on("change", update_url); $("#topic_name").on("change", update_url); + + $("#webhook_secret").on("change", update_url); }); diff --git a/web/styles/portico/integrations_dev_panel.css b/web/styles/portico/integrations_dev_panel.css index fdf156382f823..31394093e93a6 100644 --- a/web/styles/portico/integrations_dev_panel.css +++ b/web/styles/portico/integrations_dev_panel.css @@ -105,7 +105,8 @@ } #stream_name, -#topic_name { +#topic_name, +#webhook_secret { width: 206px; } diff --git a/zerver/decorator.py b/zerver/decorator.py index 039e89d0199f3..4f1df0e945563 100644 --- a/zerver/decorator.py +++ b/zerver/decorator.py @@ -54,7 +54,9 @@ from zerver.lib.utils import has_api_key_format from zerver.lib.webhooks.common import ( MissingHTTPEventHeaderError, + WebhookSignatureConfig, notify_bot_owner_about_invalid_json, + validate_webhook_signature, ) from zerver.models import UserProfile from zerver.models.clients import get_client @@ -372,6 +374,7 @@ def webhook_view( webhook_client_name: str, notify_bot_owner_on_invalid_json: bool = True, all_event_types: Sequence[str] | None = None, + signature_config: WebhookSignatureConfig | None = None, ) -> Callable[[Callable[..., HttpResponse]], Callable[..., HttpResponse]]: # Unfortunately, callback protocols are insufficient for this: # https://mypy.readthedocs.io/en/stable/protocols.html#callback-protocols @@ -390,6 +393,11 @@ def _wrapped_func_arguments( allow_webhook_access=True, client_name=full_webhook_client_name(webhook_client_name), ) + validate_webhook_signature( + request, + user_profile, + signature_config, + ) request_notes = RequestNotes.get_notes(request) request_notes.is_webhook_view = True diff --git a/zerver/lib/integrations.py b/zerver/lib/integrations.py index 35c715d6e1fe0..1955a2da0bfa4 100644 --- a/zerver/lib/integrations.py +++ b/zerver/lib/integrations.py @@ -12,7 +12,12 @@ from typing_extensions import override from zerver.lib.storage import static_path -from zerver.lib.webhooks.common import PresetUrlOption, WebhookConfigOption, WebhookUrlOption +from zerver.lib.webhooks.common import ( + PresetUrlOption, + WebhookConfigOption, + WebhookSignatureConfig, + WebhookUrlOption, +) from zerver.webhooks import fixtureless_integrations """This module declares all of the (documented) integrations available @@ -1196,6 +1201,15 @@ def is_enabled_in_catalog(self) -> bool: | hubot_integration_names ) +WEBHOOK_SIGNATURE_CONFIGS: dict[str, WebhookSignatureConfig] = { + "github": WebhookSignatureConfig( + integration_name="github", + header="X_HUB_SIGNATURE_256", + algorithm="sha256", + prefix="sha256=", + ), +} + # Add integrations that are not meant to have example screenshots here INTEGRATIONS_WITHOUT_SCREENSHOTS = ( # Integration frameworks diff --git a/zerver/lib/test_classes.py b/zerver/lib/test_classes.py index 6f3c061f8644e..fee227442b7f7 100644 --- a/zerver/lib/test_classes.py +++ b/zerver/lib/test_classes.py @@ -35,6 +35,7 @@ from django.test.testcases import SerializeMixin from django.urls import resolve from django.utils import translation +from django.utils.encoding import force_bytes from django.utils.module_loading import import_string from django.utils.timezone import now as timezone_now from fakeldap import MockLDAP @@ -54,9 +55,11 @@ from zerver.actions.user_settings import do_change_full_name, do_change_user_setting from zerver.actions.users import do_change_user_role from zerver.decorator import do_two_factor_login +from zerver.lib.bot_config import set_bot_config from zerver.lib.cache import bounce_key_prefix_for_testing from zerver.lib.email_notifications import MissedMessageData, handle_missedmessage_emails from zerver.lib.initial_password import initial_password +from zerver.lib.integrations import WEBHOOK_SIGNATURE_CONFIGS from zerver.lib.mdiff import diff_strings from zerver.lib.message import access_message from zerver.lib.notification_data import UserMessageNotificationsData @@ -92,8 +95,10 @@ from zerver.lib.upload import upload_message_attachment_from_request from zerver.lib.user_groups import get_system_user_group_for_user from zerver.lib.webhooks.common import ( + WEBHOOK_SECRET_TOKEN_KEY, call_fixture_to_headers, check_send_webhook_message, + compute_webhook_signature, standardize_headers, ) from zerver.models import ( @@ -2544,6 +2549,7 @@ class WebhookTestCase(ZulipTestCase): DEFAULT_URL_TEMPLATE: str = ( "/api/v1/external/{webhook_dir_name}?stream={stream}&api_key={api_key}" ) + WEBHOOK_TEST_SECRET: str | None = None def get_webhook_dir_name(self) -> str: module_parts = self.__module__.split(".") @@ -2650,16 +2656,52 @@ def check_webhook( """ self.subscribe(self.test_user, self.channel_name) + url = getattr(self, "url", None) + if url is None: + url = self.build_webhook_url() # nocoverage + + webhook_secret = getattr(self, "WEBHOOK_TEST_SECRET", None) + config = WEBHOOK_SIGNATURE_CONFIGS.get(self.webhook_dir_name.lower()) + + if webhook_secret is not None and config is None: + raise AssertionError( + f"WEBHOOK_TEST_SECRET was set for '{self.webhook_dir_name}', " + f"but no WebhookSignatureConfig is registered in WEBHOOK_SIGNATURE_CONFIGS." + ) + payload = self.get_payload(fixture_name) if content_type is not None: extra["content_type"] = content_type + + if webhook_secret is not None and config is not None: + set_bot_config( + self.test_user, + WEBHOOK_SECRET_TOKEN_KEY.format(integration_name=self.webhook_dir_name.lower()), + webhook_secret, + ) + + try: + raw_payload = self.get_body(fixture_name) + except FileNotFoundError: # nocoverage + raw_payload = "" + + header_val = compute_webhook_signature( + force_bytes(webhook_secret), + force_bytes(raw_payload), + config, + ) + + django_header = "HTTP_" + config.header.upper().replace("-", "_") + if django_header not in extra: + extra[django_header] = header_val + headers = call_fixture_to_headers(self.webhook_dir_name, fixture_name) headers = standardize_headers(headers) extra.update(headers) try: msg = self.send_webhook_payload( self.test_user, - self.url, + url, payload, **extra, ) @@ -2715,6 +2757,11 @@ def send_and_test_private_message( Most webhooks send to streams, and you will want to look at check_webhook. """ + + webhook_secret = getattr(self, "WEBHOOK_TEST_SECRET", None) + if webhook_secret is not None: + set_bot_config(self.test_user, "webhook_secret", webhook_secret) # nocoverage + payload = self.get_payload(fixture_name) extra["content_type"] = content_type diff --git a/zerver/lib/webhooks/common.py b/zerver/lib/webhooks/common.py index ac2d8ba7e7a16..e0918b783f562 100644 --- a/zerver/lib/webhooks/common.py +++ b/zerver/lib/webhooks/common.py @@ -25,6 +25,7 @@ check_send_stream_message_by_id, send_rate_limited_pm_notification_to_bot_owner, ) +from zerver.lib.bot_config import ConfigError, get_bot_config from zerver.lib.exceptions import ( AnomalousWebhookPayloadError, ErrorCode, @@ -58,6 +59,8 @@ SETUP_MESSAGE_TEMPLATE = "{integration} webhook has been successfully configured" SETUP_MESSAGE_USER_PART = " by {user_name}" +WEBHOOK_SECRET_TOKEN_KEY = "{integration_name}:webhook_secret_token" + OptionalUserSpecifiedTopicStr: TypeAlias = Annotated[str | None, ApiParamConfig("topic")] @@ -74,6 +77,16 @@ class WebhookConfigOption: validator: Callable[[str, str], str | bool | None] +@dataclass(frozen=True) +class WebhookSignatureConfig: + integration_name: str + header: str + algorithm: str = "sha256" + prefix: str = "" + custom_formatter: Callable[[str], str] | None = None + # This will override the default compute_webhook_signature function if provided for unique formats + + @dataclass class WebhookUrlOption: name: str @@ -321,36 +334,65 @@ def parse_multipart_string(body: str) -> dict[str, str]: def validate_webhook_signature( - request: HttpRequest, payload: str, signature: str, algorithm: str = "sha256" + request: HttpRequest, + user_profile: UserProfile, + config: WebhookSignatureConfig | None, ) -> None: - if not settings.VERIFY_WEBHOOK_SIGNATURES: # nocoverage + if not settings.VERIFY_WEBHOOK_SIGNATURES or not config: return - if algorithm not in hashlib.algorithms_available: + if config.algorithm not in hashlib.algorithms_available: raise AssertionError( - _("The algorithm '{algorithm}' is not supported.").format(algorithm=algorithm) + _("The algorithm '{algorithm}' is not supported.").format(algorithm=config.algorithm) ) - webhook_secret: str | None = request.GET.get("webhook_secret") - if webhook_secret is None: - raise JsonableError( - _( - "The webhook secret is missing. Please set the webhook_secret while generating the URL." - ) - ) - webhook_secret_bytes = force_bytes(webhook_secret) - payload_bytes = force_bytes(payload) + signature_header = request.headers.get(config.header) + if not signature_header: + return - signed_payload = hmac.new( - webhook_secret_bytes, - payload_bytes, - algorithm, - ).hexdigest() + try: + bot_config = get_bot_config(user_profile) + except ConfigError: + return - if not constant_time_compare(signed_payload, signature): + webhook_secret = bot_config.get( + WEBHOOK_SECRET_TOKEN_KEY.format(integration_name=config.integration_name.lower()) + ) + + if not webhook_secret or not webhook_secret.strip(): + raise JsonableError(_("Webhook secret is not configured for this bot.")) + + payload = request.body.decode("utf-8") + + expected_header_val = compute_webhook_signature( + force_bytes(webhook_secret), + force_bytes(payload), + config, + ) + if not constant_time_compare(expected_header_val, signature_header): raise JsonableError(_("Webhook signature verification failed.")) +def compute_webhook_signature( + secret_bytes: bytes, + payload_bytes: bytes, + config: WebhookSignatureConfig, +) -> str: + """Computes and formats the HMAC signature for a webhook payload.""" + signer = hmac.new( + secret_bytes, + payload_bytes, + config.algorithm, + ) + digest = signer.hexdigest() + + if config.custom_formatter is not None: + return config.custom_formatter(digest) + if config.prefix: + return f"{config.prefix}{digest}" + return digest + + def guess_zulip_user_from_external_account( realm: Realm, external_username: str, diff --git a/zerver/tests/test_integrations_dev_panel.py b/zerver/tests/test_integrations_dev_panel.py index ad46910f00f3a..7188affc10bb2 100644 --- a/zerver/tests/test_integrations_dev_panel.py +++ b/zerver/tests/test_integrations_dev_panel.py @@ -1,3 +1,5 @@ +import hashlib +import hmac from unittest.mock import MagicMock, patch import orjson @@ -339,3 +341,155 @@ def test_send_all_webhook_fixture_messages_for_missing_fixtures( } self.assertEqual(response.status_code, 404) self.assertEqual(orjson.loads(response.content), expected_response) + + def test_recalculate_signature_method_not_allowed(self) -> None: + target_url = "/devtools/integrations/recalculate_signature" + # The endpoint expects a POST request. GET should fail with 405. + response = self.client_get(target_url) + self.assertEqual(response.status_code, 405) + self.assertEqual(orjson.loads(response.content), {"error": "Method not allowed"}) + + def test_recalculate_signature_unsupported_integration(self) -> None: + target_url = "/devtools/integrations/recalculate_signature" + data = { + "secret": "my_secret", + "payload": '{"event": "ping"}', + "integration_name": "unsupported_platform", + } + response = self.client_post(target_url, data, content_type="application/json") + self.assertEqual(response.status_code, 200) + + expected_response = { + "supported": False, + "msg": "No signature rules configured for this platform.", + } + self.assertEqual(orjson.loads(response.content), expected_response) + + def test_recalculate_signature_empty_secret_triggers_clear(self) -> None: + target_url = "/devtools/integrations/recalculate_signature" + data = { + "secret": "", + "payload": '{"event": "ping"}', + "integration_name": "github", + } + response = self.client_post(target_url, data, content_type="application/json") + self.assertEqual(response.status_code, 200) + + expected_response = {"supported": True, "clear_signature": True} + self.assertEqual(orjson.loads(response.content), expected_response) + + def test_recalculate_signature_success_with_json_payload(self) -> None: + target_url = "/devtools/integrations/recalculate_signature" + secret = "github_webhook_secret" + + payload = '{\n "zen": "Non-blocking is better than blocking."\n}' + + data = { + "secret": secret, + "payload": payload, + "integration_name": "github ", # Tests trimming behavior + } + + # Manually compute the expected HMAC hash of minified JSON + minified_payload_bytes = orjson.dumps(orjson.loads(payload)) + expected_hash = hmac.new( + secret.encode(), minified_payload_bytes, hashlib.sha256 + ).hexdigest() + + response = self.client_post(target_url, data, content_type="application/json") + self.assertEqual(response.status_code, 200) + + expected_response = { + "supported": True, + "clear_signature": False, + "header_key": "X_HUB_SIGNATURE_256", + "signature": f"sha256={expected_hash}", + } + self.assertEqual(orjson.loads(response.content), expected_response) + + def test_recalculate_signature_success_with_non_json_payload(self) -> None: + target_url = "/devtools/integrations/recalculate_signature" + secret = "github_webhook_secret" + payload = "plain-text-payload-string" + + data = { + "secret": secret, + "payload": payload, + "integration_name": "GITHUB", + } + + # Falls back to plain text bytes computation upon JSON extraction failure + expected_hash = hmac.new(secret.encode(), payload.encode(), hashlib.sha256).hexdigest() + + response = self.client_post(target_url, data, content_type="application/json") + self.assertEqual(response.status_code, 200) + + expected_response = { + "supported": True, + "clear_signature": False, + "header_key": "X_HUB_SIGNATURE_256", + "signature": f"sha256={expected_hash}", + } + self.assertEqual(orjson.loads(response.content), expected_response) + + def test_recalculate_signature_exception_handling(self) -> None: + target_url = "/devtools/integrations/recalculate_signature" + + # Sending a malformed request context (e.g. string payload instead of valid json object) + # to force the parsing logic down the general exception handling path. + response = self.client_post( + target_url, "invalid_json_body", content_type="application/json" + ) + self.assertEqual(response.status_code, 400) + + response_data = orjson.loads(response.content) + self.assertIn("error", response_data) + + def test_sync_signature_headers_endpoint_success(self) -> None: + """Tests the backend counterpart of sync_signature_headers for a valid integration.""" + target_url = "/devtools/integrations/recalculate_signature" + data = { + "secret": "my_webhook_secret", + "payload": '{"event": "ping"}', + "integration_name": "github", + } + + response = self.client_post(target_url, data, content_type="application/json") + self.assertEqual(response.status_code, 200) + + response_data = orjson.loads(response.content) + self.assertTrue(response_data["supported"]) + self.assertFalse(response_data["clear_signature"]) + self.assertEqual(response_data["header_key"], "X_HUB_SIGNATURE_256") + self.assertTrue(response_data["signature"].startswith("sha256=")) + + def test_sync_signature_headers_endpoint_empty_secret(self) -> None: + """Tests that passing an empty secret returns clear_signature=True to clear the UI instantly.""" + target_url = "/devtools/integrations/recalculate_signature" + data = { + "secret": "", + "payload": '{"event": "ping"}', + "integration_name": "github", + } + + response = self.client_post(target_url, data, content_type="application/json") + self.assertEqual(response.status_code, 200) + + response_data = orjson.loads(response.content) + self.assertTrue(response_data["supported"]) + self.assertTrue(response_data["clear_signature"]) + + def test_sync_signature_headers_endpoint_unsupported(self) -> None: + """Tests that an unregistered integration name returns supported=False to drop headers.""" + target_url = "/devtools/integrations/recalculate_signature" + data = { + "secret": "secret", + "payload": "{}", + "integration_name": "some_random_platform", + } + + response = self.client_post(target_url, data, content_type="application/json") + self.assertEqual(response.status_code, 200) + + response_data = orjson.loads(response.content) + self.assertFalse(response_data["supported"]) diff --git a/zerver/tests/test_webhooks_common.py b/zerver/tests/test_webhooks_common.py index 02db0bdd1ce09..4fd7f6949c7ad 100644 --- a/zerver/tests/test_webhooks_common.py +++ b/zerver/tests/test_webhooks_common.py @@ -1,5 +1,3 @@ -import hashlib -import hmac from types import SimpleNamespace from unittest.mock import MagicMock, patch @@ -14,6 +12,7 @@ from zerver.actions.custom_profile_fields import try_add_realm_custom_profile_field from zerver.actions.streams import do_rename_stream from zerver.decorator import webhook_view +from zerver.lib.bot_config import ConfigError, set_bot_config from zerver.lib.exceptions import InvalidJSONError, JsonableError from zerver.lib.request import RequestNotes from zerver.lib.send_email import FromAddress @@ -22,9 +21,12 @@ from zerver.lib.webhooks.common import ( INVALID_JSON_MESSAGE, MISSING_EVENT_HEADER_MESSAGE, + WEBHOOK_SECRET_TOKEN_KEY, MissingHTTPEventHeaderError, + WebhookSignatureConfig, call_fixture_to_headers, check_send_webhook_message, + compute_webhook_signature, get_event_header, get_service_api_data, guess_zulip_user_from_external_account, @@ -152,34 +154,95 @@ def test_standardize_headers(self) -> None: @override_settings(VERIFY_WEBHOOK_SIGNATURES=True) def test_validate_webhook_signature(self) -> None: - request = HostRequestMock() - request.GET = QueryDict("", mutable=True) - - # Valid signature + webhook_bot = get_user("webhook-bot@zulip.com", get_realm("zulip")) webhook_secret = "test_secret" + config = WebhookSignatureConfig( + integration_name="github", + header="X-Hub-Signature-256", + algorithm="sha256", + prefix="sha256=", + ) payload = '{"key": "value"}' - signature = hmac.new( - force_bytes(webhook_secret), force_bytes(payload), hashlib.sha256 - ).hexdigest() + signature = compute_webhook_signature( + force_bytes(webhook_secret), force_bytes(payload), config + ) + + # Missing header early return check + request = HostRequestMock(meta_data={}) + request.user = webhook_bot + request.GET = QueryDict("", mutable=True) + request._body = force_bytes(payload) + validate_webhook_signature(request, webhook_bot, config) + + # ConfigError early return check + request = HostRequestMock(meta_data={"HTTP_X_HUB_SIGNATURE_256": signature}) + request.user = webhook_bot + request.GET = QueryDict("", mutable=True) + request._body = force_bytes(payload) + with patch("zerver.lib.webhooks.common.get_bot_config", side_effect=ConfigError): + validate_webhook_signature(request, webhook_bot, config) - request.GET.update({"webhook_secret": webhook_secret}) - validate_webhook_signature(request, payload, signature) + # Unconfigured secret initial pass + request = HostRequestMock(meta_data={"HTTP_X_HUB_SIGNATURE_256": signature}) + request.user = webhook_bot + request.GET = QueryDict("", mutable=True) + request._body = force_bytes(payload) + validate_webhook_signature(request, webhook_bot, config) - # Invalid signature - invalid_signature = "invalid_signature" + # Valid signature with configured token key + set_bot_config( + webhook_bot, WEBHOOK_SECRET_TOKEN_KEY.format(integration_name="github"), webhook_secret + ) + request = HostRequestMock(meta_data={"HTTP_X_HUB_SIGNATURE_256": signature}) + request.user = webhook_bot + request.GET = QueryDict("", mutable=True) + request._body = force_bytes(payload) + validate_webhook_signature(request, webhook_bot, config) + + # Invalid signature check + request.META["HTTP_X_HUB_SIGNATURE_256"] = "sha256=invalid_signature" + del request.headers with self.assertRaisesRegex( JsonableError, "Webhook signature verification failed.", ): - validate_webhook_signature(request, payload, invalid_signature) + validate_webhook_signature(request, webhook_bot, config) - # No webhook_secret parameter - request.GET.clear() + # Missing secret check using token key + set_bot_config(webhook_bot, WEBHOOK_SECRET_TOKEN_KEY.format(integration_name="github"), "") + request.META["HTTP_X_HUB_SIGNATURE_256"] = signature + del request.headers with self.assertRaisesRegex( JsonableError, - "The webhook secret is missing. Please set the webhook_secret while generating the URL.", + "Webhook secret is not configured for this bot.", ): - validate_webhook_signature(request, payload, signature) + validate_webhook_signature(request, webhook_bot, config=config) + + def test_compute_webhook_signature_formatter_and_prefix(self) -> None: + # Tests the custom_formatter + config_formatter = WebhookSignatureConfig( + integration_name="test", + header="X-Test-Signature", + custom_formatter=lambda d: f"sha256={d.upper()}", + ) + sig_formatter = compute_webhook_signature(b"secret", b"payload", config_formatter) + self.assertTrue(sig_formatter.startswith("sha256=")) + + # Tests prefix + config_prefix = WebhookSignatureConfig( + integration_name="test", + header="X-Test-Signature", + prefix="sha256=", + ) + sig_prefix = compute_webhook_signature(b"secret", b"payload", config_prefix) + self.assertTrue(sig_prefix.startswith("sha256=")) + + config_default = WebhookSignatureConfig( + integration_name="test", + header="X-Test-Signature", + ) + sig_default = compute_webhook_signature(b"secret", b"payload", config_default) + self.assertFalse(sig_default.startswith("sha256=")) def test_check_send_webhook_message_returns_id(self) -> None: webhook_bot = get_user("webhook-bot@zulip.com", get_realm("zulip")) diff --git a/zerver/views/development/integrations.py b/zerver/views/development/integrations.py index a3ec1ce71221c..d2f74de455526 100644 --- a/zerver/views/development/integrations.py +++ b/zerver/views/development/integrations.py @@ -3,17 +3,23 @@ from typing import TYPE_CHECKING, Any import orjson -from django.http import HttpRequest, HttpResponse +from django.http import HttpRequest, HttpResponse, JsonResponse from django.http.response import HttpResponseBase from django.shortcuts import render from django.test import Client +from django.utils.encoding import force_bytes +from django.views.decorators.csrf import csrf_exempt from pydantic import Json from zerver.lib.exceptions import JsonableError, ResourceNotFoundError -from zerver.lib.integrations import INCOMING_WEBHOOK_INTEGRATIONS +from zerver.lib.integrations import INCOMING_WEBHOOK_INTEGRATIONS, WEBHOOK_SIGNATURE_CONFIGS from zerver.lib.response import json_success from zerver.lib.typed_endpoint import PathOnly, typed_endpoint -from zerver.lib.webhooks.common import call_fixture_to_headers, standardize_headers +from zerver.lib.webhooks.common import ( + call_fixture_to_headers, + compute_webhook_signature, + standardize_headers, +) from zerver.models import UserProfile from zerver.models.realms import get_realm @@ -156,3 +162,49 @@ def send_all_webhook_fixture_messages( } ) return json_success(request, data={"responses": responses}) + + +@csrf_exempt +def recalculate_signature(request: HttpRequest) -> JsonResponse: + """ + Endpoint invoked by the frontend UI dev panel to compute + and format signature header blocks based on the integration. + """ + if request.method != "POST": + return JsonResponse({"error": "Method not allowed"}, status=405) + + try: + data = orjson.loads(request.body) + secret = data.get("secret", "") + payload_string = data.get("payload", "") + integration_name = data.get("integration_name", "").lower().strip() + + if integration_name not in WEBHOOK_SIGNATURE_CONFIGS: + return JsonResponse( + {"supported": False, "msg": "No signature rules configured for this platform."} + ) + + if not secret: + return JsonResponse({"supported": True, "clear_signature": True}) + + try: + payload_bytes = orjson.dumps(orjson.loads(payload_string)) + except Exception: + payload_bytes = force_bytes(payload_string) + + webhook_secret_bytes = force_bytes(secret) + config = WEBHOOK_SIGNATURE_CONFIGS[integration_name] + header_key = config.header + header_value = compute_webhook_signature(webhook_secret_bytes, payload_bytes, config) + + return JsonResponse( + { + "supported": True, + "clear_signature": False, + "header_key": header_key, + "signature": header_value, + } + ) + + except Exception: + return JsonResponse({"error": "Invalid request payload."}, status=400) diff --git a/zerver/webhooks/github/tests.py b/zerver/webhooks/github/tests.py index 90c563f0e2db2..ab4d183cf7c43 100644 --- a/zerver/webhooks/github/tests.py +++ b/zerver/webhooks/github/tests.py @@ -1,7 +1,9 @@ from unittest.mock import patch import orjson +from django.test import override_settings +from zerver.lib.bot_config import set_bot_config from zerver.lib.message import truncate_topic from zerver.lib.test_classes import WebhookTestCase from zerver.lib.webhooks.git import COMMITS_LIMIT @@ -22,6 +24,8 @@ class GitHubWebhookTest(WebhookTestCase): + WEBHOOK_TEST_SECRET = "testingthis" + def test_ping_event(self) -> None: expected_message = "GitHub webhook has been successfully configured by TomaszKolek." self.check_webhook("ping", TOPIC_REPO, expected_message) @@ -851,6 +855,49 @@ def test_issue_comment_silent_mention_with_multiple_matches(self) -> None: expected_message = "baxterthehacker [commented](https://github.com/baxterthehacker/public-repo/issues/2#issuecomment-99262140) on [issue #2](https://github.com/baxterthehacker/public-repo/issues/2):\n\n``` quote\nYou are totally right! I'll get this fixed right away.\n```" self.check_webhook("issue_comment", TOPIC_ISSUE, expected_message) + def test_github_webhook_bad_signature(self) -> None: + with override_settings(VERIFY_WEBHOOK_SIGNATURES=True): + url = self.build_webhook_url() + set_bot_config(self.test_user, "github:webhook_secret_token", self.WEBHOOK_TEST_SECRET) + + result = self.client_post( + url, + self.get_payload("ping"), + content_type="application/json", + HTTP_X_HUB_SIGNATURE_256="sha256=completely_invalid_hash_value", + ) + self.assert_json_error(result, "Webhook signature verification failed.") + + def test_github_webhook_signature_disabled_skips_validation(self) -> None: + """Verifies that when VERIFY_WEBHOOK_SIGNATURES is explicitly disabled, + requests pass through even if the signature value is completely bogus. + """ + with override_settings(VERIFY_WEBHOOK_SIGNATURES=False): + expected_message = "GitHub webhook has been successfully configured by TomaszKolek." + self.check_webhook( + "ping", + TOPIC_REPO, + expected_message, + HTTP_X_HUB_SIGNATURE_256="sha256=invalid_hash", + ) + + def test_github_webhook_valid_signature_success(self) -> None: + """Verifies that a mathematically correct HMAC signature passes + cleanly when verification enforcement is active.""" + expected_message = "GitHub webhook has been successfully configured by TomaszKolek." + + with override_settings(VERIFY_WEBHOOK_SIGNATURES=True): + self.check_webhook("ping", TOPIC_REPO, expected_message) + + def test_github_webhook_missing_secret(self) -> None: + """Verifies that if no webhook secret is configured for the bot, + the request is processed normally without requiring signature verification.""" + + with override_settings(VERIFY_WEBHOOK_SIGNATURES=True): + set_bot_config(self.test_user, "github:webhook_secret_token", "") + expected_message = "GitHub webhook has been successfully configured by TomaszKolek." + self.check_webhook("ping", TOPIC_REPO, expected_message) + class GitHubSponsorsHookTests(WebhookTestCase): URL_TEMPLATE = "/api/v1/external/githubsponsors?stream={stream}&api_key={api_key}" diff --git a/zerver/webhooks/github/view.py b/zerver/webhooks/github/view.py index c1a13b5ad0daf..446f2b5b21e3c 100644 --- a/zerver/webhooks/github/view.py +++ b/zerver/webhooks/github/view.py @@ -9,6 +9,7 @@ from zerver.decorator import log_unsupported_webhook_event, webhook_view from zerver.lib.exceptions import UnsupportedWebhookEventTypeError from zerver.lib.external_accounts import DEFAULT_EXTERNAL_ACCOUNTS +from zerver.lib.integrations import WEBHOOK_SIGNATURE_CONFIGS from zerver.lib.markdown.fenced_code import get_unused_fence from zerver.lib.mention import silent_mention_syntax_for_user from zerver.lib.partial import partial @@ -1196,7 +1197,12 @@ def get_topic_based_on_type(payload: WildValue, event: str) -> str: ALL_EVENT_TYPES = list(EVENT_FUNCTION_MAPPER.keys()) -@webhook_view("GitHub", notify_bot_owner_on_invalid_json=True, all_event_types=ALL_EVENT_TYPES) +@webhook_view( + "GitHub", + notify_bot_owner_on_invalid_json=True, + all_event_types=ALL_EVENT_TYPES, + signature_config=WEBHOOK_SIGNATURE_CONFIGS["github"], +) @typed_endpoint def api_github_webhook( request: HttpRequest, diff --git a/zproject/dev_urls.py b/zproject/dev_urls.py index c57717f9fe2a1..d637ca8e497c5 100644 --- a/zproject/dev_urls.py +++ b/zproject/dev_urls.py @@ -24,6 +24,7 @@ check_send_webhook_fixture_message, dev_panel, get_fixtures, + recalculate_signature, send_all_webhook_fixture_messages, ) from zerver.views.development.registration import ( @@ -98,6 +99,10 @@ "devtools/integrations/send_all_webhook_fixture_messages", send_all_webhook_fixture_messages ), path("devtools/integrations//fixtures", get_fixtures), + path( + "devtools/integrations/recalculate_signature", + recalculate_signature, + ), path("config-error/", config_error, name="config_error"), # Special endpoint to remove all the server-side caches. path("flush_caches", remove_caches),