-
Notifications
You must be signed in to change notification settings - Fork 1.4k
Migrate Unzip task to use TaskEnvironment API #13109
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
|
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. |
Co-authored-by: JanProvaznik <[email protected]>
Co-authored-by: JanProvaznik <[email protected]>
JanProvaznik
left a comment
There was a problem hiding this 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 <[email protected]>
There was a problem hiding this 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
IMultiThreadableTaskinterface and[MSBuildMultiThreadableTask]attribute to enable multithreaded execution - Replaced process-global
Path.GetFullPath()calls with thread-safeTaskEnvironment.GetAbsolutePath() - Updated all 17 test cases to provide
TaskEnvironmentinstances viaTaskEnvironmentHelper.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 |
|
blocked by #13120 for correctness |
Context
Migrates the Unzip task to use the
TaskEnvironmentAPI 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
IMultiThreadableTaskinterface and[MSBuildMultiThreadableTask]attribute toUnzipclassTaskEnvironmentpropertyPath.GetFullPath()withTaskEnvironment.GetAbsolutePath()for:TaskEnvironmentHelper.CreateForTest()when instantiating the taskTesting
TaskEnvironmentHelper.CreateForTest()Notes
The zip-slip exploit protection check uses
AbsolutePath.Valuefor string comparison to maintain the security validation while using thread-safe path resolution.Original prompt
💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.