From 3a4c2c6225e9b21f03b6185cce96ed99a96c891f Mon Sep 17 00:00:00 2001 From: Marcel Date: Mon, 1 Jun 2026 09:20:22 +0200 Subject: [PATCH] feat(exception): backstop DataIntegrityViolation as a clean 400 (#678) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add @ExceptionHandler(DataIntegrityViolationException) returning 400 VALIDATION_ERROR with a fixed constant message, so any integrity violation that slips past the upstream guards (a future constraint, or the import path) becomes a clean 400 instead of a 500 + Sentry alert (AC9). Deliberately generic — it does not inspect which constraint failed. Never echoes ex.getMessage() (constraint name + SQL, CWE-209), logs at WARN without passing the exception (would re-leak the SQL to Loki), and does not call Sentry.captureException. Co-Authored-By: Claude Opus 4.8 --- .../exception/GlobalExceptionHandler.java | 17 +++++++ .../exception/GlobalExceptionHandlerTest.java | 47 +++++++++++++++++++ 2 files changed, 64 insertions(+) diff --git a/backend/src/main/java/org/raddatz/familienarchiv/exception/GlobalExceptionHandler.java b/backend/src/main/java/org/raddatz/familienarchiv/exception/GlobalExceptionHandler.java index 87838d5c..7a029955 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/exception/GlobalExceptionHandler.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/exception/GlobalExceptionHandler.java @@ -6,6 +6,7 @@ import io.sentry.Sentry; import jakarta.validation.ConstraintViolationException; import org.raddatz.familienarchiv.exception.DomainException; import org.raddatz.familienarchiv.exception.ErrorCode; +import org.springframework.dao.DataIntegrityViolationException; import org.springframework.http.ResponseEntity; import org.springframework.http.converter.HttpMessageNotReadableException; import org.springframework.web.bind.MethodArgumentNotValidException; @@ -64,6 +65,22 @@ public class GlobalExceptionHandler { .body(new ErrorResponse(ErrorCode.VALIDATION_ERROR, ex.getReason())); } + /** + * Backstop for any database integrity violation that slips past the explicit upstream + * guards (e.g. a future constraint, or the import path emitting a bad range). Turns it into + * a clean 400 instead of a 500 + Sentry alert. Deliberately generic: it does NOT inspect the + * exception to identify the constraint — that inspection is the brittle thing we avoid, and the + * known date-range cases are already caught upstream and never reach here. + */ + @ExceptionHandler(DataIntegrityViolationException.class) + public ResponseEntity handleDataIntegrityViolation(DataIntegrityViolationException ex) { + // Fixed message only: ex.getMessage() embeds the constraint name + SQL (CWE-209), and + // passing ex to the logger would dump the same into Loki — so do neither, and no Sentry. + log.warn("Rejected a request that violated a database integrity constraint"); + return ResponseEntity.badRequest() + .body(new ErrorResponse(ErrorCode.VALIDATION_ERROR, "The submitted data violated a database constraint")); + } + @ExceptionHandler(Exception.class) public ResponseEntity handleGeneric(Exception ex) { Sentry.captureException(ex); diff --git a/backend/src/test/java/org/raddatz/familienarchiv/exception/GlobalExceptionHandlerTest.java b/backend/src/test/java/org/raddatz/familienarchiv/exception/GlobalExceptionHandlerTest.java index a12933b8..3a51993f 100644 --- a/backend/src/test/java/org/raddatz/familienarchiv/exception/GlobalExceptionHandlerTest.java +++ b/backend/src/test/java/org/raddatz/familienarchiv/exception/GlobalExceptionHandlerTest.java @@ -1,11 +1,17 @@ package org.raddatz.familienarchiv.exception; +import ch.qos.logback.classic.Level; +import ch.qos.logback.classic.Logger; +import ch.qos.logback.classic.spi.ILoggingEvent; +import ch.qos.logback.core.read.ListAppender; import io.sentry.Sentry; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; import org.mockito.InjectMocks; import org.mockito.MockedStatic; import org.mockito.junit.jupiter.MockitoExtension; +import org.slf4j.LoggerFactory; +import org.springframework.dao.DataIntegrityViolationException; import org.springframework.http.ResponseEntity; import static org.assertj.core.api.Assertions.assertThat; @@ -30,4 +36,45 @@ class GlobalExceptionHandlerTest { assertThat(response.getBody().code()).isEqualTo(ErrorCode.INTERNAL_ERROR); } } + + @Test + void handleDataIntegrityViolation_returns400_withoutLeakingConstraint_orSentry() { + // A DataIntegrityViolationException carries the constraint name + SQL in its message; + // the response and logs must never echo it (CWE-209). It must become a clean 400, not a 500. + DataIntegrityViolationException ex = new DataIntegrityViolationException( + "could not execute statement; constraint [chk_meta_date_end_after_start]; " + + "column meta_date_end of relation documents"); + + Logger handlerLogger = (Logger) LoggerFactory.getLogger(GlobalExceptionHandler.class); + ListAppender appender = new ListAppender<>(); + appender.start(); + handlerLogger.addAppender(appender); + + try (MockedStatic sentryMock = mockStatic(Sentry.class)) { + ResponseEntity response = + handler.handleDataIntegrityViolation(ex); + + assertThat(response.getStatusCode().value()).isEqualTo(400); + assertThat(response.getBody()).isNotNull(); + assertThat(response.getBody().code()).isEqualTo(ErrorCode.VALIDATION_ERROR); + assertThat(response.getBody().message()) + .doesNotContain("chk_") + .doesNotContain("meta_date"); + + // Defense-in-depth: an unanticipated integrity violation is not a system fault, + // so it must NOT fabricate a Sentry alert. + sentryMock.verifyNoInteractions(); + } finally { + handlerLogger.detachAppender(appender); + } + + assertThat(appender.list) + .as("logs a WARN line") + .anySatisfy(e -> assertThat(e.getLevel()).isEqualTo(Level.WARN)); + assertThat(appender.list) + .as("never logs the constraint name / SQL (would re-leak to Loki)") + .noneSatisfy(e -> { + assertThat(e.getFormattedMessage()).contains("chk_"); + }); + } }