Merge pull request #1085 from isso-comments/migrate-pkg-resources

Migrate pkg resources to importlib

Author: jelmer

.github/workflows/python-tests.yml

@@ -11,7 +11,7 @@ jobs:
     strategy:
       matrix:
         os: [ubuntu-latest, macos-latest]
-        python-version: [3.8, 3.9, "3.10", "3.11", "3.12"]
+        python-version: [3.9, "3.10", "3.11", "3.12", "3.13", "3.14"]
       fail-fast: false
 
     steps:

docs/conf.py

@@ -15,7 +15,6 @@
 import sys
 import io
 import re
-import pkg_resources
 
 from os.path import dirname, join
 # Make `_theme` custom sphinx theme available

isso/__init__.py

@@ -25,8 +25,8 @@
 #
 # Isso – a lightweight Disqus alternative
 
-import pkg_resources
-dist = pkg_resources.get_distribution("isso")
+from importlib.metadata import version
+__version__ = version("isso")
 
 # check if exectuable is `isso` and gevent is available
 import sys
@@ -232,7 +232,7 @@ def main():
     subparser = parser.add_subparsers(help="commands", dest="command")
 
     parser.add_argument('--version', action='version',
-                        version='%(prog)s ' + dist.version)
+                        version='%(prog)s ' + __version__)
     parser.add_argument("-c", dest="conf", default="/etc/isso.cfg",
                         metavar="/etc/isso.cfg", help="set configuration file")
 

isso/config.py

@@ -3,15 +3,15 @@
 import datetime
 import logging
 import os
-import pkg_resources
 import re
 
 from email.utils import parseaddr, formataddr
 from configparser import ConfigParser, NoOptionError, NoSectionError, DuplicateSectionError
+from importlib.resources import files
 
 logger = logging.getLogger("isso")
 
-default_config_file = pkg_resources.resource_filename('isso', 'isso.cfg')
+default_config_file = str(files('isso').joinpath('isso.cfg'))
 
 
 def timedelta(string):

isso/tests/test_comments.py

@@ -69,6 +69,22 @@ def testCreate(self):
         self.assertEqual(rv["mode"], 1)
         self.assertEqual(rv["text"], '<p>Lorem ipsum ...</p>')
 
