(allura) 06/15: [#8624] validate the solr sort parameter

[br...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

asf-gitbox-commits pushed a commit to branch db/8624b
in repository https://gitbox.apache.org/repos/asf/allura.git

commit a23b925d22c7fbad2aa16d54b6edfad9fb4a8b7e
Author: Dave Brondsema <db...@slashdotmedia.com>
AuthorDate: Tue Sep 8 17:37:35 2026 +0000

    [#8624] validate the solr sort parameter
    
    The sort parameter was passed through to solr unchecked from every search
    entry point. Solr accepts function queries there, which are evaluated per
    matching document, so a caller could choose an arbitrary and expensive
    sort expression. Require a field name with an optional direction, and drop
    anything else so the search still runs with solr's default ordering.
    
    Co-Authored-By: Claude Opus 5 <no...@anthropic.com>
    Claude-Session: https://claude.ai/code/session_01CbUqCrWrninjwW3vSrxxbC
---
 Allura/allura/lib/search.py                           | 16 +++++++++++++++-
 .../forgetracker/tests/functional/test_root.py        | 19 +++++++++++++++++++
 2 files changed, 34 insertions(+), 1 deletion(-)

diff --git a/Allura/allura/lib/search.py b/Allura/allura/lib/search.py
index 796a5cbcc..747a145bd 100644
--- a/Allura/allura/lib/search.py
+++ b/Allura/allura/lib/search.py
@@ -37,7 +37,13 @@
 
 log = getLogger(__name__)
 
-FILTER_FIELD_RE = re.compile(r'^[A-Za-z_][A-Za-z0-9_]*$')
+# \w so that custom field names keep working; they're slugified with \W, which leaves non-ascii letters in
+FILTER_FIELD_RE = re.compile(r'[^\W\d]\w*\Z')
+
+# "field", "field asc", or a comma-separated list of those.  Excluding parens
+# keeps out function queries, which solr evaluates per matching document.
+SORT_SPEC_RE = re.compile(r'[^\W\d]\w*( (asc|desc))?'
+                          r'(\s*,\s*[^\W\d]\w*( (asc|desc))?)*\Z')
 
 
 class SearchIndexable:
@@ -189,6 +195,14 @@ def search(q, short_timeout=False, ignore_errors=True, search_fn=None, **kw):
     # don't pass through sort=None etc
     kw = {k: v for k, v in kw.items() if v is not None}
 
+    sort = kw.get('sort')
+    if sort is not None and not (isinstance(sort, str) and SORT_SPEC_RE.match(sort)):
+        if asbool(config['debug']) or 'pytest' in sys.modules:
+            raise ValueError(f'Invalid sort spec: {sort!r}')
+        else:
+            log.warning('Ignoring unusable sort: %r', sort)
+            del kw['sort']
+
     try:
         # try once with opportunity to retry
         try:
diff --git a/ForgeTracker/forgetracker/tests/functional/test_root.py b/ForgeTracker/forgetracker/tests/functional/test_root.py
index 4323eec98..6278e016d 100644
--- a/ForgeTracker/forgetracker/tests/functional/test_root.py
+++ b/ForgeTracker/forgetracker/tests/functional/test_root.py
@@ -1395,12 +1395,31 @@ def test_search_filter_field_names_validated(self, search):
         self.app.get('/bugs/search/', params={'q': 'x', 'filter': '{"status": ["open"]}'})
         assert 'status_s:open' in search.call_args[1]['fq']
 
+        # custom field names are slugified with \W, so non-ascii letters survive into them
+        self.app.get('/bugs/search/', params={'q': 'x', 'filter': '{"_café_field": ["v"]}'})
+        assert '_café_field_s:v' in search.call_args[1]['fq']
+
     def test_search_filter_non_string_value(self):
         self.new_ticket(summary='a ticket')
         self.app.get('/bugs/search/',
                      params={'q': 'x', 'filter': '{"status": [{"$ne": null}]}'},
                      status=200)
 
+    @patch('allura.lib.search.g.solr_short_timeout')
+    def test_search_sort_validated(self, solr):
+        solr.search.return_value = None
+        self.new_ticket(summary='a ticket')
+
+        with pytest.raises(ValueError, match=r'Invalid sort spec'):
+            self.app.get('/bugs/search/', params={'q': 'x', 'sort': 'div(1,0) desc'})
+        assert not solr.search.called
+
+        self.app.get('/bugs/search/', params={'q': 'x', 'sort': 'ticket_num_i asc'})
+        assert solr.search.call_args[1]['sort'] == 'ticket_num_i asc'
+
+        self.app.get('/bugs/search/', params={'q': 'x', 'sort': '_café_field_s asc'})
+        assert solr.search.call_args[1]['sort'] == '_café_field_s asc,ticket_num_i asc'
+
     def test_search_canonical(self):
         self.new_ticket(summary='test first ticket')
         self.new_ticket(summary='test second ticket')