fix(security): enforce maximum file upload size limit #112

New Issue

marcel · 2026-03-27T17:33:13+01:00

marcel commented

2026-03-27 17:33:13 +01:00

Problem

FileService accepts any file size. There is no configured limit in Spring Boot's multipart settings. A user (or attacker) can upload arbitrarily large files, potentially:

Filling the MinIO disk volume
Causing OOM in the backend during stream processing
Degrading performance for all users during a large upload

Note: #84 addresses MIME type validation. This issue covers size enforcement separately.

Fix

application.yml (all profiles):

spring:
  servlet:
    multipart:
      max-file-size: 100MB
      max-request-size: 105MB

Handle the resulting MaxUploadSizeExceededException in GlobalExceptionHandler:

@ExceptionHandler(MaxUploadSizeExceededException.class)
@ResponseStatus(HttpStatus.PAYLOAD_TOO_LARGE)
public ErrorResponse handleFileTooLarge(MaxUploadSizeExceededException ex) {
    return new ErrorResponse(ErrorCode.FILE_TOO_LARGE, "File exceeds the maximum allowed size of 100 MB");
}

Add FILE_TOO_LARGE to ErrorCode enum and mirror in the frontend errors.ts + translation files.

Acceptance Criteria

Uploading a file > 100 MB returns HTTP 413 with a user-readable error message
Frontend displays a translated error message for the FILE_TOO_LARGE code
Limit is configurable via application-prod.yml without code changes

## Problem `FileService` accepts any file size. There is no configured limit in Spring Boot's multipart settings. A user (or attacker) can upload arbitrarily large files, potentially: - Filling the MinIO disk volume - Causing OOM in the backend during stream processing - Degrading performance for all users during a large upload Note: #84 addresses MIME type validation. This issue covers size enforcement separately. ## Fix **application.yml (all profiles):** ```yaml spring: servlet: multipart: max-file-size: 100MB max-request-size: 105MB ``` Handle the resulting `MaxUploadSizeExceededException` in `GlobalExceptionHandler`: ```java @ExceptionHandler(MaxUploadSizeExceededException.class) @ResponseStatus(HttpStatus.PAYLOAD_TOO_LARGE) public ErrorResponse handleFileTooLarge(MaxUploadSizeExceededException ex) { return new ErrorResponse(ErrorCode.FILE_TOO_LARGE, "File exceeds the maximum allowed size of 100 MB"); } ``` Add `FILE_TOO_LARGE` to `ErrorCode` enum and mirror in the frontend `errors.ts` + translation files. ## Acceptance Criteria - Uploading a file > 100 MB returns HTTP 413 with a user-readable error message - Frontend displays a translated error message for the `FILE_TOO_LARGE` code - Limit is configurable via `application-prod.yml` without code changes

marcel added the file-upload security labels 2026-03-31 20:49:51 +02:00

marcel commented

2026-03-31 20:58:34 +02:00

👨‍💻 Felix Brandt — Senior Fullstack Developer

Questions & Observations

The config change is trivial, but the ErrorCode chain is where the work is: add FILE_TOO_LARGE to the Java enum, mirror it in errors.ts, add the translation key to all three locale files (de/en/es), and add the Paraglide message. That's four touch points — easy to miss one.
MaxUploadSizeExceededException is thrown by the Multipart resolver filter before the request reaches the DispatcherServlet. A @ExceptionHandler in a @RestController won't catch it — you need @ControllerAdvice (i.e. a GlobalExceptionHandler bean). Confirm that's what's in place.
The frontend also needs to catch the 413 response and surface the translated error. The upload form likely uses raw fetch for multipart — make sure the error handling path there reads the code field from the response body.

Suggestions

