From dad21d86c553884b7946c0dd1b33d4b4981e4d7d Mon Sep 17 00:00:00 2001 From: Roger Barreto <19890735+RogerBarreto@users.noreply.github.com> Date: Thu, 11 Jun 2026 21:18:51 +0100 Subject: [PATCH] .NET: Scope argument-based standing approvals correctly in ToolApprovalAgent (#6486) Ensure an argument-scoped standing approval (the "always approve with exact arguments" path) records an empty argument set rather than null when the approved call has no arguments, so it matches only future no-argument calls. null remains reserved exclusively for tool-level approvals, keeping the two scopes distinct. This aligns the .NET behavior with the existing Python harness. Adds regression tests covering the no-argument standing-approval flow, the MatchesRule argument-scoping semantics, and empty-arguments rule serialization. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../Harness/ToolApproval/ToolApprovalAgent.cs | 12 +- .../Harness/ToolApproval/ToolApprovalRule.cs | 12 +- .../ToolApproval/ToolApprovalAgentTests.cs | 139 ++++++++++++++++++ .../ToolApproval/ToolApprovalRuleTests.cs | 26 ++++ 4 files changed, 185 insertions(+), 4 deletions(-) diff --git a/dotnet/src/Microsoft.Agents.AI/Harness/ToolApproval/ToolApprovalAgent.cs b/dotnet/src/Microsoft.Agents.AI/Harness/ToolApproval/ToolApprovalAgent.cs index 0c9e36f3925..16a4b04098b 100644 --- a/dotnet/src/Microsoft.Agents.AI/Harness/ToolApproval/ToolApprovalAgent.cs +++ b/dotnet/src/Microsoft.Agents.AI/Harness/ToolApproval/ToolApprovalAgent.cs @@ -741,11 +741,19 @@ private static bool ArgumentsMatch(IDictionary ruleArguments, ID /// /// Serializes function call arguments to a string dictionary for storage and comparison. /// - private static Dictionary? SerializeArguments(IDictionary? arguments, JsonSerializerOptions jsonSerializerOptions) + /// + /// Always returns a non-null dictionary so that an argument-scoped standing approval + /// (the + /// path) records an exact-arguments rule. A or empty source dictionary + /// yields an empty dictionary, which matches only future no-argument calls. A + /// value is reserved on for tool-level rules and is never + /// produced here, preventing an exact-arguments approval from widening into a tool-level approval. + /// + private static Dictionary SerializeArguments(IDictionary? arguments, JsonSerializerOptions jsonSerializerOptions) { if (arguments is null || arguments.Count == 0) { - return null; + return new Dictionary(StringComparer.Ordinal); } var serialized = new Dictionary(arguments.Count, StringComparer.Ordinal); diff --git a/dotnet/src/Microsoft.Agents.AI/Harness/ToolApproval/ToolApprovalRule.cs b/dotnet/src/Microsoft.Agents.AI/Harness/ToolApproval/ToolApprovalRule.cs index e633d1e0036..2a8ab690bb7 100644 --- a/dotnet/src/Microsoft.Agents.AI/Harness/ToolApproval/ToolApprovalRule.cs +++ b/dotnet/src/Microsoft.Agents.AI/Harness/ToolApproval/ToolApprovalRule.cs @@ -18,9 +18,16 @@ namespace Microsoft.Agents.AI; /// Tool-level: When is , /// all calls to the tool identified by are auto-approved. /// Tool+arguments: When is non-null, -/// only calls to the specified tool with exactly matching argument values are auto-approved. +/// only calls to the specified tool with exactly matching argument values are auto-approved. +/// A non-null but empty dictionary matches only calls that supply no arguments; it does +/// not widen into a tool-level rule. /// /// +/// +/// is therefore reserved exclusively for tool-level approval. An +/// argument-scoped approval (including one created from a no-argument call) is always stored +/// as a non-null dictionary so it cannot be silently broadened to all invocations of the tool. +/// /// [Experimental(DiagnosticIds.Experiments.AgentsAIExperiments)] internal sealed class ToolApprovalRule @@ -34,7 +41,8 @@ internal sealed class ToolApprovalRule /// /// Gets or sets the specific argument values that must match for this rule to apply. /// When , the rule applies to all invocations of the tool - /// regardless of arguments. + /// regardless of arguments. A non-null but empty dictionary applies only to + /// invocations that supply no arguments. /// /// /// Argument values are stored as their JSON-serialized string representations diff --git a/dotnet/tests/Microsoft.Agents.AI.UnitTests/Harness/ToolApproval/ToolApprovalAgentTests.cs b/dotnet/tests/Microsoft.Agents.AI.UnitTests/Harness/ToolApproval/ToolApprovalAgentTests.cs index 6e497d8bea7..3a5c00ff7b8 100644 --- a/dotnet/tests/Microsoft.Agents.AI.UnitTests/Harness/ToolApproval/ToolApprovalAgentTests.cs +++ b/dotnet/tests/Microsoft.Agents.AI.UnitTests/Harness/ToolApproval/ToolApprovalAgentTests.cs @@ -262,6 +262,95 @@ public async Task RunAsync_ToolWithArgsRule_DoesNotAutoApproveDifferentArgsAsync Assert.Equal("ReadFile", ((FunctionCallContent)requests[0].ToolCall).Name); } + /// + /// Verify that approving a no-argument call with the "always approve with exact arguments" + /// option does NOT auto-approve a later same-tool call that supplies arguments. The later + /// call must still surface for explicit approval (security regression for #6486). + /// + [Fact] + public async Task RunAsync_EmptyArgsToolWithArgsRule_DoesNotAutoApproveCallWithArgumentsAsync() + { + // Arrange + var session = new ChatClientAgentSession(); + + // Approve a no-argument SendPayment call with "always approve with exact arguments". + var ruleRequest = new ToolApprovalRequestContent("req0", new FunctionCallContent("call0", "SendPayment")); + var alwaysApproveResponse = ruleRequest.CreateAlwaysApproveToolWithArgumentsResponse(); + + // The inner agent then requests SendPayment WITH sensitive arguments. + var sensitiveArgs = new Dictionary + { + ["recipient"] = "attacker@example.test", + ["amount"] = 5000, + }; + var newApprovalRequest = new ToolApprovalRequestContent("req1", new FunctionCallContent("call1", "SendPayment", sensitiveArgs)); + var approvalResponseMsg = new AgentResponse([new ChatMessage(ChatRole.Assistant, [newApprovalRequest])]); + + var innerAgent = CreateMockAgent(approvalResponseMsg); + var agent = new ToolApprovalAgent(innerAgent.Object); + var inputMessages = new List + { + new(ChatRole.User, [alwaysApproveResponse]), + }; + + // Act + var response = await agent.RunAsync(inputMessages, session); + + // Assert — the argument-bearing request must surface, not be auto-approved. + var requests = response.Messages.SelectMany(m => m.Contents).OfType().ToList(); + Assert.Single(requests); + Assert.Equal("SendPayment", ((FunctionCallContent)requests[0].ToolCall).Name); + } + + /// + /// Verify that approving a no-argument call with the "always approve with exact arguments" + /// option still auto-approves a later same-tool call that also supplies no arguments, + /// preserving the intended narrow behavior. + /// + [Fact] + public async Task RunAsync_EmptyArgsToolWithArgsRule_AutoApprovesLaterEmptyArgsCallAsync() + { + // Arrange + var session = new ChatClientAgentSession(); + + var ruleRequest = new ToolApprovalRequestContent("req0", new FunctionCallContent("call0", "SendPayment")); + var alwaysApproveResponse = ruleRequest.CreateAlwaysApproveToolWithArgumentsResponse(); + + // Inner agent first re-requests SendPayment with no arguments, then returns a final response. + var emptyArgsRequest = new ToolApprovalRequestContent("req1", new FunctionCallContent("call1", "SendPayment")); + var approvalResponse = new AgentResponse([new ChatMessage(ChatRole.Assistant, [emptyArgsRequest])]); + var finalResponse = new AgentResponse([new ChatMessage(ChatRole.Assistant, "Payment sent")]); + + var callCount = 0; + var innerAgent = new Mock(); + innerAgent + .Protected() + .Setup>("RunCoreAsync", + ItExpr.IsAny>(), + ItExpr.IsAny(), + ItExpr.IsAny(), + ItExpr.IsAny()) + .ReturnsAsync(() => + { + callCount++; + return callCount == 1 ? approvalResponse : finalResponse; + }); + + var agent = new ToolApprovalAgent(innerAgent.Object); + var inputMessages = new List + { + new(ChatRole.User, [alwaysApproveResponse]), + }; + + // Act + var response = await agent.RunAsync(inputMessages, session); + + // Assert — the no-argument call is auto-approved and the inner agent reaches its final response. + Assert.Equal("Payment sent", response.Text); + var requests = response.Messages.SelectMany(m => m.Contents).OfType().ToList(); + Assert.Empty(requests); + } + #endregion #region Mixed Auto-Approve @@ -728,6 +817,56 @@ public void MatchesRule_JsonElementArgs_MatchesCorrectly() Assert.True(ToolApprovalAgent.MatchesRule(request, rules, AgentJsonUtilities.DefaultOptions)); } + /// + /// Verify that an empty-arguments rule (non-null but empty) matches only a call that + /// supplies no arguments, and never widens into a tool-level match. + /// + [Fact] + public void MatchesRule_EmptyArgumentsRule_MatchesEmptyArgumentCall_ReturnsTrue() + { + // Arrange — an exact-arguments approval of a no-argument call is stored as an empty dictionary. + var rules = new List + { + new() + { + ToolName = "SendPayment", + Arguments = new Dictionary(), + }, + }; + var request = new ToolApprovalRequestContent("req1", new FunctionCallContent("call1", "SendPayment")); + + // Act & Assert + Assert.True(ToolApprovalAgent.MatchesRule(request, rules, AgentJsonUtilities.DefaultOptions)); + } + + /// + /// Verify that an empty-arguments rule does NOT match a later same-tool call that supplies + /// arguments. This guards against an exact-arguments approval of a no-argument call being + /// widened into a tool-level approval. + /// + [Fact] + public void MatchesRule_EmptyArgumentsRule_DoesNotMatchCallWithArguments_ReturnsFalse() + { + // Arrange + var rules = new List + { + new() + { + ToolName = "SendPayment", + Arguments = new Dictionary(), + }, + }; + var request = new ToolApprovalRequestContent("req1", + new FunctionCallContent("call1", "SendPayment", new Dictionary + { + ["recipient"] = "attacker@example.test", + ["amount"] = 5000, + })); + + // Act & Assert + Assert.False(ToolApprovalAgent.MatchesRule(request, rules, AgentJsonUtilities.DefaultOptions)); + } + #endregion #region Extension Methods diff --git a/dotnet/tests/Microsoft.Agents.AI.UnitTests/Harness/ToolApproval/ToolApprovalRuleTests.cs b/dotnet/tests/Microsoft.Agents.AI.UnitTests/Harness/ToolApproval/ToolApprovalRuleTests.cs index 9cb9e617fe1..02cbca665d5 100644 --- a/dotnet/tests/Microsoft.Agents.AI.UnitTests/Harness/ToolApproval/ToolApprovalRuleTests.cs +++ b/dotnet/tests/Microsoft.Agents.AI.UnitTests/Harness/ToolApproval/ToolApprovalRuleTests.cs @@ -103,6 +103,32 @@ public void Serialize_ToolWithArgsRule_RoundTrips() Assert.Equal("utf-8", deserialized.Arguments["encoding"]); } + /// + /// Verify that an empty-arguments rule round-trips as a non-null empty dictionary, keeping it + /// distinct from a tool-level (null arguments) rule so persisted session state preserves the + /// narrower scope. + /// + [Fact] + public void Serialize_EmptyArgsRule_RoundTrips() + { + // Arrange + var rule = new ToolApprovalRule + { + ToolName = "SendPayment", + Arguments = new Dictionary(), + }; + + // Act + var json = JsonSerializer.Serialize(rule, AgentJsonUtilities.DefaultOptions); + var deserialized = JsonSerializer.Deserialize(json, AgentJsonUtilities.DefaultOptions); + + // Assert + Assert.NotNull(deserialized); + Assert.Equal("SendPayment", deserialized!.ToolName); + Assert.NotNull(deserialized.Arguments); + Assert.Empty(deserialized.Arguments!); + } + /// /// Verify that JSON property names are correctly applied. ///