Skip to content

Commit a205c42

Browse files
.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>
1 parent 9974625 commit a205c42

2 files changed

Lines changed: 204 additions & 6 deletions

File tree

‎dotnet/src/Plugins/Plugins.Core/FileIOPlugin.cs‎

Lines changed: 17 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
using System.Collections.Generic;
55
using System.ComponentModel;
66
using System.IO;
7+
using System.Security;
78
using System.Text;
89
using System.Threading.Tasks;
910

@@ -31,6 +32,7 @@ public sealed class FileIOPlugin
3132
/// Defaults to an empty collection (no folders allowed). Must be explicitly populated
3233
/// with trusted directory paths before any file operations will succeed.
3334
/// Paths are canonicalized before validation to prevent directory traversal.
35+
/// Requested file paths that cannot be safely resolved are denied.
3436
/// </remarks>
3537
public IEnumerable<string>? AllowedFolders
3638
{
@@ -135,16 +137,19 @@ private bool TryGetAllowedFilePath(string path, out string canonicalPath)
135137
throw new ArgumentException("Invalid file path, UNC paths are not supported.", nameof(path));
136138
}
137139

138-
canonicalPath = PathUtilities.GetSafeFullPath(canonicalPath);
139-
140-
if (File.Exists(canonicalPath) && File.GetAttributes(canonicalPath).HasFlag(FileAttributes.ReadOnly))
140+
// Avoid probing the filesystem when no folders are allowed.
141+
if (this._allowedFolders is null || this._allowedFolders.Count == 0)
141142
{
142-
// Most environments will throw this with OpenWrite, but running inside docker on Linux will not.
143-
throw new UnauthorizedAccessException($"File is read-only: {canonicalPath}");
143+
return false;
144144
}
145145

146-
if (this._allowedFolders is null || this._allowedFolders.Count == 0)
146+
try
147147
{
148+
canonicalPath = PathUtilities.GetSafeFullPath(canonicalPath);
149+
}
150+
catch (Exception ex) when (ex is IOException or UnauthorizedAccessException or InvalidOperationException or SecurityException)
151+
{
152+
// Caller-path resolution failures must not disclose filesystem details.
148153
return false;
149154
}
150155

@@ -166,6 +171,12 @@ private bool TryGetAllowedFilePath(string path, out string canonicalPath)
166171
if (canonicalDir.StartsWith(canonicalAllowed, PathUtilities.PathComparison)
167172
|| (canonicalDir + separator).Equals(canonicalAllowed, PathUtilities.PathComparison))
168173
{
174+
if (File.Exists(canonicalPath) && File.GetAttributes(canonicalPath).HasFlag(FileAttributes.ReadOnly))
175+
{
176+
// Most environments will throw this with OpenWrite, but running inside docker on Linux will not.
177+
throw new UnauthorizedAccessException("File is read-only.");
178+
}
179+
169180
return true;
170181
}
171182
}

‎dotnet/src/Plugins/Plugins.UnitTests/Core/FileIOPluginTests.cs‎

Lines changed: 187 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,8 @@
33
using System;
44
using System.IO;
55
using System.Runtime.InteropServices;
6+
using System.Security.AccessControl;
7+
using System.Security.Principal;
68
using System.Threading.Tasks;
79
using Microsoft.SemanticKernel;
810
using Microsoft.SemanticKernel.Plugins.Core;
@@ -111,6 +113,191 @@ public async Task ItDeniesAllPathsWithDefaultConfigAsync()
111113
await Assert.ThrowsAsync<InvalidOperationException>(async () => await plugin.WriteAsync(path, "hello world"));
112114
}
113115

