Require an execute bit before selecting a Unix apphost - #10641
Draft
Jakub Jareš (nohwnd) wants to merge 3 commits into
Draft
Require an execute bit before selecting a Unix apphost#10641Jakub Jareš (nohwnd) wants to merge 3 commits into
Jakub Jareš (nohwnd) wants to merge 3 commits into
Conversation
The server-mode client picks a sibling apphost for a managed .dll by asking File.Exists, which says yes to a file that cannot be launched. A payload built on a Windows agent and run on a Linux machine arrives with its extensionless apphost stripped of POSIX permission bits, because a zip written on Windows records none. Process.Start then throws Permission denied and aborts the run instead of falling back. Gate the choice on IsUsableApphost, which additionally requires an execute bit on Unix. File.GetUnixFileMode is .NET 7+, so net462 and netstandard2.0 consumers keep the existence-only check, which stays correct because the apphost path probe is already OS-aware. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Restores Unix apphost validation in the source-only server-mode client after vstest adopted it.
Changes:
- Requires Unix sibling apphosts to have an execute bit.
- Falls back to
dotnet <dll>for unusable apphosts. - Adds cross-platform launch-selection tests.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
MtpServerProcess.cs |
Adds apphost usability validation and fallback logic. |
MtpServerProcessTests.cs |
Covers Windows and Unix apphost selection. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
…xecute-bit-guard-mtp-appho
The check was fenced on NETCOREAPP, which the pack transform rewrites to MTP_CLIENT_USE_MODERN_DOTNET. That symbol records which JSON slice a consumer compiles and is defined only for net8.0+, but NuGet serves the net5.0 slice to net5.0, net6.0 and net7.0 consumers alike. A Linux net7.0 consumer therefore stayed on the existence-only path, selected a non-executable apphost and failed with Permission denied, even though File.GetUnixFileMode is available to it. Fence on the target framework instead, which is what actually tracks the API and which the pack transform leaves alone. Add an anti-drift test asserting the call sits inside a NET7_0_OR_GREATER block in every packed slice. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
When launching a managed
.dll, the server-mode client prefers a sibling apphost and decides withFile.Exists, which says yes to a file that cannot be started. A test payload built on a Windows agent and executed on a Linux machine arrives with its extensionless apphost stripped of POSIX permission bits, because a zip written on Windows records none.Process.Startthen throwsPermission deniedand aborts the run rather than falling back.BuildLaunchnow gates the choice onIsUsableApphost, which additionally requires an execute bit on Unix, so an unusable candidate falls back todotnet <dll>.File.GetUnixFileModeis .NET 7+, so the check is fenced onNET7_0_OR_GREATERand net462, netstandard2.0, net5.0 and net6.0 consumers keep the existence-only check. That stays correct because the apphost path probe is already OS-aware and never offers a Windows.exeon Unix.The fence is the target framework rather than the package's modern-.NET symbol, which records which JSON slice a consumer compiles and is defined only for net8.0+. NuGet serves the net5.0 slice to net5.0, net6.0 and net7.0 alike, so fencing on it would leave a Linux net7.0 consumer on the existence-only path even though it has the API. An anti-drift test asserts the call sits inside a
NET7_0_OR_GREATERblock in every packed slice.This restores the second half of the fix in microsoft/vstest#16336, which was lost when vstest deleted its own client in favour of this package in microsoft/vstest#16300.
Reproduced end to end before fixing:
dotnet publish -r linux-x64on Windows emits an extensionless apphost beside the.dll; a Windows-written zip storesExternalAttributes = 0; extracting on Linux yields mode 0644;Process.StartthrowsPermission denied. The four alternatives were all checked and none holds — the check is absent frommain, the layout is reachable,Process.Startdoes not degrade gracefully, and the.dllbranch is live for consumers even though testfx's own callers pass an apphost.Verified:
build.cmd -packpasses; the new tests pass on Windows net8.0 and net462 and on Linux net8.0, and reverting either guard makes its test fail.Follow-up in microsoft/vstest, once a package with this ships: bump
MicrosoftTestingPlatformServerModeClientSourcesVersionineng/Versions.propsand add an acceptance test for the layout.🤖