fix(extras): Improve file naming and upload handling

Refines logic for generating filenames and paths for file uploads. Uses
`Path` for cross-platform compatibility, adds sanitization, and
validates file paths to prevent security risks. Enhances support for
custom instance names and preserves specified file extensions.

Fixes #20236

Author: pheus

netbox/extras/models/models.py

@@ -1,6 +1,6 @@
 import json
-import os
 import urllib.parse
+from pathlib import Path
 
 from django.conf import settings
 from django.contrib.contenttypes.fields import GenericForeignKey, GenericRelation
@@ -728,7 +728,9 @@ def delete(self, *args, **kwargs):
 
     @property
     def filename(self):
-        return os.path.basename(self.image.name).split('_', 2)[2]
+        base_name = Path(self.image.name).name
+        prefix = f"{self.object_type.model}_{self.object_id}_"
+        return base_name.removeprefix(prefix)
 
     @property
     def html_tag(self):

netbox/extras/utils.py

@@ -1,11 +1,15 @@
 import importlib
+from pathlib import Path
 
-from django.core.exceptions import ImproperlyConfigured
+from django.core.exceptions import ImproperlyConfigured, SuspiciousFileOperation
+from django.core.files.storage import default_storage
+from django.core.files.utils import validate_file_name
 from django.db import models
 from django.db.models import Q
 from taggit.managers import _TaggableManager
 
 from netbox.context import current_request
+
 from .validators import CustomValidator
 
 __all__ = (
@@ -35,13 +39,13 @@ def get_queryset(self, request):
 
 
 def filename_from_model(model: models.Model) -> str:
-    """Standardises how we generate filenames from model class for exports"""
+    """Standardizes how we generate filenames from model class for exports"""
     base = model._meta.verbose_name_plural.lower().replace(' ', '_')
     return f'netbox_{base}'
 
 
 def filename_from_object(context: dict) -> str:
-    """Standardises how we generate filenames from model class for exports"""
+    """Standardizes how we generate filenames from model class for exports"""
     if 'device' in context:
         base = f"{context['device'].name or 'config'}"
     elif 'virtualmachine' in context:
@@ -64,17 +68,42 @@ def is_taggable(obj):
 def image_upload(instance, filename):
     """
     Return a path for uploading image attachments.
+
+    - Normalizes browser paths (e.g., C:\\fake_path\\photo.jpg)
+    - Uses the instance.name if provided (sanitized to a *basename*, no ext)
+    - Prefixes with a machine-friendly identifier
+
+    Note: Relies on Django's default_storage utility.
     """
-    path = 'image-attachments/'
+    upload_dir = 'image-attachments'
+    default_filename = 'unnamed'
+    allowed_img_extensions = ('bmp', 'gif', 'jpeg', 'jpg', 'png', 'webp')
+
+    # Normalize Windows paths and create a Path object.
+    normalized_filename = str(filename).replace('\\', '/')
+    file_path = Path(normalized_filename)
+
+    # Extract the extension from the uploaded file.
+    ext = file_path.suffix.lower().lstrip('.')
+
+    # Use the instance-provided name if available; otherwise use the file stem.
+    # Rely on Django's get_valid_filename to perform sanitization.
+    stem = (instance.name or file_path.stem).strip()
+    try:
+        safe_stem = default_storage.get_valid_name(stem)
+    except SuspiciousFileOperation:
+        safe_stem = default_filename
+
+    # Append the uploaded extension only if it's an allowed image type
+    final_name = f"{safe_stem}.{ext}" if ext in allowed_img_extensions else safe_stem
 
-    # Rename the file to the provided name, if any. Attempt to preserve the file extension.
-    extension = filename.rsplit('.')[-1].lower()
-    if instance.name and extension in ['bmp', 'gif', 'jpeg', 'jpg', 'png', 'webp']:
-        filename = '.'.join([instance.name, extension])
-    elif instance.name:
-        filename = instance.name
+    # Create a machine-friendly prefix from the instance
+    prefix = f"{instance.object_type.model}_{instance.object_id}"
+    name_with_path = f"{upload_dir}/{prefix}_{final_name}"
 
-    return '{}{}_{}_{}'.format(path, instance.object_type.name, instance.object_id, filename)
+    # Validate the generated relative path (blocks absolute/traversal)
+    validate_file_name(name_with_path, allow_relative_path=True)
+    return name_with_path
 
 
 def is_script(obj):
@@ -107,7 +136,7 @@ def run_validators(instance, validators):
     request = current_request.get()
     for validator in validators:
 
-        # Loading a validator class by dotted path
+        # Loading a validator class by a dotted path
         if type(validator) is str:
             module, cls = validator.rsplit('.', 1)
             validator = getattr(importlib.import_module(module), cls)()