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..ac64e5f6cb8 --- /dev/null +++ b/docs/decisions/0055-dotnet-declarative-mcp-allowed-hosts.md @@ -0,0 +1,54 @@ +--- +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. +- Make destination and transport configuration explicit while preserving 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: 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 +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 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 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/samples/03-workflows/Declarative/InvokeFoundryToolboxMcp/Program.cs b/dotnet/samples/03-workflows/Declarative/InvokeFoundryToolboxMcp/Program.cs index 79c6ddbbefd..9030dcf29d1 100644 --- a/dotnet/samples/03-workflows/Declarative/InvokeFoundryToolboxMcp/Program.cs +++ b/dotnet/samples/03-workflows/Declarative/InvokeFoundryToolboxMcp/Program.cs @@ -87,7 +87,8 @@ public static async Task Main(string[] args) .SetHandlerLifetime(TimeSpan.FromMinutes(2)); await using ServiceProvider serviceProvider = services.BuildServiceProvider(); DefaultMcpToolHandler mcpToolHandler = new( - serviceProvider.GetRequiredService(), McpHttpClientName); + [new Uri(toolboxMcpServerUrl).Authority], + 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 4c2e3752ec8..ee4c90ed822 100644 --- a/dotnet/samples/03-workflows/Declarative/InvokeMcpTool/Program.cs +++ b/dotnet/samples/03-workflows/Declarative/InvokeMcpTool/Program.cs @@ -73,7 +73,8 @@ public static async Task Main(string[] args) .SetHandlerLifetime(TimeSpan.FromMinutes(2)); await using ServiceProvider serviceProvider = services.BuildServiceProvider(); DefaultMcpToolHandler mcpToolHandler = new( - serviceProvider.GetRequiredService(), McpHttpClientName); + ["learn.microsoft.com", "mcp.ai.azure.com"], + 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 5ba5bf31052..8be9c35ce5e 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,8 +42,9 @@ 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; /// /// Gets or initializes the protocol version requested for every invocation. @@ -60,27 +61,34 @@ public sealed class DefaultMcpToolHandler : IMcpToolHandler public string? ProtocolVersion { get; init; } /// - /// Initializes a new instance of the class using an application-provided HTTP client factory. + /// Initializes a new instance of the class with allowed hosts + /// and an application-provided HTTP message handler 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 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 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. + /// + /// 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 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. - /// + /// The stable named-client configuration, or an empty string for the default configuration. /// A required argument is . - public DefaultMcpToolHandler(IHttpClientFactory httpClientFactory, string httpClientName) + /// An allowed host is invalid. + 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); } /// @@ -110,6 +118,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,7 +201,14 @@ private async Task CreateClientAsync( EnableStandaloneGetStream = false }; - HttpClient httpClient = this._httpClientFactory.CreateClient(this._httpClientName); + if (!this._allowedHosts.Contains(endpoint)) + { + throw new HttpRequestException("The request host and port are not in the allowed hosts list."); + } + + HttpMessageHandler messageHandler = this._httpMessageHandlerFactory.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 6ab40b3b9f5..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,5 @@ #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 *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..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,5 @@ #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 *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..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,5 @@ #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 *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..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,5 @@ #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 *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..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,5 @@ #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 *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..74fc8b20603 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,13 +129,13 @@ 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."); } HttpMessageHandler messageHandler = this._httpMessageHandlerFactory.CreateHandler(this._httpClientName); - ValidateTransport(messageHandler); + HttpTransportValidation.Validate(messageHandler); using HttpClient client = new(messageHandler, disposeHandler: false); if (timeoutCts is not null) { @@ -204,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 @@ -328,9 +269,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..a8cc8d5fc14 100644 --- a/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative/README.md +++ b/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative/README.md @@ -69,11 +69,22 @@ 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(IHttpClientFactory, string)`. The second -argument is a stable named-client configuration, or `string.Empty` for the -default configuration. Factory-created wrappers are owned by the invocation; +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 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. +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. 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. @@ -92,10 +103,18 @@ 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. +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 @@ -108,7 +127,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"], httpMessageHandlerFactory, "workflow-mcp") { ProtocolVersion = "2025-11-25" }; @@ -132,9 +151,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/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/src/Shared/Http/HttpTransportValidation.cs b/dotnet/src/Shared/Http/HttpTransportValidation.cs new file mode 100644 index 00000000000..aca24639bdd --- /dev/null +++ b/dotnet/src/Shared/Http/HttpTransportValidation.cs @@ -0,0 +1,60 @@ +// Copyright (c) Microsoft. All rights reserved. + +using System; +using System.Collections.Generic; +using System.Net.Http; +using System.Runtime.CompilerServices; + +namespace Microsoft.Shared.Diagnostics; + +/// Validates application-owned HTTP handler pipelines. +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."); + } + + 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 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.IntegrationTests/InvokeToolWorkflowTest.cs b/dotnet/tests/Microsoft.Agents.AI.Workflows.Declarative.IntegrationTests/InvokeToolWorkflowTest.cs index 92e6c87b825..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(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..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 @@ -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(serviceProvider.GetRequiredService(), "workflow-mcp") + DefaultMcpToolHandler handler = new([new Uri(server.Endpoint).Authority], serviceProvider.GetRequiredService(), "workflow-mcp") { ProtocolVersion = protocolVersion }; @@ -79,13 +168,49 @@ 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)] 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 +235,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 +255,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 +278,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 +287,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 +306,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)); @@ -205,22 +330,22 @@ 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] - 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 +357,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 +367,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,8 +388,8 @@ await handler.InvokeToolAsync( public async Task DifferentOrigins_KeepRequestConfigurationAsync() { // Arrange - ProtocolStub stub = new(); - DefaultMcpToolHandler handler = CreateHandler(stub); + using ProtocolStub stub = new(); + DefaultMcpToolHandler handler = CreateHandler(stub, allowedHosts: ["first.example", "second.example"]); using CancellationTokenSource timeout = new(TimeSpan.FromSeconds(10)); // Act @@ -291,29 +416,21 @@ 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; - }); - DefaultMcpToolHandler handler = new(factory.Object, ClientName); + 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)); // Act @@ -321,8 +438,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 +447,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 +461,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) @@ -374,7 +478,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); @@ -391,9 +495,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,23 +504,22 @@ 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())); - DefaultMcpToolHandler handler = new(factory.Object, "workflow-mcp"); + .Returns(stub.CreateMessageHandler); + DefaultMcpToolHandler handler = new(["mcp.example"], factory.Object, "workflow-mcp"); using CancellationTokenSource timeout = new(TimeSpan.FromSeconds(10)); // Act @@ -440,7 +541,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 +554,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)); @@ -494,7 +595,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 +619,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 +627,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 +662,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 { @@ -615,18 +714,70 @@ 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) { - Mock factory = new(); - factory.Setup(f => f.CreateClient("workflow-mcp")).Returns(() => new HttpClient(stub.CreateMessageHandler())); - return new(factory.Object, "workflow-mcp") { ProtocolVersion = protocolVersion }; + // Arrange + using 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 + using 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.CreateHandler("workflow-mcp")).Returns(stub.CreateMessageHandler); + return new(allowedHosts ?? ["mcp.example"], factory.Object, "workflow-mcp") { ProtocolVersion = protocolVersion }; } 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; @@ -637,35 +788,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); @@ -778,6 +945,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) { @@ -787,6 +955,7 @@ public LoopbackMcpServer(ProtocolStub stub) } public string Endpoint { get; } + public int RequestCount => this._requestCount; public List TerminatedSessions { get; } = []; @@ -819,6 +988,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 13f0c65fffa..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,7 +83,103 @@ 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.CreateHandler(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.CreateHandler(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.CreateHandler(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 @@ -112,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( @@ -122,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] 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() { diff --git a/skills/agent-framework-builder-dotnet/references/declarative.md b/skills/agent-framework-builder-dotnet/references/declarative.md index 7289253491d..483bfdf3efe 100644 --- a/skills/agent-framework-builder-dotnet/references/declarative.md +++ b/skills/agent-framework-builder-dotnet/references/declarative.md @@ -60,17 +60,16 @@ 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 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.