From f8a34b258c425358ede683dd06b09d9fe30df2ec Mon Sep 17 00:00:00 2001 From: fengting124 Date: Sat, 11 Jul 2026 07:44:49 +0800 Subject: [PATCH 1/6] docs: plan upload trust boundary --- .../plans/2026-07-11-upload-trust-boundary.md | 98 +++++++++++++++++++ 1 file changed, 98 insertions(+) create mode 100644 docs/superpowers/plans/2026-07-11-upload-trust-boundary.md diff --git a/docs/superpowers/plans/2026-07-11-upload-trust-boundary.md b/docs/superpowers/plans/2026-07-11-upload-trust-boundary.md new file mode 100644 index 0000000..8a8304e --- /dev/null +++ b/docs/superpowers/plans/2026-07-11-upload-trust-boundary.md @@ -0,0 +1,98 @@ +# Upload Trust Boundary + +**Branch:** `feature/upload-trust-boundary` + +## Goal + +Turn image upload into an explicit untrusted-input boundary that rejects +spoofed, malformed, oversized, and unsafe inputs before they enter durable +business storage. + +## Research Basis + +- [OWASP File Upload Cheat Sheet](https://cheatsheetseries.owasp.org/cheatsheets/File_Upload_Cheat_Sheet.html) + recommends allowlisted formats, size limits, signature and content + validation, generated storage names, and storage outside the web root. +- [Oracle ImageReader API](https://docs.oracle.com/en/java/javase/25/docs/api/java.desktop/javax/imageio/ImageReader.html) + exposes image dimensions before full pixel decoding. +- [TwelveMonkeys ImageIO](https://github.com/haraldk/TwelveMonkeys) provides a + maintained WebP reader and documents dimension inspection before full decode. + +No single check is treated as authoritative. The implementation uses layered +validation and keeps accepted originals outside the public frontend assets. + +## Current Risks + +- The browser-provided `Content-Type` decides whether a file is accepted. +- Validation happens after the file is written to durable upload storage. +- `ImageIO.read` decodes before any width, height, or pixel-count limit. +- Stored filenames retain user-controlled text. +- The API claims WebP support although the JDK has no built-in WebP reader. +- Spring multipart limits are not defined in repository configuration. + +## Validation Pipeline + +```text +multipart request limit + -> non-empty and application byte limit + -> read bytes once + -> file signature allowlist (JPEG, PNG, WebP) + -> select ImageIO reader and confirm reader format + -> inspect width and height without pixel allocation + -> enforce width, height, and total pixel limits with overflow-safe math + -> fully decode first image to reject truncated/corrupt content + -> derive canonical MIME and extension from validated content + -> hash original evidence bytes + -> store under generated asset id and canonical extension + -> persist asset metadata and queued task +``` + +## Forensics-Specific Decision + +The backend does not re-encode accepted images. Re-encoding would remove or +change metadata and compression artifacts that can be useful forensic signals. +Instead, the original bytes are retained under a generated non-executable name +outside the web root. Antivirus or sandbox scanning remains a future adapter +for server deployment. + +## Defaults + +- maximum encoded bytes: 10 MiB; +- maximum width: 8192 pixels; +- maximum height: 8192 pixels; +- maximum decoded pixels: 25,000,000; +- maximum display filename: 255 characters. + +Spring's multipart request limit is set to the same file limit with a slightly +larger total request allowance. Application validation repeats the byte check +because transport configuration is not a domain guarantee. + +## Java Boundaries + +- `UploadPolicyProperties`: validated configuration with safe defaults. +- `ImageUploadInspector`: signature, reader, dimensions, full decode, and + canonical type detection. +- `ValidatedImageUpload`: immutable validated metadata and original bytes. +- `UploadFilenameSanitizer`: display-only filename normalization and length + bound. +- `LocalStorageService`: generated internal filename; no user-controlled path + component. +- `DetectionWorkflowService`: orchestration and persistence only. + +## TDD Sequence + +1. Test policy defaults and invalid configuration. +2. Test valid JPEG/PNG/WebP inspection and canonical type detection. +3. Test spoofed client MIME, invalid signatures, corrupt files, dimensions, + pixels, and encoded-byte limits. +4. Test generated storage names and path isolation. +5. Refactor workflow and controller tests around content-derived metadata. +6. Run all Java and cross-project verification, document operations, then use + PR and CI gates. + +## Deferred + +- Authentication, per-user quotas, and rate limiting. +- ClamAV or sandbox integration on the GPU/server environment. +- Object-storage quarantine lifecycle and asynchronous malware scan state. +- Docker-backed filesystem/database rollback tests. From ecbed502de64d3df2526679c2920be7022fc2681 Mon Sep 17 00:00:00 2001 From: fengting124 Date: Sat, 11 Jul 2026 07:48:12 +0800 Subject: [PATCH 2/6] feat: inspect untrusted image content --- backend-java/pom.xml | 5 + .../config/UploadPolicyProperties.java | 45 +++++ .../service/ImageUploadInspector.java | 159 ++++++++++++++++++ .../aigcforensics/service/InspectedImage.java | 8 + .../config/UploadPolicyPropertiesTest.java | 27 +++ .../service/ImageUploadInspectorTest.java | 127 ++++++++++++++ 6 files changed, 371 insertions(+) create mode 100644 backend-java/src/main/java/com/fengting/aigcforensics/config/UploadPolicyProperties.java create mode 100644 backend-java/src/main/java/com/fengting/aigcforensics/service/ImageUploadInspector.java create mode 100644 backend-java/src/main/java/com/fengting/aigcforensics/service/InspectedImage.java create mode 100644 backend-java/src/test/java/com/fengting/aigcforensics/config/UploadPolicyPropertiesTest.java create mode 100644 backend-java/src/test/java/com/fengting/aigcforensics/service/ImageUploadInspectorTest.java diff --git a/backend-java/pom.xml b/backend-java/pom.xml index a83e928..98b8905 100644 --- a/backend-java/pom.xml +++ b/backend-java/pom.xml @@ -52,6 +52,11 @@ springdoc-openapi-starter-webmvc-ui 2.8.14 + + com.twelvemonkeys.imageio + imageio-webp + 3.13.1 + com.h2database h2 diff --git a/backend-java/src/main/java/com/fengting/aigcforensics/config/UploadPolicyProperties.java b/backend-java/src/main/java/com/fengting/aigcforensics/config/UploadPolicyProperties.java new file mode 100644 index 0000000..f95ce19 --- /dev/null +++ b/backend-java/src/main/java/com/fengting/aigcforensics/config/UploadPolicyProperties.java @@ -0,0 +1,45 @@ +package com.fengting.aigcforensics.config; + +import org.springframework.boot.context.properties.ConfigurationProperties; + +@ConfigurationProperties(prefix = "app.upload") +public record UploadPolicyProperties( + Long maxBytes, + Integer maxWidth, + Integer maxHeight, + Long maxPixels, + Integer maxFilenameLength) { + + private static final long DEFAULT_MAX_BYTES = 10L * 1024 * 1024; + private static final int DEFAULT_MAX_DIMENSION = 8192; + private static final long DEFAULT_MAX_PIXELS = 25_000_000L; + private static final int DEFAULT_MAX_FILENAME_LENGTH = 255; + + public UploadPolicyProperties { + maxBytes = defaultIfNull(maxBytes, DEFAULT_MAX_BYTES); + maxWidth = defaultIfNull(maxWidth, DEFAULT_MAX_DIMENSION); + maxHeight = defaultIfNull(maxHeight, DEFAULT_MAX_DIMENSION); + maxPixels = defaultIfNull(maxPixels, DEFAULT_MAX_PIXELS); + maxFilenameLength = defaultIfNull(maxFilenameLength, DEFAULT_MAX_FILENAME_LENGTH); + + requirePositive("maxBytes", maxBytes); + requirePositive("maxWidth", maxWidth); + requirePositive("maxHeight", maxHeight); + requirePositive("maxPixels", maxPixels); + requirePositive("maxFilenameLength", maxFilenameLength); + } + + private static long defaultIfNull(Long value, long defaultValue) { + return value == null ? defaultValue : value; + } + + private static int defaultIfNull(Integer value, int defaultValue) { + return value == null ? defaultValue : value; + } + + private static void requirePositive(String name, long value) { + if (value <= 0) { + throw new IllegalArgumentException(name + " must be positive"); + } + } +} diff --git a/backend-java/src/main/java/com/fengting/aigcforensics/service/ImageUploadInspector.java b/backend-java/src/main/java/com/fengting/aigcforensics/service/ImageUploadInspector.java new file mode 100644 index 0000000..711bd34 --- /dev/null +++ b/backend-java/src/main/java/com/fengting/aigcforensics/service/ImageUploadInspector.java @@ -0,0 +1,159 @@ +package com.fengting.aigcforensics.service; + +import java.awt.image.BufferedImage; +import java.io.ByteArrayInputStream; +import java.io.IOException; +import java.util.Iterator; +import java.util.Locale; + +import javax.imageio.ImageIO; +import javax.imageio.ImageReader; +import javax.imageio.stream.ImageInputStream; + +import org.springframework.stereotype.Service; + +import com.fengting.aigcforensics.config.UploadPolicyProperties; + +@Service +public class ImageUploadInspector { + + private static final String UNSUPPORTED_MESSAGE = + "Only JPEG, PNG, and WebP image content is supported"; + private static final String CORRUPT_MESSAGE = "Uploaded image is corrupt or unreadable"; + + private final UploadPolicyProperties policy; + + public ImageUploadInspector(UploadPolicyProperties policy) { + this.policy = policy; + } + + public InspectedImage inspect(byte[] content) { + if (content == null || content.length == 0) { + throw new IllegalArgumentException("Uploaded file must not be empty"); + } + if (content.length > policy.maxBytes()) { + throw new IllegalArgumentException("Uploaded file exceeds the configured byte limit"); + } + + UploadImageFormat signatureFormat = UploadImageFormat.fromSignature(content); + try (ImageInputStream input = ImageIO.createImageInputStream(new ByteArrayInputStream(content))) { + if (input == null) { + throw new IllegalArgumentException(CORRUPT_MESSAGE); + } + Iterator readers = ImageIO.getImageReaders(input); + if (!readers.hasNext()) { + throw new IllegalArgumentException(CORRUPT_MESSAGE); + } + return inspectWithReader(input, readers.next(), signatureFormat); + } catch (IOException exception) { + throw new IllegalArgumentException(CORRUPT_MESSAGE, exception); + } + } + + private InspectedImage inspectWithReader( + ImageInputStream input, + ImageReader reader, + UploadImageFormat signatureFormat) { + try { + reader.setInput(input, false, true); + UploadImageFormat readerFormat = UploadImageFormat.fromReaderName(reader.getFormatName()); + if (readerFormat != signatureFormat) { + throw new IllegalArgumentException(CORRUPT_MESSAGE); + } + + int width = reader.getWidth(0); + int height = reader.getHeight(0); + validateDimensions(width, height); + + BufferedImage decoded = reader.read(0); + if (decoded == null || decoded.getWidth() != width || decoded.getHeight() != height) { + throw new IllegalArgumentException(CORRUPT_MESSAGE); + } + return new InspectedImage( + signatureFormat.contentType, + signatureFormat.extension, + width, + height); + } catch (IOException exception) { + throw new IllegalArgumentException(CORRUPT_MESSAGE, exception); + } finally { + reader.dispose(); + } + } + + private void validateDimensions(int width, int height) { + if (width <= 0 || height <= 0) { + throw new IllegalArgumentException(CORRUPT_MESSAGE); + } + if (width > policy.maxWidth() || height > policy.maxHeight()) { + throw new IllegalArgumentException( + "Image dimensions exceed the " + policy.maxWidth() + " x " + + policy.maxHeight() + " pixel limit"); + } + long pixels = (long) width * height; + if (pixels > policy.maxPixels()) { + throw new IllegalArgumentException( + "Image exceeds the " + policy.maxPixels() + " decoded pixel limit"); + } + } + + private enum UploadImageFormat { + JPEG("image/jpeg", "jpg"), + PNG("image/png", "png"), + WEBP("image/webp", "webp"); + + private final String contentType; + private final String extension; + + UploadImageFormat(String contentType, String extension) { + this.contentType = contentType; + this.extension = extension; + } + + private static UploadImageFormat fromSignature(byte[] content) { + if (startsWith(content, 0xFF, 0xD8, 0xFF)) { + return JPEG; + } + if (startsWith(content, 0x89, 0x50, 0x4E, 0x47, 0x0D, 0x0A, 0x1A, 0x0A)) { + return PNG; + } + if (content.length >= 12 + && asciiEquals(content, 0, "RIFF") + && asciiEquals(content, 8, "WEBP")) { + return WEBP; + } + throw new IllegalArgumentException(UNSUPPORTED_MESSAGE); + } + + private static UploadImageFormat fromReaderName(String readerName) { + String normalized = readerName.toLowerCase(Locale.ROOT); + return switch (normalized) { + case "jpeg", "jpg" -> JPEG; + case "png" -> PNG; + case "webp" -> WEBP; + default -> throw new IllegalArgumentException(UNSUPPORTED_MESSAGE); + }; + } + + private static boolean startsWith(byte[] content, int... signature) { + if (content.length < signature.length) { + return false; + } + for (int index = 0; index < signature.length; index++) { + if ((content[index] & 0xFF) != signature[index]) { + return false; + } + } + return true; + } + + private static boolean asciiEquals(byte[] content, int offset, String expected) { + for (int index = 0; index < expected.length(); index++) { + if (content[offset + index] != expected.charAt(index)) { + return false; + } + } + return true; + } + } +} diff --git a/backend-java/src/main/java/com/fengting/aigcforensics/service/InspectedImage.java b/backend-java/src/main/java/com/fengting/aigcforensics/service/InspectedImage.java new file mode 100644 index 0000000..81adef7 --- /dev/null +++ b/backend-java/src/main/java/com/fengting/aigcforensics/service/InspectedImage.java @@ -0,0 +1,8 @@ +package com.fengting.aigcforensics.service; + +public record InspectedImage( + String contentType, + String extension, + int width, + int height) { +} diff --git a/backend-java/src/test/java/com/fengting/aigcforensics/config/UploadPolicyPropertiesTest.java b/backend-java/src/test/java/com/fengting/aigcforensics/config/UploadPolicyPropertiesTest.java new file mode 100644 index 0000000..3e7def7 --- /dev/null +++ b/backend-java/src/test/java/com/fengting/aigcforensics/config/UploadPolicyPropertiesTest.java @@ -0,0 +1,27 @@ +package com.fengting.aigcforensics.config; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; + +import org.junit.jupiter.api.Test; + +class UploadPolicyPropertiesTest { + + @Test + void appliesProductionSafeDefaults() { + UploadPolicyProperties properties = new UploadPolicyProperties(null, null, null, null, null); + + assertThat(properties.maxBytes()).isEqualTo(10L * 1024 * 1024); + assertThat(properties.maxWidth()).isEqualTo(8192); + assertThat(properties.maxHeight()).isEqualTo(8192); + assertThat(properties.maxPixels()).isEqualTo(25_000_000L); + assertThat(properties.maxFilenameLength()).isEqualTo(255); + } + + @Test + void rejectsNonPositiveLimits() { + assertThatThrownBy(() -> new UploadPolicyProperties(0L, 100, 100, 100L, 100)) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("maxBytes"); + } +} diff --git a/backend-java/src/test/java/com/fengting/aigcforensics/service/ImageUploadInspectorTest.java b/backend-java/src/test/java/com/fengting/aigcforensics/service/ImageUploadInspectorTest.java new file mode 100644 index 0000000..0a7a602 --- /dev/null +++ b/backend-java/src/test/java/com/fengting/aigcforensics/service/ImageUploadInspectorTest.java @@ -0,0 +1,127 @@ +package com.fengting.aigcforensics.service; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; + +import java.awt.image.BufferedImage; +import java.io.ByteArrayOutputStream; +import java.io.IOException; +import java.util.Base64; + +import javax.imageio.ImageIO; + +import org.junit.jupiter.api.Test; + +import com.fengting.aigcforensics.config.UploadPolicyProperties; + +class ImageUploadInspectorTest { + + @Test + void detectsCanonicalPngTypeAndDimensionsFromContent() throws IOException { + ImageUploadInspector inspector = inspector(1024, 1024, 1_000_000); + + InspectedImage image = inspector.inspect(imageBytes("png", 320, 180)); + + assertThat(image.contentType()).isEqualTo("image/png"); + assertThat(image.extension()).isEqualTo("png"); + assertThat(image.width()).isEqualTo(320); + assertThat(image.height()).isEqualTo(180); + } + + @Test + void detectsJpegFromBytesRegardlessOfRequestMetadata() throws IOException { + ImageUploadInspector inspector = inspector(1024, 1024, 1_000_000); + + InspectedImage image = inspector.inspect(imageBytes("jpeg", 16, 12)); + + assertThat(image.contentType()).isEqualTo("image/jpeg"); + assertThat(image.extension()).isEqualTo("jpg"); + } + + @Test + void decodesWebpWithInstalledImageIoPlugin() { + byte[] onePixelWebp = Base64.getDecoder().decode( + "UklGRlYAAABXRUJQVlA4IDoAAADwAgCdASoBAAEAAEcIhYWIhYSIAgICdaoD+AP6" + + "Ag1NGAD+/vNYf/5gZt2KO//mBv/80F4SW6//zLwASUNNVAgAAAB0ZXN0MXgxAA=="); + + InspectedImage image = inspector(1024, 1024, 1_000_000).inspect(onePixelWebp); + + assertThat(image.contentType()).isEqualTo("image/webp"); + assertThat(image.extension()).isEqualTo("webp"); + assertThat(image.width()).isEqualTo(1); + assertThat(image.height()).isEqualTo(1); + } + + @Test + void rejectsUnknownSignatureBeforeDecode() { + ImageUploadInspector inspector = inspector(1024, 1024, 1_000_000); + + assertThatThrownBy(() -> inspector.inspect("not an image".getBytes())) + .isInstanceOf(IllegalArgumentException.class) + .hasMessage("Only JPEG, PNG, and WebP image content is supported"); + } + + @Test + void rejectsTruncatedImageWithValidSignature() { + ImageUploadInspector inspector = inspector(1024, 1024, 1_000_000); + byte[] truncatedPng = new byte[] { + (byte) 0x89, 0x50, 0x4e, 0x47, 0x0d, 0x0a, 0x1a, 0x0a, + 0x00, 0x00, 0x00, 0x00 + }; + + assertThatThrownBy(() -> inspector.inspect(truncatedPng)) + .isInstanceOf(IllegalArgumentException.class) + .hasMessage("Uploaded image is corrupt or unreadable"); + } + + @Test + void rejectsEncodedContentOverByteLimit() throws IOException { + byte[] content = imageBytes("png", 32, 32); + UploadPolicyProperties policy = new UploadPolicyProperties( + (long) content.length - 1, + 1024, + 1024, + 1_000_000L, + 255); + + assertThatThrownBy(() -> new ImageUploadInspector(policy).inspect(content)) + .isInstanceOf(IllegalArgumentException.class) + .hasMessage("Uploaded file exceeds the configured byte limit"); + } + + @Test + void rejectsDimensionsBeforeFullImageIsAccepted() throws IOException { + ImageUploadInspector inspector = inspector(100, 100, 10_000); + + assertThatThrownBy(() -> inspector.inspect(imageBytes("png", 101, 50))) + .isInstanceOf(IllegalArgumentException.class) + .hasMessage("Image dimensions exceed the 100 x 100 pixel limit"); + } + + @Test + void rejectsDecodedPixelCount() throws IOException { + ImageUploadInspector inspector = inspector(1000, 1000, 10_000); + + assertThatThrownBy(() -> inspector.inspect(imageBytes("png", 101, 100))) + .isInstanceOf(IllegalArgumentException.class) + .hasMessage("Image exceeds the 10000 decoded pixel limit"); + } + + private ImageUploadInspector inspector(int maxWidth, int maxHeight, long maxPixels) { + return new ImageUploadInspector(new UploadPolicyProperties( + 10L * 1024 * 1024, + maxWidth, + maxHeight, + maxPixels, + 255)); + } + + private byte[] imageBytes(String format, int width, int height) throws IOException { + BufferedImage image = new BufferedImage(width, height, BufferedImage.TYPE_INT_RGB); + ByteArrayOutputStream output = new ByteArrayOutputStream(); + if (!ImageIO.write(image, format, output)) { + throw new IllegalStateException("No image writer for " + format); + } + return output.toByteArray(); + } +} From b8cd9041a46ee82474944e5f7c10a7bbd6b584c3 Mon Sep 17 00:00:00 2001 From: fengting124 Date: Sat, 11 Jul 2026 07:51:47 +0800 Subject: [PATCH 3/6] feat: isolate validated upload storage --- .../service/DetectionWorkflowService.java | 74 +++++++++---------- .../service/ImageMetadataService.java | 28 ------- .../service/LocalStorageService.java | 52 +++++++++---- .../aigcforensics/service/StorageService.java | 2 +- .../service/UploadFilenameSanitizer.java | 54 ++++++++++++++ .../src/main/resources/application.yml | 10 +++ .../controller/DetectionControllerTest.java | 46 +++++++++++- .../service/ImageMetadataServiceTest.java | 37 ---------- .../service/LocalStorageServiceTest.java | 20 ++++- .../service/UploadFilenameSanitizerTest.java | 30 ++++++++ 10 files changed, 226 insertions(+), 127 deletions(-) delete mode 100644 backend-java/src/main/java/com/fengting/aigcforensics/service/ImageMetadataService.java create mode 100644 backend-java/src/main/java/com/fengting/aigcforensics/service/UploadFilenameSanitizer.java delete mode 100644 backend-java/src/test/java/com/fengting/aigcforensics/service/ImageMetadataServiceTest.java create mode 100644 backend-java/src/test/java/com/fengting/aigcforensics/service/UploadFilenameSanitizerTest.java diff --git a/backend-java/src/main/java/com/fengting/aigcforensics/service/DetectionWorkflowService.java b/backend-java/src/main/java/com/fengting/aigcforensics/service/DetectionWorkflowService.java index bcb87c2..3991c7d 100644 --- a/backend-java/src/main/java/com/fengting/aigcforensics/service/DetectionWorkflowService.java +++ b/backend-java/src/main/java/com/fengting/aigcforensics/service/DetectionWorkflowService.java @@ -4,8 +4,6 @@ import java.time.Clock; import java.time.Instant; import java.util.List; -import java.util.Locale; -import java.util.Set; import java.util.UUID; import org.springframework.beans.factory.annotation.Autowired; @@ -26,23 +24,19 @@ import com.fengting.aigcforensics.repository.DetectionTaskRepository; import com.fengting.aigcforensics.repository.MediaAssetRepository; import com.fengting.aigcforensics.repository.ModelPredictionRepository; -import com.fengting.aigcforensics.service.ImageMetadataService.ImageMetadata; +import com.fengting.aigcforensics.config.UploadPolicyProperties; import com.fengting.aigcforensics.service.StorageService.StoredFile; @Service public class DetectionWorkflowService { - private static final Set SUPPORTED_IMAGE_TYPES = Set.of( - "image/jpeg", - "image/png", - "image/webp"); - private static final String UNSUPPORTED_IMAGE_MESSAGE = "Only JPEG, PNG, and WebP images are supported"; - private final MediaAssetRepository mediaAssetRepository; private final DetectionTaskRepository detectionTaskRepository; private final StorageService storageService; private final HashService hashService; - private final ImageMetadataService imageMetadataService; + private final ImageUploadInspector imageUploadInspector; + private final UploadFilenameSanitizer filenameSanitizer; + private final UploadPolicyProperties uploadPolicy; private final ModelPredictionRepository modelPredictionRepository; private final DetectionReportRepository detectionReportRepository; private final Clock clock; @@ -53,7 +47,9 @@ public DetectionWorkflowService( DetectionTaskRepository detectionTaskRepository, StorageService storageService, HashService hashService, - ImageMetadataService imageMetadataService, + ImageUploadInspector imageUploadInspector, + UploadFilenameSanitizer filenameSanitizer, + UploadPolicyProperties uploadPolicy, ModelPredictionRepository modelPredictionRepository, DetectionReportRepository detectionReportRepository) { this( @@ -61,7 +57,9 @@ public DetectionWorkflowService( detectionTaskRepository, storageService, hashService, - imageMetadataService, + imageUploadInspector, + filenameSanitizer, + uploadPolicy, modelPredictionRepository, detectionReportRepository, Clock.systemUTC()); @@ -72,7 +70,9 @@ public DetectionWorkflowService( DetectionTaskRepository detectionTaskRepository, StorageService storageService, HashService hashService, - ImageMetadataService imageMetadataService, + ImageUploadInspector imageUploadInspector, + UploadFilenameSanitizer filenameSanitizer, + UploadPolicyProperties uploadPolicy, ModelPredictionRepository modelPredictionRepository, DetectionReportRepository detectionReportRepository, Clock clock) { @@ -80,7 +80,9 @@ public DetectionWorkflowService( this.detectionTaskRepository = detectionTaskRepository; this.storageService = storageService; this.hashService = hashService; - this.imageMetadataService = imageMetadataService; + this.imageUploadInspector = imageUploadInspector; + this.filenameSanitizer = filenameSanitizer; + this.uploadPolicy = uploadPolicy; this.modelPredictionRepository = modelPredictionRepository; this.detectionReportRepository = detectionReportRepository; this.clock = clock; @@ -91,9 +93,10 @@ public CreateImageDetectionResponse createImageDetection(MultipartFile file) { validateUpload(file); byte[] content = readContent(file); + InspectedImage inspectedImage = imageUploadInspector.inspect(content); String sha256 = hashService.sha256(content); MediaAsset asset = mediaAssetRepository.findBySha256(sha256) - .orElseGet(() -> storeNewAsset(file, content, sha256)); + .orElseGet(() -> storeNewAsset(file, content, sha256, inspectedImage)); DetectionTask task = new DetectionTask( newExternalId("task"), @@ -216,26 +219,30 @@ private void validateUpload(MultipartFile file) { if (file == null || file.isEmpty()) { throw new IllegalArgumentException("Uploaded file must not be empty"); } - - String contentType = normalizeContentType(file.getContentType()); - if (!SUPPORTED_IMAGE_TYPES.contains(contentType)) { - throw new IllegalArgumentException(UNSUPPORTED_IMAGE_MESSAGE); + if (file.getSize() > uploadPolicy.maxBytes()) { + throw new IllegalArgumentException("Uploaded file exceeds the configured byte limit"); } } - private MediaAsset storeNewAsset(MultipartFile file, byte[] content, String sha256) { + private MediaAsset storeNewAsset( + MultipartFile file, + byte[] content, + String sha256, + InspectedImage inspectedImage) { String assetId = newExternalId("asset"); - StoredFile storedFile = storageService.saveUpload(assetId, file.getOriginalFilename(), content); - ImageMetadata metadata = imageMetadataService.read(storedFile.path()); + StoredFile storedFile = storageService.saveAcceptedImage( + assetId, + inspectedImage.extension(), + content); MediaAsset asset = new MediaAsset( assetId, - safeOriginalFilename(file), - normalizeContentType(file.getContentType()), + filenameSanitizer.sanitize(file.getOriginalFilename()), + inspectedImage.contentType(), storedFile.size(), sha256, - metadata.width(), - metadata.height(), + inspectedImage.width(), + inspectedImage.height(), storedFile.path().toString(), null, Instant.now(clock)); @@ -250,21 +257,6 @@ private byte[] readContent(MultipartFile file) { } } - private String safeOriginalFilename(MultipartFile file) { - String filename = file.getOriginalFilename(); - if (filename == null || filename.isBlank()) { - return "upload.bin"; - } - return filename; - } - - private String normalizeContentType(String contentType) { - if (contentType == null) { - return ""; - } - return contentType.toLowerCase(Locale.ROOT); - } - private String newExternalId(String prefix) { return prefix + "_" + UUID.randomUUID().toString().replace("-", ""); } diff --git a/backend-java/src/main/java/com/fengting/aigcforensics/service/ImageMetadataService.java b/backend-java/src/main/java/com/fengting/aigcforensics/service/ImageMetadataService.java deleted file mode 100644 index c44ccd1..0000000 --- a/backend-java/src/main/java/com/fengting/aigcforensics/service/ImageMetadataService.java +++ /dev/null @@ -1,28 +0,0 @@ -package com.fengting.aigcforensics.service; - -import java.awt.image.BufferedImage; -import java.io.IOException; -import java.nio.file.Path; - -import javax.imageio.ImageIO; - -import org.springframework.stereotype.Service; - -@Service -public class ImageMetadataService { - - public ImageMetadata read(Path imagePath) { - try { - BufferedImage image = ImageIO.read(imagePath.toFile()); - if (image == null) { - throw new IllegalArgumentException("Unsupported or unreadable image: " + imagePath); - } - return new ImageMetadata(image.getWidth(), image.getHeight()); - } catch (IOException exception) { - throw new IllegalStateException("Failed to read image metadata for " + imagePath, exception); - } - } - - public record ImageMetadata(int width, int height) { - } -} diff --git a/backend-java/src/main/java/com/fengting/aigcforensics/service/LocalStorageService.java b/backend-java/src/main/java/com/fengting/aigcforensics/service/LocalStorageService.java index fd1b4ef..1d85283 100644 --- a/backend-java/src/main/java/com/fengting/aigcforensics/service/LocalStorageService.java +++ b/backend-java/src/main/java/com/fengting/aigcforensics/service/LocalStorageService.java @@ -3,7 +3,10 @@ import java.io.IOException; import java.nio.file.Files; import java.nio.file.Path; -import java.util.Objects; +import java.nio.file.StandardCopyOption; +import java.nio.file.StandardOpenOption; +import java.util.UUID; +import java.util.regex.Pattern; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.stereotype.Service; @@ -13,6 +16,9 @@ @Service public class LocalStorageService implements StorageService { + private static final Pattern ASSET_ID_PATTERN = Pattern.compile("[A-Za-z0-9_-]{1,64}"); + private static final Pattern EXTENSION_PATTERN = Pattern.compile("[a-z0-9]{1,10}"); + private final Path root; @Autowired @@ -25,35 +31,51 @@ public LocalStorageService(Path root) { } @Override - public StoredFile saveUpload(String assetId, String originalFilename, byte[] content) { - if (assetId == null || assetId.isBlank()) { - throw new IllegalArgumentException("assetId must not be blank"); - } + public StoredFile saveAcceptedImage(String assetId, String extension, byte[] content) { + requireSafeComponent(assetId, ASSET_ID_PATTERN, "assetId"); + requireSafeComponent(extension, EXTENSION_PATTERN, "extension"); if (content == null || content.length == 0) { throw new IllegalArgumentException("content must not be empty"); } - String safeFilename = sanitizeFilename(originalFilename); Path directory = root.resolve("uploads").resolve(assetId).normalize(); - Path output = directory.resolve(safeFilename).normalize(); + Path output = directory.resolve(assetId + "." + extension).normalize(); ensurePathInsideRoot(output); + Path temporary = directory.resolve(assetId + ".tmp-" + UUID.randomUUID()).normalize(); + ensurePathInsideRoot(temporary); try { Files.createDirectories(directory); - Files.write(output, content); + Files.write(temporary, content, StandardOpenOption.CREATE_NEW); + moveIntoPlace(temporary, output); return new StoredFile(output, content.length); } catch (IOException exception) { - throw new IllegalStateException("Failed to save upload " + safeFilename, exception); + throw new IllegalStateException("Failed to save accepted image " + assetId, exception); + } finally { + deleteTemporaryFile(temporary); + } + } + + private void requireSafeComponent(String value, Pattern pattern, String name) { + if (value == null || !pattern.matcher(value).matches()) { + throw new IllegalArgumentException(name + " contains unsupported characters"); } } - private String sanitizeFilename(String originalFilename) { - String filename = Path.of(Objects.requireNonNullElse(originalFilename, "upload.bin")).getFileName().toString(); - String sanitized = filename.replaceAll("[^A-Za-z0-9._-]", "_"); - if (sanitized.isBlank() || sanitized.equals(".") || sanitized.equals("..")) { - return "upload.bin"; + private void moveIntoPlace(Path temporary, Path output) throws IOException { + try { + Files.move(temporary, output, StandardCopyOption.ATOMIC_MOVE); + } catch (java.nio.file.AtomicMoveNotSupportedException exception) { + Files.move(temporary, output); + } + } + + private void deleteTemporaryFile(Path temporary) { + try { + Files.deleteIfExists(temporary); + } catch (IOException ignored) { + // A failed cleanup is less harmful than masking the original storage error. } - return sanitized; } private void ensurePathInsideRoot(Path output) { diff --git a/backend-java/src/main/java/com/fengting/aigcforensics/service/StorageService.java b/backend-java/src/main/java/com/fengting/aigcforensics/service/StorageService.java index fbd8a67..88028a5 100644 --- a/backend-java/src/main/java/com/fengting/aigcforensics/service/StorageService.java +++ b/backend-java/src/main/java/com/fengting/aigcforensics/service/StorageService.java @@ -4,7 +4,7 @@ public interface StorageService { - StoredFile saveUpload(String assetId, String originalFilename, byte[] content); + StoredFile saveAcceptedImage(String assetId, String extension, byte[] content); record StoredFile(Path path, long size) { } diff --git a/backend-java/src/main/java/com/fengting/aigcforensics/service/UploadFilenameSanitizer.java b/backend-java/src/main/java/com/fengting/aigcforensics/service/UploadFilenameSanitizer.java new file mode 100644 index 0000000..c0a77d6 --- /dev/null +++ b/backend-java/src/main/java/com/fengting/aigcforensics/service/UploadFilenameSanitizer.java @@ -0,0 +1,54 @@ +package com.fengting.aigcforensics.service; + +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.stereotype.Component; + +import com.fengting.aigcforensics.config.UploadPolicyProperties; + +@Component +public class UploadFilenameSanitizer { + + private static final String FALLBACK_FILENAME = "upload.bin"; + + private final int maxLength; + + @Autowired + public UploadFilenameSanitizer(UploadPolicyProperties policy) { + this(policy.maxFilenameLength()); + } + + UploadFilenameSanitizer(int maxLength) { + if (maxLength <= 0) { + throw new IllegalArgumentException("maxLength must be positive"); + } + this.maxLength = maxLength; + } + + public String sanitize(String originalFilename) { + if (originalFilename == null || originalFilename.isBlank()) { + return FALLBACK_FILENAME; + } + + int separator = Math.max(originalFilename.lastIndexOf('/'), originalFilename.lastIndexOf('\\')); + String leaf = originalFilename.substring(separator + 1).trim(); + if (leaf.isBlank() || leaf.equals(".") || leaf.equals("..")) { + return FALLBACK_FILENAME; + } + + StringBuilder sanitized = new StringBuilder(leaf.length()); + leaf.codePoints().forEach(codePoint -> sanitized.appendCodePoint(isControl(codePoint) ? '_' : codePoint)); + String result = sanitized.toString(); + int codePointCount = result.codePointCount(0, result.length()); + if (codePointCount > maxLength) { + result = result.substring(0, result.offsetByCodePoints(0, maxLength)); + } + return result.isBlank() ? FALLBACK_FILENAME : result; + } + + private boolean isControl(int codePoint) { + int type = Character.getType(codePoint); + return Character.isISOControl(codePoint) + || type == Character.LINE_SEPARATOR + || type == Character.PARAGRAPH_SEPARATOR; + } +} diff --git a/backend-java/src/main/resources/application.yml b/backend-java/src/main/resources/application.yml index 704a3c7..64dcb94 100644 --- a/backend-java/src/main/resources/application.yml +++ b/backend-java/src/main/resources/application.yml @@ -15,6 +15,10 @@ spring: redis: host: ${REDIS_HOST:localhost} port: ${REDIS_PORT:6379} + servlet: + multipart: + max-file-size: ${APP_UPLOAD_MAX_FILE_SIZE:10MB} + max-request-size: ${APP_UPLOAD_MAX_REQUEST_SIZE:11MB} server: port: ${SERVER_PORT:8080} @@ -22,6 +26,12 @@ server: app: storage: root: ${STORAGE_ROOT:storage} + upload: + max-bytes: ${APP_UPLOAD_MAX_BYTES:10485760} + max-width: ${APP_UPLOAD_MAX_WIDTH:8192} + max-height: ${APP_UPLOAD_MAX_HEIGHT:8192} + max-pixels: ${APP_UPLOAD_MAX_PIXELS:25000000} + max-filename-length: ${APP_UPLOAD_MAX_FILENAME_LENGTH:255} jobs: outbox: dispatcher-enabled: ${APP_JOBS_OUTBOX_DISPATCHER_ENABLED:true} diff --git a/backend-java/src/test/java/com/fengting/aigcforensics/controller/DetectionControllerTest.java b/backend-java/src/test/java/com/fengting/aigcforensics/controller/DetectionControllerTest.java index c88d0e0..006c0e4 100644 --- a/backend-java/src/test/java/com/fengting/aigcforensics/controller/DetectionControllerTest.java +++ b/backend-java/src/test/java/com/fengting/aigcforensics/controller/DetectionControllerTest.java @@ -114,7 +114,51 @@ void createImageDetectionRejectsNonImageUpload() throws Exception { mockMvc.perform(multipart("/api/detections/images").file(text)) .andExpect(status().isBadRequest()) - .andExpect(jsonPath("$.message").value("Only JPEG, PNG, and WebP images are supported")); + .andExpect(jsonPath("$.message") + .value("Only JPEG, PNG, and WebP image content is supported")); + } + + @Test + void createImageDetectionDerivesTypeFromContentInsteadOfRequestHeader() throws Exception { + MockMultipartFile image = new MockMultipartFile( + "file", + "mislabelled.txt", + "text/plain", + onePixelPng("mislabelled")); + + mockMvc.perform(multipart("/api/detections/images").file(image)) + .andExpect(status().isAccepted()) + .andExpect(jsonPath("$.filename").value("mislabelled.txt")) + .andExpect(jsonPath("$.contentType").value("image/png")); + } + + @Test + void createImageDetectionRejectsCorruptImageBeforeStorage() throws Exception { + MockMultipartFile image = new MockMultipartFile( + "file", + "corrupt.png", + "image/png", + new byte[] { + (byte) 0x89, 0x50, 0x4e, 0x47, 0x0d, 0x0a, 0x1a, 0x0a, + 0x00, 0x00, 0x00, 0x00 + }); + + mockMvc.perform(multipart("/api/detections/images").file(image)) + .andExpect(status().isBadRequest()) + .andExpect(jsonPath("$.message").value("Uploaded image is corrupt or unreadable")); + } + + @Test + void createImageDetectionSanitizesDisplayFilename() throws Exception { + MockMultipartFile image = new MockMultipartFile( + "file", + "../../folder\\evidence.png", + "image/png", + onePixelPng("filename")); + + mockMvc.perform(multipart("/api/detections/images").file(image)) + .andExpect(status().isAccepted()) + .andExpect(jsonPath("$.filename").value("evidence.png")); } @Test diff --git a/backend-java/src/test/java/com/fengting/aigcforensics/service/ImageMetadataServiceTest.java b/backend-java/src/test/java/com/fengting/aigcforensics/service/ImageMetadataServiceTest.java deleted file mode 100644 index 75412d5..0000000 --- a/backend-java/src/test/java/com/fengting/aigcforensics/service/ImageMetadataServiceTest.java +++ /dev/null @@ -1,37 +0,0 @@ -package com.fengting.aigcforensics.service; - -import static org.assertj.core.api.Assertions.assertThat; - -import java.awt.Color; -import java.awt.Graphics2D; -import java.awt.image.BufferedImage; -import java.nio.file.Path; - -import javax.imageio.ImageIO; - -import org.junit.jupiter.api.Test; -import org.junit.jupiter.api.io.TempDir; - -class ImageMetadataServiceTest { - - private final ImageMetadataService imageMetadataService = new ImageMetadataService(); - - @TempDir - private Path tempDir; - - @Test - void readsImageDimensions() throws Exception { - Path imagePath = tempDir.resolve("sample.png"); - BufferedImage image = new BufferedImage(320, 180, BufferedImage.TYPE_INT_RGB); - Graphics2D graphics = image.createGraphics(); - graphics.setColor(Color.WHITE); - graphics.fillRect(0, 0, 320, 180); - graphics.dispose(); - ImageIO.write(image, "png", imagePath.toFile()); - - ImageMetadataService.ImageMetadata metadata = imageMetadataService.read(imagePath); - - assertThat(metadata.width()).isEqualTo(320); - assertThat(metadata.height()).isEqualTo(180); - } -} diff --git a/backend-java/src/test/java/com/fengting/aigcforensics/service/LocalStorageServiceTest.java b/backend-java/src/test/java/com/fengting/aigcforensics/service/LocalStorageServiceTest.java index 1c667df..d5ad008 100644 --- a/backend-java/src/test/java/com/fengting/aigcforensics/service/LocalStorageServiceTest.java +++ b/backend-java/src/test/java/com/fengting/aigcforensics/service/LocalStorageServiceTest.java @@ -18,14 +18,14 @@ class LocalStorageServiceTest { void savesUploadUnderAssetDirectory() throws Exception { LocalStorageService storageService = new LocalStorageService(tempDir); - StorageService.StoredFile storedFile = storageService.saveUpload( + StorageService.StoredFile storedFile = storageService.saveAcceptedImage( "asset_001", - "../unsafe name.png", + "png", new byte[] { 1, 2, 3 }); assertThat(storedFile.size()).isEqualTo(3); assertThat(storedFile.path()).startsWith(tempDir.toAbsolutePath().normalize()); - assertThat(storedFile.path().getFileName().toString()).isEqualTo("unsafe_name.png"); + assertThat(storedFile.path().getFileName().toString()).isEqualTo("asset_001.png"); assertThat(Files.readAllBytes(storedFile.path())).containsExactly(1, 2, 3); } @@ -33,8 +33,20 @@ void savesUploadUnderAssetDirectory() throws Exception { void rejectsEmptyUpload() { LocalStorageService storageService = new LocalStorageService(tempDir); - assertThatThrownBy(() -> storageService.saveUpload("asset_001", "sample.png", new byte[0])) + assertThatThrownBy(() -> storageService.saveAcceptedImage("asset_001", "png", new byte[0])) .isInstanceOf(IllegalArgumentException.class) .hasMessageContaining("content"); } + + @Test + void rejectsUntrustedAssetIdAndExtension() { + LocalStorageService storageService = new LocalStorageService(tempDir); + + assertThatThrownBy(() -> storageService.saveAcceptedImage("../escape", "png", new byte[] { 1 })) + .isInstanceOf(IllegalArgumentException.class) + .hasMessage("assetId contains unsupported characters"); + assertThatThrownBy(() -> storageService.saveAcceptedImage("asset_001", "../exe", new byte[] { 1 })) + .isInstanceOf(IllegalArgumentException.class) + .hasMessage("extension contains unsupported characters"); + } } diff --git a/backend-java/src/test/java/com/fengting/aigcforensics/service/UploadFilenameSanitizerTest.java b/backend-java/src/test/java/com/fengting/aigcforensics/service/UploadFilenameSanitizerTest.java new file mode 100644 index 0000000..64c206b --- /dev/null +++ b/backend-java/src/test/java/com/fengting/aigcforensics/service/UploadFilenameSanitizerTest.java @@ -0,0 +1,30 @@ +package com.fengting.aigcforensics.service; + +import static org.assertj.core.api.Assertions.assertThat; + +import org.junit.jupiter.api.Test; + +class UploadFilenameSanitizerTest { + + private final UploadFilenameSanitizer sanitizer = new UploadFilenameSanitizer(20); + + @Test + void keepsOnlyLeafNameAcrossPlatformSeparators() { + assertThat(sanitizer.sanitize("../../folder\\evidence.png")) + .isEqualTo("evidence.png"); + } + + @Test + void removesControlCharactersAndBoundsDisplayLength() { + assertThat(sanitizer.sanitize("report\r\n