From 05c1762fdfd51f3e4a1a59ccf0ed142a43d72932 Mon Sep 17 00:00:00 2001 From: Giles Odigwe Date: Mon, 22 Jun 2026 13:35:34 -0700 Subject: [PATCH 1/4] .NET: Enforce ApprovalRequiredAIFunction in GitHub Copilot provider The GitHub Copilot SDK owns the tool-calling loop and invokes registered custom functions directly, so the standard FunctionInvokingChatClient approval round-trip never runs for this provider. As a result a tool wrapped in ApprovalRequiredAIFunction (only a marker) could execute without any Agent Framework approval. Add an agent-level onFunctionApproval callback and wrap approval-required tools in an ApprovalGatedAIFunction that enforces approval before invoking the underlying function. Secure-by-default: with no callback, or when the callback denies or throws, execution is denied. The gate forwards tool metadata (including the Copilot skip_permission flag) so it stays transparent to the SDK. This mirrors the Python provider's behavior. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../GitHubCopilotAgent.cs | 138 ++++++++++++++- .../Microsoft.Agents.AI.GitHub.Copilot.csproj | 2 +- .../GitHubCopilotAgentTests.cs | 58 +++++++ ....AI.GitHub.Copilot.IntegrationTests.csproj | 2 +- .../GitHubCopilotAgentTests.cs | 163 ++++++++++++++++++ ....Agents.AI.GitHub.Copilot.UnitTests.csproj | 2 +- 6 files changed, 358 insertions(+), 7 deletions(-) diff --git a/dotnet/src/Microsoft.Agents.AI.GitHub.Copilot/GitHubCopilotAgent.cs b/dotnet/src/Microsoft.Agents.AI.GitHub.Copilot/GitHubCopilotAgent.cs index 006e191b3eb..e82a2361406 100644 --- a/dotnet/src/Microsoft.Agents.AI.GitHub.Copilot/GitHubCopilotAgent.cs +++ b/dotnet/src/Microsoft.Agents.AI.GitHub.Copilot/GitHubCopilotAgent.cs @@ -42,6 +42,12 @@ public sealed class GitHubCopilotAgent : AIAgent, IAsyncDisposable /// The name of the agent. /// The description of the agent. /// Optional JSON serializer options. Defaults to . + /// + /// Optional callback that approves or denies execution of custom function tools wrapped in + /// ; it must return to allow the call. + /// When not configured, or when it denies or throws, such tools are denied (secure-by-default). This is the + /// agent-level approval gate for this provider, independent of SessionConfig.OnPermissionRequest. + /// public GitHubCopilotAgent( CopilotClient copilotClient, SessionConfig? sessionConfig = null, @@ -49,12 +55,13 @@ public GitHubCopilotAgent( string? id = null, string? name = null, string? description = null, - JsonSerializerOptions? jsonSerializerOptions = null) + JsonSerializerOptions? jsonSerializerOptions = null, + Func>? onFunctionApproval = null) { _ = Throw.IfNull(copilotClient); this._copilotClient = copilotClient; - this._sessionConfig = sessionConfig; + this._sessionConfig = WrapApprovalRequiredTools(sessionConfig, onFunctionApproval); this._ownsClient = ownsClient; this._id = id; this._name = name ?? DefaultName; @@ -73,6 +80,11 @@ public GitHubCopilotAgent( /// The tools to make available to the agent. /// Optional instructions to append as a system message. /// Optional JSON serializer options. Defaults to . + /// + /// Optional callback that approves or denies execution of custom function tools wrapped in + /// ; it must return to allow the call. + /// When not configured, or when it denies or throws, such tools are denied (secure-by-default). + /// public GitHubCopilotAgent( CopilotClient copilotClient, bool ownsClient = false, @@ -81,7 +93,8 @@ public GitHubCopilotAgent( string? description = null, IList? tools = null, string? instructions = null, - JsonSerializerOptions? jsonSerializerOptions = null) + JsonSerializerOptions? jsonSerializerOptions = null, + Func>? onFunctionApproval = null) : this( copilotClient, GetSessionConfig(tools, instructions), @@ -89,7 +102,8 @@ public GitHubCopilotAgent( id, name, description, - jsonSerializerOptions) + jsonSerializerOptions, + onFunctionApproval) { } @@ -519,6 +533,49 @@ private AgentResponseUpdate ConvertToAgentResponseUpdate(SessionEvent sessionEve return new SessionConfig { Tools = mappedTools, SystemMessage = systemMessage }; } + /// + /// Returns a in which every tool wrapped in + /// is replaced with an , so the GitHub Copilot SDK tool loop cannot invoke + /// an approval-required function without first passing the Agent Framework approval callback. + /// + /// + /// The source is returned unchanged when it contains no approval-required tools. + /// Otherwise a clone is returned so the caller-supplied configuration is not mutated. + /// + private static SessionConfig? WrapApprovalRequiredTools( + SessionConfig? sessionConfig, + Func>? onFunctionApproval) + { + if (sessionConfig?.Tools is not { Count: > 0 } tools) + { + return sessionConfig; + } + + List mappedTools = new(tools.Count); + bool wrappedAny = false; + foreach (AIFunctionDeclaration tool in tools) + { + if (tool is AIFunction function && function.GetService() is not null) + { + mappedTools.Add(new ApprovalGatedAIFunction(function, onFunctionApproval)); + wrappedAny = true; + } + else + { + mappedTools.Add(tool); + } + } + + if (!wrappedAny) + { + return sessionConfig; + } + + SessionConfig wrapped = sessionConfig.Clone(); + wrapped.Tools = mappedTools; + return wrapped; + } + private static async Task<(List? Attachments, string? TempDir)> ProcessDataContentAttachmentsAsync( IEnumerable messages, CancellationToken cancellationToken) @@ -563,4 +620,77 @@ private static void CleanupTempDir(string? tempDir) } } } + + /// + /// An decorator that enforces an Agent Framework function-approval check + /// before invoking a tool wrapped in . + /// + /// + /// + /// The GitHub Copilot SDK owns the model tool-calling loop and invokes registered custom functions + /// directly, so the standard FunctionInvokingChatClient approval round-trip (emitting a + /// ToolApprovalRequestContent and waiting for a ToolApprovalResponseContent) never runs + /// for this provider. As a result, an — which is only a marker + /// and does not enforce approval on its own — would otherwise execute without any Agent Framework approval. + /// + /// + /// This decorator restores the approval boundary: the underlying function is only invoked after the + /// configured approval callback explicitly approves the call. When no callback is configured, or the + /// callback denies or throws, execution is denied (secure-by-default). The decorator forwards all tool + /// metadata (name, description, JSON schema, and additional properties such as the Copilot + /// skip_permission flag) from the inner function so the tool remains transparent to the SDK. + /// + /// + private sealed class ApprovalGatedAIFunction : DelegatingAIFunction + { + private readonly Func>? _onFunctionApproval; + + public ApprovalGatedAIFunction( + AIFunction innerFunction, + Func>? onFunctionApproval) + : base(innerFunction) + { + this._onFunctionApproval = onFunctionApproval; + } + + protected override async ValueTask InvokeCoreAsync( + AIFunctionArguments arguments, + CancellationToken cancellationToken) + { + if (!await this.IsApprovedAsync(arguments, cancellationToken).ConfigureAwait(false)) + { + return this._onFunctionApproval is null + ? $"Tool '{this.Name}' requires human approval but no approval callback is configured on the agent; the request was denied." + : $"Tool '{this.Name}' requires human approval and the request was denied."; + } + + return await base.InvokeCoreAsync(arguments, cancellationToken).ConfigureAwait(false); + } + + private async ValueTask IsApprovedAsync(AIFunctionArguments arguments, CancellationToken cancellationToken) + { + // Secure-by-default: with no callback configured an approval-required tool can never execute. + if (this._onFunctionApproval is null) + { + return false; + } + + FunctionCallContent request = new( + callId: $"github-copilot-approval::{this.Name}", + name: this.Name, + arguments: arguments); + + try + { + return await this._onFunctionApproval(request, cancellationToken).ConfigureAwait(false); + } +#pragma warning disable CA1031 // Do not catch general exception types + catch (Exception) +#pragma warning restore CA1031 + { + // Secure-by-default: any failure in the approval callback denies execution. + return false; + } + } + } } diff --git a/dotnet/src/Microsoft.Agents.AI.GitHub.Copilot/Microsoft.Agents.AI.GitHub.Copilot.csproj b/dotnet/src/Microsoft.Agents.AI.GitHub.Copilot/Microsoft.Agents.AI.GitHub.Copilot.csproj index dad0b31f59d..4feccdc691a 100644 --- a/dotnet/src/Microsoft.Agents.AI.GitHub.Copilot/Microsoft.Agents.AI.GitHub.Copilot.csproj +++ b/dotnet/src/Microsoft.Agents.AI.GitHub.Copilot/Microsoft.Agents.AI.GitHub.Copilot.csproj @@ -4,7 +4,7 @@ true $(TargetFrameworksCore) - $(NoWarn);GHCP001 + $(NoWarn);GHCP001;MEAI001 diff --git a/dotnet/tests/Microsoft.Agents.AI.GitHub.Copilot.IntegrationTests/GitHubCopilotAgentTests.cs b/dotnet/tests/Microsoft.Agents.AI.GitHub.Copilot.IntegrationTests/GitHubCopilotAgentTests.cs index cfdc8198725..5e4a4fc0ca6 100644 --- a/dotnet/tests/Microsoft.Agents.AI.GitHub.Copilot.IntegrationTests/GitHubCopilotAgentTests.cs +++ b/dotnet/tests/Microsoft.Agents.AI.GitHub.Copilot.IntegrationTests/GitHubCopilotAgentTests.cs @@ -3,6 +3,7 @@ using System; using System.Collections.Generic; using System.Linq; +using System.Threading; using System.Threading.Tasks; using GitHub.Copilot; using GitHub.Copilot.Rpc; @@ -131,6 +132,63 @@ public async Task RunAsync_WithFunctionTool_InvokesToolAsync() } } + [Fact] + public async Task RunAsync_WithApprovalRequiredTool_ExecutesWhenCallbackApprovesAsync() + { + // Arrange + SkipIfCopilotNotConfigured(); + + bool toolInvoked = false; + bool approvalRequested = false; + + AIFunction weatherTool = AIFunctionFactory.Create((string location) => + { + toolInvoked = true; + return $"The weather in {location} is sunny with a high of 25C."; + }, "GetWeather", "Get the weather for a given location."); + ApprovalRequiredAIFunction approvalRequiredTool = new(weatherTool); + + ValueTask approveAsync(FunctionCallContent request, CancellationToken cancellationToken) + { + approvalRequested = true; + return new(true); + } + + await using CopilotClient client = new(new CopilotClientOptions()); + await client.StartAsync(); + + SessionConfig sessionConfig = new() + { + Tools = [approvalRequiredTool], + OnPermissionRequest = OnPermissionRequestAsync, + SystemMessage = new SystemMessageConfig + { + Mode = SystemMessageMode.Append, + Content = "You are a weather assistant. Always use the GetWeather tool to answer weather questions.", + }, + }; + + await using GitHubCopilotAgent agent = new(client, sessionConfig, onFunctionApproval: approveAsync); + AgentSession session = await agent.CreateSessionAsync(); + + try + { + // Act + AgentResponse response = await agent.RunAsync("What's the weather like in Seattle?", session); + + // Assert - the SDK tool loop must dispatch through the Agent Framework approval gate, + // and the underlying function only runs because the callback approved the call. + Assert.NotNull(response); + Assert.NotEmpty(response.Messages); + Assert.True(approvalRequested); + Assert.True(toolInvoked); + } + finally + { + await DeleteSessionAsync(client, session); + } + } + [Fact] public async Task RunAsync_WithSession_MaintainsContextAsync() { diff --git a/dotnet/tests/Microsoft.Agents.AI.GitHub.Copilot.IntegrationTests/Microsoft.Agents.AI.GitHub.Copilot.IntegrationTests.csproj b/dotnet/tests/Microsoft.Agents.AI.GitHub.Copilot.IntegrationTests/Microsoft.Agents.AI.GitHub.Copilot.IntegrationTests.csproj index 3fd692527bb..22f15d517cc 100644 --- a/dotnet/tests/Microsoft.Agents.AI.GitHub.Copilot.IntegrationTests/Microsoft.Agents.AI.GitHub.Copilot.IntegrationTests.csproj +++ b/dotnet/tests/Microsoft.Agents.AI.GitHub.Copilot.IntegrationTests/Microsoft.Agents.AI.GitHub.Copilot.IntegrationTests.csproj @@ -3,7 +3,7 @@ $(TargetFrameworksCore) - $(NoWarn);GHCP001 + $(NoWarn);GHCP001;MEAI001 diff --git a/dotnet/tests/Microsoft.Agents.AI.GitHub.Copilot.UnitTests/GitHubCopilotAgentTests.cs b/dotnet/tests/Microsoft.Agents.AI.GitHub.Copilot.UnitTests/GitHubCopilotAgentTests.cs index 8faa842eb04..ae85d8d3c23 100644 --- a/dotnet/tests/Microsoft.Agents.AI.GitHub.Copilot.UnitTests/GitHubCopilotAgentTests.cs +++ b/dotnet/tests/Microsoft.Agents.AI.GitHub.Copilot.UnitTests/GitHubCopilotAgentTests.cs @@ -3,6 +3,7 @@ using System; using System.Collections.Generic; using System.Linq; +using System.Threading; using System.Threading.Tasks; using GitHub.Copilot; using GitHub.Copilot.Rpc; @@ -382,4 +383,166 @@ public void ConvertToAgentResponseUpdate_AssistantMessageEventWhenNotStreaming_H Assert.Null(result.MessageId); Assert.Null(result.ResponseId); } + + [Fact] + public async Task Constructor_WithApprovalRequiredTool_GatesExecutionAndDeniesWithoutCallbackAsync() + { + // Arrange + bool invoked = false; + AIFunction dangerousTool = AIFunctionFactory.Create( + () => { invoked = true; return "sensitive operation completed"; }, + "ApprovalRequiredOperation", + "Performs an approval-required operation."); + ApprovalRequiredAIFunction approvalRequiredTool = new(dangerousTool); + + CopilotClient copilotClient = new(new CopilotClientOptions()); + var agent = new GitHubCopilotAgent(copilotClient, tools: [approvalRequiredTool]); + + // Act + AIFunction exposedTool = GetExposedFunction(agent); + object? result = await exposedTool.InvokeAsync(new AIFunctionArguments()); + + // Assert - the provider must NOT expose the same directly-invokable approval-required object, + // and invoking the gate without an approval callback must deny execution. + Assert.NotSame(approvalRequiredTool, exposedTool); + Assert.False(invoked); + Assert.Contains("requires human approval", result?.ToString()); + } + + [Fact] + public async Task Constructor_WithApprovalRequiredTool_ExecutesWhenCallbackApprovesAsync() + { + // Arrange + bool invoked = false; + AIFunction dangerousTool = AIFunctionFactory.Create( + () => { invoked = true; return "sensitive operation completed"; }, + "ApprovalRequiredOperation", + "Performs an approval-required operation."); + ApprovalRequiredAIFunction approvalRequiredTool = new(dangerousTool); + + FunctionCallContent? observedRequest = null; + ValueTask approveAsync(FunctionCallContent request, CancellationToken _) { observedRequest = request; return new(true); } + + CopilotClient copilotClient = new(new CopilotClientOptions()); + var agent = new GitHubCopilotAgent(copilotClient, tools: [approvalRequiredTool], onFunctionApproval: approveAsync); + + // Act + AIFunction exposedTool = GetExposedFunction(agent); + object? result = await exposedTool.InvokeAsync(new AIFunctionArguments()); + + // Assert + Assert.True(invoked); + Assert.NotNull(observedRequest); + Assert.Equal("ApprovalRequiredOperation", observedRequest!.Name); + Assert.Contains("sensitive operation completed", result?.ToString()); + } + + [Fact] + public async Task Constructor_WithApprovalRequiredTool_DeniesWhenCallbackRejectsAsync() + { + // Arrange + bool invoked = false; + AIFunction dangerousTool = AIFunctionFactory.Create( + () => { invoked = true; return "sensitive operation completed"; }, + "ApprovalRequiredOperation", + "Performs an approval-required operation."); + ApprovalRequiredAIFunction approvalRequiredTool = new(dangerousTool); + + ValueTask denyAsync(FunctionCallContent request, CancellationToken cancellationToken) => new(false); + + CopilotClient copilotClient = new(new CopilotClientOptions()); + var agent = new GitHubCopilotAgent(copilotClient, tools: [approvalRequiredTool], onFunctionApproval: denyAsync); + + // Act + AIFunction exposedTool = GetExposedFunction(agent); + object? result = await exposedTool.InvokeAsync(new AIFunctionArguments()); + + // Assert + Assert.False(invoked); + Assert.Contains("the request was denied", result?.ToString()); + } + + [Fact] + public async Task Constructor_WithApprovalRequiredTool_DeniesWhenCallbackThrowsAsync() + { + // Arrange + bool invoked = false; + AIFunction dangerousTool = AIFunctionFactory.Create( + () => { invoked = true; return "sensitive operation completed"; }, + "ApprovalRequiredOperation", + "Performs an approval-required operation."); + ApprovalRequiredAIFunction approvalRequiredTool = new(dangerousTool); + + ValueTask throwingAsync(FunctionCallContent request, CancellationToken cancellationToken) => throw new InvalidOperationException("callback failure"); + + CopilotClient copilotClient = new(new CopilotClientOptions()); + var agent = new GitHubCopilotAgent(copilotClient, tools: [approvalRequiredTool], onFunctionApproval: throwingAsync); + + // Act + AIFunction exposedTool = GetExposedFunction(agent); + object? result = await exposedTool.InvokeAsync(new AIFunctionArguments()); + + // Assert - secure-by-default: a throwing callback denies execution rather than propagating. + Assert.False(invoked); + Assert.Contains("the request was denied", result?.ToString()); + } + + [Fact] + public void Constructor_WithApprovalRequiredTool_PreservesSkipPermissionMetadata() + { + // Arrange + AIFunction dangerousTool = CopilotTool.DefineTool( + () => "ok", + toolOptions: new CopilotToolOptions { SkipPermission = true }, + factoryOptions: new AIFunctionFactoryOptions + { + Name = "WriteSensitiveMarker", + Description = "Writes a sensitive marker file." + }); + ApprovalRequiredAIFunction approvalRequiredTool = new(dangerousTool); + + CopilotClient copilotClient = new(new CopilotClientOptions()); + var agent = new GitHubCopilotAgent(copilotClient, tools: [approvalRequiredTool]); + + // Act + AIFunction exposedTool = GetExposedFunction(agent); + + // Assert - the gate must remain transparent to the Copilot SDK, preserving the skip_permission flag. + Assert.NotSame(approvalRequiredTool, exposedTool); + Assert.Equal("WriteSensitiveMarker", exposedTool.Name); + Assert.NotNull(exposedTool.AdditionalProperties); + Assert.True(exposedTool.AdditionalProperties.TryGetValue("skip_permission", out object? skip)); + Assert.Equal(true, skip); + } + + [Fact] + public void Constructor_WithNonApprovalTool_LeavesToolUnchanged() + { + // Arrange + AIFunction plainTool = AIFunctionFactory.Create(() => "test", "TestFunc", "Test function"); + + CopilotClient copilotClient = new(new CopilotClientOptions()); + var agent = new GitHubCopilotAgent(copilotClient, tools: [plainTool]); + + // Act + AIFunction exposedTool = GetExposedFunction(agent); + + // Assert - tools that do not require approval pass through untouched. + Assert.Same(plainTool, exposedTool); + } + + private static AIFunction GetExposedFunction(GitHubCopilotAgent agent) + { + SessionConfig sessionConfig = GetSessionConfigFromAgent(agent); + AIFunctionDeclaration declaration = Assert.Single(sessionConfig.Tools!); + return declaration.GetService()!; + } + + private static SessionConfig GetSessionConfigFromAgent(GitHubCopilotAgent agent) + { + System.Reflection.FieldInfo field = typeof(GitHubCopilotAgent).GetField( + "_sessionConfig", + System.Reflection.BindingFlags.Instance | System.Reflection.BindingFlags.NonPublic)!; + return (SessionConfig)field.GetValue(agent)!; + } } diff --git a/dotnet/tests/Microsoft.Agents.AI.GitHub.Copilot.UnitTests/Microsoft.Agents.AI.GitHub.Copilot.UnitTests.csproj b/dotnet/tests/Microsoft.Agents.AI.GitHub.Copilot.UnitTests/Microsoft.Agents.AI.GitHub.Copilot.UnitTests.csproj index 185130f9f39..d1bd009f80d 100644 --- a/dotnet/tests/Microsoft.Agents.AI.GitHub.Copilot.UnitTests/Microsoft.Agents.AI.GitHub.Copilot.UnitTests.csproj +++ b/dotnet/tests/Microsoft.Agents.AI.GitHub.Copilot.UnitTests/Microsoft.Agents.AI.GitHub.Copilot.UnitTests.csproj @@ -3,7 +3,7 @@ $(TargetFrameworksCore) - $(NoWarn);GHCP001 + $(NoWarn);GHCP001;MEAI001 From de9d68f55229e8c8e4b09755687208f238a955ee Mon Sep 17 00:00:00 2001 From: Giles Odigwe Date: Mon, 22 Jun 2026 13:52:19 -0700 Subject: [PATCH 2/4] .NET: Propagate cancellation from GitHub Copilot approval callback Let OperationCanceledException propagate from the approval callback instead of swallowing it into a denial, so cooperative cancellation is honored. Other callback failures still deny by default. Added a unit test. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../GitHubCopilotAgent.cs | 7 +++++- .../GitHubCopilotAgentTests.cs | 24 +++++++++++++++++++ 2 files changed, 30 insertions(+), 1 deletion(-) diff --git a/dotnet/src/Microsoft.Agents.AI.GitHub.Copilot/GitHubCopilotAgent.cs b/dotnet/src/Microsoft.Agents.AI.GitHub.Copilot/GitHubCopilotAgent.cs index e82a2361406..53834bad754 100644 --- a/dotnet/src/Microsoft.Agents.AI.GitHub.Copilot/GitHubCopilotAgent.cs +++ b/dotnet/src/Microsoft.Agents.AI.GitHub.Copilot/GitHubCopilotAgent.cs @@ -684,11 +684,16 @@ private async ValueTask IsApprovedAsync(AIFunctionArguments arguments, Can { return await this._onFunctionApproval(request, cancellationToken).ConfigureAwait(false); } + catch (OperationCanceledException) + { + // Honor cooperative cancellation rather than treating it as a denial. + throw; + } #pragma warning disable CA1031 // Do not catch general exception types catch (Exception) #pragma warning restore CA1031 { - // Secure-by-default: any failure in the approval callback denies execution. + // Secure-by-default: any other failure in the approval callback denies execution. return false; } } diff --git a/dotnet/tests/Microsoft.Agents.AI.GitHub.Copilot.UnitTests/GitHubCopilotAgentTests.cs b/dotnet/tests/Microsoft.Agents.AI.GitHub.Copilot.UnitTests/GitHubCopilotAgentTests.cs index ae85d8d3c23..f8a73b86427 100644 --- a/dotnet/tests/Microsoft.Agents.AI.GitHub.Copilot.UnitTests/GitHubCopilotAgentTests.cs +++ b/dotnet/tests/Microsoft.Agents.AI.GitHub.Copilot.UnitTests/GitHubCopilotAgentTests.cs @@ -487,6 +487,30 @@ public async Task Constructor_WithApprovalRequiredTool_DeniesWhenCallbackThrowsA Assert.Contains("the request was denied", result?.ToString()); } + [Fact] + public async Task Constructor_WithApprovalRequiredTool_PropagatesCancellationAsync() + { + // Arrange + bool invoked = false; + AIFunction dangerousTool = AIFunctionFactory.Create( + () => { invoked = true; return "sensitive operation completed"; }, + "ApprovalRequiredOperation", + "Performs an approval-required operation."); + ApprovalRequiredAIFunction approvalRequiredTool = new(dangerousTool); + + ValueTask cancelingAsync(FunctionCallContent request, CancellationToken cancellationToken) + => throw new OperationCanceledException(cancellationToken); + + CopilotClient copilotClient = new(new CopilotClientOptions()); + var agent = new GitHubCopilotAgent(copilotClient, tools: [approvalRequiredTool], onFunctionApproval: cancelingAsync); + + // Act & Assert - cancellation must propagate rather than being swallowed into a denial. + AIFunction exposedTool = GetExposedFunction(agent); + await Assert.ThrowsAsync( + async () => await exposedTool.InvokeAsync(new AIFunctionArguments())); + Assert.False(invoked); + } + [Fact] public void Constructor_WithApprovalRequiredTool_PreservesSkipPermissionMetadata() { From 0c22eb2da31445581a473b64205892986410af05 Mon Sep 17 00:00:00 2001 From: Giles Odigwe Date: Thu, 25 Jun 2026 11:23:23 -0700 Subject: [PATCH 3/4] .NET: Enforce ApprovalRequiredAIFunction via Copilot SDK OnPreToolUse hook Replace the custom approval enforcement (ApprovalGatedAIFunction wrapper + onFunctionApproval callback) with the GitHub Copilot SDK's native OnPreToolUse hook, which the SDK already provides for pre-execution gating. When a tool wrapped in ApprovalRequiredAIFunction is registered and the caller hasn't supplied their own OnPreToolUse hook, the agent installs a default hook that returns "ask" for those tools (routing the decision to OnPermissionRequest) and defers (null) for all other tools, preserving today's behavior for non-approval tools. If the caller supplies their own OnPreToolUse hook, it takes precedence and they own approval handling; the agent logs a warning naming any approval-required tool that will not be auto-gated, and the behavior is documented. Adds an optional ILoggerFactory parameter for the warning. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../Agent_With_GitHubCopilot/README.md | 37 +++ .../GitHubCopilotAgent.cs | 198 +++++++-------- .../GitHubCopilotAgentTests.cs | 21 +- .../GitHubCopilotAgentTests.cs | 225 ++++++++---------- 4 files changed, 238 insertions(+), 243 deletions(-) diff --git a/dotnet/samples/02-agents/AgentProviders/github-copilot/Agent_With_GitHubCopilot/README.md b/dotnet/samples/02-agents/AgentProviders/github-copilot/Agent_With_GitHubCopilot/README.md index 90266f38bc7..623caa2636f 100644 --- a/dotnet/samples/02-agents/AgentProviders/github-copilot/Agent_With_GitHubCopilot/README.md +++ b/dotnet/samples/02-agents/AgentProviders/github-copilot/Agent_With_GitHubCopilot/README.md @@ -64,6 +64,43 @@ AgentResponse response = await agent.RunAsync("Write a small .NET 10 C# hello wo Console.WriteLine(response); ``` +## Approving or denying tool execution + +The GitHub Copilot SDK owns the tool-calling loop for this provider, so approval is enforced through the SDK's +native pre-execution hook rather than the Agent Framework chat-client approval round-trip. + +When you register a tool wrapped in `ApprovalRequiredAIFunction`, `GitHubCopilotAgent` installs a default +`SessionConfig.Hooks.OnPreToolUse` hook that returns `"ask"` for that tool and defers (`null`) for all other tools. +The `"ask"` decision routes to your `SessionConfig.OnPermissionRequest` handler, where you approve or deny the call +(this also fires even for tools configured with `SkipPermission = true`): + +```csharp +using GitHub.Copilot; + +AIFunction deleteFile = AIFunctionFactory.Create(DeleteFile, "DeleteFile", "Deletes a file."); + +SessionConfig sessionConfig = new() +{ + // Wrapping the tool marks it approval-required; the agent turns this into an "ask" at OnPreToolUse. + Tools = [new ApprovalRequiredAIFunction(deleteFile)], + + // OnPermissionRequest decides the "asked" tools (and Copilot's built-in shell/file/URL prompts). + OnPermissionRequest = (request, invocation) => + { + // Surface to a human, check policy, etc. + bool approved = AskHuman(request); + return Task.FromResult(approved + ? PermissionDecision.ApproveOnce() + : PermissionDecision.Reject("Denied by user.")); + }, +}; +``` + +> **⚠️ If you provide your own `OnPreToolUse` hook**, it takes precedence and the agent does **not** install its +> default approval hook. In that case **you are fully responsible** for enforcing approval — including for any +> `ApprovalRequiredAIFunction` you register (e.g. by returning a `"deny"` or `"ask"` `PreToolUseHookOutput`). The +> agent logs a warning when it detects an approval-required tool that your hook must handle. + ## Streaming Responses To get streaming responses: diff --git a/dotnet/src/Microsoft.Agents.AI.GitHub.Copilot/GitHubCopilotAgent.cs b/dotnet/src/Microsoft.Agents.AI.GitHub.Copilot/GitHubCopilotAgent.cs index 53834bad754..f6731fc8efd 100644 --- a/dotnet/src/Microsoft.Agents.AI.GitHub.Copilot/GitHubCopilotAgent.cs +++ b/dotnet/src/Microsoft.Agents.AI.GitHub.Copilot/GitHubCopilotAgent.cs @@ -12,6 +12,8 @@ using System.Threading.Tasks; using GitHub.Copilot; using Microsoft.Extensions.AI; +using Microsoft.Extensions.Logging; +using Microsoft.Extensions.Logging.Abstractions; using Microsoft.Shared.Diagnostics; namespace Microsoft.Agents.AI.GitHub.Copilot; @@ -31,6 +33,7 @@ public sealed class GitHubCopilotAgent : AIAgent, IAsyncDisposable private readonly SessionConfig? _sessionConfig; private readonly bool _ownsClient; private readonly JsonSerializerOptions _jsonSerializerOptions; + private readonly ILogger _logger; /// /// Initializes a new instance of the class. @@ -42,12 +45,15 @@ public sealed class GitHubCopilotAgent : AIAgent, IAsyncDisposable /// The name of the agent. /// The description of the agent. /// Optional JSON serializer options. Defaults to . - /// - /// Optional callback that approves or denies execution of custom function tools wrapped in - /// ; it must return to allow the call. - /// When not configured, or when it denies or throws, such tools are denied (secure-by-default). This is the - /// agent-level approval gate for this provider, independent of SessionConfig.OnPermissionRequest. - /// + /// Optional logger factory used to create the agent's logger. + /// + /// When a tool wrapped in is registered and the supplied + /// does not already define a Hooks.OnPreToolUse handler, the agent installs a + /// default OnPreToolUse hook that returns "ask" for those tools (routing the decision to + /// SessionConfig.OnPermissionRequest) and defers all other tools. If the caller supplies their own + /// OnPreToolUse hook, it takes precedence and the caller is fully responsible for approval handling; in that + /// case a warning is logged for any approval-required tool that will not be automatically gated. + /// public GitHubCopilotAgent( CopilotClient copilotClient, SessionConfig? sessionConfig = null, @@ -56,12 +62,13 @@ public GitHubCopilotAgent( string? name = null, string? description = null, JsonSerializerOptions? jsonSerializerOptions = null, - Func>? onFunctionApproval = null) + ILoggerFactory? loggerFactory = null) { _ = Throw.IfNull(copilotClient); this._copilotClient = copilotClient; - this._sessionConfig = WrapApprovalRequiredTools(sessionConfig, onFunctionApproval); + this._logger = (loggerFactory ?? NullLoggerFactory.Instance).CreateLogger(); + this._sessionConfig = ConfigureApprovalHook(sessionConfig, this._logger); this._ownsClient = ownsClient; this._id = id; this._name = name ?? DefaultName; @@ -80,11 +87,12 @@ public GitHubCopilotAgent( /// The tools to make available to the agent. /// Optional instructions to append as a system message. /// Optional JSON serializer options. Defaults to . - /// - /// Optional callback that approves or denies execution of custom function tools wrapped in - /// ; it must return to allow the call. - /// When not configured, or when it denies or throws, such tools are denied (secure-by-default). - /// + /// Optional logger factory used to create the agent's logger. + /// + /// When a tool wrapped in is registered, the agent installs a default + /// Hooks.OnPreToolUse handler that returns "ask" for those tools (routing the decision to + /// SessionConfig.OnPermissionRequest) and defers all other tools. + /// public GitHubCopilotAgent( CopilotClient copilotClient, bool ownsClient = false, @@ -94,7 +102,7 @@ public GitHubCopilotAgent( IList? tools = null, string? instructions = null, JsonSerializerOptions? jsonSerializerOptions = null, - Func>? onFunctionApproval = null) + ILoggerFactory? loggerFactory = null) : this( copilotClient, GetSessionConfig(tools, instructions), @@ -103,7 +111,7 @@ public GitHubCopilotAgent( name, description, jsonSerializerOptions, - onFunctionApproval) + loggerFactory) { } @@ -534,46 +542,90 @@ private AgentResponseUpdate ConvertToAgentResponseUpdate(SessionEvent sessionEve } /// - /// Returns a in which every tool wrapped in - /// is replaced with an , so the GitHub Copilot SDK tool loop cannot invoke - /// an approval-required function without first passing the Agent Framework approval callback. + /// Installs a default OnPreToolUse hook that gates tools wrapped in + /// by returning "ask" (routing the decision to SessionConfig.OnPermissionRequest) while deferring all + /// other tools, so the GitHub Copilot SDK enforces approval through its native pre-tool-use hook. /// /// /// The source is returned unchanged when it contains no approval-required tools. - /// Otherwise a clone is returned so the caller-supplied configuration is not mutated. + /// If the caller already supplied a Hooks.OnPreToolUse handler, it takes precedence and is left untouched; a + /// warning is logged for any approval-required tool that will therefore not be automatically gated. Otherwise a + /// clone is returned (with a fresh ) so the caller-supplied configuration is not mutated. /// - private static SessionConfig? WrapApprovalRequiredTools( - SessionConfig? sessionConfig, - Func>? onFunctionApproval) + private static SessionConfig? ConfigureApprovalHook(SessionConfig? sessionConfig, ILogger logger) { if (sessionConfig?.Tools is not { Count: > 0 } tools) { return sessionConfig; } - List mappedTools = new(tools.Count); - bool wrappedAny = false; + List approvalRequiredToolNames = []; foreach (AIFunctionDeclaration tool in tools) { if (tool is AIFunction function && function.GetService() is not null) { - mappedTools.Add(new ApprovalGatedAIFunction(function, onFunctionApproval)); - wrappedAny = true; - } - else - { - mappedTools.Add(tool); + approvalRequiredToolNames.Add(function.Name); } } - if (!wrappedAny) + if (approvalRequiredToolNames.Count == 0) { return sessionConfig; } - SessionConfig wrapped = sessionConfig.Clone(); - wrapped.Tools = mappedTools; - return wrapped; + // A caller-supplied OnPreToolUse hook takes precedence and is fully responsible for approval handling. + // Warn so the developer knows the ApprovalRequiredAIFunction marker(s) will not be automatically gated. + if (sessionConfig.Hooks?.OnPreToolUse is not null) + { + logger.LogWarning( + "A custom 'OnPreToolUse' hook is configured on the SessionConfig, so {Count} approval-required tool(s) ({Tools}) " + + "will not be automatically gated by GitHubCopilotAgent. The custom hook is responsible for enforcing approval " + + "(for example, by returning a 'deny' or 'ask' PreToolUseHookOutput).", + approvalRequiredToolNames.Count, + string.Join(", ", approvalRequiredToolNames)); + return sessionConfig; + } + + HashSet approvalRequiredNames = new(approvalRequiredToolNames, StringComparer.Ordinal); + + SessionConfig configured = sessionConfig.Clone(); + + // SessionConfig.Clone() shallow-copies Hooks, so build a fresh SessionHooks (preserving any other hooks) + // to avoid mutating the caller's instance when setting OnPreToolUse. + SessionHooks hooks = CloneHooks(configured.Hooks); + hooks.OnPreToolUse = (input, invocation) => + Task.FromResult( + approvalRequiredNames.Contains(input.ToolName) + ? new PreToolUseHookOutput + { + PermissionDecision = "ask", + PermissionDecisionReason = $"Tool '{input.ToolName}' is marked as requiring approval (ApprovalRequiredAIFunction).", + } + : null); + configured.Hooks = hooks; + + return configured; + } + + /// + /// Creates a shallow copy of a instance, preserving all configured hook delegates. + /// + private static SessionHooks CloneHooks(SessionHooks? source) + { + SessionHooks clone = new(); + if (source is not null) + { + clone.OnPreToolUse = source.OnPreToolUse; + clone.OnPreMcpToolCall = source.OnPreMcpToolCall; + clone.OnPostToolUse = source.OnPostToolUse; + clone.OnPostToolUseFailure = source.OnPostToolUseFailure; + clone.OnUserPromptSubmitted = source.OnUserPromptSubmitted; + clone.OnSessionStart = source.OnSessionStart; + clone.OnSessionEnd = source.OnSessionEnd; + clone.OnErrorOccurred = source.OnErrorOccurred; + } + + return clone; } private static async Task<(List? Attachments, string? TempDir)> ProcessDataContentAttachmentsAsync( @@ -620,82 +672,4 @@ private static void CleanupTempDir(string? tempDir) } } } - - /// - /// An decorator that enforces an Agent Framework function-approval check - /// before invoking a tool wrapped in . - /// - /// - /// - /// The GitHub Copilot SDK owns the model tool-calling loop and invokes registered custom functions - /// directly, so the standard FunctionInvokingChatClient approval round-trip (emitting a - /// ToolApprovalRequestContent and waiting for a ToolApprovalResponseContent) never runs - /// for this provider. As a result, an — which is only a marker - /// and does not enforce approval on its own — would otherwise execute without any Agent Framework approval. - /// - /// - /// This decorator restores the approval boundary: the underlying function is only invoked after the - /// configured approval callback explicitly approves the call. When no callback is configured, or the - /// callback denies or throws, execution is denied (secure-by-default). The decorator forwards all tool - /// metadata (name, description, JSON schema, and additional properties such as the Copilot - /// skip_permission flag) from the inner function so the tool remains transparent to the SDK. - /// - /// - private sealed class ApprovalGatedAIFunction : DelegatingAIFunction - { - private readonly Func>? _onFunctionApproval; - - public ApprovalGatedAIFunction( - AIFunction innerFunction, - Func>? onFunctionApproval) - : base(innerFunction) - { - this._onFunctionApproval = onFunctionApproval; - } - - protected override async ValueTask InvokeCoreAsync( - AIFunctionArguments arguments, - CancellationToken cancellationToken) - { - if (!await this.IsApprovedAsync(arguments, cancellationToken).ConfigureAwait(false)) - { - return this._onFunctionApproval is null - ? $"Tool '{this.Name}' requires human approval but no approval callback is configured on the agent; the request was denied." - : $"Tool '{this.Name}' requires human approval and the request was denied."; - } - - return await base.InvokeCoreAsync(arguments, cancellationToken).ConfigureAwait(false); - } - - private async ValueTask IsApprovedAsync(AIFunctionArguments arguments, CancellationToken cancellationToken) - { - // Secure-by-default: with no callback configured an approval-required tool can never execute. - if (this._onFunctionApproval is null) - { - return false; - } - - FunctionCallContent request = new( - callId: $"github-copilot-approval::{this.Name}", - name: this.Name, - arguments: arguments); - - try - { - return await this._onFunctionApproval(request, cancellationToken).ConfigureAwait(false); - } - catch (OperationCanceledException) - { - // Honor cooperative cancellation rather than treating it as a denial. - throw; - } -#pragma warning disable CA1031 // Do not catch general exception types - catch (Exception) -#pragma warning restore CA1031 - { - // Secure-by-default: any other failure in the approval callback denies execution. - return false; - } - } - } } diff --git a/dotnet/tests/Microsoft.Agents.AI.GitHub.Copilot.IntegrationTests/GitHubCopilotAgentTests.cs b/dotnet/tests/Microsoft.Agents.AI.GitHub.Copilot.IntegrationTests/GitHubCopilotAgentTests.cs index 5e4a4fc0ca6..c1599012f09 100644 --- a/dotnet/tests/Microsoft.Agents.AI.GitHub.Copilot.IntegrationTests/GitHubCopilotAgentTests.cs +++ b/dotnet/tests/Microsoft.Agents.AI.GitHub.Copilot.IntegrationTests/GitHubCopilotAgentTests.cs @@ -3,7 +3,6 @@ using System; using System.Collections.Generic; using System.Linq; -using System.Threading; using System.Threading.Tasks; using GitHub.Copilot; using GitHub.Copilot.Rpc; @@ -133,13 +132,13 @@ public async Task RunAsync_WithFunctionTool_InvokesToolAsync() } [Fact] - public async Task RunAsync_WithApprovalRequiredTool_ExecutesWhenCallbackApprovesAsync() + public async Task RunAsync_WithApprovalRequiredTool_AsksAndExecutesWhenPermissionApprovesAsync() { // Arrange SkipIfCopilotNotConfigured(); bool toolInvoked = false; - bool approvalRequested = false; + bool permissionRequested = false; AIFunction weatherTool = AIFunctionFactory.Create((string location) => { @@ -148,10 +147,10 @@ public async Task RunAsync_WithApprovalRequiredTool_ExecutesWhenCallbackApproves }, "GetWeather", "Get the weather for a given location."); ApprovalRequiredAIFunction approvalRequiredTool = new(weatherTool); - ValueTask approveAsync(FunctionCallContent request, CancellationToken cancellationToken) + Task OnPermissionRequestRecordingAsync(PermissionRequest request, PermissionInvocation invocation) { - approvalRequested = true; - return new(true); + permissionRequested = true; + return Task.FromResult(PermissionDecision.ApproveOnce()); } await using CopilotClient client = new(new CopilotClientOptions()); @@ -160,7 +159,7 @@ ValueTask approveAsync(FunctionCallContent request, CancellationToken canc SessionConfig sessionConfig = new() { Tools = [approvalRequiredTool], - OnPermissionRequest = OnPermissionRequestAsync, + OnPermissionRequest = OnPermissionRequestRecordingAsync, SystemMessage = new SystemMessageConfig { Mode = SystemMessageMode.Append, @@ -168,7 +167,7 @@ ValueTask approveAsync(FunctionCallContent request, CancellationToken canc }, }; - await using GitHubCopilotAgent agent = new(client, sessionConfig, onFunctionApproval: approveAsync); + await using GitHubCopilotAgent agent = new(client, sessionConfig); AgentSession session = await agent.CreateSessionAsync(); try @@ -176,11 +175,11 @@ ValueTask approveAsync(FunctionCallContent request, CancellationToken canc // Act AgentResponse response = await agent.RunAsync("What's the weather like in Seattle?", session); - // Assert - the SDK tool loop must dispatch through the Agent Framework approval gate, - // and the underlying function only runs because the callback approved the call. + // Assert - the provider-installed OnPreToolUse returns "ask" for the approval-required tool, routing the + // decision to OnPermissionRequest; the tool only runs because the permission handler approved it. Assert.NotNull(response); Assert.NotEmpty(response.Messages); - Assert.True(approvalRequested); + Assert.True(permissionRequested); Assert.True(toolInvoked); } finally diff --git a/dotnet/tests/Microsoft.Agents.AI.GitHub.Copilot.UnitTests/GitHubCopilotAgentTests.cs b/dotnet/tests/Microsoft.Agents.AI.GitHub.Copilot.UnitTests/GitHubCopilotAgentTests.cs index f8a73b86427..fe632a0d796 100644 --- a/dotnet/tests/Microsoft.Agents.AI.GitHub.Copilot.UnitTests/GitHubCopilotAgentTests.cs +++ b/dotnet/tests/Microsoft.Agents.AI.GitHub.Copilot.UnitTests/GitHubCopilotAgentTests.cs @@ -3,11 +3,11 @@ using System; using System.Collections.Generic; using System.Linq; -using System.Threading; using System.Threading.Tasks; using GitHub.Copilot; using GitHub.Copilot.Rpc; using Microsoft.Extensions.AI; +using Microsoft.Extensions.Logging; namespace Microsoft.Agents.AI.GitHub.Copilot.UnitTests; @@ -385,181 +385,141 @@ public void ConvertToAgentResponseUpdate_AssistantMessageEventWhenNotStreaming_H } [Fact] - public async Task Constructor_WithApprovalRequiredTool_GatesExecutionAndDeniesWithoutCallbackAsync() + public async Task Constructor_WithApprovalRequiredTool_InstallsAskPreToolUseHookAsync() { // Arrange - bool invoked = false; - AIFunction dangerousTool = AIFunctionFactory.Create( - () => { invoked = true; return "sensitive operation completed"; }, - "ApprovalRequiredOperation", - "Performs an approval-required operation."); - ApprovalRequiredAIFunction approvalRequiredTool = new(dangerousTool); - + AIFunction dangerousTool = AIFunctionFactory.Create(() => "sensitive", "ApprovalRequiredOperation", "A sensitive operation."); + AIFunction plainTool = AIFunctionFactory.Create(() => "ok", "PlainOperation", "A normal operation."); CopilotClient copilotClient = new(new CopilotClientOptions()); - var agent = new GitHubCopilotAgent(copilotClient, tools: [approvalRequiredTool]); // Act - AIFunction exposedTool = GetExposedFunction(agent); - object? result = await exposedTool.InvokeAsync(new AIFunctionArguments()); - - // Assert - the provider must NOT expose the same directly-invokable approval-required object, - // and invoking the gate without an approval callback must deny execution. - Assert.NotSame(approvalRequiredTool, exposedTool); - Assert.False(invoked); - Assert.Contains("requires human approval", result?.ToString()); - } - - [Fact] - public async Task Constructor_WithApprovalRequiredTool_ExecutesWhenCallbackApprovesAsync() - { - // Arrange - bool invoked = false; - AIFunction dangerousTool = AIFunctionFactory.Create( - () => { invoked = true; return "sensitive operation completed"; }, - "ApprovalRequiredOperation", - "Performs an approval-required operation."); - ApprovalRequiredAIFunction approvalRequiredTool = new(dangerousTool); + var agent = new GitHubCopilotAgent(copilotClient, tools: [new ApprovalRequiredAIFunction(dangerousTool), plainTool]); - FunctionCallContent? observedRequest = null; - ValueTask approveAsync(FunctionCallContent request, CancellationToken _) { observedRequest = request; return new(true); } - - CopilotClient copilotClient = new(new CopilotClientOptions()); - var agent = new GitHubCopilotAgent(copilotClient, tools: [approvalRequiredTool], onFunctionApproval: approveAsync); + // Assert - the provider installs a default OnPreToolUse that asks for the approval-required tool + // (routing the decision to OnPermissionRequest) and defers everything else. + SessionConfig sessionConfig = GetSessionConfigFromAgent(agent); + Assert.NotNull(sessionConfig.Hooks?.OnPreToolUse); - // Act - AIFunction exposedTool = GetExposedFunction(agent); - object? result = await exposedTool.InvokeAsync(new AIFunctionArguments()); + PreToolUseHookOutput? approvalDecision = await InvokePreToolUseAsync(sessionConfig, "ApprovalRequiredOperation"); + Assert.Equal("ask", approvalDecision?.PermissionDecision); + Assert.False(string.IsNullOrEmpty(approvalDecision?.PermissionDecisionReason)); - // Assert - Assert.True(invoked); - Assert.NotNull(observedRequest); - Assert.Equal("ApprovalRequiredOperation", observedRequest!.Name); - Assert.Contains("sensitive operation completed", result?.ToString()); + PreToolUseHookOutput? plainDecision = await InvokePreToolUseAsync(sessionConfig, "PlainOperation"); + Assert.Null(plainDecision); } [Fact] - public async Task Constructor_WithApprovalRequiredTool_DeniesWhenCallbackRejectsAsync() + public async Task Constructor_WithApprovalRequiredToolInSessionConfig_InstallsAskPreToolUseHookAsync() { // Arrange - bool invoked = false; - AIFunction dangerousTool = AIFunctionFactory.Create( - () => { invoked = true; return "sensitive operation completed"; }, - "ApprovalRequiredOperation", - "Performs an approval-required operation."); - ApprovalRequiredAIFunction approvalRequiredTool = new(dangerousTool); - - ValueTask denyAsync(FunctionCallContent request, CancellationToken cancellationToken) => new(false); - + AIFunction dangerousTool = AIFunctionFactory.Create(() => "sensitive", "ApprovalRequiredOperation", "A sensitive operation."); + SessionConfig sessionConfig = new() { Tools = [new ApprovalRequiredAIFunction(dangerousTool)] }; CopilotClient copilotClient = new(new CopilotClientOptions()); - var agent = new GitHubCopilotAgent(copilotClient, tools: [approvalRequiredTool], onFunctionApproval: denyAsync); // Act - AIFunction exposedTool = GetExposedFunction(agent); - object? result = await exposedTool.InvokeAsync(new AIFunctionArguments()); + var agent = new GitHubCopilotAgent(copilotClient, sessionConfig); // Assert - Assert.False(invoked); - Assert.Contains("the request was denied", result?.ToString()); + SessionConfig configured = GetSessionConfigFromAgent(agent); + Assert.NotNull(configured.Hooks?.OnPreToolUse); + PreToolUseHookOutput? decision = await InvokePreToolUseAsync(configured, "ApprovalRequiredOperation"); + Assert.Equal("ask", decision?.PermissionDecision); } [Fact] - public async Task Constructor_WithApprovalRequiredTool_DeniesWhenCallbackThrowsAsync() + public void Constructor_WithNoApprovalRequiredTools_DoesNotInstallPreToolUseHook() { // Arrange - bool invoked = false; - AIFunction dangerousTool = AIFunctionFactory.Create( - () => { invoked = true; return "sensitive operation completed"; }, - "ApprovalRequiredOperation", - "Performs an approval-required operation."); - ApprovalRequiredAIFunction approvalRequiredTool = new(dangerousTool); - - ValueTask throwingAsync(FunctionCallContent request, CancellationToken cancellationToken) => throw new InvalidOperationException("callback failure"); - + AIFunction plainTool = AIFunctionFactory.Create(() => "ok", "PlainOperation", "A normal operation."); CopilotClient copilotClient = new(new CopilotClientOptions()); - var agent = new GitHubCopilotAgent(copilotClient, tools: [approvalRequiredTool], onFunctionApproval: throwingAsync); // Act - AIFunction exposedTool = GetExposedFunction(agent); - object? result = await exposedTool.InvokeAsync(new AIFunctionArguments()); + var agent = new GitHubCopilotAgent(copilotClient, tools: [plainTool]); - // Assert - secure-by-default: a throwing callback denies execution rather than propagating. - Assert.False(invoked); - Assert.Contains("the request was denied", result?.ToString()); + // Assert - no approval-required tools means no hook is installed; tools flow through normally. + SessionConfig sessionConfig = GetSessionConfigFromAgent(agent); + Assert.Null(sessionConfig.Hooks?.OnPreToolUse); } [Fact] - public async Task Constructor_WithApprovalRequiredTool_PropagatesCancellationAsync() + public async Task Constructor_WithUserPreToolUseHook_PreservesItAndWarnsAsync() { - // Arrange - bool invoked = false; - AIFunction dangerousTool = AIFunctionFactory.Create( - () => { invoked = true; return "sensitive operation completed"; }, - "ApprovalRequiredOperation", - "Performs an approval-required operation."); - ApprovalRequiredAIFunction approvalRequiredTool = new(dangerousTool); - - ValueTask cancelingAsync(FunctionCallContent request, CancellationToken cancellationToken) - => throw new OperationCanceledException(cancellationToken); + // Arrange - the caller supplies their own OnPreToolUse hook and also registers an approval-required tool. + AIFunction dangerousTool = AIFunctionFactory.Create(() => "sensitive", "ApprovalRequiredOperation", "A sensitive operation."); + var userHookOutput = new PreToolUseHookOutput { PermissionDecision = "allow" }; + SessionConfig sessionConfig = new() + { + Tools = [new ApprovalRequiredAIFunction(dangerousTool)], + Hooks = new SessionHooks + { + OnPreToolUse = (input, invocation) => Task.FromResult(userHookOutput), + }, + }; + CapturingLoggerFactory loggerFactory = new(); CopilotClient copilotClient = new(new CopilotClientOptions()); - var agent = new GitHubCopilotAgent(copilotClient, tools: [approvalRequiredTool], onFunctionApproval: cancelingAsync); - // Act & Assert - cancellation must propagate rather than being swallowed into a denial. - AIFunction exposedTool = GetExposedFunction(agent); - await Assert.ThrowsAsync( - async () => await exposedTool.InvokeAsync(new AIFunctionArguments())); - Assert.False(invoked); + // Act + var agent = new GitHubCopilotAgent(copilotClient, sessionConfig, loggerFactory: loggerFactory); + + // Assert - the user's hook is preserved (not overridden), and a warning is logged for the unenforced tool. + SessionConfig configured = GetSessionConfigFromAgent(agent); + PreToolUseHookOutput? decision = await InvokePreToolUseAsync(configured, "ApprovalRequiredOperation"); + Assert.Same(userHookOutput, decision); + + (LogLevel Level, string Message) warning = Assert.Single(loggerFactory.Entries, e => e.Level == LogLevel.Warning); + Assert.Contains("ApprovalRequiredOperation", warning.Message); } [Fact] - public void Constructor_WithApprovalRequiredTool_PreservesSkipPermissionMetadata() + public void Constructor_WithUserPreToolUseHookButNoApprovalRequiredTools_DoesNotWarn() { // Arrange - AIFunction dangerousTool = CopilotTool.DefineTool( - () => "ok", - toolOptions: new CopilotToolOptions { SkipPermission = true }, - factoryOptions: new AIFunctionFactoryOptions + AIFunction plainTool = AIFunctionFactory.Create(() => "ok", "PlainOperation", "A normal operation."); + SessionConfig sessionConfig = new() + { + Tools = [plainTool], + Hooks = new SessionHooks { - Name = "WriteSensitiveMarker", - Description = "Writes a sensitive marker file." - }); - ApprovalRequiredAIFunction approvalRequiredTool = new(dangerousTool); + OnPreToolUse = (input, invocation) => Task.FromResult(null), + }, + }; + CapturingLoggerFactory loggerFactory = new(); CopilotClient copilotClient = new(new CopilotClientOptions()); - var agent = new GitHubCopilotAgent(copilotClient, tools: [approvalRequiredTool]); // Act - AIFunction exposedTool = GetExposedFunction(agent); - - // Assert - the gate must remain transparent to the Copilot SDK, preserving the skip_permission flag. - Assert.NotSame(approvalRequiredTool, exposedTool); - Assert.Equal("WriteSensitiveMarker", exposedTool.Name); - Assert.NotNull(exposedTool.AdditionalProperties); - Assert.True(exposedTool.AdditionalProperties.TryGetValue("skip_permission", out object? skip)); - Assert.Equal(true, skip); + var agent = new GitHubCopilotAgent(copilotClient, sessionConfig, loggerFactory: loggerFactory); + + // Assert - nothing is being bypassed, so no warning is emitted. + Assert.DoesNotContain(loggerFactory.Entries, e => e.Level == LogLevel.Warning); } [Fact] - public void Constructor_WithNonApprovalTool_LeavesToolUnchanged() + public void Constructor_WithApprovalRequiredTool_DoesNotMutateCallerHooks() { // Arrange - AIFunction plainTool = AIFunctionFactory.Create(() => "test", "TestFunc", "Test function"); - + AIFunction dangerousTool = AIFunctionFactory.Create(() => "sensitive", "ApprovalRequiredOperation", "A sensitive operation."); + SessionHooks callerHooks = new(); + SessionConfig sessionConfig = new() + { + Tools = [new ApprovalRequiredAIFunction(dangerousTool)], + Hooks = callerHooks, + }; CopilotClient copilotClient = new(new CopilotClientOptions()); - var agent = new GitHubCopilotAgent(copilotClient, tools: [plainTool]); // Act - AIFunction exposedTool = GetExposedFunction(agent); + _ = new GitHubCopilotAgent(copilotClient, sessionConfig); - // Assert - tools that do not require approval pass through untouched. - Assert.Same(plainTool, exposedTool); + // Assert - the caller-supplied SessionConfig and its Hooks instance are not mutated. + Assert.Null(callerHooks.OnPreToolUse); + Assert.Same(callerHooks, sessionConfig.Hooks); } - private static AIFunction GetExposedFunction(GitHubCopilotAgent agent) + private static Task InvokePreToolUseAsync(SessionConfig sessionConfig, string toolName) { - SessionConfig sessionConfig = GetSessionConfigFromAgent(agent); - AIFunctionDeclaration declaration = Assert.Single(sessionConfig.Tools!); - return declaration.GetService()!; + var input = new PreToolUseHookInput { ToolName = toolName }; + return sessionConfig.Hooks!.OnPreToolUse!(input, new HookInvocation()); } private static SessionConfig GetSessionConfigFromAgent(GitHubCopilotAgent agent) @@ -569,4 +529,29 @@ private static SessionConfig GetSessionConfigFromAgent(GitHubCopilotAgent agent) System.Reflection.BindingFlags.Instance | System.Reflection.BindingFlags.NonPublic)!; return (SessionConfig)field.GetValue(agent)!; } + + private sealed class CapturingLoggerFactory : ILoggerFactory + { + public List<(LogLevel Level, string Message)> Entries { get; } = []; + + public void AddProvider(ILoggerProvider provider) + { + } + + public ILogger CreateLogger(string categoryName) => new CapturingLogger(this.Entries); + + public void Dispose() + { + } + + private sealed class CapturingLogger(List<(LogLevel Level, string Message)> entries) : ILogger + { + public IDisposable? BeginScope(TState state) where TState : notnull => null; + + public bool IsEnabled(LogLevel logLevel) => true; + + public void Log(LogLevel logLevel, EventId eventId, TState state, Exception? exception, Func formatter) + => entries.Add((logLevel, formatter(state, exception))); + } + } } From bd3bc03289f7eb9e957589a2b9bb238e4e029ae7 Mon Sep 17 00:00:00 2001 From: Giles Odigwe Date: Fri, 26 Jun 2026 11:15:30 -0700 Subject: [PATCH 4/4] .NET: Address PR review feedback on GitHub Copilot approval hook - Build the approval-required tool-name HashSet directly instead of via an intermediate List. - Remove the redundant MEAI001 NoWarn suppression (tests already suppress it via .editorconfig and the source project builds clean without it). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../GitHubCopilotAgent.cs | 6 ++---- .../Microsoft.Agents.AI.GitHub.Copilot.csproj | 2 +- ...crosoft.Agents.AI.GitHub.Copilot.IntegrationTests.csproj | 2 +- .../Microsoft.Agents.AI.GitHub.Copilot.UnitTests.csproj | 2 +- 4 files changed, 5 insertions(+), 7 deletions(-) diff --git a/dotnet/src/Microsoft.Agents.AI.GitHub.Copilot/GitHubCopilotAgent.cs b/dotnet/src/Microsoft.Agents.AI.GitHub.Copilot/GitHubCopilotAgent.cs index f6731fc8efd..dc057097857 100644 --- a/dotnet/src/Microsoft.Agents.AI.GitHub.Copilot/GitHubCopilotAgent.cs +++ b/dotnet/src/Microsoft.Agents.AI.GitHub.Copilot/GitHubCopilotAgent.cs @@ -559,7 +559,7 @@ private AgentResponseUpdate ConvertToAgentResponseUpdate(SessionEvent sessionEve return sessionConfig; } - List approvalRequiredToolNames = []; + HashSet approvalRequiredToolNames = new(StringComparer.Ordinal); foreach (AIFunctionDeclaration tool in tools) { if (tool is AIFunction function && function.GetService() is not null) @@ -586,8 +586,6 @@ private AgentResponseUpdate ConvertToAgentResponseUpdate(SessionEvent sessionEve return sessionConfig; } - HashSet approvalRequiredNames = new(approvalRequiredToolNames, StringComparer.Ordinal); - SessionConfig configured = sessionConfig.Clone(); // SessionConfig.Clone() shallow-copies Hooks, so build a fresh SessionHooks (preserving any other hooks) @@ -595,7 +593,7 @@ private AgentResponseUpdate ConvertToAgentResponseUpdate(SessionEvent sessionEve SessionHooks hooks = CloneHooks(configured.Hooks); hooks.OnPreToolUse = (input, invocation) => Task.FromResult( - approvalRequiredNames.Contains(input.ToolName) + approvalRequiredToolNames.Contains(input.ToolName) ? new PreToolUseHookOutput { PermissionDecision = "ask", diff --git a/dotnet/src/Microsoft.Agents.AI.GitHub.Copilot/Microsoft.Agents.AI.GitHub.Copilot.csproj b/dotnet/src/Microsoft.Agents.AI.GitHub.Copilot/Microsoft.Agents.AI.GitHub.Copilot.csproj index 4feccdc691a..dad0b31f59d 100644 --- a/dotnet/src/Microsoft.Agents.AI.GitHub.Copilot/Microsoft.Agents.AI.GitHub.Copilot.csproj +++ b/dotnet/src/Microsoft.Agents.AI.GitHub.Copilot/Microsoft.Agents.AI.GitHub.Copilot.csproj @@ -4,7 +4,7 @@ true $(TargetFrameworksCore) - $(NoWarn);GHCP001;MEAI001 + $(NoWarn);GHCP001 diff --git a/dotnet/tests/Microsoft.Agents.AI.GitHub.Copilot.IntegrationTests/Microsoft.Agents.AI.GitHub.Copilot.IntegrationTests.csproj b/dotnet/tests/Microsoft.Agents.AI.GitHub.Copilot.IntegrationTests/Microsoft.Agents.AI.GitHub.Copilot.IntegrationTests.csproj index 22f15d517cc..3fd692527bb 100644 --- a/dotnet/tests/Microsoft.Agents.AI.GitHub.Copilot.IntegrationTests/Microsoft.Agents.AI.GitHub.Copilot.IntegrationTests.csproj +++ b/dotnet/tests/Microsoft.Agents.AI.GitHub.Copilot.IntegrationTests/Microsoft.Agents.AI.GitHub.Copilot.IntegrationTests.csproj @@ -3,7 +3,7 @@ $(TargetFrameworksCore) - $(NoWarn);GHCP001;MEAI001 + $(NoWarn);GHCP001 diff --git a/dotnet/tests/Microsoft.Agents.AI.GitHub.Copilot.UnitTests/Microsoft.Agents.AI.GitHub.Copilot.UnitTests.csproj b/dotnet/tests/Microsoft.Agents.AI.GitHub.Copilot.UnitTests/Microsoft.Agents.AI.GitHub.Copilot.UnitTests.csproj index d1bd009f80d..185130f9f39 100644 --- a/dotnet/tests/Microsoft.Agents.AI.GitHub.Copilot.UnitTests/Microsoft.Agents.AI.GitHub.Copilot.UnitTests.csproj +++ b/dotnet/tests/Microsoft.Agents.AI.GitHub.Copilot.UnitTests/Microsoft.Agents.AI.GitHub.Copilot.UnitTests.csproj @@ -3,7 +3,7 @@ $(TargetFrameworksCore) - $(NoWarn);GHCP001;MEAI001 + $(NoWarn);GHCP001