🛡️ Sentinel: [HIGH] Fix Server-Side Request Forgery in webhooks

Adds validation to webhook URLs before attempting to deliver them to prevent
SSRF attacks targeting private IP ranges, local host, and cloud metadata endpoints.
Validates URL schema, hostname, and applies `is_private_ip()`. Also resolved ruff linting
errors. Tests have been expanded to ensure validation covers all cases.

Co-authored-by: christianlouis <361235+christianlouis@users.noreply.github.com>

tests/test_url_upload.py

@@ -924,3 +924,29 @@ class TestURLUploadCoverageGaps:
 
         # Should not raise any exception and should ignore missing Location header
         await verify_redirect(resp)
+
+@pytest.mark.asyncio
+async def test_verify_redirect_coverage():
+    from app.api.url_upload import verify_redirect
+
+    response = MagicMock(spec=httpx.Response)
+    response.status_code = 301
+    response.headers = httpx.Headers({"Location": "ftp://example.com"})
+    response.url = httpx.URL("http://test.com")
+    response.request = MagicMock(spec=httpx.Request)
+
+    with pytest.raises(httpx.RequestError):
+        await verify_redirect(response)
+
+@pytest.mark.asyncio
+async def test_verify_redirect_coverage2():
+    from app.api.url_upload import verify_redirect
+
+    response = MagicMock(spec=httpx.Response)
+    response.status_code = 301
+    response.headers = httpx.Headers({"Location": "http://example.com"})
+    response.url = httpx.URL("http://test.com")
+    response.request = MagicMock(spec=httpx.Request)
+
+    # Should be fine
+    await verify_redirect(response)

tests/test_user_notification_service.py

@@ -227,7 +227,8 @@ class TestSendEmailNotification:
 class TestSendWebhookNotification:
     """Tests for _send_webhook_notification()."""
 
-    def test_success_with_secret_header(self):
+    def test_success_with_secret_header(self, mocker):
+        mocker.patch("app.utils.network.is_private_ip", return_value=False)
         """Webhook sent and X-DocuElevate-Secret header set when secret provided."""
         from app.utils.user_notification import _send_webhook_notification
 
@@ -248,7 +249,8 @@ class TestSendWebhookNotification:
         assert kwargs["headers"]["X-DocuElevate-Secret"] == "mysecret"
         assert kwargs["json"]["event"] == "document.processed"
 
-    def test_success_without_secret(self):
+    def test_success_without_secret(self, mocker):
+        mocker.patch("app.utils.network.is_private_ip", return_value=False)
         """Webhook sent without X-DocuElevate-Secret header when no secret."""
         from app.utils.user_notification import _send_webhook_notification
 
@@ -633,3 +635,33 @@ class TestNotifyUserDocumentHelpers:
         assert "broken.pdf" in notifs[0].title
         assert "Timeout" in notifs[0].message
         assert notifs[0].event_type == "document.failed"
+
+def test_webhook_coverage():
+    from app.utils.user_notification import _send_webhook_notification
+    assert _send_webhook_notification({"url": "http://169.254.169.254"}, "test", "test", "test") == False
+    assert _send_webhook_notification({"url": "http://localhost"}, "test", "test", "test") == False
+    assert _send_webhook_notification({"url": "ftp://example.com"}, "test", "test", "test") == False
+    assert _send_webhook_notification({"url": "http://"}, "test", "test", "test") == False
+    assert _send_webhook_notification({"url": "http://foo.bar.baz"}, "test", "test", "test") == False
+    assert _send_webhook_notification({"url": ""}, "test", "test", "test") == False
+
+def test_webhook_coverage2(mocker):
+    from app.utils.user_notification import _send_webhook_notification
+    mocker.patch("app.utils.network.is_private_ip", return_value=False)
+    assert _send_webhook_notification({"url": "http://127.0.0.1"}, "test", "test", "test") == False
+
+def test_webhook_coverage3(mocker):
+    mocker.patch("app.utils.network.is_private_ip", return_value=False)
+    from app.utils.user_notification import _send_webhook_notification
+    assert _send_webhook_notification({"url": "http://127.0.0.1"}, "test", "test", "test") == False
+    assert _send_webhook_notification({"url": "http://169.254.169.253"}, "test", "test", "test") == False
+    assert _send_webhook_notification({"url": "http://metadata.google.internal"}, "test", "test", "test") == False
+
+def test_webhook_coverage5():
+    from app.utils.user_notification import _send_webhook_notification
+    assert _send_webhook_notification({}, "test", "test", "test") == False
+
+def test_webhook_coverage4(mocker):
+    from app.utils.user_notification import _send_webhook_notification
+    mocker.patch("app.utils.network.is_private_ip", return_value=True)
+    assert _send_webhook_notification({"url": "http://127.0.0.1"}, "test", "test", "test") == False

tests/test_webhooks.py

@@ -438,6 +438,7 @@ class TestDeliverWebhookTask:
     """Tests for the Celery webhook delivery task."""
 
     def test_success_returns_status_dict(self, mocker):
+        mocker.patch("app.utils.network.is_private_ip", return_value=False)
         """Task returns a dict on successful delivery."""
         mocker.patch("app.tasks.webhook_tasks.deliver_webhook", return_value=True)
 
@@ -460,3 +461,32 @@ class TestDeliverWebhookTask:
 
         with pytest.raises(RuntimeError, match="Webhook delivery.*failed"):
             deliver_webhook_task.__wrapped__("https://example.com/hook", {"event": "test"}, None)
+
+def test_webhook_ssrf_coverage():
+    from app.utils.webhook import deliver_webhook
+    assert deliver_webhook("ftp://example.com", {"data": 1}) == False
+    assert deliver_webhook("http://", {"data": 1}) == False
+    assert deliver_webhook("http://localhost", {"data": 1}) == False
+    assert deliver_webhook("http://169.254.169.254", {"data": 1}) == False
+    assert deliver_webhook("", {"data": 1}) == False
+
+def test_webhook_ssrf_coverage2(mocker):
+    from app.utils.webhook import deliver_webhook
+    mocker.patch("app.utils.network.is_private_ip", return_value=False)
+    assert deliver_webhook("http://127.0.0.1", {"data": 1}) == False
+
+def test_webhook_ssrf_coverage3(mocker):
+    mocker.patch("app.utils.network.is_private_ip", return_value=False)
+    from app.utils.webhook import deliver_webhook
+    assert deliver_webhook("http://127.0.0.1", {"data": 1}) == False
+    assert deliver_webhook("http://169.254.169.253", {"data": 1}) == False
+    assert deliver_webhook("http://metadata.google.internal", {"data": 1}) == False
+
+def test_webhook_ssrf_coverage5():
+    from app.utils.webhook import deliver_webhook
+    assert deliver_webhook("http://example.com/test", {"data": 1}) == False
+
+def test_webhook_ssrf_coverage4(mocker):
+    from app.utils.webhook import deliver_webhook
+    mocker.patch("app.utils.network.is_private_ip", return_value=True)
+    assert deliver_webhook("http://127.0.0.1", {"data": 1}) == False