mirror of
https://github.com/dgtlmoon/changedetection.io.git
synced 2026-11-03 04:26:59 +00:00
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 <noreply@anthropic.com>
---------
Co-authored-by: dgtlmoon <dgtlmoon@gmail.com>
Co-authored-by: dgtlmoon <leigh@morresi.net>
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
11 files changed
+129
-52
No files matched your search
@@ -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
|
||||
|
||||
@@ -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'],
|
||||
|
||||
+36
-32
@@ -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)
|
||||
|
||||
@@ -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',
|
||||
]
|
||||
@@ -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)
|
||||
|
||||
@@ -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)
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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.
|
||||
|
||||
@@ -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()
|
||||
@@ -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
|
||||
|
||||
@@ -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:
|
||||
|
||||
Reference in new issue
Block a user