fix(ui): Suppress unauthorized embedded object tables

Add a `should_render()` hook to the `Panel` base class and override it
in `ObjectsTablePanel` to check the requesting user's view permission
for the panel's model. This prevents object detail pages from issuing
HTMX requests for related tables (e.g. locations, devices, image
attachments) that return 403 and disrupt the page.

Fixes #21893

Co-authored-by: Jeremy Stretch <jstretch@netboxlabs.com>

netbox/core/views.py

@@ -192,6 +192,12 @@ class DataFileView(generic.ObjectView):
             layout.Column(
                 panels.DataFilePanel(),
                 panels.DataFileContentPanel(),
+                PluginContentPanel('left_page'),
+            ),
+        ),
+        layout.Row(
+            layout.Column(
+                PluginContentPanel('full_width_page'),
             ),
         ),
     )
@@ -253,6 +259,12 @@ class JobLogView(generic.ObjectView):
         layout.Row(
             layout.Column(
                 ContextTablePanel('table', title=_('Log Entries')),
+                PluginContentPanel('left_page'),
+            ),
+        ),
+        layout.Row(
+            layout.Column(
+                PluginContentPanel('full_width_page'),
             ),
         ),
     )
@@ -393,6 +405,12 @@ class ConfigRevisionView(generic.ObjectView):
             layout.Column(
                 TemplatePanel('core/panels/configrevision_data.html'),
                 TemplatePanel('core/panels/configrevision_comment.html'),
+                PluginContentPanel('left_page'),
+            ),
+        ),
+        layout.Row(
+            layout.Column(
+                PluginContentPanel('full_width_page'),
             ),
         ),
     )

netbox/dcim/forms/object_import.py

@@ -150,9 +150,25 @@ class PortTemplateMappingImportForm(forms.ModelForm):
     class Meta:
         model = PortTemplateMapping
         fields = [
-            'front_port', 'front_port_position', 'rear_port', 'rear_port_position',
+            'device_type', 'module_type', 'front_port', 'front_port_position', 'rear_port', 'rear_port_position',
         ]
 
+    def clean_device_type(self):
+        if device_type := self.cleaned_data['device_type']:
+            front_port = self.fields['front_port']
+            rear_port = self.fields['rear_port']
+            front_port.queryset = front_port.queryset.filter(device_type=device_type)
+            rear_port.queryset = rear_port.queryset.filter(device_type=device_type)
+        return device_type
+
+    def clean_module_type(self):
+        if module_type := self.cleaned_data['module_type']:
+            front_port = self.fields['front_port']
+            rear_port = self.fields['rear_port']
+            front_port.queryset = front_port.queryset.filter(module_type=module_type)
+            rear_port.queryset = rear_port.queryset.filter(module_type=module_type)
+        return module_type
+
 
 class ModuleBayTemplateImportForm(forms.ModelForm):
 

netbox/dcim/tests/test_views.py

@@ -194,6 +194,28 @@ class SiteTestCase(ViewTestCases.PrimaryObjectViewTestCase):
             'description': 'New description',
         }
 
+    def test_get_object_with_only_site_view_permission_hides_unauthorized_embedded_panels(self):
+        site = self._get_queryset().first()
+
+        obj_perm = ObjectPermission(
+            name='Test permission',
+            actions=['view'],
+        )
+        obj_perm.save()
+        obj_perm.users.add(self.user)
+        obj_perm.object_types.add(ObjectType.objects.get_for_model(self.model))
+
+        response = self.client.get(site.get_absolute_url())
+        self.assertHttpStatus(response, 200)
+
+        for panel, url in (
+            ('locations', reverse('dcim:location_list')),
+            ('devices', reverse('dcim:device_list')),
+            ('image attachments', reverse('extras:imageattachment_list')),
+        ):
+            with self.subTest(panel=panel):
+                self.assertNotContains(response, url)
+
 
 class LocationTestCase(ViewTestCases.OrganizationalObjectViewTestCase):
     model = Location

netbox/extras/api/serializers_/scripts.py

@@ -9,6 +9,7 @@ from rest_framework import serializers
 from core.api.serializers_.jobs import JobSerializer
 from core.choices import ManagedFileRootPathChoices
 from extras.models import Script, ScriptModule
+from extras.utils import validate_script_content
 from netbox.api.serializers import ValidatedModelSerializer
 from utilities.datetime import local_now
 
@@ -39,6 +40,15 @@ class ScriptModuleSerializer(ValidatedModelSerializer):
         data = super().validate(data)
         data.pop('file_root', None)
         if file is not None:
+            # Validate that the uploaded script can be loaded as a Python module
+            content = file.read()
+            file.seek(0)
+            try:
+                validate_script_content(content, file.name)
+            except Exception as e:
+                raise serializers.ValidationError(
+                    _("Error loading script: {error}").format(error=e)
+                )
             data['file'] = file
         return data
 

netbox/extras/forms/scripts.py

@@ -4,6 +4,7 @@ from django.utils.translation import gettext_lazy as _
 
 from core.choices import JobIntervalChoices
 from core.forms import ManagedFileForm
+from extras.utils import validate_script_content
 from utilities.datetime import local_now
 from utilities.forms.widgets import DateTimePicker, NumberWithOptions
 
@@ -64,6 +65,22 @@ class ScriptFileForm(ManagedFileForm):
     """
     ManagedFileForm with a custom save method to use django-storages.
     """
+    def clean(self):
+        super().clean()
+
+        if upload_file := self.cleaned_data.get('upload_file'):
+            # Validate that the uploaded script can be loaded as a Python module
+            content = upload_file.read()
+            upload_file.seek(0)
+            try:
+                validate_script_content(content, upload_file.name)
+            except Exception as e:
+                raise forms.ValidationError(
+                    _("Error loading script: {error}").format(error=e)
+                )
+
+        return self.cleaned_data
+
     def save(self, *args, **kwargs):
         # If a file was uploaded, save it to disk
         if self.cleaned_data['upload_file']:

netbox/extras/tests/test_api.py

@@ -1450,6 +1450,21 @@ class ScriptModuleTest(APITestCase):
         self.assertTrue(ScriptModule.objects.filter(file_path='test_upload.py').exists())
         self.assertTrue(Script.objects.filter(module__file_path='test_upload.py', name='TestScript').exists())
 
+    def test_upload_faulty_script_module(self):
+        """Uploading a script with an import error should return 400 and not create a DB record."""
+        self.add_permissions('extras.add_scriptmodule', 'core.add_managedfile')
+        # 'extras.script' is invalid; the correct module is 'extras.scripts'
+        script_content = b"from extras.script import Script\nclass TestScript(Script):\n    pass\n"
+        upload_file = SimpleUploadedFile('test_faulty.py', script_content, content_type='text/plain')
+        response = self.client.post(
+            self.url,
+            {'file': upload_file},
+            format='multipart',
+            **self.header,
+        )
+        self.assertHttpStatus(response, status.HTTP_400_BAD_REQUEST)
+        self.assertFalse(ScriptModule.objects.filter(file_path='test_faulty.py').exists())
+
     def test_upload_script_module_without_file_fails(self):
         self.add_permissions('extras.add_scriptmodule', 'core.add_managedfile')
         response = self.client.post(self.url, {}, format='json', **self.header)

netbox/extras/utils.py

@@ -1,4 +1,5 @@
 import importlib
+import types
 from pathlib import Path
 
 from django.core.exceptions import ImproperlyConfigured, SuspiciousFileOperation
@@ -21,6 +22,7 @@ __all__ = (
     'is_script',
     'is_taggable',
     'run_validators',
+    'validate_script_content',
 )
 
 
@@ -134,6 +136,17 @@ def is_script(obj):
         return False
 
 
+def validate_script_content(content, filename):
+    """
+    Validate that the given content can be loaded as a Python module by compiling
+    and executing it. Raises an exception if the script cannot be loaded.
+    """
+    code = compile(content, filename, 'exec')
+    module_name = Path(filename).stem
+    module = types.ModuleType(module_name)
+    exec(code, module.__dict__)
+
+
 def is_report(obj):
     """
     Returns True if the given object is a Report.

netbox/ipam/views.py

@@ -16,6 +16,7 @@ from netbox.ui.panels import (
     CommentsPanel,
     ContextTablePanel,
     ObjectsTablePanel,
+    PluginContentPanel,
     RelatedObjectsPanel,
     TemplatePanel,
 )
@@ -55,11 +56,13 @@ class VRFView(GetRelatedModelsMixin, generic.ObjectView):
             layout.Column(
                 panels.VRFPanel(),
                 TagsPanel(),
+                PluginContentPanel('left_page'),
             ),
             layout.Column(
                 RelatedObjectsPanel(),
                 CustomFieldsPanel(),
                 CommentsPanel(),
+                PluginContentPanel('right_page'),
             ),
         ),
         layout.Row(
@@ -70,6 +73,11 @@ class VRFView(GetRelatedModelsMixin, generic.ObjectView):
                 ContextTablePanel('export_targets_table', title=_('Export route targets')),
             ),
         ),
+        layout.Row(
+            layout.Column(
+                PluginContentPanel('full_width_page'),
+            ),
+        ),
     )
 
     def get_extra_context(self, request, instance):
@@ -169,10 +177,12 @@ class RouteTargetView(generic.ObjectView):
             layout.Column(
                 panels.RouteTargetPanel(),
                 TagsPanel(),
+                PluginContentPanel('left_page'),
             ),
             layout.Column(
                 CustomFieldsPanel(),
                 CommentsPanel(),
+                PluginContentPanel('right_page'),
             ),
         ),
         layout.Row(
@@ -207,6 +217,11 @@ class RouteTargetView(generic.ObjectView):
                 ),
             ),
         ),
+        layout.Row(
+            layout.Column(
+                PluginContentPanel('full_width_page'),
+            ),
+        ),
     )
 
 

netbox/netbox/tests/test_base_classes.py

@@ -45,6 +45,7 @@ from netbox.graphql.types import (
     PrimaryObjectType,
 )
 from netbox.models import NestedGroupModel, NetBoxModel, OrganizationalModel, PrimaryModel
+from netbox.registry import registry
 from netbox.tables import (
     NestedGroupModelTable,
     NetBoxTable,
@@ -174,11 +175,10 @@ class FilterSetClassesTestCase(TestCase):
     @staticmethod
     def get_filterset_for_model(model):
         """
-        Import and return the filterset class for a given model.
+        Return the filterset class for a given model from the application registry.
         """
-        app_label = model._meta.app_label
-        model_name = model.__name__
-        return import_string(f'{app_label}.filtersets.{model_name}FilterSet')
+        label = f'{model._meta.app_label}.{model._meta.model_name}'
+        return registry['filtersets'].get(label)
 
     @staticmethod
     def get_model_filterset_base_class(model):
@@ -204,6 +204,7 @@ class FilterSetClassesTestCase(TestCase):
         for model in apps.get_models():
             if base_class := self.get_model_filterset_base_class(model):
                 filterset = self.get_filterset_for_model(model)
+                self.assertIsNotNone(filterset, f"No registered filterset found for model {model}")
                 self.assertTrue(
                     issubclass(filterset, base_class),
                     f"{filterset} does not inherit from {base_class}",

netbox/netbox/tests/test_ui.py

@@ -1,4 +1,4 @@
-from django.test import TestCase
+from django.test import RequestFactory, TestCase
 
 from circuits.choices import CircuitStatusChoices, VirtualCircuitTerminationRoleChoices
 from circuits.models import (
@@ -8,9 +8,12 @@ from circuits.models import (
     VirtualCircuitTermination,
     VirtualCircuitType,
 )
+from core.models import ObjectType
 from dcim.choices import InterfaceTypeChoices
-from dcim.models import Interface
+from dcim.models import Interface, Site
 from netbox.ui import attrs
+from netbox.ui.panels import ObjectsTablePanel
+from users.models import ObjectPermission, User
 from utilities.testing import create_test_device
 from vpn.choices import (
     AuthenticationAlgorithmChoices,
@@ -213,3 +216,55 @@ class RelatedObjectListAttrTest(TestCase):
         self.assertInHTML('<li>IKE Proposal 2</li>', rendered)
         self.assertNotIn('IKE Proposal 3', rendered)
         self.assertIn('…', rendered)
+
+
+class ObjectsTablePanelTest(TestCase):
+    """
+    Verify that ObjectsTablePanel.should_render() hides the panel when
+    the requesting user lacks view permission for the panel's model.
+    """
+
+    @classmethod
+    def setUpTestData(cls):
+        cls.user = User.objects.create_user(username='test_user', password='test_password')
+
+        # Grant view permission only for Site
+        obj_perm = ObjectPermission.objects.create(
+            name='View sites only',
+            actions=['view'],
+        )
+        obj_perm.object_types.add(ObjectType.objects.get_for_model(Site))
+        obj_perm.users.add(cls.user)
+
+    def setUp(self):
+        self.factory = RequestFactory()
+        self.panel = ObjectsTablePanel(model='dcim.site')
+        self.panel_no_perm = ObjectsTablePanel(model='dcim.location')
+
+    def _make_context(self, user=None):
+        if user is None:
+            return {}
+        request = self.factory.get('/')
+        request.user = user
+        return {'request': request}
+
+    def test_should_render_without_request(self):
+        """
+        Panel should render when no request is present in context.
+        """
+        context = self.panel.get_context({})
+        self.assertTrue(self.panel.should_render(context))
+
+    def test_should_render_with_permission(self):
+        """
+        Panel should render when the user has view permission for the panel's model.
+        """
+        context = self.panel.get_context(self._make_context(self.user))
+        self.assertTrue(self.panel.should_render(context))
+
+    def test_should_not_render_without_permission(self):
+        """
+        Panel should be hidden when the user lacks view permission for the panel's model.
+        """
+        context = self.panel_no_perm.get_context(self._make_context(self.user))
+        self.assertFalse(self.panel_no_perm.should_render(context))

netbox/netbox/ui/panels.py

@@ -5,6 +5,7 @@ from django.utils.translation import gettext_lazy as _
 from netbox.ui import attrs
 from netbox.ui.actions import CopyContent
 from utilities.data import resolve_attr_path
+from utilities.permissions import get_permission_for_model
 from utilities.querydict import dict_to_querydict
 from utilities.string import title
 from utilities.templatetags.plugins import _get_registered_content
@@ -74,6 +75,15 @@ class Panel:
             'panel_class': self.__class__.__name__,
         }
 
+    def should_render(self, context):
+        """
+        Determines whether the panel should render on the page. (Default: True)
+
+        Parameters:
+            context (dict): The panel's prepared context (the return value of get_context())
+        """
+        return True
+
     def render(self, context):
         """
         Render the panel as HTML.
@@ -81,7 +91,10 @@ class Panel:
         Parameters:
             context (dict): The template context
         """
-        return render_to_string(self.template_name, self.get_context(context))
+        ctx = self.get_context(context)
+        if not self.should_render(ctx):
+            return ''
+        return render_to_string(self.template_name, ctx, request=ctx.get('request'))
 
 
 #
@@ -314,6 +327,16 @@ class ObjectsTablePanel(Panel):
             'url_params': dict_to_querydict(url_params),
         }
 
+    def should_render(self, context):
+        """
+        Hide the panel if the user does not have view permission for the panel's model.
+        """
+        request = context.get('request')
+        if request is None:
+            return True
+
+        return request.user.has_perm(get_permission_for_model(self.model, 'view'))
+
 
 class TemplatePanel(Panel):
     """

netbox/tenancy/api/views.py

@@ -42,13 +42,7 @@ class TenantViewSet(NetBoxModelViewSet):
 #
 
 class ContactGroupViewSet(MPTTLockedMixin, NetBoxModelViewSet):
-    queryset = ContactGroup.objects.add_related_count(
-        ContactGroup.objects.all(),
-        Contact,
-        'groups',
-        'contact_count',
-        cumulative=True
-    )
+    queryset = ContactGroup.objects.annotate_contacts()
     serializer_class = serializers.ContactGroupSerializer
     filterset_class = filtersets.ContactGroupFilterSet
 

netbox/tenancy/models/contacts.py

@@ -1,12 +1,14 @@
 from django.contrib.contenttypes.fields import GenericForeignKey
 from django.core.exceptions import ValidationError
 from django.db import models
+from django.db.models.expressions import RawSQL
 from django.urls import reverse
 from django.utils.translation import gettext_lazy as _
 
 from netbox.models import ChangeLoggedModel, NestedGroupModel, OrganizationalModel, PrimaryModel
 from netbox.models.features import CustomFieldsMixin, ExportTemplatesMixin, TagsMixin, has_feature
 from tenancy.choices import *
+from utilities.mptt import TreeManager
 
 __all__ = (
     'Contact',
@@ -16,10 +18,34 @@ __all__ = (
 )
 
 
+class ContactGroupManager(TreeManager):
+
+    def annotate_contacts(self):
+        """
+        Annotate the total number of Contacts belonging to each ContactGroup.
+
+        This returns both direct children and children of child groups. Raw SQL is used here to avoid double-counting
+        contacts which are assigned to multiple child groups of the parent.
+        """
+        return self.annotate(
+            contact_count=RawSQL(
+                "SELECT COUNT(DISTINCT m2m.contact_id)"
+                " FROM tenancy_contact_groups m2m"
+                " INNER JOIN tenancy_contactgroup cg ON m2m.contactgroup_id = cg.id"
+                " WHERE cg.tree_id = tenancy_contactgroup.tree_id"
+                " AND cg.lft >= tenancy_contactgroup.lft"
+                " AND cg.lft <= tenancy_contactgroup.rght",
+                ()
+            )
+        )
+
+
 class ContactGroup(NestedGroupModel):
     """
     An arbitrary collection of Contacts.
     """
+    objects = ContactGroupManager()
+
     class Meta:
         ordering = ['name']
         # Empty tuple triggers Django migration detection for MPTT indexes

netbox/tenancy/tests/test_models.py

@@ -0,0 +1,72 @@
+from django.test import TestCase
+
+from tenancy.models import Contact, ContactGroup
+
+
+class ContactGroupTestCase(TestCase):
+
+    @classmethod
+    def setUpTestData(cls):
+        # Create a tree of contact groups:
+        #  - Group A
+        #    - Group A1
+        #    - Group A2
+        #  - Group B
+        cls.group_a = ContactGroup.objects.create(name='Group A', slug='group-a')
+        cls.group_a1 = ContactGroup.objects.create(name='Group A1', slug='group-a1', parent=cls.group_a)
+        cls.group_a2 = ContactGroup.objects.create(name='Group A2', slug='group-a2', parent=cls.group_a)
+        cls.group_b = ContactGroup.objects.create(name='Group B', slug='group-b')
+
+        # Create contacts
+        cls.contact1 = Contact.objects.create(name='Contact 1')
+        cls.contact2 = Contact.objects.create(name='Contact 2')
+        cls.contact3 = Contact.objects.create(name='Contact 3')
+        cls.contact4 = Contact.objects.create(name='Contact 4')
+
+    def test_annotate_contacts_direct(self):
+        """Contacts assigned directly to a group should be counted."""
+        self.contact1.groups.set([self.group_a])
+        self.contact2.groups.set([self.group_a])
+
+        queryset = ContactGroup.objects.annotate_contacts()
+        self.assertEqual(queryset.get(pk=self.group_a.pk).contact_count, 2)
+
+    def test_annotate_contacts_cumulative(self):
+        """Contacts assigned to child groups should be included in the parent's count."""
+        self.contact1.groups.set([self.group_a1])
+        self.contact2.groups.set([self.group_a2])
+
+        queryset = ContactGroup.objects.annotate_contacts()
+        self.assertEqual(queryset.get(pk=self.group_a.pk).contact_count, 2)
+        self.assertEqual(queryset.get(pk=self.group_a1.pk).contact_count, 1)
+        self.assertEqual(queryset.get(pk=self.group_a2.pk).contact_count, 1)
+
+    def test_annotate_contacts_no_double_counting(self):
+        """A contact assigned to multiple child groups must be counted only once for the parent."""
+        self.contact1.groups.set([self.group_a1, self.group_a2])
+
+        queryset = ContactGroup.objects.annotate_contacts()
+        self.assertEqual(queryset.get(pk=self.group_a.pk).contact_count, 1)
+
+    def test_annotate_contacts_mixed(self):
+        """Test a mix of direct and inherited contacts with overlap."""
+        self.contact1.groups.set([self.group_a])
+        self.contact2.groups.set([self.group_a1])
+        self.contact3.groups.set([self.group_a1, self.group_a2])
+        self.contact4.groups.set([self.group_b])
+
+        queryset = ContactGroup.objects.annotate_contacts()
+        # Group A: contact1 (direct) + contact2 (via A1) + contact3 (via A1 & A2) = 3
+        self.assertEqual(queryset.get(pk=self.group_a.pk).contact_count, 3)
+        # Group A1: contact2 + contact3 = 2
+        self.assertEqual(queryset.get(pk=self.group_a1.pk).contact_count, 2)
+        # Group A2: contact3 = 1
+        self.assertEqual(queryset.get(pk=self.group_a2.pk).contact_count, 1)
+        # Group B: contact4 = 1
+        self.assertEqual(queryset.get(pk=self.group_b.pk).contact_count, 1)
+
+    def test_annotate_contacts_empty(self):
+        """Groups with no contacts should return a count of zero."""
+        queryset = ContactGroup.objects.annotate_contacts()
+        self.assertEqual(queryset.get(pk=self.group_a.pk).contact_count, 0)
+        self.assertEqual(queryset.get(pk=self.group_b.pk).contact_count, 0)

netbox/tenancy/views.py

@@ -205,13 +205,7 @@ class TenantBulkDeleteView(generic.BulkDeleteView):
 
 @register_model_view(ContactGroup, 'list', path='', detail=False)
 class ContactGroupListView(generic.ObjectListView):
-    queryset = ContactGroup.objects.add_related_count(
-        ContactGroup.objects.all(),
-        Contact,
-        'groups',
-        'contact_count',
-        cumulative=True
-    )
+    queryset = ContactGroup.objects.annotate_contacts()
     filterset = filtersets.ContactGroupFilterSet
     filterset_form = forms.ContactGroupFilterForm
     table = tables.ContactGroupTable
@@ -254,7 +248,7 @@ class ContactGroupView(GetRelatedModelsMixin, generic.ObjectView):
                 request,
                 groups,
                 extra=(
-                    (Contact.objects.restrict(request.user, 'view').filter(groups__in=groups), 'group_id'),
+                    (Contact.objects.restrict(request.user, 'view').filter(groups__in=groups).distinct(), 'group_id'),
                 ),
             ),
         }
@@ -280,13 +274,7 @@ class ContactGroupBulkImportView(generic.BulkImportView):
 
 @register_model_view(ContactGroup, 'bulk_edit', path='edit', detail=False)
 class ContactGroupBulkEditView(generic.BulkEditView):
-    queryset = ContactGroup.objects.add_related_count(
-        ContactGroup.objects.all(),
-        Contact,
-        'groups',
-        'contact_count',
-        cumulative=True
-    )
+    queryset = ContactGroup.objects.annotate_contacts()
     filterset = filtersets.ContactGroupFilterSet
     table = tables.ContactGroupTable
     form = forms.ContactGroupBulkEditForm
@@ -300,13 +288,7 @@ class ContactGroupBulkRenameView(generic.BulkRenameView):
 
 @register_model_view(ContactGroup, 'bulk_delete', path='delete', detail=False)
 class ContactGroupBulkDeleteView(generic.BulkDeleteView):
-    queryset = ContactGroup.objects.add_related_count(
-        ContactGroup.objects.all(),
-        Contact,
-        'groups',
-        'contact_count',
-        cumulative=True
-    )
+    queryset = ContactGroup.objects.annotate_contacts()
     filterset = filtersets.ContactGroupFilterSet
     table = tables.ContactGroupTable
 

netbox/utilities/security.py

@@ -9,7 +9,7 @@ def validate_peppers(peppers):
     """
     Validate the given dictionary of cryptographic peppers for type & sufficient length.
     """
-    if type(peppers) is not dict:
+    if not isinstance(peppers, dict):
         raise ImproperlyConfigured("API_TOKEN_PEPPERS must be a dictionary.")
     for key, pepper in peppers.items():
         if type(key) is not int: