Fix empty error responses and test hygiene in script run API

- Replace the '_' prefix heuristic for filtering ScriptForm errors with an
  explicit exclusion list (EXEC_PARAM_FIELDS). The heuristic also stripped
  Django's NON_FIELD_ERRORS ('__all__'), so a pure form-level error (e.g.
  ScriptForm.clean()'s "Scheduled time must be in the future.") resulted
  in an empty 400 response body
- Nest script-variable errors under 'data' so they can't collide with
  ScriptInputSerializer's own top-level fields (commit, interval, ...)
- Use input_serializer.is_valid(raise_exception=True) for consistency
  with the rest of the method
- Clarify the "script class could not be loaded" error message
- Move EXEC_PARAM_FIELDS to extras/scripts.py as the single source of
  truth for both the API view and the runscript command; drop
  prepare_script_form from extras.scripts.__all__ as internal plumbing
- Fix runscript only popping 3 of 4 internal exec fields from
  cleaned_data, leaking '_notifications' into the script's own data
- Restore Script.python_class via patch.object()/addCleanup() in
  ScriptRunExecutionTestCase instead of a permanent override
- Add test coverage for schedule_at/interval forwarding and for
  rejecting a nonexistent object ID

netbox/extras/api/views.py

@@ -1,8 +1,8 @@
+from django.core.exceptions import NON_FIELD_ERRORS
 from django.http import Http404
 from django.shortcuts import get_object_or_404
 from django.utils.translation import gettext_lazy as _
 from drf_spectacular.utils import OpenApiResponse, OpenApiTypes, extend_schema, extend_schema_view
-from rest_framework import status
 from rest_framework.decorators import action
 from rest_framework.exceptions import PermissionDenied, ValidationError
 from rest_framework.generics import RetrieveUpdateDestroyAPIView
@@ -16,7 +16,7 @@ from core.choices import ManagedFileRootPathChoices
 from extras import filtersets
 from extras.jobs import ScriptJob
 from extras.models import *
-from extras.scripts import prepare_script_form
+from extras.scripts import EXEC_PARAM_FIELDS, prepare_script_form
 from netbox.api.authentication import IsAuthenticatedOrLoginNotRequired, TokenWritePermission
 from netbox.api.features import SyncedDataMixin
 from netbox.api.metadata import ContentTypeMetadata
@@ -367,8 +367,7 @@ class ScriptViewSet(ModelViewSet):
         if not any_workers_for_queue('default'):
             raise RQWorkerNotRunningException()
 
-        if not input_serializer.is_valid():
-            return Response(input_serializer.errors, status=status.HTTP_400_BAD_REQUEST)
+        input_serializer.is_valid(raise_exception=True)
 
         validated = input_serializer.validated_data
 
@@ -380,17 +379,26 @@ class ScriptViewSet(ModelViewSet):
 
         script_class = script.python_class
         if not script_class:
-            raise ValidationError({'script': _('Script class could not be loaded; cannot determine job timeout.')})
+            raise ValidationError({'script': _('Unable to load the script class.')})
         script_instance = script_class()
 
         form = prepare_script_form(script_instance, payload, files=request.FILES)
         if not form.is_valid():
-            # remove internal fields (_commit etc.) from API error message
-            errors = {k: v for k, v in form.errors.items() if not k.startswith('_')}
-            raise ValidationError(errors)
+            # Exec params (_commit etc.) are validated separately via ScriptInputSerializer;
+            # exclude them explicitly rather than via a '_' prefix, which would also strip
+            # Django's NON_FIELD_ERRORS key ('__all__') and any script var legitimately
+            # named with a leading underscore.
+            errors = {k: v for k, v in form.errors.items() if k not in EXEC_PARAM_FIELDS}
+            if not errors:
+                # Only a non-field error remains (e.g. ScriptForm.clean()'s "Scheduled
+                # time must be in the future."). Surface it instead of an empty body.
+                errors = {NON_FIELD_ERRORS: form.errors.get(NON_FIELD_ERRORS, [_('Invalid script input.')])}
+            # Nest under 'data' so script-variable errors can't collide with the
+            # serializer's own top-level fields (commit, schedule_at, interval, ...).
+            raise ValidationError({'data': errors})
 
         data = form.cleaned_data.copy()
