From 113f5ea9d4fb1ef319f27b7d6e24163acabedf51 Mon Sep 17 00:00:00 2001 From: Martin Burggraf Date: Tue, 4 Aug 2026 17:44:18 +0200 Subject: [PATCH] Fixes #22750: Converts raw IDs to ObjectVars/MultiObjectVars --- netbox/extras/jobs.py | 50 ++++++++++++++++++--- netbox/extras/tests/test_jobs.py | 74 ++++++++++++++++++++++++++++++++ 2 files changed, 118 insertions(+), 6 deletions(-) diff --git a/netbox/extras/jobs.py b/netbox/extras/jobs.py index 7514c1be0..efcb66caa 100644 --- a/netbox/extras/jobs.py +++ b/netbox/extras/jobs.py @@ -2,6 +2,7 @@ import logging import traceback from contextlib import ExitStack +from django.core.exceptions import ValidationError from django.db import DEFAULT_DB_ALIAS, router, transaction from django.utils.translation import gettext as _ @@ -112,16 +113,53 @@ class ScriptJob(JobRunner): script = script_model.python_class() self.logger.debug(f"Loaded script {script.full_name}") - # Add files to form data - if request: - files = request.FILES - for field_name, fileobj in files.items(): - data[field_name] = fileobj - # Add the current request as a property of the script script.request = request self.logger.debug(f"Request ID: {request.id if request else None}") + # Normalize incoming payload for the form: API callers submit variables under "data". + payload = data or {} + if isinstance(payload, dict) and 'data' in payload: + payload = payload['data'] or {} + + files = request.FILES if request else None + if files: + for field_name, fileobj in files.items(): + # merge into payload so script.run receives the uploaded files in data + payload[field_name] = fileobj + + # Validate & clean using the script's form so ObjectVar/MultiObjectVar IDs become model instances + if hasattr(script, 'as_form') and callable(getattr(script, 'as_form')): + try: + form = script.as_form(data=payload, files=files) + if not form.is_valid(): + raise AbortScript(f"Script input validation failed: {form.errors.as_json()}") + + cleaned = form.cleaned_data + + # Remove execution parameters + for key in list(cleaned.keys()): + if key.startswith('_'): + cleaned.pop(key) + + # Preserve uploaded files that were merged into the payload so scripts still see them + # even if the Script's form doesn't declare file fields. + if files: + for fname, fobj in files.items(): + if fname not in cleaned: + cleaned[fname] = fobj + + # Use cleaned form data as the data passed into the script + data = cleaned + except AbortScript: + # Re-raise for run_script() to log/handle + raise + except (ValidationError, TypeError, ValueError) as e: + raise AbortScript(f"Error validating script input: {e!s}") + else: + # Script doesn't provide as_form (e.g., lightweight test double); keep `data` as-is. + data = payload + if commit: self.logger.info("Executing script (commit enabled)") else: diff --git a/netbox/extras/tests/test_jobs.py b/netbox/extras/tests/test_jobs.py index 44ff5efe6..4cf985625 100644 --- a/netbox/extras/tests/test_jobs.py +++ b/netbox/extras/tests/test_jobs.py @@ -6,7 +6,10 @@ from unittest.mock import MagicMock, patch from django.db import DEFAULT_DB_ALIAS from django.test import TestCase +from dcim.models import Site from extras.jobs import ScriptJob +from extras.models import Script as ScriptModel +from extras.scripts import ObjectVar, Script from utilities.exceptions import AbortScript @@ -338,3 +341,74 @@ class ScriptJobRunTestCase(TestCase): runner.run(data={}, commit=False) self.assertEqual(entered, ['proc_a']) + + +class ScriptJobFormCleaningTestCase(TestCase): + def test_run_converts_objectvar_id_to_model_instance(self): + # Create a simple target object + site = Site.objects.create(name="Test Site", slug="test-site") + + # Real Script subclass that declares an ObjectVar and records received data + class TestScript(Script): + site = ObjectVar(label='Site', model=Site) + + def __init__(self): + super().__init__() + self.received = None + + def run(self, data, commit=True): + # record what we received and return a sentinel + self.received = data + return "ok" + + script_instance = TestScript() + + # Make ScriptModel.objects.get() return a stub whose python_class() yields our instance + script_model_stub = MagicMock() + script_model_stub.python_class.return_value = script_instance + + runner = _make_runner(object_id=1) + with patch.object(ScriptModel.objects, 'get', return_value=script_model_stub): + # Pass nested payload shape like the REST API: {"data": {"site": }} + runner.run(data={'data': {'site': site.pk}}, request=None, commit=True) + + # Assert the script received a model instance for 'site' + self.assertIsNotNone(script_instance.received) + self.assertIn('site', script_instance.received) + self.assertIsInstance(script_instance.received['site'], Site) + self.assertNotIn('_commit', script_instance.received) + + def test_run_merges_request_files_into_data_for_real_script(self): + # Create object + site = Site.objects.create(name="Test Site 2", slug="test-site-2") + + class TestScript(Script): + site = ObjectVar(label='Site', model=Site) + + def __init__(self): + super().__init__() + self.received = None + + def run(self, data, commit=True): + self.received = data + actual_type = type(data['site']) + return f"got type: {actual_type}" + + script_instance = TestScript() + script_model_stub = MagicMock() + script_model_stub.python_class.return_value = script_instance + + # Simulate a request with uploaded files + fake_request = MagicMock() + fake_request.FILES = {'upload': 'fileobj'} + fake_request.id = None + + runner = _make_runner(object_id=1) + with patch.object(ScriptModel.objects, 'get', return_value=script_model_stub): + runner.run(data={'data': {'site': site.pk}}, request=fake_request, commit=True) + + # Assert both file merged and ObjectVar conversion happened + self.assertIsNotNone(script_instance.received) + self.assertIn('upload', script_instance.received) + self.assertEqual(script_instance.received['upload'], 'fileobj') + self.assertIsInstance(script_instance.received['site'], Site)