From f20c82fb155a0e1a8f862e5f2ae2f08b5bed07d1 Mon Sep 17 00:00:00 2001 From: skkzsh Date: Fri, 23 Oct 2026 22:32:32 +0900 Subject: [PATCH] Fix Jinja2 false-positive when URL contains `{%` (#4208) * Add tests for Jinja2 template detection in URL validation (RED) * Fix Jinja2 false-positive when URL contains `{%` (GREEN) * Consolidate Jinja2 template detection across URL/body/headers validation * Propagate `FORCE_COLOR` to test containers * Apply suggestion from @dgtlmoon * Apply suggestion from @dgtlmoon * Only render request headers/body at fetch time when they contain Jinja2, drop FORCE_COLOR changes The edit form now skips Jinja2 validation for text like {%22 that is not a template, but processors/base.py still rendered every header and the body unconditionally, so a body such as q={%22key%22:1} was accepted on save and then failed every check with a TemplateSyntaxError. Use JINJA2_MARKER_PATTERN there too, so it is sent as-is. Also reset the CI workflow and run_*_tests.sh scripts to master, the FORCE_COLOR changes are unrelated to this fix. Co-Authored-By: Claude Opus 5.5 --------- Co-authored-by: dgtlmoon Co-authored-by: dgtlmoon Co-authored-by: Claude Opus 5.5 --- .../browser_steps/browser_steps.py | 6 +- changedetectionio/content_fetchers/base.py | 6 +- changedetectionio/forms.py | 68 ++++++++++--------- changedetectionio/jinja2_custom/__init__.py | 5 ++ changedetectionio/model/Watch.py | 4 +- changedetectionio/processors/base.py | 12 ++-- changedetectionio/tests/test_request.py | 33 +++++++++ changedetectionio/tests/test_security.py | 10 +++ .../tests/unit/test_jinja2_security.py | 22 ++++++ .../tests/visualselector/test_fetch_data.py | 4 +- changedetectionio/validate_url.py | 11 ++- 11 files changed, 129 insertions(+), 52 deletions(-) diff --git a/changedetectionio/browser_steps/browser_steps.py b/changedetectionio/browser_steps/browser_steps.py index 15c925786..7595b5183 100644 --- a/changedetectionio/browser_steps/browser_steps.py +++ b/changedetectionio/browser_steps/browser_steps.py @@ -6,7 +6,7 @@ from loguru import logger from changedetectionio.content_fetchers import SCREENSHOT_MAX_HEIGHT_DEFAULT from changedetectionio.content_fetchers.base import get_playwright_bypass_csp, manage_user_agent -from changedetectionio.jinja2_custom import render as jinja_render +from changedetectionio.jinja2_custom import JINJA2_MARKER_PATTERN, render as jinja_render from changedetectionio.validate_url import validate_fetch_url_async def track_latest_navigation_response(page): @@ -129,10 +129,10 @@ class steppable_browser_interface(): action_handler = getattr(self, "action_" + call_action_name) # Support for Jinja2 variables in the value and selector - if selector and ('{%' in selector or '{{' in selector): + if selector and JINJA2_MARKER_PATTERN.search(selector): selector = jinja_render(template_str=selector) - if optional_value and ('{%' in optional_value or '{{' in optional_value): + if optional_value and JINJA2_MARKER_PATTERN.search(optional_value): optional_value = jinja_render(template_str=optional_value) # Trigger click and cautiously handle potential navigation diff --git a/changedetectionio/content_fetchers/base.py b/changedetectionio/content_fetchers/base.py index 99500546b..acf3be632 100644 --- a/changedetectionio/content_fetchers/base.py +++ b/changedetectionio/content_fetchers/base.py @@ -211,7 +211,7 @@ class Fetcher(): async def iterate_browser_steps(self, start_url=None): from changedetectionio.browser_steps.browser_steps import steppable_browser_interface, browser_steps_get_valid_steps from playwright._impl._errors import TimeoutError, Error - from changedetectionio.jinja2_custom import render as jinja_render + from changedetectionio.jinja2_custom import JINJA2_MARKER_PATTERN, render as jinja_render step_n = 0 if self.browser_steps: @@ -229,9 +229,9 @@ class Fetcher(): optional_value = step['optional_value'] selector = step['selector'] # Support for jinja2 template in step values, with date module added - if '{%' in step['optional_value'] or '{{' in step['optional_value']: + if JINJA2_MARKER_PATTERN.search(step['optional_value']): optional_value = jinja_render(template_str=step['optional_value']) - if '{%' in step['selector'] or '{{' in step['selector']: + if JINJA2_MARKER_PATTERN.search(step['selector']): selector = jinja_render(template_str=step['selector']) await getattr(interface, "call_action")(action_name=step['operation'], diff --git a/changedetectionio/forms.py b/changedetectionio/forms.py index 6a50c0c7f..05a5a6041 100644 --- a/changedetectionio/forms.py +++ b/changedetectionio/forms.py @@ -1036,7 +1036,7 @@ class processor_text_json_diff_form(commonSettingsForm, commonNotificationSettin if not super().validate(): return False - from changedetectionio.jinja2_custom import render as jinja_render + from changedetectionio.jinja2_custom import JINJA2_MARKER_PATTERN, render as jinja_render result = True # Fail form validation when a body is set for a GET @@ -1045,50 +1045,54 @@ class processor_text_json_diff_form(commonSettingsForm, commonNotificationSettin result = False # Attempt to validate jinja2 templates in the URL - try: - jinja_render(template_str=self.url.data) - except ModuleNotFoundError as e: - # incase jinja2_time or others is missing - logger.error(e) - self.url.errors.append(gettext('Invalid template syntax configuration: %(error)s') % {'error': e}) - result = False - except Exception as e: - logger.error(e) - self.url.errors.append(gettext('Invalid template syntax: %(error)s') % {'error': e}) - result = False - - # Attempt to validate jinja2 templates in the optional "Link to Open" - if self.link_to_open.data and self.link_to_open.data.strip(): + if self.url.data and JINJA2_MARKER_PATTERN.search(self.url.data): try: - jinja_render(template_str=self.link_to_open.data) - except ModuleNotFoundError as e: - logger.error(e) - self.link_to_open.errors.append(gettext('Invalid template syntax configuration: %(error)s') % {'error': e}) - result = False - except Exception as e: - logger.error(e) - self.link_to_open.errors.append(gettext('Invalid template syntax: %(error)s') % {'error': e}) - result = False - - # Attempt to validate jinja2 templates in the body - if self.body.data and self.body.data.strip(): - try: - jinja_render(template_str=self.body.data) + jinja_render(template_str=self.url.data) except ModuleNotFoundError as e: # incase jinja2_time or others is missing logger.error(e) - self.body.errors.append(gettext('Invalid template syntax configuration: %(error)s') % {'error': e}) + self.url.errors.append(gettext('Invalid template syntax configuration: %(error)s') % {'error': e}) result = False except Exception as e: logger.error(e) - self.body.errors.append(gettext('Invalid template syntax: %(error)s') % {'error': e}) + self.url.errors.append(gettext('Invalid template syntax: %(error)s') % {'error': e}) result = False + # Attempt to validate jinja2 templates in the optional "Link to Open" + if self.link_to_open.data and self.link_to_open.data.strip(): + if JINJA2_MARKER_PATTERN.search(self.link_to_open.data): + try: + jinja_render(template_str=self.link_to_open.data) + except ModuleNotFoundError as e: + logger.error(e) + self.link_to_open.errors.append(gettext('Invalid template syntax configuration: %(error)s') % {'error': e}) + result = False + except Exception as e: + logger.error(e) + self.link_to_open.errors.append(gettext('Invalid template syntax: %(error)s') % {'error': e}) + result = False + + # Attempt to validate jinja2 templates in the body + if self.body.data and self.body.data.strip(): + if JINJA2_MARKER_PATTERN.search(self.body.data): + try: + jinja_render(template_str=self.body.data) + except ModuleNotFoundError as e: + # incase jinja2_time or others is missing + logger.error(e) + self.body.errors.append(gettext('Invalid template syntax configuration: %(error)s') % {'error': e}) + result = False + except Exception as e: + logger.error(e) + self.body.errors.append(gettext('Invalid template syntax: %(error)s') % {'error': e}) + result = False + # Attempt to validate jinja2 templates in the headers if len(self.headers.data) > 0: try: for header, value in self.headers.data.items(): - jinja_render(template_str=value) + if JINJA2_MARKER_PATTERN.search(value): + jinja_render(template_str=value) except ModuleNotFoundError as e: # incase jinja2_time or others is missing logger.error(e) diff --git a/changedetectionio/jinja2_custom/__init__.py b/changedetectionio/jinja2_custom/__init__.py index 39e0fe0f0..cc7ba830e 100644 --- a/changedetectionio/jinja2_custom/__init__.py +++ b/changedetectionio/jinja2_custom/__init__.py @@ -1,6 +1,8 @@ """ Jinja2 custom extensions and safe rendering utilities. """ +import re + from .extensions.TimeExtension import TimeExtension from .safe_jinja import ( render, @@ -12,6 +14,8 @@ from .safe_jinja import ( from .plugins.regex import regex_replace from .plugins.datetime_fmt import unixtime +JINJA2_MARKER_PATTERN = re.compile(r'{%[-+]?\s*[a-zA-Z_]|{{') + __all__ = [ 'TimeExtension', 'render', @@ -20,5 +24,6 @@ __all__ = [ 'JINJA2_MAX_RETURN_PAYLOAD_SIZE', 'DEFAULT_JINJA2_EXTENSIONS', 'regex_replace', + 'JINJA2_MARKER_PATTERN', 'unixtime', ] diff --git a/changedetectionio/model/Watch.py b/changedetectionio/model/Watch.py index 29be66bfa..e9dd16959 100644 --- a/changedetectionio/model/Watch.py +++ b/changedetectionio/model/Watch.py @@ -29,7 +29,7 @@ from blinker import signal from changedetectionio.validate_url import is_safe_valid_url from changedetectionio.strtobool import strtobool -from changedetectionio.jinja2_custom import render as jinja_render +from changedetectionio.jinja2_custom import JINJA2_MARKER_PATTERN, render as jinja_render from changedetectionio import gc_debounce from . import watch_base from .persistence import EntityPersistenceMixin @@ -295,7 +295,7 @@ class model(EntityPersistenceMixin, watch_base): return 'DISABLED' ready_url = url - if '{%' in url or '{{' in url: + if JINJA2_MARKER_PATTERN.search(url): # Jinja2 available in URLs along with https://pypi.org/project/jinja2-time/ try: ready_url = jinja_render(template_str=url) diff --git a/changedetectionio/processors/base.py b/changedetectionio/processors/base.py index 378371466..aa9f207e5 100644 --- a/changedetectionio/processors/base.py +++ b/changedetectionio/processors/base.py @@ -220,7 +220,7 @@ class difference_detection_processor(): self.fetcher.browser_steps_screenshot_path = os.path.join(self.datastore.datastore_path, self.watch.get('uuid')) # Tweak the base config with the per-watch ones - from changedetectionio.jinja2_custom import render as jinja_render + from changedetectionio.jinja2_custom import JINJA2_MARKER_PATTERN, render as jinja_render request_headers = CaseInsensitiveDict() ua = self.datastore.data['settings']['requests'].get('default_ua') @@ -237,14 +237,18 @@ class difference_detection_processor(): if 'Accept-Encoding' in request_headers and "br" in request_headers['Accept-Encoding']: request_headers['Accept-Encoding'] = request_headers['Accept-Encoding'].replace(', br', '') + # Only render values that really contain Jinja2, so a literal like {%22 (URL-encoded JSON) is sent as-is, + # the same rule the edit form validates with for header_name in request_headers: - request_headers.update({header_name: jinja_render(template_str=request_headers.get(header_name))}) + value = request_headers.get(header_name) + if isinstance(value, str) and JINJA2_MARKER_PATTERN.search(value): + request_headers.update({header_name: jinja_render(template_str=value)}) timeout = self.datastore.data['settings']['requests'].get('timeout') request_body = self.watch.get('body') - if request_body: - request_body = jinja_render(template_str=self.watch.get('body')) + if request_body and JINJA2_MARKER_PATTERN.search(request_body): + request_body = jinja_render(template_str=request_body) request_method = self.watch.get('method') ignore_status_codes = self.watch.get('ignore_status_codes', False) diff --git a/changedetectionio/tests/test_request.py b/changedetectionio/tests/test_request.py index 189f05477..0d1ecd616 100644 --- a/changedetectionio/tests/test_request.py +++ b/changedetectionio/tests/test_request.py @@ -171,6 +171,39 @@ def test_body_in_request(client, live_server, measure_memory_usage, datastore_pa assert b"Body must be empty when Request Method is set to GET" in res.data delete_all_watches(client) + +def test_body_with_literal_jinja_like_text_is_sent_as_is(client, live_server, measure_memory_usage, datastore_path): + """#4452 - '{%22' (URL-encoded JSON) is not Jinja2. The form accepts it, so the fetch must not try to render it.""" + test_url = url_for('test_body', _external=True) + if os.getenv('PLAYWRIGHT_DRIVER_URL'): + test_url = test_url.replace('localhost', 'cdio') + + uuid = client.application.config.get('DATASTORE').add_watch(url=test_url) + wait_for_all_checks(client) + + body_value = 'q={%22key%22:1}' + res = client.post( + url_for("ui.ui_edit.edit_page", uuid=uuid), + data={ + "url": test_url, + "tags": "", + "method": "POST", + "fetch_backend": "html_requests", + "body": body_value, + "time_between_check_use_default": "y"}, + follow_redirects=True + ) + assert b"Updated watch." in res.data + wait_for_all_checks(client) + + watch = client.application.config.get('DATASTORE').data['watching'][uuid] + assert not watch.get('last_error'), watch.get('last_error') + + # The service echoes the body back, unrendered + res = client.get(url_for("ui.ui_preview.preview_page", uuid=uuid), follow_redirects=True) + assert str.encode(body_value) in res.data + delete_all_watches(client) + def test_method_in_request(client, live_server, measure_memory_usage, datastore_path): import os # Add our URL to the import page diff --git a/changedetectionio/tests/test_security.py b/changedetectionio/tests/test_security.py index 79036c086..2f018a5ff 100644 --- a/changedetectionio/tests/test_security.py +++ b/changedetectionio/tests/test_security.py @@ -829,6 +829,16 @@ def test_unresolvable_hostname_is_allowed(client, live_server, monkeypatch): "Unresolvable hostname watch should appear in the watch overview list" +@pytest.mark.parametrize("url", [ + "https://example.com/{{watch_url}}", + "https://example.com/{% if foo %}bar{% endif %}", + "https://example.com/api?q={%22key%22:1}", +]) +def test_is_safe_valid_url_accepts_valid_urls(url): + from changedetectionio.validate_url import is_safe_valid_url + assert is_safe_valid_url(url) + + def test_ghsa_rph4_96w6_q594_urlparse_urllib3_parser_differential_ssrf(client, live_server, monkeypatch, measure_memory_usage, datastore_path): """ GHSA-rph4-96w6-q594: SSRF via urlparse/urllib3 parser differential. diff --git a/changedetectionio/tests/unit/test_jinja2_security.py b/changedetectionio/tests/unit/test_jinja2_security.py index 6a56604db..ecc320809 100644 --- a/changedetectionio/tests/unit/test_jinja2_security.py +++ b/changedetectionio/tests/unit/test_jinja2_security.py @@ -4,6 +4,7 @@ # python3 -m unittest changedetectionio.tests.unit.test_jinja2_security import unittest +import pytest from changedetectionio import jinja2_custom as safe_jinja @@ -56,6 +57,27 @@ class TestJinja2SSTI(unittest.TestCase): self.assertEqual(x, "woo <a href="https://google.com">dfdfd</a>") +@pytest.mark.parametrize("text", [ + 'https://example.com/{{watch_url}}', + 'https://example.com/{% if foo %}bar{% endif %}', + "https://example.com/{% now 'Europe/Berlin', '%Y' %}", + '{%- if foo -%}', + '{%+ if foo +%}', +]) +def test_jinja2_marker_pattern_true_positives(text): + from changedetectionio.jinja2_custom import JINJA2_MARKER_PATTERN + assert JINJA2_MARKER_PATTERN.search(text) + + +@pytest.mark.parametrize("text", [ + '', + 'https://example.com/api?q={%22key%22:1}', +]) +def test_jinja2_marker_pattern_false_positives(text): + from changedetectionio.jinja2_custom import JINJA2_MARKER_PATTERN + assert not JINJA2_MARKER_PATTERN.search(text) + + if __name__ == '__main__': unittest.main() diff --git a/changedetectionio/tests/visualselector/test_fetch_data.py b/changedetectionio/tests/visualselector/test_fetch_data.py index 939889b98..a0a2848ab 100644 --- a/changedetectionio/tests/visualselector/test_fetch_data.py +++ b/changedetectionio/tests/visualselector/test_fetch_data.py @@ -112,7 +112,7 @@ def test_basic_browserstep(client, live_server, measure_memory_usage, datastore_ # Should get set to the actual text (jinja2 rendered) 'browser_steps-5-optional_value': "Hello-Jinja2-{% now 'Europe/Berlin', '%Y-%m-%d' %}", 'browser_steps-8-operation': 'Click element', - 'browser_steps-8-selector': 'button[name=test-button]', + 'browser_steps-8-selector': "{% if true %}button[name=test-button]{% endif %}", 'browser_steps-8-optional_value': '', # For now, cookies doesnt work in headers because it must be a full cookiejar object 'headers': "testheader: yes\buser-agent: MyCustomAgent", @@ -128,7 +128,7 @@ def test_basic_browserstep(client, live_server, measure_memory_usage, datastore_ # 3874 - should have tidied up any blanks watch = live_server.app.config['DATASTORE'].data['watching'][uuid] assert watch['browser_steps'][0].get('operation') == 'Enter text in field' - assert watch['browser_steps'][1].get('selector') == 'button[name=test-button]' + assert watch['browser_steps'][1].get('selector') == "{% if true %}button[name=test-button]{% endif %}" # This part actually needs the browser, before this we are just testing data diff --git a/changedetectionio/validate_url.py b/changedetectionio/validate_url.py index a4c6a725c..c4444e57c 100644 --- a/changedetectionio/validate_url.py +++ b/changedetectionio/validate_url.py @@ -1,6 +1,6 @@ import ipaddress +import re import socket -from functools import lru_cache from loguru import logger from urllib.parse import urlparse, urlunparse, parse_qsl, urlencode, quote, unquote @@ -260,7 +260,7 @@ def is_fetch_url_allowed(url): import os import re from changedetectionio.strtobool import strtobool - from changedetectionio.jinja2_custom import render as jinja_render + from changedetectionio.jinja2_custom import JINJA2_MARKER_PATTERN, render as jinja_render if not url or not isinstance(url, str) or not url.strip(): return False, "No URL specified." @@ -268,7 +268,7 @@ def is_fetch_url_allowed(url): url = url.strip() # Jinja2 first — the fetch uses the rendered value, so the rendered value is what must pass. - if '{%' in url or '{{' in url: + if JINJA2_MARKER_PATTERN.search(url): try: url = jinja_render(template_str=url).strip() except Exception as e: @@ -365,9 +365,8 @@ def is_llm_api_base_safe(api_base): def is_safe_valid_url(test_url): from changedetectionio import strtobool - from changedetectionio.jinja2_custom import render as jinja_render + from changedetectionio.jinja2_custom import JINJA2_MARKER_PATTERN, render as jinja_render import os - import re import validators # Validate input type first - must be a non-empty string @@ -410,7 +409,7 @@ def is_safe_valid_url(test_url): # Check the actual rendered URL in case of any Jinja markup # Only run jinja_render when the URL actually contains Jinja2 syntax - creating a new # ImmutableSandboxedEnvironment is expensive and is called once per watch per page load - if '{%' in test_url or '{{' in test_url: + if JINJA2_MARKER_PATTERN.search(test_url): try: test_url = jinja_render(test_url) except Exception as e: