diff --git a/src/a2a/server/request_handlers/grpc_handler.py b/src/a2a/server/request_handlers/grpc_handler.py index d4d8ed669..66877b9fd 100644 --- a/src/a2a/server/request_handlers/grpc_handler.py +++ b/src/a2a/server/request_handlers/grpc_handler.py @@ -136,6 +136,9 @@ async def _handle_unary( result = await handler_func(server_context) except A2AError as e: await self.abort_context(e, context) + except Exception: + logger.exception('Unhandled exception in gRPC handler') + await self.abort_context(types.InternalError(), context) else: return result return default_response @@ -153,6 +156,9 @@ async def _handle_stream( yield item except A2AError as e: await self.abort_context(e, context) + except Exception: + logger.exception('Unhandled exception in gRPC handler') + await self.abort_context(types.InternalError(), context) async def SendMessage( self, @@ -413,9 +419,13 @@ async def abort_context( context.set_trailing_metadata(tuple(new_metadata)) await context.abort(rich_status.code, rich_status.details) else: + logger.error( + 'Unknown error type during request handling', + exc_info=error, + ) await context.abort( grpc.StatusCode.UNKNOWN, - f'Unknown error type: {error}', + 'Unknown error', ) def _build_call_context( diff --git a/src/a2a/server/routes/jsonrpc_dispatcher.py b/src/a2a/server/routes/jsonrpc_dispatcher.py index b59ed7551..21138da3b 100644 --- a/src/a2a/server/routes/jsonrpc_dispatcher.py +++ b/src/a2a/server/routes/jsonrpc_dispatcher.py @@ -177,7 +177,9 @@ def _generate_error_response( A `JSONResponse` object formatted as a JSON-RPC error response. """ if not isinstance(error, A2AError | JSONRPCError): - error = InternalError(message=str(error)) + # Never leak internal exception details to the client; the + # original error was logged by the caller. + error = InternalError() response_data = build_error_response(request_id, error) error_info = response_data.get('error', {}) @@ -251,11 +253,11 @@ async def handle_requests(self, request: Request) -> Response: # noqa: PLR0911, message="Invalid request: 'jsonrpc' must be exactly '2.0'" ), ) - except Exception as e: + except Exception: logger.exception('Failed to validate base JSON-RPC request') return self._generate_error_response( request_id, - InvalidRequestError(data=str(e)), + InvalidRequestError(), ) # 2) Route by method name; unknown -> -32601, known -> validate params (-32602 on failure) @@ -289,11 +291,11 @@ async def handle_requests(self, request: Request) -> Response: # noqa: PLR0911, # Parse the params field into the proto message type params = body.get('params', {}) specific_request = ParseDict(params, model_class()) - except Exception as e: + except Exception: logger.exception('Failed to parse request params') return self._generate_error_response( request_id, - InvalidParamsError(data=str(e)), + InvalidParamsError(), ) # 3) Build call context and wrap the request for downstream handling @@ -335,11 +337,9 @@ async def handle_requests(self, request: Request) -> Response: # noqa: PLR0911, raise e except A2AError as e: return self._generate_error_response(request_id, e) - except Exception as e: + except Exception: logger.exception('Unhandled exception') - return self._generate_error_response( - request_id, InternalError(message=str(e)) - ) + return self._generate_error_response(request_id, InternalError()) @validate_version(constants.PROTOCOL_VERSION_1_0) async def _process_streaming_request( @@ -585,7 +585,7 @@ async def event_generator( rpc_error: A2AError | JSONRPCError = ( e if isinstance(e, A2AError | JSONRPCError) - else InternalError(message=str(e)) + else InternalError() ) error_response = build_error_response( context.state.get('request_id'), rpc_error diff --git a/tests/integration/test_scenarios.py b/tests/integration/test_scenarios.py index 762270a9a..d1df2faec 100644 --- a/tests/integration/test_scenarios.py +++ b/tests/integration/test_scenarios.py @@ -11,7 +11,6 @@ from a2a.auth.user import User from a2a.client.client import ClientConfig from a2a.client.client_factory import ClientFactory -from a2a.client.errors import A2AClientError from a2a.helpers.proto_helpers import new_task_from_user_message from a2a.server.agent_execution import AgentExecutor, RequestContext from a2a.server.context import ServerCallContext @@ -49,6 +48,7 @@ ) from a2a.utils import TransportProtocol from a2a.utils.errors import ( + A2AError, InvalidAgentResponseError, InvalidParamsError, TaskNotCancelableError, @@ -469,7 +469,9 @@ async def cancel( ) # TODO: Is it correct error code ? - with pytest.raises(A2AClientError, match='TEST_ERROR_IN_EXECUTE'): + # Agent exceptions are sanitized server-side (internal details are + # logged, not exposed to clients); clients receive the generic error. + with pytest.raises(A2AError, match='Internal error'): async for _ in client.send_message( SendMessageRequest( message=msg, @@ -548,7 +550,9 @@ async def release_agent(): tasks.append(asyncio.create_task(release_agent())) - with pytest.raises(A2AClientError, match='TEST_ERROR_IN_EXECUTE'): + # Agent exceptions are sanitized server-side (internal details are + # logged, not exposed to clients); clients receive the generic error. + with pytest.raises(A2AError, match='Internal error'): async for _ in it: pass @@ -610,7 +614,9 @@ async def cancel( await asyncio.wait_for(started_event.wait(), timeout=1.0) - with pytest.raises(A2AClientError, match='TEST_ERROR_IN_CANCEL'): + # Agent exceptions are sanitized server-side; clients receive the + # generic error. + with pytest.raises(A2AError, match='Internal error'): await client.cancel_task(CancelTaskRequest(id=task_id)) (task,) = (await client.list_tasks(ListTasksRequest())).tasks @@ -678,7 +684,9 @@ async def consume_events(): with pytest.raises(asyncio.TimeoutError): await asyncio.wait_for(consume_task, timeout=0.1) else: - with pytest.raises(A2AClientError, match='TEST_ERROR_IN_EXECUTE'): + # Agent exceptions are sanitized server-side (internal details are + # logged, not exposed to clients); clients receive the generic error. + with pytest.raises(A2AError, match='Internal error'): await consume_task (task,) = (await client.list_tasks(ListTasksRequest())).tasks @@ -1019,8 +1027,9 @@ async def cancel( # Verify that both calls for clients finished. if use_legacy and not streaming: - # Legacy handler fails on first execution. - with pytest.raises(A2AClientError, match='NoTaskQueue'): + # Legacy handler fails on first execution; the failure is + # sanitized server-side (internal details are logged, not exposed). + with pytest.raises(A2AError, match='Internal error'): await task1 else: await task1 diff --git a/tests/server/request_handlers/test_grpc_handler.py b/tests/server/request_handlers/test_grpc_handler.py index fa504fc05..0d9ae07ce 100644 --- a/tests/server/request_handlers/test_grpc_handler.py +++ b/tests/server/request_handlers/test_grpc_handler.py @@ -746,3 +746,49 @@ async def mock_stream(*args, **kwargs): server_context = call_args[0][1] assert isinstance(server_context, ServerCallContext) assert server_context.tenant == '' + + +@pytest.mark.asyncio +async def test_unhandled_exception_is_sanitized( + grpc_handler: GrpcHandler, + mock_request_handler: AsyncMock, + mock_grpc_context: AsyncMock, +) -> None: + """A non-A2A exception must not leak its message to the client (BUG-46).""" + mock_request_handler.on_get_task.side_effect = RuntimeError( + 'internal detail: /secret/path' + ) + request_proto = a2a_pb2.GetTaskRequest(id='any') + + await grpc_handler.GetTask(request_proto, mock_grpc_context) + + mock_grpc_context.abort.assert_awaited_once() + call_args, _ = mock_grpc_context.abort.call_args + assert call_args[0] == grpc.StatusCode.INTERNAL + assert 'internal detail' not in call_args[1] + assert 'INTERNAL' in call_args[1] or 'Internal error' in call_args[1] + + +@pytest.mark.asyncio +async def test_unknown_a2a_error_type_is_sanitized( + grpc_handler: GrpcHandler, + mock_request_handler: AsyncMock, + mock_grpc_context: AsyncMock, +) -> None: + """An A2AError outside the mapping must not leak details (BUG-46).""" + from a2a.utils.errors import A2AError + + class CustomError(A2AError): + message = 'custom' + + mock_request_handler.on_get_task.side_effect = CustomError( + 'sensitive internals here' + ) + request_proto = a2a_pb2.GetTaskRequest(id='any') + + await grpc_handler.GetTask(request_proto, mock_grpc_context) + + mock_grpc_context.abort.assert_awaited_once() + call_args, _ = mock_grpc_context.abort.call_args + assert call_args[0] == grpc.StatusCode.UNKNOWN + assert 'sensitive internals' not in call_args[1] diff --git a/tests/server/routes/test_jsonrpc_dispatcher.py b/tests/server/routes/test_jsonrpc_dispatcher.py index 3bde4fc2e..8b34af0a7 100644 --- a/tests/server/routes/test_jsonrpc_dispatcher.py +++ b/tests/server/routes/test_jsonrpc_dispatcher.py @@ -653,3 +653,57 @@ async def stream_generator(): if __name__ == '__main__': pytest.main([__file__]) + + +# --- Error sanitization (BUG-12 / BUG-46) --- + + +class TestErrorSanitization: + def test_unhandled_exception_does_not_leak(self, client, mock_handler): + """Non-A2A exceptions must not leak their message to the client.""" + mock_handler.on_get_task.side_effect = RuntimeError( + 'internal detail: /secret/path' + ) + response = client.post( + '/', + json={ + 'jsonrpc': '2.0', + 'id': '1', + 'method': 'GetTask', + 'params': {'id': 'task1'}, + }, + ) + data = response.json() + assert data['error']['code'] == -32603 # InternalError + assert data['error']['message'] == 'Internal error' + assert 'internal detail' not in response.text + + def test_malformed_params_does_not_leak(self, client): + """Parse failures must not leak the raw parse error to the client.""" + response = client.post( + '/', + json={ + 'jsonrpc': '2.0', + 'id': '1', + 'method': 'GetTask', + # id must be a string; a dict fails proto parsing. + 'params': {'id': {'nested': 'invalid'}}, + }, + ) + data = response.json() + assert data['error']['code'] == -32602 # InvalidParamsError + assert 'nested' not in response.text + + def test_invalid_base_request_does_not_leak(self, client): + """Base JSON-RPC validation failures must not leak details.""" + response = client.post( + '/', + json={ + 'jsonrpc': '2.0', + 'id': '1', + 'method': 'GetTask', + 'params': 'not-a-dict', + }, + ) + data = response.json() + assert data['error']['code'] == -32600 # InvalidRequestError diff --git a/tests/server/test_integration.py b/tests/server/test_integration.py index cc0678c22..4524abee1 100644 --- a/tests/server/test_integration.py +++ b/tests/server/test_integration.py @@ -898,7 +898,7 @@ def test_validation_error(client: TestClient): def test_unhandled_exception(client: TestClient, handler: mock.AsyncMock): - """Test handling unhandled exception.""" + """Test handling unhandled exception without leaking internal details.""" handler.on_get_task.side_effect = Exception('Unexpected error') response = client.post( @@ -914,7 +914,9 @@ def test_unhandled_exception(client: TestClient, handler: mock.AsyncMock): data = response.json() assert 'error' in data assert data['error']['code'] == InternalError().code - assert 'Unexpected error' in data['error']['message'] + # The internal exception message must not leak to the client. + assert data['error']['message'] == 'Internal error' + assert 'Unexpected error' not in data['error']['message'] def test_get_method_to_rpc_endpoint(client: TestClient):