mirror of
https://github.com/microsoft/semantic-kernel.git
synced 2026-10-12 02:19:06 +00:00
.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 <[email protected]>
This commit is contained in:
SergeyMenshykh
co-authored by
Copilot App
Copilot App
parent
9974625ddc
commit
a205c428ee
@@ -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.
|
||||
/// </remarks>
|
||||
public IEnumerable<string>? 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;
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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<InvalidOperationException>(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<InvalidOperationException>(() => plugin.ReadAsync(readOnlyFile));
|
||||
var missingReadEx = await Assert.ThrowsAsync<InvalidOperationException>(() => plugin.ReadAsync(missingFile));
|
||||
var readOnlyWriteEx = await Assert.ThrowsAsync<InvalidOperationException>(() => plugin.WriteAsync(readOnlyFile, "changed"));
|
||||
var missingWriteEx = await Assert.ThrowsAsync<InvalidOperationException>(() => 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<UnauthorizedAccessException>(() => 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<InvalidOperationException>(() => 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<InvalidOperationException>(() => plugin.ReadAsync(missingPath));
|
||||
var expectedWriteError = await Assert.ThrowsAsync<InvalidOperationException>(() => plugin.WriteAsync(missingPath, "changed"));
|
||||
var readError = await Assert.ThrowsAsync<InvalidOperationException>(() => plugin.ReadAsync(unresolvablePath));
|
||||
var writeError = await Assert.ThrowsAsync<InvalidOperationException>(() => 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<InvalidOperationException>(() => plugin.ReadAsync(regularPath));
|
||||
var configurationWriteError = await Assert.ThrowsAsync<InvalidOperationException>(() => 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<InvalidOperationException>(() => plugin.ReadAsync(missingPath));
|
||||
var expectedWriteError = await Assert.ThrowsAsync<InvalidOperationException>(() => plugin.WriteAsync(missingPath, "changed"));
|
||||
var readError = await Assert.ThrowsAsync<InvalidOperationException>(() => plugin.ReadAsync(inaccessiblePath));
|
||||
var writeError = await Assert.ThrowsAsync<InvalidOperationException>(() => 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<UnauthorizedAccessException>(() => plugin.ReadAsync(regularPath));
|
||||
await Assert.ThrowsAsync<UnauthorizedAccessException>(() => plugin.WriteAsync(regularPath, "changed"));
|
||||
}
|
||||
finally
|
||||
{
|
||||
restrictedSecurity.RemoveAccessRule(denyRule);
|
||||
restrictedDir.SetAccessControl(restrictedSecurity);
|
||||
TryDeleteDirectory(tempDir);
|
||||
}
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task ItCannotWriteToDisallowedFoldersAsync()
|
||||
{
|
||||
|
||||
Reference in New Issue
Block a user