review feedback

docs/customization/custom-scripts.md

@@ -301,6 +301,9 @@ All custom script variables support the following default options:
 * `required` - Indicates whether the field is mandatory (all fields are required by default)
 * `widget` - The class of form widget to use (see the [Django documentation](https://docs.djangoproject.com/en/stable/ref/forms/widgets/))
 
+!!! warning "Reserved variable names"
+    The names `_commit`, `_schedule_at`, `_interval`, and `_notifications` are reserved for the execution parameters which NetBox renders alongside a script's own fields. A variable declared with one of these names shadows its execution parameter, and its value is not passed to `run()`. Choose a different name.
+
 ### StringVar
 
 Stores a string of characters (i.e. text). Options include:
@@ -547,7 +550,7 @@ http://netbox/api/extras/scripts/example.MyReport/ \
 Optionally `schedule_at` can be passed in the form data with a datetime string to schedule a script at the specified date and time.
 
 !!! note
-    The `data` object is validated against the script's declared variables, just as in the web UI. Keys which do not match a declared variable are ignored. Scripts declaring a `FileVar` must be run via a `multipart/form-data` request, passing `data` as a JSON string alongside the uploaded file.
+    Script input submitted through the REST API is validated against the variables declared by the script. Missing required variables or invalid values result in an HTTP 400 response, and undeclared keys are discarded rather than passed to `run()`. Existing API clients that relied on the previous pass-through behavior may need to update their requests. Scripts declaring a `FileVar` must be run via a `multipart/form-data` request, passing `data` as a JSON string alongside the uploaded file.
 
 ### Via the CLI
 

netbox/extras/api/views.py

@@ -421,12 +421,9 @@ class ScriptViewSet(ListModelMixin, RetrieveModelMixin, BaseViewSet):
 
         form = prepare_script_form(script_instance, payload, files=request.FILES)
         if not form.is_valid():
-            # 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__'). This does not disambiguate a script
-            # variable whose name collides with one of EXEC_PARAM_FIELDS: such a variable
-            # shadows the exec field on the dynamic form subclass, so its errors are filtered
-            # here and its value popped below, along with the exec param it shadows.
+            # Exec params are validated separately via ScriptInputSerializer. Excluded by name
+            # rather than by '_' prefix, which would also strip Django's NON_FIELD_ERRORS
+            # key ('__all__').
             errors = {k: v for k, v in form.errors.items() if k not in EXEC_PARAM_FIELDS}
             if not errors:
                 # Every error was on an exec-param field, which a client can bind by naming one

netbox/extras/scripts.py

@@ -715,22 +715,11 @@ def get_module_and_script(module_name, script_name):
 
 def prepare_script_form(script_instance, data, files=None):
     """
-    Build a bound ScriptForm for an already-instantiated Script object, back-filling any
-    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. `runscript` stays on the
-    plain `as_form()` call: it has never back-filled defaults, so routing it through this
-    helper would change CLI behavior (a variable with a `default` omitted from `--data` would
-    begin to be accepted rather than reported as required). That is a separate change.
-
-    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.
+    Return a bound ScriptForm for the given Script instance, back-filling the declared
+    `default` of any variable omitted from `data`.
+
+    `data` is copied rather than coerced to a plain dict, so a QueryDict retains the
+    multi-value semantics a MultiObjectVar's multi-select field depends on.
     """
     data = data.copy() if data is not None else {}
     for name, var in script_instance._get_vars().items():

netbox/extras/tests/test_api.py

@@ -1892,12 +1892,7 @@ class ScriptTestCase(APITestCase):
 
 class ScriptRunExecutionTestCase(APITestCase):
     """
-    Exercises ScriptViewSet.post() end-to-end (real request -> real serializer -> real
-    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.
+    Exercises ScriptViewSet.run() end-to-end: request -> serializer -> form -> enqueue.
     """
 
     class TestScriptClass(PythonClass):