From 72860f20db1f1b83eb962441f69d00925f78a0a8 Mon Sep 17 00:00:00 2001 From: Sammi Chong <133241942+SammiChong@users.noreply.github.com> Date: Wed, 6 May 2026 17:49:59 +0100 Subject: [PATCH 1/6] Set server.error.include-message to never --- src/contractTest/resources/application.properties | 2 +- src/main/resources/application.properties | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/contractTest/resources/application.properties b/src/contractTest/resources/application.properties index 064ab7e8e2..f16b8169ab 100644 --- a/src/contractTest/resources/application.properties +++ b/src/contractTest/resources/application.properties @@ -3,7 +3,7 @@ server.port=4452 server.servlet.contextPath= server.compression.enabled=true server.compression.mime-types=application/json,application/xml,text/html,text/xml,text/plain -server.error.include-message=always +server.error.include-message=never server.max-http-header-size=100KB spring.config.import=optional:configtree:/mnt/secrets/ccd/ diff --git a/src/main/resources/application.properties b/src/main/resources/application.properties index e6a04cd0d6..b6420f0a84 100644 --- a/src/main/resources/application.properties +++ b/src/main/resources/application.properties @@ -3,7 +3,7 @@ server.port=4452 server.servlet.contextPath= server.compression.enabled=true server.compression.mime-types=application/json,application/xml,text/html,text/xml,text/plain -server.error.include-message=always +server.error.include-message=never server.max-http-request-header-size=100KB spring.config.import=optional:configtree:/mnt/secrets/ccd/ From 25904491ab03eef58cf2f8095c9f3fa4493e97b2 Mon Sep 17 00:00:00 2001 From: Sammi Chong <133241942+SammiChong@users.noreply.github.com> Date: Wed, 6 May 2026 17:50:50 +0100 Subject: [PATCH 2/6] Remove detailed error data --- .../ccd/domain/model/common/HttpError.java | 6 ++++-- .../exceptions/RestExceptionHandler.java | 4 ++-- ...taStoreHttpStatusRequestRejectedHandler.java | 2 +- .../java/uk/gov/hmcts/ccd/ElasticsearchIT.java | 3 +-- .../ccd/domain/model/common/HttpErrorTest.java | 17 +++++++++++++++-- .../exceptions/RestExceptionHandlerTest.java | 15 ++++++++++++--- .../hmcts/ccd/endpoint/std/CallbackTest.java | 3 +-- ...oreHttpStatusRequestRejectedHandlerTest.java | 5 +++-- 8 files changed, 39 insertions(+), 16 deletions(-) diff --git a/src/main/java/uk/gov/hmcts/ccd/domain/model/common/HttpError.java b/src/main/java/uk/gov/hmcts/ccd/domain/model/common/HttpError.java index dd9b63f0a3..9c1befd690 100644 --- a/src/main/java/uk/gov/hmcts/ccd/domain/model/common/HttpError.java +++ b/src/main/java/uk/gov/hmcts/ccd/domain/model/common/HttpError.java @@ -1,5 +1,6 @@ package uk.gov.hmcts.ccd.domain.model.common; +import com.fasterxml.jackson.annotation.JsonIgnore; import org.springframework.http.HttpStatus; import org.springframework.web.bind.annotation.ResponseStatus; import org.springframework.web.context.request.ServletWebRequest; @@ -34,7 +35,7 @@ public HttpError(Exception exception, HttpServletRequest request) { this.timestamp = LocalDateTime.now(ZoneOffset.UTC); this.status = getStatusFromResponseStatus(responseStatus); this.error = getErrorReason(responseStatus); - this.message = exception.getMessage(); + this.message = error; this.path = UriUtils.encodePath(request.getRequestURI(), StandardCharsets.UTF_8); } @@ -45,7 +46,7 @@ public HttpError(Exception exception, String path, HttpStatus status) { this.timestamp = LocalDateTime.now(ZoneOffset.UTC); this.status = getStatusFromResponseStatus(responseStatus, status); this.error = getErrorReason(responseStatus, status); - this.message = exception.getMessage(); + this.message = error; this.path = UriUtils.encodePath(path, StandardCharsets.UTF_8); } @@ -120,6 +121,7 @@ private String getErrorReason(ResponseStatus responseStatus) { return DEFAULT_ERROR; } + @JsonIgnore public String getException() { return exception; } diff --git a/src/main/java/uk/gov/hmcts/ccd/endpoint/exceptions/RestExceptionHandler.java b/src/main/java/uk/gov/hmcts/ccd/endpoint/exceptions/RestExceptionHandler.java index d0976a488b..b5bd6a4167 100644 --- a/src/main/java/uk/gov/hmcts/ccd/endpoint/exceptions/RestExceptionHandler.java +++ b/src/main/java/uk/gov/hmcts/ccd/endpoint/exceptions/RestExceptionHandler.java @@ -80,6 +80,7 @@ public ResponseEntity handleCaseValidationException(HttpServletReques exception.getFields(), exception); appInsights.trackException(exception, customProperties, SeverityLevel.Warning); final HttpError error = new HttpError<>(exception, request) + .withMessage(exception.getMessage()) .withDetails(exception.getDetails()); return ResponseEntity .status(error.getStatus()) @@ -105,8 +106,7 @@ public ResponseEntity handleConstraintViolationException(final HttpSe LOG.error(exception.getMessage()); appInsights.trackException(exception); - final HttpError error = new HttpError<>(exception, request, HttpStatus.BAD_REQUEST) - .withDetails(exception.getCause()); + final HttpError error = new HttpError<>(exception, request, HttpStatus.BAD_REQUEST); return ResponseEntity .status(HttpStatus.BAD_REQUEST) .body(error); diff --git a/src/main/java/uk/gov/hmcts/ccd/security/DataStoreHttpStatusRequestRejectedHandler.java b/src/main/java/uk/gov/hmcts/ccd/security/DataStoreHttpStatusRequestRejectedHandler.java index 98b7907431..ff29fa04fd 100644 --- a/src/main/java/uk/gov/hmcts/ccd/security/DataStoreHttpStatusRequestRejectedHandler.java +++ b/src/main/java/uk/gov/hmcts/ccd/security/DataStoreHttpStatusRequestRejectedHandler.java @@ -37,6 +37,6 @@ public void handle(HttpServletRequest request, HttpServletResponse response, final String errorMsg = requestRejectedException.getMessage(); LOG.warn(errorMsg); appInsights.trackException(requestRejectedException); - response.sendError(httpError, errorMsg); + response.sendError(httpError); } } diff --git a/src/test/java/uk/gov/hmcts/ccd/ElasticsearchIT.java b/src/test/java/uk/gov/hmcts/ccd/ElasticsearchIT.java index f24aa28e43..f0f736785f 100644 --- a/src/test/java/uk/gov/hmcts/ccd/ElasticsearchIT.java +++ b/src/test/java/uk/gov/hmcts/ccd/ElasticsearchIT.java @@ -76,7 +76,6 @@ import static org.hamcrest.CoreMatchers.nullValue; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.hasProperty; -import static org.hamcrest.Matchers.startsWith; import static org.junit.jupiter.api.Assertions.assertAll; import static org.mockito.Mockito.doReturn; import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.post; @@ -1591,7 +1590,7 @@ void shouldErrorWhenInvalidCaseTypeIsProvided() throws Exception { assertAll( () -> assertThat(exceptionNode.get("message").asText(), - startsWith("Resource not found when getting case type definition for INVALID")) + is("Not Found")) ); } diff --git a/src/test/java/uk/gov/hmcts/ccd/domain/model/common/HttpErrorTest.java b/src/test/java/uk/gov/hmcts/ccd/domain/model/common/HttpErrorTest.java index 508300168e..be8102fe25 100644 --- a/src/test/java/uk/gov/hmcts/ccd/domain/model/common/HttpErrorTest.java +++ b/src/test/java/uk/gov/hmcts/ccd/domain/model/common/HttpErrorTest.java @@ -5,11 +5,13 @@ import org.springframework.http.HttpStatus; import org.springframework.web.bind.annotation.ResponseStatus; import org.springframework.web.util.UriUtils; +import uk.gov.hmcts.ccd.config.JacksonObjectMapperConfig; import jakarta.servlet.http.HttpServletRequest; import java.nio.charset.StandardCharsets; +import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.is; import static org.hamcrest.Matchers.not; @@ -40,6 +42,17 @@ public void shouldExtractExceptionNameFromException() { assertThat(error.getException(), is(equalTo("java.lang.IllegalArgumentException"))); } + @Test + public void shouldNotSerialiseExceptionName() throws Exception { + final uk.gov.hmcts.ccd.domain.model.common.HttpError error = + new uk.gov.hmcts.ccd.domain.model.common.HttpError(new IllegalArgumentException(), request); + + String response = new JacksonObjectMapperConfig().defaultObjectMapper().writeValueAsString(error); + + assertThat(response, not(containsString("exception"))); + assertThat(response, not(containsString("java.lang.IllegalArgumentException"))); + } + @Test public void shouldExtractTimestampFromException() { final uk.gov.hmcts.ccd.domain.model.common.HttpError error = @@ -97,11 +110,11 @@ public void shouldExtractErrorFromExceptionAnnotation_default() { } @Test - public void shouldExtractMessageFromException() { + public void shouldUseSafeDefaultMessageFromException() { final uk.gov.hmcts.ccd.domain.model.common.HttpError error = new uk.gov.hmcts.ccd.domain.model.common.HttpError(new IllegalArgumentException(MESSAGE), request); - assertThat(error.getMessage(), is(equalTo(MESSAGE))); + assertThat(error.getMessage(), is(equalTo(uk.gov.hmcts.ccd.domain.model.common.HttpError.DEFAULT_ERROR))); } @Test diff --git a/src/test/java/uk/gov/hmcts/ccd/endpoint/exceptions/RestExceptionHandlerTest.java b/src/test/java/uk/gov/hmcts/ccd/endpoint/exceptions/RestExceptionHandlerTest.java index 03e9ebd7c4..8894647304 100644 --- a/src/test/java/uk/gov/hmcts/ccd/endpoint/exceptions/RestExceptionHandlerTest.java +++ b/src/test/java/uk/gov/hmcts/ccd/endpoint/exceptions/RestExceptionHandlerTest.java @@ -756,6 +756,7 @@ public void handleConstraintViolationException_shouldReturnHttpErrorResponse() t final ResultActions result = mockMvc.perform(MockMvcRequestBuilders.get(TEST_URL)); assertHttpErrorResponse(result, expectedException, HttpStatus.BAD_REQUEST); + result.andExpect(jsonPath("$.details").doesNotExist()); } private void assertHttpErrorResponse(ResultActions result, Exception expectedException) throws Exception { @@ -763,6 +764,7 @@ private void assertHttpErrorResponse(ResultActions result, Exception expectedExc // NB: as we cannot mock HttpError generate an equivalent and compare to response final HttpError expectedError = new HttpError<>(expectedException, mock(HttpServletRequest.class)); + withExplicitClientVisibleMessage(expectedError, expectedException); assertBasicHttpErrorResponseProperties(result, expectedException, expectedError); } @@ -776,6 +778,7 @@ private void assertHttpErrorResponse(ResultActions result, Exception expectedExc // NB: as we cannot mock HttpError generate an equivalent and compare to response final HttpError expectedError = new HttpError<>(expectedException, mock(HttpServletRequest.class), expectedHttpStatus); + withExplicitClientVisibleMessage(expectedError, expectedException); assertBasicHttpErrorResponseProperties(result, expectedException, expectedError); } @@ -785,10 +788,16 @@ private void assertBasicHttpErrorResponseProperties(ResultActions result, Except // check the very basics result.andExpect(status().is(expectedHttpError.getStatus())); - result.andExpect(jsonPath("$.exception").value(expectedException.getClass().getName())); + result.andExpect(jsonPath("$.exception").doesNotExist()); - // check a bit more - result.andExpect(jsonPath("$.message").value(expectedException.getMessage())); + // check the public error message doesn't default to raw exception text + result.andExpect(jsonPath("$.message").value(expectedHttpError.getMessage())); + } + + private void withExplicitClientVisibleMessage(HttpError expectedError, Exception expectedException) { + if (expectedException instanceof CaseValidationException) { + expectedError.withMessage(expectedException.getMessage()); + } } private void assertExtraApiExceptionResponseProperties(ResultActions result, ApiException expectedException) diff --git a/src/test/java/uk/gov/hmcts/ccd/endpoint/std/CallbackTest.java b/src/test/java/uk/gov/hmcts/ccd/endpoint/std/CallbackTest.java index 6d19ad1090..52bdf5a36f 100644 --- a/src/test/java/uk/gov/hmcts/ccd/endpoint/std/CallbackTest.java +++ b/src/test/java/uk/gov/hmcts/ccd/endpoint/std/CallbackTest.java @@ -620,8 +620,7 @@ public void shouldReturn422WhenPostCreateCaseWithCallbackOverridingDataWithMissi ).andReturn(); assertEquals("Incorrect Response Status Code", 422, mvcResult.getResponse().getStatus()); - assertEquals("Incorrect Error Message", "\"The event cannot be complete due to a callback returned data " - + "validation error (c)\"", + assertEquals("Incorrect Error Message", "\"Unprocessable Entity\"", mapper.readTree(mvcResult.getResponse().getContentAsString()).get("message").toString()); } diff --git a/src/test/java/uk/gov/hmcts/ccd/security/DataStoreHttpStatusRequestRejectedHandlerTest.java b/src/test/java/uk/gov/hmcts/ccd/security/DataStoreHttpStatusRequestRejectedHandlerTest.java index 876dd65b9c..4dde4cf72e 100644 --- a/src/test/java/uk/gov/hmcts/ccd/security/DataStoreHttpStatusRequestRejectedHandlerTest.java +++ b/src/test/java/uk/gov/hmcts/ccd/security/DataStoreHttpStatusRequestRejectedHandlerTest.java @@ -23,6 +23,7 @@ import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.is; +import static org.hamcrest.Matchers.nullValue; import static org.junit.Assert.fail; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verify; @@ -80,9 +81,9 @@ private void checkDataStoreHttpStatusRequestRejectedHandler(DataStoreHttpStatusR // Confirm that exception was passed to application insights verify(appInsights).trackException(requestRejectedException); - // Confirm that response contains expected http error and exception message + // Confirm that response contains expected http error without leaking the exception message assertThat(mockHttpServletResponse.getStatus(), is(equalTo(expectedHttpError))); - assertThat(mockHttpServletResponse.getErrorMessage(), is(equalTo(EXCEPTION_MESSAGE))); + assertThat(mockHttpServletResponse.getErrorMessage(), is(nullValue())); // Confirm that exception message was written to log and at the expected logging level List loggerList = handlerLoggerListAppender.list; From a9dba1d37677a15e30cfdcf452606dbf1962d43f Mon Sep 17 00:00:00 2001 From: Sammi Chong <133241942+SammiChong@users.noreply.github.com> Date: Fri, 8 May 2026 14:20:57 +0100 Subject: [PATCH 3/6] Revert "Remove detailed error data" This reverts commit 25904491ab03eef58cf2f8095c9f3fa4493e97b2. --- .../ccd/domain/model/common/HttpError.java | 6 ++---- .../exceptions/RestExceptionHandler.java | 4 ++-- ...taStoreHttpStatusRequestRejectedHandler.java | 2 +- .../java/uk/gov/hmcts/ccd/ElasticsearchIT.java | 3 ++- .../ccd/domain/model/common/HttpErrorTest.java | 17 ++--------------- .../exceptions/RestExceptionHandlerTest.java | 15 +++------------ .../hmcts/ccd/endpoint/std/CallbackTest.java | 3 ++- ...oreHttpStatusRequestRejectedHandlerTest.java | 5 ++--- 8 files changed, 16 insertions(+), 39 deletions(-) diff --git a/src/main/java/uk/gov/hmcts/ccd/domain/model/common/HttpError.java b/src/main/java/uk/gov/hmcts/ccd/domain/model/common/HttpError.java index 9c1befd690..dd9b63f0a3 100644 --- a/src/main/java/uk/gov/hmcts/ccd/domain/model/common/HttpError.java +++ b/src/main/java/uk/gov/hmcts/ccd/domain/model/common/HttpError.java @@ -1,6 +1,5 @@ package uk.gov.hmcts.ccd.domain.model.common; -import com.fasterxml.jackson.annotation.JsonIgnore; import org.springframework.http.HttpStatus; import org.springframework.web.bind.annotation.ResponseStatus; import org.springframework.web.context.request.ServletWebRequest; @@ -35,7 +34,7 @@ public HttpError(Exception exception, HttpServletRequest request) { this.timestamp = LocalDateTime.now(ZoneOffset.UTC); this.status = getStatusFromResponseStatus(responseStatus); this.error = getErrorReason(responseStatus); - this.message = error; + this.message = exception.getMessage(); this.path = UriUtils.encodePath(request.getRequestURI(), StandardCharsets.UTF_8); } @@ -46,7 +45,7 @@ public HttpError(Exception exception, String path, HttpStatus status) { this.timestamp = LocalDateTime.now(ZoneOffset.UTC); this.status = getStatusFromResponseStatus(responseStatus, status); this.error = getErrorReason(responseStatus, status); - this.message = error; + this.message = exception.getMessage(); this.path = UriUtils.encodePath(path, StandardCharsets.UTF_8); } @@ -121,7 +120,6 @@ private String getErrorReason(ResponseStatus responseStatus) { return DEFAULT_ERROR; } - @JsonIgnore public String getException() { return exception; } diff --git a/src/main/java/uk/gov/hmcts/ccd/endpoint/exceptions/RestExceptionHandler.java b/src/main/java/uk/gov/hmcts/ccd/endpoint/exceptions/RestExceptionHandler.java index b5bd6a4167..d0976a488b 100644 --- a/src/main/java/uk/gov/hmcts/ccd/endpoint/exceptions/RestExceptionHandler.java +++ b/src/main/java/uk/gov/hmcts/ccd/endpoint/exceptions/RestExceptionHandler.java @@ -80,7 +80,6 @@ public ResponseEntity handleCaseValidationException(HttpServletReques exception.getFields(), exception); appInsights.trackException(exception, customProperties, SeverityLevel.Warning); final HttpError error = new HttpError<>(exception, request) - .withMessage(exception.getMessage()) .withDetails(exception.getDetails()); return ResponseEntity .status(error.getStatus()) @@ -106,7 +105,8 @@ public ResponseEntity handleConstraintViolationException(final HttpSe LOG.error(exception.getMessage()); appInsights.trackException(exception); - final HttpError error = new HttpError<>(exception, request, HttpStatus.BAD_REQUEST); + final HttpError error = new HttpError<>(exception, request, HttpStatus.BAD_REQUEST) + .withDetails(exception.getCause()); return ResponseEntity .status(HttpStatus.BAD_REQUEST) .body(error); diff --git a/src/main/java/uk/gov/hmcts/ccd/security/DataStoreHttpStatusRequestRejectedHandler.java b/src/main/java/uk/gov/hmcts/ccd/security/DataStoreHttpStatusRequestRejectedHandler.java index ff29fa04fd..98b7907431 100644 --- a/src/main/java/uk/gov/hmcts/ccd/security/DataStoreHttpStatusRequestRejectedHandler.java +++ b/src/main/java/uk/gov/hmcts/ccd/security/DataStoreHttpStatusRequestRejectedHandler.java @@ -37,6 +37,6 @@ public void handle(HttpServletRequest request, HttpServletResponse response, final String errorMsg = requestRejectedException.getMessage(); LOG.warn(errorMsg); appInsights.trackException(requestRejectedException); - response.sendError(httpError); + response.sendError(httpError, errorMsg); } } diff --git a/src/test/java/uk/gov/hmcts/ccd/ElasticsearchIT.java b/src/test/java/uk/gov/hmcts/ccd/ElasticsearchIT.java index f0f736785f..f24aa28e43 100644 --- a/src/test/java/uk/gov/hmcts/ccd/ElasticsearchIT.java +++ b/src/test/java/uk/gov/hmcts/ccd/ElasticsearchIT.java @@ -76,6 +76,7 @@ import static org.hamcrest.CoreMatchers.nullValue; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.hasProperty; +import static org.hamcrest.Matchers.startsWith; import static org.junit.jupiter.api.Assertions.assertAll; import static org.mockito.Mockito.doReturn; import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.post; @@ -1590,7 +1591,7 @@ void shouldErrorWhenInvalidCaseTypeIsProvided() throws Exception { assertAll( () -> assertThat(exceptionNode.get("message").asText(), - is("Not Found")) + startsWith("Resource not found when getting case type definition for INVALID")) ); } diff --git a/src/test/java/uk/gov/hmcts/ccd/domain/model/common/HttpErrorTest.java b/src/test/java/uk/gov/hmcts/ccd/domain/model/common/HttpErrorTest.java index be8102fe25..508300168e 100644 --- a/src/test/java/uk/gov/hmcts/ccd/domain/model/common/HttpErrorTest.java +++ b/src/test/java/uk/gov/hmcts/ccd/domain/model/common/HttpErrorTest.java @@ -5,13 +5,11 @@ import org.springframework.http.HttpStatus; import org.springframework.web.bind.annotation.ResponseStatus; import org.springframework.web.util.UriUtils; -import uk.gov.hmcts.ccd.config.JacksonObjectMapperConfig; import jakarta.servlet.http.HttpServletRequest; import java.nio.charset.StandardCharsets; -import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.is; import static org.hamcrest.Matchers.not; @@ -42,17 +40,6 @@ public void shouldExtractExceptionNameFromException() { assertThat(error.getException(), is(equalTo("java.lang.IllegalArgumentException"))); } - @Test - public void shouldNotSerialiseExceptionName() throws Exception { - final uk.gov.hmcts.ccd.domain.model.common.HttpError error = - new uk.gov.hmcts.ccd.domain.model.common.HttpError(new IllegalArgumentException(), request); - - String response = new JacksonObjectMapperConfig().defaultObjectMapper().writeValueAsString(error); - - assertThat(response, not(containsString("exception"))); - assertThat(response, not(containsString("java.lang.IllegalArgumentException"))); - } - @Test public void shouldExtractTimestampFromException() { final uk.gov.hmcts.ccd.domain.model.common.HttpError error = @@ -110,11 +97,11 @@ public void shouldExtractErrorFromExceptionAnnotation_default() { } @Test - public void shouldUseSafeDefaultMessageFromException() { + public void shouldExtractMessageFromException() { final uk.gov.hmcts.ccd.domain.model.common.HttpError error = new uk.gov.hmcts.ccd.domain.model.common.HttpError(new IllegalArgumentException(MESSAGE), request); - assertThat(error.getMessage(), is(equalTo(uk.gov.hmcts.ccd.domain.model.common.HttpError.DEFAULT_ERROR))); + assertThat(error.getMessage(), is(equalTo(MESSAGE))); } @Test diff --git a/src/test/java/uk/gov/hmcts/ccd/endpoint/exceptions/RestExceptionHandlerTest.java b/src/test/java/uk/gov/hmcts/ccd/endpoint/exceptions/RestExceptionHandlerTest.java index 8894647304..03e9ebd7c4 100644 --- a/src/test/java/uk/gov/hmcts/ccd/endpoint/exceptions/RestExceptionHandlerTest.java +++ b/src/test/java/uk/gov/hmcts/ccd/endpoint/exceptions/RestExceptionHandlerTest.java @@ -756,7 +756,6 @@ public void handleConstraintViolationException_shouldReturnHttpErrorResponse() t final ResultActions result = mockMvc.perform(MockMvcRequestBuilders.get(TEST_URL)); assertHttpErrorResponse(result, expectedException, HttpStatus.BAD_REQUEST); - result.andExpect(jsonPath("$.details").doesNotExist()); } private void assertHttpErrorResponse(ResultActions result, Exception expectedException) throws Exception { @@ -764,7 +763,6 @@ private void assertHttpErrorResponse(ResultActions result, Exception expectedExc // NB: as we cannot mock HttpError generate an equivalent and compare to response final HttpError expectedError = new HttpError<>(expectedException, mock(HttpServletRequest.class)); - withExplicitClientVisibleMessage(expectedError, expectedException); assertBasicHttpErrorResponseProperties(result, expectedException, expectedError); } @@ -778,7 +776,6 @@ private void assertHttpErrorResponse(ResultActions result, Exception expectedExc // NB: as we cannot mock HttpError generate an equivalent and compare to response final HttpError expectedError = new HttpError<>(expectedException, mock(HttpServletRequest.class), expectedHttpStatus); - withExplicitClientVisibleMessage(expectedError, expectedException); assertBasicHttpErrorResponseProperties(result, expectedException, expectedError); } @@ -788,16 +785,10 @@ private void assertBasicHttpErrorResponseProperties(ResultActions result, Except // check the very basics result.andExpect(status().is(expectedHttpError.getStatus())); - result.andExpect(jsonPath("$.exception").doesNotExist()); + result.andExpect(jsonPath("$.exception").value(expectedException.getClass().getName())); - // check the public error message doesn't default to raw exception text - result.andExpect(jsonPath("$.message").value(expectedHttpError.getMessage())); - } - - private void withExplicitClientVisibleMessage(HttpError expectedError, Exception expectedException) { - if (expectedException instanceof CaseValidationException) { - expectedError.withMessage(expectedException.getMessage()); - } + // check a bit more + result.andExpect(jsonPath("$.message").value(expectedException.getMessage())); } private void assertExtraApiExceptionResponseProperties(ResultActions result, ApiException expectedException) diff --git a/src/test/java/uk/gov/hmcts/ccd/endpoint/std/CallbackTest.java b/src/test/java/uk/gov/hmcts/ccd/endpoint/std/CallbackTest.java index 52bdf5a36f..6d19ad1090 100644 --- a/src/test/java/uk/gov/hmcts/ccd/endpoint/std/CallbackTest.java +++ b/src/test/java/uk/gov/hmcts/ccd/endpoint/std/CallbackTest.java @@ -620,7 +620,8 @@ public void shouldReturn422WhenPostCreateCaseWithCallbackOverridingDataWithMissi ).andReturn(); assertEquals("Incorrect Response Status Code", 422, mvcResult.getResponse().getStatus()); - assertEquals("Incorrect Error Message", "\"Unprocessable Entity\"", + assertEquals("Incorrect Error Message", "\"The event cannot be complete due to a callback returned data " + + "validation error (c)\"", mapper.readTree(mvcResult.getResponse().getContentAsString()).get("message").toString()); } diff --git a/src/test/java/uk/gov/hmcts/ccd/security/DataStoreHttpStatusRequestRejectedHandlerTest.java b/src/test/java/uk/gov/hmcts/ccd/security/DataStoreHttpStatusRequestRejectedHandlerTest.java index 4dde4cf72e..876dd65b9c 100644 --- a/src/test/java/uk/gov/hmcts/ccd/security/DataStoreHttpStatusRequestRejectedHandlerTest.java +++ b/src/test/java/uk/gov/hmcts/ccd/security/DataStoreHttpStatusRequestRejectedHandlerTest.java @@ -23,7 +23,6 @@ import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.is; -import static org.hamcrest.Matchers.nullValue; import static org.junit.Assert.fail; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verify; @@ -81,9 +80,9 @@ private void checkDataStoreHttpStatusRequestRejectedHandler(DataStoreHttpStatusR // Confirm that exception was passed to application insights verify(appInsights).trackException(requestRejectedException); - // Confirm that response contains expected http error without leaking the exception message + // Confirm that response contains expected http error and exception message assertThat(mockHttpServletResponse.getStatus(), is(equalTo(expectedHttpError))); - assertThat(mockHttpServletResponse.getErrorMessage(), is(nullValue())); + assertThat(mockHttpServletResponse.getErrorMessage(), is(equalTo(EXCEPTION_MESSAGE))); // Confirm that exception message was written to log and at the expected logging level List loggerList = handlerLoggerListAppender.list; From b2d6480898208020b6a7060064cfc2f189128011 Mon Sep 17 00:00:00 2001 From: Sammi Chong <133241942+SammiChong@users.noreply.github.com> Date: Mon, 11 May 2026 16:28:08 +0100 Subject: [PATCH 4/6] Override message --- .../hmcts/ccd/config/SafeErrorAttributes.java | 38 +++++++++++++++++++ .../ccd/domain/model/common/HttpError.java | 4 +- .../exceptions/RestExceptionHandler.java | 1 + .../uk/gov/hmcts/ccd/ElasticsearchIT.java | 3 +- .../ccd/config/SafeErrorAttributesTest.java | 28 ++++++++++++++ .../domain/model/common/HttpErrorTest.java | 4 +- .../exceptions/RestExceptionHandlerTest.java | 10 ++++- .../hmcts/ccd/endpoint/std/CallbackTest.java | 3 +- 8 files changed, 82 insertions(+), 9 deletions(-) create mode 100644 src/main/java/uk/gov/hmcts/ccd/config/SafeErrorAttributes.java create mode 100644 src/test/java/uk/gov/hmcts/ccd/config/SafeErrorAttributesTest.java diff --git a/src/main/java/uk/gov/hmcts/ccd/config/SafeErrorAttributes.java b/src/main/java/uk/gov/hmcts/ccd/config/SafeErrorAttributes.java new file mode 100644 index 0000000000..0387bbdac0 --- /dev/null +++ b/src/main/java/uk/gov/hmcts/ccd/config/SafeErrorAttributes.java @@ -0,0 +1,38 @@ +package uk.gov.hmcts.ccd.config; + +import org.springframework.boot.web.error.ErrorAttributeOptions; +import org.springframework.boot.web.servlet.error.DefaultErrorAttributes; +import org.springframework.http.HttpStatus; +import org.springframework.stereotype.Component; +import org.springframework.web.context.request.WebRequest; +import uk.gov.hmcts.ccd.domain.model.common.HttpError; + +import java.util.Map; + +@Component +public class SafeErrorAttributes extends DefaultErrorAttributes { + + private static final String ACCESS_DENIED_MESSAGE = "Access Denied"; + + @Override + public Map getErrorAttributes(WebRequest webRequest, ErrorAttributeOptions options) { + Map errorAttributes = super.getErrorAttributes(webRequest, options); + errorAttributes.put("message", safeMessage(errorAttributes.get("status"))); + errorAttributes.remove("exception"); + return errorAttributes; + } + + String safeMessage(Object status) { + if (status instanceof Number statusCode) { + HttpStatus httpStatus = HttpStatus.resolve(statusCode.intValue()); + if (HttpStatus.FORBIDDEN.equals(httpStatus)) { + return ACCESS_DENIED_MESSAGE; + } + if (httpStatus != null) { + return httpStatus.getReasonPhrase(); + } + } + + return HttpError.DEFAULT_ERROR; + } +} diff --git a/src/main/java/uk/gov/hmcts/ccd/domain/model/common/HttpError.java b/src/main/java/uk/gov/hmcts/ccd/domain/model/common/HttpError.java index dd9b63f0a3..837f6ac5dd 100644 --- a/src/main/java/uk/gov/hmcts/ccd/domain/model/common/HttpError.java +++ b/src/main/java/uk/gov/hmcts/ccd/domain/model/common/HttpError.java @@ -34,7 +34,7 @@ public HttpError(Exception exception, HttpServletRequest request) { this.timestamp = LocalDateTime.now(ZoneOffset.UTC); this.status = getStatusFromResponseStatus(responseStatus); this.error = getErrorReason(responseStatus); - this.message = exception.getMessage(); + this.message = error; this.path = UriUtils.encodePath(request.getRequestURI(), StandardCharsets.UTF_8); } @@ -45,7 +45,7 @@ public HttpError(Exception exception, String path, HttpStatus status) { this.timestamp = LocalDateTime.now(ZoneOffset.UTC); this.status = getStatusFromResponseStatus(responseStatus, status); this.error = getErrorReason(responseStatus, status); - this.message = exception.getMessage(); + this.message = error; this.path = UriUtils.encodePath(path, StandardCharsets.UTF_8); } diff --git a/src/main/java/uk/gov/hmcts/ccd/endpoint/exceptions/RestExceptionHandler.java b/src/main/java/uk/gov/hmcts/ccd/endpoint/exceptions/RestExceptionHandler.java index d0976a488b..5891987da7 100644 --- a/src/main/java/uk/gov/hmcts/ccd/endpoint/exceptions/RestExceptionHandler.java +++ b/src/main/java/uk/gov/hmcts/ccd/endpoint/exceptions/RestExceptionHandler.java @@ -80,6 +80,7 @@ public ResponseEntity handleCaseValidationException(HttpServletReques exception.getFields(), exception); appInsights.trackException(exception, customProperties, SeverityLevel.Warning); final HttpError error = new HttpError<>(exception, request) + .withMessage(exception.getMessage()) .withDetails(exception.getDetails()); return ResponseEntity .status(error.getStatus()) diff --git a/src/test/java/uk/gov/hmcts/ccd/ElasticsearchIT.java b/src/test/java/uk/gov/hmcts/ccd/ElasticsearchIT.java index f24aa28e43..f0f736785f 100644 --- a/src/test/java/uk/gov/hmcts/ccd/ElasticsearchIT.java +++ b/src/test/java/uk/gov/hmcts/ccd/ElasticsearchIT.java @@ -76,7 +76,6 @@ import static org.hamcrest.CoreMatchers.nullValue; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.hasProperty; -import static org.hamcrest.Matchers.startsWith; import static org.junit.jupiter.api.Assertions.assertAll; import static org.mockito.Mockito.doReturn; import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.post; @@ -1591,7 +1590,7 @@ void shouldErrorWhenInvalidCaseTypeIsProvided() throws Exception { assertAll( () -> assertThat(exceptionNode.get("message").asText(), - startsWith("Resource not found when getting case type definition for INVALID")) + is("Not Found")) ); } diff --git a/src/test/java/uk/gov/hmcts/ccd/config/SafeErrorAttributesTest.java b/src/test/java/uk/gov/hmcts/ccd/config/SafeErrorAttributesTest.java new file mode 100644 index 0000000000..f315c19df8 --- /dev/null +++ b/src/test/java/uk/gov/hmcts/ccd/config/SafeErrorAttributesTest.java @@ -0,0 +1,28 @@ +package uk.gov.hmcts.ccd.config; + +import org.junit.jupiter.api.Test; +import org.springframework.http.HttpStatus; +import uk.gov.hmcts.ccd.domain.model.common.HttpError; + +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.is; + +class SafeErrorAttributesTest { + + private final SafeErrorAttributes safeErrorAttributes = new SafeErrorAttributes(); + + @Test + void shouldUseAccessDeniedForForbiddenResponses() { + assertThat(safeErrorAttributes.safeMessage(HttpStatus.FORBIDDEN.value()), is("Access Denied")); + } + + @Test + void shouldUseReasonPhraseForNonForbiddenResponses() { + assertThat(safeErrorAttributes.safeMessage(HttpStatus.BAD_REQUEST.value()), is("Bad Request")); + } + + @Test + void shouldUseDefaultErrorWhenStatusCannotBeResolved() { + assertThat(safeErrorAttributes.safeMessage(null), is(HttpError.DEFAULT_ERROR)); + } +} diff --git a/src/test/java/uk/gov/hmcts/ccd/domain/model/common/HttpErrorTest.java b/src/test/java/uk/gov/hmcts/ccd/domain/model/common/HttpErrorTest.java index 508300168e..a9b3c2f9ad 100644 --- a/src/test/java/uk/gov/hmcts/ccd/domain/model/common/HttpErrorTest.java +++ b/src/test/java/uk/gov/hmcts/ccd/domain/model/common/HttpErrorTest.java @@ -97,11 +97,11 @@ public void shouldExtractErrorFromExceptionAnnotation_default() { } @Test - public void shouldExtractMessageFromException() { + public void shouldUseSafeDefaultMessageFromException() { final uk.gov.hmcts.ccd.domain.model.common.HttpError error = new uk.gov.hmcts.ccd.domain.model.common.HttpError(new IllegalArgumentException(MESSAGE), request); - assertThat(error.getMessage(), is(equalTo(MESSAGE))); + assertThat(error.getMessage(), is(equalTo(uk.gov.hmcts.ccd.domain.model.common.HttpError.DEFAULT_ERROR))); } @Test diff --git a/src/test/java/uk/gov/hmcts/ccd/endpoint/exceptions/RestExceptionHandlerTest.java b/src/test/java/uk/gov/hmcts/ccd/endpoint/exceptions/RestExceptionHandlerTest.java index 03e9ebd7c4..bae3f3b072 100644 --- a/src/test/java/uk/gov/hmcts/ccd/endpoint/exceptions/RestExceptionHandlerTest.java +++ b/src/test/java/uk/gov/hmcts/ccd/endpoint/exceptions/RestExceptionHandlerTest.java @@ -763,6 +763,7 @@ private void assertHttpErrorResponse(ResultActions result, Exception expectedExc // NB: as we cannot mock HttpError generate an equivalent and compare to response final HttpError expectedError = new HttpError<>(expectedException, mock(HttpServletRequest.class)); + withExplicitClientVisibleMessage(expectedError, expectedException); assertBasicHttpErrorResponseProperties(result, expectedException, expectedError); } @@ -776,10 +777,17 @@ private void assertHttpErrorResponse(ResultActions result, Exception expectedExc // NB: as we cannot mock HttpError generate an equivalent and compare to response final HttpError expectedError = new HttpError<>(expectedException, mock(HttpServletRequest.class), expectedHttpStatus); + withExplicitClientVisibleMessage(expectedError, expectedException); assertBasicHttpErrorResponseProperties(result, expectedException, expectedError); } + private void withExplicitClientVisibleMessage(HttpError expectedError, Exception expectedException) { + if (expectedException instanceof CaseValidationException) { + expectedError.withMessage(expectedException.getMessage()); + } + } + private void assertBasicHttpErrorResponseProperties(ResultActions result, Exception expectedException, HttpError expectedHttpError) throws Exception { @@ -788,7 +796,7 @@ private void assertBasicHttpErrorResponseProperties(ResultActions result, Except result.andExpect(jsonPath("$.exception").value(expectedException.getClass().getName())); // check a bit more - result.andExpect(jsonPath("$.message").value(expectedException.getMessage())); + result.andExpect(jsonPath("$.message").value(expectedHttpError.getMessage())); } private void assertExtraApiExceptionResponseProperties(ResultActions result, ApiException expectedException) diff --git a/src/test/java/uk/gov/hmcts/ccd/endpoint/std/CallbackTest.java b/src/test/java/uk/gov/hmcts/ccd/endpoint/std/CallbackTest.java index 6d19ad1090..52bdf5a36f 100644 --- a/src/test/java/uk/gov/hmcts/ccd/endpoint/std/CallbackTest.java +++ b/src/test/java/uk/gov/hmcts/ccd/endpoint/std/CallbackTest.java @@ -620,8 +620,7 @@ public void shouldReturn422WhenPostCreateCaseWithCallbackOverridingDataWithMissi ).andReturn(); assertEquals("Incorrect Response Status Code", 422, mvcResult.getResponse().getStatus()); - assertEquals("Incorrect Error Message", "\"The event cannot be complete due to a callback returned data " - + "validation error (c)\"", + assertEquals("Incorrect Error Message", "\"Unprocessable Entity\"", mapper.readTree(mvcResult.getResponse().getContentAsString()).get("message").toString()); } From 7bc705ba8696673713bae0290dbdfe0aad1f1e7b Mon Sep 17 00:00:00 2001 From: Sammi Chong <133241942+SammiChong@users.noreply.github.com> Date: Mon, 11 May 2026 17:07:13 +0100 Subject: [PATCH 5/6] Update IT --- src/test/java/uk/gov/hmcts/ccd/ElasticsearchIT.java | 9 ++++----- .../uk/gov/hmcts/ccd/endpoint/std/DraftsEndpointIT.java | 6 ++---- 2 files changed, 6 insertions(+), 9 deletions(-) diff --git a/src/test/java/uk/gov/hmcts/ccd/ElasticsearchIT.java b/src/test/java/uk/gov/hmcts/ccd/ElasticsearchIT.java index f0f736785f..9aa9bdc8c6 100644 --- a/src/test/java/uk/gov/hmcts/ccd/ElasticsearchIT.java +++ b/src/test/java/uk/gov/hmcts/ccd/ElasticsearchIT.java @@ -70,7 +70,6 @@ import static org.elasticsearch.index.query.QueryBuilders.boolQuery; import static org.elasticsearch.index.query.QueryBuilders.matchAllQuery; import static org.elasticsearch.index.query.QueryBuilders.matchQuery; -import static org.hamcrest.CoreMatchers.containsString; import static org.hamcrest.CoreMatchers.hasItem; import static org.hamcrest.CoreMatchers.is; import static org.hamcrest.CoreMatchers.nullValue; @@ -644,7 +643,7 @@ void shouldReturnErrorWithUnsupportedUseCase() throws Exception { assertAll( () -> assertThat(exceptionNode.get(ERROR_MESSAGE).asText(), - is("The provided use case 'INVALID' is unsupported for case type 'AAT'.")) + is("Bad Request")) ); } @@ -657,7 +656,7 @@ void shouldReturnErrorWithMissingQueryField() throws Exception { assertAll( () -> assertThat(exceptionNode.get(ERROR_MESSAGE).asText(), - is("missing required field 'query'")) + is("Bad Request")) ); } @@ -672,7 +671,7 @@ void shouldReturnErrorWhenElasticsearchErrors() throws Exception { assertAll( () -> assertThat(exceptionNode.get(ERROR_MESSAGE).asText(), - containsString("No mapping found for [invalid.keyword] in order to sort on")) + is("Bad Request")) ); } @@ -693,7 +692,7 @@ void shouldReturnErrorWhenInvalidJsonIsSent() throws Exception { assertAll( () -> assertThat(exceptionNode.get(ERROR_MESSAGE).asText(), - containsString("Request requires correctly formatted JSON")) + is("Bad Request")) ); } diff --git a/src/test/java/uk/gov/hmcts/ccd/endpoint/std/DraftsEndpointIT.java b/src/test/java/uk/gov/hmcts/ccd/endpoint/std/DraftsEndpointIT.java index f66b0b5c80..ae4f813183 100644 --- a/src/test/java/uk/gov/hmcts/ccd/endpoint/std/DraftsEndpointIT.java +++ b/src/test/java/uk/gov/hmcts/ccd/endpoint/std/DraftsEndpointIT.java @@ -162,8 +162,7 @@ public void shouldReturn404WhenUpdateDraftForCaseworkerWithDraftIdNotFound() thr assertEquals("Incorrect Response Status Code", 404, mvcResult.getResponse().getStatus()); String actualResponse = mapper.readTree(mvcResult.getResponse().getContentAsString()).toString(); - assertThat(actualResponse, containsString("\"message\":" + - "\"No draft found ( draft reference = '7578590391163133' )\"")); + assertThat(actualResponse, containsString("\"message\":\"Not Found\"")); } @Test @@ -299,8 +298,7 @@ public void shouldReturn404WhenGetInvalidDraft() throws Exception { assertEquals("Incorrect Response Status Code", 404, mvcResult.getResponse().getStatus()); String actualResponse = mapper.readTree(mvcResult.getResponse().getContentAsString()).toString(); - assertThat(actualResponse, containsString("\"message\":" + - "\"No draft found ( draft reference = '7578590391163133' )\"")); + assertThat(actualResponse, containsString("\"message\":\"Not Found\"")); } @Test From 29f02a69f48cf451cb616d17dd26d2d4fa59cc07 Mon Sep 17 00:00:00 2001 From: Sammi Chong <133241942+SammiChong@users.noreply.github.com> Date: Mon, 11 May 2026 18:02:31 +0100 Subject: [PATCH 6/6] Update SafeErrorAttributesTest.java --- .../ccd/config/SafeErrorAttributesTest.java | 29 +++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/src/test/java/uk/gov/hmcts/ccd/config/SafeErrorAttributesTest.java b/src/test/java/uk/gov/hmcts/ccd/config/SafeErrorAttributesTest.java index f315c19df8..5d17af9866 100644 --- a/src/test/java/uk/gov/hmcts/ccd/config/SafeErrorAttributesTest.java +++ b/src/test/java/uk/gov/hmcts/ccd/config/SafeErrorAttributesTest.java @@ -1,9 +1,15 @@ package uk.gov.hmcts.ccd.config; import org.junit.jupiter.api.Test; +import org.springframework.boot.web.error.ErrorAttributeOptions; +import org.springframework.mock.web.MockHttpServletRequest; import org.springframework.http.HttpStatus; +import org.springframework.web.context.request.ServletWebRequest; import uk.gov.hmcts.ccd.domain.model.common.HttpError; +import jakarta.servlet.RequestDispatcher; +import java.util.Map; + import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.is; @@ -25,4 +31,27 @@ void shouldUseReasonPhraseForNonForbiddenResponses() { void shouldUseDefaultErrorWhenStatusCannotBeResolved() { assertThat(safeErrorAttributes.safeMessage(null), is(HttpError.DEFAULT_ERROR)); } + + @Test + void shouldOverrideDefaultErrorAttributesWithSafeMessageAndRemoveException() { + Map errorAttributes = safeErrorAttributes.getErrorAttributes( + errorRequest(HttpStatus.BAD_REQUEST, "Raw exception message"), + ErrorAttributeOptions.defaults().including( + ErrorAttributeOptions.Include.MESSAGE, + ErrorAttributeOptions.Include.EXCEPTION + ) + ); + + assertThat(errorAttributes.get("message"), is("Bad Request")); + assertThat(errorAttributes.containsKey("exception"), is(false)); + } + + private ServletWebRequest errorRequest(HttpStatus status, String message) { + MockHttpServletRequest request = new MockHttpServletRequest(); + request.setRequestURI("/some/path"); + request.setAttribute(RequestDispatcher.ERROR_STATUS_CODE, status.value()); + request.setAttribute(RequestDispatcher.ERROR_MESSAGE, message); + request.setAttribute(RequestDispatcher.ERROR_EXCEPTION, new IllegalArgumentException(message)); + return new ServletWebRequest(request); + } }