From f2d8ae29c2960d923847b58b23cda5fff8ff8e06 Mon Sep 17 00:00:00 2001 From: Arthur Hanson Date: Thu, 2 Apr 2026 05:42:14 -0700 Subject: [PATCH] 21701 Allow scripts to be uploaded via post to API (#21756) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * #21701 allow upload script via API * #21701 allow upload script via API * add extra test * change to use Script api endpoint * ruff fix * review feedback: * review feedback: * review feedback: * Fix permission check, perform_create delegation, and test mock setup - destroy() now checks extras.delete_script (queryset is Script.objects.all()) - create() delegates to self.perform_create() instead of calling serializer.save() directly - Add comment explaining why update/partial_update intentionally return 405 - Fix test_upload_script_module: set mock_storage.save.return_value so file_path receives a real string after the _save_upload return-value fix; add DB existence check Co-Authored-By: Claude Sonnet 4.6 * Return 400 instead of 500 on duplicate script module upload Catch IntegrityError from the unique (file_root, file_path) constraint and re-raise as a ValidationError so the API returns a 400 with a clear message rather than a 500. Co-Authored-By: Claude Sonnet 4.6 * Validate upload_file + data_source conflict for multipart requests DRF 3.16 Serializer.get_value() uses parse_html_dict() or empty for all HTML/multipart input. A flat key like data_source=2 produces an empty dict ({}), which is falsy, so it falls back to empty and the nested field is silently skipped. data.get('data_source') is therefore always None in multipart requests, bypassing the conflict check. Fix: also check self.initial_data for data_source and data_file in all three guards in validate(), so the raw submitted value is detected even when DRF's HTML parser drops the deserialized object. Add test_upload_with_data_source_fails to cover the multipart conflict path explicitly. Co-Authored-By: Claude Sonnet 4.6 * Require data_file when data_source is specified data_source alone is not a valid creation payload — a data_file must also be provided to identify which file within the source to sync. Add the corresponding validation error and a test to cover the case. Co-Authored-By: Claude Sonnet 4.6 * Align ManagedFileForm validation with API serializer rules Add the missing checks to ManagedFileForm.clean(): - upload_file + data_source is rejected (matches API) - data_source without data_file is rejected with a specific message - Update the 'nothing provided' error to mention data source + data file Co-Authored-By: Claude Sonnet 4.6 * Revert "Align ManagedFileForm validation with API serializer rules" This reverts commit f0ac7c3bd2fbdbde2bc14acf0f9026508ff289f9. * Align API validation messages with UI; restore complete checks - Match UI error messages for upload+data_file conflict and no-source case - Keep API-only guards for upload+data_source and data_source-without-data_file - Restore test_upload_with_data_source_fails Co-Authored-By: Claude Sonnet 4.6 * Run source/file conflict checks before super().validate() / full_clean() super().validate() calls full_clean() on the model instance, which raises a unique-constraint error for (file_root, file_path) when file_path is empty (e.g. data_source-only requests). Move the conflict guards above the super() call so they produce clear, actionable error messages before full_clean() has a chance to surface confusing database-level errors. Co-Authored-By: Claude Sonnet 4.6 * destroy() deletes ScriptModule, not Script DELETE /api/extras/scripts// now deletes the entire ScriptModule (matching the UI's delete view), including modules with no Script children (e.g. sync hasn't run yet). Permission check updated to delete_scriptmodule. The queryset restriction for destroy is removed since the module is deleted via script.module, not super().destroy(). Co-Authored-By: Claude Sonnet 4.6 * review feedback: * cleanup * cleanup * cleanup * cleanup * change to ScriptModule * change to ScriptModule * change to ScriptModule * update docs * cleanup * restore file * cleanup * cleanup * cleanup * cleanup * cleanup * keep only upload functionality * cleanup * cleanup * cleanup * change to scripts/upload api * cleanup * cleanup * cleanup * cleanup --------- Co-authored-by: Claude Sonnet 4.6 --- docs/customization/custom-scripts.md | 12 +++++ netbox/extras/api/serializers_/scripts.py | 55 ++++++++++++++++++++++- netbox/extras/api/urls.py | 1 + netbox/extras/api/views.py | 8 +++- netbox/extras/tests/test_api.py | 53 ++++++++++++++++++++++ 5 files changed, 126 insertions(+), 3 deletions(-) diff --git a/docs/customization/custom-scripts.md b/docs/customization/custom-scripts.md index add8fafb1..0947a2c94 100644 --- a/docs/customization/custom-scripts.md +++ b/docs/customization/custom-scripts.md @@ -384,6 +384,18 @@ A calendar date. Returns a `datetime.date` object. A complete date & time. Returns a `datetime.datetime` object. +## Uploading Scripts via the API + +Script modules can be uploaded to NetBox via the REST API by sending a `multipart/form-data` POST request to `/api/extras/scripts/upload/`. The caller must have the `extras.add_scriptmodule` and `core.add_managedfile` permissions. + +```no-highlight +curl -X POST \ +-H "Authorization: Token $TOKEN" \ +-H "Accept: application/json; indent=4" \ +-F "file=@/path/to/myscript.py" \ +http://netbox/api/extras/scripts/upload/ +``` + ## Running Custom Scripts !!! note diff --git a/netbox/extras/api/serializers_/scripts.py b/netbox/extras/api/serializers_/scripts.py index a7d5b9c2a..9f0afe3f1 100644 --- a/netbox/extras/api/serializers_/scripts.py +++ b/netbox/extras/api/serializers_/scripts.py @@ -1,19 +1,70 @@ -from django.utils.translation import gettext as _ +import logging + +from django.core.files.storage import storages +from django.db import IntegrityError +from django.utils.translation import gettext_lazy as _ from drf_spectacular.utils import extend_schema_field from rest_framework import serializers from core.api.serializers_.jobs import JobSerializer -from extras.models import Script +from core.choices import ManagedFileRootPathChoices +from extras.models import Script, ScriptModule from netbox.api.serializers import ValidatedModelSerializer from utilities.datetime import local_now +logger = logging.getLogger(__name__) + __all__ = ( 'ScriptDetailSerializer', 'ScriptInputSerializer', + 'ScriptModuleSerializer', 'ScriptSerializer', ) +class ScriptModuleSerializer(ValidatedModelSerializer): + file = serializers.FileField(write_only=True) + file_path = serializers.CharField(read_only=True) + + class Meta: + model = ScriptModule + fields = ['id', 'display', 'file_path', 'file', 'created', 'last_updated'] + brief_fields = ('id', 'display') + + def validate(self, data): + # ScriptModule.save() sets file_root; inject it here so full_clean() succeeds. + # Pop 'file' before model instantiation — ScriptModule has no such field. + file = data.pop('file', None) + data['file_root'] = ManagedFileRootPathChoices.SCRIPTS + data = super().validate(data) + data.pop('file_root', None) + if file is not None: + data['file'] = file + return data + + def create(self, validated_data): + file = validated_data.pop('file') + storage = storages.create_storage(storages.backends["scripts"]) + validated_data['file_path'] = storage.save(file.name, file) + created = False + try: + instance = super().create(validated_data) + created = True + return instance + except IntegrityError as e: + if 'file_path' in str(e): + raise serializers.ValidationError( + _("A script module with this file name already exists.") + ) + raise + finally: + if not created and (file_path := validated_data.get('file_path')): + try: + storage.delete(file_path) + except Exception: + logger.warning(f"Failed to delete orphaned script file '{file_path}' from storage.") + + class ScriptSerializer(ValidatedModelSerializer): description = serializers.SerializerMethodField(read_only=True) vars = serializers.SerializerMethodField(read_only=True) diff --git a/netbox/extras/api/urls.py b/netbox/extras/api/urls.py index 9478fbeb2..cd1a9f683 100644 --- a/netbox/extras/api/urls.py +++ b/netbox/extras/api/urls.py @@ -26,6 +26,7 @@ router.register('journal-entries', views.JournalEntryViewSet) router.register('config-contexts', views.ConfigContextViewSet) router.register('config-context-profiles', views.ConfigContextProfileViewSet) router.register('config-templates', views.ConfigTemplateViewSet) +router.register('scripts/upload', views.ScriptModuleViewSet) router.register('scripts', views.ScriptViewSet, basename='script') app_name = 'extras-api' diff --git a/netbox/extras/api/views.py b/netbox/extras/api/views.py index e72ad1ab5..5a2c03212 100644 --- a/netbox/extras/api/views.py +++ b/netbox/extras/api/views.py @@ -6,7 +6,7 @@ from rest_framework import status from rest_framework.decorators import action from rest_framework.exceptions import PermissionDenied from rest_framework.generics import RetrieveUpdateDestroyAPIView -from rest_framework.mixins import ListModelMixin, RetrieveModelMixin +from rest_framework.mixins import CreateModelMixin, ListModelMixin, RetrieveModelMixin from rest_framework.renderers import JSONRenderer from rest_framework.response import Response from rest_framework.routers import APIRootView @@ -21,6 +21,7 @@ from netbox.api.features import SyncedDataMixin from netbox.api.metadata import ContentTypeMetadata from netbox.api.renderers import TextRenderer from netbox.api.viewsets import BaseViewSet, NetBoxModelViewSet +from netbox.api.viewsets.mixins import ObjectValidationMixin from utilities.exceptions import RQWorkerNotRunningException from utilities.request import copy_safe_request @@ -264,6 +265,11 @@ class ConfigTemplateViewSet(SyncedDataMixin, ConfigTemplateRenderMixin, NetBoxMo # Scripts # +class ScriptModuleViewSet(ObjectValidationMixin, CreateModelMixin, BaseViewSet): + queryset = ScriptModule.objects.all() + serializer_class = serializers.ScriptModuleSerializer + + @extend_schema_view( update=extend_schema(request=serializers.ScriptInputSerializer), partial_update=extend_schema(request=serializers.ScriptInputSerializer), diff --git a/netbox/extras/tests/test_api.py b/netbox/extras/tests/test_api.py index 1c4996bcc..c85433982 100644 --- a/netbox/extras/tests/test_api.py +++ b/netbox/extras/tests/test_api.py @@ -1,7 +1,9 @@ import datetime import hashlib +from unittest.mock import MagicMock, patch from django.contrib.contenttypes.models import ContentType +from django.core.files.uploadedfile import SimpleUploadedFile from django.urls import reverse from django.utils.timezone import make_aware, now from rest_framework import status @@ -1384,3 +1386,54 @@ class NotificationTest(APIViewTestCases.APIViewTestCase): 'event_type': OBJECT_DELETED, }, ] + + +class ScriptModuleTest(APITestCase): + """ + Tests for the POST /api/extras/scripts/upload/ endpoint. + + ScriptModule is a proxy of core.ManagedFile (a different app) so the standard + APIViewTestCases mixins cannot be used directly. All tests use add_permissions() + with explicit Django model-level permissions. + """ + + def setUp(self): + super().setUp() + self.url = reverse('extras-api:scriptmodule-list') # /api/extras/scripts/upload/ + + def test_upload_script_module_without_permission(self): + script_content = b"from extras.scripts import Script\nclass TestScript(Script):\n pass\n" + upload_file = SimpleUploadedFile('test_upload.py', script_content, content_type='text/plain') + response = self.client.post( + self.url, + {'file': upload_file}, + format='multipart', + **self.header, + ) + self.assertHttpStatus(response, status.HTTP_403_FORBIDDEN) + + def test_upload_script_module(self): + # ScriptModule is a proxy of core.ManagedFile; both permissions required. + self.add_permissions('extras.add_scriptmodule', 'core.add_managedfile') + script_content = b"from extras.scripts import Script\nclass TestScript(Script):\n pass\n" + upload_file = SimpleUploadedFile('test_upload.py', script_content, content_type='text/plain') + mock_storage = MagicMock() + mock_storage.save.return_value = 'test_upload.py' + with patch('extras.api.serializers_.scripts.storages') as mock_storages: + mock_storages.create_storage.return_value = mock_storage + mock_storages.backends = {'scripts': {}} + response = self.client.post( + self.url, + {'file': upload_file}, + format='multipart', + **self.header, + ) + self.assertHttpStatus(response, status.HTTP_201_CREATED) + self.assertEqual(response.data['file_path'], 'test_upload.py') + mock_storage.save.assert_called_once() + self.assertTrue(ScriptModule.objects.filter(file_path='test_upload.py').exists()) + + def test_upload_script_module_without_file_fails(self): + self.add_permissions('extras.add_scriptmodule', 'core.add_managedfile') + response = self.client.post(self.url, {}, format='json', **self.header) + self.assertHttpStatus(response, status.HTTP_400_BAD_REQUEST)