+    def testWebsiteXSSPayloadIsEscaped(self):
+        """Website field with XSS payload must have quotes HTML-escaped."""
+        payload = "http://x.com/?'onmouseover='alert(document.domain)'x='"
+        rv = self.post('/new?uri=%2Fpath%2F',
+                       data=json.dumps({'text': 'Hello', 'website': payload}))
+        self.assertEqual(rv.status_code, 201)
+        rv = loads(rv.data)
+        # Single quotes must be HTML-escaped so they cannot break out of an
+        # HTML attribute context (e.g. href='...')
+        self.assertNotIn("'", rv["website"])
+        self.assertNotIn('"', rv["website"])
+        self.assertEqual(
+            rv["website"],
+            "http://x.com/?&#x27;onmouseover=&#x27;alert(document.domain)&#x27;x=&#x27;",
+        )
+
     def textCreateWithNonAsciiText(self):
 
         rv = self.post('/new?uri=%2Fpath%2F',
@@ -391,6 +407,27 @@ def testUpdate(self):
         self.assertEqual(rv['website'], 'http://example.com/')
         self.assertIn('modified', rv)
 
+    def testUpdateWebsiteXSSPayloadIsEscaped(self):
+        """Website and author XSS payloads via edit endpoint must be HTML-escaped."""
+        self.post('/new?uri=%2Fpath%2F', data=json.dumps({'text': 'Lorem ipsum ...'}))
+
+        website_payload = "http://x.com/?'onmouseover='alert(document.domain)'x='"
+        author_payload = "<script>alert(1)</script>"
+        self.put('/id/1', data=json.dumps({
+            'text': 'Hello World',
+            'author': author_payload,
+            'website': website_payload,
+        }))
+
+        rv = loads(self.get('/id/1?plain=1').data)
+        self.assertNotIn("'", rv["website"])
+        self.assertEqual(
+            rv["website"],
+            "http://x.com/?&#x27;onmouseover=&#x27;alert(document.domain)&#x27;x=&#x27;",
+        )
+        self.assertNotIn("<script>", rv["author"])
+        self.assertEqual(rv["author"], "&lt;script&gt;alert(1)&lt;/script&gt;")
+
     def testUpdateForbidden(self):
 
         self.post('/new?uri=test', data=json.dumps({'text': 'Hello world!'}))
@@ -860,6 +897,34 @@ def testModerateComment(self):
         # Comment should no longer exist
         self.assertEqual(self.app.db.comments.get(id_), None)
 
+    def testModerateEditXSSPayloadIsEscaped(self):
+        """XSS payloads in author/website via moderate edit endpoint must be HTML-escaped."""
+        id_ = 1
+        signed = self.app.sign(id_)
+
+        self.client.post('/new?uri=/moderated', data=json.dumps({"text": "..."}))
+
+        website_payload = "http://x.com/?'onmouseover='alert(document.domain)'x='"
+        author_payload = "<script>alert(1)</script>"
+        rv = self.client.post(
+            '/id/%d/edit/%s' % (id_, signed),
+            data=json.dumps({
+                "text": "new text",
+                "author": author_payload,
+                "website": website_payload,
+            }),
+        )
+        self.assertEqual(rv.status_code, 200)
+
+        stored = self.app.db.comments.get(id_)
+        self.assertNotIn("'", stored["website"])
+        self.assertEqual(
+            stored["website"],
+            "http://x.com/?&#x27;onmouseover=&#x27;alert(document.domain)&#x27;x=&#x27;",
+        )
+        self.assertNotIn("<script>", stored["author"])
+        self.assertEqual(stored["author"], "&lt;script&gt;alert(1)&lt;/script&gt;")
+
 
 class TestUnsubscribe(unittest.TestCase):
 

isso/utils/http.py

@@ -6,7 +6,7 @@
 
 from urllib.parse import urlparse
 
-from isso import dist
+from isso import __version__
 from isso.wsgi import urlsplit
 
 MAX_RETRY_COUNT = 3
@@ -24,7 +24,7 @@ class curl(object):
     """
 
     headers = {
-        "User-Agent": "Isso/{0} (+https://isso-comments.de)".format(dist.version)
+        "User-Agent": "Isso/{0} (+https://isso-comments.de)".format(__version__)
     }
 
     def __init__(self, method, host, path, timeout=3):

isso/views/__init__.py

@@ -1,7 +1,7 @@
 # -*- encoding: utf-8 -*-
 
-import pkg_resources
-dist = pkg_resources.get_distribution("isso")
+from importlib.metadata import version
+__version__ = version("isso")
 
 import json
 
@@ -81,7 +81,7 @@ def __init__(self, isso):
     def show(self, environ, request):
 
         rv = {
-            "version": dist.version,
+            "version": __version__,
             "host": str(local("host")),
             "origin": str(local("origin")),
             "moderation": self.moderation,

isso/views/comments.py

@@ -5,9 +5,9 @@
 import time
 import functools
 import json  # json.dumps to put URL in <script>
-import pkg_resources
 
 from configparser import NoOptionError
+from importlib.resources import files
 from datetime import datetime, timedelta
 from html import escape
 from io import BytesIO as StringIO
@@ -335,10 +335,13 @@ def new(self, environ, request, uri):
         if not valid:
             return BadRequest(reason)
 
-        for field in ("author", "email", "website"):
+        for field in ("author", "email"):
             if data.get(field) is not None:
                 data[field] = escape(data[field], quote=False)
 
+        if data.get("website") is not None:
+            data["website"] = escape(data["website"], quote=True)
+
         if data.get("website"):
             data["website"] = normalize(data["website"])
 
@@ -548,6 +551,13 @@ def edit(self, environ, request, id):
         if not valid:
             return BadRequest(reason)
 
+        for field in ("author",):
+            if data.get(field) is not None:
+                data[field] = escape(data[field], quote=False)
+
+        if data.get("website") is not None:
+            data["website"] = escape(data["website"], quote=True)
+
         data['modified'] = time.time()
 
         with self.isso.lock:
@@ -794,6 +804,21 @@ def moderate(self, environ, request, id, action, key):
             return Response("Comment has been activated", 200)
         elif action == "edit":
             data = request.json
+
+            for key in set(data.keys()) - set(["text", "author", "website"]):
+                data.pop(key)
+
+            valid, reason = API.verify(data)
+            if not valid:
+                return BadRequest(reason)
+
+            for field in ("author",):
+                if data.get(field) is not None:
+                    data[field] = escape(data[field], quote=False)
+
+            if data.get("website") is not None:
+                data["website"] = escape(data["website"], quote=True)
+
             with self.isso.lock:
                 rv = self.comments.update(id, data)
             for key in set(rv.keys()) - API.FIELDS:
@@ -1391,7 +1416,7 @@ def config(self, environment, request):
         </body>
     """
     def demo(self, env, req):
-        index = pkg_resources.resource_filename('isso', 'demo/index.html')
+        index = str(files('isso').joinpath('demo/index.html'))
         return send_from_directory(os_path.dirname(index), 'index.html', env)
 
     """