diff mbox series

[error-report-web,2/3] Add link back validator and use it in the parser.

Message ID 20261008150446.74259-2-piotr@qbee.io
State New
Headers show
Series [error-report-web,1/3] Fix existing tests to establish a know working baseline for future work. | expand

Commit Message

Piotr Buliński Oct. 8, 2026, 3:04 p.m. UTC
This will prevent invalid URL (non-http(s)) from being recorded.

Signed-off-by: Piotr Buliński <piotr@qbee.io>
---
 Post/parser.py     |  3 +-
 Post/test.py       | 84 ++++++++++++++++++++++++++++++++++++++++++++++
 Post/validators.py | 42 +++++++++++++++++++++++
 3 files changed, 128 insertions(+), 1 deletion(-)
 create mode 100644 Post/validators.py
diff mbox series

Patch

diff --git a/Post/parser.py b/Post/parser.py
index 08f471d..b6740d0 100644
--- a/Post/parser.py
+++ b/Post/parser.py
@@ -11,6 +11,7 @@ 
 import json, re
 import bleach
 from Post.models import Build, BuildFailure, ErrorType
+from Post.validators import clean_link_back
 from django.conf import settings
 from django.utils import timezone
 from django.urls import reverse
@@ -48,7 +49,7 @@  class Parser:
             b.DISTRO = str(jsondata['distro'])
             b.NAME = str(jsondata['username'])
             b.EMAIL = str(jsondata['email'])
-            b.LINK_BACK = jsondata.get("link_back", None)
+            b.LINK_BACK = clean_link_back(jsondata.get("link_back", None))
             b.ERROR_TYPE = jsondata.get("error_type", ErrorType.RECIPE)
 
             # Extract the branch and commit
diff --git a/Post/test.py b/Post/test.py
index 15aa4be..0705f3c 100755
--- a/Post/test.py
+++ b/Post/test.py
@@ -4,6 +4,7 @@  import json
 import re
 from django.test import Client, override_settings
 from Post.models import BuildFailure, Build
+from Post.validators import clean_link_back
 
 #Delete the data between tests
 def data_runner (func):
@@ -246,3 +247,86 @@  class SimpleTest(unittest.TestCase):
 
         response = self.client.get("/Errors/Details/9898989898/")
         self.assertEqual(response.status_code, 200)
+
+
+class LinkBackValidatorTest(unittest.TestCase):
+
+    def test_allows_http(self):
+        self.assertEqual(clean_link_back("http://example.com/build/1"),
+                         "http://example.com/build/1")
+
+    def test_allows_https(self):
+        self.assertEqual(clean_link_back("https://example.com/build/1"),
+                         "https://example.com/build/1")
+
+    def test_rejects_javascript(self):
+        self.assertIsNone(
+            clean_link_back("javascript:alert(document.domain)//PROBE"))
+
+    def test_rejects_data(self):
+        self.assertIsNone(
+            clean_link_back("data:text/html,<script>alert(1)</script>"))
+
+    def test_rejects_vbscript(self):
+        self.assertIsNone(clean_link_back("vbscript:msgbox(1)"))
+
+    def test_rejects_scheme_relative(self):
+        self.assertIsNone(clean_link_back("//evil.example.com/x"))
+
+    def test_rejects_schemeless(self):
+        self.assertIsNone(clean_link_back("example.com/build/1"))
+
+    def test_rejects_control_char_obfuscation(self):
+        self.assertIsNone(clean_link_back("java\tscript:alert(1)"))
+
+    def test_rejects_leading_whitespace(self):
+        self.assertIsNone(clean_link_back("  javascript:alert(1)"))
+
+    def test_rejects_missing_host(self):
+        self.assertIsNone(clean_link_back("http:///path"))
+
+    def test_rejects_none_empty_and_non_string(self):
+        self.assertIsNone(clean_link_back(None))
+        self.assertIsNone(clean_link_back(""))
+        self.assertIsNone(clean_link_back("   "))
+        self.assertIsNone(clean_link_back(123))
+
+
+class LinkBackSubmissionTest(unittest.TestCase):
+
+    def setUp(self):
+        # The test client submits with HTTP_HOST="testhost"; allow it here.
+        override = override_settings(ALLOWED_HOSTS=["testhost"])
+        override.enable()
+        self.addCleanup(override.disable)
+        self.client = Client(HTTP_HOST="testhost")
+        # Start from a clean slate; other unittest.TestCase tests commit rows.
+        Build.objects.all().delete()
+        BuildFailure.objects.all().delete()
+        with open("test-data/test-payload.json") as f:
+            self.base_payload = json.loads(f.read())
+
+    def tearDown(self):
+        Build.objects.all().delete()
+        BuildFailure.objects.all().delete()
+
+    def _submit(self, link_back):
+        payload = dict(self.base_payload)
+        payload['link_back'] = link_back
+        data = urllib.parse.urlencode({'data': json.dumps(payload)})
+        response = self.client.post("/ClientPost/", data, "application/json")
+        self.assertEqual(response.status_code, 200, response.content)
+        return BuildFailure.objects.get()
+
+    def test_javascript_link_back_is_blanked(self):
+        bf = self._submit("javascript:alert(document.domain)//PROBE")
+        self.assertIsNone(bf.BUILD.LINK_BACK)
+
+        response = self.client.get("/Errors/Details/%d/" % bf.id)
+        self.assertEqual(response.status_code, 200)
+        self.assertNotIn(b'href="javascript:', response.content)
+
+    def test_http_link_back_is_preserved(self):
+        bf = self._submit("http://example.com/build/42")
+        self.assertEqual(bf.BUILD.LINK_BACK, "http://example.com/build/42")
+
diff --git a/Post/validators.py b/Post/validators.py
new file mode 100644
index 0000000..c549664
--- /dev/null
+++ b/Post/validators.py
@@ -0,0 +1,42 @@ 
+# SPDX-License-Identifier: MIT
+#
+# error-reporting-tool - link-back URL validation
+#
+# Licensed under the MIT license, see COPYING.MIT for details
+
+import re
+from urllib.parse import urlparse
+
+# Only absolute http(s) URLs are allowed as a "link back". Everything else
+# (javascript:, data:, vbscript:, scheme-relative "//host" or schemeless
+# values) is rejected so it can never be rendered verbatim into an href.
+ALLOWED_LINK_BACK_SCHEMES = ("http", "https")
+
+# Control characters are used to obfuscate schemes, e.g. "java\tscript:".
+_CONTROL_CHARS = re.compile(r"[\x00-\x1f\x7f]")
+
+
+def clean_link_back(value):
+    """Return a safe link-back URL, or None if the value is not acceptable."""
+    if not isinstance(value, str):
+        return None
+
+    value = value.strip()
+    if not value:
+        return None
+
+    if _CONTROL_CHARS.search(value):
+        return None
+
+    try:
+        parsed = urlparse(value)
+    except ValueError:
+        return None
+
+    if parsed.scheme.lower() not in ALLOWED_LINK_BACK_SCHEMES:
+        return None
+
+    if not parsed.netloc:
+        return None
+
+    return value