Write the test first: a @SpringBootTest (or @WebMvcTest with multipart config) that uploads a mock file of 101MB and asserts 413 + FILE_TOO_LARGE in the response body.
Also add a boundary test: 99MB file returns 200. Without this, you won't catch an off-by-one in the config.
Add a frontend Vitest test: mock a 413 response and assert the FILE_TOO_LARGE translation key renders in the UI.

## 👨‍💻 Felix Brandt — Senior Fullstack Developer ### Questions & Observations - The config change is trivial, but the `ErrorCode` chain is where the work is: add `FILE_TOO_LARGE` to the Java enum, mirror it in `errors.ts`, add the translation key to all three locale files (de/en/es), and add the Paraglide message. That's four touch points — easy to miss one. - `MaxUploadSizeExceededException` is thrown by the Multipart resolver filter *before* the request reaches the `DispatcherServlet`. A `@ExceptionHandler` in a `@RestController` won't catch it — you need `@ControllerAdvice` (i.e. a `GlobalExceptionHandler` bean). Confirm that's what's in place. - The frontend also needs to catch the 413 response and surface the translated error. The upload form likely uses raw `fetch` for multipart — make sure the error handling path there reads the `code` field from the response body. ### Suggestions - Write the test first: a `@SpringBootTest` (or `@WebMvcTest` with multipart config) that uploads a mock file of 101MB and asserts 413 + `FILE_TOO_LARGE` in the response body. - Also add a boundary test: 99MB file returns 200. Without this, you won't catch an off-by-one in the config. - Add a frontend Vitest test: mock a 413 response and assert the `FILE_TOO_LARGE` translation key renders in the UI.

marcel commented

2026-03-31 21:00:29 +02:00

🔒 Nora "NullX" Steiner — Application Security Engineer

Questions & Observations

Is 100 MB the right limit? For a family document archive of scanned letters, photos, and PDFs, 100 MB is generous. A high-resolution scan is typically 5–20 MB. Consider 50 MB as the default — it still covers edge cases (large photo albums, multi-page docs) while significantly reducing the DoS surface. Make it overrideable in application-prod.yml as the issue suggests.
Client-side validation matters too. A size check before the upload starts (in the SvelteKit upload form) saves bandwidth and gives immediate feedback. The backend limit is the security control; the frontend check is the UX improvement. Both are needed.
MaxUploadSizeExceededException is thrown by the Jetty/Tomcat multipart filter — before Spring Security, before the controller. This means an unauthenticated user can trigger it. The size limit is effectively a public DoS mitigation, not just an authenticated-user concern. Good — just make sure the 413 response doesn't accidentally expose stack traces.
MinIO also has object size limits. If the Spring limit is 100 MB but MinIO's configured max is lower (or unlimited), they'll diverge. Align them.

Suggestions

The GlobalExceptionHandler that catches MaxUploadSizeExceededException should log at INFO level (not WARN or ERROR) — it's a user error, not a system error. Logging at WARN will pollute your alert channel with false positives once real users hit the limit.
CWE-400: Uncontrolled Resource Consumption.
Add a test for the unauthenticated case: an anonymous user POSTing a 101 MB file should also get 413, not 401 or 500.

## 🔒 Nora "NullX" Steiner — Application Security Engineer ### Questions & Observations - **Is 100 MB the right limit?** For a family document archive of scanned letters, photos, and PDFs, 100 MB is generous. A high-resolution scan is typically 5–20 MB. Consider 50 MB as the default — it still covers edge cases (large photo albums, multi-page docs) while significantly reducing the DoS surface. Make it overrideable in `application-prod.yml` as the issue suggests. - **Client-side validation matters too.** A size check before the upload starts (in the SvelteKit upload form) saves bandwidth and gives immediate feedback. The backend limit is the security control; the frontend check is the UX improvement. Both are needed. - **`MaxUploadSizeExceededException` is thrown by the Jetty/Tomcat multipart filter** — before Spring Security, before the controller. This means an unauthenticated user can trigger it. The size limit is effectively a public DoS mitigation, not just an authenticated-user concern. Good — just make sure the 413 response doesn't accidentally expose stack traces. - **MinIO also has object size limits.** If the Spring limit is 100 MB but MinIO's configured max is lower (or unlimited), they'll diverge. Align them. ### Suggestions - The `GlobalExceptionHandler` that catches `MaxUploadSizeExceededException` should log at INFO level (not WARN or ERROR) — it's a user error, not a system error. Logging at WARN will pollute your alert channel with false positives once real users hit the limit. - CWE-400: Uncontrolled Resource Consumption. - Add a test for the *unauthenticated* case: an anonymous user POSTing a 101 MB file should also get 413, not 401 or 500.

