Compare commits

..

1 Commits

Author SHA1 Message Date
Jason Novinger
7621def544 Fixes #20239: Prevent shared mutable state in PluginMenuItem and PluginMenuButton
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-08 15:40:24 -06:00
8 changed files with 4064 additions and 4288 deletions

View File

@@ -34,7 +34,7 @@ jobs:
- name: Set up Python
uses: actions/setup-python@v5
with:
python-version: 3.12
python-version: 3.11
- name: Install system dependencies
run: sudo apt install -y gettext

3
.gitignore vendored
View File

@@ -9,8 +9,7 @@ yarn-error.log*
/netbox/netbox/configuration.py
/netbox/netbox/ldap_config.py
/netbox/local/*
/netbox/media/*
!/netbox/media/.gitkeep
/netbox/media
/netbox/reports/*
!/netbox/reports/__init__.py
/netbox/scripts/*

View File

@@ -259,13 +259,11 @@ class Module(TrackingModelMixin, PrimaryModel, ConfigContextModel):
module_bays = []
modules = []
while module:
module_module_bay = getattr(module, "module_bay", None)
if module.pk in modules or (module_module_bay and module_module_bay.pk in module_bays):
if module.pk in modules or module.module_bay.pk in module_bays:
raise ValidationError(_("A module bay cannot belong to a module installed within it."))
modules.append(module.pk)
if module_module_bay:
module_bays.append(module_module_bay.pk)
module = module_module_bay.module if module_module_bay else None
module_bays.append(module.module_bay.pk)
module = module.module_bay.module if module.module_bay else None
def save(self, *args, **kwargs):
is_new = self.pk is None

View File

View File

@@ -3,7 +3,7 @@ from typing import TYPE_CHECKING
import strawberry_django
from strawberry import ID
from strawberry_django import ComparisonFilterLookup, FilterLookup
from strawberry_django import FilterLookup
from core.graphql.filter_mixins import ChangeLoggingMixin
from extras.graphql.filter_mixins import CustomFieldsFilterMixin, JournalEntriesFilterMixin, TagsFilterMixin
@@ -23,7 +23,7 @@ __all__ = (
@dataclass
class BaseModelFilter:
id: ComparisonFilterLookup[ID] | None = strawberry_django.filter_field()
id: FilterLookup[ID] | None = strawberry_django.filter_field()
class ChangeLoggedModelFilter(ChangeLoggingMixin, BaseModelFilter):

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'])

File diff suppressed because it is too large Load Diff