From 7621def544de9ec0010c606ed7ecbe140e8bbfa0 Mon Sep 17 00:00:00 2001 From: Jason Novinger Date: Thu, 8 Jan 2026 15:40:24 -0600 Subject: [PATCH] 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. --- netbox/netbox/plugins/navigation.py | 9 ++++-- netbox/netbox/tests/test_plugins.py | 45 ++++++++++++++++++++++++++++- 2 files changed, 50 insertions(+), 4 deletions(-) diff --git a/netbox/netbox/plugins/navigation.py b/netbox/netbox/plugins/navigation.py index 2b18a4a0e..2062e95d5 100644 --- a/netbox/netbox/plugins/navigation.py +++ b/netbox/netbox/plugins/navigation.py @@ -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.")) diff --git a/netbox/netbox/tests/test_plugins.py b/netbox/netbox/tests/test_plugins.py index 550dca514..a8595d10d 100644 --- a/netbox/netbox/tests/test_plugins.py +++ b/netbox/netbox/tests/test_plugins.py @@ -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'])