.Net: Harden FileIOPlugin UNC path rejection to cover all separator forms - #14168
.Net: Harden FileIOPlugin UNC path rejection to cover all separator forms#14168rogerbarreto wants to merge 2 commits into
Conversation
…orms Align FileIOPlugin path validation with the sibling file plugins by rejecting any path that begins with two directory separators, in any combination of backslash and forward slash. Previously only the uniform backslash form was rejected, so forward slash and mixed separator forms could reach path canonicalization before the allowed folder check ran. Adds a post canonicalization re check and unit tests covering all separator forms for read and write.
GitHub Actions
Failure
There was a problem hiding this comment.
Pull request overview
This PR hardens FileIOPlugin path validation by improving UNC/extended-path rejection so potentially dangerous network/device roots are rejected before and after canonicalization, aligning behavior with other file-related plugins.
Changes:
- Replaced the pre-canonicalization
StartsWith("\\\\")guard with anIsUncOrExtendedPathhelper. - Added a post-canonicalization UNC/device-root re-check.
- Added new unit tests covering multiple UNC separator combinations for both read and write.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| dotnet/src/Plugins/Plugins.Core/FileIOPlugin.cs | Introduces UNC/extended-path detection helper and adds a second validation pass after canonicalization. |
| dotnet/src/Plugins/Plugins.UnitTests/Core/FileIOPluginTests.cs | Adds theory tests asserting UNC paths are rejected for both ReadAsync and WriteAsync. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| private static bool IsDirectorySeparator(char c) | ||
| => c == Path.DirectorySeparatorChar || c == Path.AltDirectorySeparatorChar; |
There was a problem hiding this comment.
Good catch. Path.DirectorySeparatorChar same as slash on non-Windows, so backslash forms slip on Linux. Now check backslash and slash chars direct, no OS depend. Fixed in 4dddc48.
| var plugin = new FileIOPlugin() { AllowedFolders = [Path.GetTempPath()] }; | ||
|
|
||
| // Act & Assert | ||
| await Assert.ThrowsAsync<ArgumentException>(() => plugin.ReadAsync(uncPath)); |
There was a problem hiding this comment.
True. Test only check type, could pass wrong reason. Now assert message say UNC paths not supported, so only UNC guard make test pass. Fixed in 4dddc48.
| }; | ||
|
|
||
| // Act & Assert | ||
| await Assert.ThrowsAsync<ArgumentException>(() => plugin.WriteAsync(uncPath, "hello world")); |
There was a problem hiding this comment.
Same fix. Test now assert UNC guard message, not just exception type. Fixed in 4dddc48.
…ard message Check for backslash and forward slash characters explicitly instead of relying on Path.DirectorySeparatorChar, which resolves to the same character on non-Windows and would miss backslash forms there. Strengthen the UNC tests to assert the rejection message so they fail if a different validation path is what throws.
GitHub Actions
Failure
Motivation and Context
FileIOPluginrestricts file operations to a configured set of allowed folders. Before validating against that allow list, the path is canonicalized. The pre-canonicalization guard rejected only the uniform backslash UNC form (\\), so other equivalent forms that Windows resolves to a UNC/device root could reach path canonicalization before the allowed folder check ran.The sibling file plugins (
DocumentPlugin,WebFileDownloadPlugin,CloudDrivePlugin) already reject both slash forms. This change alignsFileIOPluginwith them and makes the guard robust to every separator combination.Description
StartsWith("\\")guard with anIsUncOrExtendedPathhelper that flags any path whose first two characters are both directory separators, in any combination of\and/(covers\\,//,\/,/\, and extended/device prefixes like\\?\and\\.\).Validation
FileIOPluginTests: 21/21 pass.Plugins.UnitTestssuite: 403 pass, no regressions.dotnet format --verify-no-changesclean on the changed projects.Contribution Checklist