From 8892bc530f6d10c0d4f57670e08e5afd30d0ce46 Mon Sep 17 00:00:00 2001 From: Marcos Paulo Diniz Date: Mon, 10 Aug 2026 15:50:04 -0300 Subject: [PATCH 1/6] chore: ignore local worktrees --- .gitignore | 1 + 1 file changed, 1 insertion(+) diff --git a/.gitignore b/.gitignore index 0ffd0364..33c12b2b 100644 --- a/.gitignore +++ b/.gitignore @@ -91,6 +91,7 @@ Thumbs.db *.sublime-workspace # Project Specific +.worktrees/ .planning_old/ .mcp.json new_templates/ From dc0ef654356468fed96bea208e908665985ed327 Mon Sep 17 00:00:00 2001 From: Marcos Paulo Diniz Date: Mon, 10 Aug 2026 16:57:46 -0300 Subject: [PATCH 2/6] feat(notifications): update chat connections --- backend/app/api/notifications.py | 15 ++ backend/app/services/chat_webhook_service.py | 90 +++++++++++ .../test_notification_chat_connections.py | 146 ++++++++++++++++++ 3 files changed, 251 insertions(+) diff --git a/backend/app/api/notifications.py b/backend/app/api/notifications.py index 46670b81..705b9635 100644 --- a/backend/app/api/notifications.py +++ b/backend/app/api/notifications.py @@ -423,6 +423,21 @@ def add_chat_connection(): return jsonify({'connection': conn.to_dict()}), 201 +@notifications_bp.route('/admin/chat-connections/', methods=['PUT']) +@jwt_required() +@admin_required +def update_chat_connection(conn_id): + """Update mutable chat connection metadata and credentials.""" + from app.services.chat_webhook_service import ChatWebhookService + try: + conn = ChatWebhookService.update(conn_id, request.get_json() or {}) + except ValueError as exc: + return jsonify({'error': str(exc)}), 400 + if conn is None: + return jsonify({'error': 'Connection not found'}), 404 + return jsonify({'success': True, 'connection': conn.to_dict()}), 200 + + @notifications_bp.route('/admin/chat-connections/', methods=['DELETE']) @jwt_required() @admin_required diff --git a/backend/app/services/chat_webhook_service.py b/backend/app/services/chat_webhook_service.py index 0e5757f4..f2e77fd8 100644 --- a/backend/app/services/chat_webhook_service.py +++ b/backend/app/services/chat_webhook_service.py @@ -84,6 +84,96 @@ def add(cls, data): db.session.commit() return conn + @classmethod + def update(cls, conn_id, data): + """Update mutable connection fields without exposing stored secrets. + + Credential fields are patch-like: omitted values are preserved, empty + optional values clear the credential, and required destinations cannot + be cleared. The connection kind and default flag have dedicated + lifecycle semantics and cannot be changed here. + """ + if not isinstance(data, dict): + raise ValueError('request body must be an object') + + conn = db.session.get(ChatWebhookConnection, conn_id) + if conn is None: + return None + + if 'kind' in data: + kind = (data.get('kind') or '').strip().lower() + if kind != conn.kind: + raise ValueError('connection kind cannot be changed') + if 'is_default' in data: + raise ValueError('use the default endpoint to change is_default') + + new_name = conn.name + new_categories_json = conn.categories_json + new_is_active = conn.is_active + new_credentials_json = conn.credentials_json + + if 'name' in data: + new_name = str(data.get('name') or '').strip() + if not new_name: + raise ValueError('name is required') + + if 'categories' in data: + categories = data.get('categories') + if not isinstance(categories, list): + raise ValueError('categories must be a list') + categories = [category for category in categories if category] + new_categories_json = json.dumps(categories) if categories else None + + if 'is_active' in data: + new_is_active = bool(data.get('is_active')) + + url_supplied = 'url' in data or 'webhook_url' in data + credential_supplied = url_supplied or any( + field in data for field in ('secret', 'chat_id', 'bot_token') + ) + if credential_supplied: + credentials = conn.credentials() + if conn.kind == 'telegram': + if 'chat_id' in data: + chat_id = str(data.get('chat_id') or '').strip() + if not chat_id: + raise ValueError('telegram connection requires a chat_id') + credentials['chat_id'] = chat_id + if 'bot_token' in data: + bot_token = data.get('bot_token') + if bot_token in (None, ''): + credentials.pop('bot_token', None) + else: + credentials['bot_token'] = str(bot_token) + else: + if url_supplied: + url_value = data.get('url') if 'url' in data else data.get('webhook_url') + url = str(url_value or '').strip() + if not url: + raise ValueError(f'{conn.kind} connection requires a url') + credentials['url'] = url + if 'secret' in data: + secret = data.get('secret') + if secret in (None, ''): + credentials.pop('secret', None) + else: + credentials['secret'] = str(secret) + + required = 'chat_id' if conn.kind == 'telegram' else 'url' + if not credentials.get(required): # pragma: no cover - corrupt legacy row + raise ValueError(f'{conn.kind} connection requires a {required}') + new_credentials_json = json.dumps({ + key: encrypt_secret(str(value)) + for key, value in credentials.items() + }) + + conn.name = new_name + conn.categories_json = new_categories_json + conn.is_active = new_is_active + conn.credentials_json = new_credentials_json + db.session.commit() + return conn + @classmethod def delete(cls, conn_id): """Delete a connection. If it was its kind's default, promote the oldest diff --git a/backend/tests/test_notification_chat_connections.py b/backend/tests/test_notification_chat_connections.py index 3ee615cb..f422a445 100644 --- a/backend/tests/test_notification_chat_connections.py +++ b/backend/tests/test_notification_chat_connections.py @@ -71,6 +71,105 @@ def test_delete_promotes_new_default(self, app): db.session.refresh(b) assert b.is_default is True + def test_update_changes_metadata_and_preserves_omitted_credentials(self, app): + conn = ChatWebhookService.add({ + 'kind': 'webhook', + 'name': 'Old name', + 'url': 'https://hooks.example/old', + 'secret': 'keep-me', + 'categories': ['security'], + }) + + updated = ChatWebhookService.update(conn.id, { + 'name': 'New name', + 'categories': ['backups'], + 'is_active': False, + }) + + assert updated.name == 'New name' + assert updated.categories() == ['backups'] + assert updated.is_active is False + assert updated.credentials() == { + 'url': 'https://hooks.example/old', + 'secret': 'keep-me', + } + + def test_update_rotates_supplied_credentials(self, app): + conn = ChatWebhookService.add({ + 'kind': 'webhook', + 'name': 'Ops', + 'url': 'https://hooks.example/old', + 'secret': 'old-secret', + }) + + updated = ChatWebhookService.update(conn.id, { + 'url': 'https://hooks.example/new', + 'secret': 'new-secret', + }) + + assert updated.credentials() == { + 'url': 'https://hooks.example/new', + 'secret': 'new-secret', + } + assert updated.raw_credentials()['url'] != 'https://hooks.example/new' + assert updated.raw_credentials()['secret'] != 'new-secret' + + def test_update_clears_explicitly_empty_optional_credential(self, app): + conn = ChatWebhookService.add({ + 'kind': 'telegram', + 'name': 'Bot', + 'chat_id': '1234', + 'bot_token': 'token-to-clear', + }) + + updated = ChatWebhookService.update(conn.id, {'bot_token': ''}) + + assert updated.credentials() == {'chat_id': '1234'} + + def test_update_rejects_kind_change(self, app): + conn = ChatWebhookService.add({ + 'kind': 'discord', 'name': 'Ops', 'url': 'https://discord/hook', + }) + + with pytest.raises(ValueError, match='kind cannot be changed'): + ChatWebhookService.update(conn.id, {'kind': 'slack'}) + + def test_update_rejects_non_list_categories(self, app): + conn = ChatWebhookService.add({ + 'kind': 'discord', 'name': 'Ops', 'url': 'https://discord/hook', + }) + + with pytest.raises(ValueError, match='categories must be a list'): + ChatWebhookService.update(conn.id, {'categories': 'security'}) + + def test_update_rejects_empty_required_destination(self, app): + conn = ChatWebhookService.add({ + 'kind': 'telegram', 'name': 'Bot', 'chat_id': '1234', + }) + + with pytest.raises(ValueError, match='requires a chat_id'): + ChatWebhookService.update(conn.id, {'chat_id': ''}) + + def test_update_validation_failure_does_not_mutate_connection(self, app): + conn = ChatWebhookService.add({ + 'kind': 'telegram', + 'name': 'Original', + 'chat_id': '1234', + 'categories': ['security'], + }) + + with pytest.raises(ValueError, match='requires a chat_id'): + ChatWebhookService.update(conn.id, { + 'name': 'Changed', + 'categories': ['apps'], + 'is_active': False, + 'chat_id': '', + }) + + assert conn.name == 'Original' + assert conn.categories() == ['security'] + assert conn.is_active is True + class TestCategoryRouting: def test_catch_all_matches_every_category(self, app): @@ -189,3 +288,50 @@ def test_crud_roundtrip(self, app, client, auth_headers): dele = client.delete(f'/api/v1/notifications/admin/chat-connections/{cid}', headers=auth_headers) assert dele.status_code == 200 + + def test_update_connection(self, app, client, auth_headers): + conn = ChatWebhookService.add({ + 'kind': 'webhook', + 'name': 'Old name', + 'url': 'https://hooks.example/original-destination', + 'secret': 'never-serialize-me', + }) + + resp = client.put( + f'/api/v1/notifications/admin/chat-connections/{conn.id}', + json={'name': 'New name', 'categories': ['apps'], 'is_active': False}, + headers=auth_headers, + ) + + assert resp.status_code == 200 + body = resp.get_json() + assert body['success'] is True + assert body['connection']['name'] == 'New name' + assert body['connection']['categories'] == ['apps'] + assert body['connection']['is_active'] is False + assert 'never-serialize-me' not in json.dumps(body) + assert 'https://hooks.example/original-destination' not in json.dumps(body) + + def test_update_connection_returns_404_for_unknown_id(self, app, client, auth_headers): + resp = client.put( + '/api/v1/notifications/admin/chat-connections/999999', + json={'name': 'Missing'}, + headers=auth_headers, + ) + + assert resp.status_code == 404 + assert resp.get_json() == {'error': 'Connection not found'} + + def test_update_connection_returns_400_for_kind_change(self, app, client, auth_headers): + conn = ChatWebhookService.add({ + 'kind': 'discord', 'name': 'Ops', 'url': 'https://discord/hook', + }) + + resp = client.put( + f'/api/v1/notifications/admin/chat-connections/{conn.id}', + json={'kind': 'slack'}, + headers=auth_headers, + ) + + assert resp.status_code == 400 + assert 'kind cannot be changed' in resp.get_json()['error'] From 10ba94b7b7f64b2ddbcb973e9de3a94dd3c16102 Mon Sep 17 00:00:00 2001 From: Marcos Paulo Diniz Date: Mon, 10 Aug 2026 17:07:16 -0300 Subject: [PATCH 3/6] feat(notifications): test chat connections --- backend/app/api/notifications.py | 12 ++ backend/app/services/chat_webhook_service.py | 54 ++++++++ .../test_notification_chat_connections.py | 130 ++++++++++++++++++ 3 files changed, 196 insertions(+) diff --git a/backend/app/api/notifications.py b/backend/app/api/notifications.py index 705b9635..289b3491 100644 --- a/backend/app/api/notifications.py +++ b/backend/app/api/notifications.py @@ -438,6 +438,18 @@ def update_chat_connection(conn_id): return jsonify({'success': True, 'connection': conn.to_dict()}), 200 +@notifications_bp.route('/admin/chat-connections//test', methods=['POST']) +@jwt_required() +@admin_required +def test_chat_connection(conn_id): + """Send a synchronous test through one chat connection.""" + from app.services.chat_webhook_service import ChatWebhookService + result = ChatWebhookService.test(conn_id) + if result is None: + return jsonify({'error': 'Connection not found'}), 404 + return jsonify(result), 200 if result.get('success') else 400 + + @notifications_bp.route('/admin/chat-connections/', methods=['DELETE']) @jwt_required() @admin_required diff --git a/backend/app/services/chat_webhook_service.py b/backend/app/services/chat_webhook_service.py index f2e77fd8..142149e2 100644 --- a/backend/app/services/chat_webhook_service.py +++ b/backend/app/services/chat_webhook_service.py @@ -235,6 +235,60 @@ def active_for_category(category): .all()) return [c for c in conns if c.matches_category(category)] + # ------------------------------------------------------------------ + # Connection testing + # ------------------------------------------------------------------ + @classmethod + def test(cls, conn_id): + """Synchronously send a test through a connection's real formatter. + + Inactive connections remain testable so an administrator can validate + a destination before enabling it. The transient notification is never + persisted; only the connection's latest test outcome is recorded. + """ + conn = db.session.get(ChatWebhookConnection, conn_id) + if conn is None: + return None + + from app.notifications.models import Notification + + message = 'This is a test notification from ServerKit.' + notification = Notification( + event_key='notification.test', + category='system', + severity=Notification.SEVERITY_TEST, + title='ServerKit test notification', + body=message, + audience=f'chat connection:{conn.id}', + created_at=datetime.utcnow(), + ) + notification.set_data({'message': message}) + + try: + credentials = conn.credentials() + if conn.kind == 'webhook': + delivery_result = cls._deliver_webhook( + conn, credentials, notification) + else: + delivery_result = cls._deliver_chat( + conn, credentials, notification) + success = delivery_result.ok + error = delivery_result.error + except Exception as exc: # pragma: no cover - defensive formatter guard + success = False + error = str(exc) + + conn.last_tested_at = datetime.utcnow() + conn.last_test_ok = success + db.session.commit() + + if success: + return {'success': True, 'message': 'Test notification sent'} + return { + 'success': False, + 'error': str(error or 'test notification failed')[:300], + } + # ------------------------------------------------------------------ # Delivery (called by the chat channel adapter for ``conn:`` targets) # ------------------------------------------------------------------ diff --git a/backend/tests/test_notification_chat_connections.py b/backend/tests/test_notification_chat_connections.py index f422a445..c50db373 100644 --- a/backend/tests/test_notification_chat_connections.py +++ b/backend/tests/test_notification_chat_connections.py @@ -255,6 +255,74 @@ def test_discord_connection_delegates_to_formatter(self, app, monkeypatch): assert seen['cfg']['webhook_url'] == 'https://discord/webhook' +class TestConnectionTesting: + def test_inactive_connection_can_send_test_through_real_formatter(self, app, monkeypatch): + captured = {} + + class _Resp: + status_code = 204 + text = '' + + def fake_post(url, json=None, timeout=None, **kwargs): + captured.update({'url': url, 'json': json, 'timeout': timeout}) + return _Resp() + + monkeypatch.setattr('app.services.notification_service.requests.post', fake_post) + conn = ChatWebhookService.add({ + 'kind': 'discord', + 'name': 'Disabled room', + 'url': 'https://discord.example/webhook', + 'is_active': False, + }) + + result = ChatWebhookService.test(conn.id) + + assert result == {'success': True, 'message': 'Test notification sent'} + assert captured['url'] == 'https://discord.example/webhook' + assert captured['json']['embeds'][0]['description'] == ( + 'This is a test notification from ServerKit.' + ) + assert conn.last_tested_at is not None + assert conn.last_test_ok is True + + def test_failed_connection_test_persists_failure(self, app, monkeypatch): + class _Resp: + ok = False + status_code = 503 + + monkeypatch.setattr( + 'app.services.chat_webhook_service.requests.post', + lambda *args, **kwargs: _Resp(), + ) + conn = ChatWebhookService.add({ + 'kind': 'webhook', 'name': 'Ops', 'url': 'https://hooks.example/failing', + }) + + result = ChatWebhookService.test(conn.id) + + assert result == {'success': False, 'error': 'webhook returned 503'} + assert conn.last_tested_at is not None + assert conn.last_test_ok is False + + def test_connection_test_bounds_transport_errors(self, app, monkeypatch): + def fail_post(*args, **kwargs): + raise RuntimeError('x' * 500) + + monkeypatch.setattr('app.services.notification_service.requests.post', fail_post) + conn = ChatWebhookService.add({ + 'kind': 'discord', 'name': 'Ops', 'url': 'https://discord.example/webhook', + }) + + result = ChatWebhookService.test(conn.id) + + assert result['success'] is False + assert result['error'] == 'x' * 300 + assert conn.last_test_ok is False + + def test_connection_test_returns_none_for_unknown_id(self, app): + assert ChatWebhookService.test(999999) is None + + class TestImport: def test_import_is_idempotent(self, app, monkeypatch): from app.services.notification_service import NotificationService @@ -335,3 +403,65 @@ def test_update_connection_returns_400_for_kind_change(self, app, client, auth_h assert resp.status_code == 400 assert 'kind cannot be changed' in resp.get_json()['error'] + + def test_test_connection_returns_200_on_delivery(self, app, client, auth_headers, + monkeypatch): + class _Resp: + status_code = 204 + text = '' + + monkeypatch.setattr( + 'app.services.notification_service.requests.post', + lambda *args, **kwargs: _Resp(), + ) + conn = ChatWebhookService.add({ + 'kind': 'discord', + 'name': 'Disabled room', + 'url': 'https://discord.example/webhook', + 'is_active': False, + }) + + resp = client.post( + f'/api/v1/notifications/admin/chat-connections/{conn.id}/test', + headers=auth_headers, + ) + + assert resp.status_code == 200 + assert resp.get_json() == { + 'success': True, 'message': 'Test notification sent', + } + db.session.refresh(conn) + assert conn.last_test_ok is True + + def test_test_connection_returns_400_on_delivery_failure(self, app, client, + auth_headers, monkeypatch): + class _Resp: + ok = False + status_code = 503 + + monkeypatch.setattr( + 'app.services.chat_webhook_service.requests.post', + lambda *args, **kwargs: _Resp(), + ) + conn = ChatWebhookService.add({ + 'kind': 'webhook', 'name': 'Ops', 'url': 'https://hooks.example/failing', + }) + + resp = client.post( + f'/api/v1/notifications/admin/chat-connections/{conn.id}/test', + headers=auth_headers, + ) + + assert resp.status_code == 400 + assert resp.get_json() == { + 'success': False, 'error': 'webhook returned 503', + } + + def test_test_connection_returns_404_for_unknown_id(self, app, client, auth_headers): + resp = client.post( + '/api/v1/notifications/admin/chat-connections/999999/test', + headers=auth_headers, + ) + + assert resp.status_code == 404 + assert resp.get_json() == {'error': 'Connection not found'} From fb57088ab7fee3c415234cd28f84a72328c42bce Mon Sep 17 00:00:00 2001 From: Marcos Paulo Diniz Date: Mon, 10 Aug 2026 17:09:58 -0300 Subject: [PATCH 4/6] feat(notifications): select default chat connection --- backend/app/api/notifications.py | 12 ++++ backend/app/models/chat_webhook.py | 3 +- backend/app/services/chat_webhook_service.py | 13 +++++ .../test_notification_chat_connections.py | 58 +++++++++++++++++++ 4 files changed, 85 insertions(+), 1 deletion(-) diff --git a/backend/app/api/notifications.py b/backend/app/api/notifications.py index 289b3491..388398ff 100644 --- a/backend/app/api/notifications.py +++ b/backend/app/api/notifications.py @@ -450,6 +450,18 @@ def test_chat_connection(conn_id): return jsonify(result), 200 if result.get('success') else 400 +@notifications_bp.route('/admin/chat-connections//default', methods=['POST']) +@jwt_required() +@admin_required +def set_default_chat_connection(conn_id): + """Select the active administrative default for a connection kind.""" + from app.services.chat_webhook_service import ChatWebhookService + conn = ChatWebhookService.set_default(conn_id) + if conn is None: + return jsonify({'error': 'Connection not found'}), 404 + return jsonify({'success': True, 'connection': conn.to_dict()}), 200 + + @notifications_bp.route('/admin/chat-connections/', methods=['DELETE']) @jwt_required() @admin_required diff --git a/backend/app/models/chat_webhook.py b/backend/app/models/chat_webhook.py index cc152d78..42dffbb9 100644 --- a/backend/app/models/chat_webhook.py +++ b/backend/app/models/chat_webhook.py @@ -35,7 +35,8 @@ class ChatWebhookConnection(db.Model): categories_json = db.Column(db.Text) is_active = db.Column(db.Boolean, default=True, nullable=False) - # The default connection for its kind (used when nothing category-matches). + # Administrative default for this kind. Category fan-out remains driven by + # active connections whose category filters match the notification. is_default = db.Column(db.Boolean, default=False, index=True) # True when created by the one-time import of legacy notifications.json config. imported = db.Column(db.Boolean, default=False) diff --git a/backend/app/services/chat_webhook_service.py b/backend/app/services/chat_webhook_service.py index 142149e2..05ae743d 100644 --- a/backend/app/services/chat_webhook_service.py +++ b/backend/app/services/chat_webhook_service.py @@ -174,6 +174,19 @@ def update(cls, conn_id, data): db.session.commit() return conn + @classmethod + def set_default(cls, conn_id): + """Select and activate the administrative default for one kind.""" + conn = db.session.get(ChatWebhookConnection, conn_id) + if conn is None: + return None + + for candidate in ChatWebhookConnection.query.filter_by(kind=conn.kind).all(): + candidate.is_default = candidate.id == conn.id + conn.is_active = True + db.session.commit() + return conn + @classmethod def delete(cls, conn_id): """Delete a connection. If it was its kind's default, promote the oldest diff --git a/backend/tests/test_notification_chat_connections.py b/backend/tests/test_notification_chat_connections.py index c50db373..686377b9 100644 --- a/backend/tests/test_notification_chat_connections.py +++ b/backend/tests/test_notification_chat_connections.py @@ -171,6 +171,31 @@ def test_update_validation_failure_does_not_mutate_connection(self, app): assert conn.is_active is True +class TestDefaults: + def test_set_default_is_scoped_to_kind_and_activates_selection(self, app): + first_discord = ChatWebhookService.add({ + 'kind': 'discord', 'name': 'Primary', 'url': 'https://discord/primary', + }) + second_discord = ChatWebhookService.add({ + 'kind': 'discord', 'name': 'Secondary', 'url': 'https://discord/secondary', + 'is_active': False, + }) + slack = ChatWebhookService.add({ + 'kind': 'slack', 'name': 'Slack', 'url': 'https://slack/default', + }) + + selected = ChatWebhookService.set_default(second_discord.id) + + assert selected.id == second_discord.id + assert selected.is_default is True + assert selected.is_active is True + assert first_discord.is_default is False + assert slack.is_default is True + + def test_set_default_returns_none_for_unknown_id(self, app): + assert ChatWebhookService.set_default(999999) is None + + class TestCategoryRouting: def test_catch_all_matches_every_category(self, app): conn = ChatWebhookService.add({'kind': 'webhook', 'name': 'All', 'url': 'https://x/all'}) @@ -465,3 +490,36 @@ def test_test_connection_returns_404_for_unknown_id(self, app, client, auth_head assert resp.status_code == 404 assert resp.get_json() == {'error': 'Connection not found'} + + def test_set_default_connection(self, app, client, auth_headers): + first = ChatWebhookService.add({ + 'kind': 'discord', 'name': 'First', 'url': 'https://discord/first', + }) + second = ChatWebhookService.add({ + 'kind': 'discord', 'name': 'Second', 'url': 'https://discord/second', + 'is_active': False, + }) + + resp = client.post( + f'/api/v1/notifications/admin/chat-connections/{second.id}/default', + headers=auth_headers, + ) + + assert resp.status_code == 200 + body = resp.get_json() + assert body['success'] is True + assert body['connection']['id'] == second.id + assert body['connection']['is_default'] is True + assert body['connection']['is_active'] is True + db.session.refresh(first) + assert first.is_default is False + + def test_set_default_connection_returns_404_for_unknown_id(self, app, client, + auth_headers): + resp = client.post( + '/api/v1/notifications/admin/chat-connections/999999/default', + headers=auth_headers, + ) + + assert resp.status_code == 404 + assert resp.get_json() == {'error': 'Connection not found'} From a77d7eb9d40bfd296d540d8ab82116ac20cdee99 Mon Sep 17 00:00:00 2001 From: Marcos Paulo Diniz Date: Mon, 10 Aug 2026 17:17:59 -0300 Subject: [PATCH 5/6] fix(notifications): hide connection test errors Transport exceptions may contain webhook URLs or bot tokens. Return a stable public error and reject non-object update payloads. --- backend/app/api/notifications.py | 5 +- backend/app/services/chat_webhook_service.py | 14 +++--- .../test_notification_chat_connections.py | 48 +++++++++++++++++-- 3 files changed, 54 insertions(+), 13 deletions(-) diff --git a/backend/app/api/notifications.py b/backend/app/api/notifications.py index 388398ff..ac9c4b3f 100644 --- a/backend/app/api/notifications.py +++ b/backend/app/api/notifications.py @@ -429,8 +429,11 @@ def add_chat_connection(): def update_chat_connection(conn_id): """Update mutable chat connection metadata and credentials.""" from app.services.chat_webhook_service import ChatWebhookService + data = request.get_json() + if data is None: + data = {} try: - conn = ChatWebhookService.update(conn_id, request.get_json() or {}) + conn = ChatWebhookService.update(conn_id, data) except ValueError as exc: return jsonify({'error': str(exc)}), 400 if conn is None: diff --git a/backend/app/services/chat_webhook_service.py b/backend/app/services/chat_webhook_service.py index 05ae743d..b1742167 100644 --- a/backend/app/services/chat_webhook_service.py +++ b/backend/app/services/chat_webhook_service.py @@ -286,10 +286,8 @@ def test(cls, conn_id): delivery_result = cls._deliver_chat( conn, credentials, notification) success = delivery_result.ok - error = delivery_result.error - except Exception as exc: # pragma: no cover - defensive formatter guard + except Exception: # pragma: no cover - defensive formatter guard success = False - error = str(exc) conn.last_tested_at = datetime.utcnow() conn.last_test_ok = success @@ -297,10 +295,12 @@ def test(cls, conn_id): if success: return {'success': True, 'message': 'Test notification sent'} - return { - 'success': False, - 'error': str(error or 'test notification failed')[:300], - } + logger.warning( + 'Chat connection test failed (id=%s, kind=%s)', + conn.id, + conn.kind, + ) + return {'success': False, 'error': 'Test notification failed'} # ------------------------------------------------------------------ # Delivery (called by the chat channel adapter for ``conn:`` targets) diff --git a/backend/tests/test_notification_chat_connections.py b/backend/tests/test_notification_chat_connections.py index 686377b9..1b57dca4 100644 --- a/backend/tests/test_notification_chat_connections.py +++ b/backend/tests/test_notification_chat_connections.py @@ -325,11 +325,13 @@ class _Resp: result = ChatWebhookService.test(conn.id) - assert result == {'success': False, 'error': 'webhook returned 503'} + assert result == { + 'success': False, 'error': 'Test notification failed', + } assert conn.last_tested_at is not None assert conn.last_test_ok is False - def test_connection_test_bounds_transport_errors(self, app, monkeypatch): + def test_connection_test_hides_transport_errors(self, app, monkeypatch): def fail_post(*args, **kwargs): raise RuntimeError('x' * 500) @@ -340,10 +342,30 @@ def fail_post(*args, **kwargs): result = ChatWebhookService.test(conn.id) - assert result['success'] is False - assert result['error'] == 'x' * 300 + assert result == { + 'success': False, 'error': 'Test notification failed', + } assert conn.last_test_ok is False + def test_connection_test_does_not_expose_transport_secrets(self, app, monkeypatch): + def fail_post(url, *args, **kwargs): + raise RuntimeError(f'failed POST {url}') + + monkeypatch.setattr( + 'app.services.chat_webhook_service.requests.post', fail_post, + ) + secret_url = 'https://hooks.example/super-secret-token' + conn = ChatWebhookService.add({ + 'kind': 'webhook', 'name': 'Ops', 'url': secret_url, + }) + + result = ChatWebhookService.test(conn.id) + + assert result == { + 'success': False, 'error': 'Test notification failed', + } + assert secret_url not in json.dumps(result) + def test_connection_test_returns_none_for_unknown_id(self, app): assert ChatWebhookService.test(999999) is None @@ -429,6 +451,22 @@ def test_update_connection_returns_400_for_kind_change(self, app, client, auth_h assert resp.status_code == 400 assert 'kind cannot be changed' in resp.get_json()['error'] + @pytest.mark.parametrize('payload', [[], False]) + def test_update_connection_rejects_falsy_non_object_json(self, app, client, + auth_headers, payload): + conn = ChatWebhookService.add({ + 'kind': 'discord', 'name': 'Ops', 'url': 'https://discord/hook', + }) + + resp = client.put( + f'/api/v1/notifications/admin/chat-connections/{conn.id}', + json=payload, + headers=auth_headers, + ) + + assert resp.status_code == 400 + assert resp.get_json() == {'error': 'request body must be an object'} + def test_test_connection_returns_200_on_delivery(self, app, client, auth_headers, monkeypatch): class _Resp: @@ -479,7 +517,7 @@ class _Resp: assert resp.status_code == 400 assert resp.get_json() == { - 'success': False, 'error': 'webhook returned 503', + 'success': False, 'error': 'Test notification failed', } def test_test_connection_returns_404_for_unknown_id(self, app, client, auth_headers): From e414cf9a7bda3cb03ce91c1d5305f1add2858109 Mon Sep 17 00:00:00 2001 From: Marcos Paulo Diniz Date: Mon, 10 Aug 2026 17:19:57 -0300 Subject: [PATCH 6/6] fix(notifications): reject null updates --- backend/app/api/notifications.py | 4 ++-- backend/tests/test_notification_chat_connections.py | 5 +++-- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/backend/app/api/notifications.py b/backend/app/api/notifications.py index ac9c4b3f..f574c91f 100644 --- a/backend/app/api/notifications.py +++ b/backend/app/api/notifications.py @@ -429,8 +429,8 @@ def add_chat_connection(): def update_chat_connection(conn_id): """Update mutable chat connection metadata and credentials.""" from app.services.chat_webhook_service import ChatWebhookService - data = request.get_json() - if data is None: + data = request.get_json(silent=True) + if data is None and not request.is_json: data = {} try: conn = ChatWebhookService.update(conn_id, data) diff --git a/backend/tests/test_notification_chat_connections.py b/backend/tests/test_notification_chat_connections.py index 1b57dca4..13140686 100644 --- a/backend/tests/test_notification_chat_connections.py +++ b/backend/tests/test_notification_chat_connections.py @@ -451,7 +451,7 @@ def test_update_connection_returns_400_for_kind_change(self, app, client, auth_h assert resp.status_code == 400 assert 'kind cannot be changed' in resp.get_json()['error'] - @pytest.mark.parametrize('payload', [[], False]) + @pytest.mark.parametrize('payload', [[], False, None]) def test_update_connection_rejects_falsy_non_object_json(self, app, client, auth_headers, payload): conn = ChatWebhookService.add({ @@ -460,7 +460,8 @@ def test_update_connection_rejects_falsy_non_object_json(self, app, client, resp = client.put( f'/api/v1/notifications/admin/chat-connections/{conn.id}', - json=payload, + data=json.dumps(payload), + content_type='application/json', headers=auth_headers, )