Updated Code to skip files which can trigger outside agent root direc…

[sanjuyadav24]

Context

Fix for path traversal vulnerability in CodeCoverageUtilities.CopyFilesFromFileListWithDirStructure

The vulnerability allows an authenticated pipeline user to achieve arbitrary file write on self-hosted agents by crafting additionalcodecoveragefiles paths in ##vso[codecoverage.publish]. The flawed String.Replace + Path.Combine logic enables sandbox escape, potentially overwriting agent binaries and achieving RCE.

---

Description

Three-layer defense-in-depth fix for CopyFilesFromFileListWithDirStructure in CodeCoverageUtilities.cs:

1. Fix 1 — Substring instead of Replace: The original file.Replace(commonPath, "") removes all occurrences of the common path string, not just the prefix. An attacker crafts filenames where after all occurrences are removed, the result becomes an absolute path. Changed to file.StartsWith(commonPath) + file.Substring(commonPath.Length) which mathematically guarantees only the leading prefix is removed.

2. Fix 2 — TrimStart leading separators: .NET's Path.Combine(dest, newFile) ignores dest entirely if newFile starts with \ or /. After Fix 1, the remaining string can start with a separator. TrimStart(DirectorySeparatorChar, AltDirectorySeparatorChar) forces newFile to always be a relative path.

3. Fix 3 — GetFullPath canonicalization boundary check: Even with Fixes 1+2, ../ sequences in crafted paths could navigate upward out of the destination directory. Path.GetFullPath resolves all .. sequences, then StartsWith verifies the result stays inside the destination. If not, the file is skipped and reported via skippedFiles output parameter.

Caller changes (CodeCoverageCommands.cs): Passes a skippedFiles list and emits executionContext.Warning() for each skipped file, ensuring pipeline authors see which files were blocked.

Localization (strings.json): Added CodeCoverageFileSkippedPathTraversal key for the warning message.

---

Risk Assessment (Low / Medium / High)

Low

---

Unit Tests Added or Updated (Yes)

3 new L0 tests added to CodeCoverageUtilitiesTests.cs:

Test	Attack Vector	What it validates
CopyFilesSkipsPathTraversalFiles	../../../evil.xml traversal	Layer 3 catches traversal, file added to skippedFiles, not written outside destination
CopyFilesHandlesAbsolutePathInjection	Replace trick producing \sub\evil.txt	Layers 1+2 neutralize attack — file safely nested inside destination, not at drive root
CopyFilesSucceedsWithLegitimateFiles	Normal files with shared prefix	No false positives — legitimate files are copied correctly with directory structure preserved

---

Additional Testing Performed

Manual Testing

Change Behind Feature Flag (No)

This is a security fix that must apply unconditionally. A feature flag would leave the vulnerability exploitable when the flag is off, defeating the purpose of the fix. The skip+warn approach already provides graceful handling without needing a kill switch.

---

Tech Design / Approach

NA

---

Documentation Changes Required (No)

No user-facing documentation changes needed. The fix is internal to the agent's file copy logic. The new warning message is self-explanatory for pipeline authors who encounter it.

---

Logging Added/Updated (Yes)

NA

---

Telemetry Added/Updated (No)

No new telemetry added. The warning log is sufficient for detection. Telemetry can be added as a follow-up if monitoring of traversal attempts is desired.

---

Rollback Scenario and Process (Yes)

NA

---

Dependency Impact Assessed and Regression Tested (Yes)

• CopyFilesFromFileListWithDirStructure is called from one location: PublishCodeCoverageCommand.ProcessCommandInternalAsync in CodeCoverageCommands.cs.
• Default parameter skippedFiles = null ensures no other callers (if any) are affected.
• All 6 pre-existing unit tests pass without modification, confirming no regression in legitimate code coverage file handling.

[Copilot]

Pull request overview

Hardens CodeCoverageUtilities.CopyFilesFromFileListWithDirStructure against path-traversal and absolute-path-injection attacks that could cause coverage file copies to land outside the configured destination directory. The publisher now collects skipped files and emits a warning per skipped entry; new L0 tests exercise both attack vectors and the happy path.

Changes:

• Replace fragile string.Replace-based prefix stripping with Substring + leading-separator TrimStart, plus a canonicalized Path.GetFullPath boundary check that rejects paths escaping the destination.
• Add an optional skippedFiles out-list and surface skipped entries as warnings in CodeCoverageCommands.PublishCodeCoverageAsync, backed by a new CodeCoverageFileSkippedPathTraversal localized string.
• Add three L0 tests covering path-traversal skip, leading-separator absolute-path injection, and the legitimate-copy baseline.

Show a summary per file

File	Description
src/Agent.Worker/CodeCoverage/CodeCoverageUtilities.cs	Core fix: prefix stripping, separator trimming, and canonicalized boundary check; new optional skippedFiles parameter.
src/Agent.Worker/CodeCoverage/CodeCoverageCommands.cs	Passes a skippedFiles list to the utility and warns per skipped file.
src/Misc/layoutbin/en-US/strings.json	Adds the CodeCoverageFileSkippedPathTraversal warning string.
src/Test/L0/Worker/CodeCoverage/CodeCoverageUtilitiesTests.cs	Adds three L0 tests for traversal skip, absolute-path injection, and legitimate copy.

Copilot's findings

• Files reviewed: 4/4 changed files
• Comments generated: 5

[sanjuyadav24]

/azp run

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[tarunramsinghani]

can we make this error but not fail the task...

[sanjuyadav24]

this will trigger a warning, not fail the pipeline

[tarunramsinghani]

ignore case is not needed for case-sensitive os IMO, we shuold figure out side effects of it

[sanjuyadav24]

updated