{{#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..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 --}}
@@ -109,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/web/tests/compose_closed_ui.test.cjs b/web/tests/compose_closed_ui.test.cjs
index 974fe6769ff2d..938d85aca39ba 100644
--- a/web/tests/compose_closed_ui.test.cjs
+++ b/web/tests/compose_closed_ui.test.cjs
@@ -251,6 +251,34 @@ run_test("test_non_message_list_input", ({mock_template}) => {
assert.equal(label, "translated: Compose message");
});
+run_test("recipient_label_text_for_call_creation", () => {
+ recent_view_util.is_visible = () => true;
+ message_lists.current = undefined;
+ const stream = make_stream({
+ subscribed: true,
+ name: "stream test",
+ stream_id: 11,
+ });
+ stream_data.add_sub_for_tests(stream);
+
+ assert.equal(
+ compose_closed_ui.get_recipient_label({
+ stream_id: stream.stream_id,
+ topic: "topic test",
+ })?.label_text,
+ "#stream test > topic test",
+ );
+
+ // Empty topic falls back to the realm's empty-topic display name.
+ assert.equal(
+ compose_closed_ui.get_recipient_label({
+ stream_id: stream.stream_id,
+ topic: "",
+ })?.label_text,
+ `#stream test > translated: ${REALM_EMPTY_TOPIC_DISPLAY_NAME}`,
+ );
+});
+
run_test("update_reply_button_state", ({override, override_rewire}) => {
const $compose_reply_wrapper = $("#legacy-closed-compose-box .compose-reply-button-wrapper");
const $reply_button = $(".compose_reply_button");
diff --git a/zerver/actions/data_import.py b/zerver/actions/data_import.py
index 75a4ec93e41a2..9093db6b3e973 100644
--- a/zerver/actions/data_import.py
+++ b/zerver/actions/data_import.py
@@ -112,6 +112,7 @@ def import_slack_data(event: dict[str, Any]) -> None:
fh.name,
output_dir,
event["slack_access_token"],
+ convert_slack_threads=True,
)
attachment_size = sum(
os.path.getsize(os.path.join(dirpath, filename))
@@ -189,7 +190,7 @@ def record_created_realm(new_realm: Realm) -> None:
preregistration_realm.save()
logger.info("(%s) All done!", string_id)
except Exception as e:
- logger.exception(e)
+ logger.exception("Error during Slack import")
try:
preregistration_realm.created_realm = None
preregistration_realm.data_import_metadata["is_import_work_queued"] = False
diff --git a/zerver/decorator.py b/zerver/decorator.py
index dd385b412131c..039e89d0199f3 100644
--- a/zerver/decorator.py
+++ b/zerver/decorator.py
@@ -412,7 +412,7 @@ def _wrapped_func_arguments(
# the sender's fault, so tell the owner
notify_bot_owner_about_invalid_json(user_profile, webhook_client_name)
- raise err
+ raise
# Store the event types registered for this webhook as an attribute, which can be access
# later conveniently in zerver.lib.test_classes.WebhookTestCase.
@@ -828,7 +828,7 @@ def _wrapped_func_arguments(
return view_func(request, user_profile, *args, **kwargs)
except Exception as err:
if not webhook_client_name:
- raise err
+ raise
if not isinstance(err, JsonableError):
# An unexpected exception of some form -- log it
@@ -840,7 +840,7 @@ def _wrapped_func_arguments(
err.webhook_name = webhook_client_name
log_exception_to_webhook_logger(request, err)
- raise err
+ raise
return _wrapped_func_arguments
diff --git a/zerver/forms.py b/zerver/forms.py
index 30b8c43823600..d2ef200ab3b52 100644
--- a/zerver/forms.py
+++ b/zerver/forms.py
@@ -405,8 +405,8 @@ def validate_captcha_payload(request: HttpRequest, captcha_payload: str) -> None
raise forms.ValidationError(_("Validation failed, please try again."))
except forms.ValidationError:
raise
- except Exception as e:
- logging.exception(e)
+ except Exception:
+ logging.exception("Error while validating altcha solution")
raise forms.ValidationError(_("Validation failed, please try again."))
captcha_data = orjson.loads(base64.b64decode(captcha_payload))
diff --git a/zerver/lib/avatar.py b/zerver/lib/avatar.py
index d518cd837fcd7..3e5bcd83b12dc 100644
--- a/zerver/lib/avatar.py
+++ b/zerver/lib/avatar.py
@@ -206,9 +206,9 @@ def generate_avatar_jdenticon(input: str, medium: bool) -> bytes:
try:
stdout = subprocess.check_output(command)
return stdout
- except subprocess.CalledProcessError as error: # nocoverage
+ except subprocess.CalledProcessError: # nocoverage
logger.exception("Jdenticon generation failed for user_id: %s", input)
- raise error
+ raise
def generate_and_upload_jdenticon_avatar(
diff --git a/zerver/lib/cache.py b/zerver/lib/cache.py
index 6ab1b604d20a5..6d2ad85f207de 100644
--- a/zerver/lib/cache.py
+++ b/zerver/lib/cache.py
@@ -251,8 +251,8 @@ def cache_set(
val = (val,)
try:
cache_backend.set(final_key, val, timeout=timeout)
- except MemcachedException as e:
- logger.exception(e)
+ except MemcachedException:
+ logger.exception("Error while storing to cache")
remote_cache_stats_finish()
@@ -306,8 +306,8 @@ def cache_set_many(
remote_cache_stats_start()
try:
get_cache_backend(cache_name).set_many(items, timeout=timeout)
- except MemcachedException as e:
- logger.exception(e)
+ except MemcachedException:
+ logger.exception("Error while storing to cache")
remote_cache_stats_finish()
diff --git a/zerver/lib/email_mirror_server.py b/zerver/lib/email_mirror_server.py
index 512d3465b45dc..763293f019423 100644
--- a/zerver/lib/email_mirror_server.py
+++ b/zerver/lib/email_mirror_server.py
@@ -64,8 +64,8 @@ def send_to_postmaster(msg: email.message.Message) -> None:
e.smtp_error,
stack_info=True,
)
- except smtplib.SMTPException as e:
- logger.exception("Error sending bounce email to %s: %s", mail.to, str(e), stack_info=True)
+ except smtplib.SMTPException:
+ logger.exception("Error sending bounce email to %s", mail.to, stack_info=True)
class ZulipMessageHandler(MessageHandler):
diff --git a/zerver/lib/event_schema.py b/zerver/lib/event_schema.py
index 9619dc2789f3b..8a1a62b8f5ece 100644
--- a/zerver/lib/event_schema.py
+++ b/zerver/lib/event_schema.py
@@ -149,7 +149,7 @@ def f(label: str, event: dict[str, object]) -> None:
del event["id"]
try:
validate_with_model(event, base_model)
- except Exception as e: # nocoverage
+ except Exception: # nocoverage
print(f"""
FAILURE:
@@ -170,7 +170,7 @@ def f(label: str, event: dict[str, object]) -> None:
""")
PrettyPrinter(indent=4).pprint(event)
- raise e
+ raise
return f
diff --git a/zerver/lib/import_realm.py b/zerver/lib/import_realm.py
index cf6ee8a8f7605..42792f5bb57c2 100644
--- a/zerver/lib/import_realm.py
+++ b/zerver/lib/import_realm.py
@@ -1071,7 +1071,7 @@ def fix_subscriptions_is_user_active_column(
def process_avatars(sanitized_record: SanitizedRecord) -> None:
if not sanitized_record.safe_resolved_source_path.endswith(".original"):
- return None
+ return
user_profile = get_user_profile_by_id(sanitized_record.raw_record["user_profile_id"])
if settings.LOCAL_AVATARS_DIR is not None:
avatar_path = user_avatar_base_path_from_ids(
diff --git a/zerver/lib/push_notifications.py b/zerver/lib/push_notifications.py
index 51d39c93c823b..4bbc32d1f998d 100644
--- a/zerver/lib/push_notifications.py
+++ b/zerver/lib/push_notifications.py
@@ -185,6 +185,11 @@ def get_apns_context() -> APNsContext | None:
if not has_apns_credentials(): # nocoverage
return None
+ key: str | None = None
+ if settings.APNS_TOKEN_KEY_FILE: # nocoverage
+ with open(settings.APNS_TOKEN_KEY_FILE) as f:
+ key = f.read()
+
# NB if called concurrently, this will make excess connections.
# That's a little sloppy, but harmless unless a server gets
# hammered with a ton of these all at once after startup.
@@ -200,10 +205,6 @@ async def err_func(
pass # nocoverage
async def make_apns() -> aioapns.APNs:
- key: str | None = None
- if settings.APNS_TOKEN_KEY_FILE: # nocoverage
- with open(settings.APNS_TOKEN_KEY_FILE) as f:
- key = f.read()
return aioapns.APNs(
client_cert=settings.APNS_CERT_FILE,
key=key,
@@ -1566,7 +1567,7 @@ def send_push_notifications(
if is_test_notification:
# Propagate the exception to the caller to notify the client
# about the error while attempting to send test push notification.
- raise e
+ raise
return
diff --git a/zerver/lib/send_email.py b/zerver/lib/send_email.py
index 98aa223706c04..ee85b5a514d2d 100644
--- a/zerver/lib/send_email.py
+++ b/zerver/lib/send_email.py
@@ -352,7 +352,7 @@ def send_immediate_email(
connection.close()
raise EmailNotDeliveredError from e
except (smtplib.SMTPException, OSError) as e:
- logger.exception("Error sending %s email to %s: %s", template, mail.to, e, stack_info=True)
+ logger.exception("Error sending %s email to %s", template, mail.to, stack_info=True)
connection.close()
raise EmailNotDeliveredError from e
@@ -474,9 +474,9 @@ def send_future_email(
assert len(to_emails) == 1
email.address = parseaddr(to_emails[0])[1]
email.save()
- except Exception as e:
+ except Exception:
email.delete()
- raise e
+ raise
def send_email_to_admins(
diff --git a/zerver/lib/test_classes.py b/zerver/lib/test_classes.py
index 6f3c061f8644e..2f57f637caa28 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
@@ -54,6 +57,7 @@
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
@@ -2544,6 +2548,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(".")
@@ -2619,6 +2625,18 @@ def api_channel_message(
**extra,
)
+ 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 check_webhook(
self,
fixture_name: str,
@@ -2650,16 +2668,37 @@ 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)
+ 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:
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,
)
@@ -2715,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) # nocoverage
+
payload = self.get_payload(fixture_name)
extra["content_type"] = content_type
diff --git a/zerver/lib/thumbnail.py b/zerver/lib/thumbnail.py
index 67e651364204d..ff2867c7ac03a 100644
--- a/zerver/lib/thumbnail.py
+++ b/zerver/lib/thumbnail.py
@@ -194,8 +194,8 @@ def libvips_check_image(
try:
yield source_image
- except pyvips.Error as e: # nocoverage
- logging.exception(e)
+ except pyvips.Error: # nocoverage
+ logging.exception("Error while processing image")
raise BadImageError(_("Image is corrupted or truncated"))
diff --git a/zerver/lib/webhooks/common.py b/zerver/lib/webhooks/common.py
index ac2d8ba7e7a16..649575dc85535 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,
@@ -320,8 +321,45 @@ 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:
+ assert request.user.is_authenticated
+ user_profile = request.user
+ assert isinstance(user_profile, UserProfile)
+
+ try:
+ config = get_bot_config(user_profile)
+ webhook_secret = config.get("webhook_secret", "")
+ except ConfigError:
+ webhook_secret = ""
+
+ if not webhook_secret:
+ return
+
+ 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(
+ payload=payload,
+ signature=signature,
+ secret=webhook_secret,
+ 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"
+ payload: str,
+ signature: str,
+ secret: str,
+ algorithm: str = "sha256",
) -> None:
if not settings.VERIFY_WEBHOOK_SIGNATURES: # nocoverage
return
@@ -331,14 +369,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/lib/zulip_update_announcements.py b/zerver/lib/zulip_update_announcements.py
index 97a90a78d73c2..4bb3c12282321 100644
--- a/zerver/lib/zulip_update_announcements.py
+++ b/zerver/lib/zulip_update_announcements.py
@@ -809,8 +809,8 @@ def send_zulip_update_announcements(skip_delay: bool, progress: bool = False) ->
for i, realm in enumerate(realms, start=1):
try:
send_zulip_update_announcements_to_realm(realm, skip_delay)
- except Exception as e: # nocoverage
- logging.exception(e)
+ except Exception: # nocoverage
+ logging.exception("Error while sending update announcements")
finally:
if progress and i % 50 == 0: # nocoverage
print(f"Processed {i}/{len(realms)} realms...")
diff --git a/zerver/management/commands/send_password_reset_email.py b/zerver/management/commands/send_password_reset_email.py
index 1c2bcd19073f3..1ccd497fd0d15 100644
--- a/zerver/management/commands/send_password_reset_email.py
+++ b/zerver/management/commands/send_password_reset_email.py
@@ -45,7 +45,7 @@ def handle(self, *args: Any, **options: str) -> None:
raise CommandError(
"You have to pass -u/--users or -a/--all-users or --entire-server."
)
- raise error
+ raise
if options["only_never_logged_in"]:
users = users.filter(tos_version=-1)
diff --git a/zerver/middleware.py b/zerver/middleware.py
index 217108eed4d40..c23cac63cb7ce 100644
--- a/zerver/middleware.py
+++ b/zerver/middleware.py
@@ -299,8 +299,8 @@ def process_request(self, request: HttpRequest) -> None:
try:
request_notes.client_name, request_notes.client_version = parse_client(request)
- except JsonableError as e:
- logging.exception(e)
+ except JsonableError:
+ logging.exception("Error while parsing client from request")
request_notes.client_name = "Unparsable"
request_notes.client_version = None
diff --git a/zerver/tests/test_bots.py b/zerver/tests/test_bots.py
index 63769fa63f3a6..a2fda496db7e8 100644
--- a/zerver/tests/test_bots.py
+++ b/zerver/tests/test_bots.py
@@ -2307,6 +2307,85 @@ 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_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)
+ self.assertEqual(config_data["webhook_secret"], "")
+
def test_get_bot_api_key(self) -> None:
self.login("hamlet")
self.create_bot()
diff --git a/zerver/tests/test_email_mirror.py b/zerver/tests/test_email_mirror.py
index cd2555b46fde9..efc0f3f35aade 100644
--- a/zerver/tests/test_email_mirror.py
+++ b/zerver/tests/test_email_mirror.py
@@ -2107,11 +2107,14 @@ def test_send_postmaster_failure(self) -> None:
self.assertLogs("zerver.lib.email_mirror", "ERROR") as error_log,
):
send_to_postmaster(email)
- self.assert_length(error_log.output, 1)
+ self.assert_length(error_log.records, 1)
+ self.assertEqual(error_log.records[0].levelname, "ERROR")
self.assertEqual(
- error_log.output[0].splitlines()[0],
- "ERROR:zerver.lib.email_mirror:Error sending bounce email to ['desdemona+admin@zulip.com']: moose",
+ error_log.records[0].getMessage(),
+ "Error sending bounce email to ['desdemona+admin@zulip.com']",
)
+ assert error_log.records[0].exc_info is not None
+ self.assertEqual(str(error_log.records[0].exc_info[1]), "moose")
with (
mock.patch.object(
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_send_email.py b/zerver/tests/test_send_email.py
index 6f1292d37b2a6..659ffbf90a644 100644
--- a/zerver/tests/test_send_email.py
+++ b/zerver/tests/test_send_email.py
@@ -411,18 +411,22 @@ def test_send_email_exceptions(self) -> None:
self.assertEqual(mail.extra_headers["From"], f"{from_name} <{FromAddress.NOREPLY}>")
# We test the cases that should raise an EmailNotDeliveredError
- errors = {
- f"Unknown error sending password_reset email to {mail.to}": [0],
- f"Error sending password_reset email to {mail.to}": [SMTPException()],
- f"Error sending password_reset email to {mail.to}: {{'{address}': (550, b'User unknown')}}": [
- SMTPRecipientsRefused(recipients={address: (550, b"User unknown")})
- ],
- f"Error sending password_reset email to {mail.to} with error code 242: From field too long": [
- SMTPDataError(242, "From field too long.")
- ],
- }
+ errors = [
+ (f"Unknown error sending password_reset email to {mail.to}", None, [0]),
+ (f"Error sending password_reset email to {mail.to}", "", [SMTPException()]),
+ (
+ f"Error sending password_reset email to {mail.to}",
+ f"{{{address!r}: (550, b'User unknown')}}",
+ [SMTPRecipientsRefused(recipients={address: (550, b"User unknown")})],
+ ),
+ (
+ f"Error sending password_reset email to {mail.to} with error code 242: From field too long.",
+ "(242, 'From field too long.')",
+ [SMTPDataError(242, "From field too long.")],
+ ),
+ ]
- for message, side_effect in errors.items():
+ for message, exc_message, side_effect in errors:
with mock.patch.object(EmailBackend, "send_messages", side_effect=side_effect):
with (
self.assertLogs(logger=logger) as info_log,
@@ -436,11 +440,16 @@ def test_send_email_exceptions(self) -> None:
language="en",
)
self.assert_length(info_log.records, 2)
+ self.assertEqual(info_log.records[0].levelname, "INFO")
+ self.assertEqual(
+ info_log.records[0].getMessage(), f"Sending password_reset email to {mail.to}"
+ )
+ self.assertEqual(info_log.records[1].levelname, "ERROR")
+ self.assertEqual(info_log.records[1].getMessage(), message)
self.assertEqual(
- info_log.output[0],
- f"INFO:{logger.name}:Sending password_reset email to {mail.to}",
+ info_log.records[1].exc_info and str(info_log.records[1].exc_info[1]),
+ exc_message,
)
- self.assertTrue(info_log.output[1].startswith(f"ERROR:zulip.send_email:{message}"))
def test_send_email_config_error_logging(self) -> None:
hamlet = self.example_user("hamlet")
diff --git a/zerver/tests/test_slack_importer.py b/zerver/tests/test_slack_importer.py
index 56a9317dcc4ec..07de67f3bb510 100644
--- a/zerver/tests/test_slack_importer.py
+++ b/zerver/tests/test_slack_importer.py
@@ -2899,7 +2899,7 @@ def fake_do_import_realm(*args: object, **kwargs: object) -> Realm:
mock_save_attachment_contents.assert_called_once()
mock_do_convert_zipfile.assert_called_once_with(
- mock.ANY, mock.ANY, event["slack_access_token"]
+ mock.ANY, mock.ANY, event["slack_access_token"], convert_slack_threads=True
)
mock_do_import_realm.assert_called_once_with(
mock.ANY, prereg_realm.string_id, on_realm_created=mock.ANY
@@ -3067,7 +3067,7 @@ def fake_do_import_realm(*args: object, **kwargs: object) -> Realm:
mock_save_attachment_contents.assert_called_once()
mock_do_convert_zipfile.assert_called_once_with(
- mock.ANY, mock.ANY, event["slack_access_token"]
+ mock.ANY, mock.ANY, event["slack_access_token"], convert_slack_threads=True
)
mock_do_import_realm.assert_called_once_with(
mock.ANY, prereg_realm.string_id, on_realm_created=mock.ANY
diff --git a/zerver/tests/test_webhooks_common.py b/zerver/tests/test_webhooks_common.py
index 02db0bdd1ce09..48ea22a889aaf 100644
--- a/zerver/tests/test_webhooks_common.py
+++ b/zerver/tests/test_webhooks_common.py
@@ -14,6 +14,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 set_bot_config
from zerver.lib.exceptions import InvalidJSONError, JsonableError
from zerver.lib.request import RequestNotes
from zerver.lib.send_email import FromAddress
@@ -29,6 +30,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
@@ -152,18 +154,14 @@ 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_secret = "test_secret"
payload = '{"key": "value"}'
signature = hmac.new(
force_bytes(webhook_secret), force_bytes(payload), hashlib.sha256
).hexdigest()
- request.GET.update({"webhook_secret": webhook_secret})
- validate_webhook_signature(request, payload, signature)
+ # Valid signature
+ validate_webhook_signature(payload, signature, webhook_secret)
# Invalid signature
invalid_signature = "invalid_signature"
@@ -171,15 +169,47 @@ def test_validate_webhook_signature(self) -> None:
JsonableError,
"Webhook signature verification failed.",
):
- validate_webhook_signature(request, payload, invalid_signature)
+ validate_webhook_signature(payload, invalid_signature, webhook_secret)
- # No webhook_secret parameter
- request.GET.clear()
+ # Missing or empty secret
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(payload, signature, secret="")
+
+ @override_settings(VERIFY_WEBHOOK_SIGNATURES=True)
+ def test_validate_webhook_delivery(self) -> None:
+ webhook_bot = get_user("webhook-bot@zulip.com", get_realm("zulip"))
+ webhook_secret = "test_secret"
+ payload = '{"key": "value"}'
+ signature = hmac.new(
+ force_bytes(webhook_secret), force_bytes(payload), hashlib.sha256
+ ).hexdigest()
+
+ set_bot_config(webhook_bot, "webhook_secret", webhook_secret)
+ request = HostRequestMock(meta_data={"HTTP_X_HUB_SIGNATURE_256": f"sha256={signature}"})
+ request.user = webhook_bot
+ request.GET = QueryDict("", mutable=True)
+ 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 configured for this bot skips validation
+ set_bot_config(webhook_bot, "webhook_secret", "")
+ request.META["HTTP_X_HUB_SIGNATURE_256"] = f"sha256={signature}"
+ del request.headers
+ 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"))
diff --git a/zerver/views/antispam.py b/zerver/views/antispam.py
index aafc41123dae2..89c4117f70397 100644
--- a/zerver/views/antispam.py
+++ b/zerver/views/antispam.py
@@ -48,6 +48,6 @@ def get_challenge(
(challenge.challenge, expires.timestamp()),
]
return json_success(request, data=challenge.__dict__)
- except Exception as e: # nocoverage
- logging.exception(e)
+ except Exception: # nocoverage
+ logging.exception("Error while generating challenge")
raise JsonableError(_("Failed to generate challenge"))
diff --git a/zerver/views/development/integrations.py b/zerver/views/development/integrations.py
index a3ec1ce71221c..6c524999d13f7 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:
+ return JsonResponse({"error": "Invalid request payload."}, status=400)
diff --git a/zerver/views/documentation.py b/zerver/views/documentation.py
index fe64df3046c28..52ba609167e86 100644
--- a/zerver/views/documentation.py
+++ b/zerver/views/documentation.py
@@ -427,7 +427,7 @@ def get_categories_for_integration(integration: Integration) -> list[tuple[str,
for display_name in integration.categories:
slug = display_to_slug.get(display_name)
assert slug is not None
- result.append((slug, display_name))
+ result.append((slug, str(display_name)))
return result
diff --git a/zerver/views/sentry.py b/zerver/views/sentry.py
index 2310ee28ce508..f711a44823044 100644
--- a/zerver/views/sentry.py
+++ b/zerver/views/sentry.py
@@ -87,9 +87,9 @@ def sentry_tunnel(
sentry_request(url, updated_body)
except CircuitBreakerError:
logger.warning("Dropped a client exception due to circuit-breaking")
- except RequestException as e:
+ except RequestException:
# This logger has been configured, above, to not report to Sentry
- logger.exception(e)
+ logger.exception("Error while reporting to Sentry")
return HttpResponse(status=200)
diff --git a/zerver/webhooks/github/tests.py b/zerver/webhooks/github/tests.py
index 90c563f0e2db2..5effc6bda0f07 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,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)
@@ -851,6 +856,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, "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 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, "webhook_secret", "")
+ 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..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/zilencer/auth.py b/zilencer/auth.py
index a0ccad76ead17..ab0cf5b69923c 100644
--- a/zilencer/auth.py
+++ b/zilencer/auth.py
@@ -95,9 +95,9 @@ def rate_limit_remote_server(
try:
RateLimitedRemoteZulipServer(remote_server, domain=domain).rate_limit_request(request)
- except RateLimitedError as e:
+ except RateLimitedError:
logger.warning("Remote server %s exceeded rate limits on domain %s", remote_server, domain)
- raise e
+ raise
def validate_remote_server(
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),