From 64a5ee31f5ac5e659f0966d84aaa2011b08a29c1 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Tue, 11 Aug 2026 02:29:54 +0900 Subject: [PATCH 1/3] test(security): reject unsafe client trace identifiers --- ...ExceptionHandlerTraceIdValidationTest.java | 68 +++++++++++++++++++ 1 file changed, 68 insertions(+) create mode 100644 src/test/java/com/clearfolio/viewer/controller/ApiExceptionHandlerTraceIdValidationTest.java diff --git a/src/test/java/com/clearfolio/viewer/controller/ApiExceptionHandlerTraceIdValidationTest.java b/src/test/java/com/clearfolio/viewer/controller/ApiExceptionHandlerTraceIdValidationTest.java new file mode 100644 index 00000000..ff14d307 --- /dev/null +++ b/src/test/java/com/clearfolio/viewer/controller/ApiExceptionHandlerTraceIdValidationTest.java @@ -0,0 +1,68 @@ +package com.clearfolio.viewer.controller; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotEquals; + +import org.junit.jupiter.api.Test; +import org.springframework.mock.http.server.reactive.MockServerHttpRequest; +import org.springframework.mock.web.server.MockServerWebExchange; + +import com.clearfolio.viewer.api.ApiErrorResponse; + +/** + * Verifies that client-controlled trace identifiers cannot become unbounded or + * path-like diagnostic identifiers while preserving a bounded opaque value. + */ +class ApiExceptionHandlerTraceIdValidationTest { + + private final ApiExceptionHandler handler = new ApiExceptionHandler(); + + @Test + void unsafeClientTraceIdFallsBackToServerRequestId() { + MockServerWebExchange exchange = MockServerWebExchange.from( + MockServerHttpRequest.get("/api/test") + .header("X-Trace-Id", "../../tenant-secret") + .build() + ); + + ApiErrorResponse body = handler + .handleBadRequest(new IllegalArgumentException("bad request"), exchange) + .getBody(); + + assertNotEquals("../../tenant-secret", body.traceId()); + assertEquals(exchange.getRequest().getId(), body.traceId()); + } + + @Test + void oversizedClientTraceIdFallsBackToServerRequestId() { + String oversized = "a".repeat(129); + MockServerWebExchange exchange = MockServerWebExchange.from( + MockServerHttpRequest.get("/api/test") + .header("X-Trace-Id", oversized) + .build() + ); + + ApiErrorResponse body = handler + .handleBadRequest(new IllegalArgumentException("bad request"), exchange) + .getBody(); + + assertNotEquals(oversized, body.traceId()); + assertEquals(exchange.getRequest().getId(), body.traceId()); + } + + @Test + void boundedOpaqueClientTraceIdIsPreserved() { + String traceId = "req-20260811_02:17.55"; + MockServerWebExchange exchange = MockServerWebExchange.from( + MockServerHttpRequest.get("/api/test") + .header("X-Trace-Id", traceId) + .build() + ); + + ApiErrorResponse body = handler + .handleBadRequest(new IllegalArgumentException("bad request"), exchange) + .getBody(); + + assertEquals(traceId, body.traceId()); + } +} From 4bd555bab85741dbfa6fce9ee18e748f14d75551 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Tue, 11 Aug 2026 02:34:00 +0900 Subject: [PATCH 2/3] fix(security): validate client trace identifiers --- .../viewer/controller/ApiExceptionHandler.java | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/main/java/com/clearfolio/viewer/controller/ApiExceptionHandler.java b/src/main/java/com/clearfolio/viewer/controller/ApiExceptionHandler.java index 524f925d..73c187a4 100644 --- a/src/main/java/com/clearfolio/viewer/controller/ApiExceptionHandler.java +++ b/src/main/java/com/clearfolio/viewer/controller/ApiExceptionHandler.java @@ -3,6 +3,7 @@ import java.util.Map; import java.util.LinkedHashMap; import java.util.UUID; +import java.util.regex.Pattern; import java.net.URI; import org.slf4j.Logger; @@ -30,6 +31,7 @@ public class ApiExceptionHandler { private static final Logger LOGGER = LoggerFactory.getLogger(ApiExceptionHandler.class); + private static final Pattern SAFE_TRACE_ID = Pattern.compile("[A-Za-z0-9][A-Za-z0-9._:-]{0,127}"); @Value("${conversion.max-upload-size-bytes:5242880}") private long configuredMaxUploadSize = 5242880L; @@ -223,18 +225,22 @@ public ResponseEntity handleUnexpected( private String resolveTraceId(ServerWebExchange exchange) { String header = exchange.getRequest().getHeaders().getFirst("X-Trace-Id"); - if (header != null && !header.isBlank()) { + if (isSafeTraceId(header)) { return header; } String requestId = exchange.getRequest().getId(); - if (requestId != null && !requestId.isBlank()) { + if (isSafeTraceId(requestId)) { return requestId; } return UUID.randomUUID().toString(); } + private boolean isSafeTraceId(String value) { + return value != null && SAFE_TRACE_ID.matcher(value).matches(); + } + private String normalizeStatusCode(HttpStatusCode statusCode) { int code = statusCode.value(); HttpStatus resolved = HttpStatus.resolve(code); From e5f344ea02f04ea3767d031f92e3d2fe4d0e3ab6 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Tue, 11 Aug 2026 03:07:43 +0900 Subject: [PATCH 3/3] test(security): cover unsafe trace-id fallback --- ...ExceptionHandlerTraceIdValidationTest.java | 28 +++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/src/test/java/com/clearfolio/viewer/controller/ApiExceptionHandlerTraceIdValidationTest.java b/src/test/java/com/clearfolio/viewer/controller/ApiExceptionHandlerTraceIdValidationTest.java index ff14d307..db915874 100644 --- a/src/test/java/com/clearfolio/viewer/controller/ApiExceptionHandlerTraceIdValidationTest.java +++ b/src/test/java/com/clearfolio/viewer/controller/ApiExceptionHandlerTraceIdValidationTest.java @@ -2,10 +2,17 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertNotEquals; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +import java.util.UUID; import org.junit.jupiter.api.Test; +import org.springframework.http.HttpHeaders; +import org.springframework.http.server.reactive.ServerHttpRequest; import org.springframework.mock.http.server.reactive.MockServerHttpRequest; import org.springframework.mock.web.server.MockServerWebExchange; +import org.springframework.web.server.ServerWebExchange; import com.clearfolio.viewer.api.ApiErrorResponse; @@ -33,6 +40,27 @@ void unsafeClientTraceIdFallsBackToServerRequestId() { assertEquals(exchange.getRequest().getId(), body.traceId()); } + @Test + void unsafeClientAndServerTraceIdsFallBackToGeneratedUuid() { + String unsafeClientTraceId = "../../tenant-secret"; + String unsafeRequestId = "../unsafe-server-request"; + ServerWebExchange exchange = mock(ServerWebExchange.class); + ServerHttpRequest request = mock(ServerHttpRequest.class); + HttpHeaders headers = new HttpHeaders(); + headers.add("X-Trace-Id", unsafeClientTraceId); + when(exchange.getRequest()).thenReturn(request); + when(request.getHeaders()).thenReturn(headers); + when(request.getId()).thenReturn(unsafeRequestId); + + ApiErrorResponse body = handler + .handleBadRequest(new IllegalArgumentException("bad request"), exchange) + .getBody(); + + assertNotEquals(unsafeClientTraceId, body.traceId()); + assertNotEquals(unsafeRequestId, body.traceId()); + assertEquals(UUID.fromString(body.traceId()).toString(), body.traceId()); + } + @Test void oversizedClientTraceIdFallsBackToServerRequestId() { String oversized = "a".repeat(129);