Просмотр исходного кода

Validate BigInt input and register the scalar for all schemas

BigIntScalar passed any input value through to the ORM as a string, so a
non-integer or out-of-range filter value failed at query execution rather
than during validation. Coerce input to a signed 64-bit integer, accepting
numeric strings, and reject anything else with a GraphQL error.

Register BigInt in Strawberry's default scalar registry so that standalone
schemas built by plugins resolve it without declaring it in scalar_map.

Change the data_source_id, data_file_id and L2VPN termination
assigned_object_id filters to ID, consistent with the other object ID
filters.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Jeremy Stretch 5 дней назад
Родитель
Сommit
8271f05e0b

+ 3 - 4
netbox/netbox/graphql/filter_mixins.py

@@ -3,10 +3,9 @@ from typing import TYPE_CHECKING, Annotated, TypeVar
 
 import strawberry
 import strawberry_django
+from strawberry.scalars import ID
 from strawberry_django import BaseFilterLookup, DatetimeFilterLookup, FilterLookup, StrFilterLookup
 
-from netbox.graphql.scalars import BigInt
-
 __all__ = (
     'DistanceFilterMixin',
     'ImageAttachmentFilterMixin',
@@ -44,11 +43,11 @@ class SyncedDataFilterMixin:
     data_source: Annotated['DataSourceFilter', strawberry.lazy('core.graphql.filters')] | None = (
         strawberry_django.filter_field()
     )
-    data_source_id: FilterLookup[BigInt] | None = strawberry_django.filter_field()
+    data_source_id: ID | None = strawberry_django.filter_field()
     data_file: Annotated['DataFileFilter', strawberry.lazy('core.graphql.filters')] | None = (
         strawberry_django.filter_field()
     )
-    data_file_id: FilterLookup[BigInt] | None = strawberry_django.filter_field()
+    data_file_id: ID | None = strawberry_django.filter_field()
     data_path: StrFilterLookup | None = strawberry_django.filter_field()
     auto_sync_enabled: FilterLookup[bool] | None = strawberry_django.filter_field()
     data_synced: DatetimeFilterLookup | None = strawberry_django.filter_field()

+ 23 - 1
netbox/netbox/graphql/scalars.py

@@ -1,12 +1,34 @@
+import re
 from typing import NewType
 
 import strawberry
+from graphql import GraphQLError
 
 BigInt = NewType('BigInt', int)
 
+BIGINT_MIN = -2**63
+BIGINT_MAX = 2**63 - 1
+
+
+def parse_bigint(value):
+    """
+    Coerce an input value to a signed 64-bit integer. Numeric strings are accepted as well as integers, since many
+    JSON clients cannot represent integers beyond 2^53 as numbers.
+    """
+    if isinstance(value, str) and re.fullmatch(r'-?[0-9]+', value):
+        value = int(value)
+    elif isinstance(value, float) and value.is_integer():
+        value = int(value)
+    if type(value) is not int:
+        raise GraphQLError(f'BigInt cannot represent non-integer value: {value!r}')
+    if not BIGINT_MIN <= value <= BIGINT_MAX:
+        raise GraphQLError(f'BigInt cannot represent non 64-bit signed integer value: {value!r}')
+    return value
+
+
 BigIntScalar = strawberry.scalar(
     name='BigInt',
     serialize=lambda v: int(v),
-    parse_value=lambda v: str(v),
+    parse_value=parse_bigint,
     description='BigInt field',
 )

+ 7 - 2
netbox/netbox/settings.py

@@ -1041,12 +1041,13 @@ for plugin_name in PLUGINS:
 
 from django.db import models  # noqa: E402
 from rest_framework.utils import field_mapping  # noqa: E402
+from strawberry.schema.types.scalar import DEFAULT_SCALAR_REGISTRY  # noqa: E402
 from strawberry_django import pagination  # noqa: E402
 from strawberry_django.fields.field import StrawberryDjangoField  # noqa: E402
 from strawberry_django.fields.types import field_type_map  # noqa: E402
 
 from netbox.graphql.pagination import OffsetPaginationInput, apply_pagination  # noqa: E402
-from netbox.graphql.scalars import BigInt  # noqa: E402
+from netbox.graphql.scalars import BigInt, BigIntScalar  # noqa: E402
 
 # TODO: Remove this once #20547 has been implemented
 # Override DRF's get_unique_validators() function with our own (see bug #19302)
@@ -1058,12 +1059,16 @@ pagination.OffsetPaginationInput = OffsetPaginationInput
 # Patch StrawberryDjangoField to use our custom `apply_pagination()` method with support for cursor-based pagination
 StrawberryDjangoField.apply_pagination = apply_pagination
 
-# Map 64-bit integer model fields to the BigInt scalar, since GraphQL's Int type is limited to 32 bits (see #23423)
+# Map 64-bit integer model fields to the BigInt scalar, since GraphQL's Int type is limited to 32 bits
 field_type_map.update({
     models.BigIntegerField: BigInt,
     models.PositiveBigIntegerField: BigInt,
 })
 
+# Register the BigInt scalar by default, so that any Strawberry schema (including one built by a plugin) can resolve it
+# without having to declare it in its own scalar_map
+DEFAULT_SCALAR_REGISTRY[BigInt] = BigIntScalar
+
 
 # UNSUPPORTED FUNCTIONALITY: Import any local overrides.
 try:

+ 47 - 14
netbox/netbox/tests/test_graphql.py

@@ -36,7 +36,7 @@ from dcim.models import (
 )
 from extras.choices import CustomFieldTypeChoices
 from extras.models import CustomField, ExportTemplate, JournalEntry, TableConfig, Tag
-from ipam.models import RIR, VLAN, Aggregate, IPAddress, Prefix
+from ipam.models import ASN, RIR, VLAN, Aggregate, IPAddress, Prefix
 from netbox.graphql.pagination import apply_distinct_window_pagination
 from netbox.graphql.scalars import BigInt, BigIntScalar
 from netbox.graphql.schema import Query, get_schema_extensions, schema
@@ -492,13 +492,7 @@ class GraphQLAPITestCase(APITestCase):
         self.assertEqual(data['data']['journal_entry_list'], [{'assigned_object_id': object_id}])
 
         # Filter on a generic object ID
-        query = f"""
-        {{
-            l2vpn_termination_list(filters: {{assigned_object_id: {{filter_lookup: {{exact: {object_id}}}}}}}) {{
-                id
-            }}
-        }}
-        """
+        query = f'{{ l2vpn_termination_list(filters: {{assigned_object_id: "{object_id}"}}) {{ id }} }}'
         response = self.client.post(url, data={'query': query}, format='json', **self.header)
         self.assertHttpStatus(response, status.HTTP_200_OK)
         data = json.loads(response.content)
@@ -506,13 +500,56 @@ class GraphQLAPITestCase(APITestCase):
         self.assertEqual(len(data['data']['l2vpn_termination_list']), 1)
 
         # Filter on a related object ID
-        query = f'{{ export_template_list(filters: {{data_source_id: {{exact: {object_id}}}}}) {{ id }} }}'
+        query = f'{{ export_template_list(filters: {{data_source_id: "{object_id}"}}) {{ id }} }}'
         response = self.client.post(url, data={'query': query}, format='json', **self.header)
         self.assertHttpStatus(response, status.HTTP_200_OK)
         data = json.loads(response.content)
         self.assertNotIn('errors', data)
         self.assertEqual(data['data']['export_template_list'], [{'id': str(export_template.pk)}])
 
+    def test_graphql_bigint_filter_validation(self):
+        """
+        BigInt filter values must be validated as 64-bit integers before the query is executed.
+        """
+        self.add_permissions('ipam.view_asn')
+        url = reverse('graphql')
+        rir = RIR.objects.create(name='RIR 1', slug='rir-1')
+        asn = ASN.objects.create(asn=4_200_000_000, rir=rir)
+
+        # Integers and numeric strings beyond the 32-bit range are accepted
+        for value in ('4200000000', '"4200000000"'):
+            query = f'{{ asn_list(filters: {{asn: {{filter_lookup: {{exact: {value}}}}}}}) {{ id }} }}'
+            response = self.client.post(url, data={'query': query}, format='json', **self.header)
+            self.assertHttpStatus(response, status.HTTP_200_OK)
+            data = json.loads(response.content)
+            self.assertNotIn('errors', data)
+            self.assertEqual(data['data']['asn_list'], [{'id': str(asn.pk)}])
+
+        # Non-integer and out-of-range values are rejected
+        for value in ('1.5', '"abc"', 'true', str(2**63)):
+            query = f'{{ asn_list(filters: {{asn: {{filter_lookup: {{exact: {value}}}}}}}) {{ id }} }}'
+            response = self.client.post(url, data={'query': query}, format='json', **self.header)
+            data = json.loads(response.content)
+            self.assertIn('errors', data, msg=f'{value} was not rejected')
+            self.assertIn('BigInt cannot represent', data['errors'][0]['message'])
+
+        # Values passed as variables are validated the same way
+        query = 'query ($asn: BigInt) { asn_list(filters: {asn: {filter_lookup: {exact: $asn}}}) { id } }'
+        for value in (4_200_000_000, '4200000000'):
+            response = self.client.post(
+                url, data={'query': query, 'variables': {'asn': value}}, format='json', **self.header
+            )
+            self.assertHttpStatus(response, status.HTTP_200_OK)
+            data = json.loads(response.content)
+            self.assertNotIn('errors', data)
+            self.assertEqual(data['data']['asn_list'], [{'id': str(asn.pk)}])
+        response = self.client.post(
+            url, data={'query': query, 'variables': {'asn': 'abc'}}, format='json', **self.header
+        )
+        data = json.loads(response.content)
+        self.assertIn('errors', data)
+        self.assertIn('BigInt cannot represent', data['errors'][0]['message'])
+
     def test_graphql_array_length_lookup(self):
         """
         The public GraphQL ``length`` array lookup must map to Django's ``len`` transform.
@@ -918,11 +955,7 @@ class GraphQLAPITestCase(APITestCase):
         class PluginQuery:
             platforms: list[PluginPlatformType] = strawberry_django.field()
 
-        plugin_schema = strawberry.Schema(
-            query=PluginQuery,
-            config=StrawberryConfig(scalar_map={BigInt: BigIntScalar}),
-            extensions=[DjangoOptimizerExtension],
-        )
+        plugin_schema = strawberry.Schema(query=PluginQuery, extensions=[DjangoOptimizerExtension])
         self.user.is_superuser = True
         result = plugin_schema.execute_sync(
             '{ platforms { name display } }',

+ 1 - 3
netbox/vpn/graphql/filters.py

@@ -216,6 +216,4 @@ class L2VPNTerminationFilter(NetBoxModelFilter):
     assigned_object_type: Annotated['ContentTypeFilter', strawberry.lazy('core.graphql.filters')] | None = (
         strawberry_django.filter_field()
     )
-    assigned_object_id: Annotated['BigIntegerLookup', strawberry.lazy('netbox.graphql.filter_lookups')] | None = (
-        strawberry_django.filter_field()
-    )
+    assigned_object_id: ID | None = strawberry_django.filter_field()