-        for k in ('_commit', '_schedule_at', '_interval', '_notifications'):
+        for k in EXEC_PARAM_FIELDS:
             data.pop(k, None)
 
         ScriptJob.enqueue(

netbox/extras/management/commands/runscript.py

@@ -6,7 +6,7 @@ import uuid
 from django.core.management.base import BaseCommand, CommandError
 
 from extras.jobs import ScriptJob
-from extras.scripts import get_module_and_script
+from extras.scripts import EXEC_PARAM_FIELDS, get_module_and_script
 from users.models import User
 from utilities.request import NetBoxFakeRequest
 
@@ -81,17 +81,18 @@ class Command(BaseCommand):
                     logger.error(f'\t{field}: {error.get("message")}')
             raise CommandError()
 
-        # Remove extra fields from ScriptForm before passing data to script
-        form.cleaned_data.pop('_schedule_at')
-        form.cleaned_data.pop('_interval')
-        form.cleaned_data.pop('_commit')
+        # Remove exec-parameter fields from ScriptForm before passing data to the script.
+        # (Previously missed '_notifications', which leaked into the script's own data.)
+        cleaned_data = form.cleaned_data.copy()
+        for key in EXEC_PARAM_FIELDS:
+            cleaned_data.pop(key, None)
 
         # Execute the script.
         job = ScriptJob.enqueue(
             instance=script_obj,
             user=user,
             immediate=True,
-            data=form.cleaned_data,
+            data=cleaned_data,
             request=NetBoxFakeRequest({
                 'META': {},
                 'COOKIES': {},

netbox/extras/scripts.py

@@ -41,9 +41,13 @@ __all__ = (
     'StringVar',
     'TextVar',
     'get_module_and_script',
-    'prepare_script_form',
 )
 
+# Internal ScriptForm fields used to carry execution parameters (see ScriptForm in
+# extras/forms/scripts.py). These are validated/sourced separately from the script's own
+# declared variables and must never be treated as script data or surfaced as script errors.
+EXEC_PARAM_FIELDS = ('_commit', '_schedule_at', '_interval', '_notifications')
+
 
 #
 # Script variables
@@ -664,7 +668,17 @@ def prepare_script_form(script_instance, data, files=None):
     declared variable's `default` value into `data` when the caller omitted it.
 
     Used by both the UI (extras/views.py) and the REST API (extras/api/views.py) so the
-    two entry points share one contract and can't drift apart again.
+    two entry points share one contract and can't drift apart again. `runscript` deliberately
+    stays on the plain `as_form()` call, since its own test suite exercises it with bare
+    script doubles that only implement `as_form()`, not the full Script/`_get_vars()` API.
+
+    Note: `script_instance` must already be an *instance* (e.g. `script.python_class()`),
+    not the class itself.
+
+    `data` is copied via `.copy()` rather than coerced with `dict(...)`, so a QueryDict
+    (as submitted by the UI form) keeps its multi-value semantics -- collapsing it to a
+    plain dict would silently drop all but the last value for a MultiObjectVar's
+    multi-select field.
     """
     data = data.copy() if data is not None else {}
     for name, var in script_instance._get_vars().items():

netbox/extras/tests/test_api.py

@@ -1447,11 +1447,11 @@ class ScriptTestCase(APITestCase):
 class ScriptRunExecutionTestCase(APITestCase):
     """
     Exercises ScriptViewSet.post() end-to-end (real request -> real serializer -> real
-    form -> real ScriptJob.enqueue() call), covering the regressions raised in review of
-    PR #22861: execution parameters must be taken from the validated request rather than
-    the form's own defaults, ObjectVar/MultiObjectVar values must be converted from raw
-    IDs to model instances/querysets, and declared defaults must be back-filled for
-    variables the client omits.
+    form -> real ScriptJob.enqueue() call): execution parameters (commit, schedule_at,
+    interval, notifications) must be taken from the validated request rather than the
+    form's own defaults; ObjectVar/MultiObjectVar values must be converted from raw IDs
+    to model instances/querysets (see #22750); and declared defaults must be back-filled
+    for variables the client omits.
     """
 
     class TestScriptClass(PythonClass):
@@ -1486,8 +1486,11 @@ class ScriptRunExecutionTestCase(APITestCase):
         super().setUp()
         self.add_permissions('extras.run_script')
 
-        # Monkey-patch the Script model to return our TestScriptClass above
-        Script.python_class = self.TestScriptClass
+        # Monkey-patch the Script model to return our TestScriptClass above, restoring
+        # the real property afterwards so later tests aren't left with our stub.
+        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.
@@ -1526,6 +1529,25 @@ class ScriptRunExecutionTestCase(APITestCase):
             JobNotificationChoices.NOTIFICATION_NEVER,
         )
 
+    @patch('extras.jobs.ScriptJob.enqueue')
+    def test_run_forwards_schedule_at_and_interval(self, mock_enqueue):
+        # Regression: schedule_at/interval were likewise read from the form (always
+        # absent there) instead of the validated request
+        schedule_at = now() + datetime.timedelta(hours=1)
+        payload = {
+            'data': {'site': self.sites[0].pk},
+            'commit': True,
+            'schedule_at': schedule_at,
+            'interval': 60,
+        }
+
+        response = self.client.post(self.url, payload, format='json', **self.header)
+
+        self.assertHttpStatus(response, status.HTTP_200_OK)
+        kwargs = mock_enqueue.call_args.kwargs
+        self.assertEqual(kwargs['schedule_at'], schedule_at)
+        self.assertEqual(kwargs['interval'], 60)
+
     @patch('extras.jobs.ScriptJob.enqueue')
     def test_run_converts_objectvar_and_multiobjectvar_ids(self, mock_enqueue):
         payload = {
@@ -1565,6 +1587,19 @@ class ScriptRunExecutionTestCase(APITestCase):
 
         self.assertHttpStatus(response, status.HTTP_400_BAD_REQUEST)
 
+    @patch('extras.jobs.ScriptJob.enqueue')
+    def test_run_rejects_nonexistent_object_id(self, mock_enqueue):
+        # This is the primary new failure mode introduced by converting raw IDs to model
+        # instances: a PK that doesn't resolve must 400 cleanly, not enqueue a broken job
+        # or raise an unhandled DoesNotExist.
+        nonexistent_pk = Site.objects.order_by('-pk').first().pk + 1000
+        payload = {'data': {'site': nonexistent_pk}, 'commit': True}
+
+        response = self.client.post(self.url, payload, format='json', **self.header)
+
+        self.assertHttpStatus(response, status.HTTP_400_BAD_REQUEST)
+        mock_enqueue.assert_not_called()
+
 
 class CreatedUpdatedFilterTestCase(APITestCase):