From e6473e15d789c99fa97545aec7f7ef9c00417166 Mon Sep 17 00:00:00 2001 From: Oliver Walters Date: Tue, 7 Jul 2026 04:10:51 +0000 Subject: [PATCH 1/7] Create a generic handler for renaming images --- src/backend/InvenTree/common/media.py | 71 +++++++++++++++++++++++++++ 1 file changed, 71 insertions(+) create mode 100644 src/backend/InvenTree/common/media.py diff --git a/src/backend/InvenTree/common/media.py b/src/backend/InvenTree/common/media.py new file mode 100644 index 000000000000..24c6dcf8872b --- /dev/null +++ b/src/backend/InvenTree/common/media.py @@ -0,0 +1,71 @@ +"""Common functions for handling media files in InvenTree.""" + +from pathlib import Path +from typing import Optional + +from django.core.exceptions import ValidationError + + +def common_file_upload_handler( + 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 + 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 file_path.parts.count() > 1 + or '..' in file_path.parts + ): + 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)) From d748938b8754dcf61b0b460c87b75df9e48398b0 Mon Sep 17 00:00:00 2001 From: Oliver Walters Date: Tue, 7 Jul 2026 04:19:03 +0000 Subject: [PATCH 2/7] Refactor rename_attachment --- src/backend/InvenTree/common/media.py | 10 +++------- src/backend/InvenTree/common/models.py | 18 +++++++----------- 2 files changed, 10 insertions(+), 18 deletions(-) diff --git a/src/backend/InvenTree/common/media.py b/src/backend/InvenTree/common/media.py index 24c6dcf8872b..b8efc902174d 100644 --- a/src/backend/InvenTree/common/media.py +++ b/src/backend/InvenTree/common/media.py @@ -6,7 +6,7 @@ from django.core.exceptions import ValidationError -def common_file_upload_handler( +def rename_uploaded_file( filename: str, file_type: str, model_type: Optional[str] = None, @@ -35,7 +35,7 @@ def common_file_upload_handler( raise ValidationError('File type must be specified.') # First, remove any illegal characters from the filename - illegal_chars = '\'"\\`~#|!@#$%^&*()[]{}<>?;:+=,' + illegal_chars = '\'"\\\\/.`~#|!@#$%^&*()[]{}<>?;:+=,' for c in illegal_chars: filename = filename.replace(c, '') @@ -43,11 +43,7 @@ def common_file_upload_handler( # Convert to a Path, ensure the filename is not attempting to traverse directories file_path = Path(filename) - if ( - file_path.is_absolute() - or file_path.parts.count() > 1 - or '..' in file_path.parts - ): + 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 diff --git a/src/backend/InvenTree/common/models.py b/src/backend/InvenTree/common/models.py index 9f198f0e920f..59a9d74a31e1 100644 --- a/src/backend/InvenTree/common/models.py +++ b/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///'. """ - # 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, ) From 19bd6a598b004fd95f788e31227a69d0ecbdde51 Mon Sep 17 00:00:00 2001 From: Oliver Walters Date: Tue, 7 Jul 2026 04:24:15 +0000 Subject: [PATCH 3/7] rename_stock_item_test_result_attachment --- src/backend/InvenTree/stock/models.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/backend/InvenTree/stock/models.py b/src/backend/InvenTree/stock/models.py index fe7286674f09..c7dd0b5b180f 100644 --- a/src/backend/InvenTree/stock/models.py +++ b/src/backend/InvenTree/stock/models.py @@ -2,7 +2,6 @@ from __future__ import annotations -import os from datetime import timedelta from decimal import Decimal, InvalidOperation @@ -3127,8 +3126,10 @@ def label(self): def rename_stock_item_test_result_attachment(instance, filename): """Rename test result.""" - return os.path.join( - 'stock_files', str(instance.stock_item.pk), os.path.basename(filename) + from common.media import rename_uploaded_file + + return rename_uploaded_file( + filename, 'test_result', model_type='stockitem', model_id=instance.stock_item.pk ) From 5c7fb055e15247ceb0e48ed4c9a5b8147026fd53 Mon Sep 17 00:00:00 2001 From: Oliver Walters Date: Tue, 7 Jul 2026 04:39:30 +0000 Subject: [PATCH 4/7] Fix for helper function --- src/backend/InvenTree/common/media.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/backend/InvenTree/common/media.py b/src/backend/InvenTree/common/media.py index b8efc902174d..f5776d4ea366 100644 --- a/src/backend/InvenTree/common/media.py +++ b/src/backend/InvenTree/common/media.py @@ -34,8 +34,9 @@ def rename_uploaded_file( if not file_type: raise ValidationError('File type must be specified.') - # First, remove any illegal characters from the filename - illegal_chars = '\'"\\\\/.`~#|!@#$%^&*()[]{}<>?;:+=,' + # 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, '') From 294758fdca0e8334cd354a58098bce3782e5c868 Mon Sep 17 00:00:00 2001 From: Oliver Walters Date: Tue, 7 Jul 2026 04:39:52 +0000 Subject: [PATCH 5/7] Refactor report uploads --- src/backend/InvenTree/common/tests.py | 16 ++++++++++++++++ src/backend/InvenTree/report/models.py | 17 ++++++++++------- 2 files changed, 26 insertions(+), 7 deletions(-) diff --git a/src/backend/InvenTree/common/tests.py b/src/backend/InvenTree/common/tests.py index 6935e3b47555..e7229f105645 100644 --- a/src/backend/InvenTree/common/tests.py +++ b/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.""" diff --git a/src/backend/InvenTree/report/models.py b/src/backend/InvenTree/report/models.py index 2fbaf3e0bd91..8a55a1fb4829 100644 --- a/src/backend/InvenTree/report/models.py +++ b/src/backend/InvenTree/report/models.py @@ -66,17 +66,20 @@ def rename_template(instance, filename): - Retains the original uploaded filename - Checks for duplicate filenames across instance class """ - path = instance.get_upload_path(filename) + from common.media import rename_uploaded_file - # Throw error if any other model instances reference this path - instance.check_existing_file(path, raise_error=True) + # Upload report template files to MEDIA_ROOT/report// + upload_path = rename_uploaded_file(filename, instance.SUBDIR, model_type='report') + + # Check for duplicate filenames across the model class + instance.check_existing_file(upload_path, raise_error=True) # Delete file with this name if it already exists - if default_storage.exists(path): - logger.info(f'Deleting existing template file: {path}') - default_storage.delete(path) + if default_storage.exists(upload_path): + logger.info(f'Deleting existing template file: {upload_path}') + default_storage.delete(upload_path) - return path + return upload_path class TemplateUploadMixin: From 1f0fa3566c9d33c8c14169ab5927df818103a454 Mon Sep 17 00:00:00 2001 From: Oliver Walters Date: Tue, 7 Jul 2026 04:53:57 +0000 Subject: [PATCH 6/7] Refactor image upload for part and company --- src/backend/InvenTree/InvenTree/models.py | 26 +++++++----------- .../company/migrations/0001_initial.py | 2 +- .../migrations/0014_auto_20200407_0116.py | 2 +- .../migrations/0032_auto_20210403_1837.py | 2 +- .../migrations/0046_alter_company_image.py | 2 +- .../migrations/0076_alter_company_image.py | 2 +- src/backend/InvenTree/company/models.py | 24 ----------------- src/backend/InvenTree/company/tests.py | 19 +------------ src/backend/InvenTree/part/helpers.py | 27 ------------------- .../InvenTree/part/migrations/0001_initial.py | 2 +- .../migrations/0033_auto_20200404_0445.py | 2 +- .../migrations/0064_auto_20210404_2016.py | 2 +- .../part/migrations/0080_alter_part_image.py | 2 +- .../part/migrations/0143_alter_part_image.py | 2 +- src/backend/InvenTree/part/models.py | 18 ------------- src/backend/InvenTree/part/serializers.py | 17 ++++-------- src/backend/InvenTree/part/test_part.py | 8 ------ 17 files changed, 26 insertions(+), 133 deletions(-) diff --git a/src/backend/InvenTree/InvenTree/models.py b/src/backend/InvenTree/InvenTree/models.py index 31c68e87309f..04d70de3ab6d 100644 --- a/src/backend/InvenTree/InvenTree/models.py +++ b/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, diff --git a/src/backend/InvenTree/company/migrations/0001_initial.py b/src/backend/InvenTree/company/migrations/0001_initial.py index 6c6f31f9386e..d1acf715ff26 100644 --- a/src/backend/InvenTree/company/migrations/0001_initial.py +++ b/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?')), diff --git a/src/backend/InvenTree/company/migrations/0014_auto_20200407_0116.py b/src/backend/InvenTree/company/migrations/0014_auto_20200407_0116.py index 03985a1ef329..cd5aa460196d 100644 --- a/src/backend/InvenTree/company/migrations/0014_auto_20200407_0116.py +++ b/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'), ), ] diff --git a/src/backend/InvenTree/company/migrations/0032_auto_20210403_1837.py b/src/backend/InvenTree/company/migrations/0032_auto_20210403_1837.py index 8b5f6fb89ff4..267882a9ab50 100644 --- a/src/backend/InvenTree/company/migrations/0032_auto_20210403_1837.py +++ b/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', diff --git a/src/backend/InvenTree/company/migrations/0046_alter_company_image.py b/src/backend/InvenTree/company/migrations/0046_alter_company_image.py index 518f6453d5f0..27892a9a7dfc 100644 --- a/src/backend/InvenTree/company/migrations/0046_alter_company_image.py +++ b/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'), ), ] diff --git a/src/backend/InvenTree/company/migrations/0076_alter_company_image.py b/src/backend/InvenTree/company/migrations/0076_alter_company_image.py index dc59cf677d9f..4dbbfd199357 100644 --- a/src/backend/InvenTree/company/migrations/0076_alter_company_image.py +++ b/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'), ), ] diff --git a/src/backend/InvenTree/company/models.py b/src/backend/InvenTree/company/models.py index 29af645fbf7e..ef19ab36eeb9 100644 --- a/src/backend/InvenTree/company/models.py +++ b/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: diff --git a/src/backend/InvenTree/company/tests.py b/src/backend/InvenTree/company/tests.py index b5389bfbebbe..c8f3cdafbced 100644 --- a/src/backend/InvenTree/company/tests.py +++ b/src/backend/InvenTree/company/tests.py @@ -1,6 +1,5 @@ """Unit tests for the models in the 'company' app.""" -import os from decimal import Decimal from django.core.exceptions import ValidationError @@ -8,14 +7,7 @@ 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) diff --git a/src/backend/InvenTree/part/helpers.py b/src/backend/InvenTree/part/helpers.py index 7c55c9810e20..4ed7998d1188 100644 --- a/src/backend/InvenTree/part/helpers.py +++ b/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 diff --git a/src/backend/InvenTree/part/migrations/0001_initial.py b/src/backend/InvenTree/part/migrations/0001_initial.py index bc7d0a8c9edf..98a8e6abede8 100644 --- a/src/backend/InvenTree/part/migrations/0001_initial.py +++ b/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?')), diff --git a/src/backend/InvenTree/part/migrations/0033_auto_20200404_0445.py b/src/backend/InvenTree/part/migrations/0033_auto_20200404_0445.py index 4c2b8c1c968c..7e4adedd614b 100644 --- a/src/backend/InvenTree/part/migrations/0033_auto_20200404_0445.py +++ b/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'), ), ] diff --git a/src/backend/InvenTree/part/migrations/0064_auto_20210404_2016.py b/src/backend/InvenTree/part/migrations/0064_auto_20210404_2016.py index cf15ab0beab8..0f0d83ea5030 100644 --- a/src/backend/InvenTree/part/migrations/0064_auto_20210404_2016.py +++ b/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', diff --git a/src/backend/InvenTree/part/migrations/0080_alter_part_image.py b/src/backend/InvenTree/part/migrations/0080_alter_part_image.py index 9632795cc8c1..13158f04350c 100644 --- a/src/backend/InvenTree/part/migrations/0080_alter_part_image.py +++ b/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'), ), ] diff --git a/src/backend/InvenTree/part/migrations/0143_alter_part_image.py b/src/backend/InvenTree/part/migrations/0143_alter_part_image.py index 89565df62a3b..1dde009a0859 100644 --- a/src/backend/InvenTree/part/migrations/0143_alter_part_image.py +++ b/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'), ), ] diff --git a/src/backend/InvenTree/part/models.py b/src/backend/InvenTree/part/models.py index 03e0b5f1381a..2aa3d8c3c1aa 100644 --- a/src/backend/InvenTree/part/models.py +++ b/src/backend/InvenTree/part/models.py @@ -5,7 +5,6 @@ import hashlib import inspect import math -import os import re from datetime import timedelta from decimal import ROUND_HALF_UP, Decimal, InvalidOperation @@ -331,22 +330,6 @@ def set_starred(self, user, status: bool, **kwargs) -> None: PartCategoryStar.objects.filter(category=self, user=user).delete() -def rename_part_image(instance, filename): - """Function for renaming a part image file. - - Args: - instance: Instance of a Part object - filename: Name of original uploaded file - - Returns: - Cleaned filename in format part__img - """ - base = part_helpers.PART_IMAGE_DIR - fname = os.path.basename(filename) - - return os.path.join(base, fname) - - class PartCategoryParameterTemplate(InvenTree.models.InvenTreeMetadataModel): """A PartCategoryParameterTemplate creates a unique relationship between a PartCategory and a ParameterTemplate. @@ -517,7 +500,6 @@ class Part( """ NODE_PARENT_KEY = 'variant_of' - IMAGE_RENAME = rename_part_image IMPORT_ID_FIELDS = ['IPN', 'name'] objects = TreeManager() diff --git a/src/backend/InvenTree/part/serializers.py b/src/backend/InvenTree/part/serializers.py index e8b2ecff29c6..809d0c1a27ef 100644 --- a/src/backend/InvenTree/part/serializers.py +++ b/src/backend/InvenTree/part/serializers.py @@ -1,6 +1,5 @@ """DRF data serializers for Part app.""" -import os from decimal import Decimal from django.core.exceptions import ValidationError @@ -26,7 +25,6 @@ import InvenTree.helpers import InvenTree.serializers import part.filters as part_filters -import part.helpers as part_helpers import stock.models import users.models from data_exporter.mixins import DataExportSerializerMixin @@ -1017,13 +1015,8 @@ def validate_existing_image(self, img): if not img: return img - img = img.split(os.path.sep)[-1] - - # Ensure that the file actually exists - img_path = os.path.join(part_helpers.get_part_image_directory(), img) - - if not os.path.exists(img_path) or not os.path.isfile(img_path): - raise ValidationError(_('Image file does not exist')) + # TODO: REFACTOR THIS TO USE django storages API + raise ValidationError('THIS NEEDS TO BE REFACTORED') return img @@ -1117,10 +1110,10 @@ def save(self): existing_image = data.pop('existing_image', None) if existing_image: - img_path = os.path.join(part_helpers.PART_IMAGE_DIR, existing_image) - - part.image = img_path + # img_path = os.path.join(part_helpers.PART_IMAGE_DIR, existing_image) + # part.image = img_path part.save() + raise ValidationError('THIS NEEDS TO BE REFACTORED') return self.instance diff --git a/src/backend/InvenTree/part/test_part.py b/src/backend/InvenTree/part/test_part.py index 05e58d225ee5..e07020295c9d 100644 --- a/src/backend/InvenTree/part/test_part.py +++ b/src/backend/InvenTree/part/test_part.py @@ -1,7 +1,5 @@ """Tests for the Part model.""" -import os - from django.conf import settings from django.core.cache import cache from django.core.exceptions import ValidationError @@ -21,7 +19,6 @@ PartRelated, PartStar, PartTestTemplate, - rename_part_image, ) @@ -233,11 +230,6 @@ def test_category(self): self.assertIsNone(orphan.category) self.assertEqual(orphan.category_path, '') - def test_rename_img(self): - """Test that an image can be renamed.""" - img = rename_part_image(self.r1, 'hello.png') - self.assertEqual(img, os.path.join('part_images', 'hello.png')) - def test_stock(self): """Test case where there is zero stock.""" res = Part.objects.filter(description__contains='resistor') From 1d92f750d78b2f9ae885f5de180b5790cff57775 Mon Sep 17 00:00:00 2001 From: Oliver Walters Date: Tue, 7 Jul 2026 11:26:19 +0000 Subject: [PATCH 7/7] Use proper django checking --- src/backend/InvenTree/common/media.py | 25 ++++++++++--------------- 1 file changed, 10 insertions(+), 15 deletions(-) diff --git a/src/backend/InvenTree/common/media.py b/src/backend/InvenTree/common/media.py index f5776d4ea366..c4023985e4e7 100644 --- a/src/backend/InvenTree/common/media.py +++ b/src/backend/InvenTree/common/media.py @@ -3,7 +3,9 @@ from pathlib import Path from typing import Optional -from django.core.exceptions import ValidationError +from django.core.exceptions import SuspiciousFileOperation, ValidationError +from django.core.files.utils import validate_file_name +from django.utils.translation import gettext_lazy as _ def rename_uploaded_file( @@ -34,18 +36,11 @@ def rename_uploaded_file( 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.') + # Ensure the filename is not attempting to traverse directories + try: + validate_file_name(filename, allow_relative_path=False) + except SuspiciousFileOperation: + raise ValidationError(_('Invalid filename')) # Construct an upload path based on the provided parts parts = [] @@ -61,8 +56,8 @@ def rename_uploaded_file( # Include the file type in the path parts.append(str(file_type)) - # Finally, include the sanitized filename - parts.append(file_path.name) + # Finally, include the validated filename + parts.append(filename) # Join all parts to form the final upload path return str(Path(*parts))