[refactor] File uploads

src/backend/InvenTree/InvenTree/models.py

@@ -1,9 +1,8 @@
 """Generic models which provide extra functionality over base Django model types."""
 
-from collections.abc import Callable
 from datetime import datetime
 from string import Formatter
-from typing import Any, Optional
+from typing import Optional
 
 from django.contrib.auth import get_user_model
 from django.contrib.contenttypes.fields import GenericRelation
@@ -1502,6 +1501,15 @@ def after_error_logged(sender, instance: Error, created: bool, **kwargs):
         )
 
 
+def rename_image(instance, filename):
+    """Rename the uploaded image file using the IMAGE_RENAME function."""
+    from common.media import rename_uploaded_file
+
+    return rename_uploaded_file(
+        filename, 'image', instance.__class__.__name__.lower(), instance.pk
+    )
+
+
 class InvenTreeImageMixin(models.Model):
     """A mixin class for adding image functionality to a model class.
 
@@ -1510,8 +1518,6 @@ class InvenTreeImageMixin(models.Model):
     - image : An image field for storing an image
     """
 
-    IMAGE_RENAME: Callable | None = None
-
     class Meta:
         """Metaclass options for this mixin.
 
@@ -1520,18 +1526,6 @@ class Meta:
 
         abstract = True
 
-    def __init__(self, *args: Any, **kwargs: Any) -> None:
-        """Custom init method for InvenTreeImageMixin to ensure IMAGE_RENAME is implemented."""
-        if self.IMAGE_RENAME is None:
-            raise NotImplementedError(
-                'IMAGE_RENAME must be implemented in the model class'
-            )
-        super().__init__(*args, **kwargs)
-
-    def rename_image(self, filename):
-        """Rename the uploaded image file using the IMAGE_RENAME function."""
-        return self.IMAGE_RENAME(filename)
-
     image = StdImageField(
         upload_to=rename_image,
         null=True,

src/backend/InvenTree/common/media.py

@@ -0,0 +1,68 @@
+"""Common functions for handling media files in InvenTree."""
+
+from pathlib import Path
+from typing import Optional
+
+from django.core.exceptions import ValidationError
+
+
+def rename_uploaded_file(
+    filename: str,
+    file_type: str,
+    model_type: Optional[str] = None,
+    model_id: Optional[int] = None,
+) -> str:
+    """A centralized function for handling file uploads in InvenTree.
+
+    All uploaded files will be stored in a consistent directory structure,
+    which allows for repeatable and predictable file storage.
+
+    Additionally, with a consistent file storage structure,
+    it is possible to implement a permissions system for accessing uploaded files.
+
+    Arguments:
+        filename: The name of the uploaded file.
+        file_type: The type of the file (e.g., 'attachment', 'test_result').
+        model_type: The type of the model associated with the file (optional).
+        model_id: The ID of the model associated with the file (optional).
+    """
+    filename = str(filename).strip()
+
+    if not filename:
+        raise ValidationError('Filename cannot be empty.')
+
+    if not file_type:
+        raise ValidationError('File type must be specified.')
+
+    # First, remove any illegal characters from the filename.
+    # Keep '.' so valid file extensions are preserved.
+    illegal_chars = '\'"\\\\/`~#|!@#$%^&*()[]{}<>?;:+=,'
+
+    for c in illegal_chars:
+        filename = filename.replace(c, '')
+
+    # Convert to a Path, ensure the filename is not attempting to traverse directories
+    file_path = Path(filename)
+
+    if file_path.is_absolute() or '..' in file_path.parts or len(file_path.parts) > 1:
+        raise ValidationError('Invalid filename: cannot contain directory traversal.')
+
+    # Construct an upload path based on the provided parts
+    parts = []
+
+    # If provided, include the file type in the path
+    if model_type:
+        parts.append(str(model_type))
+
+    # If provided, include the model ID in the path
+    if model_id:
+        parts.append(str(model_id))
+
+    # Include the file type in the path
+    parts.append(str(file_type))
+
+    # Finally, include the sanitized filename
+    parts.append(file_path.name)
+
+    # Join all parts to form the final upload path
+    return str(Path(*parts))

src/backend/InvenTree/common/models.py

@@ -1910,7 +1910,7 @@ def after_custom_unit_updated(sender, instance, **kwargs):
     reload_unit_registry()
 
 
-def rename_attachment(instance, filename: str):
+def rename_attachment(instance, filename: str) -> str:
     """Callback function to rename an uploaded attachment file.
 
     Args:
