Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,6 @@ public class AgentRule

public class RuleConfig
{
[JsonPropertyName("topology_name")]
public string? TopologyName { get; set; }
[JsonPropertyName("criteria")]
public string? Criteria { get; set; }
}
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
namespace BotSharp.Abstraction.Coding.Options;

public class CodeGenerationOptions : LlmConfigBase
public class CodeGenerationOptions
{
/// <summary>
/// Agent id to get instruction
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
namespace BotSharp.Abstraction.Rules.Constants;

/// <summary>
/// Built-in rule criteria types. Each value maps to an
/// <see cref="IRuleCriteriaEvaluator.Type"/> registered in DI.
/// Plugins may introduce additional string types.
/// </summary>
public static class BuiltInRuleCriteria
{
/// <summary>
/// Evaluate a code script (e.g. Python) that returns a boolean result.
/// </summary>
public const string Code = "code";

/// <summary>
/// Ask an LLM whether the rule applies to the request.
/// </summary>
public const string Llm = "llm";
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
using BotSharp.Abstraction.Rules.Models;

namespace BotSharp.Abstraction.Rules;

/// <summary>
/// Decides whether a rule should be executed for the current request.
/// Implementations are resolved by <see cref="Type"/> in the rule engine,
/// so new criteria mechanisms can be added without changing the engine.
/// </summary>
public interface IRuleCriteriaEvaluator
{
/// <summary>
/// The criteria type this evaluator handles
/// </summary>
string Type { get; }

/// <summary>
/// Evaluate the criteria for a single agent's rule.
/// </summary>
/// <param name="agent">The agent whose rule is being considered</param>
/// <param name="trigger">The rule trigger</param>
/// <param name="context">The per-request criteria context</param>
/// <returns>
/// True if the rule should be executed for this request, false if it should be skipped,
/// or null when the evaluator could not produce an answer (missing script/template,
/// failed execution, error).
/// </returns>
Task<bool?> EvaluateAsync(Agent agent, IRuleTrigger trigger, RuleCriteriaContext context);
}
23 changes: 10 additions & 13 deletions src/Infrastructure/BotSharp.Abstraction/Rules/IRuleEngine.cs
Original file line number Diff line number Diff line change
@@ -1,6 +1,3 @@
using BotSharp.Abstraction.Graph;
using BotSharp.Abstraction.Rules.Models;

namespace BotSharp.Abstraction.Rules;

public interface IRuleEngine
Expand All @@ -17,14 +14,14 @@ public interface IRuleEngine
Task<IEnumerable<string>> Triggered(IRuleTrigger trigger, string text, IEnumerable<MessageState>? states = null, RuleTriggerOptions? options = null)
=> throw new NotImplementedException();

/// <summary>
/// Execute rule graph node
/// </summary>
/// <param name="node"></param>
/// <param name="graph"></param>
/// <param name="agentId"></param>
/// <param name="trigger"></param>
/// <param name="options"></param>
/// <returns></returns>
Task ExecuteGraphNode(FlowNode node, FlowGraph graph, string agentId, IRuleTrigger trigger, RuleNodeExecutionOptions options);
///// <summary>
///// Execute rule graph node
///// </summary>
///// <param name="node"></param>
///// <param name="graph"></param>
///// <param name="agentId"></param>
///// <param name="trigger"></param>
///// <param name="options"></param>
///// <returns></returns>
//Task ExecuteGraphNode(FlowNode node, FlowGraph graph, string agentId, IRuleTrigger trigger, RuleNodeExecutionOptions options);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
namespace BotSharp.Abstraction.Rules.Models;

/// <summary>
/// The per-request context passed to an <see cref="IRuleCriteriaEvaluator"/>
/// to decide whether a rule should be executed.
/// </summary>
public class RuleCriteriaContext
{
/// <summary>
/// The trigger message text.
/// </summary>
public string Text { get; set; } = string.Empty;

/// <summary>
/// The criteria options (evaluator type and its arguments).
/// </summary>
public CriteriaOptions Options { get; set; } = new();

/// <summary>
/// The conversation states carried with the request.
/// </summary>
public IEnumerable<MessageState>? States { get; set; }
}
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
using BotSharp.Abstraction.Repositories.Filters;
using BotSharp.Abstraction.Rules.Constants;
using System.Text.Json;

namespace BotSharp.Abstraction.Rules.Options;
Expand All @@ -11,12 +12,39 @@ public class RuleTriggerOptions
public AgentFilter? AgentFilter { get; set; }

/// <summary>
/// Json serializer options
/// Criteria
/// </summary>
public JsonSerializerOptions? JsonOptions { get; set; }
public CriteriaOptions? Criteria { get; set; }
}

public class CriteriaOptions
{
/// <summary>
/// Rule flow options
/// How the criteria is evaluated (see <see cref="BuiltInRuleCriteria"/>).
/// Selects which <c>IRuleCriteriaEvaluator</c> handles this criteria.
/// </summary>
public RuleFlowOptions? Flow { get; set; }
}
public string Type { get; set; } = BuiltInRuleCriteria.Code;

/// <summary>
/// Evaluator-specific settings, kept as raw JSON so each evaluator can
/// deserialize it into its own strongly-typed settings model.
/// Use <see cref="GetData{T}"/> to read it.
/// </summary>
public JsonElement? Data { get; set; }

/// <summary>
/// Deserialize <see cref="Data"/> into an evaluator-specific settings type.
/// Returns default (null) when no data is provided.
/// </summary>
public T? GetData<T>(JsonSerializerOptions? options = null)
{
if (Data == null || Data.Value.ValueKind == JsonValueKind.Null || Data.Value.ValueKind == JsonValueKind.Undefined)
{
return default;
}

return Data.Value.Deserialize<T>(options ?? _webJsonOptions);
Comment on lines +43 to +46

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remediation recommended

2. Criteria data can throw 🐞 Bug ☼ Reliability

CriteriaOptions.GetData<T>() deserializes JsonElement without handling JsonException; malformed or
non-convertible Criteria.Data can throw and abort the Triggered request. This exception occurs
before evaluator try/catch blocks (e.g., CodeCriteriaEvaluator/LlmCriteriaEvaluator) so it can
propagate out of the rule engine.
Agent Prompt
### Issue description
`CriteriaOptions.GetData<T>()` calls `JsonElement.Deserialize<T>()` directly. If `Criteria.Data` is malformed or incompatible with `T`, deserialization can throw and fail the entire rule-trigger request.

### Issue Context
Both `CodeCriteriaEvaluator` and `LlmCriteriaEvaluator` call `context.Options.GetData<...>()` before their `try {}` blocks, and `RuleEngine.EvaluateCriteria` does not guard `EvaluateAsync`, so exceptions from `GetData<T>()` can propagate.

### Fix Focus Areas
- src/Infrastructure/BotSharp.Abstraction/Rules/Options/RuleTriggerOptions.cs[39-47]
- src/Infrastructure/BotSharp.Core.Rules/Criteria/Code/CodeCriteriaEvaluator.cs[28-33]
- src/Infrastructure/BotSharp.Core.Rules/Criteria/Llm/LlmCriteriaEvaluator.cs[33-36]
- src/Infrastructure/BotSharp.Core.Rules/Engines/RuleEngine.cs[86-96]

### Proposed fix
- Wrap `Data.Value.Deserialize<T>(...)` in a `try/catch (JsonException)` (and optionally `NotSupportedException`) inside `GetData<T>()` and return `default` on failure.
- Optionally add a safe log point (if you don't want logging in Abstraction): catch in evaluators or `RuleEngine.EvaluateCriteria` and treat it as `null`/`false` so criteria enforcement doesn't crash the request.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

}

private static readonly JsonSerializerOptions _webJsonOptions = new(JsonSerializerDefaults.Web);
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
using Newtonsoft.Json.Linq;
using Newtonsoft.Json;
using System.Text.Json;

namespace BotSharp.Abstraction.Utilities;

Expand Down Expand Up @@ -38,4 +39,74 @@ public static string FormatJson(this string? json, Formatting format = Formattin
return json;
}
}

/// <summary>
/// Convert any object into a <see cref="JsonDocument"/>. A string input is treated as raw json first,
/// and only serialized as a json string value when it cannot be parsed.
/// </summary>
/// <returns>Null when the input is null or cannot be represented as json. The caller owns the returned document and should dispose it.</returns>
public static JsonDocument? ToJsonDoc(this object? obj, JsonSerializerOptions? jsonOptions = null)
{
var json = obj.ToJsonString(jsonOptions);
if (json == null)
{
return null;
}

try
{
return JsonDocument.Parse(json);
}
catch
{
return null;
}
}

/// <summary>
/// Convert any object into a <see cref="JsonElement"/>. The returned element is cloned,
/// so it stays valid after the underlying document is disposed.
/// </summary>
/// <returns>Null when the input is null or cannot be represented as json.</returns>
public static JsonElement? ToJsonElement(this object? obj, JsonSerializerOptions? jsonOptions = null)
{
using var doc = obj.ToJsonDoc(jsonOptions);
return doc?.RootElement.Clone();
}

private static string? ToJsonString(this object? obj, JsonSerializerOptions? jsonOptions)
{
if (obj == null)
{
return null;
}

// A string is most likely already json, e.g. a serialized payload or a raw llm output.
if (obj is string str)
{
if (string.IsNullOrWhiteSpace(str))
{
return null;
}

try
{
using var parsed = JsonDocument.Parse(str);
return str;
}
catch
{
// Not json, fall through and serialize it as a json string value.
}
}

try
{
return System.Text.Json.JsonSerializer.Serialize(obj, jsonOptions);
}
catch
{
return null;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@

<ItemGroup>
<ProjectReference Include="..\..\Infrastructure\BotSharp.Abstraction\BotSharp.Abstraction.csproj" />
<ProjectReference Include="..\..\Infrastructure\BotSharp.Core\BotSharp.Core.csproj" />
</ItemGroup>

</Project>
Loading
Loading