From fb6e7bcc4740eb38d2441a0f40357380733880ef Mon Sep 17 00:00:00 2001 From: andy Date: Sun, 28 Jun 2026 12:02:17 +0800 Subject: [PATCH 1/3] fix(security): sanitize REST path parameters to prevent injection attacks (#805) Add sanitize_path_id() validation for task IDs and push notification config IDs used in REST URL paths. Rejects null bytes, control characters, path traversal sequences, and other unsafe inputs. Server-side: validate all path_params in RestDispatcher before passing to request handlers. Client-side: validate request.id and request.task_id before constructing URL paths in RestTransport. Closes #805 --- src/a2a/client/transports/rest.py | 15 +-- src/a2a/server/routes/rest_dispatcher.py | 23 +++-- src/a2a/utils/__init__.py | 2 + src/a2a/utils/sanitizers.py | 56 +++++++++++ tests/server/routes/test_rest_dispatcher.py | 88 +++++++++++++++++ tests/utils/test_sanitizers.py | 101 ++++++++++++++++++++ 6 files changed, 269 insertions(+), 16 deletions(-) create mode 100644 src/a2a/utils/sanitizers.py create mode 100644 tests/utils/test_sanitizers.py diff --git a/src/a2a/client/transports/rest.py b/src/a2a/client/transports/rest.py index 3dfe95927..bc160169b 100644 --- a/src/a2a/client/transports/rest.py +++ b/src/a2a/client/transports/rest.py @@ -35,6 +35,7 @@ TaskPushNotificationConfig, ) from a2a.utils.errors import A2A_REASON_TO_ERROR, MethodNotFoundError +from a2a.utils.sanitizers import sanitize_path_id from a2a.utils.telemetry import SpanKind, trace_class @@ -149,7 +150,7 @@ async def get_task( response_data = await self._execute_request( 'GET', - f'/tasks/{request.id}', + f'/tasks/{sanitize_path_id(request.id)}', request.tenant, context=context, params=params, @@ -189,7 +190,7 @@ async def cancel_task( """Requests the agent to cancel a specific task.""" response_data = await self._execute_request( 'POST', - f'/tasks/{request.id}:cancel', + f'/tasks/{sanitize_path_id(request.id)}:cancel', request.tenant, context=context, json=MessageToDict(request), @@ -206,7 +207,7 @@ async def create_task_push_notification_config( """Sets or updates the push notification configuration for a specific task.""" response_data = await self._execute_request( 'POST', - f'/tasks/{request.task_id}/pushNotificationConfigs', + f'/tasks/{sanitize_path_id(request.task_id)}/pushNotificationConfigs', request.tenant, context=context, json=MessageToDict(request), @@ -233,7 +234,7 @@ async def get_task_push_notification_config( response_data = await self._execute_request( 'GET', - f'/tasks/{request.task_id}/pushNotificationConfigs/{request.id}', + f'/tasks/{sanitize_path_id(request.task_id)}/pushNotificationConfigs/{sanitize_path_id(request.id, "push_id")}', request.tenant, context=context, params=params, @@ -258,7 +259,7 @@ async def list_task_push_notification_configs( response_data = await self._execute_request( 'GET', - f'/tasks/{request.task_id}/pushNotificationConfigs', + f'/tasks/{sanitize_path_id(request.task_id)}/pushNotificationConfigs', request.tenant, context=context, params=params, @@ -285,7 +286,7 @@ async def delete_task_push_notification_config( await self._execute_request( 'DELETE', - f'/tasks/{request.task_id}/pushNotificationConfigs/{request.id}', + f'/tasks/{sanitize_path_id(request.task_id)}/pushNotificationConfigs/{sanitize_path_id(request.id, "push_id")}', request.tenant, context=context, params=params, @@ -300,7 +301,7 @@ async def subscribe( """Reconnects to get task updates.""" async for event in self._send_stream_request( 'POST', - f'/tasks/{request.id}:subscribe', + f'/tasks/{sanitize_path_id(request.id)}:subscribe', request.tenant, context=context, ): diff --git a/src/a2a/server/routes/rest_dispatcher.py b/src/a2a/server/routes/rest_dispatcher.py index 06c73c592..e6b4153b6 100644 --- a/src/a2a/server/routes/rest_dispatcher.py +++ b/src/a2a/server/routes/rest_dispatcher.py @@ -27,6 +27,7 @@ InvalidRequestError, TaskNotFoundError, ) +from a2a.utils.sanitizers import sanitize_path_id from a2a.utils.telemetry import SpanKind, trace_class from a2a.utils.version_validator import validate_version @@ -200,7 +201,7 @@ async def on_cancel_task(self, request: Request) -> Response: @validate_version(constants.PROTOCOL_VERSION_1_0) async def _handler(context: ServerCallContext) -> a2a_pb2.Task: - task_id = request.path_params['id'] + task_id = sanitize_path_id(request.path_params['id']) task = await self.request_handler.on_cancel_task( CancelTaskRequest(id=task_id), context ) @@ -216,7 +217,7 @@ async def on_subscribe_to_task( self, request: Request ) -> EventSourceResponse: """Handles the 'SubscribeToTask' REST method.""" - task_id = request.path_params['id'] + task_id = sanitize_path_id(request.path_params['id']) @validate_version(constants.PROTOCOL_VERSION_1_0) async def _handler( @@ -238,7 +239,7 @@ async def on_get_task(self, request: Request) -> Response: async def _handler(context: ServerCallContext) -> a2a_pb2.Task: params = a2a_pb2.GetTaskRequest() proto_utils.parse_params(request.query_params, params) - params.id = request.path_params['id'] + params.id = sanitize_path_id(request.path_params['id']) task = await self.request_handler.on_get_task(params, context) if task: return task @@ -255,8 +256,10 @@ async def get_push_notification(self, request: Request) -> Response: async def _handler( context: ServerCallContext, ) -> a2a_pb2.TaskPushNotificationConfig: - task_id = request.path_params['id'] - push_id = request.path_params['push_id'] + task_id = sanitize_path_id(request.path_params['id']) + push_id = sanitize_path_id( + request.path_params['push_id'], 'push_id' + ) params = GetTaskPushNotificationConfigRequest( task_id=task_id, id=push_id ) @@ -275,8 +278,10 @@ async def delete_push_notification(self, request: Request) -> Response: @validate_version(constants.PROTOCOL_VERSION_1_0) async def _handler(context: ServerCallContext) -> None: - task_id = request.path_params['id'] - push_id = request.path_params['push_id'] + task_id = sanitize_path_id(request.path_params['id']) + push_id = sanitize_path_id( + request.path_params['push_id'], 'push_id' + ) params = a2a_pb2.DeleteTaskPushNotificationConfigRequest( task_id=task_id, id=push_id ) @@ -298,7 +303,7 @@ async def _handler( body = await request.body() params = a2a_pb2.TaskPushNotificationConfig() Parse(body, params) - params.task_id = request.path_params['id'] + params.task_id = sanitize_path_id(request.path_params['id']) return await self.request_handler.on_create_task_push_notification_config( params, context ) @@ -316,7 +321,7 @@ async def _handler( ) -> a2a_pb2.ListTaskPushNotificationConfigsResponse: params = a2a_pb2.ListTaskPushNotificationConfigsRequest() proto_utils.parse_params(request.query_params, params) - params.task_id = request.path_params['id'] + params.task_id = sanitize_path_id(request.path_params['id']) return await self.request_handler.on_list_task_push_notification_configs( params, context ) diff --git a/src/a2a/utils/__init__.py b/src/a2a/utils/__init__.py index 04693dd0b..14fb5f74a 100644 --- a/src/a2a/utils/__init__.py +++ b/src/a2a/utils/__init__.py @@ -7,6 +7,7 @@ TransportProtocol, ) from a2a.utils.proto_utils import to_stream_response +from a2a.utils.sanitizers import sanitize_path_id __all__ = [ @@ -14,5 +15,6 @@ 'DEFAULT_RPC_URL', 'TransportProtocol', 'proto_utils', + 'sanitize_path_id', 'to_stream_response', ] diff --git a/src/a2a/utils/sanitizers.py b/src/a2a/utils/sanitizers.py new file mode 100644 index 000000000..cdb9f5c34 --- /dev/null +++ b/src/a2a/utils/sanitizers.py @@ -0,0 +1,56 @@ +"""Input sanitization utilities for A2A path parameters. + +This module provides validation functions for resource IDs used in +REST URL paths, preventing injection of control characters, path +traversal sequences, and other unsafe inputs. +""" + +import re + +from a2a.utils.errors import InvalidRequestError + + +# Allowed characters for path resource IDs: alphanumeric, hyphen, +# underscore, and period. This matches the character set typically +# used for UUIDs, task IDs, and push-notification config IDs. +_PATH_ID_PATTERN = re.compile(r'^[A-Za-z0-9._-]+$') + +# ASCII control character boundaries. +_MAX_PRINTABLE_ASCII = 0x20 # First non-control character (space) +_DEL_ASCII = 0x7F # DEL control character + + +def sanitize_path_id(value: str, param_name: str = 'id') -> str: + """Validate and sanitize a path parameter used as a resource ID. + + Rejects values containing null bytes, newlines, other control + characters, or any characters outside the safe set + ``[A-Za-z0-9._-]``. + + Args: + value: The raw path parameter value. + param_name: Name of the parameter (for error messages). + + Returns: + The validated value unchanged. + + Raises: + InvalidRequestError: If the value contains disallowed characters + or is empty. + """ + if not value: + raise InvalidRequestError( + message=f'{param_name} must not be empty', + ) + # Reject null bytes and other control characters (0x00-0x1F, 0x7F). + if any( + ord(c) < _MAX_PRINTABLE_ASCII or ord(c) == _DEL_ASCII for c in value + ): + raise InvalidRequestError( + message=f'{param_name} contains control characters', + ) + if not _PATH_ID_PATTERN.match(value): + raise InvalidRequestError( + message=f'{param_name} contains invalid characters: {value!r}', + ) + return value diff --git a/tests/server/routes/test_rest_dispatcher.py b/tests/server/routes/test_rest_dispatcher.py index 00cc45f7b..efb429e05 100644 --- a/tests/server/routes/test_rest_dispatcher.py +++ b/tests/server/routes/test_rest_dispatcher.py @@ -323,3 +323,91 @@ async def stream_with_non_ascii( payload = payload.decode('utf-8') assert non_ascii_text in payload assert '\\u4f60\\u597d' not in payload + + +@pytest.mark.asyncio +class TestPathIdSanitization: + """Regression tests for https://github.com/a2aproject/a2a-python/issues/805. + + Verify that malicious path parameters (null bytes, control characters, + path traversal) are rejected at the REST dispatcher layer. + """ + + @pytest.fixture + def dispatcher(self, mock_handler: AsyncMock) -> RestDispatcher: + return RestDispatcher(request_handler=mock_handler) + + async def test_get_task_rejects_null_byte_id( + self, dispatcher: RestDispatcher + ) -> None: + """Null byte in task ID must be rejected.""" + req = make_mock_request(method='GET', path_params={'id': 'abc\x00def'}) + response = await dispatcher.on_get_task(req) + assert response.status_code == 400 + + async def test_get_task_rejects_newline_id( + self, dispatcher: RestDispatcher + ) -> None: + """Newline in task ID must be rejected.""" + req = make_mock_request(method='GET', path_params={'id': 'abc\ndef'}) + response = await dispatcher.on_get_task(req) + assert response.status_code == 400 + + async def test_get_task_rejects_path_traversal_id( + self, dispatcher: RestDispatcher + ) -> None: + """Path traversal in task ID must be rejected.""" + req = make_mock_request( + method='GET', path_params={'id': '../../etc/passwd'} + ) + response = await dispatcher.on_get_task(req) + assert response.status_code == 400 + + async def test_get_task_rejects_space_id( + self, dispatcher: RestDispatcher + ) -> None: + """Space in task ID must be rejected.""" + req = make_mock_request(method='GET', path_params={'id': 'abc def'}) + response = await dispatcher.on_get_task(req) + assert response.status_code == 400 + + async def test_cancel_task_rejects_null_byte_id( + self, dispatcher: RestDispatcher + ) -> None: + """Null byte in task ID on cancel must be rejected.""" + req = make_mock_request(method='POST', path_params={'id': 'abc\x00def'}) + response = await dispatcher.on_cancel_task(req) + assert response.status_code == 400 + + async def test_get_push_notification_rejects_malicious_ids( + self, dispatcher: RestDispatcher + ) -> None: + """Malicious task_id and push_id must be rejected.""" + req = make_mock_request( + method='GET', + path_params={'id': 'abc\x00def', 'push_id': 'valid-id'}, + ) + response = await dispatcher.get_push_notification(req) + assert response.status_code == 400 + + async def test_get_push_notification_rejects_malicious_push_id( + self, dispatcher: RestDispatcher + ) -> None: + """Malicious push_id must be rejected even with valid task_id.""" + req = make_mock_request( + method='GET', + path_params={'id': 'valid-task-id', 'push_id': '../evil'}, + ) + response = await dispatcher.get_push_notification(req) + assert response.status_code == 400 + + async def test_valid_id_passes( + self, dispatcher: RestDispatcher, mock_handler: AsyncMock + ) -> None: + """Valid task ID passes sanitization and reaches the handler.""" + req = make_mock_request( + method='GET', path_params={'id': 'valid-task-123'} + ) + response = await dispatcher.on_get_task(req) + # Handler returns a task, so status should be 200 + assert response.status_code == 200 diff --git a/tests/utils/test_sanitizers.py b/tests/utils/test_sanitizers.py new file mode 100644 index 000000000..50823cb87 --- /dev/null +++ b/tests/utils/test_sanitizers.py @@ -0,0 +1,101 @@ +"""Tests for path parameter sanitization utilities.""" + +import pytest + +from a2a.utils.errors import InvalidRequestError +from a2a.utils.sanitizers import sanitize_path_id + + +class TestSanitizePathId: + """Tests for sanitize_path_id validation.""" + + def test_valid_alphanumeric(self) -> None: + """Alphanumeric IDs pass validation unchanged.""" + assert sanitize_path_id('abc123') == 'abc123' + + def test_valid_uuid(self) -> None: + """UUID-style IDs pass validation.""" + uid = '550e8400-e29b-41d4-a716-446655440000' + assert sanitize_path_id(uid) == uid + + def test_valid_with_underscores(self) -> None: + """IDs with underscores pass validation.""" + assert sanitize_path_id('task_123') == 'task_123' + + def test_valid_with_dots(self) -> None: + """IDs with dots pass validation.""" + assert sanitize_path_id('v1.0.3') == 'v1.0.3' + + def test_valid_with_hyphens(self) -> None: + """IDs with hyphens pass validation.""" + assert sanitize_path_id('my-task-id') == 'my-task-id' + + def test_empty_string_rejected(self) -> None: + """Empty string raises InvalidRequestError.""" + with pytest.raises(InvalidRequestError, match='must not be empty'): + sanitize_path_id('') + + def test_null_byte_rejected(self) -> None: + """Null byte raises InvalidRequestError.""" + with pytest.raises(InvalidRequestError, match='control characters'): + sanitize_path_id('abc\x00def') + + def test_newline_rejected(self) -> None: + """Newline raises InvalidRequestError.""" + with pytest.raises(InvalidRequestError, match='control characters'): + sanitize_path_id('abc\ndef') + + def test_carriage_return_rejected(self) -> None: + """Carriage return raises InvalidRequestError.""" + with pytest.raises(InvalidRequestError, match='control characters'): + sanitize_path_id('abc\rdef') + + def test_tab_rejected(self) -> None: + """Tab raises InvalidRequestError.""" + with pytest.raises(InvalidRequestError, match='control characters'): + sanitize_path_id('abc\tdef') + + def test_del_control_char_rejected(self) -> None: + """DEL (0x7F) raises InvalidRequestError.""" + with pytest.raises(InvalidRequestError, match='control characters'): + sanitize_path_id('abc\x7f') + + def test_space_rejected(self) -> None: + """Space raises InvalidRequestError (not in allowed set).""" + with pytest.raises(InvalidRequestError, match='invalid characters'): + sanitize_path_id('abc def') + + def test_slash_rejected(self) -> None: + """Forward slash raises InvalidRequestError.""" + with pytest.raises(InvalidRequestError, match='invalid characters'): + sanitize_path_id('abc/def') + + def test_backslash_rejected(self) -> None: + """Backslash raises InvalidRequestError.""" + with pytest.raises(InvalidRequestError, match='invalid characters'): + sanitize_path_id('abc\\def') + + def test_path_traversal_rejected(self) -> None: + """Path traversal sequence raises InvalidRequestError.""" + with pytest.raises(InvalidRequestError, match='invalid characters'): + sanitize_path_id('../../etc/passwd') + + def test_custom_param_name_in_error(self) -> None: + """Custom param_name appears in error messages.""" + with pytest.raises(InvalidRequestError, match='push_id'): + sanitize_path_id('', 'push_id') + + def test_unicode_rejected(self) -> None: + """Non-ASCII unicode characters raise InvalidRequestError.""" + with pytest.raises(InvalidRequestError, match='invalid characters'): + sanitize_path_id('task-你好') + + def test_question_mark_rejected(self) -> None: + """Question mark raises InvalidRequestError.""" + with pytest.raises(InvalidRequestError, match='invalid characters'): + sanitize_path_id('abc?def') + + def test_hash_rejected(self) -> None: + """Hash character raises InvalidRequestError.""" + with pytest.raises(InvalidRequestError, match='invalid characters'): + sanitize_path_id('abc#def') From 9dedabd5c58cd9b2c7c029cf347a0c7738947558 Mon Sep 17 00:00:00 2001 From: andy Date: Sun, 28 Jun 2026 13:12:29 +0800 Subject: [PATCH 2/3] fix(security): reject '.' and '..' as path IDs to prevent path traversal Addresses gemini-code-assist review on PR #1115: - Add explicit check for bare '.' and '..' in sanitize_path_id() - Add unit tests for dot and double-dot rejection --- src/a2a/utils/sanitizers.py | 5 +++++ tests/utils/test_sanitizers.py | 10 ++++++++++ 2 files changed, 15 insertions(+) diff --git a/src/a2a/utils/sanitizers.py b/src/a2a/utils/sanitizers.py index cdb9f5c34..a32db87c2 100644 --- a/src/a2a/utils/sanitizers.py +++ b/src/a2a/utils/sanitizers.py @@ -42,6 +42,11 @@ def sanitize_path_id(value: str, param_name: str = 'id') -> str: raise InvalidRequestError( message=f'{param_name} must not be empty', ) + # Reject bare dot and double-dot to prevent path traversal. + if value in ('.', '..'): + raise InvalidRequestError( + message=f'{param_name} cannot be "." or ".."', + ) # Reject null bytes and other control characters (0x00-0x1F, 0x7F). if any( ord(c) < _MAX_PRINTABLE_ASCII or ord(c) == _DEL_ASCII for c in value diff --git a/tests/utils/test_sanitizers.py b/tests/utils/test_sanitizers.py index 50823cb87..49d520036 100644 --- a/tests/utils/test_sanitizers.py +++ b/tests/utils/test_sanitizers.py @@ -75,6 +75,16 @@ def test_backslash_rejected(self) -> None: with pytest.raises(InvalidRequestError, match='invalid characters'): sanitize_path_id('abc\\def') + def test_dot_rejected(self) -> None: + """Single dot is rejected as a path traversal risk.""" + with pytest.raises(InvalidRequestError, match='cannot be "." or ".."'): + sanitize_path_id('.') + + def test_double_dot_rejected(self) -> None: + """Double dot is rejected as a path traversal risk.""" + with pytest.raises(InvalidRequestError, match='cannot be "." or ".."'): + sanitize_path_id('..') + def test_path_traversal_rejected(self) -> None: """Path traversal sequence raises InvalidRequestError.""" with pytest.raises(InvalidRequestError, match='invalid characters'): From c2988fa61e581aaf508f2f73a14c6b63088ad91b Mon Sep 17 00:00:00 2001 From: andy Date: Mon, 29 Jun 2026 13:04:13 +0800 Subject: [PATCH 3/3] fix(ci): add 'sanitizers' to spelling allow list --- .github/actions/spelling/allow.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/actions/spelling/allow.txt b/.github/actions/spelling/allow.txt index 1701f14ef..ce7ceffe1 100644 --- a/.github/actions/spelling/allow.txt +++ b/.github/actions/spelling/allow.txt @@ -131,6 +131,7 @@ RUF SECP256R1 SFIXED SLF +sanitizers socio sse starlette