marcel commented

2026-03-31 21:02:15 +02:00

🧪 Sara Holt — QA Engineer & Test Strategist

Test Strategy

Backend integration test — @SpringBootTest:

@Test
void shouldReturn413WhenFileSizeExceedsLimit() throws Exception {
    byte[] largeFile = new byte[101 * 1024 * 1024]; // 101 MB
    MockMultipartFile file = new MockMultipartFile("file", "big.pdf", "application/pdf", largeFile);

    mockMvc.perform(multipart("/api/documents").file(file).with(user("admin")))
           .andExpect(status().isPayloadTooLarge())
           .andExpect(jsonPath("$.code").value("FILE_TOO_LARGE"));
}

@Test
void shouldAcceptFileJustUnderLimit() throws Exception {
    byte[] acceptableFile = new byte[99 * 1024 * 1024]; // 99 MB
    MockMultipartFile file = new MockMultipartFile("file", "ok.pdf", "application/pdf", acceptableFile);

    mockMvc.perform(multipart("/api/documents").file(file).with(user("admin")))
           .andExpect(status().isOk()); // or 201
}

Edge Cases to Cover

Unauthenticated user uploading oversized file → 413, not 401
Frontend displays FILE_TOO_LARGE translated message (Vitest, mock 413 response)
All three locale files have the translation key (static check — just grep for the key)

Observations

@SpringBootTest is needed here (not @WebMvcTest) because MaxUploadSizeExceededException is thrown by the Multipart filter, which is part of the full servlet stack. A @WebMvcTest slice may not exercise this path correctly — test at the full stack layer to be sure.

## 🧪 Sara Holt — QA Engineer & Test Strategist ### Test Strategy **Backend integration test — `@SpringBootTest`:** ```java @Test void shouldReturn413WhenFileSizeExceedsLimit() throws Exception { byte[] largeFile = new byte[101 * 1024 * 1024]; // 101 MB MockMultipartFile file = new MockMultipartFile("file", "big.pdf", "application/pdf", largeFile); mockMvc.perform(multipart("/api/documents").file(file).with(user("admin"))) .andExpect(status().isPayloadTooLarge()) .andExpect(jsonPath("$.code").value("FILE_TOO_LARGE")); } @Test void shouldAcceptFileJustUnderLimit() throws Exception { byte[] acceptableFile = new byte[99 * 1024 * 1024]; // 99 MB MockMultipartFile file = new MockMultipartFile("file", "ok.pdf", "application/pdf", acceptableFile); mockMvc.perform(multipart("/api/documents").file(file).with(user("admin"))) .andExpect(status().isOk()); // or 201 } ``` ### Edge Cases to Cover - Unauthenticated user uploading oversized file → 413, not 401 - Frontend displays `FILE_TOO_LARGE` translated message (Vitest, mock 413 response) - All three locale files have the translation key (static check — just `grep` for the key) ### Observations - `@SpringBootTest` is needed here (not `@WebMvcTest`) because `MaxUploadSizeExceededException` is thrown by the Multipart filter, which is part of the full servlet stack. A `@WebMvcTest` slice may not exercise this path correctly — test at the full stack layer to be sure.

marcel commented

2026-03-31 21:04:12 +02:00

