From 9803aaed09cc52bbaeda478bdb4146c5a6df6e7c Mon Sep 17 00:00:00 2001 From: SergeyMenshykh <68852919+SergeyMenshykh@users.noreply.github.com> Date: Tue, 30 Jun 2026 15:51:30 +0100 Subject: [PATCH] .Net: Disable automatic HTTP redirects in HttpPlugin and WebFileDownloadPlugin default clients The default HTTP clients used by HttpPlugin and WebFileDownloadPlugin now have AllowAutoRedirect set to false. This ensures the AllowedDomains restriction cannot be circumvented by following redirects to hosts outside the allow-list. - Added GetNonRedirectingHttpClient() to HttpClientProvider - Updated both plugins to use non-redirecting clients by default - Added unit tests verifying redirect responses are not followed - Updated XML documentation Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../src/Http/HttpClientProvider.cs | 28 +++++--- dotnet/src/Plugins/Plugins.Core/HttpPlugin.cs | 7 +- .../Plugins.UnitTests/Core/HttpPluginTests.cs | 47 ++++++++++++++ .../Web/WebFileDownloadPluginTests.cs | 65 +++++++++++++++++++ .../Plugins.Web/WebFileDownloadPlugin.cs | 9 ++- 5 files changed, 146 insertions(+), 10 deletions(-) diff --git a/dotnet/src/InternalUtilities/src/Http/HttpClientProvider.cs b/dotnet/src/InternalUtilities/src/Http/HttpClientProvider.cs index 3a6b5b9fe111..e5f79b3bad21 100644 --- a/dotnet/src/InternalUtilities/src/Http/HttpClientProvider.cs +++ b/dotnet/src/InternalUtilities/src/Http/HttpClientProvider.cs @@ -26,6 +26,12 @@ internal static class HttpClientProvider /// An instance of HttpClient. public static HttpClient GetHttpClient() => new(NonDisposableHttpClientHandler.Instance, disposeHandler: false); + /// + /// Retrieves an instance of HttpClient that does not automatically follow HTTP redirects. + /// + /// An instance of HttpClient that does not follow redirects. + public static HttpClient GetNonRedirectingHttpClient() => new(NonDisposableHttpClientHandler.NonRedirectingInstance, disposeHandler: false); + /// /// Retrieves an instance of HttpClient. /// @@ -52,14 +58,19 @@ private sealed class NonDisposableHttpClientHandler : DelegatingHandler /// /// Private constructor to prevent direct instantiation of the class. /// - private NonDisposableHttpClientHandler() : base(CreateHandler()) + private NonDisposableHttpClientHandler(bool allowRedirect) : base(CreateHandler(allowRedirect)) { } /// - /// Gets the singleton instance of . + /// Gets the singleton instance of that follows HTTP redirects. + /// + public static NonDisposableHttpClientHandler Instance { get; } = new(allowRedirect: true); + + /// + /// Gets the singleton instance of that does not follow HTTP redirects. /// - public static NonDisposableHttpClientHandler Instance { get; } = new(); + public static NonDisposableHttpClientHandler NonRedirectingInstance { get; } = new(allowRedirect: false); /// /// Disposes the underlying resources held by the . @@ -74,7 +85,7 @@ protected override void Dispose(bool disposing) } #if NET - private static SocketsHttpHandler CreateHandler() + private static SocketsHttpHandler CreateHandler(bool allowRedirect) { return new SocketsHttpHandler() { @@ -86,12 +97,13 @@ private static SocketsHttpHandler CreateHandler() { CertificateRevocationCheckMode = X509RevocationMode.Online, }, + AllowAutoRedirect = allowRedirect, }; } #elif NETSTANDARD2_0_OR_GREATER - private static HttpClientHandler CreateHandler() + private static HttpClientHandler CreateHandler(bool allowRedirect) { - var handler = new HttpClientHandler(); + var handler = new HttpClientHandler() { AllowAutoRedirect = allowRedirect }; try { handler.CheckCertificateRevocationList = true; @@ -100,8 +112,8 @@ private static HttpClientHandler CreateHandler() return handler; } #elif NETFRAMEWORK - private static HttpClientHandler CreateHandler() - => new(); + private static HttpClientHandler CreateHandler(bool allowRedirect) + => new() { AllowAutoRedirect = allowRedirect }; #endif } } diff --git a/dotnet/src/Plugins/Plugins.Core/HttpPlugin.cs b/dotnet/src/Plugins/Plugins.Core/HttpPlugin.cs index 89f49d8d4705..ea11b17a258a 100644 --- a/dotnet/src/Plugins/Plugins.Core/HttpPlugin.cs +++ b/dotnet/src/Plugins/Plugins.Core/HttpPlugin.cs @@ -23,6 +23,9 @@ namespace Microsoft.SemanticKernel.Plugins.Core; /// When exposing this plugin to an LLM via auto function calling, ensure that /// is restricted to trusted values only. /// +/// +/// The default HTTP client does not follow redirects to prevent bypassing the allow-list. +/// /// [System.Diagnostics.CodeAnalysis.SuppressMessage("Design", "CA1054:URI-like parameters should not be strings", Justification = "Semantic Kernel operates on strings")] @@ -43,10 +46,11 @@ public HttpPlugin() : this(null) /// The HTTP client to use. /// /// assumes ownership of the instance and will dispose it when the plugin is disposed. + /// When providing a custom client, configure it with AllowAutoRedirect = false to preserve the guarantee. /// [ActivatorUtilitiesConstructor] public HttpPlugin(HttpClient? client = null) => - this._client = client ?? HttpClientProvider.GetHttpClient(); + this._client = client ?? HttpClientProvider.GetNonRedirectingHttpClient(); /// /// List of allowed domains to send requests to. @@ -54,6 +58,7 @@ public HttpPlugin(HttpClient? client = null) => /// /// Defaults to an empty collection (no domains allowed). Must be explicitly populated /// with trusted domains before any requests will succeed. + /// HTTP redirects are not followed to prevent bypassing the allow-list. /// public IEnumerable? AllowedDomains { diff --git a/dotnet/src/Plugins/Plugins.UnitTests/Core/HttpPluginTests.cs b/dotnet/src/Plugins/Plugins.UnitTests/Core/HttpPluginTests.cs index d867e56c0630..f9a63f84264d 100644 --- a/dotnet/src/Plugins/Plugins.UnitTests/Core/HttpPluginTests.cs +++ b/dotnet/src/Plugins/Plugins.UnitTests/Core/HttpPluginTests.cs @@ -133,6 +133,53 @@ public async Task ItThrowsInvalidOperationExceptionForInvalidDomainAsync() await Assert.ThrowsAsync(async () => await plugin.DeleteAsync(invalidUri)); } + [Fact] + public async Task ItDoesNotFollowRedirectsAsync() + { + // Arrange - start a local server that always returns a 302 redirect + using var listener = new System.Net.HttpListener(); + var port = new Random().Next(49152, 65535); + listener.Prefixes.Add($"http://localhost:{port}/"); + listener.Start(); + bool redirectTargetContacted = false; + + _ = Task.Run(async () => + { + while (listener.IsListening) + { + try + { + var ctx = await listener.GetContextAsync(); + if (ctx.Request.Url!.AbsolutePath == "/start") + { + ctx.Response.StatusCode = 302; + ctx.Response.RedirectLocation = $"http://localhost:{port}/secret"; + ctx.Response.Close(); + } + else if (ctx.Request.Url.AbsolutePath == "/secret") + { + redirectTargetContacted = true; + ctx.Response.StatusCode = 200; + ctx.Response.Close(); + } + } + catch (ObjectDisposedException) { break; } + catch (System.Net.HttpListenerException) { break; } + } + }); + + var plugin = new HttpPlugin() + { + AllowedDomains = ["localhost"] + }; + + // Act & Assert - the plugin should throw because 302 is a non-success status + await Assert.ThrowsAsync(() => plugin.GetAsync($"http://localhost:{port}/start")); + Assert.False(redirectTargetContacted, "The redirect target should not have been contacted."); + + listener.Stop(); + } + private Mock CreateMock() { var mockHandler = new Mock(); diff --git a/dotnet/src/Plugins/Plugins.UnitTests/Web/WebFileDownloadPluginTests.cs b/dotnet/src/Plugins/Plugins.UnitTests/Web/WebFileDownloadPluginTests.cs index 6d0ce6f5aeb4..92a15be0c1df 100644 --- a/dotnet/src/Plugins/Plugins.UnitTests/Web/WebFileDownloadPluginTests.cs +++ b/dotnet/src/Plugins/Plugins.UnitTests/Web/WebFileDownloadPluginTests.cs @@ -5,6 +5,7 @@ using System.Net.Http; using System.Runtime.InteropServices; using System.Threading.Tasks; +using Microsoft.SemanticKernel; using Microsoft.SemanticKernel.Plugins.Web; using Xunit; @@ -77,6 +78,70 @@ public async Task DownloadToFileFailsForInvalidDomainAsync() await Assert.ThrowsAsync(async () => await webFileDownload.DownloadToFileAsync(uri, filePath)); } + [Fact] + public async Task DownloadToFileDoesNotFollowRedirectsAsync() + { + // Arrange - start a local server that returns a 302 redirect + using var listener = new System.Net.HttpListener(); + var port = new Random().Next(49152, 65535); + listener.Prefixes.Add($"http://localhost:{port}/"); + listener.Start(); + bool redirectTargetContacted = false; + + _ = Task.Run(async () => + { + while (listener.IsListening) + { + try + { + var ctx = await listener.GetContextAsync(); + if (ctx.Request.Url!.AbsolutePath == "/start") + { + ctx.Response.StatusCode = 302; + ctx.Response.RedirectLocation = $"http://localhost:{port}/secret.png"; + ctx.Response.Close(); + } + else if (ctx.Request.Url.AbsolutePath == "/secret.png") + { + redirectTargetContacted = true; + ctx.Response.StatusCode = 200; + ctx.Response.ContentType = "image/png"; + ctx.Response.OutputStream.Write(new byte[] { 0x89, 0x50, 0x4E, 0x47 }); + ctx.Response.Close(); + } + } + catch (ObjectDisposedException) { break; } + catch (System.Net.HttpListenerException) { break; } + } + }); + + var folderPath = Path.Combine(Path.GetTempPath(), Guid.NewGuid().ToString()); + var filePath = Path.Combine(folderPath, "file.png"); + Directory.CreateDirectory(folderPath); + + var webFileDownload = new WebFileDownloadPlugin() + { + AllowedDomains = ["localhost"], + AllowedFolders = [folderPath] + }; + + try + { + // Act & Assert - the plugin should throw because 302 is a non-success status + await Assert.ThrowsAsync(() => webFileDownload.DownloadToFileAsync(new Uri($"http://localhost:{port}/start"), filePath)); + Assert.False(redirectTargetContacted, "The redirect target should not have been contacted."); + Assert.False(Path.Exists(filePath)); + } + finally + { + listener.Stop(); + if (Path.Exists(folderPath)) + { + Directory.Delete(folderPath, true); + } + } + } + [Fact] public async Task DownloadToFileDeniesAllWithDefaultConfigAsync() { diff --git a/dotnet/src/Plugins/Plugins.Web/WebFileDownloadPlugin.cs b/dotnet/src/Plugins/Plugins.Web/WebFileDownloadPlugin.cs index d410bb8e22f0..acdcd9659fd6 100644 --- a/dotnet/src/Plugins/Plugins.Web/WebFileDownloadPlugin.cs +++ b/dotnet/src/Plugins/Plugins.Web/WebFileDownloadPlugin.cs @@ -29,6 +29,9 @@ namespace Microsoft.SemanticKernel.Plugins.Web; /// values only. Unrestricted configuration may allow unintended downloads to /// local paths. /// +/// +/// The default HTTP client does not follow redirects to prevent bypassing the allow-list. +/// /// public sealed class WebFileDownloadPlugin { @@ -42,7 +45,7 @@ public sealed class WebFileDownloadPlugin /// /// The to use for logging. If null, no logging will be performed. public WebFileDownloadPlugin(ILoggerFactory? loggerFactory = null) : - this(HttpClientProvider.GetHttpClient(), loggerFactory) + this(HttpClientProvider.GetNonRedirectingHttpClient(), loggerFactory) { } @@ -51,6 +54,9 @@ public WebFileDownloadPlugin(ILoggerFactory? loggerFactory = null) : /// /// The HTTP client to use for making requests. /// The to use for logging. If null, no logging will be performed. + /// + /// When providing a custom client, configure it with AllowAutoRedirect = false to preserve the guarantee. + /// public WebFileDownloadPlugin(HttpClient httpClient, ILoggerFactory? loggerFactory = null) { this._httpClient = httpClient; @@ -63,6 +69,7 @@ public WebFileDownloadPlugin(HttpClient httpClient, ILoggerFactory? loggerFactor /// /// Defaults to an empty collection (no domains allowed). Must be explicitly populated /// with trusted domains before any downloads will succeed. + /// HTTP redirects are not followed to prevent bypassing the allow-list. /// public IEnumerable? AllowedDomains {