Compare commits

..

3 Commits

Author SHA1 Message Date
github-actions
c1bbc026e2 Update source translation strings
Some checks are pending
CodeQL / Analyze (actions) (push) Waiting to run
CodeQL / Analyze (javascript-typescript) (push) Waiting to run
CodeQL / Analyze (python) (push) Waiting to run
2026-01-15 05:05:36 +00:00
Jason Novinger
434334d927 Fixes #20239: Prevent shared mutable state in PluginMenuItem and PluginMenuButton (#21099)
Some checks failed
CI / build (20.x, 3.12) (push) Waiting to run
CI / build (20.x, 3.13) (push) Waiting to run
CI / build (20.x, 3.14) (push) Waiting to run
CodeQL / Analyze (actions) (push) Has been cancelled
CodeQL / Analyze (javascript-typescript) (push) Has been cancelled
CodeQL / Analyze (python) (push) Has been cancelled
PluginMenuItem and PluginMenuButton classes used mutable class-level
defaults for `permissions` and `buttons` attributes, causing permission
leakage between instances when these attributes were modified without
explicit parameters.

Changed to initialize these attributes as fresh lists per instance in
__init__ when not explicitly provided, following standard Python pattern
for avoiding mutable default arguments.
2026-01-14 12:50:35 -08:00
Jeremy Stretch
6bd083b7ed Closes #21142: Enable filtering device components by site/location/rack directly via GraphQL API (#21145) 2026-01-14 08:06:55 -06:00
6 changed files with 118 additions and 39 deletions

View File

@@ -140,6 +140,9 @@ class FrontPortFormMixin(forms.Form):
widget=forms.SelectMultiple(attrs={'size': 8})
)
port_mapping_model = PortMapping
parent_field = 'device'
def clean(self):
super().clean()
@@ -200,22 +203,3 @@ class FrontPortFormMixin(forms.Form):
using=connection,
update_fields=None
)
def _get_rear_port_choices(self, parent_filter, front_port):
"""
Return a list of choices representing each available rear port & position pair on the parent object (identified
by a Q filter), excluding those assigned to the specified instance.
"""
occupied_rear_port_positions = [
f'{mapping.rear_port_id}:{mapping.rear_port_position}'
for mapping in self.port_mapping_model.objects.filter(parent_filter).exclude(front_port=front_port.pk)
]
choices = []
for rear_port in self.rear_port_model.objects.filter(parent_filter):
for i in range(1, rear_port.positions + 1):
pair_id = f'{rear_port.pk}:{i}'
if pair_id not in occupied_rear_port_positions:
pair_label = f'{rear_port.name}:{i}'
choices.append((pair_id, pair_label))
return choices

View File

