Merge pull request #6 from hotwire-django/xss-vuln-fix

Remove content from f-strings

src/turbo_response/frame.py

@@ -59,20 +59,26 @@ def __init__(self, dom_id: str):
         """
         self.dom_id = dom_id
 
-    def render(self, content: str = "") -> str:
+    def render(self, content: str = "", is_safe: bool = False) -> str:
         """
         :param content: enclosed content
+        :param is_safe: mark content safe for HTML escaping.
+
         :return: a *<turbo-frame>* string
         """
-        return render_turbo_frame(dom_id=self.dom_id, content=content)
+        return render_turbo_frame(dom_id=self.dom_id, content=content, is_safe=is_safe)
 
-    def response(self, content: str = "", **response_kwargs) -> TurboFrameResponse:
+    def response(
+        self, content: str = "", is_safe: bool = False, **response_kwargs
+    ) -> TurboFrameResponse:
         """
         :param content: enclosed content
+        :param is_safe: mark content safe for HTML escaping.
+
         :return: a *<turbo-frame>* HTTP response
         """
         return TurboFrameResponse(
-            dom_id=self.dom_id, content=content, **response_kwargs
+            dom_id=self.dom_id, content=content, is_safe=is_safe, **response_kwargs
         )
 
     def template(

src/turbo_response/mixins.py

@@ -79,10 +79,12 @@ def get_response_content(self) -> str:
 
         return ""
 
-    def render_turbo_stream(self, **response_kwargs) -> HttpResponse:
+    def render_turbo_stream(
+        self, is_safe: bool = False, **response_kwargs
+    ) -> HttpResponse:
         """Returns a turbo-stream response."""
         return self.get_turbo_stream().response(
-            self.get_response_content(), **response_kwargs
+            self.get_response_content(), is_safe=is_safe, **response_kwargs
         )
 
 
@@ -254,11 +256,13 @@ class TurboFrameResponseMixin(TurboFrameMixin):
     def get_response_content(self) -> str:
         return ""
 
-    def render_turbo_frame(self, **response_kwargs) -> HttpResponse:
+    def render_turbo_frame(
+        self, is_safe: bool = False, **response_kwargs
+    ) -> HttpResponse:
         """Renders a turbo frame to response."""
 
         return self.get_turbo_frame().response(
-            self.get_response_content(), **response_kwargs
+            self.get_response_content(), is_safe=is_safe, **response_kwargs
         )
 
 

src/turbo_response/renderers.py

@@ -1,27 +1,63 @@
+# Standard Library
+from functools import lru_cache
+
+# Django
+from django.template import Context, Template
+from django.template.engine import Engine
+from django.utils.safestring import mark_safe
+
 # Local
 from .constants import Action
 
 
-def render_turbo_stream(action: Action, target: str, content: str = "") -> str:
+def render_turbo_stream(
+    action: Action, target: str, content: str = "", is_safe: bool = False
+) -> str:
     """Wraps content in correct <turbo-stream> tags.
 
     :param action: action type
     :param target: the DOM ID target of the stream
     :param content: content to be wrapped. Can be empty.
+    :param is_safe: mark content safe for HTML escaping.
 
     :return: *<turbo-stream>* string
     """
-    return f'<turbo-stream action="{action.value}" target="{target}"><template>{content.strip()}</template></turbo-stream>'
+    if is_safe:
+        content = mark_safe(content)
 
+    return get_turbo_stream_template().render(
+        Context({"action": action.value, "target": target, "content": content})
+    )
 
-def render_turbo_frame(dom_id: str, content: str = "") -> str:
+
+def render_turbo_frame(dom_id: str, content: str = "", is_safe: bool = False) -> str:
     """
 
     Wraps a response in correct *<turbo-frame>* tags.
 
     :param dom_id: a DOM ID present in the content
     :param content: content of the turbo-frame
+    :param is_safe: mark content safe for HTML escaping.
 
     :return: *<turbo-frame>* string
     """
-    return f'<turbo-frame id="{dom_id}">{content.strip()}</turbo-frame>'
+    if is_safe:
+        content = mark_safe(content)
+
+    return get_turbo_frame_template().render(
+        Context({"dom_id": dom_id, "content": content})
+    )
+
+
+@lru_cache()
+def get_turbo_stream_template() -> Template:
+    return Engine.get_default().from_string(
+        '<turbo-stream action="{{ action }}" target="{{ target }}"><template>{{ content }}</template></turbo-stream>',
+    )
+
+
+@lru_cache()
+def get_turbo_frame_template() -> Template:
+    return Engine.get_default().from_string(
+        '<turbo-frame id="{{ dom_id }}">{{ content }}</turbo-frame>'
+    )

src/turbo_response/response.py

@@ -68,11 +68,12 @@ def __init__(
         *,
         action: Optional[Action] = None,
         target: Optional[str] = None,
-        **kwargs,
+        is_safe: bool = False,
+        **response_kwargs,
     ):
         if action and target and isinstance(content, str):
-            content = render_turbo_stream(action, target, content)
-        super().__init__(content, **kwargs)
+            content = render_turbo_stream(action, target, content, is_safe=is_safe)
+        super().__init__(content, **response_kwargs)
 
 
 class TurboStreamTemplateResponse(TurboStreamResponseMixin, TemplateResponse):
@@ -117,17 +118,27 @@ def __init__(
     @property
     def rendered_content(self) -> str:
         return render_turbo_stream(
-            action=self._action, target=self._target, content=super().rendered_content
+            action=self._action,
+            target=self._target,
+            content=super().rendered_content,
+            is_safe=True,
         )
 
 
 class TurboFrameResponse(HttpResponse):
     """Handles turbo-frame template response."""
 
-    def __init__(self, content: str = "", *, dom_id: str, **kwargs):
+    def __init__(
+        self,
+        content: str = "",
+        *,
+        dom_id: str,
+        is_safe: bool = False,
+        **response_kwargs,
+    ):
         super().__init__(
-            render_turbo_frame(dom_id, content),
-            **kwargs,
+            render_turbo_frame(dom_id, content, is_safe=is_safe),
+            **response_kwargs,
         )
 
 
@@ -164,4 +175,4 @@ def __init__(
 
     @property
     def rendered_content(self) -> str:
-        return render_turbo_frame(self._dom_id, super().rendered_content)
+        return render_turbo_frame(self._dom_id, super().rendered_content, is_safe=True)

src/turbo_response/stream.py

@@ -63,22 +63,30 @@ def __init__(self, target: str, action: Action):
         self.action = action
         self.target = target
 
-    def render(self, content: str = "") -> str:
+    def render(self, content: str = "", is_safe: bool = False) -> str:
         """
         :param content: enclosed content
+        :param is_safe: mark content safe for HTML escaping.
+
         :return: a *<turbo-stream>* string
         """
         return render_turbo_stream(
-            action=self.action, target=self.target, content=content
+            action=self.action, target=self.target, content=content, is_safe=is_safe
         )
 
-    def response(self, content: str = "", **response_kwargs) -> TurboStreamResponse:
+    def response(
+        self, content: str = "", is_safe: bool = False, **response_kwargs
+    ) -> TurboStreamResponse:
         """
         :param content: enclosed content
         :return: a *<turbo-stream>* HTTP response wrapper
         """
         return TurboStreamResponse(
-            action=self.action, target=self.target, content=content, **response_kwargs
+            action=self.action,
+            target=self.target,
+            content=content,
+            is_safe=is_safe,
+            **response_kwargs,
         )
 
     def template(

src/turbo_response/template.py

@@ -36,7 +36,8 @@ def render_turbo_stream_template(
                 "is_turbo_stream": True,
             },
             **template_kwargs,
-        ),
+        ).strip(),
+        is_safe=True,
     )
 
 
@@ -58,7 +59,12 @@ def render_turbo_frame_template(
         dom_id,
         render_to_string(
             template,
-            {**(context or {}), "turbo_frame_dom_id": dom_id, "is_turbo_frame": True},
+            {
+                **(context or {}),
+                "turbo_frame_dom_id": dom_id,
+                "is_turbo_frame": True,
+            },
             **kwargs,
-        ),
+        ).strip(),
+        is_safe=True,
     )

src/turbo_response/tests/templates/simple.html

@@ -1 +1 @@
-<div>my content</div>
+<div>{{ msg }}</div>

src/turbo_response/tests/test_frame.py

@@ -7,8 +7,22 @@ def test_render(self):
         s = TurboFrame("my-form").render("OK")
         assert s == '<turbo-frame id="my-form">OK</turbo-frame>'
 
+    def test_render_xss(self):
+        s = TurboFrame("my-form").render("<script></script>")
+        assert (
+            s == '<turbo-frame id="my-form">&lt;script&gt;&lt;/script&gt;</turbo-frame>'
+        )
+
+    def test_render_is_safe(self):
+        s = TurboFrame("my-form").render("<script></script>", is_safe=True)
+        assert s == '<turbo-frame id="my-form"><script></script></turbo-frame>'
+
     def test_template(self):
-        s = TurboFrame("my-form").template("simple.html", {}).render()
+        s = (
+            TurboFrame("my-form")
+            .template("simple.html", {"msg": "my content"})
+            .render()
+        )
         assert "my content" in s
         assert '<turbo-frame id="my-form">' in s
 
@@ -18,21 +32,55 @@ def test_response(self):
         assert b"OK" in resp.content
         assert b'<turbo-frame id="my-form"' in resp.content
 
+    def test_response_xss(self):
+        resp = TurboFrame("my-form").response("<script></script>")
+        assert resp.status_code == 200
+        assert b"&lt;script&gt;&lt;/script&gt;" in resp.content
+        assert b'<turbo-frame id="my-form"' in resp.content
+
+    def test_response_is_safe(self):
+        resp = TurboFrame("my-form").response("<script></script>", is_safe=True)
+        assert resp.status_code == 200
+        assert b"<script></script>" in resp.content
+        assert b'<turbo-frame id="my-form"' in resp.content
+
     def test_template_render(self, rf):
         req = rf.get("/")
-        s = TurboFrame("my-form").template("simple.html", {}, request=req).render()
+        s = (
+            TurboFrame("my-form")
+            .template("simple.html", {"msg": "my content"}, request=req)
+            .render()
+        )
         assert "my content" in s
         assert '<turbo-frame id="my-form"' in s
 
+    def test_template_render_xss(self, rf):
+        req = rf.get("/")
+        s = (
+            TurboFrame("my-form")
+            .template("simple.html", {"msg": "<script></script>"}, request=req)
+            .render()
+        )
+        assert "&lt;script&gt;&lt;/script&gt;" in s
+        assert '<turbo-frame id="my-form"' in s
+
     def test_template_render_req_in_arg(self, rf):
         req = rf.get("/")
-        s = TurboFrame("my-form").template("simple.html", {}).render(request=req)
+        s = (
+            TurboFrame("my-form")
+            .template("simple.html", {"msg": "my content"})
+            .render(request=req)
+        )
         assert "my content" in s
         assert '<turbo-frame id="my-form"' in s
 
     def test_template_response(self, rf):
         req = rf.get("/")
-        resp = TurboFrame("my-form").template("simple.html", {}, request=req).response()
+        resp = (
+            TurboFrame("my-form")
+            .template("simple.html", {"msg": "my content"}, request=req)
+            .response()
+        )
         assert resp.status_code == 200
         assert "is_turbo_frame" in resp.context_data
         assert resp._request == req
@@ -42,10 +90,14 @@ def test_template_response(self, rf):
 
     def test_template_response_req_in_arg(self, rf):
         req = rf.get("/")
-        resp = TurboFrame("my-form").template("simple.html", {}).response(req)
+        resp = (
+            TurboFrame("my-form")
+            .template("simple.html", {"msg": "my content"})
+            .response(req)
+        )
         assert resp.status_code == 200
         assert "is_turbo_frame" in resp.context_data
         assert resp._request == req
         content = resp.render().content
-        assert b"my content" in content
+        assert b"<div>my content</div>" in content
         assert b'<turbo-frame id="my-form"' in content