From c595f846a84c15fa64b5819b461f20fa3304cf9b Mon Sep 17 00:00:00 2001 From: Marnie Date: Sun, 12 Jul 2026 03:05:49 +0800 Subject: [PATCH 1/6] fix(webhooks): accept 2xx status codes in webhook test endpoint MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The webhook test endpoint previously treated any non-200 response as a failure. This is incorrect — HTTP 201 (Created), 202 (Accepted), and 204 (No Content) are all valid webhook responses. Change the condition from \status_code != 200\ to \status_code >= 400\ so that any 2xx response is considered successful. Also update the error message to include the actual status code instead of telling users to return a 200 OK. --- api/tests/unit/webhooks/test_unit_webhooks.py | 4 ++-- api/webhooks/views.py | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/api/tests/unit/webhooks/test_unit_webhooks.py b/api/tests/unit/webhooks/test_unit_webhooks.py index 9ef69bcd2561..9561bee1f561 100644 --- a/api/tests/unit/webhooks/test_unit_webhooks.py +++ b/api/tests/unit/webhooks/test_unit_webhooks.py @@ -434,10 +434,10 @@ def test_send_test_webhook__various_error_status_codes__returns_correct_response mock_post.assert_called_once() response_json = response.json() assert response_json["status"] == external_api_response_status - assert response_json["detail"] == "Webhook returned invalid status" + assert response_json["detail"] == "Webhook returned error status" assert ( response_json["body"] - == "Please check the webhook endpoint to validate it returns a 200 OK." + == f"Webhook returned HTTP {external_api_response_status}." ) diff --git a/api/webhooks/views.py b/api/webhooks/views.py index cfb756537eb2..743860c2fce9 100644 --- a/api/webhooks/views.py +++ b/api/webhooks/views.py @@ -47,11 +47,11 @@ def test(self, request: Request) -> Response: else WebhookType.ENVIRONMENT ) response = send_test_request_to_webhook(webhook_url, secret, webhook_type) - if response.status_code != 200: + if response.status_code >= 400: return Response( { - "detail": "Webhook returned invalid status", - "body": "Please check the webhook endpoint to validate it returns a 200 OK.", + "detail": "Webhook returned error status", + "body": f"Webhook returned HTTP {response.status_code}.", "status": response.status_code, }, status=status.HTTP_400_BAD_REQUEST, From 999585befc4f20a95283c3f3cfa17f1eda105d67 Mon Sep 17 00:00:00 2001 From: Marnie0415 Date: Mon, 13 Jul 2026 19:53:13 +0800 Subject: [PATCH 2/6] fix(webhooks): use response.ok and add parametrised 2xx tests Address Zaimwa9's feedback on PR #7992: - Use not response.ok instead of status_code >= 400 to correctly treat 3xx redirects as failures - Add parametrised test for 201/202/204 status codes as success cases --- api/tests/unit/webhooks/test_unit_webhooks.py | 43 +++++++++++++++++++ api/webhooks/views.py | 2 +- 2 files changed, 44 insertions(+), 1 deletion(-) diff --git a/api/tests/unit/webhooks/test_unit_webhooks.py b/api/tests/unit/webhooks/test_unit_webhooks.py index 9561bee1f561..b9ede94112e5 100644 --- a/api/tests/unit/webhooks/test_unit_webhooks.py +++ b/api/tests/unit/webhooks/test_unit_webhooks.py @@ -392,6 +392,49 @@ def test_send_test_webhook__200_response_from_webhook__returns_correct_response( assert response_json["detail"] == "Webhook test successful" +@pytest.mark.parametrize( + "external_api_response_status", + [ + 201, + 202, + 204, + ], +) +def test_send_test_webhook__various_2xx_status_codes__returns_success( + mocker: MockerFixture, + admin_client: APIClient, + external_api_response_status: int, + organisation: Organisation, +) -> None: + # Given + webhook_url = "http://test.webhook.com" + mock_post = mocker.patch("requests.post") + mock_response = MagicMock() + mock_response.status_code = external_api_response_status + mock_response.text = "success" + mock_post.return_value = mock_response + + url = reverse("api-v1:webhooks:webhooks-test") + + data = { + "webhook_url": webhook_url, + "secret": "some-secret", + "scope": {"type": "organisation", "id": organisation.id}, + } + + # When + response = admin_client.post( + url, data=json.dumps(data), content_type="application/json" + ) + + # Then + assert response.status_code == 200 + mock_post.assert_called_once() + response_json = response.json() + assert response_json["status"] == external_api_response_status + assert response_json["detail"] == "Webhook test successful" + + @pytest.mark.parametrize( "external_api_response_status, external_api_error_text, expected_final_status", [ diff --git a/api/webhooks/views.py b/api/webhooks/views.py index 743860c2fce9..bf467c467e90 100644 --- a/api/webhooks/views.py +++ b/api/webhooks/views.py @@ -47,7 +47,7 @@ def test(self, request: Request) -> Response: else WebhookType.ENVIRONMENT ) response = send_test_request_to_webhook(webhook_url, secret, webhook_type) - if response.status_code >= 400: + if not response.ok: return Response( { "detail": "Webhook returned error status", From 74ba9f7eb060e4170e72985796a60c6605e5e20d Mon Sep 17 00:00:00 2001 From: Marnie0415 Date: Mon, 13 Jul 2026 20:06:48 +0800 Subject: [PATCH 3/6] fix(tests): correct mock path for requests.post The tests were mocking requests.post but the actual code calls requests.post in webhooks.webhooks module. Change mock path to webhooks.webhooks.requests.post to match the actual import path. --- api/tests/unit/webhooks/test_unit_webhooks.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/api/tests/unit/webhooks/test_unit_webhooks.py b/api/tests/unit/webhooks/test_unit_webhooks.py index b9ede94112e5..78b26d707117 100644 --- a/api/tests/unit/webhooks/test_unit_webhooks.py +++ b/api/tests/unit/webhooks/test_unit_webhooks.py @@ -365,7 +365,7 @@ def test_send_test_webhook__200_response_from_webhook__returns_correct_response( ) -> None: # Given webhook_url = "http://test.webhook.com" - mock_post = mocker.patch("requests.post") + mock_post = mocker.patch("webhooks.webhooks.requests.post") mock_response = MagicMock() mock_response.status_code = 200 mock_response.text = "success" @@ -408,7 +408,7 @@ def test_send_test_webhook__various_2xx_status_codes__returns_success( ) -> None: # Given webhook_url = "http://test.webhook.com" - mock_post = mocker.patch("requests.post") + mock_post = mocker.patch("webhooks.webhooks.requests.post") mock_response = MagicMock() mock_response.status_code = external_api_response_status mock_response.text = "success" @@ -453,7 +453,7 @@ def test_send_test_webhook__various_error_status_codes__returns_correct_response ) -> None: # Given webhook_url = "http://test.webhook.com" - mock_post = mocker.patch("requests.post") + mock_post = mocker.patch("webhooks.webhooks.requests.post") mock_response = MagicMock() mock_response.status_code = external_api_response_status mock_response.text = external_api_error_text @@ -518,7 +518,7 @@ def test_send_test_webhook__various_secrets__sends_correct_payload( ) -> None: # Given webhook_url = "http://test.webhook.com" - mock_post = mocker.patch("requests.post") + mock_post = mocker.patch("webhooks.webhooks.requests.post") mock_response = MagicMock() mock_response.status_code = 200 mock_post.return_value = mock_response @@ -557,7 +557,7 @@ def test_send_test_webhook__request_exception__returns_error_response( ) -> None: # Given webhook_url = "http://test.webhook.com" - mock_post = mocker.patch("requests.post") + mock_post = mocker.patch("webhooks.webhooks.requests.post") mock_post.side_effect = requests.exceptions.RequestException( "Some internal exception details that should not be exposed!" ) From d3b668e6f51f85be132329c16b64d48af7af0c0d Mon Sep 17 00:00:00 2001 From: Marnie0415 Date: Mon, 13 Jul 2026 20:11:49 +0800 Subject: [PATCH 4/6] fix(tests): use public URL to avoid SSRF protection test.webhook.com resolves to a private IP in Docker, triggering the NoSSRFURLField validation. Use https://example.com instead. --- api/tests/unit/webhooks/test_unit_webhooks.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/api/tests/unit/webhooks/test_unit_webhooks.py b/api/tests/unit/webhooks/test_unit_webhooks.py index 78b26d707117..e8455f24ced5 100644 --- a/api/tests/unit/webhooks/test_unit_webhooks.py +++ b/api/tests/unit/webhooks/test_unit_webhooks.py @@ -364,7 +364,7 @@ def test_send_test_webhook__200_response_from_webhook__returns_correct_response( organisation: Organisation, ) -> None: # Given - webhook_url = "http://test.webhook.com" + webhook_url = "https://example.com" mock_post = mocker.patch("webhooks.webhooks.requests.post") mock_response = MagicMock() mock_response.status_code = 200 @@ -407,7 +407,7 @@ def test_send_test_webhook__various_2xx_status_codes__returns_success( organisation: Organisation, ) -> None: # Given - webhook_url = "http://test.webhook.com" + webhook_url = "https://example.com" mock_post = mocker.patch("webhooks.webhooks.requests.post") mock_response = MagicMock() mock_response.status_code = external_api_response_status @@ -452,7 +452,7 @@ def test_send_test_webhook__various_error_status_codes__returns_correct_response organisation: Organisation, ) -> None: # Given - webhook_url = "http://test.webhook.com" + webhook_url = "https://example.com" mock_post = mocker.patch("webhooks.webhooks.requests.post") mock_response = MagicMock() mock_response.status_code = external_api_response_status @@ -517,7 +517,7 @@ def test_send_test_webhook__various_secrets__sends_correct_payload( secret: str, ) -> None: # Given - webhook_url = "http://test.webhook.com" + webhook_url = "https://example.com" mock_post = mocker.patch("webhooks.webhooks.requests.post") mock_response = MagicMock() mock_response.status_code = 200 @@ -556,7 +556,7 @@ def test_send_test_webhook__request_exception__returns_error_response( organisation: Organisation, ) -> None: # Given - webhook_url = "http://test.webhook.com" + webhook_url = "https://example.com" mock_post = mocker.patch("webhooks.webhooks.requests.post") mock_post.side_effect = requests.exceptions.RequestException( "Some internal exception details that should not be exposed!" From a248be115436bc1a1b85a930e8819feb84bd6a42 Mon Sep 17 00:00:00 2001 From: Marnie0415 Date: Mon, 13 Jul 2026 20:20:03 +0800 Subject: [PATCH 5/6] fix(tests): mock DNS resolution to bypass SSRF validation The NoSSRFURLField resolves hostnames to check for private IPs. In Docker, test URLs resolve to private IPs. Mock socket.getaddrinfo to return a public IP so the SSRF check passes. --- api/tests/unit/webhooks/test_unit_webhooks.py | 21 +++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/api/tests/unit/webhooks/test_unit_webhooks.py b/api/tests/unit/webhooks/test_unit_webhooks.py index e8455f24ced5..d96a8129ed06 100644 --- a/api/tests/unit/webhooks/test_unit_webhooks.py +++ b/api/tests/unit/webhooks/test_unit_webhooks.py @@ -1,6 +1,7 @@ import hashlib import hmac import json +import socket from typing import Callable, Type from unittest import mock from unittest.mock import MagicMock @@ -370,6 +371,10 @@ def test_send_test_webhook__200_response_from_webhook__returns_correct_response( mock_response.status_code = 200 mock_response.text = "success" mock_post.return_value = mock_response + mocker.patch( + "webhooks.fields.socket.getaddrinfo", + return_value=[(socket.AF_INET, None, None, None, ("93.184.216.34", 0))], + ) url = reverse("api-v1:webhooks:webhooks-test") @@ -413,6 +418,10 @@ def test_send_test_webhook__various_2xx_status_codes__returns_success( mock_response.status_code = external_api_response_status mock_response.text = "success" mock_post.return_value = mock_response + mocker.patch( + "webhooks.fields.socket.getaddrinfo", + return_value=[(socket.AF_INET, None, None, None, ("93.184.216.34", 0))], + ) url = reverse("api-v1:webhooks:webhooks-test") @@ -458,6 +467,10 @@ def test_send_test_webhook__various_error_status_codes__returns_correct_response mock_response.status_code = external_api_response_status mock_response.text = external_api_error_text mock_post.return_value = mock_response + mocker.patch( + "webhooks.fields.socket.getaddrinfo", + return_value=[(socket.AF_INET, None, None, None, ("93.184.216.34", 0))], + ) url = reverse("api-v1:webhooks:webhooks-test") @@ -522,6 +535,10 @@ def test_send_test_webhook__various_secrets__sends_correct_payload( mock_response = MagicMock() mock_response.status_code = 200 mock_post.return_value = mock_response + mocker.patch( + "webhooks.fields.socket.getaddrinfo", + return_value=[(socket.AF_INET, None, None, None, ("93.184.216.34", 0))], + ) url = reverse("api-v1:webhooks:webhooks-test") @@ -561,6 +578,10 @@ def test_send_test_webhook__request_exception__returns_error_response( mock_post.side_effect = requests.exceptions.RequestException( "Some internal exception details that should not be exposed!" ) + mocker.patch( + "webhooks.fields.socket.getaddrinfo", + return_value=[(socket.AF_INET, None, None, None, ("93.184.216.34", 0))], + ) url = reverse("api-v1:webhooks:webhooks-test") From a9f1da2e89e157ef834c443282d4dd44379ed4a5 Mon Sep 17 00:00:00 2001 From: Marnie0415 Date: Mon, 13 Jul 2026 20:24:02 +0800 Subject: [PATCH 6/6] fix(tests): set mock response.ok for proper response.ok behavior MagicMock().ok returns a truthy MagicMock, not a real boolean. Set mock_response.ok explicitly based on status code so that 'not response.ok' works correctly in the view. --- api/tests/unit/webhooks/test_unit_webhooks.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/api/tests/unit/webhooks/test_unit_webhooks.py b/api/tests/unit/webhooks/test_unit_webhooks.py index d96a8129ed06..620d210a23e6 100644 --- a/api/tests/unit/webhooks/test_unit_webhooks.py +++ b/api/tests/unit/webhooks/test_unit_webhooks.py @@ -369,6 +369,7 @@ def test_send_test_webhook__200_response_from_webhook__returns_correct_response( mock_post = mocker.patch("webhooks.webhooks.requests.post") mock_response = MagicMock() mock_response.status_code = 200 + mock_response.ok = True mock_response.text = "success" mock_post.return_value = mock_response mocker.patch( @@ -416,6 +417,7 @@ def test_send_test_webhook__various_2xx_status_codes__returns_success( mock_post = mocker.patch("webhooks.webhooks.requests.post") mock_response = MagicMock() mock_response.status_code = external_api_response_status + mock_response.ok = external_api_response_status < 400 mock_response.text = "success" mock_post.return_value = mock_response mocker.patch( @@ -465,6 +467,7 @@ def test_send_test_webhook__various_error_status_codes__returns_correct_response mock_post = mocker.patch("webhooks.webhooks.requests.post") mock_response = MagicMock() mock_response.status_code = external_api_response_status + mock_response.ok = external_api_response_status < 400 mock_response.text = external_api_error_text mock_post.return_value = mock_response mocker.patch( @@ -534,6 +537,7 @@ def test_send_test_webhook__various_secrets__sends_correct_payload( mock_post = mocker.patch("webhooks.webhooks.requests.post") mock_response = MagicMock() mock_response.status_code = 200 + mock_response.ok = True mock_post.return_value = mock_response mocker.patch( "webhooks.fields.socket.getaddrinfo",