From 4c3dd2ae6fb07a05f6cca71ddae72f766d33c46c Mon Sep 17 00:00:00 2001 From: meraklbz Date: Mon, 10 Aug 2026 20:13:50 +0800 Subject: [PATCH] fix: reject CR/LF in push notification Authorization header --- .../tasks/BasePushNotificationSender.java | 27 ++++++++- .../tasks/PushNotificationSenderTest.java | 60 +++++++++++++++++++ 2 files changed, 85 insertions(+), 2 deletions(-) diff --git a/server-common/src/main/java/org/a2aproject/sdk/server/tasks/BasePushNotificationSender.java b/server-common/src/main/java/org/a2aproject/sdk/server/tasks/BasePushNotificationSender.java index b0f6e38d8..07375f185 100644 --- a/server-common/src/main/java/org/a2aproject/sdk/server/tasks/BasePushNotificationSender.java +++ b/server-common/src/main/java/org/a2aproject/sdk/server/tasks/BasePushNotificationSender.java @@ -19,6 +19,7 @@ import org.a2aproject.sdk.client.http.A2AHttpClient; import org.a2aproject.sdk.client.http.A2AHttpClientFactory; import org.a2aproject.sdk.jsonrpc.common.json.JsonUtil; +import org.a2aproject.sdk.spec.AuthenticationInfo; import org.a2aproject.sdk.spec.ListTaskPushNotificationConfigsParams; import org.a2aproject.sdk.spec.ListTaskPushNotificationConfigsResult; import org.a2aproject.sdk.spec.Message; @@ -197,8 +198,7 @@ private boolean dispatchNotification(StreamingEventKind event, postBuilder.addHeader(X_A2A_NOTIFICATION_TOKEN, token); } if (pushInfo.authentication() != null && pushInfo.authentication().credentials() != null) { - postBuilder.addHeader("Authorization", - pushInfo.authentication().scheme() + " " + pushInfo.authentication().credentials()); + postBuilder.addHeader("Authorization", buildAuthorizationHeader(pushInfo.authentication())); } try { @@ -213,4 +213,27 @@ private boolean dispatchNotification(StreamingEventKind event, } return true; } + + /** + * Builds the Authorization header value for a push notification config. + * + *

The {@code scheme} and {@code credentials} are client-controlled values that are + * concatenated directly into the header. Rejecting CR/LF characters here prevents + * HTTP header injection (CWE-113). The {@link A2AHttpClient} SPI is pluggable, so we + * cannot rely on every implementation (or the underlying HTTP client) to validate + * header values.

+ * + * @param authentication the push notification authentication info (scheme non-null, + * credentials non-null per the caller's check) + * @return the assembled {@code "scheme credentials"} header value + * @throws IllegalArgumentException if the assembled value contains CR or LF + */ + private static String buildAuthorizationHeader(AuthenticationInfo authentication) { + String value = authentication.scheme() + " " + authentication.credentials(); + if (value.indexOf('\r') >= 0 || value.indexOf('\n') >= 0) { + throw new IllegalArgumentException( + "Push notification Authorization header must not contain CR/LF characters"); + } + return value; + } } diff --git a/server-common/src/test/java/org/a2aproject/sdk/server/tasks/PushNotificationSenderTest.java b/server-common/src/test/java/org/a2aproject/sdk/server/tasks/PushNotificationSenderTest.java index 87226524d..dfa4389d4 100644 --- a/server-common/src/test/java/org/a2aproject/sdk/server/tasks/PushNotificationSenderTest.java +++ b/server-common/src/test/java/org/a2aproject/sdk/server/tasks/PushNotificationSenderTest.java @@ -516,4 +516,64 @@ public void testSendNotificationSkipsWhenFormatterReturnsNull() throws Interrupt assertTrue(testHttpClient.rawBodies.isEmpty()); } + + @Test + public void testSendNotificationWithAuthHeader() throws InterruptedException { + String taskId = "task_send_auth"; + Task taskData = createSampleTask(taskId, TaskState.TASK_STATE_COMPLETED); + TaskPushNotificationConfig config = TaskPushNotificationConfig.builder() + .url("http://notify.me/here") + .id("cfg-auth") + .taskId(taskId) + .authentication(new org.a2aproject.sdk.spec.AuthenticationInfo("Bearer", "token123")) + .build(); + configStore.setInfo(config); + + testHttpClient.latch = new CountDownLatch(1); + sender.sendNotification(taskData, null); + + assertTrue(testHttpClient.latch.await(5, TimeUnit.SECONDS), "HTTP call should complete within 5 seconds"); + assertEquals(1, testHttpClient.headers.size()); + Map sentHeaders = testHttpClient.headers.get(0); + assertEquals("Bearer token123", sentHeaders.get("Authorization")); + } + + @Test + public void testSendNotificationRejectsCrlfInCredentials() throws InterruptedException { + String taskId = "task_send_crlf"; + Task taskData = createSampleTask(taskId, TaskState.TASK_STATE_COMPLETED); + // CRLF in client-controlled credentials must not be injected into the Authorization + // header (BUG-34). The notification is dropped instead of sending an injected header. + TaskPushNotificationConfig config = TaskPushNotificationConfig.builder() + .url("http://notify.me/here") + .id("cfg-crlf") + .taskId(taskId) + .authentication(new org.a2aproject.sdk.spec.AuthenticationInfo("Bearer", "token\r\nX-Injected: 1")) + .build(); + configStore.setInfo(config); + + sender.sendNotification(taskData, null); + + // The header is never constructed with the injected value - no HTTP call is made + assertTrue(testHttpClient.events.isEmpty(), "Notification with CRLF credentials must not be dispatched"); + assertTrue(testHttpClient.headers.isEmpty(), "No headers should have been sent"); + } + + @Test + public void testSendNotificationRejectsCrlfInScheme() throws InterruptedException { + String taskId = "task_send_crlf_scheme"; + Task taskData = createSampleTask(taskId, TaskState.TASK_STATE_COMPLETED); + TaskPushNotificationConfig config = TaskPushNotificationConfig.builder() + .url("http://notify.me/here") + .id("cfg-crlf-scheme") + .taskId(taskId) + .authentication(new org.a2aproject.sdk.spec.AuthenticationInfo("Bearer\nX-Injected: 1", "token")) + .build(); + configStore.setInfo(config); + + sender.sendNotification(taskData, null); + + assertTrue(testHttpClient.events.isEmpty(), "Notification with CRLF scheme must not be dispatched"); + assertTrue(testHttpClient.headers.isEmpty(), "No headers should have been sent"); + } }