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

fix(extras): Validate EventRule action_data is a dict or null

Add validation in EventRule.clean() to ensure action_data is a JSON
object or null. Add runtime guard in event processing to handle legacy
rows with invalid data by logging a warning and using an empty dict.

Fixes #21989
Martin Hauser 1 месяц назад
Родитель
Сommit
d413b847ab

+ 18 - 1
netbox/extras/events.py

@@ -181,9 +181,26 @@ def process_event_rules(event_rules, object_type, event):
         if not event_rule.eval_conditions(event['data']):
         if not event_rule.eval_conditions(event['data']):
             continue
             continue
 
 
+        # Guard against action_data that is valid JSON but not a dict
+        # (e.g. a bare string or number). Existing rows with bad data are
+        # tolerated at runtime; validation on EventRule.clean() prevents
+        # new ones.
+        if event_rule.action_data is None:
+            action_data = {}
+        elif isinstance(event_rule.action_data, dict):
+            action_data = event_rule.action_data
+        else:
+            logger.warning(
+                _('Ignoring invalid action_data on event rule "{rule}" (got {data_type})').format(
+                    rule=event_rule,
+                    data_type=type(event_rule.action_data).__name__,
+                )
+            )
+            action_data = {}
+
         # Merge rule-specific action_data with the event payload.
         # Merge rule-specific action_data with the event payload.
         # Copy to avoid mutating the rule's stored action_data dict.
         # Copy to avoid mutating the rule's stored action_data dict.
-        event_data = {**(event_rule.action_data or {}), **event['data']}
+        event_data = {**action_data, **event['data']}
 
 
         # Webhooks
         # Webhooks
         if event_rule.action_type == EventRuleActionChoices.WEBHOOK:
         if event_rule.action_type == EventRuleActionChoices.WEBHOOK:

+ 4 - 0
netbox/extras/models/models.py

@@ -143,6 +143,10 @@ class EventRule(CustomFieldsMixin, ExportTemplatesMixin, OwnerMixin, TagsMixin,
             except ValueError as e:
             except ValueError as e:
                 raise ValidationError({'conditions': e})
                 raise ValidationError({'conditions': e})
 
 
+        # action_data must be a JSON object (or null)
+        if self.action_data is not None and not isinstance(self.action_data, dict):
+            raise ValidationError({'action_data': _('Action data must be a JSON object or null.')})
+
     def eval_conditions(self, data):
     def eval_conditions(self, data):
         """
         """
         Test whether the given data meets the conditions of the event rule (if any). Return True
         Test whether the given data meets the conditions of the event rule (if any). Return True

+ 27 - 0
netbox/extras/tests/test_event_rules.py

@@ -563,3 +563,30 @@ class EventRuleTest(APITestCase):
         job = self.queue.get_jobs()[0]
         job = self.queue.get_jobs()[0]
         self.assertEqual(job.kwargs['event_type'], OBJECT_DELETED)
         self.assertEqual(job.kwargs['event_type'], OBJECT_DELETED)
         self.queue.empty()
         self.queue.empty()
+
+    def test_non_dict_action_data_does_not_crash_flush(self):
+        """
+        Pre-existing non-dict action_data must not cause flush_events() to
+        raise.
+        """
+        site_type = ObjectType.objects.get_for_model(Site)
+        webhook = Webhook.objects.get(name='Webhook 1')
+        webhook_type = ObjectType.objects.get_for_model(Webhook)
+
+        bad_rule = EventRule.objects.create(
+            name='Bad action_data rule',
+            event_types=[OBJECT_CREATED],
+            action_type=EventRuleActionChoices.WEBHOOK,
+            action_object_type=webhook_type,
+            action_object_id=webhook.pk,
+            action_data={},
+        )
+        bad_rule.object_types.set([site_type])
+
+        # Simulate a legacy row that predates model validation.
+        EventRule.objects.filter(pk=bad_rule.pk).update(action_data='not a dict')
+
+        url = reverse('dcim-api:site-list')
+        self.add_permissions('dcim.add_site')
+        response = self.client.post(url, {'name': 'Site X', 'slug': 'site-x'}, format='json', **self.header)
+        self.assertHttpStatus(response, status.HTTP_201_CREATED)

+ 31 - 1
netbox/extras/tests/test_models.py

@@ -11,9 +11,18 @@ from django.forms import ValidationError
 from django.test import TestCase, tag
 from django.test import TestCase, tag
 from PIL import Image
 from PIL import Image
 
 
+from core.events import OBJECT_CREATED
 from core.models import AutoSyncRecord, DataSource, ObjectType
 from core.models import AutoSyncRecord, DataSource, ObjectType
 from dcim.models import Device, DeviceRole, DeviceType, Location, Manufacturer, Platform, Region, Site, SiteGroup
 from dcim.models import Device, DeviceRole, DeviceType, Location, Manufacturer, Platform, Region, Site, SiteGroup
-from extras.models import ConfigContext, ConfigContextProfile, ConfigTemplate, ImageAttachment, Tag, TaggedItem
+from extras.models import (
+    ConfigContext,
+    ConfigContextProfile,
+    ConfigTemplate,
+    EventRule,
+    ImageAttachment,
+    Tag,
+    TaggedItem,
+)
 from tenancy.models import Tenant, TenantGroup
 from tenancy.models import Tenant, TenantGroup
 from utilities.exceptions import AbortRequest
 from utilities.exceptions import AbortRequest
 from virtualization.models import Cluster, ClusterGroup, ClusterType, VirtualMachine
 from virtualization.models import Cluster, ClusterGroup, ClusterType, VirtualMachine
@@ -889,3 +898,24 @@ class ConfigTemplateTest(TestCase):
                 object_id=config_template.pk
                 object_id=config_template.pk
             )
             )
             self.assertEqual(autosync_records.count(), 0, "AutoSyncRecord should be deleted after detaching")
             self.assertEqual(autosync_records.count(), 0, "AutoSyncRecord should be deleted after detaching")
+
+
+class EventRuleTest(TestCase):
+
+    def test_action_data_clean_accepts_dict(self):
+        """
+        clean() should accept a JSON object (or null) as action_data.
+        """
+        for value in ({'key': 'value'}, None):
+            rule = EventRule(name='test', event_types=[OBJECT_CREATED], action_data=value)
+            rule.clean()
+
+    def test_action_data_clean_rejects_non_dict(self):
+        """
+        clean() should reject action_data that is valid JSON but not an object (#21989).
+        """
+        for value in ('test', 42, [1, 2, 3], True):
+            rule = EventRule(name='test', event_types=[OBJECT_CREATED], action_data=value)
+            with self.assertRaises(ValidationError) as cm:
+                rule.clean()
+            self.assertIn('action_data', cm.exception.message_dict)