fix(notifications): scope chat credential re-encryption to supplied fields

Follow-up to PR #94. ChatWebhookService.update() decrypted every stored
credential and re-encrypted the lot on any credential-bearing request.
decrypt_secret_safe() returns its input unchanged when a value will not
decrypt, so a wrong-key or not-yet-migrated row got wrapped a second time
and became unrecoverable -- and it happened to fields the request never
named. It now patches the ciphertext dict directly and encrypts only what
it assigns, so untouched credentials survive byte-identical.

Also tolerate an unchanged is_default in a round-tripped body (to_dict()
always emits it, so GET -> edit -> PUT rejected an untouched field), and
record the delivery failure reason in the server log. The reason stays out
of the HTTP response: transport errors embed the full webhook URL and any
Telegram bot token.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

backend/app/services/chat_webhook_service.py

@@ -104,7 +104,7 @@ class ChatWebhookService:
             kind = (data.get('kind') or '').strip().lower()
             if kind != conn.kind:
                 raise ValueError('connection kind cannot be changed')
-        if 'is_default' in data:
+        if 'is_default' in data and bool(data.get('is_default')) != bool(conn.is_default):
             raise ValueError('use the default endpoint to change is_default')
 
         new_name = conn.name
@@ -132,40 +132,41 @@ class ChatWebhookService:
             field in data for field in ('secret', 'chat_id', 'bot_token')
         )
         if credential_supplied:
-            credentials = conn.credentials()
+            # Work on the stored ciphertext so untouched credentials are carried
+            # across byte-identical. Decrypting them first would re-wrap values
+            # that ``decrypt_secret_safe`` handed back unchanged (wrong key, or
+            # a not-yet-migrated plaintext row), corrupting them permanently.
+            credentials = conn.raw_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
+                    credentials['chat_id'] = encrypt_secret(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)
+                        credentials['bot_token'] = encrypt_secret(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
+                    credentials['url'] = encrypt_secret(url)
                 if 'secret' in data:
                     secret = data.get('secret')
                     if secret in (None, ''):
                         credentials.pop('secret', None)
                     else:
-                        credentials['secret'] = str(secret)
+                        credentials['secret'] = encrypt_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()
-            })
+            new_credentials_json = json.dumps(credentials)
 
         conn.name = new_name
         conn.categories_json = new_categories_json
@@ -277,6 +278,7 @@ class ChatWebhookService:
         )
         notification.set_data({'message': message})
 
+        delivery_result = None
         try:
             credentials = conn.credentials()
             if conn.kind == 'webhook':
@@ -287,6 +289,11 @@ class ChatWebhookService:
                     conn, credentials, notification)
             success = delivery_result.ok
         except Exception:  # pragma: no cover - defensive formatter guard
+            logger.exception(
+                'Chat connection test raised (id=%s, kind=%s)',
+                conn.id,
+                conn.kind,
+            )
             success = False
 
         conn.last_tested_at = datetime.utcnow()
@@ -295,10 +302,13 @@ class ChatWebhookService:
 
         if success:
             return {'success': True, 'message': 'Test notification sent'}
+        # The reason stays server-side: transport errors embed the full webhook
+        # URL (secret path segment included) and Telegram bot tokens.
         logger.warning(
-            'Chat connection test failed (id=%s, kind=%s)',
+            'Chat connection test failed (id=%s, kind=%s, reason=%s)',
             conn.id,
             conn.kind,
+            getattr(delivery_result, 'error', None),
         )
         return {'success': False, 'error': 'Test notification failed'}
 

backend/tests/BASELINE_COUNT

@@ -1 +1 @@
-3234
+3262

backend/tests/test_notification_chat_connections.py

@@ -16,7 +16,7 @@ from app.models import User
 from app.models.chat_webhook import ChatWebhookConnection
 from app.models.notification_preferences import NotificationPreferences
 from app.notifications.consumer import process_message
-from app.notifications.models import NotificationDelivery
+from app.notifications.models import Notification, NotificationDelivery
 from app.notifications.service import GROUP_SLUG, QUEUE_SLUG, NotificationBusService
 from app.queue_bus.service import QueueBusService
 from app.services.chat_webhook_service import ChatWebhookService
@@ -114,6 +114,57 @@ class TestCrud:
         assert updated.raw_credentials()['url'] != 'https://hooks.example/new'
         assert updated.raw_credentials()['secret'] != 'new-secret'
 
+    def test_update_carries_untouched_credential_across_untranslated(self, app):
+        """An untouched credential keeps its exact stored ciphertext.
+
+        Fernet is non-deterministic, so re-encrypting an unchanged plaintext
+        would visibly rewrite the column. Rewriting is how a value that failed
+        to decrypt (wrong key, or a legacy plaintext row) gets double-wrapped
+        and lost, so the byte-for-byte check is the guard against that.
+        """
+        conn = ChatWebhookService.add({
+            'kind': 'webhook',
+            'name': 'Ops',
+            'url': 'https://hooks.example/old',
+            'secret': 'keep-me',
+        })
+        secret_before = conn.raw_credentials()['secret']
+
+        updated = ChatWebhookService.update(conn.id, {
+            'url': 'https://hooks.example/new',
+        })
+
+        assert updated.raw_credentials()['secret'] == secret_before
+        assert updated.credentials() == {
+            'url': 'https://hooks.example/new',
+            'secret': 'keep-me',
+        }
+
+    def test_update_accepts_a_round_tripped_unchanged_is_default(self, app):
+        """to_dict() always emits is_default, so GET -> edit -> PUT sends it."""
+        conn = ChatWebhookService.add({
+            'kind': 'webhook',
+            'name': 'Ops',
+            'url': 'https://hooks.example/ops',
+        })
+        body = conn.to_dict()
+        body['name'] = 'Ops renamed'
+
+        updated = ChatWebhookService.update(conn.id, body)
+
+        assert updated.name == 'Ops renamed'
+        assert updated.is_default is True
+
+    def test_update_still_rejects_an_actual_is_default_change(self, app):
+        conn = ChatWebhookService.add({
+            'kind': 'webhook',
+            'name': 'Ops',
+            'url': 'https://hooks.example/ops',
+        })
+
+        with pytest.raises(ValueError, match='default endpoint'):
+            ChatWebhookService.update(conn.id, {'is_default': False})
+
     def test_update_clears_explicitly_empty_optional_credential(self, app):
         conn = ChatWebhookService.add({
             'kind': 'telegram',
@@ -309,6 +360,8 @@ class TestConnectionTesting:
         )
         assert conn.last_tested_at is not None
         assert conn.last_test_ok is True
+        # The test notification is transient: it must never reach the bell feed.
+        assert Notification.query.count() == 0
 
     def test_failed_connection_test_persists_failure(self, app, monkeypatch):
         class _Resp: