Skip to content
Draft
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -223,18 +225,22 @@ public ResponseEntity<ApiErrorResponse> 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);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,96 @@
package com.clearfolio.viewer.controller;

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;

/**
* 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 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);
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());
}
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Loading