review fixes

netbox/circuits/migrations/0061_generated_fields.py

@@ -105,4 +105,11 @@ class Migration(migrations.Migration):
                 validators=[django.core.validators.MinValueValidator(0)],
             ),
         ),
+        migrations.AddConstraint(
+            model_name="circuit",
+            constraint=models.CheckConstraint(
+                condition=models.Q(("_abs_distance__gte", 0)),
+                name="circuits_circuit_abs_distance_non_negative",
+            ),
+        ),
     ]

netbox/circuits/models/circuits.py

@@ -19,6 +19,7 @@ from netbox.models.features import (
     TagsMixin,
 )
 from netbox.models.mixins import DistanceMixin
+from utilities.conversion import non_negative_constraint
 from utilities.data import normalize_update_fields
 from utilities.string import title
 
@@ -147,6 +148,7 @@ class Circuit(ContactsMixin, ImageAttachmentsMixin, DistanceMixin, PrimaryModel)
                 fields=('provider_account', 'cid'),
                 name='%(app_label)s_%(class)s_unique_provideraccount_cid'
             ),
+            non_negative_constraint('_abs_distance'),
         )
         indexes = (
             models.Index(fields=('provider', 'provider_account', 'cid')),  # Default ordering

netbox/dcim/migrations/0252_generated_fields.py

@@ -863,4 +863,102 @@ class Migration(migrations.Migration):
                 validators=[django.core.validators.MinValueValidator(0)],
             ),
         ),
+        migrations.AddConstraint(
+            model_name="cable",
+            constraint=models.CheckConstraint(
+                condition=models.Q(("_abs_length__gte", 0)),
+                name="dcim_cable_abs_length_non_negative",
+            ),
+        ),
+        migrations.AddConstraint(
+            model_name="coolingfeed",
+            constraint=models.CheckConstraint(
+                condition=models.Q(("_abs_max_flow__gte", 0)),
+                name="dcim_coolingfeed_abs_max_flow_non_negative",
+            ),
+        ),
+        migrations.AddConstraint(
+            model_name="coolingintake",
+            constraint=models.CheckConstraint(
+                condition=models.Q(("_abs_diameter__gte", 0)),
+                name="dcim_coolingintake_abs_diameter_non_negative",
+            ),
+        ),
+        migrations.AddConstraint(
+            model_name="coolingintake",
+            constraint=models.CheckConstraint(
+                condition=models.Q(("_abs_max_flow__gte", 0)),
+                name="dcim_coolingintake_abs_max_flow_non_negative",
+            ),
+        ),
+        migrations.AddConstraint(
+            model_name="coolingintaketemplate",
+            constraint=models.CheckConstraint(
+                condition=models.Q(("_abs_diameter__gte", 0)),
+                name="dcim_coolingintaketemplate_abs_diameter_non_negative",
+            ),
+        ),
+        migrations.AddConstraint(
+            model_name="coolingintaketemplate",
+            constraint=models.CheckConstraint(
+                condition=models.Q(("_abs_max_flow__gte", 0)),
+                name="dcim_coolingintaketemplate_abs_max_flow_non_negative",
+            ),
+        ),
+        migrations.AddConstraint(
+            model_name="coolingoutflow",
+            constraint=models.CheckConstraint(
+                condition=models.Q(("_abs_diameter__gte", 0)),
+                name="dcim_coolingoutflow_abs_diameter_non_negative",
+            ),
+        ),
+        migrations.AddConstraint(
+            model_name="coolingoutflowtemplate",
+            constraint=models.CheckConstraint(
+                condition=models.Q(("_abs_diameter__gte", 0)),
+                name="dcim_coolingoutflowtemplate_abs_diameter_non_negative",
+            ),
+        ),
+        migrations.AddConstraint(
+            model_name="devicetype",
+            constraint=models.CheckConstraint(
+                condition=models.Q(("_abs_weight__gte", 0)),
+                name="dcim_devicetype_abs_weight_non_negative",
+            ),
+        ),
+        migrations.AddConstraint(
+            model_name="moduletype",
+            constraint=models.CheckConstraint(
+                condition=models.Q(("_abs_weight__gte", 0)),
+                name="dcim_moduletype_abs_weight_non_negative",
+            ),
+        ),
+        migrations.AddConstraint(
+            model_name="rack",
+            constraint=models.CheckConstraint(
+                condition=models.Q(("_abs_weight__gte", 0)),
+                name="dcim_rack_abs_weight_non_negative",
+            ),
+        ),
+        migrations.AddConstraint(
+            model_name="rack",
+            constraint=models.CheckConstraint(
+                condition=models.Q(("_abs_max_weight__gte", 0)),
+                name="dcim_rack_abs_max_weight_non_negative",
+            ),
+        ),
+        migrations.AddConstraint(
+            model_name="racktype",
+            constraint=models.CheckConstraint(
+                condition=models.Q(("_abs_weight__gte", 0)),
+                name="dcim_racktype_abs_weight_non_negative",
+            ),
+        ),
+        migrations.AddConstraint(
+            model_name="racktype",
+            constraint=models.CheckConstraint(
+                condition=models.Q(("_abs_max_weight__gte", 0)),
+                name="dcim_racktype_abs_max_weight_non_negative",
+            ),
+        ),
     ]