🏗️ Markus Keller — Application Architect

Questions & Observations

@ControllerAdvice is the correct mechanism, not @ExceptionHandler in a controller. MaxUploadSizeExceededException is thrown by the multipart resolver filter before the DispatcherServlet dispatches to a controller. A @ControllerAdvice bean is picked up by Spring's exception handling chain at the right level. Confirm GlobalExceptionHandler is annotated with @ControllerAdvice (or @RestControllerAdvice), not just @Component.
Configurable limit via application-prod.yml is the right call. The default in application.yml should be conservative (50–100 MB); the production override can be tuned to the actual use case. This is exactly the config-profile pattern this project should use — no code changes for environment tuning.
ErrorCode propagation. The issue correctly identifies the full chain: Java enum → errors.ts → Paraglide messages. This is a four-file change. Worth a checklist in the PR description to make sure none are missed — the frontend will silently show a generic error if the code isn't mirrored.

Suggestions

Consider whether FILE_TOO_LARGE should also be handled on the frontend before the request is sent — a <input type="file"> change handler can check file.size and show an error immediately without a round-trip. This is UX, not a security control, but it's a better experience.
The max-request-size: 105MB (5 MB above max-file-size) is correct and intentional — it accounts for form field overhead. Document this relationship with a comment in the YAML.

## 🏗️ Markus Keller — Application Architect ### Questions & Observations - **`@ControllerAdvice` is the correct mechanism, not `@ExceptionHandler` in a controller.** `MaxUploadSizeExceededException` is thrown by the multipart resolver filter before the `DispatcherServlet` dispatches to a controller. A `@ControllerAdvice` bean is picked up by Spring's exception handling chain at the right level. Confirm `GlobalExceptionHandler` is annotated with `@ControllerAdvice` (or `@RestControllerAdvice`), not just `@Component`. - **Configurable limit via `application-prod.yml` is the right call.** The default in `application.yml` should be conservative (50–100 MB); the production override can be tuned to the actual use case. This is exactly the config-profile pattern this project should use — no code changes for environment tuning. - **ErrorCode propagation.** The issue correctly identifies the full chain: Java enum → `errors.ts` → Paraglide messages. This is a four-file change. Worth a checklist in the PR description to make sure none are missed — the frontend will silently show a generic error if the code isn't mirrored. ### Suggestions - Consider whether `FILE_TOO_LARGE` should also be handled on the frontend *before* the request is sent — a `<input type="file">` change handler can check `file.size` and show an error immediately without a round-trip. This is UX, not a security control, but it's a better experience. - The `max-request-size: 105MB` (5 MB above `max-file-size`) is correct and intentional — it accounts for form field overhead. Document this relationship with a comment in the YAML.

marcel commented

2026-03-31 21:11:47 +02:00

🎨 Leonie Voss — UI/UX Designer & Accessibility Strategist

Questions & Observations

Show the limit upfront, not just on error. The upload form should display the maximum file size before the user selects a file — a small label like "Max. 100 MB" below the file input. Discovering the limit only after attempting to upload a large file is a frustrating experience, especially for seniors who may have spent time scanning a document.
The 413 error must appear inline in the upload form, not as a generic error page. The translated FILE_TOO_LARGE message should render as a visible, clearly styled error near the upload control — not just in a toast that disappears.
The error message needs plain language in all three locales. "Die Datei ist zu groß. Die maximale Dateigröße beträgt 100 MB." — concrete, actionable. Not "FILE_TOO_LARGE" rendered verbatim.

Suggestions

Add a client-side file size check in the <input type="file"> change handler. If the file is too large, show the error immediately without submitting — same message, zero round-trip. Better experience for everyone.
Ensure the error message has sufficient contrast against the form background (at least 4.5:1 per WCAG AA). Red error text on a white background: use #b91c1c or darker, not a light red.
Test the upload error state at 320px mobile width.

