Skip to content

.NET: Harness: Improve path validation#5404

Merged
westey-m merged 2 commits intomicrosoft:feature-harnessfrom
westey-m:harness-file-validation-improvements
Apr 22, 2026
Merged

.NET: Harness: Improve path validation#5404
westey-m merged 2 commits intomicrosoft:feature-harnessfrom
westey-m:harness-file-validation-improvements

Conversation

@westey-m
Copy link
Copy Markdown
Contributor

@westey-m westey-m commented Apr 21, 2026

Motivation and Context

Adding some further improvements and code consolidation to file path validation for the file stores and file based memory store.

Description

Adding some further improvements and code consolidation to file path validation for the file stores and file based memory store.

Contribution Checklist

  • The code builds clean without any errors or warnings
  • The PR follows the Contribution Guidelines
  • All unit tests pass, and I have added new tests where possible
  • Is this a breaking change? If yes, add "[BREAKING]" prefix to the title of the PR.

Copilot AI review requested due to automatic review settings April 21, 2026 14:07
@moonbox3 moonbox3 added the .NET label Apr 21, 2026
@github-actions github-actions Bot changed the title Harness: Improve path validation .NET: Harness: Improve path validation Apr 21, 2026
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR strengthens and centralizes relative-path validation for the Harness file stores and the file-based memory provider, aiming to reduce path-traversal risk and normalize path inputs consistently across implementations.

Changes:

  • Introduces a shared AgentFileStore.NormalizeRelativePath helper and updates in-memory/file-system stores to use it.
  • Updates FileSystemAgentFileStore to normalize before combining with the root directory.
  • Adjusts unit tests to reflect new normalization behavior for trailing slashes.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
dotnet/src/Microsoft.Agents.AI/Harness/FileMemory/AgentFileStore.cs Adds centralized relative-path normalization/validation helper.
dotnet/src/Microsoft.Agents.AI/Harness/FileMemory/InMemoryAgentFileStore.cs Switches to shared normalization helper for all operations.
dotnet/src/Microsoft.Agents.AI/Harness/FileMemory/FileSystemAgentFileStore.cs Uses normalized relative paths during safe path resolution.
dotnet/src/Microsoft.Agents.AI/Harness/FileMemory/FileMemoryProvider.cs Refactors and tightens filename validation before combining paths.
dotnet/tests/Microsoft.Agents.AI.UnitTests/Harness/FileMemory/FileSystemAgentFileStoreTests.cs Updates test expectation: trailing slashes normalize instead of throwing.
Comments suppressed due to low confidence (1)

dotnet/src/Microsoft.Agents.AI/Harness/FileMemory/FileSystemAgentFileStore.cs:250

  • The root-escape check uses fullPath.StartsWith(this._rootPath, StringComparison.Ordinal). On Windows, paths are case-insensitive and Path.GetFullPath can normalize drive-letter casing, so Ordinal comparisons can incorrectly reject valid paths (denial of service) depending on the casing of rootDirectory. Use an OS-appropriate comparison (e.g., OrdinalIgnoreCase on Windows) or compare via Path.GetRelativePath(_rootPath, fullPath) and ensure it doesn’t start with ...
        string fullPath = Path.GetFullPath(combined);

        if (!fullPath.StartsWith(this._rootPath, StringComparison.Ordinal))
        {
            throw new ArgumentException(
                $"Invalid path: '{relativePath}'. The resolved path escapes the root directory.",
                nameof(relativePath));

Comment thread dotnet/src/Microsoft.Agents.AI/Harness/FileMemory/AgentFileStore.cs Outdated
Comment thread dotnet/src/Microsoft.Agents.AI/Harness/FileMemory/FileMemoryProvider.cs Outdated
Comment thread dotnet/src/Microsoft.Agents.AI/Harness/FileMemory/FileMemoryProvider.cs Outdated
@westey-m westey-m merged commit 53274fd into microsoft:feature-harness Apr 22, 2026
12 checks passed
pull Bot pushed a commit to nagyist/ms-agent-framework that referenced this pull request May 1, 2026
* .NET: Add a TODO AIContextProvider (microsoft#5233)

* Add a TODO AIContextProvider

* Add unit tests

* Address PR comments

* Address PR comments

* Fix test after removing one tool

* .NET: Add a ModeProvider for managing agent modes (microsoft#5247)

* Add a ModeProvider for managing agent modes

* Fix typo

* Fix typo

* Fix typo

* Address PR comments

* .NET: Add sample to show how to build a harness (microsoft#5268)

* Add sample to show how to build a harness

* Improve sample

* Sample max output tokens and model

* Fix encoding

* Fix model name in readme

* Address PR comments

* .NET: Add context window size compaction strategy for harness (microsoft#5304)

* Add context window size compaction strategy for harness

* Apply suggestions from code review

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

* Address PR comments

---------

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

* .NET: Add a file memory provider (microsoft#5315)

* Add a file memory provider

* Address PR comments

* Fix review comments.

* Add additional unit tests

* Addressing PR comments.

* .NET:  Harness: Improve prompts and add FileSystem store (microsoft#5365)

* Harness: Improve prompts and add FileSystem store

* Address PR comments

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

* Harness: Improve path validation

* Address PR comments

* .NET: Add always approve helpers, improve sample and fix bug (microsoft#5451)

* Add always approve helpers, improve sample and fix bug

* Address PR comments

* .NET: Make Todo, Mode and FileMemory providers more configurable (microsoft#5477)

* Make Todo, Mode and FileMemory providers more configurable

* Address PR comments.

* .NET: Add subagents provider and sample (microsoft#5518)

* Add subagents provider and sample

* Addressing PR comments.

* .NET: Harness filememory index plus instructions consistency (microsoft#5540)

* Add FileMemoryProvider index and improve instruction consistency

* Address PR comments.

* Address PR comments

* Address PR comments.

* Apply suggestion from @rogerbarreto

Co-authored-by: Roger Barreto <19890735+rogerbarreto@users.noreply.github.com>

---------

Co-authored-by: Roger Barreto <19890735+rogerbarreto@users.noreply.github.com>

* .NET: Refactor harness console to be more extensible and easy to understand with better UX (microsoft#5573)

* Refactor harness console to be more extensible and easy to understand with better UX.

* Fix formatting issues.

* Allow multiple clarifications in one response

* Address PR comments

* .NET: Add FileAccessProvdider and concurrency fix for FileMemoryProvider (microsoft#5583)

* Add FileAccessProvdider and concurrency fix for FileMemoryProvider

* Address PR comments

---------

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Roger Barreto <19890735+rogerbarreto@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants