Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
// Copyright (c) Microsoft. All rights reserved.

using System;
using System.Collections.Generic;
using System.Diagnostics.CodeAnalysis;
using System.IO;
using System.Threading;
using System.Threading.Tasks;
using Microsoft.Extensions.FileSystemGlobbing;
Expand Down Expand Up @@ -91,6 +93,54 @@ public abstract class AgentFileStore
/// <returns>A task representing the asynchronous operation.</returns>
public abstract Task CreateDirectoryAsync(string path, CancellationToken cancellationToken = default);

/// <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>
/// <returns>The normalized forward-slash path.</returns>
/// <exception cref="ArgumentException">
/// Thrown when <paramref name="path"/> is rooted, starts with a drive letter, or contains
/// <c>.</c> or <c>..</c> segments.
/// </exception>
protected static string NormalizeRelativePath(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));
}

// 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);
}

return string.Join("/", cleanSegments);
}
Comment thread
westey-m marked this conversation as resolved.
Outdated

/// <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.
Expand All @@ -101,7 +151,7 @@ public abstract class AgentFileStore
/// <returns>A <see cref="Matcher"/> configured with the specified pattern.</returns>
protected static Matcher CreateGlobMatcher(string filePattern)
{
var matcher = new Matcher(System.StringComparison.OrdinalIgnoreCase);
var matcher = new Matcher(StringComparison.OrdinalIgnoreCase);
matcher.AddInclude(filePattern);
return matcher;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -264,26 +264,32 @@ private static string GetDescriptionFileName(string fileName)

private static string ResolvePath(string workingFolder, string fileName)
{
string normalizedFileName = fileName.Replace('\\', '/');
Comment thread
westey-m marked this conversation as resolved.
Outdated

// Prevent path traversal by rejecting rooted paths and '.'/'..' segments.
string normalized = fileName.Replace('\\', '/');
// Only fileName needs validation — workingFolder is developer-provided and trusted.
ValidateNormalizedRelativePath(normalizedFileName, nameof(fileName), "file name");

string normalizedWorkingFolder = workingFolder.Replace('\\', '/');
return CombinePaths(normalizedWorkingFolder, normalizedFileName);
}

if (Path.IsPathRooted(fileName) ||
fileName.StartsWith("/", StringComparison.Ordinal) ||
fileName.StartsWith("\\", StringComparison.Ordinal) ||
(normalized.Length >= 2 && char.IsLetter(normalized[0]) && normalized[1] == ':'))
private static void ValidateNormalizedRelativePath(string path, string parameterName, string pathDescription)
{
if (Path.IsPathRooted(path) ||
path.StartsWith("/", StringComparison.Ordinal) ||
(path.Length >= 2 && char.IsLetter(path[0]) && path[1] == ':'))
{
throw new ArgumentException($"Invalid file name: '{fileName}'. File names must be relative and must not start with '/', '\\', or a drive root.", nameof(fileName));
throw new ArgumentException($"Invalid {pathDescription}: '{path}'. Paths must be relative and must not start with '/' or a drive root.", parameterName);
}

foreach (string segment in normalized.Split('/'))
foreach (string segment in path.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));
throw new ArgumentException($"Invalid {pathDescription}: '{path}'. Paths must not contain '.' or '..' segments.", parameterName);
}
}
Comment thread
westey-m marked this conversation as resolved.
Outdated

return CombinePaths(workingFolder, fileName);
}

private static string CombinePaths(string basePath, string relativePath)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -235,12 +235,12 @@ public override Task CreateDirectoryAsync(string path, CancellationToken cancell
/// </summary>
private string ResolveSafePath(string relativePath)
{
ValidateRelativePath(relativePath);
// Normalize and validate the relative path (rejects rooted, traversal, etc.).
string normalized = 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))
Expand All @@ -266,40 +266,4 @@ private string ResolveSafeDirectoryPath(string relativeDirectory)

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));
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
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;
Expand All @@ -29,30 +28,30 @@ public sealed class InMemoryAgentFileStore : AgentFileStore
/// <inheritdoc />
public override Task WriteFileAsync(string path, string content, CancellationToken cancellationToken = default)
{
path = NormalizePath(path);
path = NormalizeRelativePath(path);
this._files[path] = content;
return Task.CompletedTask;
}

/// <inheritdoc />
public override Task<string?> ReadFileAsync(string path, CancellationToken cancellationToken = default)
{
path = NormalizePath(path);
path = NormalizeRelativePath(path);
this._files.TryGetValue(path, out string? content);
return Task.FromResult(content);
}

/// <inheritdoc />
public override Task<bool> DeleteFileAsync(string path, CancellationToken cancellationToken = default)
{
path = NormalizePath(path);
path = 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 = NormalizeRelativePath(directory);
if (prefix.Length > 0 && !prefix.EndsWith("/", StringComparison.Ordinal))
{
prefix += "/";
Expand All @@ -70,15 +69,15 @@ public override Task<IReadOnlyList<string>> ListFilesAsync(string directory, Can
/// <inheritdoc />
public override Task<bool> FileExistsAsync(string path, CancellationToken cancellationToken = default)
{
path = NormalizePath(path);
path = NormalizeRelativePath(path);
return Task.FromResult(this._files.ContainsKey(path));
}

/// <inheritdoc />
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 = NormalizeRelativePath(directory);
if (prefix.Length > 0 && !prefix.EndsWith("/", StringComparison.Ordinal))
{
prefix += "/";
Expand Down Expand Up @@ -158,31 +157,4 @@ public override Task CreateDirectoryAsync(string path, CancellationToken cancell
// 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;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -102,10 +102,14 @@ public async Task WriteFileAsync_DoubleDotsInFileName_AllowedAsync()
}

[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
Expand Down
Loading