marcel/mealprep
1
0
Fork 0
Code Issues 22 Pull Requests Actions Packages Projects Releases Wiki Activity

Missing catch-all exception handler — stack traces leak to clients #7

New Issue
Open
opened 2026-04-02 11:20:41 +02:00 by marcel · 5 comments
marcel commented 2026-04-02 11:20:41 +02:00
Owner
Copy Link

Problem

GlobalExceptionHandler only handles 4 specific exception types. Any unexpected exception (NPE, database errors, Hibernate exceptions, etc.) falls through to Spring Boot's default error handler, which returns stack traces with internal class names, file paths, and SQL details.

Affected files

  • GlobalExceptionHandler.java — missing @ExceptionHandler(Exception.class)

Attack scenario

An attacker triggers an unexpected error (e.g., malformed UUID in a path variable, database timeout) and receives a stack trace revealing internal package structure, library versions, and potentially SQL queries.

Recommended fix

Add a catch-all handler:

@ExceptionHandler(Exception.class)
public ResponseEntity<ApiError> handleUnexpected(Exception ex) {
    log.error("Unexpected error", ex);
    return ResponseEntity.status(HttpStatus.INTERNAL_SERVER_ERROR)
            .body(ApiError.of("INTERNAL_ERROR", "An unexpected error occurred"));
}

Severity

High — information disclosure via stack traces aids further exploitation.

## Problem `GlobalExceptionHandler` only handles 4 specific exception types. Any unexpected exception (NPE, database errors, Hibernate exceptions, etc.) falls through to Spring Boot's default error handler, which returns stack traces with internal class names, file paths, and SQL details. ## Affected files - `GlobalExceptionHandler.java` — missing `@ExceptionHandler(Exception.class)` ## Attack scenario An attacker triggers an unexpected error (e.g., malformed UUID in a path variable, database timeout) and receives a stack trace revealing internal package structure, library versions, and potentially SQL queries. ## Recommended fix Add a catch-all handler: ```java @ExceptionHandler(Exception.class) public ResponseEntity<ApiError> handleUnexpected(Exception ex) { log.error("Unexpected error", ex); return ResponseEntity.status(HttpStatus.INTERNAL_SERVER_ERROR) .body(ApiError.of("INTERNAL_ERROR", "An unexpected error occurred")); } ``` ## Severity High — information disclosure via stack traces aids further exploitation.
marcel added the kind/securitypriority/high labels 2026-04-02 11:21:04 +02:00
marcel commented 2026-04-03 09:27:58 +02:00
Author
Owner
Copy Link

👨‍💻 Kai — Frontend Engineer

Backend fix, but it directly affects what the SvelteKit frontend receives and has to handle. A few things I want to pin down:

Current frontend behavior on unexpected errors:

  • Right now, if the backend throws an uncaught exception, the frontend likely receives a Spring Boot default error body (JSON or HTML with a stack trace). Our +page.server.ts fetch calls need to handle 500 responses gracefully — returning a user-friendly fail() or error(), not crashing the SSR render.
  • After the fix, all 500s will return a clean { code: "INTERNAL_ERROR", message: "An unexpected error occurred" } body — which is much easier to handle uniformly in SvelteKit.

SvelteKit error boundary:

  • We should have a +error.svelte page that catches unhandled SvelteKit errors (including 500s from the backend) and renders something meaningful in German. Does one exist? If not, this issue is a good prompt to add it.
  • Custom handleFetch or handleError in hooks.server.ts — are we currently logging or transforming fetch errors from the backend? We should at minimum log the backend error on the server side without surfacing it to the client.

Questions:

  • What does the frontend currently render when a backend call returns a 500 with a stack trace body? Does it crash SSR, render a blank page, or show the raw error JSON?
  • Is there a +error.svelte in the project already, and does it handle the case of a server-side rendering failure gracefully?
## 👨‍💻 Kai — Frontend Engineer Backend fix, but it directly affects what the SvelteKit frontend receives and has to handle. A few things I want to pin down: **Current frontend behavior on unexpected errors:** - Right now, if the backend throws an uncaught exception, the frontend likely receives a Spring Boot default error body (JSON or HTML with a stack trace). Our `+page.server.ts` fetch calls need to handle 500 responses gracefully — returning a user-friendly `fail()` or `error()`, not crashing the SSR render. - After the fix, all 500s will return a clean `{ code: "INTERNAL_ERROR", message: "An unexpected error occurred" }` body — which is much easier to handle uniformly in SvelteKit. **SvelteKit error boundary:** - We should have a `+error.svelte` page that catches unhandled SvelteKit errors (including 500s from the backend) and renders something meaningful in German. Does one exist? If not, this issue is a good prompt to add it. - Custom `handleFetch` or `handleError` in `hooks.server.ts` — are we currently logging or transforming fetch errors from the backend? We should at minimum log the backend error on the server side without surfacing it to the client. Questions: - What does the frontend currently render when a backend call returns a 500 with a stack trace body? Does it crash SSR, render a blank page, or show the raw error JSON? - Is there a `+error.svelte` in the project already, and does it handle the case of a server-side rendering failure gracefully?
marcel commented 2026-04-03 09:28:18 +02:00
Author
Owner
Copy Link

🔧 Backend Engineer — Spring Boot / PostgreSQL Specialist

The fix is correct and the recommended code snippet in the issue is close to what I'd write. A few details to get right:

The catch-all handler:

  • The proposed @ExceptionHandler(Exception.class) will catch everything not handled by more specific handlers, including RuntimeException. This is what we want.
  • Make sure the handler is ordered correctly — Spring processes more specific exception handlers first, so adding Exception.class as a catch-all won't override the existing 4 handlers. No ordering annotation needed.
  • Log with log.error("Unexpected error", ex) — include the exception so the stack trace appears in server logs (where we want it) but not in the response body.

ApiError.of() consistency:

  • Confirm ApiError.of("INTERNAL_ERROR", "An unexpected error occurred") matches the same ApiError structure used by the existing 4 handlers. Inconsistent error response shapes are a maintenance problem and break frontend error handling.
  • The response must not include the exception message, cause, or any stack trace fields — just the code and user-facing message.

Spring Boot's default error handling:

  • By default, Spring Boot's BasicErrorController also serves error responses at /error. With a @ControllerAdvice catch-all, most exceptions will be intercepted before reaching BasicErrorController — but check whether server.error.include-stacktrace=never is set in application.yml as a defense-in-depth measure.
  • Consider also setting server.error.include-message=never and server.error.include-binding-errors=never in production profile.

MethodArgumentTypeMismatchException:

  • The issue mentions "malformed UUID in a path variable" as an attack vector. This would typically throw MethodArgumentTypeMismatchException or ConstraintViolationException — both should now be caught by the catch-all, but worth verifying they don't have existing specific handlers that map them somewhere unexpected.

Questions:

  • What are the existing 4 exception types currently handled? Knowing them helps confirm the catch-all won't mask any deliberate specific handlers.
  • Is server.error.include-stacktrace currently configured in application.yml? If it's set to always or on_param, that's also leaking and needs to be fixed regardless of the catch-all.
## 🔧 Backend Engineer — Spring Boot / PostgreSQL Specialist The fix is correct and the recommended code snippet in the issue is close to what I'd write. A few details to get right: **The catch-all handler:** - The proposed `@ExceptionHandler(Exception.class)` will catch everything not handled by more specific handlers, including `RuntimeException`. This is what we want. - Make sure the handler is ordered correctly — Spring processes more specific exception handlers first, so adding `Exception.class` as a catch-all won't override the existing 4 handlers. No ordering annotation needed. - Log with `log.error("Unexpected error", ex)` — include the exception so the stack trace appears in server logs (where we *want* it) but not in the response body. **`ApiError.of()` consistency:** - Confirm `ApiError.of("INTERNAL_ERROR", "An unexpected error occurred")` matches the same `ApiError` structure used by the existing 4 handlers. Inconsistent error response shapes are a maintenance problem and break frontend error handling. - The response must not include the exception message, cause, or any stack trace fields — just the code and user-facing message. **Spring Boot's default error handling:** - By default, Spring Boot's `BasicErrorController` also serves error responses at `/error`. With a `@ControllerAdvice` catch-all, most exceptions will be intercepted before reaching `BasicErrorController` — but check whether `server.error.include-stacktrace=never` is set in `application.yml` as a defense-in-depth measure. - Consider also setting `server.error.include-message=never` and `server.error.include-binding-errors=never` in production profile. **`MethodArgumentTypeMismatchException`:** - The issue mentions "malformed UUID in a path variable" as an attack vector. This would typically throw `MethodArgumentTypeMismatchException` or `ConstraintViolationException` — both should now be caught by the catch-all, but worth verifying they don't have existing specific handlers that map them somewhere unexpected. Questions: - What are the existing 4 exception types currently handled? Knowing them helps confirm the catch-all won't mask any deliberate specific handlers. - Is `server.error.include-stacktrace` currently configured in `application.yml`? If it's set to `always` or `on_param`, that's also leaking and needs to be fixed regardless of the catch-all.
marcel commented 2026-04-03 09:28:33 +02:00
Author
Owner
Copy Link

