From 34146d7309b53dde062b617cc2fc922401e5f472 Mon Sep 17 00:00:00 2001 From: Marcel Date: Sat, 13 Jun 2026 10:57:28 +0200 Subject: [PATCH] feat(exception): add optimistic-lock backstop returning generic 409 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Centralized @ExceptionHandler(ObjectOptimisticLockingFailureException) net so any write path losing a @Version race becomes a generic 409 (CONFLICT code) — never a 500 + Sentry + Hibernate internals (CWE-209). No Sentry, class-name- only parameterized logging, body free of id/version/class. Entity-agnostic by design (no switch on getPersistentClassName); the service catch keeps the precise TIMELINE_EVENT_CONFLICT. Per #775 Q2/R4/R8. Co-Authored-By: Claude Fable 5 --- .../exception/GlobalExceptionHandler.java | 24 ++++++++++ .../exception/GlobalExceptionHandlerTest.java | 46 +++++++++++++++++++ 2 files changed, 70 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 c56cc576..b6484c86 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/exception/GlobalExceptionHandler.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/exception/GlobalExceptionHandler.java @@ -104,6 +104,30 @@ public class GlobalExceptionHandler { return "unknown"; } + /** + * Generic backstop for optimistic-locking conflicts that escape a service-level catch. A + * conflict is a 409, not a system fault — so, like {@link #handleDataIntegrityViolation}, it + * must NOT fire Sentry and must NOT leak Hibernate internals (CWE-209): the response carries + * only the generic {@link ErrorCode#CONFLICT} code and a generic message — no entity id, no + * version, no persistent-class name. + * + *

Deliberately code-GENERIC: do NOT {@code switch} on {@code getPersistentClassName()} to map + * back to a per-entity code. Unlike {@link #handleDataIntegrityViolation}, which branches on + * stable schema constraint NAMES, persistent-class names are not a contract. The precise, + * code-carrying path is the service catch (e.g. {@code TIMELINE_EVENT_CONFLICT}); this is only + * the net that keeps any current or future write path from regressing to a 500. + */ + @ExceptionHandler(org.springframework.orm.ObjectOptimisticLockingFailureException.class) + public ResponseEntity handleOptimisticLock( + org.springframework.orm.ObjectOptimisticLockingFailureException ex) { + // Log the persistent-class name ONLY (schema metadata, safe for Loki). Never `ex` / + // ex.getMessage(): those embed the entity id + version (CWE-209). No Sentry: it's a 409. + log.warn("Rejected a write that lost an optimistic-lock race on: {}", ex.getPersistentClassName()); + return ResponseEntity.status(409) + .body(new ErrorResponse(ErrorCode.CONFLICT, + "The resource was modified concurrently. Please reload and try again.")); + } + @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 324c17a1..ef3028ba 100644 --- a/backend/src/test/java/org/raddatz/familienarchiv/exception/GlobalExceptionHandlerTest.java +++ b/backend/src/test/java/org/raddatz/familienarchiv/exception/GlobalExceptionHandlerTest.java @@ -14,6 +14,9 @@ import org.slf4j.LoggerFactory; import org.springframework.dao.DataIntegrityViolationException; import org.springframework.dao.IncorrectResultSizeDataAccessException; import org.springframework.http.ResponseEntity; +import org.springframework.orm.ObjectOptimisticLockingFailureException; + +import java.util.UUID; import static org.assertj.core.api.Assertions.assertThat; import static org.mockito.Mockito.mockStatic; @@ -103,6 +106,49 @@ class GlobalExceptionHandlerTest { }); } + @Test + void handleOptimisticLock_returns409_genericConflict_noSentry_noLeak() { + // CWE-209 regression: an optimistic-lock failure escaping a service catch must become a + // generic 409, never a 500 + Sentry + Hibernate internals. The generic CONFLICT code keeps + // it entity-agnostic — NOT TIMELINE_EVENT_CONFLICT, or every future entity's conflict is + // mislabeled a timeline one. + UUID entityId = UUID.randomUUID(); + ObjectOptimisticLockingFailureException ex = + new ObjectOptimisticLockingFailureException("com.example.SomeEntity", entityId); + + 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.handleOptimisticLock(ex); + + assertThat(response.getStatusCode().value()).isEqualTo(409); + assertThat(response.getBody()).isNotNull(); + assertThat(response.getBody().code()).isEqualTo(ErrorCode.CONFLICT); + // Body echoes no persistent-class name, no entity id, no version (enumeration aids). + assertThat(response.getBody().message()) + .doesNotContain("SomeEntity") + .doesNotContain(entityId.toString()); + + // A conflict is not a system fault — no fabricated Sentry alert. + sentryMock.verifyNoInteractions(); + } finally { + handlerLogger.detachAppender(appender); + } + + assertThat(appender.list) + .as("WARN names the persistent class for debuggability") + .anySatisfy(e -> { + assertThat(e.getLevel()).isEqualTo(Level.WARN); + assertThat(e.getFormattedMessage()).contains("com.example.SomeEntity"); + }); + assertThat(appender.list) + .as("but never the entity id (would leak via getMessage())") + .noneSatisfy(e -> assertThat(e.getFormattedMessage()).contains(entityId.toString())); + } + @Test void handleDataIntegrityViolation_logsConstraintName_butNotTheSql() { // Debuggability (DevOps): the WARN must name *which* constraint fired so an