From 71fde3437c4d0864b16c842efca45debbb6f0254 Mon Sep 17 00:00:00 2001 From: Vincent Biret Date: Mon, 9 Nov 2026 07:17:41 -0500 Subject: [PATCH 1/4] fix(dotnet): validate declarative MCP destinations Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 3333dcdf-889a-4ccc-af0c-4e41fd966d74 --- ...55-dotnet-declarative-mcp-allowed-hosts.md | 45 +++++++ .../InvokeFoundryToolboxMcp/Program.cs | 1 + .../Declarative/InvokeMcpTool/Program.cs | 1 + .../DefaultMcpToolHandler.cs | 38 ++++++ .../PublicAPI/net10.0/PublicAPI.Unshipped.txt | 1 + .../PublicAPI/net472/PublicAPI.Unshipped.txt | 1 + .../PublicAPI/net8.0/PublicAPI.Unshipped.txt | 1 + .../PublicAPI/net9.0/PublicAPI.Unshipped.txt | 1 + .../netstandard2.0/PublicAPI.Unshipped.txt | 1 + .../DefaultHttpRequestHandler.cs | 35 +----- .../README.md | 17 ++- dotnet/src/Shared/Http/AllowedHttpHosts.cs | 52 ++++++++ .../InvokeToolWorkflowTest.cs | 2 +- .../DefaultMcpToolHandlerLifetimeTests.cs | 68 +++++++++-- .../DefaultMcpToolHandlerTests.cs | 113 ++++++++++++++++++ .../references/declarative.md | 8 +- 16 files changed, 336 insertions(+), 49 deletions(-) create mode 100644 docs/decisions/0055-dotnet-declarative-mcp-allowed-hosts.md create mode 100644 dotnet/src/Shared/Http/AllowedHttpHosts.cs diff --git a/docs/decisions/0055-dotnet-declarative-mcp-allowed-hosts.md b/docs/decisions/0055-dotnet-declarative-mcp-allowed-hosts.md new file mode 100644 index 00000000000..4b19256c0a1 --- /dev/null +++ b/docs/decisions/0055-dotnet-declarative-mcp-allowed-hosts.md @@ -0,0 +1,45 @@ +--- +status: proposed +contact: baywet +date: 2026-11-09 +deciders: westey-m +--- + +# Explicit allowed hosts for declarative MCP invocations + +## Context and Problem Statement + +Declarative MCP actions receive their server URLs from workflow content. The application +owns destination configuration and the HTTP pipeline. The default MCP handler should apply +the same host-and-port policy as the [default HTTP handler](0049-dotnet-declarative-http-allowed-hosts.md). + +## Decision Drivers + +- Require application-configured destinations before creating an HTTP client. +- Keep host normalization and port matching consistent between HTTP and MCP. +- Preserve the existing constructor signature and per-invocation session lifetime. + +## Considered Options + +- Permit all destinations: preserves current behavior, but provides no application-owned destination policy. +- Require a new constructor signature: makes configuration explicit, but removes an existing API. +- Retain the existing constructor with an empty policy and add an allowlist overload: preserves binary + compatibility, but applications must migrate their configuration to continue invoking tools. + +## Decision Outcome + +Chosen option: retain the existing constructor with an empty policy and add an allowlist overload. + +The application supplies exact hosts or IP literals with optional ports. The list is copied, an omitted +port defaults to 443, and an empty list denies every invocation, including discovery. Matching uses the +same shared normalization as HTTP: case-insensitive IDN names, trailing DNS dots removed, and exact +effective ports, without wildcards or implicit subdomains. Workflow-supplied Host headers are rejected. + +Both HTTP and HTTPS remain supported. Applications requiring HTTPS must enforce that additional policy. +DNS, proxy configuration, and routing remain transport responsibilities. The application-owned factory +must disable cookies and automatic redirects, and any delegating handlers must preserve the validated +destination and bind credentials to the intended origin. + +This changes behavior without removing the existing constructor. Callers must supply their intended +destinations through the new overload; applications with different policies can provide an `IMcpToolHandler`. +Session creation, protocol selection, and resource disposal remain unchanged. diff --git a/dotnet/samples/03-workflows/Declarative/InvokeFoundryToolboxMcp/Program.cs b/dotnet/samples/03-workflows/Declarative/InvokeFoundryToolboxMcp/Program.cs index 79c6ddbbefd..98c958408f9 100644 --- a/dotnet/samples/03-workflows/Declarative/InvokeFoundryToolboxMcp/Program.cs +++ b/dotnet/samples/03-workflows/Declarative/InvokeFoundryToolboxMcp/Program.cs @@ -87,6 +87,7 @@ public static async Task Main(string[] args) .SetHandlerLifetime(TimeSpan.FromMinutes(2)); await using ServiceProvider serviceProvider = services.BuildServiceProvider(); DefaultMcpToolHandler mcpToolHandler = new( + [new Uri(toolboxMcpServerUrl).Authority], serviceProvider.GetRequiredService(), McpHttpClientName); // Create the workflow factory with MCP tool provider diff --git a/dotnet/samples/03-workflows/Declarative/InvokeMcpTool/Program.cs b/dotnet/samples/03-workflows/Declarative/InvokeMcpTool/Program.cs index 4c2e3752ec8..837c21f6769 100644 --- a/dotnet/samples/03-workflows/Declarative/InvokeMcpTool/Program.cs +++ b/dotnet/samples/03-workflows/Declarative/InvokeMcpTool/Program.cs @@ -73,6 +73,7 @@ public static async Task Main(string[] args) .SetHandlerLifetime(TimeSpan.FromMinutes(2)); await using ServiceProvider serviceProvider = services.BuildServiceProvider(); DefaultMcpToolHandler mcpToolHandler = new( + ["learn.microsoft.com", "mcp.ai.azure.com"], serviceProvider.GetRequiredService(), McpHttpClientName); // Create the workflow factory with MCP tool provider diff --git a/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative.Mcp/DefaultMcpToolHandler.cs b/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative.Mcp/DefaultMcpToolHandler.cs index 5ba5bf31052..3ea5d9de1b5 100644 --- a/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative.Mcp/DefaultMcpToolHandler.cs +++ b/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative.Mcp/DefaultMcpToolHandler.cs @@ -44,6 +44,7 @@ public sealed class DefaultMcpToolHandler : IMcpToolHandler private readonly IHttpClientFactory _httpClientFactory; private readonly string _httpClientName; + private readonly AllowedHttpHosts _allowedHosts; /// /// Gets or initializes the protocol version requested for every invocation. @@ -71,6 +72,7 @@ public sealed class DefaultMcpToolHandler : IMcpToolHandler /// Do not derive names from workflow identifiers, credentials, or arbitrary server URLs. /// /// + /// This overload allows no destinations. Use the overload accepting allowed hosts to enable invocations. /// The application is responsible for the HTTP pipeline's transport policy. /// Disable cookies and automatic redirects, enable certificate revocation checking, and restrict credential injection /// to the configured server origin. Pooled handlers must not retain per-invocation credentials or mutable origin state. @@ -78,9 +80,36 @@ public sealed class DefaultMcpToolHandler : IMcpToolHandler /// /// A required argument is . public DefaultMcpToolHandler(IHttpClientFactory httpClientFactory, string httpClientName) + : this([], httpClientFactory, httpClientName) + { + } + + /// + /// Initializes a new instance of the class with allowed hosts + /// and an application-provided HTTP client factory. + /// + /// + /// The exact DNS names or IP literals, optionally including a port, the application permits. + /// Omitted ports default to 443. Use brackets around IPv6 literals when specifying a port. + /// Schemes, paths, and wildcards are not supported. An empty collection rejects all invocations. + /// The collection is copied and checked before creating an HTTP client, including for tool discovery. + /// Matching is case-insensitive, with internationalized names and trailing DNS dots normalized. + /// Both HTTP and HTTPS are supported; authorization matches the host and effective port, not the scheme. + /// DNS resolution and routing remain the responsibility of the HTTP transport. + /// + /// + /// The application-owned factory that creates a separate HTTP client for every invocation. + /// Configure cookies and automatic redirects off, certificate revocation checking on, and origin-bound + /// credential injection. Delegating handlers must preserve the validated destination. + /// + /// The stable named-client configuration, or an empty string for the default configuration. + /// A required argument is . + /// An allowed host is invalid. + public DefaultMcpToolHandler(IEnumerable allowedHosts, IHttpClientFactory httpClientFactory, string httpClientName) { this._httpClientFactory = Throw.IfNull(httpClientFactory); this._httpClientName = Throw.IfNull(httpClientName); + this._allowedHosts = new(allowedHosts); } /// @@ -110,6 +139,10 @@ public async Task InvokeToolAsync( header.Value, nameof(headers), "HTTP header value contains invalid characters."); + if (string.Equals(header.Key, "Host", StringComparison.OrdinalIgnoreCase)) + { + throw new HttpRequestException("Request headers must not override the Host header."); + } } } @@ -189,6 +222,11 @@ private async Task CreateClientAsync( EnableStandaloneGetStream = false }; + if (!this._allowedHosts.Contains(endpoint)) + { + throw new HttpRequestException("The request host and port are not in the allowed hosts list."); + } + HttpClient httpClient = this._httpClientFactory.CreateClient(this._httpClientName); HttpClientTransport? transport = null; try diff --git a/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative.Mcp/PublicAPI/net10.0/PublicAPI.Unshipped.txt b/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative.Mcp/PublicAPI/net10.0/PublicAPI.Unshipped.txt index 6ab40b3b9f5..b44ce8f4efb 100644 --- a/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative.Mcp/PublicAPI/net10.0/PublicAPI.Unshipped.txt +++ b/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative.Mcp/PublicAPI/net10.0/PublicAPI.Unshipped.txt @@ -1,5 +1,6 @@ #nullable enable Microsoft.Agents.AI.Workflows.Declarative.Mcp.DefaultMcpToolHandler.DefaultMcpToolHandler(System.Net.Http.IHttpClientFactory! httpClientFactory, string! httpClientName) -> void +Microsoft.Agents.AI.Workflows.Declarative.Mcp.DefaultMcpToolHandler.DefaultMcpToolHandler(System.Collections.Generic.IEnumerable! allowedHosts, System.Net.Http.IHttpClientFactory! httpClientFactory, string! httpClientName) -> void Microsoft.Agents.AI.Workflows.Declarative.Mcp.DefaultMcpToolHandler.ProtocolVersion.get -> string? Microsoft.Agents.AI.Workflows.Declarative.Mcp.DefaultMcpToolHandler.ProtocolVersion.init -> void *REMOVED*Microsoft.Agents.AI.Workflows.Declarative.Mcp.DefaultMcpToolHandler.DefaultMcpToolHandler(System.Func!>? httpClientProvider = null) -> void diff --git a/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative.Mcp/PublicAPI/net472/PublicAPI.Unshipped.txt b/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative.Mcp/PublicAPI/net472/PublicAPI.Unshipped.txt index 6ab40b3b9f5..b44ce8f4efb 100644 --- a/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative.Mcp/PublicAPI/net472/PublicAPI.Unshipped.txt +++ b/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative.Mcp/PublicAPI/net472/PublicAPI.Unshipped.txt @@ -1,5 +1,6 @@ #nullable enable Microsoft.Agents.AI.Workflows.Declarative.Mcp.DefaultMcpToolHandler.DefaultMcpToolHandler(System.Net.Http.IHttpClientFactory! httpClientFactory, string! httpClientName) -> void +Microsoft.Agents.AI.Workflows.Declarative.Mcp.DefaultMcpToolHandler.DefaultMcpToolHandler(System.Collections.Generic.IEnumerable! allowedHosts, System.Net.Http.IHttpClientFactory! httpClientFactory, string! httpClientName) -> void Microsoft.Agents.AI.Workflows.Declarative.Mcp.DefaultMcpToolHandler.ProtocolVersion.get -> string? Microsoft.Agents.AI.Workflows.Declarative.Mcp.DefaultMcpToolHandler.ProtocolVersion.init -> void *REMOVED*Microsoft.Agents.AI.Workflows.Declarative.Mcp.DefaultMcpToolHandler.DefaultMcpToolHandler(System.Func!>? httpClientProvider = null) -> void diff --git a/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative.Mcp/PublicAPI/net8.0/PublicAPI.Unshipped.txt b/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative.Mcp/PublicAPI/net8.0/PublicAPI.Unshipped.txt index 6ab40b3b9f5..b44ce8f4efb 100644 --- a/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative.Mcp/PublicAPI/net8.0/PublicAPI.Unshipped.txt +++ b/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative.Mcp/PublicAPI/net8.0/PublicAPI.Unshipped.txt @@ -1,5 +1,6 @@ #nullable enable Microsoft.Agents.AI.Workflows.Declarative.Mcp.DefaultMcpToolHandler.DefaultMcpToolHandler(System.Net.Http.IHttpClientFactory! httpClientFactory, string! httpClientName) -> void +Microsoft.Agents.AI.Workflows.Declarative.Mcp.DefaultMcpToolHandler.DefaultMcpToolHandler(System.Collections.Generic.IEnumerable! allowedHosts, System.Net.Http.IHttpClientFactory! httpClientFactory, string! httpClientName) -> void Microsoft.Agents.AI.Workflows.Declarative.Mcp.DefaultMcpToolHandler.ProtocolVersion.get -> string? Microsoft.Agents.AI.Workflows.Declarative.Mcp.DefaultMcpToolHandler.ProtocolVersion.init -> void *REMOVED*Microsoft.Agents.AI.Workflows.Declarative.Mcp.DefaultMcpToolHandler.DefaultMcpToolHandler(System.Func!>? httpClientProvider = null) -> void diff --git a/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative.Mcp/PublicAPI/net9.0/PublicAPI.Unshipped.txt b/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative.Mcp/PublicAPI/net9.0/PublicAPI.Unshipped.txt index 6ab40b3b9f5..b44ce8f4efb 100644 --- a/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative.Mcp/PublicAPI/net9.0/PublicAPI.Unshipped.txt +++ b/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative.Mcp/PublicAPI/net9.0/PublicAPI.Unshipped.txt @@ -1,5 +1,6 @@ #nullable enable Microsoft.Agents.AI.Workflows.Declarative.Mcp.DefaultMcpToolHandler.DefaultMcpToolHandler(System.Net.Http.IHttpClientFactory! httpClientFactory, string! httpClientName) -> void +Microsoft.Agents.AI.Workflows.Declarative.Mcp.DefaultMcpToolHandler.DefaultMcpToolHandler(System.Collections.Generic.IEnumerable! allowedHosts, System.Net.Http.IHttpClientFactory! httpClientFactory, string! httpClientName) -> void Microsoft.Agents.AI.Workflows.Declarative.Mcp.DefaultMcpToolHandler.ProtocolVersion.get -> string? Microsoft.Agents.AI.Workflows.Declarative.Mcp.DefaultMcpToolHandler.ProtocolVersion.init -> void *REMOVED*Microsoft.Agents.AI.Workflows.Declarative.Mcp.DefaultMcpToolHandler.DefaultMcpToolHandler(System.Func!>? httpClientProvider = null) -> void diff --git a/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative.Mcp/PublicAPI/netstandard2.0/PublicAPI.Unshipped.txt b/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative.Mcp/PublicAPI/netstandard2.0/PublicAPI.Unshipped.txt index 6ab40b3b9f5..b44ce8f4efb 100644 --- a/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative.Mcp/PublicAPI/netstandard2.0/PublicAPI.Unshipped.txt +++ b/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative.Mcp/PublicAPI/netstandard2.0/PublicAPI.Unshipped.txt @@ -1,5 +1,6 @@ #nullable enable Microsoft.Agents.AI.Workflows.Declarative.Mcp.DefaultMcpToolHandler.DefaultMcpToolHandler(System.Net.Http.IHttpClientFactory! httpClientFactory, string! httpClientName) -> void +Microsoft.Agents.AI.Workflows.Declarative.Mcp.DefaultMcpToolHandler.DefaultMcpToolHandler(System.Collections.Generic.IEnumerable! allowedHosts, System.Net.Http.IHttpClientFactory! httpClientFactory, string! httpClientName) -> void Microsoft.Agents.AI.Workflows.Declarative.Mcp.DefaultMcpToolHandler.ProtocolVersion.get -> string? Microsoft.Agents.AI.Workflows.Declarative.Mcp.DefaultMcpToolHandler.ProtocolVersion.init -> void *REMOVED*Microsoft.Agents.AI.Workflows.Declarative.Mcp.DefaultMcpToolHandler.DefaultMcpToolHandler(System.Func!>? httpClientProvider = null) -> void diff --git a/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative/DefaultHttpRequestHandler.cs b/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative/DefaultHttpRequestHandler.cs index fb4f46bcc42..936db6e5ce2 100644 --- a/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative/DefaultHttpRequestHandler.cs +++ b/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative/DefaultHttpRequestHandler.cs @@ -3,7 +3,6 @@ using System; using System.Collections.Generic; using System.Diagnostics.CodeAnalysis; -using System.Globalization; using System.Linq; using System.Net; using System.Net.Http; @@ -53,7 +52,7 @@ public sealed class DefaultHttpRequestHandler : IHttpRequestHandler { private const int MaxAutomaticRedirections = 50; - private readonly HashSet _allowedHosts; + private readonly AllowedHttpHosts _allowedHosts; private readonly IHttpMessageHandlerFactory _httpMessageHandlerFactory; private readonly string _httpClientName; @@ -83,32 +82,7 @@ public DefaultHttpRequestHandler( { Throw.IfNull(allowedHosts); Throw.IfNull(httpMessageHandlerFactory); - this._allowedHosts = new HashSet(StringComparer.OrdinalIgnoreCase); - foreach (string host in allowedHosts) - { - string hostName = host; - int port = 443; - if (!string.IsNullOrWhiteSpace(host) && Uri.CheckHostName(host) == UriHostNameType.Unknown) - { - int separator = host.LastIndexOf(':'); - if (separator > 0 && (host.StartsWith("[", StringComparison.Ordinal) || host.IndexOf(':') == separator)) - { - hostName = host.Substring(0, separator); - if (!int.TryParse(host.Substring(separator + 1), NumberStyles.None, CultureInfo.InvariantCulture, out port)) - { - port = -1; - } - } - } - - if (string.IsNullOrWhiteSpace(hostName) || Uri.CheckHostName(hostName) == UriHostNameType.Unknown || - port is < 1 or > 65535) - { - throw new ArgumentException("Allowed hosts must be DNS names or IP literals with an optional port from 1 to 65535, without schemes, paths, or wildcards. IPv6 literals with a port must be bracketed.", nameof(allowedHosts)); - } - - this._allowedHosts.Add(NormalizeDestination(new UriBuilder(Uri.UriSchemeHttps, hostName, port).Uri)); - } + this._allowedHosts = new(allowedHosts); this._httpMessageHandlerFactory = httpMessageHandlerFactory; this._httpClientName = httpClientName ?? string.Empty; @@ -155,7 +129,7 @@ public async Task SendAsync(HttpRequestInfo request, Cancella for (int redirectCount = 0; redirectCount <= MaxAutomaticRedirections; redirectCount++) { - if (!this._allowedHosts.Contains(NormalizeDestination(currentUri))) + if (!this._allowedHosts.Contains(currentUri)) { throw new HttpRequestException("The request host and port are not in the allowed hosts list."); } @@ -328,9 +302,6 @@ private static void ValidateHeaderValue(string name, string value) $"HTTP header '{name}' contains invalid characters."); } - private static string NormalizeDestination(Uri uri) => - $"{uri.IdnHost.TrimEnd('.')}:{uri.Port.ToString(CultureInfo.InvariantCulture)}"; - private static Uri CreateAbsoluteUri(string requestUri) { if (!Uri.TryCreate(requestUri, UriKind.Absolute, out Uri? uri)) diff --git a/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative/README.md b/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative/README.md index 6cbc4885867..8ffe8bc8842 100644 --- a/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative/README.md +++ b/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative/README.md @@ -71,8 +71,17 @@ SSE responses to individual tool requests remain supported. Each factory-created HTTP client is disposed with its invocation. Construct the handler with an application-owned `IHttpClientFactory` through -`DefaultMcpToolHandler(IHttpClientFactory, string)`. The second -argument is a stable named-client configuration, or `string.Empty` for the +`DefaultMcpToolHandler(IEnumerable, IHttpClientFactory, string)`. Supply +allowed hosts from application configuration, not workflow content. An empty +list, including the existing two-argument constructor, denies all destinations. +Hosts are exact DNS names or IP literals with optional ports; omitted ports +default to 443. Matching is case-insensitive and normalizes internationalized +names and trailing DNS dots, without wildcards or implicit subdomains. +The list is copied and checked before client creation for both tool calls and +discovery. Workflow-supplied `Host` headers are rejected. +Both HTTP and HTTPS are supported; enforce HTTPS separately if required. +Allowing a name trusts the application's DNS and network routing. +The third argument is a stable named-client configuration, or `string.Empty` for the default configuration. Factory-created wrappers are owned by the invocation; the factory and its pooled handlers remain application-owned. There is no parameterless constructor or internally created HTTP pipeline. Configure handler @@ -92,7 +101,7 @@ services.AddHttpClient("workflow-mcp") .SetHandlerLifetime(TimeSpan.FromMinutes(2)); services.AddSingleton(serviceProvider => new DefaultMcpToolHandler( - serviceProvider.GetRequiredService(), "workflow-mcp")); + ["mcp.example.com"], serviceProvider.GetRequiredService(), "workflow-mcp")); ``` Factory clients use the application's transport-policy configuration. @@ -108,7 +117,7 @@ discovery and legacy fallback. If the server's supported protocol is known, set the init-only property when constructing the handler: ```csharp -DefaultMcpToolHandler handler = new(httpClientFactory, "workflow-mcp") +DefaultMcpToolHandler handler = new(["mcp.example.com"], httpClientFactory, "workflow-mcp") { ProtocolVersion = "2025-11-25" }; diff --git a/dotnet/src/Shared/Http/AllowedHttpHosts.cs b/dotnet/src/Shared/Http/AllowedHttpHosts.cs new file mode 100644 index 00000000000..01356f63a64 --- /dev/null +++ b/dotnet/src/Shared/Http/AllowedHttpHosts.cs @@ -0,0 +1,52 @@ +// Copyright (c) Microsoft. All rights reserved. + +using System; +using System.Collections.Generic; +using System.Globalization; + +namespace Microsoft.Shared.Diagnostics; + +/// Matches application-configured HTTP hosts and ports. +internal sealed class AllowedHttpHosts +{ + private readonly HashSet _destinations = new(StringComparer.OrdinalIgnoreCase); + + public AllowedHttpHosts(IEnumerable allowedHosts) + { + if (allowedHosts is null) + { + throw new ArgumentNullException(nameof(allowedHosts)); + } + + foreach (string host in allowedHosts) + { + string hostName = host; + int port = 443; + if (!string.IsNullOrWhiteSpace(host) && Uri.CheckHostName(host) == UriHostNameType.Unknown) + { + int separator = host.LastIndexOf(':'); + if (separator > 0 && (host.StartsWith("[", StringComparison.Ordinal) || host.IndexOf(':') == separator)) + { + hostName = host.Substring(0, separator); + if (!int.TryParse(host.Substring(separator + 1), NumberStyles.None, CultureInfo.InvariantCulture, out port)) + { + port = -1; + } + } + } + + if (string.IsNullOrWhiteSpace(hostName) || Uri.CheckHostName(hostName) == UriHostNameType.Unknown || + port is < 1 or > 65535) + { + throw new ArgumentException("Allowed hosts must be DNS names or IP literals with an optional port from 1 to 65535, without schemes, paths, or wildcards. IPv6 literals with a port must be bracketed.", nameof(allowedHosts)); + } + + this._destinations.Add(NormalizeDestination(new UriBuilder(Uri.UriSchemeHttps, hostName, port).Uri)); + } + } + + public bool Contains(Uri uri) => this._destinations.Contains(NormalizeDestination(uri)); + + private static string NormalizeDestination(Uri uri) => + $"{uri.IdnHost.TrimEnd('.')}:{uri.Port.ToString(CultureInfo.InvariantCulture)}"; +} diff --git a/dotnet/tests/Microsoft.Agents.AI.Workflows.Declarative.IntegrationTests/InvokeToolWorkflowTest.cs b/dotnet/tests/Microsoft.Agents.AI.Workflows.Declarative.IntegrationTests/InvokeToolWorkflowTest.cs index 92e6c87b825..fdae167c20f 100644 --- a/dotnet/tests/Microsoft.Agents.AI.Workflows.Declarative.IntegrationTests/InvokeToolWorkflowTest.cs +++ b/dotnet/tests/Microsoft.Agents.AI.Workflows.Declarative.IntegrationTests/InvokeToolWorkflowTest.cs @@ -197,7 +197,7 @@ private async Task RunInvokeMcpToolTestAsync( CheckCertificateRevocationList = true }); using ServiceProvider serviceProvider = services.BuildServiceProvider(); - DefaultMcpToolHandler mcpToolProvider = new(serviceProvider.GetRequiredService(), "workflow-mcp"); + DefaultMcpToolHandler mcpToolProvider = new(["learn.microsoft.com"], serviceProvider.GetRequiredService(), "workflow-mcp"); DeclarativeWorkflowOptions workflowOptions = await this.CreateOptionsAsync( externalConversation: false, mcpToolProvider: mcpToolProvider); diff --git a/dotnet/tests/Microsoft.Agents.AI.Workflows.Declarative.Mcp.UnitTests/DefaultMcpToolHandlerLifetimeTests.cs b/dotnet/tests/Microsoft.Agents.AI.Workflows.Declarative.Mcp.UnitTests/DefaultMcpToolHandlerLifetimeTests.cs index 1645d6a0d29..fee73650d43 100644 --- a/dotnet/tests/Microsoft.Agents.AI.Workflows.Declarative.Mcp.UnitTests/DefaultMcpToolHandlerLifetimeTests.cs +++ b/dotnet/tests/Microsoft.Agents.AI.Workflows.Declarative.Mcp.UnitTests/DefaultMcpToolHandlerLifetimeTests.cs @@ -48,7 +48,7 @@ public async Task Factory_LoopbackInvocations_ReuseHandlerAndDisposeSeparateSess }; }); using ServiceProvider serviceProvider = services.BuildServiceProvider(); - DefaultMcpToolHandler handler = new(serviceProvider.GetRequiredService(), "workflow-mcp") + DefaultMcpToolHandler handler = new([new Uri(server.Endpoint).Authority], serviceProvider.GetRequiredService(), "workflow-mcp") { ProtocolVersion = protocolVersion }; @@ -264,7 +264,7 @@ public async Task DifferentOrigins_KeepRequestConfigurationAsync() { // Arrange ProtocolStub stub = new(); - DefaultMcpToolHandler handler = CreateHandler(stub); + DefaultMcpToolHandler handler = CreateHandler(stub, allowedHosts: ["first.example", "second.example"]); using CancellationTokenSource timeout = new(TimeSpan.FromSeconds(10)); // Act @@ -313,7 +313,7 @@ public async Task Factory_SequentialInvocations_OwnClientsButReusePooledHandlers clients.Add(client); return client; }); - DefaultMcpToolHandler handler = new(factory.Object, ClientName); + DefaultMcpToolHandler handler = new(["mcp.example"], factory.Object, ClientName); using CancellationTokenSource timeout = new(TimeSpan.FromSeconds(10)); // Act @@ -374,7 +374,7 @@ public async Task InvocationFailure_DisposesClientAndAllowsNextInvocationAsync(b }; } - DefaultMcpToolHandler handler = new(factory.Object, "workflow-mcp"); + DefaultMcpToolHandler handler = new(["mcp.example"], factory.Object, "workflow-mcp"); using CancellationTokenSource timeout = new(TimeSpan.FromSeconds(10)); using CancellationTokenSource cancellation = CancellationTokenSource.CreateLinkedTokenSource(timeout.Token); @@ -418,7 +418,7 @@ public async Task FactoryFailure_DoesNotPreventNextInvocationAsync() factory.SetupSequence(f => f.CreateClient("workflow-mcp")) .Throws(new InvalidOperationException("Factory unavailable.")) .Returns(() => new HttpClient(stub.CreateMessageHandler())); - DefaultMcpToolHandler handler = new(factory.Object, "workflow-mcp"); + DefaultMcpToolHandler handler = new(["mcp.example"], factory.Object, "workflow-mcp"); using CancellationTokenSource timeout = new(TimeSpan.FromSeconds(10)); // Act @@ -453,7 +453,7 @@ public async Task ConcurrentInvocations_DoNotCoalesceOrSerializeSessionsAsync(bo await finish.WaitAsync(token); }; DefaultMcpToolHandler handler = serviceProvider is not null - ? new(serviceProvider.GetRequiredService(), "workflow-mcp") + ? new(["mcp.example"], serviceProvider.GetRequiredService(), "workflow-mcp") : CreateHandler(stub); using CancellationTokenSource timeout = new(TimeSpan.FromSeconds(10)); @@ -615,11 +615,63 @@ public async Task ClientConnection_SessionCleanupFailure_StillCleansUpTransportA } } - private static DefaultMcpToolHandler CreateHandler(ProtocolStub stub, string? protocolVersion = null) + [Theory] + [InlineData("mcp.example", "https://mcp.example/api")] + [InlineData("MCP.EXAMPLE.", "https://mcp.example/api")] + [InlineData("mcp.example", " https://MCP.EXAMPLE./api ")] + [InlineData("mcp.example:8443", "https://mcp.example:8443/api")] + [InlineData("mcp.example:80", "http://mcp.example/api")] + [InlineData("mcp.example", "http://mcp.example:443/api")] + [InlineData("127.0.0.1:8080", "http://127.0.0.1:8080/api")] + [InlineData("[::1]:8080", "http://[::1]:8080/api")] + [InlineData("xn--bcher-kva.example", "https://b\u00fccher.example/api")] + [InlineData("b\u00fccher.example", "https://xn--bcher-kva.example/api")] + public async Task AllowedDestination_PreservesToolInvocationAsync(string allowedHost, string serverUrl) + { + // Arrange + ProtocolStub stub = new(); + DefaultMcpToolHandler handler = CreateHandler(stub, allowedHosts: [allowedHost]); + using CancellationTokenSource timeout = new(TimeSpan.FromSeconds(10)); + + // Act + McpServerToolResultContent result = await handler.InvokeToolAsync( + serverUrl, null, "ping", null, null, null, timeout.Token); + + // Assert + Assert.NotNull(result.Outputs); + Assert.Equal("ok", Assert.IsType(Assert.Single(result.Outputs)).Text); + Assert.Equal(1, stub.Initializations); + Assert.Equal(1, stub.Terminations); + } + + [Fact] + public async Task AllowedHosts_AreCopiedAtConstructionAsync() + { + // Arrange + ProtocolStub stub = new(); + List allowedHosts = ["mcp.example"]; + DefaultMcpToolHandler handler = CreateHandler(stub, allowedHosts: allowedHosts); + allowedHosts.Clear(); + allowedHosts.Add("other.example"); + using CancellationTokenSource timeout = new(TimeSpan.FromSeconds(10)); + + // Act + McpServerToolResultContent result = await InvokeAsync(handler, "ping", timeout.Token); + await Assert.ThrowsAsync(() => handler.InvokeToolAsync( + "https://other.example/api", null, "ping", null, null, null, timeout.Token)); + + // Assert + Assert.NotNull(result.Outputs); + Assert.Equal("ok", Assert.IsType(Assert.Single(result.Outputs)).Text); + Assert.Single(stub.Handlers); + } + + private static DefaultMcpToolHandler CreateHandler( + ProtocolStub stub, string? protocolVersion = null, IEnumerable? allowedHosts = null) { Mock factory = new(); factory.Setup(f => f.CreateClient("workflow-mcp")).Returns(() => new HttpClient(stub.CreateMessageHandler())); - return new(factory.Object, "workflow-mcp") { ProtocolVersion = protocolVersion }; + return new(allowedHosts ?? ["mcp.example"], factory.Object, "workflow-mcp") { ProtocolVersion = protocolVersion }; } private static Task InvokeAsync( diff --git a/dotnet/tests/Microsoft.Agents.AI.Workflows.Declarative.Mcp.UnitTests/DefaultMcpToolHandlerTests.cs b/dotnet/tests/Microsoft.Agents.AI.Workflows.Declarative.Mcp.UnitTests/DefaultMcpToolHandlerTests.cs index 13f0c65fffa..cdf0af37251 100644 --- a/dotnet/tests/Microsoft.Agents.AI.Workflows.Declarative.Mcp.UnitTests/DefaultMcpToolHandlerTests.cs +++ b/dotnet/tests/Microsoft.Agents.AI.Workflows.Declarative.Mcp.UnitTests/DefaultMcpToolHandlerTests.cs @@ -86,6 +86,119 @@ await Assert.ThrowsAnyAsync(() => handler.InvokeTool factory.Verify(f => f.CreateClient(It.IsAny()), Times.Never); } + [Theory] + [InlineData("ping")] + [InlineData(DefaultMcpToolHandler.ListToolsToolName)] + public async Task InvokeToolAsync_WithoutAllowedHosts_ShouldRejectBeforeClientCreationAsync(string toolName) + { + // Arrange + Mock factory = new(MockBehavior.Strict); + DefaultMcpToolHandler handler = new(factory.Object, "workflow-mcp"); + + // Act + HttpRequestException exception = await Assert.ThrowsAsync(() => handler.InvokeToolAsync( + "https://example.com/mcp", null, toolName, null, null, null)); + + // Assert + Assert.Contains("allowed hosts", exception.Message); + factory.Verify(f => f.CreateClient(It.IsAny()), Times.Never); + } + + [Theory] + [InlineData("ping")] + [InlineData(DefaultMcpToolHandler.ListToolsToolName)] + public async Task InvokeToolAsync_WithEmptyAllowedHosts_ShouldRejectBeforeClientCreationAsync(string toolName) + { + // Arrange + Mock factory = new(MockBehavior.Strict); + DefaultMcpToolHandler handler = new([], factory.Object, "workflow-mcp"); + + // Act + await Assert.ThrowsAsync(() => handler.InvokeToolAsync( + "https://example.com/mcp", null, toolName, null, null, null)); + + // Assert + factory.Verify(f => f.CreateClient(It.IsAny()), Times.Never); + } + + [Theory] + [InlineData("https://other.example/mcp")] + [InlineData("https://sub.example.com/mcp")] + [InlineData("https://example.com.other.example/mcp")] + [InlineData("https://example.com:8443/mcp")] + [InlineData("http://example.com/mcp")] + [InlineData("https://example.com@other.example/mcp")] + public async Task InvokeToolAsync_WithUnlistedDestination_ShouldRejectBeforeClientCreationAsync(string serverUrl) + { + // Arrange + Mock factory = new(MockBehavior.Strict); + DefaultMcpToolHandler handler = new(["example.com"], factory.Object, "workflow-mcp"); + + // Act + HttpRequestException exception = await Assert.ThrowsAsync(() => handler.InvokeToolAsync( + serverUrl, null, "ping", null, null, null)); + + // Assert + Assert.Contains("allowed hosts", exception.Message); + factory.Verify(f => f.CreateClient(It.IsAny()), Times.Never); + } + + [Theory] + [InlineData("Host")] + [InlineData("hOsT")] + public async Task InvokeToolAsync_WithHostHeader_ShouldRejectBeforeClientCreationAsync(string headerName) + { + // Arrange + Mock factory = new(MockBehavior.Strict); + DefaultMcpToolHandler handler = new(["example.com"], factory.Object, "workflow-mcp"); + Dictionary headers = new() { [headerName] = "other.example" }; + + // Act + HttpRequestException exception = await Assert.ThrowsAsync(() => handler.InvokeToolAsync( + "https://example.com/mcp", null, "ping", null, headers, null)); + + // Assert + Assert.Contains("Host header", exception.Message); + factory.Verify(f => f.CreateClient(It.IsAny()), Times.Never); + } + + [Fact] + public void Constructor_WithNullAllowedHosts_ShouldThrow() + { + // Arrange + Mock factory = new(); + + // Act + ArgumentNullException exception = Assert.Throws(() => + new DefaultMcpToolHandler(null!, factory.Object, "workflow-mcp")); + + // Assert + Assert.Equal("allowedHosts", exception.ParamName); + } + + [Theory] + [InlineData(null)] + [InlineData("")] + [InlineData(" ")] + [InlineData("https://example.com")] + [InlineData("example.com/path")] + [InlineData("*.example.com")] + [InlineData("example.com:0")] + [InlineData("example.com:65536")] + [InlineData("example.com:invalid")] + public void Constructor_WithInvalidAllowedHost_ShouldThrow(string? allowedHost) + { + // Arrange + Mock factory = new(); + + // Act + ArgumentException exception = Assert.Throws(() => + new DefaultMcpToolHandler([allowedHost!], factory.Object, "workflow-mcp")); + + // Assert + Assert.Equal("allowedHosts", exception.ParamName); + } + #region Reserved Tools/List Tests [Fact] diff --git a/skills/agent-framework-builder-dotnet/references/declarative.md b/skills/agent-framework-builder-dotnet/references/declarative.md index 7289253491d..603c4e00e53 100644 --- a/skills/agent-framework-builder-dotnet/references/declarative.md +++ b/skills/agent-framework-builder-dotnet/references/declarative.md @@ -60,10 +60,10 @@ commitments. or a custom handler with a documented session-lifetime contract when continuity is required. - Standalone MCP GET/SSE streams for unsolicited messages are disabled; request-scoped SSE tool responses remain supported. Use an explicitly scoped integration for background subscriptions. -- Construct `DefaultMcpToolHandler(IHttpClientFactory, string)` with an application-provided factory and stable named - configuration for HTTP handler reuse and rotation. Factory-created clients are disposed per invocation; the - factory remains application-owned. Configure the factory once with pooled handlers to avoid connection churn and - port exhaustion. Parameterless construction, callback-based constructors, and handler-level disposal are unsupported. +- Keep MCP destination authorization in application-owned configuration rather than workflow content. + Use the default handler when an explicit destination policy and per-invocation sessions fit the application; + choose a custom handler when those policies need to differ. See the + [declarative workflow guidance](https://learn.microsoft.com/agent-framework/workflows/declarative). - Leave `DefaultMcpToolHandler.ProtocolVersion` unset for SDK automatic negotiation. When a server's protocol is known, an explicit legacy version skips the discovery probe but still creates a fresh session per invocation. Selection restricts negotiation and incompatible versions fail. Configure separate handlers for differing server From 5e6ed6ef24060734b6f2c28e7a5eb700074e061f Mon Sep 17 00:00:00 2001 From: Vincent Biret Date: Mon, 9 Nov 2026 08:02:07 -0500 Subject: [PATCH 2/4] fix(dotnet): validate declarative MCP handler pipelines Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 3333dcdf-889a-4ccc-af0c-4e41fd966d74 --- ...55-dotnet-declarative-mcp-allowed-hosts.md | 14 +- .../InvokeFoundryToolboxMcp/Program.cs | 2 +- .../Declarative/InvokeMcpTool/Program.cs | 2 +- .../DefaultMcpToolHandler.cs | 34 ++- .../PublicAPI/net10.0/PublicAPI.Unshipped.txt | 2 +- .../PublicAPI/net472/PublicAPI.Unshipped.txt | 2 +- .../PublicAPI/net8.0/PublicAPI.Unshipped.txt | 2 +- .../PublicAPI/net9.0/PublicAPI.Unshipped.txt | 2 +- .../netstandard2.0/PublicAPI.Unshipped.txt | 2 +- .../DefaultHttpRequestHandler.cs | 35 +-- .../README.md | 29 +- .../Shared/Http/HttpTransportValidation.cs | 43 +++ .../InvokeToolWorkflowTest.cs | 2 +- .../DefaultMcpToolHandlerLifetimeTests.cs | 254 ++++++++++++------ .../DefaultMcpToolHandlerTests.cs | 16 +- .../references/declarative.md | 5 +- 16 files changed, 281 insertions(+), 165 deletions(-) create mode 100644 dotnet/src/Shared/Http/HttpTransportValidation.cs diff --git a/docs/decisions/0055-dotnet-declarative-mcp-allowed-hosts.md b/docs/decisions/0055-dotnet-declarative-mcp-allowed-hosts.md index 4b19256c0a1..e87be8be1f9 100644 --- a/docs/decisions/0055-dotnet-declarative-mcp-allowed-hosts.md +++ b/docs/decisions/0055-dotnet-declarative-mcp-allowed-hosts.md @@ -36,9 +36,17 @@ same shared normalization as HTTP: case-insensitive IDN names, trailing DNS dots effective ports, without wildcards or implicit subdomains. Workflow-supplied Host headers are rejected. Both HTTP and HTTPS remain supported. Applications requiring HTTPS must enforce that additional policy. -DNS, proxy configuration, and routing remain transport responsibilities. The application-owned factory -must disable cookies and automatic redirects, and any delegating handlers must preserve the validated -destination and bind credentials to the intended origin. +DNS, proxy configuration, and routing remain transport responsibilities. The application supplies an +`IHttpMessageHandlerFactory`; before sending, the handler validates that each pipeline terminates in an +`HttpClientHandler`, or on .NET a `SocketsHttpHandler`, with cookies and automatic redirects disabled. +Unknown primary transports and incomplete pipelines are rejected. Redirect responses are not followed. +Application-owned delegating handlers must preserve the validated destination and bind credentials to +the intended origin. + +The handler creates and disposes an HTTP client per invocation without disposing the factory-owned +pipeline. Client configuration callbacks registered through `IHttpClientFactory` do not apply; configure +headers and origin-bound authentication in delegating handlers. This matches the HTTP handler's ownership +and transport-validation contract while keeping MCP protocol sessions separate per invocation. This changes behavior without removing the existing constructor. Callers must supply their intended destinations through the new overload; applications with different policies can provide an `IMcpToolHandler`. diff --git a/dotnet/samples/03-workflows/Declarative/InvokeFoundryToolboxMcp/Program.cs b/dotnet/samples/03-workflows/Declarative/InvokeFoundryToolboxMcp/Program.cs index 98c958408f9..9030dcf29d1 100644 --- a/dotnet/samples/03-workflows/Declarative/InvokeFoundryToolboxMcp/Program.cs +++ b/dotnet/samples/03-workflows/Declarative/InvokeFoundryToolboxMcp/Program.cs @@ -88,7 +88,7 @@ public static async Task Main(string[] args) await using ServiceProvider serviceProvider = services.BuildServiceProvider(); DefaultMcpToolHandler mcpToolHandler = new( [new Uri(toolboxMcpServerUrl).Authority], - serviceProvider.GetRequiredService(), McpHttpClientName); + serviceProvider.GetRequiredService(), McpHttpClientName); // Create the workflow factory with MCP tool provider WorkflowFactory workflowFactory = new("InvokeFoundryToolboxMcp.yaml", foundryEndpoint) diff --git a/dotnet/samples/03-workflows/Declarative/InvokeMcpTool/Program.cs b/dotnet/samples/03-workflows/Declarative/InvokeMcpTool/Program.cs index 837c21f6769..ee4c90ed822 100644 --- a/dotnet/samples/03-workflows/Declarative/InvokeMcpTool/Program.cs +++ b/dotnet/samples/03-workflows/Declarative/InvokeMcpTool/Program.cs @@ -74,7 +74,7 @@ public static async Task Main(string[] args) await using ServiceProvider serviceProvider = services.BuildServiceProvider(); DefaultMcpToolHandler mcpToolHandler = new( ["learn.microsoft.com", "mcp.ai.azure.com"], - serviceProvider.GetRequiredService(), McpHttpClientName); + serviceProvider.GetRequiredService(), McpHttpClientName); // Create the workflow factory with MCP tool provider WorkflowFactory workflowFactory = new("InvokeMcpTool.yaml", foundryEndpoint) diff --git a/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative.Mcp/DefaultMcpToolHandler.cs b/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative.Mcp/DefaultMcpToolHandler.cs index 3ea5d9de1b5..77b1c3dd106 100644 --- a/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative.Mcp/DefaultMcpToolHandler.cs +++ b/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative.Mcp/DefaultMcpToolHandler.cs @@ -23,7 +23,7 @@ namespace Microsoft.Agents.AI.Workflows.Declarative.Mcp; /// /// /// Every invocation creates and disposes a separate MCP client, including tools/list. -/// Factory-created HTTP clients reuse factory-managed handlers without sharing MCP session state. +/// Per-invocation HTTP clients reuse factory-managed handlers without sharing MCP session state. /// Standalone GET streams for unsolicited server messages are disabled; request-scoped SSE responses remain supported. /// The handler owns no shared disposable resources. The host must await its invocations before disposing /// the application-owned factory infrastructure. @@ -42,7 +42,7 @@ public sealed class DefaultMcpToolHandler : IMcpToolHandler private static readonly JsonWriterOptions s_toolListJsonWriterOptions = new() { Indented = true }; - private readonly IHttpClientFactory _httpClientFactory; + private readonly IHttpMessageHandlerFactory? _httpMessageHandlerFactory; private readonly string _httpClientName; private readonly AllowedHttpHosts _allowedHosts; @@ -64,8 +64,7 @@ public sealed class DefaultMcpToolHandler : IMcpToolHandler /// Initializes a new instance of the class using an application-provided HTTP client factory. /// /// - /// The factory that creates a separate for every invocation. - /// Factory-created clients are disposed after MCP cleanup; the factory and its pooled handlers remain application-owned. + /// The application-owned factory. This parameter is retained for compatibility; this overload allows no destinations. /// /// /// The stable named-client configuration to use, or an empty string for the default configuration. @@ -80,13 +79,15 @@ public sealed class DefaultMcpToolHandler : IMcpToolHandler /// /// A required argument is . public DefaultMcpToolHandler(IHttpClientFactory httpClientFactory, string httpClientName) - : this([], httpClientFactory, httpClientName) { + Throw.IfNull(httpClientFactory); + this._httpClientName = Throw.IfNull(httpClientName); + this._allowedHosts = new([]); } /// /// Initializes a new instance of the class with allowed hosts - /// and an application-provided HTTP client factory. + /// and an application-provided HTTP message handler factory. /// /// /// The exact DNS names or IP literals, optionally including a port, the application permits. @@ -97,17 +98,20 @@ public DefaultMcpToolHandler(IHttpClientFactory httpClientFactory, string httpCl /// Both HTTP and HTTPS are supported; authorization matches the host and effective port, not the scheme. /// DNS resolution and routing remain the responsibility of the HTTP transport. /// - /// - /// The application-owned factory that creates a separate HTTP client for every invocation. - /// Configure cookies and automatic redirects off, certificate revocation checking on, and origin-bound - /// credential injection. Delegating handlers must preserve the validated destination. + /// + /// The application-owned factory supplying pooled HTTP handler pipelines. The primary handler must be + /// an or, on .NET, a SocketsHttpHandler, with cookies and automatic + /// redirects disabled. Enable certificate revocation checking and bind credentials to the intended origin + /// in delegating handlers. Delegating handlers must preserve the validated destination. + /// Per-invocation HTTP clients do not dispose factory-owned pipelines. Client configuration registered + /// through does not apply; configure headers in delegating handlers. /// /// The stable named-client configuration, or an empty string for the default configuration. /// A required argument is . /// An allowed host is invalid. - public DefaultMcpToolHandler(IEnumerable allowedHosts, IHttpClientFactory httpClientFactory, string httpClientName) + public DefaultMcpToolHandler(IEnumerable allowedHosts, IHttpMessageHandlerFactory httpMessageHandlerFactory, string httpClientName) { - this._httpClientFactory = Throw.IfNull(httpClientFactory); + this._httpMessageHandlerFactory = Throw.IfNull(httpMessageHandlerFactory); this._httpClientName = Throw.IfNull(httpClientName); this._allowedHosts = new(allowedHosts); } @@ -227,7 +231,11 @@ private async Task CreateClientAsync( throw new HttpRequestException("The request host and port are not in the allowed hosts list."); } - HttpClient httpClient = this._httpClientFactory.CreateClient(this._httpClientName); + IHttpMessageHandlerFactory factory = this._httpMessageHandlerFactory + ?? throw new InvalidOperationException("MCP invocations require an HTTP message handler factory."); + HttpMessageHandler messageHandler = factory.CreateHandler(this._httpClientName); + HttpTransportValidation.Validate(messageHandler); + HttpClient httpClient = new(messageHandler, disposeHandler: false); HttpClientTransport? transport = null; try { diff --git a/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative.Mcp/PublicAPI/net10.0/PublicAPI.Unshipped.txt b/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative.Mcp/PublicAPI/net10.0/PublicAPI.Unshipped.txt index b44ce8f4efb..932e29e4996 100644 --- a/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative.Mcp/PublicAPI/net10.0/PublicAPI.Unshipped.txt +++ b/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative.Mcp/PublicAPI/net10.0/PublicAPI.Unshipped.txt @@ -1,6 +1,6 @@ #nullable enable Microsoft.Agents.AI.Workflows.Declarative.Mcp.DefaultMcpToolHandler.DefaultMcpToolHandler(System.Net.Http.IHttpClientFactory! httpClientFactory, string! httpClientName) -> void -Microsoft.Agents.AI.Workflows.Declarative.Mcp.DefaultMcpToolHandler.DefaultMcpToolHandler(System.Collections.Generic.IEnumerable! allowedHosts, System.Net.Http.IHttpClientFactory! httpClientFactory, string! httpClientName) -> void +Microsoft.Agents.AI.Workflows.Declarative.Mcp.DefaultMcpToolHandler.DefaultMcpToolHandler(System.Collections.Generic.IEnumerable! allowedHosts, System.Net.Http.IHttpMessageHandlerFactory! httpMessageHandlerFactory, string! httpClientName) -> void Microsoft.Agents.AI.Workflows.Declarative.Mcp.DefaultMcpToolHandler.ProtocolVersion.get -> string? Microsoft.Agents.AI.Workflows.Declarative.Mcp.DefaultMcpToolHandler.ProtocolVersion.init -> void *REMOVED*Microsoft.Agents.AI.Workflows.Declarative.Mcp.DefaultMcpToolHandler.DefaultMcpToolHandler(System.Func!>? httpClientProvider = null) -> void diff --git a/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative.Mcp/PublicAPI/net472/PublicAPI.Unshipped.txt b/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative.Mcp/PublicAPI/net472/PublicAPI.Unshipped.txt index b44ce8f4efb..932e29e4996 100644 --- a/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative.Mcp/PublicAPI/net472/PublicAPI.Unshipped.txt +++ b/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative.Mcp/PublicAPI/net472/PublicAPI.Unshipped.txt @@ -1,6 +1,6 @@ #nullable enable Microsoft.Agents.AI.Workflows.Declarative.Mcp.DefaultMcpToolHandler.DefaultMcpToolHandler(System.Net.Http.IHttpClientFactory! httpClientFactory, string! httpClientName) -> void -Microsoft.Agents.AI.Workflows.Declarative.Mcp.DefaultMcpToolHandler.DefaultMcpToolHandler(System.Collections.Generic.IEnumerable! allowedHosts, System.Net.Http.IHttpClientFactory! httpClientFactory, string! httpClientName) -> void +Microsoft.Agents.AI.Workflows.Declarative.Mcp.DefaultMcpToolHandler.DefaultMcpToolHandler(System.Collections.Generic.IEnumerable! allowedHosts, System.Net.Http.IHttpMessageHandlerFactory! httpMessageHandlerFactory, string! httpClientName) -> void Microsoft.Agents.AI.Workflows.Declarative.Mcp.DefaultMcpToolHandler.ProtocolVersion.get -> string? Microsoft.Agents.AI.Workflows.Declarative.Mcp.DefaultMcpToolHandler.ProtocolVersion.init -> void *REMOVED*Microsoft.Agents.AI.Workflows.Declarative.Mcp.DefaultMcpToolHandler.DefaultMcpToolHandler(System.Func!>? httpClientProvider = null) -> void diff --git a/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative.Mcp/PublicAPI/net8.0/PublicAPI.Unshipped.txt b/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative.Mcp/PublicAPI/net8.0/PublicAPI.Unshipped.txt index b44ce8f4efb..932e29e4996 100644 --- a/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative.Mcp/PublicAPI/net8.0/PublicAPI.Unshipped.txt +++ b/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative.Mcp/PublicAPI/net8.0/PublicAPI.Unshipped.txt @@ -1,6 +1,6 @@ #nullable enable Microsoft.Agents.AI.Workflows.Declarative.Mcp.DefaultMcpToolHandler.DefaultMcpToolHandler(System.Net.Http.IHttpClientFactory! httpClientFactory, string! httpClientName) -> void -Microsoft.Agents.AI.Workflows.Declarative.Mcp.DefaultMcpToolHandler.DefaultMcpToolHandler(System.Collections.Generic.IEnumerable! allowedHosts, System.Net.Http.IHttpClientFactory! httpClientFactory, string! httpClientName) -> void +Microsoft.Agents.AI.Workflows.Declarative.Mcp.DefaultMcpToolHandler.DefaultMcpToolHandler(System.Collections.Generic.IEnumerable! allowedHosts, System.Net.Http.IHttpMessageHandlerFactory! httpMessageHandlerFactory, string! httpClientName) -> void Microsoft.Agents.AI.Workflows.Declarative.Mcp.DefaultMcpToolHandler.ProtocolVersion.get -> string? Microsoft.Agents.AI.Workflows.Declarative.Mcp.DefaultMcpToolHandler.ProtocolVersion.init -> void *REMOVED*Microsoft.Agents.AI.Workflows.Declarative.Mcp.DefaultMcpToolHandler.DefaultMcpToolHandler(System.Func!>? httpClientProvider = null) -> void diff --git a/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative.Mcp/PublicAPI/net9.0/PublicAPI.Unshipped.txt b/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative.Mcp/PublicAPI/net9.0/PublicAPI.Unshipped.txt index b44ce8f4efb..932e29e4996 100644 --- a/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative.Mcp/PublicAPI/net9.0/PublicAPI.Unshipped.txt +++ b/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative.Mcp/PublicAPI/net9.0/PublicAPI.Unshipped.txt @@ -1,6 +1,6 @@ #nullable enable Microsoft.Agents.AI.Workflows.Declarative.Mcp.DefaultMcpToolHandler.DefaultMcpToolHandler(System.Net.Http.IHttpClientFactory! httpClientFactory, string! httpClientName) -> void -Microsoft.Agents.AI.Workflows.Declarative.Mcp.DefaultMcpToolHandler.DefaultMcpToolHandler(System.Collections.Generic.IEnumerable! allowedHosts, System.Net.Http.IHttpClientFactory! httpClientFactory, string! httpClientName) -> void +Microsoft.Agents.AI.Workflows.Declarative.Mcp.DefaultMcpToolHandler.DefaultMcpToolHandler(System.Collections.Generic.IEnumerable! allowedHosts, System.Net.Http.IHttpMessageHandlerFactory! httpMessageHandlerFactory, string! httpClientName) -> void Microsoft.Agents.AI.Workflows.Declarative.Mcp.DefaultMcpToolHandler.ProtocolVersion.get -> string? Microsoft.Agents.AI.Workflows.Declarative.Mcp.DefaultMcpToolHandler.ProtocolVersion.init -> void *REMOVED*Microsoft.Agents.AI.Workflows.Declarative.Mcp.DefaultMcpToolHandler.DefaultMcpToolHandler(System.Func!>? httpClientProvider = null) -> void diff --git a/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative.Mcp/PublicAPI/netstandard2.0/PublicAPI.Unshipped.txt b/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative.Mcp/PublicAPI/netstandard2.0/PublicAPI.Unshipped.txt index b44ce8f4efb..932e29e4996 100644 --- a/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative.Mcp/PublicAPI/netstandard2.0/PublicAPI.Unshipped.txt +++ b/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative.Mcp/PublicAPI/netstandard2.0/PublicAPI.Unshipped.txt @@ -1,6 +1,6 @@ #nullable enable Microsoft.Agents.AI.Workflows.Declarative.Mcp.DefaultMcpToolHandler.DefaultMcpToolHandler(System.Net.Http.IHttpClientFactory! httpClientFactory, string! httpClientName) -> void -Microsoft.Agents.AI.Workflows.Declarative.Mcp.DefaultMcpToolHandler.DefaultMcpToolHandler(System.Collections.Generic.IEnumerable! allowedHosts, System.Net.Http.IHttpClientFactory! httpClientFactory, string! httpClientName) -> void +Microsoft.Agents.AI.Workflows.Declarative.Mcp.DefaultMcpToolHandler.DefaultMcpToolHandler(System.Collections.Generic.IEnumerable! allowedHosts, System.Net.Http.IHttpMessageHandlerFactory! httpMessageHandlerFactory, string! httpClientName) -> void Microsoft.Agents.AI.Workflows.Declarative.Mcp.DefaultMcpToolHandler.ProtocolVersion.get -> string? Microsoft.Agents.AI.Workflows.Declarative.Mcp.DefaultMcpToolHandler.ProtocolVersion.init -> void *REMOVED*Microsoft.Agents.AI.Workflows.Declarative.Mcp.DefaultMcpToolHandler.DefaultMcpToolHandler(System.Func!>? httpClientProvider = null) -> void diff --git a/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative/DefaultHttpRequestHandler.cs b/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative/DefaultHttpRequestHandler.cs index 936db6e5ce2..74fc8b20603 100644 --- a/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative/DefaultHttpRequestHandler.cs +++ b/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative/DefaultHttpRequestHandler.cs @@ -135,7 +135,7 @@ public async Task SendAsync(HttpRequestInfo request, Cancella } HttpMessageHandler messageHandler = this._httpMessageHandlerFactory.CreateHandler(this._httpClientName); - ValidateTransport(messageHandler); + HttpTransportValidation.Validate(messageHandler); using HttpClient client = new(messageHandler, disposeHandler: false); if (timeoutCts is not null) { @@ -178,39 +178,6 @@ public async Task SendAsync(HttpRequestInfo request, Cancella throw new HttpRequestException($"The maximum number of HTTP redirects ({MaxAutomaticRedirections}) was exceeded."); } - private static void ValidateTransport(HttpMessageHandler handler) - { - while (handler is DelegatingHandler delegatingHandler) - { - handler = delegatingHandler.InnerHandler - ?? throw new InvalidOperationException("The HTTP handler pipeline must have a primary handler."); - } - - bool allowAutoRedirect; - bool useCookies; - if (handler is HttpClientHandler httpClientHandler) - { - allowAutoRedirect = httpClientHandler.AllowAutoRedirect; - useCookies = httpClientHandler.UseCookies; - } -#if NET - else if (handler is SocketsHttpHandler socketsHttpHandler) - { - allowAutoRedirect = socketsHttpHandler.AllowAutoRedirect; - useCookies = socketsHttpHandler.UseCookies; - } -#endif - else - { - throw new InvalidOperationException("The HTTP handler pipeline must use a supported primary HTTP handler."); - } - - if (allowAutoRedirect || useCookies) - { - throw new InvalidOperationException("The primary HTTP handler must disable automatic redirects and cookies."); - } - } - private static async Task ReadResponseBodyAsStringAsync(HttpContent content, CancellationToken cancellationToken) { #if NET diff --git a/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative/README.md b/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative/README.md index 8ffe8bc8842..bc31438b788 100644 --- a/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative/README.md +++ b/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative/README.md @@ -69,9 +69,10 @@ Standalone GET/SSE streams for unsolicited server messages are disabled because this handler performs request-scoped tool operations, not background subscriptions. SSE responses to individual tool requests remain supported. -Each factory-created HTTP client is disposed with its invocation. Construct the -handler with an application-owned `IHttpClientFactory` through -`DefaultMcpToolHandler(IEnumerable, IHttpClientFactory, string)`. Supply +Each per-invocation HTTP client is disposed with its invocation without disposing +the pooled handler pipeline. Construct the handler with an application-owned +`IHttpMessageHandlerFactory` through +`DefaultMcpToolHandler(IEnumerable, IHttpMessageHandlerFactory, string)`. Supply allowed hosts from application configuration, not workflow content. An empty list, including the existing two-argument constructor, denies all destinations. Hosts are exact DNS names or IP literals with optional ports; omitted ports @@ -82,7 +83,7 @@ discovery. Workflow-supplied `Host` headers are rejected. Both HTTP and HTTPS are supported; enforce HTTPS separately if required. Allowing a name trusts the application's DNS and network routing. The third argument is a stable named-client configuration, or `string.Empty` for the -default configuration. Factory-created wrappers are owned by the invocation; +default configuration. HTTP client wrappers are owned by the invocation; the factory and its pooled handlers remain application-owned. There is no parameterless constructor or internally created HTTP pipeline. Configure handler reuse in the factory to avoid connection churn and port exhaustion. @@ -101,10 +102,18 @@ services.AddHttpClient("workflow-mcp") .SetHandlerLifetime(TimeSpan.FromMinutes(2)); services.AddSingleton(serviceProvider => new DefaultMcpToolHandler( - ["mcp.example.com"], serviceProvider.GetRequiredService(), "workflow-mcp")); + ["mcp.example.com"], serviceProvider.GetRequiredService(), "workflow-mcp")); ``` -Factory clients use the application's transport-policy configuration. +The pipeline must terminate in an `HttpClientHandler`, or on .NET a +`SocketsHttpHandler`, with automatic redirects and cookies disabled. The handler +rejects unknown transports, incomplete pipelines, or either setting enabled +before sending. Redirect responses are not followed. +Application-owned delegating handlers must preserve the validated destination +and must not follow redirects themselves. Client configuration callbacks +registered through `IHttpClientFactory`, including default headers, timeout, +and base address, do not apply. Configure headers and authentication in +delegating handlers instead. Restrict credential-injecting handlers to the configured server origin. Do not store per-workflow credentials, cookies, or mutable origin policy in pooled handlers. Use a bounded set of configuration @@ -117,7 +126,7 @@ discovery and legacy fallback. If the server's supported protocol is known, set the init-only property when constructing the handler: ```csharp -DefaultMcpToolHandler handler = new(["mcp.example.com"], httpClientFactory, "workflow-mcp") +DefaultMcpToolHandler handler = new(["mcp.example.com"], httpMessageHandlerFactory, "workflow-mcp") { ProtocolVersion = "2025-11-25" }; @@ -141,9 +150,9 @@ original error. Parameterless construction, the callback-based constructor, and handler-level `DisposeAsync`/`IAsyncDisposable` are not supported. Migrate default construction -and callback authentication to application-owned named factory -clients and origin-bound delegating handlers; every factory call must return a -fresh HTTP client wrapper owned by the invocation, not a borrowed shared client. +and callback authentication to an application-owned named message-handler factory +and origin-bound delegating handlers. The default handler creates a fresh HTTP +client wrapper per invocation while the factory owns the pooled pipeline. Remove handler `await using`/disposal calls and await active invocations in the host. This also changes the previous default behavior, which retained initialized MCP diff --git a/dotnet/src/Shared/Http/HttpTransportValidation.cs b/dotnet/src/Shared/Http/HttpTransportValidation.cs new file mode 100644 index 00000000000..18d20ab52bc --- /dev/null +++ b/dotnet/src/Shared/Http/HttpTransportValidation.cs @@ -0,0 +1,43 @@ +// Copyright (c) Microsoft. All rights reserved. + +using System; +using System.Net.Http; + +namespace Microsoft.Shared.Diagnostics; + +/// Validates application-owned HTTP handler pipelines. +internal static class HttpTransportValidation +{ + public static void Validate(HttpMessageHandler handler) + { + while (handler is DelegatingHandler delegatingHandler) + { + handler = delegatingHandler.InnerHandler + ?? throw new InvalidOperationException("The HTTP handler pipeline must have a primary handler."); + } + + bool allowAutoRedirect; + bool useCookies; + if (handler is HttpClientHandler httpClientHandler) + { + allowAutoRedirect = httpClientHandler.AllowAutoRedirect; + useCookies = httpClientHandler.UseCookies; + } +#if NET + else if (handler is SocketsHttpHandler socketsHttpHandler) + { + allowAutoRedirect = socketsHttpHandler.AllowAutoRedirect; + useCookies = socketsHttpHandler.UseCookies; + } +#endif + else + { + throw new InvalidOperationException("The HTTP handler pipeline must use a supported primary HTTP handler."); + } + + if (allowAutoRedirect || useCookies) + { + throw new InvalidOperationException("The primary HTTP handler must disable automatic redirects and cookies."); + } + } +} diff --git a/dotnet/tests/Microsoft.Agents.AI.Workflows.Declarative.IntegrationTests/InvokeToolWorkflowTest.cs b/dotnet/tests/Microsoft.Agents.AI.Workflows.Declarative.IntegrationTests/InvokeToolWorkflowTest.cs index fdae167c20f..e0632ec6d78 100644 --- a/dotnet/tests/Microsoft.Agents.AI.Workflows.Declarative.IntegrationTests/InvokeToolWorkflowTest.cs +++ b/dotnet/tests/Microsoft.Agents.AI.Workflows.Declarative.IntegrationTests/InvokeToolWorkflowTest.cs @@ -197,7 +197,7 @@ private async Task RunInvokeMcpToolTestAsync( CheckCertificateRevocationList = true }); using ServiceProvider serviceProvider = services.BuildServiceProvider(); - DefaultMcpToolHandler mcpToolProvider = new(["learn.microsoft.com"], serviceProvider.GetRequiredService(), "workflow-mcp"); + DefaultMcpToolHandler mcpToolProvider = new(["learn.microsoft.com"], serviceProvider.GetRequiredService(), "workflow-mcp"); DeclarativeWorkflowOptions workflowOptions = await this.CreateOptionsAsync( externalConversation: false, mcpToolProvider: mcpToolProvider); diff --git a/dotnet/tests/Microsoft.Agents.AI.Workflows.Declarative.Mcp.UnitTests/DefaultMcpToolHandlerLifetimeTests.cs b/dotnet/tests/Microsoft.Agents.AI.Workflows.Declarative.Mcp.UnitTests/DefaultMcpToolHandlerLifetimeTests.cs index fee73650d43..f1b8abe87bf 100644 --- a/dotnet/tests/Microsoft.Agents.AI.Workflows.Declarative.Mcp.UnitTests/DefaultMcpToolHandlerLifetimeTests.cs +++ b/dotnet/tests/Microsoft.Agents.AI.Workflows.Declarative.Mcp.UnitTests/DefaultMcpToolHandlerLifetimeTests.cs @@ -26,13 +26,102 @@ namespace Microsoft.Agents.AI.Workflows.Declarative.Mcp.UnitTests; /// public sealed class DefaultMcpToolHandlerLifetimeTests { + [Theory] + [InlineData(true, false)] + [InlineData(false, true)] + public async Task Factory_InvalidTransportSettings_RejectBeforeInitializationAsync(bool allowAutoRedirect, bool useCookies) + { + // Arrange + using ProtocolStub stub = new(); + await using LoopbackMcpServer server = new(stub); + ServiceCollection services = new(); + services.AddHttpClient("workflow-mcp") + .ConfigurePrimaryHttpMessageHandler(() => new HttpClientHandler + { + AllowAutoRedirect = allowAutoRedirect, + UseCookies = useCookies, + CheckCertificateRevocationList = true + }); + using ServiceProvider serviceProvider = services.BuildServiceProvider(); + DefaultMcpToolHandler handler = new( + [new Uri(server.Endpoint).Authority], serviceProvider.GetRequiredService(), "workflow-mcp"); + using CancellationTokenSource timeout = new(TimeSpan.FromSeconds(10)); + + // Act + Exception? error = await Record.ExceptionAsync(() => handler.InvokeToolAsync( + server.Endpoint, null, "ping", null, null, null, timeout.Token)); + + // Assert + Assert.IsType(error); + Assert.Equal(0, stub.Initializations); + Assert.Equal(0, server.RequestCount); + } + + [Theory] + [InlineData(307)] + [InlineData(308)] + public async Task Factory_RedirectResponse_DoesNotContactOtherDestinationAsync(int statusCode) + { + // Arrange + using ProtocolStub destinationStub = new(); + await using LoopbackMcpServer destination = new(destinationStub); + using ProtocolStub originStub = new() + { + RedirectEndpoint = new Uri(destination.Endpoint), + RedirectStatusCode = (HttpStatusCode)statusCode + }; + await using LoopbackMcpServer origin = new(originStub); + ServiceCollection services = new(); + services.AddHttpClient("workflow-mcp") + .ConfigurePrimaryHttpMessageHandler(() => new HttpClientHandler + { + AllowAutoRedirect = false, + UseCookies = false, + CheckCertificateRevocationList = true + }); + using ServiceProvider serviceProvider = services.BuildServiceProvider(); + DefaultMcpToolHandler handler = new( + [new Uri(origin.Endpoint).Authority], serviceProvider.GetRequiredService(), "workflow-mcp"); + using CancellationTokenSource timeout = new(TimeSpan.FromSeconds(10)); + + // Act + Exception? error = await Record.ExceptionAsync(() => handler.InvokeToolAsync( + origin.Endpoint, null, "ping", null, null, null, timeout.Token)); + + // Assert + Assert.NotNull(error); + Assert.True(origin.RequestCount > 0); + Assert.Equal(0, destination.RequestCount); + } + + [Theory] + [InlineData(false)] + [InlineData(true)] + public async Task Factory_UnsupportedPipeline_RejectsBeforeSendingAsync(bool incomplete) + { + // Arrange + using HttpMessageHandler pipeline = incomplete + ? new Mock { CallBase = true }.Object + : new Mock { CallBase = true }.Object; + Mock factory = new(); + factory.Setup(f => f.CreateHandler("workflow-mcp")).Returns(pipeline); + DefaultMcpToolHandler handler = new(["mcp.example"], factory.Object, "workflow-mcp"); + + // Act + await Assert.ThrowsAsync(() => handler.InvokeToolAsync( + "https://mcp.example/api", null, "ping", null, null, null)); + + // Assert + factory.Verify(f => f.CreateHandler("workflow-mcp"), Times.Once); + } + [Theory] [InlineData(null)] [InlineData("2025-11-25")] public async Task Factory_LoopbackInvocations_ReuseHandlerAndDisposeSeparateSessionsBeforeReturningAsync(string? protocolVersion) { // Arrange - ProtocolStub stub = new(); + using ProtocolStub stub = new(); await using LoopbackMcpServer server = new(stub); int primaryHandlersCreated = 0; ServiceCollection services = new(); @@ -48,7 +137,7 @@ public async Task Factory_LoopbackInvocations_ReuseHandlerAndDisposeSeparateSess }; }); using ServiceProvider serviceProvider = services.BuildServiceProvider(); - DefaultMcpToolHandler handler = new([new Uri(server.Endpoint).Authority], serviceProvider.GetRequiredService(), "workflow-mcp") + DefaultMcpToolHandler handler = new([new Uri(server.Endpoint).Authority], serviceProvider.GetRequiredService(), "workflow-mcp") { ProtocolVersion = protocolVersion }; @@ -85,7 +174,7 @@ public async Task Factory_LoopbackInvocations_ReuseHandlerAndDisposeSeparateSess public async Task ProtocolSelection_PreservesPerInvocationInitializationAsync(string? protocolVersion, int expectedDiscoveries) { // Arrange - ProtocolStub stub = new(); + using ProtocolStub stub = new(); DefaultMcpToolHandler handler = CreateHandler(stub, protocolVersion); using CancellationTokenSource timeout = new(TimeSpan.FromSeconds(10)); @@ -110,7 +199,7 @@ public async Task ProtocolSelection_PreservesPerInvocationInitializationAsync(st public async Task ExplicitNewProtocol_DiscoversWithoutLegacyFallbackAsync(bool supportsDiscovery) { // Arrange - ProtocolStub stub = new() { SupportsDiscovery = supportsDiscovery }; + using ProtocolStub stub = new() { SupportsDiscovery = supportsDiscovery }; DefaultMcpToolHandler handler = CreateHandler(stub, "2026-07-28"); using CancellationTokenSource timeout = new(TimeSpan.FromSeconds(10)); @@ -130,14 +219,14 @@ public async Task ExplicitNewProtocol_DiscoversWithoutLegacyFallbackAsync(bool s Assert.Equal(1, stub.DiscoverRequests); Assert.Equal(0, stub.Initializations); Assert.Equal(0, stub.Terminations); - Assert.Single(stub.Handlers).Protected().Verify("Dispose", Times.Once(), ItExpr.Is(disposing => disposing)); + Assert.Single(stub.Handlers).Protected().Verify("Dispose", Times.Never(), ItExpr.Is(disposing => disposing)); } [Fact] public async Task ExplicitLegacyProtocol_RejectsVersionMismatchAndDisposesResourcesAsync() { // Arrange - ProtocolStub stub = new() + using ProtocolStub stub = new() { InitializationProtocolVersion = "2025-06-18", OnOperation = _ => Assert.Fail("A tool must not run after a protocol version mismatch.") @@ -153,7 +242,7 @@ public async Task ExplicitLegacyProtocol_RejectsVersionMismatchAndDisposesResour Assert.Equal(0, stub.DiscoverRequests); Assert.Equal(1, stub.Initializations); Assert.Equal(1, stub.Terminations); - Assert.Single(stub.Handlers).Protected().Verify("Dispose", Times.Once(), ItExpr.Is(disposing => disposing)); + Assert.Single(stub.Handlers).Protected().Verify("Dispose", Times.Never(), ItExpr.Is(disposing => disposing)); } [Theory] @@ -162,7 +251,7 @@ public async Task ExplicitLegacyProtocol_RejectsVersionMismatchAndDisposesResour public async Task RequestOnlyInvocation_DoesNotOpenStandaloneStreamAsync(bool useEventStream) { // Arrange - ProtocolStub stub = new() { UseEventStreamResult = useEventStream }; + using ProtocolStub stub = new() { UseEventStreamResult = useEventStream }; DefaultMcpToolHandler handler = CreateHandler(stub); using CancellationTokenSource timeout = new(TimeSpan.FromSeconds(10)); @@ -181,7 +270,7 @@ public async Task RequestOnlyInvocation_DoesNotOpenStandaloneStreamAsync(bool us public async Task SequentialInvocations_UseSeparateSessionsAsync() { // Arrange - ProtocolStub stub = new(); + using ProtocolStub stub = new(); DefaultMcpToolHandler handler = CreateHandler(stub); using CancellationTokenSource timeout = new(TimeSpan.FromSeconds(10)); @@ -217,10 +306,10 @@ await Assert.ThrowsAsync(() => handler.InvokeToolAsync( } [Fact] - public async Task DiscoveryAndToolCalls_DisposeSeparateHttpClientsAndSessionsAsync() + public async Task DiscoveryAndToolCalls_DisposeSessionsWithoutDisposingFactoryHandlersAsync() { // Arrange - ProtocolStub stub = new(); + using ProtocolStub stub = new(); DefaultMcpToolHandler handler = CreateHandler(stub); using CancellationTokenSource timeout = new(TimeSpan.FromSeconds(10)); @@ -232,9 +321,9 @@ public async Task DiscoveryAndToolCalls_DisposeSeparateHttpClientsAndSessionsAsy Assert.Equal(2, stub.Initializations); Assert.Equal(2, stub.Terminations); Assert.Equal(2, stub.Handlers.Count); - foreach (Mock transport in stub.Handlers) + foreach (Mock transport in stub.Handlers) { - transport.Protected().Verify("Dispose", Times.Once(), ItExpr.Is(disposing => disposing)); + transport.Protected().Verify("Dispose", Times.Never(), ItExpr.Is(disposing => disposing)); } } @@ -242,7 +331,7 @@ public async Task DiscoveryAndToolCalls_DisposeSeparateHttpClientsAndSessionsAsy public async Task DifferentConnectionNames_DoNotRetainSessionsAsync() { // Arrange - ProtocolStub stub = new(); + using ProtocolStub stub = new(); DefaultMcpToolHandler handler = CreateHandler(stub); using CancellationTokenSource timeout = new(TimeSpan.FromSeconds(10)); @@ -263,7 +352,7 @@ await handler.InvokeToolAsync( public async Task DifferentOrigins_KeepRequestConfigurationAsync() { // Arrange - ProtocolStub stub = new(); + using ProtocolStub stub = new(); DefaultMcpToolHandler handler = CreateHandler(stub, allowedHosts: ["first.example", "second.example"]); using CancellationTokenSource timeout = new(TimeSpan.FromSeconds(10)); @@ -291,28 +380,20 @@ await handler.InvokeToolAsync( [Theory] [InlineData("session")] [InlineData(DefaultMcpToolHandler.ListToolsToolName)] - public async Task Factory_SequentialInvocations_OwnClientsButReusePooledHandlersAsync(string toolName) + public async Task Factory_SequentialInvocations_ReusePooledHandlersAndConfigurationAsync(string toolName) { // Arrange const string ClientName = "workflow-mcp"; - ProtocolStub stub = new() - { - OnOperation = request => Assert.Equal("configured", Assert.Single(request.Headers.GetValues("X-Client-Configuration"))) - }; + using ProtocolStub stub = new(); ServiceCollection services = new(); - services.AddHttpClient(ClientName, client => client.DefaultRequestHeaders.Add("X-Client-Configuration", "configured")) + services.AddHttpClient(ClientName, client => client.DefaultRequestHeaders.Add("X-Client-Configuration", "client-only")) .ConfigurePrimaryHttpMessageHandler(stub.CreateMessageHandler); using ServiceProvider serviceProvider = services.BuildServiceProvider(); - IHttpClientFactory applicationFactory = serviceProvider.GetRequiredService(); - List clients = []; - Mock factory = new(); + IHttpMessageHandlerFactory applicationFactory = serviceProvider.GetRequiredService(); + stub.OnOperation = request => Assert.False(request.Headers.Contains("X-Client-Configuration")); + Mock factory = new(); Mock factoryOwner = factory.As(); - factory.Setup(f => f.CreateClient(ClientName)).Returns(() => - { - HttpClient client = applicationFactory.CreateClient(ClientName); - clients.Add(client); - return client; - }); + factory.Setup(f => f.CreateHandler(ClientName)).Returns(() => applicationFactory.CreateHandler(ClientName)); DefaultMcpToolHandler handler = new(["mcp.example"], factory.Object, ClientName); using CancellationTokenSource timeout = new(TimeSpan.FromSeconds(10)); @@ -321,8 +402,6 @@ public async Task Factory_SequentialInvocations_OwnClientsButReusePooledHandlers McpServerToolResultContent second = await InvokeAsync(handler, toolName, timeout.Token); // Assert - Assert.Equal(2, clients.Count); - Assert.NotSame(clients[0], clients[1]); if (toolName == "session") { Assert.NotNull(first.Outputs); @@ -332,17 +411,12 @@ public async Task Factory_SequentialInvocations_OwnClientsButReusePooledHandlers Assert.IsType(Assert.Single(second.Outputs)).Text); } - foreach (HttpClient client in clients) - { - await Assert.ThrowsAsync(() => client.GetAsync(new Uri("https://mcp.example/api"), timeout.Token)); - } - Assert.Equal(2, stub.Initializations); Assert.Equal(2, stub.Terminations); - factory.Verify(f => f.CreateClient(ClientName), Times.Exactly(2)); + factory.Verify(f => f.CreateHandler(ClientName), Times.Exactly(2)); factoryOwner.Verify(f => f.Dispose(), Times.Never); Assert.Single(stub.Handlers).Protected().Verify("Dispose", Times.Never(), ItExpr.Is(disposing => disposing)); - using HttpClient subsequentClient = applicationFactory.CreateClient(ClientName); + using HttpClient subsequentClient = new(applicationFactory.CreateHandler(ClientName), disposeHandler: false); using HttpResponseMessage response = await subsequentClient.GetAsync(new Uri("https://mcp.example/api"), timeout.Token); Assert.Equal(HttpStatusCode.MethodNotAllowed, response.StatusCode); } @@ -351,18 +425,12 @@ public async Task Factory_SequentialInvocations_OwnClientsButReusePooledHandlers [InlineData(true, false)] [InlineData(false, true)] [InlineData(false, false)] - public async Task InvocationFailure_DisposesClientAndAllowsNextInvocationAsync(bool failInitialization, bool failOperation) + public async Task InvocationFailure_PreservesFactoryHandlersAndAllowsNextInvocationAsync(bool failInitialization, bool failOperation) { // Arrange - ProtocolStub stub = new() { FailInitialization = failInitialization, FailOperation = failOperation }; - List clients = []; - Mock factory = new(); - factory.Setup(f => f.CreateClient("workflow-mcp")).Returns(() => - { - HttpClient client = new(stub.CreateMessageHandler()); - clients.Add(client); - return client; - }); + using ProtocolStub stub = new() { FailInitialization = failInitialization, FailOperation = failOperation }; + Mock factory = new(); + factory.Setup(f => f.CreateHandler("workflow-mcp")).Returns(stub.CreateMessageHandler); using SemaphoreSlim started = new(0); bool cancel = !failInitialization && !failOperation; if (cancel) @@ -391,9 +459,7 @@ public async Task InvocationFailure_DisposesClientAndAllowsNextInvocationAsync(b await Assert.ThrowsAsync(() => invocation); } - HttpClient firstClient = Assert.Single(clients); - await Assert.ThrowsAsync(() => firstClient.GetAsync(new Uri("https://mcp.example/api"), timeout.Token)); - Assert.Single(stub.Handlers).Protected().Verify("Dispose", Times.Once(), ItExpr.Is(disposing => disposing)); + Assert.Single(stub.Handlers).Protected().Verify("Dispose", Times.Never(), ItExpr.Is(disposing => disposing)); stub.BeforeOperationAsync = null; stub.FailInitialization = false; stub.FailOperation = false; @@ -402,22 +468,21 @@ public async Task InvocationFailure_DisposesClientAndAllowsNextInvocationAsync(b // Assert Assert.NotNull(result.Outputs); Assert.Equal("ok", Assert.IsType(Assert.Single(result.Outputs)).Text); - Assert.Equal(2, clients.Count); + Assert.Equal(2, stub.Handlers.Count); Assert.Equal(failInitialization ? 1 : 2, stub.Initializations); Assert.Equal(failInitialization ? 1 : 2, stub.Terminations); - await Assert.ThrowsAsync(() => clients[1].GetAsync(new Uri("https://mcp.example/api"), timeout.Token)); - stub.Handlers[1].Protected().Verify("Dispose", Times.Once(), ItExpr.Is(disposing => disposing)); + stub.Handlers[1].Protected().Verify("Dispose", Times.Never(), ItExpr.Is(disposing => disposing)); } [Fact] public async Task FactoryFailure_DoesNotPreventNextInvocationAsync() { // Arrange - ProtocolStub stub = new(); - Mock factory = new(); - factory.SetupSequence(f => f.CreateClient("workflow-mcp")) + using ProtocolStub stub = new(); + Mock factory = new(); + factory.SetupSequence(f => f.CreateHandler("workflow-mcp")) .Throws(new InvalidOperationException("Factory unavailable.")) - .Returns(() => new HttpClient(stub.CreateMessageHandler())); + .Returns(stub.CreateMessageHandler); DefaultMcpToolHandler handler = new(["mcp.example"], factory.Object, "workflow-mcp"); using CancellationTokenSource timeout = new(TimeSpan.FromSeconds(10)); @@ -440,7 +505,7 @@ public async Task FactoryFailure_DoesNotPreventNextInvocationAsync() public async Task ConcurrentInvocations_DoNotCoalesceOrSerializeSessionsAsync(bool reuseHandlers) { // Arrange - ProtocolStub stub = new(); + using ProtocolStub stub = new(); using ServiceProvider? serviceProvider = reuseHandlers ? new ServiceCollection().AddHttpClient("workflow-mcp") .ConfigurePrimaryHttpMessageHandler(stub.CreateMessageHandler).Services.BuildServiceProvider() @@ -453,7 +518,7 @@ public async Task ConcurrentInvocations_DoNotCoalesceOrSerializeSessionsAsync(bo await finish.WaitAsync(token); }; DefaultMcpToolHandler handler = serviceProvider is not null - ? new(["mcp.example"], serviceProvider.GetRequiredService(), "workflow-mcp") + ? new(["mcp.example"], serviceProvider.GetRequiredService(), "workflow-mcp") : CreateHandler(stub); using CancellationTokenSource timeout = new(TimeSpan.FromSeconds(10)); @@ -494,7 +559,7 @@ public async Task ConcurrentInvocations_DoNotCoalesceOrSerializeSessionsAsync(bo public async Task ActiveInvocation_RetainsResourcesUntilCompletionAsync(bool pauseInitialization) { // Arrange - ProtocolStub stub = new(); + using ProtocolStub stub = new(); using SemaphoreSlim started = new(0); using SemaphoreSlim finish = new(0); async Task PauseAsync(CancellationToken token) @@ -518,7 +583,7 @@ async Task PauseAsync(CancellationToken token) // Act Task invocation = InvokeAsync(handler, "ping", timeout.Token); await started.WaitAsync(timeout.Token); - Mock transport = Assert.Single(stub.Handlers); + Mock transport = Assert.Single(stub.Handlers); transport.Protected().Verify("Dispose", Times.Never(), ItExpr.Is(disposing => disposing)); finish.Release(); await invocation; @@ -526,21 +591,20 @@ async Task PauseAsync(CancellationToken token) // Assert Assert.Equal(1, stub.Initializations); Assert.Equal(1, stub.Terminations); - transport.Protected().Verify("Dispose", Times.Once(), ItExpr.Is(disposing => disposing)); + transport.Protected().Verify("Dispose", Times.Never(), ItExpr.Is(disposing => disposing)); } [Theory] [InlineData(false, true)] [InlineData(true, false)] [InlineData(false, false)] - public async Task CleanupFailure_DoesNotReplaceOutcomeAsync(bool failInitialization, bool failOperation) + public async Task FactoryHandlerCleanup_IsNotPerformedByInvocationAsync(bool failInitialization, bool failOperation) { // Arrange - ProtocolStub stub = new() + using ProtocolStub stub = new() { FailInitialization = failInitialization, - FailOperation = failOperation, - FailTransportDisposal = true + FailOperation = failOperation }; DefaultMcpToolHandler handler = CreateHandler(stub); using CancellationTokenSource timeout = new(TimeSpan.FromSeconds(10)); @@ -562,10 +626,9 @@ public async Task CleanupFailure_DoesNotReplaceOutcomeAsync(bool failInitializat // Assert Assert.Equal(failInitialization ? 0 : 1, stub.Terminations); - Assert.Contains("Failed to dispose MCP ", listener.Output); - Assert.Contains("transport cleanup failed", listener.Output); + Assert.DoesNotContain("Failed to dispose MCP ", listener.Output); Assert.Single(stub.Handlers).Protected().Verify( - "Dispose", Times.Once(), ItExpr.Is(disposing => disposing)); + "Dispose", Times.Never(), ItExpr.Is(disposing => disposing)); } finally { @@ -629,7 +692,7 @@ public async Task ClientConnection_SessionCleanupFailure_StillCleansUpTransportA public async Task AllowedDestination_PreservesToolInvocationAsync(string allowedHost, string serverUrl) { // Arrange - ProtocolStub stub = new(); + using ProtocolStub stub = new(); DefaultMcpToolHandler handler = CreateHandler(stub, allowedHosts: [allowedHost]); using CancellationTokenSource timeout = new(TimeSpan.FromSeconds(10)); @@ -648,7 +711,7 @@ public async Task AllowedDestination_PreservesToolInvocationAsync(string allowed public async Task AllowedHosts_AreCopiedAtConstructionAsync() { // Arrange - ProtocolStub stub = new(); + using ProtocolStub stub = new(); List allowedHosts = ["mcp.example"]; DefaultMcpToolHandler handler = CreateHandler(stub, allowedHosts: allowedHosts); allowedHosts.Clear(); @@ -669,8 +732,8 @@ await Assert.ThrowsAsync(() => handler.InvokeToolAsync( private static DefaultMcpToolHandler CreateHandler( ProtocolStub stub, string? protocolVersion = null, IEnumerable? allowedHosts = null) { - Mock factory = new(); - factory.Setup(f => f.CreateClient("workflow-mcp")).Returns(() => new HttpClient(stub.CreateMessageHandler())); + Mock factory = new(); + factory.Setup(f => f.CreateHandler("workflow-mcp")).Returns(stub.CreateMessageHandler); return new(allowedHosts ?? ["mcp.example"], factory.Object, "workflow-mcp") { ProtocolVersion = protocolVersion }; } @@ -678,7 +741,7 @@ private static Task InvokeAsync( DefaultMcpToolHandler handler, string toolName, CancellationToken cancellationToken) => handler.InvokeToolAsync("https://mcp.example/api", null, toolName, null, null, null, cancellationToken); - private sealed class ProtocolStub + private sealed class ProtocolStub : IDisposable { private int _initializations; private int _terminations; @@ -689,35 +752,51 @@ private sealed class ProtocolStub public int Terminations => this._terminations; public int StandaloneGetRequests => this._standaloneGetRequests; public int DiscoverRequests => this._discoverRequests; - public List> Handlers { get; } = []; + public List> Handlers { get; } = []; public Func? BeforeInitializationAsync { get; set; } public Func? BeforeOperationAsync { get; set; } public Action? OnOperation { get; set; } public bool FailInitialization { get; set; } public bool FailOperation { get; set; } - public bool FailTransportDisposal { get; set; } public bool UseEventStreamResult { get; set; } public bool SupportsDiscovery { get; set; } public string? InitializationProtocolVersion { get; set; } + public Uri? RedirectEndpoint { get; set; } + public HttpStatusCode RedirectStatusCode { get; set; } - public HttpMessageHandler CreateMessageHandler() + public DelegatingHandler CreateMessageHandler() { - Mock handler = new() { CallBase = true }; + Mock handler = new() { CallBase = true }; + handler.Object.InnerHandler = new HttpClientHandler + { + AllowAutoRedirect = false, + UseCookies = false, + CheckCertificateRevocationList = true + }; handler.Protected() .Setup>("SendAsync", ItExpr.IsAny(), ItExpr.IsAny()) .Returns(this.SendAsync); - if (this.FailTransportDisposal) - { - handler.Protected().Setup("Dispose", ItExpr.Is(disposing => disposing)) - .Throws(new InvalidOperationException("transport cleanup failed")); - } - this.Handlers.Add(handler); return handler.Object; } + public void Dispose() + { + foreach (Mock handler in this.Handlers) + { + handler.Object.Dispose(); + } + } + public async Task SendAsync(HttpRequestMessage request, CancellationToken cancellationToken) { + if (this.RedirectEndpoint is { } redirectEndpoint) + { + HttpResponseMessage redirectResponse = EmptyResponse(this.RedirectStatusCode, request); + redirectResponse.Headers.Location = redirectEndpoint; + return redirectResponse; + } + if (request.Method == HttpMethod.Get) { Interlocked.Increment(ref this._standaloneGetRequests); @@ -830,6 +909,7 @@ private sealed class LoopbackMcpServer : IAsyncDisposable private readonly TcpListener _listener = new(IPAddress.Loopback, 0); private readonly CancellationTokenSource _shutdown = new(TimeSpan.FromSeconds(30)); private readonly Task _requestsTask; + private int _requestCount; public LoopbackMcpServer(ProtocolStub stub) { @@ -839,6 +919,7 @@ public LoopbackMcpServer(ProtocolStub stub) } public string Endpoint { get; } + public int RequestCount => this._requestCount; public List TerminatedSessions { get; } = []; @@ -871,6 +952,7 @@ private async Task ProcessRequestsAsync(ProtocolStub stub) using CancellationTokenRegistration closeClient = cancellationToken.Register(client.Close); using NetworkStream stream = client.GetStream(); string requestLine = await ReadLineAsync(stream, cancellationToken); + Interlocked.Increment(ref this._requestCount); using HttpRequestMessage request = new(new HttpMethod(requestLine.Split(' ')[0]), this.Endpoint); Dictionary headers = new(StringComparer.OrdinalIgnoreCase); string header; diff --git a/dotnet/tests/Microsoft.Agents.AI.Workflows.Declarative.Mcp.UnitTests/DefaultMcpToolHandlerTests.cs b/dotnet/tests/Microsoft.Agents.AI.Workflows.Declarative.Mcp.UnitTests/DefaultMcpToolHandlerTests.cs index cdf0af37251..8db03c9522b 100644 --- a/dotnet/tests/Microsoft.Agents.AI.Workflows.Declarative.Mcp.UnitTests/DefaultMcpToolHandlerTests.cs +++ b/dotnet/tests/Microsoft.Agents.AI.Workflows.Declarative.Mcp.UnitTests/DefaultMcpToolHandlerTests.cs @@ -110,7 +110,7 @@ public async Task InvokeToolAsync_WithoutAllowedHosts_ShouldRejectBeforeClientCr public async Task InvokeToolAsync_WithEmptyAllowedHosts_ShouldRejectBeforeClientCreationAsync(string toolName) { // Arrange - Mock factory = new(MockBehavior.Strict); + Mock factory = new(MockBehavior.Strict); DefaultMcpToolHandler handler = new([], factory.Object, "workflow-mcp"); // Act @@ -118,7 +118,7 @@ await Assert.ThrowsAsync(() => handler.InvokeToolAsync( "https://example.com/mcp", null, toolName, null, null, null)); // Assert - factory.Verify(f => f.CreateClient(It.IsAny()), Times.Never); + factory.Verify(f => f.CreateHandler(It.IsAny()), Times.Never); } [Theory] @@ -131,7 +131,7 @@ await Assert.ThrowsAsync(() => handler.InvokeToolAsync( public async Task InvokeToolAsync_WithUnlistedDestination_ShouldRejectBeforeClientCreationAsync(string serverUrl) { // Arrange - Mock factory = new(MockBehavior.Strict); + Mock factory = new(MockBehavior.Strict); DefaultMcpToolHandler handler = new(["example.com"], factory.Object, "workflow-mcp"); // Act @@ -140,7 +140,7 @@ public async Task InvokeToolAsync_WithUnlistedDestination_ShouldRejectBeforeClie // Assert Assert.Contains("allowed hosts", exception.Message); - factory.Verify(f => f.CreateClient(It.IsAny()), Times.Never); + factory.Verify(f => f.CreateHandler(It.IsAny()), Times.Never); } [Theory] @@ -149,7 +149,7 @@ public async Task InvokeToolAsync_WithUnlistedDestination_ShouldRejectBeforeClie public async Task InvokeToolAsync_WithHostHeader_ShouldRejectBeforeClientCreationAsync(string headerName) { // Arrange - Mock factory = new(MockBehavior.Strict); + Mock factory = new(MockBehavior.Strict); DefaultMcpToolHandler handler = new(["example.com"], factory.Object, "workflow-mcp"); Dictionary headers = new() { [headerName] = "other.example" }; @@ -159,14 +159,14 @@ public async Task InvokeToolAsync_WithHostHeader_ShouldRejectBeforeClientCreatio // Assert Assert.Contains("Host header", exception.Message); - factory.Verify(f => f.CreateClient(It.IsAny()), Times.Never); + factory.Verify(f => f.CreateHandler(It.IsAny()), Times.Never); } [Fact] public void Constructor_WithNullAllowedHosts_ShouldThrow() { // Arrange - Mock factory = new(); + Mock factory = new(); // Act ArgumentNullException exception = Assert.Throws(() => @@ -189,7 +189,7 @@ public void Constructor_WithNullAllowedHosts_ShouldThrow() public void Constructor_WithInvalidAllowedHost_ShouldThrow(string? allowedHost) { // Arrange - Mock factory = new(); + Mock factory = new(); // Act ArgumentException exception = Assert.Throws(() => diff --git a/skills/agent-framework-builder-dotnet/references/declarative.md b/skills/agent-framework-builder-dotnet/references/declarative.md index 603c4e00e53..483bfdf3efe 100644 --- a/skills/agent-framework-builder-dotnet/references/declarative.md +++ b/skills/agent-framework-builder-dotnet/references/declarative.md @@ -68,9 +68,8 @@ commitments. known, an explicit legacy version skips the discovery probe but still creates a fresh session per invocation. Selection restricts negotiation and incompatible versions fail. Configure separate handlers for differing server policies; explicit newer revisions still perform discovery and do not downgrade to legacy-only servers. -- Configure factory clients with cookies and redirects disabled, certificate revocation checking enabled, and - origin-bound credential injection. Do not retain per-workflow credentials or mutable origin state in pooled - handlers, and do not derive client names from workflow IDs, credentials, or arbitrary URLs. +- Keep HTTP pipeline configuration and origin-bound credential injection application-owned. Avoid retaining + per-workflow credentials or mutable origin state in pooled handlers. - Bound concurrent MCP work and supply invocation deadlines/cancellation from the host. The handler owns no shared disposable resources; cancel and await calls before disposing the application's HTTP factory infrastructure. - Keep configuration exposure restricted to intentional values. From 381b58e962bd5deb68c14284dbe2d66a2005208f Mon Sep 17 00:00:00 2001 From: Vincent Biret Date: Mon, 9 Nov 2026 08:54:09 -0500 Subject: [PATCH 3/4] fix(dotnet): reject cyclic HTTP handler pipelines Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 3333dcdf-889a-4ccc-af0c-4e41fd966d74 --- .../Shared/Http/HttpTransportValidation.cs | 17 +++++++++ .../DefaultMcpToolHandlerLifetimeTests.cs | 36 +++++++++++++++++++ .../DefaultHttpRequestHandlerTests.cs | 33 +++++++++++++++++ 3 files changed, 86 insertions(+) diff --git a/dotnet/src/Shared/Http/HttpTransportValidation.cs b/dotnet/src/Shared/Http/HttpTransportValidation.cs index 18d20ab52bc..aca24639bdd 100644 --- a/dotnet/src/Shared/Http/HttpTransportValidation.cs +++ b/dotnet/src/Shared/Http/HttpTransportValidation.cs @@ -1,7 +1,9 @@ // Copyright (c) Microsoft. All rights reserved. using System; +using System.Collections.Generic; using System.Net.Http; +using System.Runtime.CompilerServices; namespace Microsoft.Shared.Diagnostics; @@ -10,8 +12,14 @@ internal static class HttpTransportValidation { public static void Validate(HttpMessageHandler handler) { + HashSet visited = new(HttpMessageHandlerReferenceComparer.Instance); while (handler is DelegatingHandler delegatingHandler) { + if (!visited.Add(handler)) + { + throw new InvalidOperationException("The HTTP handler pipeline must not contain a cycle."); + } + handler = delegatingHandler.InnerHandler ?? throw new InvalidOperationException("The HTTP handler pipeline must have a primary handler."); } @@ -40,4 +48,13 @@ public static void Validate(HttpMessageHandler handler) throw new InvalidOperationException("The primary HTTP handler must disable automatic redirects and cookies."); } } + + private sealed class HttpMessageHandlerReferenceComparer : IEqualityComparer + { + public static HttpMessageHandlerReferenceComparer Instance { get; } = new(); + + public bool Equals(HttpMessageHandler? x, HttpMessageHandler? y) => ReferenceEquals(x, y); + + public int GetHashCode(HttpMessageHandler obj) => RuntimeHelpers.GetHashCode(obj); + } } diff --git a/dotnet/tests/Microsoft.Agents.AI.Workflows.Declarative.Mcp.UnitTests/DefaultMcpToolHandlerLifetimeTests.cs b/dotnet/tests/Microsoft.Agents.AI.Workflows.Declarative.Mcp.UnitTests/DefaultMcpToolHandlerLifetimeTests.cs index f1b8abe87bf..05cc8c51d27 100644 --- a/dotnet/tests/Microsoft.Agents.AI.Workflows.Declarative.Mcp.UnitTests/DefaultMcpToolHandlerLifetimeTests.cs +++ b/dotnet/tests/Microsoft.Agents.AI.Workflows.Declarative.Mcp.UnitTests/DefaultMcpToolHandlerLifetimeTests.cs @@ -168,6 +168,42 @@ public async Task Factory_LoopbackInvocations_ReuseHandlerAndDisposeSeparateSess Assert.Equal(1, primaryHandlersCreated); } + [Theory] + [InlineData(false)] + [InlineData(true)] + public async Task Factory_CyclicPipeline_RejectsBeforeSendingAsync(bool multipleHandlers) + { + // Arrange + using HttpMessageHandler primary = Mock.Of(); + using DelegatingHandler first = new Mock { CallBase = true }.Object; + using DelegatingHandler second = new Mock { CallBase = true }.Object; + first.InnerHandler = multipleHandlers ? second : first; + second.InnerHandler = first; + Mock factory = new(); + factory.Setup(f => f.CreateHandler("workflow-mcp")).Returns(first); + DefaultMcpToolHandler handler = new(["mcp.example"], factory.Object, "workflow-mcp"); + + // Act + Task invocation = Task.Run(async () => await Record.ExceptionAsync(() => handler.InvokeToolAsync( + "https://mcp.example/api", null, "ping", null, null, null, TestContext.Current.CancellationToken))); + try + { + // Assert + Task completed = await Task.WhenAny(invocation, Task.Delay(TimeSpan.FromSeconds(5), TestContext.Current.CancellationToken)); + Assert.True(ReferenceEquals(invocation, completed), "Cyclic pipeline validation did not complete."); + InvalidOperationException exception = Assert.IsType(await invocation); + Assert.Contains("cycle", exception.Message, StringComparison.Ordinal); + factory.Verify(f => f.CreateHandler("workflow-mcp"), Times.Once); + } + finally + { + // Break the cycle before disposing the handlers. + first.InnerHandler = primary; + second.InnerHandler = primary; + await invocation; + } + } + [Theory] [InlineData(null, 2)] [InlineData("2025-11-25", 0)] diff --git a/dotnet/tests/Microsoft.Agents.AI.Workflows.Declarative.UnitTests/DefaultHttpRequestHandlerTests.cs b/dotnet/tests/Microsoft.Agents.AI.Workflows.Declarative.UnitTests/DefaultHttpRequestHandlerTests.cs index 0ceb1810836..ee3b078c66b 100644 --- a/dotnet/tests/Microsoft.Agents.AI.Workflows.Declarative.UnitTests/DefaultHttpRequestHandlerTests.cs +++ b/dotnet/tests/Microsoft.Agents.AI.Workflows.Declarative.UnitTests/DefaultHttpRequestHandlerTests.cs @@ -262,6 +262,39 @@ public async Task SendAsyncRejectsIncompletePipelineBeforeTransportAsync() Assert.Contains("must have a primary handler", exception.Message, StringComparison.Ordinal); } + [Theory] + [InlineData(false)] + [InlineData(true)] + public async Task SendAsyncRejectsCyclicPipelineBeforeTransportAsync(bool multipleHandlers) + { + // Arrange + using HttpMessageHandler primary = Mock.Of(); + using DelegatingHandler first = new Mock { CallBase = true }.Object; + using DelegatingHandler second = new Mock { CallBase = true }.Object; + first.InnerHandler = multipleHandlers ? second : first; + second.InnerHandler = first; + DefaultHttpRequestHandler handler = new(["api.example.test"], CreateHttpMessageHandlerFactory(first).Object); + + // Act + Task invocation = Task.Run(async () => await Record.ExceptionAsync(() => + handler.SendAsync(new HttpRequestInfo { Url = TestUrl }, TestContext.Current.CancellationToken))); + try + { + // Assert + Task completed = await Task.WhenAny(invocation, Task.Delay(TimeSpan.FromSeconds(5), TestContext.Current.CancellationToken)); + Assert.True(ReferenceEquals(invocation, completed), "Cyclic pipeline validation did not complete."); + InvalidOperationException exception = Assert.IsType(await invocation); + Assert.Contains("cycle", exception.Message, StringComparison.Ordinal); + } + finally + { + // Break the cycle before disposing the handlers. + first.InnerHandler = primary; + second.InnerHandler = primary; + await invocation; + } + } + [Fact] public async Task SendAsyncWithNullRequestThrowsAsync() { From 66993a14018d090e4f5deb46a1ea2ea852c678cb Mon Sep 17 00:00:00 2001 From: Vincent Biret Date: Tue, 10 Nov 2026 14:10:20 -0500 Subject: [PATCH 4/4] fix(dotnet): remove unused MCP client factory constructor Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 3333dcdf-889a-4ccc-af0c-4e41fd966d74 --- ...55-dotnet-declarative-mcp-allowed-hosts.md | 9 ++-- .../DefaultMcpToolHandler.cs | 31 +----------- .../PublicAPI/net10.0/PublicAPI.Unshipped.txt | 1 - .../PublicAPI/net472/PublicAPI.Unshipped.txt | 1 - .../PublicAPI/net8.0/PublicAPI.Unshipped.txt | 1 - .../PublicAPI/net9.0/PublicAPI.Unshipped.txt | 1 - .../netstandard2.0/PublicAPI.Unshipped.txt | 1 - .../README.md | 3 +- .../DefaultMcpToolHandlerLifetimeTests.cs | 6 +-- .../DefaultMcpToolHandlerTests.cs | 49 ++++++------------- 10 files changed, 28 insertions(+), 75 deletions(-) diff --git a/docs/decisions/0055-dotnet-declarative-mcp-allowed-hosts.md b/docs/decisions/0055-dotnet-declarative-mcp-allowed-hosts.md index e87be8be1f9..ac64e5f6cb8 100644 --- a/docs/decisions/0055-dotnet-declarative-mcp-allowed-hosts.md +++ b/docs/decisions/0055-dotnet-declarative-mcp-allowed-hosts.md @@ -17,7 +17,7 @@ the same host-and-port policy as the [default HTTP handler](0049-dotnet-declarat - Require application-configured destinations before creating an HTTP client. - Keep host normalization and port matching consistent between HTTP and MCP. -- Preserve the existing constructor signature and per-invocation session lifetime. +- Make destination and transport configuration explicit while preserving per-invocation session lifetime. ## Considered Options @@ -28,7 +28,8 @@ the same host-and-port policy as the [default HTTP handler](0049-dotnet-declarat ## Decision Outcome -Chosen option: retain the existing constructor with an empty policy and add an allowlist overload. +Chosen option: require a new constructor signature with allowed hosts and an `IHttpMessageHandlerFactory`. +Remove the constructor accepting `IHttpClientFactory` rather than retain an overload that cannot invoke tools. The application supplies exact hosts or IP literals with optional ports. The list is copied, an omitted port defaults to 443, and an empty list denies every invocation, including discovery. Matching uses the @@ -48,6 +49,6 @@ pipeline. Client configuration callbacks registered through `IHttpClientFactory` headers and origin-bound authentication in delegating handlers. This matches the HTTP handler's ownership and transport-validation contract while keeping MCP protocol sessions separate per invocation. -This changes behavior without removing the existing constructor. Callers must supply their intended -destinations through the new overload; applications with different policies can provide an `IMcpToolHandler`. +This removes the existing constructor. Callers must migrate to the constructor accepting their intended +destinations and a message handler factory; applications with different policies can provide an `IMcpToolHandler`. Session creation, protocol selection, and resource disposal remain unchanged. diff --git a/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative.Mcp/DefaultMcpToolHandler.cs b/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative.Mcp/DefaultMcpToolHandler.cs index 77b1c3dd106..8be9c35ce5e 100644 --- a/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative.Mcp/DefaultMcpToolHandler.cs +++ b/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative.Mcp/DefaultMcpToolHandler.cs @@ -42,7 +42,7 @@ public sealed class DefaultMcpToolHandler : IMcpToolHandler private static readonly JsonWriterOptions s_toolListJsonWriterOptions = new() { Indented = true }; - private readonly IHttpMessageHandlerFactory? _httpMessageHandlerFactory; + private readonly IHttpMessageHandlerFactory _httpMessageHandlerFactory; private readonly string _httpClientName; private readonly AllowedHttpHosts _allowedHosts; @@ -60,31 +60,6 @@ public sealed class DefaultMcpToolHandler : IMcpToolHandler /// public string? ProtocolVersion { get; init; } - /// - /// Initializes a new instance of the class using an application-provided HTTP client factory. - /// - /// - /// The application-owned factory. This parameter is retained for compatibility; this overload allows no destinations. - /// - /// - /// The stable named-client configuration to use, or an empty string for the default configuration. - /// Do not derive names from workflow identifiers, credentials, or arbitrary server URLs. - /// - /// - /// This overload allows no destinations. Use the overload accepting allowed hosts to enable invocations. - /// The application is responsible for the HTTP pipeline's transport policy. - /// Disable cookies and automatic redirects, enable certificate revocation checking, and restrict credential injection - /// to the configured server origin. Pooled handlers must not retain per-invocation credentials or mutable origin state. - /// Configure a finite handler lifetime so new invocations can pick up endpoint changes without disrupting active calls. - /// - /// A required argument is . - public DefaultMcpToolHandler(IHttpClientFactory httpClientFactory, string httpClientName) - { - Throw.IfNull(httpClientFactory); - this._httpClientName = Throw.IfNull(httpClientName); - this._allowedHosts = new([]); - } - /// /// Initializes a new instance of the class with allowed hosts /// and an application-provided HTTP message handler factory. @@ -231,9 +206,7 @@ private async Task CreateClientAsync( throw new HttpRequestException("The request host and port are not in the allowed hosts list."); } - IHttpMessageHandlerFactory factory = this._httpMessageHandlerFactory - ?? throw new InvalidOperationException("MCP invocations require an HTTP message handler factory."); - HttpMessageHandler messageHandler = factory.CreateHandler(this._httpClientName); + HttpMessageHandler messageHandler = this._httpMessageHandlerFactory.CreateHandler(this._httpClientName); HttpTransportValidation.Validate(messageHandler); HttpClient httpClient = new(messageHandler, disposeHandler: false); HttpClientTransport? transport = null; diff --git a/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative.Mcp/PublicAPI/net10.0/PublicAPI.Unshipped.txt b/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative.Mcp/PublicAPI/net10.0/PublicAPI.Unshipped.txt index 932e29e4996..2f4cfa85712 100644 --- a/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative.Mcp/PublicAPI/net10.0/PublicAPI.Unshipped.txt +++ b/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative.Mcp/PublicAPI/net10.0/PublicAPI.Unshipped.txt @@ -1,5 +1,4 @@ #nullable enable -Microsoft.Agents.AI.Workflows.Declarative.Mcp.DefaultMcpToolHandler.DefaultMcpToolHandler(System.Net.Http.IHttpClientFactory! httpClientFactory, string! httpClientName) -> void Microsoft.Agents.AI.Workflows.Declarative.Mcp.DefaultMcpToolHandler.DefaultMcpToolHandler(System.Collections.Generic.IEnumerable! allowedHosts, System.Net.Http.IHttpMessageHandlerFactory! httpMessageHandlerFactory, string! httpClientName) -> void Microsoft.Agents.AI.Workflows.Declarative.Mcp.DefaultMcpToolHandler.ProtocolVersion.get -> string? Microsoft.Agents.AI.Workflows.Declarative.Mcp.DefaultMcpToolHandler.ProtocolVersion.init -> void diff --git a/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative.Mcp/PublicAPI/net472/PublicAPI.Unshipped.txt b/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative.Mcp/PublicAPI/net472/PublicAPI.Unshipped.txt index 932e29e4996..2f4cfa85712 100644 --- a/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative.Mcp/PublicAPI/net472/PublicAPI.Unshipped.txt +++ b/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative.Mcp/PublicAPI/net472/PublicAPI.Unshipped.txt @@ -1,5 +1,4 @@ #nullable enable -Microsoft.Agents.AI.Workflows.Declarative.Mcp.DefaultMcpToolHandler.DefaultMcpToolHandler(System.Net.Http.IHttpClientFactory! httpClientFactory, string! httpClientName) -> void Microsoft.Agents.AI.Workflows.Declarative.Mcp.DefaultMcpToolHandler.DefaultMcpToolHandler(System.Collections.Generic.IEnumerable! allowedHosts, System.Net.Http.IHttpMessageHandlerFactory! httpMessageHandlerFactory, string! httpClientName) -> void Microsoft.Agents.AI.Workflows.Declarative.Mcp.DefaultMcpToolHandler.ProtocolVersion.get -> string? Microsoft.Agents.AI.Workflows.Declarative.Mcp.DefaultMcpToolHandler.ProtocolVersion.init -> void diff --git a/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative.Mcp/PublicAPI/net8.0/PublicAPI.Unshipped.txt b/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative.Mcp/PublicAPI/net8.0/PublicAPI.Unshipped.txt index 932e29e4996..2f4cfa85712 100644 --- a/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative.Mcp/PublicAPI/net8.0/PublicAPI.Unshipped.txt +++ b/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative.Mcp/PublicAPI/net8.0/PublicAPI.Unshipped.txt @@ -1,5 +1,4 @@ #nullable enable -Microsoft.Agents.AI.Workflows.Declarative.Mcp.DefaultMcpToolHandler.DefaultMcpToolHandler(System.Net.Http.IHttpClientFactory! httpClientFactory, string! httpClientName) -> void Microsoft.Agents.AI.Workflows.Declarative.Mcp.DefaultMcpToolHandler.DefaultMcpToolHandler(System.Collections.Generic.IEnumerable! allowedHosts, System.Net.Http.IHttpMessageHandlerFactory! httpMessageHandlerFactory, string! httpClientName) -> void Microsoft.Agents.AI.Workflows.Declarative.Mcp.DefaultMcpToolHandler.ProtocolVersion.get -> string? Microsoft.Agents.AI.Workflows.Declarative.Mcp.DefaultMcpToolHandler.ProtocolVersion.init -> void diff --git a/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative.Mcp/PublicAPI/net9.0/PublicAPI.Unshipped.txt b/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative.Mcp/PublicAPI/net9.0/PublicAPI.Unshipped.txt index 932e29e4996..2f4cfa85712 100644 --- a/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative.Mcp/PublicAPI/net9.0/PublicAPI.Unshipped.txt +++ b/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative.Mcp/PublicAPI/net9.0/PublicAPI.Unshipped.txt @@ -1,5 +1,4 @@ #nullable enable -Microsoft.Agents.AI.Workflows.Declarative.Mcp.DefaultMcpToolHandler.DefaultMcpToolHandler(System.Net.Http.IHttpClientFactory! httpClientFactory, string! httpClientName) -> void Microsoft.Agents.AI.Workflows.Declarative.Mcp.DefaultMcpToolHandler.DefaultMcpToolHandler(System.Collections.Generic.IEnumerable! allowedHosts, System.Net.Http.IHttpMessageHandlerFactory! httpMessageHandlerFactory, string! httpClientName) -> void Microsoft.Agents.AI.Workflows.Declarative.Mcp.DefaultMcpToolHandler.ProtocolVersion.get -> string? Microsoft.Agents.AI.Workflows.Declarative.Mcp.DefaultMcpToolHandler.ProtocolVersion.init -> void diff --git a/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative.Mcp/PublicAPI/netstandard2.0/PublicAPI.Unshipped.txt b/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative.Mcp/PublicAPI/netstandard2.0/PublicAPI.Unshipped.txt index 932e29e4996..2f4cfa85712 100644 --- a/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative.Mcp/PublicAPI/netstandard2.0/PublicAPI.Unshipped.txt +++ b/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative.Mcp/PublicAPI/netstandard2.0/PublicAPI.Unshipped.txt @@ -1,5 +1,4 @@ #nullable enable -Microsoft.Agents.AI.Workflows.Declarative.Mcp.DefaultMcpToolHandler.DefaultMcpToolHandler(System.Net.Http.IHttpClientFactory! httpClientFactory, string! httpClientName) -> void Microsoft.Agents.AI.Workflows.Declarative.Mcp.DefaultMcpToolHandler.DefaultMcpToolHandler(System.Collections.Generic.IEnumerable! allowedHosts, System.Net.Http.IHttpMessageHandlerFactory! httpMessageHandlerFactory, string! httpClientName) -> void Microsoft.Agents.AI.Workflows.Declarative.Mcp.DefaultMcpToolHandler.ProtocolVersion.get -> string? Microsoft.Agents.AI.Workflows.Declarative.Mcp.DefaultMcpToolHandler.ProtocolVersion.init -> void diff --git a/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative/README.md b/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative/README.md index bc31438b788..a8cc8d5fc14 100644 --- a/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative/README.md +++ b/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative/README.md @@ -74,7 +74,8 @@ the pooled handler pipeline. Construct the handler with an application-owned `IHttpMessageHandlerFactory` through `DefaultMcpToolHandler(IEnumerable, IHttpMessageHandlerFactory, string)`. Supply allowed hosts from application configuration, not workflow content. An empty -list, including the existing two-argument constructor, denies all destinations. +list denies all destinations. The former constructor accepting `IHttpClientFactory` +is removed; migrate to the constructor above with an explicit destination policy. Hosts are exact DNS names or IP literals with optional ports; omitted ports default to 443. Matching is case-insensitive and normalizes internationalized names and trailing DNS dots, without wildcards or implicit subdomains. diff --git a/dotnet/tests/Microsoft.Agents.AI.Workflows.Declarative.Mcp.UnitTests/DefaultMcpToolHandlerLifetimeTests.cs b/dotnet/tests/Microsoft.Agents.AI.Workflows.Declarative.Mcp.UnitTests/DefaultMcpToolHandlerLifetimeTests.cs index 05cc8c51d27..86501a710d0 100644 --- a/dotnet/tests/Microsoft.Agents.AI.Workflows.Declarative.Mcp.UnitTests/DefaultMcpToolHandlerLifetimeTests.cs +++ b/dotnet/tests/Microsoft.Agents.AI.Workflows.Declarative.Mcp.UnitTests/DefaultMcpToolHandlerLifetimeTests.cs @@ -330,15 +330,15 @@ public async Task SequentialInvocations_UseSeparateSessionsAsync() public async Task UnsupportedScheme_DoesNotAcquireHttpClientAsync(string serverUrl) { // Arrange - Mock factory = new(MockBehavior.Strict); - DefaultMcpToolHandler handler = new(factory.Object, "workflow-mcp"); + Mock factory = new(MockBehavior.Strict); + DefaultMcpToolHandler handler = new(["mcp.example"], factory.Object, "workflow-mcp"); // Act await Assert.ThrowsAsync(() => handler.InvokeToolAsync( serverUrl, null, "ping", null, null, null)); // Assert - factory.Verify(f => f.CreateClient(It.IsAny()), Times.Never); + factory.Verify(f => f.CreateHandler(It.IsAny()), Times.Never); } [Fact] diff --git a/dotnet/tests/Microsoft.Agents.AI.Workflows.Declarative.Mcp.UnitTests/DefaultMcpToolHandlerTests.cs b/dotnet/tests/Microsoft.Agents.AI.Workflows.Declarative.Mcp.UnitTests/DefaultMcpToolHandlerTests.cs index 8db03c9522b..726faf6ba63 100644 --- a/dotnet/tests/Microsoft.Agents.AI.Workflows.Declarative.Mcp.UnitTests/DefaultMcpToolHandlerTests.cs +++ b/dotnet/tests/Microsoft.Agents.AI.Workflows.Declarative.Mcp.UnitTests/DefaultMcpToolHandlerTests.cs @@ -19,23 +19,23 @@ namespace Microsoft.Agents.AI.Workflows.Declarative.Mcp.UnitTests; public sealed class DefaultMcpToolHandlerTests { [Fact] - public void Constructor_WithNullHttpClientFactory_ShouldThrow() + public void Constructor_WithNullHttpMessageHandlerFactory_ShouldThrow() { // Act - ArgumentNullException exception = Assert.Throws(() => new DefaultMcpToolHandler(null!, "workflow-mcp")); + ArgumentNullException exception = Assert.Throws(() => new DefaultMcpToolHandler(["example.com"], null!, "workflow-mcp")); // Assert - Assert.Equal("httpClientFactory", exception.ParamName); + Assert.Equal("httpMessageHandlerFactory", exception.ParamName); } [Fact] public void Constructor_WithNullHttpClientName_ShouldThrow() { // Arrange - Mock factory = new(); + Mock factory = new(); // Act - ArgumentNullException exception = Assert.Throws(() => new DefaultMcpToolHandler(factory.Object, null!)); + ArgumentNullException exception = Assert.Throws(() => new DefaultMcpToolHandler(["example.com"], factory.Object, null!)); // Assert Assert.Equal("httpClientName", exception.ParamName); @@ -53,8 +53,8 @@ public async Task InvokeToolAsync_WithInvalidHeader_ShouldRejectBeforeClientCrea bool invalidName) { // Arrange - Mock factory = new(MockBehavior.Strict); - DefaultMcpToolHandler handler = new(factory.Object, "workflow-mcp"); + Mock factory = new(MockBehavior.Strict); + DefaultMcpToolHandler handler = new(["example.com"], factory.Object, "workflow-mcp"); Dictionary headers = new() { [invalidName ? $"X-Test{invalidCharacter}" : "X-Test"] = invalidName ? "value" : $"value{invalidCharacter}" @@ -66,15 +66,15 @@ public async Task InvokeToolAsync_WithInvalidHeader_ShouldRejectBeforeClientCrea // Assert Assert.Equal("headers", exception.ParamName); - factory.Verify(f => f.CreateClient(It.IsAny()), Times.Never); + factory.Verify(f => f.CreateHandler(It.IsAny()), Times.Never); } [Fact] public async Task InvokeToolAsync_WithCancelledToken_ShouldRejectBeforeClientCreationAsync() { // Arrange - Mock factory = new(MockBehavior.Strict); - DefaultMcpToolHandler handler = new(factory.Object, "workflow-mcp"); + Mock factory = new(MockBehavior.Strict); + DefaultMcpToolHandler handler = new(["example.com"], factory.Object, "workflow-mcp"); using CancellationTokenSource cancellation = new(); cancellation.Cancel(); @@ -83,41 +83,24 @@ await Assert.ThrowsAnyAsync(() => handler.InvokeTool "https://example.com/mcp", null, "ping", null, null, null, cancellation.Token)); // Assert - factory.Verify(f => f.CreateClient(It.IsAny()), Times.Never); + factory.Verify(f => f.CreateHandler(It.IsAny()), Times.Never); } [Theory] [InlineData("ping")] [InlineData(DefaultMcpToolHandler.ListToolsToolName)] public async Task InvokeToolAsync_WithoutAllowedHosts_ShouldRejectBeforeClientCreationAsync(string toolName) - { - // Arrange - Mock factory = new(MockBehavior.Strict); - DefaultMcpToolHandler handler = new(factory.Object, "workflow-mcp"); - - // Act - HttpRequestException exception = await Assert.ThrowsAsync(() => handler.InvokeToolAsync( - "https://example.com/mcp", null, toolName, null, null, null)); - - // Assert - Assert.Contains("allowed hosts", exception.Message); - factory.Verify(f => f.CreateClient(It.IsAny()), Times.Never); - } - - [Theory] - [InlineData("ping")] - [InlineData(DefaultMcpToolHandler.ListToolsToolName)] - public async Task InvokeToolAsync_WithEmptyAllowedHosts_ShouldRejectBeforeClientCreationAsync(string toolName) { // Arrange Mock factory = new(MockBehavior.Strict); DefaultMcpToolHandler handler = new([], factory.Object, "workflow-mcp"); // Act - await Assert.ThrowsAsync(() => handler.InvokeToolAsync( + HttpRequestException exception = await Assert.ThrowsAsync(() => handler.InvokeToolAsync( "https://example.com/mcp", null, toolName, null, null, null)); // Assert + Assert.Contains("allowed hosts", exception.Message); factory.Verify(f => f.CreateHandler(It.IsAny()), Times.Never); } @@ -225,8 +208,8 @@ public void IsListToolsToolName_WithRegularToolName_ShouldReturnFalse() public async Task InvokeToolAsync_WithListToolsArguments_ShouldThrowArgumentExceptionAsync() { // Arrange - Mock factory = new(MockBehavior.Strict); - DefaultMcpToolHandler handler = new(factory.Object, "workflow-mcp"); + Mock factory = new(MockBehavior.Strict); + DefaultMcpToolHandler handler = new(["example.com"], factory.Object, "workflow-mcp"); // Act ArgumentException exception = await Assert.ThrowsAsync(() => handler.InvokeToolAsync( @@ -235,7 +218,7 @@ public async Task InvokeToolAsync_WithListToolsArguments_ShouldThrowArgumentExce // Assert Assert.Contains("does not accept tool arguments", exception.Message); - factory.Verify(f => f.CreateClient(It.IsAny()), Times.Never); + factory.Verify(f => f.CreateHandler(It.IsAny()), Times.Never); } [Fact]