netbox/dcim/models/cables.py

@@ -22,7 +22,7 @@ from dcim.fields import PathField
 from dcim.utils import decompile_path_node, object_to_path_node, rebuild_cable_paths
 from netbox.choices import ColorChoices
 from netbox.models import ChangeLoggedModel, PrimaryModel
-from utilities.conversion import LENGTH_TO_METERS, normalized_unit_expression
+from utilities.conversion import LENGTH_TO_METERS, non_negative_constraint, normalized_unit_expression
 from utilities.data import normalize_update_fields
 from utilities.exceptions import AbortRequest
 from utilities.fields import ColorField, GenericArrayForeignKey
@@ -173,6 +173,9 @@ class Cable(PrimaryModel):
 
     class Meta:
         ordering = ('pk',)
+        constraints = (
+            non_negative_constraint('_abs_length'),
+        )
         verbose_name = _('cable')
         verbose_name_plural = _('cables')
 

netbox/dcim/models/cooling.py

@@ -7,6 +7,7 @@ from dcim.choices import *
 from dcim.models.mixins import MaxFlowMixin
 from netbox.models import PrimaryModel
 from netbox.models.features import ContactsMixin, ImageAttachmentsMixin
+from utilities.conversion import non_negative_constraint
 
 __all__ = (
     'CoolingFeed',
@@ -164,6 +165,7 @@ class CoolingFeed(MaxFlowMixin, PrimaryModel):
                 fields=('cooling_source', 'name'),
                 name='%(app_label)s_%(class)s_unique_cooling_source_name'
             ),
+            non_negative_constraint('_abs_max_flow'),
         )
         verbose_name = _('cooling feed')
         verbose_name_plural = _('cooling feeds')

netbox/dcim/models/device_component_templates.py

@@ -13,6 +13,7 @@ from dcim.utils import get_module_bay_positions, resolve_module_placeholder
 from netbox.models import ChangeLoggedModel
 from netbox.models.features import ChangeLoggingMixin
 from netbox.models.ltree import LtreeManager, LtreeModel
+from utilities.conversion import non_negative_constraint
 from utilities.exceptions import AbortRequest
 from utilities.fields import ColorField, NaturalOrderingField
 from utilities.ordering import naturalize_interface
@@ -459,6 +460,11 @@ class CoolingIntakeTemplate(DiameterMixin, MaxFlowMixin, ModularComponentTemplat
     component_model = CoolingIntake
 
     class Meta(ModularComponentTemplateModel.Meta):
+        constraints = (
+            *ModularComponentTemplateModel.Meta.constraints,
+            non_negative_constraint('_abs_diameter'),
+            non_negative_constraint('_abs_max_flow'),
+        )
         verbose_name = _('cooling intake template')
         verbose_name_plural = _('cooling intake templates')
 
@@ -515,6 +521,10 @@ class CoolingOutflowTemplate(DiameterMixin, ModularComponentTemplateModel):
     component_model = CoolingOutflow
 
     class Meta(ModularComponentTemplateModel.Meta):
+        constraints = (
+            *ModularComponentTemplateModel.Meta.constraints,
+            non_negative_constraint('_abs_diameter'),
+        )
         verbose_name = _('cooling outflow template')
         verbose_name_plural = _('cooling outflow templates')
 

netbox/dcim/models/device_components.py

@@ -24,6 +24,7 @@ from netbox.models import NetBoxModel, OrganizationalModel
 from netbox.models.features import ChangeLoggingMixin
 from netbox.models.ltree import LtreeManager, LtreeModel, SortPathField
 from netbox.models.mixins import OwnerMixin
+from utilities.conversion import non_negative_constraint
 from utilities.fields import ColorField, NaturalOrderingField
 from utilities.ordering import naturalize_interface
 from utilities.query_functions import CollateAsChar
@@ -734,6 +735,11 @@ class CoolingIntake(
     upstream_field = 'cooling_outflow'
 
     class Meta(ModularComponentModel.Meta):
+        constraints = (
+            *ModularComponentModel.Meta.constraints,
+            non_negative_constraint('_abs_diameter'),
+            non_negative_constraint('_abs_max_flow'),
+        )
         verbose_name = _('cooling intake')
         verbose_name_plural = _('cooling intakes')
 
@@ -770,6 +776,10 @@ class CoolingOutflow(CoolingLoopValidationMixin, DiameterMixin, ModularComponent
     upstream_field = 'cooling_intake'
 
     class Meta(ModularComponentModel.Meta):
+        constraints = (
+            *ModularComponentModel.Meta.constraints,
+            non_negative_constraint('_abs_diameter'),
+        )
         verbose_name = _('cooling outflow')
         verbose_name_plural = _('cooling outflows')
 

netbox/dcim/models/devices.py

@@ -27,6 +27,7 @@ from netbox.config import ConfigItem
 from netbox.models import NestedLtreeGroupModel, OrganizationalModel, PrimaryModel
 from netbox.models.features import ContactsMixin, ImageAttachmentsMixin
 from netbox.models.mixins import WeightMixin
+from utilities.conversion import non_negative_constraint
 from utilities.data import normalize_update_fields
 from utilities.exceptions import AbortRequest
 from utilities.fields import ColorField, CounterCacheField
@@ -232,6 +233,7 @@ class DeviceType(ImageAttachmentsMixin, PrimaryModel, WeightMixin):
                 fields=('manufacturer', 'slug'),
                 name='%(app_label)s_%(class)s_unique_manufacturer_slug'
             ),
+            non_negative_constraint('_abs_weight'),
         )
         verbose_name = _('device type')
         verbose_name_plural = _('device types')

netbox/dcim/models/modules.py

@@ -15,6 +15,7 @@ from extras.models import CustomField
 from netbox.models import PrimaryModel
 from netbox.models.features import ImageAttachmentsMixin
 from netbox.models.mixins import WeightMixin
+from utilities.conversion import non_negative_constraint
 from utilities.data import normalize_update_fields
 from utilities.exceptions import AbortRequest
 from utilities.fields import ColorField, CounterCacheField
@@ -235,6 +236,7 @@ class ModuleType(ImageAttachmentsMixin, PrimaryModel, WeightMixin):
                 fields=('manufacturer', 'model'),
                 name='%(app_label)s_%(class)s_unique_manufacturer_model'
             ),
+            non_negative_constraint('_abs_weight'),
         )
         indexes = (
             models.Index(fields=('profile', 'manufacturer', 'model')),  # Default ordering

netbox/dcim/models/racks.py

@@ -17,7 +17,7 @@ from netbox.choices import ColorChoices
 from netbox.models import OrganizationalModel, PrimaryModel
 from netbox.models.features import ContactsMixin, ImageAttachmentsMixin
 from netbox.models.mixins import WeightMixin
-from utilities.conversion import WEIGHT_TO_GRAMS, normalized_unit_expression
+from utilities.conversion import WEIGHT_TO_GRAMS, non_negative_constraint, normalized_unit_expression
 from utilities.data import array_to_string, drange
 from utilities.fields import ColorField, CounterCacheField
 from utilities.tracking import TrackingModelMixin
@@ -215,6 +215,8 @@ class RackType(ImageAttachmentsMixin, RackBase):
                 fields=('manufacturer', 'slug'),
                 name='%(app_label)s_%(class)s_unique_manufacturer_slug'
             ),
+            non_negative_constraint('_abs_weight'),
+            non_negative_constraint('_abs_max_weight'),
         )
         verbose_name = _('rack type')
         verbose_name_plural = _('rack types')
@@ -408,6 +410,8 @@ class Rack(ContactsMixin, ImageAttachmentsMixin, TrackingModelMixin, RackBase):
                 fields=('location', 'facility_id'),
                 name='%(app_label)s_%(class)s_unique_location_facility_id'
             ),
+            non_negative_constraint('_abs_weight'),
+            non_negative_constraint('_abs_max_weight'),
         )
         indexes = (
             models.Index(fields=('site', 'location', 'name', 'id')),  # Default ordering

netbox/dcim/tests/test_models.py

@@ -5,7 +5,7 @@ from decimal import Decimal
 from unittest.mock import patch
 
 from django.core.exceptions import ValidationError
-from django.db import connection, transaction
+from django.db import IntegrityError, connection, transaction
 from django.db.models import ProtectedError
 from django.db.models.signals import post_save
 from django.test import RequestFactory, TestCase, override_settings, tag
@@ -3355,6 +3355,16 @@ class CableTestCase(TestCase):
         self.assertIsNone(cable.length_unit)
         self.assertIsNone(cable._abs_length)
 
+    def test_negative_length_rejected_by_database(self):
+        """
+        A negative length must not reach the database, even through a write which skips validation.
+        """
+        cable = Cable.objects.first()
+        with self.assertRaises(IntegrityError), transaction.atomic():
+            Cable.objects.filter(pk=cable.pk).update(
+                length=Decimal('-1'), length_unit=CableLengthUnitChoices.UNIT_METER
+            )
+
     def test_partial_save_reads_the_stored_length_only_for_an_unwritten_length(self):
         """
         Only a save writing length_unit without length must read the row back, to decide whether to clear the
@@ -4760,3 +4770,32 @@ class CoolingComponentTestCase(TestCase):
         cooling_outflow = CoolingOutflow.objects.get(device=device, name='Cooling Outlet 1')
         self.assertIsNone(cooling_outflow.diameter)
         self.assertIsNone(cooling_outflow.diameter_unit)
+
+    def test_negative_measurements_rejected_by_database(self):
+        """
+        A negative diameter or max flow must not reach the database, even through a write which skips validation.
+        """
+        device_type = DeviceType.objects.create(
+            manufacturer=self.manufacturer, model='Device Type 9', slug='device-type-9'
+        )
+        device = Device.objects.create(
+            site=self.site, device_type=device_type, role=self.role, name='Device G'
+        )
+        writes = {
+            CoolingIntakeTemplate: CoolingIntakeTemplate.objects.create(device_type=device_type, name='Intake 1'),
+            CoolingOutflowTemplate: CoolingOutflowTemplate.objects.create(device_type=device_type, name='Outflow 1'),
+            CoolingIntake: CoolingIntake.objects.create(device=device, name='Intake 1'),
+            CoolingOutflow: CoolingOutflow.objects.create(device=device, name='Outflow 1'),
+        }
+        for model, instance in writes.items():
+            with self.subTest(model=model.__name__, field='diameter'):
+                with self.assertRaises(IntegrityError), transaction.atomic():
+                    model.objects.filter(pk=instance.pk).update(
+                        diameter=Decimal('-1'), diameter_unit=DiameterUnitChoices.UNIT_MILLIMETER
+                    )
+        for model in (CoolingIntakeTemplate, CoolingIntake):
+            with self.subTest(model=model.__name__, field='max_flow'):
+                with self.assertRaises(IntegrityError), transaction.atomic():
+                    model.objects.filter(pk=writes[model].pk).update(
+                        max_flow=Decimal('-1'), max_flow_unit=FlowRateUnitChoices.UNIT_LITERS_PER_MINUTE
+                    )

netbox/utilities/conversion.py

@@ -1,6 +1,6 @@
 from decimal import ROUND_HALF_UP, Decimal, InvalidOperation
 
-from django.db.models import BigIntegerField, Case, DecimalField, F, Value, When
+from django.db.models import BigIntegerField, Case, CheckConstraint, DecimalField, F, Q, Value, When
 from django.db.models.functions import Cast, Floor
 from django.utils.translation import gettext as _
 
@@ -16,6 +16,7 @@ __all__ = (
     'FLOW_RATE_TO_LITERS_PER_MINUTE',
     'LENGTH_TO_METERS',
     'WEIGHT_TO_GRAMS',
+    'non_negative_constraint',
     'normalized_unit_expression',
     'to_grams',
     'to_liters_per_minute',
@@ -82,6 +83,17 @@ def normalized_unit_expression(value_field, unit_field, factors, integer=False):
     return expression
 
 
+def non_negative_constraint(field_name):
+    """
+    Return a CheckConstraint rejecting a negative value in the given normalized column. Field validators run only
+    in full_clean(), so this keeps negative measurements out of the database on every write path.
+    """
+    return CheckConstraint(
+        condition=Q(**{f'{field_name}__gte': 0}),
+        name=f"%(app_label)s_%(class)s_{field_name.lstrip('_')}_non_negative",
+    )
+
+
 #
 # Python conversions
 #

netbox/utilities/testing/mixins.py

@@ -2,6 +2,7 @@ import itertools
 from decimal import Decimal
 
 from django.core.exceptions import ValidationError
+from django.db import IntegrityError, transaction
 from django.test.testcases import SerializeMixin
 from django_rq import get_queue
 from django_rq.workers import get_worker
@@ -151,13 +152,42 @@ class NormalizedMeasurementTestMixin:
 
     def test_negative_value_rejected(self):
         """
-        The database no longer rejects a negative measurement, so the source field's validator must.
+        A negative measurement must fail validation on the source field.
         """
         instance = self.create_instance(**{self.value_field: -self.value, self.unit_field: self.units[0]})
         with self.assertRaises(ValidationError) as cm:
             instance.full_clean()
         self.assertIn(self.value_field, cm.exception.error_dict)
 
+    def test_negative_value_rejected_by_database(self):
+        """
+        Write paths which skip validation must still be unable to store a negative measurement.
+        """
+        unit = self.units[0]
+        negative = {self.value_field: -self.value, self.unit_field: unit}
+        instance = self.create_instance(**{self.value_field: self.value, self.unit_field: unit})
+        instance.save()
+
+        def assert_rejected(label, write):
+            with self.subTest(write_path=label):
+                with self.assertRaises(IntegrityError), transaction.atomic():
+                    write()
+
+        def save():
+            for attr, value in negative.items():
+                setattr(instance, attr, value)
+            instance.save()
+
+        def bulk_update():
+            for attr, value in negative.items():
+                setattr(instance, attr, value)
+            self.model.objects.bulk_update([instance], list(negative))
+
+        assert_rejected('save', save)
+        assert_rejected('update', lambda: self.model.objects.filter(pk=instance.pk).update(**negative))
+        assert_rejected('bulk_update', bulk_update)
+        assert_rejected('bulk_create', lambda: self.model.objects.bulk_create([self.create_instance(**negative)]))
+
     def test_null_without_value_or_unit(self):
         unit = self.units[0]
         instance = self.create_instance(**{self.value_field: self.value, self.unit_field: unit})

netbox/wireless/migrations/0025_generated_fields.py

@@ -105,4 +105,11 @@ class Migration(migrations.Migration):
                 validators=[django.core.validators.MinValueValidator(0)],
             ),
         ),
+        migrations.AddConstraint(
+            model_name="wirelesslink",
+            constraint=models.CheckConstraint(
+                condition=models.Q(("_abs_distance__gte", 0)),
+                name="wireless_wirelesslink_abs_distance_non_negative",
+            ),
+        ),
     ]

netbox/wireless/models.py

@@ -8,6 +8,7 @@ from dcim.constants import WIRELESS_IFACE_TYPES
 from dcim.models.mixins import CachedScopeMixin
 from netbox.models import NestedLtreeGroupModel, PrimaryModel
 from netbox.models.mixins import DistanceMixin
+from utilities.conversion import non_negative_constraint
 
 from .choices import *
 from .constants import *
@@ -189,6 +190,7 @@ class WirelessLink(WirelessAuthenticationBase, DistanceMixin, PrimaryModel):
                 fields=('interface_a', 'interface_b'),
                 name='%(app_label)s_%(class)s_unique_interfaces'
             ),
+            non_negative_constraint('_abs_distance'),
         )
         verbose_name = _('wireless link')
         verbose_name_plural = _('wireless links')