From cf3dd7d0299c1c0d47fb8199154774fe881762ba Mon Sep 17 00:00:00 2001 From: Peter Braun Date: Fri, 25 Sep 2026 19:59:56 +0200 Subject: [PATCH] harden notification backends against redirect-based credential leakage (#16673) * harden notification backends against redirect-based credential leakage * address code review feedback --- awx/main/notifications/grafana_backend.py | 9 ++- awx/main/notifications/mattermost_backend.py | 5 +- awx/main/notifications/rocketchat_backend.py | 9 ++- awx/main/notifications/webhook_backend.py | 15 +++++ .../tests/unit/notifications/test_grafana.py | 7 ++ .../unit/notifications/test_rocketchat.py | 2 + .../tests/unit/notifications/test_webhook.py | 66 +++++++++++++++++++ 7 files changed, 107 insertions(+), 6 deletions(-) diff --git a/awx/main/notifications/grafana_backend.py b/awx/main/notifications/grafana_backend.py index b1ac6ab335..a48a1f389b 100644 --- a/awx/main/notifications/grafana_backend.py +++ b/awx/main/notifications/grafana_backend.py @@ -97,12 +97,17 @@ class GrafanaBackend(AWXBaseEmailBackend, CustomNotificationBase): grafana_headers['Authorization'] = "Bearer {}".format(self.grafana_key) grafana_headers['Content-Type'] = "application/json" r = requests.post( - "{}/api/annotations".format(m.recipients()[0]), json=grafana_data, headers=grafana_headers, verify=(not self.grafana_no_verify_ssl) + "{}/api/annotations".format(m.recipients()[0]), + json=grafana_data, + headers=grafana_headers, + verify=(not self.grafana_no_verify_ssl), + allow_redirects=False, ) - if r.status_code >= 400: + if r.status_code < 200 or r.status_code >= 300: logger.error(smart_str(_("Error sending notification grafana: {}").format(r.status_code))) if not self.fail_silently: raise Exception(smart_str(_("Error sending notification grafana: {}").format(r.status_code))) + continue sent_messages += 1 return sent_messages diff --git a/awx/main/notifications/mattermost_backend.py b/awx/main/notifications/mattermost_backend.py index ed8711220f..70476eaace 100644 --- a/awx/main/notifications/mattermost_backend.py +++ b/awx/main/notifications/mattermost_backend.py @@ -41,10 +41,11 @@ class MattermostBackend(AWXBaseEmailBackend, CustomNotificationBase): payload['text'] = m.subject - r = requests.post("{}".format(m.recipients()[0]), json=payload, verify=(not self.mattermost_no_verify_ssl)) - if r.status_code >= 400: + r = requests.post("{}".format(m.recipients()[0]), json=payload, verify=(not self.mattermost_no_verify_ssl), allow_redirects=False) + if r.status_code < 200 or r.status_code >= 300: logger.error(smart_str(_("Error sending notification mattermost: {}").format(r.status_code))) if not self.fail_silently: raise Exception(smart_str(_("Error sending notification mattermost: {}").format(r.status_code))) + continue sent_messages += 1 return sent_messages diff --git a/awx/main/notifications/rocketchat_backend.py b/awx/main/notifications/rocketchat_backend.py index c741878172..a3eac782c2 100644 --- a/awx/main/notifications/rocketchat_backend.py +++ b/awx/main/notifications/rocketchat_backend.py @@ -39,12 +39,17 @@ class RocketChatBackend(AWXBaseEmailBackend, CustomNotificationBase): payload[optval] = optvalue.strip() r = requests.post( - "{}".format(m.recipients()[0]), data=json.dumps(payload), headers=get_awx_http_client_headers(), verify=(not self.rocketchat_no_verify_ssl) + "{}".format(m.recipients()[0]), + data=json.dumps(payload), + headers=get_awx_http_client_headers(), + verify=(not self.rocketchat_no_verify_ssl), + allow_redirects=False, ) - if r.status_code >= 400: + if r.status_code < 200 or r.status_code >= 300: logger.error(smart_str(_("Error sending notification rocket.chat: {}").format(r.status_code))) if not self.fail_silently: raise Exception(smart_str(_("Error sending notification rocket.chat: {}").format(r.status_code))) + continue sent_messages += 1 return sent_messages diff --git a/awx/main/notifications/webhook_backend.py b/awx/main/notifications/webhook_backend.py index dc4adbcb81..92f85b93fa 100644 --- a/awx/main/notifications/webhook_backend.py +++ b/awx/main/notifications/webhook_backend.py @@ -4,6 +4,8 @@ import base64 import json import logging +from urllib.parse import urlparse + import requests from awx.main.notifications.base import AWXBaseEmailBackend @@ -70,6 +72,14 @@ class WebhookBackend(AWXBaseEmailBackend, CustomNotificationBase): err = None + def _origin(value): + parsed = urlparse(value) + scheme = parsed.scheme.lower() + port = parsed.port or {"http": 80, "https": 443}.get(scheme) + return scheme, parsed.hostname, port + + original_origin = _origin(url) + for retries in range(self.MAX_RETRIES): # Sometimes we hit redirect URLs. We must account for this. We still extract the redirect URL from the response headers and try again. Max retires == 5 resp = chosen_method( @@ -101,6 +111,11 @@ class WebhookBackend(AWXBaseEmailBackend, CustomNotificationBase): if url is None: err = f"Webhook notification received redirect to a blank URL from {url_log_safe}. Response headers={resp.headers}" break + + if _origin(url) != original_origin: + logger.warning("Redirect changed origin; stripping credentials") + auth = None + headers = get_awx_http_client_headers() else: # no break condition in the loop encountered; therefore we have hit the maximum number of retries err = f"Webhook notification max number of retries [{self.MAX_RETRIES}] exceeded. Failed to send webhook notification to {url_log_safe}" diff --git a/awx/main/tests/unit/notifications/test_grafana.py b/awx/main/tests/unit/notifications/test_grafana.py index 4e21e683ea..0b11ef0936 100644 --- a/awx/main/tests/unit/notifications/test_grafana.py +++ b/awx/main/tests/unit/notifications/test_grafana.py @@ -32,6 +32,7 @@ def test_send_messages(): headers={'Content-Type': 'application/json', 'Authorization': 'Bearer testapikey'}, json={'text': 'test subject', 'isRegion': True, 'timeEnd': 120000, 'time': 60000}, verify=True, + allow_redirects=False, ) assert sent_messages == 1 @@ -62,6 +63,7 @@ def test_send_messages_with_no_verify_ssl(): headers={'Content-Type': 'application/json', 'Authorization': 'Bearer testapikey'}, json={'text': 'test subject', 'isRegion': True, 'timeEnd': 120000, 'time': 60000}, verify=False, + allow_redirects=False, ) assert sent_messages == 1 @@ -93,6 +95,7 @@ def test_send_messages_with_dashboardid(dashboardId): headers={'Content-Type': 'application/json', 'Authorization': 'Bearer testapikey'}, json={'text': 'test subject', 'isRegion': True, 'timeEnd': 120000, 'time': 60000, 'dashboardId': dashboardId}, verify=True, + allow_redirects=False, ) assert sent_messages == 1 @@ -124,6 +127,7 @@ def test_send_messages_with_panelid(panelId): headers={'Content-Type': 'application/json', 'Authorization': 'Bearer testapikey'}, json={'text': 'test subject', 'isRegion': True, 'timeEnd': 120000, 'panelId': int(panelId), 'time': 60000}, verify=True, + allow_redirects=False, ) assert sent_messages == 1 @@ -154,6 +158,7 @@ def test_send_messages_with_bothids(): headers={'Content-Type': 'application/json', 'Authorization': 'Bearer testapikey'}, json={'text': 'test subject', 'isRegion': True, 'timeEnd': 120000, 'panelId': 42, 'time': 60000, 'dashboardId': 42}, verify=True, + allow_redirects=False, ) assert sent_messages == 1 @@ -184,6 +189,7 @@ def test_send_messages_with_emptyids(): headers={'Content-Type': 'application/json', 'Authorization': 'Bearer testapikey'}, json={'text': 'test subject', 'isRegion': True, 'timeEnd': 120000, 'time': 60000}, verify=True, + allow_redirects=False, ) assert sent_messages == 1 @@ -214,5 +220,6 @@ def test_send_messages_with_tags(): headers={'Content-Type': 'application/json', 'Authorization': 'Bearer testapikey'}, json={'tags': ['ansible'], 'text': 'test subject', 'isRegion': True, 'timeEnd': 120000, 'time': 60000}, verify=True, + allow_redirects=False, ) assert sent_messages == 1 diff --git a/awx/main/tests/unit/notifications/test_rocketchat.py b/awx/main/tests/unit/notifications/test_rocketchat.py index 98d2bd2cf6..88cbe148b0 100644 --- a/awx/main/tests/unit/notifications/test_rocketchat.py +++ b/awx/main/tests/unit/notifications/test_rocketchat.py @@ -32,6 +32,7 @@ def test_send_messages(): data='{"text": "test subject"}', headers={'Content-Type': 'application/json', 'User-Agent': 'AWX 0.0.1.dev (open)'}, verify=True, + allow_redirects=False, ) assert sent_messages == 1 @@ -118,5 +119,6 @@ def test_send_messages_with_no_verify_ssl(): data='{"text": "test subject"}', headers={'Content-Type': 'application/json', 'User-Agent': 'AWX 0.0.1.dev (open)'}, verify=False, + allow_redirects=False, ) assert sent_messages == 1 diff --git a/awx/main/tests/unit/notifications/test_webhook.py b/awx/main/tests/unit/notifications/test_webhook.py index 4d0c8937ce..0a6e1a1e45 100644 --- a/awx/main/tests/unit/notifications/test_webhook.py +++ b/awx/main/tests/unit/notifications/test_webhook.py @@ -377,3 +377,69 @@ def test_send_messages_with_error_status_code(): error_call_args = logger_mock.error.call_args[0][0] assert "Error sending webhook notification: 404" in error_call_args assert sent_messages == 0 + + +def test_send_messages_redirect_strips_credentials_on_host_change(): + """Auth and custom headers are stripped when a redirect changes the host.""" + safe_headers = {'Content-Type': 'application/json', 'User-Agent': 'AWX 0.0.1.dev (open)'} + with ( + mock.patch('awx.main.notifications.webhook_backend.requests') as requests_mock, + mock.patch('awx.main.notifications.webhook_backend.get_awx_http_client_headers') as version_mock, + ): + requests_mock.post.side_effect = [ + mock.Mock(status_code=301, headers={"Location": "http://other-host.com/hook"}), + mock.Mock(status_code=200), + ] + version_mock.return_value = safe_headers + backend = webhook_backend.WebhookBackend('POST', {'Authorization': 'Bearer secret-token'}, username='user', password='secret') + message = EmailMessage('test subject', {'text': 'test body'}, [], ['http://example.com']) + sent_messages = backend.send_messages([message]) + assert requests_mock.post.call_count == 2 + first_call = requests_mock.post.call_args_list[0] + assert first_call.kwargs['auth'] == ('user', 'secret') + assert 'Authorization' in first_call.kwargs['headers'] + second_call = requests_mock.post.call_args_list[1] + assert second_call.kwargs['auth'] is None + assert second_call.kwargs['headers'] == safe_headers + assert sent_messages == 1 + + +def test_send_messages_redirect_strips_credentials_on_scheme_downgrade(): + """Auth is stripped when a redirect downgrades from HTTPS to HTTP.""" + safe_headers = {'Content-Type': 'application/json', 'User-Agent': 'AWX 0.0.1.dev (open)'} + with ( + mock.patch('awx.main.notifications.webhook_backend.requests') as requests_mock, + mock.patch('awx.main.notifications.webhook_backend.get_awx_http_client_headers') as version_mock, + ): + requests_mock.post.side_effect = [ + mock.Mock(status_code=301, headers={"Location": "http://example.com/hook"}), + mock.Mock(status_code=200), + ] + version_mock.return_value = safe_headers + backend = webhook_backend.WebhookBackend('POST', None, username='user', password='secret') + message = EmailMessage('test subject', {'text': 'test body'}, [], ['https://example.com']) + sent_messages = backend.send_messages([message]) + assert requests_mock.post.call_count == 2 + second_call = requests_mock.post.call_args_list[1] + assert second_call.kwargs['auth'] is None + assert sent_messages == 1 + + +def test_send_messages_redirect_keeps_auth_on_same_origin(): + """Auth credentials are preserved when a redirect stays on the same origin.""" + with ( + mock.patch('awx.main.notifications.webhook_backend.requests') as requests_mock, + mock.patch('awx.main.notifications.webhook_backend.get_awx_http_client_headers') as version_mock, + ): + requests_mock.post.side_effect = [ + mock.Mock(status_code=301, headers={"Location": "http://example.com/new-path"}), + mock.Mock(status_code=200), + ] + version_mock.return_value = {'Content-Type': 'application/json', 'User-Agent': 'AWX 0.0.1.dev (open)'} + backend = webhook_backend.WebhookBackend('POST', None, username='user', password='secret') + message = EmailMessage('test subject', {'text': 'test body'}, [], ['http://example.com']) + sent_messages = backend.send_messages([message]) + assert requests_mock.post.call_count == 2 + for call in requests_mock.post.call_args_list: + assert call.kwargs['auth'] == ('user', 'secret') + assert sent_messages == 1