From 66adf3339fd71b03ec00b66ebc513e80592c046c Mon Sep 17 00:00:00 2001 From: Srinandha Murugesan Date: Thu, 23 Jul 2026 00:38:10 +0000 Subject: [PATCH 01/14] webhooks: Add validate_webhook_delivery validation helper. Add validate_webhook_delivery to parse request signatures and handle JsonableError exceptions during payload verification. Tested with: ./tools/test-backend zerver/tests/test_webhooks_common.py Co-authored-by: Srinandha Murugesan Co-authored-by: Isaiah Marte Co-authored-by: Akshaj Katkuri Co-authored-by: Jason Zheng --- zerver/lib/webhooks/common.py | 18 ++++++++++++++ zerver/tests/test_webhooks_common.py | 36 ++++++++++++++++++++++++++++ 2 files changed, 54 insertions(+) diff --git a/zerver/lib/webhooks/common.py b/zerver/lib/webhooks/common.py index f2bfa6f7bc2bf..cbd6fa5bcda66 100644 --- a/zerver/lib/webhooks/common.py +++ b/zerver/lib/webhooks/common.py @@ -320,6 +320,24 @@ def parse_multipart_string(body: str) -> dict[str, str]: return data +def validate_webhook_delivery( + request: HttpRequest, signature_header_name: str, algorithm: str = "sha256" +) -> None: + signature_header = request.headers.get(signature_header_name, "") + signature = signature_header.split("=")[-1] if "=" in signature_header else signature_header + + payload = request.body.decode("utf-8") + + try: + validate_webhook_signature( + request=request, payload=payload, signature=signature, algorithm=algorithm + ) + except JsonableError: + raise + except Exception as err: # nocoverage + raise JsonableError(str(err)) + + def validate_webhook_signature( request: HttpRequest, payload: str, signature: str, algorithm: str = "sha256" ) -> None: diff --git a/zerver/tests/test_webhooks_common.py b/zerver/tests/test_webhooks_common.py index 02db0bdd1ce09..d347726cf6417 100644 --- a/zerver/tests/test_webhooks_common.py +++ b/zerver/tests/test_webhooks_common.py @@ -29,6 +29,7 @@ get_service_api_data, guess_zulip_user_from_external_account, standardize_headers, + validate_webhook_delivery, validate_webhook_signature, ) from zerver.models import Client, CustomProfileField, Message, UserProfile @@ -181,6 +182,41 @@ def test_validate_webhook_signature(self) -> None: ): validate_webhook_signature(request, payload, signature) + @override_settings(VERIFY_WEBHOOK_SIGNATURES=True) + def test_validate_webhook_delivery(self) -> None: + webhook_secret = "test_secret" + payload = '{"key": "value"}' + signature = hmac.new( + force_bytes(webhook_secret), force_bytes(payload), hashlib.sha256 + ).hexdigest() + + request = HostRequestMock(meta_data={"HTTP_X_HUB_SIGNATURE_256": f"sha256={signature}"}) + request.GET = QueryDict("", mutable=True) + request.GET.update({"webhook_secret": webhook_secret}) + request._body = force_bytes(payload) + + # Valid signature + validate_webhook_delivery(request, "X_HUB_Signature_256") + + # Invalid signature + request.META["HTTP_X_HUB_SIGNATURE_256"] = "sha256=invalid_signature" + del request.headers + with self.assertRaisesRegex( + JsonableError, + "Webhook signature verification failed.", + ): + validate_webhook_delivery(request, "X_HUB_Signature_256") + + # No webhook_secret parameter + request.META["HTTP_X_HUB_SIGNATURE_256"] = f"sha256={signature}" + del request.headers + request.GET.clear() + with self.assertRaisesRegex( + JsonableError, + "The webhook secret is missing. Please set the webhook_secret while generating the URL.", + ): + validate_webhook_delivery(request, "X_HUB_Signature_256") + def test_check_send_webhook_message_returns_id(self) -> None: webhook_bot = get_user("webhook-bot@zulip.com", get_realm("zulip")) stream = self.make_stream("test_stream") From c0374d134f8903049ad9aef9e1e3608d7b990c26 Mon Sep 17 00:00:00 2001 From: Srinandha Murugesan Date: Thu, 23 Jul 2026 00:38:24 +0000 Subject: [PATCH 02/14] tests: Support WEBHOOK_TEST_SECRET in WebhookTestCase class. Update WebhookTestCase to generate HMAC signatures when testing signed webhook payloads. Update GitHub webhook tests accordingly. Tested with: ./tools/test-backend zerver/webhooks/github/ Co-authored-by: Srinandha Murugesan Co-authored-by: Isaiah Marte Co-authored-by: Akshaj Katkuri Co-authored-by: Jason Zheng --- zerver/lib/test_classes.py | 45 +++++++++++++++++++++++++++- zerver/webhooks/github/tests.py | 52 +++++++++++++++++++++++++++++++++ 2 files changed, 96 insertions(+), 1 deletion(-) diff --git a/zerver/lib/test_classes.py b/zerver/lib/test_classes.py index 6f3c061f8644e..b1d5ec9e5434e 100644 --- a/zerver/lib/test_classes.py +++ b/zerver/lib/test_classes.py @@ -1,5 +1,7 @@ import asyncio import base64 +import hashlib +import hmac import os import re import shutil @@ -35,6 +37,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 @@ -2544,6 +2547,8 @@ class WebhookTestCase(ZulipTestCase): DEFAULT_URL_TEMPLATE: str = ( "/api/v1/external/{webhook_dir_name}?stream={stream}&api_key={api_key}" ) + WEBHOOK_SIGNATURE_HEADER: str | None = None + WEBHOOK_TEST_SECRET: str | None = None def get_webhook_dir_name(self) -> str: module_parts = self.__module__.split(".") @@ -2650,16 +2655,42 @@ def check_webhook( """ self.subscribe(self.test_user, self.channel_name) + url = getattr(self, "url", None) + webhook_secret = getattr(self, "WEBHOOK_TEST_SECRET", None) + + if url is None: + if webhook_secret is not None: # nocoverage + url = self.build_webhook_url(webhook_secret=webhook_secret) # nocoverage + else: + url = self.build_webhook_url() # nocoverage + else: + if webhook_secret is not None and "webhook_secret=" not in url: + separator = "&" if "?" in url else "?" + url = f"{url}{separator}webhook_secret={quote(webhook_secret)}" + payload = self.get_payload(fixture_name) if content_type is not None: extra["content_type"] = content_type + + signature_header_name = getattr(self, "WEBHOOK_SIGNATURE_HEADER", None) + if signature_header_name is not None: + try: + raw_payload = self.get_body(fixture_name) + except FileNotFoundError: # nocoverage + raw_payload = "" + + signature_value = self.get_webhook_signature(force_bytes(raw_payload)) + if signature_value is not None: + django_header = "HTTP_" + signature_header_name.upper().replace("-", "_") + extra[django_header] = signature_value + 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, ) @@ -2699,6 +2730,18 @@ def assert_channel_message( self.assertEqual(message.topic_name(), topic_name) self.assertEqual(message.content, content) + def get_webhook_signature(self, raw_payload: bytes) -> str | None: + """ + Generate the signature header value for a given payload. + Override this method in child classes if the integration uses different signature format. + """ + secret = getattr(self, "WEBHOOK_TEST_SECRET", None) + if secret is None: + return None # nocoverage + + # Default implementation matches the current GitHub standard format + return "sha256=" + hmac.new(force_bytes(secret), raw_payload, hashlib.sha256).hexdigest() + def send_and_test_private_message( self, fixture_name: str, diff --git a/zerver/webhooks/github/tests.py b/zerver/webhooks/github/tests.py index 5f5909e4419b0..5fc1d7e279721 100644 --- a/zerver/webhooks/github/tests.py +++ b/zerver/webhooks/github/tests.py @@ -1,6 +1,7 @@ from unittest.mock import patch import orjson +from django.test import override_settings from zerver.lib.message import truncate_topic from zerver.lib.test_classes import WebhookTestCase @@ -22,6 +23,9 @@ class GitHubWebhookTest(WebhookTestCase): + WEBHOOK_SIGNATURE_HEADER = "X_HUB_Signature_256" + 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) @@ -853,6 +857,54 @@ 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(webhook_secret=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 the backend drops the request if the webhook url + is invoked without providing the required webhook_secret parameter.""" + with override_settings(VERIFY_WEBHOOK_SIGNATURES=True): + url = self.build_webhook_url() + result = self.client_post( + url, + self.get_payload("ping"), + content_type="application/json", + HTTP_X_HUB_SIGNATURE_256="sha256=somehash", + ) + self.assert_json_error( + result, + "The webhook secret is missing. Please set the webhook_secret while generating the URL.", + ) + class GitHubSponsorsHookTests(WebhookTestCase): URL_TEMPLATE = "/api/v1/external/githubsponsors?stream={stream}&api_key={api_key}" From 4c02b64132321e28a481ca08e7a5a35c2892419d Mon Sep 17 00:00:00 2001 From: Srinandha Murugesan Date: Thu, 23 Jul 2026 00:38:34 +0000 Subject: [PATCH 03/14] integrations_dev_panel: Add webhook secret UI options and sync. Add webhook secret field options to integration definitions and dev panel UI. Synchronize header recalculation on user input. Tested via dev panel frontend UI and ./tools/lint. Co-authored-by: Srinandha Murugesan Co-authored-by: Isaiah Marte Co-authored-by: Akshaj Katkuri Co-authored-by: Jason Zheng --- .../development/integrations_dev_panel.html | 4 + web/src/portico/integrations_dev_panel.ts | 89 ++++++++++++++++++- web/styles/portico/integrations_dev_panel.css | 3 +- zerver/lib/integrations.py | 5 ++ 4 files changed, 97 insertions(+), 4 deletions(-) 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 8f1b7d5965a2f..881b10e57afaa 100644 --- a/web/src/portico/integrations_dev_panel.ts +++ b/web/src/portico/integrations_dev_panel.ts @@ -25,6 +25,7 @@ type HTMLSelectOneElement = HTMLSelectElement & {type: "select-one"}; type ClearHandlers = { stream_name: string; topic_name: string; + webhook_secret: string; URL: string; results_notice: string; bot_name: () => void; @@ -47,6 +48,8 @@ const integrations_api_response_schema = z.object({ result: z.string(), }); +let last_computed_header_key: string | null = null; // Tracks the current signature header for auto-clearing when switching integrations + type ServerResponse = z.infer; const loaded_fixtures = new Map(); @@ -56,6 +59,7 @@ const url_base = "/api/v1/external/"; const clear_handlers: ClearHandlers = { stream_name: "#stream_name", topic_name: "#topic_name", + webhook_secret: "#webhook_secret", URL: "#URL", results_notice: "#results_notice", bot_name() { @@ -180,6 +184,8 @@ function load_fixture_body(fixture_name: string): void { null, 4, ); + const webhook_secret = $("input#webhook_secret").val()!; + sync_signature_headers(integration_name, webhook_secret); return; } @@ -210,8 +216,8 @@ function load_fixture_options(integration_name: string): void { function update_url(): void { /* Construct the URL that the webhook should be targeting, using - the bot's API key and the integration name. The stream and topic - are both optional, and for the sake of completeness, it should be + the bot's API key, the integration name, and webhook secret. The stream, topic, + and webhook secret are all optional, and for the sake of completeness, it should be noted that the topic is irrelevant without specifying the stream. */ const url_field = $("input#URL")[0]; @@ -231,11 +237,86 @@ function update_url(): void { params.set("topic", topic_name); } } + const webhook_secret = $("input#webhook_secret").val()!; + if (webhook_secret !== "") { + params.set("webhook_secret", webhook_secret); + } 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 && last_computed_header_key in headers_object) { + 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 = raw_payload; + + 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 +521,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/lib/integrations.py b/zerver/lib/integrations.py index fae0cd7bf9f09..98390931566ec 100644 --- a/zerver/lib/integrations.py +++ b/zerver/lib/integrations.py @@ -673,6 +673,11 @@ def is_enabled_in_catalog(self) -> bool: label="Include emoji indicators in the notifications", input_type="checkbox_enabled", ), + WebhookUrlOption( + name="webhook_secret", + label="GitHub Webhook Secret (Optional)", + input_type="text", + ), ], ), IncomingWebhookIntegration( From 188ee499c9aa189ca32ed22dad89f83c34a529f9 Mon Sep 17 00:00:00 2001 From: Srinandha Murugesan Date: Thu, 23 Jul 2026 00:38:41 +0000 Subject: [PATCH 04/14] integrations_dev_panel: Add recalculate_signature endpoint. Add backend recalculate_signature view and signature registry for dev panel UI recalculation. Add unit tests for signature hashing. Tested with: ./tools/test-backend zerver/tests/test_integrations_dev_panel.py Co-authored-by: Srinandha Murugesan Co-authored-by: Isaiah Marte Co-authored-by: Akshaj Katkuri Co-authored-by: Jason Zheng --- zerver/tests/test_integrations_dev_panel.py | 154 ++++++++++++++++++++ zerver/views/development/integrations.py | 67 ++++++++- 2 files changed, 220 insertions(+), 1 deletion(-) 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/views/development/integrations.py b/zerver/views/development/integrations.py index a3ec1ce71221c..b33baec3fb207 100644 --- a/zerver/views/development/integrations.py +++ b/zerver/views/development/integrations.py @@ -1,12 +1,17 @@ +import hashlib +import hmac import os +from collections.abc import Callable from contextlib import suppress 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 @@ -156,3 +161,63 @@ def send_all_webhook_fixture_messages( } ) return json_success(request, data={"responses": responses}) + + +def format_github_signature(secret_bytes: bytes, payload_bytes: bytes) -> tuple[str, str]: + """Formats signature header following X-Hub-Signature-256 standard.""" + signed_payload = hmac.new(secret_bytes, payload_bytes, hashlib.sha256).hexdigest() + return "X_HUB_SIGNATURE_256", f"sha256={signed_payload}" + + +SIGNATURE_REGISTRY: dict[str, Callable[[bytes, bytes], tuple[str, str]]] = { + "github": format_github_signature +} + + +@csrf_exempt +def recalculate_signature(request: HttpRequest) -> JsonResponse: + """ + Unified endpoint invoked by the frontend UI dev panel to dynamically 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() + + # Check if the integration has signature management registered + if integration_name not in SIGNATURE_REGISTRY: + return JsonResponse( + {"supported": False, "msg": "No signature rules configured for this platform."} + ) + + if not secret: + return JsonResponse({"supported": True, "clear_signature": True}) + + # Normalize and minify JSON formats for crypto verification stability + try: + payload_bytes = orjson.dumps(orjson.loads(payload_string)) + except Exception: + payload_bytes = force_bytes(payload_string) + + webhook_secret_bytes = force_bytes(secret) + + # Execute the registered structural format strategy + formatter = SIGNATURE_REGISTRY[integration_name] + header_key, header_value = formatter(webhook_secret_bytes, payload_bytes) + + return JsonResponse( + { + "supported": True, + "clear_signature": False, + "header_key": header_key, + "signature": header_value, + } + ) + + except Exception as e: + return JsonResponse({"error": str(e)}, status=400) From af96c9f50350fb4ff22508c8a03eb4c06572169a Mon Sep 17 00:00:00 2001 From: Srinandha Murugesan Date: Thu, 23 Jul 2026 00:38:48 +0000 Subject: [PATCH 05/14] github: Enforce webhook signature check and register dev route. Invoke validate_webhook_delivery in GitHub webhook handler view and register dev recalculate_signature endpoint in zproject dev URLs. Tested with: ./tools/test-backend zerver/webhooks/github/ Co-authored-by: Srinandha Murugesan Co-authored-by: Isaiah Marte Co-authored-by: Akshaj Katkuri Co-authored-by: Jason Zheng --- zerver/webhooks/github/view.py | 3 +++ zproject/dev_urls.py | 5 +++++ 2 files changed, 8 insertions(+) diff --git a/zerver/webhooks/github/view.py b/zerver/webhooks/github/view.py index c1a13b5ad0daf..b276df37fd713 100644 --- a/zerver/webhooks/github/view.py +++ b/zerver/webhooks/github/view.py @@ -22,6 +22,7 @@ get_event_header, get_setup_webhook_message, guess_zulip_user_from_external_account, + validate_webhook_delivery, ) from zerver.lib.webhooks.git import ( CONTENT_MESSAGE_TEMPLATE, @@ -1215,6 +1216,8 @@ def api_github_webhook( directly to the X-GitHub-Event header's event, but we sometimes refine it based on the payload. """ + validate_webhook_delivery(request, "X_HUB_Signature_256", "sha256") + header_event = get_event_header(request, "X-GitHub-Event", "GitHub") # Ignore events from private repositories if the URL option is set 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), From c38e26ffe29662b71d505849363d0df19dd9fc78 Mon Sep 17 00:00:00 2001 From: Srinandha Murugesan Date: Thu, 23 Jul 2026 02:24:57 +0000 Subject: [PATCH 06/14] integrations: Avoid leaking exception details in signature response. Sanitize the error response in recalculate_signature to prevent exposing internal stack traces or server implementation details via raw exception strings. Co-authored-by: Srinandha Murugesan Co-authored-by: Isaiah Marte Co-authored-by: Akshaj Katkuri Co-authored-by: Jason Zheng --- zerver/views/development/integrations.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/zerver/views/development/integrations.py b/zerver/views/development/integrations.py index b33baec3fb207..6c524999d13f7 100644 --- a/zerver/views/development/integrations.py +++ b/zerver/views/development/integrations.py @@ -219,5 +219,5 @@ def recalculate_signature(request: HttpRequest) -> JsonResponse: } ) - except Exception as e: - return JsonResponse({"error": str(e)}, status=400) + except Exception: + return JsonResponse({"error": "Invalid request payload."}, status=400) From 699c79f82387e06a7aacf09c77947c6f8677f70d Mon Sep 17 00:00:00 2001 From: Srinandha Murugesan Date: Thu, 23 Jul 2026 08:13:48 -0700 Subject: [PATCH 07/14] Fixed overall parsing and UI to not use url param --- web/src/portico/integrations_dev_panel.ts | 3 --- web/templates/settings/add_new_bot_form.hbs | 12 +++++++++ zerver/lib/integrations.py | 5 ---- zerver/lib/test_classes.py | 21 ++++++++-------- zerver/lib/webhooks/common.py | 27 ++++++++++++++------- zerver/webhooks/github/tests.py | 13 +++++++--- 6 files changed, 50 insertions(+), 31 deletions(-) diff --git a/web/src/portico/integrations_dev_panel.ts b/web/src/portico/integrations_dev_panel.ts index 881b10e57afaa..70015a93b8fd9 100644 --- a/web/src/portico/integrations_dev_panel.ts +++ b/web/src/portico/integrations_dev_panel.ts @@ -238,9 +238,6 @@ function update_url(): void { } } const webhook_secret = $("input#webhook_secret").val()!; - if (webhook_secret !== "") { - params.set("webhook_secret", webhook_secret); - } const url = `${url_base}${integration_name}?${params.toString()}`; url_field!.value = url; diff --git a/web/templates/settings/add_new_bot_form.hbs b/web/templates/settings/add_new_bot_form.hbs index 14c89e4fd2b15..72fb01f2d7154 100644 --- a/web/templates/settings/add_new_bot_form.hbs +++ b/web/templates/settings/add_new_bot_form.hbs @@ -28,6 +28,18 @@ maxlength=100 placeholder="{{t 'Cookie Bot' }}" value="" />
+ {{> ../dropdown_widget_with_label + widget_name="integration-name" + label=(t "Integration")}} + +
+ + +
+
bool: label="Include emoji indicators in the notifications", input_type="checkbox_enabled", ), - WebhookUrlOption( - name="webhook_secret", - label="GitHub Webhook Secret (Optional)", - input_type="text", - ), ], ), IncomingWebhookIntegration( diff --git a/zerver/lib/test_classes.py b/zerver/lib/test_classes.py index b1d5ec9e5434e..f00df0a4ba320 100644 --- a/zerver/lib/test_classes.py +++ b/zerver/lib/test_classes.py @@ -58,6 +58,7 @@ from zerver.actions.users import do_change_user_role from zerver.decorator import do_two_factor_login from zerver.lib.cache import bounce_key_prefix_for_testing +from zerver.lib.bot_config import set_bot_config from zerver.lib.email_notifications import MissedMessageData, handle_missedmessage_emails from zerver.lib.initial_password import initial_password from zerver.lib.mdiff import diff_strings @@ -2656,17 +2657,12 @@ def check_webhook( self.subscribe(self.test_user, self.channel_name) url = getattr(self, "url", None) - webhook_secret = getattr(self, "WEBHOOK_TEST_SECRET", None) - if url is None: - if webhook_secret is not None: # nocoverage - url = self.build_webhook_url(webhook_secret=webhook_secret) # nocoverage - else: - url = self.build_webhook_url() # nocoverage - else: - if webhook_secret is not None and "webhook_secret=" not in url: - separator = "&" if "?" in url else "?" - url = f"{url}{separator}webhook_secret={quote(webhook_secret)}" + url = self.build_webhook_url() + + webhook_secret = getattr(self, "WEBHOOK_TEST_SECRET", None) + if webhook_secret is not None: + set_bot_config(self.test_user, "webhook_secret", webhook_secret) payload = self.get_payload(fixture_name) if content_type is not None: @@ -2758,6 +2754,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) + 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 cbd6fa5bcda66..dfc418a9471fe 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, @@ -323,6 +324,15 @@ def parse_multipart_string(body: str) -> dict[str, str]: def validate_webhook_delivery( request: HttpRequest, signature_header_name: str, algorithm: str = "sha256" ) -> None: + try: + config = get_bot_config(request.user) + webhook_secret = config.get("webhook_secret", "") + except ConfigError: + webhook_secret = "" + + if not webhook_secret: + raise JsonableError(_("Webhook secret is not configured for this bot.")) + signature_header = request.headers.get(signature_header_name, "") signature = signature_header.split("=")[-1] if "=" in signature_header else signature_header @@ -339,7 +349,10 @@ def validate_webhook_delivery( def validate_webhook_signature( - request: HttpRequest, payload: str, signature: str, algorithm: str = "sha256" + payload: str, + signature: str, + secret: str, + algorithm: str = "sha256", ) -> None: if not settings.VERIFY_WEBHOOK_SIGNATURES: # nocoverage return @@ -349,14 +362,10 @@ def validate_webhook_signature( _("The algorithm '{algorithm}' is not supported.").format(algorithm=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) + if not secret: + raise JsonableError(_("Webhook secret is not configured for this bot.")) + + webhook_secret_bytes = force_bytes(secret) payload_bytes = force_bytes(payload) signed_payload = hmac.new( diff --git a/zerver/webhooks/github/tests.py b/zerver/webhooks/github/tests.py index 5fc1d7e279721..a083ef6bec40f 100644 --- a/zerver/webhooks/github/tests.py +++ b/zerver/webhooks/github/tests.py @@ -3,6 +3,7 @@ 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 @@ -859,7 +860,9 @@ def test_issue_comment_silent_mention_with_multiple_matches(self) -> None: def test_github_webhook_bad_signature(self) -> None: with override_settings(VERIFY_WEBHOOK_SIGNATURES=True): - url = self.build_webhook_url(webhook_secret=self.WEBHOOK_TEST_SECRET) + url = self.build_webhook_url() + set_bot_config(self.test_user, "webhook_secret", self.WEBHOOK_TEST_SECRET) + result = self.client_post( url, self.get_payload("ping"), @@ -890,10 +893,12 @@ def test_github_webhook_valid_signature_success(self) -> None: self.check_webhook("ping", TOPIC_REPO, expected_message) def test_github_webhook_missing_secret(self) -> None: - """Verifies that the backend drops the request if the webhook url - is invoked without providing the required webhook_secret parameter.""" + """Verifies that the backend drops the request if the webhook secret + is not configured in BotConfigData.""" with override_settings(VERIFY_WEBHOOK_SIGNATURES=True): url = self.build_webhook_url() + set_bot_config(self.test_user, "webhook_secret", "") + result = self.client_post( url, self.get_payload("ping"), @@ -902,7 +907,7 @@ def test_github_webhook_missing_secret(self) -> None: ) self.assert_json_error( result, - "The webhook secret is missing. Please set the webhook_secret while generating the URL.", + "Webhook secret is not configured for this bot.", ) From dc832df7c0d95093eb6a05750f21a51869c647a2 Mon Sep 17 00:00:00 2001 From: JDoe-code Date: Fri, 24 Jul 2026 14:17:46 -0400 Subject: [PATCH 08/14] webhooks: Store incoming webhook secrets in BotConfigData. Co-authored-by: Srinandha Murugesan Co-authored-by: Isaiah Marte Co-authored-by: Akshaj Katkuri Co-authored-by: Jason Zheng --- web/src/bot_type_values.ts | 1 + web/src/settings_bots.ts | 25 +++++++++++++++++++-- web/src/user_profile.ts | 7 ++++++ web/templates/settings/add_new_bot_form.hbs | 7 ++++++ web/templates/settings/edit_bot_form.hbs | 6 +++++ zerver/actions/users.py | 4 ++-- 6 files changed, 46 insertions(+), 4 deletions(-) diff --git a/web/src/bot_type_values.ts b/web/src/bot_type_values.ts index a93e47a8ad333..5b4e302ed6c72 100644 --- a/web/src/bot_type_values.ts +++ b/web/src/bot_type_values.ts @@ -4,5 +4,6 @@ export const INCOMING_WEBHOOK_BOT_TYPE_INT = 2; export const OUTGOING_WEBHOOK_BOT_TYPE_INT = 3; // String forms used as HTML form values. +export const INCOMING_WEBHOOK_BOT_TYPE = "2"; export const OUTGOING_WEBHOOK_BOT_TYPE = "3"; export const EMBEDDED_BOT_TYPE = "4"; diff --git a/web/src/settings_bots.ts b/web/src/settings_bots.ts index 1bff3e00a8585..f8dabea1bc4cc 100644 --- a/web/src/settings_bots.ts +++ b/web/src/settings_bots.ts @@ -13,6 +13,7 @@ import * as bot_helper from "./bot_helper.ts"; import { EMBEDDED_BOT_TYPE, GENERIC_BOT_TYPE, + INCOMING_WEBHOOK_BOT_TYPE, INCOMING_WEBHOOK_BOT_TYPE_INT, OUTGOING_WEBHOOK_BOT_TYPE, OUTGOING_WEBHOOK_BOT_TYPE_INT, @@ -298,6 +299,20 @@ export function add_a_new_bot(): void { formData.append("interface_type", interface_type); break; } + case INCOMING_WEBHOOK_BOT_TYPE: { + const config_data: Record = {}; + $("#webhook_secret_inputbox input").each(function () { + const key = $(this).attr("name")!; + const value = $(this).val()?.trim()!; + if (value) { + config_data[key] = value; + } + }); + if (Object.keys(config_data).length > 0) { + formData.append("config_data", JSON.stringify(config_data)); + } + break; + } case EMBEDDED_BOT_TYPE: { formData.append("service_name", service_name); const config_data: Record = {}; @@ -336,7 +351,7 @@ export function add_a_new_bot(): void { } function set_up_form_fields(): void { - $("#create_bot_type").val(INCOMING_WEBHOOK_BOT_TYPE_INT); + $("#create_bot_type").val(INCOMING_WEBHOOK_BOT_TYPE).trigger("change"); $("#payload_url_inputbox").hide(); $("#create_payload_url").val(""); $("#service_name_list").hide(); @@ -362,7 +377,13 @@ export function add_a_new_bot(): void { $("#payload_url_inputbox").hide(); $("#create_payload_url").removeClass("required"); + + $("#webhook_secret_inputbox").hide(); switch (bot_type) { + case INCOMING_WEBHOOK_BOT_TYPE: { + $("#webhook_secret_inputbox").show(); + break; + } case OUTGOING_WEBHOOK_BOT_TYPE: { $("#payload_url_inputbox").show(); $("#create_payload_url").addClass("required"); @@ -377,7 +398,7 @@ export function add_a_new_bot(): void { } } }); - + $("#create_bot_type").val(INCOMING_WEBHOOK_BOT_TYPE).trigger("change"); $("#select_service_name").on("change", () => { $("#config_inputbox").children().hide(); const selected_bot = $( diff --git a/web/src/user_profile.ts b/web/src/user_profile.ts index 5a93b4bb22384..7739f5e0f6b7a 100644 --- a/web/src/user_profile.ts +++ b/web/src/user_profile.ts @@ -23,6 +23,7 @@ import * as bot_data from "./bot_data.ts"; import * as bot_helper from "./bot_helper.ts"; import { EMBEDDED_BOT_TYPE, + INCOMING_WEBHOOK_BOT_TYPE, INCOMING_WEBHOOK_BOT_TYPE_INT, OUTGOING_WEBHOOK_BOT_TYPE, } from "./bot_type_values.ts"; @@ -930,6 +931,11 @@ export function show_edit_bot_info_modal(user_id: number, $container: JQuery): v config_data[$(this).attr("name")!] = $(this).val()!; }); formData.append("config_data", JSON.stringify(config_data)); + } else if (bot_type === INCOMING_WEBHOOK_BOT_TYPE) { + const webhook_secret = $("#edit_webhook_secret").val()?.trim(); + if (webhook_secret) { + formData.append("config_data", JSON.stringify({webhook_secret})); + } } const files = util.the( @@ -952,6 +958,7 @@ export function show_edit_bot_info_modal(user_id: number, $container: JQuery): v contentType: false, success() { $("#bot-edit-form-error").hide(); + $("#edit_webhook_secret").val(""); avatar_widget.clear(); hide_button_spinner($submit_button); original_values = get_current_values($("#bot-edit-form")); diff --git a/web/templates/settings/add_new_bot_form.hbs b/web/templates/settings/add_new_bot_form.hbs index 14c89e4fd2b15..645b1161caf5a 100644 --- a/web/templates/settings/add_new_bot_form.hbs +++ b/web/templates/settings/add_new_bot_form.hbs @@ -53,6 +53,13 @@
+
+
+ + +
+
+
{{#each realm_embedded_bots}} {{#each (object_entries config) as |entry|}} diff --git a/web/templates/settings/edit_bot_form.hbs b/web/templates/settings/edit_bot_form.hbs index 4d71e501168b8..8909b263751c8 100644 --- a/web/templates/settings/edit_bot_form.hbs +++ b/web/templates/settings/edit_bot_form.hbs @@ -41,6 +41,12 @@
+ {{#if is_incoming_webhook_bot}} +
+ + +
+ {{/if}}
{{!-- Shows the current avatar --}} diff --git a/zerver/actions/users.py b/zerver/actions/users.py index 3be5a98f257a4..0dd1a9e0f0c2a 100644 --- a/zerver/actions/users.py +++ b/zerver/actions/users.py @@ -827,7 +827,7 @@ def do_update_outgoing_webhook_service( def do_update_bot_config_data(bot_profile: UserProfile, config_data: dict[str, str]) -> None: for key, value in config_data.items(): set_bot_config(bot_profile, key, value) - updated_config_data = get_bot_config(bot_profile) + service_dicts = get_service_dicts_for_bot(bot_profile.id) send_event_on_commit( bot_profile.realm, dict( @@ -835,7 +835,7 @@ def do_update_bot_config_data(bot_profile: UserProfile, config_data: dict[str, s op="update", bot=dict( user_id=bot_profile.id, - services=[dict(config_data=updated_config_data)], + services=service_dicts, ), ), bot_owner_user_ids(bot_profile), From 960a8d82cf2bf4f7625ca79a63e95d33d90f4c5c Mon Sep 17 00:00:00 2001 From: Akshaj-Katkuri Date: Fri, 24 Jul 2026 19:44:37 -0400 Subject: [PATCH 09/14] tests: Added test cases for creating incoming webhook bot with and without secret --- zerver/tests/test_bots.py | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/zerver/tests/test_bots.py b/zerver/tests/test_bots.py index de5feb4f992a1..83a996c67afb6 100644 --- a/zerver/tests/test_bots.py +++ b/zerver/tests/test_bots.py @@ -2305,6 +2305,30 @@ def test_create_incoming_webhook_bot_with_incorrect_service_name(self) -> None: with self.assertRaises(UserProfile.DoesNotExist): UserProfile.objects.get(full_name="My Stripe Bot") + def test_create_incoming_webhook_bot_with_secret(self) -> None: + self.login("hamlet") + self.assert_num_bots_equal(0) + self.create_bot( + bot_type=UserProfile.INCOMING_WEBHOOK_BOT, + config_data=orjson.dumps({"webhook_secret": "test-secret-key-123"}).decode(), + ) + self.assert_num_bots_equal(1) + + new_bot = UserProfile.objects.get(full_name="The Bot of Hamlet") + config_data = get_bot_config(new_bot) + self.assertEqual(config_data["webhook_secret"], "test-secret-key-123") + + def test_create_incoming_webhook_bot_without_secret(self) -> None: + self.login("hamlet") + self.assert_num_bots_equal(0) + self.create_bot(bot_type=UserProfile.INCOMING_WEBHOOK_BOT) + self.assert_num_bots_equal(1) + + new_bot = UserProfile.objects.get(full_name="The Bot of Hamlet") + + with self.assertRaisesMessage(ConfigError, "No config data available."): + get_bot_config(new_bot) + def test_get_bot_api_key(self) -> None: self.login("hamlet") self.create_bot() From cc7c4a0f7253bacda7a5032c58edea3c48a51d41 Mon Sep 17 00:00:00 2001 From: Srinandha Murugesan Date: Mon, 27 Jul 2026 23:20:18 +0000 Subject: [PATCH 10/14] Cleared up frontend and test case issues --- web/src/user_profile.ts | 1 - web/templates/settings/add_new_bot_form.hbs | 16 ++-------------- web/templates/settings/edit_bot_form.hbs | 6 ------ 3 files changed, 2 insertions(+), 21 deletions(-) diff --git a/web/src/user_profile.ts b/web/src/user_profile.ts index 7739f5e0f6b7a..9beb391b9d17a 100644 --- a/web/src/user_profile.ts +++ b/web/src/user_profile.ts @@ -958,7 +958,6 @@ export function show_edit_bot_info_modal(user_id: number, $container: JQuery): v contentType: false, success() { $("#bot-edit-form-error").hide(); - $("#edit_webhook_secret").val(""); avatar_widget.clear(); hide_button_spinner($submit_button); original_values = get_current_values($("#bot-edit-form")); diff --git a/web/templates/settings/add_new_bot_form.hbs b/web/templates/settings/add_new_bot_form.hbs index 1afc9f73713b6..3f44deb8d1536 100644 --- a/web/templates/settings/add_new_bot_form.hbs +++ b/web/templates/settings/add_new_bot_form.hbs @@ -28,18 +28,6 @@ maxlength=100 placeholder="{{t 'Cookie Bot' }}" value="" />
- {{> ../dropdown_widget_with_label - widget_name="integration-name" - label=(t "Integration")}} - -
- - -
-
- - + +
diff --git a/web/templates/settings/edit_bot_form.hbs b/web/templates/settings/edit_bot_form.hbs index 326209c32742d..4d71e501168b8 100644 --- a/web/templates/settings/edit_bot_form.hbs +++ b/web/templates/settings/edit_bot_form.hbs @@ -41,12 +41,6 @@
- {{#if is_incoming_webhook_bot}} -
- - -
- {{/if}}
{{!-- Shows the current avatar --}} From ec3287c61d6402070f414283434e4e5d4107e8b5 Mon Sep 17 00:00:00 2001 From: Srinandha Murugesan Date: Mon, 27 Jul 2026 16:44:54 -0700 Subject: [PATCH 11/14] Cleared up frontend and test case issues --- web/templates/settings/add_new_bot_form.hbs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/web/templates/settings/add_new_bot_form.hbs b/web/templates/settings/add_new_bot_form.hbs index 3f44deb8d1536..b5dadbff1346a 100644 --- a/web/templates/settings/add_new_bot_form.hbs +++ b/web/templates/settings/add_new_bot_form.hbs @@ -56,7 +56,7 @@
- +
From a647b549ed3a657682bff6ea8ad5cb6dca906c1b Mon Sep 17 00:00:00 2001 From: JDoe-code Date: Tue, 28 Jul 2026 15:07:24 -0400 Subject: [PATCH 12/14] webhooks: Add back the feature to edit the secret. --- web/src/user_profile.ts | 1 + web/templates/settings/edit_bot_form.hbs | 10 +++------- 2 files changed, 4 insertions(+), 7 deletions(-) diff --git a/web/src/user_profile.ts b/web/src/user_profile.ts index 9beb391b9d17a..34bf892819b6b 100644 --- a/web/src/user_profile.ts +++ b/web/src/user_profile.ts @@ -958,6 +958,7 @@ export function show_edit_bot_info_modal(user_id: number, $container: JQuery): v contentType: false, success() { $("#bot-edit-form-error").hide(); + $("#edit-webhook-secret").val(""); avatar_widget.clear(); hide_button_spinner($submit_button); original_values = get_current_values($("#bot-edit-form")); diff --git a/web/templates/settings/edit_bot_form.hbs b/web/templates/settings/edit_bot_form.hbs index 4d71e501168b8..743aec5627ae4 100644 --- a/web/templates/settings/edit_bot_form.hbs +++ b/web/templates/settings/edit_bot_form.hbs @@ -67,13 +67,9 @@
{{#if is_incoming_webhook_bot}} -
- {{> ../components/action_button - label=(t "Generate URL for an integration") - variant="subtle" - intent="neutral" - custom_classes="generate_url_for_integration" - }} +
+ +
{{/if}} {{#if (and is_active is_bot_owner_current_user)}} From a163475c90280fc6e74c393d24c42dc4811962d3 Mon Sep 17 00:00:00 2001 From: Akshaj-Katkuri Date: Tue, 28 Jul 2026 15:47:36 -0400 Subject: [PATCH 13/14] bots: Add test cases for updating, adding, or clearing a webhook secret of an existing bot --- zerver/tests/test_bots.py | 56 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 56 insertions(+) diff --git a/zerver/tests/test_bots.py b/zerver/tests/test_bots.py index 83a996c67afb6..64377fefe09c6 100644 --- a/zerver/tests/test_bots.py +++ b/zerver/tests/test_bots.py @@ -2329,6 +2329,62 @@ def test_create_incoming_webhook_bot_without_secret(self) -> None: with self.assertRaisesMessage(ConfigError, "No config data available."): get_bot_config(new_bot) + def test_patch_incoming_webhook_bot_add_secret(self) -> None: + self.login("hamlet") + self.create_bot(bot_type=UserProfile.INCOMING_WEBHOOK_BOT) + + bot_email = "hambot-bot@zulip.testserver" + bot_id = self.get_bot_user(bot_email).id + + bot_update = { + "config_data": orjson.dumps({"webhook_secret": "test-secret-key-123"}).decode() + } + result = self.client_patch(f"/json/bots/{bot_id}", bot_update) + self.assert_json_success(result) + + bot = self.get_bot_user(bot_email) + config_data = get_bot_config(bot) + self.assertEqual(config_data["webhook_secret"], "test-secret-key-123") + + def test_patch_incoming_webhook_bot_update_secret(self) -> None: + self.login("hamlet") + self.create_bot( + bot_type=UserProfile.INCOMING_WEBHOOK_BOT, + config_data=orjson.dumps({"webhook_secret": "old-test-secret-123"}).decode(), + ) + + bot_email = "hambot-bot@zulip.testserver" + bot_id = self.get_bot_user(bot_email).id + + bot_update = { + "config_data": orjson.dumps({"webhook_secret": "new-test-secret-123"}).decode() + } + result = self.client_patch(f"/json/bots/{bot_id}", bot_update) + self.assert_json_success(result) + + bot = self.get_bot_user(bot_email) + config_data = get_bot_config(bot) + self.assertEqual(config_data["webhook_secret"], "new-test-secret-123") + + def test_patch_incoming_webhook_bot_clear_secret(self) -> None: + self.login("hamlet") + self.create_bot( + bot_type=UserProfile.INCOMING_WEBHOOK_BOT, + config_data=orjson.dumps({"webhook_secret": "test-secret-key-123"}).decode(), + ) + + bot_email = "hambot-bot@zulip.testserver" + bot_id = self.get_bot_user(bot_email).id + + bot_update = {"config_data": orjson.dumps({"webhook_secret": ""}).decode()} + result = self.client_patch(f"/json/bots/{bot_id}", bot_update) + self.assert_json_success(result) + + bot = self.get_bot_user(bot_email) + config_data = get_bot_config(bot) + print(config_data) + self.assertEqual(config_data["webhook_secret"], "") + def test_get_bot_api_key(self) -> None: self.login("hamlet") self.create_bot() From cf93405b046af74841b504aa84cf3e093aa7eaa8 Mon Sep 17 00:00:00 2001 From: JDoe-code Date: Tue, 28 Jul 2026 16:41:39 -0400 Subject: [PATCH 14/14] webhooks: Add the ability to delete a secret. --- web/src/user_profile.ts | 30 ++++++++++++++++++++++- web/templates/settings/edit_bot_form.hbs | 31 +++++++++++++++++++++--- zerver/tests/test_bots.py | 1 - 3 files changed, 56 insertions(+), 6 deletions(-) diff --git a/web/src/user_profile.ts b/web/src/user_profile.ts index 34bf892819b6b..3a3021c0ec3df 100644 --- a/web/src/user_profile.ts +++ b/web/src/user_profile.ts @@ -875,7 +875,32 @@ export function show_edit_bot_info_modal(user_id: number, $container: JQuery): v const bot_type = bot.bot_type.toString(); const services = bot_data.get_services(bot.user_id); const service = services?.[0]; + let is_delete_requested = false; edit_bot_post_render(); + + $("#bot-edit-form").on("click", "#clear_webhook_secret_button", (e) => { + e.preventDefault(); + is_delete_requested = true; + + // Clear the input value and set visual feedback + const $secret_input = $("#edit_webhook_secret"); + $secret_input.val(""); + $secret_input.attr( + "placeholder", + $t({defaultMessage: "Secret will be deleted when saved."}), + ); + + // Notify form handler that a change was made so the save button is enabled + $("#user-profile-modal .dialog_submit_button").prop("disabled", false); + }); + + // If the user types anything manually into the input, reset the clear flag + $("#bot-edit-form").on("input", "#edit_webhook_secret", function () { + if ($(this).val() !== "") { + $(this).data("clear-secret", false); + } + }); + original_values = get_current_values($("#bot-edit-form")); $("#bot-edit-form").on("input", "input, select, button", (e) => { e.preventDefault(); @@ -933,7 +958,10 @@ export function show_edit_bot_info_modal(user_id: number, $container: JQuery): v formData.append("config_data", JSON.stringify(config_data)); } else if (bot_type === INCOMING_WEBHOOK_BOT_TYPE) { const webhook_secret = $("#edit_webhook_secret").val()?.trim(); - if (webhook_secret) { + if (is_delete_requested) { + formData.append("config_data", JSON.stringify({webhook_secret: ""})); + is_delete_requested = false; + } else if (webhook_secret) { formData.append("config_data", JSON.stringify({webhook_secret})); } } diff --git a/web/templates/settings/edit_bot_form.hbs b/web/templates/settings/edit_bot_form.hbs index 743aec5627ae4..9b460fc21eb4c 100644 --- a/web/templates/settings/edit_bot_form.hbs +++ b/web/templates/settings/edit_bot_form.hbs @@ -39,8 +39,15 @@ widget_name="edit_bot_owner" label=(t 'Owner')}} -
+
+ + {{#if is_incoming_webhook_bot}} +
+ +
+ {{/if}} +
{{!-- Shows the current avatar --}} @@ -67,9 +74,13 @@
{{#if is_incoming_webhook_bot}} -
- - +
+ {{> ../components/action_button + label=(t "Generate URL for an integration") + variant="subtle" + intent="neutral" + custom_classes="generate_url_for_integration" + }}
{{/if}} {{#if (and is_active is_bot_owner_current_user)}} @@ -105,6 +116,18 @@ }}
{{/if}} + + {{#if is_incoming_webhook_bot}} +
+ {{> ../components/action_button + label=(t "Delete secret") + variant="subtle" + intent="danger" + id="clear_webhook_secret_button" + }} +
+ {{/if}} +
{{#if is_active}} {{> ../components/action_button diff --git a/zerver/tests/test_bots.py b/zerver/tests/test_bots.py index 64377fefe09c6..dc92dd2666072 100644 --- a/zerver/tests/test_bots.py +++ b/zerver/tests/test_bots.py @@ -2382,7 +2382,6 @@ def test_patch_incoming_webhook_bot_clear_secret(self) -> None: bot = self.get_bot_user(bot_email) config_data = get_bot_config(bot) - print(config_data) self.assertEqual(config_data["webhook_secret"], "") def test_get_bot_api_key(self) -> None: