feat(exception): backstop DataIntegrityViolation as a clean 400 (#678)
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 <noreply@anthropic.com>
This commit is contained in:
@@ -6,6 +6,7 @@ import io.sentry.Sentry;
|
|||||||
import jakarta.validation.ConstraintViolationException;
|
import jakarta.validation.ConstraintViolationException;
|
||||||
import org.raddatz.familienarchiv.exception.DomainException;
|
import org.raddatz.familienarchiv.exception.DomainException;
|
||||||
import org.raddatz.familienarchiv.exception.ErrorCode;
|
import org.raddatz.familienarchiv.exception.ErrorCode;
|
||||||
|
import org.springframework.dao.DataIntegrityViolationException;
|
||||||
import org.springframework.http.ResponseEntity;
|
import org.springframework.http.ResponseEntity;
|
||||||
import org.springframework.http.converter.HttpMessageNotReadableException;
|
import org.springframework.http.converter.HttpMessageNotReadableException;
|
||||||
import org.springframework.web.bind.MethodArgumentNotValidException;
|
import org.springframework.web.bind.MethodArgumentNotValidException;
|
||||||
@@ -64,6 +65,22 @@ public class GlobalExceptionHandler {
|
|||||||
.body(new ErrorResponse(ErrorCode.VALIDATION_ERROR, ex.getReason()));
|
.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<ErrorResponse> 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)
|
@ExceptionHandler(Exception.class)
|
||||||
public ResponseEntity<ErrorResponse> handleGeneric(Exception ex) {
|
public ResponseEntity<ErrorResponse> handleGeneric(Exception ex) {
|
||||||
Sentry.captureException(ex);
|
Sentry.captureException(ex);
|
||||||
|
|||||||
@@ -1,11 +1,17 @@
|
|||||||
package org.raddatz.familienarchiv.exception;
|
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 io.sentry.Sentry;
|
||||||
import org.junit.jupiter.api.Test;
|
import org.junit.jupiter.api.Test;
|
||||||
import org.junit.jupiter.api.extension.ExtendWith;
|
import org.junit.jupiter.api.extension.ExtendWith;
|
||||||
import org.mockito.InjectMocks;
|
import org.mockito.InjectMocks;
|
||||||
import org.mockito.MockedStatic;
|
import org.mockito.MockedStatic;
|
||||||
import org.mockito.junit.jupiter.MockitoExtension;
|
import org.mockito.junit.jupiter.MockitoExtension;
|
||||||
|
import org.slf4j.LoggerFactory;
|
||||||
|
import org.springframework.dao.DataIntegrityViolationException;
|
||||||
import org.springframework.http.ResponseEntity;
|
import org.springframework.http.ResponseEntity;
|
||||||
|
|
||||||
import static org.assertj.core.api.Assertions.assertThat;
|
import static org.assertj.core.api.Assertions.assertThat;
|
||||||
@@ -30,4 +36,45 @@ class GlobalExceptionHandlerTest {
|
|||||||
assertThat(response.getBody().code()).isEqualTo(ErrorCode.INTERNAL_ERROR);
|
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<ILoggingEvent> appender = new ListAppender<>();
|
||||||
|
appender.start();
|
||||||
|
handlerLogger.addAppender(appender);
|
||||||
|
|
||||||
|
try (MockedStatic<Sentry> sentryMock = mockStatic(Sentry.class)) {
|
||||||
|
ResponseEntity<GlobalExceptionHandler.ErrorResponse> 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_");
|
||||||
|
});
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user