Fixes #23511: Show correct filepath in script source tab (#23560)

* Fixes #23511: Show correct filepath in script source tab

The Source tab displays the Script's filepath via BaseScript.filename,
which resolves to the loaded module's __file__ attribute.
PythonModuleMixin.get_module() builds the module spec from the bare
storage key, and spec_from_file_location() resolves that against the
process' working dir, so every script showed a nonexistent local path
(e.g. <CWD>/test_script.py).

Record the script's actual location in the scripts storage backend in
the module spec before creating the module: the absolute path for local
file-system storage, and the storage key for remote backends.
module.__file__ - and therefore BaseScript.filename - then reports that
location, so the Source tab displays it without template changes.

* fix(tests): Use patch for python_class

Fix the handful of test cases which reassigned Script.python_class
without restoring it, and potentially broke later tests in the same
process.

* fix(scripts): Identification of scripts in tracebacks

CustomStoragesLoader.exec_module() exec'd the raw script bytes without a
source filename, so every code object in a loaded script carried
co_filename '<string>' and tracebacks from script failures showed
`File "<string>"` regardless of the storage backend.

Compile with the module's __file__ (the script's location in the scripts
storage backend, set by PythonModuleMixin.get_module) as the source
filename, so load-time SyntaxErrors and tracebacks consistently identify
the script.

netbox/extras/models/mixins.py

@@ -40,7 +40,7 @@ class CustomStoragesLoader(importlib.abc.Loader):
     def exec_module(self, module):
         with storages["scripts"].open(self.filename, 'rb') as f:
             code = f.read()
-        exec(code, module.__dict__)
+        exec(compile(code, module.__file__, 'exec'), module.__dict__)
 
 
 class PythonModuleMixin:
@@ -80,6 +80,7 @@ class PythonModuleMixin:
         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}")
+        spec.origin = self.full_path
         loader = CustomStoragesLoader(self.name)
         module = importlib.util.module_from_spec(spec)
         sys.modules[module_name] = module

netbox/extras/tests/test_api.py

@@ -2215,16 +2215,14 @@ class ScriptTestCase(APITestCase):
         )
         cls.url = reverse('extras-api:script-detail', kwargs={'pk': script.pk})
 
-    @property
-    def python_class(self):
-        return self.TestScriptClass
-
     def setUp(self):
         super().setUp()
         self.add_permissions('extras.view_script')
 
         # Monkey-patch the Script model to return our TestScriptClass above
-        Script.python_class = self.python_class
+        python_class_patch = patch.object(Script, 'python_class', new=self.TestScriptClass)
+        python_class_patch.start()
+        self.addCleanup(python_class_patch.stop)
 
         # The script-run endpoint gates on a live RQ worker. Tests run without
         # one, so bypass the check to exercise validation and the enqueue path.

netbox/extras/tests/test_scripts.py

@@ -1,10 +1,14 @@
+import inspect
 import io
+import os
 import sys
+import traceback
 import uuid
 from datetime import UTC, date, datetime
 from decimal import Decimal
 from unittest.mock import PropertyMock, patch
 
+from django.conf import settings
 from django.contrib.auth import get_user_model
 from django.core.exceptions import ValidationError
 from django.core.files.uploadedfile import SimpleUploadedFile
@@ -477,6 +481,100 @@ class ScriptModuleLoadingTestCase(TestCase):
             script.log_success('Start')
         self.assertIn('Start', captured.output[0])
 