🧪 QA Engineer

This is a small code change with broad impact — every unhandled exception in the application now flows through this handler. Test coverage needs to verify both that it works and that it doesn't accidentally swallow exceptions that should be handled more specifically.

Unit tests (GlobalExceptionHandler):

  • shouldReturn500WithGenericBodyForNullPointerException() — most common unexpected runtime error
  • shouldReturn500WithGenericBodyForRuntimeException() — generic case
  • shouldReturn500WithGenericBodyForDatabaseException() — simulates a JPA/Hibernate error reaching the handler
  • shouldNotExposeExceptionMessageInResponseBody() — response body contains only the generic code + message, no exception detail
  • shouldLogExceptionToServerLog() — verify log.error() is called with the exception (use a log appender spy or verify via Mockito captor)

Integration tests:

  • GET request with malformed UUID in path variable → 500 with { code: "INTERNAL_ERROR", message: "An unexpected error occurred" } (not a stack trace)
  • Trigger a database timeout scenario → same 500 response shape
  • Verify the response has Content-Type: application/json, not text/html (Spring's default error page is HTML)

Regression — existing handlers not affected:

  • Requests that previously triggered the 4 existing handlers still return their specific status codes and bodies — the catch-all must not intercept them

Spring Boot default error endpoint:

  • Verify that GET /error (Spring's fallback) also returns a clean response, not a stack trace — this is separate from the @ControllerAdvice handler and may need independent configuration

Edge cases:

  • OutOfMemoryError — this is a Throwable, not an Exception. The catch-all won't catch it. Is that acceptable, or should we add Throwable.class handling?
  • What happens if the catch-all handler itself throws? (e.g., the logger is broken) — worth understanding the failure mode

Questions:

  • Is there an existing test that intentionally triggers an unhandled exception to verify the current (broken) behavior? If so, it should be updated to assert the new clean response.
  • How do we simulate a "database timeout" in integration tests — is there a Testcontainers setup that can introduce artificial failures?
## 🧪 QA Engineer This is a small code change with broad impact — every unhandled exception in the application now flows through this handler. Test coverage needs to verify both that it works and that it doesn't accidentally swallow exceptions that should be handled more specifically. **Unit tests (GlobalExceptionHandler):** - `shouldReturn500WithGenericBodyForNullPointerException()` — most common unexpected runtime error - `shouldReturn500WithGenericBodyForRuntimeException()` — generic case - `shouldReturn500WithGenericBodyForDatabaseException()` — simulates a JPA/Hibernate error reaching the handler - `shouldNotExposeExceptionMessageInResponseBody()` — response body contains only the generic code + message, no exception detail - `shouldLogExceptionToServerLog()` — verify `log.error()` is called with the exception (use a log appender spy or verify via Mockito captor) **Integration tests:** - GET request with malformed UUID in path variable → 500 with `{ code: "INTERNAL_ERROR", message: "An unexpected error occurred" }` (not a stack trace) - Trigger a database timeout scenario → same 500 response shape - Verify the response has `Content-Type: application/json`, not `text/html` (Spring's default error page is HTML) **Regression — existing handlers not affected:** - Requests that previously triggered the 4 existing handlers still return their specific status codes and bodies — the catch-all must not intercept them **Spring Boot default error endpoint:** - Verify that `GET /error` (Spring's fallback) also returns a clean response, not a stack trace — this is separate from the `@ControllerAdvice` handler and may need independent configuration **Edge cases:** - `OutOfMemoryError` — this is a `Throwable`, not an `Exception`. The catch-all won't catch it. Is that acceptable, or should we add `Throwable.class` handling? - What happens if the catch-all handler itself throws? (e.g., the logger is broken) — worth understanding the failure mode Questions: - Is there an existing test that intentionally triggers an unhandled exception to verify the current (broken) behavior? If so, it should be updated to assert the new clean response. - How do we simulate a "database timeout" in integration tests — is there a Testcontainers setup that can introduce artificial failures?
marcel commented 2026-04-03 09:28:50 +02:00
Author
Owner
Copy Link

🔐 Sable — Security Engineer

High priority and easy to fix — this is a textbook OWASP A05 (Security Misconfiguration) / information disclosure issue. The attack scenario is real: stack traces reliably reveal package names, library versions, internal file paths, and sometimes SQL fragments — all useful for building a targeted exploit chain.

The fix is necessary but layer 1 of defense-in-depth:

  • The @ExceptionHandler(Exception.class) catch-all handles most cases, but Spring Boot also has a fallback error controller at /error for errors that occur outside the servlet filter chain (e.g., in a Filter itself). Confirm server.error.include-stacktrace=never is set in application.yml for all profiles — don't rely solely on the @ControllerAdvice.
  • Additionally set: server.error.include-message=never, server.error.include-exception=false. These suppress the exception class name and message from Spring Boot's own error responses.

Information in the log:

  • log.error("Unexpected error", ex) is correct — the full stack trace belongs in the server log, not the response. Confirm the logging configuration doesn't write to a file or endpoint accessible from outside (e.g., an /actuator/logfile endpoint that is publicly accessible).

Actuator endpoints:

  • While reviewing application.yml for the error config, also check that /actuator/health is the only actuator endpoint exposed, or that actuator is properly gated. Exposed actuator endpoints (especially /actuator/env, /actuator/beans, /actuator/mappings) leak far more than a stack trace.

Exception message leakage in existing handlers:

  • The 4 existing handlers may be passing ex.getMessage() into the ApiError body. If ex.getMessage() contains internal details (table names, column names, file paths), those are also information leaks — worth auditing alongside this fix.

Questions:

  • Is spring.profiles.active set correctly in the production deployment so the production application-prod.yml (with include-stacktrace=never) is actually loaded?
  • Are actuator endpoints currently gated behind authentication or restricted to localhost only?
  • Do any of the existing 4 @ExceptionHandler methods pass raw ex.getMessage() into the response body?
## 🔐 Sable — Security Engineer High priority and easy to fix — this is a textbook OWASP A05 (Security Misconfiguration) / information disclosure issue. The attack scenario is real: stack traces reliably reveal package names, library versions, internal file paths, and sometimes SQL fragments — all useful for building a targeted exploit chain. **The fix is necessary but layer 1 of defense-in-depth:** - The `@ExceptionHandler(Exception.class)` catch-all handles most cases, but Spring Boot also has a fallback error controller at `/error` for errors that occur outside the servlet filter chain (e.g., in a `Filter` itself). Confirm `server.error.include-stacktrace=never` is set in `application.yml` for all profiles — don't rely solely on the `@ControllerAdvice`. - Additionally set: `server.error.include-message=never`, `server.error.include-exception=false`. These suppress the exception class name and message from Spring Boot's own error responses. **Information in the log:** - `log.error("Unexpected error", ex)` is correct — the full stack trace belongs in the server log, not the response. Confirm the logging configuration doesn't write to a file or endpoint accessible from outside (e.g., an `/actuator/logfile` endpoint that is publicly accessible). **Actuator endpoints:** - While reviewing `application.yml` for the error config, also check that `/actuator/health` is the only actuator endpoint exposed, or that actuator is properly gated. Exposed actuator endpoints (especially `/actuator/env`, `/actuator/beans`, `/actuator/mappings`) leak far more than a stack trace. **Exception message leakage in existing handlers:** - The 4 existing handlers may be passing `ex.getMessage()` into the `ApiError` body. If `ex.getMessage()` contains internal details (table names, column names, file paths), those are also information leaks — worth auditing alongside this fix. Questions: - Is `spring.profiles.active` set correctly in the production deployment so the production `application-prod.yml` (with `include-stacktrace=never`) is actually loaded? - Are actuator endpoints currently gated behind authentication or restricted to localhost only? - Do any of the existing 4 `@ExceptionHandler` methods pass raw `ex.getMessage()` into the response body?
marcel commented 2026-04-03 09:29:06 +02:00
Author
Owner
Copy Link

🎨 Atlas — UI/UX Designer

Backend-only fix, but it has a direct UX consequence: what does the user see when an unexpected error occurs? That's a design concern.

Error state UX for unexpected failures:

  • A generic 500 is one of the most disorienting experiences a user can have — "something went wrong" without any actionable guidance. The clean JSON response from the fix is great for the frontend to parse, but we need to decide what to render.
  • The +error.svelte page (if it exists) should show: a friendly German message, a clear explanation that it's not the user's fault, and a single action — typically "Zur Startseite" or "Neu laden" depending on context.
  • This screen should use our standard design tokens: --color-page background, Fraunces for the heading (human, warm), DM Sans for the body copy, the standard button style (13px, weight 500, tracking 0.04em).

Avoid the "white screen of death":

  • If the SSR render itself fails, SvelteKit may fall back to an empty page or a raw error. The +error.svelte boundary needs to handle this gracefully and always render something useful.

No new visual components needed today — but this issue is a good prompt to confirm the error page exists and matches the design system. If it's a plain unstyled page right now, that's a design debt item.

Questions:

  • Is there a designed spec for the error/500 page, or has it been left as a default? If not, I should create one — it's a small but important part of the user experience.
  • Should the error page offer a "reload" button, a "go home" button, or both? The answer depends on whether the error is likely transient (network/server blip) or persistent (broken route).
## 🎨 Atlas — UI/UX Designer Backend-only fix, but it has a direct UX consequence: what does the user see when an unexpected error occurs? That's a design concern. **Error state UX for unexpected failures:** - A generic 500 is one of the most disorienting experiences a user can have — "something went wrong" without any actionable guidance. The clean JSON response from the fix is great for the frontend to parse, but we need to decide what to *render*. - The `+error.svelte` page (if it exists) should show: a friendly German message, a clear explanation that it's not the user's fault, and a single action — typically "Zur Startseite" or "Neu laden" depending on context. - This screen should use our standard design tokens: `--color-page` background, `Fraunces` for the heading (human, warm), `DM Sans` for the body copy, the standard button style (13px, weight 500, tracking 0.04em). **Avoid the "white screen of death":** - If the SSR render itself fails, SvelteKit may fall back to an empty page or a raw error. The `+error.svelte` boundary needs to handle this gracefully and always render *something* useful. **No new visual components needed today** — but this issue is a good prompt to confirm the error page exists and matches the design system. If it's a plain unstyled page right now, that's a design debt item. Questions: - Is there a designed spec for the error/500 page, or has it been left as a default? If not, I should create one — it's a small but important part of the user experience. - Should the error page offer a "reload" button, a "go home" button, or both? The answer depends on whether the error is likely transient (network/server blip) or persistent (broken route).
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
kind/bug
Something isn't working
 kind/chore
Maintenance, dependency updates, tooling
 kind/devops
CI/CD, infra, deployment
 kind/docs
Documentation additions or updates
 kind/feature
New functionality
 kind/refactor
Code restructuring without behaviour change
 kind/security
Security vulnerability or hardening
 kind/test
Adding or fixing tests
 kind/ui
User interface or UX change
 priority/critical
Drop everything
 priority/high
Next up
 priority/low
Nice to have
 priority/medium
Important but not urgent
 status/blocked
Blocked by another issue or external dependency
 status/in-progress
Actively being worked on
 status/needs-review
Waiting for review or feedback
 status/wontfix
Intentionally not being addressed
No Label kind/security priority/high
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
marcel
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: marcel/mealprep#7
Delete Branch "%!s()"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?