mirror of
https://github.com/netbox-community/netbox.git
synced 2026-01-11 14:22:16 -06:00
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.
This commit is contained in:
@@ -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."))
|
||||
|
||||
@@ -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'])
|
||||
|
||||
Reference in New Issue
Block a user