@@ -1124,8 +1124,9 @@ class FrontPortTemplateForm(FrontPortFormMixin, ModularComponentTemplateForm):
),
)
# Override FrontPortFormMixin attrs
port_mapping_model = PortTemplateMapping
rear_port_model = RearPortTemplate
parent_field = 'device_type'
class Meta:
model = FrontPortTemplate
@@ -1136,14 +1137,13 @@ class FrontPortTemplateForm(FrontPortFormMixin, ModularComponentTemplateForm):
def __init__(self, *args, **kwargs):
super().__init__(*args, **kwargs)
# Populate rear port choices based on parent DeviceType or ModuleType
if device_type_id := self.data.get('device_type') or self.initial.get('device_type'):
parent_filter = Q(device_type=device_type_id)
elif module_type_id := self.data.get('module_type') or self.initial.get('module_type'):
parent_filter = Q(module_type=module_type_id)
device_type = DeviceType.objects.get(pk=device_type_id)
else:
return
self.fields['rear_ports'].choices = self._get_rear_port_choices(parent_filter, self.instance)
# Populate rear port choices
self.fields['rear_ports'].choices = self._get_rear_port_choices(device_type, self.instance)
# Set initial rear port mappings
if self.instance.pk:
@@ -1152,6 +1152,27 @@ class FrontPortTemplateForm(FrontPortFormMixin, ModularComponentTemplateForm):
for mapping in PortTemplateMapping.objects.filter(front_port_id=self.instance.pk)
]
def _get_rear_port_choices(self, device_type, front_port):
"""
Return a list of choices representing each available rear port & position pair on the device type, excluding
those assigned to the specified instance.
"""
occupied_rear_port_positions = [
f'{mapping.rear_port_id}:{mapping.rear_port_position}'
for mapping in device_type.port_mappings.exclude(front_port=front_port.pk)
]
choices = []
for rear_port in RearPortTemplate.objects.filter(device_type=device_type):
for i in range(1, rear_port.positions + 1):
pair_id = f'{rear_port.pk}:{i}'
if pair_id not in occupied_rear_port_positions:
pair_label = f'{rear_port.name}:{i}'
choices.append(
(pair_id, pair_label)
)
return choices
class RearPortTemplateForm(ModularComponentTemplateForm):
fieldsets = (
@@ -1598,9 +1619,6 @@ class FrontPortForm(FrontPortFormMixin, ModularDeviceComponentForm):
),
)
port_mapping_model = PortMapping
rear_port_model = RearPort
class Meta:
model = FrontPort
fields = [
@@ -1611,12 +1629,13 @@ class FrontPortForm(FrontPortFormMixin, ModularDeviceComponentForm):
def __init__(self, *args, **kwargs):
super().__init__(*args, **kwargs)
# Populate rear port choices
if device_id := self.data.get('device') or self.initial.get('device'):
parent_filter = Q(device=device_id)
device = Device.objects.get(pk=device_id)
else:
return
self.fields['rear_ports'].choices = self._get_rear_port_choices(parent_filter, self.instance)
# Populate rear port choices
self.fields['rear_ports'].choices = self._get_rear_port_choices(device, self.instance)
# Set initial rear port mappings
if self.instance.pk:
@@ -1625,6 +1644,27 @@ class FrontPortForm(FrontPortFormMixin, ModularDeviceComponentForm):
for mapping in PortMapping.objects.filter(front_port_id=self.instance.pk)
]
def _get_rear_port_choices(self, device, front_port):
"""
Return a list of choices representing each available rear port & position pair on the device, excluding those
assigned to the specified instance.
"""
occupied_rear_port_positions = [
f'{mapping.rear_port_id}:{mapping.rear_port_position}'
for mapping in device.port_mappings.exclude(front_port=front_port.pk)
]
choices = []
for rear_port in RearPort.objects.filter(device=device):
for i in range(1, rear_port.positions + 1):
pair_id = f'{rear_port.pk}:{i}'
if pair_id not in occupied_rear_port_positions:
pair_label = f'{rear_port.name}:{i}'
choices.append(
(pair_id, pair_label)
)
return choices
class RearPortForm(ModularDeviceComponentForm):
fieldsets = (

View File

@@ -38,6 +38,15 @@ class ScopedFilterMixin:
@dataclass
class ComponentModelFilterMixin:
_site: Annotated['SiteFilter', strawberry.lazy('dcim.graphql.filters')] | None = (
strawberry_django.filter_field(name='site')
)
_location: Annotated['LocationFilter', strawberry.lazy('dcim.graphql.filters')] | None = (
strawberry_django.filter_field(name='location')
)
_rack: Annotated['RackFilter', strawberry.lazy('dcim.graphql.filters')] | None = (
strawberry_django.filter_field(name='rack')
)
device: Annotated['DeviceFilter', strawberry.lazy('dcim.graphql.filters')] | None = strawberry_django.filter_field()
device_id: ID | None = strawberry_django.filter_field()
name: FilterLookup[str] | None = strawberry_django.filter_field()

View File

@@ -37,8 +37,6 @@ class PluginMenuItem:
Alternatively, a pre-generated url can be set on the object which will be rendered literally.
Buttons are each specified as a list of PluginMenuButton instances.
"""
permissions = []
buttons = []
_url = None
def __init__(
@@ -54,10 +52,14 @@ class PluginMenuItem:
if type(permissions) not in (list, tuple):
raise TypeError(_("Permissions must be passed as a tuple or list."))
self.permissions = permissions
else:
self.permissions = []
if buttons is not None:
if type(buttons) not in (list, tuple):
raise TypeError(_("Buttons must be passed as a tuple or list."))
self.buttons = buttons
else:
self.buttons = []
@property
def url(self):
@@ -74,7 +76,6 @@ class PluginMenuButton:
ButtonColorChoices.
"""
color = ButtonColorChoices.DEFAULT
permissions = []
_url = None
def __init__(self, link, title, icon_class, color=None, permissions=None):
@@ -87,6 +88,8 @@ class PluginMenuButton:
if type(permissions) not in (list, tuple):
raise TypeError(_("Permissions must be passed as a tuple or list."))
self.permissions = permissions
else:
self.permissions = []
if color is not None:
if color not in ButtonColorChoices.values():
raise ValueError(_("Button color must be a choice within ButtonColorChoices."))

View File

@@ -11,7 +11,7 @@ from netbox.tests.dummy_plugin import config as dummy_config
from netbox.tests.dummy_plugin.data_backends import DummyBackend
from netbox.tests.dummy_plugin.jobs import DummySystemJob
from netbox.tests.dummy_plugin.webhook_callbacks import set_context
from netbox.plugins.navigation import PluginMenu
from netbox.plugins.navigation import PluginMenu, PluginMenuItem, PluginMenuButton
from netbox.plugins.utils import get_plugin_config
from netbox.graphql.schema import Query
from netbox.registry import registry
@@ -227,3 +227,46 @@ class PluginTest(TestCase):
Test the registration of webhook callbacks.
"""
self.assertIn(set_context, registry['webhook_callbacks'])
class PluginNavigationTest(TestCase):
def test_plugin_menu_item_independent_permissions(self):
item1 = PluginMenuItem(link='test1', link_text='Test 1')
item1.permissions.append('leaked_permission')
item2 = PluginMenuItem(link='test2', link_text='Test 2')
self.assertIsNot(item1.permissions, item2.permissions)
self.assertEqual(item1.permissions, ['leaked_permission'])
self.assertEqual(item2.permissions, [])
def test_plugin_menu_item_independent_buttons(self):
item1 = PluginMenuItem(link='test1', link_text='Test 1')
button = PluginMenuButton(link='button1', title='Button 1', icon_class='mdi-test')
item1.buttons.append(button)
item2 = PluginMenuItem(link='test2', link_text='Test 2')
self.assertIsNot(item1.buttons, item2.buttons)
self.assertEqual(len(item1.buttons), 1)
self.assertEqual(item1.buttons[0], button)
self.assertEqual(item2.buttons, [])
def test_plugin_menu_button_independent_permissions(self):
button1 = PluginMenuButton(link='button1', title='Button 1', icon_class='mdi-test')
button1.permissions.append('leaked_permission')
button2 = PluginMenuButton(link='button2', title='Button 2', icon_class='mdi-test')
self.assertIsNot(button1.permissions, button2.permissions)
self.assertEqual(button1.permissions, ['leaked_permission'])
self.assertEqual(button2.permissions, [])
def test_explicit_permissions_remain_independent(self):
item1 = PluginMenuItem(link='test1', link_text='Test 1', permissions=['explicit_permission'])
item2 = PluginMenuItem(link='test2', link_text='Test 2', permissions=['different_permission'])
self.assertIsNot(item1.permissions, item2.permissions)
self.assertEqual(item1.permissions, ['explicit_permission'])
self.assertEqual(item2.permissions, ['different_permission'])

View File

@@ -8,7 +8,7 @@ msgid ""
msgstr ""
"Project-Id-Version: PACKAGE VERSION\n"
"Report-Msgid-Bugs-To: \n"
"POT-Creation-Date: 2026-01-13 05:05+0000\n"
"POT-Creation-Date: 2026-01-15 05:05+0000\n"
"PO-Revision-Date: YEAR-MO-DA HO:MI+ZONE\n"
"Last-Translator: FULL NAME <EMAIL@ADDRESS>\n"
"Language-Team: LANGUAGE <LL@li.org>\n"
@@ -12489,8 +12489,8 @@ msgstr ""
msgid "Delete Selected"
msgstr ""
#: netbox/netbox/plugins/navigation.py:55
#: netbox/netbox/plugins/navigation.py:88
#: netbox/netbox/plugins/navigation.py:53
#: netbox/netbox/plugins/navigation.py:89
msgid "Permissions must be passed as a tuple or list."
msgstr ""
@@ -12498,7 +12498,7 @@ msgstr ""
msgid "Buttons must be passed as a tuple or list."
msgstr ""
#: netbox/netbox/plugins/navigation.py:92
#: netbox/netbox/plugins/navigation.py:95
msgid "Button color must be a choice within ButtonColorChoices."
msgstr ""