🛡️ Sentinel: [HIGH] Fix SSRF in webhook delivery (#846)
* fix: validate webhook targets before delivery * test: cover webhook SSRF validation
This commit is contained in:
committed by
GitHub
parent
416c3c4758
commit
e2fa96318f
@@ -235,7 +235,10 @@ class TestSendWebhookNotification:
|
||||
mock_response.status_code = 200
|
||||
mock_response.raise_for_status = MagicMock()
|
||||
|
||||
with patch("app.utils.user_notification.httpx.post", return_value=mock_response) as mock_post:
|
||||
with (
|
||||
patch("app.utils.user_notification.is_private_ip", return_value=False),
|
||||
patch("app.utils.user_notification.httpx.post", return_value=mock_response) as mock_post,
|
||||
):
|
||||
result = _send_webhook_notification(
|
||||
{"url": "https://hook.example.com/test", "secret": "mysecret"},
|
||||
"document.processed",
|
||||
@@ -256,7 +259,10 @@ class TestSendWebhookNotification:
|
||||
mock_response.status_code = 200
|
||||
mock_response.raise_for_status = MagicMock()
|
||||
|
||||
with patch("app.utils.user_notification.httpx.post", return_value=mock_response) as mock_post:
|
||||
with (
|
||||
patch("app.utils.user_notification.is_private_ip", return_value=False),
|
||||
patch("app.utils.user_notification.httpx.post", return_value=mock_response) as mock_post,
|
||||
):
|
||||
result = _send_webhook_notification(
|
||||
{"url": "https://hook.example.com/test"},
|
||||
"document.failed",
|
||||
@@ -272,9 +278,12 @@ class TestSendWebhookNotification:
|
||||
"""_send_webhook_notification returns False when httpx raises."""
|
||||
from app.utils.user_notification import _send_webhook_notification
|
||||
|
||||
with patch(
|
||||
"app.utils.user_notification.httpx.post",
|
||||
side_effect=Exception("connection error"),
|
||||
with (
|
||||
patch("app.utils.user_notification.is_private_ip", return_value=False),
|
||||
patch(
|
||||
"app.utils.user_notification.httpx.post",
|
||||
side_effect=Exception("connection error"),
|
||||
),
|
||||
):
|
||||
result = _send_webhook_notification(
|
||||
{"url": "https://hook.example.com/test"},
|
||||
@@ -300,7 +309,10 @@ class TestSendWebhookNotification:
|
||||
)
|
||||
)
|
||||
|
||||
with patch("app.utils.user_notification.httpx.post", return_value=mock_response):
|
||||
with (
|
||||
patch("app.utils.user_notification.is_private_ip", return_value=False),
|
||||
patch("app.utils.user_notification.httpx.post", return_value=mock_response),
|
||||
):
|
||||
result = _send_webhook_notification(
|
||||
{"url": "https://hook.example.com/test"},
|
||||
"document.processed",
|
||||
@@ -310,6 +322,69 @@ class TestSendWebhookNotification:
|
||||
|
||||
assert result is False
|
||||
|
||||
def test_blocks_private_webhook_target(self):
|
||||
"""Webhook delivery is skipped for private network targets."""
|
||||
from app.utils.user_notification import _send_webhook_notification
|
||||
|
||||
with (
|
||||
patch("app.utils.user_notification.is_private_ip", return_value=True),
|
||||
patch("app.utils.user_notification.httpx.post") as mock_post,
|
||||
):
|
||||
result = _send_webhook_notification(
|
||||
{"url": "https://10.0.0.5/test"},
|
||||
"document.processed",
|
||||
"T",
|
||||
"M",
|
||||
)
|
||||
|
||||
assert result is False
|
||||
mock_post.assert_not_called()
|
||||
|
||||
def test_blocks_metadata_webhook_target(self):
|
||||
"""Webhook delivery is skipped for cloud metadata endpoints."""
|
||||
from app.utils.user_notification import _send_webhook_notification
|
||||
|
||||
with patch("app.utils.user_notification.httpx.post") as mock_post:
|
||||
result = _send_webhook_notification(
|
||||
{"url": "http://169.254.169.254/latest/meta-data"},
|
||||
"document.processed",
|
||||
"T",
|
||||
"M",
|
||||
)
|
||||
|
||||
assert result is False
|
||||
mock_post.assert_not_called()
|
||||
|
||||
def test_blocks_invalid_webhook_scheme(self):
|
||||
"""Webhook delivery is skipped for unsupported URL schemes."""
|
||||
from app.utils.user_notification import _send_webhook_notification
|
||||
|
||||
with patch("app.utils.user_notification.httpx.post") as mock_post:
|
||||
result = _send_webhook_notification(
|
||||
{"url": "file:///etc/passwd"},
|
||||
"document.processed",
|
||||
"T",
|
||||
"M",
|
||||
)
|
||||
|
||||
assert result is False
|
||||
mock_post.assert_not_called()
|
||||
|
||||
def test_blocks_webhook_without_hostname(self):
|
||||
"""Webhook delivery is skipped when the URL has no hostname."""
|
||||
from app.utils.user_notification import _send_webhook_notification
|
||||
|
||||
with patch("app.utils.user_notification.httpx.post") as mock_post:
|
||||
result = _send_webhook_notification(
|
||||
{"url": "https:///missing-host"},
|
||||
"document.processed",
|
||||
"T",
|
||||
"M",
|
||||
)
|
||||
|
||||
assert result is False
|
||||
mock_post.assert_not_called()
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# dispatch_user_notification – preference loop
|
||||
|
||||
@@ -81,6 +81,7 @@ class TestDeliverWebhook:
|
||||
|
||||
def test_success_returns_true(self, mocker):
|
||||
"""A 200 response returns True."""
|
||||
mocker.patch("app.utils.webhook.is_private_ip", return_value=False)
|
||||
mock_post = mocker.patch("app.utils.webhook.requests.post")
|
||||
mock_post.return_value = MagicMock(ok=True, status_code=200)
|
||||
|
||||
@@ -90,6 +91,7 @@ class TestDeliverWebhook:
|
||||
|
||||
def test_non_2xx_returns_false(self, mocker):
|
||||
"""A non-2xx response returns False."""
|
||||
mocker.patch("app.utils.webhook.is_private_ip", return_value=False)
|
||||
mock_post = mocker.patch("app.utils.webhook.requests.post")
|
||||
mock_post.return_value = MagicMock(ok=False, status_code=500)
|
||||
|
||||
@@ -100,6 +102,7 @@ class TestDeliverWebhook:
|
||||
"""A network error returns False."""
|
||||
import requests
|
||||
|
||||
mocker.patch("app.utils.webhook.is_private_ip", return_value=False)
|
||||
mocker.patch("app.utils.webhook.requests.post", side_effect=requests.ConnectionError("fail"))
|
||||
|
||||
result = deliver_webhook("https://example.com/hook", {"event": "test"})
|
||||
@@ -107,6 +110,7 @@ class TestDeliverWebhook:
|
||||
|
||||
def test_signature_header_included_when_secret(self, mocker):
|
||||
"""X-Webhook-Signature header is present when a secret is supplied."""
|
||||
mocker.patch("app.utils.webhook.is_private_ip", return_value=False)
|
||||
mock_post = mocker.patch("app.utils.webhook.requests.post")
|
||||
mock_post.return_value = MagicMock(ok=True, status_code=200)
|
||||
|
||||
@@ -118,6 +122,7 @@ class TestDeliverWebhook:
|
||||
|
||||
def test_no_signature_header_without_secret(self, mocker):
|
||||
"""X-Webhook-Signature header is absent when no secret is supplied."""
|
||||
mocker.patch("app.utils.webhook.is_private_ip", return_value=False)
|
||||
mock_post = mocker.patch("app.utils.webhook.requests.post")
|
||||
mock_post.return_value = MagicMock(ok=True, status_code=200)
|
||||
|
||||
@@ -126,6 +131,43 @@ class TestDeliverWebhook:
|
||||
headers = call_kwargs.kwargs.get("headers") or call_kwargs[1].get("headers")
|
||||
assert "X-Webhook-Signature" not in headers
|
||||
|
||||
def test_private_target_is_blocked(self, mocker):
|
||||
"""Private network webhook targets are not called."""
|
||||
mocker.patch("app.utils.webhook.is_private_ip", return_value=True)
|
||||
mock_post = mocker.patch("app.utils.webhook.requests.post")
|
||||
|
||||
result = deliver_webhook("https://10.0.0.5/hook", {"event": "test"})
|
||||
|
||||
assert result is False
|
||||
mock_post.assert_not_called()
|
||||
|
||||
def test_metadata_target_is_blocked(self, mocker):
|
||||
"""Cloud metadata webhook targets are not called."""
|
||||
mock_post = mocker.patch("app.utils.webhook.requests.post")
|
||||
|
||||
result = deliver_webhook("http://169.254.169.254/latest/meta-data", {"event": "test"})
|
||||
|
||||
assert result is False
|
||||
mock_post.assert_not_called()
|
||||
|
||||
def test_invalid_scheme_is_blocked(self, mocker):
|
||||
"""Unsupported webhook URL schemes are not called."""
|
||||
mock_post = mocker.patch("app.utils.webhook.requests.post")
|
||||
|
||||
result = deliver_webhook("file:///etc/passwd", {"event": "test"})
|
||||
|
||||
assert result is False
|
||||
mock_post.assert_not_called()
|
||||
|
||||
def test_missing_hostname_is_blocked(self, mocker):
|
||||
"""Webhook URLs without a hostname are not called."""
|
||||
mock_post = mocker.patch("app.utils.webhook.requests.post")
|
||||
|
||||
result = deliver_webhook("https:///missing-host", {"event": "test"})
|
||||
|
||||
assert result is False
|
||||
mock_post.assert_not_called()
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Unit tests – get_active_webhooks_for_event (DB)
|
||||
|
||||
Reference in New Issue
Block a user