From ef152559b20ad4bfee32f13ff8e150858eada85f Mon Sep 17 00:00:00 2001 From: SergeyMenshykh Date: Tue, 5 May 2026 10:31:43 +0100 Subject: [PATCH 1/7] Add deny-by-default AllowedUploadDirectories to CloudDrivePlugin - Add AllowedUploadDirectories property (defaults to empty = deny-all) - Validate local file path against allowlist before uploading - Canonicalize paths with env var expansion, UNC rejection, and traversal protection - Add unit tests for security behavior Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../Plugins.MsGraph/CloudDrivePlugin.cs | 86 ++++++++++++- .../MsGraph/CloudDrivePluginTests.cs | 116 +++++++++++++++++- 2 files changed, 198 insertions(+), 4 deletions(-) diff --git a/dotnet/src/Plugins/Plugins.MsGraph/CloudDrivePlugin.cs b/dotnet/src/Plugins/Plugins.MsGraph/CloudDrivePlugin.cs index de8660092fe4..b5b28e770eb4 100644 --- a/dotnet/src/Plugins/Plugins.MsGraph/CloudDrivePlugin.cs +++ b/dotnet/src/Plugins/Plugins.MsGraph/CloudDrivePlugin.cs @@ -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; @@ -14,10 +15,21 @@ namespace Microsoft.SemanticKernel.Plugins.MsGraph; /// /// Cloud drive plugin (e.g. OneDrive). /// +/// +/// +/// This plugin is secure by default. must be explicitly configured +/// before any file upload operations are permitted. By default, all local file paths are denied. +/// +/// +/// When exposing this plugin to an LLM via auto function calling, ensure that +/// is restricted to trusted values only. +/// +/// public sealed class CloudDrivePlugin { private readonly ICloudDriveConnector _connector; private readonly ILogger _logger; + private HashSet? _allowedUploadDirectories = []; /// /// Initializes a new instance of the class. @@ -32,6 +44,20 @@ public CloudDrivePlugin(ICloudDriveConnector connector, ILoggerFactory? loggerFa this._logger = loggerFactory?.CreateLogger(typeof(CloudDrivePlugin)) ?? NullLogger.Instance; } + /// + /// List of allowed local directories from which files may be uploaded. Subdirectories of allowed directories are also permitted. + /// + /// + /// Defaults to an empty collection (no directories allowed). Must be explicitly populated + /// with trusted directory paths before any file upload operations will succeed. + /// Paths are canonicalized before validation to prevent directory traversal. + /// + public IEnumerable? AllowedUploadDirectories + { + get => this._allowedUploadDirectories; + set => this._allowedUploadDirectories = value is null ? null : new HashSet(value, StringComparer.OrdinalIgnoreCase); + } + /// /// Get the contents of a file stored in a cloud drive. /// @@ -77,10 +103,17 @@ public async Task UploadFileAsync( throw new ArgumentException("Variable was null or whitespace", nameof(destinationPath)); } - this._logger.LogDebug("Uploading file '{0}'", filePath); + var canonicalPath = Path.GetFullPath(Environment.ExpandEnvironmentVariables(filePath)); + + if (!this.IsUploadPathAllowed(canonicalPath)) + { + throw new InvalidOperationException("Uploading from the provided location is not allowed."); + } + + this._logger.LogDebug("Uploading file '{0}'", canonicalPath); // TODO Add support for large file uploads (i.e. upload sessions) - await this._connector.UploadSmallFileAsync(filePath, destinationPath, cancellationToken).ConfigureAwait(false); + await this._connector.UploadSmallFileAsync(canonicalPath, destinationPath, cancellationToken).ConfigureAwait(false); } /// @@ -100,4 +133,53 @@ public async Task CreateLinkAsync( return await this._connector.CreateShareLinkAsync(filePath, Type, Scope, cancellationToken).ConfigureAwait(false); } + + #region private + /// + /// If a list of allowed upload 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. + /// + private bool IsUploadPathAllowed(string path) + { + Ensure.NotNullOrWhitespace(path, nameof(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._allowedUploadDirectories is null || this._allowedUploadDirectories.Count == 0) + { + return false; + } + + var canonicalDir = Path.GetFullPath(directoryPath); + + foreach (var allowedDirectory in this._allowedUploadDirectories) + { + 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 } diff --git a/dotnet/src/Plugins/Plugins.UnitTests/MsGraph/CloudDrivePluginTests.cs b/dotnet/src/Plugins/Plugins.UnitTests/MsGraph/CloudDrivePluginTests.cs index ee15a1a92725..2168d9706fe2 100644 --- a/dotnet/src/Plugins/Plugins.UnitTests/MsGraph/CloudDrivePluginTests.cs +++ b/dotnet/src/Plugins/Plugins.UnitTests/MsGraph/CloudDrivePluginTests.cs @@ -17,13 +17,14 @@ public class CloudDrivePluginTests public async Task UploadSmallFileAsyncSucceedsAsync() { // Arrange - string anyFilePath = Guid.NewGuid().ToString(); + string allowedDir = Path.GetTempPath(); + string anyFilePath = Path.Combine(allowedDir, Guid.NewGuid().ToString()); Mock connectorMock = new(); connectorMock.Setup(c => c.UploadSmallFileAsync(It.IsAny(), It.IsAny(), It.IsAny())) .Returns(Task.CompletedTask); - CloudDrivePlugin target = new(connectorMock.Object); + CloudDrivePlugin target = new(connectorMock.Object) { AllowedUploadDirectories = [allowedDir] }; // Act await target.UploadFileAsync(anyFilePath, Guid.NewGuid().ToString()); @@ -74,4 +75,115 @@ public async Task GetFileContentAsyncSucceedsAsync() Assert.Equal(expectedContent, actual); connectorMock.VerifyAll(); } + + [Fact] + public async Task ItDeniesAllPathsByDefaultAsync() + { + // Arrange + string filePath = Path.Combine(Path.GetTempPath(), "somefile.txt"); + + Mock connectorMock = new(); + CloudDrivePlugin target = new(connectorMock.Object); + + // Act & Assert — default config denies all paths + await Assert.ThrowsAsync(async () => + await target.UploadFileAsync(filePath, "/remote.txt")); + } + + [Fact] + public async Task ItDeniesPathTraversalAsync() + { + // Arrange + var allowedDir = Path.Combine(Path.GetTempPath(), "allowed-folder"); + var traversalPath = Path.Combine(allowedDir, "..", "outside-folder", "secret.txt"); + + Mock connectorMock = new(); + CloudDrivePlugin target = new(connectorMock.Object) { AllowedUploadDirectories = [allowedDir] }; + + // Act & Assert — traversal path is canonicalized and rejected + await Assert.ThrowsAsync(async () => + await target.UploadFileAsync(traversalPath, "/remote.txt")); + } + + [Fact] + public async Task ItDeniesUncPathsAsync() + { + // Arrange + Mock connectorMock = new(); + CloudDrivePlugin target = new(connectorMock.Object) { AllowedUploadDirectories = [Path.GetTempPath()] }; + + // Act & Assert — UNC paths are rejected + await Assert.ThrowsAnyAsync(async () => + await target.UploadFileAsync("\\\\UNC\\server\\folder\\file.txt", "/remote.txt")); + } + + [Fact] + public async Task ItDeniesDisallowedDirectoriesAsync() + { + // Arrange + var allowedDir = Path.Combine(Path.GetTempPath(), "allowed"); + var disallowedPath = Path.Combine(Path.GetTempPath(), "disallowed", "file.txt"); + + Mock connectorMock = new(); + CloudDrivePlugin target = new(connectorMock.Object) { AllowedUploadDirectories = [allowedDir] }; + + // Act & Assert + await Assert.ThrowsAsync(async () => + await target.UploadFileAsync(disallowedPath, "/remote.txt")); + } + + [Fact] + public async Task ItAllowsSubdirectoriesOfAllowedDirectoriesAsync() + { + // Arrange + var allowedDir = Path.GetTempPath(); + var subDirPath = Path.Combine(allowedDir, "subdir", "nested", "file.txt"); + + Mock connectorMock = new(); + connectorMock.Setup(c => c.UploadSmallFileAsync(It.IsAny(), It.IsAny(), It.IsAny())) + .Returns(Task.CompletedTask); + + CloudDrivePlugin target = new(connectorMock.Object) { AllowedUploadDirectories = [allowedDir] }; + + // Act — subdirectory of allowed folder should succeed + await target.UploadFileAsync(subDirPath, "/remote.txt"); + + // Assert + connectorMock.VerifyAll(); + } + + [Fact] + public async Task ItExpandsEnvironmentVariablesAndValidatesAsync() + { + // Arrange — use %TEMP% which should expand to the temp directory + var tempDir = Path.GetTempPath().TrimEnd(Path.DirectorySeparatorChar); + var envVarPath = Path.Combine("%TEMP%", "testfile.txt"); + + Mock connectorMock = new(); + connectorMock.Setup(c => c.UploadSmallFileAsync(It.IsAny(), It.IsAny(), It.IsAny())) + .Returns(Task.CompletedTask); + + CloudDrivePlugin target = new(connectorMock.Object) { AllowedUploadDirectories = [tempDir] }; + + // Act — env var should be expanded and path should be allowed + await target.UploadFileAsync(envVarPath, "/remote.txt"); + + // Assert + connectorMock.VerifyAll(); + } + + [Fact] + public async Task ItDeniesExpandedEnvironmentVariablePathsOutsideAllowedAsync() + { + // Arrange — allow a specific subdirectory, but env var expands outside it + var allowedDir = Path.Combine(Path.GetTempPath(), "specific-allowed"); + var envVarPath = Path.Combine("%TEMP%", "outside-file.txt"); + + Mock connectorMock = new(); + CloudDrivePlugin target = new(connectorMock.Object) { AllowedUploadDirectories = [allowedDir] }; + + // Act & Assert — expanded path is outside allowed directory + await Assert.ThrowsAsync(async () => + await target.UploadFileAsync(envVarPath, "/remote.txt")); + } } From 7c519b95360419ef136ddbd6d7d20664c31fd597 Mon Sep 17 00:00:00 2001 From: SergeyMenshykh Date: Tue, 5 May 2026 10:47:20 +0100 Subject: [PATCH 2/7] Use platform-aware path comparison in IsUploadPathAllowed Use OrdinalIgnoreCase on Windows and Ordinal on Linux/macOS to respect case-sensitive file systems. Add test for platform-aware case sensitivity behavior. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../Plugins.MsGraph/CloudDrivePlugin.cs | 13 +++++++-- .../MsGraph/CloudDrivePluginTests.cs | 28 +++++++++++++++++++ 2 files changed, 38 insertions(+), 3 deletions(-) diff --git a/dotnet/src/Plugins/Plugins.MsGraph/CloudDrivePlugin.cs b/dotnet/src/Plugins/Plugins.MsGraph/CloudDrivePlugin.cs index b5b28e770eb4..36174ca2acae 100644 --- a/dotnet/src/Plugins/Plugins.MsGraph/CloudDrivePlugin.cs +++ b/dotnet/src/Plugins/Plugins.MsGraph/CloudDrivePlugin.cs @@ -4,6 +4,7 @@ using System.Collections.Generic; using System.ComponentModel; using System.IO; +using System.Runtime.InteropServices; using System.Threading; using System.Threading.Tasks; using Microsoft.Extensions.Logging; @@ -135,6 +136,12 @@ public async Task CreateLinkAsync( } #region private + // Use case-insensitive comparison on Windows (case-insensitive FS), case-sensitive on Linux/macOS. + private static readonly StringComparison s_pathComparison = + RuntimeInformation.IsOSPlatform(OSPlatform.Windows) + ? StringComparison.OrdinalIgnoreCase + : StringComparison.Ordinal; + /// /// If a list of allowed upload 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. @@ -167,13 +174,13 @@ private bool IsUploadPathAllowed(string path) { var canonicalAllowed = Path.GetFullPath(allowedDirectory); var separator = Path.DirectorySeparatorChar.ToString(); - if (!canonicalAllowed.EndsWith(separator, StringComparison.OrdinalIgnoreCase)) + if (!canonicalAllowed.EndsWith(separator, s_pathComparison)) { canonicalAllowed += separator; } - if (canonicalDir.StartsWith(canonicalAllowed, StringComparison.OrdinalIgnoreCase) - || (canonicalDir + separator).Equals(canonicalAllowed, StringComparison.OrdinalIgnoreCase)) + if (canonicalDir.StartsWith(canonicalAllowed, s_pathComparison) + || (canonicalDir + separator).Equals(canonicalAllowed, s_pathComparison)) { return true; } diff --git a/dotnet/src/Plugins/Plugins.UnitTests/MsGraph/CloudDrivePluginTests.cs b/dotnet/src/Plugins/Plugins.UnitTests/MsGraph/CloudDrivePluginTests.cs index 2168d9706fe2..32b25d67cb0e 100644 --- a/dotnet/src/Plugins/Plugins.UnitTests/MsGraph/CloudDrivePluginTests.cs +++ b/dotnet/src/Plugins/Plugins.UnitTests/MsGraph/CloudDrivePluginTests.cs @@ -2,6 +2,7 @@ using System; using System.IO; +using System.Runtime.InteropServices; using System.Text; using System.Threading; using System.Threading.Tasks; @@ -186,4 +187,31 @@ public async Task ItDeniesExpandedEnvironmentVariablePathsOutsideAllowedAsync() await Assert.ThrowsAsync(async () => await target.UploadFileAsync(envVarPath, "/remote.txt")); } + + [Fact] + public async Task ItRespectsPlatformCaseSensitivityAsync() + { + // Arrange — use differently-cased allowed dir vs file path + var allowedDir = Path.Combine(Path.GetTempPath(), "AllowedFolder"); + var filePath = Path.Combine(Path.GetTempPath(), "allowedfolder", "file.txt"); + + Mock connectorMock = new(); + connectorMock.Setup(c => c.UploadSmallFileAsync(It.IsAny(), It.IsAny(), It.IsAny())) + .Returns(Task.CompletedTask); + + CloudDrivePlugin target = new(connectorMock.Object) { AllowedUploadDirectories = [allowedDir] }; + + if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) + { + // Windows: case-insensitive FS — differently-cased path should be allowed + await target.UploadFileAsync(filePath, "/remote.txt"); + connectorMock.VerifyAll(); + } + else + { + // Linux/macOS: case-sensitive FS — differently-cased path should be denied + await Assert.ThrowsAsync(async () => + await target.UploadFileAsync(filePath, "/remote.txt")); + } + } } From 838742d2ac915c4138bb75b25dd25c2fc381751e Mon Sep 17 00:00:00 2001 From: SergeyMenshykh Date: Tue, 5 May 2026 11:55:10 +0100 Subject: [PATCH 3/7] Add filePath null guard and improve error message in UploadFileAsync Validate filePath before canonicalization to provide a clear error. Include AllowedUploadDirectories remediation hint in deny message. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- dotnet/src/Plugins/Plugins.MsGraph/CloudDrivePlugin.cs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/dotnet/src/Plugins/Plugins.MsGraph/CloudDrivePlugin.cs b/dotnet/src/Plugins/Plugins.MsGraph/CloudDrivePlugin.cs index 36174ca2acae..40cd162350ad 100644 --- a/dotnet/src/Plugins/Plugins.MsGraph/CloudDrivePlugin.cs +++ b/dotnet/src/Plugins/Plugins.MsGraph/CloudDrivePlugin.cs @@ -104,11 +104,13 @@ public async Task UploadFileAsync( throw new ArgumentException("Variable was null or whitespace", nameof(destinationPath)); } + Ensure.NotNullOrWhitespace(filePath, nameof(filePath)); + var canonicalPath = Path.GetFullPath(Environment.ExpandEnvironmentVariables(filePath)); if (!this.IsUploadPathAllowed(canonicalPath)) { - throw new InvalidOperationException("Uploading from the provided location is not allowed."); + throw new InvalidOperationException("Uploading from the provided location is not allowed. Configure 'AllowedUploadDirectories' with trusted directory paths to enable uploads."); } this._logger.LogDebug("Uploading file '{0}'", canonicalPath); From fd3d6216c025cfc4539deb1cae16db7b13b1677f Mon Sep 17 00:00:00 2001 From: SergeyMenshykh Date: Tue, 5 May 2026 12:00:05 +0100 Subject: [PATCH 4/7] make AllowedUploadDirectories not nullable --- dotnet/src/Plugins/Plugins.MsGraph/CloudDrivePlugin.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/dotnet/src/Plugins/Plugins.MsGraph/CloudDrivePlugin.cs b/dotnet/src/Plugins/Plugins.MsGraph/CloudDrivePlugin.cs index 40cd162350ad..95b05a641e3d 100644 --- a/dotnet/src/Plugins/Plugins.MsGraph/CloudDrivePlugin.cs +++ b/dotnet/src/Plugins/Plugins.MsGraph/CloudDrivePlugin.cs @@ -53,10 +53,10 @@ public CloudDrivePlugin(ICloudDriveConnector connector, ILoggerFactory? loggerFa /// with trusted directory paths before any file upload operations will succeed. /// Paths are canonicalized before validation to prevent directory traversal. /// - public IEnumerable? AllowedUploadDirectories + public IEnumerable AllowedUploadDirectories { get => this._allowedUploadDirectories; - set => this._allowedUploadDirectories = value is null ? null : new HashSet(value, StringComparer.OrdinalIgnoreCase); + set => this._allowedUploadDirectories = value is null ? [] : new HashSet(value, StringComparer.OrdinalIgnoreCase); } /// @@ -165,7 +165,7 @@ private bool IsUploadPathAllowed(string path) throw new ArgumentException("Invalid file path, a fully qualified file location must be specified.", nameof(path)); } - if (this._allowedUploadDirectories is null || this._allowedUploadDirectories.Count == 0) + if (this._allowedUploadDirectories.Count == 0) { return false; } From 55ab3a31f45d261ed738839f9aee9077afb04e43 Mon Sep 17 00:00:00 2001 From: SergeyMenshykh Date: Tue, 5 May 2026 12:35:00 +0100 Subject: [PATCH 5/7] Make AllowedUploadDirectories non-nullable, improve error message, add filePath guard - Make AllowedUploadDirectories non-nullable; null assignments treated as empty (deny-all) - Add filePath null/whitespace guard before canonicalization - Include AllowedUploadDirectories hint in deny error message Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- dotnet/src/Plugins/Plugins.MsGraph/CloudDrivePlugin.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dotnet/src/Plugins/Plugins.MsGraph/CloudDrivePlugin.cs b/dotnet/src/Plugins/Plugins.MsGraph/CloudDrivePlugin.cs index 95b05a641e3d..374bdd4ef1de 100644 --- a/dotnet/src/Plugins/Plugins.MsGraph/CloudDrivePlugin.cs +++ b/dotnet/src/Plugins/Plugins.MsGraph/CloudDrivePlugin.cs @@ -30,7 +30,7 @@ public sealed class CloudDrivePlugin { private readonly ICloudDriveConnector _connector; private readonly ILogger _logger; - private HashSet? _allowedUploadDirectories = []; + private HashSet _allowedUploadDirectories = []; /// /// Initializes a new instance of the class. From 1bdcac7bce350c2ac3fc65a2496261db6e0800fd Mon Sep 17 00:00:00 2001 From: SergeyMenshykh Date: Tue, 5 May 2026 13:02:57 +0100 Subject: [PATCH 6/7] Improve test robustness for cross-platform CI - Use specific ArgumentException assertion for UNC path test - Use dedicated test env var instead of platform-specific TEMP - Restore env var in finally blocks for test isolation Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../MsGraph/CloudDrivePluginTests.cs | 61 +++++++++++++------ 1 file changed, 41 insertions(+), 20 deletions(-) diff --git a/dotnet/src/Plugins/Plugins.UnitTests/MsGraph/CloudDrivePluginTests.cs b/dotnet/src/Plugins/Plugins.UnitTests/MsGraph/CloudDrivePluginTests.cs index 32b25d67cb0e..1daa08d2c50f 100644 --- a/dotnet/src/Plugins/Plugins.UnitTests/MsGraph/CloudDrivePluginTests.cs +++ b/dotnet/src/Plugins/Plugins.UnitTests/MsGraph/CloudDrivePluginTests.cs @@ -113,8 +113,8 @@ public async Task ItDeniesUncPathsAsync() Mock connectorMock = new(); CloudDrivePlugin target = new(connectorMock.Object) { AllowedUploadDirectories = [Path.GetTempPath()] }; - // Act & Assert — UNC paths are rejected - await Assert.ThrowsAnyAsync(async () => + // Act & Assert — UNC paths are rejected with ArgumentException + await Assert.ThrowsAsync(async () => await target.UploadFileAsync("\\\\UNC\\server\\folder\\file.txt", "/remote.txt")); } @@ -156,36 +156,57 @@ public async Task ItAllowsSubdirectoriesOfAllowedDirectoriesAsync() [Fact] public async Task ItExpandsEnvironmentVariablesAndValidatesAsync() { - // Arrange — use %TEMP% which should expand to the temp directory + // Arrange — set a dedicated test env var to avoid platform-specific assumptions var tempDir = Path.GetTempPath().TrimEnd(Path.DirectorySeparatorChar); - var envVarPath = Path.Combine("%TEMP%", "testfile.txt"); + var envVarName = "SK_TEST_UPLOAD_DIR"; + var originalValue = Environment.GetEnvironmentVariable(envVarName); + try + { + Environment.SetEnvironmentVariable(envVarName, tempDir); + var envVarPath = Path.Combine($"%{envVarName}%", "testfile.txt"); - Mock connectorMock = new(); - connectorMock.Setup(c => c.UploadSmallFileAsync(It.IsAny(), It.IsAny(), It.IsAny())) - .Returns(Task.CompletedTask); + Mock connectorMock = new(); + connectorMock.Setup(c => c.UploadSmallFileAsync(It.IsAny(), It.IsAny(), It.IsAny())) + .Returns(Task.CompletedTask); - CloudDrivePlugin target = new(connectorMock.Object) { AllowedUploadDirectories = [tempDir] }; + CloudDrivePlugin target = new(connectorMock.Object) { AllowedUploadDirectories = [tempDir] }; - // Act — env var should be expanded and path should be allowed - await target.UploadFileAsync(envVarPath, "/remote.txt"); + // Act — env var should be expanded and path should be allowed + await target.UploadFileAsync(envVarPath, "/remote.txt"); - // Assert - connectorMock.VerifyAll(); + // Assert + connectorMock.VerifyAll(); + } + finally + { + Environment.SetEnvironmentVariable(envVarName, originalValue); + } } [Fact] public async Task ItDeniesExpandedEnvironmentVariablePathsOutsideAllowedAsync() { - // Arrange — allow a specific subdirectory, but env var expands outside it - var allowedDir = Path.Combine(Path.GetTempPath(), "specific-allowed"); - var envVarPath = Path.Combine("%TEMP%", "outside-file.txt"); + // Arrange — set a dedicated test env var; allow a subdirectory but env var expands outside it + var tempDir = Path.GetTempPath().TrimEnd(Path.DirectorySeparatorChar); + var allowedDir = Path.Combine(tempDir, "specific-allowed"); + var envVarName = "SK_TEST_UPLOAD_DIR"; + var originalValue = Environment.GetEnvironmentVariable(envVarName); + try + { + Environment.SetEnvironmentVariable(envVarName, tempDir); + var envVarPath = Path.Combine($"%{envVarName}%", "outside-file.txt"); - Mock connectorMock = new(); - CloudDrivePlugin target = new(connectorMock.Object) { AllowedUploadDirectories = [allowedDir] }; + Mock connectorMock = new(); + CloudDrivePlugin target = new(connectorMock.Object) { AllowedUploadDirectories = [allowedDir] }; - // Act & Assert — expanded path is outside allowed directory - await Assert.ThrowsAsync(async () => - await target.UploadFileAsync(envVarPath, "/remote.txt")); + // Act & Assert — expanded path is outside allowed directory + await Assert.ThrowsAsync(async () => + await target.UploadFileAsync(envVarPath, "/remote.txt")); + } + finally + { + Environment.SetEnvironmentVariable(envVarName, originalValue); + } } [Fact] From 226422b02aa11d8425d44253cec8d398ec7fc216 Mon Sep 17 00:00:00 2001 From: SergeyMenshykh Date: Tue, 5 May 2026 14:08:51 +0100 Subject: [PATCH 7/7] Fix UNC test for cross-platform: accept any exception type On Linux, UNC paths are canonicalized differently and hit the allowlist denial (InvalidOperationException) instead of the UNC check (ArgumentException). Both correctly deny the upload. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../Plugins.UnitTests/MsGraph/CloudDrivePluginTests.cs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/dotnet/src/Plugins/Plugins.UnitTests/MsGraph/CloudDrivePluginTests.cs b/dotnet/src/Plugins/Plugins.UnitTests/MsGraph/CloudDrivePluginTests.cs index 1daa08d2c50f..a6e34f14dbe2 100644 --- a/dotnet/src/Plugins/Plugins.UnitTests/MsGraph/CloudDrivePluginTests.cs +++ b/dotnet/src/Plugins/Plugins.UnitTests/MsGraph/CloudDrivePluginTests.cs @@ -113,8 +113,9 @@ public async Task ItDeniesUncPathsAsync() Mock connectorMock = new(); CloudDrivePlugin target = new(connectorMock.Object) { AllowedUploadDirectories = [Path.GetTempPath()] }; - // Act & Assert — UNC paths are rejected with ArgumentException - await Assert.ThrowsAsync(async () => + // Act & Assert — UNC paths are rejected (ArgumentException on Windows, InvalidOperationException on Linux + // where the path is canonicalized differently and fails the allowlist check instead) + await Assert.ThrowsAnyAsync(async () => await target.UploadFileAsync("\\\\UNC\\server\\folder\\file.txt", "/remote.txt")); }