Fixes #22566: Avoid name collisions when custom script name matches core module (#22625)

netbox/extras/constants.py

@@ -28,6 +28,11 @@ IMAGE_ATTACHMENT_IMAGE_FORMATS = {
 # Template Export
 DEFAULT_MIME_TYPE = 'text/plain; charset=utf-8'
 
+# Scripts
+# Prefix applied to dynamically-loaded script/report module names so that a script whose filename
+# matches a core app or package (e.g. "circuits.py") cannot shadow it in sys.modules.
+SCRIPT_MODULE_NAME_PREFIX = '_netbox_script_module_'
+
 # Webhooks
 HTTP_CONTENT_TYPE_JSON = 'application/json'
 

netbox/extras/models/mixins.py

@@ -12,7 +12,7 @@ from django.utils.module_loading import import_string
 from django.utils.translation import gettext_lazy as _
 
 from core.models import ObjectType
-from extras.constants import DEFAULT_MIME_TYPE, JINJA_ENV_PARAMS_ALLOWED
+from extras.constants import DEFAULT_MIME_TYPE, JINJA_ENV_PARAMS_ALLOWED, SCRIPT_MODULE_NAME_PREFIX
 from extras.utils import filename_from_model, filename_from_object
 from utilities.jinja2 import render_jinja2
 
@@ -68,12 +68,16 @@ class PythonModuleMixin:
         Load the module using importlib, but use a custom loader to use django-storages
         instead of the file system.
         """
-        spec = importlib.util.spec_from_file_location(self.python_name, self.name)
+        # Load the module under a namespaced name rather than its bare name. Using the bare name
+        # (e.g. "circuits") would replace the like-named core app package in sys.modules, breaking
+        # app and migration graph resolution.
+        module_name = f'{SCRIPT_MODULE_NAME_PREFIX}{self.python_name}'
+        spec = importlib.util.spec_from_file_location(module_name, self.name)
         if spec is None:
             raise ModuleNotFoundError(f"Could not find module: {self.python_name}")
         loader = CustomStoragesLoader(self.name)
         module = importlib.util.module_from_spec(spec)
-        sys.modules[self.python_name] = module
+        sys.modules[module_name] = module
         loader.exec_module(module)
 
         return module

netbox/extras/scripts.py

@@ -12,6 +12,7 @@ from django.utils.translation import gettext as _
 
 from core.choices import JobNotificationChoices
 from extras.choices import LogLevelChoices
+from extras.constants import SCRIPT_MODULE_NAME_PREFIX
 from extras.models import ScriptModule
 from ipam.formfields import IPAddressFormField, IPNetworkFormField
 from ipam.validators import MaxPrefixLengthValidator, MinPrefixLengthValidator, prefix_validator
@@ -349,7 +350,12 @@ class BaseScript:
 
     @classproperty
     def module(self):
-        return self.__module__
+        # Strip the internal prefix applied when the module is loaded (see #22566) so that
+        # user-facing names (full_name, logger namespaces) reflect the original script filename.
+        name = self.__module__
+        if name.startswith(SCRIPT_MODULE_NAME_PREFIX):
+            name = name[len(SCRIPT_MODULE_NAME_PREFIX):]
+        return name
 
     @classproperty
     def class_name(self):
@@ -361,7 +367,7 @@ class BaseScript:
 
     @classmethod
     def root_module(cls):
-        return cls.__module__.split(".")[0]
+        return cls.module.split(".")[0]
 
     # Author-defined attributes
 

netbox/extras/tests/test_scripts.py

@@ -1,11 +1,16 @@
+import io
+import sys
 from datetime import UTC, date, datetime
 from decimal import Decimal
+from unittest.mock import patch
 
 from django.core.files.uploadedfile import SimpleUploadedFile
 from django.test import TestCase
 from netaddr import IPAddress, IPNetwork
 
 from dcim.models import DeviceRole
+from extras.constants import SCRIPT_MODULE_NAME_PREFIX
+from extras.models import ScriptModule
 from extras.scripts import *
 
 CHOICES = (
@@ -388,3 +393,49 @@ class ScriptVariablesTestCase(TestCase):
         self.assertEqual(form.cleaned_data['var1'], input_datetime)
         # Validate required=False works for this Var type
         self.assertEqual(form.cleaned_data['var2'], None)
+
+
+class ScriptModuleLoadingTestCase(TestCase):
+
+    def test_module_does_not_shadow_core_app(self):
+        """
+        Loading a custom script whose filename matches a core app label must not replace that
+        app's package in sys.modules. Regression test for issue #22566.
+        """
+        import circuits  # The real core app package
+
+        script_content = (
+            b"from extras.scripts import Script\n\n\n"
+            b"class TestScript(Script):\n    pass\n"
+        )
+
+        class _Storage:
+            def open(self, name, mode='rb'):
+                return io.BytesIO(script_content)
+
+        module = ScriptModule(file_root='scripts', file_path='circuits.py')
+        namespaced_key = f'{SCRIPT_MODULE_NAME_PREFIX}circuits'
+        self.addCleanup(lambda: sys.modules.pop(namespaced_key, None))
+
+        with patch('extras.models.mixins.storages') as mock_storages:
+            mock_storages.__getitem__.return_value = _Storage()
+            loaded = module.get_module()
+
+            # The script module is registered under the private, namespaced key, and its own
+            # __name__ matches that key (i.e. sys.modules[module.__name__] resolves to the module)
+            self.assertIs(sys.modules[namespaced_key], loaded)
+            self.assertEqual(loaded.__name__, namespaced_key)
+
+            # The namespacing must not leak into the derived script name stored in the database
+            self.assertEqual(next(iter(module.module_scripts)), 'TestScript')
+
+            # Nor into the user-facing names exposed on the Script class (used for logger
+            # namespaces, page headers, etc.): these must reflect the original filename.
+            script_class = loaded.TestScript
+            self.assertEqual(script_class.module, 'circuits')
+            self.assertEqual(script_class.full_name, 'circuits.TestScript')
+            self.assertEqual(script_class.root_module(), 'circuits')
+
+        # The real circuits app must be untouched and remain an importable package
+        self.assertIs(sys.modules['circuits'], circuits)
+        self.assertTrue(hasattr(circuits, '__path__'))