From 916812675b3aef524f2b1ca77df4a116b1485dd7 Mon Sep 17 00:00:00 2001 From: Tulgaa Date: Mon, 23 Mar 2026 13:48:02 +0800 Subject: [PATCH 1/2] fix: URL-encode userId and sessionId in VertexAiClient API URLs User-supplied userId and sessionId values were concatenated directly into Vertex AI REST API URL paths and query parameters without encoding. This allows query parameter injection via userId (e.g., "attacker&extra_param=value") and path manipulation via sessionId (e.g., "../../other-resource"). Apply URLEncoder.encode() to all user-supplied values before URL construction in listSessions, listEvents, getSession, deleteSession, and appendEvent. Bug: CWE-116 (Improper Encoding or Escaping of Output) --- .../google/adk/sessions/VertexAiClient.java | 25 +++++++++++++++---- 1 file changed, 20 insertions(+), 5 deletions(-) diff --git a/core/src/main/java/com/google/adk/sessions/VertexAiClient.java b/core/src/main/java/com/google/adk/sessions/VertexAiClient.java index 1168d1166..a776dcc93 100644 --- a/core/src/main/java/com/google/adk/sessions/VertexAiClient.java +++ b/core/src/main/java/com/google/adk/sessions/VertexAiClient.java @@ -14,6 +14,8 @@ import io.reactivex.rxjava3.core.Single; import java.io.IOException; import java.io.UncheckedIOException; +import java.net.URLEncoder; +import java.nio.charset.StandardCharsets; import java.util.HashMap; import java.util.List; import java.util.Map; @@ -103,10 +105,15 @@ private Completable pollOperation(String operationId, int attempt) { }); } + private static String encodeParam(String value) { + return URLEncoder.encode(value, StandardCharsets.UTF_8); + } + Maybe listSessions(String reasoningEngineId, String userId) { return performApiRequest( "GET", - "reasoningEngines/" + reasoningEngineId + "/sessions?filter=user_id=" + userId, + "reasoningEngines/" + reasoningEngineId + + "/sessions?filter=user_id=" + encodeParam(userId), "") .flatMapMaybe(VertexAiClient::getJsonResponse); } @@ -114,7 +121,8 @@ Maybe listSessions(String reasoningEngineId, String userId) { Maybe listEvents(String reasoningEngineId, String sessionId) { return performApiRequest( "GET", - "reasoningEngines/" + reasoningEngineId + "/sessions/" + sessionId + "/events", + "reasoningEngines/" + reasoningEngineId + + "/sessions/" + encodeParam(sessionId) + "/events", "") .doOnSuccess(apiResponse -> logger.debug("List events response {}", apiResponse)) .flatMapMaybe(VertexAiClient::getJsonResponse); @@ -122,13 +130,19 @@ Maybe listEvents(String reasoningEngineId, String sessionId) { Maybe getSession(String reasoningEngineId, String sessionId) { return performApiRequest( - "GET", "reasoningEngines/" + reasoningEngineId + "/sessions/" + sessionId, "") + "GET", + "reasoningEngines/" + reasoningEngineId + + "/sessions/" + encodeParam(sessionId), + "") .flatMapMaybe(apiResponse -> getJsonResponse(apiResponse)); } Completable deleteSession(String reasoningEngineId, String sessionId) { return performApiRequest( - "DELETE", "reasoningEngines/" + reasoningEngineId + "/sessions/" + sessionId, "") + "DELETE", + "reasoningEngines/" + reasoningEngineId + + "/sessions/" + encodeParam(sessionId), + "") .doOnSuccess(ApiResponse::close) .ignoreElement(); } @@ -136,7 +150,8 @@ Completable deleteSession(String reasoningEngineId, String sessionId) { Completable appendEvent(String reasoningEngineId, String sessionId, String eventJson) { return performApiRequest( "POST", - "reasoningEngines/" + reasoningEngineId + "/sessions/" + sessionId + ":appendEvent", + "reasoningEngines/" + reasoningEngineId + + "/sessions/" + encodeParam(sessionId) + ":appendEvent", eventJson) .flatMapCompletable( response -> { From 9611a18232166efbee3b657bc865be7164b4f203 Mon Sep 17 00:00:00 2001 From: Tulgaa Date: Mon, 23 Mar 2026 14:08:16 +0800 Subject: [PATCH 2/2] test: add VertexAiClientTest for URL encoding of user parameters 10 tests covering all 5 methods that construct URLs with user-supplied values. Verifies query parameter injection (& = characters), path traversal (../ sequences), and normal values pass through correctly. --- .../google/adk/sessions/VertexAiClient.java | 26 +- .../adk/sessions/VertexAiClientTest.java | 241 ++++++++++++++++++ 2 files changed, 257 insertions(+), 10 deletions(-) create mode 100644 core/src/test/java/com/google/adk/sessions/VertexAiClientTest.java diff --git a/core/src/main/java/com/google/adk/sessions/VertexAiClient.java b/core/src/main/java/com/google/adk/sessions/VertexAiClient.java index a776dcc93..16c2c871b 100644 --- a/core/src/main/java/com/google/adk/sessions/VertexAiClient.java +++ b/core/src/main/java/com/google/adk/sessions/VertexAiClient.java @@ -112,8 +112,10 @@ private static String encodeParam(String value) { Maybe listSessions(String reasoningEngineId, String userId) { return performApiRequest( "GET", - "reasoningEngines/" + reasoningEngineId - + "/sessions?filter=user_id=" + encodeParam(userId), + "reasoningEngines/" + + reasoningEngineId + + "/sessions?filter=user_id=" + + encodeParam(userId), "") .flatMapMaybe(VertexAiClient::getJsonResponse); } @@ -121,8 +123,11 @@ Maybe listSessions(String reasoningEngineId, String userId) { Maybe listEvents(String reasoningEngineId, String sessionId) { return performApiRequest( "GET", - "reasoningEngines/" + reasoningEngineId - + "/sessions/" + encodeParam(sessionId) + "/events", + "reasoningEngines/" + + reasoningEngineId + + "/sessions/" + + encodeParam(sessionId) + + "/events", "") .doOnSuccess(apiResponse -> logger.debug("List events response {}", apiResponse)) .flatMapMaybe(VertexAiClient::getJsonResponse); @@ -131,8 +136,7 @@ Maybe listEvents(String reasoningEngineId, String sessionId) { Maybe getSession(String reasoningEngineId, String sessionId) { return performApiRequest( "GET", - "reasoningEngines/" + reasoningEngineId - + "/sessions/" + encodeParam(sessionId), + "reasoningEngines/" + reasoningEngineId + "/sessions/" + encodeParam(sessionId), "") .flatMapMaybe(apiResponse -> getJsonResponse(apiResponse)); } @@ -140,8 +144,7 @@ Maybe getSession(String reasoningEngineId, String sessionId) { Completable deleteSession(String reasoningEngineId, String sessionId) { return performApiRequest( "DELETE", - "reasoningEngines/" + reasoningEngineId - + "/sessions/" + encodeParam(sessionId), + "reasoningEngines/" + reasoningEngineId + "/sessions/" + encodeParam(sessionId), "") .doOnSuccess(ApiResponse::close) .ignoreElement(); @@ -150,8 +153,11 @@ Completable deleteSession(String reasoningEngineId, String sessionId) { Completable appendEvent(String reasoningEngineId, String sessionId, String eventJson) { return performApiRequest( "POST", - "reasoningEngines/" + reasoningEngineId - + "/sessions/" + encodeParam(sessionId) + ":appendEvent", + "reasoningEngines/" + + reasoningEngineId + + "/sessions/" + + encodeParam(sessionId) + + ":appendEvent", eventJson) .flatMapCompletable( response -> { diff --git a/core/src/test/java/com/google/adk/sessions/VertexAiClientTest.java b/core/src/test/java/com/google/adk/sessions/VertexAiClientTest.java new file mode 100644 index 000000000..a4abf3e94 --- /dev/null +++ b/core/src/test/java/com/google/adk/sessions/VertexAiClientTest.java @@ -0,0 +1,241 @@ +package com.google.adk.sessions; + +import static com.google.common.truth.Truth.assertThat; +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +import okhttp3.MediaType; +import okhttp3.ResponseBody; +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.JUnit4; +import org.mockito.ArgumentCaptor; +import org.mockito.Mock; +import org.mockito.MockitoAnnotations; + +/** + * Unit tests for URL encoding in {@link VertexAiClient}. + * + *

Verifies that userId and sessionId values are properly URL-encoded before being concatenated + * into API request paths, preventing query parameter injection and path traversal attacks. + */ +@RunWith(JUnit4.class) +public class VertexAiClientTest { + + private static final MediaType JSON_MEDIA_TYPE = + MediaType.parse("application/json; charset=utf-8"); + + @Mock private HttpApiClient mockApiClient; + + private VertexAiClient client; + + @Before + public void setUp() { + MockitoAnnotations.openMocks(this); + client = new VertexAiClient("test-project", "test-location", mockApiClient); + } + + /** Returns a mock ApiResponse with the given JSON body. */ + private static ApiResponse responseWithBody(String body) { + return new ApiResponse() { + @Override + public ResponseBody getResponseBody() { + return ResponseBody.create(JSON_MEDIA_TYPE, body); + } + + @Override + public void close() {} + }; + } + + // --------------------------------------------------------------------------- + // listSessions: userId encoding + // --------------------------------------------------------------------------- + + @Test + public void listSessions_encodesUserIdWithQueryInjection() { + String maliciousUserId = "user&extra=value"; + when(mockApiClient.request(anyString(), anyString(), anyString())) + .thenReturn(responseWithBody("{\"sessions\": []}")); + + client.listSessions("123", maliciousUserId).blockingGet(); + + ArgumentCaptor pathCaptor = ArgumentCaptor.forClass(String.class); + verify(mockApiClient).request(anyString(), pathCaptor.capture(), anyString()); + + String path = pathCaptor.getValue(); + // The ampersand must be encoded as %26, not left raw + assertThat(path).contains("user%26extra%3Dvalue"); + assertThat(path).doesNotContain("&extra=value"); + } + + @Test + public void listSessions_encodesUserIdWithSpaces() { + String userIdWithSpaces = "user name with spaces"; + when(mockApiClient.request(anyString(), anyString(), anyString())) + .thenReturn(responseWithBody("{\"sessions\": []}")); + + client.listSessions("123", userIdWithSpaces).blockingGet(); + + ArgumentCaptor pathCaptor = ArgumentCaptor.forClass(String.class); + verify(mockApiClient).request(anyString(), pathCaptor.capture(), anyString()); + + String path = pathCaptor.getValue(); + // Spaces must be encoded (as + or %20) + assertThat(path).doesNotContain(" name "); + assertThat(path).contains("filter=user_id=user"); + } + + @Test + public void listSessions_normalUserIdPassesThroughCorrectly() { + String normalUserId = "user123"; + when(mockApiClient.request(anyString(), anyString(), anyString())) + .thenReturn(responseWithBody("{\"sessions\": []}")); + + client.listSessions("456", normalUserId).blockingGet(); + + ArgumentCaptor pathCaptor = ArgumentCaptor.forClass(String.class); + verify(mockApiClient).request(anyString(), pathCaptor.capture(), anyString()); + + String path = pathCaptor.getValue(); + assertThat(path).isEqualTo("reasoningEngines/456/sessions?filter=user_id=user123"); + } + + // --------------------------------------------------------------------------- + // getSession: sessionId encoding + // --------------------------------------------------------------------------- + + @Test + public void getSession_encodesSessionIdWithPathTraversal() { + String maliciousSessionId = "../../secret"; + when(mockApiClient.request(anyString(), anyString(), anyString())) + .thenReturn( + responseWithBody( + "{\"name\": \"sessions/safe\", \"updateTime\": \"2024-12-12T12:12:12.123456Z\"}")); + + client.getSession("123", maliciousSessionId).blockingGet(); + + ArgumentCaptor pathCaptor = ArgumentCaptor.forClass(String.class); + verify(mockApiClient).request(anyString(), pathCaptor.capture(), anyString()); + + String path = pathCaptor.getValue(); + // Path traversal characters must be encoded + assertThat(path).doesNotContain("../../"); + assertThat(path).contains("..%2F..%2Fsecret"); + } + + @Test + public void getSession_encodesSessionIdWithSlashes() { + String sessionIdWithSlashes = "session/with/slashes"; + when(mockApiClient.request(anyString(), anyString(), anyString())) + .thenReturn( + responseWithBody( + "{\"name\": \"sessions/safe\", \"updateTime\": \"2024-12-12T12:12:12.123456Z\"}")); + + client.getSession("123", sessionIdWithSlashes).blockingGet(); + + ArgumentCaptor pathCaptor = ArgumentCaptor.forClass(String.class); + verify(mockApiClient).request(anyString(), pathCaptor.capture(), anyString()); + + String path = pathCaptor.getValue(); + // Slashes in sessionId must be encoded as %2F + assertThat(path).contains("session%2Fwith%2Fslashes"); + } + + @Test + public void getSession_normalSessionIdPassesThroughCorrectly() { + String normalSessionId = "abc123"; + when(mockApiClient.request(anyString(), anyString(), anyString())) + .thenReturn( + responseWithBody( + "{\"name\": \"sessions/abc123\", \"updateTime\": \"2024-12-12T12:12:12.123456Z\"}")); + + client.getSession("456", normalSessionId).blockingGet(); + + ArgumentCaptor pathCaptor = ArgumentCaptor.forClass(String.class); + verify(mockApiClient).request(anyString(), pathCaptor.capture(), anyString()); + + String path = pathCaptor.getValue(); + assertThat(path).isEqualTo("reasoningEngines/456/sessions/abc123"); + } + + // --------------------------------------------------------------------------- + // deleteSession: sessionId encoding + // --------------------------------------------------------------------------- + + @Test + public void deleteSession_encodesSessionIdWithSpecialCharacters() { + String maliciousSessionId = "session&admin=true"; + when(mockApiClient.request(anyString(), anyString(), anyString())) + .thenReturn(responseWithBody("")); + + client.deleteSession("123", maliciousSessionId).blockingAwait(); + + ArgumentCaptor pathCaptor = ArgumentCaptor.forClass(String.class); + verify(mockApiClient).request(anyString(), pathCaptor.capture(), anyString()); + + String path = pathCaptor.getValue(); + assertThat(path).doesNotContain("&admin=true"); + assertThat(path).contains("session%26admin%3Dtrue"); + } + + // --------------------------------------------------------------------------- + // listEvents: sessionId encoding + // --------------------------------------------------------------------------- + + @Test + public void listEvents_encodesSessionIdWithPathTraversal() { + String maliciousSessionId = "../other-engine/sessions/target/events"; + when(mockApiClient.request(anyString(), anyString(), anyString())) + .thenReturn(responseWithBody("{\"sessionEvents\": []}")); + + client.listEvents("123", maliciousSessionId).blockingGet(); + + ArgumentCaptor pathCaptor = ArgumentCaptor.forClass(String.class); + verify(mockApiClient).request(anyString(), pathCaptor.capture(), anyString()); + + String path = pathCaptor.getValue(); + // The slashes and dots must be encoded, not treated as path separators + assertThat(path).doesNotContain("../other-engine"); + assertThat(path).startsWith("reasoningEngines/123/sessions/"); + assertThat(path).endsWith("/events"); + } + + // --------------------------------------------------------------------------- + // appendEvent: sessionId encoding + // --------------------------------------------------------------------------- + + @Test + public void appendEvent_encodesSessionIdWithSpecialCharacters() { + String maliciousSessionId = "sess%00ion"; + when(mockApiClient.request(anyString(), anyString(), anyString())) + .thenReturn(responseWithBody("{}")); + + client.appendEvent("123", maliciousSessionId, "{}").blockingAwait(); + + ArgumentCaptor pathCaptor = ArgumentCaptor.forClass(String.class); + verify(mockApiClient).request(anyString(), pathCaptor.capture(), anyString()); + + String path = pathCaptor.getValue(); + // The % must itself be encoded as %25 + assertThat(path).contains("sess%2500ion"); + assertThat(path).endsWith(":appendEvent"); + } + + @Test + public void appendEvent_normalSessionIdPassesThroughCorrectly() { + String normalSessionId = "session42"; + when(mockApiClient.request(anyString(), anyString(), anyString())) + .thenReturn(responseWithBody("{}")); + + client.appendEvent("789", normalSessionId, "{}").blockingAwait(); + + ArgumentCaptor pathCaptor = ArgumentCaptor.forClass(String.class); + verify(mockApiClient).request(anyString(), pathCaptor.capture(), anyString()); + + String path = pathCaptor.getValue(); + assertThat(path).isEqualTo("reasoningEngines/789/sessions/session42:appendEvent"); + } +}