## 🎨 Leonie Voss — UI/UX Designer & Accessibility Strategist ### Questions & Observations - **Show the limit upfront, not just on error.** The upload form should display the maximum file size before the user selects a file — a small label like *"Max. 100 MB"* below the file input. Discovering the limit only after attempting to upload a large file is a frustrating experience, especially for seniors who may have spent time scanning a document. - **The 413 error must appear inline in the upload form**, not as a generic error page. The translated `FILE_TOO_LARGE` message should render as a visible, clearly styled error near the upload control — not just in a toast that disappears. - **The error message needs plain language in all three locales.** *"Die Datei ist zu groß. Die maximale Dateigröße beträgt 100 MB."* — concrete, actionable. Not *"FILE_TOO_LARGE"* rendered verbatim. ### Suggestions - Add a client-side file size check in the `<input type="file">` change handler. If the file is too large, show the error immediately without submitting — same message, zero round-trip. Better experience for everyone. - Ensure the error message has sufficient contrast against the form background (at least 4.5:1 per WCAG AA). Red error text on a white background: use `#b91c1c` or darker, not a light red. - Test the upload error state at 320px mobile width.

marcel commented

2026-03-31 21:13:22 +02:00

🚀 Tobias Wendt — DevOps & Platform Engineer

Questions & Observations

MinIO and the Spring limit must agree. Spring will reject files over 100 MB at the multipart filter. But if MinIO's configured max object size is lower (or if Hetzner Object Storage imposes a per-object limit), a file that passes the Spring check could still fail at the storage layer with a confusing error. Check the MinIO/Hetzner Object Storage limits and align them.
The limit should be configurable via environment variable in production, not just application-prod.yml. If the limit needs changing without a redeploy, a SPRING_SERVLET_MULTIPART_MAX_FILE_SIZE env var override in docker-compose.prod.yml is cleaner than a config file edit + redeploy.
Disk space consideration. 100 MB per upload × potentially many family members uploading simultaneously could fill the MinIO volume quickly. This isn't a reason to block the issue, but a note in the PR to revisit when monitoring (issue #140) is in place — a Grafana alert on MinIO disk usage would catch this before it becomes an outage.

Suggestions

Add the env var override pattern to docker-compose.prod.yml as part of this PR:

backend:
  environment:
    SPRING_SERVLET_MULTIPART_MAX_FILE_SIZE: ${MAX_FILE_SIZE:-100MB}

Document the chosen limit and the reasoning in a comment in application.yml so it's not a mystery number.

## 🚀 Tobias Wendt — DevOps & Platform Engineer ### Questions & Observations - **MinIO and the Spring limit must agree.** Spring will reject files over 100 MB at the multipart filter. But if MinIO's configured max object size is lower (or if Hetzner Object Storage imposes a per-object limit), a file that passes the Spring check could still fail at the storage layer with a confusing error. Check the MinIO/Hetzner Object Storage limits and align them. - **The limit should be configurable via environment variable in production**, not just `application-prod.yml`. If the limit needs changing without a redeploy, a `SPRING_SERVLET_MULTIPART_MAX_FILE_SIZE` env var override in `docker-compose.prod.yml` is cleaner than a config file edit + redeploy. - **Disk space consideration.** 100 MB per upload × potentially many family members uploading simultaneously could fill the MinIO volume quickly. This isn't a reason to block the issue, but a note in the PR to revisit when monitoring (issue #140) is in place — a Grafana alert on MinIO disk usage would catch this before it becomes an outage. ### Suggestions - Add the env var override pattern to `docker-compose.prod.yml` as part of this PR: ```yaml backend: environment: SPRING_SERVLET_MULTIPART_MAX_FILE_SIZE: ${MAX_FILE_SIZE:-100MB} ``` - Document the chosen limit and the reasoning in a comment in `application.yml` so it's not a mystery number.

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: marcel/familienarchiv#112