Skip to content

Migrate Unzip task to use TaskEnvironment API#13109

Merged
SimaTian merged 5 commits intomainfrom
copilot/migrate-unzip-task-environment
Feb 5, 2026
Merged

Migrate Unzip task to use TaskEnvironment API#13109
SimaTian merged 5 commits intomainfrom
copilot/migrate-unzip-task-environment

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Jan 26, 2026

Context

Migrates the Unzip task to use the TaskEnvironment API for thread-safe path resolution, following the pattern established in #12914. This enables the task to run safely in MSBuild's multithreaded execution model.

Changes Made

  • Add IMultiThreadableTask interface and [MSBuildMultiThreadableTask] attribute to Unzip class
  • Add TaskEnvironment property
  • Replace Path.GetFullPath() with TaskEnvironment.GetAbsolutePath() for:
    • Destination folder creation (inside try block for proper error handling)
    • Source file path resolution (wrapped in try-catch to continue processing on invalid paths)
  • Keep Path.GetFullPath() in the Extract method for DirectoryInfo-based paths where it's cleaner
  • Update all unit tests to provide TaskEnvironmentHelper.CreateForTest() when instantiating the task

Testing

  • All 14 Unzip unit tests pass
  • Updated all test instantiations to provide TaskEnvironmentHelper.CreateForTest()

Notes

  • The GetAbsolutePath calls are wrapped in try-catch blocks to handle exceptions from invalid paths gracefully
  • The zip-slip exploit protection continues to use Path.GetFullPath() on DirectoryInfo.FullName as it's cleaner for paths that are already absolute
Original prompt

Migrate the Unzip task to use TaskEnvironment API based on the pattern used here: #12914

Make sure to preserve correctness, performance and that the tests pass


💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

@dotnet-policy-service
Copy link
Copy Markdown
Contributor

Hello @@copilot, I noticed that you’re changing an .swr file or any file under src/Package/MSBuild.VSSetup.. Please make sure to validate this change by an experimental VS insertion. This is accomplished by pushing to an exp/* branch, which requires write permissions to this repo.

Copilot AI and others added 2 commits January 26, 2026 10:00
Co-authored-by: JanProvaznik <25267098+JanProvaznik@users.noreply.github.com>
Co-authored-by: JanProvaznik <25267098+JanProvaznik@users.noreply.github.com>
Copilot AI changed the title [WIP] Migrate Unzip task to use TaskEnvironment API Migrate Unzip task to use TaskEnvironment API Jan 26, 2026
Copilot AI requested a review from JanProvaznik January 26, 2026 10:05
Copy link
Copy Markdown
Member

@JanProvaznik JanProvaznik left a comment

Choose a reason for hiding this comment

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

@copilot one comment

Co-authored-by: JanProvaznik <25267098+JanProvaznik@users.noreply.github.com>
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 migrates the Unzip task to use the TaskEnvironment API for thread-safe path resolution, enabling safe execution in MSBuild's multithreaded environment. This follows the established pattern from PR #12914 that migrated other file I/O tasks (Copy, Delete, MakeDir, RemoveDir, Touch).

Changes:

  • Added IMultiThreadableTask interface and [MSBuildMultiThreadableTask] attribute to enable multithreaded execution
  • Replaced process-global Path.GetFullPath() calls with thread-safe TaskEnvironment.GetAbsolutePath()
  • Updated all 17 test cases to provide TaskEnvironment instances via TaskEnvironmentHelper.CreateForTest()

Reviewed changes

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

File Description
src/Tasks/Unzip.cs Added multithreadable task support and migrated three path resolution calls (destination folder, source file, extracted file path) to use TaskEnvironment API while preserving zip-slip security validation
src/Tasks.UnitTests/Unzip_Tests.cs Updated all 17 Unzip task instantiations to include TaskEnvironment property initialization

@JanProvaznik
Copy link
Copy Markdown
Member

blocked by #13120 for correctness

@JanProvaznik JanProvaznik marked this pull request as draft January 27, 2026 15:31
Copy link
Copy Markdown
Member

@JanProvaznik JanProvaznik left a comment

Choose a reason for hiding this comment

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

@copilot fix issues

… Path.GetFullPath in Extract method

Co-authored-by: JanProvaznik <25267098+JanProvaznik@users.noreply.github.com>
@JanProvaznik JanProvaznik requested a review from SimaTian February 2, 2026 09:30
@JanProvaznik JanProvaznik marked this pull request as ready for review February 2, 2026 09:30
@SimaTian SimaTian merged commit 594c3cb into main Feb 5, 2026
9 checks passed
@SimaTian SimaTian deleted the copilot/migrate-unzip-task-environment branch February 5, 2026 15:15
JanProvaznik added a commit to JanProvaznik/msbuild that referenced this pull request Feb 25, 2026
### Context

Migrates the Unzip task to use the `TaskEnvironment` API for thread-safe
path resolution, following the pattern established in dotnet#12914. This
enables the task to run safely in MSBuild's multithreaded execution
model.

### Changes Made

- Add `IMultiThreadableTask` interface and
`[MSBuildMultiThreadableTask]` attribute to `Unzip` class
- Add `TaskEnvironment` property
- Replace `Path.GetFullPath()` with `TaskEnvironment.GetAbsolutePath()`
for:
- Destination folder creation (inside try block for proper error
handling)
- Source file path resolution (wrapped in try-catch to continue
processing on invalid paths)
- Keep `Path.GetFullPath()` in the `Extract` method for
DirectoryInfo-based paths where it's cleaner
- Update all unit tests to provide
`TaskEnvironmentHelper.CreateForTest()` when instantiating the task

### Testing

- All 14 Unzip unit tests pass
- Updated all test instantiations to provide
`TaskEnvironmentHelper.CreateForTest()`

### Notes

- The `GetAbsolutePath` calls are wrapped in try-catch blocks to handle
exceptions from invalid paths gracefully
- The zip-slip exploit protection continues to use `Path.GetFullPath()`
on `DirectoryInfo.FullName` as it's cleaner for paths that are already
absolute

<!-- START COPILOT CODING AGENT SUFFIX -->



<!-- START COPILOT ORIGINAL PROMPT -->



<details>

<summary>Original prompt</summary>

> Migrate the Unzip task to use TaskEnvironment API based on the pattern
used here: dotnet#12914
> 
> Make sure to preserve correctness, performance and that the tests pass


</details>



<!-- START COPILOT CODING AGENT TIPS -->
---

💬 We'd love your input! Share your thoughts on Copilot coding agent in
our [2 minute survey](https://gh.io/copilot-coding-agent-survey).

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: JanProvaznik <25267098+JanProvaznik@users.noreply.github.com>
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.

4 participants