Skip to content

fix: validate output paths in reporters to prevent path traversal#4927

Merged
thomhurst merged 1 commit intomainfrom
fix/junit-path-traversal
Feb 19, 2026
Merged

fix: validate output paths in reporters to prevent path traversal#4927
thomhurst merged 1 commit intomainfrom
fix/junit-path-traversal

Conversation

@thomhurst
Copy link
Owner

Summary

  • Adds a PathValidator helper class that rejects paths containing .. traversal sequences and normalizes all paths via Path.GetFullPath
  • Validates and sanitizes file paths from environment variables (JUNIT_XML_OUTPUT_PATH, GITHUB_STEP_SUMMARY) and the --junit-output-path command-line argument in both JUnitReporter and GitHubReporter
  • Sanitizes the assembly name used in the default JUnit output path to strip invalid filename characters

Details

The JUnitReporter reads environment variables (like JUNIT_XML_OUTPUT_PATH) and command-line arguments to determine output paths, using them to construct file paths without validation. Similarly, GitHubReporter reads GITHUB_STEP_SUMMARY directly. This could allow path traversal attacks if environment variables are manipulated.

The new PathValidator.ValidateAndNormalizePath method:

  1. Rejects null/empty/whitespace paths
  2. Rejects paths containing .. directory traversal segments
  3. Normalizes paths via Path.GetFullPath
  4. For relative paths, verifies the resolved path stays within the current working directory

Closes #4881

Test plan

  • Verify dotnet build TUnit.Engine/TUnit.Engine.csproj compiles successfully across all target frameworks (netstandard2.0, net8.0, net9.0, net10.0)
  • Verify existing JUnitReporterTests still pass
  • Verify existing GitHubReporterTests still pass
  • Verify that setting JUNIT_XML_OUTPUT_PATH=../../etc/malicious.xml causes an ArgumentException
  • Verify that normal absolute paths like /tmp/results/junit.xml still work
  • Verify that normal relative paths like TestResults/output.xml still work

@claude
Copy link
Contributor

claude bot commented Feb 19, 2026

Code review

No issues found. Checked for bugs and CLAUDE.md compliance.

This is a solid security fix. The PathValidator helper is well-structured with two complementary layers of defense:

  1. Explicit .. segment detection via ContainsPathTraversal — catches traversal attempts before normalization.
  2. Boundary check post-normalization — ensures relative paths don't escape the working directory after Path.GetFullPath resolution.

The integration into both GitHubReporter and JUnitReporter is correct, and the assembly name sanitization in GetDefaultOutputPath is a nice hardening detail. The command-line validation in JUnitReporterCommandProvider provides good early feedback, with SetOutputPath as a reliable security backstop.

One minor observation (not a blocker): the StartsWith(currentDir) check without a trailing separator could theoretically match sibling directories with a common prefix (e.g., CWD /foo/bar matching /foo/barbaz/...). However, since ContainsPathTraversal already rejects all .. segments, and a relative path without .. cannot escape the current directory on a standard filesystem, this is not exploitable in practice. Appending Path.DirectorySeparatorChar to currentDir before the comparison would be a more defensive pattern for future readers of this code.

Add PathValidator helper that rejects paths containing ".." traversal
sequences and normalizes all paths via Path.GetFullPath. Applied to:

- JUnitReporter: environment variable (JUNIT_XML_OUTPUT_PATH), command-line
  argument (--junit-output-path), and default output path
- GitHubReporter: environment variable (GITHUB_STEP_SUMMARY)
- JUnitReporterCommandProvider: command-line argument validation

Closes #4881
@thomhurst thomhurst force-pushed the fix/junit-path-traversal branch from f5f7be6 to 1af30d2 Compare February 19, 2026 11:46
@thomhurst thomhurst merged commit 79de461 into main Feb 19, 2026
13 of 14 checks passed
@thomhurst thomhurst deleted the fix/junit-path-traversal branch February 19, 2026 13:53
This was referenced Mar 3, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

security: validate output paths from environment variables in JUnitReporter

1 participant