@@ -1920,17 +1920,13 @@ def rename_attachment(instance, filename: str):
     Returns:
         str: The new filename for the uploaded file, e.g. 'attachments/<model_type>/<model_id>/<filename>'.
     """
-    # Remove any illegal characters from the filename
-    illegal_chars = '\'"\\`~#|!@#$%^&*()[]{}<>?;:+=,'
+    from common.media import rename_uploaded_file
 
-    for c in illegal_chars:
-        filename = filename.replace(c, '')
-
-    filename = os.path.basename(filename)
-
-    # Generate a new filename for the attachment
-    return os.path.join(
-        'attachments', str(instance.model_type), str(instance.model_id), filename
+    return rename_uploaded_file(
+        filename,
+        'attachment',
+        model_type=instance.model_type,
+        model_id=instance.model_id,
     )
 
 

src/backend/InvenTree/common/tests.py

@@ -36,6 +36,7 @@
 from plugin import registry
 
 from .api import WebhookView
+from .media import rename_uploaded_file
 from .models import (
     Attachment,
     CustomUnit,
@@ -56,6 +57,21 @@
 CONTENT_TYPE_JSON = 'application/json'
 
 
+class MediaHelpersTest(TestCase):
+    """Unit tests for media helper functions."""
+
+    def test_rename_uploaded_file_preserves_extension(self):
+        """Ensure normal file extensions are preserved."""
+        upload_path = rename_uploaded_file('report.v1.txt', 'attachments', 'part', 123)
+
+        self.assertEqual(upload_path, 'part/123/attachments/report.v1.txt')
+
+    def test_rename_uploaded_file_rejects_dotdot_filename(self):
+        """Ensure explicit directory traversal token is blocked."""
+        with self.assertRaises(ValidationError):
+            rename_uploaded_file('..', 'attachments', 'part', 123)
+
+
 class AttachmentTest(InvenTreeAPITestCase):
     """Unit tests for the 'Attachment' model."""
 

src/backend/InvenTree/company/migrations/0001_initial.py

@@ -26,7 +26,7 @@ class Migration(migrations.Migration):
                 ('email', models.EmailField(blank=True, help_text='Contact email address', max_length=254)),
                 ('contact', models.CharField(blank=True, help_text='Point of contact', max_length=100)),
                 ('URL', models.URLField(blank=True, help_text='Link to external company information')),
-                ('image', models.ImageField(blank=True, max_length=255, null=True, upload_to=company.models.rename_company_image)),
+                ('image', models.ImageField(blank=True, max_length=255, null=True, upload_to='_image')),
                 ('notes', models.TextField(blank=True)),
                 ('is_customer', models.BooleanField(default=False, help_text='Do you sell items to this company?')),
                 ('is_supplier', models.BooleanField(default=True, help_text='Do you purchase items from this company?')),

src/backend/InvenTree/company/migrations/0014_auto_20200407_0116.py

@@ -15,6 +15,6 @@ class Migration(migrations.Migration):
         migrations.AlterField(
             model_name='company',
             name='image',
-            field=stdimage.models.StdImageField(blank=True, null=True, upload_to=company.models.rename_company_image),
+            field=stdimage.models.StdImageField(blank=True, null=True, upload_to='_image'),
         ),
     ]

src/backend/InvenTree/company/migrations/0032_auto_20210403_1837.py

@@ -18,7 +18,7 @@ class Migration(migrations.Migration):
         migrations.AlterField(
             model_name='company',
             name='image',
-            field=stdimage.models.StdImageField(blank=True, null=True, upload_to=company.models.rename_company_image, verbose_name='Image'),
+            field=stdimage.models.StdImageField(blank=True, null=True, upload_to='_image', verbose_name='Image'),
         ),
         migrations.AlterField(
             model_name='company',

src/backend/InvenTree/company/migrations/0046_alter_company_image.py

@@ -15,6 +15,6 @@ class Migration(migrations.Migration):
         migrations.AlterField(
             model_name='company',
             name='image',
-            field=stdimage.models.StdImageField(blank=True, force_min_size=False, null=True, upload_to=company.models.rename_company_image, variations={'preview': (256, 256), 'thumbnail': (128, 128)}, verbose_name='Image'),
+            field=stdimage.models.StdImageField(blank=True, force_min_size=False, null=True, upload_to='_image', variations={'preview': (256, 256), 'thumbnail': (128, 128)}, verbose_name='Image'),
         ),
     ]

src/backend/InvenTree/company/migrations/0076_alter_company_image.py

@@ -15,6 +15,6 @@ class Migration(migrations.Migration):
         migrations.AlterField(
             model_name='company',
             name='image',
-            field=stdimage.models.StdImageField(blank=True, force_min_size=False, null=True, upload_to=InvenTree.models.InvenTreeImageMixin.rename_image, variations={'preview': (256, 256), 'thumbnail': (128, 128)}, verbose_name='Image'),
+            field=stdimage.models.StdImageField(blank=True, force_min_size=False, null=True, upload_to=InvenTree.models.rename_image, variations={'preview': (256, 256), 'thumbnail': (128, 128)}, verbose_name='Image'),
         ),
     ]

src/backend/InvenTree/company/models.py

@@ -1,6 +1,5 @@
 """Company database model definitions."""
 
-import os
 from decimal import Decimal
 from typing import TypedDict
 
@@ -31,28 +30,6 @@
 from order.status_codes import PurchaseOrderStatusGroups
 
 
-def rename_company_image(instance, filename):
-    """Function to rename a company image after upload.
-
-    Args:
-        instance: Company object
-        filename: uploaded image filename
-
-    Returns:
-        New image filename
-    """
-    base = 'company_images'
-
-    ext = filename.split('.')[-1] if filename.count('.') > 0 else ''
-
-    fn = f'company_{instance.pk}_img'
-
-    if ext:
-        fn += '.' + ext
-
-    return os.path.join(base, fn)
-
-
 class CompanyReportContext(report.mixins.BaseReportContext, TypedDict):
     """Report context for the Company model.
 
@@ -111,7 +88,6 @@ class Company(
         tax_id: Tax ID for the company
     """
 
-    IMAGE_RENAME = rename_company_image
     IMPORT_ID_FIELDS = ['name']
 
     class Meta:

src/backend/InvenTree/company/tests.py

@@ -1,21 +1,13 @@
 """Unit tests for the models in the 'company' app."""
 
-import os
 from decimal import Decimal
 
 from django.core.exceptions import ValidationError
 from django.test import TestCase
 
 from part.models import Part
 
-from .models import (
-    Address,
-    Company,
-    Contact,
-    ManufacturerPart,
-    SupplierPart,
-    rename_company_image,
-)
+from .models import Address, Company, Contact, ManufacturerPart, SupplierPart
 
 
 class CompanySimpleTest(TestCase):
@@ -61,15 +53,6 @@ def test_company_url(self):
         c = Company.objects.get(pk=1)
         self.assertEqual(c.get_absolute_url(), '/web/purchasing/manufacturer/1')
 
-    def test_image_renamer(self):
-        """Test the company image upload functionality."""
-        c = Company.objects.get(pk=1)
-        rn = rename_company_image(c, 'test.png')
-        self.assertEqual(rn, 'company_images' + os.path.sep + 'company_1_img.png')
-
-        rn = rename_company_image(c, 'test2')
-        self.assertEqual(rn, 'company_images' + os.path.sep + 'company_1_img')
-
     def test_price_breaks(self):
         """Unit tests for price breaks."""
         self.assertTrue(self.acme0001.has_price_breaks)

src/backend/InvenTree/part/helpers.py

@@ -1,9 +1,5 @@
 """Various helper functions for the part app."""
 
-import os
-
-from django.conf import settings
-
 import structlog
 from jinja2.sandbox import SandboxedEnvironment
 
@@ -71,26 +67,3 @@ def render_part_full_name(part) -> str:
     # Fallback to the default format
     elements = [el for el in [part.IPN, part.name, part.revision] if el]
     return ' | '.join(elements)
-
-
-# Subdirectory for storing part images
-PART_IMAGE_DIR = 'part_images'
-
-
-def get_part_image_directory() -> str:
-    """Return the directory where part images are stored.
-
-    Returns:
-        str: Directory where part images are stored
-
-    TODO: Future work may be needed here to support other storage backends, such as S3
-    """
-    part_image_directory = os.path.abspath(
-        os.path.join(settings.MEDIA_ROOT, PART_IMAGE_DIR)
-    )
-
-    # Create the directory if it does not exist
-    if not os.path.exists(part_image_directory):
-        os.makedirs(part_image_directory)
-
-    return part_image_directory

src/backend/InvenTree/part/migrations/0001_initial.py

@@ -55,7 +55,7 @@ class Migration(migrations.Migration):
                 ('keywords', models.CharField(blank=True, help_text='Part keywords to improve visibility in search results', max_length=250)),
                 ('IPN', models.CharField(blank=True, help_text='Internal Part Number', max_length=100)),
                 ('URL', models.URLField(blank=True, help_text='Link to external URL')),
-                ('image', models.ImageField(blank=True, max_length=255, null=True, upload_to=part.models.rename_part_image)),
+                ('image', models.ImageField(blank=True, max_length=255, null=True, upload_to='_image')),
                 ('minimum_stock', models.PositiveIntegerField(default=0, help_text='Minimum allowed stock level', validators=[django.core.validators.MinValueValidator(0)])),
                 ('units', models.CharField(blank=True, default='pcs', help_text='Stock keeping units for this part', max_length=20)),
                 ('buildable', models.BooleanField(default=False, help_text='Can this part be built from other parts?')),

src/backend/InvenTree/part/migrations/0033_auto_20200404_0445.py

@@ -15,6 +15,6 @@ class Migration(migrations.Migration):
         migrations.AlterField(
             model_name='part',
             name='image',
-            field=stdimage.models.StdImageField(blank=True, null=True, upload_to=part.models.rename_part_image),
+            field=stdimage.models.StdImageField(blank=True, null=True, upload_to='_image', variations={'thumbnail': (128, 128)}, verbose_name='Image'),
         ),
     ]

src/backend/InvenTree/part/migrations/0064_auto_20210404_2016.py

@@ -88,7 +88,7 @@ class Migration(migrations.Migration):
         migrations.AlterField(
             model_name='part',
             name='image',
-            field=stdimage.models.StdImageField(blank=True, null=True, upload_to=part.models.rename_part_image, verbose_name='Image'),
+            field=stdimage.models.StdImageField(blank=True, null=True, upload_to='_image', verbose_name='Image'),
         ),
         migrations.AlterField(
             model_name='part',

src/backend/InvenTree/part/migrations/0080_alter_part_image.py

@@ -15,6 +15,6 @@ class Migration(migrations.Migration):
         migrations.AlterField(
             model_name='part',
             name='image',
-            field=stdimage.models.StdImageField(blank=True, force_min_size=False, null=True, upload_to=part.models.rename_part_image, variations={'preview': (256, 256), 'thumbnail': (128, 128)}, verbose_name='Image'),
+            field=stdimage.models.StdImageField(blank=True, force_min_size=False, null=True, upload_to='_image', variations={'preview': (256, 256), 'thumbnail': (128, 128)}, verbose_name='Image'),
         ),
     ]

src/backend/InvenTree/part/migrations/0143_alter_part_image.py

@@ -15,6 +15,6 @@ class Migration(migrations.Migration):
         migrations.AlterField(
             model_name='part',
             name='image',
-            field=stdimage.models.StdImageField(blank=True, force_min_size=False, null=True, upload_to=InvenTree.models.InvenTreeImageMixin.rename_image, variations={'preview': (256, 256), 'thumbnail': (128, 128)}, verbose_name='Image'),
+            field=stdimage.models.StdImageField(blank=True, force_min_size=False, null=True, upload_to=InvenTree.models.rename_image, variations={'preview': (256, 256), 'thumbnail': (128, 128)}, verbose_name='Image'),
         ),
     ]