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
12 changed files with 4070 additions and 4294 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

@@ -232,7 +232,7 @@ VPN_MENU = Menu(
label=_('L2VPNs'),
items=(
get_model_item('vpn', 'l2vpn', _('L2VPNs')),
get_model_item('vpn', 'l2vpntermination', _('L2VPN Terminations')),
get_model_item('vpn', 'l2vpntermination', _('Terminations')),
),
),
MenuGroup(

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 one or more lines are too long

View File

@@ -1 +1 @@
.docExplorerWrap{height:unset!important;min-width:unset!important;width:unset!important}.docExplorerWrap svg{display:unset}.doc-explorer-title{font-size:var(--font-size-h2);font-weight:var(--font-weight-medium)}.doc-explorer-rhs{display:none}.graphiql-explorer-root{font-family:var(--font-family-mono)!important;font-size:var(--font-size-body)!important;padding:0!important}.graphiql-explorer-root>div>div{padding-top:var(--px-16);border-color:hsla(var(--color-neutral),var(--alpha-background-heavy))!important}.graphiql-explorer-root>div{overflow:auto!important}.graphiql-explorer-root input{background:unset}.graphiql-explorer-root select{border:1px solid hsla(var(--color-neutral),var(--alpha-secondary));border-radius:var(--border-radius-4);margin:0 var(--px-8);padding:var(--px-4)var(--px-6);background:hsl(var(--color-base))!important;color:hsl(var(--color-neutral))!important}.toolbar-button{all:unset;cursor:pointer;margin-left:var(--px-6);color:hsl(var(--color-primary));line-height:0!important;font-size:var(--font-size-h3)!important}.graphiql-explorer-slug .toolbar-button,.graphiql-explorer-graphql-arguments .toolbar-button{font-size:inherit!important}.graphiql-explorer-graphql-arguments input{min-width:2rem;line-height:0}.graphiql-explorer-actions{border-color:hsla(var(--color-neutral),var(--alpha-background-heavy))!important}
.docExplorerWrap{height:unset!important;min-width:unset!important;width:unset!important}.docExplorerWrap svg{display:unset}.doc-explorer-title{font-size:var(--font-size-h2);font-weight:var(--font-weight-medium)}.doc-explorer-rhs{display:none}.graphiql-explorer-root{font-family:var(--font-family-mono)!important;font-size:var(--font-size-body)!important;padding:0!important}.graphiql-explorer-root>div>div{border-color:hsla(var(--color-neutral),var(--alpha-background-heavy))!important;padding-top:var(--px-16)}.graphiql-explorer-root input{background:unset}.graphiql-explorer-root select{background:hsl(var(--color-base))!important;border:1px solid hsla(var(--color-neutral),var(--alpha-secondary));border-radius:var(--border-radius-4);color:hsl(var(--color-neutral))!important;margin:0 var(--px-8);padding:var(--px-4) var(--px-6)}.graphiql-operation-title-bar .toolbar-button{line-height:0;margin-left:var(--px-8);color:hsla(var(--color-neutral),var(--alpha-secondary, .6));font-size:var(--font-size-h3);vertical-align:middle}.graphiql-explorer-graphql-arguments input{line-height:0}.graphiql-explorer-actions{border-color:hsla(var(--color-neutral),var(--alpha-background-heavy))!important}

View File

@@ -6,7 +6,7 @@
"license": "Apache-2.0",
"private": true,
"dependencies": {
"@graphiql/plugin-explorer": "4.0.6",
"@graphiql/plugin-explorer": "3.2.6",
"graphiql": "4.1.2",
"graphql": "16.12.0",
"js-cookie": "3.0.5",

View File

@@ -294,10 +294,10 @@
react-compiler-runtime "19.1.0-rc.1"
zustand "^5"
"@graphiql/plugin-explorer@4.0.6":
version "4.0.6"
resolved "https://registry.yarnpkg.com/@graphiql/plugin-explorer/-/plugin-explorer-4.0.6.tgz#bec1207dc27334914590ab31f46c2e944bbf4ebf"
integrity sha512-TppIi92YPER3v70nlF01KTQrq9AiYqkZicSd1hpU7aqGmbqw/pLwBNLUEcfENBoJtw574Qxjswb01+GaYK0Tzw==
"@graphiql/plugin-explorer@3.2.6":
version "3.2.6"
resolved "https://registry.npmjs.org/@graphiql/plugin-explorer/-/plugin-explorer-3.2.6.tgz"
integrity sha512-MXzG/zVNzZfes4Em253bHyAbD/lwwAZkPKvxCAQkjz0i3dtcv4uF3D8iqJ7214iu3SCphbORYZZUC93fik1yew==
dependencies:
graphiql-explorer "^0.9.0"

File diff suppressed because it is too large Load Diff