From a205c428ee03094751dd33943a8e23571b4e410a Mon Sep 17 00:00:00 2001 From: SergeyMenshykh <68852919+SergeyMenshykh@users.noreply.github.com> Date: Mon, 5 Oct 2026 09:39:41 +0000 Subject: [PATCH] .Net: Update file handling for FileIOPlugin (#14525) ### Motivation and Context Ensure `FileIOPlugin` applies its configured access rules consistently and provides predictable error responses. This improves reliability for applications exposing file operations through kernel functions. ### Description Refines `FileIOPlugin` path validation and error handling while preserving its public API and configured folder restrictions. Adds regression coverage for both default and explicitly configured settings, including consistent error responses and continued enforcement of file-access rules. --------- Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../src/Plugins/Plugins.Core/FileIOPlugin.cs | 27 ++- .../Core/FileIOPluginTests.cs | 187 ++++++++++++++++++ 2 files changed, 206 insertions(+), 8 deletions(-) diff --git a/dotnet/src/Plugins/Plugins.Core/FileIOPlugin.cs b/dotnet/src/Plugins/Plugins.Core/FileIOPlugin.cs index 5d11b4b023..2df1770a63 100644 --- a/dotnet/src/Plugins/Plugins.Core/FileIOPlugin.cs +++ b/dotnet/src/Plugins/Plugins.Core/FileIOPlugin.cs @@ -4,6 +4,7 @@ using System; using System.Collections.Generic; using System.ComponentModel; using System.IO; +using System.Security; using System.Text; using System.Threading.Tasks; @@ -31,6 +32,7 @@ public sealed class FileIOPlugin /// Defaults to an empty collection (no folders allowed). Must be explicitly populated /// with trusted directory paths before any file operations will succeed. /// Paths are canonicalized before validation to prevent directory traversal. + /// Requested file paths that cannot be safely resolved are denied. /// public IEnumerable? AllowedFolders { @@ -135,19 +137,22 @@ public sealed class FileIOPlugin throw new ArgumentException("Invalid file path, UNC paths are not supported.", nameof(path)); } - canonicalPath = PathUtilities.GetSafeFullPath(canonicalPath); - - if (File.Exists(canonicalPath) && File.GetAttributes(canonicalPath).HasFlag(FileAttributes.ReadOnly)) - { - // Most environments will throw this with OpenWrite, but running inside docker on Linux will not. - throw new UnauthorizedAccessException($"File is read-only: {canonicalPath}"); - } - + // Avoid probing the filesystem when no folders are allowed. if (this._allowedFolders is null || this._allowedFolders.Count == 0) { return false; } + try + { + canonicalPath = PathUtilities.GetSafeFullPath(canonicalPath); + } + catch (Exception ex) when (ex is IOException or UnauthorizedAccessException or InvalidOperationException or SecurityException) + { + // Caller-path resolution failures must not disclose filesystem details. + return false; + } + var canonicalDir = Path.GetDirectoryName(canonicalPath); if (string.IsNullOrEmpty(canonicalDir)) { @@ -166,6 +171,12 @@ public sealed class FileIOPlugin if (canonicalDir.StartsWith(canonicalAllowed, PathUtilities.PathComparison) || (canonicalDir + separator).Equals(canonicalAllowed, PathUtilities.PathComparison)) { + if (File.Exists(canonicalPath) && File.GetAttributes(canonicalPath).HasFlag(FileAttributes.ReadOnly)) + { + // Most environments will throw this with OpenWrite, but running inside docker on Linux will not. + throw new UnauthorizedAccessException("File is read-only."); + } + return true; } } diff --git a/dotnet/src/Plugins/Plugins.UnitTests/Core/FileIOPluginTests.cs b/dotnet/src/Plugins/Plugins.UnitTests/Core/FileIOPluginTests.cs index 0b309f1dfb..46e1665f90 100644 --- a/dotnet/src/Plugins/Plugins.UnitTests/Core/FileIOPluginTests.cs +++ b/dotnet/src/Plugins/Plugins.UnitTests/Core/FileIOPluginTests.cs @@ -3,6 +3,8 @@ using System; using System.IO; using System.Runtime.InteropServices; +using System.Security.AccessControl; +using System.Security.Principal; using System.Threading.Tasks; using Microsoft.SemanticKernel; using Microsoft.SemanticKernel.Plugins.Core; @@ -111,6 +113,191 @@ public class FileIOPluginTests await Assert.ThrowsAsync(async () => await plugin.WriteAsync(path, "hello world")); } + [Fact] + public async Task ItDoesNotRevealReadOnlyOrExistenceOfDisallowedFilesAsync() + { + // Arrange + var tempDir = Path.Combine(Path.GetTempPath(), $"FileIOPluginTests_{Guid.NewGuid():N}"); + var allowedDir = Path.Combine(tempDir, "allowed"); + var outsideDir = Path.Combine(tempDir, "outside"); + Directory.CreateDirectory(allowedDir); + Directory.CreateDirectory(outsideDir); + + var readOnlyFile = Path.Combine(outsideDir, "readonly.txt"); + var missingFile = Path.Combine(outsideDir, "missing.txt"); + await File.WriteAllTextAsync(readOnlyFile, "secret"); + File.SetAttributes(readOnlyFile, FileAttributes.ReadOnly); + + try + { + foreach (var plugin in new[] + { + new FileIOPlugin(), + new FileIOPlugin() { AllowedFolders = [allowedDir], DisableFileOverwrite = false } + }) + { + // Act + var readOnlyReadEx = await Assert.ThrowsAsync(() => plugin.ReadAsync(readOnlyFile)); + var missingReadEx = await Assert.ThrowsAsync(() => plugin.ReadAsync(missingFile)); + var readOnlyWriteEx = await Assert.ThrowsAsync(() => plugin.WriteAsync(readOnlyFile, "changed")); + var missingWriteEx = await Assert.ThrowsAsync(() => plugin.WriteAsync(missingFile, "changed")); + + // Assert - responses for existing read-only and missing files must be indistinguishable + Assert.Equal(missingReadEx.Message, readOnlyReadEx.Message); + Assert.Equal(missingWriteEx.Message, readOnlyWriteEx.Message); + Assert.DoesNotContain(outsideDir, readOnlyReadEx.Message, StringComparison.OrdinalIgnoreCase); + Assert.DoesNotContain(outsideDir, readOnlyWriteEx.Message, StringComparison.OrdinalIgnoreCase); + } + } + finally + { + File.SetAttributes(readOnlyFile, FileAttributes.Normal); + TryDeleteDirectory(tempDir); + } + } + + [Fact] + public async Task ItDoesNotIncludePathInReadOnlyExceptionAsync() + { + // Arrange + var plugin = new FileIOPlugin() + { + AllowedFolders = [Path.GetTempPath()], + DisableFileOverwrite = false + }; + var path = Path.GetTempFileName(); + File.SetAttributes(path, FileAttributes.ReadOnly); + + try + { + // Act + var ex = await Assert.ThrowsAsync(() => plugin.WriteAsync(path, "hello world")); + + // Assert + Assert.DoesNotContain(Path.GetFileName(path), ex.Message, StringComparison.OrdinalIgnoreCase); + } + finally + { + File.SetAttributes(path, FileAttributes.Normal); + File.Delete(path); + } + } + + [Fact] + public async Task ItDeniesUnresolvablePathsWithoutHidingConfigurationErrorsAsync() + { + var tempDir = Path.Combine(Path.GetTempPath(), $"FileIOPluginTests_{Guid.NewGuid():N}"); + var allowedDir = Path.Combine(tempDir, "allowed"); + var outsideDir = Path.Combine(tempDir, "outside"); + Directory.CreateDirectory(allowedDir); + Directory.CreateDirectory(outsideDir); + var linkPath = Path.Combine(outsideDir, "loop"); + + try + { + try + { + Directory.CreateSymbolicLink(linkPath, linkPath); + } + catch (Exception ex) when (ex is IOException or UnauthorizedAccessException) + { + // Skip if this environment does not permit symbolic link creation. + return; + } + + var resolutionError = Assert.Throws(() => PathUtilities.GetSafeFullPath(linkPath)); + var plugin = new FileIOPlugin() { AllowedFolders = [allowedDir] }; + var missingPath = Path.Combine(outsideDir, "missing.txt"); + var unresolvablePath = Path.Combine(linkPath, "file.txt"); + + var expectedReadError = await Assert.ThrowsAsync(() => plugin.ReadAsync(missingPath)); + var expectedWriteError = await Assert.ThrowsAsync(() => plugin.WriteAsync(missingPath, "changed")); + var readError = await Assert.ThrowsAsync(() => plugin.ReadAsync(unresolvablePath)); + var writeError = await Assert.ThrowsAsync(() => plugin.WriteAsync(unresolvablePath, "changed")); + + Assert.Equal(expectedReadError.Message, readError.Message); + Assert.Equal(expectedWriteError.Message, writeError.Message); + + plugin.AllowedFolders = [linkPath]; + var regularPath = Path.Combine(allowedDir, "missing.txt"); + var configurationReadError = await Assert.ThrowsAsync(() => plugin.ReadAsync(regularPath)); + var configurationWriteError = await Assert.ThrowsAsync(() => plugin.WriteAsync(regularPath, "changed")); + Assert.Equal(resolutionError.Message, configurationReadError.Message); + Assert.Equal(resolutionError.Message, configurationWriteError.Message); + } + finally + { + TryDeleteDirectory(tempDir); + } + } + + [Fact] + public async Task ItDeniesInaccessiblePathsWithoutHidingConfigurationErrorsAsync() + { + if (!OperatingSystem.IsWindows()) + { + return; + } + + var tempDir = Path.Combine(Path.GetTempPath(), $"FileIOPluginTests_{Guid.NewGuid():N}"); + var allowedDir = Path.Combine(tempDir, "allowed"); + var outsideDir = Path.Combine(tempDir, "outside"); + Directory.CreateDirectory(allowedDir); + Directory.CreateDirectory(outsideDir); + var restrictedDir = Directory.CreateDirectory(Path.Combine(outsideDir, "restricted")); + var restrictedRoot = Directory.CreateDirectory(Path.Combine(restrictedDir.FullName, "nested")); + var inaccessiblePath = Path.Combine(restrictedRoot.FullName, "file.txt"); + await File.WriteAllTextAsync(inaccessiblePath, "secret"); + + var restrictedSecurity = restrictedDir.GetAccessControl(AccessControlSections.Access); + using var identity = WindowsIdentity.GetCurrent(); + Assert.NotNull(identity.User); + var denyRule = new FileSystemAccessRule( + identity.User, + FileSystemRights.Read, + InheritanceFlags.ContainerInherit | InheritanceFlags.ObjectInherit, + PropagationFlags.None, + AccessControlType.Deny); + restrictedSecurity.AddAccessRule(denyRule); + + try + { + restrictedDir.SetAccessControl(restrictedSecurity); + try + { + PathUtilities.GetSafeFullPath(inaccessiblePath); + + // Skip if this environment does not enforce the deny rule (e.g. elevated processes). + return; + } + catch (UnauthorizedAccessException) + { + } + + var plugin = new FileIOPlugin() { AllowedFolders = [allowedDir] }; + var missingPath = Path.Combine(outsideDir, "missing.txt"); + + var expectedReadError = await Assert.ThrowsAsync(() => plugin.ReadAsync(missingPath)); + var expectedWriteError = await Assert.ThrowsAsync(() => plugin.WriteAsync(missingPath, "changed")); + var readError = await Assert.ThrowsAsync(() => plugin.ReadAsync(inaccessiblePath)); + var writeError = await Assert.ThrowsAsync(() => plugin.WriteAsync(inaccessiblePath, "changed")); + + Assert.Equal(expectedReadError.Message, readError.Message); + Assert.Equal(expectedWriteError.Message, writeError.Message); + + plugin.AllowedFolders = [restrictedRoot.FullName]; + var regularPath = Path.Combine(allowedDir, "missing.txt"); + await Assert.ThrowsAsync(() => plugin.ReadAsync(regularPath)); + await Assert.ThrowsAsync(() => plugin.WriteAsync(regularPath, "changed")); + } + finally + { + restrictedSecurity.RemoveAccessRule(denyRule); + restrictedDir.SetAccessControl(restrictedSecurity); + TryDeleteDirectory(tempDir); + } + } + [Fact] public async Task ItCannotWriteToDisallowedFoldersAsync() {