BotSharp изнутри: ищем слабые места в коде ИИ-платформы на .NET

• Введение
• О проекте
• Обзор ошибок
  • Неиспользуемый параметр
  • Нехватка case в блоке switch
  • Потеря значения параметра
  • Невнимательный copy-paste условия
  • Ошибка из-за неверного представления о приоритетах операторов
  • Потенциальное null-разыменование
• Заключение

Считается, что C# — это язык для enterprise-разработки, но не для машинного обучения. BotSharp — один из проектов, готовых поспорить с этим стереотипом, предлагая мощную ИИ-платформу на .NET. Давайте познакомимся с проектом поближе, а заодно и с ошибками в его исходном коде.

Введение

Исторически сложилось, что в сфере машинного обучения балом правит Python, в то время как C# прочно закрепил за собой статус языка для enterprise-систем, веб-разработки и gamedev-направления. Однако появление всё новых и новых ИИ-инструментов, таких как ML.NET и Microsoft Agent Framework, демонстрирует, что .NET отлично подходит и для создания ИИ-систем. В этой статье мы познакомимся с BotSharp — одним из таких инструментов. Однако основное внимание будет уделено обзору ошибок, найденных в его исходном коде с помощью статического анализатора PVS-Studio. Поехали!

О проекте

BotSharp — это open source фреймворк на языке C#, предназначенный для создания одноагентных и многоагентных систем, где центром принятия решений являются одна или несколько моделей ИИ. Эти модели способны как обрабатывать и создавать мультимедийные данные, так и выполнять действия посредством вызова предоставленных инструментов. В чем отличие BotSharp от Semantic Kernel? Сейчас BotSharp и Semantic Kernel решают схожие задачи и предоставляют компоненты для разработки агентных систем на основе LLM. Отличительной особенностью BotSharp является более высокий уровень абстракции и наличие встроенной инфраструктуры для управления агентами и диалогами, что позволяет быстрее развёртывать типовые агентные решения.

Рисунок 1 — веб-интерфейс BotSharp, предоставляемый для визуализации агентских систем

Больше узнать о BotSharp вы можете в документации фреймворка, но предлагаю повременить с этим и перейти к самому интересному — обзору найденных в его коде ошибок!

Обзор ошибок

Ошибки, которые мы будем рассматривать далее, были найдены с помощью PVS-Studio в последней на момент написания статьи версии проекта (5.2).

Неиспользуемый параметр

public class SettingService : ISettingService
{
  ....
  private readonly ILogger _logger;

  public SettingService(IServiceProvider services,
                        IConfiguration config,
                        ILogger<SettingService> logger)
  {
    _services = services;
     _config = config;
  }
  ....
}

Предупреждение PVS-Studio: V3117. Constructor parameter 'logger' is not used. SettingService.cs 15

Классическая опечатка из-за невнимательности: параметр logger в конструктор передали, но присвоить его одноименному полю _logger забыли. Прямо сейчас код работает без проблем по одной причине: поле _logger пока вообще нигде не используется. Но это ловушка, в которую можно попасть в будущем.

Рано или поздно кто-то решит добавить логирование, разработчик увидит готовое поле, напишет привычный вызов логирования и код без проблем скомпилируется. Вот только при первом же вызове на проде приложение эффектно выбросит NullReferenceException. В общем, ловить такие "мины" в будущем — сомнительное удовольствие.

Нехватка case в блоке switch

public async Task UpdateAgentTask(....)
{
  ....
  switch (field)
  {
    case AgentTaskField.Name:
      metaData.Name = task.Name;
      break;

    case AgentTaskField.Description:
      metaData.Description = task.Description;
      break;

    case AgentTaskField.Enabled:
      metaData.Enabled = task.Enabled;
      break;

    case AgentTaskField.Content:
      content = task.Content;
      break;

    case AgentTaskField.All:
      metaData.Name = task.Name;
      metaData.Description = task.Description;
      metaData.Enabled = task.Enabled;
      content = task.Content;
      break;
  }
  ....
}

Предупреждение PVS-Studio: V3002. The switch statement does not cover all values of the 'AgentTaskField' enum: Status. FileRepository.AgentTask.cs 234

Перед нами switch-конструкция, которая обновляет свойства metaData в зависимости от переданного флага. Разработчик перебрал почти все элементы перечисления AgentTaskField, но упустил один-единственный — Status.

Из-за этой нестыковки в логике приложение некорректно поведёт себя сразу в двух сценариях. Во-первых, при явном запросе на обновление Status switch просто проигнорирует команду, и данные о статусе в metaData не изменятся. Во-вторых, ошибка кроется в блоке case AgentTaskField.All: по логике он должен перезаписывать все свойства metadata свойствами task, но так как свойство Status было упущено из внимания, этот блок отработает не полностью.

Это классический пример того, как расширение enum-перечислений без проверки всех зависимых switch приводит к скрытым багам.

Потеря значения параметра

public async Task<FunctionCallFromLlm> GetNextInstruction(
  ...., 
  List<RoleDialogModel> dialogs)
{
  ....
  dialogs = new List<RoleDialogModel>
  {
    new RoleDialogModel(AgentRole.User, next)
    {
      FunctionName = nameof(HFReasoner),
      MessageId = messageId
    }
  };

  var response = await completion.GetChatCompletions(router, dialogs);
  ....
}

Предупреждение PVS-Studio: V3061. Parameter 'dialogs' is always rewritten in method body before being used. HFReasoner.cs 45

Метод GetNextInstruction принимает на вход коллекцию dialogs, но вместо того, чтобы дополнить её, полностью перезаписывает коллекцию новой заготовкой.

Передаваемый параметр, скорее всего, хранит некоторый контекст беседы, необходимый для генерации релевантного ответа. Перезапись переменной намертво стирает эту память прямо перед отправкой запроса в GetChatCompletions, что может значительно снизить качество ответа.

Для исправления ситуации новые данные можно добавить через Add или, если требуется, сохранить коллекцию dialogs неизменной либо создать новую коллекцию на основе dialogs, но с дополнительным элементом.

Невнимательный copy-paste условия

public string Make_Payment(
  [Description("order number"), Required] string order_number,
  [Description("total amount"), Required] int total_amount)
{
  if (order_number is null)
  {
    throw new McpException("Missing required argument 'order_number'");
  }
  if (order_number is null)
  {
    throw new McpException("Missing required argument 'total_amount'");
  }
}

Предупреждение PVS-Studio: V3021. There are two 'if' statements with identical conditional expressions. The first 'if' statement contains method return. This means that the second 'if' statement is senseless MakePaymentTool.cs 11

Перед нами классическое следствие невнимательного копирования кода. В методе задумывалась проверка двух обязательных параметров, но из-за идентичных условий в блоках if валидация order_number выполняется дважды, в то время как второй аргумент остаётся без внимания.

По тексту исключения во втором блоке очевидно, что разработчик планировал проверить параметр total_amount. Такие дефекты легко пропускаются при беглом просмотре, но мгновенно выявляются статическим анализом.

Ошибка из-за неверного представления о приоритетах операторов

private async Task<List<RoleDialogModel>> OnResponsedDone(....)
{
  ....
  new TokenStatsModel
  {
    ....
    TextInputTokens =  inputTokenDetails?
                      .TextTokens ?? 0 -  inputTokenDetails?
                                         .CachedTokenDetails?
                                         .TextTokens ?? 0,
    ....

    AudioInputTokens =  inputTokenDetails?
                       .AudioTokens ?? 0 -  inputTokenDetails?
                                           .CachedTokenDetails?
                                           .AudioTokens ?? 0,

    ....
  }
  ....
}

Предупреждения PVS-Studio:

V3123. Perhaps the '??' operator works in a different way than it was expected. Its priority is lower than priority of other operators in its left part. RealTimeCompletionProvider.cs 542

V3123. Perhaps the '??' operator works in a different way than it was expected. Its priority is lower than priority of other operators in its left part. RealTimeCompletionProvider.cs 544

Перед нами пример кода, который выглядит абсолютно логично для человека, но компилятором понимается совершенно иначе.