+    def test_module_file_is_filesystem_path_for_local_storage(self):
+        """
+        For the local file-system storage backend the display location must be the script's
+        actual filesystem path (<SCRIPTS_ROOT>/<file_path>), not a path derived from the
+        current working directory (#23511).
+        """
+        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='example.py')
+        namespaced_key = f'{SCRIPT_MODULE_NAME_PREFIX}example'
+        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()
+
+        self.assertEqual(loaded.__file__, module.full_path)
+        self.assertEqual(loaded.__file__, os.path.join(settings.SCRIPTS_ROOT, 'example.py'))
+        self.assertEqual(inspect.getfile(loaded.TestScript), module.full_path)
+
+    def test_module_file_is_storage_name_for_remote_storage(self):
+        """
+        For non-local storage backends (e.g. S3) the display location must be the storage name
+        (the key within the storage), not a local filesystem path (#23511).
+        """
+        script_content = (
+            b"from extras.scripts import Script\n\n\n"
+            b"class TestScript(Script):\n    pass\n"
+        )
+
+        class _Storage:
+            # Deliberately not a ScriptFileSystemStorage: emulates a remote backend (e.g. S3)
+            def open(self, name, mode='rb'):
+                return io.BytesIO(script_content)
+
+        storage = _Storage()
+        module = ScriptModule(file_root='scripts', file_path='example.py')
+        namespaced_key = f'{SCRIPT_MODULE_NAME_PREFIX}example'
+        self.addCleanup(lambda: sys.modules.pop(namespaced_key, None))
+
+        with (
+            patch('core.models.files.storages') as core_storages,
+            patch('extras.models.mixins.storages') as mock_storages,
+        ):
+            core_storages.backends = {'scripts': {}}
+            core_storages.create_storage.return_value = storage
+            mock_storages.__getitem__.return_value = storage
+            # No local root is prepended for non-local backends
+            self.assertEqual(module.full_path, 'example.py')
+            loaded = module.get_module()
+
+        self.assertEqual(loaded.__file__, 'example.py')
+
+    def test_code_objects_named_after_storage_location(self):
+        """
+        Code objects of a loaded script must be named after the script's storage location,
+        so tracebacks identify the script instead of showing '<string>' (#23511).
+        """
+        script_content = (
+            b"from extras.scripts import Script\n\n\n"
+            b"class TestScript(Script):\n"
+            b"    def run(self):\n"
+            b"        raise ValueError('boom')\n"
+        )
+
+        class _Storage:
+            def open(self, name, mode='rb'):
+                return io.BytesIO(script_content)
+
+        module = ScriptModule(file_root='scripts', file_path='example.py')
+        namespaced_key = f'{SCRIPT_MODULE_NAME_PREFIX}example'
+        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()
+
+        self.assertEqual(loaded.TestScript.run.__code__.co_filename, module.full_path)
+
+        # The storage location also appears in tracebacks from script code
+        try:
+            loaded.TestScript().run()
+            self.fail('expected ValueError from script code')
+        except ValueError:
+            formatted = traceback.format_exc()
+        self.assertIn(f'File "{module.full_path}"', formatted)
+
 
 class ScriptMetaValidationTestCase(TestCase):
     """

netbox/extras/tests/test_views.py

@@ -1,5 +1,6 @@
 import datetime
 import hashlib
+import io
 import logging
 import uuid
 from unittest.mock import PropertyMock, patch
@@ -1527,7 +1528,9 @@ class ScriptValidationErrorTestCase(TestCase):
 
     def setUp(self):
         super().setUp()
-        Script.python_class = property(lambda self: ScriptValidationErrorTestCase.TestScriptClass)
+        python_class_patch = patch.object(Script, 'python_class', new=self.TestScriptClass)
+        python_class_patch.start()
+        self.addCleanup(python_class_patch.stop)
 
     @tag('regression')
     def test_script_validation_error_displays_message(self):
@@ -1653,7 +1656,9 @@ class ScriptDefaultValuesTestCase(TestCase):
 
     def setUp(self):
         super().setUp()
-        Script.python_class = property(lambda self: ScriptDefaultValuesTestCase.TestScriptClass)
+        python_class_patch = patch.object(Script, 'python_class', new=self.TestScriptClass)
+        python_class_patch.start()
+        self.addCleanup(python_class_patch.stop)
 
     def test_default_values_are_used(self):
         url = reverse('extras:script', kwargs={'pk': self.script.pk})
@@ -1851,3 +1856,53 @@ class ScriptJobsRestrictionTestCase(TestCase):
     def test_without_view_job_permission(self):
         self._assert_script_views_show(set())
         self._assert_script_list_shows(None)
+
+
+class ScriptSourceViewTestCase(TestCase):
+    """
+    The Source tab must show the script's actual location in the scripts storage backend,
+    not a path derived from the current working directory (#23511).
+    """
+    user_permissions = ['extras.view_script']
+
+    SCRIPT_CONTENT = (
+        "from extras.scripts import Script\n\n\n"
+        "class TestScript(Script):\n    pass\n"
+    )
+
+    @classmethod
+    def setUpTestData(cls):
+        with patch.object(ScriptModule, 'sync_classes'):
+            cls.module = ScriptModule.objects.create(
+                file_root=ManagedFileRootPathChoices.SCRIPTS,
+                file_path='test_script.py',
+            )
+        cls.script = Script.objects.create(module=cls.module, name='TestScript', is_executable=True)
+
+    def setUp(self):
+        super().setUp()
+        content = self.SCRIPT_CONTENT.encode()
+
+        class _Storage:
+            def open(self, name, mode='rb'):
+                if 'b' in mode:
+                    return io.BytesIO(content)
+                return io.StringIO(content.decode())
+
+        self._module_storage_patch = patch('extras.models.mixins.storages')
+        self._script_storage_patch = patch('extras.scripts.storages')
+        module_storage = self._module_storage_patch.start()
+        script_storage = self._script_storage_patch.start()
+        module_storage.__getitem__.return_value = _Storage()
+        script_storage.backends = {'scripts': {}}
+        script_storage.create_storage.return_value = _Storage()
+        self.addCleanup(self._script_storage_patch.stop)
+        self.addCleanup(self._module_storage_patch.stop)
+
+    def test_source_view_shows_storage_location(self):
+        url = reverse('extras:script_source', kwargs={'pk': self.script.pk})
+        response = self.client.get(url)
+
+        self.assertHttpStatus(response, 200)
+        self.assertContains(response, self.module.full_path)
+        self.assertContains(response, 'class TestScript(Script):')