mirror of
https://github.com/jhd3197/ServerKit.git
synced 2026-11-10 05:53:05 +09:00
fix(security): route webhook posts through safe_http
monitoring and generic chat webhooks now refuse private/loopback targets.
This commit is contained in:
1 parent
81aa19dafd
commit
800a54dc1a
4 files changed
+52
-11
No files matched your search
@@ -19,10 +19,9 @@ import logging
|
||||
import re
|
||||
from datetime import datetime
|
||||
|
||||
import requests
|
||||
|
||||
from app import db
|
||||
from app.models.chat_webhook import ChatWebhookConnection
|
||||
from app.utils import safe_http
|
||||
from app.utils.crypto import encrypt_secret
|
||||
|
||||
logger = logging.getLogger(__name__)
|
||||
@@ -405,8 +404,11 @@ class ChatWebhookService:
|
||||
headers[SIGNATURE_HEADER] = 'sha256=' + signature
|
||||
|
||||
try:
|
||||
resp = requests.post(url, data=body, headers=headers,
|
||||
timeout=_REQUEST_TIMEOUT)
|
||||
resp = safe_http.post(url, data=body, headers=headers,
|
||||
timeout=_REQUEST_TIMEOUT)
|
||||
except safe_http.UnsafeURLError as exc:
|
||||
# Non-retryable: the URL itself is the problem, retrying can't help.
|
||||
return DeliveryResult.skipped(f'webhook url refused: {exc}')
|
||||
except Exception as exc: # network error -> retryable failure
|
||||
return DeliveryResult.failed(str(exc))
|
||||
|
||||
|
||||
@@ -214,7 +214,7 @@ class MonitoringService:
|
||||
@classmethod
|
||||
def send_webhook_alert(cls, alerts: List[Dict]) -> Dict:
|
||||
"""Send webhook alert notification."""
|
||||
import requests
|
||||
from app.utils import safe_http
|
||||
|
||||
config = cls.get_config()
|
||||
webhook_config = config.get('webhook', {})
|
||||
@@ -229,7 +229,7 @@ class MonitoringService:
|
||||
'alerts': alerts
|
||||
}
|
||||
|
||||
response = requests.post(
|
||||
response = safe_http.post(
|
||||
webhook_config['url'],
|
||||
json=payload,
|
||||
timeout=10
|
||||
@@ -239,6 +239,8 @@ class MonitoringService:
|
||||
return {'success': True, 'message': 'Webhook sent'}
|
||||
return {'success': False, 'error': f'Webhook returned {response.status_code}'}
|
||||
|
||||
except safe_http.UnsafeURLError as e:
|
||||
return {'success': False, 'error': f'Webhook URL refused: {e}'}
|
||||
except Exception as e:
|
||||
return {'success': False, 'error': str(e)}
|
||||
|
||||
|
||||
@@ -321,7 +321,7 @@ class TestSigningAndDelivery:
|
||||
captured.update({'url': url, 'data': data, 'headers': headers})
|
||||
return _Resp()
|
||||
|
||||
monkeypatch.setattr('app.services.chat_webhook_service.requests.post', fake_post)
|
||||
monkeypatch.setattr('app.services.chat_webhook_service.safe_http.post', fake_post)
|
||||
|
||||
user = _make_user()
|
||||
conn = ChatWebhookService.add({'kind': 'webhook', 'name': 'Ops', 'url': 'https://hooks/x',
|
||||
@@ -392,7 +392,7 @@ class TestConnectionTesting:
|
||||
status_code = 503
|
||||
|
||||
monkeypatch.setattr(
|
||||
'app.services.chat_webhook_service.requests.post',
|
||||
'app.services.chat_webhook_service.safe_http.post',
|
||||
lambda *args, **kwargs: _Resp(),
|
||||
)
|
||||
conn = ChatWebhookService.add({
|
||||
@@ -428,7 +428,7 @@ class TestConnectionTesting:
|
||||
raise RuntimeError(f'failed POST {url}')
|
||||
|
||||
monkeypatch.setattr(
|
||||
'app.services.chat_webhook_service.requests.post', fail_post,
|
||||
'app.services.chat_webhook_service.safe_http.post', fail_post,
|
||||
)
|
||||
secret_url = 'https://hooks.example/super-secret-token'
|
||||
conn = ChatWebhookService.add({
|
||||
@@ -454,7 +454,7 @@ class TestConnectionTesting:
|
||||
raise RuntimeError(f'failed POST {url}')
|
||||
|
||||
monkeypatch.setattr(
|
||||
'app.services.chat_webhook_service.requests.post', fail_post,
|
||||
'app.services.chat_webhook_service.safe_http.post', fail_post,
|
||||
)
|
||||
conn = ChatWebhookService.add({
|
||||
'kind': 'webhook',
|
||||
@@ -624,7 +624,7 @@ class TestApi:
|
||||
status_code = 503
|
||||
|
||||
monkeypatch.setattr(
|
||||
'app.services.chat_webhook_service.requests.post',
|
||||
'app.services.chat_webhook_service.safe_http.post',
|
||||
lambda *args, **kwargs: _Resp(),
|
||||
)
|
||||
conn = ChatWebhookService.add({
|
||||
|
||||
@@ -0,0 +1,37 @@
|
||||
"""User-configured webhook URLs must go through app.utils.safe_http."""
|
||||
import pytest
|
||||
|
||||
from app.notifications.channels.base import DeliveryResult
|
||||
from app.services.chat_webhook_service import ChatWebhookService
|
||||
from app.services.monitoring_service import MonitoringService
|
||||
|
||||
PRIVATE_URLS = ['http://127.0.0.1:5000/hook', 'http://169.254.169.254/latest/meta-data', 'http://localhost/x']
|
||||
|
||||
|
||||
@pytest.mark.parametrize('url', PRIVATE_URLS)
|
||||
def test_monitoring_webhook_refuses_private_targets(url, monkeypatch):
|
||||
monkeypatch.setattr(MonitoringService, 'get_config',
|
||||
classmethod(lambda cls: {'webhook': {'enabled': True, 'url': url}}))
|
||||
result = MonitoringService.send_webhook_alert([{'type': 'cpu'}])
|
||||
assert result['success'] is False
|
||||
assert 'refused' in result['error']
|
||||
|
||||
|
||||
@pytest.mark.parametrize('url', PRIVATE_URLS)
|
||||
def test_chat_webhook_refuses_private_targets(url):
|
||||
class _Notification:
|
||||
severity = 'info'
|
||||
event_key = 'x'
|
||||
title = 't'
|
||||
body = 'b'
|
||||
created_at = None
|
||||
category = 'c'
|
||||
action_path = None
|
||||
id = 1
|
||||
|
||||
def get_data(self):
|
||||
return {}
|
||||
|
||||
result = ChatWebhookService._deliver_webhook(None, {'url': url}, _Notification())
|
||||
assert result.status == DeliveryResult.SKIPPED # terminal: the queue must not retry
|
||||
assert 'refused' in result.error
|
||||
Reference in new issue
Block a user