agent/agent-framework
2
0
Fork 0
mirror of https://github.com/microsoft/agent-framework.git synced 2026-06-16 21:04:09 +08:00
Code Issues Packages Projects Releases Wiki Activity

.NET: Harness: Improve path validation (#5404)

Browse Source 
* Harness: Improve path validation

* Address PR comments
This commit is contained in:
westey
2026-04-22 10:28:37 +01:00
committed by GitHub
Unverified
parent 7f661e8524
commit 53274fde85
7 changed files with 313 additions and 137 deletions
Download Patch File Download Diff File Expand all files Collapse all files
dotnet/src/Microsoft.Agents.AI/Harness/FileMemory/AgentFileStore.cs
-35
View File
@@ -90,39 +90,4 @@ public abstract class AgentFileStore
/// <param name="cancellationToken">A token to cancel the operation.</param>
/// <returns>A task representing the asynchronous operation.</returns>
public abstract Task CreateDirectoryAsync(string path, CancellationToken cancellationToken = default);
/// <summary>
/// Creates a <see cref="Matcher"/> for the specified glob pattern. Use the returned instance
/// to test multiple file names without allocating a new matcher for each one.
/// </summary>
/// <param name="filePattern">
/// The glob pattern to match against (e.g., <c>"*.md"</c>, <c>"research*"</c>).
/// </param>
/// <returns>A <see cref="Matcher"/> configured with the specified pattern.</returns>
protected static Matcher CreateGlobMatcher(string filePattern)
{
var matcher = new Matcher(System.StringComparison.OrdinalIgnoreCase);
matcher.AddInclude(filePattern);
return matcher;
}
/// <summary>
/// Determines whether a file name matches a pre-built glob <see cref="Matcher"/>.
/// </summary>
/// <param name="fileName">The file name to test (not a full path — just the name).</param>
/// <param name="matcher">
/// A pre-built <see cref="Matcher"/> to test against.
/// When <see langword="null"/>, this method returns <see langword="true"/> for any file name.
/// </param>
/// <returns><see langword="true"/> if the file name matches the pattern or if the matcher is <see langword="null"/>; otherwise, <see langword="false"/>.</returns>
protected static bool MatchesGlob(string fileName, Matcher? matcher)
{
if (matcher is null)
{
return true;
}
PatternMatchingResult result = matcher.Match(fileName);
return result.HasMatches;
}
}
dotnet/src/Microsoft.Agents.AI/Harness/FileMemory/FileMemoryProvider.cs
+5 -20
View File
@@ -4,7 +4,6 @@ using System;
using System.Collections.Generic;
using System.ComponentModel;
using System.Diagnostics.CodeAnalysis;
using System.IO;
using System.Threading;
using System.Threading.Tasks;
using Microsoft.Extensions.AI;
@@ -264,26 +263,12 @@ public sealed class FileMemoryProvider : AIContextProvider
private static string ResolvePath(string workingFolder, string fileName)
{
// Prevent path traversal by rejecting rooted paths and '.'/'..' segments.
string normalized = fileName.Replace('\\', '/');
// Validate and normalize the file name (rejects rooted, traversal, empty, etc.).
// Only fileName needs validation — workingFolder is developer-provided and trusted.
string normalizedFileName = StorePaths.NormalizeRelativePath(fileName);
if (Path.IsPathRooted(fileName) ||
fileName.StartsWith("/", StringComparison.Ordinal) ||
fileName.StartsWith("\\", StringComparison.Ordinal) ||
(normalized.Length >= 2 && char.IsLetter(normalized[0]) && normalized[1] == ':'))
{
throw new ArgumentException($"Invalid file name: '{fileName}'. File names must be relative and must not start with '/', '\\', or a drive root.", nameof(fileName));
}
foreach (string segment in normalized.Split('/'))
{
if (segment.Equals(".", StringComparison.Ordinal) || segment.Equals("..", StringComparison.Ordinal))
{
throw new ArgumentException($"Invalid file name: '{fileName}'. File names must not contain '.' or '..' segments.", nameof(fileName));
}
}
return CombinePaths(workingFolder, fileName);
string normalizedWorkingFolder = workingFolder.Replace('\\', '/');
return CombinePaths(normalizedWorkingFolder, normalizedFileName);
}
private static string CombinePaths(string basePath, string relativePath)
dotnet/src/Microsoft.Agents.AI/Harness/FileMemory/FileSystemAgentFileStore.cs
+7 -43
View File
@@ -152,7 +152,7 @@ public sealed class FileSystemAgentFileStore : AgentFileStore
// Compile the regex with a timeout to guard against catastrophic backtracking (ReDoS).
var regex = new Regex(regexPattern, RegexOptions.IgnoreCase, TimeSpan.FromSeconds(5));
Matcher? matcher = filePattern is not null ? CreateGlobMatcher(filePattern) : null;
Matcher? matcher = filePattern is not null ? StorePaths.CreateGlobMatcher(filePattern) : null;
var results = new List<FileSearchResult>();
foreach (string filePath in Directory.GetFiles(fullDir))
@@ -164,7 +164,7 @@ public sealed class FileSystemAgentFileStore : AgentFileStore
}
// Apply the optional glob filter on the file name.
if (!MatchesGlob(fileName, matcher))
if (!StorePaths.MatchesGlob(fileName, matcher))
{
continue;
}
@@ -235,12 +235,12 @@ public sealed class FileSystemAgentFileStore : AgentFileStore
/// </summary>
private string ResolveSafePath(string relativePath)
{
ValidateRelativePath(relativePath);
// Normalize and validate the relative path (rejects rooted, traversal, etc.).
string normalized = StorePaths.NormalizeRelativePath(relativePath);
// Normalize separators before combining to prevent backslashes from becoming
// literal filename characters on Unix.
string normalized = relativePath.Replace('\\', '/').Replace('/', Path.DirectorySeparatorChar);
string combined = Path.Combine(this._rootPath, normalized);
// Convert to OS-native separators before combining.
string nativePath = normalized.Replace('/', Path.DirectorySeparatorChar);
string combined = Path.Combine(this._rootPath, nativePath);
string fullPath = Path.GetFullPath(combined);
if (!fullPath.StartsWith(this._rootPath, StringComparison.Ordinal))
@@ -266,40 +266,4 @@ public sealed class FileSystemAgentFileStore : AgentFileStore
return this.ResolveSafePath(relativeDirectory);
}
/// <summary>
/// Validates that a relative path does not contain rooted paths or traversal segments.
/// </summary>
private static void ValidateRelativePath(string path)
{
string normalized = path.Replace('\\', '/');
if (Path.IsPathRooted(path) ||
path.StartsWith("/", StringComparison.Ordinal) ||
path.StartsWith("\\", StringComparison.Ordinal) ||
(normalized.Length >= 2 && char.IsLetter(normalized[0]) && normalized[1] == ':'))
{
throw new ArgumentException(
$"Invalid path: '{path}'. Paths must be relative and must not start with '/', '\\', or a drive root.",
nameof(path));
}
foreach (string segment in normalized.Split('/'))
{
if (segment.Equals(".", StringComparison.Ordinal) || segment.Equals("..", StringComparison.Ordinal))
{
throw new ArgumentException(
$"Invalid path: '{path}'. Paths must not contain '.' or '..' segments.",
nameof(path));
}
}
if (normalized.StartsWith("/", StringComparison.Ordinal) ||
normalized.EndsWith("/", StringComparison.Ordinal))
{
throw new ArgumentException(
$"Invalid path: '{path}'. Paths must not start or end with a directory separator.",
nameof(path));
}
}
}
dotnet/src/Microsoft.Agents.AI/Harness/FileMemory/InMemoryAgentFileStore.cs
+8 -36
View File
@@ -4,7 +4,6 @@ using System;
using System.Collections.Concurrent;
using System.Collections.Generic;
using System.Diagnostics.CodeAnalysis;
using System.IO;
using System.Linq;
using System.Text.RegularExpressions;
using System.Threading;
@@ -29,7 +28,7 @@ public sealed class InMemoryAgentFileStore : AgentFileStore
/// <inheritdoc />
public override Task WriteFileAsync(string path, string content, CancellationToken cancellationToken = default)
{
path = NormalizePath(path);
path = StorePaths.NormalizeRelativePath(path);
this._files[path] = content;
return Task.CompletedTask;
}
@@ -37,7 +36,7 @@ public sealed class InMemoryAgentFileStore : AgentFileStore
/// <inheritdoc />
public override Task<string?> ReadFileAsync(string path, CancellationToken cancellationToken = default)
{
path = NormalizePath(path);
path = StorePaths.NormalizeRelativePath(path);
this._files.TryGetValue(path, out string? content);
return Task.FromResult(content);
}
@@ -45,14 +44,14 @@ public sealed class InMemoryAgentFileStore : AgentFileStore
/// <inheritdoc />
public override Task<bool> DeleteFileAsync(string path, CancellationToken cancellationToken = default)
{
path = NormalizePath(path);
path = StorePaths.NormalizeRelativePath(path);
return Task.FromResult(this._files.TryRemove(path, out _));
}
/// <inheritdoc />
public override Task<IReadOnlyList<string>> ListFilesAsync(string directory, CancellationToken cancellationToken = default)
{
string prefix = NormalizePath(directory);
string prefix = StorePaths.NormalizeRelativePath(directory, isDirectory: true);
if (prefix.Length > 0 && !prefix.EndsWith("/", StringComparison.Ordinal))
{
prefix += "/";
@@ -70,7 +69,7 @@ public sealed class InMemoryAgentFileStore : AgentFileStore
/// <inheritdoc />
public override Task<bool> FileExistsAsync(string path, CancellationToken cancellationToken = default)
{
path = NormalizePath(path);
path = StorePaths.NormalizeRelativePath(path);
return Task.FromResult(this._files.ContainsKey(path));
}
@@ -78,7 +77,7 @@ public sealed class InMemoryAgentFileStore : AgentFileStore
public override Task<IReadOnlyList<FileSearchResult>> SearchFilesAsync(string directory, string regexPattern, string? filePattern = null, CancellationToken cancellationToken = default)
{
// Normalize the directory prefix for path matching.
string prefix = NormalizePath(directory);
string prefix = StorePaths.NormalizeRelativePath(directory, isDirectory: true);
if (prefix.Length > 0 && !prefix.EndsWith("/", StringComparison.Ordinal))
{
prefix += "/";
@@ -86,7 +85,7 @@ public sealed class InMemoryAgentFileStore : AgentFileStore
// Compile the regex with a timeout to guard against catastrophic backtracking (ReDoS).
var regex = new Regex(regexPattern, RegexOptions.IgnoreCase, TimeSpan.FromSeconds(5));
Matcher? matcher = filePattern is not null ? CreateGlobMatcher(filePattern) : null;
Matcher? matcher = filePattern is not null ? StorePaths.CreateGlobMatcher(filePattern) : null;
var results = new List<FileSearchResult>();
foreach (var kvp in this._files)
@@ -105,7 +104,7 @@ public sealed class InMemoryAgentFileStore : AgentFileStore
}
// Apply the optional glob filter on the file name.
if (!MatchesGlob(relativeName, matcher))
if (!StorePaths.MatchesGlob(relativeName, matcher))
{
continue;
}
@@ -158,31 +157,4 @@ public sealed class InMemoryAgentFileStore : AgentFileStore
// No-op: directories are implicit from file paths in the in-memory store.
return Task.CompletedTask;
}
private static string NormalizePath(string path)
{
string normalized = path.Replace('\\', '/').Trim('/');
if (Path.IsPathRooted(path) ||
path.StartsWith("/", StringComparison.Ordinal) ||
path.StartsWith("\\", StringComparison.Ordinal) ||
(normalized.Length >= 2 && char.IsLetter(normalized[0]) && normalized[1] == ':'))
{
throw new ArgumentException(
$"Invalid path: '{path}'. Paths must be relative and must not start with '/', '\\', or a drive root.",
nameof(path));
}
foreach (string segment in normalized.Split('/'))
{
if (segment.Equals(".", StringComparison.Ordinal) || segment.Equals("..", StringComparison.Ordinal))
{
throw new ArgumentException(
$"Invalid path: '{path}'. Paths must not contain '.' or '..' segments.",
nameof(path));
}
}
return normalized;
}
}
dotnet/src/Microsoft.Agents.AI/Harness/FileMemory/StorePaths.cs
+109
View File
@@ -0,0 +1,109 @@
// Copyright (c) Microsoft. All rights reserved.
using System;
using System.Collections.Generic;
using System.IO;
using Microsoft.Extensions.FileSystemGlobbing;
namespace Microsoft.Agents.AI;
/// <summary>
/// Internal helper for normalizing and validating relative store paths and matching glob patterns.
/// Shared across <see cref="AgentFileStore"/> implementations and <see cref="FileMemoryProvider"/>.
/// </summary>
internal static class StorePaths
{
/// <summary>
/// Normalizes a relative path by replacing backslashes with forward slashes, trimming leading
/// and trailing separators, and collapsing consecutive separators. Also validates that the path
/// does not contain rooted paths, drive roots, or <c>.</c>/<c>..</c> traversal segments.
/// </summary>
/// <param name="path">The relative path to normalize.</param>
/// <param name="isDirectory">
/// When <see langword="true"/>, the path represents a directory and an empty result (meaning root) is allowed.
/// When <see langword="false"/> (default), the path represents a file and an empty result is rejected.
/// </param>
/// <returns>The normalized forward-slash path.</returns>
/// <exception cref="ArgumentException">
/// Thrown when <paramref name="path"/> is rooted, starts with a drive letter, contains
/// <c>.</c> or <c>..</c> segments, or is empty when <paramref name="isDirectory"/> is <see langword="false"/>.
/// </exception>
internal static string NormalizeRelativePath(string path, bool isDirectory = false)
{
string normalized = path.Replace('\\', '/').Trim('/');
if (Path.IsPathRooted(path) ||
path.StartsWith("/", StringComparison.Ordinal) ||
path.StartsWith("\\", StringComparison.Ordinal) ||
(normalized.Length >= 2 && char.IsLetter(normalized[0]) && normalized[1] == ':'))
{
throw new ArgumentException(
$"Invalid path: '{path}'. Paths must be relative and must not start with '/', '\\', or a drive root.",
nameof(path));
}
// Split, validate segments, and filter out empty segments to collapse consecutive separators.
string[] segments = normalized.Split('/');
var cleanSegments = new List<string>(segments.Length);
foreach (string segment in segments)
{
if (segment.Length == 0)
{
continue;
}
if (segment.Equals(".", StringComparison.Ordinal) || segment.Equals("..", StringComparison.Ordinal))
{
throw new ArgumentException(
$"Invalid path: '{path}'. Paths must not contain '.' or '..' segments.",
nameof(path));
}
cleanSegments.Add(segment);
}
string result = string.Join("/", cleanSegments);
if (!isDirectory && result.Length == 0)
{
throw new ArgumentException("A file path must not be empty.", nameof(path));
}
return result;
}
/// <summary>
/// Creates a <see cref="Matcher"/> for the specified glob pattern. Use the returned instance
/// to test multiple file names without allocating a new matcher for each one.
/// </summary>
/// <param name="filePattern">
/// The glob pattern to match against (e.g., <c>"*.md"</c>, <c>"research*"</c>).
/// </param>
/// <returns>A <see cref="Matcher"/> configured with the specified pattern.</returns>
internal static Matcher CreateGlobMatcher(string filePattern)
{
var matcher = new Matcher(StringComparison.OrdinalIgnoreCase);
matcher.AddInclude(filePattern);
return matcher;
}
/// <summary>
/// Determines whether a file name matches a pre-built glob <see cref="Matcher"/>.
/// </summary>
/// <param name="fileName">The file name to test (not a full path — just the name).</param>
/// <param name="matcher">
/// A pre-built <see cref="Matcher"/> to test against.
/// When <see langword="null"/>, this method returns <see langword="true"/> for any file name.
/// </param>
/// <returns><see langword="true"/> if the file name matches the pattern or if the matcher is <see langword="null"/>; otherwise, <see langword="false"/>.</returns>
internal static bool MatchesGlob(string fileName, Matcher? matcher)
{
if (matcher is null)
{
return true;
}
PatternMatchingResult result = matcher.Match(fileName);
return result.HasMatches;
}
}
dotnet/tests/Microsoft.Agents.AI.UnitTests/Harness/FileMemory/FileSystemAgentFileStoreTests.cs
+7 -3
View File
@@ -102,10 +102,14 @@ public sealed class FileSystemAgentFileStoreTests : IDisposable
}
[Fact]
public async Task WriteFileAsync_TrailingSlash_ThrowsAsync()
public async Task WriteFileAsync_TrailingSlash_NormalizesAsync()
{
// Act & Assert
await Assert.ThrowsAsync<ArgumentException>(() => this._store.WriteFileAsync("subdir/", "content"));
// Act — trailing slash is trimmed during normalization.
await this._store.WriteFileAsync("subdir/", "content");
// Assert — the file is accessible via the normalized name.
string? result = await this._store.ReadFileAsync("subdir");
Assert.Equal("content", result);
}
#endregion
dotnet/tests/Microsoft.Agents.AI.UnitTests/Harness/FileMemory/StorePathsTests.cs
+177
View File
@@ -0,0 +1,177 @@
// Copyright (c) Microsoft. All rights reserved.
using System;
using Microsoft.Extensions.FileSystemGlobbing;
namespace Microsoft.Agents.AI.UnitTests.Harness.FileMemory;
public class StorePathsTests
{
#region NormalizeRelativePath — valid paths
[Theory]
[InlineData("file.md", "file.md")]
[InlineData("folder/file.md", "folder/file.md")]
[InlineData("a/b/c.txt", "a/b/c.txt")]
public void NormalizeRelativePath_ValidPath_ReturnsNormalized(string input, string expected)
{
// Act
string result = StorePaths.NormalizeRelativePath(input);
// Assert
Assert.Equal(expected, result);
}
[Theory]
[InlineData("folder\\file.md", "folder/file.md")]
[InlineData("a\\b\\c.txt", "a/b/c.txt")]
public void NormalizeRelativePath_Backslashes_NormalizesToForwardSlash(string input, string expected)
{
// Act
string result = StorePaths.NormalizeRelativePath(input);
// Assert
Assert.Equal(expected, result);
}
[Theory]
[InlineData("folder//file.md", "folder/file.md")]
[InlineData("a///b////c.txt", "a/b/c.txt")]
public void NormalizeRelativePath_ConsecutiveSeparators_Collapsed(string input, string expected)
{
// Act
string result = StorePaths.NormalizeRelativePath(input);
// Assert
Assert.Equal(expected, result);
}
[Fact]
public void NormalizeRelativePath_TrailingSlash_Trimmed()
{
// Act
string result = StorePaths.NormalizeRelativePath("file.md/");
// Assert
Assert.Equal("file.md", result);
}
[Theory]
[InlineData("/file.md")]
[InlineData("/folder/file.md/")]
public void NormalizeRelativePath_LeadingSlash_Throws(string input)
{
// Act & Assert — leading slash is treated as a rooted path.
Assert.Throws<ArgumentException>(() => StorePaths.NormalizeRelativePath(input));
}
#endregion
#region NormalizeRelativePath — rejected paths
[Theory]
[InlineData("../file.md")]
[InlineData("folder/../file.md")]
[InlineData("./file.md")]
[InlineData("folder/./file.md")]
public void NormalizeRelativePath_TraversalSegments_Throws(string input)
{
// Act & Assert
Assert.Throws<ArgumentException>(() => StorePaths.NormalizeRelativePath(input));
}
[Theory]
[InlineData("C:\\file.md")]
[InlineData("C:/file.md")]
[InlineData("D:file.md")]
public void NormalizeRelativePath_DriveRoot_Throws(string input)
{
// Act & Assert
Assert.Throws<ArgumentException>(() => StorePaths.NormalizeRelativePath(input));
}
[Fact]
public void NormalizeRelativePath_EmptyFile_Throws()
{
// Act & Assert
Assert.Throws<ArgumentException>(() => StorePaths.NormalizeRelativePath(""));
}
[Fact]
public void NormalizeRelativePath_WhitespaceOnlyFile_DoesNotThrowAsTraversal()
{
// Act — whitespace characters are not path separators, so " " becomes a valid segment.
string result = StorePaths.NormalizeRelativePath(" ");
// Assert
Assert.Equal(" ", result);
}
#endregion
#region NormalizeRelativePath — directory mode
[Fact]
public void NormalizeRelativePath_EmptyDirectory_ReturnsEmpty()
{
// Act
string result = StorePaths.NormalizeRelativePath("", isDirectory: true);
// Assert
Assert.Equal("", result);
}
[Theory]
[InlineData("folder", "folder")]
[InlineData("a/b", "a/b")]
[InlineData("a\\b/", "a/b")]
public void NormalizeRelativePath_DirectoryMode_NormalizesPath(string input, string expected)
{
// Act
string result = StorePaths.NormalizeRelativePath(input, isDirectory: true);
// Assert
Assert.Equal(expected, result);
}
[Fact]
public void NormalizeRelativePath_DirectoryTraversal_Throws()
{
// Act & Assert
Assert.Throws<ArgumentException>(() => StorePaths.NormalizeRelativePath("../folder", isDirectory: true));
}
#endregion
#region CreateGlobMatcher and MatchesGlob
[Theory]
[InlineData("*.md", "notes.md", true)]
[InlineData("*.md", "notes.txt", false)]
[InlineData("research*", "research_results.md", true)]
[InlineData("research*", "notes.md", false)]
[InlineData("*.md", "NOTES.MD", true)] // case-insensitive
public void MatchesGlob_WithMatcher_MatchesCorrectly(string pattern, string fileName, bool expected)
{
// Arrange
Matcher matcher = StorePaths.CreateGlobMatcher(pattern);
// Act
bool result = StorePaths.MatchesGlob(fileName, matcher);
// Assert
Assert.Equal(expected, result);
}
[Fact]
public void MatchesGlob_NullMatcher_ReturnsTrue()
{
// Act
bool result = StorePaths.MatchesGlob("anything.txt", null);
// Assert
Assert.True(result);
}
#endregion
}