Closes #22427: Validate JSONFilter.path; add JSONStringLookup with regex

- Add _validate_json_path(): each __-separated path segment must match
  [A-Za-z0-9_][A-Za-z0-9_-]* (allows leading underscores per Jeremy's
  suggestion; ORM operator names like 'date'/'regex' are valid JSON keys
  and are not blocked — the trailing __ JSONFilter appends makes them
  key traversal steps, not ORM transforms)
- Add JSONStringLookup: explicit string-filter type for JSONLookup.
  regex/i_regex are included (they offer no additional oracle power
  beyond starts_with, which is also present, per Jeremy's observation)
- JSONFilter.filter() validates self.path and returns empty Q() on
  invalid input rather than passing untrusted user input to the ORM
- 19 unit tests for path validation and JSONStringLookup field presence

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

netbox/netbox/graphql/filter_lookups.py

@@ -1,3 +1,4 @@
+import re
 from enum import Enum
 from typing import Generic, TypeVar
 
@@ -13,12 +14,46 @@ from strawberry_django import (
     ComparisonFilterLookup,
     FilterLookup,
     RangeLookup,
-    StrFilterLookup,
     process_filters,
 )
 
 from netbox.graphql.scalars import BigInt
 
+# ------------------------------------------------------------------
+# JSON path validation (VM-323)
+# ------------------------------------------------------------------
+
+# Each segment of a JSON path may only contain alphanumerics, underscores, and
+# hyphens.  Hyphens are included because JSON keys commonly use them; leading
+# underscores are permitted (e.g. _foo is a valid key name).
+_JSON_PATH_SEGMENT_RE = re.compile(r'^[A-Za-z0-9_][A-Za-z0-9_-]*$')
+
+
+def _validate_json_path(path: str) -> str:
+    """Validate a JSON traversal path for use in ORM lookups.
+
+    Each ``__``-separated segment must match ``[A-Za-z0-9_][A-Za-z0-9_-]*``.
+    Raises ``ValueError`` on an empty path, empty segment, or segment with
+    disallowed characters.
+
+    ORM operator names (``date``, ``regex``, etc.) are intentionally *not*
+    blocked here: ``JSONFilter.filter()`` always appends ``__`` to the path
+    before handing it to ``process_filters``, so a segment named ``regex``
+    becomes another level of JSON key traversal (``data__key__regex__exact``),
+    not the ORM regex transform (``data__key__regex=…``).
+    """
+    if not path:
+        raise ValueError("JSON path cannot be empty")
+
+    for segment in path.split('__'):
+        if not segment:
+            raise ValueError("JSON path contains consecutive or trailing '__'")
+        if not _JSON_PATH_SEGMENT_RE.match(segment):
+            raise ValueError(f"Invalid JSON path segment: {segment!r}")
+
+    return path
+
+
 __all__ = (
     'ArrayLookup',
     'BigIntegerLookup',
@@ -28,6 +63,8 @@ __all__ = (
     'IntegerLookup',
     'IntegerRangeArrayLookup',
     'JSONFilter',
+    'JSONLookup',
+    'JSONStringLookup',
     'StringArrayLookup',
     'TreeNodeFilter',
 )
@@ -78,9 +115,33 @@ class JSONDatetimeFilterLookup(ComparisonFilterLookup[str]):
     time: ComparisonFilterLookup[int] | None = strawberry.UNSET
 
 
+@strawberry.input(description='String lookups for JSON field values.')
+class JSONStringLookup:
+    """
+    String-filter type for use inside JSONLookup.
+
+    Equivalent to ``StrFilterLookup`` but defined explicitly so that the type
+    name remains stable and any future per-field restrictions are easy to add.
+    ``regex`` / ``i_regex`` are included: they provide no additional oracle
+    power beyond ``starts_with``, which is also present.
+    """
+    exact: str | None = strawberry_django.filter_field()
+    i_exact: str | None = strawberry_django.filter_field()
+    contains: str | None = strawberry_django.filter_field()
+    i_contains: str | None = strawberry_django.filter_field()
+    starts_with: str | None = strawberry_django.filter_field()
+    i_starts_with: str | None = strawberry_django.filter_field()
+    ends_with: str | None = strawberry_django.filter_field()
+    i_ends_with: str | None = strawberry_django.filter_field()
+    in_: list[str] | None = strawberry_django.filter_field()
+    isnull: bool | None = strawberry_django.filter_field()
+    regex: str | None = strawberry_django.filter_field()
+    i_regex: str | None = strawberry_django.filter_field()
+
+
 @strawberry.input(one_of=True, description='Lookup for JSON field. Only one of the lookup fields can be set.')
 class JSONLookup:
-    string_lookup: StrFilterLookup | None = strawberry_django.filter_field()
+    string_lookup: JSONStringLookup | None = strawberry_django.filter_field()
     int_range_lookup: RangeLookup[int] | None = strawberry_django.filter_field()
     int_comparison_lookup: ComparisonFilterLookup[int] | None = strawberry_django.filter_field()
     float_range_lookup: RangeLookup[float] | None = strawberry_django.filter_field()
@@ -158,7 +219,12 @@ class JSONFilter:
         if not filters:
             return queryset, Q()
 
-        json_path = f'{prefix}{self.path}__'
+        try:
+            safe_path = _validate_json_path(self.path)
+        except ValueError:
+            return queryset, Q()
+
+        json_path = f'{prefix}{safe_path}__'
         return process_filters(filters=filters, queryset=queryset, info=info, prefix=json_path)
 
 

netbox/netbox/tests/test_graphql.py

@@ -505,3 +505,97 @@ class GraphQLAPITestCase(APITestCase):
         data = json.loads(response.content)
         self.assertIn('errors', data)
         self.assertEqual(data['errors'][0]['message'], 'Cannot specify both `start` and `offset` in pagination.')
+
+
+class JSONPathValidationTestCase(TestCase):
+    """Unit tests for _validate_json_path (VM-323 security fix)."""
+
+    def setUp(self):
+        from netbox.graphql.filter_lookups import _validate_json_path
+        self.validate = _validate_json_path
+
+    # --- Valid paths ---
+
+    def test_single_key(self):
+        self.assertEqual(self.validate('key'), 'key')
+
+    def test_nested_key(self):
+        self.assertEqual(self.validate('parent__child'), 'parent__child')
+
+    def test_deeply_nested(self):
+        self.assertEqual(self.validate('a__b__c'), 'a__b__c')
+
+    def test_key_with_underscores(self):
+        self.assertEqual(self.validate('my_key'), 'my_key')
+
+    def test_key_with_hyphens(self):
+        self.assertEqual(self.validate('my-key'), 'my-key')
+
+    def test_numeric_array_index(self):
+        self.assertEqual(self.validate('items__0'), 'items__0')
+
+    def test_alphanumeric_segment(self):
+        self.assertEqual(self.validate('key123'), 'key123')
+
+    def test_key_with_leading_underscore(self):
+        # JSON keys may start with underscore (e.g. _foo)
+        self.assertEqual(self.validate('_key'), '_key')
+
+    def test_orm_operator_name_as_key(self):
+        # 'date', 'regex' etc. are valid JSON key names; the path validator
+        # must not block them.  The ORM injection risk is neutralised by the
+        # trailing __ that JSONFilter always appends before process_filters.
+        self.assertEqual(self.validate('date'), 'date')
+        self.assertEqual(self.validate('key__regex'), 'key__regex')
+        self.assertEqual(self.validate('key__exact'), 'key__exact')
+
+    # --- Invalid paths ---
+
+    def test_rejects_empty_string(self):
+        with self.assertRaises(ValueError):
+            self.validate('')
+
+    def test_rejects_all_underscores(self):
+        # '___' splits into segments ['', '', ''] via '__' — empty segments rejected
+        with self.assertRaises(ValueError):
+            self.validate('___')
+
+    def test_accepts_trailing_single_underscore(self):
+        # A single trailing underscore is a valid JSON key character
+        self.assertEqual(self.validate('key_'), 'key_')
+
+    def test_rejects_trailing_double_underscore(self):
+        with self.assertRaises(ValueError):
+            self.validate('key__')
+
+    def test_rejects_leading_double_underscore(self):
+        with self.assertRaises(ValueError):
+            self.validate('__key')
+
+    def test_rejects_consecutive_double_underscores(self):
+        with self.assertRaises(ValueError):
+            self.validate('key1____key2')
+
+    def test_rejects_segment_starting_with_special_char(self):
+        with self.assertRaises(ValueError):
+            self.validate('$secret')
+
+    def test_rejects_path_with_spaces(self):
+        with self.assertRaises(ValueError):
+            self.validate('key one')
+
+    def test_rejects_path_with_dot(self):
+        with self.assertRaises(ValueError):
+            self.validate('key.subkey')
+
+
+class JSONStringLookupTestCase(TestCase):
+    """Verify JSONStringLookup exposes the expected set of string operators."""
+
+    def test_string_operators_present(self):
+        from netbox.graphql.filter_lookups import JSONStringLookup
+        field_names = {f.name for f in JSONStringLookup.__strawberry_definition__.fields}
+        for expected in ('exact', 'i_exact', 'contains', 'i_contains',
+                         'starts_with', 'i_starts_with', 'ends_with', 'i_ends_with',
+                         'in_', 'isnull', 'regex', 'i_regex'):
+            self.assertIn(expected, field_names, f"{expected!r} must be present on JSONStringLookup")