Merge commit from fork · a2aproject/a2a-java@8882710 · GitHub
Skip to content

Commit 8882710

Browse files
ehsavoiekabir
ehsavoie
and
kabir
authored
Merge commit from fork
- Modified RestHandler.java to log body length instead of content on parse errors - Modified RestHandler_v0_3.java to log body length instead of content on parse errors - Added security tests to verify sensitive data is not logged - Fixes vulnerability GHSA-x32g-jvvm-4725 This fixes #GHSA-x32g-jvvm-4725 Co-authored-by: Kabir Khan <kabirkhanuk@ibm.com>
1 parent c59bf76 commit 8882710

5 files changed

Lines changed: 111 additions & 2 deletions

File tree

‎compat-0.3/transport/rest/src/main/java/org/a2aproject/sdk/compat03/transport/rest/handler/RestHandler_v0_3.java‎

Lines changed: 2 additions & 1 deletion

‎compat-0.3/transport/rest/src/test/java/org/a2aproject/sdk/compat03/transport/rest/handler/RestHandler_v0_3_Test.java‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55

66
import org.a2aproject.sdk.compat03.conversion.A2AProtocol_v0_3;
77
import org.a2aproject.sdk.compat03.conversion.AbstractA2ARequestHandlerTest_v0_3;
8+
import org.a2aproject.sdk.server.requesthandlers.LogCaptureAssertions;
89
import org.a2aproject.sdk.compat03.conversion.Convert_v0_3_To10RequestHandler;
910
import org.a2aproject.sdk.compat03.conversion.mappers.domain.TaskMapper_v0_3;
1011
import org.a2aproject.sdk.compat03.spec.AgentCapabilities_v0_3;
@@ -374,4 +375,33 @@ public void testListPushNotificationConfigs() {
374375
assertEquals(200, response.getStatusCode());
375376
assertEquals("application/json", response.getContentType());
376377
}
378+
379+
@Test
380+
public void testPushNotificationConfigParseError_DoesNotLogSensitiveData() {
381+
RestHandler_v0_3 handler = new RestHandler_v0_3(CARD, internalExecutor, convert03To10Handler);
382+
taskStore.save(TaskMapper_v0_3.INSTANCE.toV10(MINIMAL_TASK), false);
383+
384+
String requestBody = """
385+
{
386+
"name": "tasks/%s/pushNotificationConfigs/",
387+
"pushNotificationConfig": {
388+
"url": "https://webhook.example.com",
389+
"token": "secret-token-12345",
390+
"authentication": {
391+
"scheme": "Bearer",
392+
"credentials": "oauth-secret-67890"
393+
},
394+
"unknownField": "trigger-parse-error"
395+
}
396+
}""".formatted(MINIMAL_TASK.id());
397+
398+
// Request body with sensitive data and an unknown field to trigger parse error
399+
java.util.logging.Logger logger = java.util.logging.Logger.getLogger(RestHandler_v0_3.class.getName());
400+
LogCaptureAssertions.assertSensitiveDataNotLogged(logger,
401+
() -> assertEquals(422,
402+
handler.setTaskPushNotificationConfiguration(MINIMAL_TASK.id(), requestBody, callContext)
403+
.getStatusCode()),
404+
"secret-token-12345", "oauth-secret-67890");
405+
}
406+
377407
}

‎server-common/src/test/java/org/a2aproject/sdk/server/requesthandlers/LogCaptureAssertions.java‎

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
package org.a2aproject.sdk.server.requesthandlers;
2+
3+
import java.util.ArrayList;
4+
import java.util.List;
5+
6+
import org.junit.jupiter.api.Assertions;
7+
8+
public class LogCaptureAssertions {
9+
10+
public static class TestLogHandler extends java.util.logging.Handler {
11+
private final List<String> logMessages = new ArrayList<>();
12+
13+
@Override
14+
public void publish(java.util.logging.LogRecord record) {
15+
logMessages.add(record.getMessage());
16+
}
17+
18+
@Override
19+
public void flush() {
20+
}
21+
22+
@Override
23+
public void close() throws SecurityException {
24+
}
25+
26+
public List<String> getLogMessages() {
27+
return logMessages;
28+
}
29+
}
30+
31+
public static void assertSensitiveDataNotLogged(
32+
java.util.logging.Logger logger, Runnable operation, String... sensitiveValues) {
33+
TestLogHandler testLogHandler = new TestLogHandler();
34+
logger.addHandler(testLogHandler);
35+
try {
36+
operation.run();
37+
List<String> logMessages = testLogHandler.getLogMessages();
38+
for (String sensitive : sensitiveValues) {
39+
Assertions.assertTrue(logMessages.stream().noneMatch(msg -> msg.contains(sensitive)),
40+
"Log should not contain sensitive value: " + sensitive);
41+
}
42+
Assertions.assertTrue(
43+
logMessages.stream().anyMatch(msg -> msg.contains("Error parsing JSON request body (length=")),
44+
"Log should contain body length message");
45+
} finally {
46+
logger.removeHandler(testLogHandler);
47+
}
48+
}
49+
}

‎transport/rest/src/main/java/org/a2aproject/sdk/transport/rest/handler/RestHandler.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -682,7 +682,8 @@ private void parseRequestBody(String body, com.google.protobuf.Message.Builder b
682682
validate(body);
683683
JsonFormat.parser().merge(body, builder);
684684
} catch (InvalidProtocolBufferException e) {
685-
log.log(Level.SEVERE, "Error parsing JSON request body: {0}", body);
685+
log.log(Level.SEVERE, "Error parsing JSON request body (length={0})",
686+
body != null ? body.length() : 0);
686687
log.log(Level.SEVERE, "Parse error details", e);
687688
throw new InvalidParamsError("Failed to parse request body: " + e.getMessage());
688689
}

‎transport/rest/src/test/java/org/a2aproject/sdk/transport/rest/handler/RestHandlerTest.java‎

Lines changed: 28 additions & 0 deletions

0 commit comments

Comments
 (0)