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
15 changed files with 4074 additions and 4351 deletions

View File

@@ -1,26 +1,20 @@
---
name: Deprecation
name: 🗑 Deprecation
type: Deprecation
description: Designation of a feature or behavior that will be removed in a future release
description: The removal of an existing feature or resource
labels: ["netbox", "type: deprecation"]
body:
- type: textarea
attributes:
label: Deprecated Functionality
label: Proposed Changes
description: >
Describe the feature(s) and/or behavior that is being flagged for deprecation.
validations:
required: true
- type: input
attributes:
label: Scheduled removal
description: In what future release will the deprecated functionality be removed?
Describe in detail the proposed changes. What is being removed?
validations:
required: true
- type: textarea
attributes:
label: Justification
description: Please provide justification for the deprecation.
description: Please provide justification for the proposed change(s).
validations:
required: true
- type: textarea

View File

@@ -1,20 +0,0 @@
---
name: 🗑️ Feature Removal
type: Removal
description: The removal of a deprecated feature or resource
labels: ["netbox", "type: removal"]
body:
- type: input
attributes:
label: Deprecation Issue
description: Specify the issue in which this deprecation was announced.
placeholder: "#1234"
validations:
required: true
- type: textarea
attributes:
label: Summary of Changes
description: >
List all changes necessary to remove the deprecated feature or resource.
validations:
required: true

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

@@ -372,8 +372,8 @@ class IPAddressForm(TenancyForm, PrimaryModelForm):
'virtual_machine_id': instance.assigned_object.virtual_machine.pk,
})
# Disable object assignment fields if the IP address is designated as primary or OOB
if self.initial.get('primary_for_parent') or self.initial.get('oob_for_parent'):
# Disable object assignment fields if the IP address is designated as primary
if self.initial.get('primary_for_parent'):
self.fields['interface'].disabled = True
self.fields['vminterface'].disabled = True
self.fields['fhrpgroup'].disabled = True

View File

@@ -940,13 +940,6 @@ class IPAddress(ContactsMixin, PrimaryModel):
_("Cannot reassign IP address while it is designated as the primary IP for the parent object")
)
# can't use is_oob_ip as self.assigned_object might be changed
if hasattr(original_parent, 'oob_ip') and original_parent.oob_ip_id == self.pk:
if parent != original_parent:
raise ValidationError(
_("Cannot reassign IP address while it is designated as the OOB IP for the parent object")
)
# Validate IP status selection
if self.status == IPAddressStatusChoices.STATUS_SLAAC and self.family != 6:
raise ValidationError({

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 it is too large Load Diff

View File

@@ -123,7 +123,7 @@ class UserTokenForm(forms.ModelForm):
token = forms.CharField(
label=_('Token'),
help_text=_(
'Tokens must be at least 40 characters in length. <strong>Be sure to record your token</strong> prior to '
'Tokens must be at least 40 characters in length. <strong>Be sure to record your key</strong> prior to '
'submitting this form, as it will no longer be accessible once the token has been created.'
),
widget=forms.TextInput(

View File

@@ -69,7 +69,7 @@ class Token(models.Model):
write_enabled = models.BooleanField(
verbose_name=_('write enabled'),
default=True,
help_text=_('Permit create/update/delete operations using this token')
help_text=_('Permit create/update/delete operations using this key')
)
# For legacy v1 tokens, this field stores the plaintext 40-char token value. Not used for v2.
plaintext = models.CharField(
@@ -213,9 +213,6 @@ class Token(models.Model):
def clean(self):
super().clean()
if self.version == TokenVersionChoices.V2 and not settings.API_TOKEN_PEPPERS:
raise ValidationError(_("Unable to save v2 tokens: API_TOKEN_PEPPERS is not defined."))
if self._state.adding:
if self.pepper_id is not None and self.pepper_id not in settings.API_TOKEN_PEPPERS:
raise ValidationError(_(

View File

@@ -1,10 +1,9 @@
from datetime import timedelta
from django.core.exceptions import ValidationError
from django.test import TestCase, override_settings
from django.test import TestCase
from django.utils import timezone
from users.choices import TokenVersionChoices
from users.models import User, Token
from utilities.testing import create_test_user
@@ -95,15 +94,6 @@ class TokenTest(TestCase):
token.refresh_from_db()
self.assertEqual(token.description, 'New Description')
@override_settings(API_TOKEN_PEPPERS={})
def test_v2_without_peppers_configured(self):
"""
Attempting to save a v2 token without API_TOKEN_PEPPERS defined should raise a ValidationError.
"""
token = Token(version=TokenVersionChoices.V2)
with self.assertRaises(ValidationError):
token.clean()
class UserConfigTest(TestCase):