Notifications - Fix 'str' object is not callable when {{ diff(...) }} callable tokens are used with HTML/htmlcolor output

changedetectionio/notification/handler.py

@@ -364,6 +364,10 @@ def process_notification(n_object: NotificationContextData, datastore):
         # Should always be false for 'text' mode or its too hard to read
         # But otherwise, this could be some setting
         word_diff=False if requested_output_format_original == 'text' else True,
+        # HTML-format notifications must escape diff content (GHSA-q8xq-qg4x-wphg).
+        # FormattableDiff/Extract escape internally so {{ diff(...) }} stays callable —
+        # the post-Jinja escape loop below would otherwise convert them to plain str.
+        escape_output='html' in requested_output_format,
         )
     )
 
@@ -394,10 +398,19 @@ def process_notification(n_object: NotificationContextData, datastore):
     # so they survive escape and are still replaced with <span> tags later.
     if 'html' in requested_output_format:
         from markupsafe import escape as html_escape
+        from changedetectionio.notification_service import FormattableDiff, FormattableExtract
         _page_content_keys = {'raw_diff', 'current_snapshot', 'prev_snapshot', 'triggered_text'}
         for key in [k for k in notification_parameters if k.startswith('diff') or k in _page_content_keys]:
-            if notification_parameters.get(key):
-                notification_parameters[key] = str(html_escape(str(notification_parameters[key])))
+            value = notification_parameters.get(key)
+            if not value:
+                continue
+            # FormattableDiff / FormattableExtract are callable str subclasses — {{ diff(lines=5) }}
+            # etc. relies on __call__. Wrapping them with str(html_escape(...)) here would lose
+            # __call__ and break those tokens. They escape internally via escape_output=True
+            # (set by add_rendered_diff_to_notification_vars above) for both __str__ and __call__.
+            if isinstance(value, (FormattableDiff, FormattableExtract)):
+                continue
+            notification_parameters[key] = str(html_escape(str(value)))
 
     with (apprise.LogCapture(level=apprise.logging.DEBUG) as logs):
         for url in n_object['notification_urls']:

changedetectionio/notification_service.py

@@ -99,7 +99,7 @@ class FormattableExtract(str):
     Multiple changed fragments are joined with newlines.
     Being a str subclass means it is natively JSON serializable.
     """
-    def __new__(cls, prev_snapshot, current_snapshot, extract_fn):
+    def __new__(cls, prev_snapshot, current_snapshot, extract_fn, escape_output=False):
         if prev_snapshot or current_snapshot:
             from changedetectionio import diff as diff_module
             # word_diff=True is required — placemarker extraction regexes only exist in word-diff output
@@ -107,6 +107,12 @@ class FormattableExtract(str):
             extracted = extract_fn(raw)
         else:
             extracted = ''
+        if escape_output and extracted:
+            # Placemarkers (@removed_PLACEMARKER_OPEN etc) contain no HTML chars,
+            # so html_escape leaves them intact — they still get swapped to <span>
+            # tags later by apply_service_tweaks. See GHSA-q8xq-qg4x-wphg.
+            from markupsafe import escape as html_escape
+            extracted = str(html_escape(extracted))
         instance = super().__new__(cls, extracted)
         return instance
 
@@ -128,16 +134,23 @@ class FormattableDiff(str):
 
     Being a str subclass means it is natively JSON serializable.
     """
-    def __new__(cls, prev_snapshot, current_snapshot, **base_kwargs):
+    def __new__(cls, prev_snapshot, current_snapshot, escape_output=False, **base_kwargs):
         if prev_snapshot or current_snapshot:
             from changedetectionio import diff as diff_module
             rendered = diff_module.render_diff(prev_snapshot, current_snapshot, **base_kwargs)
         else:
             rendered = ''
+        if escape_output and rendered:
+            # Placemarkers (@removed_PLACEMARKER_OPEN etc) contain no HTML chars,
+            # so html_escape leaves them intact — they still get swapped to <span>
+            # tags later by apply_service_tweaks. See GHSA-q8xq-qg4x-wphg.
+            from markupsafe import escape as html_escape
+            rendered = str(html_escape(rendered))
         instance = super().__new__(cls, rendered)
         instance._prev = prev_snapshot
         instance._current = current_snapshot
         instance._base_kwargs = base_kwargs
+        instance._escape_output = escape_output
         return instance
 
     def __call__(self, lines=None, added_only=False, removed_only=False, context=0,
@@ -163,6 +176,10 @@ class FormattableDiff(str):
         if lines is not None:
             result = '\n'.join(result.splitlines()[:int(lines)])
 
+        if self._escape_output and result:
+            from markupsafe import escape as html_escape
+            result = str(html_escape(result))
+
         return result
 
 
@@ -236,7 +253,7 @@ class NotificationContextData(dict):
 
         super().__setitem__(key, value)
 
-def add_rendered_diff_to_notification_vars(notification_scan_text:str, prev_snapshot:str, current_snapshot:str, word_diff:bool):
+def add_rendered_diff_to_notification_vars(notification_scan_text:str, prev_snapshot:str, current_snapshot:str, word_diff:bool, escape_output:bool=False):
     """
     Efficiently renders only the diff placeholders that are actually used in the notification text.
 
@@ -249,6 +266,9 @@ def add_rendered_diff_to_notification_vars(notification_scan_text:str, prev_snap
         prev_snapshot: Previous version of content for diff comparison
         current_snapshot: Current version of content for diff comparison
         word_diff: Whether to use word-level (True) or line-level (False) diffing
+        escape_output: If True, the rendered diff output is HTML-escaped. Used for HTML-format
+            notifications so attacker-controlled page content can't inject live markup.
+            Both the cached str representation and the result of {{ diff(...) }} calls are escaped.
 
     Returns:
         dict: Only the diff placeholders that were found in notification_scan_text, with rendered content
@@ -287,10 +307,10 @@ def add_rendered_diff_to_notification_vars(notification_scan_text:str, prev_snap
         if not re.search(pattern, notification_scan_text, re.IGNORECASE):
             continue
         if key in diff_specs:
-            ret[key] = FormattableDiff(prev_snapshot, current_snapshot, **diff_specs[key])
+            ret[key] = FormattableDiff(prev_snapshot, current_snapshot, escape_output=escape_output, **diff_specs[key])
             rendered_count += 1
         elif key in extract_specs:
-            ret[key] = FormattableExtract(prev_snapshot, current_snapshot, extract_fn=extract_specs[key])
+            ret[key] = FormattableExtract(prev_snapshot, current_snapshot, extract_fn=extract_specs[key], escape_output=escape_output)
             rendered_count += 1
 
     if rendered_count:

changedetectionio/tests/test_notification.py

@@ -634,6 +634,12 @@ def _test_color_notifications(client, notification_body_token, datastore_path):
 def test_html_color_notifications(client, live_server, measure_memory_usage, datastore_path):
     _test_color_notifications(client, '{{diff}}',datastore_path=datastore_path)
     _test_color_notifications(client, '{{diff_full}}',datastore_path=datastore_path)
+    # Regression: the html-output escape pass in handler.py used to convert
+    # FormattableDiff into a plain str, stripping its __call__ and breaking any
+    # {{ diff(...) }} / {{ diff_added(...) }} token on htmlcolor/html notifications
+    # with 'str' object is not callable (see commit 08d30c6 + #3923).
+    # word_diff=false reproduces the exact form the user-reported failure used.
+    _test_color_notifications(client, '{{diff(word_diff=false)}}', datastore_path=datastore_path)
 
 
 def _test_custom_html_in_notification_body_not_escaped(client, datastore_path, content_type=None):