116+
[Fact]
117+
public async Task ItDoesNotRevealReadOnlyOrExistenceOfDisallowedFilesAsync()
118+
{
119+
// Arrange
120+
var tempDir = Path.Combine(Path.GetTempPath(), $"FileIOPluginTests_{Guid.NewGuid():N}");
121+
var allowedDir = Path.Combine(tempDir, "allowed");
122+
var outsideDir = Path.Combine(tempDir, "outside");
123+
Directory.CreateDirectory(allowedDir);
124+
Directory.CreateDirectory(outsideDir);
125+
126+
var readOnlyFile = Path.Combine(outsideDir, "readonly.txt");
127+
var missingFile = Path.Combine(outsideDir, "missing.txt");
128+
await File.WriteAllTextAsync(readOnlyFile, "secret");
129+
File.SetAttributes(readOnlyFile, FileAttributes.ReadOnly);
130+
131+
try
132+
{
133+
foreach (var plugin in new[]
134+
{
135+
new FileIOPlugin(),
136+
new FileIOPlugin() { AllowedFolders = [allowedDir], DisableFileOverwrite = false }
137+
})
138+
{
139+
// Act
140+
var readOnlyReadEx = await Assert.ThrowsAsync<InvalidOperationException>(() => plugin.ReadAsync(readOnlyFile));
141+
var missingReadEx = await Assert.ThrowsAsync<InvalidOperationException>(() => plugin.ReadAsync(missingFile));
142+
var readOnlyWriteEx = await Assert.ThrowsAsync<InvalidOperationException>(() => plugin.WriteAsync(readOnlyFile, "changed"));
143+
var missingWriteEx = await Assert.ThrowsAsync<InvalidOperationException>(() => plugin.WriteAsync(missingFile, "changed"));
144+
145+
// Assert - responses for existing read-only and missing files must be indistinguishable
146+
Assert.Equal(missingReadEx.Message, readOnlyReadEx.Message);
147+
Assert.Equal(missingWriteEx.Message, readOnlyWriteEx.Message);
148+
Assert.DoesNotContain(outsideDir, readOnlyReadEx.Message, StringComparison.OrdinalIgnoreCase);
149+
Assert.DoesNotContain(outsideDir, readOnlyWriteEx.Message, StringComparison.OrdinalIgnoreCase);
150+
}
151+
}
152+
finally
153+
{
154+
File.SetAttributes(readOnlyFile, FileAttributes.Normal);
155+
TryDeleteDirectory(tempDir);
156+
}
157+
}
158+
159+
[Fact]
160+
public async Task ItDoesNotIncludePathInReadOnlyExceptionAsync()
161+
{
162+
// Arrange
163+
var plugin = new FileIOPlugin()
164+
{
165+
AllowedFolders = [Path.GetTempPath()],
166+
DisableFileOverwrite = false
167+
};
168+
var path = Path.GetTempFileName();
169+
File.SetAttributes(path, FileAttributes.ReadOnly);
170+
171+
try
172+
{
173+
// Act
174+
var ex = await Assert.ThrowsAsync<UnauthorizedAccessException>(() => plugin.WriteAsync(path, "hello world"));
175+
176+
// Assert
177+
Assert.DoesNotContain(Path.GetFileName(path), ex.Message, StringComparison.OrdinalIgnoreCase);
178+
}
179+
finally
180+
{
181+
File.SetAttributes(path, FileAttributes.Normal);
182+
File.Delete(path);
183+
}
184+
}
185+
186+
[Fact]
187+
public async Task ItDeniesUnresolvablePathsWithoutHidingConfigurationErrorsAsync()
188+
{
189+
var tempDir = Path.Combine(Path.GetTempPath(), $"FileIOPluginTests_{Guid.NewGuid():N}");
190+
var allowedDir = Path.Combine(tempDir, "allowed");
191+
var outsideDir = Path.Combine(tempDir, "outside");
192+
Directory.CreateDirectory(allowedDir);
193+
Directory.CreateDirectory(outsideDir);
194+
var linkPath = Path.Combine(outsideDir, "loop");
195+
196+
try
197+
{
198+
try
199+
{
200+
Directory.CreateSymbolicLink(linkPath, linkPath);
201+
}
202+
catch (Exception ex) when (ex is IOException or UnauthorizedAccessException)
203+
{
204+
// Skip if this environment does not permit symbolic link creation.
205+
return;
206+
}
207+
208+
var resolutionError = Assert.Throws<InvalidOperationException>(() => PathUtilities.GetSafeFullPath(linkPath));
209+
var plugin = new FileIOPlugin() { AllowedFolders = [allowedDir] };
210+
var missingPath = Path.Combine(outsideDir, "missing.txt");
211+
var unresolvablePath = Path.Combine(linkPath, "file.txt");
212+
213+
var expectedReadError = await Assert.ThrowsAsync<InvalidOperationException>(() => plugin.ReadAsync(missingPath));
214+
var expectedWriteError = await Assert.ThrowsAsync<InvalidOperationException>(() => plugin.WriteAsync(missingPath, "changed"));
215+
var readError = await Assert.ThrowsAsync<InvalidOperationException>(() => plugin.ReadAsync(unresolvablePath));
216+
var writeError = await Assert.ThrowsAsync<InvalidOperationException>(() => plugin.WriteAsync(unresolvablePath, "changed"));
217+
218+
Assert.Equal(expectedReadError.Message, readError.Message);
219+
Assert.Equal(expectedWriteError.Message, writeError.Message);
220+
221+
plugin.AllowedFolders = [linkPath];
222+
var regularPath = Path.Combine(allowedDir, "missing.txt");
223+
var configurationReadError = await Assert.ThrowsAsync<InvalidOperationException>(() => plugin.ReadAsync(regularPath));
224+
var configurationWriteError = await Assert.ThrowsAsync<InvalidOperationException>(() => plugin.WriteAsync(regularPath, "changed"));
225+
Assert.Equal(resolutionError.Message, configurationReadError.Message);
226+
Assert.Equal(resolutionError.Message, configurationWriteError.Message);
227+
}
228+
finally
229+
{
230+
TryDeleteDirectory(tempDir);
231+
}
232+
}
233+
234+
[Fact]
235+
public async Task ItDeniesInaccessiblePathsWithoutHidingConfigurationErrorsAsync()
236+
{
237+
if (!OperatingSystem.IsWindows())
238+
{
239+
return;
240+
}
241+
242+
var tempDir = Path.Combine(Path.GetTempPath(), $"FileIOPluginTests_{Guid.NewGuid():N}");
243+
var allowedDir = Path.Combine(tempDir, "allowed");
244+
var outsideDir = Path.Combine(tempDir, "outside");
245+
Directory.CreateDirectory(allowedDir);
246+
Directory.CreateDirectory(outsideDir);
247+
var restrictedDir = Directory.CreateDirectory(Path.Combine(outsideDir, "restricted"));
248+
var restrictedRoot = Directory.CreateDirectory(Path.Combine(restrictedDir.FullName, "nested"));
249+
var inaccessiblePath = Path.Combine(restrictedRoot.FullName, "file.txt");
250+
await File.WriteAllTextAsync(inaccessiblePath, "secret");
251+
252+
var restrictedSecurity = restrictedDir.GetAccessControl(AccessControlSections.Access);
253+
using var identity = WindowsIdentity.GetCurrent();
254+
Assert.NotNull(identity.User);
255+
var denyRule = new FileSystemAccessRule(
256+
identity.User,
257+
FileSystemRights.Read,
258+
InheritanceFlags.ContainerInherit | InheritanceFlags.ObjectInherit,
259+
PropagationFlags.None,
260+
AccessControlType.Deny);
261+
restrictedSecurity.AddAccessRule(denyRule);
262+
263+
try
264+
{
265+
restrictedDir.SetAccessControl(restrictedSecurity);
266+
try
267+
{
268+
PathUtilities.GetSafeFullPath(inaccessiblePath);
269+
270+
// Skip if this environment does not enforce the deny rule (e.g. elevated processes).
271+
return;
272+
}
273+
catch (UnauthorizedAccessException)
274+
{
275+
}
276+
277+
var plugin = new FileIOPlugin() { AllowedFolders = [allowedDir] };
278+
var missingPath = Path.Combine(outsideDir, "missing.txt");
279+
280+
var expectedReadError = await Assert.ThrowsAsync<InvalidOperationException>(() => plugin.ReadAsync(missingPath));
281+
var expectedWriteError = await Assert.ThrowsAsync<InvalidOperationException>(() => plugin.WriteAsync(missingPath, "changed"));
282+
var readError = await Assert.ThrowsAsync<InvalidOperationException>(() => plugin.ReadAsync(inaccessiblePath));
283+
var writeError = await Assert.ThrowsAsync<InvalidOperationException>(() => plugin.WriteAsync(inaccessiblePath, "changed"));
284+
285+
Assert.Equal(expectedReadError.Message, readError.Message);
286+
Assert.Equal(expectedWriteError.Message, writeError.Message);
287+
288+
plugin.AllowedFolders = [restrictedRoot.FullName];
289+
var regularPath = Path.Combine(allowedDir, "missing.txt");
290+
await Assert.ThrowsAsync<UnauthorizedAccessException>(() => plugin.ReadAsync(regularPath));
291+
await Assert.ThrowsAsync<UnauthorizedAccessException>(() => plugin.WriteAsync(regularPath, "changed"));
292+
}
293+
finally
294+
{
295+
restrictedSecurity.RemoveAccessRule(denyRule);
296+
restrictedDir.SetAccessControl(restrictedSecurity);
297+
TryDeleteDirectory(tempDir);
298+
}
299+
}
300+
114301
[Fact]
115302
public async Task ItCannotWriteToDisallowedFoldersAsync()
116303
{

0 commit comments

Comments
 (0)