From 5ce6410cbc036dd0df46c2403a5591e9df3650c8 Mon Sep 17 00:00:00 2001 From: Peter Ibekwe Date: Fri, 29 May 2026 10:29:55 -0700 Subject: [PATCH] Fix InvokeMcpTool approval path for declarative workflows --- .../ObjectModel/InvokeMcpToolExecutor.cs | 10 +- .../ObjectModel/InvokeMcpToolExecutorTest.cs | 96 +++++++++++++++++++ 2 files changed, 99 insertions(+), 7 deletions(-) diff --git a/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative/ObjectModel/InvokeMcpToolExecutor.cs b/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative/ObjectModel/InvokeMcpToolExecutor.cs index 7796a6f409..c4c490551a 100644 --- a/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative/ObjectModel/InvokeMcpToolExecutor.cs +++ b/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative/ObjectModel/InvokeMcpToolExecutor.cs @@ -75,18 +75,14 @@ internal sealed class InvokeMcpToolExecutor( if (requireApproval) { - // Create tool call content for approval request + // Create tool call content for approval request. + // Transport headers (e.g. Authorization) are intentionally excluded from the + // approval event: they must not cross into the externally-surfaced approval request. McpServerToolCallContent toolCall = new(this.Id, toolName, serverLabel ?? serverUrl) { Arguments = arguments }; - if (headers != null) - { - toolCall.AdditionalProperties ??= []; - toolCall.AdditionalProperties.Add(headers); - } - ToolApprovalRequestContent approvalRequest = new(this.Id, toolCall); ChatMessage requestMessage = new(ChatRole.Assistant, [approvalRequest]); diff --git a/dotnet/tests/Microsoft.Agents.AI.Workflows.Declarative.UnitTests/ObjectModel/InvokeMcpToolExecutorTest.cs b/dotnet/tests/Microsoft.Agents.AI.Workflows.Declarative.UnitTests/ObjectModel/InvokeMcpToolExecutorTest.cs index d047badaf7..b8e081f239 100644 --- a/dotnet/tests/Microsoft.Agents.AI.Workflows.Declarative.UnitTests/ObjectModel/InvokeMcpToolExecutorTest.cs +++ b/dotnet/tests/Microsoft.Agents.AI.Workflows.Declarative.UnitTests/ObjectModel/InvokeMcpToolExecutorTest.cs @@ -1,9 +1,11 @@ // Copyright (c) Microsoft. All rights reserved. using System.Collections.Generic; +using System.Linq; using System.Threading; using System.Threading.Tasks; using Microsoft.Agents.AI.Workflows.Declarative.Events; +using Microsoft.Agents.AI.Workflows.Declarative.Interpreter; using Microsoft.Agents.AI.Workflows.Declarative.Kit; using Microsoft.Agents.AI.Workflows.Declarative.ObjectModel; using Microsoft.Agents.AI.Workflows.Declarative.PowerFx; @@ -290,6 +292,100 @@ public sealed class InvokeMcpToolExecutorTest(ITestOutputHelper output) : Workfl await this.ExecuteTestAsync(model); } + [Fact] + public async Task InvokeMcpToolApprovalRequestExcludesTransportHeadersAsync() + { + // Arrange + this.State.InitializeSystem(); + InvokeMcpTool model = this.CreateModel( + displayName: nameof(InvokeMcpToolApprovalRequestExcludesTransportHeadersAsync), + serverUrl: TestServerUrl, + serverLabel: TestServerLabel, + toolName: TestToolName, + requireApproval: true, + headerKey: "Authorization", + headerValue: "Bearer super-secret-token"); + MockMcpToolProvider mockProvider = new(); + MockAgentProvider mockAgentProvider = new(); + InvokeMcpToolExecutor action = new(model, mockProvider.Object, mockAgentProvider.Object, this.State); + + ExternalInputRequest? capturedRequest = null; + + // Act + await this.ExecuteAsync( + [ + action, + new DelegateActionExecutor( + InvokeMcpToolExecutor.Steps.ExternalInput(action.Id), + this.State, + CaptureRequestAsync) + ], + isDiscrete: false); + + // Assert - the approval event must not carry any transport headers (e.g. Authorization). + Assert.NotNull(capturedRequest); + ToolApprovalRequestContent approvalRequest = + capturedRequest!.AgentResponse.Messages + .SelectMany(message => message.Contents) + .OfType() + .Single(); + + AdditionalPropertiesDictionary? additionalProperties = approvalRequest.ToolCall.AdditionalProperties; + Assert.True(additionalProperties is null || additionalProperties.Count == 0); + + // Defense in depth: the credential value must not appear anywhere in the serialized approval content. + string serializedApproval = System.Text.Json.JsonSerializer.Serialize(capturedRequest.AgentResponse); + Assert.DoesNotContain("super-secret-token", serializedApproval); + + ValueTask CaptureRequestAsync(IWorkflowContext context, ExternalInputRequest request, CancellationToken cancellationToken) + { + capturedRequest = request; + return default; + } + } + + [Fact] + public async Task InvokeMcpToolInvocationForwardsHeadersToTransportAsync() + { + // Arrange + this.State.InitializeSystem(); + const string HeaderKey = "Authorization"; + const string HeaderValue = "Bearer super-secret-token"; + InvokeMcpTool model = this.CreateModel( + displayName: nameof(InvokeMcpToolInvocationForwardsHeadersToTransportAsync), + serverUrl: TestServerUrl, + serverLabel: TestServerLabel, + toolName: TestToolName, + requireApproval: false, + headerKey: HeaderKey, + headerValue: HeaderValue); + + IDictionary? capturedHeaders = null; + Mock mockProvider = new(); + mockProvider + .Setup(provider => provider.InvokeToolAsync( + It.IsAny(), + It.IsAny(), + It.IsAny(), + It.IsAny?>(), + It.IsAny?>(), + It.IsAny(), + It.IsAny())) + .Callback?, IDictionary?, string?, CancellationToken>( + (_, _, _, _, headers, _, _) => capturedHeaders = headers) + .ReturnsAsync(new McpServerToolResultContent("mock-call-id") { Outputs = [new TextContent("ok")] }); + MockAgentProvider mockAgentProvider = new(); + InvokeMcpToolExecutor action = new(model, mockProvider.Object, mockAgentProvider.Object, this.State); + + // Act + await this.ExecuteAsync(action, isDiscrete: false); + + // Assert - headers remain available to the actual transport invocation. + Assert.NotNull(capturedHeaders); + Assert.True(capturedHeaders!.TryGetValue(HeaderKey, out string? forwardedValue)); + Assert.Equal(HeaderValue, forwardedValue); + } + [Fact] public async Task InvokeMcpToolExecuteWithEmptyHeaderValueAsync() {