Compare commits

...

2 Commits

Author SHA1 Message Date
Marcel
1dd162f1be test(document): prove the DB rejects end-before-start; assert persisted end (#678)
All checks were successful
CI / Unit & Component Tests (pull_request) Successful in 3m20s
CI / OCR Service Tests (pull_request) Successful in 22s
CI / Backend Unit Tests (pull_request) Successful in 3m31s
CI / fail2ban Regex (pull_request) Successful in 45s
CI / Semgrep Security Scan (pull_request) Successful in 20s
CI / Compose Bucket Idempotency (pull_request) Successful in 1m4s
CI / Unit & Component Tests (push) Successful in 3m23s
CI / OCR Service Tests (push) Successful in 23s
CI / Backend Unit Tests (push) Successful in 3m37s
CI / fail2ban Regex (push) Successful in 43s
CI / Semgrep Security Scan (push) Successful in 22s
CI / Compose Bucket Idempotency (push) Successful in 1m5s
Addresses Sara's review concerns:
- Add a negative Testcontainers test: saveAndFlush of a RANGE with end < start
  throws DataIntegrityViolationException, proving chk_meta_date_end_after_start
  actually fires (H2 wouldn't) and exercising the backstop's trigger end-to-end.
  Guards against silent app/DB drift if the service guard ever regresses.
- Tighten updateDocument_acceptsRange_whenEndAfterStart to assert the persisted
  end value, not just that save was called.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
2026-06-01 11:03:28 +02:00
Marcel
ff7cfd4b1a fix(exception): log the violated constraint name at WARN (#678)
Addresses Tobias's review concern: the generic DataIntegrityViolation
backstop turned every integrity violation into a silent 400 with no
constraint name, no stack, no Sentry — an unanticipated write bug would
fail invisibly in production.

Now extract the constraint NAME from the cause chain (schema metadata, safe
for Loki) and log it parameterized at WARN, so the failure is debuggable.
Still never pass `ex`/`getMessage()` (SQL + values, CWE-209) and still no
Sentry — the response stays generic, so the response logic is not brittle.

New test proves the WARN names the constraint but never carries the SQL.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
2026-06-01 11:03:04 +02:00
4 changed files with 86 additions and 8 deletions

View File

@@ -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<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");
// 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<ErrorResponse> handleGeneric(Exception ex) {
Sentry.captureException(ex);

View File

@@ -38,7 +38,10 @@ import java.util.Optional;
import java.util.Set;
import java.util.UUID;
import org.springframework.dao.DataIntegrityViolationException;
import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatThrownBy;
@DataJpaTest
@AutoConfigureTestDatabase(replace = AutoConfigureTestDatabase.Replace.NONE)
@@ -635,6 +638,25 @@ class DocumentRepositoryTest {
assertThat(found.getMetaDatePrecision()).isEqualTo(DatePrecision.RANGE);
}
@Test
void save_rejectsRange_whenEndBeforeStart_atDbLevel() {
// The app guard normally intercepts this, so the DB CHECK never fires in practice.
// Persisting directly proves chk_meta_date_end_after_start actually rejects end < start
// (H2 would not) — if the app guard ever regresses, a bad row still can't reach the table,
// and this is exactly the violation the GlobalExceptionHandler backstop turns into a 400.
Document doc = Document.builder()
.title("Verdrehte Spanne")
.originalFilename("verdreht.pdf")
.status(DocumentStatus.UPLOADED)
.documentDate(LocalDate.of(1917, 1, 11))
.metaDatePrecision(DatePrecision.RANGE)
.metaDateEnd(LocalDate.of(1917, 1, 10))
.build();
assertThatThrownBy(() -> documentRepository.saveAndFlush(doc))
.isInstanceOf(DataIntegrityViolationException.class);
}
// ─── seeding helpers ─────────────────────────────────────────────────────
private Document uploaded(String title) {

View File

@@ -283,6 +283,7 @@ class DocumentServiceTest {
documentService.updateDocument(id,
rangeDto(LocalDate.of(1917, 1, 10), LocalDate.of(1917, 1, 11)), null, null);
assertThat(doc.getMetaDateEnd()).isEqualTo(LocalDate.of(1917, 1, 11));
verify(documentRepository, atLeastOnce()).save(any());
}

View File

@@ -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<ILoggingEvent> appender = new ListAppender<>();
appender.start();
handlerLogger.addAppender(appender);
try (MockedStatic<Sentry> sentryMock = mockStatic(Sentry.class)) {
ResponseEntity<GlobalExceptionHandler.ErrorResponse> 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"));
}
}