Skip to content

Add Path Validation for DiskBasedResponseCache and DiskBasedResultStore#7397

Merged
peterwald merged 13 commits intodotnet:mainfrom
peterwald:pw-reporting-path
Mar 18, 2026
Merged

Add Path Validation for DiskBasedResponseCache and DiskBasedResultStore#7397
peterwald merged 13 commits intodotnet:mainfrom
peterwald:pw-reporting-path

Conversation

@peterwald
Copy link
Copy Markdown
Member

@peterwald peterwald commented Mar 16, 2026

Microsoft Reviewers: Open in CodeFlow

@peterwald peterwald requested a review from a team as a code owner March 16, 2026 15:26
Copilot AI review requested due to automatic review settings March 16, 2026 15:26
@peterwald peterwald self-assigned this Mar 16, 2026
@github-actions github-actions Bot added the area-ai-eval Microsoft.Extensions.AI.Evaluation and related label Mar 16, 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

Adds centralized filesystem path-segment validation to prevent path traversal in disk-backed evaluation reporting storage components, improving safety when user-controlled names influence on-disk paths.

Changes:

  • Introduces PathValidation utility for validating path segments and ensuring resolved paths stay within a configured root.
  • Applies validation/containment checks in DiskBasedResultStore and DiskBasedResponseCache.
  • Adds unit + integration tests covering valid/invalid segments and traversal attempts.

Reviewed changes

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

File Description
test/Libraries/Microsoft.Extensions.AI.Evaluation.Reporting.Tests/DiskBased/PathValidationTests.cs Adds coverage for path-segment validation, root containment, and traversal rejection in disk-backed components.
src/Libraries/Microsoft.Extensions.AI.Evaluation.Reporting/CSharp/Utilities/PathValidation.cs New shared helper to validate path segments and prevent resolved paths from escaping a configured root.
src/Libraries/Microsoft.Extensions.AI.Evaluation.Reporting/CSharp/Storage/DiskBasedResultStore.cs Validates inputs and ensures constructed paths remain under the results root to prevent traversal.
src/Libraries/Microsoft.Extensions.AI.Evaluation.Reporting/CSharp/Storage/DiskBasedResponseCache.cs Validates scenario/iteration/key segments and ensures cache paths stay within the cache root.
Comments suppressed due to low confidence (1)

test/Libraries/Microsoft.Extensions.AI.Evaluation.Reporting.Tests/DiskBased/PathValidationTests.cs:203

  • xUnit async tests should return Task rather than using async void. Using async void can lead to unobserved exceptions and flaky behavior; update the signature to public async Task ....
    [Fact]
    public async void DiskBasedResponseCacheProvider_TraversalInScenarioName_Throws()
    {

You can also share your feedback on Copilot code review. Take the survey.

Copy link
Copy Markdown
Contributor

@shyamnamboodiripad shyamnamboodiripad left a comment

Choose a reason for hiding this comment

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

LGTM modulo comments and edge cases

@peterwald peterwald merged commit e62391b into dotnet:main Mar 18, 2026
6 checks passed
@peterwald peterwald deleted the pw-reporting-path branch March 18, 2026 18:56
jozkee pushed a commit to jozkee/extensions that referenced this pull request Apr 3, 2026
…re (dotnet#7397)

* Add Path Validation for DiskBasedResponseCache and DiskBasedResultStore

* Refactor async test methods in PathValidationTests to use Task instead of void

* Update error message 'value' to 'parameter'

Co-authored-by: Copilot Autofix powered by AI <[email protected]>

* Fix separator tests on Mac/Linux

* Update src/Libraries/Microsoft.Extensions.AI.Evaluation.Reporting/CSharp/Utilities/PathValidation.cs

Co-authored-by: Shyam N <[email protected]>

* Update src/Libraries/Microsoft.Extensions.AI.Evaluation.Reporting/CSharp/Utilities/PathValidation.cs

Co-authored-by: Shyam N <[email protected]>

* Improve root path check.

* Enhance EnsureWithinRoot method to handle directory separators for .NET Framework and add tests for UNC paths and edge cases

* Fix net standard builds

* Retrigger build

---------

Co-authored-by: Copilot Autofix powered by AI <[email protected]>
Co-authored-by: Shyam N <[email protected]>
@github-actions github-actions Bot locked and limited conversation to collaborators Apr 18, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

area-ai-eval Microsoft.Extensions.AI.Evaluation and related

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants