From d413b847abe8e87267877cb5021ac09c893ead11 Mon Sep 17 00:00:00 2001 From: Martin Hauser Date: Fri, 24 Apr 2026 14:14:48 +0200 Subject: [PATCH] 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 --- netbox/extras/events.py | 19 ++++++++++++++- netbox/extras/models/models.py | 4 ++++ netbox/extras/tests/test_event_rules.py | 27 +++++++++++++++++++++ netbox/extras/tests/test_models.py | 32 ++++++++++++++++++++++++- 4 files changed, 80 insertions(+), 2 deletions(-) diff --git a/netbox/extras/events.py b/netbox/extras/events.py index 55e8b83e7..470adc875 100644 --- a/netbox/extras/events.py +++ b/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']): 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. # 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 if event_rule.action_type == EventRuleActionChoices.WEBHOOK: diff --git a/netbox/extras/models/models.py b/netbox/extras/models/models.py index 8aca58a08..b117ab6f0 100644 --- a/netbox/extras/models/models.py +++ b/netbox/extras/models/models.py @@ -143,6 +143,10 @@ class EventRule(CustomFieldsMixin, ExportTemplatesMixin, OwnerMixin, TagsMixin, except ValueError as 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): """ Test whether the given data meets the conditions of the event rule (if any). Return True diff --git a/netbox/extras/tests/test_event_rules.py b/netbox/extras/tests/test_event_rules.py index ae3f802c3..8426a3fbb 100644 --- a/netbox/extras/tests/test_event_rules.py +++ b/netbox/extras/tests/test_event_rules.py @@ -563,3 +563,30 @@ class EventRuleTest(APITestCase): job = self.queue.get_jobs()[0] self.assertEqual(job.kwargs['event_type'], OBJECT_DELETED) 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) diff --git a/netbox/extras/tests/test_models.py b/netbox/extras/tests/test_models.py index 7a2bbed5c..626cd219e 100644 --- a/netbox/extras/tests/test_models.py +++ b/netbox/extras/tests/test_models.py @@ -11,9 +11,18 @@ from django.forms import ValidationError from django.test import TestCase, tag from PIL import Image +from core.events import OBJECT_CREATED from core.models import AutoSyncRecord, DataSource, ObjectType 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 utilities.exceptions import AbortRequest from virtualization.models import Cluster, ClusterGroup, ClusterType, VirtualMachine @@ -889,3 +898,24 @@ class ConfigTemplateTest(TestCase): object_id=config_template.pk ) 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)