Разработчик пытался вычислить разность, операнды которой подстрахованы оператором ?? на случай null. Однако приоритет оператора вычитания - выше, чем у оператора null-объединения ??. Из-за этого вместо логики (A ?? 0) - (B ?? 0) будет выполнена логика A ?? ((0 - B) ?? 0).

Чтобы код заработал так, как задумывалось, выражения вокруг ?? необходимо явно обернуть в круглые скобки, например:

 (inputTokenDetails?.TextTokens ?? 0) - ( inputTokenDetails?
                                         .CachedTokenDetails?
                                         .TextTokens ?? 0)

Потенциальное null-разыменование

Issue 1

public async Task<RoleDialogModel> GetChatCompletions(....)
{
  ....
  var toolcall = response.choices.FirstOrDefault()?
                         .message
                         .tool_calls.FirstOrDefault();

  responseMessage = new RoleDialogModel(
    AgentRole.Function, JsonSerializer.Serialize(toolcall.function)
  )
  ....
}

Предупреждение PVS-Studio: V3146. Possible null dereference of 'toolcall'. The 'FirstOrDefault' can return default null value. ChatCompletionProvider.cs 62

В этом фрагменте кода вычисляется значение переменной toolcall. Для этого используется цепочка вызовов, в которой дважды фигурирует метод FirstOrDefault.

В этой цепочке разработчик предпринял все меры предосторожности, поэтому даже если одна из коллекций окажется пустой, переменная без ошибок инициализируется значением null.

Однако сразу после — внутри JsonSerializer.Serialize — выполняется разыменование этой переменной без какой-либо предварительной проверки, что в какой-то момент приведёт к выбросу NullReferenceException.

Issue 2

public async Task<BrowserActionResult> InputUserText(....)
{
  ....
  locator = Locator(actionParams.ContextId, htmlElementContextOut);
  count = await locator.CountAsync();
  ....
}

Предупреждение PVS-Studio: V3080. Possible null dereference. Consider inspecting 'locator'. PlaywrightWebDriver.InputUserText.cs 46

И снова анализатор предупреждает о потенциальном null-разыменовании в выражении locator.CountAsync().

Но на чём оно основано? Давайте заглянем в декларацию метода Locator:

private ILocator? Locator(....)
{
  ....
  else if (!string.IsNullOrEmpty(context.ElementName))
  {
    ....
    if (count == 0)
    {
      _logger.LogError(....);
      return null;
    }
    else if (count > 1)
    {
      _logger.LogError(....);
      return null;
    }
  }
  else
  {
    if (context.Index < 0)
    {
       _logger.LogError(....);
       return null;
     }
     ....
  }

  return element;
}

Как видим, есть целых три случая, в которых этот метод может вернуть null. При этом в коде, на который указывает анализатор, возвращаемое значение этого метода никак не проверяется.

Issue 3

public async Task<RoutableAgent[]> GetRoutableAgents(List<string> profiles)
{
  ....
  if (routableAgents.Length > 0 && profiles.Count > 0)
  {
    ....
  }
  else if (profiles == null || profiles.Count == 0)
  {
    ....
  }
  ....
}

Предупреждение PVS-Studio: V3095. The 'profiles' object was used before it was verified against null. Check lines: 131, 137. RoutingService.cs 131

Перед нами пример нарушения порядка валидации данных. Разработчик предусмотрел сценарий, при котором список profiles может быть не инициализирован, и добавил соответствующую проверку в блок else if. Проблема в том, что эта защита обесценивается предыдущим кодом, где в if выполняется обращение к profiles.Count. Чтобы исправить эту логическую ошибку, достаточно поменять блоки местами.

Заключение

Надеюсь, эта экскурсия по исходному коду BotSharp была для вас не только полезной, но и увлекательной. Будь то неочевидный приоритет операторов, забытый case в switch или потерянное значение аргумента — подобные дефекты отлично маскируются в масштабных проектах. Найти их в огромной кодовой базе вручную намного сложнее, чем может показаться, и именно здесь спасает проверка с помощью статического анализатора кода.

Если вдруг эта статья пробудила в вас желание проверить свой проект на наличие ошибок, то можете бесплатно попробовать статический анализатор, запросив триал.

До встречи в следующих статьях!