🛡️ Sentinel: [CRITICAL] Fix SSRF in Discord Webhook Client

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

@@ -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.

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."""

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):