Skip to content

.Net: [Breaking] Harden DocumentPlugin security defaults with deny-by-default AllowedDirectories #13683

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
rogerbarreto merged 3 commits into from
Mar 18, 2026
Merged

.Net: [Breaking] Harden DocumentPlugin security defaults with deny-by-default AllowedDirectories #13683

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
fd232f5
.Net: Harden DocumentPlugin security defaults with deny-by-default Al…
rogerbarreto Mar 18, 2026
d6d569d
Merge branch 'main' into issues/sve
rogerbarreto Mar 18, 2026
7853b07
Merge branch 'main' into issues/sve
rogerbarreto Mar 18, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
86 changes: 84 additions & 2 deletions dotnet/src/Plugins/Plugins.Document/DocumentPlugin.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
// Copyright (c) Microsoft. All rights reserved.

using System;
using System.Collections.Generic;
using System.ComponentModel;
using System.IO;
using System.Threading;
Expand Down Expand Up @@ -35,6 +36,16 @@ namespace Microsoft.SemanticKernel.Plugins.Document;
/// <summary>
/// Plugin for interacting with documents (e.g. Microsoft Word)
/// </summary>
/// <remarks>
/// <para>
/// This plugin is secure by default. <see cref="AllowedDirectories"/> must be explicitly configured
/// before any file operations are permitted. By default, all file paths are denied.
/// </para>
/// <para>
/// When exposing this plugin to an LLM via auto function calling, ensure that
/// <see cref="AllowedDirectories"/> is restricted to trusted values only.
/// </para>
/// </remarks>
public sealed class DocumentPlugin
{
private readonly IDocumentConnector _documentConnector;
Expand All @@ -54,6 +65,20 @@ public DocumentPlugin(IDocumentConnector documentConnector, IFileSystemConnector
this._logger = loggerFactory?.CreateLogger(typeof(DocumentPlugin)) ?? NullLogger.Instance;
}

/// <summary>
/// List of allowed directories for file operations. Subdirectories of allowed directories are also permitted.
/// </summary>
/// <remarks>
/// Defaults to an empty collection (no directories allowed). Must be explicitly populated
/// with trusted directory paths before any file operations will succeed.
/// Paths are canonicalized before validation to prevent directory traversal.
/// </remarks>
public IEnumerable<string>? AllowedDirectories
{
get => this._allowedDirectories;
set => this._allowedDirectories = value is null ? null : new HashSet<string>(value, StringComparer.OrdinalIgnoreCase);
}

/// <summary>
/// Read all text from a document, using the filePath argument as the file path.
/// </summary>
Expand All @@ -63,6 +88,12 @@ public async Task<string> ReadTextAsync(
CancellationToken cancellationToken = default)
{
this._logger.LogDebug("Reading text from {0}", filePath);

if (!this.IsFilePathAllowed(filePath))
{
throw new InvalidOperationException("Reading from the provided location is not allowed.");
}

using var stream = await this._fileSystemConnector.GetFileContentStreamAsync(filePath, cancellationToken).ConfigureAwait(false);
return this._documentConnector.ReadText(stream);
}
Expand All @@ -76,9 +107,9 @@ public async Task AppendTextAsync(
[Description("Destination file path")] string filePath,
CancellationToken cancellationToken = default)
{
if (string.IsNullOrWhiteSpace(filePath))
if (!this.IsFilePathAllowed(filePath))
{
throw new ArgumentException("Variable was null or whitespace", nameof(filePath));
throw new InvalidOperationException("Writing to the provided location is not allowed.");
}

// If the document already exists, open it. If not, create it.
Expand All @@ -98,4 +129,55 @@ public async Task AppendTextAsync(
this._documentConnector.AppendText(stream, text);
}
}

#region private
private HashSet<string>? _allowedDirectories = [];

/// <summary>
/// If a list of allowed directories has been provided, the directory of the provided filePath is checked
/// to verify it is in the allowed directory list. Paths are canonicalized before comparison.
/// Subdirectories of allowed directories are also permitted.
/// </summary>
private bool IsFilePathAllowed(string path)
{
Verify.NotNullOrWhiteSpace(path);

if (path.StartsWith("\\\\", StringComparison.OrdinalIgnoreCase))
{
throw new ArgumentException("Invalid file path, UNC paths are not supported.", nameof(path));
}

string? directoryPath = Path.GetDirectoryName(path);

if (string.IsNullOrEmpty(directoryPath))
{
throw new ArgumentException("Invalid file path, a fully qualified file location must be specified.", nameof(path));
}

if (this._allowedDirectories is null || this._allowedDirectories.Count == 0)
{
return false;
}

var canonicalDir = Path.GetFullPath(directoryPath);

foreach (var allowedDirectory in this._allowedDirectories)
{
var canonicalAllowed = Path.GetFullPath(allowedDirectory);
var separator = Path.DirectorySeparatorChar.ToString();
if (!canonicalAllowed.EndsWith(separator, StringComparison.OrdinalIgnoreCase))
{
canonicalAllowed += separator;
}

if (canonicalDir.StartsWith(canonicalAllowed, StringComparison.OrdinalIgnoreCase)
|| (canonicalDir + separator).Equals(canonicalAllowed, StringComparison.OrdinalIgnoreCase))
{
return true;
}
}

return false;
}
#endregion
}
137 changes: 131 additions & 6 deletions dotnet/src/Plugins/Plugins.UnitTests/Document/DocumentPluginTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,8 @@ public async Task ReadTextAsyncSucceedsAsync()
{
// Arrange
var expectedText = Guid.NewGuid().ToString();
var anyFilePath = Guid.NewGuid().ToString();
var folderPath = Path.GetTempPath();
var anyFilePath = Path.Combine(folderPath, Guid.NewGuid().ToString());

var fileSystemConnectorMock = new Mock<IFileSystemConnector>();
fileSystemConnectorMock
Expand All @@ -31,7 +32,10 @@ public async Task ReadTextAsyncSucceedsAsync()
.Setup(mock => mock.ReadText(It.IsAny<Stream>()))
.Returns(expectedText);

var target = new DocumentPlugin(documentConnectorMock.Object, fileSystemConnectorMock.Object);
var target = new DocumentPlugin(documentConnectorMock.Object, fileSystemConnectorMock.Object)
{
AllowedDirectories = [folderPath]
};

// Act
string actual = await target.ReadTextAsync(anyFilePath);
Expand All @@ -47,7 +51,8 @@ public async Task AppendTextAsyncFileExistsSucceedsAsync()
{
// Arrange
var anyText = Guid.NewGuid().ToString();
var anyFilePath = Guid.NewGuid().ToString();
var folderPath = Path.GetTempPath();
var anyFilePath = Path.Combine(folderPath, Guid.NewGuid().ToString());

var fileSystemConnectorMock = new Mock<IFileSystemConnector>();
fileSystemConnectorMock
Expand All @@ -63,7 +68,10 @@ public async Task AppendTextAsyncFileExistsSucceedsAsync()
documentConnectorMock
.Setup(mock => mock.AppendText(It.IsAny<Stream>(), It.Is<string>(text => text.Equals(anyText, StringComparison.Ordinal))));

var target = new DocumentPlugin(documentConnectorMock.Object, fileSystemConnectorMock.Object);
var target = new DocumentPlugin(documentConnectorMock.Object, fileSystemConnectorMock.Object)
{
AllowedDirectories = [folderPath]
};

// Act
await target.AppendTextAsync(anyText, anyFilePath);
Expand All @@ -78,7 +86,8 @@ public async Task AppendTextAsyncFileDoesNotExistSucceedsAsync()
{
// Arrange
var anyText = Guid.NewGuid().ToString();
var anyFilePath = Guid.NewGuid().ToString();
var folderPath = Path.GetTempPath();
var anyFilePath = Path.Combine(folderPath, Guid.NewGuid().ToString());

var fileSystemConnectorMock = new Mock<IFileSystemConnector>();
fileSystemConnectorMock
Expand All @@ -96,7 +105,10 @@ public async Task AppendTextAsyncFileDoesNotExistSucceedsAsync()
documentConnectorMock
.Setup(mock => mock.AppendText(It.IsAny<Stream>(), It.Is<string>(text => text.Equals(anyText, StringComparison.Ordinal))));

var target = new DocumentPlugin(documentConnectorMock.Object, fileSystemConnectorMock.Object);
var target = new DocumentPlugin(documentConnectorMock.Object, fileSystemConnectorMock.Object)
{
AllowedDirectories = [folderPath]
};

// Act
await target.AppendTextAsync(anyText, anyFilePath);
Expand Down Expand Up @@ -125,4 +137,117 @@ await Assert.ThrowsAnyAsync<ArgumentException>(() =>
fileSystemConnectorMock.Verify(mock => mock.GetWriteableFileStreamAsync(It.IsAny<string>(), It.IsAny<CancellationToken>()), Times.Never());
documentConnectorMock.Verify(mock => mock.AppendText(It.IsAny<Stream>(), It.IsAny<string>()), Times.Never());
}

[Fact]
public async Task ItDeniesAllPathsWithDefaultConfigAsync()
{
// Arrange
var fileSystemConnectorMock = new Mock<IFileSystemConnector>();
var documentConnectorMock = new Mock<IDocumentConnector>();
var target = new DocumentPlugin(documentConnectorMock.Object, fileSystemConnectorMock.Object);

var filePath = Path.Combine(Path.GetTempPath(), "test.docx");

// Act & Assert — default config denies all paths
await Assert.ThrowsAsync<InvalidOperationException>(async () => await target.ReadTextAsync(filePath));
await Assert.ThrowsAsync<InvalidOperationException>(async () => await target.AppendTextAsync("text", filePath));
}

[Fact]
public async Task ItDeniesPathTraversalAsync()
{
// Arrange
var folderPath = Path.Combine(Path.GetTempPath(), "allowed-folder");
var traversalPath = Path.Combine(folderPath, "..", "outside-folder", "secret.docx");

var fileSystemConnectorMock = new Mock<IFileSystemConnector>();
var documentConnectorMock = new Mock<IDocumentConnector>();
var target = new DocumentPlugin(documentConnectorMock.Object, fileSystemConnectorMock.Object)
{
AllowedDirectories = [folderPath]
};

// Act & Assert — traversal path is canonicalized and rejected
await Assert.ThrowsAsync<InvalidOperationException>(async () => await target.ReadTextAsync(traversalPath));
await Assert.ThrowsAsync<InvalidOperationException>(async () => await target.AppendTextAsync("text", traversalPath));
}

[Fact]
public async Task ItDeniesUncPathsAsync()
{
// Arrange
var fileSystemConnectorMock = new Mock<IFileSystemConnector>();
var documentConnectorMock = new Mock<IDocumentConnector>();
var target = new DocumentPlugin(documentConnectorMock.Object, fileSystemConnectorMock.Object)
{
AllowedDirectories = [Path.GetTempPath()]
};

// Act & Assert — UNC paths are rejected
await Assert.ThrowsAnyAsync<Exception>(async () => await target.ReadTextAsync("\\\\UNC\\server\\folder\\file.docx"));
await Assert.ThrowsAnyAsync<Exception>(async () => await target.AppendTextAsync("text", "\\\\UNC\\server\\folder\\file.docx"));
}

[Fact]
public async Task ItDeniesDisallowedFoldersAsync()
{
// Arrange
var allowedFolder = Path.Combine(Path.GetTempPath(), "allowed");
var disallowedPath = Path.Combine(Path.GetTempPath(), "disallowed", "file.docx");

var fileSystemConnectorMock = new Mock<IFileSystemConnector>();
var documentConnectorMock = new Mock<IDocumentConnector>();
var target = new DocumentPlugin(documentConnectorMock.Object, fileSystemConnectorMock.Object)
{
AllowedDirectories = [allowedFolder]
};

// Act & Assert
await Assert.ThrowsAsync<InvalidOperationException>(async () => await target.ReadTextAsync(disallowedPath));
await Assert.ThrowsAsync<InvalidOperationException>(async () => await target.AppendTextAsync("text", disallowedPath));
}

[Fact]
public async Task ItAllowsSubdirectoriesOfAllowedFoldersAsync()
{
// Arrange
var allowedFolder = Path.GetTempPath();
var subDirPath = Path.Combine(allowedFolder, "subdir", "nested", "file.docx");

var fileSystemConnectorMock = new Mock<IFileSystemConnector>();
fileSystemConnectorMock
.Setup(mock => mock.GetFileContentStreamAsync(It.IsAny<string>(), It.IsAny<CancellationToken>()))
.ReturnsAsync(Stream.Null);

var documentConnectorMock = new Mock<IDocumentConnector>();
documentConnectorMock
.Setup(mock => mock.ReadText(It.IsAny<Stream>()))
.Returns("content");

var target = new DocumentPlugin(documentConnectorMock.Object, fileSystemConnectorMock.Object)
{
AllowedDirectories = [allowedFolder]
};

// Act — subdirectory of allowed folder should succeed
string result = await target.ReadTextAsync(subDirPath);

// Assert
Assert.Equal("content", result);
}

[Fact]
public async Task ItDeniesRelativePathsAsync()
{
// Arrange
var fileSystemConnectorMock = new Mock<IFileSystemConnector>();
var documentConnectorMock = new Mock<IDocumentConnector>();
var target = new DocumentPlugin(documentConnectorMock.Object, fileSystemConnectorMock.Object)
{
AllowedDirectories = [Path.GetTempPath()]
};

// Act & Assert — relative paths are caught by the "fully qualified" check
await Assert.ThrowsAsync<ArgumentException>(async () => await target.ReadTextAsync("myfile.docx"));
}
}
Loading