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 7a029955..686ef457 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/exception/GlobalExceptionHandler.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/exception/GlobalExceptionHandler.java @@ -68,19 +68,35 @@ public class GlobalExceptionHandler { /** * 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. + * a clean 400 instead of a 500 + Sentry alert. The known date-range cases are caught upstream + * and never reach here; this only catches the unanticipated ones — so it logs the constraint + * NAME at WARN to stay debuggable, without re-leaking SQL and without branching the response + * on it (the response stays generic, which is the non-brittle part). */ @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"); + // Log the constraint NAME only — schema metadata, safe for Loki, and enough to tell which + // constraint fired at 2am. Never pass `ex` / `ex.getMessage()`: those embed the SQL + the + // offending values (CWE-209). No Sentry: an integrity violation is a 400, not a system fault. + log.warn("Rejected a request that violated a database integrity constraint: {}", constraintNameOf(ex)); return ResponseEntity.badRequest() .body(new ErrorResponse(ErrorCode.VALIDATION_ERROR, "The submitted data violated a database constraint")); } + /** + * Returns the offending constraint's name from the cause chain, or {@code "unknown"}. + * Reads only the name (a non-sensitive schema identifier) — never the SQL or the values. + */ + private static String constraintNameOf(Throwable ex) { + for (Throwable t = ex; t != null && t != t.getCause(); t = t.getCause()) { + if (t instanceof org.hibernate.exception.ConstraintViolationException cve + && cve.getConstraintName() != null) { + return cve.getConstraintName(); + } + } + return "unknown"; + } + @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 3a51993f..c75e9fae 100644 --- a/backend/src/test/java/org/raddatz/familienarchiv/exception/GlobalExceptionHandlerTest.java +++ b/backend/src/test/java/org/raddatz/familienarchiv/exception/GlobalExceptionHandlerTest.java @@ -72,9 +72,48 @@ class GlobalExceptionHandlerTest { .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)") + .as("never logs the SQL statement / values (would re-leak to Loki)") .noneSatisfy(e -> { - assertThat(e.getFormattedMessage()).contains("chk_"); + assertThat(e.getFormattedMessage()).contains("could not execute statement"); }); } + + @Test + void handleDataIntegrityViolation_logsConstraintName_butNotTheSql() { + // Debuggability (DevOps): the WARN must name *which* constraint fired so an + // unanticipated violation isn't a silent mystery — but it must carry the name only, + // never the SQL statement or the offending values that the SQLException message holds. + java.sql.SQLException sql = new java.sql.SQLException( + "ERROR: violates check constraint; could not execute statement; values (1917-01-10)"); + org.hibernate.exception.ConstraintViolationException cve = + new org.hibernate.exception.ConstraintViolationException( + "constraint violation", sql, "chk_meta_date_end_after_start"); + DataIntegrityViolationException ex = new DataIntegrityViolationException("wrapper", cve); + + 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); + + // Response stays generic and leak-free (CWE-209) regardless of what we log. + assertThat(response.getStatusCode().value()).isEqualTo(400); + assertThat(response.getBody().message()) + .doesNotContain("chk_") + .doesNotContain("meta_date"); + sentryMock.verifyNoInteractions(); + } finally { + handlerLogger.detachAppender(appender); + } + + assertThat(appender.list) + .as("WARN names the constraint for debuggability") + .anySatisfy(e -> assertThat(e.getFormattedMessage()).contains("chk_meta_date_end_after_start")); + assertThat(appender.list) + .as("but never the SQL statement or values") + .noneSatisfy(e -> assertThat(e.getFormattedMessage()).contains("could not execute statement")); + } }