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 8b0a410d8bd..49541ebf428 100644 --- a/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative.Mcp/DefaultMcpToolHandler.cs +++ b/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative.Mcp/DefaultMcpToolHandler.cs @@ -50,6 +50,13 @@ public sealed class DefaultMcpToolHandler : IMcpToolHandler, IAsyncDisposable /// An optional callback that provides an for each MCP server. /// The callback receives (serverUrl, cancellationToken) and should return an HttpClient /// configured with any required authentication. Return to use a default HttpClient with no auth. + /// + /// Security: HttpClients created by this handler pin credential headers to the configured server + /// origin and disable auto-redirect. When you supply your own , you are + /// responsible for equivalent protection — attach credentials only for the configured server origin + /// (for example via a ) so a server-advertised endpoint or redirect on + /// a different origin cannot capture the Authorization token. + /// /// public DefaultMcpToolHandler(Func>? httpClientProvider = null) { @@ -201,9 +208,25 @@ private async Task CreateClientAsync( // Disable cookies so handler-level state (cookie jar) cannot cross the cache-key // isolation boundary established by GetOrCreateClientAsync. The actual MCP auth // travels via AdditionalHeaders (set per-transport below), not session cookies. + // Disable auto-redirect so an HTTP redirect cannot carry credential headers to a + // different origin (defense-in-depth alongside the origin pinning applied below). // CheckCertificateRevocationList satisfies CA5399 since we're explicitly constructing the handler. - HttpClientHandler handler = new() { UseCookies = false, CheckCertificateRevocationList = true }; - httpClient = new HttpClient(handler); + HttpClientHandler handler = new() + { + UseCookies = false, + AllowAutoRedirect = false, + CheckCertificateRevocationList = true + }; + + // Pin credential headers to the configured server origin as defense-in-depth. Forcing + // StreamableHttp (below) already removes the primary vector (a server-advertised cross-origin + // SSE message endpoint), and AllowAutoRedirect=false blocks auto-redirects. This handler is the + // backstop: it guarantees the Authorization token and other credentials never leave the pinned + // origin even if a future change re-enables AutoDetect or redirects, or the SDK constructs a + // request to a new URI (AdditionalHeaders are re-stamped by the transport, so HttpClient's own + // redirect header-stripping does not cover them). + OriginPinningHandler pinningHandler = new(new Uri(serverUrl)) { InnerHandler = handler }; + httpClient = new HttpClient(pinningHandler); this._ownedHttpClients[httpClientCacheKey] = httpClient; } @@ -212,7 +235,11 @@ private async Task CreateClientAsync( Endpoint = new Uri(serverUrl), Name = serverLabel ?? "McpClient", AdditionalHeaders = headers, - TransportMode = HttpTransportMode.AutoDetect + // Use Streamable HTTP rather than AutoDetect so the client does not negotiate down to the + // legacy HTTP+SSE transport, which trusts a server-advertised message endpoint. That + // server-controlled endpoint is the primary vector for redirecting the Authorization token + // to a different origin; Streamable HTTP keeps every request on the configured origin. + TransportMode = HttpTransportMode.StreamableHttp }; HttpClientTransport transport = new(transportOptions, httpClient); @@ -394,3 +421,93 @@ private static string SerializeToolsList(IEnumerable tools) return Encoding.UTF8.GetString(stream.GetBuffer(), 0, (int)stream.Length); } } + +/// +/// A that strips credential-bearing headers from any outbound request +/// whose origin does not match the origin the MCP client was configured with. +/// +/// +/// +/// This pins the Authorization token (and other credential headers) to the trusted server origin so a +/// compromised or malicious MCP server cannot redirect them to a different origin — for example by +/// advertising a cross-origin SSE message endpoint or issuing an HTTP redirect. Same-origin requests are +/// left untouched so legitimate MCP traffic continues to carry its credentials. +/// +/// +/// This is a defense-in-depth control. Using already removes +/// the primary vector (a server-advertised cross-origin message endpoint) and disabling auto-redirect blocks +/// redirect-based leakage; this handler backstops both so credentials stay pinned even if those settings +/// change or the SDK builds a request to a new URI (AdditionalHeaders are re-stamped by the transport +/// and are therefore not covered by 's own redirect header-stripping). +/// +/// +/// Caveat for future maintainers: this strips credentials on every cross-origin request. That is safe +/// today because carries auth via static request headers, not the SDK's +/// built-in OAuth flow. If OAuth support is ever added here, requests to the authorization server (a different +/// origin than the MCP resource server) legitimately carry credentials, so those auth-server origins would need +/// to be allow-listed to avoid breaking the token exchange. +/// +/// +internal sealed class OriginPinningHandler : DelegatingHandler +{ + // Credential-bearing headers that must never cross the pinned-origin boundary. + private static readonly string[] s_credentialHeaderNames = + [ + "Authorization", + "Proxy-Authorization", + "Cookie", + ]; + + private readonly Uri _pinnedEndpoint; + + /// + /// Initializes a new instance of the class. + /// + /// The configured MCP server endpoint whose origin credentials are pinned to. Must be an absolute URI. + public OriginPinningHandler(Uri pinnedEndpoint) + { + Throw.IfNull(pinnedEndpoint); + if (!pinnedEndpoint.IsAbsoluteUri) + { + throw new ArgumentException("The pinned endpoint must be an absolute URI.", nameof(pinnedEndpoint)); + } + + this._pinnedEndpoint = pinnedEndpoint; + } + + /// + protected override Task SendAsync(HttpRequestMessage request, CancellationToken cancellationToken) + { + StripCredentialHeadersOnCrossOrigin(request, this._pinnedEndpoint); + return base.SendAsync(request, cancellationToken); + } + + /// + /// Removes credential headers from when its origin differs from that of + /// . Same-origin requests are left untouched. Origin comparison covers + /// scheme, host, and port (case-insensitive) via , which normalizes default + /// ports so an explicit default port (for example :443 for https) matches an omitted one. A + /// relative request URI is resolved by against its base address (the pinned + /// origin) and so can never target a different origin; its headers are left untouched. + /// + internal static void StripCredentialHeadersOnCrossOrigin(HttpRequestMessage request, Uri pinnedEndpoint) + { + Throw.IfNull(request); + Throw.IfNull(pinnedEndpoint); + + if (request.RequestUri is not { IsAbsoluteUri: true } requestUri) + { + return; + } + + if (Uri.Compare(requestUri, pinnedEndpoint, UriComponents.SchemeAndServer, UriFormat.Unescaped, StringComparison.OrdinalIgnoreCase) == 0) + { + return; + } + + foreach (string headerName in s_credentialHeaderNames) + { + request.Headers.Remove(headerName); + } + } +} 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 1470e4f0f56..8aa559e87ba 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 @@ -936,4 +936,178 @@ public void ConvertContentBlock_BlockWithMeta_ShouldPropagateToAdditionalPropert } #endregion + + #region Origin Pinning Tests + + [Fact] + public void StripCredentialHeadersOnCrossOrigin_SameOrigin_RetainsAuthorization() + { + // Arrange + using HttpRequestMessage request = new(HttpMethod.Post, "https://trusted.example.com/mcp/message"); + request.Headers.TryAddWithoutValidation("Authorization", "Bearer secret-token"); + + // Act + OriginPinningHandler.StripCredentialHeadersOnCrossOrigin(request, new Uri("https://trusted.example.com")); + + // Assert — same origin, credential is preserved + request.Headers.Contains("Authorization").Should().BeTrue(); + } + + [Fact] + public void StripCredentialHeadersOnCrossOrigin_DifferentHost_RemovesAuthorization() + { + // Arrange — server-advertised endpoint on an attacker origin + using HttpRequestMessage request = new(HttpMethod.Post, "https://attacker.example/collect"); + request.Headers.TryAddWithoutValidation("Authorization", "Bearer secret-token"); + + // Act + OriginPinningHandler.StripCredentialHeadersOnCrossOrigin(request, new Uri("https://trusted.example.com")); + + // Assert — credential must not cross the origin boundary + request.Headers.Contains("Authorization").Should().BeFalse(); + } + + [Fact] + public void StripCredentialHeadersOnCrossOrigin_DifferentPort_RemovesAuthorization() + { + // Arrange — same host but a different port is a different origin + using HttpRequestMessage request = new(HttpMethod.Post, "https://trusted.example.com:8443/mcp"); + request.Headers.TryAddWithoutValidation("Authorization", "Bearer secret-token"); + + // Act + OriginPinningHandler.StripCredentialHeadersOnCrossOrigin(request, new Uri("https://trusted.example.com")); + + // Assert + request.Headers.Contains("Authorization").Should().BeFalse(); + } + + [Fact] + public void StripCredentialHeadersOnCrossOrigin_ExplicitDefaultPort_RetainsAuthorization() + { + // Arrange — pinned endpoint carries an explicit :443 while the request omits it; these are + // the same origin and Uri.Compare must normalize the default port rather than strip. + using HttpRequestMessage request = new(HttpMethod.Post, "https://trusted.example.com/mcp/message"); + request.Headers.TryAddWithoutValidation("Authorization", "Bearer secret-token"); + + // Act + OriginPinningHandler.StripCredentialHeadersOnCrossOrigin(request, new Uri("https://trusted.example.com:443/mcp")); + + // Assert — explicit vs implicit default port is the same origin + request.Headers.Contains("Authorization").Should().BeTrue(); + } + + [Fact] + public void StripCredentialHeadersOnCrossOrigin_RelativeRequestUri_RetainsAuthorization() + { + // Arrange — a relative URI resolves against the client's base address (the pinned origin) and + // therefore can never target a foreign origin. It must not throw and must retain credentials. + using HttpRequestMessage request = new(HttpMethod.Post, new Uri("/mcp/message", UriKind.Relative)); + request.Headers.TryAddWithoutValidation("Authorization", "Bearer secret-token"); + + // Act + Action act = () => OriginPinningHandler.StripCredentialHeadersOnCrossOrigin(request, new Uri("https://trusted.example.com")); + + // Assert — does not throw and leaves the credential in place + act.Should().NotThrow(); + request.Headers.Contains("Authorization").Should().BeTrue(); + } + + [Fact] + public void StripCredentialHeadersOnCrossOrigin_DifferentScheme_RemovesAuthorization() + { + // Arrange — downgrade to http is a different origin (and would leak over plaintext) + using HttpRequestMessage request = new(HttpMethod.Post, "http://trusted.example.com/mcp"); + request.Headers.TryAddWithoutValidation("Authorization", "Bearer secret-token"); + + // Act + OriginPinningHandler.StripCredentialHeadersOnCrossOrigin(request, new Uri("https://trusted.example.com")); + + // Assert + request.Headers.Contains("Authorization").Should().BeFalse(); + } + + [Fact] + public void StripCredentialHeadersOnCrossOrigin_CrossOrigin_RemovesCookieAndProxyAuthorization() + { + // Arrange + using HttpRequestMessage request = new(HttpMethod.Post, "https://attacker.example/collect"); + request.Headers.TryAddWithoutValidation("Authorization", "Bearer secret-token"); + request.Headers.TryAddWithoutValidation("Cookie", "session=abc"); + request.Headers.TryAddWithoutValidation("Proxy-Authorization", "Bearer proxy-token"); + + // Act + OriginPinningHandler.StripCredentialHeadersOnCrossOrigin(request, new Uri("https://trusted.example.com")); + + // Assert — all credential-bearing headers are stripped + request.Headers.Contains("Authorization").Should().BeFalse(); + request.Headers.Contains("Cookie").Should().BeFalse(); + request.Headers.Contains("Proxy-Authorization").Should().BeFalse(); + } + + [Fact] + public void StripCredentialHeadersOnCrossOrigin_CrossOrigin_PreservesNonCredentialHeaders() + { + // Arrange + using HttpRequestMessage request = new(HttpMethod.Post, "https://attacker.example/collect"); + request.Headers.TryAddWithoutValidation("Authorization", "Bearer secret-token"); + request.Headers.TryAddWithoutValidation("X-Trace-Id", "trace-123"); + + // Act + OriginPinningHandler.StripCredentialHeadersOnCrossOrigin(request, new Uri("https://trusted.example.com")); + + // Assert — only credential headers are removed; other headers are untouched + request.Headers.Contains("Authorization").Should().BeFalse(); + request.Headers.Contains("X-Trace-Id").Should().BeTrue(); + } + + [Fact] + public async Task OriginPinningHandler_CrossOriginRequest_DoesNotForwardAuthorizationAsync() + { + // Arrange — capture what the inner handler actually receives on the wire + CapturingHandler inner = new(); + using OriginPinningHandler pinning = new(new Uri("https://trusted.example.com/mcp")) { InnerHandler = inner }; + using HttpMessageInvoker invoker = new(pinning); + + using HttpRequestMessage request = new(HttpMethod.Post, "https://attacker.example/collect"); + request.Headers.TryAddWithoutValidation("Authorization", "Bearer secret-token"); + + // Act + using HttpResponseMessage response = await invoker.SendAsync(request, CancellationToken.None); + + // Assert — the credential never reached the inner handler for the foreign origin + response.StatusCode.Should().Be(System.Net.HttpStatusCode.OK); + inner.LastRequestHadAuthorization.Should().BeFalse(); + } + + [Fact] + public async Task OriginPinningHandler_SameOriginRequest_ForwardsAuthorizationAsync() + { + // Arrange + CapturingHandler inner = new(); + using OriginPinningHandler pinning = new(new Uri("https://trusted.example.com/mcp")) { InnerHandler = inner }; + using HttpMessageInvoker invoker = new(pinning); + + using HttpRequestMessage request = new(HttpMethod.Post, "https://trusted.example.com/mcp/message"); + request.Headers.TryAddWithoutValidation("Authorization", "Bearer secret-token"); + + // Act + using HttpResponseMessage response = await invoker.SendAsync(request, CancellationToken.None); + + // Assert — same-origin credential flows through normally + response.StatusCode.Should().Be(System.Net.HttpStatusCode.OK); + inner.LastRequestHadAuthorization.Should().BeTrue(); + } + + private sealed class CapturingHandler : HttpMessageHandler + { + public bool LastRequestHadAuthorization { get; private set; } + + protected override Task SendAsync(HttpRequestMessage request, CancellationToken cancellationToken) + { + this.LastRequestHadAuthorization = request.Headers.Contains("Authorization"); + return Task.FromResult(new HttpResponseMessage(System.Net.HttpStatusCode.OK)); + } + } + + #endregion }