From e241a6817a7bd1ef7daf7fa54b95d8f89d39fc4b Mon Sep 17 00:00:00 2001 From: "google-labs-jules[bot]" <161369871+google-labs-jules[bot]@users.noreply.github.com> Date: Thu, 15 Jan 2026 01:05:18 +0000 Subject: [PATCH] =?UTF-8?q?=F0=9F=9B=A1=EF=B8=8F=20Sentinel:=20[CRITICAL]?= =?UTF-8?q?=20Fix=20SSRF=20in=20Discord=20Webhook=20Client?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixed a critical Server-Side Request Forgery (SSRF) vulnerability where arbitrary URLs could be passed to the webhook client. - Strengthened `validate_webhook_url` in `utils/discord_api.py` to remove lenient checks. - Enforced URL validation in `send_to_discord_with_retry`. - Added regression tests in `tests/test_utils.py`. --- .jules/sentinel.md | 4 ++++ tests/test_utils.py | 42 +++++++++++++++++++++++++++++++++++++++++- utils/discord_api.py | 10 ++++++---- 3 files changed, 51 insertions(+), 5 deletions(-) create mode 100644 .jules/sentinel.md diff --git a/.jules/sentinel.md b/.jules/sentinel.md new file mode 100644 index 0000000..893dbd7 --- /dev/null +++ b/.jules/sentinel.md @@ -0,0 +1,4 @@ +## 2025-01-26 - Critical SSRF in Webhook Client +**Vulnerability:** `send_to_discord_with_retry` accepted arbitrary URLs, allowing Server-Side Request Forgery (SSRF). A malicious user could probe internal services or cloud metadata services. +**Learning:** The validation function `validate_webhook_url` existed but was not called in the main sending function. Also, `validate_webhook_url` had a fallback lenient check that could be bypassed. +**Prevention:** Always enforce input validation at the point of use. Avoid "lenient" fallback checks for security-critical inputs like URLs. diff --git a/tests/test_utils.py b/tests/test_utils.py index be674c8..4546dc3 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -13,7 +13,8 @@ import unittest sys.path.insert(0, os.path.dirname(os.path.dirname(os.path.abspath(__file__)))) from utils.sanitizer import sanitize_json_for_export -from utils.discord_api import validate_webhook_url, sanitize_webhook_for_logging +from utils.discord_api import validate_webhook_url, sanitize_webhook_for_logging, send_to_discord_with_retry +from unittest.mock import patch, MagicMock class TestSanitizer(unittest.TestCase): @@ -118,6 +119,45 @@ class TestWebhookValidation(unittest.TestCase): is_valid, message = validate_webhook_url("https://example.com") self.assertFalse(is_valid) + def test_bypass_attempt_url(self): + """Should reject URLs that attempt to bypass validation.""" + # This URL contains 'discord' and 'webhook' but is not hosted on discord.com + bypass_url = "http://evil-site.com/discord/webhook" + is_valid, message = validate_webhook_url(bypass_url) + self.assertFalse(is_valid) + + def test_localhost_url(self): + """Should reject localhost URLs (SSRF protection).""" + is_valid, message = validate_webhook_url("http://localhost:8080/admin") + self.assertFalse(is_valid) + + +class TestSSRFPrevention(unittest.TestCase): + """Tests for SSRF prevention mechanisms.""" + + def test_send_to_discord_validates_url(self): + """Should raise ValueError for invalid URLs before sending request.""" + malicious_url = "http://localhost:8080/admin/delete" + + # We don't need to mock requests.post because it should fail before calling it + with self.assertRaises(ValueError) as cm: + send_to_discord_with_retry(malicious_url, data={"content": "test"}) + + self.assertIn("Invalid webhook URL", str(cm.exception)) + + @patch('requests.post') + def test_send_to_discord_allows_valid_url(self, mock_post): + """Should allow valid Discord URLs.""" + valid_url = "https://discord.com/api/webhooks/123/abc" + + mock_response = MagicMock() + mock_response.status_code = 200 + mock_post.return_value = mock_response + + send_to_discord_with_retry(valid_url, data={"content": "test"}) + + mock_post.assert_called_once() + class TestWebhookSanitization(unittest.TestCase): """Tests for webhook URL sanitization for logging.""" diff --git a/utils/discord_api.py b/utils/discord_api.py index dd49293..98e5abc 100644 --- a/utils/discord_api.py +++ b/utils/discord_api.py @@ -45,10 +45,6 @@ def validate_webhook_url(url: str) -> Tuple[bool, str]: if re.match(pattern, url, re.IGNORECASE): return True, "Valid Discord webhook URL" - # More lenient check - if "discord" in url.lower() and "webhook" in url.lower(): - return True, "Appears to be a Discord webhook URL" - return False, "URL does not appear to be a valid Discord webhook URL" @@ -345,7 +341,13 @@ def send_to_discord_with_retry( Raises: requests.exceptions.RequestException: If all retries fail + ValueError: If the webhook URL is invalid """ + # Validate URL to prevent SSRF + is_valid, error_msg = validate_webhook_url(webhook_url) + if not is_valid: + raise ValueError(f"Invalid webhook URL: {error_msg}") + last_exception = None for attempt in range(max_retries):