This is an automated email from the ASF dual-hosted git repository.
gcruz pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/allura.git
commit 63f12b9a7d754e5bf02a1aa9e4e1f9c17b7aa87e
Author: Dave Brondsema <dbronds...@slashdotmedia.com>
AuthorDate: Thu Feb 15 12:05:00 2024 -0500
[#8536] don't use jinja for site notifications; add autoescape
---
Allura/allura/lib/helpers.py | 24 ++++++++++++++++++----
Allura/allura/lib/widgets/search.py | 1 +
Allura/allura/public/nf/js/allura-base.js | 2 +-
.../templates/jinja_master/theme_macros.html | 2 +-
.../jinja_master/theme_macros.html | 2 +-
Allura/allura/tests/test_helpers.py | 19 +++++++++++++++++
6 files changed, 43 insertions(+), 7 deletions(-)
diff --git a/Allura/allura/lib/helpers.py b/Allura/allura/lib/helpers.py
index bc52a5638..26dd2d94f 100644
--- a/Allura/allura/lib/helpers.py
+++ b/Allura/allura/lib/helpers.py
@@ -790,10 +790,26 @@ def render_any_markup(name, txt, code_mode=False,
linenumbers_style=TABLE):
@pass_context
-def subrender_jinja_filter(context, value):
- _template = context.eval_ctx.environment.from_string(value)
- result = _template.render(**context)
- return result
+def subrender_jinja_filter(context, html_tmpl: str) -> Markup:
+ # jinja templates can execute potentially dangerous things
+ # _template = context.eval_ctx.environment.from_string(html_tmpl)
+ # return _template.render(**context)
+
+ # so instead, support just a few things
+
+ limited_vars = {
+ '{{ c.project.url() }}': lambda: c.project.url(),
+ }
+ for var, fn in limited_vars.items():
+ if var not in html_tmpl:
+ continue
+ try:
+ val = fn()
+ except Exception:
+ log.exception(f'Could not replace {var} in jinja "subrender" for
site notification')
+ continue
+ html_tmpl = html_tmpl.replace(var, val)
+ return Markup(html_tmpl)
def nl2br_jinja_filter(value):
diff --git a/Allura/allura/lib/widgets/search.py
b/Allura/allura/lib/widgets/search.py
index cec39ad44..c772992d1 100644
--- a/Allura/allura/lib/widgets/search.py
+++ b/Allura/allura/lib/widgets/search.py
@@ -53,6 +53,7 @@ class SearchHelp(ffw.Lightbox):
super().__init__()
# can't use g.jinja2_env since this widget gets imported too early :(
jinja2_env = jinja2.Environment(
+ autoescape=True,
loader=jinja2.PackageLoader('allura', 'templates/widgets'))
self.content =
Markup(jinja2_env.get_template('search_help.html').render(dict(
comments=comments,
diff --git a/Allura/allura/public/nf/js/allura-base.js
b/Allura/allura/public/nf/js/allura-base.js
index 779f4ebca..839408dee 100644
--- a/Allura/allura/public/nf/js/allura-base.js
+++ b/Allura/allura/public/nf/js/allura-base.js
@@ -209,7 +209,7 @@ $(function(){
});
$('#site-notification .btn-close').click(function(e) {
- var $note = $(this).parent();
+ var $note = $(this).parents('section:first');
$note.hide();
var note_id = $note.attr('data-notification-id');
var cookie = $.cookie('site-notification');
diff --git a/Allura/allura/templates/jinja_master/theme_macros.html
b/Allura/allura/templates/jinja_master/theme_macros.html
index e06f5d7a2..c9ee789b3 100644
--- a/Allura/allura/templates/jinja_master/theme_macros.html
+++ b/Allura/allura/templates/jinja_master/theme_macros.html
@@ -178,7 +178,7 @@
http://stackoverflow.com/questions/26582731/redefining-imported-jinja-macros
{% if note %}
<div id="site-notification">
<section class="site-message info"
data-notification-id="{{note._id}}">
- {{ note.content|subrender|safe }}
+ {{ note.content|subrender }}
<a href="" class="btn btn-close">Close</a>
</section>
</div>
diff --git a/Allura/allura/templates_responsive/jinja_master/theme_macros.html
b/Allura/allura/templates_responsive/jinja_master/theme_macros.html
index 5c639115d..dd0e70ed1 100644
--- a/Allura/allura/templates_responsive/jinja_master/theme_macros.html
+++ b/Allura/allura/templates_responsive/jinja_master/theme_macros.html
@@ -195,7 +195,7 @@
http://stackoverflow.com/questions/26582731/redefining-imported-jinja-macros
{% if note %}
<div id="site-notification">
<section class="callout primary"
data-notification-id="{{note._id}}">
- {{note.content|safe}}
+ {{ note.content|subrender }}
<button class="close-button btn-close" aria-label="Dismiss
alert" type="button">
{# .btn-close instead of data-close, since allura-base.js
handles closing it, not Foundation #}
<span aria-hidden="true">×</span>
diff --git a/Allura/allura/tests/test_helpers.py
b/Allura/allura/tests/test_helpers.py
index 9a12062bd..bb7908c9b 100644
--- a/Allura/allura/tests/test_helpers.py
+++ b/Allura/allura/tests/test_helpers.py
@@ -24,6 +24,8 @@ import time
import PIL
from mock import Mock, patch
from tg import tmpl_context as c
+from tg import config
+
from alluratest.tools import module_not_available
from webob import Request
from webob.exc import HTTPUnauthorized
@@ -346,6 +348,23 @@ def test_nl2br_jinja_filter():
Markup('foo<script>alert(1)</script><br>\nbar<br>\nbaz'))
+def test_subrender_jinja_filter():
+ # if we need a real ctx, have to do all this which probably isn't even
quite right
+ # from allura.config.app_cfg import AlluraJinjaRenderer
+ # j2env = AlluraJinjaRenderer.create(config, None)['jinja'].jinja2_env
+ # from jinja2.runtime import new_context
+ # j_ctx = new_context(j2env, template_name=None, blocks={})
+ j_ctx = None
+
+ # HTML, but no jinja:
+ assert h.subrender_jinja_filter(j_ctx, '<b>hi</b>{% foo %}') ==
'<b>hi</b>{% foo %}'
+ # var does not error if no `c.project`
+ assert h.subrender_jinja_filter(j_ctx, '<a href="{{ c.project.url()
}}"></a>') == '<a href="{{ c.project.url() }}"></a>'
+ # with `c.project` set, replacement of this var works:
+ with h.push_context('test', neighborhood='Projects'):
+ assert h.subrender_jinja_filter(j_ctx, '<a href="{{ c.project.url()
}}"></a>') == '<a href="/p/test/"></a>'
+
+
def test_split_select_field_options():
assert (h.split_select_field_options('"test message" test2') ==
['test message', 'test2'])
